Page MenuHome

Brush size in terms of a floating point diameter
Needs ReviewPublic

Authored by CandleComet (CandleComet) on Jun 9 2021, 8:48 PM.

Details

Summary

This patch is an attempt to be as comprehensive as possible in changing the brush size parameter from being an integer radius to a floating point diameter. The patch spans multiple modules and affects everywhere brushes show up:

  • image paint
  • texture paint mode
  • sculpt mode
  • uv sculpt
  • gp sculpt
  • gp draw
  • particle edit
  • radial control operator (F hotkey) now controls diameter to match these changes

Why?

  • Controlling brush size in terms of diameter is standard across other programs.
  • Not having an integer radius also allows for painting single pixels.

References
D6209
T71403
T85204

Diff Detail

Event Timeline

CandleComet (CandleComet) requested review of this revision.Jun 9 2021, 8:48 PM
CandleComet (CandleComet) created this revision.

Thank you for working on this! I want to take a look at this tomorrow again, but some comments:

  • Make sure the grease pencil team also reviews the patch
  • Even if this patch allows painting individual pixels, that should not be the main motivation for making it. The current brush tool will create discontinuous and weirdly jagged lines even with antialiasing and falloff disabled. For filling and painting individual pixels, a separate tools needs to be developed which takes all those cases into account. Most painting software also have these functionalities as separate tools/brush engines for the same reason.
  • One of the motivations of my patch was to allow control on brush sizes smaller than one unit. I remember that non linear UI sliders were merged some weeks ago, so that is something that can be considered for the future. I would not make it part of this patch as ideally, the radial control widget and the UI slider should behave in the same way.

The patch it's self seems mostly fine.

If we only want to show the brush size as a diameter in the UI, this could be done in RNA, without having to update all internal logic (with the minor down-side that going from 3.0 to 2.9x and back again will double brush sizes).

In general radius is easier to deal with for internal logic so I think it's worth considering.

Further I don't have a strong opinion on this change.


Building shows this warning.

source/blender/editors/sculpt_paint/paint_ops.c: In function ‘brush_scale_size_exec’:
source/blender/editors/sculpt_paint/paint_ops.c:153:11: warning: using integer absolute value function ‘abs’ when argument is of floating-point type ‘float’ [-Wabsolute-value]
  153 |       if (abs(old_radius - radius) < U.pixelsize) {
      |           ^~~

One option is to split this patch into two parts:

  1. use a floating point radius instead of an integer radius
  2. control brush size using diameter

It might be easier to review that way. Also, we could implement the first part before having to decide about part 2 (e.g. whether to convert diameter to radius in the RNA vs in the internal logic).