refactor: remove redundant geometry comments

Author: ampagent

src/lib/litegraph/src/LGraph.ts

@@ -10,8 +10,10 @@ import type { UUID } from '@/utils/uuid'
 import { createUuidv4, zeroUuid } from '@/utils/uuid'
 import {
   registerGroupLayout,
+  registerNodeLayout,
   unregisterAllGraphLayout,
   unregisterGroupLayout,
+  unregisterNodeLayout,
   unregisterRerouteLayout
 } from '@/renderer/core/layout/operations/graphLayoutRegistration'
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
@@ -46,10 +48,8 @@ import { LGraphGroup } from './LGraphGroup'
 import type { GroupId } from './LGraphGroup'
 import {
   LGraphNode,
-  registerNodeLayout,
   registerNodeState,
   unregisterAllNodeStates,
-  unregisterNodeLayout,
   unregisterNodeState
 } from './LGraphNode'
 import {

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

@@ -863,6 +863,41 @@ describe('snapToGrid', () => {
   })
 })
 
+describe('layout geometry projection', () => {
+  beforeEach(() => {
+    setActivePinia(createTestingPinia({ stubActions: false }))
+    layoutStore.reset()
+  })
+
+  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([
+      {
+        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(node.id).value).toMatchObject({
+      position: { x: 50, y: 40 },
+      size: { width: 200, height: 90 }
+    })
+  })
+})
+
 describe('_setConcreteSlots', () => {
   beforeEach(() => {
     setActivePinia(createTestingPinia({ stubActions: false }))

src/lib/litegraph/src/LGraphNode.ts

@@ -126,8 +126,7 @@ export type NodeProperty = string | number | boolean | object | null
 /** Captures only the {@link layoutStore} singleton, so shared across nodes. */
 const layoutMutations = useLayoutMutations()
 
-/** Scratch for store rect reads that must not allocate. */
-const storedRect = new Float64Array(4)
+const storedRectScratch = new Float64Array(4)
 
 interface INodePropertyInfo {
   name?: string
@@ -622,19 +621,6 @@ export class LGraphNode
     return [posX - bX, posY - bY]
   }
 
-  /**
-   * A frame-scoped projection of the node's geometry, which `layoutStore` owns.
-   *
-   * Reading straight from the store on every access costs ~57ns against a
-   * realistic document, and the legacy canvas reads a node's geometry dozens of
-   * times per frame. The rect is refreshed once per dirty signal and read
-   * locally after that - ADR 0008's first render-loop mitigation, pre-collecting
-   * a query rather than probing the store per draw call.
-   *
-   * It is a projection, never a source: nothing reads it back into the store,
-   * and it cannot go stale because every store write bumps the version that
-   * invalidates it.
-   */
   _posSize = new Rectangle()
   _pos: Point = this._posSize.pos
   _size: Size = this._posSize.size
@@ -647,17 +633,10 @@ export class LGraphNode
     synchronize: () => this.refreshGeometry()
   })
 
-  /** The `layoutStore.geometryVersion` this projection was built from. */
   _geometryVersion = -1
 
-  /**
-   * Whether the store holds this node's geometry. Until it does — before the
-   * node is attached — {@link _posSize} is the geometry, not a projection of
-   * it, and must not be overwritten by whatever entry happens to share the id.
-   */
   _layoutRegistered = false
 
-  /** Rebuilds {@link _posSize} if the store has changed since it was read. */
   private refreshGeometry(): void {
     if (!this._layoutRegistered) return
 
@@ -686,9 +665,9 @@ export class LGraphNode
 
     const position = { x: this._pos[0], y: this._pos[1] }
     if (
-      layoutStore.readNodeRect(this.id, storedRect) &&
-      storedRect[0] === position.x &&
-      storedRect[1] === position.y
+      layoutStore.readNodeRect(this.id, storedRectScratch) &&
+      storedRectScratch[0] === position.x &&
+      storedRectScratch[1] === position.y
     ) {
       return
     }
@@ -721,12 +700,10 @@ export class LGraphNode
   private _sizeUpdated(): void {
     if (this.id === UNASSIGNED_NODE_ID || !this.graph) return
 
-    // An equal write would bump the store version, which invalidates every
-    // node's geometry projection, so it is worth a read to avoid.
     if (
-      layoutStore.readNodeRect(this.id, storedRect) &&
-      storedRect[2] === this._size[0] &&
-      storedRect[3] === this._size[1]
+      layoutStore.readNodeRect(this.id, storedRectScratch) &&
+      storedRectScratch[2] === this._size[0] &&
+      storedRectScratch[3] === this._size[1]
     ) {
       return
     }
@@ -4413,36 +4390,6 @@ export function registerNodeState(
  * node. No-op for nodes that were never registered.
  * @param node The node to unregister
  */
-/**
- * Hands the node's geometry to {@link layoutStore} and switches it to reading
- * from there. Called from `LGraph.add`, after the node has an id.
- * @param node The node to register
- */
-export function registerNodeLayout(node: LGraphNode, zIndex: number): void {
-  layoutMutations.setSource(LayoutSource.Canvas)
-  layoutMutations.createNode(node.id, {
-    position: { x: node._pos[0], y: node._pos[1] },
-    size: { width: node._size[0], height: node._size[1] },
-    zIndex,
-    visible: true
-  })
-  node._layoutRegistered = true
-  node._geometryVersion = layoutStore.geometryVersion
-}
-
-/**
- * Drops the node's layout entry. The node keeps its last known geometry
- * locally, so a detached node still reports where it was.
- * @param node The node to unregister
- */
-export function unregisterNodeLayout(node: LGraphNode): void {
-  if (!node._layoutRegistered) return
-
-  node._layoutRegistered = false
-  layoutMutations.setSource(LayoutSource.Canvas)
-  layoutMutations.deleteNode(node.id)
-}
-
 export function unregisterNodeState(node: LGraphNode): void {
   if (!node._graphId) return
   useNodeDataStore().deleteNode(node._graphId, node._state)

src/renderer/core/canvas/litegraph/notifyLayoutChanges.ts

@@ -1,23 +1,6 @@
 import type { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
 import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
 
-/**
- * Tells the legacy canvas about geometry the store changed on its own — a Vue
- * drag, a DOM size measurement — which it would otherwise never repaint for.
- *
- * Node geometry itself needs no forwarding: `pos` and `size` project from the
- * store, so they are already current. What does not carry across is
- * `onResize`, which extensions such as `useLoad3d` and DOM widgets chain onto.
- * `LGraphNode.setSize` fires it for resizes the class initiates; this covers
- * the ones the store initiates.
- *
- * It fires on the operation rather than on a class/store size difference:
- * there is no longer any difference to detect, so a diff-based check would sit
- * silently inert.
- *
- * @param canvas The canvas to repaint and whose nodes should be notified
- * @returns A function that stops the notifications
- */
 export function notifyLayoutChanges(canvas: LGraphCanvas): () => void {
   return layoutStore.onChange((change) => {
     if (change.nodeIds.length === 0) return

src/renderer/core/layout/operations/graphLayoutRegistration.ts

@@ -7,8 +7,9 @@
  */
 import type { LGraph } from '@/lib/litegraph/src/LGraph'
 import type { LGraphGroup } from '@/lib/litegraph/src/LGraphGroup'
-import { unregisterNodeLayout } from '@/lib/litegraph/src/LGraphNode'
+import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations'
+import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
 import { LayoutSource } from '@/renderer/core/layout/types'
 import type { RerouteId } from '@/renderer/core/layout/types'
 
@@ -23,6 +24,24 @@ function canvasMutations() {
   return mutations
 }
 
+export function registerNodeLayout(node: LGraphNode, zIndex: number): void {
+  canvasMutations().createNode(node.id, {
+    position: { x: node._pos[0], y: node._pos[1] },
+    size: { width: node._size[0], height: node._size[1] },
+    zIndex,
+    visible: true
+  })
+  node._layoutRegistered = true
+  node._geometryVersion = layoutStore.geometryVersion
+}
+
+export function unregisterNodeLayout(node: LGraphNode): void {
+  if (!node._layoutRegistered) return
+
+  node._layoutRegistered = false
+  canvasMutations().deleteNode(node.id)
+}
+
 export function registerGroupLayout(
   graph: Pick<LGraph, 'rootGraph'>,
   group: Pick<LGraphGroup, 'id' | 'pos' | 'size'>

src/renderer/core/layout/operations/layoutMutations.test.ts

@@ -13,7 +13,6 @@ const NODE_2 = toNodeId('2')
 const MISSING_NODE = toNodeId('999')
 const NEW_NODE = toNodeId('99')
 
-/** Production registers nodes from `LGraph.add`; these tests need only the entry. */
 function seedNode(
   nodeId: NodeId,
   [x, y]: [number, number],

src/renderer/core/layout/store/layoutStore.ts

@@ -367,23 +367,10 @@ class LayoutStoreImpl implements LayoutStore {
     return ygroup ? yGroupToLayout(ygroup, groupId) : null
   }
 
-  /**
-   * Get current version for change detection
-   */
-  /**
-   * Bumped by every layout change. Read as a dirty signal by consumers that
-   * hold a frame-scoped projection of geometry and need to know when to refresh
-   * it — a plain number, so it stays cheap enough for a per-read check.
-   */
   get geometryVersion(): number {
     return this.version.value
   }
 
-  /**
-   * Copies a node's stored rect into `out` without allocating. The one read a
-   * frame's worth of geometry access should cost; see ADR 0008 on pre-collected
-   * render queries.
-   */
   readNodeRect(nodeId: NodeId, out: Float64Array): boolean {
     const rect = this.ynodes.get(String(nodeId))?.get('rect') as
       | number[]
@@ -397,6 +384,9 @@ class LayoutStoreImpl implements LayoutStore {
     return true
   }
 
+  /**
+   * Get current version for change detection
+   */
   getVersion(): ComputedRef<number> {
     return computed(() => this.version.value)
   }