Clean up merge modal and allow "live-merge" of explorative and project (

#2935) * remove merge task type and nml from merge modal (fixes #2930) * remove unused function * allow live-merge of explorative annotations * add old backend merging as an option to the merge modal * add tooltip to merge-modal to explain options better * allow live-merge of compound projects * create button-with-checkbox abstraction; show checkbox for project and explorative merge * fix linting * pretty code * improve wording in merge-modal message * chunk save actions into multiple batches and don't send all batches at once to the server * measure save progress via reducers * fix setting busy flag back to false * fix bug in compact function; fix tests * fix linting * refactor * add tests for chunked saving * update changelog * disallow merge for differing datasets * fix redirect in merge-modal * close merge modal after merge * incorporate PR feedback
scalableminds · Jul 30, 2018 · 3a29f15 · 3a29f15
1 parent b66ef25
commit 3a29f15
Show file tree

Hide file tree

Showing 14 changed files with 448 additions and 270 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,8 +12,11 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
 ### Added
 
 - All dates in webknossos will be shown in the browser's timezone. On hover, a tooltip will show the date in UTC. [#2916](https://github.com/scalableminds/webknossos/pull/2916) ![image](https://user-images.githubusercontent.com/2486553/42888385-74c82bc0-8aa8-11e8-9c3e-7cfc90ce93bc.png)
+- When merging datasets within a tracing via the merge-modal, the user can choose whether the merge should be executed directly in the currently opened tracing. Alternatively, a new annotation can be created which is accessible via the dashboard (as it has been before).
+- When a lot of changes need to be persisted to the server (e.g., after importing a large NML), the save button will show a percentage-based progress indicator.
 - Added the possibility to import multiple NML files into the active tracing. This can be done by dragging and dropping the files directly into the tracing view. [#2908](https://github.com/scalableminds/webknossos/pull/2908)            
 - During the import of multiple NML files, the user can select an option to automatically create a group per file so that the imported trees are organized in a hierarchy. [#2908](https://github.com/scalableminds/webknossos/pull/2908)
+
 ### Changed
 
 -
@@ -56,6 +59,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
 - Added the shortcut to copy the currently hovered cell id (CTRL + I) to non-volume-tracings, too. [#2726](https://github.com/scalableminds/webknossos/pull/2726)
 - Added permission for team managers to refresh datasets. [#2688](https://github.com/scalableminds/webknossos/pull/2688)
 - Added backend-unit-test setup and a first test for NML validation. [#2829](https://github.com/scalableminds/webknossos/pull/2829)
+- Added progress indicators to the save button for cases where the saving takes some time (e.g., when importing a large NML). [#2947](https://github.com/scalableminds/webknossos/pull/2947)
 - Added the possibility to not sort comments by name. When clicking the sort button multiple times, sorting is switched to sort by IDs. [#2915](https://github.com/scalableminds/webknossos/pull/2915)
 - Added displayName for organizations. [#2869](https://github.com/scalableminds/webknossos/pull/2869)
 - Added onboarding flow for initial setup of WebKnossos. [#2859](https://github.com/scalableminds/webknossos/pull/2859)
@@ -69,6 +73,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
 - Renamed "Cancel" to "Reset and Cancel" for tasks. [#2910](https://github.com/scalableminds/webknossos/pull/2910)
 - Changed the type of the initial node of new tasks to be a branchpoint (if not created via NML). [#2799](https://github.com/scalableminds/webknossos/pull/2799)
 - The dataset gallery got a redesign with mobile support. [#2761](https://github.com/scalableminds/webknossos/pull/2761)
+- Improved the performance of saving large changes to a tracing (e.g., when importing a large NML). [#2947](https://github.com/scalableminds/webknossos/pull/2947)
 - Improved loading speed of buckets. [#2724](https://github.com/scalableminds/webknossos/pull/2724)
 - Changed the task search, when filtered by user, to show all instead of just active tasks (except for canceled tasks). [#2774](https://github.com/scalableminds/webknossos/pull/2774)
 - Improved the import dialog for datasets. Important fields can now be edited via form inputs instead of having to change the JSON. The JSON is still changeable when enabling an "Advanced" mode. [#2881](https://github.com/scalableminds/webknossos/pull/2881)

diff --git a/app/assets/javascripts/messages.js b/app/assets/javascripts/messages.js
@@ -48,6 +48,8 @@ In order to restore the current window, a reload is necessary.`,
   "tracing.delete_tree_with_initial_node":
     "This tree contains the initial node. Do you really want to delete the whole tree?",
   "tracing.merged": "Merging successfully done",
+  "tracing.merged_with_redirect":
+    "Merging successfully done. You will be redirected to the new annotation.",
   "tracing.tree_viewer_no_cyclic_trees":
     "Cyclic trees are not supported by the abstract tree viewer.",
   "tracing.changed_move_value": "The move value was changed to: ",
@@ -148,6 +150,9 @@ In order to restore the current window, a reload is necessary.`,
     "NML contains <edge ...> with same source and target id: Edge",
   "nml.tree_not_connected": "NML contains tree that is not fully connected: Tree with id",
   "nml.different_dataset": "Imported NML was originally for a different dataset.",
+  "merge.different_dataset":
+    "The merge cannot be executed, because the underlying datasets are not the same.",
+  "merge.volume_unsupported": "Merging is not supported for volume tracings.",
   "users.is_admin":
     "At least one of the selected users is an admin of this organization and already has access to all teams. No team assignments are necessary for this user.",
   "users.grant_admin_rights_title": "Do you really want to grant admin rights?",

diff --git a/app/assets/javascripts/oxalis/model/helpers/generate_dummy_trees.js b/app/assets/javascripts/oxalis/model/helpers/generate_dummy_trees.js
@@ -47,7 +47,7 @@ export default function generateDummyTrees(
       treeId: currentTreeId++,
       nodes,
       edges,
-      color: { r: Math.random(), g: Math.random(), b: Math.random() },
+      color: { r: 0, g: 0, b: 0 },
       branchPoints: [],
       comments: [],
       name: "explorative_2017-10-09_SCM_Boy_023",

diff --git a/app/assets/javascripts/oxalis/model/reducers/save_reducer.js b/app/assets/javascripts/oxalis/model/reducers/save_reducer.js
@@ -18,7 +18,8 @@ function SaveReducer(state: OxalisState, action: ActionType): OxalisState {
       const stats = _.some(action.items, ua => ua.name !== "updateTracing")
         ? Utils.toNullable(getStats(state.tracing))
         : null;
-      if (action.items.length > 0) {
+      const { items } = action;
+      if (items.length > 0) {
         return update(state, {
           save: {
             queue: {
@@ -27,29 +28,55 @@ function SaveReducer(state: OxalisState, action: ActionType): OxalisState {
                   // Placeholder, the version number will be updated before sending to the server
                   version: -1,
                   timestamp: Date.now(),
-                  actions: action.items,
+                  actions: items,
                   stats,
                 },
               ],
             },
+            progressInfo: {
+              totalActionCount: { $apply: count => count + items.length },
+            },
           },
         });
       }
       return state;
     }
 
     case "SHIFT_SAVE_QUEUE": {
-      if (action.count > 0) {
+      const { count } = action;
+      if (count > 0) {
+        const processedQueueActions = _.sumBy(
+          state.save.queue.slice(0, count),
+          batch => batch.actions.length,
+        );
+        const remainingQueue = state.save.queue.slice(count);
+        const resetCounter = remainingQueue.length === 0;
         return update(state, {
-          save: { queue: { $set: state.save.queue.slice(action.count) } },
+          save: {
+            queue: { $set: remainingQueue },
+            progressInfo: {
+              // Reset progress counters if the queue is empty. Otherwise,
+              // increase processedActionCount and leave totalActionCount as is
+              processedActionCount: {
+                $apply: oldCount => (resetCounter ? 0 : oldCount + processedQueueActions),
+              },
+              totalActionCount: { $apply: oldCount => (resetCounter ? 0 : oldCount) },
+            },
+          },
         });
       }
       return state;
     }
 
     case "DISCARD_SAVE_QUEUE": {
       return update(state, {
-        save: { queue: { $set: [] } },
+        save: {
+          queue: { $set: [] },
+          progressInfo: {
+            processedActionCount: { $set: 0 },
+            totalActionCount: { $set: 0 },
+          },
+        },
       });
     }
 

diff --git a/app/assets/javascripts/oxalis/model/reducers/skeletontracing_reducer.js b/app/assets/javascripts/oxalis/model/reducers/skeletontracing_reducer.js
@@ -7,7 +7,6 @@ import _ from "lodash";
 import update from "immutability-helper";
 import Utils from "libs/utils";
 import ColorGenerator from "libs/color_generator";
-import EdgeCollection from "oxalis/model/edge_collection";
 import {
   createBranchPoint,
   deleteBranchPoint,
@@ -23,6 +22,7 @@ import {
   toggleAllTreesReducer,
   toggleTreeGroupReducer,
   addTreesAndGroups,
+  createTreeMapFromTreeArray,
 } from "oxalis/model/reducers/skeletontracing_reducer_helpers";
 import { convertServerBoundingBoxToFrontend } from "oxalis/model/reducers/reducer_helpers";
 import {
@@ -32,40 +32,11 @@ import {
   getNodeAndTree,
 } from "oxalis/model/accessors/skeletontracing_accessor";
 import Constants from "oxalis/constants";
-import type {
-  OxalisState,
-  SkeletonTracingType,
-  NodeType,
-  BranchPointType,
-  TreeType,
-} from "oxalis/store";
-import DiffableMap from "libs/diffable_map";
+import type { OxalisState, SkeletonTracingType } from "oxalis/store";
 import type { ActionType } from "oxalis/model/actions/actions";
-import type { ServerNodeType, ServerBranchPointType } from "admin/api_flow_types";
 import Maybe from "data.maybe";
 import Toast from "libs/toast";
 
-function serverNodeToNode(n: ServerNodeType): NodeType {
-  return {
-    id: n.id,
-    position: Utils.point3ToVector3(n.position),
-    rotation: Utils.point3ToVector3(n.rotation),
-    bitDepth: n.bitDepth,
-    viewport: n.viewport,
-    resolution: n.resolution,
-    radius: n.radius,
-    timestamp: n.createdTimestamp,
-    interpolation: n.interpolation,
-  };
-}
-
-function serverBranchPointToBranchPoint(b: ServerBranchPointType): BranchPointType {
-  return {
-    timestamp: b.createdTimestamp,
-    nodeId: b.nodeId,
-  };
-}
-
 function SkeletonTracingReducer(state: OxalisState, action: ActionType): OxalisState {
   switch (action.type) {
     case "INITIALIZE_SKELETONTRACING": {
@@ -75,26 +46,7 @@ function SkeletonTracingReducer(state: OxalisState, action: ActionType): OxalisS
         action.annotation.settings,
       );
 
-      const trees = _.keyBy(
-        action.tracing.trees.map(
-          (tree): TreeType => ({
-            comments: tree.comments,
-            edges: EdgeCollection.loadFromArray(tree.edges),
-            name: tree.name,
-            treeId: tree.treeId,
-            nodes: new DiffableMap(tree.nodes.map(serverNodeToNode).map(node => [node.id, node])),
-            color:
-              tree.color != null
-                ? [tree.color.r, tree.color.g, tree.color.b]
-                : ColorGenerator.distinctColorForId(tree.treeId),
-            branchPoints: _.map(tree.branchPoints, serverBranchPointToBranchPoint),
-            isVisible: true,
-            timestamp: tree.createdTimestamp,
-            groupId: tree.groupId,
-          }),
-        ),
-        "treeId",
-      );
+      const trees = createTreeMapFromTreeArray(action.tracing.trees);
 
       const activeNodeIdMaybe = Maybe.fromNullable(action.tracing.activeNodeId);
       let cachedMaxNodeId = _.max(_.flatMap(trees, __ => __.nodes.map(node => node.id)));

diff --git a/app/assets/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.js b/app/assets/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.js
@@ -33,6 +33,11 @@ import type {
 } from "oxalis/store";
 import DiffableMap from "libs/diffable_map";
 import EdgeCollection from "oxalis/model/edge_collection";
+import type {
+  ServerSkeletonTracingTreeType,
+  ServerNodeType,
+  ServerBranchPointType,
+} from "admin/api_flow_types";
 
 export function generateTreeName(state: OxalisState, timestamp: number, treeId: number) {
   let user = "";
@@ -703,3 +708,49 @@ export function toggleTreeGroupReducer(
     },
   });
 }
+
+function serverNodeToNode(n: ServerNodeType): NodeType {
+  return {
+    id: n.id,
+    position: Utils.point3ToVector3(n.position),
+    rotation: Utils.point3ToVector3(n.rotation),
+    bitDepth: n.bitDepth,
+    viewport: n.viewport,
+    resolution: n.resolution,
+    radius: n.radius,
+    timestamp: n.createdTimestamp,
+    interpolation: n.interpolation,
+  };
+}
+
+function serverBranchPointToBranchPoint(b: ServerBranchPointType): BranchPointType {
+  return {
+    timestamp: b.createdTimestamp,
+    nodeId: b.nodeId,
+  };
+}
+
+export function createTreeMapFromTreeArray(
+  trees: Array<ServerSkeletonTracingTreeType>,
+): TreeMapType {
+  return _.keyBy(
+    trees.map(
+      (tree): TreeType => ({
+        comments: tree.comments,
+        edges: EdgeCollection.loadFromArray(tree.edges),
+        name: tree.name,
+        treeId: tree.treeId,
+        nodes: new DiffableMap(tree.nodes.map(serverNodeToNode).map(node => [node.id, node])),
+        color:
+          tree.color != null
+            ? [tree.color.r, tree.color.g, tree.color.b]
+            : ColorGenerator.distinctColorForId(tree.treeId),
+        branchPoints: _.map(tree.branchPoints, serverBranchPointToBranchPoint),
+        isVisible: true,
+        timestamp: tree.createdTimestamp,
+        groupId: tree.groupId,
+      }),
+    ),
+    "treeId",
+  );
+}
diff --git a/app/assets/javascripts/oxalis/model/sagas/save_saga.js b/app/assets/javascripts/oxalis/model/sagas/save_saga.js
@@ -37,6 +37,9 @@ const PUSH_THROTTLE_TIME = 30000; // 30s
 const SAVE_RETRY_WAITING_TIME = 5000;
 const UNDO_HISTORY_SIZE = 100;
 
+export const maximumActionCountPerBatch = 5000;
+const maximumActionCountPerSave = 15000;
+
 export function* collectUndoStates(): Generator<*, *, *> {
   const undoStack = [];
   const redoStack = [];
@@ -88,6 +91,7 @@ export function* pushAnnotationAsync(): Generator<*, *, *> {
     // could have been triggered during the call to sendRequestToServer
     saveQueue = yield select(state => state.save.queue);
     if (saveQueue.length === 0) {
+      yield put(setSaveBusyAction(false));
       // Save queue is empty, wait for push event
       yield take("PUSH_SAVE_QUEUE");
     }
@@ -100,7 +104,6 @@ export function* pushAnnotationAsync(): Generator<*, *, *> {
     if (saveQueue.length > 0) {
       yield call(sendRequestToServer);
     }
-    yield put(setSaveBusyAction(false));
   }
 }
 
@@ -111,9 +114,30 @@ export function sendRequestWithToken(
   return doWithToken(token => Request.sendJSONReceiveJSON(`${urlWithoutToken}${token}`, data));
 }
 
+// This function returns the first n batches of the provided array, so that the count of
+// all actions in these n batches does not exceed maximumActionCountPerSave
+function sliceAppropriateBatchCount(batches: Array<SaveQueueEntryType>): Array<SaveQueueEntryType> {
+  const slicedBatches = [];
+  let actionCount = 0;
+
+  for (const batch of batches) {
+    const newActionCount = actionCount + batch.actions.length;
+    if (newActionCount <= maximumActionCountPerSave) {
+      actionCount = newActionCount;
+      slicedBatches.push(batch);
+    } else {
+      break;
+    }
+  }
+
+  return slicedBatches;
+}
+
 export function* sendRequestToServer(timestamp: number = Date.now()): Generator<*, *, *> {
-  const saveQueue = yield select(state => state.save.queue);
-  let compactedSaveQueue = compactUpdateActions(saveQueue);
+  const fullSaveQueue = yield select(state => state.save.queue);
+  const saveQueue = sliceAppropriateBatchCount(fullSaveQueue);
+
+  let compactedSaveQueue = compactSaveQueue(saveQueue);
   const { version, type, tracingId } = yield select(state => state.tracing);
   const dataStoreUrl = yield select(state => state.dataset.dataStore.url);
   compactedSaveQueue = addVersionNumbers(compactedSaveQueue, version);
@@ -307,19 +331,18 @@ function compactDeletedTrees(updateActions: Array<UpdateAction>) {
   );
 }
 
-export function compactUpdateActions(
+export function compactUpdateActions(updateActions: Array<UpdateAction>): Array<UpdateAction> {
+  return compactDeletedTrees(
+    compactMovedNodesAndEdges(removeUnrelevantUpdateActions(updateActions)),
+  );
+}
+
+export function compactSaveQueue(
   updateActionsBatches: Array<SaveQueueEntryType>,
 ): Array<SaveQueueEntryType> {
-  const result = updateActionsBatches
-    .map(updateActionsBatch =>
-      _.chain(updateActionsBatch)
-        .cloneDeep()
-        .update("actions", removeUnrelevantUpdateActions)
-        .update("actions", compactMovedNodesAndEdges)
-        .update("actions", compactDeletedTrees)
-        .value(),
-    )
-    .filter(updateActionsBatch => updateActionsBatch.actions.length > 0);
+  const result = updateActionsBatches.filter(
+    updateActionsBatch => updateActionsBatch.actions.length > 0,
+  );
 
   // This part of the code removes all entries from the save queue that consist only of
   // an updateTracing update action, except for the last one
@@ -371,9 +394,15 @@ export function* saveTracingAsync(): Generator<any, any, any> {
     }
     const tracing = yield select(state => state.tracing);
     const flycam = yield select(state => state.flycam);
-    const items = Array.from(yield call(performDiffTracing, prevTracing, tracing, flycam));
+    const items = compactUpdateActions(
+      Array.from(yield call(performDiffTracing, prevTracing, tracing, flycam)),
+    );
     if (items.length > 0) {
-      yield put(pushSaveQueueAction(items));
+      const updateActionChunks = _.chunk(items, maximumActionCountPerBatch);
+
+      for (const updateActionChunk of updateActionChunks) {
+        yield put(pushSaveQueueAction(updateActionChunk));
+      }
     }
     prevTracing = tracing;
   }