Skip to content

Commit b6c1c5c

Browse files
ampagentDrJKL
authored andcommitted
refactor: remove redundant geometry comments
1 parent 4fa698d commit b6c1c5c

6 files changed

Lines changed: 46 additions & 74 deletions

File tree

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,41 @@ describe('snapToGrid', () => {
872872
})
873873
})
874874

875+
describe('layout geometry projection', () => {
876+
beforeEach(() => {
877+
setActivePinia(createTestingPinia({ stubActions: false }))
878+
layoutStore.resetForTests()
879+
})
880+
881+
test('refreshes stable views before indexed mutations', () => {
882+
const graph = new LGraph()
883+
const node = new LGraphNode('test')
884+
node.pos = [10, 20]
885+
node.size = [100, 50]
886+
graph.add(node)
887+
const pos = node.pos
888+
const size = node.size
889+
890+
layoutStore.batchUpdateNodeBounds(graph.rootGraph.id, [
891+
{
892+
nodeId: node.id,
893+
bounds: { x: 30, y: 40, width: 200, height: 80 }
894+
}
895+
])
896+
pos[0] = 50
897+
size[1] = 90
898+
899+
expect(node.pos).toBe(pos)
900+
expect(node.size).toBe(size)
901+
expect([...pos]).toEqual([50, 40])
902+
expect([...size]).toEqual([200, 90])
903+
expect(layoutStore.getNodeLayoutRef(graph.rootGraph.id, node.id).value).toMatchObject({
904+
position: { x: 50, y: 40 },
905+
size: { width: 200, height: 90 }
906+
})
907+
})
908+
})
909+
875910
describe('_setConcreteSlots', () => {
876911
beforeEach(() => {
877912
setActivePinia(createTestingPinia({ stubActions: false }))

src/lib/litegraph/src/LGraphNode.ts

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,7 @@ export type NodeProperty = string | number | boolean | object | null
126126
/** Captures only the {@link layoutStore} singleton, so shared across nodes. */
127127
const layoutMutations = useLayoutMutations()
128128

129-
/** Scratch for store rect reads that must not allocate. */
130-
const storedRect = new Float64Array(4)
129+
const storedRectScratch = new Float64Array(4)
131130

132131
interface INodePropertyInfo {
133132
name?: string
@@ -622,19 +621,6 @@ export class LGraphNode
622621
return [posX - bX, posY - bY]
623622
}
624623

625-
/**
626-
* A frame-scoped projection of the node's geometry, which `layoutStore` owns.
627-
*
628-
* Reading straight from the store on every access costs ~57ns against a
629-
* realistic document, and the legacy canvas reads a node's geometry dozens of
630-
* times per frame. The rect is refreshed once per dirty signal and read
631-
* locally after that - ADR 0008's first render-loop mitigation, pre-collecting
632-
* a query rather than probing the store per draw call.
633-
*
634-
* It is a projection, never a source: nothing reads it back into the store,
635-
* and it cannot go stale because every store write bumps the version that
636-
* invalidates it.
637-
*/
638624
_posSize = new Rectangle()
639625
_pos: Point = this._posSize.pos
640626
_size: Size = this._posSize.size
@@ -647,17 +633,10 @@ export class LGraphNode
647633
synchronize: () => this.refreshGeometry()
648634
})
649635

650-
/** The `layoutStore.geometryVersion` this projection was built from. */
651636
_geometryVersion = -1
652637

653-
/**
654-
* Whether the store holds this node's geometry. Until it does — before the
655-
* node is attached — {@link _posSize} is the geometry, not a projection of
656-
* it, and must not be overwritten by whatever entry happens to share the id.
657-
*/
658638
_layoutRegistered = false
659639

660-
/** Rebuilds {@link _posSize} if the store has changed since it was read. */
661640
private refreshGeometry(): void {
662641
if (!this._layoutRegistered || !this.graph) return
663642

@@ -688,9 +667,9 @@ export class LGraphNode
688667
const rootGraphId = this.graph.rootGraph.id
689668
const position = { x: this._pos[0], y: this._pos[1] }
690669
if (
691-
layoutStore.readNodeRect(rootGraphId, this.id, storedRect) &&
692-
storedRect[0] === position.x &&
693-
storedRect[1] === position.y
670+
layoutStore.readNodeRect(rootGraphId, this.id, storedRectScratch) &&
671+
storedRectScratch[0] === position.x &&
672+
storedRectScratch[1] === position.y
694673
) {
695674
return
696675
}
@@ -724,12 +703,10 @@ export class LGraphNode
724703
if (this.id === UNASSIGNED_NODE_ID || !this.graph) return
725704

726705
const rootGraphId = this.graph.rootGraph.id
727-
// An equal write would bump the store version, which invalidates every
728-
// node's geometry projection, so it is worth a read to avoid.
729706
if (
730-
layoutStore.readNodeRect(rootGraphId, this.id, storedRect) &&
731-
storedRect[2] === this._size[0] &&
732-
storedRect[3] === this._size[1]
707+
layoutStore.readNodeRect(rootGraphId, this.id, storedRectScratch) &&
708+
storedRectScratch[2] === this._size[0] &&
709+
storedRectScratch[3] === this._size[1]
733710
) {
734711
return
735712
}
@@ -4416,18 +4393,6 @@ export function registerNodeState(
44164393
* node. No-op for nodes that were never registered.
44174394
* @param node The node to unregister
44184395
*/
4419-
/**
4420-
* Hands the node's geometry to {@link layoutStore} and switches it to reading
4421-
* from there. Called from `LGraph.add`, after the node has an id.
4422-
* @param node The node to register
4423-
*/
4424-
4425-
/**
4426-
* Drops the node's layout entry. The node keeps its last known geometry
4427-
* locally, so a detached node still reports where it was.
4428-
* @param node The node to unregister
4429-
*/
4430-
44314396
export function unregisterNodeState(node: LGraphNode): void {
44324397
if (!node._graphId) return
44334398
useNodeDataStore().deleteNode(node._graphId, node._state)

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,6 @@
11
import type { LGraphCanvas } from '@/lib/litegraph/src/litegraph'
22
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
33

4-
/**
5-
* Tells the legacy canvas about geometry the store changed on its own — a Vue
6-
* drag, a DOM size measurement — which it would otherwise never repaint for.
7-
*
8-
* Node geometry itself needs no forwarding: `pos` and `size` project from the
9-
* store, so they are already current. What does not carry across is
10-
* `onResize`, which extensions such as `useLoad3d` and DOM widgets chain onto.
11-
* `LGraphNode.setSize` fires it for resizes the class initiates; this covers
12-
* the ones the store initiates.
13-
*
14-
* It fires on the operation rather than on a class/store size difference:
15-
* there is no longer any difference to detect, so a diff-based check would sit
16-
* silently inert.
17-
*
18-
* @param canvas The canvas to repaint and whose nodes should be notified
19-
* @returns A function that stops the notifications
20-
*/
214
export function notifyLayoutChanges(canvas: LGraphCanvas): () => void {
225
return layoutStore.onChange((change) => {
236
if (change.nodeIds.length === 0) return

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
import type { LGraph } from '@/lib/litegraph/src/LGraph'
88
import type { LGraphGroup } from '@/lib/litegraph/src/LGraphGroup'
99
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
10-
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
1110
import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations'
11+
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
1212
import { LayoutSource } from '@/renderer/core/layout/types'
1313

1414
/** Layout mutations attributed to the canvas, for direct delete calls. */

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ const NODE_2 = toNodeId('2')
1717
const MISSING_NODE = toNodeId('999')
1818
const NEW_NODE = toNodeId('99')
1919

20-
/** Production registers nodes from `LGraph.add`; these tests need only the entry. */
2120
function seedNode(
2221
nodeId: NodeId,
2322
[x, y]: [number, number],

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

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -345,23 +345,10 @@ class LayoutStoreImpl implements LayoutStore {
345345
return ygroup ? yGroupToLayout(ygroup, groupId) : null
346346
}
347347

348-
/**
349-
* Get current version for change detection
350-
*/
351-
/**
352-
* Bumped by every layout change. Read as a dirty signal by consumers that
353-
* hold a frame-scoped projection of geometry and need to know when to refresh
354-
* it — a plain number, so it stays cheap enough for a per-read check.
355-
*/
356348
get geometryVersion(): number {
357349
return this.version.value
358350
}
359351

360-
/**
361-
* Copies a node's stored rect into `out` without allocating. The one read a
362-
* frame's worth of geometry access should cost; see ADR 0008 on pre-collected
363-
* render queries.
364-
*/
365352
readNodeRect(
366353
rootGraphId: UUID,
367354
nodeId: NodeId,
@@ -381,6 +368,9 @@ class LayoutStoreImpl implements LayoutStore {
381368
return true
382369
}
383370

371+
/**
372+
* Get current version for change detection
373+
*/
384374
getVersion(): ComputedRef<number> {
385375
return computed(() => this.version.value)
386376
}

0 commit comments

Comments
 (0)