diff --git a/AGENTS.md b/AGENTS.md index 1b56f9141..006a634ec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -191,6 +191,7 @@ Keep responsibilities distinct: engine tests cover state contracts, Playwright b - `requestRender()` bumps `renderVersion` and `sceneVersion`; `requestRepaint()` bumps only `renderVersion` - `renderNow()` is only for surface recreation and font loading (need immediate draw) - Resize observer uses rAF throttle, not debounce — debounce causes canvas skew +- Overscan images accelerate navigation; settled scenes rasterize existing retained pictures at the live viewport size/origin. Pixel-grid alignment alone does not guarantee Skia AA parity. Keep settlement pending until the viewport pass completes; do not add a second viewport image cache. - Viewport culling skips off-screen nodes; unclipped parents are NOT culled (children may extend beyond bounds) - Selection border width must be constant regardless of zoom — divide by scale - Section/frame title text never scales — render at fixed font size, ellipsize to fit diff --git a/CHANGELOG.md b/CHANGELOG.md index 166ff9d1c..423b2305d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,7 +59,7 @@ ### Fixed -- Keep property fields and paint previews live during editing, and keep selection labels attached and aligned during rotation. +- Keep property fields and paint previews live during editing, and keep rotated selection labels aligned and object edges stable when previews settle. - Keep Undo and Redo commands available as edit history changes, without requiring another scene edit. - Avoid recursive desktop HTTP proxy requests when font downloads intercept Tauri IPC traffic. - Keep FIT image fills proportional, centered, and fully visible without stretching or cropped edges. diff --git a/packages/core/src/canvas/renderer/pipeline.ts b/packages/core/src/canvas/renderer/pipeline.ts index d5183d67e..af32c1523 100644 --- a/packages/core/src/canvas/renderer/pipeline.ts +++ b/packages/core/src/canvas/renderer/pipeline.ts @@ -240,14 +240,12 @@ export function render( renderedScene = true p.setScenePictureMode('hit', tiled.covered ? 'tiled' : 'tiled-fallback') } - if ( - !renderedScene && - layer === 'scene' && - !requiresUncachedSceneRender && - renderSceneBacking(r, canvas, graph, sceneVersion) - ) { - renderedScene = true - p.setScenePictureMode('hit', 'backing') + if (!renderedScene && layer === 'scene' && !requiresUncachedSceneRender) { + const presentation = renderSceneBacking(r, canvas, graph, sceneVersion) + if (presentation) { + renderedScene = true + p.setScenePictureMode('hit', presentation) + } } if (!renderedScene) { canvas.translate(r.panX, r.panY) diff --git a/packages/core/src/canvas/renderer/retained-backing.ts b/packages/core/src/canvas/renderer/retained-backing.ts index 8e762a84e..46e8d9379 100644 --- a/packages/core/src/canvas/renderer/retained-backing.ts +++ b/packages/core/src/canvas/renderer/retained-backing.ts @@ -288,6 +288,55 @@ function cachedSubtreePicture( } } +function drawRetainedChild( + r: SkiaRenderer, + graph: SceneGraph, + canvas: Canvas, + childId: string, + sceneVersion: number, + renderingSceneBacking: boolean +): void { + const child = graph.getNode(childId) + const hasCacheableEffects = child?.effects.some( + (effect) => effect.visible && (effect.type === 'DROP_SHADOW' || effect.type === 'INNER_SHADOW') + ) + if (hasCacheableEffects) { + const previous = r.renderingSceneBacking + r.renderingSceneBacking = renderingSceneBacking + try { + r.renderNode(canvas, graph, childId, {}) + } finally { + r.renderingSceneBacking = previous + } + } else { + const picture = cachedSubtreePicture(r, graph, childId, sceneVersion) + if (picture) canvas.drawPicture(picture) + else r.renderNode(canvas, graph, childId, {}) + } +} + +function drawSettledScene( + r: SkiaRenderer, + canvas: Canvas, + graph: SceneGraph, + sceneVersion: number +): void { + // Analytic AA depends on framebuffer dimensions as well as pixel/quad phase. + // Reuse existing pictures at the live viewport origin and size. Overscan + // remains available for navigation; this creates no additional cache. + canvas.save() + try { + canvas.translate(r.panX, r.panY) + canvas.scale(r.zoom, r.zoom) + const page = graph.getNode(r.pageId ?? graph.rootId) + for (const childId of page?.childIds ?? []) { + drawRetainedChild(r, graph, canvas, childId, sceneVersion, false) + } + } finally { + canvas.restore() + } +} + function renderBackingChild( r: SkiaRenderer, graph: SceneGraph, @@ -309,24 +358,7 @@ function renderBackingChild( canvas.scale(r.dpr, r.dpr) canvas.translate(backing.panX, backing.panY) canvas.scale(r.zoom, r.zoom) - const previousRenderingSceneBacking = r.renderingSceneBacking - const child = graph.getNode(childId) - const hasCacheableEffects = child?.effects.some( - (effect) => - effect.visible && (effect.type === 'DROP_SHADOW' || effect.type === 'INNER_SHADOW') - ) - if (hasCacheableEffects) { - r.renderingSceneBacking = true - try { - r.renderNode(canvas, graph, childId, {}) - } finally { - r.renderingSceneBacking = previousRenderingSceneBacking - } - } else { - const picture = cachedSubtreePicture(r, graph, childId, sceneVersion) - if (picture) canvas.drawPicture(picture) - else r.renderNode(canvas, graph, childId, {}) - } + drawRetainedChild(r, graph, canvas, childId, sceneVersion, true) } finally { canvas.restore() r.worldViewport = prevViewport @@ -504,18 +536,16 @@ export function renderSceneBacking( canvas: Canvas, graph: SceneGraph, sceneVersion: number -): boolean { +): false | 'backing' | 'retained-pictures' { if (r.sceneBackingAllocationFailed) return false - const navigationActive = - r.navigationPhase === 'pan' || - r.navigationPhase === 'zoom' || - r.navigationPhase === 'momentum' || - r.navigationPhase === 'settling' + const navigationActive = r.navigationPhase !== 'idle' if (navigationActive && r.sceneBacking) { r.sceneBackingBuild?.surface.delete() r.sceneBackingBuild = null r.sceneBackingNeedsCrispRender = true return drawSceneBacking(r, canvas, sceneVersion, true, graph.positionPreviewVersion) + ? 'backing' + : false } const positionPreviewVersion = graph.positionPreviewVersion const allowStaleZoom = now() < r.sceneBackingPreviewUntil @@ -542,7 +572,15 @@ export function renderSceneBacking( } const crisp = backingPixelGridMatchesLiveViewport(r) - r.sceneBackingNeedsCrispRender = !crisp || !!r.sceneBackingBuild + r.sceneBackingNeedsCrispRender = allowStaleZoom || !crisp || !!r.sceneBackingBuild + if ( + !allowStaleZoom && + !r.sceneBackingBuild && + backingMetadataMatches(r, sceneVersion, positionPreviewVersion) + ) { + drawSettledScene(r, canvas, graph, sceneVersion) + return 'retained-pictures' + } return drawSceneBacking( r, canvas, @@ -550,4 +588,6 @@ export function renderSceneBacking( allowStaleZoom || !!r.sceneBackingBuild, positionPreviewVersion ) + ? 'backing' + : false } diff --git a/tests/e2e/canvas/frame-overlays.spec.ts b/tests/e2e/canvas/frame-overlays.spec.ts index 6ba077bff..bf6f7ed3d 100644 --- a/tests/e2e/canvas/frame-overlays.spec.ts +++ b/tests/e2e/canvas/frame-overlays.spec.ts @@ -81,6 +81,19 @@ async function createOverlayDemo(rotation: number) { test('rotated frame selection labels render with hovered child', async () => { await createOverlayDemo(18) + const settled = await editor.canvas.screenshotCanvasRegion() + const release = await editor.page.evaluateHandle(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('Editor unavailable') + return store.beginInteractiveEdit() + }) + try { + await editor.canvas.waitForRender() + expect((await editor.canvas.screenshotCanvasRegion()).equals(settled)).toBe(true) + } finally { + await release.evaluate((stop) => stop()) + await release.dispose() + } await expectCanvas('rotated-frame-selection-labels') }) diff --git a/tests/e2e/canvas/frame-overlays.spec.ts-snapshots/rotated-frame-selection-labels-openpencil-darwin.png b/tests/e2e/canvas/frame-overlays.spec.ts-snapshots/rotated-frame-selection-labels-openpencil-darwin.png index 6809354fa..a806f23a0 100644 Binary files a/tests/e2e/canvas/frame-overlays.spec.ts-snapshots/rotated-frame-selection-labels-openpencil-darwin.png and b/tests/e2e/canvas/frame-overlays.spec.ts-snapshots/rotated-frame-selection-labels-openpencil-darwin.png differ diff --git a/tests/e2e/canvas/retained-parity.spec.ts b/tests/e2e/canvas/retained-parity.spec.ts new file mode 100644 index 000000000..f40d818c1 --- /dev/null +++ b/tests/e2e/canvas/retained-parity.spec.ts @@ -0,0 +1,71 @@ +import { writeFile } from 'node:fs/promises' + +import { expect, test } from '#tests/e2e/fixtures' +import { CanvasHelper } from '#tests/helpers/canvas' + +for (const dpr of [1, 1.25, 1.5, 2]) { + test.describe(`retained raster parity at DPR ${dpr}`, () => { + test.use({ viewport: { width: 1111, height: 999 }, deviceScaleFactor: dpr }) + test('rounded geometry matches direct rendering after odd and even device pans', async ({ + page + }) => { + await page.goto('/?test&no-chrome&no-rulers&navigation-benchmark') + const canvas = new CanvasHelper(page) + await canvas.waitForInit() + await page.evaluate(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('Editor unavailable') + const id = store.createShape('SECTION', 140, 140, 500, 350) + store.updateNode(id, { rotation: 25 }) + store.clearSelection() + }) + for (const devicePan of [0, 1, 2, -1, -2, 0.3]) { + await page.evaluate( + ({ devicePan, dpr }) => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('Editor unavailable') + store.pan(devicePan / dpr - store.state.panX, -devicePan / dpr - store.state.panY) + }, + { devicePan, dpr } + ) + await page.evaluate(() => window.openPencil?.test?.navigation?.waitForSettlement()) + await canvas.waitForRender() + const retained = await canvas.screenshotCanvasRegion() + const metadata = await page.evaluate(() => { + const r = window.openPencil + ?.getStore?.() + .canvasRenderers.find((r) => r.tracksSceneSettlement) + return { + targetHeight: r?.surface.height(), + backingHeight: r?.sceneBacking?.image.height(), + marginY: r?.sceneBacking?.marginDeviceY, + anchorX: r?.sceneBacking?.anchorPanX, + anchorY: r?.sceneBacking?.anchorPanY, + panX: r?.panX, + panY: r?.panY + } + }) + const release = await page.evaluateHandle(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('Editor unavailable') + return store.beginInteractiveEdit() + }) + try { + await canvas.waitForRender() + const direct = await canvas.screenshotCanvasRegion() + if (!direct.equals(retained)) { + await writeFile(test.info().outputPath('retained.png'), retained) + await writeFile(test.info().outputPath('direct.png'), direct) + } + expect(direct.equals(retained), JSON.stringify({ dpr, devicePan, ...metadata })).toBe( + true + ) + } finally { + await release.evaluate((stop) => stop()) + await release.dispose() + } + } + canvas.assertNoErrors() + }) + }) +} diff --git a/tests/engine/render/canvas/retained-backing-pixels.test.ts b/tests/engine/render/canvas/retained-backing-pixels.test.ts index 72fb42d05..e3432435b 100644 --- a/tests/engine/render/canvas/retained-backing-pixels.test.ts +++ b/tests/engine/render/canvas/retained-backing-pixels.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, expect, test } from 'bun:test' +import { beforeAll, expect, spyOn, test } from 'bun:test' import { SceneGraph } from '@open-pencil/scene-graph' @@ -124,6 +124,44 @@ test('capped backing preserves the pixel grid after fractional pan and pixel-den } }) +test('settlement reuses pictures and the navigation image without another cache', () => { + const { graph, pageId } = createFixture() + const renderer = createRenderer(pageId) + try { + renderer.render(graph, new Set(), {}, 1, 'scene') + const backing = expectDefined(renderer.sceneBacking, 'backing') + const pictures = new Map( + [...renderer.subtreePictureCache].map(([id, entry]) => [id, entry.picture]) + ) + expect(pictures.size).toBeGreaterThan(0) + const nodeDraws = spyOn(renderer, 'renderNode') + const allocations = spyOn(renderer.surface, 'makeSurface') + try { + renderer.sceneBackingPreviewUntil = 0 + renderer.render(graph, new Set(), {}, 1, 'scene') + expect(renderer.profiler.stats.scenePictureMissReason).toBe('retained-pictures') + expect(renderer.sceneBacking).toBe(backing) + expect(renderer.scenePicture).toBeNull() + expect(renderer.subtreePictureCache.size).toBe(pictures.size) + for (const [id, picture] of pictures) + expect(renderer.subtreePictureCache.get(id)?.picture).toBe(picture) + expect(nodeDraws).not.toHaveBeenCalled() + expect(allocations).not.toHaveBeenCalled() + renderer.navigationPhase = 'pan' + renderer.render(graph, new Set(), {}, 1, 'scene') + expect(renderer.profiler.stats.scenePictureMissReason).toBe('backing') + expect(renderer.sceneBacking).toBe(backing) + expect(nodeDraws).not.toHaveBeenCalled() + expect(allocations).not.toHaveBeenCalled() + } finally { + nodeDraws.mockRestore() + allocations.mockRestore() + } + } finally { + renderer.destroy() + } +}) + test('active edits draw directly without replacing the overscan backing', () => { const { graph, pageId } = createFixture() const renderer = createRenderer(pageId) diff --git a/tests/engine/render/canvas/retained-backing.test.ts b/tests/engine/render/canvas/retained-backing.test.ts index 742e4122e..71b77e4cd 100644 --- a/tests/engine/render/canvas/retained-backing.test.ts +++ b/tests/engine/render/canvas/retained-backing.test.ts @@ -40,6 +40,7 @@ function createRenderer(surfaceFactory: (info: ImageInfo) => Surface | null) { viewportHeight: 100, pageColor: { r: 1, g: 1, b: 1 }, pageId: 'page', + navigationPhase: 'idle', sceneBacking: null, sceneBackingBuild: null, sceneBackingAllocationFailed: false, @@ -63,7 +64,11 @@ function createRenderer(surfaceFactory: (info: ImageInfo) => Surface | null) { function createCanvas() { const canvas: Partial = { drawImageRect: mock(), - drawImageRectOptions: mock() + drawImageRectOptions: mock(), + save: mock(), + restore: mock(), + translate: mock(), + scale: mock() } return canvas as Canvas } @@ -235,7 +240,7 @@ test('retained scene backing filters cross-zoom previews instead of falling back const canvas = createCanvas() const graph = createGraph() - expect(renderSceneBacking(r, canvas, graph, 1)).toBe(true) + expect(renderSceneBacking(r, canvas, graph, 1)).toBe('backing') expect(canvas.drawImageRectOptions).toHaveBeenCalledWith( r.sceneBacking.image, expect.anything(), @@ -273,8 +278,15 @@ test('retained scene backing allows same-zoom previews while panning', () => { const canvas = createCanvas() const graph = createGraph() - expect(renderSceneBacking(r, canvas, graph, 1)).toBe(true) - expect(canvas.drawImageRectOptions).toHaveBeenCalled() + expect(renderSceneBacking(r, canvas, graph, 1)).toBe('backing') + expect(canvas.drawImageRectOptions).toHaveBeenCalledTimes(1) + expect(r.sceneBackingNeedsCrispRender).toBe(true) + + r.sceneBackingPreviewUntil = 0 + expect(renderSceneBacking(r, canvas, graph, 1)).toBe('retained-pictures') + expect(r.sceneBackingNeedsCrispRender).toBe(false) + expect(canvas.drawImageRectOptions).toHaveBeenCalledTimes(1) + expect(r.surface.makeSurface).not.toHaveBeenCalled() }) test('retained scene backing invalidates stale position-preview metadata', () => {