diff --git a/.gitignore b/.gitignore index 1cbcbc77bd1..3a03c1c3996 100755 --- a/.gitignore +++ b/.gitignore @@ -76,3 +76,4 @@ coverage-ts .bsp temp-webknossos-schema** conf/application.conf-e +.env diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index a48f47438a2..d93a84d6dee 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -23,6 +23,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released ### Changed - Datasets stored in WKW format are no longer loaded with memory mapping, reducing memory demands. [#7528](https://github.com/scalableminds/webknossos/pull/7528) - Content Security Policy (CSP) settings are now relaxed by default. To keep stricter CSP rules, add them to your specific `application.conf`. [#7589](https://github.com/scalableminds/webknossos/pull/7589) +- The state of whether a mapping is active and what exact mapping is now locked to the annotation upon the first volume annotation action to ensure future consistent results. Moreover, while a JSON mapping is active, no volume annotation can be done. [#7549](https://github.com/scalableminds/webknossos/pull/7549) - WEBKNOSSOS now uses Java 21. [#7599](https://github.com/scalableminds/webknossos/pull/7599) - Email verification is disabled by default. To enable it, set `webKnossos.user.emailVerification.activated` to `true` in your `application.conf`. [#7620](https://github.com/scalableminds/webknossos/pull/7620) [#7621](https://github.com/scalableminds/webknossos/pull/7621) - Added more documentation for N5 and Neuroglancer precomputed web upload. [#7622](https://github.com/scalableminds/webknossos/pull/7622) diff --git a/frontend/javascripts/messages.tsx b/frontend/javascripts/messages.tsx index 9ba9b0fa36a..4330c887180 100644 --- a/frontend/javascripts/messages.tsx +++ b/frontend/javascripts/messages.tsx @@ -254,6 +254,10 @@ instead. Only enable this option if you understand its effect. All layers will n 'All trees are currently hidden. You can disable this by toggling the "Skeleton" layer in the layer settings in the left sidebar.', "tracing.invalid_json_url_hash": "Cannot parse JSON URL hash. More information was printed to the browser's console.", + "tracing.locked_mapping_info": + "The active volume annotation layer has an active mapping. By mutating the layer, the mapping will be permanently locked and can no longer be changed or disabled. This can only be undone by restoring an older version of this annotation. Are you sure you want to continue?", + "tracing.locked_mapping_confirmed": (mappingName: string) => + `The mapping ${mappingName} is now locked for this annotation and can no longer be changed or disabled.`, "layouting.missing_custom_layout_info": "The annotation views are separated into four classes. Each of them has their own layouts. If you can't find your layout please open the annotation in the correct view mode or just add it here manually.", "datastore.unknown_type": "Unknown datastore type:", diff --git a/frontend/javascripts/oxalis/model/accessors/tool_accessor.ts b/frontend/javascripts/oxalis/model/accessors/tool_accessor.ts index cf315fe4561..7618f841da8 100644 --- a/frontend/javascripts/oxalis/model/accessors/tool_accessor.ts +++ b/frontend/javascripts/oxalis/model/accessors/tool_accessor.ts @@ -31,6 +31,7 @@ const getExplanationForDisabledVolume = ( isZoomInvalidForTracing: boolean, isEditableMappingActive: boolean, isSegmentationTracingTransformed: boolean, + isJSONMappingActive: boolean, ) => { if (!isSegmentationTracingVisible) { return "Volume annotation is disabled since no segmentation tracing layer is enabled. Enable it in the left settings sidebar."; @@ -55,6 +56,9 @@ const getExplanationForDisabledVolume = ( if (isSegmentationTracingTransformed) { return "Volume annotation is disabled because the visible segmentation layer is transformed. Use the left sidebar to render the segmentation layer without any transformations."; } + if (isJSONMappingActive) { + return "Volume annotation is disabled because a JSON mapping is currently active for the the visible segmentation layer. Disable the JSON mapping to enable volume annotation."; + } return "Volume annotation is currently disabled."; }; @@ -112,13 +116,61 @@ function _getDisabledInfoWhenVolumeIsDisabled( }; } +function _getDisabledInfoForProofreadTool( + hasSkeleton: boolean, + agglomerateState: AgglomerateState, + isProofReadingToolAllowed: boolean, + isUneditableMappingLocked: boolean, + activeOrganization: APIOrganization | null, + activeUser: APIUser | null | undefined, +) { + // The explanations are prioritized according to effort the user has to put into + // activating proofreading. + // 1) If a non editable mapping is locked to the annotation, proofreading actions are + // not allowed for this annotation. + // 2) If no agglomerate mapping is available (or activated), the user should know + // about this requirement and be able to set it up (this can be the most difficult + // step). + // 3) If a mapping is available, the pricing plan is potentially warned upon. + // 4) In the end, a potentially missing skeleton is warned upon (quite rare, because + // most annotations have a skeleton). + const isDisabled = + !hasSkeleton || + !agglomerateState.value || + !isProofReadingToolAllowed || + isUneditableMappingLocked; + let explanation = "Proofreading actions are not supported after modifying the segmentation."; + if (!isUneditableMappingLocked) { + if (!agglomerateState.value) { + explanation = agglomerateState.reason; + } else if (!isProofReadingToolAllowed) { + explanation = getFeatureNotAvailableInPlanMessage( + PricingPlanEnum.Power, + activeOrganization, + activeUser, + ); + } else { + explanation = disabledSkeletonExplanation; + } + } else { + explanation = + "A mapping that does not support proofreading actions is locked to this annotation. Most likely, the annotation layer was modified earlier (e.g. by brushing)."; + } + return { + isDisabled, + explanation, + }; +} + const getDisabledInfoWhenVolumeIsDisabled = memoizeOne(_getDisabledInfoWhenVolumeIsDisabled); +const getDisabledInfoForProofreadTool = memoizeOne(_getDisabledInfoForProofreadTool); function _getDisabledInfoFromArgs( hasSkeleton: boolean, isZoomStepTooHighForBrushing: boolean, isZoomStepTooHighForTracing: boolean, isZoomStepTooHighForFilling: boolean, + isUneditableMappingLocked: boolean, agglomerateState: AgglomerateState, genericDisabledExplanation: string, activeOrganization: APIOrganization | null, @@ -170,27 +222,14 @@ function _getDisabledInfoFromArgs( isDisabled: isZoomStepTooHighForFilling, explanation: zoomInToUseToolMessage, }, - [AnnotationToolEnum.PROOFREAD]: { - isDisabled: !hasSkeleton || !agglomerateState.value || !isProofReadingToolAllowed, - explanation: - // The explanations are prioritized according to effort the user has to put into - // activating proofreading. - // 1) If no agglomerate mapping is available (or activated), the user should know - // about this requirement and be able to set it up (this can be the most difficult - // step). - // 2) If a mapping is available, the pricing plan is potentially warned upon. - // 3) In the end, a potentially missing skeleton is warned upon (quite rare, because - // most annotations have a skeleton). - agglomerateState.value - ? isProofReadingToolAllowed - ? disabledSkeletonExplanation - : getFeatureNotAvailableInPlanMessage( - PricingPlanEnum.Power, - activeOrganization, - activeUser, - ) - : agglomerateState.reason, - }, + [AnnotationToolEnum.PROOFREAD]: getDisabledInfoForProofreadTool( + hasSkeleton, + agglomerateState, + isProofReadingToolAllowed, + isUneditableMappingLocked, + activeOrganization, + activeUser, + ), [AnnotationToolEnum.LINE_MEASUREMENT]: { isDisabled: false, explanation: genericDisabledExplanation, @@ -211,6 +250,7 @@ export function getDisabledInfoForTools(state: OxalisState): Record< } > { const isInMergerMode = state.temporaryConfiguration.isMergerModeEnabled; + const { activeMappingByLayer } = state.temporaryConfiguration; const isZoomInvalidForTracing = isMagRestrictionViolated(state); const hasVolume = state.tracing.volumes.length > 0; const hasSkeleton = state.tracing.skeleton != null; @@ -234,6 +274,10 @@ export function getDisabledInfoForTools(state: OxalisState): Record< visibleSegmentationLayer.name === segmentationTracingLayer.tracingId; const isEditableMappingActive = segmentationTracingLayer != null && !!segmentationTracingLayer.mappingIsEditable; + const isJSONMappingActive = + segmentationTracingLayer != null && + activeMappingByLayer[segmentationTracingLayer.tracingId]?.mappingType === "JSON" && + activeMappingByLayer[segmentationTracingLayer.tracingId]?.mappingStatus === "ENABLED"; const genericDisabledExplanation = getExplanationForDisabledVolume( isSegmentationTracingVisible, isInMergerMode, @@ -241,7 +285,11 @@ export function getDisabledInfoForTools(state: OxalisState): Record< isZoomInvalidForTracing, isEditableMappingActive, isSegmentationTracingTransformed, + isJSONMappingActive, ); + const isUneditableMappingLocked = + (segmentationTracingLayer?.mappingIsLocked && !segmentationTracingLayer?.mappingIsEditable) ?? + false; const isVolumeDisabled = !hasVolume || @@ -250,6 +298,7 @@ export function getDisabledInfoForTools(state: OxalisState): Record< // this condition doesn't need to be checked here !isSegmentationTracingVisibleForMag || isInMergerMode || + isJSONMappingActive || isSegmentationTracingTransformed; if (isVolumeDisabled || isEditableMappingActive) { @@ -268,6 +317,7 @@ export function getDisabledInfoForTools(state: OxalisState): Record< isVolumeAnnotationDisallowedForZoom(AnnotationToolEnum.BRUSH, state), isVolumeAnnotationDisallowedForZoom(AnnotationToolEnum.TRACE, state), isVolumeAnnotationDisallowedForZoom(AnnotationToolEnum.FILL_CELL, state), + isUneditableMappingLocked, agglomerateState, genericDisabledExplanation, state.activeOrganization, diff --git a/frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts b/frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts index 4c83bab628a..19ec2e896fa 100644 --- a/frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts +++ b/frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts @@ -527,10 +527,10 @@ export function getMappingInfoForVolumeTracing( return getMappingInfo(state.temporaryConfiguration.activeMappingByLayer, tracingId); } -export function hasEditableMapping( +function getVolumeTracingForLayerName( state: OxalisState, layerName?: string | null | undefined, -): boolean { +): VolumeTracing | null | undefined { if (layerName != null) { // This needs to be checked before calling getRequestedOrDefaultSegmentationTracingLayer, // as the function will throw an error if layerName is given but a corresponding tracing layer @@ -538,24 +538,45 @@ export function hasEditableMapping( const layer = getSegmentationLayerByName(state.dataset, layerName); const tracing = getTracingForSegmentationLayer(state, layer); - if (tracing == null) return false; + if (tracing == null) return null; } const volumeTracing = getRequestedOrDefaultSegmentationTracingLayer(state, layerName); + return volumeTracing; +} + +export function hasEditableMapping( + state: OxalisState, + layerName?: string | null | undefined, +): boolean { + const volumeTracing = getVolumeTracingForLayerName(state, layerName); + if (volumeTracing == null) return false; return !!volumeTracing.mappingIsEditable; } +export function isMappingLocked( + state: OxalisState, + layerName?: string | null | undefined, +): boolean { + const volumeTracing = getVolumeTracingForLayerName(state, layerName); + + if (volumeTracing == null) return false; + + return !!volumeTracing.mappingIsLocked; +} + export function isMappingActivationAllowed( state: OxalisState, mappingName: string | null | undefined, layerName?: string | null | undefined, ): boolean { const isEditableMappingActive = hasEditableMapping(state, layerName); + const isActiveMappingLocked = isMappingLocked(state, layerName); - if (!isEditableMappingActive) return true; + if (!isEditableMappingActive && !isActiveMappingLocked) return true; const volumeTracing = getRequestedOrDefaultSegmentationTracingLayer(state, layerName); diff --git a/frontend/javascripts/oxalis/model/actions/volumetracing_actions.ts b/frontend/javascripts/oxalis/model/actions/volumetracing_actions.ts index 3b805c9f69c..574e3f32958 100644 --- a/frontend/javascripts/oxalis/model/actions/volumetracing_actions.ts +++ b/frontend/javascripts/oxalis/model/actions/volumetracing_actions.ts @@ -43,6 +43,7 @@ export type RemoveSegmentAction = ReturnType; export type DeleteSegmentDataAction = ReturnType; export type SetSegmentGroupsAction = ReturnType; export type SetMappingIsEditableAction = ReturnType; +export type SetMappingIsLockedAction = ReturnType; export type ComputeQuickSelectForRectAction = ReturnType; export type MaybePrefetchEmbeddingAction = ReturnType; @@ -89,6 +90,7 @@ export type VolumeTracingAction = | SetLargestSegmentIdAction | SetSelectedSegmentsOrGroupAction | SetMappingIsEditableAction + | SetMappingIsLockedAction | InitializeEditableMappingAction | ComputeQuickSelectForRectAction | MaybePrefetchEmbeddingAction @@ -110,6 +112,8 @@ export const VolumeTracingSaveRelevantActions = [ "SET_MAPPING", "SET_MAPPING_ENABLED", "BATCH_UPDATE_GROUPS_AND_SEGMENTS", + "SET_MAPPING_IS_EDITABLE", + "SET_MAPPING_IS_LOCKED", ]; export const VolumeTracingUndoRelevantActions = ["START_EDITING", "COPY_SEGMENTATION_LAYER"]; @@ -356,6 +360,11 @@ export const setMappingIsEditableAction = () => type: "SET_MAPPING_IS_EDITABLE", }) as const; +export const setMappingIsLockedAction = () => + ({ + type: "SET_MAPPING_IS_LOCKED", + }) as const; + export const computeQuickSelectForRectAction = ( startPosition: Vector3, endPosition: Vector3, diff --git a/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer.ts b/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer.ts index a754d771e08..e38c7e61022 100644 --- a/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer.ts +++ b/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer.ts @@ -229,6 +229,7 @@ export function serverVolumeToClientVolumeTracing(tracing: ServerVolumeTracing): userBoundingBoxes, mappingName: tracing.mappingName, mappingIsEditable: tracing.mappingIsEditable, + mappingIsLocked: tracing.mappingIsLocked, hasSegmentIndex: tracing.hasSegmentIndex || false, additionalAxes: convertServerAdditionalAxesToFrontEnd(tracing.additionalAxes), }; @@ -391,18 +392,27 @@ function VolumeTracingReducer( case "SET_MAPPING_NAME": { // Editable mappings cannot be disabled or switched for now - if (volumeTracing.mappingIsEditable) return state; + if (volumeTracing.mappingIsEditable || volumeTracing.mappingIsLocked) return state; const { mappingName, mappingType } = action; return setMappingNameReducer(state, volumeTracing, mappingName, mappingType); } case "SET_MAPPING_IS_EDITABLE": { - // Editable mappings cannot be disabled or switched for now - if (volumeTracing.mappingIsEditable) return state; + // Editable mappings cannot be disabled or switched for now. + if (volumeTracing.mappingIsEditable || volumeTracing.mappingIsLocked) return state; + // An editable mapping is always locked. return updateVolumeTracing(state, volumeTracing.tracingId, { mappingIsEditable: true, + mappingIsLocked: true, + }); + } + case "SET_MAPPING_IS_LOCKED": { + if (volumeTracing.mappingIsLocked) return state; + + return updateVolumeTracing(state, volumeTracing.tracingId, { + mappingIsLocked: true, }); } diff --git a/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer_helpers.ts b/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer_helpers.ts index 846e8636107..c93e555225d 100644 --- a/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer_helpers.ts +++ b/frontend/javascripts/oxalis/model/reducers/volumetracing_reducer_helpers.ts @@ -153,8 +153,10 @@ export function setMappingNameReducer( mappingType: MappingType, isMappingEnabled: boolean = true, ) { - // Editable mappings cannot be disabled or switched for now - if (volumeTracing.mappingIsEditable) return state; + // Editable mappings or locked mappings cannot be disabled or switched for now + if (volumeTracing.mappingIsEditable || volumeTracing.mappingIsLocked) { + return state; + } // Only HDF5 mappings are persisted in volume annotations for now if (mappingType !== "HDF5" || !isMappingEnabled) { mappingName = null; diff --git a/frontend/javascripts/oxalis/model/sagas/quick_select_saga.ts b/frontend/javascripts/oxalis/model/sagas/quick_select_saga.ts index 6a6b56866ac..5fa9653da5f 100644 --- a/frontend/javascripts/oxalis/model/sagas/quick_select_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/quick_select_saga.ts @@ -18,6 +18,9 @@ import performQuickSelectML, { } from "./quick_select_ml_saga"; import { AnnotationToolEnum } from "oxalis/constants"; import getSceneController from "oxalis/controller/scene_controller_provider"; +import { getActiveSegmentationTracing } from "../accessors/volumetracing_accessor"; +import { VolumeTracing } from "oxalis/store"; +import { ensureMaybeActiveMappingIsLocked } from "./saga_helpers"; function* shouldUseHeuristic() { const useHeuristic = yield* select((state) => state.userConfiguration.quickSelect.useHeuristic); @@ -29,6 +32,19 @@ export default function* listenToQuickSelect(): Saga { "COMPUTE_QUICK_SELECT_FOR_RECT", function* guard(action: ComputeQuickSelectForRectAction) { try { + const volumeTracing: VolumeTracing | null | undefined = yield* select( + getActiveSegmentationTracing, + ); + 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, + volumeTracing, + ); + if (!isMappingLockedIfNeeded) { + return; + } + } yield* put(setBusyBlockingInfoAction(true, "Selecting segment")); yield* put(setQuickSelectStateAction("active")); diff --git a/frontend/javascripts/oxalis/model/sagas/saga_helpers.ts b/frontend/javascripts/oxalis/model/sagas/saga_helpers.ts index eb82b00a2a5..bc5572b4d74 100644 --- a/frontend/javascripts/oxalis/model/sagas/saga_helpers.ts +++ b/frontend/javascripts/oxalis/model/sagas/saga_helpers.ts @@ -1,9 +1,16 @@ +import { Modal } from "antd"; +import messages from "messages"; import type { Action } from "oxalis/model/actions/actions"; +import { setBusyBlockingInfoAction } from "oxalis/model/actions/ui_actions"; import type { Saga } from "oxalis/model/sagas/effect-generators"; -import { call, put, takeEvery } from "typed-redux-saga"; import { select } from "oxalis/model/sagas/effect-generators"; -import { setBusyBlockingInfoAction } from "oxalis/model/actions/ui_actions"; +import { ActiveMappingInfo, VolumeTracing } from "oxalis/store"; +import { call, put, takeEvery } from "typed-redux-saga"; +import Toast from "libs/toast"; +import { Store } from "oxalis/singletons"; import { ActionPattern } from "@redux-saga/types"; +import { setMappingIsLockedAction } from "../actions/volumetracing_actions"; +import { MappingStatusEnum } from "oxalis/constants"; export function* takeEveryUnlessBusy

( actionDescriptor: P, @@ -38,4 +45,72 @@ export function* takeEveryUnlessBusy

( yield* takeEvery(actionDescriptor, sagaBusyWrapper); } +type EnsureMappingIsLockedReturnType = { + isMappingLockedIfNeeded: boolean; + reason?: string; +}; + +export function askUserForLockingActiveMapping( + volumeTracing: VolumeTracing, + activeMappingByLayer: Record, +): Promise { + return new Promise((resolve, reject) => { + if (!volumeTracing.mappingIsLocked) { + const lockMapping = async () => { + // A mapping that is active and is being annotated needs to be locked to ensure a consistent state in the future. + // See https://github.com/scalableminds/webknossos/issues/5431 for more information. + const activeMapping = activeMappingByLayer[volumeTracing.tracingId]; + if (activeMapping.mappingName) { + Store.dispatch(setMappingIsLockedAction()); + const message = messages["tracing.locked_mapping_confirmed"](activeMapping.mappingName); + Toast.info(message, { timeout: 10000 }); + console.log(message); + resolve({ isMappingLockedIfNeeded: true, reason: "User confirmed." }); + } else { + // Having an active mapping without a name should be impossible. Therefore, no further error handling is done. + reject({ isMappingLockedIfNeeded: false, reason: "No mapping name." }); + } + }; + Modal.confirm({ + title: "Should the active Mapping be locked?", + content: messages["tracing.locked_mapping_info"], + okText: "Lock Mapping", + cancelText: "Abort Annotation Action", + width: 600, + onOk: lockMapping, + onCancel: async () => { + reject({ isMappingLockedIfNeeded: false, reason: "User aborted." }); + }, + }); + } + }); +} + +export function* ensureMaybeActiveMappingIsLocked( + volumeTracing: VolumeTracing, +): Saga { + if (volumeTracing.mappingIsLocked) { + return { isMappingLockedIfNeeded: true, reason: "Mapping is already locked." }; + } + const activeMappingByLayer = yield* select( + (state) => state.temporaryConfiguration.activeMappingByLayer, + ); + const isSomeMappingActive = + volumeTracing.tracingId in activeMappingByLayer && + activeMappingByLayer[volumeTracing.tracingId].mappingStatus === MappingStatusEnum.ENABLED; + const isHDF5Mapping = + volumeTracing.tracingId in activeMappingByLayer && + activeMappingByLayer[volumeTracing.tracingId].mappingType === "HDF5"; + if (isSomeMappingActive && isHDF5Mapping) { + try { + return yield* call(askUserForLockingActiveMapping, volumeTracing, activeMappingByLayer); + } catch (error: any) { + return error as EnsureMappingIsLockedReturnType; + } + } else { + yield* put(setMappingIsLockedAction()); + return { isMappingLockedIfNeeded: true, reason: "Locked that no mapping is active." }; + } +} + export default {}; diff --git a/frontend/javascripts/oxalis/model/sagas/update_actions.ts b/frontend/javascripts/oxalis/model/sagas/update_actions.ts index b7f202ec8af..51fc2f118ba 100644 --- a/frontend/javascripts/oxalis/model/sagas/update_actions.ts +++ b/frontend/javascripts/oxalis/model/sagas/update_actions.ts @@ -416,11 +416,12 @@ export function serverCreateTracing(timestamp: number) { } export function updateMappingName( mappingName: string | null | undefined, - isEditable: boolean | undefined, + isEditable: boolean | null | undefined, + isLocked: boolean | undefined, ) { return { name: "updateMappingName", - value: { mappingName, isEditable }, + value: { mappingName, isEditable, isLocked }, } as const; } export function splitAgglomerate( diff --git a/frontend/javascripts/oxalis/model/sagas/volume/volume_interpolation_saga.ts b/frontend/javascripts/oxalis/model/sagas/volume/volume_interpolation_saga.ts index 789997b713d..28d84b8ac95 100644 --- a/frontend/javascripts/oxalis/model/sagas/volume/volume_interpolation_saga.ts +++ b/frontend/javascripts/oxalis/model/sagas/volume/volume_interpolation_saga.ts @@ -12,12 +12,11 @@ import { TypedArrayWithoutBigInt, Vector3, } from "oxalis/constants"; -import { Model, api } from "oxalis/singletons"; import { reuseInstanceOnEquality } from "oxalis/model/accessors/accessor_helpers"; import { getResolutionInfo } from "oxalis/model/accessors/dataset_accessor"; import { - getFlooredPosition, getActiveMagIndexForLayer, + getFlooredPosition, } from "oxalis/model/accessors/flycam_accessor"; import { enforceActiveVolumeTracing, @@ -35,9 +34,11 @@ import Dimensions from "oxalis/model/dimensions"; import type { Saga } from "oxalis/model/sagas/effect-generators"; import { select } from "oxalis/model/sagas/effect-generators"; import { VoxelBuffer2D } from "oxalis/model/volumetracing/volumelayer"; +import { Model, api } from "oxalis/singletons"; import { OxalisState } from "oxalis/store"; import { call, put } from "typed-redux-saga"; import { createVolumeLayer, getBoundingBoxForViewport, labelWithVoxelBuffer2D } from "./helpers"; +import { ensureMaybeActiveMappingIsLocked } from "../saga_helpers"; /* * This saga is capable of doing segment interpolation between two slices. @@ -435,6 +436,11 @@ export default function* maybeInterpolateSegmentationLayer(): Saga { ); 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) { + return; + } // In the extrusion case, we don't need any distance transforms. The binary // masks are enough to decide whether a voxel needs to be written. diff --git a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx index 948a61cc7a9..c0f1d4d8d76 100644 --- a/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx +++ b/frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx @@ -81,7 +81,10 @@ import type { Saga } from "oxalis/model/sagas/effect-generators"; 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 { takeEveryUnlessBusy } from "oxalis/model/sagas/saga_helpers"; +import { + ensureMaybeActiveMappingIsLocked, + takeEveryUnlessBusy, +} from "oxalis/model/sagas/saga_helpers"; import { deleteSegmentDataVolumeAction, UpdateAction, @@ -219,6 +222,14 @@ export function* editVolumeLayerAsync(): Saga { } 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, + volumeTracing, + ); + if (!isMappingLockedIfNeeded) { + continue; + } if (isDrawing && activeCellId === 0) { yield* call( @@ -435,7 +446,15 @@ export function* floodFill(): Saga { console.warn(`Ignoring floodfill request (reason: ${busyBlockingInfo.reason || "unknown"})`); continue; } - + // 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, + volumeTracing, + ); + if (!isMappingLockedIfNeeded) { + continue; + } yield* put(setBusyBlockingInfoAction(true, "Floodfill is being computed.")); const boundingBoxForFloodFill = yield* call(getBoundingBoxForFloodFill, seedPosition, planeId); const progressCallback = createProgressCallback({ @@ -693,8 +712,18 @@ export function* diffVolumeTracing( yield removeFallbackLayer(); } - if (prevVolumeTracing.mappingName !== volumeTracing.mappingName) { - yield updateMappingName(volumeTracing.mappingName, volumeTracing.mappingIsEditable); + if ( + prevVolumeTracing.mappingName !== volumeTracing.mappingName || + prevVolumeTracing.mappingIsLocked !== volumeTracing.mappingIsLocked + ) { + // Once the first volume action is performed on a volume layer, the mapping state is locked. + // In case no mapping is active, this is denoted by setting the mapping name to null. + const action = updateMappingName( + volumeTracing.mappingName || null, + volumeTracing.mappingIsEditable || null, + volumeTracing.mappingIsLocked, + ); + yield action; } } } diff --git a/frontend/javascripts/oxalis/model_initialization.ts b/frontend/javascripts/oxalis/model_initialization.ts index 181b85f4988..48907b62d8f 100644 --- a/frontend/javascripts/oxalis/model_initialization.ts +++ b/frontend/javascripts/oxalis/model_initialization.ts @@ -625,18 +625,21 @@ function determineDefaultState( (tracing) => tracing.typ === "Volume", ) as ServerVolumeTracing[]; for (const volumeTracing of volumeTracings) { - const { id: layerName, mappingName } = volumeTracing; - - if (mappingName == null) continue; + const { id: layerName, mappingName, mappingIsLocked } = volumeTracing; if (!(layerName in stateByLayer)) { stateByLayer[layerName] = {}; } - if (stateByLayer[layerName].mappingInfo == null) { - stateByLayer[layerName].mappingInfo = { - mappingName, - mappingType: "HDF5", - }; + if (stateByLayer[layerName].mappingInfo == null || mappingIsLocked) { + // A locked mapping always takes precedence over the URL configuration. + if (mappingName == null) { + delete stateByLayer[layerName].mappingInfo; + } else { + stateByLayer[layerName].mappingInfo = { + mappingName, + mappingType: "HDF5", + }; + } } } diff --git a/frontend/javascripts/oxalis/store.ts b/frontend/javascripts/oxalis/store.ts index a7a52a7e44f..3d3ead7d606 100644 --- a/frontend/javascripts/oxalis/store.ts +++ b/frontend/javascripts/oxalis/store.ts @@ -254,6 +254,7 @@ export type VolumeTracing = TracingBase & { readonly fallbackLayer?: string; readonly mappingName?: string | null | undefined; readonly mappingIsEditable?: boolean; + readonly mappingIsLocked?: boolean; readonly hasSegmentIndex: boolean; }; export type ReadOnlyTracing = TracingBase & { diff --git a/frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx b/frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx index f41e0aaec18..85c660a46d3 100644 --- a/frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx +++ b/frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx @@ -405,9 +405,14 @@ function AdditionalSkeletonModesButtons() { ); const isEditableMappingActive = segmentationTracingLayer != null && !!segmentationTracingLayer.mappingIsEditable; + const isMappingLocked = + segmentationTracingLayer != null && !!segmentationTracingLayer.mappingIsLocked; + const isMergerModeDisabled = isEditableMappingActive || isMappingLocked; const mergerModeTooltipText = isEditableMappingActive ? "Merger mode cannot be enabled while an editable mapping is active." - : "Toggle Merger Mode - When enabled, skeletons that connect multiple segments will merge those segments."; + : isMappingLocked + ? "Merger mode cannot be enabled while a mapping is locked. Please create a new annotation and use the merger mode there." + : "Toggle Merger Mode - When enabled, skeletons that connect multiple segments will merge those segments."; const toggleNewNodeNewTreeMode = () => dispatch(updateUserSettingAction("newNodeNewTree", !isNewNodeNewTreeModeOn)); @@ -440,10 +445,10 @@ function AdditionalSkeletonModesButtons() { | null | undefined; editableMapping: EditableMapping | null | undefined; + isMappingLocked: boolean; isMergerModeEnabled: boolean; allowUpdate: boolean; isEditableMappingActive: boolean; @@ -173,6 +175,12 @@ class MappingSettingsView extends React.Component { (shouldMappingBeEnabled || this.props.isMergerModeEnabled) && this.props.mapping && this.props.hideUnmappedIds != null; + const isDisabled = this.props.isEditableMappingActive || this.props.isMappingLocked; + const disabledMessage = this.props.isEditableMappingActive + ? "The mapping has been edited through proofreading actions and can no longer be disabled or changed." + : this.props.mapping + ? "This mapping has been locked to this annotation, because the segmentation was modified while it was active. It can no longer be disabled or changed." + : "The segmentation was modified while no mapping was active. To ensure a consistent state, mappings can no longer be enabled."; return ( { @@ -180,20 +188,24 @@ class MappingSettingsView extends React.Component { to avoid conflicts in the logic of the UI. */ !this.props.isMergerModeEnabled ? ( -

- -
+ +
+ +
+ {/* Show mapping-select even when the mapping is disabled but the UI was used before @@ -210,7 +222,7 @@ class MappingSettingsView extends React.Component { {...selectValueProp} onChange={this.handleChangeMapping} notFoundContent="No mappings found." - disabled={this.props.isEditableMappingActive} + disabled={isDisabled} > {renderCategoryOptions(availableMappings, "JSON")} {renderCategoryOptions(availableAgglomerates, "HDF5")} @@ -262,6 +274,7 @@ function mapStateToProps(state: OxalisState, ownProps: OwnProps) { allowUpdate: state.tracing.restrictions.allowUpdate, editableMapping, isEditableMappingActive: hasEditableMapping(state, ownProps.layerName), + isMappingLocked: isMappingLocked(state, ownProps.layerName), }; } diff --git a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts index 4b5bea72c07..57ff2b9346f 100644 --- a/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts +++ b/frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts @@ -1,12 +1,14 @@ import "test/sagas/volumetracing/volumetracing_saga.mock"; import { take, put, call } from "redux-saga/effects"; import update from "immutability-helper"; +import _ from "lodash"; import type { APISegmentationLayer, ServerVolumeTracing } from "types/api_flow_types"; import { OrthoViews, AnnotationToolEnum, ContourModeEnum, OverwriteModeEnum, + MappingStatusEnum, } from "oxalis/constants"; import { convertFrontendBoundingBoxToServer } from "oxalis/model/reducers/reducer_helpers"; import { enforce } from "libs/utils"; @@ -19,6 +21,8 @@ import sinon from "sinon"; import test from "ava"; import { expectValueDeepEqual, execCall } from "test/helpers/sagaHelpers"; import { withoutUpdateTracing } from "test/helpers/saveHelpers"; +import { ActiveMappingInfo } from "oxalis/store"; +import { askUserForLockingActiveMapping } from "oxalis/model/sagas/saga_helpers"; mockRequire("app", { currentUser: { @@ -29,10 +33,14 @@ mockRequire("app", { mockRequire("oxalis/model/sagas/root_saga", function* () { yield; }); +mockRequire("libs/toast", { + info: _.noop, +}); 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 VolumeLayer = require("oxalis/model/volumetracing/volumelayer").default; @@ -109,6 +117,20 @@ const initialState = update(defaultState, { }, }, }); + +const dummyActiveMapping: ActiveMappingInfo = { + mappingName: "dummy-mapping-name", + mapping: {}, + mappingKeys: [], + mappingColors: [], + hideUnmappedIds: false, + mappingStatus: "ENABLED", + mappingSize: 0, + mappingType: "HDF5", +}; + +const ensureMaybeMappingIsLockedReturnValueDummy = { isMappingLockedIfNeeded: true }; + const ACTIVE_CELL_ID = 5; const setActiveCellAction = VolumeTracingActions.setActiveCellAction(ACTIVE_CELL_ID); const startEditingAction = VolumeTracingActions.startEditingAction([0, 0, 0], OrthoViews.PLANE_XY); @@ -185,6 +207,7 @@ test("VolumeTracingSaga should create a volume layer (saga test)", (t) => { zoomStep: 0, }); saga.next(ACTIVE_CELL_ID); // pass active cell id + saga.next(ensureMaybeMappingIsLockedReturnValueDummy); expectValueDeepEqual( t, saga.next([]), // pass empty additional coords @@ -225,6 +248,7 @@ test("VolumeTracingSaga should add values to volume layer (saga test)", (t) => { }); // pass labeled resolution saga.next(ACTIVE_CELL_ID); // pass active cell id + saga.next(ensureMaybeMappingIsLockedReturnValueDummy); expectValueDeepEqual( t, saga.next([]), // pass empty additional coords @@ -275,6 +299,7 @@ test("VolumeTracingSaga should finish a volume layer (saga test)", (t) => { }); // pass labeled resolution saga.next(ACTIVE_CELL_ID); // pass active cell id + saga.next(ensureMaybeMappingIsLockedReturnValueDummy); expectValueDeepEqual( t, saga.next([]), // pass empty additional coords @@ -336,6 +361,7 @@ test("VolumeTracingSaga should finish a volume layer in delete mode (saga test)" zoomStep: 0, }); // pass labeled resolution saga.next(ACTIVE_CELL_ID); // pass active cell id + saga.next(ensureMaybeMappingIsLockedReturnValueDummy); expectValueDeepEqual( t, saga.next([]), // pass empty additional coords @@ -394,3 +420,72 @@ test("VolumeTracingSaga should ignore brush action when busy (saga test)", (t) = take("START_EDITING"), ); }); + +test("VolumeTracingSaga should lock an active mapping upon first volume annotation", (t) => { + const saga = editVolumeLayerAsync(); + saga.next(); + saga.next(); + expectValueDeepEqual(t, saga.next(true), take("START_EDITING")); + saga.next(startEditingAction); + saga.next({ + isBusy: false, + }); + saga.next(volumeTracing); + saga.next(OverwriteModeEnum.OVERWRITE_ALL); + saga.next(AnnotationToolEnum.BRUSH); + saga.next(false); + // pass labeled resolution + saga.next({ + resolution: [1, 1, 1], + zoomStep: 0, + }); + // Test whether nested saga ensureMaybeActiveMappingIsLocked is called. + expectValueDeepEqual( + t, + saga.next(ACTIVE_CELL_ID), + call(ensureMaybeActiveMappingIsLocked, volumeTracing), + ); +}); + +test("ensureMaybeActiveMappingIsLocked should lock an existing mapping to the annotation", (t) => { + const activeMappingByLayer = { [volumeTracing.tracingId]: dummyActiveMapping }; + const saga = ensureMaybeActiveMappingIsLocked(volumeTracing); + saga.next(); + expectValueDeepEqual( + t, + saga.next({ [volumeTracing.tracingId]: dummyActiveMapping }), + call(askUserForLockingActiveMapping, volumeTracing, activeMappingByLayer), + ); + t.true(saga.next().done); +}); + +test("ensureMaybeActiveMappingIsLocked should lock 'no mapping' in case no mapping is active.", (t) => { + const saga = ensureMaybeActiveMappingIsLocked(volumeTracing); + saga.next(); + expectValueDeepEqual(t, saga.next({}), put(VolumeTracingActions.setMappingIsLockedAction())); + t.true(saga.next().done); +}); + +test("ensureMaybeActiveMappingIsLocked should lock 'no mapping' in case a mapping is active but disabled.", (t) => { + const jsonDummyMapping = { ...dummyActiveMapping, mappingStatus: MappingStatusEnum.DISABLED }; + const saga = ensureMaybeActiveMappingIsLocked(volumeTracing); + saga.next(); + expectValueDeepEqual( + t, + saga.next({ [volumeTracing.tracingId]: jsonDummyMapping }), + put(VolumeTracingActions.setMappingIsLockedAction()), + ); + t.true(saga.next().done); +}); + +test("ensureMaybeActiveMappingIsLocked should lock 'no mapping' in case a JSON mapping is active.", (t) => { + const jsonDummyMapping = { ...dummyActiveMapping, mappingType: "JSON" }; + const saga = ensureMaybeActiveMappingIsLocked(volumeTracing); + saga.next(); + expectValueDeepEqual( + t, + saga.next({ [volumeTracing.tracingId]: jsonDummyMapping }), + put(VolumeTracingActions.setMappingIsLockedAction()), + ); + t.true(saga.next().done); +}); diff --git a/frontend/javascripts/types/api_flow_types.ts b/frontend/javascripts/types/api_flow_types.ts index 01b20ba0cfe..09fc30c9ffa 100644 --- a/frontend/javascripts/types/api_flow_types.ts +++ b/frontend/javascripts/types/api_flow_types.ts @@ -773,6 +773,7 @@ export type ServerVolumeTracing = ServerTracingBase & { resolutions?: Array; mappingName?: string | null | undefined; mappingIsEditable?: boolean; + mappingIsLocked?: boolean; hasSegmentIndex?: boolean; }; export type ServerTracing = ServerSkeletonTracing | ServerVolumeTracing; diff --git a/webknossos-datastore/proto/VolumeTracing.proto b/webknossos-datastore/proto/VolumeTracing.proto index 8108b789ade..94814da1fd4 100644 --- a/webknossos-datastore/proto/VolumeTracing.proto +++ b/webknossos-datastore/proto/VolumeTracing.proto @@ -40,11 +40,12 @@ message VolumeTracing { repeated Vec3IntProto resolutions = 15; repeated Segment segments = 16; optional string mappingName = 17; // either a mapping present in the fallback layer, or an editable mapping on the tracingstore - optional bool mappingIsEditable = 18; + optional bool mappingIsEditable = 18; // the selected mapping is an editable mapping repeated SegmentGroup segmentGroups = 19; optional bool hasSegmentIndex = 20; 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) } message SegmentGroup { diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/controllers/VolumeTracingController.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/controllers/VolumeTracingController.scala index 44c35d09ab7..548427c5bf4 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/controllers/VolumeTracingController.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/controllers/VolumeTracingController.scala @@ -357,11 +357,13 @@ class VolumeTracingController @Inject()( for { tracing <- tracingService.find(tracingId) tracingMappingName <- tracing.mappingName ?~> "annotation.noMappingSet" + _ <- assertMappingIsNotLocked(tracing) _ <- bool2Fox(tracingService.volumeBucketsAreEmpty(tracingId)) ?~> "annotation.volumeBucketsNotEmpty" (editableMappingId, editableMappingInfo) <- editableMappingService.create( baseMappingName = tracingMappingName) volumeUpdate = UpdateMappingNameAction(Some(editableMappingId), isEditable = Some(true), + isLocked = Some(true), actionTimestamp = Some(System.currentTimeMillis())) _ <- tracingService.handleUpdateGroup( tracingId, @@ -386,6 +388,9 @@ class VolumeTracingController @Inject()( } } + private def assertMappingIsNotLocked(volumeTracing: VolumeTracing): Fox[Unit] = + bool2Fox(!volumeTracing.mappingIsLocked.getOrElse(false)) ?~> "annotation.mappingIsLocked" + def agglomerateGraphMinCut(token: Option[String], tracingId: String): Action[MinCutParameters] = Action.async(validateJson[MinCutParameters]) { implicit request => log() { diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala index 961aecc9a3e..2cb0f710558 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeUpdateActions.scala @@ -348,6 +348,7 @@ object DeleteSegmentDataVolumeAction { case class UpdateMappingNameAction(mappingName: Option[String], isEditable: Option[Boolean], + isLocked: Option[Boolean], actionTimestamp: Option[Long], actionAuthorId: Option[String] = None) extends ApplyableVolumeAction { @@ -361,7 +362,11 @@ case class UpdateMappingNameAction(mappingName: Option[String], Json.obj("mappingName" -> mappingName)) override def applyOn(tracing: VolumeTracing): VolumeTracing = - tracing.copy(mappingName = mappingName, mappingIsEditable = Some(isEditable.getOrElse(false))) + if (tracing.mappingIsLocked.getOrElse(false)) tracing // cannot change mapping name if it is locked + else + tracing.copy(mappingName = mappingName, + mappingIsEditable = Some(isEditable.getOrElse(false)), + mappingIsLocked = Some(isLocked.getOrElse(false))) } object UpdateMappingNameAction {