diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c5376c25..36d9d77d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,6 +125,8 @@ ### Fixed +- Keep an opened `.fig` file saved until it is edited. Laying out its first page, which recomputes auto-layout sizes and positions, and showing another page for the first time, which loads its layers from the file, marked it unsaved, so closing it asked to save changes nobody made. +- Run crash recovery and autosave after edits, not whenever the canvas redraws. Opening a document, laying out a page, or loading a font started a recovery snapshot or an autosave, which encoded the whole document again once another page had loaded. In Safari, where every opened file gets recovery snapshots, a large page froze the browser for minutes after it first appeared. - Open image-heavy `.fig` files without the canvas running out of memory (#924). Decoded images stay within a fixed budget, and documents with many large images draw previews sized to the view, decoded in the background a few at a time, while exports keep the full images. - Show tables in AI chat replies at the chat's text size and weight, with a light header and copy as their only action, and task lists with a checkbox in place of the bullet. Tooltips, the copy menu, and the confirmation before opening a link follow the app's style, and links no longer load each site's favicon. - Keep an opened `.fig` file saved until it is edited. Laying out its first page, which recomputes auto-layout sizes and positions, marked it unsaved, so closing it asked to save changes nobody made. @@ -222,6 +224,7 @@ ### Performance +- Open and draw large pages faster: guides no longer scan every layer of the page on each frame, a layout pass only writes the layers it moved and asks for one redraw, and opening a `.fig` keeps one copy of the file on the main thread instead of three. - Edit variables in large documents without stalls: renaming, reordering, or adding a variable, or changing its CSS name, unit, scopes, or conditions, no longer redraws the canvas, and changing a value or mode updates only the layers bound to those variables or to variables aliasing them instead of re-resolving and laying out every bound layer in the document. - Open the `/demo` document like any `.fig` file, built ahead of time, instead of generating it in the browser, which froze the page for several seconds. - Open large `.fig` files with less memory in the macOS desktop app and Safari: imported layers now share one object layout in JavaScriptCore instead of each being stored as a slower, larger dictionary. diff --git a/packages/core/src/canvas/guides/draw.ts b/packages/core/src/canvas/guides/draw.ts index 204181bbb..f6c45f105 100644 --- a/packages/core/src/canvas/guides/draw.ts +++ b/packages/core/src/canvas/guides/draw.ts @@ -1,6 +1,7 @@ import type { Canvas } from 'canvaskit-wasm' import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph' +import { guideOwnersOnPage } from '@open-pencil/scene-graph/guides' import type { RenderOverlays, SkiaRenderer } from '#core/canvas/renderer' @@ -87,7 +88,7 @@ export function drawGuides( const selected = guides?.selected r.auxStroke.setStrokeWidth(1) - const visit = (owner: SceneNode) => { + for (const owner of guideOwnersOnPage(graph, page.id)) { for (const guide of owner.guides) { if (preview?.source?.ownerId === owner.id && preview.source.guideId === guide.id) continue drawGuide( @@ -104,12 +105,7 @@ export function drawGuides( ) ) } - for (const childId of owner.childIds) { - const child = graph.getNode(childId) - if (child) visit(child) - } } - visit(page) if (preview) { const owner = graph.getNode(preview.ownerId) diff --git a/packages/core/src/canvas/guides/hit-test.ts b/packages/core/src/canvas/guides/hit-test.ts index 7fa63c2be..1da7c15d3 100644 --- a/packages/core/src/canvas/guides/hit-test.ts +++ b/packages/core/src/canvas/guides/hit-test.ts @@ -1,5 +1,5 @@ -import type { SceneGraph, SceneNode } from '@open-pencil/scene-graph' -import type { CanvasGuide } from '@open-pencil/scene-graph/guides' +import type { SceneGraph } from '@open-pencil/scene-graph' +import { guideOwnersOnPage, type CanvasGuide } from '@open-pencil/scene-graph/guides' import { distanceToGuideSegment, getGuideScreenSegment, type GuideViewport } from './geometry' @@ -23,7 +23,7 @@ export function hitTestGuides( if (!page) return null let closest: GuideHit | null = null - const visit = (owner: SceneNode) => { + for (const owner of guideOwnersOnPage(graph, page.id)) { for (const guide of owner.guides) { const distance = distanceToGuideSegment( x, @@ -40,12 +40,6 @@ export function hitTestGuides( } } } - for (const childId of owner.childIds) { - const child = graph.getNode(childId) - if (child) visit(child) - } } - - visit(page) return closest } diff --git a/packages/core/src/editor/graph-events.ts b/packages/core/src/editor/graph-events.ts index 9b7cdd86c..593c9d00d 100644 --- a/packages/core/src/editor/graph-events.ts +++ b/packages/core/src/editor/graph-events.ts @@ -82,12 +82,32 @@ function invalidateRenderersForChange( export function createGraphEventSubscription(options: GraphEventOptions) { let unbindGraphEvents: (() => void) | null = null + let batchRenderPending = false + + /** + * A layout pass or a page's layers loading updates many layers at once, and each render + * request bumps reactive versions; the batch asks for one render after it, before the next + * frame draws. + */ + function requestRenderFor(graph: SceneGraph) { + if (!graph.isApplyingLayout && !graph.isApplyingImportedState) { + options.requestRender() + return + } + if (batchRenderPending) return + batchRenderPending = true + queueMicrotask(() => { + batchRenderPending = false + options.requestRender() + }) + } function onNodeUpdated(id: string, changes: Partial) { - invalidateRenderersForChange(options.getGraph(), options.getRenderers(), id, changes, true) + const graph = options.getGraph() + invalidateRenderersForChange(graph, options.getRenderers(), id, changes, true) options.emitEditorEvent('node:updated', id, changes) options.scheduleComponentSync(id) - options.requestRender() + requestRenderFor(graph) } function onNodePreviewUpdated(id: string, changes: Partial) { @@ -108,7 +128,7 @@ export function createGraphEventSubscription(options: GraphEventOptions) { renderer.tiledScene.invalidateStructure() } options.scheduleComponentSync(nodeId) - options.requestRender() + requestRenderFor(options.getGraph()) } function subscribeToGraph() { diff --git a/packages/core/src/io/formats/fig/read.ts b/packages/core/src/io/formats/fig/read.ts index be4f91748..ec1680f0d 100644 --- a/packages/core/src/io/formats/fig/read.ts +++ b/packages/core/src/io/formats/fig/read.ts @@ -114,15 +114,13 @@ export function parseFigFileViaWorker( reject(new Error(err.message || 'Worker failed to parse .fig file')) } const workerBuffer = buffer.slice(0) - const archiveBuffer = buffer.slice(0) const request: FigSessionOpenRequest = { type: 'open', - originalBuffer: workerBuffer, - archiveBuffer, + buffer: workerBuffer, options: { populate: options.populate }, port: channel.port2 } - worker.postMessage(request, [workerBuffer, archiveBuffer, channel.port2]) + worker.postMessage(request, [workerBuffer, channel.port2]) }) } @@ -132,13 +130,13 @@ export async function parseFigFile( ): Promise { options.signal?.throwIfAborted() if (typeof Worker !== 'undefined' && IS_BROWSER) { - const copy = buffer.slice(0) try { + // The worker gets its own copy, so `buffer` is still whole for the fallback. return await parseFigFileViaWorker(buffer, options) } catch (error) { if (options.signal?.aborted || error instanceof ReaderSemanticError) throw error console.warn('Worker parsing failed, falling back to main thread:', error) - return parseFigFileSync(copy, options) + return parseFigFileSync(buffer, options) } } options.signal?.throwIfAborted() diff --git a/packages/core/src/kiwi/fig/population/client.ts b/packages/core/src/kiwi/fig/population/client.ts index 85c1bfda2..508fc55c0 100644 --- a/packages/core/src/kiwi/fig/population/client.ts +++ b/packages/core/src/kiwi/fig/population/client.ts @@ -62,8 +62,9 @@ export function registerOriginalArchiveRequest( request: () => Promise ): void { const entry: OriginalArchiveRequest = { request, valid: true, unbind: () => undefined } + // Layout and the layers a page loads from this archive leave it describing the document. const invalidate = () => { - if (!graph.isApplyingLayout) entry.valid = false + if (!graph.isApplyingLayout && !graph.isApplyingImportedState) entry.valid = false } entry.unbind = graph.onNodeEvents({ created: invalidate, diff --git a/packages/core/src/kiwi/fig/session/document-state.ts b/packages/core/src/kiwi/fig/session/document-state.ts index bac9969f4..efe4c40e3 100644 --- a/packages/core/src/kiwi/fig/session/document-state.ts +++ b/packages/core/src/kiwi/fig/session/document-state.ts @@ -101,7 +101,9 @@ export function recoverReaderPage(graph: SceneGraph, pageId: string): boolean { } const page = state.session.pages.find((page) => state.session?.graphPageId(page.id) === pageId) if (!page) throw new Error(`Unknown graph page ${pageId}`) - const populated = !state.session.loadedPageIds.has(page.id) - state.session.loadPage(page.id) + const session = state.session + const populated = !session.loadedPageIds.has(page.id) + // The layers come from the opened file, as they do through the population worker's delta. + graph.applyImportedStateDuring(() => session.loadPage(page.id)) return populated } diff --git a/packages/core/src/kiwi/fig/session/protocol.ts b/packages/core/src/kiwi/fig/session/protocol.ts index 5da107e02..caca6e9d4 100644 --- a/packages/core/src/kiwi/fig/session/protocol.ts +++ b/packages/core/src/kiwi/fig/session/protocol.ts @@ -7,8 +7,8 @@ import type { FigPopulationDelta } from '#core/kiwi/fig/population/delta' export interface FigSessionOpenRequest { type: 'open' - originalBuffer: ArrayBuffer - archiveBuffer: ArrayBuffer + /** The file, transferred; the worker keeps its own copy as the original archive. */ + buffer: ArrayBuffer options?: Pick port: MessagePort } diff --git a/packages/core/src/kiwi/fig/session/worker.ts b/packages/core/src/kiwi/fig/session/worker.ts index 9252ad218..a2238e5ec 100644 --- a/packages/core/src/kiwi/fig/session/worker.ts +++ b/packages/core/src/kiwi/fig/session/worker.ts @@ -60,9 +60,10 @@ self.onmessage = (event: MessageEvent) => { port = request.port port.onmessage = (message: MessageEvent) => handleRequest(message.data) port.start() - originalArchive = new Uint8Array(request.archiveBuffer) + // The worker copies the archive itself, so the main thread sends the file once. + originalArchive = new Uint8Array(request.buffer.slice(0)) try { - const opened = openReaderSession(request.originalBuffer, request.options?.populate) + const opened = openReaderSession(request.buffer, request.options?.populate) respond({ type: 'page-manifest', pages: opened.pages }) session = request.options?.populate === 'first-page' || request.options?.populate === 'none' diff --git a/packages/core/src/layout/apply.ts b/packages/core/src/layout/apply.ts index bcfd82edf..f9e411f95 100644 --- a/packages/core/src/layout/apply.ts +++ b/packages/core/src/layout/apply.ts @@ -21,10 +21,27 @@ function preservesImportedHugCrossSize( ) } +type LayoutGeometry = Partial> + +/** + * Writes only the geometry layout changed. Laying out a large page leaves most layers where they + * were, and each write would notify every graph listener for nothing. + */ +function writeLayoutGeometry(graph: SceneGraph, id: string, geometry: LayoutGeometry): void { + const node = graph.getNode(id) + if (!node) return + const changes: LayoutGeometry = {} + for (const key of ['x', 'y', 'width', 'height'] as const) { + const value = geometry[key] + if (value !== undefined && value !== node[key]) changes[key] = value + } + if (Object.keys(changes).length > 0) graph.updateNode(id, changes) +} + function applyFrameSize(graph: SceneGraph, frame: SceneNode, yogaNode: YogaNode): void { if (frame.layoutMode === 'GRID') { if (frame.gridTemplateRows.length === 0) { - graph.updateNode(frame.id, { height: yogaNode.getComputedHeight() }) + writeLayoutGeometry(graph, frame.id, { height: yogaNode.getComputedHeight() }) } return } @@ -33,7 +50,7 @@ function applyFrameSize(graph: SceneGraph, frame: SceneNode, yogaNode: YogaNode) const computedW = yogaNode.getComputedWidth() const computedH = yogaNode.getComputedHeight() - const updates: Partial = {} + const updates: LayoutGeometry = {} const derived = frame.derivedLayout if (frame.primaryAxisSizing === 'HUG') { @@ -52,7 +69,7 @@ function applyFrameSize(graph: SceneGraph, frame: SceneNode, yogaNode: YogaNode) } } - graph.updateNode(frame.id, updates) + writeLayoutGeometry(graph, frame.id, updates) } function frameSourceIsFig(graph: SceneGraph, parentId: string | null): boolean { @@ -107,7 +124,7 @@ function updateChildFromYoga(graph: SceneGraph, child: SceneNode, yogaChild: Yog const preservesImportedPosition = preservesImportedFrameGeometry || (child.source.format === 'fig' && Math.abs(child.rotation) > 0.001) - graph.updateNode(child.id, { + writeLayoutGeometry(graph, child.id, { x: computedChildPosition(child, yogaChild, 'x', preservesImportedPosition), y: computedChildPosition(child, yogaChild, 'y', preservesImportedPosition), width: computedChildSize(child, yogaChild, 'width', preservesImportedFrameGeometry), diff --git a/packages/core/tests/editor/layout/layout-events.test.ts b/packages/core/tests/editor/layout/layout-events.test.ts new file mode 100644 index 000000000..5e51cf7be --- /dev/null +++ b/packages/core/tests/editor/layout/layout-events.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, test } from 'bun:test' + +import { createEditor } from '@open-pencil/core/editor' +import { computeLayout } from '@open-pencil/core/layout' +import { SceneGraph } from '@open-pencil/scene-graph' + +function hugRow(graph: SceneGraph, children: number) { + const row = graph.createNode('FRAME', graph.getPages()[0].id, { + layoutMode: 'HORIZONTAL', + primaryAxisSizing: 'HUG', + counterAxisSizing: 'HUG', + itemSpacing: 4 + }) + for (let i = 0; i < children; i++) { + graph.createNode('RECTANGLE', row.id, { width: 10 + i, height: 10 }) + } + return row +} + +describe('layout and graph events', () => { + test('laying out an unchanged frame again writes nothing', () => { + const graph = new SceneGraph() + const row = hugRow(graph, 5) + computeLayout(graph, row.id) + + let updates = 0 + const unbind = graph.onNodeEvents({ updated: () => updates++ }) + computeLayout(graph, row.id) + unbind() + expect(updates).toBe(0) + }) + + test('a layout pass that moves many layers asks for one render', async () => { + const editor = createEditor() + const row = hugRow(editor.graph, 50) + await Promise.resolve() + + let renders = 0 + const unbind = editor.onEditorEvent('render:requested', () => renders++) + computeLayout(editor.graph, row.id) + expect(renders).toBe(0) + await Promise.resolve() + unbind() + expect(renders).toBe(1) + editor.dispose() + }) +}) diff --git a/packages/core/tests/editor/pages/loading.test.ts b/packages/core/tests/editor/pages/loading.test.ts index 8acf1b7d1..4058e0360 100644 --- a/packages/core/tests/editor/pages/loading.test.ts +++ b/packages/core/tests/editor/pages/loading.test.ts @@ -1,6 +1,9 @@ import { expect, test } from 'bun:test' import { createEditor } from '@open-pencil/core/editor' +import { exportFigFile, parseFigFile } from '@open-pencil/core/io' +import { populateFigPage } from '@open-pencil/core/io/formats/fig' +import { initCodec } from '@open-pencil/core/kiwi' import { SceneGraph } from '@open-pencil/scene-graph' import type { PageSwitchProgress } from '#core/editor/pages' @@ -238,3 +241,26 @@ test('preparing a page reports failure when the document is replaced meanwhile', expect(await preparing).toBe(false) }) + +test("loading a page's layers from the opened file asks for one render", async () => { + await initCodec() + const source = new SceneGraph() + const second = source.addPage('Second') + for (let i = 0; i < 30; i++) source.createNode('RECTANGLE', second.id, { name: `Layer ${i}` }) + const bytes = await exportFigFile(source) + const graph = await parseFigFile(bytes.slice().buffer, { populate: 'first-page' }) + const editor = createEditor({ graph }) + const page = graph.getPages()[1] + if (!page) throw new Error('Expected a second page') + await Promise.resolve() + + let renders = 0 + const unbind = editor.onEditorEvent('render:requested', () => renders++) + expect(populateFigPage(graph, page.id)).toBe(true) + await Promise.resolve() + unbind() + + expect(graph.getChildren(page.id)).toHaveLength(30) + expect(renders).toBe(1) + editor.dispose() +}) diff --git a/packages/core/tests/io/formats/fig/session/worker-original-archive.test.ts b/packages/core/tests/io/formats/fig/session/worker-original-archive.test.ts index fe63d066f..5cd14c3ab 100644 --- a/packages/core/tests/io/formats/fig/session/worker-original-archive.test.ts +++ b/packages/core/tests/io/formats/fig/session/worker-original-archive.test.ts @@ -5,7 +5,10 @@ import { initCodec } from '@open-pencil/core/kiwi' import { SceneGraph } from '@open-pencil/scene-graph' import { parseFigFileViaWorker } from '#core/io/formats/fig/read' -import { releaseFigPopulationWorker } from '#core/kiwi/fig/population/client' +import { + createFigPopulationWorker, + releaseFigPopulationWorker +} from '#core/kiwi/fig/population/client' test('saving an unedited worker-opened .fig returns its original bytes', async () => { await initCodec() @@ -21,3 +24,21 @@ test('saving an unedited worker-opened .fig returns its original bytes', async ( releaseFigPopulationWorker(opened) } }, 20000) + +test('loading a page through the worker keeps saving the original bytes', async () => { + await initCodec() + const source = new SceneGraph() + source.createNode('TEXT', source.getPages()[0].id, { text: 'First' }) + source.createNode('TEXT', source.addPage('Second').id, { text: 'Second' }) + const bytes = await exportFigFile(source) + + const opened = await parseFigFileViaWorker(bytes.slice().buffer, { populate: 'first-page' }) + try { + const second = opened.getPages()[1] + expect(await createFigPopulationWorker(opened)?.populate(second.id)).toBe(true) + expect(opened.getChildren(second.id)).toHaveLength(1) + expect(await exportFigFile(opened)).toEqual(bytes) + } finally { + releaseFigPopulationWorker(opened) + } +}, 20000) diff --git a/packages/core/tests/io/formats/fig/session/worker-reader.test.ts b/packages/core/tests/io/formats/fig/session/worker-reader.test.ts index bb1ba40fe..acac83fb6 100644 --- a/packages/core/tests/io/formats/fig/session/worker-reader.test.ts +++ b/packages/core/tests/io/formats/fig/session/worker-reader.test.ts @@ -44,8 +44,7 @@ test.each(['first-page', 'none'] as const)( worker.postMessage( { type: 'open', - originalBuffer: bytes.slice().buffer, - archiveBuffer: bytes.slice().buffer, + buffer: bytes.slice().buffer, options: { populate }, port: channel.port2 }, diff --git a/packages/scene-graph/src/guides.ts b/packages/scene-graph/src/guides.ts index 91d2b4fd2..8ef297c0f 100644 --- a/packages/scene-graph/src/guides.ts +++ b/packages/scene-graph/src/guides.ts @@ -1,4 +1,6 @@ +import type { SceneGraph } from './index' import type { GUID } from './primitives' +import type { SceneNode } from './types' export interface CanvasGuide { id: string @@ -6,3 +8,41 @@ export interface CanvasGuide { position: number figGuid?: GUID } + +const ownersByGraph = new WeakMap>() + +function ownerSet(graph: SceneGraph): Set { + const existing = ownersByGraph.get(graph) + if (existing) return existing + const owners = new Set() + for (const node of graph.getAllNodes()) if (node.guides.length > 0) owners.add(node.id) + const track = (node: SceneNode | undefined, id: string) => { + if (node && node.guides.length > 0) owners.add(id) + else owners.delete(id) + } + graph.onNodeEvents({ + created: (node) => track(node, node.id), + updated: (id, changes) => { + if ('guides' in changes) track(graph.getNode(id), id) + }, + deleted: (id) => owners.delete(id) + }) + ownersByGraph.set(graph, owners) + return owners +} + +/** + * The layers on a page that carry guides, which a frame anywhere in the tree can, as in Figma. + * The graph keeps the set current from its node events, so drawing and hit-testing guides do not + * walk the page's whole tree on every frame. + */ +export function guideOwnersOnPage(graph: SceneGraph, pageId: string): SceneNode[] { + const owners: SceneNode[] = [] + for (const id of ownerSet(graph)) { + const node = graph.getNode(id) + if (!node || node.guides.length === 0) continue + if (graph.closest(id, (ancestor) => ancestor.type === 'CANVAS')?.id === pageId) + owners.push(node) + } + return owners +} diff --git a/packages/scene-graph/tests/guides.test.ts b/packages/scene-graph/tests/guides.test.ts new file mode 100644 index 000000000..ed47a9f47 --- /dev/null +++ b/packages/scene-graph/tests/guides.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from 'bun:test' + +import { SceneGraph } from '@open-pencil/scene-graph' +import { guideOwnersOnPage } from '@open-pencil/scene-graph/guides' + +const guide = (id: string) => ({ id, axis: 'x' as const, position: 10 }) +const names = (graph: SceneGraph, pageId: string) => + guideOwnersOnPage(graph, pageId).map((node) => node.name).toSorted() + +describe('guide owners', () => { + test('lists the page and frames at any depth that carry guides, on that page only', () => { + const graph = new SceneGraph() + const page = graph.getPages()[0] + graph.updateNode(page.id, { guides: [guide('page')] }) + const top = graph.createNode('FRAME', page.id, { name: 'Top' }) + graph.createNode('FRAME', top.id, { name: 'Nested', guides: [guide('nested')] }) + graph.createNode('FRAME', page.id, { name: 'Plain' }) + const other = graph.addPage('Other') + graph.createNode('FRAME', other.id, { name: 'Elsewhere', guides: [guide('elsewhere')] }) + + expect(names(graph, page.id)).toEqual(['Nested', page.name].toSorted()) + expect(names(graph, other.id)).toEqual(['Elsewhere']) + }) + + test('follows guides added, removed, created, deleted, and moved after the first read', () => { + const graph = new SceneGraph() + const page = graph.getPages()[0] + const frame = graph.createNode('FRAME', page.id, { name: 'Frame' }) + expect(names(graph, page.id)).toEqual([]) + + graph.updateNode(frame.id, { guides: [guide('a')] }) + expect(names(graph, page.id)).toEqual(['Frame']) + const late = graph.createNode('FRAME', page.id, { name: 'Late', guides: [guide('b')] }) + expect(names(graph, page.id)).toEqual(['Frame', 'Late']) + + graph.updateNode(frame.id, { guides: [] }) + graph.deleteNode(late.id) + expect(names(graph, page.id)).toEqual([]) + + const moved = graph.createNode('FRAME', page.id, { name: 'Moved', guides: [guide('c')] }) + const other = graph.addPage('Other') + graph.reparentNode(moved.id, other.id) + expect(names(graph, page.id)).toEqual([]) + expect(names(graph, other.id)).toEqual(['Moved']) + }) +}) diff --git a/src/app/document/autosave/create.ts b/src/app/document/autosave/create.ts index 5c7ae5f37..869a4606b 100644 --- a/src/app/document/autosave/create.ts +++ b/src/app/document/autosave/create.ts @@ -1,11 +1,9 @@ import { watchDebounced } from '@vueuse/core' -import type { EditorState } from '@open-pencil/core/editor' - -type AutosaveState = EditorState & { autosaveEnabled: boolean } - type AutosaveOptions = { - state: AutosaveState + state: { autosaveEnabled: boolean } + /** The document's content revision; rendering and layout do not advance it. */ + version: () => number getSavedVersion: () => number hasWritableSource: () => boolean saveCurrentDocument: (version: number) => Promise @@ -13,6 +11,7 @@ type AutosaveOptions = { export function createAutosave({ state, + version: currentVersion, getSavedVersion, hasWritableSource, saveCurrentDocument @@ -54,7 +53,7 @@ export function createAutosave({ } const stop = watchDebounced( - () => state.sceneVersion, + currentVersion, (version) => { void requestSave(version).catch(reportFailure) }, diff --git a/src/app/document/io/changes.ts b/src/app/document/io/changes.ts index 86056f378..31a9f958c 100644 --- a/src/app/document/io/changes.ts +++ b/src/app/document/io/changes.ts @@ -3,8 +3,8 @@ import { computed, ref } from 'vue' import type { Editor } from '@open-pencil/core/editor' /** - * Content-only revisions: viewport repainting, layout, and recovery never mark a document - * changed or saved. + * Content-only revisions: viewport repainting, layout, loading a page's layers, and recovery + * never mark a document changed or saved. Autosave and recovery follow the same revision. */ export function createDocumentChanges(editor: Editor) { const revision = ref(0) @@ -13,17 +13,22 @@ export function createDocumentChanges(editor: Editor) { const changed = () => { revision.value++ } + // A page's layers load from the opened file when it is first shown; they are the document + // as saved, not an edit. + const contentChanged = () => { + if (!editor.graph.isApplyingImportedState) changed() + } // Layout derives sizes and positions from the document; opening a page or loading a font // relays it out without an edit, and an edit that relays it out has already counted. const updated = () => { - if (!editor.graph.isApplyingLayout) changed() + if (!editor.graph.isApplyingLayout) contentChanged() } const unsubscribers = [ - editor.onEditorEvent('node:created', changed), + editor.onEditorEvent('node:created', contentChanged), editor.onEditorEvent('node:updated', updated), - editor.onEditorEvent('node:deleted', changed), - editor.onEditorEvent('node:reparented', changed), - editor.onEditorEvent('node:reordered', changed), + editor.onEditorEvent('node:deleted', contentChanged), + editor.onEditorEvent('node:reparented', contentChanged), + editor.onEditorEvent('node:reordered', contentChanged), editor.onEditorEvent('graph:replaced', changed), editor.onEditorEvent('history:changed', changed) ] diff --git a/src/app/document/io/create.ts b/src/app/document/io/create.ts index fa38a5ee0..d445382ba 100644 --- a/src/app/document/io/create.ts +++ b/src/app/document/io/create.ts @@ -29,10 +29,7 @@ export function createDocumentIOActions( state, getFilePath: sourceState.getFilePath, getFileHandle: sourceState.getFileHandle, - setSavedVersion: (version) => { - sourceState.setSavedVersion(version) - sourceActions.markDocumentSaved() - }, + markDocumentSaved: () => sourceActions.markDocumentSaved(), preparationController }) const { startWatchingFile, stopWatchingFile } = createFileWatcher({ diff --git a/src/app/document/io/read.ts b/src/app/document/io/read.ts index d7b3a8458..e93b3a298 100644 --- a/src/app/document/io/read.ts +++ b/src/app/document/io/read.ts @@ -34,7 +34,7 @@ type ReloadActionsOptions = { state: ReloadDocumentState getFilePath: () => string | null getFileHandle: () => FileSystemFileHandle | null - setSavedVersion: (version: number) => void + markDocumentSaved: () => void preparationController: EditorPreparationController } @@ -96,7 +96,7 @@ export function createReloadActions({ state, getFilePath, getFileHandle, - setSavedVersion, + markDocumentSaved, preparationController }: ReloadActionsOptions) { async function reloadFromDisk() { @@ -121,7 +121,7 @@ export function createReloadActions({ await applyImportedDocument(editor, imported, load) restoreReloadState(editor, state, snapshot) editor.requestRender() - setSavedVersion(state.sceneVersion) + markDocumentSaved() succeeded = true } catch (error) { if (load.signal.aborted) return diff --git a/src/app/document/io/save.ts b/src/app/document/io/save.ts index 0e09ffdf1..e0030d96f 100644 --- a/src/app/document/io/save.ts +++ b/src/app/document/io/save.ts @@ -12,6 +12,8 @@ type SaveDocumentState = EditorState & { documentName: string } type SaveActionsOptions = Omit & { state: SaveDocumentState + /** The document's content revision, recorded as the saved version. */ + version: () => number buildFigFile: () => Uint8Array | Promise startWatchingFile: () => void onWriteSuccess?: (version: number) => void | Promise @@ -20,6 +22,7 @@ type SaveActionsOptions = Omit & { export function createSaveActions({ state, + version: currentVersion, buildFigFile, getFilePath, setFilePath, @@ -47,7 +50,7 @@ export function createSaveActions({ }) async function buildVersionedFigFile() { - const version = state.sceneVersion + const version = currentVersion() return { data: await buildFigFile(), version } } diff --git a/src/app/document/io/source.ts b/src/app/document/io/source.ts index 5214a7aa9..508a61b0f 100644 --- a/src/app/document/io/source.ts +++ b/src/app/document/io/source.ts @@ -69,6 +69,7 @@ export function createDocumentSourceActions({ const recovery = createDocumentRecovery({ state, + version: changes.capture, isEnabled: () => recoveryEnabled.value, buildFigFile: buildRecoveryFigFile, hasWritableSource: () => !!getFileHandle() || !!getFilePath() || !!getStorageBinding() @@ -76,6 +77,7 @@ export function createDocumentSourceActions({ const { saveFigFile, saveFigFileAs, writeFile } = createSaveActions({ state, + version: changes.capture, buildFigFile, getFilePath, setFilePath, @@ -97,6 +99,7 @@ export function createDocumentSourceActions({ const autosave = createAutosave({ state, + version: changes.capture, getSavedVersion, hasWritableSource: () => !!getFileHandle() || !!getFilePath() || !!getStorageBinding(), saveCurrentDocument: async (version) => { @@ -106,6 +109,11 @@ export function createDocumentSourceActions({ } }) + function markDocumentSaved() { + changes.markSaved() + setSavedVersion(changes.capture()) + } + function setDocumentSource( fileName: string, sourceFormat: string, @@ -119,9 +127,8 @@ export function createDocumentSourceActions({ setFilePath(isFig ? (path ?? null) : null) setDownloadName(figDownloadName(fileName, sourceFormat)) setSourceIdentity({ handle: handle ?? null, path: path ?? null }) - setSavedVersion(state.sceneVersion) - changes.markSaved() - void recovery.markProtectedVersion(state.sceneVersion) + markDocumentSaved() + void recovery.markProtectedVersion(changes.capture()) if (isFig && (handle || path)) { void startWatchingFile() } @@ -136,9 +143,8 @@ export function createDocumentSourceActions({ setStorageBinding(binding) state.documentName = documentName state.autosaveEnabled = true - setSavedVersion(state.sceneVersion) - changes.markSaved() - void recovery.markProtectedVersion(state.sceneVersion) + markDocumentSaved() + void recovery.markProtectedVersion(changes.capture()) } function setPlannedFilePath(path: string) { @@ -201,12 +207,12 @@ export function createDocumentSourceActions({ saveFigFile: () => saveAndTrack(saveFigFile), saveFigFileAs: () => saveAndTrack(saveFigFileAs), hasUnsavedChanges: changes.hasUnsavedChanges, - markDocumentSaved: changes.markSaved, + markDocumentSaved, getStorageBinding, getRecoveryId: () => recovery.getRecoveryId(), - adoptRecoverySnapshot: (id: string, version: number) => { + adoptRecoverySnapshot: (id: string) => { changes.markChanged() - return recovery.adoptRecoverySnapshot(id, version) + return recovery.adoptRecoverySnapshot(id) }, persistRecoveryNow: () => recovery.persistNow(), discardRecovery: () => recovery.discardRecovery() diff --git a/src/app/document/io/write.ts b/src/app/document/io/write.ts index e1c4708f7..346141a6e 100644 --- a/src/app/document/io/write.ts +++ b/src/app/document/io/write.ts @@ -1,11 +1,9 @@ -import type { EditorState } from '@open-pencil/core/editor' - import { describeDiagnosticError, recordDocumentFailure } from '@/app/diagnostics' import type { StorageDocumentBinding } from '@/app/integrations/storage/types' import { persistStorageCanvasLocally } from '@/app/storage/sync/persist' import { isTauri } from '@/app/tauri/env' -type WriteDocumentState = EditorState & { documentName: string } +type WriteDocumentState = { documentName: string } type DocumentWriterOptions = { state: WriteDocumentState @@ -36,10 +34,7 @@ export function createDocumentWriter({ return true } - return async function writeFile( - data: Uint8Array, - version = state.sceneVersion - ): Promise { + return async function writeFile(data: Uint8Array, version: number): Promise { setLastWriteTime(Date.now()) try { const storage = getStorageBinding() diff --git a/src/app/document/recovery/controller.ts b/src/app/document/recovery/controller.ts index aad106e0b..7b9129d77 100644 --- a/src/app/document/recovery/controller.ts +++ b/src/app/document/recovery/controller.ts @@ -1,16 +1,14 @@ import { watchDebounced } from '@vueuse/core' import { watch, type WatchHandle } from 'vue' -import type { EditorState } from '@open-pencil/core/editor' - import { getRecoveryStore } from '@/app/document/recovery/store' import type { RecoveryStore } from '@/app/document/recovery/types' import { createCanvasId } from '@/app/storage/id' -type RecoveryState = EditorState & { documentName: string } - interface DocumentRecoveryOptions { - state: RecoveryState + state: { documentName: string } + /** The document's content revision; rendering and layout do not advance it. */ + version: () => number buildFigFile: () => Promise | Uint8Array hasWritableSource: () => boolean isEnabled?: () => boolean @@ -20,7 +18,7 @@ interface DocumentRecoveryOptions { export interface DocumentRecoveryController { getRecoveryId(): string - adoptRecoverySnapshot(id: string, sceneVersion: number): Promise + adoptRecoverySnapshot(id: string): Promise persistNow(): Promise markProtectedVersion(version: number): Promise discardRecovery(): Promise @@ -29,6 +27,7 @@ export interface DocumentRecoveryController { export function createDocumentRecovery({ state, + version: currentVersion, buildFigFile, hasWritableSource, isEnabled = () => true, @@ -36,7 +35,7 @@ export function createDocumentRecovery({ recoveryId = createCanvasId() }: DocumentRecoveryOptions): DocumentRecoveryController { let id = recoveryId - let protectedVersion = state.sceneVersion + let protectedVersion = currentVersion() let persistedVersion: number | null = null let requestedVersion = protectedVersion let lifecycleGeneration = 0 @@ -53,7 +52,6 @@ export function createDocumentRecovery({ await store.write({ id, documentName: state.documentName, - sceneVersion: version, figBytes: bytes }) persistedVersion = version @@ -65,7 +63,7 @@ export function createDocumentRecovery({ async function persistNow(): Promise { await cleanup if (disposed || hasWritableSource() || !isEnabled()) return - requestedVersion = state.sceneVersion + requestedVersion = currentVersion() if (requestedVersion === protectedVersion) return if (!writing) { const generation = lifecycleGeneration @@ -77,7 +75,7 @@ export function createDocumentRecovery({ } const stopVersionWatch: WatchHandle = watchDebounced( - () => state.sceneVersion, + currentVersion, () => { void persistNow().catch((error) => console.warn('[Recovery] Snapshot failed:', error)) }, @@ -88,15 +86,15 @@ export function createDocumentRecovery({ isEnabled, (enabled) => { if (enabled) { - protectedVersion = state.sceneVersion - requestedVersion = state.sceneVersion + protectedVersion = currentVersion() + requestedVersion = currentVersion() return } lifecycleGeneration++ const cleanupGeneration = lifecycleGeneration const snapshotId = id - requestedVersion = state.sceneVersion - protectedVersion = state.sceneVersion + requestedVersion = currentVersion() + protectedVersion = currentVersion() const activeWrite = writing cleanup = cleanup .then(async () => { @@ -117,13 +115,15 @@ export function createDocumentRecovery({ return { getRecoveryId: () => id, - async adoptRecoverySnapshot(nextId, sceneVersion) { + async adoptRecoverySnapshot(nextId) { const previousId = id await invalidateActiveWrite() id = nextId - protectedVersion = sceneVersion - persistedVersion = sceneVersion - requestedVersion = sceneVersion + // The adopted snapshot holds the document as it is now. + const version = currentVersion() + protectedVersion = version + persistedVersion = version + requestedVersion = version disposed = false if (previousId !== nextId) await store.remove(previousId) }, @@ -131,7 +131,7 @@ export function createDocumentRecovery({ async markProtectedVersion(version) { await invalidateActiveWrite() protectedVersion = version - requestedVersion = state.sceneVersion + requestedVersion = currentVersion() if (persistedVersion == null || persistedVersion <= version) { await store.remove(id) persistedVersion = null @@ -139,9 +139,9 @@ export function createDocumentRecovery({ }, async discardRecovery() { await invalidateActiveWrite() - protectedVersion = state.sceneVersion + protectedVersion = currentVersion() persistedVersion = null - requestedVersion = state.sceneVersion + requestedVersion = currentVersion() await store.remove(id) }, disposeRecovery() { diff --git a/src/app/document/recovery/idb.ts b/src/app/document/recovery/idb.ts index cc5c27203..200218b95 100644 --- a/src/app/document/recovery/idb.ts +++ b/src/app/document/recovery/idb.ts @@ -60,7 +60,6 @@ export function createIdbRecoveryStore(): RecoveryStore { id: input.id, documentName: input.documentName || 'Untitled', updatedAt: new Date().toISOString(), - sceneVersion: input.sceneVersion, byteLength: input.figBytes.byteLength, formatVersion: 1 } diff --git a/src/app/document/recovery/memory.ts b/src/app/document/recovery/memory.ts index 0fa50d2ee..56e34ed21 100644 --- a/src/app/document/recovery/memory.ts +++ b/src/app/document/recovery/memory.ts @@ -23,7 +23,6 @@ export function createMemoryRecoveryStore(): RecoveryStore { id: input.id, documentName: input.documentName || 'Untitled', updatedAt: new Date().toISOString(), - sceneVersion: input.sceneVersion, byteLength: input.figBytes.byteLength, formatVersion: 1 } diff --git a/src/app/document/recovery/types.ts b/src/app/document/recovery/types.ts index 722f0a523..c2b96bd5d 100644 --- a/src/app/document/recovery/types.ts +++ b/src/app/document/recovery/types.ts @@ -2,7 +2,6 @@ export interface RecoverySnapshotMeta { id: string documentName: string updatedAt: string - sceneVersion: number byteLength: number formatVersion: 1 } @@ -14,7 +13,6 @@ export interface RecoverySnapshot extends RecoverySnapshotMeta { export interface RecoverySnapshotInput { id: string documentName: string - sceneVersion: number figBytes: Uint8Array } diff --git a/src/app/tabs/index.ts b/src/app/tabs/index.ts index 57496f9d0..9ee91e4b2 100644 --- a/src/app/tabs/index.ts +++ b/src/app/tabs/index.ts @@ -543,7 +543,7 @@ export async function restoreRecoverySnapshot(id: string): Promise { imported, async () => { store.state.documentName = snapshot.documentName - await store.adoptRecoverySnapshot(id, snapshot.sceneVersion) + await store.adoptRecoverySnapshot(id) }, load ) diff --git a/tests/e2e/app/opened-documents.spec.ts b/tests/e2e/app/opened-documents.spec.ts index be8b27edc..1859640b0 100644 --- a/tests/e2e/app/opened-documents.spec.ts +++ b/tests/e2e/app/opened-documents.spec.ts @@ -1,32 +1,60 @@ -import { expect, test } from '@playwright/test' +import { expect, test, type Page } from '@playwright/test' import type * as AppTabs from '@/app/tabs' import { CanvasHelper } from '#tests/helpers/canvas' import { testPath } from '#tests/helpers/paths' -// An auto-layout file whose layout the app recomputes on its first page. -const FIXTURE = 'gold-preview.fig' - -// Opening a page lays it out, which updates node sizes and positions but is not an edit. -test('an opened .fig file stays saved until it is edited', async ({ page }) => { - await page.route(`**/__fixtures/${FIXTURE}`, (route) => - route.fulfill({ path: testPath('fixtures', FIXTURE) }) +async function openFixture(page: Page, fixture: string) { + await page.route(`**/__fixtures/${fixture}`, (route) => + route.fulfill({ path: testPath('fixtures', fixture) }) ) await page.goto('/?test') await new CanvasHelper(page).waitForInit() - await page.evaluate(async (fixture) => { + await page.evaluate(async (name) => { const tabsURL = '/src/app/tabs/index.ts' const tabs: typeof AppTabs = await import(tabsURL) - const response = await fetch(`/__fixtures/${fixture}`) - await tabs.openFileInNewTab(new File([await response.arrayBuffer()], fixture)) - }, FIXTURE) - - const tab = page.locator('[data-slot="tab-item"]').filter({ hasText: 'gold-preview' }) + const response = await fetch(`/__fixtures/${name}`) + await tabs.openFileInNewTab(new File([await response.arrayBuffer()], name)) + }, fixture) + const tab = page + .locator('[data-slot="tab-item"]') + .filter({ hasText: fixture.replace(/\.fig$/, '') }) await expect(tab).toBeVisible() + await waitForPreparation(page) + return tab +} + +async function waitForPreparation(page: Page) { await expect .poll(() => page.evaluate(() => window.openPencil?.getStore?.()?.state.preparation ?? null)) .toBeNull() +} + +// Opening a page lays it out, which updates node sizes and positions but is not an edit. +test('an opened .fig file stays saved until it is edited', async ({ page }) => { + // An auto-layout file whose layout the app recomputes on its first page. + const tab = await openFixture(page, 'gold-preview.fig') + await expect(tab.getByRole('img', { name: 'Unsaved changes' })).toHaveCount(0) + + await tab.getByTestId('tabbar-close').click() + await expect(page.getByRole('alertdialog')).toHaveCount(0) + await expect(tab).toHaveCount(0) +}) + +// A page's layers load from the opened file the first time it is shown. +test('showing another page of an opened .fig file keeps it saved', async ({ page }) => { + const tab = await openFixture(page, 'slots.fig') + await page.getByTestId('pages-item').filter({ hasText: 'Slots fixture' }).click() + await waitForPreparation(page) + await expect + .poll(() => + page.evaluate(() => { + const store = window.openPencil?.getStore?.() + return store ? store.graph.getChildren(store.state.currentPageId).length : 0 + }) + ) + .toBeGreaterThan(0) await expect(tab.getByRole('img', { name: 'Unsaved changes' })).toHaveCount(0) await tab.getByTestId('tabbar-close').click() diff --git a/tests/engine/app/document/autosave/create.test.ts b/tests/engine/app/document/autosave/create.test.ts index 211300124..27e780fac 100644 --- a/tests/engine/app/document/autosave/create.test.ts +++ b/tests/engine/app/document/autosave/create.test.ts @@ -1,8 +1,6 @@ import { describe, expect, test } from 'bun:test' -import { reactive } from 'vue' - -import { createDefaultEditorState } from '@open-pencil/core/editor' +import { reactive, ref } from 'vue' import { createAutosave } from '@/app/document/autosave/create' @@ -15,14 +13,13 @@ function deferred() { } function setup(saveCurrentDocument: (version: number) => Promise) { - const state = reactive({ - ...createDefaultEditorState('page-1'), - autosaveEnabled: true - }) + const state = reactive({ autosaveEnabled: true }) + const version = ref(0) let savedVersion = 0 let writable = true const autosave = createAutosave({ state, + version: () => version.value, getSavedVersion: () => savedVersion, hasWritableSource: () => writable, saveCurrentDocument: async (version) => { @@ -32,6 +29,7 @@ function setup(saveCurrentDocument: (version: number) => Promise) { }) return { state, + version, autosave, setWritable: (value: boolean) => { writable = value @@ -42,19 +40,19 @@ function setup(saveCurrentDocument: (version: number) => Promise) { describe('document autosave', () => { test('skips saved versions and documents without writable sources', async () => { const versions: number[] = [] - const { state, autosave, setWritable } = setup(async (version) => { - versions.push(version) + const { version, autosave, setWritable } = setup(async (saved) => { + versions.push(saved) }) await autosave.requestSave(0) setWritable(false) - state.sceneVersion = 1 + version.value = 1 await autosave.requestSave(1) expect(versions).toEqual([]) setWritable(true) await autosave.requestSave(1) - state.sceneVersion = 0 + version.value = 0 await autosave.requestSave(0) await autosave.requestSave(1) expect(versions).toEqual([1]) @@ -64,17 +62,17 @@ describe('document autosave', () => { test('coalesces 100 edits during an in-flight save into the latest version', async () => { const firstSave = deferred() const started: number[] = [] - const { state, autosave } = setup(async (version) => { - started.push(version) + const { version, autosave } = setup(async (saved) => { + started.push(saved) if (started.length === 1) await firstSave.promise }) - state.sceneVersion = 1 + version.value = 1 const pending = autosave.requestSave(1) await Promise.resolve() - for (let version = 2; version <= 101; version++) { - state.sceneVersion = version - void autosave.requestSave(version) + for (let next = 2; next <= 101; next++) { + version.value = next + void autosave.requestSave(next) } firstSave.resolve() await pending @@ -85,11 +83,11 @@ describe('document autosave', () => { test('retries the current version after a failed save', async () => { let attempts = 0 - const { state, autosave } = setup(async () => { + const { version, autosave } = setup(async () => { attempts++ if (attempts === 1) throw new Error('write failed') }) - state.sceneVersion = 1 + version.value = 1 await expect(autosave.requestSave(1)).rejects.toThrow('write failed') await autosave.requestSave(1) @@ -101,18 +99,18 @@ describe('document autosave', () => { test('preserves a newer requested version when the active save fails', async () => { const firstSave = deferred() const started: number[] = [] - const { state, autosave } = setup(async (version) => { - started.push(version) - if (version === 1) { + const { version, autosave } = setup(async (saved) => { + started.push(saved) + if (saved === 1) { await firstSave.promise throw new Error('write failed') } }) - state.sceneVersion = 1 + version.value = 1 const failed = autosave.requestSave(1) await Promise.resolve() - state.sceneVersion = 2 + version.value = 2 void autosave.requestSave(2) firstSave.resolve() await expect(failed).rejects.toThrow('write failed') diff --git a/tests/engine/app/document/io/changes.test.ts b/tests/engine/app/document/io/changes.test.ts index 520cc945d..3b4e233fd 100644 --- a/tests/engine/app/document/io/changes.test.ts +++ b/tests/engine/app/document/io/changes.test.ts @@ -1,7 +1,11 @@ import { expect, test } from 'bun:test' import { createEditor } from '@open-pencil/core/editor' +import { exportFigFile, parseFigFile } from '@open-pencil/core/io' +import { populateFigPage } from '@open-pencil/core/io/formats/fig' +import { initCodec } from '@open-pencil/core/kiwi' import { computeAllLayouts } from '@open-pencil/core/layout' +import { SceneGraph } from '@open-pencil/scene-graph' import { createDocumentChanges } from '@/app/document/io/changes' @@ -76,3 +80,26 @@ test('laying out a page does not dirty a document; an edit that relays it out do editor.dispose() } }) + +test("loading a page's layers from the opened file does not dirty a document", async () => { + await initCodec() + const source = new SceneGraph() + source.createNode('RECTANGLE', source.addPage('Second').id, { name: 'Loaded later' }) + const bytes = await exportFigFile(source) + const graph = await parseFigFile(bytes.slice().buffer, { populate: 'first-page' }) + const editor = createEditor({ graph }) + const changes = createDocumentChanges(editor) + try { + const page = graph.getPages()[1] + expect(populateFigPage(graph, page.id)).toBe(true) + computeAllLayouts(graph, page.id) + expect(graph.getChildren(page.id)).toHaveLength(1) + expect(changes.hasUnsavedChanges()).toBe(false) + + editor.graph.createNode('RECTANGLE', page.id, { width: 10, height: 10 }) + expect(changes.hasUnsavedChanges()).toBe(true) + } finally { + changes.dispose() + editor.dispose() + } +}) diff --git a/tests/engine/app/document/io/source-identity.test.ts b/tests/engine/app/document/io/source-identity.test.ts index 394e39c21..c060a32a2 100644 --- a/tests/engine/app/document/io/source-identity.test.ts +++ b/tests/engine/app/document/io/source-identity.test.ts @@ -26,6 +26,7 @@ function createSaveHarness(handle: FileSystemFileHandle) { const setSourceIdentity = vi.fn() const actions = createSaveActions({ state, + version: () => 1, buildFigFile: () => new Uint8Array([1, 2, 3]), getFilePath: () => null, setFilePath: vi.fn(), diff --git a/tests/engine/app/document/recovery/controller.test.ts b/tests/engine/app/document/recovery/controller.test.ts index df63f40eb..0ac329036 100644 --- a/tests/engine/app/document/recovery/controller.test.ts +++ b/tests/engine/app/document/recovery/controller.test.ts @@ -2,8 +2,9 @@ import { describe, expect, test } from 'bun:test' import { reactive, ref } from 'vue' -import { createDefaultEditorState } from '@open-pencil/core/editor' +import { createEditor } from '@open-pencil/core/editor' +import { createDocumentChanges } from '@/app/document/io/changes' import { createDocumentRecovery } from '@/app/document/recovery/controller' import { createMemoryRecoveryStore } from '@/app/document/recovery/memory' import type { RecoverySnapshotInput, RecoveryStore } from '@/app/document/recovery/types' @@ -38,25 +39,28 @@ function deferredRemoveStore() { return { store, release: () => release?.() } } +/** Snapshots built by default hold the version they were built at, to show which one was kept. */ function setup( - buildFigFile = async () => new Uint8Array([1, 2, 3]), + buildFigFile?: () => Promise, initialEnabled = true, injectedStore?: RecoveryStore ) { - const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Agent draft' }) + const state = reactive({ documentName: 'Agent draft' }) + const version = ref(0) const store = injectedStore ?? createMemoryRecoveryStore() let writable = false const enabled = ref(initialEnabled) const recovery = createDocumentRecovery({ state, + version: () => version.value, store, recoveryId: 'recovery-1', hasWritableSource: () => writable, isEnabled: () => enabled.value, - buildFigFile + buildFigFile: buildFigFile ?? (async () => new Uint8Array([version.value])) }) return { - state, + version, store, recovery, setWritable: (value: boolean) => (writable = value), @@ -66,23 +70,23 @@ function setup( describe('document recovery controller', () => { test('persists source-less changes and skips untouched documents', async () => { - const { state, store, recovery } = setup() + const { version, store, recovery } = setup() await recovery.persistNow() expect(await store.list()).toEqual([]) - state.sceneVersion = 1 + version.value = 1 await recovery.persistNow() - expect((await store.read('recovery-1'))?.sceneVersion).toBe(1) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(1) recovery.disposeRecovery() }) test('does not serialize or persist while disabled', async () => { let builds = 0 - const { state, store, recovery } = setup(async () => { + const { version, store, recovery } = setup(async () => { builds++ return new Uint8Array([1]) }, false) - state.sceneVersion = 1 + version.value = 1 await recovery.persistNow() expect(builds).toBe(0) expect(await store.list()).toEqual([]) @@ -90,8 +94,8 @@ describe('document recovery controller', () => { }) test('removes the owned snapshot when disabled and resumes from the current version', async () => { - const { state, store, recovery, setEnabled } = setup() - state.sceneVersion = 1 + const { version, store, recovery, setEnabled } = setup() + version.value = 1 await recovery.persistNow() expect(await store.list()).toHaveLength(1) @@ -100,62 +104,62 @@ describe('document recovery controller', () => { await Promise.resolve() expect(await store.list()).toEqual([]) - state.sceneVersion = 2 + version.value = 2 setEnabled(true) await recovery.persistNow() expect(await store.list()).toEqual([]) - state.sceneVersion = 3 + version.value = 3 await recovery.persistNow() - expect((await store.read('recovery-1'))?.sceneVersion).toBe(3) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(3) recovery.disposeRecovery() }) test('waits for disable cleanup before writing after re-enable', async () => { const deferred = deferredRemoveStore() - const { state, store, recovery, setEnabled } = setup(undefined, true, deferred.store) - state.sceneVersion = 1 + const { version, store, recovery, setEnabled } = setup(undefined, true, deferred.store) + version.value = 1 await recovery.persistNow() setEnabled(false) setEnabled(true) - state.sceneVersion = 2 + version.value = 2 const nextWrite = recovery.persistNow() await Promise.resolve() - expect((await store.read('recovery-1'))?.sceneVersion).toBe(1) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(1) deferred.release() await nextWrite - expect((await store.read('recovery-1'))?.sceneVersion).toBe(2) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(2) recovery.disposeRecovery() }) test('does not persist documents with writable sources', async () => { - const { state, store, recovery, setWritable } = setup() + const { version, store, recovery, setWritable } = setup() setWritable(true) - state.sceneVersion = 1 + version.value = 1 await recovery.persistNow() expect(await store.list()).toEqual([]) recovery.disposeRecovery() }) - test('recovery coalesces 100 changes during encoding to the latest scene version', async () => { + test('recovery coalesces 100 changes during encoding to the latest version', async () => { let release: (() => void) | null = null let calls = 0 - const { state, store, recovery } = setup(async () => { + const { version, store, recovery } = setup(async () => { calls++ if (calls === 1) { await new Promise((resolve) => { release = resolve }) } - return new Uint8Array([calls]) + return new Uint8Array([version.value]) }) - state.sceneVersion = 1 + version.value = 1 const pending = recovery.persistNow() await Promise.resolve() - for (let version = 2; version <= 101; version++) { - state.sceneVersion = version + for (let next = 2; next <= 101; next++) { + version.value = next void recovery.persistNow() } const releaseFirst = () => { @@ -165,12 +169,13 @@ describe('document recovery controller', () => { await pending expect(calls).toBe(2) - expect((await store.read('recovery-1'))?.sceneVersion).toBe(101) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(101) recovery.disposeRecovery() }) test('propagates persistence failures to close and reload callers', async () => { - const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Draft' }) + const state = reactive({ documentName: 'Draft' }) + const version = ref(0) const store = createMemoryRecoveryStore() const memoryWrite = store.write.bind(store) let writeAttempts = 0 @@ -181,23 +186,24 @@ describe('document recovery controller', () => { } const recovery = createDocumentRecovery({ state, + version: () => version.value, store, recoveryId: 'recovery-1', hasWritableSource: () => false, - buildFigFile: () => new Uint8Array([1]) + buildFigFile: () => new Uint8Array([version.value]) }) - state.sceneVersion = 1 + version.value = 1 await expect(recovery.persistNow()).rejects.toThrow('recovery storage unavailable') await recovery.persistNow() expect(writeAttempts).toBe(2) - expect((await store.read('recovery-1'))?.sceneVersion).toBe(1) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(1) recovery.disposeRecovery() }) test('successful save removes recovery data', async () => { - const { state, store, recovery } = setup() - state.sceneVersion = 1 + const { version, store, recovery } = setup() + version.value = 1 await recovery.persistNow() expect(await store.list()).toHaveLength(1) @@ -208,15 +214,17 @@ describe('document recovery controller', () => { test('save waits for an active write before deleting its snapshot', async () => { const deferred = deferredWriteStore() - const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Draft' }) + const state = reactive({ documentName: 'Draft' }) + const version = ref(0) const recovery = createDocumentRecovery({ state, + version: () => version.value, store: deferred.store, recoveryId: 'recovery-1', hasWritableSource: () => false, buildFigFile: () => new Uint8Array([1]) }) - state.sceneVersion = 1 + version.value = 1 const write = recovery.persistNow() await Promise.resolve() const cleanup = recovery.markProtectedVersion(1) @@ -229,15 +237,17 @@ describe('document recovery controller', () => { test('discard waits for an active write before deleting its snapshot', async () => { const deferred = deferredWriteStore() - const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Draft' }) + const state = reactive({ documentName: 'Draft' }) + const version = ref(0) const recovery = createDocumentRecovery({ state, + version: () => version.value, store: deferred.store, recoveryId: 'recovery-1', hasWritableSource: () => false, buildFigFile: () => new Uint8Array([1]) }) - state.sceneVersion = 1 + version.value = 1 const write = recovery.persistNow() await Promise.resolve() const discard = recovery.discardRecovery() @@ -250,18 +260,20 @@ describe('document recovery controller', () => { test('adoption waits for an active write and removes the previous recovery id', async () => { const deferred = deferredWriteStore() - const state = reactive({ ...createDefaultEditorState('page-1'), documentName: 'Draft' }) + const state = reactive({ documentName: 'Draft' }) + const version = ref(0) const recovery = createDocumentRecovery({ state, + version: () => version.value, store: deferred.store, recoveryId: 'previous', hasWritableSource: () => false, buildFigFile: () => new Uint8Array([1]) }) - state.sceneVersion = 1 + version.value = 1 const write = recovery.persistNow() await Promise.resolve() - const adoption = recovery.adoptRecoverySnapshot('recovered', 7) + const adoption = recovery.adoptRecoverySnapshot('recovered') deferred.release() await Promise.all([write, adoption]) @@ -271,13 +283,40 @@ describe('document recovery controller', () => { }) test('preserves a snapshot newer than the saved version', async () => { - const { state, store, recovery } = setup() - state.sceneVersion = 2 + const { version, store, recovery } = setup() + version.value = 2 await recovery.persistNow() await recovery.markProtectedVersion(1) - expect((await store.read('recovery-1'))?.sceneVersion).toBe(2) + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(2) recovery.disposeRecovery() }) + + test('follows content edits, not render requests', async () => { + const editor = createEditor() + const changes = createDocumentChanges(editor) + const store = createMemoryRecoveryStore() + const recovery = createDocumentRecovery({ + state: { documentName: 'Draft' }, + version: changes.capture, + store, + recoveryId: 'recovery-1', + hasWritableSource: () => false, + buildFigFile: () => new Uint8Array([changes.capture()]) + }) + try { + for (let i = 0; i < 5; i++) editor.requestRender() + await recovery.persistNow() + expect(await store.list()).toEqual([]) + + editor.createShape('RECTANGLE', 0, 0, 100, 100) + await recovery.persistNow() + expect((await store.read('recovery-1'))?.figBytes[0]).toBe(changes.capture()) + } finally { + recovery.disposeRecovery() + changes.dispose() + editor.dispose() + } + }) }) diff --git a/tests/engine/app/document/recovery/store.test.ts b/tests/engine/app/document/recovery/store.test.ts index e4ac0ef14..e2f107374 100644 --- a/tests/engine/app/document/recovery/store.test.ts +++ b/tests/engine/app/document/recovery/store.test.ts @@ -29,14 +29,12 @@ describe('document recovery store', () => { const metadata = await store.write({ id: 'recovery-1', documentName: 'Agent draft', - sceneVersion: 12, figBytes: bytes }) expect(metadata).toMatchObject({ id: 'recovery-1', documentName: 'Agent draft', - sceneVersion: 12, byteLength: 4, formatVersion: 1 }) @@ -51,7 +49,7 @@ describe('document recovery store', () => { test('memory store owns input and output bytes', async () => { const store = createMemoryRecoveryStore() const input = new Uint8Array(bytes) - await store.write({ id: 'one', documentName: 'Draft', sceneVersion: 1, figBytes: input }) + await store.write({ id: 'one', documentName: 'Draft', figBytes: input }) input[0] = 99 const first = await store.read('one') diff --git a/tests/engine/io/fig/import/lazy-pages.test.ts b/tests/engine/io/fig/import/lazy-pages.test.ts index 3bc5f6d59..438853f85 100644 --- a/tests/engine/io/fig/import/lazy-pages.test.ts +++ b/tests/engine/io/fig/import/lazy-pages.test.ts @@ -31,3 +31,19 @@ test('replacement session populates all remaining pages once', async () => { expect(graph.getPages().map((page) => graph.getChildren(page.id).length)).toEqual([1, 1]) expect(populateAllFigPages(graph)).toBe(false) }) + +test('loading a page keeps saving the opened bytes; an edit encodes the document again', async () => { + await initCodec() + const source = new SceneGraph() + source.createNode('RECTANGLE', source.getPages()[0].id, { name: 'First' }) + source.createNode('RECTANGLE', source.addPage('Page 2').id, { name: 'Second' }) + const bytes = await exportFigFile(source) + const graph = await parseFigFile(bytes.slice().buffer as ArrayBuffer, { populate: 'first-page' }) + + expect(populateFigPage(graph, graph.getPages()[1].id)).toBe(true) + expect(await exportFigFile(graph)).toEqual(bytes) + + const [second] = graph.getChildren(graph.getPages()[1].id) + graph.updateNode(second.id, { name: 'Renamed' }) + expect(await exportFigFile(graph)).not.toEqual(bytes) +}) diff --git a/tests/engine/render/canvas/guides/draw.test.ts b/tests/engine/render/canvas/guides/draw.test.ts index ea850bc25..38eaf635b 100644 --- a/tests/engine/render/canvas/guides/draw.test.ts +++ b/tests/engine/render/canvas/guides/draw.test.ts @@ -6,14 +6,15 @@ import type { SceneNode } from '@open-pencil/scene-graph' import { drawGuides } from '#core/canvas/guides/draw' import { createMockCanvas, createMockRenderer, mockCalls } from '../effects/helpers' -import { asCanvas, asDouble } from '../helpers' +import { asCanvas } from '../helpers' function graphWithGuides(guides: SceneNode['guides']): SceneGraph { - const page = createDefaultNode(() => 'page', 'CANVAS', { childIds: [], guides }) - return asDouble({ - rootId: 'root', - getNode: (id: string) => (id === 'page' ? page : null) - }) + const graph = new SceneGraph() + graph.rootId = 'root' + graph.nodes = new Map([ + ['page', createDefaultNode(() => 'page', 'CANVAS', { childIds: [], guides })] + ]) + return graph } describe('page guide rendering', () => { diff --git a/tests/engine/tauri/document-io.test.ts b/tests/engine/tauri/document-io.test.ts index 92c6be56f..230fb9829 100644 --- a/tests/engine/tauri/document-io.test.ts +++ b/tests/engine/tauri/document-io.test.ts @@ -37,7 +37,7 @@ describe('Tauri document IO helpers', () => { }) const savedVersions: number[] = [] const write = createDocumentWriter({ - state: { sceneVersion: 42 } as Parameters[0]['state'], + state: { documentName: 'Document' }, getFilePath: () => '/tmp/document.fig', getFileHandle: () => null, getStorageBinding: () => null, @@ -45,7 +45,7 @@ describe('Tauri document IO helpers', () => { setLastWriteTime: () => undefined }) - await write(new Uint8Array([1, 2, 3])) + await write(new Uint8Array([1, 2, 3]), 42) expect(calls).toHaveLength(1) expect(calls[0]?.cmd).toBe('plugin:fs|write_file') @@ -60,7 +60,7 @@ describe('Tauri document IO helpers', () => { await mockTauriIPC(() => null) const savedVersions: number[] = [] const write = createDocumentWriter({ - state: { sceneVersion: 42 } as Parameters[0]['state'], + state: { documentName: 'Document' }, getFilePath: () => '/tmp/document.fig', getFileHandle: () => null, getStorageBinding: () => null,