search

rgthree / rgthree-comfy

Making ComfyUI more comfortable!
MIT License
1.02k stars 77 forks source link

Bypasser not bypassing? (or me not understanding?) #12

Closed alessandroperilli closed 1 year ago

alessandroperilli commented 1 year ago

I'm starting to experiment with the Mute / Bypass Repeater node, using it to bypass the entire Detailer function of my workflow, rather than just some portions of it. It's very useful to save computing cycles, but I'm afraid I've not completely grasped how it works.

In the situation below, I have two main functions generating an image: either the SDXL Base+Refiner or the ReVision. They end up into a context switch which passes the non-null image to the Detailer and the Upscaler, organized in a chain:

Context Switch Test - Detail

I hook all the nodes of the Detailer to a Mute / Bypass Repeater node and that node to a Bypasser node.

At this point, I ask to bypass the Detailer, but not the Upscaler:

Screenshot 2023-09-03 at 01 09 25

I would expect that the generated image (coming either from the Refiner or ReVision) would pass through all the Detailer nodes and end up being processed by the Upscaler. But no.

If the Detailer is bypassed, ComfyUI simply refuses to work. I press Queue Prompt and nothing happens. No errors, no warnings, no console messages. It simply doesn't do anything.

Similarly, if I want to bypass both the Detailer and the Upscaler, and simply keep my image as it comes out of the Refiner or ReVision, I can't. ComfyUI doesn't operate.

For the workflow to work, I have to un-bypass at least the Detailer. Then, it does its thing splendidly. All your nodes are very powerful and useful, thank you.

As usual, the full workflow for reference:

Context Switch Test

rgthree commented 1 year ago

The state in your full workflow does look like it should work. Bypassing isn't provided by my nodes here, but looking at the code in comfy, when your "Upscale Image (using Model)" image input works backwards, it looks like it should follow the two Detailer nodes up to the switch. If you've pulled the latest version (after fixing #10), then that Switch should be choosing the Refiner Context with a valid Image.

Can you add a "Preview Image" node after that switch's with the switch's image output, and again after the two Detailers? If Comfy is working as it looks like it should, those two should be the same and from your refiner (looking at your full workflow, with Revision muted)

alessandroperilli commented 1 year ago

Well, at least I understood how it works.

I tried to place a debug Preview Image node everywhere but it's useless. The workflow simply doesn't start, so nothing gets processed at all. The only thing that works is the generation of a new seed.

rgthree commented 1 year ago

If you’d like, you can share your workflow JSON and I’ll take a look when I have a chance

alessandroperilli commented 1 year ago

Much appreciated. The json is here.

rgthree commented 1 year ago

Huh, so I found the error, and as I thought it's nothing to do with my nodes (phew!) or even your general workflow.

Looks like the two SEGS Reroute nodes you have going from the 'Size of the Small Faces' node over to the 'Detailer for BigFaces; node are using "Primitive Reroute" nodes from https://github.com/pythongosssss/ComfyUI-Custom-Scripts which, I guess, aren't compatible with ComfyUI's bypassing. The error comes from that node.

If you replace those with normal Reroute (or, my Reroute) nodes it seems to work fine.

(Note that in the pythongosssss README, they do state that their Primitive Reroute node should not be used for normal rerouting. I'm sure it was just a mistake, but just making note).

alessandroperilli commented 1 year ago

A valuable lesson. Thank you so much for the time you dedicated to it.