Conversation
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.
christian-byrne
requested review from
Kosinkadink,
comfyanonymous,
ltdrdata,
pythongosssss,
robinjhuang,
webfiltered and
yoland68
as code owners
webfiltered
left a comment
webfiltered
left a comment
There was a problem hiding this comment.
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.
|
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. |
100% it should. |
+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
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." |
|
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. |
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".
mine provides the results as vectors (you dont need that) but you should put the tensor batch size. |
|
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. |
|
hm, I see your point. You might be right. I think of it too much from programming perspective. |
* [feat] Add GetImageSize node to return image dimensions 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. * add display name mapping * [fix] Add server module mock to unit tests for PromptServer import 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.
5 participants
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.