mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
[backport cloud/1.42] fix: Ensure zero uuid root graphs get assigned a valid id (#10961)
Backport of #10825 to cloud/1.42. Fixes an issue where handlers would be leaked causing Vue node rendering to be corrupted due to the 00000000-0000-0000-0000-000000000000 ID being used on the root graph. ## Conflict resolution - `browser_tests/tests/subgraphZeroUuid.spec.ts`: adjusted import path from `../../fixtures/ComfyPage` to `../fixtures/ComfyPage` since cloud/1.42 has flat test directory (no `subgraph/` subdirectory). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10961-backport-cloud-1-42-fix-Ensure-zero-uuid-root-graphs-get-assigned-a-valid-id-33c6d73d3650819a8ef1f1e05f120c19) by [Unito](https://www.unito.io)
This commit is contained in:
pythongosssss
197
browser_tests/assets/subgraphs/basic-subgraph-zero-uuid.json
Normal file
197
browser_tests/assets/subgraphs/basic-subgraph-zero-uuid.json
Normal file
@@ -0,0 +1,197 @@
|
||||
{
|
||||
"id": "00000000-0000-0000-0000-000000000000",
|
||||
"revision": 0,
|
||||
"last_node_id": 2,
|
||||
"last_link_id": 0,
|
||||
"nodes": [
|
||||
{
|
||||
"id": 2,
|
||||
"type": "e5fb1765-9323-4548-801a-5aead34d879e",
|
||||
"pos": [627.5973510742188, 423.0972900390625],
|
||||
"size": [144.15234375, 46],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"name": "positive",
|
||||
"type": "CONDITIONING",
|
||||
"link": null
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"links": null
|
||||
}
|
||||
],
|
||||
"properties": {},
|
||||
"widgets_values": []
|
||||
}
|
||||
],
|
||||
"links": [],
|
||||
"groups": [],
|
||||
"definitions": {
|
||||
"subgraphs": [
|
||||
{
|
||||
"id": "e5fb1765-9323-4548-801a-5aead34d879e",
|
||||
"version": 1,
|
||||
"state": {
|
||||
"lastGroupId": 0,
|
||||
"lastNodeId": 2,
|
||||
"lastLinkId": 4,
|
||||
"lastRerouteId": 0
|
||||
},
|
||||
"revision": 0,
|
||||
"config": {},
|
||||
"name": "New Subgraph",
|
||||
"inputNode": {
|
||||
"id": -10,
|
||||
"bounding": [347.90441582814213, 417.3822440655296, 120, 60]
|
||||
},
|
||||
"outputNode": {
|
||||
"id": -20,
|
||||
"bounding": [892.5973510742188, 416.0972900390625, 120, 60]
|
||||
},
|
||||
"inputs": [
|
||||
{
|
||||
"id": "c5cc99d8-a2b6-4bf3-8be7-d4949ef736cd",
|
||||
"name": "positive",
|
||||
"type": "CONDITIONING",
|
||||
"linkIds": [1],
|
||||
"pos": {
|
||||
"0": 447.9044189453125,
|
||||
"1": 437.3822326660156
|
||||
}
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"id": "9bd488b9-e907-4c95-a7a4-85c5597a87af",
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"linkIds": [2],
|
||||
"pos": {
|
||||
"0": 912.5973510742188,
|
||||
"1": 436.0972900390625
|
||||
}
|
||||
}
|
||||
],
|
||||
"widgets": [],
|
||||
"nodes": [
|
||||
{
|
||||
"id": 1,
|
||||
"type": "KSampler",
|
||||
"pos": [554.8743286132812, 100.95539093017578],
|
||||
"size": [270, 262],
|
||||
"flags": {},
|
||||
"order": 1,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"localized_name": "model",
|
||||
"name": "model",
|
||||
"type": "MODEL",
|
||||
"link": null
|
||||
},
|
||||
{
|
||||
"localized_name": "positive",
|
||||
"name": "positive",
|
||||
"type": "CONDITIONING",
|
||||
"link": 1
|
||||
},
|
||||
{
|
||||
"localized_name": "negative",
|
||||
"name": "negative",
|
||||
"type": "CONDITIONING",
|
||||
"link": null
|
||||
},
|
||||
{
|
||||
"localized_name": "latent_image",
|
||||
"name": "latent_image",
|
||||
"type": "LATENT",
|
||||
"link": null
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"localized_name": "LATENT",
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"links": [2]
|
||||
}
|
||||
],
|
||||
"properties": {
|
||||
"Node name for S&R": "KSampler"
|
||||
},
|
||||
"widgets_values": [0, "randomize", 20, 8, "euler", "simple", 1]
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"type": "VAEEncode",
|
||||
"pos": [685.1265869140625, 439.1734619140625],
|
||||
"size": [140, 46],
|
||||
"flags": {},
|
||||
"order": 0,
|
||||
"mode": 0,
|
||||
"inputs": [
|
||||
{
|
||||
"localized_name": "pixels",
|
||||
"name": "pixels",
|
||||
"type": "IMAGE",
|
||||
"link": null
|
||||
},
|
||||
{
|
||||
"localized_name": "vae",
|
||||
"name": "vae",
|
||||
"type": "VAE",
|
||||
"link": null
|
||||
}
|
||||
],
|
||||
"outputs": [
|
||||
{
|
||||
"localized_name": "LATENT",
|
||||
"name": "LATENT",
|
||||
"type": "LATENT",
|
||||
"links": [4]
|
||||
}
|
||||
],
|
||||
"properties": {
|
||||
"Node name for S&R": "VAEEncode"
|
||||
}
|
||||
}
|
||||
],
|
||||
"groups": [],
|
||||
"links": [
|
||||
{
|
||||
"id": 1,
|
||||
"origin_id": -10,
|
||||
"origin_slot": 0,
|
||||
"target_id": 1,
|
||||
"target_slot": 1,
|
||||
"type": "CONDITIONING"
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"origin_id": 1,
|
||||
"origin_slot": 0,
|
||||
"target_id": -20,
|
||||
"target_slot": 0,
|
||||
"type": "LATENT"
|
||||
}
|
||||
],
|
||||
"extra": {}
|
||||
}
|
||||
]
|
||||
},
|
||||
"config": {},
|
||||
"extra": {
|
||||
"ds": {
|
||||
"scale": 0.8894351682943402,
|
||||
"offset": [58.7671207025881, 137.7124650620126]
|
||||
},
|
||||
"frontendVersion": "1.24.1"
|
||||
},
|
||||
"version": 0.4
|
||||
}
|
||||
49
browser_tests/tests/subgraphZeroUuid.spec.ts
Normal file
49
browser_tests/tests/subgraphZeroUuid.spec.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
||||
|
||||
test.describe(
|
||||
'Zero UUID workflow: subgraph undo rendering',
|
||||
{ tag: ['@workflow', '@subgraph'] },
|
||||
() => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
test.setTimeout(30000) // Extend timeout as we need to reload the page an additional time
|
||||
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.page.reload() // Reload page as we need to enter in Vue mode
|
||||
await comfyPage.page.waitForFunction(() => !!window.app?.graph)
|
||||
})
|
||||
|
||||
test('Undo after subgraph enter/exit renders all nodes when workflow starts with zero UUID', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await comfyPage.workflow.loadWorkflow(
|
||||
'subgraphs/basic-subgraph-zero-uuid'
|
||||
)
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
|
||||
const assertInSubgraph = async (inSubgraph: boolean) => {
|
||||
await expect
|
||||
.poll(() => comfyPage.subgraph.isInSubgraph())
|
||||
.toBe(inSubgraph)
|
||||
}
|
||||
|
||||
// Root graph has 1 subgraph node, rendered in the DOM
|
||||
await expect.poll(() => comfyPage.nodeOps.getGraphNodesCount()).toBe(1)
|
||||
await expect.poll(() => comfyPage.vueNodes.getNodeCount()).toBe(1)
|
||||
|
||||
await comfyPage.vueNodes.enterSubgraph()
|
||||
await assertInSubgraph(true)
|
||||
|
||||
await comfyPage.subgraph.exitViaBreadcrumb()
|
||||
await assertInSubgraph(false)
|
||||
|
||||
await comfyPage.canvas.focus()
|
||||
await comfyPage.keyboard.undo()
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
// After undo, the subgraph node is still visible and rendered
|
||||
await expect.poll(() => comfyPage.nodeOps.getGraphNodesCount()).toBe(1)
|
||||
await expect.poll(() => comfyPage.vueNodes.getNodeCount()).toBe(1)
|
||||
})
|
||||
}
|
||||
)
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
import type { SerialisableGraph } from '@/lib/litegraph/src/types/serialisation'
|
||||
import type { UUID } from '@/lib/litegraph/src/utils/uuid'
|
||||
import { zeroUuid } from '@/lib/litegraph/src/utils/uuid'
|
||||
import { usePromotionStore } from '@/stores/promotionStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import {
|
||||
@@ -1008,3 +1009,25 @@ describe('deduplicateSubgraphNodeIds (via configure)', () => {
|
||||
expect(nodeIdSet(graph, SUBGRAPH_B)).toEqual(new Set([20, 21, 22]))
|
||||
})
|
||||
})
|
||||
|
||||
describe('Zero UUID handling in configure', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia())
|
||||
})
|
||||
|
||||
it('rejects zeroUuid for root graphs and assigns a new ID', () => {
|
||||
const graph = new LGraph()
|
||||
const data = graph.serialize()
|
||||
data.id = zeroUuid
|
||||
graph.configure(data)
|
||||
expect(graph.id).not.toBe(zeroUuid)
|
||||
})
|
||||
|
||||
it('preserves zeroUuid for subgraphs', () => {
|
||||
const graph = new LGraph()
|
||||
const subgraphData = { ...createTestSubgraphData(), id: zeroUuid }
|
||||
const subgraph = graph.createSubgraph(subgraphData)
|
||||
subgraph.configure(subgraphData)
|
||||
expect(subgraph.id).toBe(zeroUuid)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2525,8 +2525,8 @@ export class LGraph
|
||||
protected _configureBase(data: ISerialisedGraph | SerialisableGraph): void {
|
||||
const { id, extra } = data
|
||||
|
||||
// Create a new graph ID if none is provided
|
||||
if (id) {
|
||||
// Create a new graph ID if none is provided or the zero UUID is used on the root graph
|
||||
if (id && !(this.isRootGraph && id === zeroUuid)) {
|
||||
this.id = id
|
||||
} else if (this.id === zeroUuid) {
|
||||
this.id = createUuidv4()
|
||||
|
||||
Reference in New Issue
Block a user