Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix brush performance for coarse mags & avoid some unnecessary re-renders #6708

Merged
merged 8 commits into from
Dec 21, 2022
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fixed a bug where malformed json files could lead to uncaught exceptions.[#6691](https://github.com/scalableminds/webknossos/pull/6691)
- Fixed rare crash in publications page. [#6700](https://github.com/scalableminds/webknossos/pull/6700)
- Respect the config value mail.smtp.auth (used to be ignored, always using true) [#6692](https://github.com/scalableminds/webknossos/pull/6692)
- Fixed performance for brushing in coarse magnifications. [#6708](https://github.com/scalableminds/webknossos/pull/6708)

### Removed

Expand Down
6 changes: 4 additions & 2 deletions frontend/javascripts/libs/drawing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,11 @@ class Drawing {
paint: (arg0: number, arg1: number) => void,
) {
const squaredRadius = radius ** 2;
const scaledRadiusX = Math.ceil(radius * scaleX);
const scaledRadiusY = Math.ceil(radius * scaleY);

for (let posX = x - radius; posX < x + radius; posX++) {
for (let posY = y - radius; posY < y + radius; posY++) {
for (let posX = x - scaledRadiusX; posX < x + scaledRadiusX; posX++) {
daniel-wer marked this conversation as resolved.
Show resolved Hide resolved
for (let posY = y - scaledRadiusY; posY < y + scaledRadiusY; posY++) {
if (((posX - x) / scaleX) ** 2 + ((posY - y) / scaleY) ** 2 < squaredRadius) {
paint(posX, posY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,31 @@ import {
} from "oxalis/controller/combinations/volume_handlers";
import { setActiveConnectomeAgglomerateIdsAction } from "oxalis/model/actions/connectome_actions";
import { getTreeNameForAgglomerateSkeleton } from "oxalis/model/accessors/skeletontracing_accessor";

const AGGLOMERATE_STATES = {
NO_SEGMENTATION: {
value: false,
reason: "A segmentation layer needs to be visible to load an agglomerate skeleton.",
},
NO_MAPPING: {
value: false,
reason: messages["tracing.agglomerate_skeleton.no_mapping"],
},
NO_AGGLOMERATE_FILE: {
value: false,
reason: messages["tracing.agglomerate_skeleton.no_agglomerate_file"],
},
YES: {
value: true,
reason: "",
},
};

export function hasAgglomerateMapping(state: OxalisState) {
const segmentation = Model.getVisibleSegmentationLayer();

if (!segmentation) {
return {
value: false,
reason: "A segmentation layer needs to be visible to load an agglomerate skeleton.",
};
return AGGLOMERATE_STATES.NO_SEGMENTATION;
}

const { mappingName, mappingType, mappingStatus } = getMappingInfo(
Expand All @@ -34,48 +51,46 @@ export function hasAgglomerateMapping(state: OxalisState) {
);

if (mappingName == null || mappingStatus !== MappingStatusEnum.ENABLED) {
return {
value: false,
reason: messages["tracing.agglomerate_skeleton.no_mapping"],
};
return AGGLOMERATE_STATES.NO_MAPPING;
}

if (mappingType !== "HDF5") {
return {
value: false,
reason: messages["tracing.agglomerate_skeleton.no_agglomerate_file"],
};
return AGGLOMERATE_STATES.NO_AGGLOMERATE_FILE;
}

return {
return AGGLOMERATE_STATES.YES;
}

const CONNECTOME_STATES = {
NO_SEGMENTATION: {
value: false,
reason: "A segmentation layer needs to be visible to load the synapses of a segment.",
},
NO_CONNECTOME_FILE: {
value: false,
reason: "A connectome file needs to be available to load the synapses of a segment.",
},
YES: {
value: true,
reason: "",
};
}
},
};

export function hasConnectomeFile(state: OxalisState) {
const segmentationLayer = getVisibleOrLastSegmentationLayer(state);

if (segmentationLayer == null) {
return {
value: false,
reason: "A segmentation layer needs to be visible to load the synapses of a segment.",
};
return CONNECTOME_STATES.NO_SEGMENTATION;
}

const { currentConnectomeFile } =
state.localSegmentationData[segmentationLayer.name].connectomeData;

if (currentConnectomeFile == null) {
return {
value: false,
reason: "A connectome file needs to be available to load the synapses of a segment.",
};
return CONNECTOME_STATES.NO_CONNECTOME_FILE;
}

return {
value: true,
reason: "",
};
return CONNECTOME_STATES.YES;
}
export async function handleAgglomerateSkeletonAtClick(clickPosition: Point2) {
const state = Store.getState();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
Vector3,
} from "oxalis/constants";
import Model from "oxalis/model";
import { reuseInstanceOnEquality } from "oxalis/model/accessors/accessor_helpers";
import { getResolutionInfo } from "oxalis/model/accessors/dataset_accessor";
import { getFlooredPosition, getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor";
import {
Expand Down Expand Up @@ -52,7 +53,7 @@ import { createVolumeLayer, getBoundingBoxForViewport, labelWithVoxelBuffer2D }

export const MAXIMUM_INTERPOLATION_DEPTH = 100;

export function getInterpolationInfo(state: OxalisState, explanationPrefix: string) {
function _getInterpolationInfo(state: OxalisState, explanationPrefix: string) {
const isAllowed = state.tracing.restrictions.volumeInterpolationAllowed;
const onlyExtrude = state.userConfiguration.interpolationMode === InterpolationModeEnum.EXTRUDE;
const volumeTracing = getActiveSegmentationTracing(state);
Expand Down Expand Up @@ -146,6 +147,8 @@ export function getInterpolationInfo(state: OxalisState, explanationPrefix: stri
};
}

export const getInterpolationInfo = reuseInstanceOnEquality(_getInterpolationInfo);

const isEqual = cwise({
args: ["array", "scalar"],
body: function body(a: number, b: number) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Input, Modal } from "antd";
import * as React from "react";
type Props = {
addLayout: (arg0: string) => void;
visible: boolean;
open: boolean;
onCancel: () => void;
};
type State = {
Expand All @@ -25,7 +25,7 @@ class AddNewLayoutModal extends React.PureComponent<Props, State> {
return (
<Modal
title="Add a new layout"
visible={this.props.visible}
open={this.props.open}
onOk={this.onConfirm}
onCancel={this.props.onCancel}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,11 @@ with wk.webknossos_context(
`;

const alertTokenIsPrivate = () => {
Toast.warning(
"The clipboard contains private data. Do not share this information with anyone you do not trust!",
);
if (authToken) {
Toast.warning(
"The clipboard contains private data. Do not share this information with anyone you do not trust!",
);
}
};

const hasVolumes = hasVolumeTracings(tracing);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,11 @@ with wk.webknossos_context(token="${authToken || "<insert token here>"}"${contex
`;

const alertTokenIsPrivate = () => {
Toast.warning(
"The clipboard contains private data. Do not share this information with anyone you do not trust!",
);
if (authToken) {
Toast.warning(
"The clipboard contains private data. Do not share this information with anyone you do not trust!",
);
}
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class SaveButton extends React.PureComponent<Props, State> {
</Tooltip>
{showUnsavedWarning ? (
<Tooltip
visible
open
title={`There are unsaved changes which are older than ${Math.ceil(
UNSAVED_WARNING_THRESHOLD / 1000 / 60,
)} minutes. Please ensure that your Internet connection works and wait until this warning disappears.`}
Expand Down
23 changes: 13 additions & 10 deletions frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -391,24 +391,28 @@ function AdditionalSkeletonModesButtons() {
);
}

const mapId = (volumeTracing: VolumeTracing | null | undefined, id: number) => {
if (!volumeTracing) {
const mapId = (volumeTracingId: string | null | undefined, id: number) => {
if (!volumeTracingId) {
return null;
}
const { cube } = Model.getSegmentationTracingLayer(volumeTracing.tracingId);
const { cube } = Model.getSegmentationTracingLayer(volumeTracingId);
return cube.mapId(id);
};

function CreateCellButton() {
const volumeTracing = useSelector((state: OxalisState) => getActiveSegmentationTracing(state));
const unmappedActiveCellId = volumeTracing != null ? volumeTracing.activeCellId : 0;
const volumeTracingId = useSelector(
(state: OxalisState) => getActiveSegmentationTracing(state)?.tracingId,
);
const unmappedActiveCellId = useSelector(
(state: OxalisState) => getActiveSegmentationTracing(state)?.activeCellId || 0,
);
const { mappingStatus } = useSelector((state: OxalisState) =>
getMappingInfoForVolumeTracing(state, volumeTracing != null ? volumeTracing.tracingId : null),
getMappingInfoForVolumeTracing(state, volumeTracingId),
);
const isMappingEnabled = mappingStatus === MappingStatusEnum.ENABLED;

const activeCellId = isMappingEnabled
? mapId(volumeTracing, unmappedActiveCellId)
? mapId(volumeTracingId, unmappedActiveCellId)
: unmappedActiveCellId;

const activeCellColor = useSelector((state: OxalisState) => {
Expand Down Expand Up @@ -538,9 +542,8 @@ function ChangeBrushSizeButton() {
roundTo={0}
min={userSettings.brushSize.minimum}
max={maximumBrushSize}
// @ts-expect-error ts-migrate(2769) FIXME: No overload matches this call.
step={5}
spans={[0, 14, 10]}
precision={0}
spans={[0, 16, 8]}
value={brushSize}
onChange={handleUpdateBrushSize}
/>
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/view/action_bar_view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ class ActionBarView extends React.PureComponent<Props, State> {
</div>
<AddNewLayoutModal
addLayout={this.addNewLayout}
visible={this.state.isNewLayoutModalVisible}
open={this.state.isNewLayoutModalVisible}
onCancel={() =>
this.setState({
isNewLayoutModalVisible: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ type LogSliderSettingProps = {
min: number;
roundTo: number;
disabled?: boolean;
spans: Vector3;
precision?: number;
};

const LOG_SLIDER_MIN = -100;
Expand All @@ -104,6 +106,7 @@ export class LogSliderSetting extends React.PureComponent<LogSliderSettingProps>
static defaultProps = {
disabled: false,
roundTo: 3,
spans: [SETTING_LEFT_SPAN, SETTING_MIDDLE_SPAN, SETTING_VALUE_SPAN],
};

onChangeInput = (value: number | null) => {
Expand Down Expand Up @@ -153,10 +156,10 @@ export class LogSliderSetting extends React.PureComponent<LogSliderSettingProps>
const { label, roundTo, value, min, max, disabled } = this.props;
return (
<Row align="middle" gutter={ROW_GUTTER}>
<Col span={SETTING_LEFT_SPAN}>
<Col span={this.props.spans[0]}>
<label className="setting-label">{label}</label>
</Col>
<Col span={SETTING_MIDDLE_SPAN}>
<Col span={this.props.spans[1]}>
<Slider
min={LOG_SLIDER_MIN}
max={LOG_SLIDER_MAX}
Expand All @@ -166,7 +169,7 @@ export class LogSliderSetting extends React.PureComponent<LogSliderSettingProps>
disabled={disabled}
/>
</Col>
<Col span={SETTING_VALUE_SPAN}>
<Col span={this.props.spans[2]}>
<InputNumber
controls={false}
bordered={false}
Expand All @@ -176,7 +179,7 @@ export class LogSliderSetting extends React.PureComponent<LogSliderSettingProps>
width: "100%",
}}
step={value / 10}
precision={2}
precision={this.props.precision ?? 2}
value={roundTo != null ? Utils.roundTo(value, roundTo) : value}
onChange={this.onChangeInput}
disabled={disabled}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ export default class AdvancedSearchPopover<
trigger="click"
placement="left"
overlayClassName="search-input-popover"
visible={isVisible}
onVisibleChange={(newVisibility) =>
open={isVisible}
onOpenChange={(newVisibility) =>
newVisibility ? this.openSearchPopover() : this.closeSearchPopover()
}
content={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,8 @@ class SynapseTree extends React.Component<Props, State> {
overlay={() => this.createSegmentDropdownMenu(data.id)}
autoDestroy
placement="bottom"
visible={this.state.activeSegmentDropdownKey === key}
onVisibleChange={(isVisible) =>
this.handleSegmentDropdownMenuVisibility(key, isVisible)
}
open={this.state.activeSegmentDropdownKey === key}
onOpenChange={(isVisible) => this.handleSegmentDropdownMenuVisibility(key, isVisible)}
// @ts-expect-error ts-migrate(2322) FIXME: Type 'string[]' is not assignable to type '("conte... Remove this comment to see the full error message
trigger={contextMenuTrigger}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,8 @@ function _SegmentListItem({
// @ts-expect-error ts-migrate(2322) FIXME: Type '{ children: Element; overlay: () => Element;... Remove this comment to see the full error message
autoDestroy
placement="bottom"
visible={activeDropdownSegmentId === segment.id}
onVisibleChange={(isVisible) => handleSegmentDropdownMenuVisibility(segment.id, isVisible)}
open={activeDropdownSegmentId === segment.id}
onOpenChange={(isVisible) => handleSegmentDropdownMenuVisibility(segment.id, isVisible)}
trigger={["contextMenu"]}
>
<Tooltip title={getSegmentTooltip(segment)}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ class TreeHierarchyView extends React.PureComponent<Props, State> {
// @ts-expect-error ts-migrate(2322) FIXME: Type '{ children: Element; overlay: () => Element;... Remove this comment to see the full error message
autoDestroy
visible={this.state.activeGroupDropdownId === id} // explicit visibility handling is required here otherwise the color picker component for "Change Tree color" is rendered/positioned incorrectly
onVisibleChange={(isVisible) => this.handleGroupDropdownMenuVisibility(id, isVisible)}
onOpenChange={(isVisible) => this.handleGroupDropdownMenuVisibility(id, isVisible)}
trigger={["contextMenu"]}
>
<span>
Expand Down Expand Up @@ -589,8 +589,8 @@ class TreeHierarchyView extends React.PureComponent<Props, State> {
// @ts-expect-error ts-migrate(2322) FIXME: Type '{ children: Element; overlay: () => Element;... Remove this comment to see the full error message
autoDestroy
placement="bottom"
visible={this.state.activeTreeDropdownId === tree.treeId} // explicit visibility handling is required here otherwise the color picker component for "Change Tree color" is rendered/positioned incorrectly
onVisibleChange={(isVisible) =>
open={this.state.activeTreeDropdownId === tree.treeId} // explicit visibility handling is required here otherwise the color picker component for "Change Tree color" is rendered/positioned incorrectly
onOpenChange={(isVisible) =>
this.handleTreeDropdownMenuVisibility(tree.treeId, isVisible)
}
trigger={["contextMenu"]}
Expand Down
Loading