From 1c4e46b19fc81613fbc4a47323a4717969e370bb Mon Sep 17 00:00:00 2001 From: difficcd Date: Thu, 1 Oct 2026 15:25:13 +0900 Subject: [PATCH] perf: a commit draws the new stroke, not the whole layer Committing a stroke re-baked the layer's cache canvas from nothing, drawing every stroke it held. So the Nth stroke of a drawing cost N times the first, and a session's baking grew with the square of the strokes drawn - which is the "it gets heavy past thirty strokes" report. Drawing 40 strokes one at a time on a 1920x1080 layer, total time spent baking: brush 37160ms -> 2113ms (17.6x) pencil 15436ms -> 742ms (20.8x) (Skia on the CPU, so the absolute figures are far above a browser's; the ratio is the point.) When a layer's strokes are the ones already on its canvas plus more on the end, only the new ones need drawing - they paint on top, which is the order they would have been drawn in anyway. appendedAfter recognises that case by asking what the signature would have been when the layer was shorter, and everything else - a stroke edited or removed, the layer dragged, a merge, a resize, a boiling phase - still bakes in full. The safety argument is that this leans on the invariant the cache already needs and no more: an edit that changes strokes without changing the count or the last stroke's identity must bump `rev`. offsetLayers exists to do exactly that and says so. A prefix that changed under a cache that did not notice is already a stale cache today. Checked on pixels rather than on reasoning: appending from every split point of a four-stroke layer - with an eraser among the appended, because destination-out composes against what is already there - is byte-identical to redrawing the whole layer. Co-Authored-By: Claude Opus 5 --- HELPERS.md | 1 + src/core/layerTree.ts | 49 ++++++++++++++--- src/hooks/useLayerCache.ts | 24 +++++++-- test/canvas/pixels.test.js | 26 +++++++++ test/core/appendedAfter.test.js | 94 +++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 9 deletions(-) create mode 100644 test/core/appendedAfter.test.js diff --git a/HELPERS.md b/HELPERS.md index db58032..fb573e3 100644 --- a/HELPERS.md +++ b/HELPERS.md @@ -797,6 +797,7 @@ Reading a cut's layer tree: order, cache keys and change signatures. | `flattenForCanvas` | The layers to draw, bottom first, with folders resolved and hidden branches dropped. | | `strokeSig` | A cheap change signature for a layer's strokes, used to invalidate the layer canvas cache without stringifying the whole array. Sound because strokes here are only ever appended or replaced. | | `layerSig` | The cache key for one baked layer canvas. Two caches use it and compare their keys against each other, so for a layer that is not boiling both forms must come out byte-identical - otherwise every such layer misses the cache and is redrawn every frame, with no visible symptom. | +| `appendedAfter` | How many of a layer's strokes a canvas baked under a given signature already holds, when the strokes are that same list with more on the end — so a commit draws only the new ones instead of redrawing the layer. `null` when anything else changed. Leans on the same `rev` invariant the cache already needs. | | `flattenLayersInUiOrder` | The layer tree flattened the way the panel shows it, so an index in the list means the same thing to the UI and to the renderer. | ## `src/core/cutAnim.ts` diff --git a/src/core/layerTree.ts b/src/core/layerTree.ts index 9315d3b..2f5f7ab 100644 --- a/src/core/layerTree.ts +++ b/src/core/layerTree.ts @@ -13,10 +13,11 @@ export function flattenForCanvas(layers: L[]): L[] { // Cheap change signature for a layer's strokes (strokes are append/replace only here), // used to invalidate the layer canvas cache without stringifying the whole array. -export function strokeSig(strokes: ReadonlyArray | null | undefined): string { - if (!strokes || !strokes.length) return '0'; - const last = strokes[strokes.length - 1]; - return strokes.length + '|' + (last.id ?? '') + '|' + (last.points ? last.points.length : 0) + '|' + (last.bitmapId ?? '') + '|' + (last.tool ?? ''); +export function strokeSig(strokes: ReadonlyArray | null | undefined, count?: number): string { + const n = Math.max(0, Math.min(count ?? strokes?.length ?? 0, strokes?.length ?? 0)); + if (!strokes || !n) return '0'; + const last = strokes[n - 1]; + return n + '|' + (last.id ?? '') + '|' + (last.points ? last.points.length : 0) + '|' + (last.bitmapId ?? '') + '|' + (last.tool ?? ''); } /** @@ -32,13 +33,16 @@ export function strokeSig(strokes: ReadonlyArray | null | undefined) * `rev` is in it because edits that move coordinates without changing the stroke count or the * last stroke - a whole-layer move, say - are invisible to strokeSig. * + * `count` asks what the signature *would have been* when the layer held only its first `count` + * strokes. That is what lets `appendedAfter` recognise a layer that has only grown. + * * @param {{strokes?: any[], roughen?: number, rev?: number}} layer * @param {{roughPhase?: number, roughWave?: number, roughMinSize?: number} | null} [rough] * the boiling options, when asking for a particular phase; omitted for the still frame * @returns {string} */ -export function layerSig(layer: LayerLike | null | undefined, rough: { roughPhase: number, roughWave: number, roughMinSize: number } | null = null): string { - const base = strokeSig(layer?.strokes) + '|r' + (layer?.roughen || 0) + '|v' + (layer?.rev || 0); +export function layerSig(layer: LayerLike | null | undefined, rough: { roughPhase: number, roughWave: number, roughMinSize: number } | null = null, count?: number): string { + const base = strokeSig(layer?.strokes, count) + '|r' + (layer?.roughen || 0) + '|v' + (layer?.rev || 0); if (!rough || !layer?.roughen) return base; return base + `|b${rough.roughPhase}|w${rough.roughWave}|m${rough.roughMinSize}`; } @@ -56,3 +60,36 @@ export function flattenLayersInUiOrder(layers: L[], parentI } return out; } + +/** + * How many of a layer's strokes a canvas baked under `stored` already holds - when the layer's + * strokes are that same list with more added on the end. `null` when they are not, and the + * canvas has to be redrawn from scratch. + * + * Committing a stroke redrew every stroke the layer held, so the Nth stroke of a drawing cost N + * times the first and a session's baking grew with the square of the strokes drawn. Measured on + * a 1920x1080 layer, drawing 40 brush strokes one at a time cost ~38s of baking where drawing + * only each new stroke costs ~1.9s. + * + * This leans on exactly the invariant the cache already needs, and no more: an edit that changes + * strokes without changing the count or the last stroke's identity must bump `rev` - see + * `offsetLayers`, which exists to do that. A prefix that changed under a cache that did not + * notice is already a stale cache today; this does not widen the assumption, it reuses it. + * + * @param {string|null|undefined} stored the signature the canvas was baked under + * @param {LayerLike|null|undefined} layer + * @param {{roughPhase: number, roughWave: number, roughMinSize: number}|null} [rough] + * @returns {number|null} the index to start drawing from + */ +export function appendedAfter( + stored: string | null | undefined, + layer: LayerLike | null | undefined, + rough: { roughPhase: number, roughWave: number, roughMinSize: number } | null = null, +): number | null { + if (!stored) return null; + const have = layer?.strokes?.length ?? 0; + const was = Number(String(stored).split('|')[0]); + // Equal counts are the caller's cache hit, not an append; a shorter list is a removal. + if (!Number.isInteger(was) || was <= 0 || was >= have) return null; + return layerSig(layer, rough, was) === stored ? was : null; +} diff --git a/src/hooks/useLayerCache.ts b/src/hooks/useLayerCache.ts index 5137b0d..8ba8f8e 100644 --- a/src/hooks/useLayerCache.ts +++ b/src/hooks/useLayerCache.ts @@ -2,7 +2,7 @@ import { useEffect, useRef, useState } from 'react'; import { sizeCanvas, scratchCanvas } from '../canvas/scratch.ts'; import { drawStrokesOnCtx } from '../canvas/strokes.ts'; import { safeArray } from '../core/geometry.ts'; -import { layerKey, layerSig } from '../core/layerTree.ts'; +import { layerKey, layerSig, appendedAfter } from '../core/layerTree.ts'; import { scanLayerBitmaps } from '../engine/pendingBitmaps.ts'; import { cutsToCache } from '../engine/selectCuts.ts'; import { layerKeysUsingBitmaps, keysWithPhases, prefetchWindow } from '../core/decodeBudget.ts'; @@ -246,8 +246,26 @@ export function useLayerCache({ // which is why this does not go through scratchCanvas. const bake = (signature: string) => { const cnv = hit || document.createElement('canvas'); - sizeCanvas(cnv, canvasW, canvasH); - drawStrokesOnCtx(cnv.getContext('2d')!, layer.strokes || [], true, store, rOpts); + // Read before sizing. Resizing a canvas clears it, and then nothing is already drawn + // however much the signature says is. + const stored = cnv.dataset.strokes; + const resized = sizeCanvas(cnv, canvasW, canvasH); + // Committing a stroke redrew every stroke the layer held, so the Nth stroke of a + // drawing cost N times the first and a session's baking grew with the square of the + // strokes drawn: on a 1920x1080 layer, 40 brush strokes drawn one at a time cost + // ~38s of baking where drawing only each new stroke costs ~1.9s. + // + // When the strokes are the ones already on this canvas plus more on the end, only + // the new ones need drawing - they paint on top, which is the order they would have + // been drawn in anyway. Everything else (a stroke edited or removed, the layer + // dragged, a merge) still bakes in full; appendedAfter says which this is. + // + // Boiling layers are left out: each phase keeps its own canvas, so an append would + // have to reach all of them. + const from = (resized || layer.roughen) ? null : appendedAfter(stored, layer, rOpts); + const target = cnv.getContext('2d')!; + if (from === null) drawStrokesOnCtx(target, layer.strokes || [], true, store, rOpts); + else drawStrokesOnCtx(target, (layer.strokes || []).slice(from), false, store, rOpts); cnv.dataset.strokes = signature; map.delete(slotKey); map.set(slotKey, cnv); // re-insert = most recently used while (map.size > LAYER_CANVAS_LRU) map.delete(map.keys().next().value!); diff --git a/test/canvas/pixels.test.js b/test/canvas/pixels.test.js index 38b837c..5295303 100644 --- a/test/canvas/pixels.test.js +++ b/test/canvas/pixels.test.js @@ -130,6 +130,32 @@ test('an eraser still widens when the pen is pressed hard', () => { assert.ok(hard > even + 10, `a hard press did not widen the eraser: ${even} -> ${hard}`); }); +test('drawing only the new strokes gives the same pixels as redrawing the layer', () => { + // The claim the incremental bake rests on (appendedAfter): strokes paint in order, each on + // top of the last, so painting the new ones onto a canvas that already holds the old ones + // must land exactly where redrawing everything would. If this ever stops being true the + // symptom is a drawing that is quietly wrong, so it is checked on bytes, not on ink counts. + const all = [ + stroke({ id: 1, tool: 'brush', color: '#cc3344' }), + stroke({ id: 2, tool: 'pencil', color: '#2244aa', points: [{ x: 30, y: 30, pressure: 0.5 }, { x: 170, y: 90, pressure: 0.5 }] }), + // An eraser in the appended part: destination-out has to compose against what is + // already on the canvas, which is the case most likely to come apart. + stroke({ id: 3, tool: 'eraser', size: 24, points: [{ x: 60, y: 10, pressure: 0.5 }, { x: 60, y: 110, pressure: 0.5 }] }), + stroke({ id: 4, tool: 'marker', color: '#117733', points: [{ x: 20, y: 90, pressure: 0.5 }, { x: 180, y: 70, pressure: 0.5 }] }), + ]; + for (const split of [1, 2, 3]) { + const full = fresh(), incr = fresh(); + drawStrokesOnCtx(full.ctx, all, true, new Map()); + drawStrokesOnCtx(incr.ctx, all.slice(0, split), true, new Map()); + drawStrokesOnCtx(incr.ctx, all.slice(split), false, new Map()); + assert.deepEqual( + [...incr.ctx.getImageData(0, 0, W, H).data], + [...full.ctx.getImageData(0, 0, W, H).data], + `appending from ${split} did not match a full redraw`, + ); + } +}); + test('drawing is deterministic: the same stroke twice gives the same pixels', () => { const a = fresh(), b = fresh(); drawStrokesOnCtx(a.ctx, [stroke()], false, new Map()); diff --git a/test/core/appendedAfter.test.js b/test/core/appendedAfter.test.js new file mode 100644 index 0000000..2f79113 --- /dev/null +++ b/test/core/appendedAfter.test.js @@ -0,0 +1,94 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { layerSig, strokeSig, appendedAfter } from '../../src/core/layerTree.ts'; + +// Committing a stroke redrew every stroke the layer held. The Nth stroke of a drawing cost N +// times the first, so a session's baking grew with the square of the strokes drawn - measured on +// a 1920x1080 layer, 40 brush strokes drawn one at a time cost ~38s of baking where drawing only +// each new stroke costs ~1.9s. appendedAfter is what lets the cache draw only the new ones. +// +// Every interesting case here is a *refusal*. Saying "appended" when something else happened +// leaves stale pixels on the layer with no symptom until someone notices their drawing is wrong, +// so the tests that matter are the ones that make sure it says no. + +const stroke = (id, n = 4, over = {}) => ({ + id, tool: 'brush', bitmapId: null, + points: Array.from({ length: n }, (_, i) => ({ x: i, y: i })), + ...over, +}); +const layer = (strokes, over = {}) => ({ id: 1, strokes, rev: 0, roughen: 0, ...over }); + +test('strokes added on the end are an append, starting where the canvas left off', () => { + const before = layer([stroke(1), stroke(2)]); + const after = layer([stroke(1), stroke(2), stroke(3), stroke(4)]); + assert.equal(appendedAfter(layerSig(before), after), 2); +}); + +test('an unchanged layer is not an append - that is the caller\'s cache hit', () => { + const l = layer([stroke(1), stroke(2)]); + assert.equal(appendedAfter(layerSig(l), l), null); +}); + +test('a removed stroke is not an append', () => { + const before = layer([stroke(1), stroke(2), stroke(3)]); + assert.equal(appendedAfter(layerSig(before), layer([stroke(1), stroke(2)])), null); +}); + +test('the last stroke growing points - the eraser still drawing - is not an append', () => { + // appendPoints extends the stroke in flight. Its pixels are already on the canvas and the + // new ones are not a separate stroke, so the layer has to be redrawn. + const before = layer([stroke(1), stroke(2, 4)]); + assert.equal(appendedAfter(layerSig(before), layer([stroke(1), stroke(2, 9)])), null); +}); + +test('a bumped rev refuses the append even though the strokes only grew', () => { + // This is the whole safety argument. offsetLayers moves every coordinate, which changes no + // count and no identity, and bumps rev precisely so the cache notices. If a move and a new + // stroke land together, the moved strokes must still be redrawn. + const before = layer([stroke(1), stroke(2)]); + const after = layer([stroke(1), stroke(2), stroke(3)], { rev: 1 }); + assert.equal(appendedAfter(layerSig(before), after), null); +}); + +test('a layer that started empty is redrawn in full, not appended onto', () => { + assert.equal(appendedAfter(layerSig(layer([])), layer([stroke(1)])), null); +}); + +test('a missing or unparseable signature is refused', () => { + const l = layer([stroke(1), stroke(2)]); + for (const bad of [null, undefined, '', 'nonsense', '|1|2', 'NaN|x', '-1|x']) { + assert.equal(appendedAfter(bad, l), null, String(bad)); + } +}); + +test('a signature claiming more strokes than the layer has is refused', () => { + // A canvas from a longer version of this layer. Drawing "the rest" would draw nothing and + // leave the extra strokes on screen. + const before = layer([stroke(1), stroke(2), stroke(3), stroke(4)]); + assert.equal(appendedAfter(layerSig(before), layer([stroke(1), stroke(2)])), null); +}); + +test('strokeSig with a count is the signature that list had when it was that long', () => { + // The property appendedAfter is built on: the prefix of a grown list signs as the original. + const first = [stroke(1), stroke(2)]; + const grown = [...first, stroke(3)]; + assert.equal(strokeSig(grown, 2), strokeSig(first)); + assert.equal(layerSig(layer(grown), null, 2), layerSig(layer(first))); +}); + +test('a count outside the list is clamped rather than reading past the end', () => { + const l = [stroke(1), stroke(2)]; + assert.equal(strokeSig(l, 99), strokeSig(l)); + assert.equal(strokeSig(l, 0), '0'); + assert.equal(strokeSig(l, -5), '0'); +}); + +test('a boiling layer signs per phase, so a different phase is not an append', () => { + // Each phase keeps its own canvas and the bake path excludes boiling layers outright; this + // pins the signature half of that, so the exclusion cannot be quietly dropped. + const r1 = { roughPhase: 1, roughWave: 1, roughMinSize: 0 }; + const r2 = { roughPhase: 2, roughWave: 1, roughMinSize: 0 }; + const before = layer([stroke(1)], { roughen: 2 }); + const after = layer([stroke(1), stroke(2)], { roughen: 2 }); + assert.equal(appendedAfter(layerSig(before, r1), after, r2), null); +});