mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-18 22:10:03 +00:00
Migrate parentIds when converting to subgraph (#5708)
The parentId property on links and reroutes was not handled at all in the "Convert to Subgraph" code. This needs to be addressed in 4 cases - A new external input link must have parentId set to the first non-migrated reroute - A new external output link must have the parentId of it's eldest remaining child set to undefined - A new internal input link must have the parentId of it's eldest remaining child set to undefined - A new internal output link must have the parentId set to the first migrated reroute This is handled in two parts by adding logic where the boundry links is created - The change involves mutation of inputs (which isn't great) but the function here was already mutating inputs into an invalid state - @DrJKL Do you see a quick way to better fix both these cases? Looks like litegraph tests aren't enabled and cursory glance shows multiple need to be updated to reflect recent changes. I'll still try to add some tests anyways. EDIT: Tests are non functional. Seems the subgraph conversion call requires the rest of the frontend is running and has event listeners to register the subgraph node def. More work than anticipated, best revisited later Resolves #5669 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5708-Migrate-parentIds-when-converting-to-subgraph-2746d73d365081f78acff4454092c74a) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>
This commit is contained in:
@@ -1571,8 +1571,21 @@ export class LGraph
|
||||
|
||||
// Inputs, outputs, and links
|
||||
const links = internalLinks.map((x) => x.asSerialisable())
|
||||
const inputs = mapSubgraphInputsAndLinks(resolvedInputLinks, links)
|
||||
const outputs = mapSubgraphOutputsAndLinks(resolvedOutputLinks, links)
|
||||
|
||||
const internalReroutes = new Map([...reroutes].map((r) => [r.id, r]))
|
||||
const externalReroutes = new Map(
|
||||
[...this.reroutes].filter(([id]) => !internalReroutes.has(id))
|
||||
)
|
||||
const inputs = mapSubgraphInputsAndLinks(
|
||||
resolvedInputLinks,
|
||||
links,
|
||||
internalReroutes
|
||||
)
|
||||
const outputs = mapSubgraphOutputsAndLinks(
|
||||
resolvedOutputLinks,
|
||||
links,
|
||||
externalReroutes
|
||||
)
|
||||
|
||||
// Prepare subgraph data
|
||||
const data = {
|
||||
@@ -1714,10 +1727,10 @@ export class LGraph
|
||||
// Reconnect output links in parent graph
|
||||
i = 0
|
||||
for (const [, connections] of outputsGroupedByOutput.entries()) {
|
||||
// Special handling: Subgraph output node
|
||||
i++
|
||||
for (const connection of connections) {
|
||||
const { input, inputNode, link, subgraphOutput } = connection
|
||||
// Special handling: Subgraph output node
|
||||
if (link.target_id === SUBGRAPH_OUTPUT_ID) {
|
||||
link.origin_id = subgraphNode.id
|
||||
link.origin_slot = i - 1
|
||||
@@ -2013,33 +2026,50 @@ export class LGraph
|
||||
while (parentId) {
|
||||
instance.parentId = parentId
|
||||
instance = this.reroutes.get(parentId)
|
||||
if (!instance) throw new Error('Broken Id link when unpacking')
|
||||
if (!instance) {
|
||||
console.error('Broken Id link when unpacking')
|
||||
break
|
||||
}
|
||||
if (instance.linkIds.has(linkInstance.id))
|
||||
throw new Error('Infinite parentId loop')
|
||||
instance.linkIds.add(linkInstance.id)
|
||||
parentId = instance.parentId
|
||||
}
|
||||
}
|
||||
if (!instance) continue
|
||||
parentId = newLink.iparent
|
||||
while (parentId) {
|
||||
const migratedId = rerouteIdMap.get(parentId)
|
||||
if (!migratedId) throw new Error('Broken Id link when unpacking')
|
||||
if (!migratedId) {
|
||||
console.error('Broken Id link when unpacking')
|
||||
break
|
||||
}
|
||||
instance.parentId = migratedId
|
||||
instance = this.reroutes.get(migratedId)
|
||||
if (!instance) throw new Error('Broken Id link when unpacking')
|
||||
if (!instance) {
|
||||
console.error('Broken Id link when unpacking')
|
||||
break
|
||||
}
|
||||
if (instance.linkIds.has(linkInstance.id))
|
||||
throw new Error('Infinite parentId loop')
|
||||
instance.linkIds.add(linkInstance.id)
|
||||
const oldReroute = subgraphNode.subgraph.reroutes.get(parentId)
|
||||
if (!oldReroute) throw new Error('Broken Id link when unpacking')
|
||||
if (!oldReroute) {
|
||||
console.error('Broken Id link when unpacking')
|
||||
break
|
||||
}
|
||||
parentId = oldReroute.parentId
|
||||
}
|
||||
if (!instance) break
|
||||
if (!newLink.externalFirst) {
|
||||
parentId = newLink.eparent
|
||||
while (parentId) {
|
||||
instance.parentId = parentId
|
||||
instance = this.reroutes.get(parentId)
|
||||
if (!instance) throw new Error('Broken Id link when unpacking')
|
||||
if (!instance) {
|
||||
console.error('Broken Id link when unpacking')
|
||||
break
|
||||
}
|
||||
if (instance.linkIds.has(linkInstance.id))
|
||||
throw new Error('Infinite parentId loop')
|
||||
instance.linkIds.add(linkInstance.id)
|
||||
|
||||
@@ -4,6 +4,7 @@ import { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
||||
import { LLink } from '@/lib/litegraph/src/LLink'
|
||||
import type { ResolvedConnection } from '@/lib/litegraph/src/LLink'
|
||||
import { Reroute } from '@/lib/litegraph/src/Reroute'
|
||||
import type { RerouteId } from '@/lib/litegraph/src/Reroute'
|
||||
import {
|
||||
SUBGRAPH_INPUT_ID,
|
||||
SUBGRAPH_OUTPUT_ID
|
||||
@@ -259,10 +260,29 @@ export function groupResolvedByOutput(
|
||||
|
||||
return groupedByOutput
|
||||
}
|
||||
function mapReroutes(
|
||||
link: SerialisableLLink,
|
||||
reroutes: Map<RerouteId, Reroute>
|
||||
) {
|
||||
let child: SerialisableLLink | Reroute = link
|
||||
let nextReroute =
|
||||
child.parentId === undefined ? undefined : reroutes.get(child.parentId)
|
||||
|
||||
while (child.parentId !== undefined && nextReroute) {
|
||||
child = nextReroute
|
||||
nextReroute =
|
||||
child.parentId === undefined ? undefined : reroutes.get(child.parentId)
|
||||
}
|
||||
|
||||
const lastId = child.parentId
|
||||
child.parentId = undefined
|
||||
return lastId
|
||||
}
|
||||
|
||||
export function mapSubgraphInputsAndLinks(
|
||||
resolvedInputLinks: ResolvedConnection[],
|
||||
links: SerialisableLLink[]
|
||||
links: SerialisableLLink[],
|
||||
reroutes: Map<RerouteId, Reroute>
|
||||
): SubgraphIO[] {
|
||||
// Group matching links
|
||||
const groupedByOutput = groupResolvedByOutput(resolvedInputLinks)
|
||||
@@ -279,8 +299,10 @@ export function mapSubgraphInputsAndLinks(
|
||||
if (!input) continue
|
||||
|
||||
const linkData = link.asSerialisable()
|
||||
link.parentId = mapReroutes(link, reroutes)
|
||||
linkData.origin_id = SUBGRAPH_INPUT_ID
|
||||
linkData.origin_slot = inputs.length
|
||||
|
||||
links.push(linkData)
|
||||
inputLinks.push(linkData)
|
||||
}
|
||||
@@ -340,7 +362,8 @@ export function mapSubgraphInputsAndLinks(
|
||||
*/
|
||||
export function mapSubgraphOutputsAndLinks(
|
||||
resolvedOutputLinks: ResolvedConnection[],
|
||||
links: SerialisableLLink[]
|
||||
links: SerialisableLLink[],
|
||||
reroutes: Map<RerouteId, Reroute>
|
||||
): SubgraphIO[] {
|
||||
// Group matching links
|
||||
const groupedByOutput = groupResolvedByOutput(resolvedOutputLinks)
|
||||
@@ -355,10 +378,11 @@ export function mapSubgraphOutputsAndLinks(
|
||||
const { link, output } = resolved
|
||||
if (!output) continue
|
||||
|
||||
// Link
|
||||
const linkData = link.asSerialisable()
|
||||
linkData.parentId = mapReroutes(link, reroutes)
|
||||
linkData.target_id = SUBGRAPH_OUTPUT_ID
|
||||
linkData.target_slot = outputs.length
|
||||
|
||||
links.push(linkData)
|
||||
outputLinks.push(linkData)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user