Vertex groups with identical names can exist, and when they do, a lot of things break. #93087

New Issue

Nathan Vasil · 2021-11-15T07:13:59+01:00

Nathan Vasil commented

2021-11-15 07:13:59 +01:00

Renaming a bone of an armature also renames the matching vertex group of linked objects/obdata, if found.

Currently there is no check made for potential collisions in vgroups names, which can lead to two or more vgroups having the exact same name, which is considered as invalid data.

This could be solved in several ways:

Rename the existing/clashing vertex group (and make sure all usages elsewhere are updated along)
Change the specified bone name to be unique in a way that it does not conflict with any existing vertex group name.
Merge the (renamed) vertex group with the already existing one with that new name.
Refuse to rename the bone in case it would conflict with another existing vgroup name.

Original Report

System Information
Operating system: Windows-10-10.0.18362-SP0 64 Bits
Graphics card: GeForce GTX 1070/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 456.71

Blender Version
Broken: version: 2.93.5, branch: master, commit date: 2021-10-05 12:04, hash: a791bdabd0
Broken: version: 2.83.0, branch: master, commit date: 2020-06-03 14:38, hash: 211b6c29f7
Didn't test earlier than that (thought 2.79 is too different to matter); didn't test intervening versions; 2.83.0 is just the oldest version since 2.80 that got saved in my Blender folder.

Short description of error
Renaming bones in an armature renames vertex groups of child mesh objects without dealing with name collision. When that happens, a lot (most?) functions involving vertex group weights no longer work correctly-- it appears that they assume vertex group names are unique identifiers.

Exact steps for others to reproduce the error

Make a monkey. Give it a vertex group. It's named "Group".
Create an armature. Parent the monkey to the armature with automatic weights. Monkey now has VGs "Group" and "Bone".
Enter edit on the armature. In properties/bone/name, change the name of Bone to "Group".

Monkey now has 2 VGs named "Group".

And, when they get created, stuff breaks. I consider the real bug here to be, "Vertex groups can acquire identical names", but one could also consider the bug to be, "Multiple operations assume VG names to be unique identifiers when they are not necessarily unique."

Here's a quick screenshot of a bugged mesh (not one that I made.) Notice the sidebar list of vertex groups: we've got three different duplicated vertex groups in that list, by my count.

What breaks when this happens? The first obvious thing, apparent from the screenshot, is interpolation of vertex groups from a subdivision modifier. We can see that the selected vertex group is no longer interpolating smoothly between verts.

What else breaks? A "normalize all" operation no longer actually normalizes all vertex groups properly. Post normalization, these don't add up to 1:

I'm sure that there are other operations that would stop working as well-- those are just the quickies that I noticed. If subdivision interpolation doesn't work right, I wouldn't expect poke faces interpolation or inset faces interpolation to work any better, for example. But I haven't tested all that.

I'd consider this a really low priority bug. The problem doesn't actually show up that often (I'd estimate five times in five years for me.) And it's reasonably easy to work around, provided you're willing to throw your weights away-- you just delete all groups and start again. If unwilling to delete all groups, you can probably get what you want with some math on the VGs (with a VG mix modifier or with geo nodes) and then delete only the problem groups.

Unfortunately, the one big problem with this bug is that it's not very apparent when it happens, which can lead to extra wasted work.

Renaming a bone of an armature also renames the matching vertex group of linked objects/obdata, if found. Currently there is no check made for potential collisions in vgroups names, which can lead to two or more vgroups having the exact same name, which is considered as invalid data. This could be solved in several ways: - Rename the existing/clashing vertex group *(and make sure all usages elsewhere are updated along)* - Change the specified bone name to be unique in a way that it does not conflict with any existing vertex group name. - Merge the (renamed) vertex group with the already existing one with that new name. - Refuse to rename the bone in case it would conflict with another existing vgroup name. -------------------------- *Original Report* **System Information** Operating system: Windows-10-10.0.18362-SP0 64 Bits Graphics card: GeForce GTX 1070/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 456.71 **Blender Version** Broken: version: 2.93.5, branch: master, commit date: 2021-10-05 12:04, hash: `a791bdabd0` Broken: version: 2.83.0, branch: master, commit date: 2020-06-03 14:38, hash: `211b6c29f7` Didn't test earlier than that (thought 2.79 is too different to matter); didn't test intervening versions; 2.83.0 is just the oldest version since 2.80 that got saved in my Blender folder. **Short description of error** Renaming bones in an armature renames vertex groups of child mesh objects without dealing with name collision. When that happens, a lot (most?) functions involving vertex group weights no longer work correctly-- it appears that they assume vertex group names are unique identifiers. **Exact steps for others to reproduce the error** 1) Make a monkey. Give it a vertex group. It's named "Group". 2) Create an armature. Parent the monkey to the armature with automatic weights. Monkey now has VGs "Group" and "Bone". 3) Enter edit on the armature. In properties/bone/name, change the name of Bone to "Group". Monkey now has 2 VGs named "Group". And, when they get created, stuff breaks. I consider the real bug here to be, "Vertex groups can acquire identical names", but one could also consider the bug to be, "Multiple operations assume VG names to be unique identifiers when they are not necessarily unique." ![image.png](https://archive.blender.org/developer/F11806731/image.png) Here's a quick screenshot of a bugged mesh (not one that I made.) Notice the sidebar list of vertex groups: we've got three different duplicated vertex groups in that list, by my count. What breaks when this happens? The first obvious thing, apparent from the screenshot, is interpolation of vertex groups from a subdivision modifier. We can see that the selected vertex group is no longer interpolating smoothly between verts. What else breaks? A "normalize all" operation no longer actually normalizes all vertex groups properly. Post normalization, these don't add up to 1: ![image.png](https://archive.blender.org/developer/F11806915/image.png) I'm sure that there are other operations that would stop working as well-- those are just the quickies that I noticed. If subdivision interpolation doesn't work right, I wouldn't expect poke faces interpolation or inset faces interpolation to work any better, for example. But I haven't tested all that. I'd consider this a really low priority bug. The problem doesn't actually show up that often (I'd estimate five times in five years for me.) And it's reasonably easy to work around, provided you're willing to throw your weights away-- you just delete all groups and start again. If unwilling to delete all groups, you can probably get what you want with some math on the VGs (with a VG mix modifier or with geo nodes) and then delete only the problem groups. Unfortunately, the one big problem with this bug is that it's not very apparent when it happens, which can lead to extra wasted work.

Nathan Vasil commented

2021-11-15 07:13:59 +01:00

Added subscriber: @vasiln

Philipp Oeser commented

2021-11-15 11:16:39 +01:00

Added subscribers: @HooglyBoogly, @mont29, @lichtwerk

Philipp Oeser commented

2021-11-15 11:16:39 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser commented

2021-11-15 11:16:39 +01:00

Can confirm in that file (do think this should be looked at though).

Just at a very quick glance, I can see that BKE_defgroup_duplicate / BKE_defgroup_copy_list does not do BKE_object_defgroup_unique_name

Used here:

mesh_copy_data (indirectly used when doing BKE_id_copy on a mesh)
BKE_mesh_copy_parameters_for_eval
BKE_mesh_nomain_to_mesh

Sourrounding code suggests though that this is handled properly (e.g. BKE_defgroup_copy_list clears before (BLI_listbase_clear)

Maybe this rings a bell with @mont29 or @HooglyBoogly ?

Will confirm this for now (even though clear reproductions steps have not been found -- feel free to set this back to Needs Information from User if this is totally required)

Can confirm in that file (do think this should be looked at though). Just at a very quick glance, I can see that `BKE_defgroup_duplicate` / `BKE_defgroup_copy_list` does not do `BKE_object_defgroup_unique_name` Used here: - `mesh_copy_data` (indirectly used when doing `BKE_id_copy` on a mesh) - `BKE_mesh_copy_parameters_for_eval` - `BKE_mesh_nomain_to_mesh` Sourrounding code suggests though that this is handled properly (e.g. `BKE_defgroup_copy_list` clears before (`BLI_listbase_clear`) Maybe this rings a bell with @mont29 or @HooglyBoogly ? Will confirm this for now (even though clear reproductions steps have not been found -- feel free to set this back to `Needs Information from User` if this is totally required)

Bastien Montagne commented

2021-11-15 14:40:07 +01:00

Changed status from 'Confirmed' to: 'Needs User Info'

Bastien Montagne commented

2021-11-15 14:40:07 +01:00

We indeed need a reproducible case, this should not happen, but there is no real way to investigate this just from the broken result blend file alone.

Nathan Vasil commented

2021-11-15 17:14:31 +01:00

This comment was removed by @vasiln

*This comment was removed by @vasiln*

Nathan Vasil commented

2021-11-15 18:26:15 +01:00

Okay, I figured out a way to reproduce this:

Create a monkey. Give it a vertex group (named "Group" is fine.)
Create an armature. Parent your monkey to the armature with automatic weights. Monkey now has VGs "Group" and "Bone".
Enter edit on the armature. Rename Bone to Group. (I did this by typing a new name in the name field in properties/bone/name.)

Monkey now has two VGs, each named "Group".

I've edited the task description to reflect this.

Okay, I figured out a way to reproduce this: 1) Create a monkey. Give it a vertex group (named "Group" is fine.) 2) Create an armature. Parent your monkey to the armature with automatic weights. Monkey now has VGs "Group" and "Bone". 3) Enter edit on the armature. Rename Bone to Group. (I did this by typing a new name in the name field in properties/bone/name.) Monkey now has two VGs, each named "Group". I've edited the task description to reflect this.

Philipp Oeser self-assigned this 2021-11-15 18:58:43 +01:00

Philipp Oeser commented

2021-11-15 18:58:43 +01:00

Changed status from 'Needs User Info' to: 'Confirmed'

Philipp Oeser commented

2021-11-15 18:58:43 +01:00

Oh boy, that was almost too easy to break... thx for the repro steps.
Cant believe this has not been reported more often (this goes back to 2.79 even).
Will check on a fix.

Oh boy, that was almost too easy to break... thx for the repro steps. Cant believe this has not been reported more often (this goes back to 2.79 even). Will check on a fix.

Bastien Montagne commented

2021-11-16 08:55:01 +01:00

@lichtwerk please do not add #core to all and everything. This is strictly anim/mesh edit related, core has nothing to do here (this is not ID management issue, but internal data of some specific IDs).

Philipp Oeser commented

2021-11-16 09:21:30 +01:00

In #93087#1254268, @mont29 wrote:
@lichtwerk please do not add #core to all and everything. This is strictly anim/mesh edit related, core has nothing to do here (this is not ID management issue, but internal data of some specific IDs).

Noted (I was assuming duplicate names were touching #Core, but right, this is not about IDs per se), appologies.

> In #93087#1254268, @mont29 wrote: > @lichtwerk please do not add #core to all and everything. This is strictly anim/mesh edit related, core has nothing to do here (this is not ID management issue, but internal data of some specific IDs). Noted (I was assuming duplicate names were touching #Core, but right, this is not about IDs per se), appologies.

Philipp Oeser commented

2021-11-16 14:44:49 +01:00

So while it would be easy to rename a existing/clashing vertex group:

P2598: T93087_snippet



diff --git a/source/blender/editors/armature/armature_naming.c b/source/blender/editors/armature/armature_naming.c
index de1c14a15ce..e3b583d53b6 100644
--- a/source/blender/editors/armature/armature_naming.c
+++ b/source/blender/editors/armature/armature_naming.c
@@ -266,10 +266,15 @@ void ED_armature_bone_rename(Main *bmain,
       }
 
       if (BKE_modifiers_uses_armature(ob, arm) && BKE_object_supports_vertex_groups(ob)) {
+        /* In case of a name clash (equally named vertex group already exists), we give the existing one a unique name. */
+        bDeformGroup *dg_clash = BKE_object_defgroup_find_name(ob, newname);
         bDeformGroup *dg = BKE_object_defgroup_find_name(ob, oldname);
         if (dg) {
           BLI_strncpy(dg->name, newname, MAXBONENAME);
         }
+        if (dg_clash) {
+          BKE_object_defgroup_unique_name(dg_clash, ob);
+        }
       }
 
       /* fix modifiers that might be using this name */

I am not sure if this is the right solution.
This existing/clashing vertex group could be in use elsewhere (e.g. in a modifier) and since this is all name based the other place would then break (modifier would all of a sudden use a different vertex group without the user being aware -- which is equally bad).

Seems there is not a real system in place that handles name changes of vertex groups (in the sense that all usages of that vertex group's name would be updated).
(Part of ED_armature_bone_rename could be extracted int a separate function that takes care of this.)
And even if such a system would be there, it is always questionable what to rename in case of clashes, see also #71244 (Renaming an ID through RNA does not consistently increment OTHER or CURRENT ID in case of name collision).

So I think there are these possibilities:

rename the existing/clashing vertex group (but make sure all usages elsewhere are updated along)
change the specified bone name to be unique in a way that it does not conflict with a possible clashing vertex group name)
would be good if there is another solution, because both of the above seem awkward

So I think this needs a design decision on how we handle name clashes like these, will leave up to module devs to decide and step down to not block others looking at this.

So while it would be easy to rename a existing/clashing vertex group: [P2598: T93087_snippet](https://archive.blender.org/developer/P2598.txt) ``` diff --git a/source/blender/editors/armature/armature_naming.c b/source/blender/editors/armature/armature_naming.c index de1c14a15ce..e3b583d53b6 100644 --- a/source/blender/editors/armature/armature_naming.c +++ b/source/blender/editors/armature/armature_naming.c @@ -266,10 +266,15 @@ void ED_armature_bone_rename(Main *bmain, } if (BKE_modifiers_uses_armature(ob, arm) && BKE_object_supports_vertex_groups(ob)) { + /* In case of a name clash (equally named vertex group already exists), we give the existing one a unique name. */ + bDeformGroup *dg_clash = BKE_object_defgroup_find_name(ob, newname); bDeformGroup *dg = BKE_object_defgroup_find_name(ob, oldname); if (dg) { BLI_strncpy(dg->name, newname, MAXBONENAME); } + if (dg_clash) { + BKE_object_defgroup_unique_name(dg_clash, ob); + } } /* fix modifiers that might be using this name */ ``` I am not sure if this is the right solution. This existing/clashing vertex group could be in use elsewhere (e.g. in a modifier) and since this is all name based the other place would then break (modifier would all of a sudden use a different vertex group without the user being aware -- which is equally bad). Seems there is not a real system in place that handles name changes of vertex groups (in the sense that all usages of that vertex group's name would be updated). (Part of `ED_armature_bone_rename` could be extracted int a separate function that takes care of this.) And even if such a system would be there, it is always questionable what to rename in case of clashes, see also #71244 (Renaming an ID through RNA does not consistently increment OTHER or CURRENT ID in case of name collision). So I think there are these possibilities: - rename the existing/clashing vertex group (but make sure all usages elsewhere are updated along) - change the specified bone name to be unique in a way that it does not conflict with a possible clashing vertex group name) - would be good if there is another solution, because both of the above seem awkward So I think this needs a design decision on how we handle name clashes like these, will leave up to module devs to decide and step down to not block others looking at this.

Philipp Oeser removed their assignment 2021-11-16 14:46:50 +01:00

Bastien Montagne commented

2021-11-16 15:00:24 +01:00

Think this indeed needs a design work & decision first, probably for the #animation_rigging module. Will update the task accordingly.

Bastien Montagne commented

2021-11-16 15:10:34 +01:00

Personally I think I would go for solution #2, or #4 if it is not easily doable.

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Vertex groups with identical names can exist, and when they do, a lot of things break. #93087