8000 [feat] Add GetImageSize node by christian-byrne · Pull Request #8386 · comfyanonymous/ComfyUI · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[feat] Add GetImageSize node #8386

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

Merged
merged 3 commits into from
Jun 3, 2025
Merged

[feat] Add GetImageSize node #8386

merged 3 commits into from
Jun 3, 2025

Conversation

christian-byrne
Copy link
Collaborator
@christian-byrne christian-byrne commented Jun 2, 2025

Simple node that returns width and height dimensions of input images. Displays dimensions on the node UI and provides width/height outputs for further processing.

Selection_1462

Added a simple GetImageSize node in comfy_extras/nodes_images.py that returns width and height of input images. The node displays dimensions on the UI via PromptServer and provides width/height as outputs for further processing.
Copy link
Collaborator
@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this node be expanded to handle masks / others?

Updated test to mock server module preventing import errors from the new PromptServer usage in GetImageSize node. Uses direct import pattern consistent with rest of codebase.
@Amorano
Copy link
Contributor
Amorano commented Jun 2, 2025

You should add depth as well. 1 chan? 3 chan? 4 chan? why? why not? You should honestly also add bit-depth, since I assume the intention is to allow comfy to go beyond only sRGB 8-bit images someday.

@Amorano
Copy link
Contributor
Amorano commented Jun 2, 2025

Should this node be expanded to handle masks / others?

100% it should.

@huchenlei
Copy link
Contributor

Should this node be expanded to handle masks / others?

+1 for general tensor size check

@christian-byrne
Copy link
Collaborator Author

+1 for general tensor size check

What does that even mean? There are a lot of Comfy types that are tensors and there is not a generalized concept of "size" for them, from perspective of user

Should this node be expanded to handle masks / others?

You should add depth as well. 1 chan? 3 chan? 4 chan? why? why not? You should honestly also add bit-depth, since I assume the intention is to allow comfy to go beyond only sRGB 8-bit images someday.

I think coercion should be happening at the graph level. If each node has overloaded input types it requires repeating coercion code at every node and losing out on optimizations. As a visual programming language, the workflow author can coerce the types manually using dedicted conversion nodes like "Mask to Image."

< 8000 div data-view-component="true" class="comment-reactions js-reactions-container js-reaction-buttons-container social-reactions reactions-container d-none">

@comfyanonymous comfyanonymous merged commit 8564480 into master Jun 3, 2025
6 checks passed
@comfyanonymous comfyanonymous deleted the get-size-node branch June 3, 2025 01:57
@christian-byrne
Copy link
Collaborator Author
christian-byrne commented Jun 3, 2025

Note: You can add outputs and change display names of node without breaking workflows, so we can easily change the node name to "Get Image Info" or "Get Image Components" later on and add more outputs like pixel depth, color space, channels, and so on.

@Amorano
Copy link
Contributor
Amorano commented Jun 3, 2025

+1 for general tensor size check

What does that even mean? There are a lot of Comfy types that are tensors and there is not a generalized concept of "size" for them, from perspective of user

Should this node be expanded to handle masks / others?
You should add depth as well. 1 chan? 3 chan? 4 chan? why? why not? You should honestly also add bit-depth, since I assume the intention is to allow comfy to go beyond only sRGB 8-bit images someday.

I think coercion should be happening at the graph level. If each node has overloaded input types it requires repeating coercion code at every node and losing out on optimizations. As a visual programming language, the workflow author can coerce the types manually using dedicted conversion nodes like "Mask to Image."

The batch size. All tensors are B, H, W (even masks). Masks just dont have C.

B, H, W, C for all others.

Has nothing to do with "coercion".

image

mine provides the results as vectors (you dont need that) but you should put the tensor batch size.

@christian-byrne
Copy link
Collaborator Author

My point is that the types should be converted explicitly at the workflow level. Making node that has singular functionality work on multiple different types requires coercion or some other type of ad hoc polymorphism, which is inefficient, opaque, and more error prone (in my opinion).

@Amorano
Copy link
Contributor
Amorano commented Jun 3, 2025

My point is that the types should be converted explicitly at the workflow level. Making node that has singular functionality work on multiple different types requires coercion or some other type of ad hoc polymorphism, which is inefficient, opaque, and more error prone (in my opinion).

I guess you are talking about the input? image vs mask?

Masks are just images, same same. Why there is a distinction, someone just made up, I will wonder until it changes.

Fundamentally, there is no "coercion". IMAGE, MASK at the input (your own system) allows for it, and both are the same "type" i.e. Tensors.

I can only tell you, as a VFX professional, what makes more sense in the way we use thing, but if you really want to make an "IMAGE INFO" and "MASK INFO" node as two separate things, then I obviously cant stop you from doing it.

I would then just request to accelerate the ability to excise nodes from my core list as none of these are useful and I don't want to have to search a now growing list of nodes that have similar names to the thing I need to use, but don't do the thing I need.

With 3rd party packs that have 100+ nodes, I can just skip downloading them. Here I am given no choice.

@christian-byrne
Copy link
Collaborator Author

hm, I see your point. You might be right. I think of it too much from programming perspective.

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
0