Fix T92083: Crash renaming bone used in Armature modifier on curve
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Oct 15 2021, 11:25 AM.

Details

Reviewers

Jacques Lucke (JacquesLucke)
Hans Goudey (HooglyBoogly)

Maniphest Tasks

T92083: Crash when renaming bone if parented to curve

Commits

rB7bf9c70b1402: Fix T92083: Crash renaming bone used in Armature modifier on curve

Summary

This is caused by rB3b6ee8cee708: Refactor: Move vertex group names to object data.

Since rigging curves with armatures only works with envelopes (if I am
not mistaken), this stirs up the question again why we actually give the
choice for vertex groups in parenting.
Anyways, curves can have armature modifiers and renaming bones should not
crash.

Now remove the vertex group support assert in
BKE_object_defgroup_list, return NULL instead (and check for this in
BKE_object_defgroup_find_name).

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Oct 15 2021, 11:25 AM

Harbormaster completed remote builds in B17864: Diff 43361.Oct 15 2021, 11:25 AM

Philipp Oeser (lichtwerk) requested review of this revision.Oct 15 2021, 11:25 AM

Jacques Lucke (JacquesLucke) accepted this revision.Oct 15 2021, 11:40 AM

Jacques Lucke (JacquesLucke) added 1 blocking reviewer(s): Hans Goudey (HooglyBoogly).

I don't really understand why this ends up getting called for a curve object (I added the assert so we could try to avoid doing that, since I figured it meant that the caller was making incorrect assumptions).
However, I suppose it's reasonable to check, since there are bound to be other weird situations like there are in the report.

This revision is now accepted and ready to land.Oct 15 2021, 4:11 PM

In D12876#337979, @Hans Goudey (HooglyBoogly) wrote:

I don't really understand why this ends up getting called for a curve object (I added the assert so we could try to avoid doing that, since I figured it meant that the caller was making incorrect assumptions).
However, I suppose it's reasonable to check, since there are bound to be other weird situations like there are in the report.

We can also put the check in the caller if that is preferred:

P2506 T92083_alternative

1
2
3	diff --git a/source/blender/editors/armature/armature_naming.c b/source/blender/editors/armature/armature_naming.c
4	index 35bd30377c8..de1c14a15ce 100644
5	--- a/source/blender/editors/armature/armature_naming.c
6	+++ b/source/blender/editors/armature/armature_naming.c
7	@@ -265,7 +265,7 @@ void ED_armature_bone_rename(Main *bmain,
8	}
9	}
10
11	- if (BKE_modifiers_uses_armature(ob, arm)) {
12	+ if (BKE_modifiers_uses_armature(ob, arm) && BKE_object_supports_vertex_groups(ob)) {
13	bDeformGroup *dg = BKE_object_defgroup_find_name(ob, oldname);
14	if (dg) {
15	BLI_strncpy(dg->name, newname, MAXBONENAME);