Fix occasional broken node selection

CHANGELOG.unreleased.md

@@ -17,6 +17,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Redesigned organization page to include more infos on organization users, storage, webKnossos plan and provided opportunities to upgrade. [#6602](https://github.com/scalableminds/webknossos/pull/6602)
 
 ### Fixed
+- Fixed node selection and context menu for node ids greater than 130813. [#6724](https://github.com/scalableminds/webknossos/pull/6724)
 - Fixed the validation of some neuroglancer URLs during import. [#6722](https://github.com/scalableminds/webknossos/pull/6722)
 
 ### Removed

frontend/javascripts/oxalis/api/api_latest.ts

@@ -311,7 +311,7 @@ class TracingApi {
       const tree =
         treeId != null
           ? skeletonTracing.trees[treeId]
-          : findTreeByNodeId(skeletonTracing.trees, nodeId).get();
+          : findTreeByNodeId(skeletonTracing.trees, nodeId);
       assertExists(tree, `Couldn't find node ${nodeId}.`);
       Store.dispatch(createCommentAction(commentText, nodeId, tree.treeId));
     } else {

frontend/javascripts/oxalis/api/api_v2.ts

@@ -159,7 +159,7 @@ class TracingApi {
       const tree =
         treeId != null
           ? skeletonTracing.trees[treeId]
-          : findTreeByNodeId(skeletonTracing.trees, nodeId).get();
+          : findTreeByNodeId(skeletonTracing.trees, nodeId);
       assertExists(tree, `Couldn't find node ${nodeId}.`);
       Store.dispatch(createCommentAction(commentText, nodeId, tree.treeId));
     } else {

frontend/javascripts/oxalis/controller/combinations/skeleton_handlers.ts

@@ -331,8 +331,8 @@ export function maybeGetNodeIdFromPosition(
   // compute the index of the pixel under the cursor,
   // while inverting along the y-axis, because WebGL has its origin bottom-left :/
   const index = (x + (height - y) * width) * 4;
-  // the nodeId can be reconstructed by interpreting the RGB values of the pixel as a base-255 number
-  const nodeId = buffer.subarray(index, index + 3).reduce((a, b) => a * 255 + b, 0);
+  // the nodeId can be reconstructed by interpreting the RGB values of the pixel as a base-256 number
+  const nodeId = buffer.subarray(index, index + 3).reduce((a, b) => a * 256 + b, 0);
   skeleton.stopPicking();
   // prevent flickering sometimes caused by picking
   planeView.renderFunction(true);

frontend/javascripts/oxalis/geometries/materials/node_shader.ts

@@ -185,12 +185,12 @@ void main() {
 
     // NODE COLOR FOR PICKING
     if (isPicking == 1) {
-      // the nodeId is encoded in the RGB channels as a 3 digit base-255 number in a number of steps:
-      // - nodeId is divided by the first three powers of 255.
+      // the nodeId is encoded in the RGB channels as a 3 digit base-256 number in a number of steps:
+      // - nodeId is divided by the first three powers of 256.
       // - each quotient is rounded down to the nearest integer (since the fractional part of each quotient is covered by a less significant digit)
-      // - each digit is divided by 255 again, since color values in OpenGL must be in the range [0, 1]
+      // - each digit is divided by 256 again, since color values in OpenGL must be in the range [0, 1]
       // - finally, the non-fractional part of each digit is removed (since it is covered by a more significant digit)
-      color = fract(floor(nodeId / vec3(255.0 * 255.0, 255.0, 1.0)) / 255.0);
+      color = fract(floor(nodeId / vec3(256.0 * 256.0, 256.0, 1.0)) / 256.0);
       // Enlarge the nodes on mobile, so they're easier to select
       gl_PointSize = isTouch == 1 ? max(gl_PointSize * 1.5, 30.0) : max(gl_PointSize * 1.5, 15.0);
       return;

frontend/javascripts/oxalis/model/accessors/skeletontracing_accessor.ts

@@ -97,11 +97,11 @@ export function getActiveNodeFromTree(skeletonTracing: SkeletonTracing, tree: Tr
 
   return Maybe.Nothing();
 }
-export function findTreeByNodeId(trees: TreeMap, nodeId: number): Maybe<Tree> {
-  return Maybe.fromNullable(_.values(trees).find((tree) => tree.nodes.has(nodeId)));
+export function findTreeByNodeId(trees: TreeMap, nodeId: number): Tree | undefined {
+  return _.values(trees).find((tree) => tree.nodes.has(nodeId));
 }
-export function findTreeByName(trees: TreeMap, treeName: string): Maybe<Tree> {
-  return Maybe.fromNullable(_.values(trees).find((tree: Tree) => tree.name === treeName));
+export function findTreeByName(trees: TreeMap, treeName: string): Tree | undefined {
+  return _.values(trees).find((tree: Tree) => tree.name === treeName);
 }
 export function getTree(
   skeletonTracing: SkeletonTracing,

frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx

@@ -56,7 +56,6 @@ type SetShowSkeletonsAction = ReturnType<typeof setShowSkeletonsAction>;
 type SetMergerModeEnabledAction = ReturnType<typeof setMergerModeEnabledAction>;
 type UpdateNavigationListAction = ReturnType<typeof updateNavigationListAction>;
 export type LoadAgglomerateSkeletonAction = ReturnType<typeof loadAgglomerateSkeletonAction>;
-export type RemoveAgglomerateSkeletonAction = ReturnType<typeof removeAgglomerateSkeletonAction>;
 type NoAction = ReturnType<typeof noAction>;
 
 export type SkeletonTracingAction =
@@ -102,8 +101,7 @@ export type SkeletonTracingAction =
   | SetShowSkeletonsAction
   | SetMergerModeEnabledAction
   | UpdateNavigationListAction
-  | LoadAgglomerateSkeletonAction
-  | RemoveAgglomerateSkeletonAction;
+  | LoadAgglomerateSkeletonAction;
 
 export const SkeletonTracingSaveRelevantActions = [
   "INITIALIZE_SKELETONTRACING",
@@ -528,15 +526,3 @@ export const loadAgglomerateSkeletonAction = (
     mappingName,
     agglomerateId,
   } as const);
-
-export const removeAgglomerateSkeletonAction = (
-  layerName: string,
-  mappingName: string,
-  agglomerateId: number,
-) =>
-  ({
-    type: "REMOVE_AGGLOMERATE_SKELETON",
-    layerName,
-    mappingName,
-    agglomerateId,
-  } as const);

frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts

@@ -53,9 +53,9 @@ function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState
       const activeTreeIdMaybe = activeNodeIdMaybe
         .chain((nodeId) => {
           // use activeNodeId to find active tree
-          const treeIdMaybe = findTreeByNodeId(trees, nodeId).map((tree) => tree.treeId);
+          const treeIdMaybe = findTreeByNodeId(trees, nodeId)?.treeId;
 
-          if (treeIdMaybe.isNothing) {
+          if (treeIdMaybe == null) {
             // There is an activeNodeId without a corresponding tree.
             // Warn the user, since this shouldn't happen, but clear the activeNodeId
             // so that wk is usable.
@@ -68,7 +68,7 @@ function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState
             activeNodeId = null;
           }
 
-          return treeIdMaybe;
+          return Maybe.fromNullable(treeIdMaybe);
         })
         .orElse(() => {
           // use last tree for active tree
@@ -127,25 +127,26 @@ function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState
       switch (action.type) {
         case "SET_ACTIVE_NODE": {
           const { nodeId } = action;
-          return findTreeByNodeId(skeletonTracing.trees, nodeId)
-            .map((tree) =>
-              update(state, {
-                tracing: {
-                  skeleton: {
-                    activeNodeId: {
-                      $set: nodeId,
-                    },
-                    activeTreeId: {
-                      $set: tree.treeId,
-                    },
-                    activeGroupId: {
-                      $set: null,
-                    },
+          const tree = findTreeByNodeId(skeletonTracing.trees, nodeId);
+          if (tree) {
+            return update(state, {
+              tracing: {
+                skeleton: {
+                  activeNodeId: {
+                    $set: nodeId,
+                  },
+                  activeTreeId: {
+                    $set: tree.treeId,
+                  },
+                  activeGroupId: {
+                    $set: null,
                   },
                 },
-              }),
-            )
-            .getOrElse(state);
+              },
+            });
+          } else {
+            return state;
+          }
         }
 
         case "SET_NODE_RADIUS": {

frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts

@@ -215,7 +215,10 @@ export function deleteNode(
 
     const newActiveNodeId = neighborIds.length > 0 ? Math.min(...neighborIds) : null;
     const newActiveTree =
-      newActiveNodeId != null ? findTreeByNodeId(newTrees, newActiveNodeId).get() : activeTree;
+      newActiveNodeId != null ? findTreeByNodeId(newTrees, newActiveNodeId) : activeTree;
+    if (newActiveTree == null) {
+      throw new Error(`Could not find node with id ${newActiveNodeId}`);
+    }
     const newActiveTreeId = newActiveTree.treeId;
     return Maybe.Just([newTrees, newActiveTreeId, newActiveNodeId, newMaxNodeId]);
   });
@@ -227,7 +230,7 @@ export function deleteEdge(
   targetTree: Tree,
   targetNode: Node,
   timestamp: number,
-): Maybe<[TreeMap, number | null]> {
+): Maybe<[TreeMap, number | null | undefined]> {
   return getSkeletonTracing(state.tracing).chain((skeletonTracing) => {
     if (sourceTree.treeId !== targetTree.treeId) {
       // The two selected nodes are in different trees
@@ -257,9 +260,7 @@ export function deleteEdge(
     );
     // The treeId of the tree the active node belongs to could have changed
     const activeNodeId = skeletonTracing.activeNodeId;
-    const newActiveTreeId = activeNodeId
-      ? findTreeByNodeId(newTrees, activeNodeId).get().treeId
-      : null;
+    const newActiveTreeId = activeNodeId ? findTreeByNodeId(newTrees, activeNodeId)?.treeId : null;
     return Maybe.Just([newTrees, newActiveTreeId]);
   });
 }
@@ -647,8 +648,8 @@ export function mergeTrees(
   targetNodeId: number,
 ): Maybe<[TreeMap, number, number]> {
   const { trees } = skeletonTracing;
-  const sourceTree = findTreeByNodeId(trees, sourceNodeId).get();
-  const targetTree = findTreeByNodeId(trees, targetNodeId).get(); // should be activeTree
+  const sourceTree = findTreeByNodeId(trees, sourceNodeId);
+  const targetTree = findTreeByNodeId(trees, targetNodeId); // should be activeTree
 
   if (sourceTree == null || targetTree == null || sourceTree === targetTree) {
     return Maybe.Nothing();

frontend/javascripts/oxalis/model/sagas/proofread_saga.ts

@@ -206,8 +206,8 @@ function* splitOrMergeOrMinCutAgglomerate(
   const skeletonTracing = yield* select((state) => enforceSkeletonTracing(state.tracing));
 
   const { trees } = skeletonTracing;
-  const sourceTree = findTreeByNodeId(trees, sourceNodeId).getOrElse(null);
-  const targetTree = findTreeByNodeId(trees, targetNodeId).getOrElse(null);
+  const sourceTree = findTreeByNodeId(trees, sourceNodeId);
+  const targetTree = findTreeByNodeId(trees, targetNodeId);
 
   if (sourceTree == null || targetTree == null) {
     yield* put(setBusyBlockingInfoAction(false));

frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts

@@ -214,7 +214,6 @@ export function* watchAgglomerateLoading(): Saga<void> {
   yield* take("INITIALIZE_SKELETONTRACING");
   yield* take("WK_READY");
   yield* takeEvery(channel, loadAgglomerateSkeletonWithId);
-  yield* takeEvery("REMOVE_AGGLOMERATE_SKELETON", removeAgglomerateSkeletonWithId);
 }
 export function* watchConnectomeAgglomerateLoading(): Saga<void> {
   // Buffer actions since they might be dispatched before WK_READY
@@ -345,7 +344,7 @@ export function* loadAgglomerateSkeletonWithId(
 
   const treeName = getTreeNameForAgglomerateSkeleton(agglomerateId, mappingName);
   const trees = yield* select((state) => enforceSkeletonTracing(state.tracing).trees);
-  const maybeTree = findTreeByName(trees, treeName).getOrElse(null);
+  const maybeTree = findTreeByName(trees, treeName);
 
   if (maybeTree != null) {
     console.warn(
@@ -425,16 +424,6 @@ function* loadConnectomeAgglomerateSkeletonWithId(
   }
 }
 
-function* removeAgglomerateSkeletonWithId(action: LoadAgglomerateSkeletonAction): Saga<void> {
-  const allowUpdate = yield* select((state) => state.tracing.restrictions.allowUpdate);
-  if (!allowUpdate) return;
-  const { mappingName, agglomerateId } = action;
-  const treeName = getTreeNameForAgglomerateSkeleton(agglomerateId, mappingName);
-  const trees = yield* select((state) => enforceSkeletonTracing(state.tracing).trees);
-  // @ts-expect-error ts-migrate(2552) FIXME: Cannot find name '_all'. Did you mean 'all'?
-  yield _all(findTreeByName(trees, treeName).map((tree) => put(deleteTreeAction(tree.treeId))));
-}
-
 function* removeConnectomeAgglomerateSkeletonWithId(
   action: LoadAgglomerateSkeletonAction,
 ): Saga<void> {
@@ -445,11 +434,10 @@ function* removeConnectomeAgglomerateSkeletonWithId(
   );
   if (skeleton == null) return;
   const { trees } = skeleton;
-  yield* all(
-    findTreeByName(trees, treeName).map((tree) =>
-      put(deleteConnectomeTreesAction([tree.treeId], layerName)),
-    ),
-  );
+  const tree = findTreeByName(trees, treeName);
+  if (tree) {
+    yield* put(deleteConnectomeTreesAction([tree.treeId], layerName));
+  }
 }
 
 export function* watchSkeletonTracingAsync(): Saga<void> {

frontend/javascripts/oxalis/view/context_menu.tsx

@@ -372,7 +372,22 @@ function NodeContextMenuOptions({
 
   const { userBoundingBoxes } = skeletonTracing;
   const { activeTreeId, trees, activeNodeId } = skeletonTracing;
-  const clickedTree = findTreeByNodeId(trees, clickedNodeId).get();
+  const clickedTree = findTreeByNodeId(trees, clickedNodeId);
+
+  if (clickedTree == null) {
+    return (
+      <Menu
+        onClick={hideContextMenu}
+        style={{
+          borderRadius: 6,
+        }}
+        mode="vertical"
+      >
+        <Menu.Item disabled>Error: Could not find clicked node</Menu.Item>
+      </Menu>
+    );
+  }
+
   const areInSameTree = activeTreeId === clickedTree.treeId;
   const isBranchpoint = clickedTree.branchPoints.find((bp) => bp.nodeId === clickedNodeId) != null;
   const isTheSameNode = activeNodeId === clickedNodeId;