mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
[backport cloud/1.42] fix: resync vue node layout store after legacy normalization (#10264)
Backport of #10256 to `cloud/1.42` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10264-backport-cloud-1-42-fix-resync-vue-node-layout-store-after-legacy-normalization-3276d73d3650813099dfe0f5105d9424) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -2,9 +2,116 @@ import {
|
||||
comfyExpect as expect,
|
||||
comfyPageFixture as test
|
||||
} from '../../../fixtures/ComfyPage'
|
||||
import type { ComfyPage } from '../../../fixtures/ComfyPage'
|
||||
|
||||
const CREATE_GROUP_HOTKEY = 'Control+g'
|
||||
|
||||
type NodeGroupCenteringError = {
|
||||
horizontal: number
|
||||
vertical: number
|
||||
}
|
||||
|
||||
type NodeGroupCenteringErrors = {
|
||||
innerGroup: NodeGroupCenteringError
|
||||
outerGroup: NodeGroupCenteringError
|
||||
}
|
||||
|
||||
const LEGACY_VUE_CENTERING_BASELINE: NodeGroupCenteringErrors = {
|
||||
innerGroup: {
|
||||
horizontal: 16.308832840862777,
|
||||
vertical: 17.390899314547084
|
||||
},
|
||||
outerGroup: {
|
||||
horizontal: 20.30164329441476,
|
||||
vertical: 42.196324096481476
|
||||
}
|
||||
} as const
|
||||
|
||||
const CENTERING_TOLERANCE = {
|
||||
innerGroup: 6,
|
||||
outerGroup: 12
|
||||
} as const
|
||||
|
||||
function expectWithinBaseline(
|
||||
actual: number,
|
||||
baseline: number,
|
||||
tolerance: number
|
||||
) {
|
||||
expect(Math.abs(actual - baseline)).toBeLessThan(tolerance)
|
||||
}
|
||||
|
||||
async function getNodeGroupCenteringErrors(
|
||||
comfyPage: ComfyPage
|
||||
): Promise<NodeGroupCenteringErrors> {
|
||||
return comfyPage.page.evaluate(() => {
|
||||
type GraphNode = {
|
||||
id: number | string
|
||||
pos: ReadonlyArray<number>
|
||||
}
|
||||
type GraphGroup = {
|
||||
title: string
|
||||
pos: ReadonlyArray<number>
|
||||
size: ReadonlyArray<number>
|
||||
}
|
||||
|
||||
const app = window.app!
|
||||
const node = app.graph.nodes[0] as GraphNode | undefined
|
||||
|
||||
if (!node) {
|
||||
throw new Error('Expected a node in the loaded workflow')
|
||||
}
|
||||
|
||||
const nodeElement = document.querySelector<HTMLElement>(
|
||||
`[data-node-id="${node.id}"]`
|
||||
)
|
||||
|
||||
if (!nodeElement) {
|
||||
throw new Error(`Vue node element not found for node ${node.id}`)
|
||||
}
|
||||
|
||||
const groups = app.graph.groups as GraphGroup[]
|
||||
const innerGroup = groups.find((group) => group.title === 'Inner Group')
|
||||
const outerGroup = groups.find((group) => group.title === 'Outer Group')
|
||||
|
||||
if (!innerGroup || !outerGroup) {
|
||||
throw new Error('Expected both Inner Group and Outer Group in graph')
|
||||
}
|
||||
|
||||
const nodeRect = nodeElement.getBoundingClientRect()
|
||||
|
||||
const getCenteringError = (group: GraphGroup): NodeGroupCenteringError => {
|
||||
const [groupStartX, groupStartY] = app.canvasPosToClientPos([
|
||||
group.pos[0],
|
||||
group.pos[1]
|
||||
])
|
||||
const [groupEndX, groupEndY] = app.canvasPosToClientPos([
|
||||
group.pos[0] + group.size[0],
|
||||
group.pos[1] + group.size[1]
|
||||
])
|
||||
|
||||
const groupLeft = Math.min(groupStartX, groupEndX)
|
||||
const groupRight = Math.max(groupStartX, groupEndX)
|
||||
const groupTop = Math.min(groupStartY, groupEndY)
|
||||
const groupBottom = Math.max(groupStartY, groupEndY)
|
||||
|
||||
const leftGap = nodeRect.left - groupLeft
|
||||
const rightGap = groupRight - nodeRect.right
|
||||
const topGap = nodeRect.top - groupTop
|
||||
const bottomGap = groupBottom - nodeRect.bottom
|
||||
|
||||
return {
|
||||
horizontal: Math.abs(leftGap - rightGap),
|
||||
vertical: Math.abs(topGap - bottomGap)
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
innerGroup: getCenteringError(innerGroup),
|
||||
outerGroup: getCenteringError(outerGroup)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
test.describe('Vue Node Groups', { tag: '@screenshot' }, () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
@@ -74,4 +181,45 @@ test.describe('Vue Node Groups', { tag: '@screenshot' }, () => {
|
||||
expect(finalOffsetY).toBeCloseTo(initialOffsetY, 0)
|
||||
}).toPass({ timeout: 5000 })
|
||||
})
|
||||
|
||||
test('should keep groups aligned after loading legacy Vue workflows', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow('groups/nested-groups-1-inner-node')
|
||||
await comfyPage.vueNodes.waitForNodes(1)
|
||||
|
||||
const workflowRendererVersion = await comfyPage.page.evaluate(() => {
|
||||
const extra = window.app!.graph.extra as
|
||||
| { workflowRendererVersion?: string }
|
||||
| undefined
|
||||
return extra?.workflowRendererVersion
|
||||
})
|
||||
|
||||
expect(workflowRendererVersion).toMatch(/^Vue/)
|
||||
|
||||
await expect(async () => {
|
||||
const centeringErrors = await getNodeGroupCenteringErrors(comfyPage)
|
||||
|
||||
expectWithinBaseline(
|
||||
centeringErrors.innerGroup.horizontal,
|
||||
LEGACY_VUE_CENTERING_BASELINE.innerGroup.horizontal,
|
||||
CENTERING_TOLERANCE.innerGroup
|
||||
)
|
||||
expectWithinBaseline(
|
||||
centeringErrors.innerGroup.vertical,
|
||||
LEGACY_VUE_CENTERING_BASELINE.innerGroup.vertical,
|
||||
CENTERING_TOLERANCE.innerGroup
|
||||
)
|
||||
expectWithinBaseline(
|
||||
centeringErrors.outerGroup.horizontal,
|
||||
LEGACY_VUE_CENTERING_BASELINE.outerGroup.horizontal,
|
||||
CENTERING_TOLERANCE.outerGroup
|
||||
)
|
||||
expectWithinBaseline(
|
||||
centeringErrors.outerGroup.vertical,
|
||||
LEGACY_VUE_CENTERING_BASELINE.outerGroup.vertical,
|
||||
CENTERING_TOLERANCE.outerGroup
|
||||
)
|
||||
}).toPass({ timeout: 5000 })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
|
||||
import { syncLayoutStoreNodeBoundsFromGraph } from './syncLayoutStoreFromGraph'
|
||||
|
||||
function createGraph(nodes: LGraphNode[]): LGraph {
|
||||
return {
|
||||
nodes
|
||||
} as LGraph
|
||||
}
|
||||
|
||||
function createNode(
|
||||
id: number,
|
||||
pos: [number, number],
|
||||
size: [number, number]
|
||||
): LGraphNode {
|
||||
return {
|
||||
id,
|
||||
pos,
|
||||
size
|
||||
} as LGraphNode
|
||||
}
|
||||
|
||||
describe('syncLayoutStoreNodeBoundsFromGraph', () => {
|
||||
beforeEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
LiteGraph.vueNodesMode = false
|
||||
})
|
||||
|
||||
it('syncs node bounds to layout store when Vue nodes mode is enabled', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
|
||||
const batchUpdateNodeBounds = vi
|
||||
.spyOn(layoutStore, 'batchUpdateNodeBounds')
|
||||
.mockImplementation(() => {})
|
||||
|
||||
const graph = createGraph([
|
||||
createNode(1, [100, 200], [320, 140]),
|
||||
createNode(2, [450, 300], [225, 96])
|
||||
])
|
||||
|
||||
syncLayoutStoreNodeBoundsFromGraph(graph)
|
||||
|
||||
expect(batchUpdateNodeBounds).toHaveBeenCalledWith([
|
||||
{
|
||||
nodeId: '1',
|
||||
bounds: {
|
||||
x: 100,
|
||||
y: 200,
|
||||
width: 320,
|
||||
height: 140
|
||||
}
|
||||
},
|
||||
{
|
||||
nodeId: '2',
|
||||
bounds: {
|
||||
x: 450,
|
||||
y: 300,
|
||||
width: 225,
|
||||
height: 96
|
||||
}
|
||||
}
|
||||
])
|
||||
})
|
||||
|
||||
it('does nothing when Vue nodes mode is disabled', () => {
|
||||
const batchUpdateNodeBounds = vi
|
||||
.spyOn(layoutStore, 'batchUpdateNodeBounds')
|
||||
.mockImplementation(() => {})
|
||||
|
||||
const graph = createGraph([createNode(1, [100, 200], [320, 140])])
|
||||
|
||||
syncLayoutStoreNodeBoundsFromGraph(graph)
|
||||
|
||||
expect(batchUpdateNodeBounds).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does nothing when graph has no nodes', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
|
||||
const batchUpdateNodeBounds = vi
|
||||
.spyOn(layoutStore, 'batchUpdateNodeBounds')
|
||||
.mockImplementation(() => {})
|
||||
|
||||
const graph = createGraph([])
|
||||
|
||||
syncLayoutStoreNodeBoundsFromGraph(graph)
|
||||
|
||||
expect(batchUpdateNodeBounds).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
23
src/renderer/core/layout/sync/syncLayoutStoreFromGraph.ts
Normal file
23
src/renderer/core/layout/sync/syncLayoutStoreFromGraph.ts
Normal file
@@ -0,0 +1,23 @@
|
||||
import type { LGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import type { NodeBoundsUpdate } from '@/renderer/core/layout/types'
|
||||
|
||||
export function syncLayoutStoreNodeBoundsFromGraph(graph: LGraph): void {
|
||||
if (!LiteGraph.vueNodesMode) return
|
||||
|
||||
const nodes = graph.nodes ?? []
|
||||
if (nodes.length === 0) return
|
||||
|
||||
const updates: NodeBoundsUpdate[] = nodes.map((node) => ({
|
||||
nodeId: String(node.id),
|
||||
bounds: {
|
||||
x: node.pos[0],
|
||||
y: node.pos[1],
|
||||
width: node.size[0],
|
||||
height: node.size[1]
|
||||
}
|
||||
}))
|
||||
|
||||
layoutStore.batchUpdateNodeBounds(updates)
|
||||
}
|
||||
@@ -6,6 +6,7 @@ import { shallowRef } from 'vue'
|
||||
|
||||
import { useCanvasPositionConversion } from '@/composables/element/useCanvasPositionConversion'
|
||||
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
|
||||
import { syncLayoutStoreNodeBoundsFromGraph } from '@/renderer/core/layout/sync/syncLayoutStoreFromGraph'
|
||||
import { flushScheduledSlotLayoutSync } from '@/renderer/extensions/vueNodes/composables/useSlotElementTracking'
|
||||
|
||||
import { st, t } from '@/i18n'
|
||||
@@ -1281,6 +1282,7 @@ export class ComfyApp {
|
||||
|
||||
ChangeTracker.isLoadingGraph = true
|
||||
try {
|
||||
let normalizedMainGraph = false
|
||||
try {
|
||||
// @ts-expect-error Discrepancies between zod and litegraph - in progress
|
||||
this.rootGraph.configure(graphData)
|
||||
@@ -1290,7 +1292,10 @@ export class ComfyApp {
|
||||
this.rootGraph.extra.workflowRendererVersion
|
||||
|
||||
// Scale main graph
|
||||
ensureCorrectLayoutScale(originalMainGraphRenderer, this.rootGraph)
|
||||
normalizedMainGraph = ensureCorrectLayoutScale(
|
||||
originalMainGraphRenderer,
|
||||
this.rootGraph
|
||||
)
|
||||
|
||||
// Scale all subgraphs that were loaded with the workflow
|
||||
// Use original main graph renderer as fallback (not the modified one)
|
||||
@@ -1368,6 +1373,10 @@ export class ComfyApp {
|
||||
useExtensionService().invokeExtensions('loadedGraphNode', node)
|
||||
})
|
||||
|
||||
if (normalizedMainGraph) {
|
||||
syncLayoutStoreNodeBoundsFromGraph(this.rootGraph)
|
||||
}
|
||||
|
||||
await useExtensionService().invokeExtensionsAsync(
|
||||
'afterConfigureGraph',
|
||||
missingNodeTypes
|
||||
|
||||
Reference in New Issue
Block a user