From eee4141d572faf054394b99d4d3f7b5904c5ddc4 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Thu, 5 Jan 2023 13:42:58 +0100 Subject: [PATCH] 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 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 * 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 * Update frontend/javascripts/admin/organization/upgrade_plan_modal.tsx Co-authored-by: Philipp Otto * Update conf/messages Co-authored-by: Philipp Otto * Update app/views/mail/upgradePricingPlanUsers.scala.html Co-authored-by: Philipp Otto * Update app/views/mail/upgradePricingPlanToTeam.scala.html Co-authored-by: Philipp Otto * Update app/views/mail/upgradePricingPlanToPower.scala.html Co-authored-by: Philipp Otto * Update app/views/mail/upgradePricingPlanStorage.scala.html Co-authored-by: Philipp Otto * Update app/views/mail/extendPricingPlan.scala.html Co-authored-by: Philipp Otto * 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 * Update frontend/javascripts/admin/organization/organization_cards.tsx Co-authored-by: Norman Rzepka * Update frontend/javascripts/admin/organization/pricing_plan_utils.ts Co-authored-by: Norman Rzepka * Update frontend/javascripts/admin/organization/organization_cards.tsx Co-authored-by: Norman Rzepka * Update frontend/javascripts/admin/organization/pricing_plan_utils.ts Co-authored-by: Norman Rzepka * Update frontend/javascripts/admin/admin_rest_api.ts Co-authored-by: Philipp Otto * 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 * 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 Co-authored-by: Florian M Co-authored-by: Florian M Co-authored-by: Philipp Otto Co-authored-by: Norman Rzepka fixed unreleased changelog fix changelog typo disabled "about to exceed storage space" warning --- CHANGELOG.unreleased.md | 1 + frontend/javascripts/oxalis/api/api_latest.ts | 2 +- frontend/javascripts/oxalis/api/api_v2.ts | 2 +- .../combinations/skeleton_handlers.ts | 4 +- .../geometries/materials/node_shader.ts | 8 ++-- .../accessors/skeletontracing_accessor.ts | 8 ++-- .../model/actions/skeletontracing_actions.tsx | 16 +------- .../model/reducers/skeletontracing_reducer.ts | 41 ++++++++++--------- .../skeletontracing_reducer_helpers.ts | 15 +++---- .../oxalis/model/sagas/proofread_saga.ts | 4 +- .../model/sagas/skeletontracing_saga.ts | 22 +++------- .../javascripts/oxalis/view/context_menu.tsx | 17 +++++++- 12 files changed, 66 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index cb0205f3180..e33c459ed77 100644 --- 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 index 8afe44d5c2c..c5b6382adf9 100644 --- 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 index 9f473e4f5ca..6619be410b0 100644 --- 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 index 4c3bd80a22e..79688b5f1d8 100644 --- 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 index 8433b329f16..36437b1057d 100644 --- 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 index e646d09aec5..dadb2206941 100644 --- 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 { - 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 { - 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 index bd93b559f91..c835d2ec2af 100644 --- 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; type SetMergerModeEnabledAction = ReturnType; type UpdateNavigationListAction = ReturnType; export type LoadAgglomerateSkeletonAction = ReturnType; -export type RemoveAgglomerateSkeletonAction = ReturnType; type NoAction = ReturnType; 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 index 891fc3e156a..104163b03c2 100644 --- 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 index 28ac4994c7b..9bb461f017d 100644 --- 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 index 6ba9c892a67..7df0c12ac3b 100644 --- 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 index e8528ac049e..1edddbd18b7 100644 --- 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 { yield* take("INITIALIZE_SKELETONTRACING"); yield* take("WK_READY"); yield* takeEvery(channel, loadAgglomerateSkeletonWithId); - yield* takeEvery("REMOVE_AGGLOMERATE_SKELETON", removeAgglomerateSkeletonWithId); } export function* watchConnectomeAgglomerateLoading(): Saga { // 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 { - 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 { @@ -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 { diff --git a/frontend/javascripts/oxalis/view/context_menu.tsx b/frontend/javascripts/oxalis/view/context_menu.tsx index 0185c76c7b5..5423d4dc5bf 100644 --- 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 ( + + Error: Could not find clicked node + + ); + } + const areInSameTree = activeTreeId === clickedTree.treeId; const isBranchpoint = clickedTree.branchPoints.find((bp) => bp.nodeId === clickedNodeId) != null; const isTheSameNode = activeNodeId === clickedNodeId;