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

Load image batches #707

Closed
wants to merge 11 commits into from

Conversation

space-nuko
Copy link
Contributor

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

2023-05-29 17_11_37-ComfyUI - Chromium 2023-05-29 16_40_37-ComfyUI - Chromium

Closes #628, closes #23

@comfyanonymous
Copy link
Owner

I think there should be an OK button beside the clear button in the "Pick Images" UI.

With this I can no longer queue prompts if there is an extra SaveImage in the workflow not connected to anything for example.

@space-nuko
Copy link
Contributor Author

I think there should be an OK button beside the clear button in the "Pick Images" UI.

Added

With this I can no longer queue prompts if there is an extra SaveImage in the workflow not connected to anything for example.

Isn't that how it's always been? The executor always tries to verify starting from output nodes even if they're not attached to the "main" graph right? (It still works if the detached node isn't an output node)

@space-nuko space-nuko force-pushed the load-image-batch branch 2 times, most recently from 3fc7875 to 75dcbce Compare June 2, 2023 15:44
@comfyanonymous
Copy link
Owner

I have 30000 images in my output folder so this is a bit slow, it also doesn't allow you to filter the list like the current LoadImage node

The pick images should default to the Input folder and there should be a "Replace" and an "Add" instead of just an "Add" because I assume most people just want to pick a single image.

- Uses filter list behavior for LiteGraph context menu
- "Replace" button
- Can select from combo widget on node directly
@space-nuko
Copy link
Contributor Author

space-nuko commented Jun 9, 2023

Okay I made it use the list filtering behavior for normal combo boxes like with current LoadImage, so it shouldn't be any slower than that.

I also made the widget display on the node back into an interactable combo box like current LoadImage does. The only thing I couldn't preserve was the behavior when clicking the arrow buttons on the combo box which unfortunately needs litegraph changes to work

Also added the Replace button and it defaults to the input folder

@WASasquatch
Copy link
Contributor

Looks clunky and unrefined, but functionality is great.

@comfyanonymous
Copy link
Owner

Is it normal that the arrow buttons for the file combo are broken?

@WASasquatch
Copy link
Contributor

WASasquatch commented Jun 11, 2023

How is this working? I see no mode? This should output

  1. A image tensor batch
  2. A tensor image list
  3. A single tensor per prompt run

That way it can be used with all methods of image handling and diffusion.

@space-nuko
Copy link
Contributor Author

Is it normal that the arrow buttons for the file combo are broken?

With this implementation yes. The list of paths is sent back as an array (which other widgets/types haven't done before) but litegraph assumes that when picking an item with the left/right combo buttons the index of the selected value can be found in the choices array. Since the value is an array instead of a string, and list of choices are strings, it won't report the correct value

It needs a way to ignore the index if it isn't found, or better yet a way for widgets to override their behavior if a button is pressed, but both of those are litegraph patches

I see no mode?

I could add this but I can foresee it needing changes to the executor, currently I don't think you can configure a node to be different modes of OUTPUT_IS_LIST based on settings, instead you'd have to create a new node with it set to true or false

@WASasquatch
Copy link
Contributor

WASasquatch commented Jun 12, 2023

This is why, like I have mentioned a few times, all execution should double check the class.

1 get defaults
2 class function ran
3 double check class var settings and handle accordingly
4 return result from class function

We're using classes and can use them much more powerfully. When we envoke the class to run the function we have an opportunity to check the class attributes again for dynamic changes.

@M1kep
Copy link
Contributor

M1kep commented Jul 20, 2023

I think OUTPUT_IS_LIST supports values per output:

ComfyUI/execution.py

Lines 87 to 98 in 7ddfed0

if len(results) > 0:
# check which outputs need concatenating
output_is_list = [False] * len(results[0])
if hasattr(obj, "OUTPUT_IS_LIST"):
output_is_list = obj.OUTPUT_IS_LIST
# merge node execution results
for i, is_list in zip(range(len(results[0])), output_is_list):
if is_list:
output.append([x for o in results for x in o[i]])
else:
output.append([o[i] for o in results])

Whereas the INPUT_IS_LIST applies to all inputs.

While less dynamic, it should be doable with an output for each "mode"

@WASasquatch
Copy link
Contributor

Can it be dynamically set or is it static? I've tried changing like return types and stuff and didn't take.

@joaodafonseca
Copy link

where can i find this node?

@WASasquatch
Copy link
Contributor

where can i find this node?

This is pretty old code and not sure it's compatible anymore. The fork its in is pretty ancient

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.

Support loading a batch of multiple images with LoadImage [Feature Request] Batch image processing
6 participants