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

[Bug]: setSizeForImage should be called when image get attached? #2417

Open
XZiar opened this issue Feb 4, 2025 · 5 comments
Open

[Bug]: setSizeForImage should be called when image get attached? #2417

XZiar opened this issue Feb 4, 2025 · 5 comments
Labels
bug Something isn't working

Comments

@XZiar
Copy link

XZiar commented Feb 4, 2025

Frontend Version

v1.7.14

Expected Behavior

IMAGEUPLOAD get resized correctly when extension add more widget.

Actual Behavior

IMAGEUPLOAD not expanded for image.

Steps to Reproduce

extension's onNodeCreated add custom widget after original onNodeCreated.
after node setup, image get default selected but image element not shown in node.

Debug Logs

***

Browser Logs

console.trace()

VM9431:1 console.trace
eval @ VM9431:1
node3.setSizeForImage @ index-QvfM__ze.js:193607
showImage @ index-QvfM__ze.js:185554
(anonymous) @ index-QvfM__ze.js:185589
requestAnimationFrame
IMAGEUPLOAD @ index-QvfM__ze.js:185587
ComfyNode @ index-QvfM__ze.js:193341
createNode @ index-QvfM__ze.js:65296
callback @ index-QvfM__ze.js:57477
inner_onclick @ index-QvfM__ze.js:64655
undefined

this.imgs

undefined

getImageTop(this)

NaN

Image

Setting JSON


What browsers do you use to access the UI ?

Google Chrome

Other

function showImage(name) {
const img = new Image()
img.onload = () => {
node.imgs = [img]
app.graph.setDirtyCanvas(true)
}
let folder_separator = name.lastIndexOf('/')
let subfolder = ''
if (folder_separator > -1) {
subfolder = name.substring(0, folder_separator)
name = name.substring(folder_separator + 1)
}
img.src = api.apiURL(
`/view?filename=${encodeURIComponent(name)}&type=input&subfolder=${subfolder}${app.getPreviewFormatParam()}${app.getRandParam()}`
)
node.setSizeForImage?.()

In IMAGEUPLOAD, it's calling setSizeForImage at the end, but img element is attached to node.imgs in onload callback.
When node.imgs is empty(undefined), getImageTop returns NaN and actually size not get updated.

const minHeight = getImageTop(this) + 220
if (this.size[1] < minHeight) {
this.setSize([this.size[0], minHeight])
}

so setSizeForImage should be called in onload callback.

┆Issue is synchronized with this Notion page by Unito

@XZiar XZiar added the Potential Bug Untriaged bug label Feb 4, 2025
@christian-byrne christian-byrne added bug Something isn't working and removed Potential Bug Untriaged bug labels Feb 4, 2025
@christian-byrne
Copy link
Collaborator

christian-byrne commented Feb 4, 2025

Reproduction Steps:

  1. Register extension that adds a new widget to a node with an existing IMAGEUPLOAD widget:

    import { app } from "../../scripts/app.js";
    app.registerExtension({
      name: "extension name",
      async beforeRegisterNodeDef(nodeType, nodeData, app) {
        // Patch a node that has an IMAGEUPLOAD widget
        if (nodeData.name == "LoadImage") {
          const originalOnNodeCreated = nodeType.prototype.onNodeCreated;
          nodeType.prototype.onNodeCreated = function () {
            // Call original onNodeCreated
            originalOnNodeCreated.apply(this, arguments);
    
            // Add any type of DOM widget, including multiline text widget
            const divEl = document.createElement("div");
            divEl.style.height = "100px";
            divEl.style.width = "100px";
            this.addDOMWidget("dom widget", "dom", divEl, {});
          };
        }
      },
    });
  2. Add the node to the graph using the node search bar

  3. Observe that the image preview (added by IMAGEUPLOAD widget) is not visible on initial load
    Image

  4. The image preview doesn't resize/appear until after the widget's value is changed

@christian-byrne
Copy link
Collaborator

so setSizeForImage should be called in onload callback.

This doesn't seem to change things. Can you elaborate?

@XZiar
Copy link
Author

XZiar commented Feb 5, 2025

Sorry, I haven't tried this "solution", chrome devtool does not allow me to hot-reload this change.
I just think setSizeForImage should happen after the img get attached, maybe there's other reasons causing the size not being correctly updated...

In my case I am adding a multiline STRING widget, the textarea element takes the whole remaining space and image will not show unless I manually resize the node.
(Finally, I manually hook the draw function to re-calculate the height to workaround this...)

@webfiltered
Copy link
Contributor

The multiline string input and the image display function are unforunately not designed to work together like that. This could definitely be added as a feature request, though.

@christian-byrne
Copy link
Collaborator

I see, it's an issue between IMAGEUPLOAD widget's preview image and DOM widgets. I've updated the reproduction steps to properly reproduce the bug now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants