refactor: node geometry reads from the store, delete useLayoutSync (#14133)

## Summary

Make `layoutStore` authoritative for node geometry and remove the Layout
→ LiteGraph writeback loop. `LGraphNode.pos` / `size` remain stable
compatibility views backed by a lazily refreshed `Rectangle` projection.

## Why a projection

At 1000 nodes, a `Rectangle` read measured ~6 ns versus ~63.5 ns for
`ynodes.get(id)` → rect. The legacy canvas reads node geometry ~44 times
per frame, making direct Yjs reads cost roughly 2.5 ms/frame. A
geometry-version check keeps the common path local and refreshes each
accessed projection once after invalidation.

## Changes

- Project `pos` / `size` from the store while preserving indexed
mutation compatibility and command-based writes.
- Register and unregister projection lifetime with node layout lifetime.
- Delete `useLayoutSync`, including its RAF/microtask batching and
writeback machinery.
- Observe layout changes through a Vue-scoped effect: invalidate the
canvas for node changes and forward actual non-Canvas size changes to
legacy `onResize` callbacks. Canvas resizes retain synchronous callback
ownership through `setSize()`.
- Share the stored `[x, y, width, height]` tuple type between Yjs
mappers and validation.

## Review Focus

- The projection stays inert until the node's own layout entry exists,
preventing accidental adoption of geometry from an ID collision during
attach.
- Two `test.fails` cases pin known whole-value setter cache gaps in this
PR alone. Child PR [#14480](../pull/14480) fixes both; rebasing it
should produce unexpected passes, after which the `.fails` markers must
be removed.
- Projection lifecycle extraction and node-scoped invalidation are
deferred until after #14480 and documented in the ECS migration plan.

## Tests

- Stable-view refresh and indexed mutation behavior
- Removal/re-addition and graph-scoped layout state
- Exact-once Canvas resize callbacks and store-originated resize
callbacks
- Layout notification graph filtering and cleanup

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: 4 people

docs/architecture/ecs-migration-plan.md

@@ -434,9 +434,9 @@ entity now has exactly one write path, and the workarounds that existed because
 writes bypassed it are gone.
 
 **One write path per entity.** Whole-value assignment through `pos` / `size` is
-the only way geometry is written; the setters commit. Element-wise writes reach
-the backing `Rectangle` and never the store, so all of them were converted —
-in three passes, as the forms became apparent:
+the normal path, and the setters commit to the store. `createGeometryView`
+commits indexed writes too. Most in-repo indexed writes now use whole-value
+assignments. The migration found three forms:
 
 | Form               | Example                              | Where it hid         |
 | ------------------ | ------------------------------------ | -------------------- |
@@ -450,34 +450,66 @@ reached the store, and `LGraph.configure` carried a local workaround
 others had none.
 
 **Groups and reroutes joined the store.** `GroupLayout` is id/position/size
-with no zIndex or spatial index — groups draw beneath nodes in insertion order
-and nothing queries them positionally — and geometry is a single
+with no zIndex or spatial index. Groups draw beneath nodes in insertion order,
+and nothing queries them positionally. Geometry is a single
 `setGroupBounds` operation, because `pos` and `size` are two views onto one
 `Rectangle` and must never be stored apart. Reroutes went further: `posInternal`
 is deleted, `pos` reads the stored point, and a reroute registers its own
 geometry in its constructor, which removed two seeding sites.
 
-**The store -> legacy direction is unchanged and still needed.** `useLayoutSync`
-stays, because `LGraphNode.serialize()` reads `this.pos` / `this.size` and the
-canvas renders from `_posSize`. Removing it means the class getters read from
-the store, but they return `Point` / `Size` views onto one buffer and hundreds
-of element-indexed reads across the renderer depend on that. Yjs cannot hold a
-`Float64Array` by reference, so this needs a store-backed geometry view type,
-not a refactor. The re-entrancy the draft worried about did not materialise: the
-writeback compares before writing, so an equal write-back is a no-op.
-
-**Not done, and each needs a decision rather than more inference:**
+**`LGraphNode` no longer runs a continuous store-to-class sync.** When code
+reads geometry, the node checks the global geometry version and copies the
+stored rectangle into `_posSize` if its cache is stale. Serialization and legacy
+rendering read current values through `this.pos` and `this.size` without walking
+every node after each layout change.
+
+While Vue-node rendering is active, `notifyLayoutChanges` dirties the canvas for
+node layout changes. For non-canvas `resizeNode` and `batchUpdateBounds`
+operations, it also calls `onResize`; canvas resizes already own that callback
+through `LGraphNode.setSize()`. It does not copy geometry into nodes.
+
+**Whole-value setters re-read after committing.** `_positionUpdated` and
+`_sizeUpdated` write only their own half of `_posSize`, so stamping
+`_geometryVersion` to the post-commit store version would mark the untouched
+half fresh while it was still stale. The next commit of that half would then
+publish the stale value to the store and to CRDT peers. Both invalidate and
+refresh instead, which also picks up any value the store clamped or merged.
+
+**Deferred follow-up: extract geometry projection ownership.** Land this after
+the node geometry facade and its CRDT-safety follow-up (PRs 14133 and 14480) so
+the ownership and compensation rules are stable before moving them. Preserve
+behavior while making one focused projection module responsible for the legacy
+geometry cache:
+
+1. Move `_geometryVersion`, `_layoutRegistered`, and `refreshGeometry()` out of
+   `LGraphNode`. Keep ephemeral projection state private to the module, keyed by
+   node identity; do not add another entity store for compatibility-only state.
+2. Make layout registration and removal call that module instead of coordinating
+   node flags and backing buffers directly. `LGraphNode.pos` and `.size` remain
+   compatibility accessors, but delegate synchronization rather than owning its
+   lifecycle.
+3. Once the lifecycle has one owner, replace the store-wide geometry version
+   check with node-scoped invalidation. A geometry change should make only the
+   affected nodes refresh; unrelated node, group, and reroute operations should
+   not force every rendered node through a Yjs lookup.
+
+Completion requires no projection lifecycle fields or methods on `LGraphNode`,
+no registration code that mutates such fields, unchanged extension-facing
+`pos` / `size` behavior, and coverage for store-originated updates, indexed
+writes, removal/re-addition, and CRDT compensation.
+
+**Open decisions:**
 
 1. The geometry views. The hand-written `size` Proxy is gone, replaced by
    `createGeometryView` over `pos` and `size` on `LGraphNode` plus `pos`,
-   `size` and `bounding` on `LGraphGroup`. Every in-repo element write is gone,
-   so their only remaining job is third-party `node.size[1] = h`. Retiring them
-   means accepting that ecosystem writes stop reflowing, or landing a stable
-   resize API.
+   `size` and `bounding` on `LGraphGroup`. The views still handle the indexed
+   position write in `distributeNodes()` and extension writes such as
+   `node.size[1] = h`. Retiring them requires whole-value assignment in
+   `distributeNodes()` and a stable resize API for extensions.
 2. Subgraph IO nodes have conforming write paths but no store entry. A keyed
    entry needs subgraph scoping (`SUBGRAPH_INPUT_ID` is a constant shared by
    every subgraph) and `Subgraph.id` is reassigned by `clear()`, so the key can
-   go stale — the pattern rejected for the link store. Nothing needs keyed
+   go stale. The link store rejected the same pattern. Nothing needs keyed
    access, since callers reach them as `subgraph.inputNode`.
 3. Two hit-testing systems: litegraph against class geometry, `layoutStore`
    against a spatial index. Node bounds are duplicated between `_boundingRect`
@@ -914,6 +946,7 @@ Phase 3c (ConnectivitySystem)  ──── depends on 2c
 Phase 3->4 gate checklist  ──────── depends on 3a, 3b, 3c
 
 Phase 4a (Position writes)  ────── depends on 2a, 3b
+  Geometry projection ownership ── after PRs 14133, 14480; depends on 4a
 Phase 4b (Connectivity mutations) ─ depends on 3c, 3->4 gate
 Phase 4c (Widget writes)  ─────── ✅ largely shipped; depends on 2b
 Phase 4d (Layout decoupling)  ─── depends on 2a, 3->4 gate

src/components/graph/GraphCanvas.vue

@@ -113,7 +113,6 @@
 import { until, useEventListener } from '@vueuse/core'
 import {
   computed,
-  nextTick,
   onMounted,
   onUnmounted,
   ref,
@@ -174,8 +173,8 @@ import { useNodeDataStore } from '@/stores/nodeDataStore'
 import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
 import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
 import { arrangeForLegacyRender } from '@/renderer/core/canvas/litegraph/arrangeForLegacyRender'
+import { notifyLayoutChanges } from '@/renderer/core/canvas/litegraph/notifyLayoutChanges'
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
-import { useLayoutSync } from '@/renderer/core/layout/sync/useLayoutSync'
 import TransformPane from '@/renderer/core/layout/transform/TransformPane.vue'
 import MiniMap from '@/renderer/extensions/minimap/MiniMap.vue'
 import LGraphNode from '@/renderer/extensions/vueNodes/components/LGraphNode.vue'
@@ -255,8 +254,6 @@ const minimapEnabled = computed(() => settingStore.get('Comfy.Minimap.Visible'))
 // Feature flags
 const { shouldRenderVueNodes } = useVueFeatureFlags()
 
-const { startSync, stopSync } = useLayoutSync()
-
 // Error-clearing hooks run regardless of rendering mode (Vue or legacy canvas).
 let cleanupErrorHooks: (() => void) | null = null
 watch(
@@ -267,35 +264,31 @@ watch(
   }
 )
 
-async function enterVueRendering(graph: LGraph | null) {
-  stopSync()
-  layoutStore.clearViewGeometry()
-  await nextTick()
-
-  // Revalidate after nextTick: rendering mode or the current graph may change.
-  if (!shouldRenderVueNodes.value || canvasStore.currentGraph !== graph) return
-  startSync(canvasStore.canvas)
-}
-
 function exitToLegacyRendering(graph: LGraph | null) {
-  stopSync()
   layoutStore.clearViewGeometry()
   if (graph) arrangeForLegacyRender(graph)
   canvasStore.canvas?.setDirty(true, true)
 }
 
 watch(
   [shouldRenderVueNodes, () => canvasStore.currentGraph],
-  async ([enabled, graph], previous) => {
+  ([enabled, graph], previous) => {
     if (enabled) {
-      await enterVueRendering(graph)
+      layoutStore.clearViewGeometry()
     } else if (previous?.[0]) {
       exitToLegacyRendering(graph)
     }
   },
   { immediate: true }
 )
 
+watchEffect((onCleanup) => {
+  if (!shouldRenderVueNodes.value) return
+
+  const canvas = canvasStore.canvas
+  if (canvas) onCleanup(notifyLayoutChanges(canvas))
+})
+
 watch(
   () => canvasStore.isInSubgraph,
   (newValue, oldValue) => {
@@ -604,7 +597,6 @@ onMounted(async () => {
 onUnmounted(() => {
   cleanupErrorHooks?.()
   cleanupErrorHooks = null
-  stopSync()
 })
 function forwardPointerDownPanEvent(e: PointerEvent) {
   forwardPanEvent(e, isMiddlePointerInput)

src/lib/litegraph/src/LGraph.ts

@@ -12,7 +12,8 @@ import {
   canvasLayoutMutations,
   registerGroupLayout,
   registerNodeLayout,
-  unregisterAllGraphLayout
+  unregisterAllGraphLayout,
+  unregisterNodeLayout
 } from '@/renderer/core/layout/operations/graphLayoutRegistration'
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
 import { toLinkId } from '@/types/linkId'
@@ -1247,7 +1248,7 @@ export class LGraph
     node.onRemoved?.()
 
     unregisterNodeState(node)
-    canvasLayoutMutations().deleteNode(this.rootGraph.id, node.id)
+    unregisterNodeLayout(this, node)
 
     node.graph = null
     this.incrementVersion()

src/lib/litegraph/src/LGraphNode.test.ts

@@ -872,6 +872,161 @@ describe('snapToGrid', () => {
   })
 })
 
+describe('layout geometry projection', () => {
+  beforeEach(() => {
+    setActivePinia(createTestingPinia({ stubActions: false }))
+    layoutStore.resetForTests()
+  })
+
+  test('moves from the latest stored position', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    graph.add(node)
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 30, y: 40, width: 200, height: 80 }
+      }
+    ])
+
+    node.move(5, 10)
+
+    expect(
+      layoutStore.getNodeLayoutRef(graph.rootGraph.id, node.id).value?.position
+    ).toEqual({ x: 35, y: 50 })
+  })
+
+  test('snaps the latest stored position', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    graph.add(node)
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 103, y: 97, width: 200, height: 80 }
+      }
+    ])
+
+    node.snapToGrid(20)
+
+    expect(
+      layoutStore.getNodeLayoutRef(graph.rootGraph.id, node.id).value?.position
+    ).toEqual({ x: 100, y: 100 })
+  })
+
+  test('preserves stored geometry when removed and re-added', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    graph.add(node)
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 30, y: 40, width: 200, height: 80 }
+      }
+    ])
+
+    graph.remove(node)
+    graph.add(node)
+
+    expect(
+      layoutStore.getNodeLayoutRef(graph.rootGraph.id, node.id).value
+    ).toMatchObject({
+      position: { x: 30, y: 40 },
+      size: { width: 200, height: 80 }
+    })
+  })
+
+  test('refreshes stable views before indexed mutations', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    node.pos = [10, 20]
+    node.size = [100, 50]
+    graph.add(node)
+    const pos = node.pos
+    const size = node.size
+
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 30, y: 40, width: 200, height: 80 }
+      }
+    ])
+    pos[0] = 50
+    size[1] = 90
+
+    expect(node.pos).toBe(pos)
+    expect(node.size).toBe(size)
+    expect([...pos]).toEqual([50, 40])
+    expect([...size]).toEqual([200, 90])
+    expect(
+      layoutStore.getNodeLayoutRef(graph.rootGraph.id, node.id).value
+    ).toMatchObject({
+      position: { x: 50, y: 40 },
+      size: { width: 200, height: 90 }
+    })
+  })
+
+  test('preserves stored size when assigning position', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    node.pos = [10, 20]
+    node.size = [100, 50]
+    graph.add(node)
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 30, y: 40, width: 200, height: 80 }
+      }
+    ])
+
+    node.pos = [50, 60]
+
+    expect([...node.size]).toEqual([200, 80])
+  })
+
+  test('preserves stored position when assigning size', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    node.pos = [10, 20]
+    node.size = [100, 50]
+    graph.add(node)
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 30, y: 40, width: 200, height: 80 }
+      }
+    ])
+
+    node.size = [300, 90]
+
+    expect([...node.pos]).toEqual([30, 40])
+  })
+
+  test('does not write a stale width back to the store', () => {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    node.pos = [10, 20]
+    node.size = [100, 50]
+    graph.add(node)
+    layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
+      {
+        nodeId: node.id,
+        bounds: { x: 30, y: 40, width: 200, height: 80 }
+      }
+    ])
+
+    node.pos = [50, 60]
+    node.setSize([node.size[0], 120])
+
+    expect(
+      layoutStore.getNodeLayoutRef(graph.rootGraph.id, node.id).value
+    ).toMatchObject({
+      position: { x: 50, y: 60 },
+      size: { width: 200, height: 120 }
+    })
+  })
+})
+
 describe('_setConcreteSlots', () => {
   beforeEach(() => {
     setActivePinia(createTestingPinia({ stubActions: false }))