Page MenuHome

Object: Switch Subdivision Level operator
Needs RevisionPublic

Authored by Pablo Dobarro (pablodp606) on Apr 28 2020, 9:14 PM.
Tags
None
Tokens
"Haypence" token, awarded by andruxa696."Love" token, awarded by RodDavis."Love" token, awarded by johnsyed."Love" token, awarded by julperado."Love" token, awarded by xrg."100" token, awarded by Frozen_Death_Knight."Love" token, awarded by Brandon777.

Details

Summary

This operator manages sculpt level switching of a Multires Modifier in Sculpt mode and Multires/Subdivision Surface in Object. It is assigned to Alt + Shift + Q to go down one level and Shift + Q to go up one level.

It has the following features:

  • When a higher/lower level is available, it switches the sculpt level one level up/down directly
  • If a higher/lower level is not available, the operator will show a confirmation dialog to perform a subdivide or unsubdivide operation. In subdividing operations this dialog shows the vertex and poly count of the next subdivision level before executing the subdivision.
  • If there is now Multires Modifier available, the operator will create a new one with a higher/lower level already available.
  • In object mode, if a Multires modifier is available it will change the preview subdivision level, creating new levels when necessary
  • In object mode, if no Multires modifier is available it will create a Subdivision Surface modifier and control its subdivisions directly.

For now, the unsubdivide operation is never executed as it depends on D7372

Diff Detail

Repository
rB Blender
Branch
sculpt-multires-switch-level (branched from master)
Build Status
Buildable 7807
Build 7807: arc lint + arc unit

Event Timeline

Pablo Dobarro (pablodp606) requested review of this revision.Apr 28 2020, 9:14 PM
Pablo Dobarro (pablodp606) created this revision.
Pablo Dobarro (pablodp606) edited the summary of this revision. (Show Details)
  • Fix bug in level down operation
  • Fix info content when adding a modifier
Sergey Sharybin (sergey) requested changes to this revision.Apr 29 2020, 9:16 AM

This was already working in 2,7 series with Ctrl-<Number> shortcut (as in, Subdivision Set modifier already have logic for this).
I do not see a reason to re-implement similar logic in a new operator.

This revision now requires changes to proceed.Apr 29 2020, 9:16 AM

@Sergey Sharybin (sergey) The previous way of switching to a level directly with Ctrl + Number is not convenient and does not make much sense when sculpting. You usually want to move one level up/down at a time. Also, this operator allows you to switching to Multires sculpting directly and control all the basic actions from Multires without going into the modifiers panel. It is just a much better UX for what we have now and it probably should replace the old shortcuts.

Ah, so the difference is that it's relative instead of absolute. This makes sense, but I don't see why this behavior is limited to multires in sculpt mode. One would expect relative adjustment of subdivision level to also work for Subdivision Surface modifier in object mode, or to preview subdivisions of multires in object mode.

The popup which happens in user's face I'm not super happy with either. Would want UI team to have a pass at least.

source/blender/editors/object/object_modifier.c
1777–1795

Use existing findamental functions, such as get_multires_modifier. Don't re-implement logic even if it seems trivial.

Pablo Dobarro (pablodp606) marked an inline comment as done.
  • Review update

@Sergey Sharybin (sergey) @William Reynish (billreynish) I can add the functionality to control preview levels from object mode and compatibility with the last subsurf modifier in the stack if not multires is found, but then we will be in a similar situation as D7510 where the problem is finding a keymap that works consistently in all modes. In this case, this should not be that hard as this does not need a mouse position in order to work, but probably Alt + 1 and Alt + 2 are not the most discoverable shortcuts for this.

I'm not a coder but I want to voice a minor concern.
I recommend using "repeat": False for the ALT+2 keymap. This operation subdivides Multires if sculpt_level=total_level. The operation can repeatedly subdivide multires while the buttons are held down. This is because repeat is true by default. It can cause crashes or stalls if the buttons are held down for too long. Unless instead, it only looks for available sculpt levels while the buttons are held down.

Same goes for ALT+1 Multires Unsubdivide in the future. The repeat may cause issues if repeat is True.


It's the same concept if you set "subdivision set" as relative and kept "repeat" as true. If you held down the keys in the viewport, then subdivision levels increases until release. This also goes above the soft limit of 6 levels.

  • Add support for Subsurf Modifier and Multires Preview Levels in Object
Pablo Dobarro (pablodp606) retitled this revision from Sculpt: Multires Switch Sculpt Level operator to Object: Switch Subdivision Level operator.Apr 30 2020, 2:12 AM
Sergey Sharybin (sergey) requested changes to this revision.Apr 30 2020, 9:42 AM

I am not super thrilled about having two operators which behavior does intersect. For example, in object mode this new operator with SUBDIVISION_LEVEL_SWITCH_UP is the same as SubdivisionSet with level set to 1 and relative set to True.

Other difference is that the new modifier operates on an active object, while SubdivisionSet operates on selection. I do believe that this is a common Blender mental paradigm: operator operates on the entire selection.

And last but not least, SubdivisionSet already does multires subdivide, and does it without any extra popup ups, which are super annoying.

I don't see how this new behavior can not be achieved by:

  • Allowing multires_subdivide in relative mode of SubdivisionSet
  • Allowing multires_unsubdivide when multires_subdivide tries to go below level 1
  • Setting keymap accordingly (level set to 1 for SUBDIVISION_LEVEL_SWITCH_UP behavior, -1 for SUBDIVISION_LEVEL_SWITCH_DOWN behavior, and relative set to True).

Please spend time on figuring clear design on a system level, avoiding having several ways of achieving the same goal.

This revision now requires changes to proceed.Apr 30 2020, 9:42 AM

I guess people would get annoyed by a pop up, hmm.
I seen repeat was changed to false, thank you for that. Then you could get rid of the basic info/warning popup to unsubdivide/subdivide. Because now it doesn't continuously unsubdivide or subdivide the mesh when buttons are held.


My own opinions:
If there is any problems with vertex colors, uv's, or other vertex customdata when un-subdividing. Then, in my own opinion, you may want to put out a warning for unsubdiv. I'm only saying this because of my issues in the past with the Decimate Modifier's Unsubdivide. Anyway, the warning would be similar to dyntopo's warning for vertex customdata.


But if there is no problems with those mentioned, then you wouldn't need this warning.