search

chrisgoringe / cg-image-picker

227 stars 15 forks source link

Issue with chained choosers (edit: when used with Context node from rgthree-comfy) #32

Closed chrisgoringe closed 10 months ago

chrisgoringe commented 11 months ago

https://reddit.com/r/comfyui/s/hEMQtJJjto

chrisgoringe commented 11 months ago

Fixed in new nodes

Erehr commented 11 months ago

One bug on unifer-chooser branch with chained choosers where restart will always trigger first chooser.

Choose from 1st one -> Choose from 2nd one -> finish -> restart from 2nd -> now 1st one is waiting for selection

Previously it probably wasn't an issue because choosers were different nodes (1st for latent, 2nd for images) and now they are the same.

chrisgoringe commented 11 months ago

I can't reproduce - this workflow seems to work as it ought to

workflow-chain.zip

If you have an example of it not working, could you share the workflow?

Erehr commented 11 months ago

Couldn't reproduce it with simple workflow either, so I slowly simplifying my current one to see at which point it will work and what may cause it. Seems the restart has a problem with Context node from rgthree-comfy and simply decides to start from begining.

Replaced it combination of 'Pipe In / Pipe out' from other extensions and it's working. Have to keep workflow organized somehow (UE Nodes wink wink)

image

chrisgoringe commented 11 months ago

If you were to share a simple-as-possible workflow that fails I could look at whether they can be made to play nice.

Erehr commented 11 months ago

https://gist.github.com/Erehr/3d13a20a3f797fa4a1098e8704b75457

Restarting from second chooser will run from sampler again with the same result (even with randomized seed)

chrisgoringe commented 10 months ago

Need to check if this is still a problem with the changes in latest versions

chrisgoringe commented 10 months ago

OK, this is actually expected behavior...

Why? When you restart at chooser 2, we need to work out what nodes are still needed. So we head downstream, finding everywhere the data from the restart will go. So that's nodes with id (14, 16, 17, 8).

But then each of those nodes needs it's inputs. So we go back upstream to work out what's needed. But node 17 gets its VAE from the rightmost context node (19), so that has to run, and it gets data from the middle context node (15), so that has to run, and it gets data from the left hand context node (18) and from (13), so 18 and 13 have to run etc. In fact, everything needs to run!

It should be possible to fix with UE like this (21 connected back to 1), because then the UE runs, and 1 runs, but nothing else: image but at the moment restart doesn't know about UE links - that's issue #26 and I plan to fix it soon!

This (with the reroute connected back to 1) does work: image

So - closing this, but watch #26 for news!

chrisgoringe commented 10 months ago

OK - update both image chooser and UE, and this now works:

workflow-works.zip

(note I had to connect the latent input to the first chooser node... ALWAYS connect the latents! image