mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-10 01:50:08 +00:00
Fix edge cases in subgraph removal logic (#8758)
#8187 made removal of subgraphs cleanup the subgraph definition for the removed graph and call onRemove handlers. However, it missed some edge cases and broke subgraph conversion of selections containing subgraphs which this PR tries to address. - Deeply nested subgraphs are now also cleaned - Adding a subgraphNode to the graph also ensures nested subgraphs are added to subgraph definitions Reminder: under this change, nodes can continue to exist after their onRemoved handler has been called - It may be better to instead perform the "garbage collection" of subgraphs outside of the graph removal step to better handle edge cases like subgraph conversion where a subgraph may continue to persist after a parent subgraphNode has been removed from a graph. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8758-Fix-edge-cases-in-subgraph-removal-logic-3026d73d36508177b34ffdd2e0a114fe) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -8,6 +8,7 @@ import type { UUID } from '@/lib/litegraph/src/utils/uuid'
|
||||
import { createUuidv4, zeroUuid } from '@/lib/litegraph/src/utils/uuid'
|
||||
import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations'
|
||||
import { LayoutSource } from '@/renderer/core/layout/types'
|
||||
import { forEachNode } from '@/utils/graphTraversalUtil'
|
||||
|
||||
import type { DragAndScaleState } from './DragAndScale'
|
||||
import { LGraphCanvas } from './LGraphCanvas'
|
||||
@@ -973,6 +974,13 @@ export class LGraph
|
||||
this.canvasAction((c) => c.startGhostPlacement(node, opts.dragEvent))
|
||||
}
|
||||
|
||||
if (node.isSubgraphNode?.()) {
|
||||
forEachNode(node.subgraph, (innerNode) => {
|
||||
if (innerNode.isSubgraphNode())
|
||||
this.subgraphs.set(innerNode.subgraph.id, innerNode.subgraph)
|
||||
})
|
||||
}
|
||||
|
||||
// to chain actions
|
||||
return node
|
||||
}
|
||||
@@ -1034,14 +1042,14 @@ export class LGraph
|
||||
}
|
||||
}
|
||||
|
||||
// Subgraph cleanup (use local const to avoid type narrowing affecting node.graph assignment)
|
||||
const subgraphNode = node.isSubgraphNode() ? node : null
|
||||
if (subgraphNode) {
|
||||
for (const innerNode of subgraphNode.subgraph.nodes) {
|
||||
if (node.isSubgraphNode()) {
|
||||
forEachNode(node.subgraph, (innerNode) => {
|
||||
innerNode.onRemoved?.()
|
||||
subgraphNode.subgraph.onNodeRemoved?.(innerNode)
|
||||
}
|
||||
this.rootGraph.subgraphs.delete(subgraphNode.subgraph.id)
|
||||
innerNode.graph?.onNodeRemoved?.(innerNode)
|
||||
if (innerNode.isSubgraphNode())
|
||||
this.rootGraph.subgraphs.delete(innerNode.subgraph.id)
|
||||
})
|
||||
this.rootGraph.subgraphs.delete(node.subgraph.id)
|
||||
}
|
||||
|
||||
// callback
|
||||
|
||||
Reference in New Issue
Block a user