show error in context menu if clicked node wasn't found (instead of c…

…rashing whole page); remove maybe container from findTreeById (#6724) remove maybe container from findTreeByName; remove dead REMOVE_AGGLOMERATE_SKELETON action code fix incorrect encoding/decoding of node ids between webGL and JS update changelog Update config.yml show error in context menu if clicked node wasn't found (instead of crashing whole page); remove maybe container from findTreeById remove maybe container from findTreeByName; remove dead REMOVE_AGGLOMERATE_SKELETON action code fix incorrect encoding/decoding of node ids between webGL and JS update changelog Update config.yml Fix some failing validations of neuroglancer URLs (#6722) * make neuroglancer import compatible with URLs that encode subsources and improve validation in general * update changelog Add Pricing Plans to Organization Page (#6602) * first draft for new organization page * enable access to orga page in navbar avatar menu * Add pricing plan schema * fix navbar links to orga page * more pricing stuff * orga page refactoring * Use default user/storage values for different plans * Add new model fields * Assert user count does not exceed includedUsers when joining org * Pass incudedStorage in MB * fix deprecation warning * refactor orga view into sub-components * adapt enum in scala * many tweaks to orga view * added modals for extending pricing plans * added actual user count to orga page limits * Insert new fields into organization table * REVIEW fixed some TS errors: @ts-expect-error ts-migrate(2769) FIXME: No overload matches this call. * enforce user quota for email invites * map null value to max int * handle infinite orga storage * small re-phrasing * enforce user limit on email invites * pretty + lint * Update conf/evolutions/091-pricing-plans.sql Co-authored-by: Florian M <[email protected]> * added background images for pricing plan modals * added alert when plans is about to exceed * show a plan expriation warning diectly on the dashboard * fix content for upgrade plan modal * prettier * updated changelog * adapt test db * added backend routes for send out pricing plan uprgade emails * backend formatting * connect frontend and backend prciing email routes * stuff * Update frontend/javascripts/admin/admin_rest_api.ts Co-authored-by: Philipp Otto <[email protected]> * Update frontend/javascripts/admin/organization/upgrade_plan_modal.tsx Co-authored-by: Philipp Otto <[email protected]> * Update conf/messages Co-authored-by: Philipp Otto <[email protected]> * Update app/views/mail/upgradePricingPlanUsers.scala.html Co-authored-by: Philipp Otto <[email protected]> * Update app/views/mail/upgradePricingPlanToTeam.scala.html Co-authored-by: Philipp Otto <[email protected]> * Update app/views/mail/upgradePricingPlanToPower.scala.html Co-authored-by: Philipp Otto <[email protected]> * Update app/views/mail/upgradePricingPlanStorage.scala.html Co-authored-by: Philipp Otto <[email protected]> * Update app/views/mail/extendPricingPlan.scala.html Co-authored-by: Philipp Otto <[email protected]> * applied PR feedback and switch background images to JPEGs * applied PR feedback #2 * PR feedback #3 / fix upgrade modals * Update frontend/javascripts/admin/onboarding.tsx Co-authored-by: Norman Rzepka <[email protected]> * Update frontend/javascripts/admin/organization/organization_cards.tsx Co-authored-by: Norman Rzepka <[email protected]> * Update frontend/javascripts/admin/organization/pricing_plan_utils.ts Co-authored-by: Norman Rzepka <[email protected]> * Update frontend/javascripts/admin/organization/organization_cards.tsx Co-authored-by: Norman Rzepka <[email protected]> * Update frontend/javascripts/admin/organization/pricing_plan_utils.ts Co-authored-by: Norman Rzepka <[email protected]> * Update frontend/javascripts/admin/admin_rest_api.ts Co-authored-by: Philipp Otto <[email protected]> * PR feedback #4 * pretty * PR feedback #5 * refactored pricing upgrade emails to be a confirmation to the user * formatting * fixed warning "plan is about to expire" when it already has expired * fix evolution schema versioning * rephrase all reference to "Free" plan to "Basic" * Add route /pricing/status * redesigned upgrade modal to show both team power plans * integrated pricing plan status API * fix evolutions * also respect paidUntil in exceeded checks * disable pricing plan warnings on dashboard for now * make linter happy * linting & formatting * fix default orga DB * fixed typescript typing errors * Update frontend/javascripts/admin/organization/upgrade_plan_modal.tsx Co-authored-by: Philipp Otto <[email protected]> * applied PR feedback * update schema version to 94 * fix CI? * prevent prcing plan alarms from showing actions for unauthorized people * added owner check for warnings * added confirmation toasts on sucessful upgrade requests * fix toast messages * migration guide Co-authored-by: frcroth <[email protected]> Co-authored-by: Florian M <[email protected]> Co-authored-by: Florian M <[email protected]> Co-authored-by: Philipp Otto <[email protected]> Co-authored-by: Norman Rzepka <[email protected]> fixed unreleased changelog fix changelog typo disabled "about to exceed storage space" warning
scalableminds · Jan 9, 2023 · a5b6576 · a5b6576
1 parent 6bedf79
commit a5b6576
Show file tree

Hide file tree

Showing 12 changed files with 66 additions and 74 deletions.
diff --git a/CHANGELOG.unreleased.md b/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

diff --git a/frontend/javascripts/oxalis/api/api_latest.ts b/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 {

diff --git a/frontend/javascripts/oxalis/api/api_v2.ts b/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 {

diff --git a/frontend/javascripts/oxalis/controller/combinations/skeleton_handlers.ts b/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);

diff --git a/frontend/javascripts/oxalis/geometries/materials/node_shader.ts b/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;

diff --git a/frontend/javascripts/oxalis/model/accessors/skeletontracing_accessor.ts b/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,

diff --git a/frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx b/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);
diff --git a/frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts b/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": {

diff --git a/frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts b/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();

diff --git a/frontend/javascripts/oxalis/model/sagas/proofread_saga.ts b/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));

diff --git a/frontend/javascripts/oxalis/model/sagas/skeletontracing_saga.ts b/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> {

diff --git a/frontend/javascripts/oxalis/view/context_menu.tsx b/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;