Skip to content

Commit

Permalink
Warn user when using precomputed meshes after brushing (#8218)
Browse files Browse the repository at this point in the history
* Add key in volumetracing to store if volume bucket data has changed

* show warning in context menu for loading precomputed meshes when bucket data has changed

* avoid unnecessary rerenders in context menu

* ensure SET_VOLUME_BUCKET_DATA_HAS_CHANGED is dispatched when buckets are changed

* add missing file

* fix linting

* rename load_mesh_menu_item_label file

* use action creator instead of hardcoding action

* fix that volumeBucketDataHashChanged was not handled when the server sent it

* add comments

* update snapshots

* add changelog entry

* remove unintentional changelog entry

* change action property from layerName to tracingId

---------

Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Philipp Otto <[email protected]>
Co-authored-by: Michael Büßemeyer <[email protected]>
  • Loading branch information
4 people authored Dec 3, 2024
1 parent fb1f275 commit c3729ea
Show file tree
Hide file tree
Showing 17 changed files with 140 additions and 35 deletions.
3 changes: 2 additions & 1 deletion app/models/annotation/AnnotationService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ class AnnotationService @Inject()(
mappingName = mappingName,
mags = magsRestricted.map(vec3IntToProto),
hasSegmentIndex = Some(fallbackLayer.isEmpty || fallbackLayerHasSegmentIndex),
additionalAxes = AdditionalAxis.toProto(additionalAxes)
additionalAxes = AdditionalAxis.toProto(additionalAxes),
volumeBucketDataHasChanged = Some(false)
)
}

Expand Down
10 changes: 10 additions & 0 deletions frontend/javascripts/oxalis/model/actions/volumetracing_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export type SetSegmentGroupsAction = ReturnType<typeof setSegmentGroupsAction>;
export type SetExpandedSegmentGroupsAction = ReturnType<typeof setExpandedSegmentGroupsAction>;
export type SetHasEditableMappingAction = ReturnType<typeof setHasEditableMappingAction>;
export type SetMappingIsLockedAction = ReturnType<typeof setMappingIsLockedAction>;
export type SetVolumeBucketDataHasChangedAction = ReturnType<
typeof setVolumeBucketDataHasChangedAction
>;

export type ComputeQuickSelectForRectAction = ReturnType<typeof computeQuickSelectForRectAction>;
export type ComputeQuickSelectForPointAction = ReturnType<typeof computeQuickSelectForPointAction>;
Expand Down Expand Up @@ -99,6 +102,7 @@ export type VolumeTracingAction =
| FineTuneQuickSelectAction
| CancelQuickSelectAction
| ConfirmQuickSelectAction
| SetVolumeBucketDataHasChangedAction
| BatchUpdateGroupsAndSegmentsAction;

export const VolumeTracingSaveRelevantActions = [
Expand Down Expand Up @@ -437,3 +441,9 @@ export const batchUpdateGroupsAndSegmentsAction = (actions: BatchableUpdateSegme
export const cancelQuickSelectAction = () => ({ type: "CANCEL_QUICK_SELECT" }) as const;

export const confirmQuickSelectAction = () => ({ type: "CONFIRM_QUICK_SELECT" }) as const;

export const setVolumeBucketDataHasChangedAction = (tracingId: string) =>
({
type: "SET_VOLUME_BUCKET_DATA_HAS_CHANGED",
tracingId,
}) as const;
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ export function serverVolumeToClientVolumeTracing(tracing: ServerVolumeTracing):
mappingName: tracing.mappingName,
hasEditableMapping: tracing.hasEditableMapping,
mappingIsLocked: tracing.mappingIsLocked,
volumeBucketDataHasChanged: tracing.volumeBucketDataHasChanged,
hasSegmentIndex: tracing.hasSegmentIndex || false,
additionalAxes: convertServerAdditionalAxesToFrontEnd(tracing.additionalAxes),
};
Expand Down Expand Up @@ -377,6 +378,12 @@ function VolumeTracingReducer(
return expandSegmentParents(state, action);
}

case "SET_VOLUME_BUCKET_DATA_HAS_CHANGED": {
return updateVolumeTracing(state, action.tracingId, {
volumeBucketDataHasChanged: true,
});
}

default: // pass
}

Expand Down
8 changes: 4 additions & 4 deletions frontend/javascripts/oxalis/model/sagas/quick_select_saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import performQuickSelectML from "./quick_select_ml_saga";
import getSceneController from "oxalis/controller/scene_controller_provider";
import { getActiveSegmentationTracing } from "../accessors/volumetracing_accessor";
import type { VolumeTracing } from "oxalis/store";
import { ensureMaybeActiveMappingIsLocked } from "./saga_helpers";
import { requestBucketModificationInVolumeTracing } from "./saga_helpers";

function* shouldUseHeuristic() {
const useHeuristic = yield* select((state) => state.userConfiguration.quickSelect.useHeuristic);
Expand All @@ -32,11 +32,11 @@ export default function* listenToQuickSelect(): Saga<void> {
);
if (volumeTracing) {
// As changes to the volume layer will be applied, the potentially existing mapping should be locked to ensure a consistent state.
const { isMappingLockedIfNeeded } = yield* call(
ensureMaybeActiveMappingIsLocked,
const isModificationAllowed = yield* call(
requestBucketModificationInVolumeTracing,
volumeTracing,
);
if (!isMappingLockedIfNeeded) {
if (!isModificationAllowed) {
return;
}
}
Expand Down
34 changes: 33 additions & 1 deletion frontend/javascripts/oxalis/model/sagas/saga_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import { call, put, takeEvery } from "typed-redux-saga";
import Toast from "libs/toast";
import { Store } from "oxalis/singletons";
import type { ActionPattern } from "@redux-saga/types";
import { setMappingIsLockedAction } from "../actions/volumetracing_actions";
import {
setMappingIsLockedAction,
setVolumeBucketDataHasChangedAction,
} from "../actions/volumetracing_actions";
import { MappingStatusEnum } from "oxalis/constants";

export function* takeEveryUnlessBusy<P extends ActionPattern>(
Expand Down Expand Up @@ -113,4 +116,33 @@ export function* ensureMaybeActiveMappingIsLocked(
}
}

export function* requestBucketModificationInVolumeTracing(
volumeTracing: VolumeTracing,
): Saga<boolean> {
/*
* Should be called when the modification of bucket data is about to happen. If
* the saga returns false, the modification should be cancelled (this happens if
* the user is not okay with locking the mapping).
*
* In detail: When the bucket data of a volume tracing is supposed to be mutated, we need to do
* two things:
* 1) ensure that the current mapping (or no mapping) is locked so that the mapping is not
* changed later (this would lead to inconsistent data). If the mapping state is not yet
* locked, the user is asked whether it is okay to lock it.
* If the user confirms this, the mapping is locked and we can continue with (2). If the
* user denies the locking request, the original bucket mutation will NOT be executed.
* 2) volumeTracing.volumeBucketDataHasChanged will bet set to true if the user didn't
* deny the request for locking the mapping.
*/

const { isMappingLockedIfNeeded } = yield* call(ensureMaybeActiveMappingIsLocked, volumeTracing);
if (!isMappingLockedIfNeeded) {
return false;
}

// Mark that bucket data has changed
yield* put(setVolumeBucketDataHasChangedAction(volumeTracing.tracingId));
return true;
}

export default {};
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { Model, api } from "oxalis/singletons";
import type { OxalisState } from "oxalis/store";
import { call, put } from "typed-redux-saga";
import { createVolumeLayer, getBoundingBoxForViewport, labelWithVoxelBuffer2D } from "./helpers";
import { ensureMaybeActiveMappingIsLocked } from "../saga_helpers";
import { requestBucketModificationInVolumeTracing } from "../saga_helpers";

/*
* This saga is capable of doing segment interpolation between two slices.
Expand Down Expand Up @@ -436,8 +436,11 @@ export default function* maybeInterpolateSegmentationLayer(): Saga<void> {
return;
}
// As the interpolation will be applied, the potentially existing mapping should be locked to ensure a consistent state.
const { isMappingLockedIfNeeded } = yield* call(ensureMaybeActiveMappingIsLocked, volumeTracing);
if (!isMappingLockedIfNeeded) {
const isModificationAllowed = yield* call(
requestBucketModificationInVolumeTracing,
volumeTracing,
);
if (!isModificationAllowed) {
return;
}

Expand Down
15 changes: 8 additions & 7 deletions frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ import { select, take } from "oxalis/model/sagas/effect-generators";
import listenToMinCut from "oxalis/model/sagas/min_cut_saga";
import listenToQuickSelect from "oxalis/model/sagas/quick_select_saga";
import {
ensureMaybeActiveMappingIsLocked,
requestBucketModificationInVolumeTracing,
takeEveryUnlessBusy,
} from "oxalis/model/sagas/saga_helpers";
import {
Expand Down Expand Up @@ -223,11 +223,11 @@ export function* editVolumeLayerAsync(): Saga<any> {

const activeCellId = yield* select((state) => enforceActiveVolumeTracing(state).activeCellId);
// As changes to the volume layer will be applied, the potentially existing mapping should be locked to ensure a consistent state.
const { isMappingLockedIfNeeded } = yield* call(
ensureMaybeActiveMappingIsLocked,
const isModificationAllowed = yield* call(
requestBucketModificationInVolumeTracing,
volumeTracing,
);
if (!isMappingLockedIfNeeded) {
if (!isModificationAllowed) {
continue;
}

Expand Down Expand Up @@ -453,11 +453,11 @@ export function* floodFill(): Saga<void> {
}
// As the flood fill will be applied to the volume layer,
// the potentially existing mapping should be locked to ensure a consistent state.
const { isMappingLockedIfNeeded } = yield* call(
ensureMaybeActiveMappingIsLocked,
const isModificationAllowed = yield* call(
requestBucketModificationInVolumeTracing,
volumeTracing,
);
if (!isMappingLockedIfNeeded) {
if (!isModificationAllowed) {
continue;
}
yield* put(setBusyBlockingInfoAction(true, "Floodfill is being computed."));
Expand Down Expand Up @@ -603,6 +603,7 @@ export function* finishLayer(

yield* put(registerLabelPointAction(layer.getUnzoomedCentroid()));
}

export function* ensureToolIsAllowedInMag(): Saga<any> {
yield* take("INITIALIZE_VOLUMETRACING");

Expand Down
1 change: 1 addition & 0 deletions frontend/javascripts/oxalis/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ export type VolumeTracing = TracingBase & {
readonly hasEditableMapping?: boolean;
readonly mappingIsLocked?: boolean;
readonly hasSegmentIndex: boolean;
readonly volumeBucketDataHasChanged?: boolean;
};
export type ReadOnlyTracing = TracingBase & {
readonly type: "readonly";
Expand Down
13 changes: 9 additions & 4 deletions frontend/javascripts/oxalis/view/context_menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ import {
import { hideContextMenuAction, setActiveUserBoundingBoxId } from "oxalis/model/actions/ui_actions";
import { getDisabledInfoForTools } from "oxalis/model/accessors/tool_accessor";
import FastTooltip from "components/fast_tooltip";
import { LoadMeshMenuItemLabel } from "./right-border-tabs/segments_tab/load_mesh_menu_item_label";

type ContextMenuContextValue = React.MutableRefObject<HTMLElement | null> | null;
export const ContextMenuContext = createContext<ContextMenuContextValue>(null);
Expand Down Expand Up @@ -1179,7 +1180,9 @@ function getNoNodeContextMenuOptions(props: NoNodeContextMenuProps): ItemType[]
segmentationLayerName,
mappingInfo,
),
label: "Load Mesh (precomputed)",
label: (
<LoadMeshMenuItemLabel currentMeshFile={currentMeshFile} volumeTracing={volumeTracing} />
),
};
const computeMeshAdHocItem = {
key: "compute-mesh-adhc",
Expand Down Expand Up @@ -1434,12 +1437,13 @@ function ContextMenuInner(propsWithInputRef: Props) {
maybeClickedMeshId != null ? maybeClickedMeshId : segmentIdAtPosition;
const wasSegmentOrMeshClicked = clickedSegmentOrMeshId > 0;

const { dataset, tracing, flycam } = useSelector((state: OxalisState) => state);
const dataset = useSelector((state: OxalisState) => state.dataset);
useEffect(() => {
Store.dispatch(ensureSegmentIndexIsLoadedAction(visibleSegmentationLayer?.name));
}, [visibleSegmentationLayer]);
const isSegmentIndexAvailable = useSelector((state: OxalisState) =>
getMaybeSegmentIndexAvailability(state.dataset, visibleSegmentationLayer?.name),
const isSegmentIndexAvailable = getMaybeSegmentIndexAvailability(
dataset,
visibleSegmentationLayer?.name,
);
const mappingName: string | null | undefined = useSelector((state: OxalisState) => {
if (volumeTracing?.mappingName != null) return volumeTracing?.mappingName;
Expand All @@ -1453,6 +1457,7 @@ function ContextMenuInner(propsWithInputRef: Props) {
const isLoadingVolumeAndBB = [isLoadingMessage, isLoadingMessage];
const [segmentVolumeLabel, boundingBoxInfoLabel] = useFetch(
async () => {
const { tracing, flycam } = Store.getState();
// The value that is returned if the context menu is closed is shown if it's still loading
if (contextMenuPosition == null || !wasSegmentOrMeshClicked) return isLoadingVolumeAndBB;
if (visibleSegmentationLayer == null || !isSegmentIndexAvailable) return [];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { WarningOutlined } from "@ant-design/icons";
import FastTooltip from "components/fast_tooltip";
import type { VolumeTracing } from "oxalis/store";
import type { APIMeshFile } from "types/api_flow_types";

type Props = {
currentMeshFile: APIMeshFile | null | undefined;
volumeTracing: VolumeTracing | null | undefined;
};

export function LoadMeshMenuItemLabel({ currentMeshFile, volumeTracing }: Props) {
const showWarning =
volumeTracing?.volumeBucketDataHasChanged ??
// For older annotations, volumeBucketDataHasChanged can be undefined.
// In that case, we still want to show a warning if no proofreading was
// done, but the mapping is still locked (i.e., the user brushed).
(!volumeTracing?.hasEditableMapping && volumeTracing?.mappingIsLocked);

return (
<span style={{ display: "flex", alignItems: "center", gap: "4px" }}>
<FastTooltip
title={
currentMeshFile != null
? `Load mesh for centered segment from file ${currentMeshFile.meshFileName}`
: "There is no mesh file."
}
>
<span>Load Mesh (precomputed)</span>
</FastTooltip>
{showWarning && (
<FastTooltip title="Warning: The segmentation data has changed since the mesh file was created. The mesh may not match the current data.">
<WarningOutlined style={{ color: "var(--ant-color-warning)" }} />
</FastTooltip>
)}
</span>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { V4 } from "libs/mjs";
import { ChangeColorMenuItemContent } from "components/color_picker";
import type { MenuItemType } from "antd/es/menu/interface";
import { withMappingActivationConfirmation } from "./segments_view_helper";
import { LoadMeshMenuItemLabel } from "./load_mesh_menu_item_label";
import type { AdditionalCoordinate } from "types/api_flow_types";
import {
getAdditionalCoordinatesAsString,
Expand Down Expand Up @@ -80,8 +81,10 @@ const getLoadPrecomputedMeshMenuItem = (
hideContextMenu: (_ignore?: any) => void,
layerName: string | null | undefined,
mappingInfo: ActiveMappingInfo,
activeVolumeTracing: VolumeTracing | null | undefined,
) => {
const mappingName = currentMeshFile != null ? currentMeshFile.mappingName : undefined;

return {
key: "loadPrecomputedMesh",
disabled: !currentMeshFile,
Expand Down Expand Up @@ -114,16 +117,10 @@ const getLoadPrecomputedMeshMenuItem = (
mappingInfo,
),
label: (
<FastTooltip
key="tooltip"
title={
currentMeshFile != null
? `Load mesh for centered segment from file ${currentMeshFile.meshFileName}`
: "There is no mesh file."
}
>
Load Mesh (precomputed)
</FastTooltip>
<LoadMeshMenuItemLabel
currentMeshFile={currentMeshFile}
volumeTracing={activeVolumeTracing}
/>
),
};
};
Expand Down Expand Up @@ -429,6 +426,7 @@ function _SegmentListItem({
hideContextMenu,
visibleSegmentationLayer != null ? visibleSegmentationLayer.name : null,
mappingInfo,
activeVolumeTracing,
),
getComputeMeshAdHocMenuItem(
segment,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ mockRequire("libs/toast", {
const { setupSavingForTracingType } = require("oxalis/model/sagas/save_saga");

const { editVolumeLayerAsync, finishLayer } = require("oxalis/model/sagas/volumetracing_saga");
const { ensureMaybeActiveMappingIsLocked } = require("oxalis/model/sagas/saga_helpers");
const {
requestBucketModificationInVolumeTracing,
ensureMaybeActiveMappingIsLocked,
} = require("oxalis/model/sagas/saga_helpers");

const VolumeLayer = require("oxalis/model/volumetracing/volumelayer").default;

Expand Down Expand Up @@ -437,11 +440,11 @@ test("VolumeTracingSaga should lock an active mapping upon first volume annotati
mag: [1, 1, 1],
zoomStep: 0,
});
// Test whether nested saga ensureMaybeActiveMappingIsLocked is called.
// Test whether nested saga requestBucketModificationInVolumeTracing is called.
expectValueDeepEqual(
t,
saga.next(ACTIVE_CELL_ID),
call(ensureMaybeActiveMappingIsLocked, volumeTracing),
call(requestBucketModificationInVolumeTracing, volumeTracing),
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,7 @@ Generated by [AVA](https://avajs.dev).
typ: 'Volume',
userBoundingBoxes: [],
version: 0,
volumeBucketDataHasChanged: false,
zoomLevel: 1,
}

Expand Down Expand Up @@ -1408,6 +1409,7 @@ Generated by [AVA](https://avajs.dev).
typ: 'Volume',
userBoundingBoxes: [],
version: 0,
volumeBucketDataHasChanged: false,
zoomLevel: 1,
},
}
Expand Down
Binary file not shown.
4 changes: 4 additions & 0 deletions frontend/javascripts/types/api_flow_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,10 @@ export type ServerVolumeTracing = ServerTracingBase & {
hasEditableMapping?: boolean;
mappingIsLocked?: boolean;
hasSegmentIndex?: boolean;
// volumeBucketDataHasChanged is automatically set to true by the back-end
// once a bucket was mutated. There is no need to send an explicit UpdateAction
// for that.
volumeBucketDataHasChanged?: boolean;
};
export type ServerTracing = ServerSkeletonTracing | ServerVolumeTracing;
export type ServerEditableMapping = {
Expand Down
1 change: 1 addition & 0 deletions webknossos-datastore/proto/VolumeTracing.proto
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ message VolumeTracing {
repeated AdditionalCoordinateProto editPositionAdditionalCoordinates = 21;
repeated AdditionalAxisProto additionalAxes = 22; // Additional axes for which this tracing is defined
optional bool mappingIsLocked = 23; // user may not select another mapping (e.g. because they have already mutated buckets)
optional bool volumeBucketDataHasChanged = 24; // The volume bucket data has been edited at least once
}

message SegmentGroup {
Expand Down
Loading

0 comments on commit c3729ea

Please sign in to comment.