Make merger mode work for all skeleton modifications

CHANGELOG.unreleased.md

@@ -26,6 +26,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Fixed a bug where requesting volume tracing fallback layer data from webknossos-connect failed. [#4644](https://github.com/scalableminds/webknossos/pull/4644)
 - Fixed a bug where imported invisible trees were still visible. [#4659](https://github.com/scalableminds/webknossos/issues/4659)
 - Fixed the message formatting for standalone datastores and tracingstores. [#4656](https://github.com/scalableminds/webknossos/pull/4656)
+- Fixed that merger mode didn't work with undo and redo. Also fixed that the mapping was not disabled when disabling merger mode. [#4669](https://github.com/scalableminds/webknossos/pull/4669)
 
 ### Removed
 

frontend/javascripts/oxalis/api/api_latest.js

@@ -75,6 +75,7 @@ import {
   updateDatasetSettingAction,
   updateLayerSettingAction,
   setMappingAction,
+  setMappingEnabledAction,
 } from "oxalis/model/actions/settings_actions";
 import { wkReadyAction, restartSagaAction } from "oxalis/model/actions/actions";
 import Model, { type OxalisModel } from "oxalis/model";
@@ -771,6 +772,13 @@ class DataApi {
     );
   }
 
+  /**
+   * Enables/Disables the active mapping.
+   */
+  setMappingEnabled(isEnabled: boolean) {
+    Store.dispatch(setMappingEnabledAction(isEnabled));
+  }
+
   /**
    * Gets all available mapping names for a given layer. When the layer name
    * is omitted, "segmentation" is assumed.
@@ -1216,7 +1224,7 @@ class UtilsApi {
    * // ... optionally:
    * // removeToast();
    */
-  showToast(type: ToastStyle, message: string, timeout: number): ?Function {
+  showToast(type: ToastStyle, message: string, timeout?: number): ?Function {
     Toast.message(type, message, { sticky: timeout === 0, timeout });
     return () => Toast.close(message);
   }

frontend/javascripts/oxalis/merger_mode.js

@@ -1,8 +1,12 @@
 // @flow
 import { Modal } from "antd";
-import type { Node, TreeMap } from "oxalis/store";
+import type { Node, TreeMap, SkeletonTracing } from "oxalis/store";
 import api from "oxalis/api/internal_api";
 import _ from "lodash";
+import type { Vector3 } from "oxalis/constants";
+import { getSkeletonTracing } from "oxalis/model/accessors/skeletontracing_accessor";
+import Store from "oxalis/throttled_store";
+import { cachedDiffTrees } from "oxalis/model/sagas/skeletontracing_saga";
 
 type NodeWithTreeId = Node & { treeId: number };
 
@@ -13,10 +17,11 @@ type MergerModeState = {
   nodes: Array<NodeWithTreeId>,
   segmentationLayerName: string,
   nodeSegmentMap: Object,
+  prevTracing: SkeletonTracing,
 };
 
 const unregisterKeyHandlers = [];
-const unregisterOverwrites = [];
+const unsubscribeFunctions = [];
 let isCodeActive = false;
 
 function mapSegmentColorToTree(segId: number, treeId: number, mergerModeState: MergerModeState) {
@@ -80,33 +85,32 @@ function getAllNodesWithTreeId(): Array<NodeWithTreeId> {
   return nodes;
 }
 
-/* Here we intercept calls to the "addNode" method. This allows us to look up the segment id at the specified
-   point and display it in the same color as the rest of the aggregate. */
-async function createNodeOverwrite(store, call, action, mergerModeState: MergerModeState) {
-  call(action);
+/* React to added nodes. Look up the segment id at the node position and
+  display it in the same color as the rest of the aggregate. */
+async function createNode(
+  mergerModeState: MergerModeState,
+  nodeId: number,
+  treeId: number,
+  position: Vector3,
+) {
   const { colorMapping, segmentationLayerName, nodeSegmentMap } = mergerModeState;
-  const pos = action.position;
-  const segmentId = await api.data.getDataValue(segmentationLayerName, pos);
+  const segmentId = await api.data.getDataValue(segmentationLayerName, position);
 
-  const activeTreeId = api.tracing.getActiveTreeId();
-  const activeNodeId = api.tracing.getActiveNodeId();
-  // If the node wasn't created. This should never happen.
-  if (activeTreeId == null || activeNodeId == null) {
-    Modal.info({ title: "The created node could not be detected." });
-    return;
-  }
   // If there is no segment id, the node was set too close to a border between segments.
   if (!segmentId) {
-    Modal.info({ title: "You've set a point too close to grey. The node will be removed now." });
-    api.tracing.deleteNode(activeNodeId, activeTreeId);
+    api.utils.showToast(
+      "warning",
+      "You've set a node too close to grey. The node will be removed now.",
+    );
+    api.tracing.deleteNode(nodeId, treeId);
     return;
   }
 
   // Set segment id
-  nodeSegmentMap[activeNodeId] = segmentId;
+  nodeSegmentMap[nodeId] = segmentId;
   // Count references
   increaseNodesOfSegment(segmentId, mergerModeState);
-  mapSegmentColorToTree(segmentId, activeTreeId, mergerModeState);
+  mapSegmentColorToTree(segmentId, treeId, mergerModeState);
 
   // Update mapping
   api.data.setMapping(segmentationLayerName, colorMapping);
@@ -127,52 +131,34 @@ function onNodeDeleted(mergerModeState: MergerModeState, nodeId: number) {
   return false;
 }
 
-/* Overwrite the "deleteActiveNode" method in such a way that a segment changes back its color as soon as all
+/* Make sure that a segment changes back its color as soon as all
    nodes are deleted from it. */
-function deleteActiveNodeOverwrite(store, call, action, mergerModeState: MergerModeState) {
-  const activeNodeId = api.tracing.getActiveNodeId();
-  if (activeNodeId == null) {
-    return;
-  }
-  const noNodesLeftForTheSegment = onNodeDeleted(mergerModeState, activeNodeId);
+function deleteNode(mergerModeState: MergerModeState, nodeId: number) {
+  const noNodesLeftForTheSegment = onNodeDeleted(mergerModeState, nodeId);
   if (noNodesLeftForTheSegment) {
     api.data.setMapping(mergerModeState.segmentationLayerName, mergerModeState.colorMapping);
   }
-  call(action);
 }
 
-/* Overwrite the "deleteActiveTree" method in such a way that all segment changes back its color as soon as all
-   nodes are deleted from it. */
-function deleteTree(store, action, mergerModeState: MergerModeState) {
-  let { treeId } = action;
-  if (treeId == null) {
-    treeId = api.tracing.getActiveTreeId();
-  }
-  if (treeId == null) {
-    return;
-  }
-  const deletedTree = api.tracing.getAllTrees()[treeId];
-  let didMappingChange = false;
-  for (const nodeId of deletedTree.nodes.keys()) {
-    didMappingChange = onNodeDeleted(mergerModeState, nodeId) || didMappingChange;
-  }
-  if (didMappingChange) {
-    api.data.setMapping(mergerModeState.segmentationLayerName, mergerModeState.colorMapping);
-  }
-}
+function updateState(mergerModeState: MergerModeState, skeletonTracing: SkeletonTracing) {
+  const diff = cachedDiffTrees(mergerModeState.prevTracing, skeletonTracing);
 
-// Overwrite deleting multiple trees as part of a batched action
-function deleteTreesBatchedOverwrite(store, call, action, mergerModeState: MergerModeState) {
-  for (const subAction of action.payload) {
-    if (subAction.type === "DELETE_TREE") deleteTree(store, subAction, mergerModeState);
+  for (const action of diff) {
+    switch (action.name) {
+      case "createNode": {
+        const { treeId, id: nodeId, position } = action.value;
+        createNode(mergerModeState, nodeId, treeId, position);
+        break;
+      }
+      case "deleteNode":
+        deleteNode(mergerModeState, action.value.nodeId);
+        break;
+      default:
+        break;
+    }
   }
-  call(action);
-}
 
-// Overwrite deleting a single tree
-function deleteTreeOverwrite(store, call, action, mergerModeState: MergerModeState) {
-  deleteTree(store, action, mergerModeState);
-  call(action);
+  mergerModeState.prevTracing = skeletonTracing;
 }
 
 type WriteableDatasetLayerConfiguration = {
@@ -270,41 +256,40 @@ async function mergeSegmentsOfAlreadyExistingTrees(
   api.data.setMapping(segmentationLayerName, colorMapping);
 }
 
-export async function enableMergerMode(onProgressUpdate: number => void) {
-  if (isCodeActive) {
-    return;
-  }
-  isCodeActive = true;
+function resetState(mergerModeState?: MergerModeState = {}) {
   const segmentationLayerName = api.data.getVolumeTracingLayerName();
-  // Create an object that store the state of the merger mode.
-  const mergerModeState: MergerModeState = {
+  const defaults = {
     treeColors: {},
     colorMapping: {},
     nodesPerSegment: {},
     nodes: getAllNodesWithTreeId(),
     segmentationLayerName,
     nodeSegmentMap: {},
+    prevTracing: getSkeletonTracing(Store.getState().tracing).get(),
   };
-  // Register the overwrites
-  unregisterOverwrites.push(
-    api.utils.registerOverwrite("CREATE_NODE", (store, next, originalAction) =>
-      createNodeOverwrite(store, next, originalAction, mergerModeState),
-    ),
-  );
-  unregisterOverwrites.push(
-    api.utils.registerOverwrite("DELETE_NODE", (store, next, originalAction) =>
-      deleteActiveNodeOverwrite(store, next, originalAction, mergerModeState),
-    ),
-  );
-  unregisterOverwrites.push(
-    api.utils.registerOverwrite("DELETE_TREE", (store, next, originalAction) =>
-      deleteTreeOverwrite(store, next, originalAction, mergerModeState),
-    ),
-  );
-  unregisterOverwrites.push(
-    api.utils.registerOverwrite("DELETE_GROUP_AND_TREES", (store, next, originalAction) =>
-      deleteTreesBatchedOverwrite(store, next, originalAction, mergerModeState),
-    ),
+  // Keep the object identity when resetting
+  return Object.assign(mergerModeState, defaults);
+}
+
+export async function enableMergerMode(onProgressUpdate: number => void) {
+  if (isCodeActive) {
+    return;
+  }
+  isCodeActive = true;
+  // Create an object that stores the state of the merger mode.
+  const mergerModeState: MergerModeState = resetState();
+  // Register for tracing changes
+  unsubscribeFunctions.push(
+    Store.subscribe(() => {
+      getSkeletonTracing(Store.getState().tracing).map(skeletonTracing => {
+        if (skeletonTracing.tracingId !== mergerModeState.prevTracing.tracingId) {
+          resetState(mergerModeState);
+          api.data.setMappingEnabled(false);
+        } else {
+          updateState(mergerModeState, skeletonTracing);
+        }
+      });
+    }),
   );
   // Register the additional key handlers
   unregisterKeyHandlers.push(
@@ -326,6 +311,9 @@ export function disableMergerMode() {
     return;
   }
   isCodeActive = false;
-  unregisterOverwrites.forEach(unregisterFunction => unregisterFunction());
+  unsubscribeFunctions.forEach(unsubscribeFunction => unsubscribeFunction());
   unregisterKeyHandlers.forEach(unregisterObject => unregisterObject.unregister());
+
+  // Disable the custom merger mode mapping
+  api.data.setMappingEnabled(false);
 }

frontend/javascripts/oxalis/model/bucket_data_handling/mappings.js

@@ -258,9 +258,13 @@ class Mappings {
       },
     );
 
+    // updateMappingColorTexture has to be called at least once to guarantee
+    // proper initialization of the texture with -1.
+    // There is a race condition otherwise leading to hard-to-debug errors.
     listenToStoreProperty(
       state => state.temporaryConfiguration.activeMapping.mappingColors,
       mappingColors => this.updateMappingColorTexture(mappingColors),
+      true,
     );
   }
 
@@ -325,6 +329,10 @@ class Mappings {
     );
     await progressCallback(true, "Mapping successfully applied.");
     Store.dispatch(setMappingEnabledAction(true));
+
+    // Reset progressCallback, so it doesn't trigger when setting mappings
+    // programmatically, later, e.g. in merger mode
+    this.progressCallback = noopProgressCallback;
   }
 
   getMappingTextures() {

frontend/javascripts/oxalis/view/right-menu/mapping_info_view.js

@@ -394,8 +394,12 @@ class MappingInfoView extends React.Component<Props, State> {
       return useGroups ? <OptGroup label={category}>{elements}</OptGroup> : elements;
     };
 
+    // The mapping toggle should be active if either the user clicked on it (this.state.shouldMappingBeEnabled)
+    // or a mapping was activated, e.g. from the API or by selecting one from the dropdown (this.props.isMappingEnabled).
+    const shouldMappingBeEnabled = this.state.shouldMappingBeEnabled || this.props.isMappingEnabled;
+
     const renderHideUnmappedSegmentsSwitch =
-      (this.state.shouldMappingBeEnabled || this.props.isMergerModeEnabled) &&
+      (shouldMappingBeEnabled || this.props.isMergerModeEnabled) &&
       this.props.mapping &&
       this.props.hideUnmappedIds != null;
 
@@ -412,7 +416,7 @@ class MappingInfoView extends React.Component<Props, State> {
                   ID Mapping
                   <Switch
                     onChange={this.handleSetMappingEnabled}
-                    checked={this.state.shouldMappingBeEnabled}
+                    checked={shouldMappingBeEnabled}
                     style={{ float: "right" }}
                     loading={this.state.isRefreshingMappingList}
                   />
@@ -423,7 +427,7 @@ class MappingInfoView extends React.Component<Props, State> {
             Show mapping-select even when the mapping is disabled but the UI was used before
             (i.e., mappingName != null)
           */}
-              {this.state.shouldMappingBeEnabled || this.props.mappingName != null ? (
+              {shouldMappingBeEnabled || this.props.mappingName != null ? (
                 <Select
                   placeholder="Select mapping"
                   defaultActiveFirstOption={false}
@@ -494,11 +498,12 @@ function mapStateToProps(state: OxalisState) {
 }
 
 const debounceTime = 100;
+const maxWait = 500;
 export default connect<Props, OwnProps, _, _, _, _>(
   mapStateToProps,
   mapDispatchToProps,
   null,
   {
     pure: false,
   },
-)(debounceRender(MappingInfoView, debounceTime));
+)(debounceRender(MappingInfoView, debounceTime, { maxWait }));