From d7613f34ec16431aa4c3880e91e9da712e60c550 Mon Sep 17 00:00:00 2001 From: Danila Poyarkov Date: Sun, 16 Aug 2026 15:43:25 +0300 Subject: [PATCH] perf(scene-graph): accelerate axis-aligned hit tests (#534) * perf(scene-graph): accelerate axis-aligned hit tests - Reuse cached world positions for untransformed node chains - Preserve exact matrix hit testing beneath rotated and flipped ancestors - Profile pointer, hit-test, cached, overlay, and volatile rendering at 500 and 2,000 nodes * fix(vue): restore layer drop indicators - Forward reactive drag instructions through virtualized layer item slots - Render above, below, and child feedback before the drop completes - Add browser regressions for reorder lines and container highlights * fix(scene-graph): harden hit-test performance coverage - Cache transformed ancestry within each hit-test traversal - Invalidate absolute positions after preview flips and reparenting - Attach scale-relative browser profiles and correct auto-layout fixture geometry * fix(scene-graph): reject cyclic layer reorder - Prevent reorderChild from parenting a node beneath its descendant - Cover graph preservation after a rejected cyclic reorder --- CHANGELOG.md | 2 + packages/scene-graph/src/hit-test.ts | 65 +++++-- packages/scene-graph/src/index.ts | 3 +- packages/scene-graph/src/preview.ts | 2 + .../primitives/LayerTree/LayerTreeItem.vue | 2 + src/components/LayerTree/LayerTree.vue | 17 +- tests/e2e/layers/drop-indicator.spec.ts | 103 +++++++++++ tests/e2e/perf/large-document.spec.ts | 169 ++++++++++++++++++ tests/engine/hit-test/scope.test.ts | 49 +++++ tests/helpers/large-document.ts | 97 ++++++++++ 10 files changed, 492 insertions(+), 17 deletions(-) create mode 100644 tests/e2e/layers/drop-indicator.spec.ts create mode 100644 tests/e2e/perf/large-document.spec.ts create mode 100644 tests/helpers/large-document.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 679b8ac9c..6eb0a31af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ ### Performance +- Use cached axis-aligned world positions for hit testing untransformed layer chains and add representative 500/2,000-node interaction profiles. (#527) - Coalesce writable-document autosaves that overlap an active `.fig` export while preserving a trailing save for newer edits. (#528) - Defer JSX generation and syntax highlighting until the Code panel is active, keeping large canvas selections responsive. (#500) - Index Figma clipboard children once during import instead of rescanning every pasted node, keeping large flat pastes linear. (#500) @@ -54,6 +55,7 @@ ### Fixed +- Restore visible above, below, and child drop feedback while dragging layers in the Layers panel. - Place editor-created instances beside nested source components in world space, including transformed source and destination parents. - Harden collaboration node synchronization against malformed remote source metadata and geometry while excluding derived text-renderer caches. - Transfer native `.fig` exports over binary Tauri IPC instead of JSON byte arrays, preventing large desktop saves from being truncated or exhausting WebView memory. (#484) diff --git a/packages/scene-graph/src/hit-test.ts b/packages/scene-graph/src/hit-test.ts index 3c263fabc..5665795cd 100644 --- a/packages/scene-graph/src/hit-test.ts +++ b/packages/scene-graph/src/hit-test.ts @@ -17,7 +17,40 @@ function hasVisibleFillOrStroke(node: SceneNode): boolean { return node.fills.some((f) => f.visible) || node.strokes.some((s) => s.visible) } -function containsPoint(px: number, py: number, node: SceneNode, graph: SceneGraph): boolean { +function hasTransformedAncestor( + node: SceneNode, + graph: SceneGraph, + cache: Map +): boolean { + const cached = cache.get(node.id) + if (cached !== undefined) return cached + const parent = node.parentId ? graph.getNode(node.parentId) : undefined + const transformed = + node.rotation !== 0 || + node.flipX || + node.flipY || + (parent ? hasTransformedAncestor(parent, graph, cache) : false) + cache.set(node.id, transformed) + return transformed +} + +function containsPoint( + px: number, + py: number, + node: SceneNode, + graph: SceneGraph, + transformCache: Map +): boolean { + if (!hasTransformedAncestor(node, graph, transformCache)) { + const absolute = graph.getAbsolutePosition(node.id) + return ( + px >= absolute.x && + px <= absolute.x + node.width && + py >= absolute.y && + py <= absolute.y + node.height + ) + } + const m = getWorldMatrix(node, graph) const inv = Matrix.invert(m) @@ -33,10 +66,11 @@ function hitTestOpaqueContainer( py: number, child: SceneNode, childId: string, - deep: boolean + deep: boolean, + transformCache: Map ): SceneNode | null { - if (!containsPoint(px, py, child, graph)) return null - const childHit = hitTestChildren(graph, px, py, childId, deep) + if (!containsPoint(px, py, child, graph, transformCache)) return null + const childHit = hitTestChildren(graph, px, py, childId, deep, transformCache) if (childHit) return child if (hasVisibleFillOrStroke(child)) return child return null @@ -47,23 +81,25 @@ function hitTestTransparentContainer( py: number, child: SceneNode, childId: string, - deep: boolean + deep: boolean, + transformCache: Map ): SceneNode | null { if (child.type === 'GROUP') { - if (!containsPoint(px, py, child, graph)) return null + if (!containsPoint(px, py, child, graph, transformCache)) return null - if (deep) return hitTestChildren(graph, px, py, childId, deep) ?? child + if (deep) return hitTestChildren(graph, px, py, childId, deep, transformCache) ?? child return child } - const childHit = hitTestChildren(graph, px, py, childId, deep) + const childHit = hitTestChildren(graph, px, py, childId, deep, transformCache) if (childHit) { if (child.locked) return child return childHit } - if (containsPoint(px, py, child, graph) && hasVisibleFillOrStroke(child)) return child + if (containsPoint(px, py, child, graph, transformCache) && hasVisibleFillOrStroke(child)) + return child return null } @@ -72,13 +108,14 @@ function hitTestChildren( px: number, py: number, parentId: string, - deep = false + deep = false, + transformCache = new Map() ): SceneNode | null { const parent = graph.nodes.get(parentId) if (!parent) return null if (parent.clipsContent) { - if (!containsPoint(px, py, parent, graph)) return null + if (!containsPoint(px, py, parent, graph, transformCache)) return null } for (let i = parent.childIds.length - 1; i >= 0; i--) { @@ -87,17 +124,17 @@ function hitTestChildren( if (!child || child.internalOnly || !child.visible) continue if (CONTAINER_TYPES.has(child.type)) { if (OPAQUE_CONTAINER_TYPES.has(child.type) && !deep) { - const hit = hitTestOpaqueContainer(graph, px, py, child, childId, deep) + const hit = hitTestOpaqueContainer(graph, px, py, child, childId, deep, transformCache) if (hit) return hit continue } - const hit = hitTestTransparentContainer(graph, px, py, child, childId, deep) + const hit = hitTestTransparentContainer(graph, px, py, child, childId, deep, transformCache) if (hit) return hit continue } - if (containsPoint(px, py, child, graph)) return child + if (containsPoint(px, py, child, graph, transformCache)) return child } return null diff --git a/packages/scene-graph/src/index.ts b/packages/scene-graph/src/index.ts index c25cc7218..9d073d07c 100644 --- a/packages/scene-graph/src/index.ts +++ b/packages/scene-graph/src/index.ts @@ -481,7 +481,7 @@ export class SceneGraph { const oldParent = node.parentId ? this.nodes.get(node.parentId) : undefined const newParent = this.nodes.get(parentId) - if (!newParent) return + if (!newParent || this.isDescendant(parentId, nodeId)) return // Remove from old parent if (oldParent) { @@ -498,6 +498,7 @@ export class SceneGraph { } node.parentId = parentId + this.absPosCache.clear() idx = Math.min(idx, newParent.childIds.length) newParent.childIds.splice(idx, 0, nodeId) diff --git a/packages/scene-graph/src/preview.ts b/packages/scene-graph/src/preview.ts index fd7be38af..ca5531fdd 100644 --- a/packages/scene-graph/src/preview.ts +++ b/packages/scene-graph/src/preview.ts @@ -14,6 +14,8 @@ const LAYOUT_AFFECTING_KEYS = new Set([ 'width', 'height', 'rotation', + 'flipX', + 'flipY', 'parentId', 'childIds', 'layoutMode', diff --git a/packages/vue/src/primitives/LayerTree/LayerTreeItem.vue b/packages/vue/src/primitives/LayerTree/LayerTreeItem.vue index 3088a3d52..a38d34c11 100644 --- a/packages/vue/src/primitives/LayerTree/LayerTreeItem.vue +++ b/packages/vue/src/primitives/LayerTree/LayerTreeItem.vue @@ -74,6 +74,8 @@ defineExpose({ rowEl }) :has-children="hasChildren" :is-selected="isSelected" :is-dragging="isDragging" + :instruction="ctx.instruction.value" + :instruction-target-id="ctx.instructionTargetId.value" :focused="ctx.focused.value" :pad-left="padLeft" :actions="actions" diff --git a/src/components/LayerTree/LayerTree.vue b/src/components/LayerTree/LayerTree.vue index b2086819a..e7c3f6e9e 100644 --- a/src/components/LayerTree/LayerTree.vue +++ b/src/components/LayerTree/LayerTree.vue @@ -186,7 +186,14 @@ function onFocusOut(event: FocusEvent, actions: LayerTreeRootActions) { " > diff --git a/tests/e2e/layers/drop-indicator.spec.ts b/tests/e2e/layers/drop-indicator.spec.ts new file mode 100644 index 000000000..c0c711f69 --- /dev/null +++ b/tests/e2e/layers/drop-indicator.spec.ts @@ -0,0 +1,103 @@ +import { expect, test, type Page } from '@playwright/test' + +import type { Vector } from '@open-pencil/scene-graph' + +import { CanvasHelper } from '#tests/helpers/canvas' + +async function dragLayerAndObserveIndicator( + page: Page, + sourceId: string, + targetId: string, + targetPosition: Vector +) { + await page.evaluate(() => { + const positions: string[] = [] + new MutationObserver(() => { + for (const element of document.querySelectorAll( + '[data-slot="drop-indicator"]' + )) { + const position = element.dataset.dropPosition + if (position) positions.push(position) + } + }).observe(document.body, { subtree: true, childList: true, attributes: true }) + Object.assign(window, { __layerDropPositions: positions }) + }) + + const source = page.locator(`[data-node-id="${sourceId}"]`) + const target = page.locator(`[data-node-id="${targetId}"]`) + const sourceBox = await source.boundingBox() + const targetBox = await target.boundingBox() + if (!sourceBox || !targetBox) throw new Error('Layer row bounds unavailable') + + await page.mouse.move(sourceBox.x + 80, sourceBox.y + 12) + await page.mouse.down() + await page.mouse.move(sourceBox.x + 84, sourceBox.y + 8, { steps: 5 }) + await page.mouse.move(targetBox.x + targetPosition.x, targetBox.y + targetPosition.y, { + steps: 20 + }) + await expect(target.locator('[data-slot="drop-indicator"]')).toBeVisible() + await page.mouse.up() + + return page.evaluate( + () => (window as typeof window & { __layerDropPositions?: string[] }).__layerDropPositions ?? [] + ) +} + +test('layer reorder exposes a visible drop indicator before dropping', async ({ page }) => { + await page.goto('/') + const canvas = new CanvasHelper(page) + await canvas.waitForInit() + canvas.errors.length = 0 + await canvas.clearCanvas() + + const ids = await page.evaluate(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const pageId = store.state.currentPageId + const first = store.graph.createNode('RECTANGLE', pageId, { name: 'Layer A' }) + store.graph.createNode('RECTANGLE', pageId, { name: 'Layer B' }) + const third = store.graph.createNode('RECTANGLE', pageId, { name: 'Layer C' }) + store.requestRender() + return { first: first.id, third: third.id } + }) + await canvas.waitForRender() + + const positions = await dragLayerAndObserveIndicator(page, ids.third, ids.first, { + x: 80, + y: 2 + }) + + expect(positions).toContain('above') + canvas.assertNoErrors() +}) + +test('layer child drop exposes a visible container highlight before dropping', async ({ page }) => { + await page.goto('/') + const canvas = new CanvasHelper(page) + await canvas.waitForInit() + canvas.errors.length = 0 + await canvas.clearCanvas() + + const ids = await page.evaluate(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const pageId = store.state.currentPageId + const frame = store.graph.createNode('FRAME', pageId, { + name: 'Drop Frame', + width: 200, + height: 120 + }) + const rect = store.graph.createNode('RECTANGLE', pageId, { name: 'Child Candidate' }) + store.requestRender() + return { frame: frame.id, rect: rect.id } + }) + await canvas.waitForRender() + + const positions = await dragLayerAndObserveIndicator(page, ids.rect, ids.frame, { + x: 80, + y: 12 + }) + + expect(positions).toContain('child') + canvas.assertNoErrors() +}) diff --git a/tests/e2e/perf/large-document.spec.ts b/tests/e2e/perf/large-document.spec.ts new file mode 100644 index 000000000..fd126f68a --- /dev/null +++ b/tests/e2e/perf/large-document.spec.ts @@ -0,0 +1,169 @@ +import { expect, test } from '@playwright/test' + +import { CanvasHelper } from '#tests/helpers/canvas' +import { seedLargeDocument } from '#tests/helpers/large-document' + +type TimingSummary = { + hitTestMissMs: number + hitTestHitMs: number + cachedFrameMs: number + overlayFrameMs: number + volatileFrameMs: number + volatileMode: string +} + +type PerformanceProfile = { + timings: TimingSummary + pointer: { calls: number; totalMs: number } +} + +const SCALES = [500, 2000] +const profiles = new Map() + +test.describe.serial('large-document performance', () => { + for (const nodeCount of SCALES) { + test(`profiles representative ${nodeCount}-node interactions`, async ({ page }, testInfo) => { + test.setTimeout(120_000) + await page.goto('/?test&no-chrome&no-rulers') + const canvas = new CanvasHelper(page) + await canvas.waitForInit() + await canvas.clearCanvas() + const fixture = await seedLargeDocument(page, nodeCount) + await canvas.waitForRender() + + await page.evaluate(() => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const originalHitTest = store.graph.hitTest.bind(store.graph) + let calls = 0 + let totalMs = 0 + store.graph.hitTest = ((...args) => { + const startedAt = performance.now() + const result = originalHitTest(...args) + totalMs += performance.now() - startedAt + calls++ + return result + }) as typeof store.graph.hitTest + Object.assign(window, { + __largeDocumentPointerProfile: () => ({ calls, totalMs }) + }) + }) + const bounds = await canvas.canvas.boundingBox() + if (!bounds) throw new Error('Canvas bounds unavailable') + await page.mouse.move(bounds.x + 10, bounds.y + 10) + await page.mouse.move(bounds.x + bounds.width - 10, bounds.y + bounds.height - 10, { + steps: 40 + }) + const pointerProfile = await page.evaluate(() => { + const profile = ( + window as typeof window & { + __largeDocumentPointerProfile?: () => { calls: number; totalMs: number } + } + ).__largeDocumentPointerProfile + return profile?.() ?? { calls: 0, totalMs: 0 } + }) + + const result = await page.evaluate((profile): Promise => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const renderer = store.renderer + if (!renderer) throw new Error('OpenPencil renderer not initialized') + const graph = store.graph + const iterations = 50 + + function average(run: () => void) { + const startedAt = performance.now() + for (let index = 0; index < iterations; index++) run() + return (performance.now() - startedAt) / iterations + } + + renderer.dpr = window.devicePixelRatio || 1 + renderer.panX = store.state.panX + renderer.panY = store.state.panY + renderer.zoom = store.state.zoom + renderer.viewportWidth = 1280 + renderer.viewportHeight = 800 + renderer.showRulers = false + renderer.pageColor = store.state.pageColor + renderer.pageId = store.state.currentPageId + renderer.render(graph, store.state.selectedIds, {}, store.state.sceneVersion) + + const lastId = profile.leafIds.at(-1) + const lastNode = lastId ? graph.getNode(lastId) : null + if (!lastNode) throw new Error('Large-document target not found') + const lastPosition = graph.getAbsolutePosition(lastNode.id) + + const hitTestMissMs = average(() => { + graph.hitTest( + profile.worldWidth + 100, + profile.worldHeight + 100, + store.state.currentPageId + ) + }) + const hitTestHitMs = average(() => { + graph.hitTest( + lastPosition.x + lastNode.width / 2, + lastPosition.y + lastNode.height / 2, + store.state.currentPageId + ) + }) + const cachedFrameMs = average(() => { + renderer.render(graph, store.state.selectedIds, {}, store.state.sceneVersion) + }) + const overlayFrameMs = average(() => { + renderer.render( + graph, + store.state.selectedIds, + { hoveredNodeId: lastNode.id }, + store.state.sceneVersion + ) + }) + const volatileFrameMs = average(() => { + renderer.render( + graph, + store.state.selectedIds, + { rotationPreview: { nodeId: lastNode.id, angle: 1 } }, + store.state.sceneVersion + ) + }) + + return { + hitTestMissMs, + hitTestHitMs, + cachedFrameMs, + overlayFrameMs, + volatileFrameMs, + volatileMode: renderer.profiler.stats.scenePictureMode + } + }, fixture) + + const profile = { timings: result, pointer: pointerProfile } + profiles.set(nodeCount, profile) + await testInfo.attach(`large-document-${nodeCount}.json`, { + body: JSON.stringify(profile, null, 2), + contentType: 'application/json' + }) + + expect(fixture.nodeCount).toBe(nodeCount) + expect(pointerProfile.calls).toBeGreaterThan(0) + expect(result.volatileMode).toBe('volatile') + expect(result.hitTestMissMs).toBeGreaterThanOrEqual(0) + expect(result.hitTestHitMs).toBeGreaterThanOrEqual(0) + expect(result.cachedFrameMs).toBeGreaterThanOrEqual(0) + expect(result.overlayFrameMs).toBeGreaterThanOrEqual(0) + expect(result.volatileFrameMs).toBeGreaterThanOrEqual(0) + + const smallerProfile = profiles.get(SCALES[0]) + if (nodeCount !== SCALES[0] && smallerProfile) { + const scaleRatio = nodeCount / SCALES[0] + expect(result.hitTestMissMs).toBeLessThanOrEqual( + Math.max(smallerProfile.timings.hitTestMissMs * scaleRatio * 2, 1) + ) + expect(pointerProfile.totalMs).toBeLessThanOrEqual( + Math.max(smallerProfile.pointer.totalMs * scaleRatio * 2, 10) + ) + } + expect(canvas.errors.filter((error) => !error.includes('127.0.0.1:7600'))).toEqual([]) + }) + } +}) diff --git a/tests/engine/hit-test/scope.test.ts b/tests/engine/hit-test/scope.test.ts index c959058ad..cbed52a16 100644 --- a/tests/engine/hit-test/scope.test.ts +++ b/tests/engine/hit-test/scope.test.ts @@ -288,6 +288,55 @@ describe('hitTest — frame with children', () => { const hitOutside = graph.hitTest(160, 130, frame.id) expect(hitOutside).toBeNull() }) + + test('cached absolute positions refresh after preview flips and reparenting', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const left = graph.createNode('FRAME', page, { x: 100, y: 100, width: 200, height: 100 }) + const right = graph.createNode('FRAME', page, { x: 400, y: 100, width: 200, height: 100 }) + const child = graph.createNode('RECTANGLE', left.id, { + x: 20, + y: 20, + width: 40, + height: 40 + }) + + expect(graph.hitTest(130, 130, left.id)?.id).toBe(child.id) + graph.updateNodePreview(left.id, { flipX: true }) + expect(graph.hitTest(270, 130, left.id)?.id).toBe(child.id) + expect(graph.hitTest(130, 130, left.id)).toBeNull() + + graph.updateNode(left.id, { flipX: false }) + expect(graph.getAbsolutePosition(child.id)).toEqual({ x: 120, y: 120 }) + graph.reorderChild(child.id, right.id, 0) + expect(graph.getAbsolutePosition(child.id)).toEqual({ x: 420, y: 120 }) + expect(graph.hitTest(430, 130, right.id)?.id).toBe(child.id) + + graph.reorderChild(right.id, child.id, 0) + expect(right.parentId).toBe(page) + expect(child.parentId).toBe(right.id) + }) + + test('unrotated child inside a flipped ancestor uses transformed hit testing', () => { + const graph = new SceneGraph() + const page = pageId(graph) + const frame = graph.createNode('FRAME', page, { + x: 100, + y: 100, + width: 200, + height: 100, + flipX: true + }) + const child = graph.createNode('RECTANGLE', frame.id, { + x: 20, + y: 20, + width: 40, + height: 40 + }) + + expect(graph.hitTest(250, 140, frame.id)?.id).toBe(child.id) + expect(graph.hitTest(130, 140, frame.id)).toBeNull() + }) }) describe('hitTest — opaque containers (COMPONENT/INSTANCE)', () => { diff --git a/tests/helpers/large-document.ts b/tests/helpers/large-document.ts new file mode 100644 index 000000000..4656755c9 --- /dev/null +++ b/tests/helpers/large-document.ts @@ -0,0 +1,97 @@ +import type { Page } from '@playwright/test' + +export type LargeDocumentProfile = { + nodeCount: number + leafIds: string[] + worldWidth: number + worldHeight: number +} + +/** Build a deterministic mixed document without timing browser-side setup. */ +export async function seedLargeDocument( + page: Page, + nodeCount: number +): Promise { + return page.evaluate((count) => { + const store = window.openPencil?.getStore?.() + if (!store) throw new Error('OpenPencil store not initialized') + const graph = store.graph + const pageId = store.state.currentPageId + const leafIds: string[] = [] + const cards = Math.ceil(count / 10) + const columns = Math.ceil(Math.sqrt(cards)) + const cardWidth = 240 + const cardHeight = 236 + const gap = 32 + + for (let cardIndex = 0; cardIndex < cards && leafIds.length < count; cardIndex++) { + const column = cardIndex % columns + const row = Math.floor(cardIndex / columns) + const frame = graph.createNode('FRAME', pageId, { + name: `Card ${cardIndex}`, + x: column * (cardWidth + gap), + y: row * (cardHeight + gap), + width: cardWidth, + height: cardHeight, + layoutMode: 'VERTICAL', + itemSpacing: 8, + paddingTop: 12, + paddingRight: 12, + paddingBottom: 12, + paddingLeft: 12, + fills: [ + { + type: 'SOLID', + color: { r: 0.96, g: 0.97, b: 0.99, a: 1 }, + opacity: 1, + visible: true + } + ], + effects: + cardIndex % 4 === 0 + ? [ + { + type: 'DROP_SHADOW', + color: { r: 0, g: 0, b: 0, a: 0.12 }, + offset: { x: 0, y: 3 }, + radius: 8, + spread: 0, + visible: true + } + ] + : [] + }) + + for (let childIndex = 0; childIndex < 10 && leafIds.length < count; childIndex++) { + const isText = childIndex % 3 === 0 + const node = graph.createNode(isText ? 'TEXT' : 'RECTANGLE', frame.id, { + name: `${isText ? 'Label' : 'Row'} ${cardIndex}-${childIndex}`, + text: isText ? `Item ${cardIndex}-${childIndex}` : undefined, + width: 200, + height: 14, + cornerRadius: isText ? 0 : 4, + fills: [ + { + type: 'SOLID', + color: isText + ? { r: 0.12, g: 0.14, b: 0.18, a: 1 } + : { r: 0.25, g: 0.48, b: 0.92, a: 1 }, + opacity: 1, + visible: true + } + ] + }) + leafIds.push(node.id) + } + } + + store.requestRender() + const rows = Math.ceil(cards / columns) + return { + nodeCount: leafIds.length, + leafIds, + worldWidth: columns * (cardWidth + gap), + worldHeight: rows * (cardHeight + gap) + } + }, nodeCount) +}