liusida / ComfyUI-Login

A custom node that implements basic login for ComfyUI
MIT License
92 stars 12 forks source link

Image.open is self closing, del img removes the loaded object #6

Closed shawnington closed 6 months ago

shawnington commented 6 months ago

Per PIL documentation, the Image.open method is self closing when any operations are performed on the image. Calling Image.close() is only required if Image.open() was called and no operations are performed. https://pillow.readthedocs.io/en/stable/reference/Image.html#PIL.Image.Image.load

Replacing:

img.close() 

with:

del img

deletes the loaded image object

Also the later nodes don't work with the Image object, the image is covered to a tensor, and output_image is a tensor, not a copy of img that was loaded by PIL.

shawnington commented 6 months ago

https://github.com/bigcat88/pillow_heif

Look at how the Image.open function is hooked/overloaded in this code to include support for heic/heif formats, I think a similar approach can be used to change the behavior of the upload_image / image_upload functions

What I don't know is if the local path is maintained or if the path to the saved image is used for repeat generations. That could potentially create break something if it does rely on the image from the inputs folder instead of the local link.

liusida commented 6 months ago

https://github.com/bigcat88/pillow_heif

Look at how the Image.open function is hooked/overloaded in this code to include support for heic/heif formats, I think a similar approach can be used to change the behavior of the upload_image / image_upload functions

What I don't know is if the local path is maintained or if the path to the saved image is used for repeat generations. That could potentially create break something if it does rely on the image from the inputs folder instead of the local link.

There are two steps, upload and prompt. See https://github.com/liusida/ComfyUI-Login/issues/5#issuecomment-2111350481

shawnington commented 6 months ago

https://github.com/bigcat88/pillow_heif Look at how the Image.open function is hooked/overloaded in this code to include support for heic/heif formats, I think a similar approach can be used to change the behavior of the upload_image / image_upload functions What I don't know is if the local path is maintained or if the path to the saved image is used for repeat generations. That could potentially create break something if it does rely on the image from the inputs folder instead of the local link.

There are two steps, upload and prompt. See #5 (comment)

Yes, and I am suggesting those processes can be overloaded using the methods suggested. To clarify I think it's possible to have a function or method that overloads the base functions to prevent the default save behavior.

liusida commented 6 months ago

https://github.com/bigcat88/pillow_heif Look at how the Image.open function is hooked/overloaded in this code to include support for heic/heif formats, I think a similar approach can be used to change the behavior of the upload_image / image_upload functions What I don't know is if the local path is maintained or if the path to the saved image is used for repeat generations. That could potentially create break something if it does rely on the image from the inputs folder instead of the local link.

There are two steps, upload and prompt. See #5 (comment)

Yes, and I am suggesting those processes can be overloaded using the methods suggested. To clarify I think it's possible to have a function or method that overloads the base functions to prevent the default save behavior.

But the base ComfyUI is moving very fast. If we override it here, it might be deprecated very soon. How could this be solved?

shawnington commented 6 months ago

https://github.com/bigcat88/pillow_heif Look at how the Image.open function is hooked/overloaded in this code to include support for heic/heif formats, I think a similar approach can be used to change the behavior of the upload_image / image_upload functions What I don't know is if the local path is maintained or if the path to the saved image is used for repeat generations. That could potentially create break something if it does rely on the image from the inputs folder instead of the local link.

There are two steps, upload and prompt. See #5 (comment)

Yes, and I am suggesting those processes can be overloaded using the methods suggested. To clarify I think it's possible to have a function or method that overloads the base functions to prevent the default save behavior.

But the base ComfyUI is moving very fast. If we override it here, it might be deprecated very soon. How could this be solved?

Thats unfortunately the reality of wanting to modify core behavior of a library instead of simply extending it. If the goal is to prevent server side saving, the nodes and code base need to keep up with any changes that would break them. I guess thats true about writing extensions for any code base, instead of committing to the main repo.

But as I discussed, if the goal of the node is privacy, server side saving of images is not compatible with that goal.

I haven't done a full stack trace of the calls from when you select an image, to when it gets saved to the server, but I imagine there are several places to interrupt the process, or its probably possible to write your own javascript for the node to handle all of the image loading and bypass the server.py functions entirely.

My only experience with the javascript in the repo is tracing to which functions need changing for commits, Ive not written any for any of my own custom nodes, but Id recommend you look at the repo for ltdrdata's impact pack, he seems to have written javascript code for his own image loading.

He generally seems quite open to answering questions, and is very knowledgeable about the code base, he might be able to suggest a way to implement this.

https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/Main/js/impact-image-util.js