Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo/redo for tools #394

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Undo/redo for tools #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79bf34a
feat(history): add core history mechanism
floryst Aug 16, 2023
12a22af
feat(useKeyboardShortcuts): add undo/redo
floryst Aug 16, 2023
de35b68
feat(tools): add basic undo/redo to tools
floryst Aug 16, 2023
3f48d1c
feat(tools): undo/redo tool deletion
floryst Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/components/MeasurementsToolList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import { computed } from 'vue';
import { useCurrentImage } from '@/src/composables/useCurrentImage';
import { AnnotationToolStore } from '@/src/store/tools/useAnnotationTool';
import { frameOfReferenceToImageSliceAndAxis } from '@/src/utils/frameOfReference';
import useHistoryStore from '@/src/store/history';
import { createRemoveToolOperation } from '@/src/store/operations/tools';
import { Store } from 'pinia';

const props = defineProps<{
toolStore: AnnotationToolStore<ToolID>;
toolStore: AnnotationToolStore<ToolID> & Store;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, should I put & Store on the AnnotationToolStore shared type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that is a good idea. I could add that as an extra commit to this PR if you'd like.

icon: string;
}>();

Expand All @@ -33,7 +36,10 @@ const tools = computed(() => {
});

const remove = (id: ToolID) => {
props.toolStore.removeTool(id);
const imageID = currentImageID.value;
if (!imageID) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No imageID, but showing remove button in toolList would be strange. Throw Error or derive imageID from toolID so imageID is guarrentied? Worried that defensive programming could lead to corner case runtime quirks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like deriving the image ID from the tool ID in this case.

const op = createRemoveToolOperation(props.toolStore, id);
useHistoryStore().pushOperation({ datasetID: imageID }, op, true);
};

const jumpTo = (id: ToolID) => {
Expand Down
43 changes: 33 additions & 10 deletions src/components/tools/polygon/PolygonTool.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ import {
} from '@/src/utils/frameOfReference';
import { usePolygonStore } from '@/src/store/tools/polygons';
import { Polygon, PolygonID } from '@/src/types/polygon';
import {
createAddToolOperation,
createRemoveToolOperation,
} from '@/src/store/operations/tools';
import useHistoryStore from '@/src/store/history';
import { IHistoryOperation } from '@/src/types/history';
import { Maybe } from '@/src/types';
import PolygonWidget2D from './PolygonWidget2D.vue';

type ToolID = PolygonID;
Expand Down Expand Up @@ -100,6 +107,17 @@ export default defineComponent({
const viewAxis = computed(() => getLPSAxisFromDir(viewDirection.value));

const placingToolID = ref<ToolID | null>(null);
let addToolOperation: Maybe<IHistoryOperation<PolygonID>> = null;

const addPlacingTool = () => {
const imageID = currentImageID.value;
if (!imageID) return;
addToolOperation = createAddToolOperation(activeToolStore, {
imageID,
placing: true,
});
placingToolID.value = addToolOperation.apply();
};

// --- active tool management --- //

Expand All @@ -124,10 +142,7 @@ export default defineComponent({
placingToolID.value = null;
}
if (active && imageID) {
placingToolID.value = activeToolStore.addTool({
imageID,
placing: true,
});
addPlacingTool();
}
},
{ immediate: true }
Expand All @@ -154,11 +169,13 @@ export default defineComponent({
});

const onToolPlaced = () => {
if (currentImageID.value) {
placingToolID.value = activeToolStore.addTool({
imageID: currentImageID.value,
placing: true,
});
const imageID = currentImageID.value;
if (imageID) {
useHistoryStore().pushOperation(
{ datasetID: imageID },
addToolOperation!
);
Comment on lines +174 to +177
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about hiding pushOperation and its ilk in the tool stores?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to do it later down the road. I'm opting not to for now because I don't want to make the tool stores undo/redo aware just yet.

addPlacingTool();
}
};

Expand Down Expand Up @@ -207,7 +224,13 @@ export default defineComponent({
};

const deleteToolFromContextMenu = () => {
activeToolStore.removeTool(contextMenu.forToolID);
const imageID = currentImageID.value;
if (!imageID) return;
const op = createRemoveToolOperation(
activeToolStore,
contextMenu.forToolID
);
useHistoryStore().pushOperation({ datasetID: imageID }, op, true);
};

// --- tool data --- //
Expand Down
43 changes: 33 additions & 10 deletions src/components/tools/rectangle/RectangleTool.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ import {
} from '@/src/utils/frameOfReference';
import { useRectangleStore } from '@/src/store/tools/rectangles';
import { Rectangle, RectangleID } from '@/src/types/rectangle';
import {
createAddToolOperation,
createRemoveToolOperation,
} from '@/src/store/operations/tools';
import useHistoryStore from '@/src/store/history';
import { IHistoryOperation } from '@/src/types/history';
import { Maybe } from '@/src/types';
import RectangleWidget2D from './RectangleWidget2D.vue';

type ToolID = RectangleID;
Expand Down Expand Up @@ -100,6 +107,17 @@ export default defineComponent({
const viewAxis = computed(() => getLPSAxisFromDir(viewDirection.value));

const placingToolID = ref<ToolID | null>(null);
let addToolOperation: Maybe<IHistoryOperation<RectangleID>> = null;

const addPlacingTool = () => {
const imageID = currentImageID.value;
if (!imageID) return;
addToolOperation = createAddToolOperation(activeToolStore, {
imageID,
placing: true,
});
placingToolID.value = addToolOperation.apply();
};

// --- active tool management --- //

Expand All @@ -124,10 +142,7 @@ export default defineComponent({
placingToolID.value = null;
}
if (active && imageID) {
placingToolID.value = activeToolStore.addTool({
imageID,
placing: true,
});
addPlacingTool();
}
},
{ immediate: true }
Expand All @@ -154,11 +169,13 @@ export default defineComponent({
});

const onToolPlaced = () => {
if (currentImageID.value) {
placingToolID.value = activeToolStore.addTool({
imageID: currentImageID.value,
placing: true,
});
const imageID = currentImageID.value;
if (imageID) {
useHistoryStore().pushOperation(
{ datasetID: imageID },
addToolOperation!
);
addPlacingTool();
}
};

Expand Down Expand Up @@ -208,7 +225,13 @@ export default defineComponent({
};

const deleteToolFromContextMenu = () => {
activeToolStore.removeTool(contextMenu.forToolID);
const imageID = currentImageID.value;
if (!imageID) return;
const op = createRemoveToolOperation(
activeToolStore,
contextMenu.forToolID
);
useHistoryStore().pushOperation({ datasetID: imageID }, op, true);
};

// --- tool data --- //
Expand Down
50 changes: 35 additions & 15 deletions src/components/tools/ruler/RulerTool.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
close-on-content-click
>
<v-list density="compact">
<v-list-item @click="deleteRulerFromContextMenu">
<v-list-item @click="deleteToolFromContextMenu">
<v-list-item-title>Delete</v-list-item-title>
</v-list-item>
</v-list>
Expand Down Expand Up @@ -60,6 +60,13 @@ import {
} from '@/src/utils/frameOfReference';
import { Ruler } from '@/src/types/ruler';
import { vec3 } from 'gl-matrix';
import {
createAddToolOperation,
createRemoveToolOperation,
} from '@/src/store/operations/tools';
import useHistoryStore from '@/src/store/history';
import { IHistoryOperation } from '@/src/types/history';
import { Maybe } from '@/src/types';

export default defineComponent({
name: 'RulerTool',
Expand Down Expand Up @@ -95,6 +102,17 @@ export default defineComponent({
const viewAxis = computed(() => getLPSAxisFromDir(viewDirection.value));

const placingRulerID = ref<string | null>(null);
let addToolOperation: Maybe<IHistoryOperation<string>> = null;

const addPlacingTool = () => {
const imageID = currentImageID.value;
if (!imageID) return;
addToolOperation = createAddToolOperation(rulerStore, {
imageID,
placing: true,
});
placingRulerID.value = addToolOperation.apply();
};

// --- active ruler management --- //

Expand All @@ -119,10 +137,7 @@ export default defineComponent({
placingRulerID.value = null;
}
if (active && imageID) {
placingRulerID.value = rulerStore.addRuler({
imageID,
placing: true,
});
addPlacingTool();
}
},
{ immediate: true }
Expand All @@ -149,11 +164,13 @@ export default defineComponent({
});

const onRulerPlaced = () => {
if (currentImageID.value) {
placingRulerID.value = rulerStore.addRuler({
imageID: currentImageID.value,
placing: true,
});
const imageID = currentImageID.value;
if (imageID) {
useHistoryStore().pushOperation(
{ datasetID: imageID },
addToolOperation!
);
addPlacingTool();
}
};

Expand Down Expand Up @@ -196,17 +213,20 @@ export default defineComponent({
show: false,
x: 0,
y: 0,
forRulerID: '',
forToolID: '',
});

const openContextMenu = (rulerID: string, displayXY: Vector2) => {
[contextMenu.x, contextMenu.y] = displayXY;
contextMenu.show = true;
contextMenu.forRulerID = rulerID;
contextMenu.forToolID = rulerID;
};

const deleteRulerFromContextMenu = () => {
rulerStore.removeRuler(contextMenu.forRulerID);
const deleteToolFromContextMenu = () => {
const imageID = currentImageID.value;
if (!imageID) return;
const op = createRemoveToolOperation(rulerStore, contextMenu.forToolID);
useHistoryStore().pushOperation({ datasetID: imageID }, op, true);
};

// --- ruler data --- //
Expand Down Expand Up @@ -252,7 +272,7 @@ export default defineComponent({
placingRulerID,
contextMenu,
openContextMenu,
deleteRulerFromContextMenu,
deleteToolFromContextMenu,
onRulerPlaced,
};
},
Expand Down
36 changes: 32 additions & 4 deletions src/composables/useKeyboardShortcuts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import { onKeyDown } from '@vueuse/core';
import { onKeyDown, onKeyStroke } from '@vueuse/core';

import { DECREMENT_LABEL_KEY, INCREMENT_LABEL_KEY } from '../config';
import useHistoryStore from '@/src/store/history';
import { useCurrentImage } from '@/src/composables/useCurrentImage';
import { isDarwin } from '@/src/constants';
import { DEFAULT_KEYMAP } from '../config';
import { useToolStore } from '../store/tools';
import { Tools } from '../store/tools/types';
import { useRectangleStore } from '../store/tools/rectangles';
import { useRulerStore } from '../store/tools/rulers';
import { usePolygonStore } from '../store/tools/polygons';

const Keymap = DEFAULT_KEYMAP;

const isCtrlOrCmd = (ev: KeyboardEvent) => {
return isDarwin ? ev.metaKey : ev.ctrlKey;
};

const applyLabelOffset = (offset: number) => {
const toolToStore = {
[Tools.Rectangle]: useRectangleStore(),
Expand All @@ -28,7 +37,26 @@ const applyLabelOffset = (offset: number) => {
activeToolStore.setActiveLabel(nextLabel);
};

const undo = () => {
const { currentImageID } = useCurrentImage();
if (!currentImageID.value) return;
useHistoryStore().undo({ datasetID: currentImageID.value });
};

const redo = () => {
const { currentImageID } = useCurrentImage();
if (!currentImageID.value) return;
useHistoryStore().redo({ datasetID: currentImageID.value });
};

export function useKeyboardShortcuts() {
onKeyDown(DECREMENT_LABEL_KEY, () => applyLabelOffset(-1));
onKeyDown(INCREMENT_LABEL_KEY, () => applyLabelOffset(1));
onKeyDown(Keymap.DecrementLabel, () => applyLabelOffset(-1));
onKeyDown(Keymap.IncrementLabel, () => applyLabelOffset(1));
onKeyStroke(Keymap.UndoRedo, (ev) => {
if (isCtrlOrCmd(ev)) {
ev.preventDefault();
if (ev.shiftKey) redo();
else undo();
}
});
}
7 changes: 5 additions & 2 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,11 @@ export const POLYGON_LABEL_DEFAULTS = {
white: { color: '#ffffff' },
};

export const DECREMENT_LABEL_KEY = 'q';
export const INCREMENT_LABEL_KEY = 'w';
export const DEFAULT_KEYMAP = {
UndoRedo: 'z',
DecrementLabel: 'q',
IncrementLabel: 'w',
};

export const DEFAULT_PRESET_BY_MODALITY: Record<string, string> = {
CT: 'CT-AAA',
Expand Down
2 changes: 2 additions & 0 deletions src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,5 @@ export const Messages = {
'Lost the WebGL context! Please reload the webpage. If the problem persists, you may need to restart your web browser.',
},
} as const;

export const isDarwin = /Mac|iPhone|iPad/.test(navigator.platform);
Loading