Page MenuHome
Differential D14881

Normalize All Weights: default to Deform Pose Bones when there is an Armature
AbandonedPublic
Actions

Authored by Nate Rupsis (nrupsis) on May 7 2022, 2:17 AM.

Details

Reviewers
Daniel Salazar (zanqdo)
Sybren A. Stüvel (sybren)
Brad Clark (RiggingDojo)
Maniphest Tasks
T95038: Change defaults for Normalize All Vertex Groups operator
Summary

If armature is present, the default behavior is targeted to "All Groups". It should default to Deform Pose bone instead.

If we don't do that, we risk normalizing deforming and non deforming groups together which is catastrophic for all of them.

This patch sets the default Subset to Deform Pose bone and falls back to "All" if no armature is present.

Videos:
Old

New

Diff Detail

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

Event Timeline

Nate Rupsis (nrupsis) requested review of this revision.May 7 2022, 2:17 AM
Nate Rupsis (nrupsis) created this revision.
Harbormaster completed remote builds in B22011: Diff 51258.May 7 2022, 2:17 AM
Daniel Salazar (zanqdo) awarded a token.May 7 2022, 2:24 AM
Nate Rupsis (nrupsis) edited the summary of this revision. (Show Details)May 7 2022, 2:25 AM
Daniel Salazar (zanqdo) added a comment.May 7 2022, 2:29 AM

From an user's perspective this looks complete to me

Nate Rupsis (nrupsis) updated this revision to Diff 51299.May 9 2022, 5:20 PM
  • fixing typo in comment
Harbormaster completed remote builds in B22031: Diff 51299.May 9 2022, 5:20 PM
Nate Rupsis (nrupsis) updated this revision to Diff 51300.May 9 2022, 5:22 PM
  • and fixing extra spacing
Harbormaster completed remote builds in B22032: Diff 51300.May 9 2022, 5:22 PM
Nate Rupsis (nrupsis) updated this revision to Diff 51317.May 9 2022, 11:47 PM
  • Merge branch 'master' into T95038
  • actually fixing typo
Harbormaster completed remote builds in B22038: Diff 51317.May 9 2022, 11:47 PM
Philipp Oeser (lichtwerk) added a project: Animation & Rigging.May 10 2022, 11:03 AM
Philipp Oeser (lichtwerk) added a subscriber: Philipp Oeser (lichtwerk).

Love those little enhancements that just make more sense.
Check comment formatting in https://wiki.blender.org/wiki/Style_Guide/C_Cpp

Nate Rupsis (nrupsis) updated this revision to Diff 51343.May 10 2022, 4:33 PM
This comment was removed by Nate Rupsis (nrupsis).
Harbormaster completed remote builds in B22049: Diff 51343.May 10 2022, 4:34 PM
Deen (Bit) awarded a token.May 11 2022, 5:59 PM
Nate Rupsis (nrupsis) updated this revision to Diff 51542.EditedMay 16 2022, 7:59 PM
  • Splitting Diff into two separate revisions.
Harbormaster completed remote builds in B22121: Diff 51542.May 16 2022, 8:00 PM
Nate Rupsis (nrupsis) edited the summary of this revision. (Show Details)May 16 2022, 10:21 PM
Colin Basnett (cmbasnett) awarded a token.Jun 3 2022, 1:22 AM
Sybren A. Stüvel (sybren) requested changes to this revision.EditedJun 3 2022, 10:52 AM

Great improvement @Nate Rupsis (nrupsis)!

A bit of feedback on the patch itself:

  • The title should be understandable by everybody, so also regular users / artists. Mentioning constants like WT_VGROUP_BONE_DEFORM makes things harder to understand. Something like "Normalize All Weights: default to Deform Only when there is an Armature" would be clearer.
  • The videos are really nice as a demo of what's going wrong, but they have the disadvantage that they take much time to view, but more importantly, they cannot be indexed by the search engine. For bug reports & patches, please ensure that the information is written down, and that videos are only used as illustration, not as the main source fo info. That way there is a bigger chance the patch is found when people search for related terms. The patch description will also be used as commit message, which is another reason to take some time to write things down beforehand.

The functionality of the patch is certainly accepted. The comment & patch description could use some more love though.

source/blender/editors/object/object_vgroup.c
734

This explains the what, but not why this is done. The patch title and description also don't explain this, so for future developers it becomes tricky to figure out the motivation.

This revision now requires changes to proceed.Jun 3 2022, 10:52 AM
Daniel Salazar (zanqdo) accepted this revision.Jun 3 2022, 8:11 PM
Daniel Salazar (zanqdo) retitled this revision from Setting default enum option to WT_VGROUP_BONE_DEFORM, and falling back to All Groups if no armature is found to Normalize All Weights: default to Deform Pose Bones when there is an Armature.Aug 4 2022, 1:15 AM
Daniel Salazar (zanqdo) edited the summary of this revision. (Show Details)
Daniel Salazar (zanqdo) edited the summary of this revision. (Show Details)Aug 4 2022, 1:18 AM
Daniel Salazar (zanqdo) edited the summary of this revision. (Show Details)
Daniel Salazar (zanqdo) added a comment.Aug 4 2022, 1:25 AM
This comment was removed by Daniel Salazar (zanqdo).
Daniel Salazar (zanqdo) added a comment.Aug 4 2022, 1:37 AM

Updated the title and description as requested.

As for the code comment I suggest something like this:

/* Set Deform Bone as default selection if armature is present to avoid normalizing deforming and non deforming groups together. */

Daniel Salazar (zanqdo) edited the summary of this revision. (Show Details)Aug 4 2022, 1:39 AM
Daniel Salazar (zanqdo) edited the summary of this revision. (Show Details)
Nate Rupsis (nrupsis) abandoned this revision.Aug 4 2022, 5:48 PM

Duplicate of https://developer.blender.org/D14881 (which has already been committed)

Revision Contents
Changeset List

PathSize
source/
blender/
editors/
object/
13 lines

Diff 51258

View Options

source/blender/editors/object/object_vgroup.c

Loading...