Skip to content

Commit 6e2ba8f

Browse files
committed
fix(annotation): read mark bounds in frame layout space, not the scaled client rect
Full-matrix runtime verification of #6361 surfaced a third coordinate seam: in a fit-to-window scaled device frame (tablet/mobile preview in a narrow window) the structured annotation bounds were read from canvas.getBoundingClientRect(), which reports the on-screen size after the ancestor transform: scale(). The composited screenshot paints marks across the snapshot's artifact-local dimensions, so the PNG showed the right region while the structured position sent to the agent was shrunk by the fit scale — measured 0.42x: a mark on a 784px-wide band went out as a 330px rect at the wrong offset. Bounds now derive from the canvas layout box (offsetWidth/Height, the same source the canvas is sized from and the anchor pass measures against), with the client rect kept only as a fallback for environments without layout metrics. Text-label bounds divide their client-space offsets by the live scale for the same reason. Verified against a live tools-dev runtime driving the real UI: box marks, pen strokes at all four preview edges, sidebar collapsed/expanded, and a 0.42x-scaled tablet frame — the decoded screenshot's painted mark, the structured bounds, and the pointer-targeted band now agree within 2px in every case, at DPR 1 and 2. Refs #6361
1 parent 677fa56 commit 6e2ba8f

2 files changed

Lines changed: 154 additions & 12 deletions

File tree

apps/web/src/components/PreviewDrawOverlay.tsx

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -956,9 +956,26 @@ export function PreviewDrawOverlay({
956956
bumpLayoutRevision();
957957
}, [active, redraw]);
958958

959+
// Bounds read for the annotation payload must be in the frame's *layout*
960+
// space (offsetWidth/Height), matching the snapshot the marks are composited
961+
// onto. In a scaled tablet/phone device frame getBoundingClientRect() returns
962+
// the on-screen (transform-scaled) size, which shrank the structured bounds
963+
// by the fit scale while the painted PNG stayed artifact-local (#6361).
964+
function canvasLayoutSize(): { width: number; height: number } | null {
965+
const cvs = canvasRef.current;
966+
if (!cvs) return null;
967+
// offsetWidth/Height are the untransformed layout size. Fall back to the
968+
// client rect when layout metrics are unavailable (detached node, JSDOM) —
969+
// in an unscaled frame the two agree.
970+
const width = cvs.offsetWidth > 0 ? cvs.offsetWidth : cvs.getBoundingClientRect().width;
971+
const height = cvs.offsetHeight > 0 ? cvs.offsetHeight : cvs.getBoundingClientRect().height;
972+
if (width <= 0 || height <= 0) return null;
973+
return { width, height };
974+
}
975+
959976
function normalizedRectToCanvasRect(box: NormalizedRect): Rect | null {
960-
const rect = canvasRef.current?.getBoundingClientRect();
961-
if (!rect || rect.width <= 0 || rect.height <= 0) return null;
977+
const rect = canvasLayoutSize();
978+
if (!rect) return null;
962979
return {
963980
x: box.x * rect.width,
964981
y: box.y * rect.height,
@@ -990,9 +1007,9 @@ export function PreviewDrawOverlay({
9901007
}
9911008

9921009
function strokeRect(stroke: Stroke | null | undefined): Rect | null {
993-
const rect = canvasRef.current?.getBoundingClientRect();
1010+
const rect = canvasLayoutSize();
9941011
const points = stroke?.points ?? [];
995-
if (!rect || rect.width <= 0 || rect.height <= 0 || points.length === 0) return null;
1012+
if (!rect || points.length === 0) return null;
9961013
const xs = points.map((point) => point.x * rect.width);
9971014
const ys = points.map((point) => point.y * rect.height);
9981015
const minX = Math.min(...xs);
@@ -1009,25 +1026,31 @@ export function PreviewDrawOverlay({
10091026
}
10101027

10111028
function textBounds(): { x: number; y: number; width: number; height: number } | null {
1012-
const rect = canvasRef.current?.getBoundingClientRect();
1013-
if (!rect || rect.width <= 0 || rect.height <= 0) return null;
1029+
const layout = canvasLayoutSize();
1030+
const client = canvasRef.current?.getBoundingClientRect();
1031+
if (!layout || !client || client.width <= 0 || client.height <= 0) return null;
1032+
// Client-space offsets shrink under a device-frame `transform: scale()`;
1033+
// divide by the live scale so label boxes land in layout space like every
1034+
// other bounds source (#6361).
1035+
const scaleX = client.width / layout.width;
1036+
const scaleY = client.height / layout.height;
10141037
const rects: { left: number; top: number; right: number; bottom: number }[] = [];
10151038
for (const mark of textMarksRef.current) {
10161039
if (mark.text.trim().length === 0) continue;
10171040
const el = textAreaRefs.current.get(mark.id);
10181041
if (el) {
10191042
const box = el.getBoundingClientRect();
10201043
rects.push({
1021-
left: box.left - rect.left,
1022-
top: box.top - rect.top,
1023-
right: box.right - rect.left,
1024-
bottom: box.bottom - rect.top,
1044+
left: (box.left - client.left) / scaleX,
1045+
top: (box.top - client.top) / scaleY,
1046+
right: (box.right - client.left) / scaleX,
1047+
bottom: (box.bottom - client.top) / scaleY,
10251048
});
10261049
} else {
10271050
// No live element (e.g. capture path measured after unmount): fall back
10281051
// to the drop point so the label still contributes to the crop bounds.
1029-
const left = mark.x * rect.width;
1030-
const top = mark.y * rect.height;
1052+
const left = mark.x * layout.width;
1053+
const top = mark.y * layout.height;
10311054
rects.push({ left, top, right: left + 1, bottom: top + 1 });
10321055
}
10331056
}
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// @vitest-environment jsdom
2+
3+
import { fireEvent, render, waitFor } from '@testing-library/react';
4+
import { describe, expect, it, vi } from 'vitest';
5+
6+
import { PreviewDrawOverlay } from '../../src/components/PreviewDrawOverlay';
7+
8+
// In a scaled tablet/phone device frame the overlay lives inside a
9+
// `transform: scale()` shell: getBoundingClientRect() reports the on-screen
10+
// (scaled) size while offsetWidth/Height keep the untransformed layout size.
11+
// The structured annotation bounds must be layout-space — the same space the
12+
// composited screenshot uses — or the agent receives a rect shrunk by the fit
13+
// scale while the painted mark stays artifact-local (#6361).
14+
describe('PreviewDrawOverlay scaled device frame bounds', () => {
15+
function installImageCompositeMocks() {
16+
const originalImage = globalThis.Image;
17+
class MockImage {
18+
onload: (() => void) | null = null;
19+
onerror: (() => void) | null = null;
20+
set src(_value: string) {
21+
window.setTimeout(() => this.onload?.(), 0);
22+
}
23+
}
24+
Object.defineProperty(globalThis, 'Image', {
25+
configurable: true,
26+
value: MockImage,
27+
writable: true,
28+
});
29+
const getContext = vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockImplementation((() => ({
30+
beginPath: vi.fn(),
31+
clearRect: vi.fn(),
32+
drawImage: vi.fn(),
33+
fillRect: vi.fn(),
34+
fillText: vi.fn(),
35+
lineCap: 'round',
36+
lineJoin: 'round',
37+
lineTo: vi.fn(),
38+
lineWidth: 1,
39+
measureText: vi.fn(() => ({ width: 10 })),
40+
moveTo: vi.fn(),
41+
restore: vi.fn(),
42+
save: vi.fn(),
43+
scale: vi.fn(),
44+
setLineDash: vi.fn(),
45+
stroke: vi.fn(),
46+
strokeRect: vi.fn(),
47+
strokeStyle: '',
48+
fillStyle: '',
49+
font: '',
50+
})) as unknown as typeof HTMLCanvasElement.prototype.getContext);
51+
const toBlob = vi
52+
.spyOn(HTMLCanvasElement.prototype, 'toBlob')
53+
.mockImplementation((callback: BlobCallback) => callback(new Blob(['png'], { type: 'image/png' })));
54+
return () => {
55+
Object.defineProperty(globalThis, 'Image', {
56+
configurable: true,
57+
value: originalImage,
58+
writable: true,
59+
});
60+
getContext.mockRestore();
61+
toBlob.mockRestore();
62+
};
63+
}
64+
65+
it('sends layout-space bounds when the frame is fit-to-window scaled', async () => {
66+
const restoreCompositeMocks = installImageCompositeMocks();
67+
const annotation = vi.fn((event: Event) => {
68+
const detail = (event as CustomEvent<{ ack?: (result: { ok: boolean }) => void }>).detail;
69+
detail.ack?.({ ok: true });
70+
});
71+
window.addEventListener('opendesign:annotation', annotation);
72+
73+
try {
74+
const { container, getByRole } = render(
75+
<PreviewDrawOverlay
76+
active
77+
captureViewport
78+
captureSnapshot={async () => ({ dataUrl: 'data:image/png;base64,cG5n', w: 820, h: 1180 })}
79+
>
80+
<div style={{ width: 320, height: 200 }} />
81+
</PreviewDrawOverlay>,
82+
);
83+
84+
const canvas = container.querySelector<HTMLCanvasElement>('canvas')!;
85+
// Layout box: full 820×1180 device frame. On-screen rect: scaled to 42%.
86+
Object.defineProperty(canvas, 'offsetWidth', { configurable: true, get: () => 820 });
87+
Object.defineProperty(canvas, 'offsetHeight', { configurable: true, get: () => 1180 });
88+
canvas.getBoundingClientRect = () =>
89+
({
90+
x: 0, y: 0, left: 0, top: 0,
91+
right: 820 * 0.42, bottom: 1180 * 0.42,
92+
width: 820 * 0.42, height: 1180 * 0.42,
93+
toJSON: () => ({}),
94+
}) as DOMRect;
95+
96+
// Draw a box over the middle of the *on-screen* frame: client px are
97+
// scaled, so the normalized fractions are what a real pointer produces.
98+
fireEvent.pointerDown(canvas, { clientX: 0.42 * 100, clientY: 0.42 * 300, pointerId: 1 });
99+
fireEvent.pointerMove(canvas, { clientX: 0.42 * 500, clientY: 0.42 * 400, pointerId: 1 });
100+
fireEvent.pointerUp(canvas, { clientX: 0.42 * 500, clientY: 0.42 * 400, pointerId: 1 });
101+
102+
fireEvent.click(getByRole('button', { name: 'Send' }));
103+
104+
await waitFor(() => expect(annotation).toHaveBeenCalledTimes(1));
105+
const detail = (annotation.mock.calls[0]?.[0] as CustomEvent).detail as {
106+
bounds?: { x: number; y: number; width: number; height: number };
107+
};
108+
// Layout-space expectation: fractions × layout size (not × scaled rect).
109+
expect(detail.bounds).toBeDefined();
110+
expect(detail.bounds!.x).toBeCloseTo(100, 0);
111+
expect(detail.bounds!.y).toBeCloseTo(300, 0);
112+
expect(detail.bounds!.width).toBeCloseTo(400, 0);
113+
expect(detail.bounds!.height).toBeCloseTo(100, 0);
114+
} finally {
115+
window.removeEventListener('opendesign:annotation', annotation);
116+
restoreCompositeMocks();
117+
}
118+
});
119+
});

0 commit comments

Comments
 (0)