FIX #110946: vgroup normalize all check if armature is deforming before normalizing #112648

Nate Rupsis · 2023-09-21T03:58:49+02:00

Nate Rupsis commented

2023-09-21 03:58:49 +02:00

The issue in #110946 is if an armature is present, but not active the group_select_mode defaults to WT_VGROUP_BONE_DEFORM, and throws an error because it can't find any active vertex groups.

To fix this, we're now checking to see if any bone is actively deforming before switching toWT_VGROUP_BONE_DEFORM (else defaulting to WT_VGROUP_ALL)

The issue in #110946 is if an armature is present, but not active the group_select_mode defaults to WT_VGROUP_BONE_DEFORM, and throws an error because it can't find any active vertex groups. To fix this, we're now checking to see if any bone is actively deforming before switching toWT_VGROUP_BONE_DEFORM (else defaulting to WT_VGROUP_ALL)

Nate Rupsis added the

Module

Animation & Rigging

label 2023-09-21 03:58:49 +02:00

Sybren A. Stüvel reviewed 2023-09-22 08:42:02 +02:00

source/blender/editors/object/object_vgroup.cc Outdated

						
				@ -2897,2 +2895,2 @@

				               "group_select_mode",

				               BKE_modifiers_is_deformed_by_armature(ob) ? WT_VGROUP_BONE_DEFORM : WT_VGROUP_ALL);

				  /* Default to All Groups. */

				  eVGroupSelect targetGroup = WT_VGROUP_ALL;

Sybren A. Stüvel commented

2023-09-22 08:42:02 +02:00

This functionality is at a different abstraction level than the surrounding code. Because of that, I think it's better to put it into a little static function of its own that returns either WT_VGROUP_BONE_DEFORM or WT_VGROUP_ALL.

This functionality is at a different abstraction level than the surrounding code. Because of that, I think it's better to put it into a little `static` function of its own that returns either `WT_VGROUP_BONE_DEFORM` or `WT_VGROUP_ALL`.

Nate Rupsis commented

2023-09-25 19:10:53 +02:00

That's a good call, and the reason this is still a WIP. It's a bit ugly at the moment.

👍 1

dr.sybren marked this conversation as resolved

Nate Rupsis changed title from ~~WIP: FIX #110946: vgroup normalize all check if armature is deforming before normalizing~~ to FIX #110946: vgroup normalize all check if armature is deforming before normalizing

2023-09-27 03:01:45 +02:00

Nate Rupsis requested review from Sybren A. Stüvel 2023-09-27 03:02:02 +02:00

Nate Rupsis requested review from Daniel Salazar 2023-09-27 03:02:02 +02:00

Nate Rupsis commented

2023-09-27 03:02:26 +02:00

@blender-bot package

Blender Bot commented

2023-09-27 03:02:30 +02:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR112648) when ready.

Nate Rupsis referenced this pull request

2023-09-27 03:03:44 +02:00

Normalize Vertex Groups while Armature modifier is hidden #110946

Sybren A. Stüvel requested changes 2023-09-28 11:52:36 +02:00

Sybren A. Stüvel left a comment

Thanks, I can confirm this fixes things :)

Please rebase this PR onto the blender-v4.0-release branch. This takes two steps:

Actually rebase your branch & force-push
Edit this PR so that Gitea also understands it shouldn't go onto main but onto the release branch.

More info on https://wiki.blender.org/wiki/Process/Using_Stabilizing_Branch

Thanks, I can confirm this fixes things :) Please rebase this PR onto the `blender-v4.0-release` branch. This takes two steps: - Actually rebase your branch & force-push - Edit this PR so that Gitea also understands it shouldn't go onto `main` but onto the release branch. More info on https://wiki.blender.org/wiki/Process/Using_Stabilizing_Branch

source/blender/editors/object/object_vgroup.cc Outdated

						
				@ -2891,0 +2901,4 @@

				  if (BKE_modifiers_is_deformed_by_armature(ob)) {

				    bool *defgroup_validmap = nullptr;

				    int r_defgroup_tot = BKE_object_defgroup_count(ob);

Sybren A. Stüvel commented

2023-09-28 11:44:12 +02:00

r_defgroup_tot is not used as a return parameter anywhere, so remove the r_ prefix and make it const.

`r_defgroup_tot` is not used as a return parameter anywhere, so remove the `r_` prefix and make it `const`.

nrupsis marked this conversation as resolved

source/blender/editors/object/object_vgroup.cc Outdated

						
				@ -2891,0 +2903,4 @@

				    int r_defgroup_tot = BKE_object_defgroup_count(ob);

				    defgroup_validmap = BKE_object_defgroup_validmap_get(ob, r_defgroup_tot);

Sybren A. Stüvel commented

2023-09-28 11:39:31 +02:00

I think these three lines are tightly related, so better to remove the newlines between them. Also it shows that defgroup_validmap is not used between its initialisation to nullptr and its later assignment, so you can shorten it further to:

    int r_defgroup_tot = BKE_object_defgroup_count(ob);
    bool *defgroup_validmap = BKE_object_defgroup_validmap_get(ob, r_defgroup_tot);

I think these three lines are tightly related, so better to remove the newlines between them. Also it shows that `defgroup_validmap` is not used between its initialisation to `nullptr` and its later assignment, so you can shorten it further to: ```cpp int r_defgroup_tot = BKE_object_defgroup_count(ob); bool *defgroup_validmap = BKE_object_defgroup_validmap_get(ob, r_defgroup_tot); ```

source/blender/editors/object/object_vgroup.cc Outdated

						
				@ -2891,0 +2906,4 @@

				    defgroup_validmap = BKE_object_defgroup_validmap_get(ob, r_defgroup_tot);

				    for (int i = 0; i < r_defgroup_tot; i++) {

				      if (defgroup_validmap[i] == true) {

Sybren A. Stüvel commented

2023-09-28 11:44:42 +02:00

Usually I'm not a fan of == true, but here it kinda helps to drive the point home that this is a boolean array.

Usually I'm not a fan of `== true`, but here it kinda helps to drive the point home that this is a boolean array.

Nate Rupsis changed title from ~~FIX #110946: vgroup normalize all check if armature is deforming before normalizing~~ to FIX #110946: vgroup normalize all check if armature is deforming before normalizing

2023-09-28 20:19:40 +02:00

nrupsis changed target branch from main to blender-v4.0-release

2023-09-28 20:19:42 +02:00

Nate Rupsis force-pushed 110946-Normalize-V-Groups-Armature-Mod-Hidden from 0d6121e346 to a286b90ff8

2023-09-28 20:21:16 +02:00

Compare

Nate Rupsis commented

2023-09-28 20:25:37 +02:00

Looks like I switched the PR target before the rebase took effect 😬 sorry for the clutter.

Nate Rupsis added 1 commit 2023-09-28 22:54:16 +02:00

5b32fc4ea2 slight cleanup

Sybren A. Stüvel approved these changes 2023-09-29 08:41:37 +02:00

Sybren A. Stüvel left a comment

Looks like I switched the PR target before the rebase took effect 😬 sorry for the clutter.

No fuss, that happens.

The patch works like a charm! Thanks :)

> Looks like I switched the PR target before the rebase took effect 😬 sorry for the clutter. No fuss, that happens. The patch works like a charm! Thanks :)

Nate Rupsis merged commit d1f250c0bc into blender-v4.0-release

2023-09-29 15:32:08 +02:00

Nate Rupsis referenced this issue from a commit

2023-09-29 15:32:09 +02:00

FIX #110946: vgroup normalize all check if armature is deforming before normalizing

Nate Rupsis deleted branch 110946-Normalize-V-Groups-Armature-Mod-Hidden

2023-09-29 15:32:12 +02:00

Daniel Salazar refused to review 2023-10-02 23:57:21 +02:00

Joseph Eagar referenced this issue from a commit

2023-10-07 21:46:16 +02:00

FIX #110946: vgroup normalize all check if armature is deforming before normalizing

Joseph Eagar referenced this issue from a commit