From c408a728d6fe68be3be77540686ab8fc35b1d536 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 08:09:44 -0500 Subject: [PATCH] fix(desktop): long annotation strokes stay cheap to draw Each pointer move in the Draw tool copied every point, rescanned them through Math.min/Math.max spreads and rebuilt the whole SVG path. Points are now appended in place, bounds widen per point, and the stroke is drawn as a run of 100-segment paths so only the last one is rewritten. Fixes #8059 Co-Authored-By: Claude Opus 5.5 --- .../src/preview/AnnotationStroke.test.ts | 73 +++++++++++++++ apps/desktop/src/preview/AnnotationStroke.ts | 69 ++++++++++++++ apps/desktop/src/preview/PickPreload.ts | 91 +++++++++---------- 3 files changed, 183 insertions(+), 50 deletions(-) create mode 100644 apps/desktop/src/preview/AnnotationStroke.test.ts create mode 100644 apps/desktop/src/preview/AnnotationStroke.ts diff --git a/apps/desktop/src/preview/AnnotationStroke.test.ts b/apps/desktop/src/preview/AnnotationStroke.test.ts new file mode 100644 index 0000000000000..ad8a21c140b45 --- /dev/null +++ b/apps/desktop/src/preview/AnnotationStroke.test.ts @@ -0,0 +1,73 @@ +import type { PreviewAnnotationPoint } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { + addStrokePoint, + openStrokePath, + startStroke, + strokeBounds, + STROKE_SEGMENTS_PER_PATH, +} from "./AnnotationStroke.ts"; + +// The Draw tool used to rebuild the whole path and rescan every point on each move. +const fullPath = (points: ReadonlyArray): string => { + let path = `M ${points[0]!.x} ${points[0]!.y}`; + for (let index = 1; index < points.length - 1; index += 1) { + const current = points[index]!; + const next = points[index + 1]!; + path += ` Q ${current.x} ${current.y} ${(current.x + next.x) / 2} ${(current.y + next.y) / 2}`; + } + const last = points[points.length - 1]!; + return `${path} L ${last.x} ${last.y}`; +}; + +const fullBounds = (points: ReadonlyArray, width: number) => { + const xs = points.map((point) => point.x); + const ys = points.map((point) => point.y); + const padding = width + 3; + const left = Math.min(...xs) - padding; + const top = Math.min(...ys) - padding; + const right = Math.max(...xs) + padding; + const bottom = Math.max(...ys) + padding; + return { x: left, y: top, width: right - left, height: bottom - top }; +}; + +// Each later path starts with a move to where the previous one ended. +const joinPaths = (paths: ReadonlyArray): string => + paths + .map((path, index) => { + if (index === 0) return path; + const resume = path.match(/^M (\S+ \S+)/)![1]!; + expect(paths[index - 1]!.endsWith(` ${resume}`)).toBe(true); + return path.slice(`M ${resume}`.length); + }) + .join(""); + +describe("stroke geometry", () => { + it("draws the same curve and bounds as a full rescan at every length", () => { + const points = Array.from({ length: STROKE_SEGMENTS_PER_PATH * 2 + 30 }, (_, index) => ({ + x: index * 3 + ((index * 7) % 11), + y: 200 + ((index * 13) % 29) - index, + })); + const geometry = startStroke(points[0]!); + const closedPaths: string[] = []; + for (let index = 1; index < points.length; index += 1) { + const closedPath = addStrokePoint(geometry, points[index]!); + if (closedPath !== null) closedPaths.push(closedPath); + const drawn = points.slice(0, index + 1); + expect(joinPaths([...closedPaths, openStrokePath(geometry)])).toBe(fullPath(drawn)); + expect(strokeBounds(geometry, 4)).toEqual(fullBounds(drawn, 4)); + } + expect(closedPaths).toHaveLength(2); + expect(geometry.points).toEqual(points); + }); + + it("keeps the rewritten path short for a stroke past the engine's argument limit", () => { + const geometry = startStroke({ x: 0, y: 0 }); + for (let index = 1; index < 250_000; index += 1) { + addStrokePoint(geometry, { x: index % 800, y: index % 600 }); + } + expect(strokeBounds(geometry, 4)).toEqual({ x: -7, y: -7, width: 813, height: 613 }); + expect(openStrokePath(geometry).length).toBeLessThan(STROKE_SEGMENTS_PER_PATH * 40); + }); +}); diff --git a/apps/desktop/src/preview/AnnotationStroke.ts b/apps/desktop/src/preview/AnnotationStroke.ts new file mode 100644 index 0000000000000..66510fc0b7cfe --- /dev/null +++ b/apps/desktop/src/preview/AnnotationStroke.ts @@ -0,0 +1,69 @@ +import type { PreviewAnnotationPoint, PreviewAnnotationRect } from "@t3tools/contracts"; + +// Setting a path's `d` makes Chromium parse the whole string again, so a long +// stroke is drawn as a run of paths and only the last one is rewritten per move. +export const STROKE_SEGMENTS_PER_PATH = 100; + +export interface StrokeGeometry { + readonly points: PreviewAnnotationPoint[]; + minX: number; + minY: number; + maxX: number; + maxY: number; + // The curve through the newest point is still open: where it ends depends on + // the point after it. This holds only the settled segments of the last path. + openPath: string; + openSegments: number; +} + +export function startStroke(point: PreviewAnnotationPoint): StrokeGeometry { + return { + points: [point], + minX: point.x, + minY: point.y, + maxX: point.x, + maxY: point.y, + openPath: `M ${point.x} ${point.y}`, + openSegments: 0, + }; +} + +// Returns the final `d` of the path this point filled, if it filled one. +export function addStrokePoint( + geometry: StrokeGeometry, + point: PreviewAnnotationPoint, +): string | null { + const previous = geometry.points[geometry.points.length - 1]!; + let closedPath: string | null = null; + if (geometry.points.length > 1) { + const midX = (previous.x + point.x) / 2; + const midY = (previous.y + point.y) / 2; + geometry.openPath += ` Q ${previous.x} ${previous.y} ${midX} ${midY}`; + geometry.openSegments += 1; + if (geometry.openSegments === STROKE_SEGMENTS_PER_PATH) { + closedPath = geometry.openPath; + geometry.openPath = `M ${midX} ${midY}`; + geometry.openSegments = 0; + } + } + geometry.points.push(point); + geometry.minX = Math.min(geometry.minX, point.x); + geometry.minY = Math.min(geometry.minY, point.y); + geometry.maxX = Math.max(geometry.maxX, point.x); + geometry.maxY = Math.max(geometry.maxY, point.y); + return closedPath; +} + +export function openStrokePath(geometry: StrokeGeometry): string { + const last = geometry.points[geometry.points.length - 1]!; + return `${geometry.openPath} L ${last.x} ${last.y}`; +} + +export function strokeBounds(geometry: StrokeGeometry, width: number): PreviewAnnotationRect { + const padding = width + 3; + const left = geometry.minX - padding; + const top = geometry.minY - padding; + const right = geometry.maxX + padding; + const bottom = geometry.maxY + padding; + return { x: left, y: top, width: right - left, height: bottom - top }; +} diff --git a/apps/desktop/src/preview/PickPreload.ts b/apps/desktop/src/preview/PickPreload.ts index 4059468da22e9..71c14e5a940e9 100644 --- a/apps/desktop/src/preview/PickPreload.ts +++ b/apps/desktop/src/preview/PickPreload.ts @@ -16,6 +16,13 @@ import type { import { createAnnotationIdSource } from "./AnnotationIds.ts"; import { resolveAnnotationSubmission } from "./AnnotationKeyboard.ts"; +import { + addStrokePoint, + openStrokePath, + startStroke, + strokeBounds, + type StrokeGeometry, +} from "./AnnotationStroke.ts"; import { previewAnnotationStyles } from "./AnnotationStyles.generated.ts"; import { installRecordingCursor } from "./RecordingCursor.ts"; import { DEFAULT_RECORDING_INPUT_OPTIONS } from "./RecordingInput.ts"; @@ -508,34 +515,6 @@ function createUnitInput(unit: string, placeholder = "0"): HTMLInputElement { return input; } -function pathFromPoints(points: ReadonlyArray): string { - if (points.length === 0) return ""; - if (points.length === 1) return `M ${points[0]!.x} ${points[0]!.y} l 0.01 0.01`; - let path = `M ${points[0]!.x} ${points[0]!.y}`; - for (let index = 1; index < points.length - 1; index += 1) { - const current = points[index]!; - const next = points[index + 1]!; - path += ` Q ${current.x} ${current.y} ${(current.x + next.x) / 2} ${(current.y + next.y) / 2}`; - } - const last = points[points.length - 1]!; - path += ` L ${last.x} ${last.y}`; - return path; -} - -function strokeBounds( - points: ReadonlyArray, - width: number, -): PreviewAnnotationRect { - const xs = points.map((point) => point.x); - const ys = points.map((point) => point.y); - const padding = width + 3; - const left = Math.min(...xs) - padding; - const top = Math.min(...ys) - padding; - const right = Math.max(...xs) + padding; - const bottom = Math.max(...ys) + padding; - return { x: left, y: top, width: right - left, height: bottom - top }; -} - function startAnnotation(sendEnabled: boolean): void { activeSession?.teardown(false); let finished = false; @@ -650,7 +629,12 @@ function startAnnotation(sendEnabled: boolean): void { } let tool: AnnotationTool = isPdf ? "marquee" : "select"; let dragStart: PreviewAnnotationPoint | null = null; - let activeStroke: { target: PreviewAnnotationStrokeTarget; path: SVGPathElement } | null = null; + let activeStroke: { + target: PreviewAnnotationStrokeTarget; + geometry: StrokeGeometry; + group: SVGGElement; + path: SVGPathElement; + } | null = null; let pendingCapture = false; let editorExpanded = false; let editorWasShown = false; @@ -1243,15 +1227,18 @@ function startAnnotation(sendEnabled: boolean): void { return; } if (tool === "draw" && activeStroke) { - activeStroke.target.points = [ - ...activeStroke.target.points, - { x: event.clientX, y: event.clientY }, - ]; - activeStroke.target.bounds = strokeBounds( - activeStroke.target.points, - activeStroke.target.width, - ); - activeStroke.path.setAttribute("d", pathFromPoints(activeStroke.target.points)); + const closedPath = addStrokePoint(activeStroke.geometry, { + x: event.clientX, + y: event.clientY, + }); + activeStroke.target.bounds = strokeBounds(activeStroke.geometry, activeStroke.target.width); + if (closedPath !== null) { + activeStroke.path.setAttribute("d", closedPath); + activeStroke.path = activeStroke.group.appendChild( + document.createElementNS("http://www.w3.org/2000/svg", "path"), + ); + } + activeStroke.path.setAttribute("d", openStrokePath(activeStroke.geometry)); } }; @@ -1270,23 +1257,27 @@ function startAnnotation(sendEnabled: boolean): void { } dragStart = { x: event.clientX, y: event.clientY }; if (tool === "draw") { + const geometry = startStroke(dragStart); const stroke: PreviewAnnotationStrokeTarget = { id: nextId("stroke"), color: annotationTheme?.primary ?? "#2563eb", width: 4, - points: [dragStart], + points: geometry.points, bounds: { x: dragStart.x, y: dragStart.y, width: 1, height: 1 }, }; - const path = document.createElementNS("http://www.w3.org/2000/svg", "path"); - path.setAttribute(OVERLAY_ATTRIBUTE, ""); - path.setAttribute("data-stroke-id", stroke.id); - path.setAttribute("fill", "none"); - path.setAttribute("stroke", stroke.color); - path.setAttribute("stroke-width", String(stroke.width)); - path.setAttribute("stroke-linecap", "round"); - path.setAttribute("stroke-linejoin", "round"); - svg.appendChild(path); - activeStroke = { target: stroke, path }; + const group = document.createElementNS("http://www.w3.org/2000/svg", "g"); + group.setAttribute(OVERLAY_ATTRIBUTE, ""); + group.setAttribute("data-stroke-id", stroke.id); + group.setAttribute("fill", "none"); + group.setAttribute("stroke", stroke.color); + group.setAttribute("stroke-width", String(stroke.width)); + group.setAttribute("stroke-linecap", "round"); + group.setAttribute("stroke-linejoin", "round"); + const path = group.appendChild( + document.createElementNS("http://www.w3.org/2000/svg", "path"), + ); + svg.appendChild(group); + activeStroke = { target: stroke, geometry, group, path }; } }; @@ -1313,7 +1304,7 @@ function startAnnotation(sendEnabled: boolean): void { } } else if (tool === "draw" && activeStroke) { if (activeStroke.target.points.length > 1) strokes.push(activeStroke.target); - else activeStroke.path.remove(); + else activeStroke.group.remove(); activeStroke = null; } dragStart = null;