Page MenuHome

Sculpt: 'Unifiy' Dyntopo Detail Size operators
ClosedPublic

Authored by Philipp Oeser (lichtwerk) on Dec 16 2020, 1:45 PM.

Details

Summary

Prior to rB99a7c917eab7, Shift + D was used to set detail size for both
constant and relative detail. The commit added an improved operator for
doing this for constant detail, but left the user without a shortcut to
do this for relative detail.

Interestingly rB99a7c917eab7 only changed this for the Blender keymap,
the Industy Compatible keymap still has the "old" entry.

This patch changes both keymaps to have both entries.

For user experience, the real change here is to have both operators
available on one 'primary' shortcut (Shift+D), the improved
'dyntopo_detail_size_edit' operator will now act on all possible cases.
If it deals with constant detail, it acts as before, if it deals with
relative detail etc, it will call the "old" 'set_detail_size' internally
instead. I assume this adresses what was stated in rB99a7c917eab7:
"Deciding if both detail sizes can be unified needs a separate
discussion"

Also, move dyntopo_detail_size_edit to sculpt_detail.c

Fixes T83828

Diff Detail

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

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Dec 16 2020, 1:45 PM
Philipp Oeser (lichtwerk) created this revision.
  • unify both operators under one shortcut
  • move dyntopo_detail_size_edit to sculpt_detail.c
Philipp Oeser (lichtwerk) retitled this revision from Fix Dyntopo Relative Detail Size not in keymap to Sculpt: 'Unifiy' Dyntopo Detail Size operators.Dec 17 2020, 5:40 PM
Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Dec 29 2020, 4:37 PM
Philipp Oeser (lichtwerk) added inline comments.
source/blender/editors/sculpt_paint/sculpt_detail.c
692

Looking at it again, I am a bit unsure if doing it like this is the preferred way to do this.

Would prefer a second eye here, @Julian Eisel (Severin): do I get green light?

source/blender/editors/sculpt_paint/sculpt_detail.c
692

We do this in a couple of places, but I prefer not doing it. It makes things more confusing (how is undo handled? Can the operator safely be executed on redo? Could there be conflicts when the poll of the called operator changes? ...).

Functions are much more predictable. E.g. this could be ED_sculpt_detail_size_set().

Note that SCULPT_OT_set_detail_size also calls an operator - for which I don't see a reason. Was added in rB10813996e80c.

  • dont call the other operator, move into funtion
  • update comment
  • update operator description
source/blender/editors/sculpt_paint/sculpt_detail.c
692

Think calling WM_OT_radial_control in rB10813996e80cdda was neccessary to distinguish the different modes?
At least as of now, I wouldnt know how to set different paths to radial_control depending on toolsettings inside the keymap?

Other than that, have updated the patch...

If this is still interesting, it would need another review look (code changed since first accepting)

There is not much for me to review here, most changes are for the sculpt module to approve or are just moving around code. Don't see any problematic things on a quick glance.