AlekPet / ComfyUI_Custom_Nodes_AlekPet

Custom nodes that extend the capabilities of Comfyui
MIT License
887 stars 59 forks source link

PainterNode: no disk data being stored, any Undo wipes the canvas #84

Open dryodryo opened 2 months ago

dryodryo commented 2 months ago

Hello,

Thank you for the PainterNode, it seems that it may be immensely helpful for controlling composition.

I am having an issue where if I apply the CTRL+Z command to Undo, the PainterNode resets the canvas size to 512x512 and erases the drawing. It does not matter how many steps back the canvas size was changed. The Undo command wipes out the drawing even if the canvas size was changed many steps earlier.

Without any documentation, I do not understand what is going on with image files in general. I see the "Save canvas" option in Settings. I see that the "Image" parameter field reads "Paint_26.png". I see that a black 512x512 file with that name exists in my ComfyUI/input folder. But I am not able to select any other file from the "Image" field, even though all of the images in the ComfyUI/input folder are listed there.

If I save the workflow to .JSON and reload it, the PainterNode is reset to the default 512x512 and the canvas is black.

How do I get the PainterNode to actually store and load from disk?

How can I use the global Undo command without wiping out the drawing?

If these issues persist, then the PainterNode is of no use to me whatsoever. I might as well just draw in some other application.

OS is Windows 10, browser is Firefox. Ad blockers are disabled.

Please help. Thank you again.

AlekPet commented 2 months ago

Hi, about ctrl+z, I don't have such key combinations, only buttons on the canvas, maybe these are global settings, I need to look. The Save canvas setting was there before and I forgot to delete it, it was used to select for each node separately where to save the canvas, either in the browser's LocalStorage or in a json file, because large images cannot be saved in the browser's localstorage, it is limited and if the quota is increased, the canvas is not saved. The thing is that localstorage and json are separate storages and if you switch from one to another, you need to make at least one change on the canvas, there is also a storage manager in the settings, where you can see what is saved. Now in the settings it is global for all nodes. The field with the name Paint_26.png is the name of the saved image, and it does not change, I could make it changeable, but then I would have to add an image from the selected list to the canvas, similar to inserting an image as a background or as just an image (P or IMG button, or drag and drop).

dryodryo commented 2 months ago

Thank you for your reply.

CTRL+Z is the global Undo for all of ComfyUI. Performing an Undo in ComfyUI deletes all PainterNode data and resets the canvas resolution to 512x512. Unless this bug is fixed, I can't use the PainterNode. Global Undo is an absolutely necessary functionality.

I looked in my browser's local storage and was not able to find any .PNG or .JSON files in there. But there is a data.sqlite file, I guess that is the PainterNode image, formatted for SQL?

I did not see any Storage Manager in the node settings for PainterNode itself. Finally found it in the alekpet settings within ComfyUI settings. The only thing listed there is the Paint_26.png, again.

So I am still trying to figure out how your storage system works. It appears that Paint.26.png is a temporary file that is overwritten? The actual pixel data is managed by browser storage, in a format that users can't use?

Why did you choose to do this? Is there some permissions issue with storing bitmap files in the ComfyUI/input folder? Because many other nodes have no problem with this, including the factory default Load Image node.

With the current setup, I am not sure how to proceed to ensure that a workflow that includes a PainterNode image is portable. This is a critical need for me. I come from a production background and anything I do needs to be sharable, future proofed, and able to be backed up and restored. I need to be able to load the workflow, including the PainterNode image, at some point in the future, on some other workstation.

If the PainterNode just created ordinary PNG files, and permanently stored them in ComfyUI/input as usual, then I could easily back up that PNG file with my project.

So, how can I ensure that the data that is created by the PainterNode is accessible at some point in the future? Because right now it is not persistent. If I use CTRL+Z, the data is erased. If I use a different browser, the data is not accessible. I can't back up the data, or share it, or edit it.

Thank you

AlekPet commented 2 months ago

Hello! As for ctrl+z, it is more likely used to return changes in nodes (adding, deleting, editing) and it is logical that it resets the Panther node, since it most likely recreates it if you roll back and return, and you would like it to remain the same ?, I'll have to look, the point is that I didn't use these keys. Everything is much easier if the data is stored in Local Storage (you can find it in the browser chrome (firefox about the same)> ctrl+shift+i > the inspector opens and there is the Application tab > Local storage > your comfyui ip > the list saved in localstorage for this site (domain)), there you will find Paint_26.png. And if in the settings (which are in Comfyui settings), the use JSON check box is selected, then all saves are written to a json file (path /custom_nodes/ComfyUI_Custom_Nodes_AlekPet/PainterNode/settings_nodes) named Paint#.png.json. All data is saved in json format so that it can be changed and moved in the middle. But the file after modify canvas is always saved in the input folder and has the name Paint#.png, which is constantly overwritten this is convenient for infinite generation and drawing.

AlekPet commented 2 months ago

Plus to the previous message, changed the behavior with ctrl+z, ctrl+y 😃 , install update

dryodryo commented 2 months ago

Thank you for fixing the bug with the global Undo! This is a great step forward, and makes the node usable.

I am still having difficulty trying to understand how to ensure that a workflow will open with the correct data.

There is probably a language barrier here which is making everything more difficult, but please bear with me. I have had to perform a great deal of detective work in an attempt to establish what is going on with the PainterNode.

It appears that the PainterNode is actually a vector art tool. You are storing spline and shape primitives, then rasterizing them to a bitmap file to produce the pixel data required for ComfyUI. All of this allows non-destructive editing. Is this correct?

It appears that there are two schemes for storing the vector data. Either in the browser local storage, or in the custom node directory called "PainterNode/settings_nodes". But this is problematic naming, because it is not the SETTINGS that are being stored in those .JSON files. The actual VECTOR DATA is what is being stored there.

Then it appears that the rasterized bitmap data is being stored as a PNG file in ComfyUI/input.

The node badge ID is used to track all of this data.

But this badge ID scheme is problematic, because similar workflows will access the same vector and bitmap data. For example, I always create numbered versions of everything I create. MyWorkflow_01.json, MyWorkflow_02.json, etc. Likewise, I have many workflows stored as starting points for developing specific projects. If these different workflow files happen to have the same PainterNode badge IDs, then they will all reference the same vector and bitmap data. And they will overwrite one another.

For example, I open MyWorkflow_01.json. It has a PainterNode with badge ID #26. I make a drawing and it is stored in both the vector and bitmap data. So far, so good. Then, I make a change to PainterNode #26. This overwrites the existing vector and bitmap data. I save the workflow as MyWorkflow_02.json. Then I decide I want to revert the changes. I open up MyWorkflow_01.json. Because it references PainterNode badge ID #26, it loads the appropriately numbered vector and bitmap data. But the version of the drawing that I created in MyWorkflow_01.json does not exist anymore, it has been overwritten.

This is a major, very serious issue with asset management. I can partly work around it if I watch everything like a hawk and back up all of the PainterNode .json and .png files manually every time I make a change. But the facts that drawings are tied to badge IDs, and that badge IDs are not unique across workflow files, will only result in big problems, too many to list here.

I am not a developer, but I have two ideas to solve these issues.

  1. Just embed the vector data directly in the workflow .json document. This makes the most sense for asset management, because all data is encapsulated within a single file. There are no external dependencies. Badge IDs can still be used without conflicts, because the system is only referencing data within the current workflow document. The rasterized bitmaps stored in the ComfyUI/inputs folder can have filenames derived by concatenating the workflow filename with the badge ID.

  2. Track vector and bitmap data with UUID or similar, unique values. This is less intuitive, and still requires users to manually manage vector and/or bitmap files. But it's an option in case there is some technical reason why the vector data cannot be embedded directly in the workflow.

Thank you again for your hard work and creative genius in producing this highly valuable tool. If you have a Patreon or other method for users to contribute financially, please let me know.

AlekPet commented 2 months ago

Hi. Yes, everything is correct, canvas data is stored in vectors, as well as some other settings, except for images, they are in base64. Regarding names and rewriting, you can try adding the name of the workflow to the name (indicator). There may be one problem, when saving the workflow, the nodes maybe are not re-created, but the workflow file is simply saved, need to look at how to change the name value.

dryodryo commented 2 months ago

Thanks, can you explain what you mean by "adding the name of the workflow to the name (indicator)"?

Are you recommending that I edit your python code?

AlekPet commented 2 months ago

Thanks, can you explain what you mean by "adding the name of the workflow to the name (indicator)"?

Are you recommending that I edit your python code?

This means changing the name under which it is saved paint_#.png. It is assigned when creating a node in the onCreated method and I hope it is possible to get the name of the workflow via the API, and not get it from the elements. Okay, I will look at how best to do it, because when saving a new workflow, you need to take into account other PainterNodes and they also need to make changes to save in json, localstorage and the .png file itself. And change the structure of json and its reading. In python, there are small edits, mostly on the frontend.

dryodryo commented 2 months ago

Thank you, I do not fully understand what you are telling me, but it sounds like you are trying to address the naming conflicts caused by the current code.

Much appreciated!