TinyTerra / ComfyUI_tinyterraNodes

A selection of nodes for Stable Diffusion ComfyUI
GNU General Public License v3.0
422 stars 45 forks source link

pipeedit problem #105

Closed cardenluo closed 6 months ago

cardenluo commented 6 months ago

image there is some wrong with piptedit ,how does it work

TinyTerra commented 6 months ago

All of the inputs are optional. You can either put in all of the inputs to compile them into a pipe line:

Or you can 'edit' an existing pipe_line - any of the other inputs will override the same value currently in the pipe: in this example the latent in will override the current pipe latent

The outputs on the right are the same values that are packed into the output pipe

cardenluo commented 6 months ago

image Thank you for your reply,I really like your node. I’ve used it to build many workflows, and my thinking has become clearer. 1)Based on the test results, it is not possible to edit at both ends simultaneously in the pip edit mode. You can either only input or only output. So, will this node replace the “Pipe in and pipe out”? 2)However, in this case, re-editing is not supported?

cardenluo commented 6 months ago

image Additionally, I’d like to express my personal experience with the loader, which feels a bit like a ‘behemoth.’ These ports are typically used in branch routes, and if they could be organized with a pipeout, it would make the workflow cleaner. This is just my personal impression .

TinyTerra commented 6 months ago

1)Based on the test results, it is not possible to edit at both ends simultaneously in the pip edit mode. You can either only input or only output. So, will this node replace the “Pipe in and pipe out”?

That was a bug, apologies. it wasn't ouputting the data correctly, and i've now pushed a fix for it.

In that specific example though you could also take the clip from the loader/wherever the pipe is coming from and put it straight into the KSamp override. (i feel like the pipeEdit is somewhat made redundant with the KSamp overrides since it achieves the same thing)

For example: the negative achieves the same result without the pipeEdit image

however if you want to explicitly change the clip the pipe is using before the pos embed (to use in ksamp for lora loading) you could do it like this image

That is if you want it explicitly for that specific conditioning as well, otherwise using the ksamp override would suffice. In fact, if you were to put a clip into the pipeEdit, and then also use the KSamp override with a different clip, the override clip would take priority and modify the pipeline after the KSamp anyway.

Additionally, I’d like to express my personal experience with the loader, which feels a bit like a ‘behemoth.’

That is somewhat by design, it's entire purpose is to group a large number of setup nodes into one, which makes it becoming a bit of a beast somewhat unavoidable

These ports are typically used in branch routes, and if they could be organized with a pipeout, it would make the workflow cleaner.

That is a fair point, the main issue i have with adding that kind of 'metadata' info to the pipeline, is also already a bit of an issue with the entire pipeline currently. That is - it becomes incredibly murky and unclear what the data is / where its coming from. Pipe is great at minimizing noodles but then it hides a lot of the actual flow. Adding metadata into the mix would cause more confusion imo.

if you were to take the ksampler pipe should the metadata reference the original latent width/height, or the KSamp images? And you would perhaps expect the pos/neg string to also include external/added conditioning text - which wouldn't really be feasible. on top of the added confusion/complexity it would then 'require' an additional node for a simple output which in my mind, goes against the original idea of ttNs ideology of minimizing node soup

Thats my thought currently, but open to discuss it more

cardenluo commented 6 months ago

The new version, this pipeEDIT outputs and inputs are very smooth, which is great.