Subgraph support

[space-nuko]

Support litegraph's built-in graph/subgraph type for grouped nodes

NOTE: Requires patches to litegraph, this was the least amount of modification necessary for the feature to work - https://github.com/space-nuko/litegraph.js/tree/comfyui-subgraphs

[ltdrdata]

Wouldn't it be better to edit the interfaces only within the subgraph nodes instead of adding them from outside using the "+" button? Also, currently, the name and type are determined based on the user's text input, but it seems more beneficial to have the type automatically determined through wiring.

Additionally, If subgraphs are allowed to have connections through widget conversions and thus the interfaces within subgraphs are not limited only to connections from external nodes, wouldn't it be better?

Based on the serialized structure of the subgraph where nodes contain subgraphs, it seems like it would be more efficient to elevate the subgraph to the workflow level and share it as much as possible until the subgraph is edited. This approach can help reduce the workflow. In particular, if the subgraph is componentized and used repeatedly, it could lead to an excessively large workflow due to duplication.

[space-nuko]

Wouldn't it be better to edit the interfaces only within the subgraph nodes instead of adding them from outside using the "+" button?

I agree with this and don't understand why litegraph offers those buttons, could disable the rendering for them

Also, currently, the name and type are determined based on the user's text input, but it seems more beneficial to have the type automatically determined through wiring.

I think it could get complicated to sync state with the way they're designed, are there other systems that have auto-wired subgraphs like that

Personally I prefer defining the names and types of the inputs beforehand. The names have to be unique on the same graph since the inputs are referenced by name. The types are edited with a textbox currently but I have made it in ComfyBox so all the known types can be selected from a dropdown instead

Additionally, If subgraphs are allowed to have connections through widget conversions and thus the interfaces within subgraphs are not limited only to connections from external nodes, wouldn't it be better?

By which you mean exposing widgets on subgraphs? That could be very useful for ComfyUI specifically since everything is widget-oriented. Only issue is how to synchronize the state internally. I think an extension could be made for that like there is for other ComfyUI nodes

Based on the serialized structure of the subgraph where nodes contain subgraphs, it seems like it would be more efficient to elevate the subgraph to the workflow level and share it as much as possible until the subgraph is edited. This approach can help reduce the workflow. This approach can help reduce the workflow. In particular, if the subgraph is componentized and used repeatedly, it could lead to an excessively large workflow due to duplication.

I hadn't thought of that actually, having a "frozen subgraph" feature would help reduce workflow size considerably. But I think that's a major change that would need to go in base litegraph since it sounds like it could cause a lot of edge cases. Plus how would this work with subgraphs that can have different state but the same structure?

I would estimate the overhead caused by introducing subgraphs wouldn't be as large as the large duplicated structures that subgraphs would be most useful for but I'll have to see

[ltdrdata]

I agree with this and don't understand why litegraph offers those buttons, could disable the rendering for them

Also, currently, the name and type are determined based on the user's text input, but it seems more beneficial to have the type automatically determined through wiring.

I think it could get complicated to sync state with the way they're designed, are there other systems that have auto-wired subgraphs like that

My idea actually originated from the PrimitiveNode. During the subgraph editing phase, the concept is to have interface nodes set as a '*' type by default, which allows all connections. When an interface node is connected to a slot within an internal node, the slot's type, which was initially set as '*', automatically changes to match the type of the connected slot.

Personally I prefer defining the names and types of the inputs beforehand. The names have to be unique on the same graph since the inputs are referenced by name. The types are edited with a textbox currently but I have made it in ComfyBox so all the known types can be selected from a dropdown instead

Indeed, the top-down approach can be convenient. To cover such cases, one possibility is to have users specify only the name when generating inputs for the subgraph node from external nodes, while leaving the type as '*' initially. Since the type can be determined during the internal node's connection process, it would be cumbersome to have users manually adjust this through input.

Additionally, If subgraphs are allowed to have connections through widget conversions and thus the interfaces within subgraphs are not limited only to connections from external nodes, wouldn't it be better?

By which you mean exposing widgets on subgraphs? That could be very useful for ComfyUI specifically since everything is widget-oriented. Only issue is how to synchronize the state internally. I think an extension could be made for that like there is for other ComfyUI nodes

Currently, in the workflow component, this is being handled by relying on the registerNodes function. I'm not familiar with the node registration structure of subgraphs, but I hope this information can be helpful as a reference.

https://github.com/ltdrdata/ComfyUI/blob/5c38958e49efd11b5234cb5ff472d752698c5090/web/scripts/app.js#L1044

Based on the serialized structure of the subgraph where nodes contain subgraphs, it seems like it would be more efficient to elevate the subgraph to the workflow level and share it as much as possible until the subgraph is edited. This approach can help reduce the workflow. This approach can help reduce the workflow. In particular, if the subgraph is componentized and used repeatedly, it could lead to an excessively large workflow due to duplication.

I hadn't thought of that actually, having a "frozen subgraph" feature would help reduce workflow size considerably. But I think that's a major change that would need to go in base litegraph since it sounds like it could cause a lot of edge cases. Plus how would this work with subgraphs that can have different state but the same structure?

I would estimate the overhead caused by introducing subgraphs wouldn't be as large as the large duplicated structures that subgraphs would be most useful for but I'll have to see

If we take a very conservative approach, it may be sufficient to consider any changes in the configuration of a subgraph as the creation of a completely separate subgraph.

Indeed, it seems that recently the size of workflows has become too large, and there have been concerns about the resulting PNG images being too big in relation to their actual content. Considering options to compress or reduce the size of workflows could be a worthwhile consideration.

[space-nuko]

My idea actually originated from the PrimitiveNode. During the subgraph editing phase, the concept is to have interface nodes set as a '' type by default, which allows all connections. When an interface node is connected to a slot within an internal node, the slot's type, which was initially set as '', automatically changes to match the type of the connected slot.

I can see this being useful for certain generic node types like the reroute but it might not be appropriate for subgraphs in particular

• What happens when an input is disconnected? Will it return to wildcard status?
• If the types of the inputs/outputs change dynamically it can become hard to understand what a subgraph is meant to do at the top level
• The inputs/outputs have to be connected externally to the subgraph, and internally inside the inner graph. The inner graph assumes a certain layout of inputs/outputs and types already, so with them changing with every connection it would be hard to keep track of?

Indeed, the top-down approach can be convenient. To cover such cases, one possibility is to have users specify only the name when generating inputs for the subgraph node from external nodes, while leaving the type as '*' initially. Since the type can be determined during the internal node's connection process, it would be cumbersome to have users manually adjust this through input.

This can be made less cumbersome with a better dropdown instead of a text input. If the type changes permanently on first connect it could be a mild convenience, but what if the user changes their mind afterwards? Litegraph offers no undo so they'd have to go and change the input/output type by hand anyway.

I think it's at most a few minutes of inconvenience you have to work through up front before the layout can be configured for the rest of the future. Subgraphs are a somewhat more advanced feature than working with top-level nodes after all

Currently, in the workflow component, this is being handled by relying on the registerNodes function. I'm not familiar with the node registration structure of subgraphs, but I hope this information can be helpful as a reference.

Aware of that but there probably needs to be a new mechanism for keeping track of widgets added solely by the frontend. I think the widgets must correspond exactly to a single subgraph input/output represented by the special nodes in the inner graph (same as with converted inputs/outputs). With a JS extension it should be possible

Indeed, it seems that recently the size of workflows has become too large, and there have been concerns about the resulting PNG images being too big in relation to their actual content. Considering options to compress or reduce the size of workflows could be a worthwhile consideration.

GZip can compress text with a lot of redundant data at a ratio of around 75%, so that ought to be used at a bare minimum before any other optimizations are considered

[ltdrdata]

• What happens when an input is disconnected? Will it return to wildcard status?
• If the types of the inputs/outputs change dynamically it can become hard to understand what a subgraph is meant to do at the top level
• The inputs/outputs have to be connected externally to the subgraph, and internally inside the inner graph. The inner graph assumes a certain layout of inputs/outputs and types already, so with them changing with every connection it would be hard to keep track of?

My intention was not to determine the type based on external connections, but rather to determine the type based on internal connections. When creating an interface from the outside, it is initially set as the '*' type to allow any connection, and if it is connected internally, it reflects on the external interface.

This can be made less cumbersome with a better dropdown instead of a text input. If the type changes permanently on first connect it could be a mild convenience, but what if the user changes their mind afterwards? Litegraph offers no undo so they'd have to go and change the input/output type by hand anyway.

Wouldn't it be better to change the type back to '*' when the link was disconnected, instead of determining the type permanently?

[space-nuko]

My intention was not to determine the type based on external connections, but rather to determine the type based on internal connections. When creating an interface from the outside, it is initially set as the '*' type to allow any connection, and if it is connected internally, it reflects on the external interface.

I think it could work for the inner nodes but it would be a litegraph change too

Wouldn't it be better to change the type back to '*' when the link was disconnected, instead of determining the type permanently?

Maybe for inner nodes, afterward you'd have to make sure the links connected to the subgraph from other nodes are valid when doing so or some work in hooking them up would be lost

[space-nuko]

I think this will be more involved of a change than I previously thought

Some frontend extensions use app.graph, the top-level graph for queries. Now there can be more than one graph, so the correct graph might not be used in that case. Those callers need to use node.graph instead

Plus getNodeById() only operates on the top-level graph, it does not recurse. I had to add my own getNodeByIdRecursive() to fix that

[mcmonkey4eva]

Node-groupings were added recently

[daniel-lewis-ab]

Read through this.

I am interested in implementing subgraphs in ComfyUI and have the litegraph repo. We should talk with the learnings from this discussion in hand, when we're ready to implement subgraph.

I do think this is super powerful and a relevant PR to address.