diff --git a/CHANGELOG.md b/CHANGELOG.md index a9e3e6027..acd582aee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -224,6 +224,7 @@ - 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. - Open multi-page `.fig` documents faster: the archive is indexed once rather than once for every page, each page resolves only the layers it adds instead of rescanning the whole document, placing an instance no longer re-synchronises every other instance of its component, and archive records are copied directly rather than through `structuredClone`. A 33-page file loads about a fifth quicker, and a page of repeated components opens three to four times faster once a document is already open. +- Lay out documents with many text layers and component instances without long freezes: text checks whether its fonts cover every glyph once rather than on every layout pass, and layout updating sizes no longer re-synchronises the components it touched. Building the demo document takes less than half as long. ### Security diff --git a/packages/core/src/canvas/renderer.ts b/packages/core/src/canvas/renderer.ts index 9b992a659..f2d0be4cf 100644 --- a/packages/core/src/canvas/renderer.ts +++ b/packages/core/src/canvas/renderer.ts @@ -511,8 +511,9 @@ export class SkiaRenderer { RendererState.invalidateAllPictures(this) } - invalidateNodePicture(nodeId: string): void { - RendererState.invalidateNodePicture(this, nodeId) + /** Drops `nodeId`'s cached drawing; `changedKeys`, when known, lets text keep its glyph coverage. */ + invalidateNodePicture(nodeId: string, changedKeys?: readonly (keyof SceneNode)[]): void { + RendererState.invalidateNodePicture(this, nodeId, changedKeys) } flashNode(nodeId: string): void { diff --git a/packages/core/src/canvas/renderer/state.ts b/packages/core/src/canvas/renderer/state.ts index 56ab4fe2c..ca0d986c6 100644 --- a/packages/core/src/canvas/renderer/state.ts +++ b/packages/core/src/canvas/renderer/state.ts @@ -1,4 +1,7 @@ +import type { SceneNode } from '@open-pencil/scene-graph' + import type { SkiaRenderer } from '#core/canvas/renderer' +import { changesGlyphCoverage } from '#core/canvas/text/paragraph-inputs' export function invalidateScenePicture(r: SkiaRenderer): void { r.scenePicture?.delete() @@ -32,8 +35,14 @@ export function invalidateAllPictures(r: SkiaRenderer): void { clearSubtreePictureCache(r) } -export function invalidateNodePicture(r: SkiaRenderer, nodeId: string): void { - r.textPreparationCache.deleteNode(nodeId) +export function invalidateNodePicture( + r: SkiaRenderer, + nodeId: string, + changedKeys?: readonly (keyof SceneNode)[] +): void { + r.textPreparationCache.deleteNode(nodeId, { + keepGlyphCoverage: changedKeys !== undefined && !changesGlyphCoverage(changedKeys) + }) r.effectRasterCache.delete(nodeId) r.effectRasterCache.deleteDependencies(nodeId) for (const [ownerId, dependencyIds] of r.nodePictureCacheDependencies) { diff --git a/packages/core/src/canvas/text/paragraph-inputs.ts b/packages/core/src/canvas/text/paragraph-inputs.ts index 04e13ba90..88fdf6a63 100644 --- a/packages/core/src/canvas/text/paragraph-inputs.ts +++ b/packages/core/src/canvas/text/paragraph-inputs.ts @@ -29,3 +29,24 @@ export const PARAGRAPH_INPUT_KEYS = [ ] as const satisfies readonly (keyof SceneNode)[] export type ParagraphNode = Pick + +const BOX_SIZE_KEYS = new Set(['width', 'height']) + +/** + * The paragraph inputs that decide which glyphs are shaped. The box size only matters when the + * text is truncated, since it decides which characters are shown; otherwise layout resizing the + * box leaves coverage as it was. + */ +export function glyphCoverageInputs(node: ParagraphNode): ParagraphNode[keyof ParagraphNode][] { + const truncated = node.textTruncation === 'ENDING' + return PARAGRAPH_INPUT_KEYS.filter((key) => truncated || !BOX_SIZE_KEYS.has(key)).map( + (key) => node[key] + ) +} + +/** Whether a change to `keys` can change glyph coverage beyond what `glyphCoverageInputs` sees. */ +export function changesGlyphCoverage(keys: readonly (keyof SceneNode)[]): boolean { + return keys.some( + (key) => !BOX_SIZE_KEYS.has(key) && (PARAGRAPH_INPUT_KEYS as readonly string[]).includes(key) + ) +} diff --git a/packages/core/src/canvas/text/preparation-cache.ts b/packages/core/src/canvas/text/preparation-cache.ts index 937005088..03f04fec1 100644 --- a/packages/core/src/canvas/text/preparation-cache.ts +++ b/packages/core/src/canvas/text/preparation-cache.ts @@ -10,7 +10,7 @@ import type { missingGlyphOccurrences } from '#core/text/resolver' const MAX_PREPARED_PARAGRAPHS = 1024 const MAX_PREPARED_TEXT_UNITS = 262_144 -import { PARAGRAPH_INPUT_KEYS } from './paragraph-inputs' +import { glyphCoverageInputs, PARAGRAPH_INPUT_KEYS } from './paragraph-inputs' type PreparationInput = SceneNode[(typeof PARAGRAPH_INPUT_KEYS)[number]] @@ -106,7 +106,12 @@ export class TextPreparationCache { } const inputs = this.glyphCoverage.get(node) if (!inputs) return false - if (PARAGRAPH_INPUT_KEYS.every((prop, index) => inputs[index] === node[prop])) return true + const current = glyphCoverageInputs(node) + if ( + current.length === inputs.length && + current.every((input, index) => input === inputs[index]) + ) + return true this.glyphCoverage.delete(node) return false } @@ -114,16 +119,17 @@ export class TextPreparationCache { /** Call only after observing complete coverage with this cache's current font scope. */ recordGlyphCoverage(node: SceneNode): void { this.invalidatedCoverage.delete(node.id) - this.glyphCoverage.set( - node, - PARAGRAPH_INPUT_KEYS.map((prop) => node[prop]) - ) + this.glyphCoverage.set(node, glyphCoverageInputs(node)) } - deleteNode(id: string): void { + /** + * Drops `id`'s paragraphs and, unless `keepGlyphCoverage`, its coverage. Keep coverage only for + * changes `glyphCoverageInputs` compares, such as layout resizing the box. + */ + deleteNode(id: string, { keepGlyphCoverage = false } = {}): void { // Invalidation arrives by ID; don't add strong node ownership just to find // weak observations. Bound pending IDs and conservatively reset on overflow. - this.invalidatedCoverage.add(id) + if (!keepGlyphCoverage) this.invalidatedCoverage.add(id) if (this.invalidatedCoverage.size > this.maxEntries) { this.glyphCoverage = new WeakMap() this.invalidatedCoverage.clear() diff --git a/packages/core/src/editor/component-sync.ts b/packages/core/src/editor/component-sync.ts index bc134f9e7..23d201df5 100644 --- a/packages/core/src/editor/component-sync.ts +++ b/packages/core/src/editor/component-sync.ts @@ -100,7 +100,10 @@ export function createComponentSyncScheduler( function scheduleComponentSync(nodeId: string) { // Import/materialization has already resolved component overrides. These updates // are not authored component edits and must not reset instances to their defaults. - if (isFlushingComponentSync || getGraph().isApplyingImportedState) return + // Layout's own writes are results, not edits: it lays out the instances too, and the edit + // that made it run has already scheduled its sync. + const graph = getGraph() + if (isFlushingComponentSync || graph.isApplyingImportedState || graph.isApplyingLayout) return if (!pendingComponentSync) { pendingComponentSync = new Set() queueMicrotask(flushComponentSync) diff --git a/packages/core/src/editor/graph-events.ts b/packages/core/src/editor/graph-events.ts index ccad4f94e..9b7cdd86c 100644 --- a/packages/core/src/editor/graph-events.ts +++ b/packages/core/src/editor/graph-events.ts @@ -70,7 +70,8 @@ function invalidateRenderersForChange( const invalidation = rendererInvalidationForChanges(changes, { preview: !invalidateNodePicture }) for (const renderer of renderers) { if (invalidation.geometryCache) renderer.invalidateVectorPath(id) - if (invalidation.nodePicture) renderer.invalidateNodePicture(id) + if (invalidation.nodePicture) + renderer.invalidateNodePicture(id, Object.keys(changes) as (keyof SceneNode)[]) if (Object.keys(changes).some((key) => TILED_CHUNK_TOPOLOGY_KEYS.has(key as keyof SceneNode))) { renderer.tiledScene.invalidateStructure() } else { diff --git a/packages/core/tests/editor/components/sync-layout-scope.test.ts b/packages/core/tests/editor/components/sync-layout-scope.test.ts index 3a379aa72..5b5aca81c 100644 --- a/packages/core/tests/editor/components/sync-layout-scope.test.ts +++ b/packages/core/tests/editor/components/sync-layout-scope.test.ts @@ -89,6 +89,27 @@ describe('component sync layout scope', () => { expect(scopes).toEqual([]) }) + test('layout writing its own results schedules no sync', async () => { + const { graph, label } = createGraph() + const scopes: (string | undefined)[] = [] + const { scheduleComponentSync } = createComponentSyncScheduler( + () => graph, + () => undefined, + (innerGraph, scopeId) => { + scopes.push(scopeId) + computeAllLayouts(innerGraph, scopeId) + } + ) + + graph.withLayoutMutations(() => { + graph.updateNode(label.id, { width: 120 }) + scheduleComponentSync(label.id) + }) + + await Promise.resolve() + expect(scopes).toEqual([]) + }) + test('a cross-page instance still receives the component layout', async () => { const { graph, instance, component } = createGraph() const { scheduleComponentSync } = createComponentSyncScheduler( diff --git a/tests/engine/render/canvas/picture-invalidation.test.ts b/tests/engine/render/canvas/picture-invalidation.test.ts index 5e2c0cbc9..07081fc26 100644 --- a/tests/engine/render/canvas/picture-invalidation.test.ts +++ b/tests/engine/render/canvas/picture-invalidation.test.ts @@ -71,8 +71,19 @@ test('node picture invalidation removes pictures that depend on a changed child' invalidateNodePicture(renderer, 'child') - expect(textPreparationCache.deleteNode).toHaveBeenCalledWith('child') + expect(textPreparationCache.deleteNode).toHaveBeenCalledWith('child', { + keepGlyphCoverage: false + }) expect(parentPicture.delete).toHaveBeenCalledTimes(1) expect(childPicture.delete).toHaveBeenCalledTimes(1) expect(renderer.nodePictureCache.size).toBe(0) + + invalidateNodePicture(renderer, 'child', ['width', 'height', 'x']) + expect(textPreparationCache.deleteNode).toHaveBeenLastCalledWith('child', { + keepGlyphCoverage: true + }) + invalidateNodePicture(renderer, 'child', ['width', 'text']) + expect(textPreparationCache.deleteNode).toHaveBeenLastCalledWith('child', { + keepGlyphCoverage: false + }) }) diff --git a/tests/engine/render/canvas/text-preparation-cache.test.ts b/tests/engine/render/canvas/text-preparation-cache.test.ts index 9632704ef..d337997ee 100644 --- a/tests/engine/render/canvas/text-preparation-cache.test.ts +++ b/tests/engine/render/canvas/text-preparation-cache.test.ts @@ -135,7 +135,19 @@ describe('text preparation cache', () => { expect(f.cache.hasGlyphCoverage(f.node, 1, otherProvider)).toBe(false) f.graph.updateNodePreview(f.node.id, { x: 50, rotation: 20 }) expect(f.cache.hasGlyphCoverage(f.node, 1, f.provider)).toBe(true) + // Layout resizing the box shapes the same glyphs, unless truncation hides some of them. f.graph.updateNodePreview(f.node.id, { width: 80 }) + expect(f.cache.hasGlyphCoverage(f.node, 1, f.provider)).toBe(true) + f.cache.deleteNode(f.node.id, { keepGlyphCoverage: true }) + expect(f.cache.hasGlyphCoverage(f.node, 1, f.provider)).toBe(true) + f.graph.updateNodePreview(f.node.id, { textTruncation: 'ENDING' }) + expect(f.cache.hasGlyphCoverage(f.node, 1, f.provider)).toBe(false) + f.cache.recordGlyphCoverage(f.node) + f.graph.updateNodePreview(f.node.id, { width: 60 }) + expect(f.cache.hasGlyphCoverage(f.node, 1, f.provider)).toBe(false) + f.graph.updateNodePreview(f.node.id, { textTruncation: 'DISABLED' }) + f.cache.recordGlyphCoverage(f.node) + f.cache.deleteNode(f.node.id) expect(f.cache.hasGlyphCoverage(f.node, 1, f.provider)).toBe(false) f.use('coverage', 1) f.cache.recordGlyphCoverage(f.node)