Page MenuHome

Add Non Linear Sliders
ClosedPublic

Authored by Henrik Dick (weasel) on Oct 1 2020, 12:57 PM.
Tokens
"Love" token, awarded by jonathanl."Burninate" token, awarded by rbx775."Burninate" token, awarded by CAEL."Love" token, awarded by mindinsomnia."Love" token, awarded by Alaska."Love" token, awarded by gilberto_rodrigues."Like" token, awarded by ChrisLend."Like" token, awarded by phocomelus."Like" token, awarded by RedMser."Like" token, awarded by duarteframos.

Details

Summary

This patch introduces non linear sliders. That means, that the movement of the mouse doesn't map linearly to the value of the slider.

There are many sliders in blender, where the linear default behavior is not helpful. For example all the sliders for merge thresholds.
The important values there are spaced logarithmic. There is also one particular slider, which causes massive issues in voxel remesh (T77868).
The voxel remesh slider should now be much more tame, so nobody must fear tweaking it anymore, but this is still no absolute solution to T77868 IMO.

This patch includes:

  • Free logarithmic sliders with maximum range of (0 <= x < inf)
  • Logarithmic sliders with correct value indication bar.
  • Free cubic sliders with maximum range of (-inf < x < inf)
  • Cubic sliders with correct value indication bar.

I decided to add the cubic mapping as well, because Krita uses it for its brush size and I figured why not.

NOTE: The scale types are available for all float sliders. For integer sliders they are only available if they use the visible value bar. Sliders with logarithmic scale and value bar must have a range > 0 while logarithmic sliders without the value bar can have a range of >= 0.

Since we don't have a float brush size in blender and we also usually need different brush sizes than Krita does, I didn't add it to the sliders here.
I did however add the behavior to the following sliders:

  • Boolean Modifier > Fast > Overlap Threshold (Logarithmic)
  • Remesh Modifier > Voxel > Voxel Size (Logarithmic)
  • Sculpt > Dyntopo > Detail Size (Cubic)

For Developers:
To make a slider have a different scale type use following line in the rna:
RNA_def_property_ui_scale_type(prop, PROP_SCALE_LOG);
or
RNA_def_property_ui_scale_type(prop, PROP_SCALE_CUBIC);
Remember to test the precision, step size and softmin if you change the scale type of a property as it will feel very different and may need tweaking.

Diff Detail

Repository
rB Blender
Branch
betterslider (branched from master)
Build Status
Buildable 13822
Build 13822: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Henrik Dick (weasel) requested review of this revision.Oct 1 2020, 12:57 PM
source/blender/makesrna/intern/rna_modifier.c
5510

didn't mean to change this back... will revert this in the next revision of the diff.

  • rebase
  • revert voxel softmin

I LOVE this idea!

Being a subtype though, doesn't see to fit well. I might be thinking about this wrong, but I intuitively (or maybe naively LOL) want to see something like this:

prop = RNA_def_property(srna, "double_threshold", PROP_FLOAT, PROP_DISTANCE);
RNA_def_property_range(prop, 0, 1.0f);
RNA_def_property_ui_range(prop, 0, 1, 1.0, 6);
RNA_def_property_ui_scale(prop, SCALE_LOGARITHMIC);
...

RNA_def_property_ui_scale would just set one of a few new PropertyRNA.flag values

No gif/video to show what it does?

@Nelson (NAS) - No gif/video to show what it does?

This particular site assumes that any reviewers here will simply apply the patch, compile, and use the feature to evaluate it. If you are not currently able to do so I would highly recommend it: https://wiki.blender.org/wiki/Building_Blender. Afterward you could always make a gif and post it here.

Henrik Dick (weasel) updated this revision to Diff 29557.EditedOct 4 2020, 1:15 AM
  • change rna scale location
  • fix cubic interpolation for free sliders

In my opinion the flag doesn't seem like a good place to put the scale option.
It is already pretty crammed with options and not a lot more will fit in without bumping the size.
Also the scale only affects int and float properties, so I added it just like the parameter step.

EDIT:
sorry for the whitespace error and the changed stepsize for detail_size. I will cleanup the patch in the next revision together with requested changes.

@Henrik Dick (weasel) - In my opinion the flag doesn't seem like a good place to put the scale option. It is already pretty crammed with options and not a lot more will fit in without bumping the size. Also the scale only affects int and float properties, so I added it just like the parameter step.

For sure. I like how you did it; makes more sense than using a flag. Looks much nicer without the subtype.

  • rebase
  • fix whitespace

As nobody did pick this up for thorough review yet, here's the fixed version.

Henrik Dick (weasel) edited the summary of this revision. (Show Details)Oct 23 2020, 9:09 PM

This seems like a great idea in general.

One issue is with continuous grab: If that option is disabled (or if the event comes from a tablet), we already use a non-linear dragging. In that case we shouldn't try to apply the dragging changes done here in the patch, it should just use the already existing logic (which is fine-tuned for tablets).

@Julian Eisel (Severin) I wouldn't mind removing the cubic sliders for non continuous grab, but for the logarithmic sliders I still think that continuous grab should use the log/pow function. That way it will tame the voxel sliders for stylus as well.
In case of removing the cubic interpolation, should I just delete the code or would a #if 0/#endif block be useful here?

Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/interface/interface_intern.h
655–656

Prefer ui_but_is_scale_log, ui_but_is_scale_cubic

source/blender/makesrna/RNA_types.h
105

Is this still needed?

source/blender/makesrna/intern/rna_define.c
1757

Prefer RNA_def_property_ui_scale_type.

source/blender/makesrna/intern/rna_internal_types.h
403

scale_type otherwise it reads like a scale, other use here too.

  • rebase
  • address inline comments by Campbell Barton

I didn't remove any of the code for the non continuous grab yet.
I think it is correct to have the chosen non linearity of the slider also
with non continous grab input. I think if thats a problem then the non linearity
that was used before there, should be disabled if there is a non linear scale set
for the slider.

Henrik Dick (weasel) marked 4 inline comments as done.Oct 29 2020, 11:31 PM
Campbell Barton (campbellbarton) requested changes to this revision.Mar 30 2021, 11:44 AM
Campbell Barton (campbellbarton) added inline comments.
source/blender/makesrna/intern/rna_define.c
1762–1779

I'm not keen on the scale type changing thehttps://www.gnu.org/software/stow/ minimum/maximum. It could cause some confusing situations when values slightly below zero are expected to work.

Instead, this could be left out, then checked when generating.
So any non-linear scaled float/int buttons that have minimums below zero report an error.

This revision now requires changes to proceed.Mar 30 2021, 11:44 AM
source/blender/makesrna/intern/rna_define.c
1762–1779

tsk, pasted URL in by accident, ignore that.

Henrik Dick (weasel) marked 2 inline comments as done.
  • rebase
  • add error check for negative ranges on log scale sliders
Henrik Dick (weasel) edited the summary of this revision. (Show Details)Mar 30 2021, 2:25 PM
This revision is now accepted and ready to land.Mar 30 2021, 2:31 PM
source/blender/makesrna/RNA_access.h
844

Missed last review, this should be named ui_scale too: RNA_property_ui_scale.

Henrik Dick (weasel) marked an inline comment as done.
  • change name to ui_scale

Doing some more detailed review and I've noticed some issues, I'm about to update this patch before committing to master.

I'll post some notes here before doing so.

  • Rename PROP_SCALE_LOGARITHMIC -> PROP_SCALE_LOG (for brevity - log is a common abbreviation).
  • Rename PropertyScale -> PropertyScaleType (for clarity).
  • Replace 2x booleans with a single enum.
  • Replace fmaxf with max_ff
  • Added comments to PropertyScaleType based on the patch submission.
  • Use switch statements in the UI, so each sliders logic is clearly labeled, with the most common (linear) scale, ordered first.
  • Use define for magic number 0.5e-8f.
  • Removed redundant non-zero check (replaced with an assert).
  • Add define for log offset
Campbell Barton (campbellbarton) requested changes to this revision.May 12 2021, 12:45 PM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/interface/interface_handlers.c
128

This is an oddly spesific value if it's just to avoid divide by zero.

What's the significance of this value? (needs doc-string)

130

Moved this from a magic number to a define, likely it should be renamed (needs doc-string).

5260–5262

Other similar uses of UI_PROP_SCALE_LOG_OFFSET use float versions of functions, where these use double values. If this is intentional there should be a comment explaining why.

This revision now requires changes to proceed.May 12 2021, 12:45 PM
Henrik Dick (weasel) marked 2 inline comments as done.May 13 2021, 6:30 PM
Henrik Dick (weasel) added inline comments.
source/blender/editors/interface/interface_handlers.c
5260–5262

honestly, I didn't want to change anything about the normal behavior and as value_step was double I just used double here. I don't think value_step needs to be double, even though data->value is double.

This revision is now accepted and ready to land.May 17 2021, 9:37 AM
Henrik Dick (weasel) edited the summary of this revision. (Show Details)Aug 20 2021, 10:05 PM