Avoid proofreading actions while the last one is not finished (#6325)

* show frontend as busy during proofreading action, missing: disable context menu if frontend is busy * remove unused import * disable inputs for input catchers if wk is busy * replace dropdown placement bottomCenter by bottom according to deprecation warning * update changelog * upgrade antd to 4.18.9, remove unused type ignores * Merge branch 'master' of github.com:scalableminds/webknossos into busy-proofreading * revert change to yarn.lock and fix typing * Merge branch 'master' of github.com:scalableminds/webknossos into busy-proofreading * save available mappings in store * Merge branch 'master' into busy-proofreading * Merge branch 'master' into busy-proofreading * Merge branch 'master' into busy-proofreading * Merge branch 'master' into busy-proofreading
scalableminds · Jul 26, 2022 · 0139cbd · 0139cbd
1 parent 2594c97
commit 0139cbd
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 11 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -19,6 +19,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 
 ### Fixed
 - Fixed a regression where the mapping activation confirmation dialog was never shown. [#6346](https://github.com/scalableminds/webknossos/pull/6346)
+- Fixed an error if multiple proofreading actions were performed in rapid succession. If webKnossos is busy, inputs to the viewports are disabled from now on. [#6325](https://github.com/scalableminds/webknossos/pull/6325)
 
 ### Removed
 - Annotation Type was removed from the info tab. [#6330](https://github.com/scalableminds/webknossos/pull/6330)

diff --git a/frontend/javascripts/oxalis/model/sagas/mapping_saga.ts b/frontend/javascripts/oxalis/model/sagas/mapping_saga.ts
@@ -14,6 +14,7 @@ import {
   getAgglomeratesForDatasetLayer,
 } from "admin/admin_rest_api";
 import type { APIMapping } from "types/api_flow_types";
+import { setLayerMappingsAction } from "oxalis/model/actions/dataset_actions";
 import { getLayerByName, getMappingInfo } from "oxalis/model/accessors/dataset_accessor";
 import type { ActiveMappingInfo, Mapping } from "oxalis/store";
 import ErrorHandling from "libs/error_handling";
@@ -114,6 +115,12 @@ function* maybeFetchMapping(
     // @ts-expect-error ts-migrate(2769) FIXME: No overload matches this call.
     call(getAgglomeratesForDatasetLayer, ...params),
   ]);
+  // Make sure the available mappings are persisted in the store if they are not already
+  const areServerHdf5MappingsInStore =
+    "agglomerates" in layerInfo && layerInfo.agglomerates != null;
+  if (!areServerHdf5MappingsInStore) {
+    yield* put(setLayerMappingsAction(layerName, jsonMappings, serverHdf5Mappings));
+  }
   const editableMappings = yield* select((state) =>
     state.tracing.volumes
       .filter((volumeTracing) => volumeTracing.mappingIsEditable)

diff --git a/frontend/javascripts/oxalis/model/sagas/proofread_saga.ts b/frontend/javascripts/oxalis/model/sagas/proofread_saga.ts
@@ -23,7 +23,6 @@ import {
 import {
   pushSaveQueueTransaction,
   setVersionNumberAction,
-  undoAction,
 } from "oxalis/model/actions/save_actions";
 import { splitAgglomerate, mergeAgglomerate } from "oxalis/model/sagas/update_actions";
 import Model from "oxalis/model";
@@ -47,6 +46,7 @@ import { loadAgglomerateSkeletonWithId } from "oxalis/model/sagas/skeletontracin
 import { getConstructorForElementClass } from "oxalis/model/bucket_data_handling/bucket";
 import { Tree } from "oxalis/store";
 import { APISegmentationLayer } from "types/api_flow_types";
+import { setBusyBlockingInfoAction } from "oxalis/model/actions/ui_actions";
 
 export default function* proofreadMapping(): Saga<any> {
   yield* take("INITIALIZE_SKELETONTRACING");
@@ -279,13 +279,20 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
   const isHdf5MappingEnabled = yield* call(ensureHdf5MappingIsEnabled, layerName);
   if (!isHdf5MappingEnabled) return;
 
+  const busyBlockingInfo = yield* select((state) => state.uiInformation.busyBlockingInfo);
+  if (busyBlockingInfo.isBusy) {
+    console.warn(`Ignoring proofreading action (reason: ${busyBlockingInfo.reason || "null"})`);
+    return;
+  }
+
+  yield* put(setBusyBlockingInfoAction(true, "Proofreading action"));
+
   if (!volumeTracing.mappingIsEditable) {
     try {
       yield* call(createEditableMapping);
     } catch (e) {
       console.error(e);
-      // Undo the last splitting/merging action since the proofreading action did not succeed
-      yield* put(undoAction());
+      yield* put(setBusyBlockingInfoAction(false));
       return;
     }
   }
@@ -305,6 +312,7 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
   const targetTree = findTreeByNodeId(trees, targetNodeId).getOrElse(null);
 
   if (sourceTree == null || targetTree == null) {
+    yield* put(setBusyBlockingInfoAction(false));
     return;
   }
 
@@ -330,6 +338,7 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
   if (action.type === "MERGE_TREES") {
     if (sourceNodeAgglomerateId === targetNodeAgglomerateId) {
       Toast.error("Segments that should be merged need to be in different agglomerates.");
+      yield* put(setBusyBlockingInfoAction(false));
       return;
     }
     items.push(
@@ -344,6 +353,7 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
   } else if (action.type === "DELETE_EDGE") {
     if (sourceNodeAgglomerateId !== targetNodeAgglomerateId) {
       Toast.error("Segments that should be split need to be in the same agglomerate.");
+      yield* put(setBusyBlockingInfoAction(false));
       return;
     }
     items.push(
@@ -356,7 +366,10 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
     );
   }
 
-  if (items.length === 0) return;
+  if (items.length === 0) {
+    yield* put(setBusyBlockingInfoAction(false));
+    return;
+  }
 
   yield* put(pushSaveQueueTransaction(items, "mapping", volumeTracingId));
   yield* call([Model, Model.ensureSavedState]);
@@ -372,6 +385,7 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
     volumeTracingWithEditableMapping == null ||
     volumeTracingWithEditableMapping.mappingName == null
   ) {
+    yield* put(setBusyBlockingInfoAction(false));
     return;
   }
 
@@ -412,6 +426,8 @@ function* splitOrMergeAgglomerate(action: MergeTreesAction | DeleteEdgeAction) {
     );
   }
 
+  yield* put(setBusyBlockingInfoAction(false));
+
   /* Remove old meshes and load updated meshes */
 
   if (proofreadUsingMeshes()) {

diff --git a/frontend/javascripts/oxalis/view/input_catcher.tsx b/frontend/javascripts/oxalis/view/input_catcher.tsx
@@ -134,18 +134,23 @@ function InputCatcher({
   );
 
   return (
-    <div className="flexlayout-dont-overflow">
+    <div
+      className="flexlayout-dont-overflow"
+      onContextMenu={ignoreContextMenu}
+      style={{ cursor: busyBlockingInfo.isBusy ? "wait" : cursorForTool[adaptedTool] }}
+    >
       <div
         id={`inputcatcher_${viewportID}`}
         ref={(domElement) => {
           domElementRef.current = domElement;
         }}
-        onContextMenu={ignoreContextMenu}
         data-value={viewportID}
         className={`inputcatcher ${viewportID}`}
         style={{
           position: "relative",
-          cursor: busyBlockingInfo.isBusy ? "wait" : cursorForTool[adaptedTool],
+          // Disable inputs while wk is busy. However, keep the custom cursor and the ignoreContextMenu handler
+          // which is why those are defined at the outer element.
+          pointerEvents: busyBlockingInfo.isBusy ? "none" : "auto",
         }}
       >
         <ViewportStatusIndicator />

diff --git a/frontend/javascripts/oxalis/view/right-border-tabs/connectome_tab/synapse_tree.tsx b/frontend/javascripts/oxalis/view/right-border-tabs/connectome_tab/synapse_tree.tsx
@@ -234,7 +234,7 @@ class SynapseTree extends React.Component<Props, State> {
           <Dropdown // Lazily create the dropdown menu and destroy it again, afterwards
             overlay={() => this.createSegmentDropdownMenu(data.id)}
             autoDestroy
-            placement="bottomCenter"
+            placement="bottom"
             visible={this.state.activeSegmentDropdownKey === key}
             onVisibleChange={(isVisible) =>
               this.handleSegmentDropdownMenuVisibility(key, isVisible)

diff --git a/frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_list_item.tsx b/frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_list_item.tsx
@@ -384,7 +384,7 @@ function _SegmentListItem({
         // does not work properly. See https://github.com/react-component/trigger/issues/106#issuecomment-948532990
         // @ts-expect-error ts-migrate(2322) FIXME: Type '{ children: Element; overlay: () => Element;... Remove this comment to see the full error message
         autoDestroy
-        placement="bottomCenter"
+        placement="bottom"
         visible={activeDropdownSegmentId === segment.id}
         onVisibleChange={(isVisible) => handleSegmentDropdownMenuVisibility(segment.id, isVisible)}
         trigger={["contextMenu"]}

diff --git a/frontend/javascripts/oxalis/view/right-border-tabs/tree_hierarchy_view.tsx b/frontend/javascripts/oxalis/view/right-border-tabs/tree_hierarchy_view.tsx
@@ -393,7 +393,7 @@ class TreeHierarchyView extends React.PureComponent<Props, State> {
       <div>
         <Dropdown
           overlay={createMenu}
-          placement="bottomCenter" // The overlay is generated lazily. By default, this would make the overlay
+          placement="bottom" // The overlay is generated lazily. By default, this would make the overlay
           // re-render on each parent's render() after it was shown for the first time.
           // The reason for this is that it's not destroyed after closing.
           // Therefore, autoDestroy is passed.
@@ -533,7 +533,7 @@ class TreeHierarchyView extends React.PureComponent<Props, State> {
             // does not work properly. See https://github.com/react-component/trigger/issues/106#issuecomment-948532990
             // @ts-expect-error ts-migrate(2322) FIXME: Type '{ children: Element; overlay: () => Element;... Remove this comment to see the full error message
             autoDestroy
-            placement="bottomCenter"
+            placement="bottom"
             visible={this.state.activeTreeDropdownId === tree.treeId} // explicit visibility handling is required here otherwise the color picker component for "Change Tree color" is rendered/positioned incorrectly
             onVisibleChange={(isVisible) =>
               this.handleTreeDropdownMenuVisibility(tree.treeId, isVisible)