From 4c9faa721d3e9f96b548f00a0cfb7469768c4a50 Mon Sep 17 00:00:00 2001 From: Matt Dawkins Date: Mon, 21 Sep 2026 22:56:15 -0400 Subject: [PATCH 1/3] Confirm a point-segmented mask without repeating its stereo copy The confirm event re-sent the last prediction with its click points, so the other-camera segmentation, and any error dialog, ran twice. --- client/dive-common/use/useModeManager.spec.ts | 37 ++++++++++++++++++- client/dive-common/use/useModeManager.ts | 4 +- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/client/dive-common/use/useModeManager.spec.ts b/client/dive-common/use/useModeManager.spec.ts index 93374e519..c854c32e7 100644 --- a/client/dive-common/use/useModeManager.spec.ts +++ b/client/dive-common/use/useModeManager.spec.ts @@ -15,8 +15,10 @@ import type { AnnotationId } from 'vue-media-annotator/BaseAnnotation'; import type { MarkChangesPending } from 'vue-media-annotator/BaseAnnotationStore'; import Track from 'vue-media-annotator/track'; import { ROTATION_ATTRIBUTE_NAME } from 'vue-media-annotator/utils'; -import useModeManager from './useModeManager'; +import useModeManager, { type StereoAnnotationCompleteParams } from './useModeManager'; import HeadTail from '../recipes/headtail'; +import SegmentationPointClick from '../recipes/segmentationpointclick'; +import { clientSettings } from '../store/settings'; import { headTailFeatures } from '../../src/headTail'; import type Recipe from '../../src/recipe'; @@ -24,7 +26,11 @@ function translation(tx: number, ty: number): Matrix3 { return [[1, 0, tx], [0, 1, ty], [0, 0, 1]]; } -function makeHarness(markChangesPending: MarkChangesPending = () => undefined, recipes: Recipe[] = []) { +function makeHarness( + markChangesPending: MarkChangesPending = () => undefined, + recipes: Recipe[] = [], + onStereoAnnotationComplete: ((params: StereoAnnotationCompleteParams) => void) | undefined = undefined, +) { const cameraStore = new CameraStore({ markChangesPending }); cameraStore.removeCamera('singleCam'); cameraStore.addCamera('left'); @@ -77,6 +83,7 @@ function makeHarness(markChangesPending: MarkChangesPending = () => undefined, r readonlyState: ref(false), recipes, alignedView, + onStereoAnnotationComplete, }); modeManager.selectedCamera.value = 'left'; return { @@ -456,3 +463,29 @@ describe('centerline editing continuity', () => { expect(track.features[0].bounds).toEqual([0, 0, 100, 100]); }); }); + +describe('stereo copy of a point-segmented mask', () => { + it('runs once per click and not again when the mask is confirmed', () => { + const wasAutoCompute = clientSettings.stereoSettings.autoComputeOtherCamera; + clientSettings.stereoSettings.autoComputeOtherCamera = true; + try { + const recipe = new SegmentationPointClick(); + const events: StereoAnnotationCompleteParams[] = []; + const { modeManager: manager } = makeHarness(undefined, [recipe], (params) => events.push(params)); + manager.handler.trackAdd(); + const result = { + polygon: [[0, 0], [10, 0], [10, 10]] as [number, number][], + bounds: null, + frameNum: 0, + controlPoints: { points: [[5, 5]] as [number, number][], labels: [1] }, + }; + recipe.bus.$emit('prediction-ready', result); + expect(events.map((e) => e.type)).toEqual(['segmentation']); + recipe.bus.$emit('prediction-confirmed', result); + recipe.bus.$emit('prediction-confirmed-multi', { frames: new Map([[0, result]]) }); + expect(events.map((e) => e.type)).toEqual(['segmentation']); + } finally { + clientSettings.stereoSettings.autoComputeOtherCamera = wasAutoCompute; + } + }); +}); diff --git a/client/dive-common/use/useModeManager.ts b/client/dive-common/use/useModeManager.ts index f4eedce16..f5988a956 100644 --- a/client/dive-common/use/useModeManager.ts +++ b/client/dive-common/use/useModeManager.ts @@ -1626,7 +1626,9 @@ export default function useModeManager({ * This is called when the user confirms the segmentation (right-click or Enter). */ function handleSegmentationPredictionConfirmed(result: SegmentationPredictionResult) { - handleSegmentationPredictionReady(result); + // Each click already ran the stereo copy and auto-populate for this mask; + // confirming only commits it. + handleSegmentationPredictionReady({ ...result, controlPoints: undefined }); } /** Click-path variant: a fresh point click that should honor continuous mode. */ From 30592cbc281da2790a75691cc67c9873b2646a45 Mon Sep 17 00:00:00 2001 From: Matt Dawkins Date: Wed, 23 Sep 2026 00:17:38 -0400 Subject: [PATCH 2/3] Keep a right-clicked detection in point segmentation editing on Windows Selecting a different detection clears the segmentation recipe through resetPoints, which marked the recipe as reset by the user. On Windows the contextmenu of the right-click that selected the detection arrives after it has entered Point edit mode, and handleConfirmRecipe took that mark as a request to finalize, deselecting the detection at once. Clear the recipe on a selection change without counting it as a user reset. Co-Authored-By: Claude Fable 5.1 --- .../recipes/segmentationpointclick.ts | 8 ++++-- client/dive-common/use/useModeManager.spec.ts | 28 +++++++++++++++++++ client/dive-common/use/useModeManager.ts | 7 +++-- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/client/dive-common/recipes/segmentationpointclick.ts b/client/dive-common/recipes/segmentationpointclick.ts index 79e626858..e228b1267 100644 --- a/client/dive-common/recipes/segmentationpointclick.ts +++ b/client/dive-common/recipes/segmentationpointclick.ts @@ -727,15 +727,19 @@ export default class SegmentationPointClick implements Recipe { /** * Public method to reset (clear) all accumulated points and pending prediction. * Called from UI Reset button. Clears all frames. + * + * @param byUser false when a selection change clears the points instead of + * the user, so a right-click that only entered edit mode cannot count as + * a reset to finalize. */ - resetPoints(): void { + resetPoints(byUser = true): void { // Emit reset event for all frames with data const framesToReset = [this.currentFrame, ...this.frameData.keys()]; framesToReset.forEach((frameNum) => { this.bus.$emit('prediction-reset', { frameNum }); }); this.reset(); - this._wasReset = true; + this._wasReset = byUser; this.icon.value = 'mdi-auto-fix'; } diff --git a/client/dive-common/use/useModeManager.spec.ts b/client/dive-common/use/useModeManager.spec.ts index c854c32e7..d519d4252 100644 --- a/client/dive-common/use/useModeManager.spec.ts +++ b/client/dive-common/use/useModeManager.spec.ts @@ -489,3 +489,31 @@ describe('stereo copy of a point-segmented mask', () => { } }); }); + +describe('a right-click that enters point segmentation editing', () => { + it('is not finalized by the contextmenu that follows it, unlike a user reset', () => { + const recipe = new SegmentationPointClick(); + const { modeManager: manager } = makeHarness(undefined, [recipe]); + recipe.activate(); + const first = manager.handler.trackAdd(); + manager.handler.updateRectBounds(0, 0, [0, 0, 10, 10]); + const second = manager.handler.trackAdd(); + manager.handler.updateRectBounds(0, 0, [20, 20, 30, 30]); + expect(manager.selectedTrackId.value).toBe(second); + // Selecting another detection clears the recipe, but not as a user reset. + manager.handler.trackEdit(first); + expect(manager.selectedTrackId.value).toBe(first); + expect(manager.editingTrack.value).toBe(true); + expect(recipe.wasReset).toBe(false); + // On Windows the contextmenu of that right-click arrives after edit mode began. + manager.handler.confirmRecipe(); + expect(manager.selectedTrackId.value).toBe(first); + expect(manager.editingTrack.value).toBe(true); + // A reset by the user still lets the next right-click finalize. + recipe.resetPoints(); + expect(recipe.wasReset).toBe(true); + manager.handler.confirmRecipe(); + expect(manager.selectedTrackId.value).toBeNull(); + expect(manager.editingTrack.value).toBe(false); + }); +}); diff --git a/client/dive-common/use/useModeManager.ts b/client/dive-common/use/useModeManager.ts index f5988a956..6b175847a 100644 --- a/client/dive-common/use/useModeManager.ts +++ b/client/dive-common/use/useModeManager.ts @@ -240,11 +240,14 @@ export default function useModeManager({ function selectTrack(trackId: AnnotationId | null, edit = false) { // Reset segmentation recipe state when switching to a different track - // so stale points/mask from the previous detection don't interfere + // so stale points/mask from the previous detection don't interfere. This + // is not a user reset: on Windows the contextmenu of the right-click that + // selected the detection arrives after it entered Point edit mode, and + // handleConfirmRecipe must not take it as a request to finalize. if (trackId !== selectedTrackId.value) { recipes.forEach((r) => { if (r instanceof SegmentationPointClick && r.active.value) { - r.resetPoints(); + r.resetPoints(false); } }); } From f4f4ebde80cc5a0eede9d3e48709e0c984058f65 Mon Sep 17 00:00:00 2001 From: Matt Dawkins Date: Wed, 23 Sep 2026 12:29:41 -0400 Subject: [PATCH 3/3] Let a right-click with no points placed still finalize point segmentation Clearing the recipe on a selection change without marking it as a user reset left a right-click that placed no points doing nothing. Remember which mouse press entered edit mode instead: the contextmenu of that press (delivered after mouseup on Windows) is ignored, and any later right-click with nothing placed finalizes the detection as before. Co-Authored-By: Claude Fable 5.1 --- client/dive-common/use/useModeManager.spec.ts | 19 +++++++++-- client/dive-common/use/useModeManager.ts | 33 +++++++++++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/client/dive-common/use/useModeManager.spec.ts b/client/dive-common/use/useModeManager.spec.ts index d519d4252..a1b3e8f9d 100644 --- a/client/dive-common/use/useModeManager.spec.ts +++ b/client/dive-common/use/useModeManager.spec.ts @@ -1,3 +1,4 @@ +// @vitest-environment jsdom /** * Functional tests for the Align View cross-camera mirror: drawing/editing a * track on one camera while the aligned view is active re-projects the @@ -491,7 +492,8 @@ describe('stereo copy of a point-segmented mask', () => { }); describe('a right-click that enters point segmentation editing', () => { - it('is not finalized by the contextmenu that follows it, unlike a user reset', () => { + const press = () => document.dispatchEvent(new MouseEvent('mousedown', { button: 2 })); + it('is not finalized by the contextmenu that follows it, unlike a later right-click or a user reset', () => { const recipe = new SegmentationPointClick(); const { modeManager: manager } = makeHarness(undefined, [recipe]); recipe.activate(); @@ -501,6 +503,7 @@ describe('a right-click that enters point segmentation editing', () => { manager.handler.updateRectBounds(0, 0, [20, 20, 30, 30]); expect(manager.selectedTrackId.value).toBe(second); // Selecting another detection clears the recipe, but not as a user reset. + press(); manager.handler.trackEdit(first); expect(manager.selectedTrackId.value).toBe(first); expect(manager.editingTrack.value).toBe(true); @@ -509,11 +512,21 @@ describe('a right-click that enters point segmentation editing', () => { manager.handler.confirmRecipe(); expect(manager.selectedTrackId.value).toBe(first); expect(manager.editingTrack.value).toBe(true); - // A reset by the user still lets the next right-click finalize. + // A later right-click with no points placed finalizes the detection. + press(); + manager.handler.confirmRecipe(); + expect(manager.selectedTrackId.value).toBeNull(); + expect(manager.editingTrack.value).toBe(false); + // So does one after a reset by the user, even within the same press. + press(); + manager.handler.trackEdit(second); recipe.resetPoints(); expect(recipe.wasReset).toBe(true); manager.handler.confirmRecipe(); expect(manager.selectedTrackId.value).toBeNull(); - expect(manager.editingTrack.value).toBe(false); + // With nothing selected a right-click changes nothing. + press(); + manager.handler.confirmRecipe(); + expect(recipe.active.value).toBe(true); }); }); diff --git a/client/dive-common/use/useModeManager.ts b/client/dive-common/use/useModeManager.ts index 6b175847a..68b63f653 100644 --- a/client/dive-common/use/useModeManager.ts +++ b/client/dive-common/use/useModeManager.ts @@ -238,12 +238,20 @@ export default function useModeManager({ return false; } + // The right mousedown that selects a detection for point segmentation is + // followed on Windows by its contextmenu only after mouseup, once edit mode + // has begun. That contextmenu must not finalize the fresh edit, while a + // later right-click with no points placed still does, so remember which + // press entered edit mode. + let mouseDownCount = 0; + let editEnteredOnMouseDown = -1; + const countMouseDown = () => { mouseDownCount += 1; }; + if (typeof document !== 'undefined') document.addEventListener('mousedown', countMouseDown, true); + function selectTrack(trackId: AnnotationId | null, edit = false) { // Reset segmentation recipe state when switching to a different track // so stale points/mask from the previous detection don't interfere. This - // is not a user reset: on Windows the contextmenu of the right-click that - // selected the detection arrives after it entered Point edit mode, and - // handleConfirmRecipe must not take it as a request to finalize. + // is not a user reset (see handleConfirmRecipe). if (trackId !== selectedTrackId.value) { recipes.forEach((r) => { if (r instanceof SegmentationPointClick && r.active.value) { @@ -251,6 +259,9 @@ export default function useModeManager({ } }); } + if (trackId !== null && edit && (trackId !== selectedTrackId.value || !editingTrack.value)) { + editEnteredOnMouseDown = mouseDownCount; + } // Clean up empty tracks when leaving edit mode (e.g., created a detection // but never drew an annotation, then clicked away or right-clicked to deselect) if ( @@ -1296,12 +1307,10 @@ export default function useModeManager({ * Called when right-click is used in Point mode to lock the annotation. */ function handleConfirmRecipe() { - // First check if any active segmentation recipe has a pending prediction - // or was explicitly reset by the user (Escape key). - // If neither, there's nothing to confirm - this happens when the contextmenu - // event from a right-click that entered Point edit mode triggers - // confirm-annotation before any points are placed. In that case, don't - // confirm/deactivate recipes or deselect - let the edit mode continue. + // A pending prediction is committed; with nothing placed (or after an + // Escape reset) the right-click just finalizes the detection, leaving + // edit mode. Neither applies to the contextmenu of the very press that + // entered edit mode, which Windows delivers after mouseup. let hadPendingPredictionOrReset = false; recipes.forEach((r) => { if (r.active.value && r.confirm && r instanceof SegmentationPointClick) { @@ -1311,7 +1320,10 @@ export default function useModeManager({ } }); if (!hadPendingPredictionOrReset) { - return; + if (selectedTrackId.value === null || !editingTrack.value + || mouseDownCount === editEnteredOnMouseDown) { + return; + } } const activeSegRecipes: SegmentationPointClick[] = []; recipes.forEach((r) => { @@ -1847,6 +1859,7 @@ export default function useModeManager({ /* Unsubscribe before unmount */ onBeforeUnmount(() => { + if (typeof document !== 'undefined') document.removeEventListener('mousedown', countMouseDown, true); recipes.forEach((r) => r.bus.$off('activate', handleSetAnnotationState)); recipes.forEach((r) => { if (r instanceof SegmentationPointClick) {