Improve Proofreading: Add clear button for auxiliary skeletons/meshes…

… & provide merge/split via context menu on voxel (#6459) * add clear button to toolbar when proofreading tool is active to delete meshes and skeletons after proofreading * update changelog * make case of proofreading consistent * Apply suggestions from code review * Allow merge and min-cut when rightclicking on voxel (instead of node) (#6464) * implement splitting and merging in proofreading via context menu on non-node position * parallelize some async operations * also validate unmapped ids when doing node-less split; refactor; don't center active node when reloading skeleton * also expose min-cut in context menu when no node is clicked * also remove trees when cleaning up that were added due to reloads * clean up a bit * update changelog * keep currently active node despite of reloading the agglomerate skeleton after split/cut * improve wording in connectome tab * remove the split-only option from the context menu (when not rightclicking a node) * update changelog * Update frontend/javascripts/oxalis/model/sagas/proofread_saga.ts Co-authored-by: Tom Herold <[email protected]> * remove unnecessary log * fix data loading route when floating positions are used * only show proofreading tool if an agglomerate mapping is selected; don't show merge/min-cut context menu items if proofreading tool is not even shown * fix ts error * pretty Co-authored-by: Tom Herold <[email protected]> Co-authored-by: Tom Herold <[email protected]>
scalableminds · Sep 14, 2022 · c944273 · c944273
1 parent 55d5705
commit c944273
Show file tree

Hide file tree

Showing 13 changed files with 613 additions and 180 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -14,6 +14,8 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Zarr-based remote dataset import now also works for public AWS S3 endpoints with no credentials. [#6421](https://github.com/scalableminds/webknossos/pull/6421)
 - Added a context menu option to extract the shortest path between two nodes as a new tree. Select the source node and open the context menu by right-clicking on another node in the same tree. [#6423](https://github.com/scalableminds/webknossos/pull/6423)
 - Added a context menu option to separate an agglomerate skeleton using Min-Cut. Activate the Proofreading tool, select the source node and open the context menu by right-clicking on the target node which you would like to separate through Min-Cut. [#6361](https://github.com/scalableminds/webknossos/pull/6361)
+- Added a "clear" button to reset skeletons/meshes after successful mergers/split. [#6459](https://github.com/scalableminds/webknossos/pull/6459)
+- The proofreading tool now supports merging and splitting (via min-cut) agglomerates by rightclicking a segment (and not a node). Note that there still has to be an active node so that both partners of the operation are defined. [#6464](https://github.com/scalableminds/webknossos/pull/6464)
 
 ### Changed
 

diff --git a/frontend/javascripts/libs/toast.tsx b/frontend/javascripts/libs/toast.tsx
@@ -45,7 +45,8 @@ const Toast = {
     details: string | React.ReactNode | null,
   ) {
     if (!details) {
-      return title;
+      // This also handles empty title strings
+      return title || "Unknown Error";
     }
 
     return (

diff --git a/frontend/javascripts/oxalis/api/api_latest.ts b/frontend/javascripts/oxalis/api/api_latest.ts
@@ -1546,12 +1546,12 @@ class DataApi {
     return (
       `${dataset.dataStore.url}/data/datasets/${dataset.owningOrganization}/${dataset.name}/layers/${layerName}/data?mag=${magString}&` +
       `token=${token}&` +
-      `x=${topLeft[0]}&` +
-      `y=${topLeft[1]}&` +
-      `z=${topLeft[2]}&` +
-      `width=${bottomRight[0] - topLeft[0]}&` +
-      `height=${bottomRight[1] - topLeft[1]}&` +
-      `depth=${bottomRight[2] - topLeft[2]}`
+      `x=${Math.floor(topLeft[0])}&` +
+      `y=${Math.floor(topLeft[1])}&` +
+      `z=${Math.floor(topLeft[2])}&` +
+      `width=${Math.floor(bottomRight[0] - topLeft[0])}&` +
+      `height=${Math.floor(bottomRight[1] - topLeft[1])}&` +
+      `depth=${Math.floor(bottomRight[2] - topLeft[2])}`
     );
   }
 

diff --git a/frontend/javascripts/oxalis/model/actions/proofread_actions.ts b/frontend/javascripts/oxalis/model/actions/proofread_actions.ts
@@ -1,11 +1,42 @@
 import { Vector3 } from "oxalis/constants";
 
 export type ProofreadAtPositionAction = ReturnType<typeof proofreadAtPosition>;
+export type ClearProofreadingByProductsAction = ReturnType<typeof clearProofreadingByProducts>;
+export type ProofreadMergeAction = ReturnType<typeof proofreadMerge>;
+export type MinCutAgglomerateAction = ReturnType<typeof minCutAgglomerateAction>;
+export type MinCutAgglomerateWithPositionAction = ReturnType<
+  typeof minCutAgglomerateWithPositionAction
+>;
 
-export type ProofreadAction = ProofreadAtPositionAction;
+export type ProofreadAction = ProofreadAtPositionAction | ClearProofreadingByProductsAction;
 
 export const proofreadAtPosition = (position: Vector3) =>
   ({
     type: "PROOFREAD_AT_POSITION",
     position,
   } as const);
+
+export const clearProofreadingByProducts = () =>
+  ({
+    type: "CLEAR_PROOFREADING_BY_PRODUCTS",
+  } as const);
+
+export const proofreadMerge = (position: Vector3) =>
+  ({
+    type: "PROOFREAD_MERGE",
+    position,
+  } as const);
+
+export const minCutAgglomerateAction = (sourceNodeId: number, targetNodeId: number) =>
+  ({
+    type: "MIN_CUT_AGGLOMERATE",
+    sourceNodeId,
+    targetNodeId,
+  } as const);
+
+export const minCutAgglomerateWithPositionAction = (sourceNodeId: number, position: Vector3) =>
+  ({
+    type: "MIN_CUT_AGGLOMERATE_WITH_POSITION",
+    sourceNodeId,
+    position,
+  } as const);
diff --git a/frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx b/frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx
@@ -41,7 +41,6 @@ type DeselectActiveTreeAction = ReturnType<typeof deselectActiveTreeAction>;
 type SetActiveGroupAction = ReturnType<typeof setActiveGroupAction>;
 type DeselectActiveGroupAction = ReturnType<typeof deselectActiveGroupAction>;
 export type MergeTreesAction = ReturnType<typeof mergeTreesAction>;
-export type MinCutAgglomerateAction = ReturnType<typeof minCutAgglomerateAction>;
 type SetTreeNameAction = ReturnType<typeof setTreeNameAction>;
 type SelectNextTreeAction = ReturnType<typeof selectNextTreeAction>;
 type SetTreeColorIndexAction = ReturnType<typeof setTreeColorIndexAction>;
@@ -83,7 +82,6 @@ export type SkeletonTracingAction =
   | SetActiveTreeByNameAction
   | DeselectActiveTreeAction
   | MergeTreesAction
-  | MinCutAgglomerateAction
   | SetTreeNameAction
   | SelectNextTreeAction
   | SetTreeColorAction
@@ -197,11 +195,16 @@ export const deleteEdgeAction = (
     timestamp,
   } as const);
 
-export const setActiveNodeAction = (nodeId: number, suppressAnimation: boolean = false) =>
+export const setActiveNodeAction = (
+  nodeId: number,
+  suppressAnimation: boolean = false,
+  suppressCentering: boolean = false,
+) =>
   ({
     type: "SET_ACTIVE_NODE",
     nodeId,
     suppressAnimation,
+    suppressCentering,
   } as const);
 
 export const centerActiveNodeAction = (suppressAnimation: boolean = false) =>
@@ -264,17 +267,26 @@ export const createTreeAction = (timestamp: number = Date.now()) =>
 export const addTreesAndGroupsAction = (
   trees: MutableTreeMap,
   treeGroups: Array<TreeGroup> | null | undefined,
+  treeIdsCallback: ((ids: number[]) => void) | undefined = undefined,
 ) =>
   ({
     type: "ADD_TREES_AND_GROUPS",
     trees,
     treeGroups: treeGroups || [],
+    treeIdsCallback,
   } as const);
 
-export const deleteTreeAction = (treeId?: number) =>
+export const deleteTreeAction = (treeId?: number, suppressActivatingNextNode: boolean = false) =>
+  // If suppressActivatingNextNode is true, the tree will be deleted without activating
+  // another node (nor tree). Use this in cases where you want to avoid changing
+  // the active position (due to the auto-centering). One could also suppress the auto-centering
+  // behavior, but the semantics of changing the active node might also be confusing to the user
+  // (e.g., when proofreading). So, it might be clearer to not have an active node in the first
+  // place.
   ({
     type: "DELETE_TREE",
     treeId,
+    suppressActivatingNextNode,
   } as const);
 
 export const resetSkeletonTracingAction = () =>
@@ -352,13 +364,6 @@ export const mergeTreesAction = (sourceNodeId: number, targetNodeId: number) =>
     targetNodeId,
   } as const);
 
-export const minCutAgglomerateAction = (sourceNodeId: number, targetNodeId: number) =>
-  ({
-    type: "MIN_CUT_AGGLOMERATE",
-    sourceNodeId,
-    targetNodeId,
-  } as const);
-
 export const setTreeNameAction = (
   name: string | undefined | null = null,
   treeId?: number | null | undefined,

diff --git a/frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts b/frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts
@@ -81,18 +81,16 @@ export class NullBucket {
     return Promise.resolve();
   }
 }
+
+export type TypedArrayConstructor =
+  | Uint8ArrayConstructor
+  | Uint16ArrayConstructor
+  | Uint32ArrayConstructor
+  | Float32ArrayConstructor
+  | BigUint64ArrayConstructor;
 export const getConstructorForElementClass = (
   type: ElementClass,
-): [
-  (
-    | Uint8ArrayConstructor
-    | Uint16ArrayConstructor
-    | Uint32ArrayConstructor
-    | Float32ArrayConstructor
-    | BigUint64ArrayConstructor
-  ),
-  number,
-] => {
+): [TypedArrayConstructor, number] => {
   switch (type) {
     case "int8":
     case "uint8":

diff --git a/frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts b/frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts
@@ -803,8 +803,11 @@ function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState
           const { trees, treeGroups } = action;
           const treesWithNames = ensureTreeNames(state, trees);
           return addTreesAndGroups(skeletonTracing, treesWithNames, treeGroups)
-            .map(([updatedTrees, updatedTreeGroups, newMaxNodeId]) =>
-              update(state, {
+            .map(([updatedTrees, updatedTreeGroups, newMaxNodeId]) => {
+              if (action.treeIdsCallback) {
+                action.treeIdsCallback(Utils.values(updatedTrees).map((tree) => tree.treeId));
+              }
+              return update(state, {
                 tracing: {
                   skeleton: {
                     treeGroups: {
@@ -818,15 +821,15 @@ function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState
                     },
                   },
                 },
-              }),
-            )
+              });
+            })
             .getOrElse(state);
         }
 
         case "DELETE_TREE": {
-          const { treeId } = action;
+          const { treeId, suppressActivatingNextNode } = action;
           return getTree(skeletonTracing, treeId)
-            .chain((tree) => deleteTree(skeletonTracing, tree))
+            .chain((tree) => deleteTree(skeletonTracing, tree, suppressActivatingNextNode))
             .map(([trees, newActiveTreeId, newActiveNodeId, newMaxNodeId]) =>
               update(state, {
                 tracing: {

diff --git a/frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts b/frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts
@@ -621,14 +621,15 @@ export function addTreesAndGroups(
 export function deleteTree(
   skeletonTracing: SkeletonTracing,
   tree: Tree,
+  suppressActivatingNextNode: boolean = false,
 ): Maybe<[TreeMap, number | null | undefined, number | null | undefined, number]> {
   // Delete tree
   const newTrees = _.omit(skeletonTracing.trees, tree.treeId);
 
   let newActiveTreeId = null;
   let newActiveNodeId = null;
 
-  if (_.size(newTrees) > 0) {
+  if (_.size(newTrees) > 0 && !suppressActivatingNextNode) {
     // Setting the tree active whose id is the next highest compared to the id of the deleted tree.
     newActiveTreeId = getNearestTreeId(tree.treeId, newTrees);
     // @ts-expect-error ts-migrate(2571) FIXME: Object is of type 'unknown'.