Page MenuHome

Brush Settings UI
Needs ReviewPublic

Authored by Demeter Dzadik (Mets) on Sat, Sep 28, 12:47 AM.
Tokens
"Love" token, awarded by charlie."100" token, awarded by Frozen_Death_Knight."Love" token, awarded by amonpaike."Like" token, awarded by Alumx."Love" token, awarded by MetinSeven."Love" token, awarded by HooglyBoogly."Like" token, awarded by ThatAsherGuy.

Details

Summary

This patch tries to clean up Brush UI panels. This affects almost every mode that uses brushes; Sculpt, Texture/Weight/Vertex/2D Paint, UV Sculpt, Grease Pencil Draw/Sculpt/Weight. There are no changes to Particle Edit.

Sidebar (All modes)

  • Brush panel was split up to "Brushes" and "Brush Settings" panel.
  • All brush-related settings are now sub-panels of the Brush Settings panel.
  • Custom Icon settings moved from the Display settings(now known as Cursor) to the Brushes panel.

  • UnifiedPaintSettings panels are removed and the contained options are now next to their relevant setting with a globe icon toggle. This is not displayed in the header.

  • 2-option enums are annoying as a drop-down menu, so they are now drawn with expand=True. (These are Sculpt radius unit and Weight Paint Gradient Tool type)
  • Cursor panel was reorganized, settings renamed.
  • "Adjust Strenght for Spacing" option was in the Options panel in some modes, but in the Stroke panel in others. It is now always under Stroke.

Header

  • In Weight Paint, don't display blend mode in the header because it's rarely if ever used.
  • In Texture/Vertex/2D Paint, Blend mode moved next to color.

Sculpt

  • "Radius Unit" option was affected by UnifiedPaintSettings size option, but this was hard to tell since these three things (radius, radius unit, unified size) were in three separate panels. Now they're all in one place.
  • De-duplicated some drawing code between 2D/3D brush options.

Weight Paint

  • No longer expose Blend mode option for all tools except Draw. Afaik this option doesn't make sense for the other tools.
  • The Options sub-panel(now known as Advanced) had some settings that were not actually brush options, but mode options. These were moved accordingly.

UV Sculpt

  • Similar issue as in weight paint mode. Some brush settings were now moved to a new Options panel, to be consistent with other modes.
  • For the same consistency, UV Sculpt now also uses large brush previews, although they do not have icons.

Grease Pencil Draw

  • Fill Options panel was merged with rest of the fill options, so that they are now available from the sidebar, instead of only from the header popover.
  • Expose eraser_mode option.

All feedback is welcome!

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

First of all, this doesn't apply to current master. Can you update to patch to it applies correctly?

Without proper testing, I can comment on some things still:

  • UnifiedPaintSettings: Seems like a nice change, and means we can remove the Unified Brush panel. However, we should not include these in the tool settings header - it becomes too crowded then:
  • Radius Unit: Fine with this. This is now more similar to areas like the camera unit & the size unit:

  • Settings that were not actually brush options: Ok, makes sense to clear this up
  • 2-option enums are annoying as a drop-down menu: Ok to change this
  • Populated the Weight Paint Context Menu a bit: This seem rather inconsistent to me. The idea with the right-click popups here, is to add the new brush selector when we get to add that. And if we were to add more options in here, it should apply to all paint & sculpt modes. I'd rather we hold off on this one.
  • Brush settings that belong to the same brush capability are now organized together by via box(): This is rather inconsistent with other areas. I think we can perhaps better use spacing here instead.
  • Minor changes to Tool Settings Header: seems ok. Adds a bit of inconsistency between the paint modes though.
  • Color options are no longer in a drop-down menu: Disagree with this, because it depends if you are using color swatches or the color wheel. You need to be able to collapse each element
  • Which brush options were under the Options drop-down seemed arbitrary The intention was for this section to be called Advanced. There are some brush settings that users use all the time, which are very basic, and then there are more advanced settings that are less common and harder to understand.
  • Particle brush options were headless. This was intentional, because particle edit mode doesn't use brushes. We need it to be headless, otherwise there is an unnecessary panel:

It's consistent with other tool settings:

  • Weight Paint and Vertex Paint now don't use the giant ugly previews. Indeed they are *terrible*. But, I think it's too inconsistent to remove brush icons for only weight and vertex paint. Soon we should have proper dynamic previews to replace this. Instead, what we could do, is to put the brush selectors in a separate panel at the top in all paint & sculpt modes, so that you can collapse it, and we could then collapse by default even. This is also needed when we add the better brush selector later on.

Here's how I think we should handle this, for all paint & sculpt modes:


Brush selector is in a separate panel from the brush settings, and we can collapse it by default for vertex and weight paint.
We should also rename Options to Advanced, because that's what it is, and it also avoids confusion with the other Options panel - it's not good to have two panels with the same name.

  • A lot of the drawing code used to be duplicate code. I have no strong opinion. The de-duplicated code is more messy, but is more centralized.

If you update the patch to work with current master, and remove the items marked with the red dot, I will approve it. If you want, you can also include the above change to separate the brush selector from brush settings, so it can be selectively collapsed.

This revision now requires changes to proceed.Sat, Sep 28, 11:34 AM
Demeter Dzadik (Mets) updated this revision to Diff 18597.EditedSat, Sep 28, 12:11 PM

Should now be able to merge with latest. Also fixed an issue(introduced by me) when drawing gradient UI.

I failed to update with arcanist, so I did it manually, hope it doesn't break everything...

@William Reynish (billreynish) Thanks for the detailed review. You make very good points, so I'll get started on all of these soon. If you will try out the patch and have more suggestions, let me know. Note for example, that the global toggles also show up in context menus, so it might make those a bit more cluttered too, like the headers. Should it be removed from there too?

Demeter Dzadik (Mets) planned changes to this revision.Sat, Sep 28, 12:11 PM

Great. There are still a few lines that don't apply (the removed options_unified panels).

Was able to test anyway.

Another small comment: For the expanded enums, we should not expand vertically. It makes the property way too large and prominent. And, the spacing is larger between Radius and Radius Unit, than from Radius Unit to Strength.

Here I changed the code so the enum is expanded horizontally (by using size_ui.row().prop to the property). Also, the spacing better communicates what property Size Unit relates to:

This should now have all the pieces of feedback implemented.
If it still doesn't apply to latest master then I must be doing something wrong.

Some things to discuss still:

  • I didn't split the brush selector into a separate panel for UV Sculpt mode, because contrary to other modes, it adds clutter rather than remove any. I know this leaves it inconsistent, but I thought this would be for the best.
  • No longer draw Blend Mode in Weight Paint on all tools except Draw because afaik it makes no sense for the other tools and just results in unpredictable, non-useful behaviour.
  • In many cases, there aren't a whole lot of brush options, so the Advanced drop-down feels unneccessary. I would get rid of it in all modes except Sculpt. What do you think?

Tested. This seems pretty good now, and has some nice advantages over the layout in master. I would like to see some additional testing with some more users before commit though.

There is a related problem in the layout, which has been there since 2.7x, which is that in the Tool Properties, it's not obvious which panels relate to the individual brush vs the mode. Currently it's not really possible at all to see this.

For the Tool Properties, we could use sub-panels, so that all brush settings are grouped together, like so:

The main drawback, is that then in the top bar, they are also all grouped under a single panel, which is probably not desirable:

Ideas for how to solve this are welcome.

This revision is now accepted and ready to land.Sat, Oct 5, 1:18 PM

A few other small issues we could fix:

1:

  • We should replace the lock with an explicit enum to choose between View and Scene. (A lock is just an incorrect metaphor, and also just obscure)

2:

  • We should rename Curve Alpha to Falloff Opacity. Texture Alpha could be Texture Opacity.

3:

  • We could change this enum to be horizontally expanded, like the other similar controls.

4:

  • Show Brush could be renamed to Show Brush Tip
Demeter Dzadik (Mets) planned changes to this revision.EditedSun, Oct 6, 2:10 AM

I found some issues(including a major one in 2D paint mode), and I found a way to turn the brush related panels into sub-panels without screwing up the header. Will try to get it done tomorrow.

Demeter Dzadik (Mets) updated this revision to Diff 18857.EditedTue, Oct 8, 12:02 AM
  • Fixed 2D Paint mode showing wrong mode's options
  • Cleaned up some useless code I accidently left in brush_shared_settings().
  • Spacing Distance now uses expand=True.
  • Changed "Alpha" => "Opacity".
  • This patch now also affects Grease Pencil paint modes.
  • De-duplicated drawing code for 2D/3D brush settings panels. (If you see large chunks of code removed, you'll find it in properties_paint_common.py)
  • All brush settings are now sub-panels of the Brush Settings panel. Tool Settings Header should remain intact.
  • "Show Brush" option is now a checkbox in the Brush Settings->Display header.

Video:

This revision is now accepted and ready to land.Tue, Oct 8, 12:02 AM
Demeter Dzadik (Mets) requested review of this revision.Tue, Oct 8, 12:04 AM

Since turning the panels into sub-panels was a rather large change, I guess I should un-accept this? :P

  • Fix some stupid obvious mistakes.
William Reynish (billreynish) requested changes to this revision.EditedTue, Oct 8, 10:21 AM

Changes seem good to me in general. A few small issues:

  • The Display panel now has a toggle in the header, but this doesn't affect the Custom Icon toggle. For this reason it's better if each sub-panel inside it has a separate toggle.

Instead should be like this:

  • Display
    • Brush Tip
    • Custom Icon

  • Rather than 'Curve Opacity' this option is really about the Falloff Opacity.


  • We should close the Advanced panel by default:


Other than that this seems like a nice set of changes. We should also hide all the brush settings when a brush tool is not enabled - not sure if you want to include that in this patch or not.

Next, we'll just get a few more folks to test it, and someone to review the code. The UI changes seem pretty good to me.

This revision now requires changes to proceed.Tue, Oct 8, 10:21 AM

We should also hide all the brush settings when a brush tool is not enabled - not sure if you want to include that in this patch or not.

I was worried that the patch is starting to do too many things, so it would be too annoying to review.
I think it's best if I just make the small adjustments that you pointed out just now, and make further changes in a separate patch?

Display panel now has a toggle in the header, but this doesn't affect the Custom Icon toggle

I thought this would be acceptable because the Custom Icon settings would go away soon, when we get dynamic brush previews. But I guess I should make this change when/if that happens, instead of getting ahead of myself. However, what do you think about calling it "Brush Cursor" rather than "Brush Tip"?

Demeter Dzadik (Mets) updated this revision to Diff 18889.EditedWed, Oct 9, 1:44 AM

Edit: I messed up this diff(I know how), please ignore. Will fix when I get home. ._.

Some changes as discussed in this thread and over chat.

  • Brush options should now only display when the active tool has a brush. (We decided to include this in this patch afterall)
  • Custom Icon settings were moved to the Brushes panel, as a small toggle button that reveals a file browser. This lets us keep the Display panel header checkbox.
  • Display panel was renamed to Cursor.
  • Fixed Advanced panel not hidden by default
  • Fixed Texture Curve not displaying in Texture Paint mode
  • "Curve Opacity" -> "Falloff Opacity"
  • Code quality improvements.
  • Fixed order of popovers in the header was wrong.

Still got some TODO's (including testing), so not requesting code review yet.

Demeter Dzadik (Mets) planned changes to this revision.Wed, Oct 9, 1:44 AM
Demeter Dzadik (Mets) updated this revision to Diff 18909.EditedThu, Oct 10, 12:31 AM

Here we go, this should be a potentially final version, assuming no more issues pop up. Testers welcome, especially for more obscure modes like grease pencil.

There should be no major changes from here, so I guess the "code" part of this "code review" could begin (Should I maybe re-submit the patch, to separate the code discussion from the design discussion? Let me know.). And again, I promise I'll keep patches smaller in the future! :)

William Reynish (billreynish) requested changes to this revision.Thu, Oct 10, 4:47 PM

Even though you know this, just marking this for transparency:

This version contains errors in the layout which causes it to fail in the Properties. Once the errors are fixed, we can review properly.

This revision now requires changes to proceed.Thu, Oct 10, 4:47 PM

You're mistaken, Radius Unit works the same way as Radius and Radius Pen Pressure. It is global when unified size is enabled. Otherwise, it is very much per-brush!

@Demeter Dzadik (Mets) Ah yeah, I totally forgot about the unified size, since it's enabled by default. I need a coffee... lol 👍

No worries, I appreciate any attempt at testing and feedback! ^^

Properties panel should be fixed now!

Tested and accepted.

IMO this is quite a bit better than what we have in master now.

Major advantages:

  • It's now possible to tell which settings relate to the current brush vs not (!!!)
  • It's now possible to collapse the brush picker separately from the brush settings, which is a major benefit both for users who use many brushes, and for users who never do that.
  • Many settings are grouped better, so that related features stay together
  • Brush panels are now correctly hidden when not using a brush (a bad bug in master)
  • Many smaller nice tweaks, such as the more dedicated panel for cursor controls

Just tested it as well and I agree. Generally nice improvements and for now couldn't see any issue with the patch anymore.

Pablo Dobarro (pablodp606) requested changes to this revision.Fri, Oct 11, 2:23 PM
Pablo Dobarro (pablodp606) added inline comments.
release/scripts/startup/bl_ui/space_view3d_toolbar.py
429

This property is missing in the patch

This revision now requires changes to proceed.Fri, Oct 11, 2:23 PM

Just to say it here too: I noticed one last issue in the UI in GP Draw Mode. There is a Brush Settings tab in the topbar that includes all brush settings even though it's also split into individual tabs next to it.

Demeter Dzadik (Mets) updated this revision to Diff 18973.EditedSat, Oct 12, 2:49 PM
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)

Many tiny changes and polish. Updated original post to reflect current state of the patch, and added Grease Pencil reviewers to maybe help test.

  • Changed panel order in Texture Paint mode so that Advanced now comes after Color and Color Palette.
  • Fix entire Brush Settings panel having a pop-over in grease pencil draw mode
  • Fix pose_offset option missing
  • Fix typo in popover "DIsplay"->"Display"
  • Fix some Display panels not being renamed to Cursor.
  • Use expand=True for Spacing Distance
  • Move "Adjust Strength for Spacing"(use_space_attenuation) from the Advanced panel to the Stroke panel in modes where it wasn't there already.
  • In Grease Pencil, the Eraser tool no longer has an empty Cursor panel, but instead just a regular checkbox like before. Except instead of "Show Brush" it's called "Display Cursor".
  • Similarly in UV Sculpt, "Show Brush" was renamed to "Display Cursor".
  • Renamed Grease Pencil "Stabilize" panel to "Smooth Stroke" to be consistent with other modes.
Demeter Dzadik (Mets) marked an inline comment as done.EditedSat, Oct 12, 2:49 PM

Fixed, good catch.

Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Sat, Oct 12, 2:53 PM

Changed panel order in Texture Paint mode so that Advanced now comes after Color and Color Palette.

Forgot to do this in 2D paint.

  • Add some spacing to spacing options ;)
  • Fixed accidently renamed Spacing Distance.
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Sat, Oct 12, 5:56 PM
William Reynish (billreynish) updated this revision to Diff 18979.

Replace the old lock icon toggle in the Jitter panel with a proper enum for switching between view/brush relative jittering.

Replace '2D Falloff' toggle with a proper enum, and move to the Falloff panel:

@Matias Mendiola (mendio) Hi, if you've not seen it this yet, can you take a look at this patch for GPencil?

Matias Mendiola (mendio) requested changes to this revision.Mon, Oct 14, 3:49 PM

@Matias Mendiola (mendio) Hi, if you've not seen it this yet, can you take a look at this patch for GPencil?

Some comments related to Grease Pencil:

  • Show Fill Color While Drawing (Draw brushes)

This control is not related to the Cursor. It's a Display setting

  • Display cursor (Eraser brushes)

Display Cursor is not the correct name because the cursor is always displayed. What the chekbox does is to show or not the "brush" contour/shape (circle).
So the name "Show Brush" or "Show Brush Contour" is a more appropiate.

  • Fill Brush

Add a new Advanced panel with this properties: Resolution, Ignore Transparent Strokes and Threshold.
This way we avoid a crowded topbar that hides some of the controls

This revision now requires changes to proceed.Mon, Oct 14, 3:49 PM
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Wed, Oct 16, 11:35 PM
Demeter Dzadik (Mets) commandeered this revision.Wed, Oct 16, 11:51 PM

Ah, I found the commandeer button. Updating...

Demeter Dzadik (Mets) updated this revision to Diff 19082.EditedWed, Oct 16, 11:52 PM

Made the requested changes in Grease Pencil mode.

Fix GP brushes not showing in properties editor.

This is overall a very nice patch now in terms of UI and layout. The main thing now is to get the code approved also.

@Matias Mendiola (mendio) @William Reynish (billreynish) @Demeter Dzadik (Mets)
I think we just need to decide on a common term for the rings (among other things) around the brush.
I generally refer to it as the brush cursor but others apparently use brush tip, brush contours, just "brush" or other terms.
There needs to be consistent terminology in all these modes, otherwise it becomes too messy.

In my opinion it should be called Show Brush Tip.

Calling it just 'Brush' is not really descriptive enough, and calling it the 'cursor' isn't really right, since the cursor is shown regardless of the tip being visible or not.

Other possibilities:

  • Brush Tip Size
  • Brush Circumference
  • Brush Circle

Ideally this would later be refactored so we could choose to actually hide the crosshair and only show the tip, for a cleaner, simpler display.

Although I understand the importance of differentiating between the mouse cursor and the brush cursor, I'm still in favor of calling the circles "Brush Cursor", but ofc when it's already inside a panel called "Brush Settings", putting "Brush" again is redundant, hence why it's just labelled "Cursor" at the moment (in all modes except grease pencil, as of the latest patch). I'm not dead set on this, but this does make the most sense to my brain.

I agree it would be nice to refer to this thing consistently in all modes.

But while we figure out this little piece of semantics, there's definitely no need to delay code review any further, so I'll start poking Julian when I can ;)

@Matias Mendiola (mendio) @William Reynish (billreynish) @Demeter Dzadik (Mets)
I think we just need to decide on a common term for the rings (among other things) around the brush.
I generally refer to it as the brush cursor but others apparently use brush tip, brush contours, just "brush" or other terms.
There needs to be consistent terminology in all these modes, otherwise it becomes too messy.

Agree with @William Reynish (billreynish), Show Brush Tip is the right term for me.

and talking about consistency in common terms, some thoughts about "Smooth Stroke" vs "Stabilizer":

Stabilizer (as call in Grease Pencil) is a toggle to activate a tool helper that damp out little irregularities while drawing with your input device while Smooth Stroke is the result of applying the stabilizer.
What you really do when activate the checkbox is the use of the stabilization helper (call Lazy Mouse in other programs). This control is more related to the input device than the line/stroke itself.

In Grease Pencil we have many controls that also affect the smooth of the strokes (Post-processing Smooth, Active smooth, Smooth Thickness) , so the name "Stabilizer" is more clear to differentiate.

IMO Stabilizer is a better name, but I agree we should use the same name (Smooth Storke or Stabilizer) for all Blender paint tools.