Skip to content

[Feature] Enhanced MaskEditor to an Image Canvas #4361

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

Open
wants to merge 54 commits into
base: main
Choose a base branch
from

Conversation

brucew4yn3rp
Copy link

@brucew4yn3rp brucew4yn3rp commented Jul 6, 2025

@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.

image

image

Some relevant context from the old PR

I need a way to only erase the drawing or the mask part. The easiest solution would be to make the layer clickable to select which layer to work on.

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.

@brucew4yn3rp brucew4yn3rp marked this pull request as ready for review July 6, 2025 00:54
@brucew4yn3rp brucew4yn3rp requested review from trsommer and a team as code owners July 6, 2025 00:54
@brucew4yn3rp
Copy link
Author

@christian-byrne @webfiltered Tagging you in case you have a chance to take a look.

@brucew4yn3rp brucew4yn3rp changed the title Changed MaskEditor to an Image Canvas [Feature] Changed MaskEditor to an Image Canvas Jul 7, 2025
@brucew4yn3rp brucew4yn3rp changed the title [Feature] Changed MaskEditor to an Image Canvas [Feature] Enhanced MaskEditor to an Image Canvas Jul 7, 2025
Copy link
Collaborator

@trsommer trsommer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
image
image

after reloading, it seems to be combined:
image

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

@brucew4yn3rp
Copy link
Author

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.

@duckcomfy
Copy link
Contributor

duckcomfy commented Jul 7, 2025

Thank you for your quick review @trsommer.
@brucew4yn3rp Please note, if it helps understanding the issue at hand, that when refreshing the page, although the correct preview is displayed, the paint layer is no longer editable.
I'm going to have a look and try to understand how the Load Image node knows what to preview, how the Mask Editor loads the different layers, and why the Load Image node contents are different when refreshing the page. If you have any insight to share on these topics please do.

@brucew4yn3rp
Copy link
Author

@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)

@asagi4
Copy link

asagi4 commented Jul 7, 2025

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.

brucew4yn3rp and others added 3 commits July 8, 2025 07:04
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.
brucew4yn3rp and others added 5 commits July 12, 2025 08:57
… 'Activate Layer' buttons are only visible when the Eraser tool is selected, for which they are used to discern which layer is being erased. Otherwise, the buttons are hidden and a blue border reflects which layer is currently active.
…ts & improve the padding consistency when that happens
fix: allow maskeditor right-pane to have a scrollbar on small viewports & improve the padding consistency when that happens
@brucew4yn3rp brucew4yn3rp requested a review from webfiltered July 12, 2025 14:05
@brucew4yn3rp
Copy link
Author

brucew4yn3rp commented Jul 12, 2025

Thanks, @webfiltered and @PabloWiedemann. The vue-tsc version, reversions requested, and nits have been fixed.

In terms of UX, please see a more comprehensive description of changes below. To preface, it is important to have the Activate Layer buttons specifically for the eraser tool. As you mentioned, it automatically updates the layer based on the selected tool, but when using the eraser tool, the user must be able to discern which layer they are erasing. For convenience, there is the added functionality of right clicking while using either paint or mask to erase on that layer.

For better coherence of use, the Mask Blending options have been moved to their own container, similar to the Mask Opacity slider.

Layer Interface Reorganization:

The structure of the different layer containers is now standardized. There is a checkbox to hide/show the associated pixels, and an activation button for each of the mask and paint layers.

  • Moved "Activate Mask Layer" button into the Mask Layer container alongside the mask icon
  • Created separate "Mask Blending Options" container with centered dropdown (black, white, negative options)
  • Reorganized layer order: Mask Layer → Mask Blending Options → Mask Opacity → Paint Layer → Base Image Layer
  • Renamed "Image Layer" to "Base Image Layer" for clarity

Activate Button Behavior:

  • Changed activate buttons from hiding/showing to greying out (50% opacity) and disabling when that layer is active
  • Buttons now are only visible when Eraser tool is selected and remain visible but disabled for the currently active layer

Active Layer Visual Feedback:

  • Added blue border (2px solid #007acc) around the currently active layer container
  • Implemented updateActiveLayerHighlight() method to manage border highlighting

Scrollbar

  • Given the enhancements and additions made to the side panel, we've implemented a scrollbar for resolutions that aren't tall enough for the entire menu
ComfyUI.Image.Canvas.mp4

Copy link
Contributor

@webfiltered webfiltered left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good! Have some follow-ups but nothing major. The biggest is a comment on app.ts about node.imgs.

Have also added suggestions which you can just click commit on if that works for you. If any tests fail because of them, feel free to ping and I can resolve.

@brucew4yn3rp brucew4yn3rp requested a review from a team as a code owner July 13, 2025 13:45
@brucew4yn3rp brucew4yn3rp requested a review from webfiltered July 13, 2025 13:45
duckcomfy and others added 2 commits July 13, 2025 15:58
@brucew4yn3rp
Copy link
Author

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 data-nopreview workaround either.

Originally, I had implemented the storing of images in node.imgs as it was the only way I'd found to be able to persist which correct files to reference for each layer upon closing and re-opening of the canvas.

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 node.imgs which did not persist through app refresh),

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 maskeditor.test.ts file that will assist in ensuring the new functionality does not break in the future.

Copy link

@PabloWiedemann PabloWiedemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@comfy-pr-bot comfy-pr-bot requested a review from snomiao July 16, 2025 04:52
@kevinpaik1
Copy link

@brucew4yn3rp once your PR gets merged, would you please write into me at [email protected]? i'd love to compensate you as a thank you for your significant effort here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants