Skip to content
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

Canvas: Undo & Clear hotkey, brush scales by area, consistent max brush size #16668

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

w-e-w
Copy link
Collaborator

@w-e-w w-e-w commented Nov 19, 2024

Description

  • add Undo hotkey to canvas
    • default Z key
  • add Clear hotkey to canvas
    • default C key
2024-11-20.06_59_08_899.chrome.mp4

  • Brush size adjustment mode, Area and Radius (new commit removed Radius mode)
    • currently the brush size increase or decrease by 5% (hardcoded) of the maximum allowed Radius of the brush
      • I'm not sure how gradio determines the maximum brush size

      • this poses some usability issues especially when dealing with small brush sizes
    • to solve this issue I implemented a new scaling method based on the Area
      • with the Area method the brush size is increase or decrease by 10% (configurable) in Area allowing for finer adjustment

in my opinion Area mode it's much more usable than Radius mode
as such I have made the Area mode the Default changing old behavior
users who prefer the old method can configure it in settings

I have been testing with Area mode for a while locally and it feels quite natural

Radius

2024-11-20.07_00_34_898.chrome.mp4

Area

2024-11-20.07_01_34_712.chrome.mp4

notice that the Area mode brush size step is much more smoother then Radius


Brush size is set to 1/2 diagonal image, beyond the gradio default after the usere uses either the hotkeys or scroll wheel to change the brush size


Considering there's no timeline for Gradio 4 or 5
I think implement this relatively trivial function is worth it


future potential improvements / discussion

  • add hotkey for removeing the image?

    • what key should it use
  • in the PR only the factor / step for Area mode is configurable, while the 5% step in Radius model is hardcoded

  • should it be made configurable, if so should it share the same setting key as the factor used for Area mode

    • if so it's going to be hard to set a default value
    • or should it use a different setting key

Checklist:

@catboxanon
Copy link
Collaborator

I don't see reason to include a toggle between Radius and Area brush resizing. Area is clearly better, and software such as Photoshop functions in the same way.

@w-e-w
Copy link
Collaborator Author

w-e-w commented Nov 20, 2024

I don't see reason to include a toggle between Radius and Area brush resizing. Area is clearly better, and software such as Photoshop functions in the same way.

while I do agree scaling by Area is better, but I wouldn't say there's no point using linear radius
it's actually quite easy to find popular software that scales the brush size linearly
for example Gimp, but the step size is fixed to 1px

the the real issue with our scaling by linear Radius scaling is more due to the large steps size 5% of the maximum brush size
instead of 5% maybe it should like 1% or set to static value and it actually becomes quite usable
this is why I suggest that it should be made it configurable (just didn't explain it well)

if we really want to be advance we could also make it so that users can quickly toggle between both modes with a hotkey
or adjusting via keyboard hotkeys scales linearly, while adjusting by scroll wheel scales by area

remove the overly complex option of radius / area brush size change mode
@w-e-w w-e-w force-pushed the canvas-undo-hotkey,adjust-brush-size-by-area branch from 395580c to 1b9dea7 Compare November 22, 2024 02:30
@w-e-w
Copy link
Collaborator Author

w-e-w commented Nov 22, 2024

you're right adding a complexity of two modes is a bit needless

image
if uses wants linear for better control then they most likely want step size of 1 in which case they can just set the
brush size change rate 0, and since there is a minimum step size of 1 in the logic this is effectively that


I also improved on a behavior about maximum brush size
the default brush size is inconsistent and often too small

with the new commit after the user has adjusted the brush size with not hotkey for scroll wheel
the maximum brush size is set to 1/2 diagonal image, which behaves much more consistently
this won't impact users that don't use a scroll wheel for brush size

Inpaint Controlnet
Default max image image
new max image image

notice that the default max is different in different canvas
also that it is quite small especially in inpaint mode
the new max is consistent based on the actual size of the image and so it is consistent

@w-e-w w-e-w changed the title Canvas: Undo and Clear hotkey, adjust brush size by area Canvas: Undo and Clear hotkey, adjust brush size by area, consistent max brush size Nov 22, 2024
@w-e-w w-e-w changed the title Canvas: Undo and Clear hotkey, adjust brush size by area, consistent max brush size Canvas: Undo and Clear hotkey, brush scales by area, consistent max brush size Nov 22, 2024
@w-e-w w-e-w changed the title Canvas: Undo and Clear hotkey, brush scales by area, consistent max brush size Canvas: Undo & Clear hotkey, brush scales by area, consistent max brush size Nov 22, 2024
@w-e-w
Copy link
Collaborator Author

w-e-w commented Nov 22, 2024

I think I figured out how gradio decides the maximum brush size
it would seems like it's 75 pixels of the browser's viewport
and it's not affected to the actual image size canvas element size

so basically across each element the maximum plus size but appear "visually" to be the same
if the canvas element size is small then the effective maximum size would be larger
I guess this design Choice makes sense if you cannot zoom the canvas
since we allow zooming the reason no longer applies for us

in my current implementation the maximum breast size is only updated after user has adjusted to the brush size using the hotkey for scroll wheel
this means that users that only uses the slider for changing the brush size will not be impacted
for users that uses the hotkeys or screw wheel the slider is essentially irrelevant

@w-e-w
Copy link
Collaborator Author

w-e-w commented Nov 22, 2024

  • set min brush size to 1px

the minimum supported brush size is 1 pixel
but based on gradios logic sometimes the minimum size will be set to be greater then 1px

image
in this case 4px is the default minimum
after min of 1px is set you can draw a thinner line

not too really useful in context of masking, but since I'm already here why not allow it

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

Successfully merging this pull request may close these issues.

2 participants