search

ltdrdata / ComfyUI-Impact-Pack

Custom nodes pack for ComfyUI This custom node helps to conveniently enhance images through Detector, Detailer, Upscaler, Pipe, and more.
GNU General Public License v3.0
1.9k stars 186 forks source link

Update core.py - save mask when edited #817

Open LuminousPath opened 2 days ago

LuminousPath commented 2 days ago

When using the restore_mask functionality, it only works if the image already had a mask attached, as "Open in MaskEditor" doesn't save the mask to the cache set specifically for masks, so when the next generation fires, if the image is different, the cache misses and the mask doesn't get restored.

In order to facilitate the restore_mask functionality better, one needs to save the mask to the cache when the mask is edited.

ltdrdata commented 2 days ago

Do you want the alpha to be treated as a mask?

LuminousPath commented 2 days ago

The image that gets saved with "open in mask editor" uses the alpha for the mask (as far as I know), I just assumed the functionality that's in:

https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/24de5a846bb9b1c1830af0218f76266c375cc904/modules/impact/bridge_nodes.py#L114-L116

could work similarly when those same objects are set in set_previewbridge_image

ltdrdata commented 2 days ago

I was mistaken about the code.

Your explanation of the PR is a bit confusing.

In the existing implementation, if the image resolution doesn't change, the previously stored mask remains intact even if the source image changes.

Are you saying this functionality isn't working?

LuminousPath commented 2 days ago

Right, this isn't working in the case of:

  1. gen an image and have it pipe into the preview bridge
  2. edit the mask on the image
  3. run the gen again and the image slightly changes

The issue I found was that the mask isn't saved into the preview_bridge_last_mask_cache or preview_bridge_cache objects when you edit the mask using comfy's mask editor. It does get saved into preview_bridge_image_id_map and preview_bridge_image_name_map but there's no functionality in doit that tries to see if a previous mask exists from these objects.

The code only tests preview_bridge_last_mask_cache:

https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/24de5a846bb9b1c1830af0218f76266c375cc904/modules/impact/bridge_nodes.py#L93-L94

and preview_bridge_cache:

https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/24de5a846bb9b1c1830af0218f76266c375cc904/modules/impact/bridge_nodes.py#L82-L83

I'm not sure of the intended functionality, but because neither of these things are set when the mask editor returns, the mask doesn't exist as far as doit is concerned.

ltdrdata commented 2 days ago

I don't see any issues on my side. Could you show exactly what situation you're referring to by recording a video?

reproduce.webm

This code contains the logic to restore the previous image when the input image changes. (The restoration condition is met if a previously saved mask exists and the restore option is enabled.)

https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/24de5a846bb9b1c1830af0218f76266c375cc904/modules/impact/bridge_nodes.py#L93-L94

This is where the mask is actually cached. If it is not an empty mask, it will always be cached. https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/24de5a846bb9b1c1830af0218f76266c375cc904/modules/impact/bridge_nodes.py#L132

LuminousPath commented 2 days ago

Sure: (sorry for the long/slow video, my computer isn't that powerful so recording and genning at the same time takes some time)

This is the mask effect without my change: showing_preview_bridge_issue.webm

and this is the effect after my change: after_change.webm

LuminousPath commented 2 days ago

This is where the mask is actually cached. If it is not an empty mask, it will always be cached.

This code only fires when the node actually runs during processing, and is not cached when the mask is actually edited.

I'm not really sure if this is a bug or if it's outside the intended functionality

ltdrdata commented 1 day ago

Oh, so you want to include in the restore even the masks that have never run a node.

ltdrdata commented 1 day ago

When I applied your PR, I encountered this issue. Please consider this aspect as well. This only occurs if if_same_size mode.

https://github.com/user-attachments/assets/51eb800c-cca7-4bb7-8ff9-6d6928570779

Additionally, the patch also affects PreviewBridge (Latent), so please check if it has any impact on that as well.

LuminousPath commented 1 day ago

@ltdrdata I believe the change should fix the problem with the size comparison (I forgot to put unsqueeze so the returned dimensionality was too low and the comparison broke)

I also tested it with the latent preview and didn't run into any issues.

Let me know if there's anything else you want me to change!