From b0134c4b3243ca4781146f449e9e0d3924ca2f1b Mon Sep 17 00:00:00 2001 From: Martin Turoci Date: Wed, 7 Dec 2022 10:24:52 +0100 Subject: [PATCH 1/4] fix: Do not add too small rects into wave.args. #1726 --- ui/src/image_annotator.test.tsx | 11 +++++++++++ ui/src/image_annotator_rect.ts | 4 +++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ui/src/image_annotator.test.tsx b/ui/src/image_annotator.test.tsx index d0b352f02d..7ad2c1911d 100644 --- a/ui/src/image_annotator.test.tsx +++ b/ui/src/image_annotator.test.tsx @@ -129,6 +129,17 @@ describe('ImageAnnotator.tsx', () => { expect(wave.args[name]).toMatchObject(items) }) + it('Does not draw a new rect if dimensions too small', async () => { + const { container, getByText } = render() + await waitForLoad() + const canvasEl = container.querySelectorAll('canvas')[1] + fireEvent.click(getByText('Rectangle')) + fireEvent.mouseDown(canvasEl, { clientX: 110, clientY: 110, buttons: 1 }) + fireEvent.click(canvasEl, { clientX: 115, clientY: 115 }) + + expect(wave.args[name]).toMatchObject(items) + }) + it('Removes rect after clicking remove btn', async () => { const { container, getByText } = render() await waitForLoad() diff --git a/ui/src/image_annotator_rect.ts b/ui/src/image_annotator_rect.ts index 37553b408a..cd3d614551 100644 --- a/ui/src/image_annotator_rect.ts +++ b/ui/src/image_annotator_rect.ts @@ -59,7 +59,9 @@ export class RectAnnotator { onClick = (cursor_x: U, cursor_y: U, tag: S, start?: Position): DrawnShape | undefined => { let newRect if (!this.resizedCorner && start?.dragging) { - newRect = { shape: { rect: this.createRect(start.x, cursor_x, start.y, cursor_y) }, tag } + const rect = this.createRect(start.x, cursor_x, start.y, cursor_y) + if (!rect) return + newRect = { shape: { rect }, tag } } this.resetDragging() From 9f0d76496b6733f827a631ac4eefa7f7f0d73245 Mon Sep 17 00:00:00 2001 From: Martin Turoci Date: Thu, 8 Dec 2022 10:10:22 +0100 Subject: [PATCH 2/4] fix: Update wave.args if dragging finished outside of canvas. #1726 --- ui/src/image_annotator.test.tsx | 47 ++++++++++++++++++++++++++++++--- ui/src/image_annotator.tsx | 12 +++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/ui/src/image_annotator.test.tsx b/ui/src/image_annotator.test.tsx index 7ad2c1911d..67e2c5adf0 100644 --- a/ui/src/image_annotator.test.tsx +++ b/ui/src/image_annotator.test.tsx @@ -218,6 +218,30 @@ describe('ImageAnnotator.tsx', () => { expect(wave.args[name]).toMatchObject([{ tag: 'person', shape: { rect: { x1: 20, x2: 110, y1: 20, y2: 110 } } }, polygon]) }) + it('Moves rect correctly if click happened outside the canvas', async () => { + const { container } = render() + await waitForLoad() + const canvasEl = container.querySelectorAll('canvas')[1] + fireEvent.click(canvasEl, { clientX: 50, clientY: 50 }) + fireEvent.mouseDown(canvasEl, { clientX: 50, clientY: 50, buttons: 1 }) + fireEvent.mouseMove(canvasEl, { clientX: 45, clientY: 50, buttons: 1 }) + fireEvent.mouseLeave(canvasEl, { clientX: -10, clientY: 60, buttons: 1 }) + + expect(wave.args[name]).toMatchObject([{ tag: 'person', shape: { rect: { x1: 5, x2: 95, y1: 10, y2: 100 } } }, polygon]) + }) + + it('Does not move rect if click happened outside the canvas but left mouse btn not pressed', async () => { + const { container } = render() + await waitForLoad() + const canvasEl = container.querySelectorAll('canvas')[1] + fireEvent.click(canvasEl, { clientX: 50, clientY: 50 }) + fireEvent.mouseDown(canvasEl, { clientX: 50, clientY: 50, buttons: 1 }) + fireEvent.mouseMove(canvasEl, { clientX: 45, clientY: 50, buttons: 1 }) + fireEvent.mouseLeave(canvasEl, { clientX: -10, clientY: 60 }) + + expect(wave.args[name]).toMatchObject(items) + }) + it('Does not move rect if left mouse btn not pressed (dragging)', async () => { const { container } = render() await waitForLoad() @@ -553,6 +577,19 @@ describe('ImageAnnotator.tsx', () => { expect(pushMock).toBeCalledTimes(1) }) + it('Calls sync after moving rect and finishing outside of canvas', async () => { + const { container } = render() + await waitForLoad() + const canvasEl = container.querySelectorAll('canvas')[1] + fireEvent.click(canvasEl, { clientX: 50, clientY: 50 }) + fireEvent.mouseDown(canvasEl, { clientX: 50, clientY: 50, buttons: 1 }) + fireEvent.mouseMove(canvasEl, { clientX: 60, clientY: 60, buttons: 1 }) + fireEvent.click(canvasEl, { clientX: 60, clientY: 60 }) + + expect(pushMock).toBeCalledTimes(1) + }) + + it('Calls sync after resizing rect', async () => { const { container } = render() await waitForLoad() @@ -565,14 +602,16 @@ describe('ImageAnnotator.tsx', () => { expect(pushMock).toBeCalledTimes(1) }) - it('Calls sync after removing rect', async () => { - const { container, getByText } = render() + it('Calls sync after removing rect and finishing outside canvas', async () => { + const { container } = render() await waitForLoad() const canvasEl = container.querySelectorAll('canvas')[1] fireEvent.click(canvasEl, { clientX: 50, clientY: 50 }) - await waitForLoad() - fireEvent.click(getByText('Remove selection').parentElement?.parentElement?.parentElement!) + fireEvent.mouseDown(canvasEl, { clientX: 50, clientY: 50, buttons: 1 }) + fireEvent.mouseMove(canvasEl, { clientX: 45, clientY: 50, buttons: 1 }) + fireEvent.mouseLeave(canvasEl, { clientX: -10, clientY: 60, buttons: 1 }) + await waitForLoad() expect(pushMock).toBeCalledTimes(1) }) diff --git a/ui/src/image_annotator.tsx b/ui/src/image_annotator.tsx index f1955d90f3..5b0ba76ca6 100644 --- a/ui/src/image_annotator.tsx +++ b/ui/src/image_annotator.tsx @@ -204,6 +204,17 @@ export const XImageAnnotator = ({ model }: { model: ImageAnnotator }) => { redrawExistingShapes() // eslint-disable-next-line react-hooks/exhaustive-deps }, [redrawExistingShapes]), + + onMouseLeave = (e: React.MouseEvent) => { + const canvas = canvasRef.current + if (!canvas || e.buttons !== 1) return + + setWaveArgs(drawnShapes) + + polygonRef.current?.resetDragging() + rectRef.current?.resetDragging() + redrawExistingShapes() + }, onMouseDown = (e: React.MouseEvent) => { const canvas = canvasRef.current if (!canvas) return @@ -462,6 +473,7 @@ export const XImageAnnotator = ({ model }: { model: ImageAnnotator }) => { className={css.canvas} onMouseMove={onMouseMove} onMouseDown={onMouseDown} + onMouseLeave={onMouseLeave} onKeyDown={onKeyDown} // Do not show context menu on right click. onContextMenu={e => e.preventDefault()} From 17277dd2179d7bd2e2afd1a0e0a912d68703b269 Mon Sep 17 00:00:00 2001 From: Martin Turoci Date: Thu, 8 Dec 2022 10:20:29 +0100 Subject: [PATCH 3/4] chore: Bring back accidental refactor. #1726 --- ui/src/image_annotator.test.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/ui/src/image_annotator.test.tsx b/ui/src/image_annotator.test.tsx index 67e2c5adf0..13119d6b41 100644 --- a/ui/src/image_annotator.test.tsx +++ b/ui/src/image_annotator.test.tsx @@ -602,20 +602,16 @@ describe('ImageAnnotator.tsx', () => { expect(pushMock).toBeCalledTimes(1) }) - it('Calls sync after removing rect and finishing outside canvas', async () => { - const { container } = render() + it('Calls sync after removing rect', async () => { + const { container, getByText } = render() await waitForLoad() const canvasEl = container.querySelectorAll('canvas')[1] fireEvent.click(canvasEl, { clientX: 50, clientY: 50 }) - fireEvent.mouseDown(canvasEl, { clientX: 50, clientY: 50, buttons: 1 }) - fireEvent.mouseMove(canvasEl, { clientX: 45, clientY: 50, buttons: 1 }) - fireEvent.mouseLeave(canvasEl, { clientX: -10, clientY: 60, buttons: 1 }) - await waitForLoad() + fireEvent.click(getByText('Remove selection').parentElement!.parentElement!.parentElement!) expect(pushMock).toBeCalledTimes(1) }) - it('Calls sync after removing polygon', async () => { const { container, getByText } = render() await waitForLoad() From eb892fdf86b3ade2635e6bd789344932db64bb5e Mon Sep 17 00:00:00 2001 From: Martin Turoci Date: Thu, 8 Dec 2022 10:21:24 +0100 Subject: [PATCH 4/4] chore: Refactor. #1726 --- ui/src/image_annotator.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/image_annotator.test.tsx b/ui/src/image_annotator.test.tsx index 13119d6b41..ee248952d5 100644 --- a/ui/src/image_annotator.test.tsx +++ b/ui/src/image_annotator.test.tsx @@ -589,7 +589,6 @@ describe('ImageAnnotator.tsx', () => { expect(pushMock).toBeCalledTimes(1) }) - it('Calls sync after resizing rect', async () => { const { container } = render() await waitForLoad() @@ -609,6 +608,7 @@ describe('ImageAnnotator.tsx', () => { fireEvent.click(canvasEl, { clientX: 50, clientY: 50 }) await waitForLoad() fireEvent.click(getByText('Remove selection').parentElement!.parentElement!.parentElement!) + expect(pushMock).toBeCalledTimes(1) })