[Feature] Enhanced MaskEditor to an Image Canvas #4361
Conversation
… Still need to figure out how to load the painted pixels onto RGB Canvas when the MaskEditor is re-opened after painting and saving
… them as part of the image. Maybe a combinedCanvas?
|
@christian-byrne @webfiltered Tagging you in case you have a chance to take a look. |
brucew4yn3rp
changed the title
brucew4yn3rp
changed the title
trsommer
left a comment
trsommer
left a comment
There was a problem hiding this comment.
I just tried it. There seems to be a problem where the resulting layers are still split into 2 images in the load image preview. I tried it in both chrome and firefox and just painting on the canvas with the paint tool results in 2 images in the load image node
after reloading, it seems to be combined:
If you run the workflow, the correct image seems to be handed over. The core process seems to work, just the previews in the load image node are wrong
UI fixes, not that important:
when the user selects the mask or paint tool, auto select the mask or image layer. It does not make sense for the user paint/mask without a selected layer
|
Thanks @trsommer. I think with the current infrastructure across the frontend, for the user to be able to edit the painting after closing out and re-opening the interface, the node has to have the images separate. @duckcomfy Changing the rendering of the LoadImage node doesn't seem to work well. Let me know if you have any other ideas. |
|
Thank you for your quick review @trsommer. |
|
@duckcomfy As it stands, in app.ts, we are pushing RGBCanvas and CombinedCanvas to node.imgs. This is important because (if I remember correctly) when the MaskEditor is initialized, it accesses its images and adds the relevant painted pixels to the paint layer. That functionality is, as far as I can see with the current infrastructure, the only way to allow for continued editing. We cannot leverage the mask’s logic because it would not work across all RGB colors. It is interesting that upon refresh of the page, the node defaults to the combined, flattened image. My guess is that it is displaying the last saved image in the input/clipspace folder. I’m happy to discuss on Discord for easier communication if you’d like (@batman.76) |
|
I gave this a quick try as well. Seems to work well enough and definitely solves a pain point with ComfyUI. I did find a bug: the brush shape seems to be stuck to circular. If I select the rectangular brush, it'll show a rectangular outline but still make circular marks. |
Also comes with a refactor of the associated code to be more DRY and maintainable. In order to explain the new UX let me first explain what I'll call the different tools, as the names we previously used are now ambiguous. - "Mask Pen": Previously named "Pen" internally, this is an existing tool lets you paint a mask on the image. - "Paint Pen": New tool which lets you paint colored pixels on the image. - "Eraser": Exising tool which lets you erase parts of your mask. As of @brucew4yn3rp's PR, you can also erase parts of your painted pixels, depending on which layer is active. - "Mask Bucket": Previously named "Paint Bucket" internally, this is an existing tool which lets you take a mask that's just an outline, and fill in the inside of your mask outline with a full masked area. Equivalent functionality for the Paint layer is NOT implemented. - "Mask Color Fill": Previously named "Color Select" internally, this is an existing tool which can turn pixels into a masked area, based on color similarity in contiguous pixels on the base image. Equivalent functionality for the Paint layer is NOT implemented. New UX: - The "Mask" layer is now selected when you open the Mask Editor, instead of no layer selected. - Selecting the "Mask Pen", "Mask Bucket", or "Mask Color Fill" tool automatically selects the Mask layer. - Selecting the "Paint Pen" tool automatically selects the Paint layer. - Selecting the "Mask" layer while the "Paint Pen" tool is selected automatically selects the "Mask Pen" tool. - Selecting the "Paint" layer while the "Mask Pen", "Mask Bucket", or "Mask Color Fill" tool is selected automatically selects the "Paint Pen" tool. - Changing the selected layer while the "Eraser" tool is selected has no additional effect, because Eraser supports both layers. - Switching to the "Eraser" tool has no additional effect, because Eraser supports both layers.
…iewed images)" This reverts commit c6eb0ba.
…e their url from the main image url, which also ensures the layers can be opened even if the page is refreshed
refactor: address review items on main Image Editor PR
fix: add back change to ensure node preview is only the combined image
|
Hi @webfiltered, thanks for the attentiveness on this - I know there's lots going on. All of your comments have been addressed in the latest commits. We discussed offline a way to circumvent the need to store images within the node. Therefore, we no longer need the Originally, I had implemented the storing of images in Now, the timestamp has been unified across all the related files, allowing us to derive the names of each layer's saved file from the final filename that gets saved to the node/workflow. As a byproduct, this allows editability to persist through refreshes because deriving the filenames is static (as opposed to This seems to work well, and without the additional clog in retaining images within the node, which you mentioned may be a strain for future maintainability. To that end, @duckcomfy has also added a |
|
@brucew4yn3rp once your PR gets merged, would you please write into me at bounty@comfy.org? i'd love to compensate you as a thank you for your significant effort here! |
|
Hi @trsommer — we’d like to propose adding @brucew4yn3rp as a CODEOWNER for the MaskEditor component, based on:
@christian-byrne mentioned you're currently listed as the CODEOWNER here, so I wanted to check with you. Would you be open to adding @brucew4yn3rp alongside you? Thanks in advance for considering! |
|
I found a bug I think may be related to this implementation #5251. The mask editor is now anti-aliasing even at max hardness, making fills not fill completely and cleanly. |
|
@brucew4yn3rp You did not notice how this completely broke the right side tool configuration menu for now 2 months? It is supposed to change when selecting the color select tool with the dropper icon to the left. Code is still there, just that this was so sloppily implemented and completely lacked quality control and even the mere basic manual testing. |
I have fixed paint bucket and color select tool settings not showing up in #5733 , I think that was the issue you were talking about. I have no idea why @brucew4yn3rp removed this. I did not notice because I only tested the new features implemented |
|
@trsommer |
|
Thank you @trsommer for the quick fix. That was indeed an oversight during the UI redesign. The PR went through multiple iterations covering both new functionalities and the UI (including the sidebar), so given the scope it’s unfortunate but not surprising that this slipped through during the rebuild. Again, given the scope and difficulty of implementing the new functions, I was also admittedly focused on making sure the new features worked well and missed the regression which probably happened early on.
@silveroxides, instead of assuming “sloppy implementation” or lack of testing, it would be more constructive to check the PR history, which clearly shows extensive review and collaboration over several months. In a refactor of this size, oversights are inevitable, which is why collaboration exists. If you run into issues in the future, the most helpful approach is to either open an issue or submit a PR. And if you remain convinced this reflects “sloppy” work, you’re always free to continue using the old mask editor. |
10 participants
@trsommer
Revamped the MaskEditor UI and changed it to an Image Canvas. The context menu name has also been renamed accordingly.
Cleaner PR of 2921
The interface now supports drawing on the image itself (on the image layer) in addition to covering the area with a mask. Users can select a color or use an eyedropper tool for color matching. This is a powerful tool to guide the image generation process post-masking (e.g., inpainting). Speaking with ComfyUI users for months, this has been highly desired addition.
The tools on the left sidebar have been updated accordingly. The top one, is the existing mask brush, and has a matching mask logo. The second one is the image painting brush.
Some relevant context from the old PR
Layer buttons have been implemented. Also, right click will only apply to erasing whichever tool is activated. Additionally, the eraser tool will only erase on the active layer. Selecting the mask tool automatically activates the mask layer, and selecting the Paint tool automatically activates the Paint layer.
Thank you very much for your help in getting this over the line @duckcomfy. I had some issues with the prior PR so decided to close it and initiate a new, cleaner PR.