Skip to content

Commit c93b711

Browse files
DrJKLampagentgithub-actions
committed
fix(layout): keep DOM measurements out of saved node sizes (#14758)
Keep the size a user requested separate from the size the browser rendered. DOM measurements now stay local instead of being saved or synced, so a resized node keeps its width after collapse, save, and reload. - Save the requested node position and size. Use measured content size only when rendering the current view. - Send resize position and size through the node's layout attachment as one update. - Bind Vue node dimensions to layout state instead of copying them into CSS from several call sites. - Replace the minimap data-source hierarchy and factory with one graph-backed data source that reads rendered geometry. - Cover resize persistence, collapsed reloads, subgraph layout updates, and touch panning in browser tests. - Browser fonts, locale, and custom node content must not change saved workflow geometry. - Collapsing a node must not overwrite its expanded size. - Rendering, slots, selection bounds, and the minimap should agree on the node's rendered dimensions. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com>
1 parent d931448 commit c93b711

38 files changed

Lines changed: 773 additions & 799 deletions

browser_tests/fixtures/VueNodeHelpers.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,10 @@ export class VueNodeHelpers {
9797
const node = window.app?.canvas.graph?.getNodeById(id)
9898
if (!node) throw new Error(`Node ${id} not found`)
9999

100-
node.setSize([node.size[0] + growth[0], node.size[1] + growth[1]])
100+
node.setSize([
101+
node.renderingSize[0] + growth[0],
102+
node.renderingSize[1] + growth[1]
103+
])
101104
return window.app!.canvas.ds.scale
102105
},
103106
{ id: toNodeId(nodeId), growth: GRAPH_SIZE_GROWTH }

browser_tests/tests/vueNodes/interactions/node/resize.spec.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,53 @@ test.describe(
7373
}
7474
})
7575

76+
test('should keep a resized width across a save and reload', async ({
77+
comfyPage
78+
}) => {
79+
test.slow()
80+
81+
const measureWidth = async (title: string) => {
82+
const node = await comfyPage.vueNodes.getFixtureByTitle(title)
83+
await expect(node.root).toBeVisible()
84+
const box = await node.boundingBox()
85+
expect(box, `Measure node "${title}"`).not.toBeNull()
86+
return box?.width ?? 0
87+
}
88+
89+
const { beforeResize, referenceWidth, resizedWidth } =
90+
await test.step('Resize the node', async () => {
91+
const initialWidth = await measureWidth('KSampler')
92+
const referenceWidth = await measureWidth('Save Image')
93+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
94+
const beforeResize = Date.now()
95+
96+
await vueNode.resizeFromCorner('SE', 120, 0)
97+
await comfyPage.nextFrame()
98+
99+
const resizedWidth = await measureWidth('KSampler')
100+
expect(resizedWidth).toBeGreaterThan(initialWidth + 100)
101+
return { beforeResize, referenceWidth, resizedWidth }
102+
})
103+
104+
await test.step('Save and reload the workflow', async () => {
105+
await comfyPage.workflow.waitForDraftIndexUpdatedSince(beforeResize)
106+
await comfyPage.workflow.reloadAndWaitForApp()
107+
})
108+
109+
await test.step('Restore the width without changing viewport scale', async () => {
110+
await expect
111+
.poll(async () => measureWidth('Save Image'))
112+
.toBeCloseTo(referenceWidth, 0)
113+
114+
await expect
115+
.poll(async () => measureWidth('KSampler'))
116+
.toBeGreaterThan(resizedWidth - 5)
117+
await expect
118+
.poll(async () => measureWidth('KSampler'))
119+
.toBeLessThan(resizedWidth + 5)
120+
})
121+
})
122+
76123
test.describe('minimum size enforcement', () => {
77124
test('SW resize clamps width, keeping right edge fixed', async ({
78125
comfyPage
179 Bytes
Loading

browser_tests/tests/vueNodes/nodeStates/collapse.spec.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,53 @@ test.describe('Vue Node Collapse', { tag: '@vue-nodes' }, () => {
5858
await expect(vueNode.collapseIcon).not.toHaveClass(/-rotate-90/)
5959
})
6060

61+
test('should keep a resized width across a collapsed save and reload', async ({
62+
comfyPage
63+
}) => {
64+
test.setTimeout(30000)
65+
66+
const { beforeCollapse, resizedWidth } =
67+
await test.step('Resize and collapse the node', async () => {
68+
const vueNode = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
69+
await expect(vueNode.root).toBeVisible()
70+
71+
await vueNode.resizeFromCorner('SE', 120, 0)
72+
await comfyPage.nextFrame()
73+
const resized = await vueNode.boundingBox()
74+
expect(resized, 'Measure resized node').not.toBeNull()
75+
const resizedWidth = resized?.width ?? 0
76+
77+
const beforeCollapse = Date.now()
78+
await vueNode.select()
79+
await comfyPage.keyboard.press('Alt+KeyC')
80+
await expect
81+
.poll(async () => (await vueNode.boundingBox())?.width)
82+
.toBeLessThan(resizedWidth)
83+
84+
return { beforeCollapse, resizedWidth }
85+
})
86+
87+
await test.step('Save and reload the collapsed node', async () => {
88+
await comfyPage.workflow.waitForDraftIndexUpdatedSince(beforeCollapse)
89+
await comfyPage.workflow.reloadAndWaitForApp()
90+
})
91+
92+
await test.step('Expand to the saved width', async () => {
93+
const reloaded = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
94+
await expect(reloaded.root).toBeVisible()
95+
await reloaded.select()
96+
await comfyPage.keyboard.press('Alt+KeyC')
97+
await comfyPage.nextFrame()
98+
99+
await expect
100+
.poll(async () => (await reloaded.boundingBox())?.width)
101+
.toBeGreaterThan(resizedWidth - 5)
102+
await expect
103+
.poll(async () => (await reloaded.boundingBox())?.width)
104+
.toBeLessThan(resizedWidth + 5)
105+
})
106+
})
107+
61108
test('should preserve title when collapsing/expanding', async ({
62109
comfyPage
63110
}) => {
174 Bytes
Loading
-3.88 KB
Loading
2.84 KB
Loading
178 Bytes
Loading

docs/adr/0003-crdt-based-layout-system.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,56 @@ entry. All three entity types key by `makeScopedLayoutKey(rootGraphId, id)`, so
161161
a root graph's teardown is one `clearGraph`; graphs sharing that bucket drop
162162
their entries individually through `detachGraphLayouts`.
163163

164+
### Amendment (2026-08-04): the replicated document holds intent, not measurement
165+
166+
`NodeLayout.size` conflates two values with different natures. **Requested**
167+
size is what a user, a workflow file, or `computeSize()` asked for. **Rendered**
168+
size is what the DOM produced, which for height is `max(requested, natural
169+
content)` because the node container is `min-h-(--node-height)`. A shared
170+
`ResizeObserver` in `useVueNodeResizeTracking.ts` measures the second and writes
171+
it into the first through `batchUpdateNodeBounds`.
172+
173+
A measurement is not a command. Replayed on a peer with different fonts, locale,
174+
browser, or installed custom-node versions it produces a different — and equally
175+
correct — answer, so it is neither deterministic nor meaningfully undoable. This
176+
is the contract in ADR 0008 that every mutation is supposed to satisfy, and the
177+
observer's write is the one place in the layout system that structurally cannot.
178+
179+
Three consequences follow, and they are the reason to act rather than to
180+
document and move on:
181+
182+
- `serialize()` reads `this.size`, which reads through to the store. Saved
183+
workflows therefore carry a DOM measurement and are not byte-portable across
184+
machines. Saving while a node is collapsed persists the header box as the
185+
node's size; on reload `min-h` heals the height and nothing heals the width.
186+
- Under `min-h` semantics with last-writer-wins, a replicated height converges
187+
to the largest natural height across every connected peer's rendering
188+
environment. Node geometry becomes a function of who has the document open.
189+
- Collapse is not stored. It is inferred from a box shrinking to its header,
190+
which is why the store holds rendered rather than requested geometry at all.
191+
In Vue mode the collapsed width this produces is read by nothing:
192+
`_collapsed_width` is assigned only in the branch `vueNodesMode` skips
193+
(`LGraphNode.ts`), so every reader falls through to `NODE_COLLAPSED_WIDTH`.
194+
195+
**Decision.** The Yjs document holds requested geometry, written only by named
196+
commands. Measured geometry belongs to the local, view-scoped tier that
197+
`slotLayouts` already occupies — a plain map, not replicated, dropped by
198+
`clearViewGeometry`. Slot geometry is measured from the DOM at higher frequency
199+
than node height and has never been considered a violation, because it lands in
200+
a tier that nothing replicates and nothing writes back into the DOM. Recording a
201+
measurement stays an explicit, named mutation rather than an ambient observer
202+
write. `LGraphNode.size` exposes requested size; `renderingSize`, `measure()`,
203+
and `boundingRect` expose rendered geometry. Collapse becomes stored data.
204+
205+
**This does not forbid measurement.** Content-driven height is real: widget
206+
hydration, media loading, badges, slot changes, and third-party DOM inserted by
207+
reference through `WidgetDOM.vue`. Making layout strictly one-directional would
208+
mean modelling in TypeScript what CSS already computes, enforced as a height
209+
contract across 40+ repositories we do not control, and paid for in clipped
210+
content in someone else's node. The defect was never that a measurement exists.
211+
It was that a view-derived value was promoted into replicated entity state by a
212+
mutation with no name.
213+
164214
## Notes
165215

166216
This centralized state + CRDT architecture follows patterns from modern collaborative applications:

src/composables/canvas/useSelectionToolboxPosition.ts

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,8 @@ import type { Ref } from 'vue'
44

55
import { useSelectedLiteGraphItems } from '@/composables/canvas/useSelectedLiteGraphItems'
66
import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags'
7-
import type { ReadOnlyRect } from '@/lib/litegraph/src/interfaces'
8-
import {
9-
LGraphGroup,
10-
LGraphNode,
11-
LiteGraph
12-
} from '@/lib/litegraph/src/litegraph'
7+
import type { ReadOnlyRect, Rect } from '@/lib/litegraph/src/interfaces'
8+
import { LGraphGroup, LGraphNode } from '@/lib/litegraph/src/litegraph'
139
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
1410
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
1511
import { isLGraphGroup, isLGraphNode } from '@/utils/litegraphUtil'
@@ -48,43 +44,12 @@ function currentSelectionMatchesSignature(
4844
return buildSelectionSignature(store) === moreOptionsSelectionSignature
4945
}
5046

51-
function getFullNodeBounds(item: LGraphNode | LGraphGroup): ReadOnlyRect {
52-
if (item instanceof LGraphGroup) {
53-
return [item.pos[0], item.pos[1], item.size[0], item.size[1]]
54-
}
55-
56-
return [
57-
item.pos[0],
58-
item.pos[1] - LiteGraph.NODE_TITLE_HEIGHT,
59-
item.size[0],
60-
item.size[1] + LiteGraph.NODE_TITLE_HEIGHT
61-
]
62-
}
63-
64-
function getVueNodeBounds(item: LGraphNode): ReadOnlyRect | null {
65-
const rootGraphId = item.graph?.rootGraph.id
66-
if (!rootGraphId) return null
67-
68-
const layout = layoutStore.getNodeLayoutRef(rootGraphId, item.id).value
69-
if (!layout) return null
70-
71-
return [
72-
layout.bounds.x,
73-
layout.bounds.y - LiteGraph.NODE_TITLE_HEIGHT,
74-
layout.bounds.width,
75-
layout.bounds.height + LiteGraph.NODE_TITLE_HEIGHT
76-
]
77-
}
78-
79-
function getSelectionBounds(
80-
item: LGraphNode | LGraphGroup,
81-
shouldUseVueLayout: boolean
82-
): ReadOnlyRect {
83-
if (shouldUseVueLayout && item instanceof LGraphNode) {
84-
return getVueNodeBounds(item) ?? getFullNodeBounds(item)
85-
}
47+
function getSelectionBounds(item: LGraphNode | LGraphGroup): ReadOnlyRect {
48+
if (item instanceof LGraphGroup) return item.boundingRect
8649

87-
return getFullNodeBounds(item)
50+
const bounds: Rect = [0, 0, 0, 0]
51+
item.measure(bounds)
52+
return bounds
8853
}
8954

9055
export function useSelectionToolboxPosition(
@@ -139,7 +104,7 @@ export function useSelectionToolboxPosition(
139104
if (item.id == null) continue
140105

141106
if (item instanceof LGraphNode || item instanceof LGraphGroup) {
142-
allBounds.push(getSelectionBounds(item, shouldRenderVueNodes.value))
107+
allBounds.push(getSelectionBounds(item))
143108
}
144109
}
145110

0 commit comments

Comments
 (0)