From 3a29f15e59804df7976892695e7abb98b75ae7e8 Mon Sep 17 00:00:00 2001 From: Philipp Otto Date: Mon, 30 Jul 2018 13:08:36 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 5 + app/assets/javascripts/messages.js | 5 + .../model/helpers/generate_dummy_trees.js | 2 +- .../oxalis/model/reducers/save_reducer.js | 37 ++- .../model/reducers/skeletontracing_reducer.js | 54 +--- .../skeletontracing_reducer_helpers.js | 51 +++ .../oxalis/model/sagas/save_saga.js | 61 +++- app/assets/javascripts/oxalis/store.js | 10 + .../view/action-bar/merge_modal_view.js | 299 +++++++++--------- .../oxalis/view/action-bar/save_button.js | 40 ++- .../test/reducers/save_reducer.spec.js | 4 + .../test/sagas/saga_integration.spec.js | 99 ++++-- .../javascripts/test/sagas/save_saga.spec.js | 19 +- .../test/sagas/skeletontracing_saga.spec.js | 32 +- 14 files changed, 448 insertions(+), 270 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61ab0ff19cc..7ba582ce9b9 100644 --- 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 index 87a8279eb14..0a41530747b 100644 --- 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 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 index c838d514b15..503e085b90d 100644 --- 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 index 80a6cbc5f3b..2d3c5749afa 100644 --- 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,11 +28,14 @@ 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 }, + }, }, }); } @@ -39,9 +43,26 @@ function SaveReducer(state: OxalisState, action: ActionType): OxalisState { } 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; @@ -49,7 +70,13 @@ function SaveReducer(state: OxalisState, action: ActionType): OxalisState { 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 index 9ff22536ba8..2d0405fe5b2 100644 --- 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 index fcb04b7fa15..147315cd572 100644 --- 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, +): 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 index 9146398571f..f2d2d53031a 100644 --- 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): Array { + 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) { ); } -export function compactUpdateActions( +export function compactUpdateActions(updateActions: Array): Array { + return compactDeletedTrees( + compactMovedNodesAndEdges(removeUnrelevantUpdateActions(updateActions)), + ); +} + +export function compactSaveQueue( updateActionsBatches: Array, ): Array { - 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 { } 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; } diff --git a/app/assets/javascripts/oxalis/store.js b/app/assets/javascripts/oxalis/store.js index 33ce1c52935..be52b7fc9b4 100644 --- a/app/assets/javascripts/oxalis/store.js +++ b/app/assets/javascripts/oxalis/store.js @@ -244,10 +244,16 @@ export type SaveQueueEntryType = { actions: Array, }; +export type ProgressInfoType = { + +processedActionCount: number, + +totalActionCount: number, +}; + export type SaveStateType = { +isBusy: boolean, +queue: Array, +lastSaveTimestamp: number, + +progressInfo: ProgressInfoType, }; export type FlycamType = { @@ -410,6 +416,10 @@ export const defaultState: OxalisState = { queue: [], isBusy: false, lastSaveTimestamp: 0, + progressInfo: { + processedActionCount: 0, + totalActionCount: 0, + }, }, flycam: { zoomStep: 1.3, diff --git a/app/assets/javascripts/oxalis/view/action-bar/merge_modal_view.js b/app/assets/javascripts/oxalis/view/action-bar/merge_modal_view.js index c35104f2fc7..a654d951d08 100644 --- a/app/assets/javascripts/oxalis/view/action-bar/merge_modal_view.js +++ b/app/assets/javascripts/oxalis/view/action-bar/merge_modal_view.js @@ -1,25 +1,20 @@ // @flow import React, { PureComponent } from "react"; import { connect } from "react-redux"; -import { withRouter } from "react-router-dom"; import Toast from "libs/toast"; import Request from "libs/request"; -import { Alert, Modal, Button, Upload, Select, Form, Spin } from "antd"; +import { Icon, Alert, Modal, Button, Select, Form, Spin, Checkbox, Tooltip } from "antd"; import messages from "messages"; import InputComponent from "oxalis/view/components/input_component"; import api from "oxalis/api/internal_api"; -import type { OxalisState } from "oxalis/store"; -import type { RouterHistory } from "react-router-dom"; - -type AnnotationInfoType = { - typ: string, - id: string, -}; - -type TaskTypeInfoType = { - id: string, - label: string, -}; +import type { OxalisState, TreeMapType, TreeGroupType } from "oxalis/store"; +import { getAnnotationInformation, getTracingForAnnotation } from "admin/admin_rest_api"; +import { addTreesAndGroupsAction } from "oxalis/model/actions/skeletontracing_actions"; +import { createTreeMapFromTreeArray } from "oxalis/model/reducers/skeletontracing_reducer_helpers"; +import Utils from "libs/utils"; +import type { APIAnnotationType } from "admin/api_flow_types"; +import Store from "oxalis/store"; +import { location } from "libs/window"; type ProjectInfoType = { id: string, @@ -30,52 +25,67 @@ type StateProps = { annotationId: string, tracingType: string, }; + type Props = { isVisible: boolean, onOk: () => void, - history: RouterHistory, + addTreesAndGroupsAction: (TreeMapType, Array) => void, } & StateProps; type MergeModalViewState = { - taskTypes: Array, projects: Array, - selectedTaskType: ?string, selectedProject: ?string, selectedExplorativeAnnotation: string, - selectedNML: ?AnnotationInfoType, isUploading: boolean, }; -type UploadInfoType = { - file: - | { - status: "uploading", - } - | { - status: "done" | "error", - response: T, - }, +type ButtonWithCheckboxProps = { + checkboxContent: React$Element<*>, + button: React$Element<*>, + onButtonClick: (SyntheticInputEvent<>, boolean) => Promise | void, }; +type ButtonWithCheckboxState = { + isChecked: boolean, +}; + +class ButtonWithCheckbox extends PureComponent { + state = { + isChecked: true, + }; + render() { + return ( + + + this.setState({ isChecked: event.target.checked })} + checked={this.state.isChecked} + > + {this.props.checkboxContent} + + + + {React.cloneElement(this.props.button, { + onClick: evt => this.props.onButtonClick(evt, this.state.isChecked), + })} + + + ); + } +} + class MergeModalView extends PureComponent { - state: MergeModalViewState = { - taskTypes: [], + state = { projects: [], - selectedTaskType: null, selectedProject: null, selectedExplorativeAnnotation: "", - selectedNML: null, isUploading: false, }; componentWillMount() { (async () => { - const [taskTypes, projects] = await Promise.all([ - Request.receiveJSON("/api/taskTypes", { doNotCatch: true }), - Request.receiveJSON("/api/projects", { doNotCatch: true }), - ]); + const projects = await Request.receiveJSON("/api/projects", { doNotCatch: true }); this.setState({ - taskTypes: taskTypes.map(taskType => ({ id: taskType.id, label: taskType.summary })), projects: projects.map(project => ({ id: project.id, label: project.name })), }); })(); @@ -84,15 +94,12 @@ class MergeModalView extends PureComponent { async merge(url: string) { await api.tracing.save(); const annotation = await Request.receiveJSON(url); - Toast.success(messages["tracing.merged"]); + Toast.success(messages["tracing.merged_with_redirect"]); const redirectUrl = `/annotations/${annotation.typ}/${annotation.id}`; - this.props.history.push(redirectUrl); + await Utils.sleep(1500); + location.href = redirectUrl; } - handleChangeMergeTaskType = (taskType: string) => { - this.setState({ selectedTaskType: taskType }); - }; - handleChangeMergeProject = (project: string) => { this.setState({ selectedProject: project }); }; @@ -101,111 +108,96 @@ class MergeModalView extends PureComponent { this.setState({ selectedExplorativeAnnotation: event.target.value }); }; - handleChangeNML = ( - info: UploadInfoType<{ annotation: AnnotationInfoType, messages: Array }>, - ) => { - if (info.file.status === "done") { - const { annotation } = info.file.response; - Toast.messages(info.file.response.messages); - this.setState({ isUploading: false }); - const url = - `/api/annotations/${annotation.typ}/${annotation.id}/merge/` + - `${this.props.tracingType}/${this.props.annotationId}`; - this.merge(url); - } else if (info.file.status === "error") { - Toast.messages(info.file.response.messages); - this.setState({ isUploading: false }); - } - }; - handleBeforeUploadNML = () => { this.setState({ isUploading: true }); }; - handleMergeTaskType = (event: SyntheticInputEvent<>) => { - event.preventDefault(); - const { selectedTaskType } = this.state; - if (selectedTaskType != null) { - const url = - `/api/annotations/CompoundTaskType/${selectedTaskType}/` + - `merge/${this.props.tracingType}/${this.props.annotationId}`; - this.merge(url); - } - }; - - handleMergeProject = (event: SyntheticInputEvent<>) => { + handleMergeProject = async (event: SyntheticInputEvent<>, isLocalMerge: boolean) => { event.preventDefault(); const { selectedProject } = this.state; if (selectedProject != null) { - const url = - `/api/annotations/CompoundProject/${selectedProject}/merge/` + - `${this.props.tracingType}/${this.props.annotationId}`; - this.merge(url); + if (isLocalMerge) { + const annotation = await getAnnotationInformation(selectedProject, "CompoundProject"); + this.mergeAnnotationIntoActiveTracing(annotation); + } else { + const url = + `/api/annotations/CompoundProject/${selectedProject}/merge/` + + `${this.props.tracingType}/${this.props.annotationId}`; + this.merge(url); + } } }; - handleMergeExplorativeAnnotation = async (event: SyntheticInputEvent<>) => { + handleMergeExplorativeAnnotation = async ( + event: SyntheticInputEvent<>, + isLocalMerge: boolean, + ) => { event.preventDefault(); const { selectedExplorativeAnnotation } = this.state; if (selectedExplorativeAnnotation != null) { - const url = - `/api/annotations/Explorational/${selectedExplorativeAnnotation}/merge/` + - `${this.props.tracingType}/${this.props.annotationId}`; - this.merge(url); + if (isLocalMerge) { + const annotation = await getAnnotationInformation( + selectedExplorativeAnnotation, + "Explorational", + ); + this.mergeAnnotationIntoActiveTracing(annotation); + } else { + const url = + `/api/annotations/Explorational/${selectedExplorativeAnnotation}/merge/` + + `${this.props.tracingType}/${this.props.annotationId}`; + this.merge(url); + } } }; + async mergeAnnotationIntoActiveTracing(annotation: APIAnnotationType): Promise { + if (annotation.dataSetName !== Store.getState().dataset.name) { + Toast.error(messages["merge.different_dataset"]); + return; + } + const tracing = await getTracingForAnnotation(annotation); + if (!tracing.trees) { + Toast.error(messages["merge.volume_unsupported"]); + return; + } + const { trees, treeGroups } = tracing; + this.setState({ isUploading: true }); + // Wait for an animation frame so that the loading animation is kicked off + await Utils.animationFrame(); + this.props.addTreesAndGroupsAction(createTreeMapFromTreeArray(trees), treeGroups || []); + this.setState({ isUploading: false }); + Toast.success(messages["tracing.merged"]); + this.props.onOk(); + } + render() { + const mergeIntoActiveTracingCheckbox = ( + + Merge into active tracing{" "} + + + + + ); + return ( - The merged tracing will be saved as a new explorative tracing in your - account. If you wish to import NML files right into the current tracing, just drag - and drop them into the tracing view. - - } - type="warning" + type="info" style={{ marginBottom: 12 }} + message="If you would like to import NML files, please drag and drop them into the tracing view." /> -
- - - - - - -
- -
+ - - - + + + Merge + + } + onButtonClick={this.handleMergeProject} + /> -
+ { onChange={this.handleChangeMergeExplorativeAnnotation} /> - - - -
- -
- - - - - + } + onButtonClick={this.handleMergeExplorativeAnnotation} + />
@@ -282,4 +258,13 @@ function mapStateToProps(state: OxalisState): StateProps { }; } -export default connect(mapStateToProps)(withRouter(MergeModalView)); +const mapDispatchToProps = (dispatch: Dispatch<*>) => ({ + addTreesAndGroupsAction(trees: TreeMapType, treeGroups: Array) { + dispatch(addTreesAndGroupsAction(trees, treeGroups)); + }, +}); + +export default connect( + mapStateToProps, + mapDispatchToProps, +)(MergeModalView); diff --git a/app/assets/javascripts/oxalis/view/action-bar/save_button.js b/app/assets/javascripts/oxalis/view/action-bar/save_button.js index 6dc17608163..3f3c63a1204 100644 --- a/app/assets/javascripts/oxalis/view/action-bar/save_button.js +++ b/app/assets/javascripts/oxalis/view/action-bar/save_button.js @@ -1,9 +1,17 @@ // @flow import React from "react"; +import { connect } from "react-redux"; import Model from "oxalis/model"; import ButtonComponent from "oxalis/view/components/button_component"; +import type { OxalisState, ProgressInfoType } from "oxalis/store"; + +type StateProps = {| + progressInfo: ProgressInfoType, + isBusy: boolean, +|}; type Props = { + ...StateProps, onClick: (SyntheticInputEvent) => Promise<*>, }; @@ -29,18 +37,29 @@ class SaveButton extends React.PureComponent { savedPollingInterval: number = 0; _forceUpdate = () => { - this.setState({ isStateSaved: Model.stateSaved() }); + const isStateSaved = Model.stateSaved(); + this.setState({ + isStateSaved, + }); }; getSaveButtonIcon() { if (this.state.isStateSaved) { return "check"; + } else if (this.props.isBusy) { + return "loading"; } else { return "hourglass"; } } + shouldShowProgress(): boolean { + // For a low action count, the progress info would show only for a very short amount of time + return this.props.isBusy && this.props.progressInfo.totalActionCount > 5000; + } + render() { + const { progressInfo } = this.props; return ( { onClick={this.props.onClick} icon={this.getSaveButtonIcon()} > - Save + {this.shouldShowProgress() ? ( + + {Math.floor((progressInfo.processedActionCount / progressInfo.totalActionCount) * 100)}{" "} + % + + ) : ( + Save + )} ); } } -export default SaveButton; +function mapStateToProps(state: OxalisState): StateProps { + const { progressInfo, isBusy } = state.save; + return { + progressInfo, + isBusy, + }; +} + +export default connect(mapStateToProps)(SaveButton); diff --git a/app/assets/javascripts/test/reducers/save_reducer.spec.js b/app/assets/javascripts/test/reducers/save_reducer.spec.js index 312ebcc7a3b..36d75d894ca 100644 --- a/app/assets/javascripts/test/reducers/save_reducer.spec.js +++ b/app/assets/javascripts/test/reducers/save_reducer.spec.js @@ -32,6 +32,10 @@ const initialState = { isBusy: false, queue: [], lastSaveTimestamp: 0, + progressInfo: { + processedActionCount: 0, + totalActionCount: 0, + }, }, }; diff --git a/app/assets/javascripts/test/sagas/saga_integration.spec.js b/app/assets/javascripts/test/sagas/saga_integration.spec.js index 5fb0c087085..68244d40d7f 100644 --- a/app/assets/javascripts/test/sagas/saga_integration.spec.js +++ b/app/assets/javascripts/test/sagas/saga_integration.spec.js @@ -3,40 +3,101 @@ import mockRequire from "mock-require"; import { setupOxalis, TIMESTAMP } from "test/helpers/apiHelpers"; import update from "immutability-helper"; import Store from "oxalis/store"; -import { wkReadyAction } from "oxalis/model/actions/actions"; +import { restartSagaAction, wkReadyAction } from "oxalis/model/actions/actions"; import { createSaveQueueFromUpdateActions } from "test/helpers/saveHelpers"; import { generateTreeName } from "oxalis/model/reducers/skeletontracing_reducer_helpers"; import { getStats } from "oxalis/model/accessors/skeletontracing_accessor"; import Utils from "libs/utils"; +import generateDummyTrees from "oxalis/model/helpers/generate_dummy_trees"; +import { maximumActionCountPerBatch } from "oxalis/model/sagas/save_saga"; const UpdateActions = mockRequire.reRequire("oxalis/model/sagas/update_actions"); +const { addTreesAndGroupsAction, deleteNodeAction } = mockRequire.reRequire( + "oxalis/model/actions/skeletontracing_actions", +); +const { createTreeMapFromTreeArray } = mockRequire.reRequire( + "oxalis/model/reducers/skeletontracing_reducer_helpers", +); +const { discardSaveQueueAction } = mockRequire.reRequire("oxalis/model/actions/save_actions"); test.beforeEach(async t => { // Setup oxalis, this will execute model.fetch(...) and initialize the store with the tracing, etc. + Store.dispatch(restartSagaAction()); + Store.dispatch(discardSaveQueueAction()); await setupOxalis(t, "task"); + // Dispatch the wkReadyAction, so the sagas are started Store.dispatch(wkReadyAction()); }); -test("watchTreeNames saga should rename empty trees in tasks and these updates should be persisted", t => { - const state = Store.getState(); - const treeWithEmptyName = state.tracing.trees[1]; - const treeWithCorrectName = update(treeWithEmptyName, { - name: { $set: generateTreeName(state, treeWithEmptyName.timestamp, treeWithEmptyName.treeId) }, - }); +test.serial( + "watchTreeNames saga should rename empty trees in tasks and these updates should be persisted", + t => { + const state = Store.getState(); + const treeWithEmptyName = state.tracing.trees[1]; + const treeWithCorrectName = update(treeWithEmptyName, { + name: { + $set: generateTreeName(state, treeWithEmptyName.timestamp, treeWithEmptyName.treeId), + }, + }); - const expectedSaveQueue = createSaveQueueFromUpdateActions( - [ + const expectedSaveQueue = createSaveQueueFromUpdateActions( [ - UpdateActions.updateTree(treeWithCorrectName), - UpdateActions.updateSkeletonTracing(Store.getState().tracing, [1, 2, 3], [0, 0, 0], 2), + [ + UpdateActions.updateTree(treeWithCorrectName), + UpdateActions.updateSkeletonTracing(Store.getState().tracing, [1, 2, 3], [0, 0, 0], 2), + ], ], - ], - TIMESTAMP, - Utils.toNullable(getStats(state.tracing)), - ); - - // Once the updateTree update action is in the save queue, we're good. - // This means the setTreeName action was dispatched, the diffing ran, and the change will be persisted. - t.deepEqual(expectedSaveQueue, state.save.queue); + TIMESTAMP, + Utils.toNullable(getStats(state.tracing)), + ); + + // Once the updateTree update action is in the save queue, we're good. + // This means the setTreeName action was dispatched, the diffing ran, and the change will be persisted. + t.deepEqual(expectedSaveQueue, state.save.queue); + }, +); + +test.serial("Save actions should not be chunked below the chunk limit (1/3)", t => { + Store.dispatch(discardSaveQueueAction()); + t.deepEqual(Store.getState().save.queue, []); + + const trees = generateDummyTrees(1000, 1); + Store.dispatch(addTreesAndGroupsAction(createTreeMapFromTreeArray(trees), [])); + t.is(Store.getState().save.queue.length, 1); + t.true(Store.getState().save.queue[0].actions.length < maximumActionCountPerBatch); +}); + +test.serial("Save actions should be chunked above the chunk limit (2/3)", t => { + Store.dispatch(discardSaveQueueAction()); + t.deepEqual(Store.getState().save.queue, []); + + const trees = generateDummyTrees(5000, 1); + Store.dispatch(addTreesAndGroupsAction(createTreeMapFromTreeArray(trees), [])); + + const state = Store.getState(); + + t.true(state.save.queue.length > 1); + t.is(state.save.queue[0].actions.length, maximumActionCountPerBatch); +}); + +test.serial("Save actions should be chunked after compacting (3/3)", t => { + const nodeCount = 20000; + // Test that a tree split is detected even when the involved node count is above the chunk limit + const trees = generateDummyTrees(1, nodeCount); + Store.dispatch(addTreesAndGroupsAction(createTreeMapFromTreeArray(trees), [])); + + Store.dispatch(discardSaveQueueAction()); + t.deepEqual(Store.getState().save.queue, []); + + // Delete node in the middle + const middleNodeId = trees[0].nodes[nodeCount / 2].id; + Store.dispatch(deleteNodeAction(middleNodeId)); + const saveQueue = Store.getState().save.queue; + + // There should only be one chunk + t.is(saveQueue.length, 1); + t.true(saveQueue[0].actions.length < maximumActionCountPerBatch); + t.is(saveQueue[0].actions[1].name, "moveTreeComponent"); + t.true(saveQueue[0].actions[1].value.nodeIds.length >= nodeCount / 2); }); diff --git a/app/assets/javascripts/test/sagas/save_saga.spec.js b/app/assets/javascripts/test/sagas/save_saga.spec.js index f9f0a7d3c97..30e97a593ed 100644 --- a/app/assets/javascripts/test/sagas/save_saga.spec.js +++ b/app/assets/javascripts/test/sagas/save_saga.spec.js @@ -5,8 +5,9 @@ import test from "ava"; import mockRequire from "mock-require"; import DiffableMap from "libs/diffable_map"; import { alert } from "libs/window"; -import { expectValueDeepEqual } from "../helpers/sagaHelpers"; +import { setSaveBusyAction } from "oxalis/model/actions/save_actions"; import { createSaveQueueFromUpdateActions } from "../helpers/saveHelpers"; +import { expectValueDeepEqual } from "../helpers/sagaHelpers"; mockRequire.stopAll(); @@ -25,7 +26,7 @@ const SaveActions = mockRequire.reRequire("oxalis/model/actions/save_actions"); const { take, call, put } = mockRequire.reRequire("redux-saga/effects"); const { - compactUpdateActions, + compactSaveQueue, pushAnnotationAsync, sendRequestToServer, toggleErrorHighlighting, @@ -83,7 +84,7 @@ test("SaveSaga should compact multiple updateTracing update actions", t => { TIMESTAMP, ); - t.deepEqual(compactUpdateActions(saveQueue), [saveQueue[1]]); + t.deepEqual(compactSaveQueue(saveQueue), [saveQueue[1]]); }); test("SaveSaga should send update actions", t => { @@ -94,16 +95,17 @@ test("SaveSaga should send update actions", t => { expectValueDeepEqual(t, saga.next(), take(INIT_ACTIONS)); saga.next(); // setLastSaveTimestampAction saga.next(); // select state - expectValueDeepEqual(t, saga.next([]), take("PUSH_SAVE_QUEUE")); + expectValueDeepEqual(t, saga.next([]), put(setSaveBusyAction(false))); + expectValueDeepEqual(t, saga.next(), take("PUSH_SAVE_QUEUE")); saga.next(); // race saga.next(SaveActions.pushSaveQueueAction(updateActions)); saga.next(); expectValueDeepEqual(t, saga.next(saveQueue), call(sendRequestToServer)); - saga.next(); // SET_SAVE_BUSY // Test that loop repeats saga.next(); // select state - expectValueDeepEqual(t, saga.next([]), take("PUSH_SAVE_QUEUE")); + expectValueDeepEqual(t, saga.next([]), put(setSaveBusyAction(false))); + expectValueDeepEqual(t, saga.next(), take("PUSH_SAVE_QUEUE")); }); test("SaveSaga should send request to server", t => { @@ -194,7 +196,8 @@ test("SaveSaga should send update actions right away", t => { expectValueDeepEqual(t, saga.next(), take(INIT_ACTIONS)); saga.next(); saga.next(); // select state - expectValueDeepEqual(t, saga.next([]), take("PUSH_SAVE_QUEUE")); + expectValueDeepEqual(t, saga.next([]), put(setSaveBusyAction(false))); + expectValueDeepEqual(t, saga.next(), take("PUSH_SAVE_QUEUE")); saga.next(); // race saga.next(SaveActions.pushSaveQueueAction(updateActions)); saga.next(SaveActions.saveNowAction()); @@ -256,7 +259,7 @@ test("SaveSaga should set the correct version numbers if the save queue was comp saga.next(saveQueue); saga.next({ version: LAST_VERSION, type: "skeleton", tracingId: "1234567890" }); saga.next(DATASTORE_URL); - // two of the updateTracing update actions are removed by compactUpdateActions + // two of the updateTracing update actions are removed by compactSaveQueue expectValueDeepEqual(t, saga.next(), put(SaveActions.setVersionNumberAction(LAST_VERSION + 1))); expectValueDeepEqual(t, saga.next(), put(SaveActions.setLastSaveTimestampAction())); expectValueDeepEqual(t, saga.next(), put(SaveActions.shiftSaveQueueAction(3))); diff --git a/app/assets/javascripts/test/sagas/skeletontracing_saga.spec.js b/app/assets/javascripts/test/sagas/skeletontracing_saga.spec.js index 43814426d00..6d562fe09ad 100644 --- a/app/assets/javascripts/test/sagas/skeletontracing_saga.spec.js +++ b/app/assets/javascripts/test/sagas/skeletontracing_saga.spec.js @@ -6,6 +6,7 @@ import ChainReducer from "test/helpers/chainReducer"; import { createSaveQueueFromUpdateActions, withoutUpdateTracing } from "../helpers/saveHelpers"; import DiffableMap from "libs/diffable_map"; import EdgeCollection from "oxalis/model/edge_collection"; +import type { SaveQueueEntryType } from "oxalis/store"; const TIMESTAMP = 1494347146379; @@ -20,7 +21,7 @@ mockRequire("oxalis/model/sagas/root_saga", function*() { }); const { diffSkeletonTracing } = mockRequire.reRequire("oxalis/model/sagas/skeletontracing_saga"); -const { saveTracingAsync, compactUpdateActions } = mockRequire.reRequire( +const { saveTracingAsync, compactUpdateActions, compactSaveQueue } = mockRequire.reRequire( "oxalis/model/sagas/save_saga", ); const SkeletonTracingActions = mockRequire.reRequire( @@ -37,6 +38,17 @@ function testDiffing(prevTracing, nextTracing, flycam) { return withoutUpdateTracing(Array.from(diffSkeletonTracing(prevTracing, nextTracing, flycam))); } +function compactSaveQueueWithUpdateActions( + queue: Array, +): Array { + return compactSaveQueue( + queue.map(batch => ({ + ...batch, + actions: compactUpdateActions(batch.actions), + })), + ); +} + const initialState = { dataset: { dataSource: { @@ -382,7 +394,7 @@ test("compactUpdateActions should detect a tree merge (1/3)", t => { const updateActions = testDiffing(testState.tracing, newState.tracing, newState.flycam); const saveQueue = createSaveQueueFromUpdateActions([updateActions], TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; // This should result in a moved treeComponent of size three @@ -432,7 +444,7 @@ test("compactUpdateActions should detect a tree merge (2/3)", t => { // compactUpdateActions is triggered by the saving, it can therefore contain the results of more than one diffing const saveQueue = createSaveQueueFromUpdateActions(updateActions, TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); // This should result in one created node and its edge (a) const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; @@ -505,7 +517,7 @@ test("compactUpdateActions should detect a tree merge (3/3)", t => { // compactUpdateActions is triggered by the saving, it can therefore contain the results of more than one diffing const saveQueue = createSaveQueueFromUpdateActions(updateActions, TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); // This should result in a moved treeComponent of size one (a) const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; @@ -567,7 +579,7 @@ test("compactUpdateActions should detect a tree split (1/3)", t => { const updateActions = testDiffing(testState.tracing, newState.tracing, newState.flycam); const saveQueue = createSaveQueueFromUpdateActions([updateActions], TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); // This should result in a new tree const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; @@ -611,7 +623,7 @@ test("compactUpdateActions should detect a tree split (2/3)", t => { const updateActions = testDiffing(testState.tracing, newState.tracing, newState.flycam); const saveQueue = createSaveQueueFromUpdateActions([updateActions], TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); // This should result in two new trees and two moved treeComponents of size three and two const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; @@ -664,7 +676,7 @@ test("compactUpdateActions should detect a tree split (3/3)", t => { updateActions.push(testDiffing(newState1.tracing, newState2.tracing, newState2.flycam)); const saveQueue = createSaveQueueFromUpdateActions(updateActions, TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); // This should result in the creation of a new tree (a) const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; @@ -729,7 +741,7 @@ test("compactUpdateActions should do nothing if it cannot compact", t => { // This will currently never be the result of one diff (see description of the test) const updateActions = testDiffing(testState.tracing, newState.tracing, newState.flycam); const saveQueue = createSaveQueueFromUpdateActions([updateActions], TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); // The deleteTree optimization in compactUpdateActions (that is unrelated to this test) // will remove the first deleteNode update action as the first tree is deleted because of the merge, @@ -755,7 +767,7 @@ test("compactUpdateActions should detect a deleted tree", t => { const updateActions = testDiffing(testState.tracing, newState.tracing, newState.flycam); const saveQueue = createSaveQueueFromUpdateActions([updateActions], TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; t.deepEqual(simplifiedFirstBatch[0], { @@ -781,7 +793,7 @@ test("compactUpdateActions should not detect a deleted tree if there is no delet const updateActions = testDiffing(testState.tracing, newState.tracing, newState.flycam); const saveQueue = createSaveQueueFromUpdateActions([updateActions], TIMESTAMP); - const simplifiedUpdateActions = compactUpdateActions(saveQueue); + const simplifiedUpdateActions = compactSaveQueueWithUpdateActions(saveQueue); const simplifiedFirstBatch = simplifiedUpdateActions[0].actions; t.deepEqual(simplifiedFirstBatch[0], {