Page MenuHome

Brush Settings UI
ClosedPublic

Authored by Demeter Dzadik (Mets) on Sep 28 2019, 12:47 AM.
Tokens
"Love" token, awarded by Tetone."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 is a slight overhaul to how brush settings are displayed and organized. 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. Thanks to William for feedback, help, and even some contributions.

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.
  • Display(now 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.
  • "2D Falloff" option was renamed to "Falloff Type", turned into an enum, and moved to Falloff panel.
  • "Smooth Stroke" and "Stabilizer" options in grease pencil and other paint modes are now both called "Stabilize Stroke", for consistency and clarity.
  • De-duplicated some drawing code between various painting modes' brush options. I tried to keep de-duplication reasonable and easy to follow.

Vertex/Texture/2D Paint

  • In the header, moved Blend mode next to color.
  • Move Advanced options underneath Color options.

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.

Weight Paint

  • No longer expose Blend mode option for all tools except Draw. Afaik this option doesn't make sense for the other tools.
  • Don't display blend mode in the header because it's rarely if ever used.
  • 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 actually mode settings, and were moved accordingly to a new Options panel.
  • Also for the sake of consistency, UV Sculpt now also uses large brush previews, although they do not have icons.

Grease Pencil Draw

  • Fill Options panel was renamed to Advanced for consistency, and is now available from the sidebar, instead of only from the header popover.
  • Expose eraser_mode option.
  • Display settings moved to their own panel, to be consistent with other modes. (except Eraser because no sub-options)

Now we're just waiting for code review! ;)

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Matias Mendiola (mendio) requested changes to this revision.Oct 14 2019, 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.Oct 14 2019, 3:49 PM
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Oct 16 2019, 11:35 PM
Demeter Dzadik (Mets) commandeered this revision.Oct 16 2019, 11:51 PM

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

Demeter Dzadik (Mets) updated this revision to Diff 19082.EditedOct 16 2019, 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.

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.

I don't think anything else is called "something-er" in Blender. There's an Extrude Tool, not an Extruder. So I think it's between "Stabilize" and "Smooth Stroke". And "smooth stroke" being the "result of applying the stabilizer" is just interpreting the word "smooth" as a noun rather than a verb. That said, "Stabilize Stroke" does sound more descriptive. A perfect middle ground? :)

Stabilize Stroke seems fine for both cases I think.

Stabilize Stroke seems fine for both cases I think.

Stabilize Stroke is fine for me too

Demeter Dzadik (Mets) updated this revision to Diff 19154.EditedOct 21 2019, 10:38 PM
  • Update to latest master. Someone hacked yet another grease pencil tool setting into the brush settings... In Draw mode, stored in sculpt settings RNA... Interesting.
  • "Smooth Stroke"->"Stabilize Stroke"
  • Updating from Arcanist now so Phabricator's file context features should work.
Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Oct 23 2019, 11:38 AM

Code wise not much to be said here, besides the few codestyle notes below. So if UI/UX team is fine with those changes, patch LGTM. Please wait for after 2.81 release to commit though, such heavy reshuffle changes can make merging from 2.81 to master problematic. ;)

release/scripts/startup/bl_ui/properties_grease_pencil_common.py
133–134

This seems to be a weird hack to 'hide' a panel, unless am mistaken? Do not do that (fine in dev process, but not for final patch), just remove whatever becomes unused. We have git history in case we need to find that code again.

release/scripts/startup/bl_ui/space_view3d_toolbar.py
1772

tabs on empty lines are bad! ;)

Lots of those here, to be fixed.

source/blender/makesrna/intern/rna_brush.c
1601

Please do not use "magic numbers" ever, always use proper enum value (or #define if really needed).

Now everything seems to be fine.

Just only one thing, I still think that the checkbox in Cursor Panel affects the visibility of the Brush tip and not the Cursor itself that is always visible.
So IMO Cursor does not seem to be the right name. What do you think @William Reynish (billreynish) ?

I'd still rather differentiate those things as Mouse Cursor vs Brush Cursor, rather than Brush Tip vs Cursor. Brush Tip means other things in other software, and even ignoring that, to me it feels like it implies that it affects the behaviour of the brush. But, if you guys want it to be Brush Tip, it will be Brush Tip :P I'll try to find time to implement this and Bastien's feedback in the coming days.

Demeter Dzadik (Mets) updated this revision to Diff 19323.EditedNov 1 2019, 4:29 PM
  • Update to latest master (new anti-aliasing option in 2D paint)
  • Nuke all trailing whitespace via ^[ \t]+$
  • @Antonio Vazquez (antoniov) GreasePencilStrokeEditPanel seemed to be unused code so I removed it on Bastien's request, thought I'd let you know.
  • I turned the magic number into a proper enum value, although I can only hope I did it in the best way? If yes, this could also be done in a few other places, but maybe in a separate clean-up patch.
  • Rename Brush Cursor to Brush Tip
  • Fixed an issue where Grease Pencil Weight Paint/Sculpt wouldn't have a Brush Tip panel when the active tool in Draw mode was Eraser.
Demeter Dzadik (Mets) marked 3 inline comments as done.Nov 1 2019, 5:01 PM

I wonder if I'm likely breaking anything by moving BRUSH_ABSOLUTE_JITTER out of Brush->flag and into its own enum? @Bastien Montagne (mont29) if you could take a quick look

source/blender/makesdna/DNA_brush_types.h
435 ↗(On Diff #19324)

Is this safe/good to remove from here? @Bastien Montagne (mont29)

Antonio Vazquez (antoniov) added inline comments.
release/scripts/startup/bl_ui/properties_grease_pencil_common.py
133–134

No problem to remove these panels. These were from 2.7x when Gpencil hadn't modes and now with the mdoes and the split of annotation tool, we don't need them.

Bastien Montagne (mont29) requested changes to this revision.Nov 14 2019, 10:30 AM

In comments below you'll find details about how to handle such 'breaking' change in DNA.

But in that case, I don’t think this is needed. Unless you plan to add more than two jittering 'modes', it would be simpler to keep current way of doing things in DNA, with a single bitflag (like you sort of did in previous version of that patch).

What you need to do then is:

  1. Add a proper define (to zero) for the relative jitter case (in RNA only, probably just above your RNA enum values definition).
  2. Use custom getter and setter callbacks, as default RNA bitflags enum setters/getters assume the whole range of the integer to be covered, which is not the case here. You only use one bit of flag, and absolutely do not want to alter any other ones, so you need to mask them in your setter & getter. Check usages of RNA_def_property_enum_funcs() in code for examples.
source/blender/makesdna/DNA_brush_types.h
435 ↗(On Diff #19324)

No it is not! At least not without some extra work.

You need to handle all existing brush datablocks in doversion code, to 'transfer' that flag from the old one to the new one. That also imply not removing that flag from here, but rather tagging it as DNA_DEPRECATED, so that you can still access its value in code dealing with older DNA, like the .blend reading one.

See e.g. how this is done in DNA for CD_MTEXPOLY or CD_MSTICKY deprecated enum values (in DNA_customdata_types.h).

This ensures that deprecated enums are not available (and hence used) in normal code, but are still available for doversion of older files and such.

Note that you do not necessarily have to update the minor version number of Blender, since you are adding a new member to DNA, you can rather use something like if (!DNA_struct_elem_find(fd->filesdna, "Brush", "chqr", "jitter_unit")) {<versionning code... >;}, just put in the end block (Versioning code until next subversion bump goes here.).

Also, think you will have to handle updates of defaults for brushes as well?

599 ↗(On Diff #19324)

Should be renamed to something like BRUSH_JITTER_UNIT_ABSOLUTE (and same for the other one of course).

This revision now requires changes to proceed.Nov 14 2019, 10:30 AM
  • Update to apply to latest master
  • Revert changes to DNA. For the enum, simply use bitflag instead. This should remove any need for doversion

I would like to get this patch into Blender 2.82.

Pablo Dobarro (pablodp606) requested changes to this revision.Nov 30 2019, 6:22 PM

Some comments related to sculpt mode:

  • The brush pressure/size and pressure/alpha toggle should always be visible. They should not be hidden when using unified properties.
  • The cursor colors and alpha panel should not be named "Brush Tip". We already have some patches that introduce options to modify the actual brush tip, such as D6165. I would prefer something like "Brush Cursor"
  • The dyntopo remesh and the remesh panels are next to each other, which can be confusing
  • There is a missing separator between falloff type and falloff shape
  • It does not make much sense to have radius unit as the first property in the brush popover, it is an option that is rarely used.
  • Dash ratio and dash samples options are missing from the stroke popover
  • All multiplane scrape options are missing from the brush popover
  • Invert to fill and invert to scrape are missing from the brush popover
This revision now requires changes to proceed.Nov 30 2019, 6:22 PM
  • The brush pressure/size and pressure/alpha toggle should always be visible. They should not be hidden when using unified properties.

This appears to be a bug - but cannot immediately work out why this happens. @Demeter Dzadik (Mets) Since this happens due to your code re-org changes, could you look at this one?

  • The cursor colors and alpha panel should not be named "Brush Tip". We already have some patches that introduce options to modify the actual brush tip, such as D6165. I would prefer something like "Brush Cursor"

I changed it to Cursor. 'Brush' is implicit I think since it's a sub-panel under the Brush Settings parent.

  • The dyntopo remesh and the remesh panels are next to each other, which can be confusing

AFAIK that's how it is in master too - would rather not change more things in this mega-patch. It's already really probably much bigger than it should be

  • There is a missing separator between falloff type and falloff shape

Fixed

  • It does not make much sense to have radius unit as the first property in the brush popover, it is an option that is rarely used.

Agreed, but see previous comment - since this is unchanged from master, better to solve/change that separately.

  • Dash ratio and dash samples options are missing from the stroke popover

Fixed

  • All multiplane scrape options are missing from the brush popover

Fixed

  • Invert to fill and invert to scrape are missing from the brush popover

Fixed

Thanks a lot for helping out, I'll take a look at the final issue tonight.

@Pablo Dobarro (pablodp606) No wonder unified brush pressure is not being displayed properly when you removed unified brush pressure 3 days beforehand in rB9251b077203c763f1be83505ed0a4d1e19dab947. Could've saved me a few minutes just mentioning that, really.

Should be fixed now! Somebody let this be over! :^)

I still think that there are quite some elements that are not in their optimal place (the special properties of each brush not showing first in the popover, for example), but we can fix that in a later patch once we know the final feature set of 2.82.

The properties in Grease Pencil post-processing panel without separations are difficult to read, the current version is better because it joins the properties that are related to each other.

William Reynish (billreynish) updated this revision to Diff 20068.

Fix wrong alignment in Grease Pencil Post-Processing panel.

Thanks for the change @William Reynish (billreynish) , any comments about the name change for the 'Cursor' panel?

Now everything seems to be fine.

Just only one thing, I still think that the checkbox in Cursor Panel affects the visibility of the Brush tip and not the Cursor itself that is always visible.
So IMO Cursor does not seem to be the right name. What do you think @William Reynish (billreynish) ?

@Matias Mendiola (mendio) That's why it was 'Brush Tip' before, but Pablo Dobarro prefers not to use that. I think it's probably going to be ok with Cursor, given that it's a sub-section of the Brush panel.

Ok, then everything is fine now for Grease Pencil

LGTM on C changes side now, did not check in details the whole Py UI changes, I trust the UI and GP teams did the job here. ;)

This revision is now accepted and ready to land.Dec 13 2019, 4:41 PM

Fixed some errors (due to changes in master and some possible merge mishaps) based on chat with William.

This revision was automatically updated to reflect the committed changes.

In the latest Blender 2.82 alpha master builds, the right-click brush settings dialog in Sculpt Mode has its appearance hotspot set at the top right of the dialog, which is very inconvenient when you want to frequently adjust the brush size or strength slider.

The dialog appearance hotspot used to be at a more convenient, slider-centric position. It’d be great if that could be fixed, thanks a lot.

Hm, I don't think that could've been caused by this patch.

OK. I had posted it as an issue, and @ronsn pointed me to this discussion.

Looks like he did a bisect so must be right. Interesting. Will look into it after the new year's.

ronsn added a comment.Dec 31 2019, 3:59 PM

Looks like he did a bisect so must be right. Interesting. Will look into it after the new year's.

Yes I did :)