Skip to content

Commit ecbae00

Browse files
r-vager-vage
committed
fix: harden Vue node viewport virtualization
1 parent e882216 commit ecbae00

8 files changed

Lines changed: 402 additions & 135 deletions

File tree

‎browser_tests/fixtures/ComfyPage.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,12 @@ export class ComfyPage {
233233
this.templatesDialog = new TemplatesDialog(page)
234234
this.titleEditor = new TitleEditor(page)
235235
this.mediaLightbox = new MediaLightbox(page)
236-
this.vueNodes = new VueNodeHelpers(page)
236+
this.settings = new SettingsHelper(page)
237+
this.vueNodes = new VueNodeHelpers(page, this.settings)
237238
this.appMode = new AppModeHelper(this)
238239
this.subgraph = new SubgraphHelper(this)
239240
this.canvasOps = new CanvasHelper(page, this.canvas, this.resetViewButton)
240241
this.nodeOps = new NodeOperationsHelper(this)
241-
this.settings = new SettingsHelper(page)
242242
this.keyboard = new KeyboardHelper(page, this.canvas)
243243
this.clipboard = new ClipboardHelper(this.keyboard, page)
244244
this.workflow = new WorkflowHelper(this)

‎browser_tests/fixtures/VueNodeHelpers.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
import type { Locator, Page } from '@playwright/test'
55

66
import { TestIds } from '@e2e/fixtures/selectors'
7+
import type { SettingsHelper } from '@e2e/fixtures/helpers/SettingsHelper'
8+
import { nextFrame } from '@e2e/fixtures/utils/timing'
9+
import type { Point } from '@/lib/litegraph/src/interfaces'
710
import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier'
811
import { toNodeId } from '@/types/nodeId'
912
import { VueNodeFixture } from '@e2e/fixtures/utils/vueNodeFixtures'
@@ -18,7 +21,10 @@ export class VueNodeHelpers {
1821
*/
1922
public readonly selectedNodes: Locator
2023

21-
constructor(private page: Page) {
24+
constructor(
25+
private page: Page,
26+
private settings: SettingsHelper
27+
) {
2228
this.nodes = page.locator('[data-node-id]')
2329
this.selectedNodes = page.locator(
2430
'[data-node-id].outline-node-component-outline'
@@ -84,6 +90,45 @@ export class VueNodeHelpers {
8490
)
8591
}
8692

93+
async layoutNodesAndEnableVirtualization(
94+
getPosition: (nodeId: string, index: number) => Point
95+
): Promise<string[]> {
96+
const nodeIds = await this.page.evaluate(() =>
97+
window.app!.graph.nodes.map((node) => String(node.id))
98+
)
99+
const entries = nodeIds.map(
100+
(nodeId, index) => [nodeId, getPosition(nodeId, index)] as const
101+
)
102+
103+
await this.page.evaluate((entries) => {
104+
const graph = window.app!.graph
105+
if (graph.nodes.length !== entries.length) {
106+
throw new Error('Graph nodes changed while applying test layout')
107+
}
108+
const positionById = new Map(entries)
109+
graph.nodes.forEach((node) => {
110+
const position = positionById.get(String(node.id))
111+
if (!position) throw new Error(`Missing position for node ${node.id}`)
112+
node.setPos(...position)
113+
node.updateArea()
114+
})
115+
graph.groups.forEach((group) => group.recomputeInsideNodes())
116+
117+
const canvas = window.app!.canvas
118+
canvas.ds.offset[0] = 0
119+
canvas.ds.offset[1] = 0
120+
canvas.ds.scale = 1
121+
canvas.setDirty(true, true)
122+
}, entries)
123+
await nextFrame(this.page)
124+
await this.settings.setSetting(
125+
'Comfy.VueNodes.ViewportVirtualization',
126+
true
127+
)
128+
129+
return nodeIds
130+
}
131+
87132
/**
88133
* Select a specific Vue node by ID
89134
*/

‎browser_tests/tests/vueNodes/viewportVirtualization.spec.ts‎

Lines changed: 38 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,16 @@ test.describe(
1818
await comfyPage.canvasOps.resetView()
1919
})
2020

21-
test('hydrates all nodes, then swaps only after the viewport settles', async ({
21+
test('hydrates all nodes, then swaps to the settled viewport', async ({
2222
comfyPage
2323
}) => {
24-
const graphNodeIds = await comfyPage.page.evaluate(() => {
25-
const nodes = window.app!.graph.nodes
26-
nodes.forEach((node, index) => {
27-
const x = index < 2 ? 100 + index * 500 : 1800 + (index - 2) * 1800
28-
node.setPos(x, 100)
29-
node.updateArea()
30-
})
31-
const canvas = window.app!.canvas
32-
canvas.ds.offset[0] = 0
33-
canvas.ds.offset[1] = 0
34-
canvas.ds.scale = 1
35-
canvas.setDirty(true, true)
36-
return nodes.map((node) => String(node.id))
37-
})
38-
await comfyPage.nextFrame()
39-
40-
await comfyPage.settings.setSetting(
41-
'Comfy.VueNodes.ViewportVirtualization',
42-
true
43-
)
24+
const graphNodeIds =
25+
await comfyPage.vueNodes.layoutNodesAndEnableVirtualization(
26+
(_nodeId, index) => [
27+
index < 2 ? 100 + index * 500 : 1800 + (index - 2) * 1800,
28+
100
29+
]
30+
)
4431

4532
const firstViewportIds = graphNodeIds.slice(0, 2)
4633
await expect
@@ -54,7 +41,6 @@ test.describe(
5441
})
5542
await comfyPage.nextFrame()
5643

57-
expect(await comfyPage.vueNodes.getNodeIds()).toEqual(firstViewportIds)
5844
await expect
5945
.poll(() => comfyPage.vueNodes.getNodeIds())
6046
.toEqual([graphNodeIds[2]])
@@ -66,25 +52,13 @@ test.describe(
6652
test('keeps a multi-node drag mount set frozen during edge auto-pan', async ({
6753
comfyPage
6854
}) => {
69-
const graphNodeIds = await comfyPage.page.evaluate(() => {
70-
const nodes = window.app!.graph.nodes
71-
nodes.forEach((node, index) => {
72-
const x = index < 2 ? 100 + index * 500 : 1800 + (index - 2) * 1800
73-
node.setPos(x, 100)
74-
node.updateArea()
75-
})
76-
const canvas = window.app!.canvas
77-
canvas.ds.offset[0] = 0
78-
canvas.ds.offset[1] = 0
79-
canvas.ds.scale = 1
80-
canvas.setDirty(true, true)
81-
return nodes.map((node) => String(node.id))
82-
})
83-
await comfyPage.nextFrame()
84-
await comfyPage.settings.setSetting(
85-
'Comfy.VueNodes.ViewportVirtualization',
86-
true
87-
)
55+
const graphNodeIds =
56+
await comfyPage.vueNodes.layoutNodesAndEnableVirtualization(
57+
(_nodeId, index) => [
58+
index < 2 ? 100 + index * 500 : 1800 + (index - 2) * 1800,
59+
100
60+
]
61+
)
8862

8963
const initialIds = graphNodeIds.slice(0, 2)
9064
await expect
@@ -123,26 +97,16 @@ test.describe(
12397
comfyPage
12498
}) => {
12599
await comfyPage.workflow.loadWorkflow('groups/oversized_group')
126-
const nodeId = await comfyPage.page.evaluate(() => {
127-
const node = window.app!.graph.nodes[0]
100+
await comfyPage.page.evaluate(() => {
128101
const group = window.app!.graph.groups[0]
129-
node.setPos(100, 100)
130-
node.updateArea()
131102
group.pos = [50, 50]
132103
group.size = [900, 825]
133-
group.recomputeInsideNodes()
134-
const canvas = window.app!.canvas
135-
canvas.ds.offset[0] = 0
136-
canvas.ds.offset[1] = 0
137-
canvas.ds.scale = 1
138-
canvas.setDirty(true, true)
139-
return String(node.id)
140104
})
141-
await comfyPage.nextFrame()
142-
await comfyPage.settings.setSetting(
143-
'Comfy.VueNodes.ViewportVirtualization',
144-
true
145-
)
105+
const [nodeId] =
106+
await comfyPage.vueNodes.layoutNodesAndEnableVirtualization(() => [
107+
100, 100
108+
])
109+
if (!nodeId) throw new Error('Expected a node in the group workflow')
146110
await expect.poll(() => comfyPage.vueNodes.getNodeIds()).toEqual([nodeId])
147111

148112
await comfyPage.page.evaluate(() => {
@@ -153,8 +117,9 @@ test.describe(
153117
node.updateArea()
154118
canvas.setDirty(true, true)
155119
})
156-
await comfyPage.page.waitForTimeout(400)
157-
expect(await comfyPage.vueNodes.getNodeIds()).toEqual([nodeId])
120+
await expect
121+
.poll(() => comfyPage.vueNodes.getNodeIds(), { timeout: 1_000 })
122+
.toEqual([nodeId])
158123

159124
await comfyPage.page.evaluate(() => {
160125
window.app!.canvas.isDragging = false
@@ -166,26 +131,14 @@ test.describe(
166131
test('hydrates a node pasted into a distant settled viewport', async ({
167132
comfyPage
168133
}) => {
169-
const sourceId = await comfyPage.page.evaluate(() => {
170-
const [source, ...otherNodes] = window.app!.graph.nodes
171-
source.setPos(100, 100)
172-
source.updateArea()
173-
otherNodes.forEach((node, index) => {
174-
node.setPos(4000 + index * 1800, 100)
175-
node.updateArea()
176-
})
177-
const canvas = window.app!.canvas
178-
canvas.ds.offset[0] = 0
179-
canvas.ds.offset[1] = 0
180-
canvas.ds.scale = 1
181-
canvas.setDirty(true, true)
182-
return String(source.id)
183-
})
184-
await comfyPage.nextFrame()
185-
await comfyPage.settings.setSetting(
186-
'Comfy.VueNodes.ViewportVirtualization',
187-
true
188-
)
134+
const [sourceId] =
135+
await comfyPage.vueNodes.layoutNodesAndEnableVirtualization(
136+
(_nodeId, index) => [
137+
index === 0 ? 100 : 4000 + (index - 1) * 1800,
138+
100
139+
]
140+
)
141+
if (!sourceId) throw new Error('Expected a source node')
189142

190143
await expect
191144
.poll(() => comfyPage.vueNodes.getNodeIds())
@@ -234,29 +187,16 @@ test.describe(
234187
const target = graph.nodes.find(
235188
(node) => node.getTitle() === 'KSampler'
236189
)!
237-
source.setPos(100, 100)
238-
target.setPos(1800, 100)
239-
source.updateArea()
240-
target.updateArea()
241-
graph.nodes
242-
.filter((node) => node !== source && node !== target)
243-
.forEach((node, index) => {
244-
node.setPos(4000 + index * 1800, 100)
245-
node.updateArea()
246-
})
247190
const targetSlot = target.findInputSlot('model')
248191
if (targetSlot >= 0) target.disconnectInput(targetSlot)
249-
const canvas = window.app!.canvas
250-
canvas.ds.offset[0] = 0
251-
canvas.ds.offset[1] = 0
252-
canvas.ds.scale = 1
253-
canvas.setDirty(true, true)
254192
return { source: String(source.id), target: String(target.id) }
255193
})
256-
await comfyPage.nextFrame()
257-
await comfyPage.settings.setSetting(
258-
'Comfy.VueNodes.ViewportVirtualization',
259-
true
194+
await comfyPage.vueNodes.layoutNodesAndEnableVirtualization(
195+
(nodeId, index) => {
196+
if (nodeId === ids.source) return [100, 100]
197+
if (nodeId === ids.target) return [1800, 100]
198+
return [4000 + index * 1800, 100]
199+
}
260200
)
261201

262202
await expect(comfyPage.vueNodes.getNodeLocator(ids.source)).toBeVisible()

‎src/components/graph/GraphCanvas.vue‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,9 @@ const allNodes = computed((): VueNodeData[] =>
297297
Array.from(vueNodeLifecycle.nodeManager.value?.vueNodeData?.values() ?? [])
298298
)
299299
const viewportVirtualizationEnabled = computed(
300-
() => settingStore.get('Comfy.VueNodes.ViewportVirtualization') ?? false
300+
() =>
301+
shouldRenderVueNodes.value &&
302+
(settingStore.get('Comfy.VueNodes.ViewportVirtualization') ?? false)
301303
)
302304
const { onNodeMounted, renderedNodes } = useViewportVirtualization({
303305
allNodes,

‎src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,10 @@ describe('useSlotElementTracking', () => {
199199
})
200200
})
201201

202-
it('invalidates cached geometry when a virtualized node resizes', async () => {
202+
it('keeps cached geometry invalid after a virtualized node resizes and moves', async () => {
203203
const { unmount } = await mountAndRegisterSlot('input')
204204
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true)
205+
const registryStore = useNodeSlotRegistryStore()
205206
replaceViewportVirtualizedNodeIds([NODE_ID])
206207
unmount()
207208

@@ -217,6 +218,23 @@ describe('useSlotElementTracking', () => {
217218
})
218219
await nextTick()
219220

221+
expect(layoutStore.getSlotLayout(slotKey)).toBeNull()
222+
expect(
223+
registryStore.getNode(NODE_ID)?.slots.get(slotKey)?.cachedOffset
224+
).toBeUndefined()
225+
226+
layoutStore.applyOperation({
227+
type: 'moveNode',
228+
entity: 'node',
229+
nodeId: NODE_ID,
230+
position: { x: 50, y: 75 },
231+
previousPosition: { x: 0, y: 0 },
232+
timestamp: Date.now(),
233+
source: LayoutSource.External,
234+
actor: 'test'
235+
})
236+
await nextTick()
237+
220238
expect(layoutStore.getSlotLayout(slotKey)).toBeNull()
221239
})
222240

@@ -352,6 +370,37 @@ describe('useSlotElementTracking', () => {
352370
expect(batchUpdateSpy).not.toHaveBeenCalled()
353371
})
354372

373+
it('uses a matching node container when a foreign slot entry comes first', () => {
374+
const foreignContainer = document.createElement('div')
375+
foreignContainer.dataset.nodeId = 'other-node'
376+
document.body.appendChild(foreignContainer)
377+
378+
const foreignSlot = document.createElement('div')
379+
foreignContainer.appendChild(foreignSlot)
380+
381+
const matchingSlot = createSlotElement()
382+
const foreignSlotKey = getSlotKey(NODE_ID, 0, true)
383+
const matchingSlotKey = getSlotKey(NODE_ID, 1, false)
384+
const node = useNodeSlotRegistryStore().ensureNode(NODE_ID)
385+
node.slots.set(foreignSlotKey, {
386+
el: foreignSlot,
387+
index: 0,
388+
type: 'input'
389+
})
390+
node.slots.set(matchingSlotKey, {
391+
el: matchingSlot,
392+
index: 1,
393+
type: 'output'
394+
})
395+
396+
syncNodeSlotLayoutsFromDOM(NODE_ID)
397+
398+
expect(layoutStore.getSlotLayout(matchingSlotKey)?.position).toEqual({
399+
x: 15,
400+
y: 35 - LiteGraph.NODE_TITLE_HEIGHT
401+
})
402+
})
403+
355404
describe('collapsed node slot sync', () => {
356405
function registerCollapsedSlot() {
357406
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true)

‎src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,18 @@ export function syncNodeSlotLayoutsFromDOM(nodeId: NodeId) {
138138
// share the same DOM transform, so their pixel difference divided by the
139139
// effective scale yields a correct canvas-space offset regardless of
140140
// whether the TransformPane has flushed its latest transform to the DOM.
141-
const connectedSlotElement = Array.from(node.slots.values()).find(
142-
(entry) => entry.el?.isConnected
143-
)?.el
144-
const closestNode = connectedSlotElement?.closest('[data-node-id]')
145-
const nodeEl = closestNode instanceof HTMLElement ? closestNode : null
141+
let nodeEl: HTMLElement | null = null
142+
for (const entry of node.slots.values()) {
143+
if (!entry.el?.isConnected) continue
144+
const closestNode = entry.el.closest('[data-node-id]')
145+
if (
146+
closestNode instanceof HTMLElement &&
147+
closestNode.dataset.nodeId === String(nodeId)
148+
) {
149+
nodeEl = closestNode
150+
break
151+
}
152+
}
146153
const nodeRect = nodeEl?.getBoundingClientRect()
147154

148155
// Collapsed nodes preserve expanded size in layoutStore, so DOM-relative
@@ -307,7 +314,8 @@ export function useSlotElementTracking(options: {
307314
if (!newSize) return
308315
if (!oldSize || !isSizeEqual(newSize, oldSize)) {
309316
if (isNodeViewportVirtualized(nodeId)) {
310-
for (const slotKey of node.slots.keys()) {
317+
for (const [slotKey, entry] of node.slots) {
318+
entry.cachedOffset = undefined
311319
layoutStore.deleteSlotLayout(slotKey)
312320
}
313321
return

0 commit comments

Comments
 (0)