Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preview sampled images with TAESD #713

Merged
merged 11 commits into from
Jun 6, 2023

Conversation

space-nuko
Copy link
Contributor

@space-nuko space-nuko commented May 31, 2023

Lets KSampler show previews with taesd

Download the .pth models from https://github.com/madebyollin/taesd and stick them in models/taesd first

Previews are encoded to JPEG bytes first then sent over websockets, they're resized to 512 pixels first so the network load shouldn't be too much (~50kb per frame for 2048x2048 resolution)

Also adds taesd-related nodes, they're functionally equivalent to VAE loader/encoder/decoder nodes

output2.webm
2023-05-30 20_47_37-ComfyUI - Chromium

@WASasquatch
Copy link
Contributor

This is great. Definitely a great core feature. Though, why is the sampler preview version so much richer in color?

@ltdrdata
Copy link
Collaborator

This feature will be a powerful impression for users looking at ComfyUI.

@space-nuko
Copy link
Contributor Author

Though, why is the sampler preview version so much richer in color?

Not terribly certain, it could be an artifact of the PIL conversion to JPEG. Here is another comparison using the WD VAE for decoding instead

2023-05-30 20_57_41-ComfyUI - Chromium

@WASasquatch
Copy link
Contributor

Though, why is the sampler preview version so much richer in color?

Not terribly certain, it could be an artifact of the PIL conversion to JPEG. Here is another comparison using the WD VAE for decoding instead
2023-05-30 20_57_41-ComfyUI - Chromium

Is it normalizing to 0-255 before Image.fromarray()? I'd think if it wasn't it'd be more noticeable in inverted areas but maybe

@space-nuko
Copy link
Contributor Author

space-nuko commented May 31, 2023

Yeah it's clamped beforehand, I think some of the code I adapted from A1111 is suspect...

Some of it unscales latents to a range within [-2, 2] which are then scaled again by 0.5, if I scale by 0.25 instead it looks slightly better but has a more washed out look

@space-nuko
Copy link
Contributor Author

Update: Now wraps taesd nodes in a new node to convert them to a LATENT_PREVIEWER, making the preview method plug-and-play

2023-05-31 08_24_48-ComfyUI - Chromium

@comfyanonymous
Copy link
Owner

After testing this a bit I don't think having a previewer node connection is a good idea. Since previews are not something that affect the output image they should not be a part of the workflow itself.

I think to start previews should just be a command line option: --enable-previews method and we can add a way to control it from the interface afterwards.

@WASasquatch
Copy link
Contributor

WASasquatch commented Jun 5, 2023

I think to start previews should just be a command line option: --enable-previews method and we can add a way to control it from the interface afterwards.

I agree with the first part, but not this. You may want a preview for something like a taxing HR image, but not something like a small 512x512 hand fix, or face fix. Simple preview boolean toggle would be fine.

@space-nuko
Copy link
Contributor Author

space-nuko commented Jun 5, 2023

Is there a use case for some nodes having previews and others not/having less expensive preview methods? Or being able to individually change settings for each node's preview?

Also this still doesn't help with the case of nodes that want custom preview methods besides taesd like Disco Diffusion

@WASasquatch
Copy link
Contributor

WASasquatch commented Jun 5, 2023

Is there a use case for some nodes having previews and others not/having less expensive preview methods? Or being able to individually change settings for each node's preview?

I literally just gave one. You may not want to slow down a process via callback previews if it's not necessary, especially if you're not caring about the result as something you may want to cancel. And if isn't about whether or not the result is worth continuing, it's not really a useful function (in general) at all besides "ooh,look, the diffusion process"

@comfyanonymous comfyanonymous merged commit 081134f into comfyanonymous:master Jun 6, 2023
@mflux
Copy link

mflux commented Dec 20, 2023

I followed the instructions but still don't understand. Where's the TAESD Decode node from your screenshot?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants