From 3a68067079d6190b3879a75ee04bb8b5401271d7 Mon Sep 17 00:00:00 2001 From: MelekTaus Date: Sun, 20 Sep 2026 21:55:02 +0200 Subject: [PATCH] Dragging a point in the vector art tab keeps up with the mouse (#129) Dragging a point only redrew once the hand stopped moving, which the user reported after stage 3 and which measures as a real backlog, not a feeling: every pointer move wrote the model and fired state.preview(), and a project event rebuilds the panels, the run in the editor and the picture the drawing is rasterised into. A mouse reports about 125 times a second while the screen takes 60, so the queue grew instead of the drawing moving. - Measured with a throwaway e2e probe sending 120 moves at the 8 ms of a real mouse, against a growing scene. Per move event: 10 -> 7 ms with an empty scene, 19 -> 4 ms at 40 objects, 36 -> 3 ms at 120. The 120 moves took 4173 ms before and 960 ms after, which is the time they were supposed to take. - New onFrame(key, fn) in src/ui/render.ts collects such work per key and runs it in the ticker before app.render(), because afterwards it would be one frame late in the picture. The rule lives in ui/ rather than in the canvas so a second caller gets it for free. - VectorArtCanvas only queues in pointermove now, and cancels its slot on pointer up, where the command writes the final state anyway. During the gesture the drag's own copy of the rings is the truth and the model is never read back, so nothing is lost by the delay. The same cost sits in the scene viewport, the texture tab and the graph, which all call preview() from their move handler. They are untouched: that is outside the reported bug. Tests: check.sh 127 files, 1883 tests, exit 0 (was 1878). Full e2e suite locally instead of CI only because the change sits in the render loop: ALL PASSED - 1172 checks, the sum unchanged - a timing check in the browser would flutter under CI load. Five new unit tests in tests/ui/render.test.ts. Two counter-checks hold: running the work after the frame fails one unit test, and taking the coalescing out of the canvas puts the measurement back at 36 ms and 4153 ms. Co-Authored-By: Claude Opus 5 --- src/editor/vectorart/VectorArtCanvas.ts | 35 +++++++++++-- src/ui/index.ts | 2 + src/ui/render.ts | 29 +++++++++++ tests/ui/render.test.ts | 69 +++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/editor/vectorart/VectorArtCanvas.ts b/src/editor/vectorart/VectorArtCanvas.ts index f5f27c27..6b5cde46 100644 --- a/src/editor/vectorart/VectorArtCanvas.ts +++ b/src/editor/vectorart/VectorArtCanvas.ts @@ -22,7 +22,17 @@ import { findAsset } from '../../core/model/Project'; import { cloneVectorRing, findShape, handleAt, vectorCanvasSize } from '../../core/model/vectorart'; import { vectorArtDrawings } from '../../core/model/vectorPath'; import { paintDrawing } from '../../viewport/vectorArtPicture'; -import { pressCount, type Size, Theme, textStyle, ui, Widget, zoomFactor } from '../../ui'; +import { + cancelFrameWork, + onFrame, + pressCount, + type Size, + Theme, + textStyle, + ui, + Widget, + zoomFactor, +} from '../../ui'; import { insideRing, type RingGrip, @@ -68,6 +78,9 @@ interface ShapeDrag { label: string; } +/** Counts canvases so each gets its own key in the frame queue. */ +let instances = 0; + export class VectorArtCanvas extends Widget { private readonly bg = new Graphics(); /** Everything the view moves: the drawing, scaled by the zoom, and the overlay above it. */ @@ -86,6 +99,8 @@ export class VectorArtCanvas extends Widget { private drag: ShapeDrag | null = null; /** The ring the pen is drawing into; empty while no line is open. */ private pen = { shapeId: '', ring: -1 }; + /** This canvas's slot in the frame queue: one pending preview at a time, see `preview()`. */ + private readonly frameKey = `vectorart:${(instances += 1)}`; constructor(private readonly state: EditorState) { super(); @@ -409,11 +424,23 @@ export class VectorArtCanvas extends Widget { this.preview(); } - /** Writes the gesture into the model as a preview; no undo step until the pointer is up. */ + /** + * Writes the gesture into the model as a preview; no undo step until the pointer is up. + * + * Once per frame, not once per pointer move: a model event rebuilds the panels, the run in the + * editor and the picture the drawing is rasterised into, which costs more than a frame on a busy + * scene, so a mouse reporting faster than the screen refreshes would leave the drawing behind + * until the hand stopped. `drag.rings` is the truth in the meantime and the gesture reads it, + * not the model. + */ private preview(): void { + onFrame(this.frameKey, () => this.writePreview()); + } + + private writePreview(): void { const drag = this.drag; const shape = drag && this.shape(drag.shapeId); - if (!drag || !shape) return; + if (this.destroyed || !drag || !shape) return; shape.rings = drag.rings; this.state.preview(); } @@ -422,6 +449,8 @@ export class VectorArtCanvas extends Widget { this.panning = null; const drag = this.drag; this.drag = null; + // The gesture is over, so a preview still waiting for its frame has nothing left to write. + cancelFrameWork(this.frameKey); const shape = drag && this.shape(drag.shapeId); if (!drag || !shape) return; // The preview moved it already; put it back so the command records the state that was there. diff --git a/src/ui/index.ts b/src/ui/index.ts index 41f5b371..dd393f9c 100644 --- a/src/ui/index.ts +++ b/src/ui/index.ts @@ -4,7 +4,9 @@ export { Theme, textStyle, type TextStyleToken } from './Theme'; export { initUI, ui, type UIContext } from './UIContext'; export { afterFrames, + cancelFrameWork, initRenderLoop, + onFrame, isRendering, renderCount, requestRender, diff --git a/src/ui/render.ts b/src/ui/render.ts index 69241bc4..c5e404f0 100644 --- a/src/ui/render.ts +++ b/src/ui/render.ts @@ -37,6 +37,28 @@ export function renderCount(): number { return rendered; } +/** Work put off to the next frame by `onFrame`, one entry per key. */ +let frameWork = new Map void>(); + +/** + * Runs `fn` once before the next frame is drawn, and only once however often it is asked for under + * the same key – the answer to a gesture that costs more per pointer event than a frame is worth. + * + * A drag writes the model on every pointer move, and a model event rebuilds panels, the run in the + * editor and any picture drawn from it. A mouse reports around 125 times a second while the screen + * takes 60, so without this the pointer outruns the drawing and the editor only catches up once the + * hand stops (#129). Coalescing here rather than in each canvas keeps the rule in one place. + */ +export function onFrame(key: string, fn: () => void): void { + frameWork.set(key, fn); + requestRender(); +} + +/** Drops work queued under `key` – a gesture that ended has nothing left to catch up on. */ +export function cancelFrameWork(key: string): void { + frameWork.delete(key); +} + /** Resolved once the next frame has been drawn; see `afterFrames`. */ let frameWaiters: Array<() => void> = []; @@ -80,6 +102,12 @@ export function initRenderLoop(app: Application): void { app.ticker.add( () => { if (!isRendering()) return; + // Before the frame, not after: what the work writes is meant to be in the picture it delays. + if (frameWork.size) { + const due = frameWork; + frameWork = new Map(); + for (const fn of due.values()) fn(); + } rendered += 1; app.render(); // Taken first: a waiter that queues itself again must land in the next round, not this one. @@ -98,4 +126,5 @@ export function resetRenderLoop(): void { deadline = 0; rendered = 0; frameWaiters = []; + frameWork = new Map(); } diff --git a/tests/ui/render.test.ts b/tests/ui/render.test.ts index 9d964220..e1196bb6 100644 --- a/tests/ui/render.test.ts +++ b/tests/ui/render.test.ts @@ -2,8 +2,10 @@ import type { Application } from 'pixi.js'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { afterFrames, + cancelFrameWork, initRenderLoop, isRendering, + onFrame, renderCount, requestRender, resetRenderLoop, @@ -125,6 +127,73 @@ describe('render loop', () => { expect(app.drawn).toBe(1); }); + it('runs work put off to a frame once, however often it was asked for', () => { + const { app, tick } = fakeApp(); + initRenderLoop(app as unknown as Application); + now += 500; + + let ran = 0; + for (let i = 0; i < 20; i++) onFrame('drag', () => (ran += 1)); + expect(ran).toBe(0); + tick(); + expect(ran).toBe(1); + + // And it is gone afterwards: the next frame has nothing left to do. + tick(); + expect(ran).toBe(1); + }); + + it('keeps the work of two keys apart, and the last one wins per key', () => { + const { app, tick } = fakeApp(); + initRenderLoop(app as unknown as Application); + now += 500; + + const ran: string[] = []; + onFrame('a', () => ran.push('a1')); + onFrame('a', () => ran.push('a2')); + onFrame('b', () => ran.push('b')); + tick(); + expect(ran).toEqual(['a2', 'b']); + }); + + it('runs the work before the frame it is meant to be in', () => { + const { app, tick } = fakeApp(); + initRenderLoop(app as unknown as Application); + now += 500; + + let drawnWhenRun = -1; + onFrame('drag', () => (drawnWhenRun = app.drawn)); + tick(); + expect(drawnWhenRun).toBe(0); + expect(app.drawn).toBe(1); + }); + + it('wakes the loop itself, so work asked for by an idle editor is not lost', () => { + const { app, tick } = fakeApp(); + initRenderLoop(app as unknown as Application); + now += 500; + tick(); + expect(app.drawn).toBe(0); + + let ran = 0; + onFrame('drag', () => (ran += 1)); + tick(); + expect(ran).toBe(1); + }); + + it('cancelled work never runs – a gesture that ended writes nothing more', () => { + const { app, tick } = fakeApp(); + initRenderLoop(app as unknown as Application); + now += 500; + + let ran = 0; + onFrame('drag', () => (ran += 1)); + onFrame('other', () => (ran += 10)); + cancelFrameWork('drag'); + tick(); + expect(ran).toBe(10); + }); + it('afterFrames gives up rather than hanging when no frame ever comes', async () => { vi.useFakeTimers(); try {