diff --git a/browser_tests/assets/subgraphs/subgraph-nested-duplicate-ids.json b/browser_tests/assets/subgraphs/subgraph-nested-duplicate-ids.json new file mode 100644 index 0000000000..c5469594a5 --- /dev/null +++ b/browser_tests/assets/subgraphs/subgraph-nested-duplicate-ids.json @@ -0,0 +1,599 @@ +{ + "id": "9a37f747-e96b-4304-9212-7abcaad7bdac", + "revision": 0, + "last_node_id": 5, + "last_link_id": 5, + "nodes": [ + { + "id": 5, + "type": "1e38d8ea-45e1-48a5-aa20-966584201867", + "pos": [788, 433.5], + "size": [210, 108], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "name": "string_a", + "type": "STRING", + "widget": { + "name": "string_a" + }, + "link": 4 + } + ], + "outputs": [ + { + "name": "STRING", + "type": "STRING", + "links": [5] + } + ], + "properties": { + "proxyWidgets": [["-1", "string_a"]] + }, + "widgets_values": [""] + }, + { + "id": 2, + "type": "PreviewAny", + "pos": [1135, 429], + "size": [250, 145.5], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "name": "source", + "type": "*", + "link": 5 + } + ], + "outputs": [], + "properties": { + "Node name for S&R": "PreviewAny" + }, + "widgets_values": [null, null, false] + }, + { + "id": 1, + "type": "PrimitiveStringMultiline", + "pos": [456, 450], + "size": [225, 121.5], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "name": "STRING", + "type": "STRING", + "links": [4] + } + ], + "properties": { + "Node name for S&R": "PrimitiveStringMultiline" + }, + "widgets_values": ["Outer\n"] + } + ], + "links": [ + [4, 1, 0, 5, 0, "STRING"], + [5, 5, 0, 2, 0, "STRING"] + ], + "groups": [], + "definitions": { + "subgraphs": [ + { + "id": "1e38d8ea-45e1-48a5-aa20-966584201867", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 6, + "lastLinkId": 9, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "New Subgraph", + "inputNode": { + "id": -10, + "bounding": [351, 432.5, 120, 60] + }, + "outputNode": { + "id": -20, + "bounding": [1315, 432.5, 120, 60] + }, + "inputs": [ + { + "id": "7bf3e1d4-0521-4b5c-92f5-47ca598b7eb4", + "name": "string_a", + "type": "STRING", + "linkIds": [1], + "localized_name": "string_a", + "pos": [451, 452.5] + } + ], + "outputs": [ + { + "id": "fbe975ba-d7c2-471e-a99a-a1e2c6ab466d", + "name": "STRING", + "type": "STRING", + "linkIds": [9], + "localized_name": "STRING", + "pos": [1335, 452.5] + } + ], + "widgets": [], + "nodes": [ + { + "id": 3, + "type": "StringConcatenate", + "pos": [815, 373], + "size": [347, 231], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "localized_name": "string_a", + "name": "string_a", + "type": "STRING", + "widget": { + "name": "string_a" + }, + "link": 1 + }, + { + "localized_name": "string_b", + "name": "string_b", + "type": "STRING", + "widget": { + "name": "string_b" + }, + "link": 2 + } + ], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [7] + } + ], + "properties": { + "Node name for S&R": "StringConcatenate" + }, + "widgets_values": ["", "", ""] + }, + { + "id": 6, + "type": "9be42452-056b-4c99-9f9f-7381d11c4454", + "pos": [955, 775], + "size": [210, 88], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "localized_name": "string_a", + "name": "string_a", + "type": "STRING", + "widget": { + "name": "string_a" + }, + "link": 7 + } + ], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [9] + } + ], + "properties": { + "proxyWidgets": [["-1", "string_a"]] + }, + "widgets_values": [""] + }, + { + "id": 4, + "type": "PrimitiveStringMultiline", + "pos": [313, 685], + "size": [325, 109], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [2] + } + ], + "properties": { + "Node name for S&R": "PrimitiveStringMultiline" + }, + "widgets_values": ["Inner 1\n"] + } + ], + "groups": [], + "links": [ + { + "id": 2, + "origin_id": 4, + "origin_slot": 0, + "target_id": 3, + "target_slot": 1, + "type": "STRING" + }, + { + "id": 1, + "origin_id": -10, + "origin_slot": 0, + "target_id": 3, + "target_slot": 0, + "type": "STRING" + }, + { + "id": 7, + "origin_id": 3, + "origin_slot": 0, + "target_id": 6, + "target_slot": 0, + "type": "STRING" + }, + { + "id": 6, + "origin_id": 6, + "origin_slot": 0, + "target_id": -20, + "target_slot": 1, + "type": "STRING" + }, + { + "id": 9, + "origin_id": 6, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "STRING" + } + ], + "extra": {} + }, + { + "id": "9be42452-056b-4c99-9f9f-7381d11c4454", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 9, + "lastLinkId": 12, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "New Subgraph", + "inputNode": { + "id": -10, + "bounding": [680, 774, 120, 60] + }, + "outputNode": { + "id": -20, + "bounding": [1320, 774, 120, 60] + }, + "inputs": [ + { + "id": "01c05c51-86b5-4bad-b32f-9c911683a13d", + "name": "string_a", + "type": "STRING", + "linkIds": [4], + "localized_name": "string_a", + "pos": [780, 794] + } + ], + "outputs": [ + { + "id": "a8bcf3bf-a66a-4c71-8d92-17a2a4d03686", + "name": "STRING", + "type": "STRING", + "linkIds": [12], + "localized_name": "STRING", + "pos": [1340, 794] + } + ], + "widgets": [], + "nodes": [ + { + "id": 5, + "type": "StringConcatenate", + "pos": [860, 719], + "size": [400, 200], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [ + { + "localized_name": "string_a", + "name": "string_a", + "type": "STRING", + "widget": { + "name": "string_a" + }, + "link": 4 + }, + { + "localized_name": "string_b", + "name": "string_b", + "type": "STRING", + "widget": { + "name": "string_b" + }, + "link": 7 + } + ], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [11] + } + ], + "properties": { + "Node name for S&R": "StringConcatenate" + }, + "widgets_values": ["", "", ""] + }, + { + "id": 6, + "type": "PrimitiveStringMultiline", + "pos": [401, 973], + "size": [400, 200], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [7] + } + ], + "properties": { + "Node name for S&R": "PrimitiveStringMultiline" + }, + "widgets_values": ["Inner 2\n"] + }, + { + "id": 9, + "type": "7c2915a5-5eb8-4958-a8fd-4beb30f370ce", + "pos": [1046, 985], + "size": [210, 88], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "localized_name": "string_a", + "name": "string_a", + "type": "STRING", + "widget": { + "name": "string_a" + }, + "link": 11 + } + ], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [12] + } + ], + "properties": { + "proxyWidgets": [["-1", "string_a"]] + }, + "widgets_values": [""] + } + ], + "groups": [], + "links": [ + { + "id": 4, + "origin_id": -10, + "origin_slot": 0, + "target_id": 5, + "target_slot": 0, + "type": "STRING" + }, + { + "id": 7, + "origin_id": 6, + "origin_slot": 0, + "target_id": 5, + "target_slot": 1, + "type": "STRING" + }, + { + "id": 11, + "origin_id": 5, + "origin_slot": 0, + "target_id": 9, + "target_slot": 0, + "type": "STRING" + }, + { + "id": 10, + "origin_id": 9, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "STRING" + }, + { + "id": 12, + "origin_id": 9, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "STRING" + } + ], + "extra": {} + }, + { + "id": "7c2915a5-5eb8-4958-a8fd-4beb30f370ce", + "version": 1, + "state": { + "lastGroupId": 0, + "lastNodeId": 8, + "lastLinkId": 10, + "lastRerouteId": 0 + }, + "revision": 0, + "config": {}, + "name": "New Subgraph", + "inputNode": { + "id": -10, + "bounding": [262, 1222, 120, 60] + }, + "outputNode": { + "id": -20, + "bounding": [1330, 1222, 120, 60] + }, + "inputs": [ + { + "id": "934a8baa-d79c-428c-8ec9-814ad437d7c7", + "name": "string_a", + "type": "STRING", + "linkIds": [9], + "localized_name": "string_a", + "pos": [362, 1242] + } + ], + "outputs": [ + { + "id": "4c3d243b-9ff6-4dcd-9dbf-e4ec8e1fc879", + "name": "STRING", + "type": "STRING", + "linkIds": [10], + "localized_name": "STRING", + "pos": [1350, 1242] + } + ], + "widgets": [], + "nodes": [ + { + "id": 7, + "type": "StringConcatenate", + "pos": [870, 1038], + "size": [400, 200], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { + "localized_name": "string_a", + "name": "string_a", + "type": "STRING", + "widget": { + "name": "string_a" + }, + "link": 9 + }, + { + "localized_name": "string_b", + "name": "string_b", + "type": "STRING", + "widget": { + "name": "string_b" + }, + "link": 8 + } + ], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [10] + } + ], + "properties": { + "Node name for S&R": "StringConcatenate" + }, + "widgets_values": ["", "", ""] + }, + { + "id": 8, + "type": "PrimitiveStringMultiline", + "pos": [442, 1296], + "size": [400, 200], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [], + "outputs": [ + { + "localized_name": "STRING", + "name": "STRING", + "type": "STRING", + "links": [8] + } + ], + "properties": { + "Node name for S&R": "PrimitiveStringMultiline" + }, + "widgets_values": ["Inner 3\n"] + } + ], + "groups": [], + "links": [ + { + "id": 8, + "origin_id": 8, + "origin_slot": 0, + "target_id": 7, + "target_slot": 1, + "type": "STRING" + }, + { + "id": 9, + "origin_id": -10, + "origin_slot": 0, + "target_id": 7, + "target_slot": 0, + "type": "STRING" + }, + { + "id": 10, + "origin_id": 7, + "origin_slot": 0, + "target_id": -20, + "target_slot": 0, + "type": "STRING" + } + ], + "extra": {} + } + ] + }, + "config": {}, + "extra": { + "ds": { + "scale": 1, + "offset": [-7, 144] + }, + "frontendVersion": "1.38.13" + }, + "version": 0.4 +} diff --git a/browser_tests/tests/subgraph-duplicate-ids.spec.ts b/browser_tests/tests/subgraph-duplicate-ids.spec.ts new file mode 100644 index 0000000000..1fb77bed8d --- /dev/null +++ b/browser_tests/tests/subgraph-duplicate-ids.spec.ts @@ -0,0 +1,107 @@ +import { expect } from '@playwright/test' + +import { comfyPageFixture as test } from '../fixtures/ComfyPage' + +test.describe('Subgraph duplicate ID remapping', { tag: ['@subgraph'] }, () => { + const WORKFLOW = 'subgraphs/subgraph-nested-duplicate-ids' + + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting( + 'Comfy.Graph.DeduplicateSubgraphNodeIds', + true + ) + }) + + test('All node IDs are globally unique after loading', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + + const result = await comfyPage.page.evaluate(() => { + const graph = window.app!.canvas.graph! + // TODO: Extract allGraphs accessor (root + subgraphs) into LGraph + // TODO: Extract allNodeIds accessor into LGraph + const allGraphs = [graph, ...graph.subgraphs.values()] + const allIds = allGraphs + .flatMap((g) => g._nodes) + .map((n) => n.id) + .filter((id): id is number => typeof id === 'number') + + return { allIds, uniqueCount: new Set(allIds).size } + }) + + expect(result.uniqueCount).toBe(result.allIds.length) + expect(result.allIds.length).toBeGreaterThanOrEqual(10) + }) + + test('Root graph node IDs are preserved as canonical', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + + const rootIds = await comfyPage.page.evaluate(() => { + const graph = window.app!.canvas.graph! + return graph._nodes + .map((n) => n.id) + .filter((id): id is number => typeof id === 'number') + .sort((a, b) => a - b) + }) + + expect(rootIds).toEqual([1, 2, 5]) + }) + + test('All links reference valid nodes in their graph', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + + const invalidLinks = await comfyPage.page.evaluate(() => { + const graph = window.app!.canvas.graph! + const labeledGraphs: [string, typeof graph][] = [ + ['root', graph], + ...[...graph.subgraphs.entries()].map( + ([id, sg]) => [`subgraph:${id}`, sg] as [string, typeof graph] + ) + ] + + const isNonNegative = (id: number | string) => + typeof id === 'number' && id >= 0 + + return labeledGraphs.flatMap(([label, g]) => + [...g._links.values()].flatMap((link) => + [ + isNonNegative(link.origin_id) && + !g._nodes_by_id[link.origin_id] && + `${label}: origin_id ${link.origin_id} not found`, + isNonNegative(link.target_id) && + !g._nodes_by_id[link.target_id] && + `${label}: target_id ${link.target_id} not found` + ].filter(Boolean) + ) + ) + }) + + expect(invalidLinks).toEqual([]) + }) + + test('Subgraph navigation works after ID remapping', async ({ + comfyPage + }) => { + await comfyPage.workflow.loadWorkflow(WORKFLOW) + + const subgraphNode = await comfyPage.nodeOps.getNodeRefById('5') + await subgraphNode.navigateIntoSubgraph() + + const isInSubgraph = () => + comfyPage.page.evaluate( + () => window.app!.canvas.graph?.isRootGraph === false + ) + + expect(await isInSubgraph()).toBe(true) + + await comfyPage.page.keyboard.press('Escape') + await comfyPage.nextFrame() + + expect(await isInSubgraph()).toBe(false) + }) +}) diff --git a/src/lib/litegraph/src/LGraph.test.ts b/src/lib/litegraph/src/LGraph.test.ts index 1c597f9155..208a454cd1 100644 --- a/src/lib/litegraph/src/LGraph.test.ts +++ b/src/lib/litegraph/src/LGraph.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest' -import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph' +import type { Subgraph } from '@/lib/litegraph/src/litegraph' +import { + LGraph, + LGraphNode, + LiteGraph, + LLink +} from '@/lib/litegraph/src/litegraph' import { createTestSubgraphData, createTestSubgraphNode @@ -288,3 +294,182 @@ describe('Legacy LGraph Compatibility Layer', () => { expect(LiteGraph.LGraph).toBe(LGraph) }) }) + +describe('Shared LGraphState', () => { + function createSubgraphOnGraph(rootGraph: LGraph): Subgraph { + const data = createTestSubgraphData() + return rootGraph.createSubgraph(data) + } + + it('subgraph state is the same object as rootGraph state', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + expect(subgraph.state).toBe(rootGraph.state) + }) + + it('adding a node in a subgraph increments the root counter', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + rootGraph.add(new DummyNode()) + const rootNodeId = rootGraph.state.lastNodeId + + subgraph.add(new DummyNode()) + expect(rootGraph.state.lastNodeId).toBe(rootNodeId + 1) + }) + + it('node IDs never collide between root and subgraph', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const rootNode = new DummyNode() + rootGraph.add(rootNode) + + const subNode = new DummyNode() + subgraph.add(subNode) + + expect(rootNode.id).not.toBe(subNode.id) + }) + + it('configure merges state using max', () => { + const rootGraph = new LGraph() + rootGraph.state.lastNodeId = 10 + + const data = createTestSubgraphData() + data.state = { + lastNodeId: 5, + lastLinkId: 20, + lastGroupId: 0, + lastRerouteId: 0 + } + const subgraph = rootGraph.createSubgraph(data) + subgraph.configure(data) + + expect(rootGraph.state.lastNodeId).toBe(10) + expect(rootGraph.state.lastLinkId).toBe(20) + }) +}) + +describe('ensureGlobalIdUniqueness', () => { + function createSubgraphOnGraph(rootGraph: LGraph): Subgraph { + const data = createTestSubgraphData() + return rootGraph.createSubgraph(data) + } + + it('reassigns duplicate node IDs in subgraphs', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const rootNode = new DummyNode() + rootGraph.add(rootNode) + + const subNode = new DummyNode() + subNode.id = rootNode.id + subgraph._nodes.push(subNode) + subgraph._nodes_by_id[subNode.id] = subNode + + rootGraph.ensureGlobalIdUniqueness() + + expect(subNode.id).not.toBe(rootNode.id) + expect(subgraph._nodes_by_id[subNode.id]).toBe(subNode) + expect(subgraph._nodes_by_id[rootNode.id as number]).toBeUndefined() + }) + + it('preserves root graph node IDs as canonical', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const rootNode = new DummyNode() + rootGraph.add(rootNode) + const originalRootId = rootNode.id + + const subNode = new DummyNode() + subNode.id = rootNode.id + subgraph._nodes.push(subNode) + subgraph._nodes_by_id[subNode.id] = subNode + + rootGraph.ensureGlobalIdUniqueness() + + expect(rootNode.id).toBe(originalRootId) + }) + + it('updates lastNodeId to reflect reassigned IDs', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const rootNode = new DummyNode() + rootGraph.add(rootNode) + + const subNode = new DummyNode() + subNode.id = rootNode.id + subgraph._nodes.push(subNode) + subgraph._nodes_by_id[subNode.id] = subNode + + rootGraph.ensureGlobalIdUniqueness() + + expect(rootGraph.state.lastNodeId).toBeGreaterThanOrEqual( + subNode.id as number + ) + }) + + it('patches link origin_id and target_id after reassignment', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const rootNode = new DummyNode() + rootGraph.add(rootNode) + + const subNodeA = new DummyNode() + subNodeA.id = rootNode.id + subgraph._nodes.push(subNodeA) + subgraph._nodes_by_id[subNodeA.id] = subNodeA + + const subNodeB = new DummyNode() + subNodeB.id = 999 + subgraph._nodes.push(subNodeB) + subgraph._nodes_by_id[subNodeB.id] = subNodeB + + const link = new LLink(1, 'number', subNodeA.id, 0, subNodeB.id, 0) + subgraph._links.set(link.id, link) + + rootGraph.ensureGlobalIdUniqueness() + + expect(link.origin_id).toBe(subNodeA.id) + expect(link.target_id).toBe(subNodeB.id) + expect(link.origin_id).not.toBe(rootNode.id) + }) + + it('detects collisions with reserved (not-yet-created) node IDs', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const subNode = new DummyNode() + subNode.id = 42 + subgraph._nodes.push(subNode) + subgraph._nodes_by_id[subNode.id] = subNode + + rootGraph.ensureGlobalIdUniqueness([42]) + + expect(subNode.id).not.toBe(42) + expect(subgraph._nodes_by_id[subNode.id]).toBe(subNode) + }) + + it('is a no-op when there are no collisions', () => { + const rootGraph = new LGraph() + const subgraph = createSubgraphOnGraph(rootGraph) + + const rootNode = new DummyNode() + rootGraph.add(rootNode) + + const subNode = new DummyNode() + subgraph.add(subNode) + + const rootId = rootNode.id + const subId = subNode.id + + rootGraph.ensureGlobalIdUniqueness() + + expect(rootNode.id).toBe(rootId) + expect(subNode.id).toBe(subId) + }) +}) diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index 579d4a5655..e96b61c5cb 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -158,6 +158,7 @@ export class LGraph static STATUS_STOPPED = 1 static STATUS_RUNNING = 2 + static deduplicateSubgraphIds = false /** List of LGraph properties that are manually handled by {@link LGraph.configure}. */ static readonly ConfigureProperties = new Set([ @@ -199,13 +200,21 @@ export class LGraph list_of_graphcanvas: LGraphCanvas[] | null status: number = LGraph.STATUS_STOPPED - state: LGraphState = { + private _state: LGraphState = { lastGroupId: 0, lastNodeId: 0, lastLinkId: 0, lastRerouteId: 0 } + get state(): LGraphState { + return this._state + } + + set state(value: LGraphState) { + this._state = value + } + readonly events = new CustomEventTarget() readonly _subgraphs: Map = new Map() @@ -2377,15 +2386,19 @@ export class LGraph } else { // New schema - one version so far, no check required. - // State + // State - use max to prevent ID collisions across root and subgraphs if (data.state) { const { lastGroupId, lastLinkId, lastNodeId, lastRerouteId } = data.state const { state } = this - if (lastGroupId != null) state.lastGroupId = lastGroupId - if (lastLinkId != null) state.lastLinkId = lastLinkId - if (lastNodeId != null) state.lastNodeId = lastNodeId - if (lastRerouteId != null) state.lastRerouteId = lastRerouteId + if (lastGroupId != null) + state.lastGroupId = Math.max(state.lastGroupId, lastGroupId) + if (lastLinkId != null) + state.lastLinkId = Math.max(state.lastLinkId, lastLinkId) + if (lastNodeId != null) + state.lastNodeId = Math.max(state.lastNodeId, lastNodeId) + if (lastRerouteId != null) + state.lastRerouteId = Math.max(state.lastRerouteId, lastRerouteId) } // Links @@ -2424,6 +2437,13 @@ export class LGraph this.subgraphs.get(subgraph.id)?.configure(subgraph) } + if (this.isRootGraph && LGraph.deduplicateSubgraphIds) { + const reservedNodeIds = nodesData + ?.map((n) => n.id) + .filter((id): id is number => typeof id === 'number') + this.ensureGlobalIdUniqueness(reservedNodeIds) + } + let error = false const nodeDataMap = new Map() @@ -2516,6 +2536,50 @@ export class LGraph } } + /** + * Ensures all node IDs are globally unique across the root graph and all + * subgraphs. Reassigns any colliding IDs found in subgraphs, preserving + * root graph IDs as canonical. Updates link references (`origin_id`, + * `target_id`) within the affected graph to match the new node IDs. + */ + ensureGlobalIdUniqueness(reservedNodeIds?: Iterable): void { + const { state } = this + + const allGraphs: LGraph[] = [this, ...this._subgraphs.values()] + + const usedNodeIds = new Set(reservedNodeIds) + for (const graph of allGraphs) { + const remappedIds = new Map() + + for (const node of graph._nodes) { + if (typeof node.id !== 'number') continue + + if (usedNodeIds.has(node.id)) { + const oldId = node.id + while (usedNodeIds.has(++state.lastNodeId)); + const newId = state.lastNodeId + delete graph._nodes_by_id[oldId] + node.id = newId + graph._nodes_by_id[newId] = node + usedNodeIds.add(newId) + remappedIds.set(oldId, newId) + console.warn( + `LiteGraph: duplicate node ID ${oldId} reassigned to ${newId} in graph ${graph.id}` + ) + } else { + usedNodeIds.add(node.id as number) + if ((node.id as number) > state.lastNodeId) + state.lastNodeId = node.id as number + } + } + + if (remappedIds.size > 0) { + patchLinkNodeIds(graph._links, remappedIds) + patchLinkNodeIds(graph.floatingLinksInternal, remappedIds) + } + } + } + private _canvas?: LGraphCanvas get primaryCanvas(): LGraphCanvas | undefined { return this.rootGraph._canvas @@ -2596,6 +2660,14 @@ export class Subgraph return this._rootGraph } + override get state(): LGraphState { + return this._rootGraph.state + } + + override set state(_value: LGraphState) { + // No-op: subgraphs share the root graph's state. + } + constructor(rootGraph: LGraph, data: ExportedSubgraph) { if (!rootGraph) throw new Error('Root graph is required') @@ -2850,3 +2922,16 @@ export class Subgraph } } } + +function patchLinkNodeIds( + links: Map, + remappedIds: Map +): void { + for (const link of links.values()) { + const newOrigin = remappedIds.get(link.origin_id) + if (newOrigin !== undefined) link.origin_id = newOrigin + + const newTarget = remappedIds.get(link.target_id) + if (newTarget !== undefined) link.target_id = newTarget + } +} diff --git a/src/platform/settings/composables/useLitegraphSettings.ts b/src/platform/settings/composables/useLitegraphSettings.ts index ffdb58c92b..a056b7c58a 100644 --- a/src/platform/settings/composables/useLitegraphSettings.ts +++ b/src/platform/settings/composables/useLitegraphSettings.ts @@ -2,6 +2,7 @@ import { watchEffect } from 'vue' import { CanvasPointer, + LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph' @@ -162,4 +163,10 @@ export const useLitegraphSettings = () => { 'Comfy.EnableWorkflowViewRestore' ) }) + + watchEffect(() => { + LGraph.deduplicateSubgraphIds = settingStore.get( + 'Comfy.Graph.DeduplicateSubgraphNodeIds' + ) + }) } diff --git a/src/platform/settings/constants/coreSettings.ts b/src/platform/settings/constants/coreSettings.ts index 13dddf0389..38e635cb9b 100644 --- a/src/platform/settings/constants/coreSettings.ts +++ b/src/platform/settings/constants/coreSettings.ts @@ -1201,5 +1201,16 @@ export const CORE_SETTINGS: SettingParams[] = [ defaultValue: false, experimental: true, versionAdded: '1.40.0' + }, + { + id: 'Comfy.Graph.DeduplicateSubgraphNodeIds', + category: ['Comfy', 'Graph', 'Subgraph'], + name: 'Deduplicate subgraph node IDs', + tooltip: + 'Automatically reassign duplicate node IDs in subgraphs when loading a workflow.', + type: 'boolean', + defaultValue: false, + experimental: true, + versionAdded: '1.40.0' } ] diff --git a/src/schemas/apiSchema.ts b/src/schemas/apiSchema.ts index daca43c00d..062ef416f2 100644 --- a/src/schemas/apiSchema.ts +++ b/src/schemas/apiSchema.ts @@ -288,6 +288,7 @@ const zSettings = z.object({ 'Comfy.Graph.CanvasInfo': z.boolean(), 'Comfy.Graph.CanvasMenu': z.boolean(), 'Comfy.Graph.CtrlShiftZoom': z.boolean(), + 'Comfy.Graph.DeduplicateSubgraphNodeIds': z.boolean(), 'Comfy.Graph.LiveSelection': z.boolean(), 'Comfy.Graph.LinkMarkers': z.nativeEnum(LinkMarkerShape), 'Comfy.Graph.ZoomSpeed': z.number(),