search

comfyanonymous / ComfyUI

The most powerful and modular diffusion model GUI, api and backend with a graph/nodes interface.
https://www.comfy.org/
GNU General Public License v3.0
49.85k stars 5.25k forks source link

[FEATURE REQUEST] sub workflows with customizable inputs / output pins #669

Open lemmy101 opened 1 year ago

lemmy101 commented 1 year ago

Apologies if this has already been suggested, it probably has, but I think its the biggest improvement that could possibly be made. I've not manged to dive into ComfyUI that much yet, but from my limited experiments its amazing and I suspect the future of SD generation and certainly going to be my tool of choice. However, one thing i've yet to find in any extensions is the ability to further modularise development of complex workflows often found in other node based scripting that makes me hesitate. Being able to create a custom pseudo node as an end user, the equivalent of a code function, with defined inputs and outputs, that can be expanded to a completely new canvas and exported / imported between workflows. If this functionality is available anywhere and I missed it i apologise, but i can't seem to find anything.

This would open up the community who aren't coders making all sorts of really cool scripted nodes with nothing more than ComfyUI itself, modules that can be easily dropped into someone's workflow with customizable in/out pins they just need to connect up, or go poking at the sub-canvas innards if they want, potentially even with the ability to expose sub-node's parameters on the face of the node to allow access to important relevant properties that influence how the nodes inside it operate.

It's also a great organizational way to clean up the ui and promote reuse of previous work, where groups connecting to other groups can lead to quite a cluttered page, esp as node connections still need to connect to specific nodes within the group, having pseudo-nodes that hide the implementation away unless needed can allow users to create a library of oft repeated node combinations, with their own inputs and outputs to quickly reuse as if they were built in nodes.

space-nuko commented 1 year ago

In case you're interested I'm working on a new frontend for ComfyUI that supports more than one workflow per graph. Below is an example, first an image is drawn with an editor widget, then it's processed through img2img. Finally the user can press an "Upscale" button to have the selected output upscaled with diffusion, all within the same graph/UI

2023-05-18 02_48_56-http___localhost_8000_# - Chromium

The graph itself looks like this, I've been working on modularizing pieces of the graph into subgraphs like model loading/sampling. The light blue node in the middle is first-stage txt2img/img2img, the one to the right is for diffusion upscaling. At some point I want these pieces to be savable as templates to reuse elsewhere

2023-05-18 02_57_46-http___localhost_8000_# - Chromium

I also want to implement the ability to send image/other outputs between entire graphs, so you can do some processing in one workflow and go back to e.g. img2img with that result

https://github.com/space-nuko/ComfyBox

lemmy101 commented 1 year ago

oh nice will give it a gander for sure, thanks!

levaleureux commented 1 year ago

Hi @space-nuko will you create a simple PR for sub-graph for comfyUI like you show screen shot on https://matrix.to/#/!SLQulqwEQbBXcnBrCi:matrix.org/$vNtC68liVPsGoSK_AKZ2uiuOz_LfQDKuGpgoE8X15Ho?via=matrix.org&via=fx45.in&via=envs.net

will be glad to test the PR

space-nuko commented 1 year ago

I think it will require several changes in litegraph to fix bugs. The most major change is for subgraphs to be compatible with ComfyUI's backend, the node IDs have to be globally unique across all subgraphs. But this is an issue since node and link IDs are steadily increasing integers within each graph, so you can end up with more than one node/link with the same ID. So I added new logic in litegraph that instantiates new nodes with UUIDs instead.

There were also a lot of other issues stemming from graph inputs/outputs not receiving the correct types so they couldn't be hooked up to backend only nodes. Also, when cloning nodes you have to reassign new UUIDs to not end up with duplicate nodes/links again, and to duplicate the corresponding UI state I produced a mapping from old->new UUID to pass into the frontend

These changes are in my fork of litegraph which isn't API compatible with upstream litegraph however as the pace of development is too significant (it is also a TypeScript conversion and the original author is against introducing extra tooling into his repo, but it was needed for interfacing with my own TypeScript code without errors). https://github.com/space-nuko/litegraph.ts

rickcx commented 1 year ago

Apologies if this has already been suggested, it probably has, but I think its the biggest improvement that could possibly be made. I've not manged to dive into ComfyUI that much yet, but from my limited experiments its amazing and I suspect the future of SD generation and certainly going to be my tool of choice. However, one thing i've yet to find in any extensions is the ability to further modularise development of complex workflows often found in other node based scripting that makes me hesitate. Being able to create a custom pseudo node as an end user, the equivalent of a code function, with defined inputs and outputs, that can be expanded to a completely new canvas and exported / imported between workflows. If this functionality is available anywhere and I missed it i apologise, but i can't seem to find anything.

This would open up the community who aren't coders making all sorts of really cool scripted nodes with nothing more than ComfyUI itself, modules that can be easily dropped into someone's workflow with customizable in/out pins they just need to connect up, or go poking at the sub-canvas innards if they want, potentially even with the ability to expose sub-node's parameters on the face of the node to allow access to important relevant properties that influence how the nodes inside it operate.

It's also a great organizational way to clean up the ui and promote reuse of previous work, where groups connecting to other groups can lead to quite a cluttered page, esp as node connections still need to connect to specific nodes within the group, having pseudo-nodes that hide the implementation away unless needed can allow users to create a library of oft repeated node combinations, with their own inputs and outputs to quickly reuse as if they were built in nodes.

I completely agree! This function is extremely important and very useful! It's just like the geometry node in Blender.

kenjiqq commented 1 year ago

Subflows in nodered is a good example of this as well https://nodered.org/docs/user-guide/editor/workspace/subflows Very useful in organizing larger graphs to avoid duplications

rsamf commented 11 months ago

Any progress on this? I'm thinking of implementing this myself

levaleureux commented 11 months ago

@rsamf If you have enough python skill for it please do it. It's a feature that a large part of the user base is waiting for.

rsamf commented 11 months ago

@rsamf If you have enough python skill for it please do it. It's a feature that a large part of the user base is waiting for.

I would love to make a PR as this requires more changes to the codebase than just creating a few custom nodes. However, I don't know how the owner feels about adding this extra feature. Nonetheless, I'll start development now in my ComfyUI fork.

The following describes my idea on how the user would create a subflow. The custom nodes will be made available:

In order to use a subflow, the user can load a normal workflow (.json file or comfy .png file) through a loaders/subflow node. However, in order for the loaded subflow node to recognize which pins are used as inputs and outputs, we need some way to tag a subset of the inputs/output variables in the workflow. This means when a user is creating a subflow, they can export whichever variables they want via the export/ nodes.

The export/ nodes are functionally reroute nodes and essentially do nothing to the workflow execution. They only act as tags to mark which variables to export to the subflow node. Then, when the subflow node is loaded, it will render only the input/output pins that were exported.

I'm also thinking of adding other quality-of-life features to the subflow node like showing:

Let me know if you would like more features added to this.

rsamf commented 11 months ago

Update: instead of having all of the export/ nodes, I just allow the user to right click the node and choose Export > {pin} and then the metadata will appear in the node's properties. This seems more intuitive.

levaleureux commented 11 months ago

OK, so you know your job... I would like to contribute by discussing usability. From what I know blender subnode system is very well done, it will be great if we can mimic it. Especially the edit subflow feature.

If you want we can discuss mockup and UX ?

rsamf commented 11 months ago

@levaleureux If you want to speak in a synchronous manner, I am up for it. I am also looking at geometry nodes in Blender, and I found Node Groups. My idea is conceptually the same: there's a set of user-chosen inputs and outputs that is exported to control the behavior of the node group. I hope to have a working version of subflows this week for anyone interested in testing.

rsamf commented 11 months ago

@levaleureux There's a beta version in https://github.com/heddleai/ComfyUI/tree/subflows. You can clone this repo and checkout the subflows branch to test it out. Right click a node, Export > {choose your var} to create your subflow. Then, once you save the workflow like normal, you can move the json file over to ComfyUI/subflows directory. Then to use it, Add a Node > loaders > Load Subflow. I still need to squash some bugs (you may notice some) and add more functionality, but the gist of the idea is there. Note that I have an extra node called InMemorySubflow in that branch which isn't ready yet, so it's not worth testing.

Waiting for feedback.

levaleureux commented 10 months ago

@rsamf Thanks for your reply I will test it during this week.

rsamf commented 10 months ago

Ok, I didn't get any feedback, but I finished up a new revision. This one has no bugs (that I'm aware of) and it supports exporting widgets too now. This update is located in the the filesubflows branch in my ComfyUI fork. Again, right click a node to export inputs, widgets, or outputs, then save the workflow or generated image, and then, load the workflow .json or .png with the File Subflow node (located in loaders > Load Subflow). ... and yes, you can have subflows within subflows within subflows, etc...

As a note, I am worried that the owner of the repo may not accept the change because a lot of work was needed to be done to support recursive subflows (i.e. subflows within a subflow) and in the way that the prompt is constructed. The way it works, is if any node has a subflow (e.g. File Subflow), then it constructs the prompt of that subflow, and then it adds that prompt to its own prompt. To avoid having to convert every node ID to UUID's to support this, I made sure that the child node ids do not conflict. I can go into more detail on how it works, but that may be too verbose for this thread.

0xdevalias commented 9 months ago

I think it will require several changes in litegraph to fix bugs.

These changes are in my fork of litegraph which isn't API compatible with upstream litegraph however as the pace of development is too significant

This is probably a little off topic for this issue, and I suspect that it would be a rather major / potentially breaking change to do so.. but I'm curious if there's a reason that litegraph is used instead of something like xyflow / reactflow / etc? I haven't deeply evaluated both projects yet, but at least in my initial cursory exploration, xyflow seems like it's capable of everything liteflow does, but in a more standard are modern developed/maintained way.

Also, while not directly related to this issue, I feel like some of the potential use cases/benefits of subgraphs have crossover with these issues:

ThomasDerlande commented 8 months ago

Hello @rsamf I've tested your subflow system with the most simple subflow. Giving you some feedbacks below.

What I have done:

  1. Loaded the ComfyUI default workflow: image
  2. Removed everything exept for the KSampler and VAE decode nodes.
  3. Exported the model/positive/negative/latent_image/seed inputs of the KSampler node
  4. Exported the vae input and IMAGE output of the VAE Decode node
  5. Saved the workflow: image
  6. Loaded the default subflow again.
  7. Removed the KSampler and VAE Decode nodes.
  8. Replaced them by my subflow saved at step 5
  9. Queued the Prompt

Results

The VAE is not passed to the VAE Decode node of the subflow even though the VAE is plugged into the vae input of the subflow. Got the Required input is missing: vae error message. Exporting the LATENT output of the KSampler and decoding it after the subflow call works but I can still see the error in the console (It's just that ComfyUI is able to run the workflow and get a result, so it's ignoring the VAE Decode node): image ➡️ No "IMAGE" comes out of the subflow because of the Required input is missing: vae error

Issues

  1. Reroutes break a subflow ➡️ A subflow containing a reroute makes the "Queue Prompt" button of the caller workflow have no effect (Nothing happens, even in the CLI)
  2. VAE is not passed to the subflow, even if plugged in. I also had the same issue with an image passed to a subflow in the "target" input of the "Bounded Image Blend with Mask" node (Error: "Required input is missing: target"). It may be a more general problem because I often have issues with inputs passed to a subflow in the caller workflow but in the end they are not passed and I have missing inputs errors in the CLI, even with types that work in a subflow, but are not passed in another one (e.g. model/positive/negative work in my first test (see steps above), but are not passed in another subflow (Missing input errors).
  3. If I play with the exports of a subflow, I may end up with errors like Return type mismatch between linked nodes: model, CONDITIONING != MODEL along with Return type mismatch between linked nodes: negative, VAE != CONDITIONING even though I plugged a model in the model input of the subflow node. Seems that the inputs get mixed up with each others.
  4. Your node doesn't seem to handle texts. The exported text from conditionning node does not appear in your File Subflow node.

Nice to have

A nice to have thing would be to have a list of all exported inputs and outputs of a subflow. For now it is difficult to determine which inputs and outputs are being exported. I'm thinking of the Blender "group" system (See image below) where we have such list in a side panel in which we can even set default values for each input and also set the order of the inputs, rename them and so on. Ideally if we could recreate the "group inputs" and "group outputs" behaviour of Blender, it would really improve the usability. It would require having a new node "subflow inputs" with only outputs, that we can plug in the inputs of the subflow nodes and another node "subflow outputs" with only inputs, in which we plug the outputs of the subflow nodes. Then a subflow input could be added by plugging the input of a node into an unnamed grey output of the "subflow input" node, automatically adding an input to the subflow and defining its data type and name. These subflow inputs could be modified in a side panel. Same behaviour for defining outputs. This would replicate the Blender bahaviour, however I know that it requires a LOT of work to implement such a feature... image

Conclusion

Overall I think the main core of the subflow feature is here, but the usability could be greatly improved. Also for now the feature does not seem to be ready for productive use as it does not seem stable enough yet. I would love to help in the development but I don't have the time nor the python coding skills to do so. Anyway keep up the good work, I think you're on the right track.

rsamf commented 8 months ago

@ThomasDerlande Thank you so much for the amazing feedback. I can go back and fix all of the issues and implement the Blender behavior, but I'm not sure if I have the capacity to do so right now because I have a lot of work in other areas. Also, I'm hesitant to keep working on this because I don't know if this will be accepted into ComfyUI's main branch as (1) it requires a lot of re-work to how the graph (or prompt) is sent to the server, and (2) this may be something that not enough people actually want. I have 2 PR's up right now, one for this issue and one for a bug fix. Neither are being accepted or being reviewed by the owner of the repo. For the meantime, I have abandoned collaborating on the repo. If anyone would like to pick up where I left off, I can give you permission to work on my filesubflows branch.