From 49965841bb08a0ec91d3506d69a8ebfecd418671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Thu, 12 Sep 2024 17:35:35 +0200 Subject: [PATCH] Fix: GPv3: Armature deform using the wrong vertex group list Grease Pencil armature deformation has to use the correct `vertex_group_names` list from the `Curves` (i.e. drawings) rather than the names from its parent `GreasePencil` data block. The ancient armature deformation code used a `defbase` list (`ListBase` of type `bDeformGroup`) from the object via `BKE_id_defgroup_list_get`. This function does not have enough information to determine the correct vertex groups for GPv3, and it's use inside supposedly generic code is highly questionable. This patch removes the `defbase` lookup inside `armature_deform_coords_impl` and instead passes an explicit list of vertex group names to be used for resolving indices. We already have context in every place this is called to determine the correct vertex groups, without having to make assumptions down the line based on just the object type. Pull Request: https://projects.blender.org/blender/blender/pulls/127509 --- source/blender/blenkernel/BKE_armature.hh | 1 + .../blenkernel/intern/armature_deform.cc | 66 +++++++++---------- .../intern/MOD_grease_pencil_armature.cc | 1 + 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/source/blender/blenkernel/BKE_armature.hh b/source/blender/blenkernel/BKE_armature.hh index 0c3bf459a8e..131e310ec93 100644 --- a/source/blender/blenkernel/BKE_armature.hh +++ b/source/blender/blenkernel/BKE_armature.hh @@ -642,6 +642,7 @@ void BKE_armature_deform_coords_with_gpencil_stroke(const Object *ob_arm, void BKE_armature_deform_coords_with_curves( const Object &ob_arm, const Object &ob_target, + const ListBase *defbase, blender::MutableSpan vert_coords, std::optional> vert_coords_prev, std::optional> vert_deform_mats, diff --git a/source/blender/blenkernel/intern/armature_deform.cc b/source/blender/blenkernel/intern/armature_deform.cc index 60246ab63cf..dab72be491f 100644 --- a/source/blender/blenkernel/intern/armature_deform.cc +++ b/source/blender/blenkernel/intern/armature_deform.cc @@ -481,6 +481,7 @@ static void armature_vert_task_editmesh_no_dvert(void *__restrict userdata, static void armature_deform_coords_impl(const Object *ob_arm, const Object *ob_target, + const ListBase *defbase, float (*vert_coords)[3], float (*vert_deform_mats)[3][3], const int vert_coords_len, @@ -489,8 +490,7 @@ static void armature_deform_coords_impl(const Object *ob_arm, const char *defgrp_name, blender::Span dverts, const Mesh *me_target, - const BMEditMesh *em_target, - bGPDstroke *gps_target) + const BMEditMesh *em_target) { const bArmature *arm = static_cast(ob_arm->data); bPoseChannel **pchan_from_defbase = nullptr; @@ -515,31 +515,8 @@ static void armature_deform_coords_impl(const Object *ob_arm, } if (BKE_object_supports_vertex_groups(ob_target)) { - const ID *target_data_id = nullptr; - if (ob_target->type == OB_MESH) { - target_data_id = me_target == nullptr ? (const ID *)ob_target->data : &me_target->id; - if (em_target == nullptr) { - const Mesh *mesh = (const Mesh *)target_data_id; - dverts = mesh->deform_verts(); - } - } - else if (ob_target->type == OB_LATTICE) { - const Lattice *lt = static_cast(ob_target->data); - target_data_id = (const ID *)ob_target->data; - dverts = blender::Span(lt->dvert, lt->pntsu * lt->pntsv * lt->pntsw); - } - else if (ob_target->type == OB_GPENCIL_LEGACY) { - target_data_id = (const ID *)ob_target->data; - dverts = blender::Span(gps_target->dvert, gps_target->totpoints); - } - else if (ob_target->type == OB_GREASE_PENCIL) { - target_data_id = (const ID *)ob_target->data; - BLI_assert(dverts.size() == vert_coords_len); - } - /* Collect the vertex group names from the evaluated data. */ - armature_def_nr = BKE_id_defgroup_name_index(target_data_id, defgrp_name); - const ListBase *defbase = BKE_id_defgroup_list_get(target_data_id); + armature_def_nr = BLI_findstringindex(defbase, defgrp_name, offsetof(bDeformGroup, name)); defbase_len = BLI_listbase_count(defbase); /* get a vertex-deform-index to posechannel array */ @@ -639,23 +616,26 @@ void BKE_armature_deform_coords_with_gpencil_stroke(const Object *ob_arm, const char *defgrp_name, bGPDstroke *gps_target) { + const ListBase *defbase = BKE_id_defgroup_list_get(static_cast(ob_target->data)); + const blender::Span dverts = {gps_target->dvert, gps_target->totpoints}; armature_deform_coords_impl(ob_arm, ob_target, + defbase, vert_coords, vert_deform_mats, vert_coords_len, deformflag, vert_coords_prev, defgrp_name, - {}, + dverts, nullptr, - nullptr, - gps_target); + nullptr); } void BKE_armature_deform_coords_with_curves( const Object &ob_arm, const Object &ob_target, + const ListBase *defbase, blender::MutableSpan vert_coords, std::optional> vert_coords_prev, std::optional> vert_deform_mats, @@ -663,9 +643,14 @@ void BKE_armature_deform_coords_with_curves( int deformflag, blender::StringRefNull defgrp_name) { + /* Vertex groups must be provided explicitly, cannot rely on object vertex groups since this is + * used for Grease Pencil layers as well. */ + BLI_assert(dverts.size() == vert_coords.size()); + armature_deform_coords_impl( &ob_arm, &ob_target, + defbase, reinterpret_cast(vert_coords.data()), vert_deform_mats ? reinterpret_cast(vert_deform_mats->data()) : nullptr, vert_coords.size(), @@ -674,7 +659,6 @@ void BKE_armature_deform_coords_with_curves( defgrp_name.c_str(), dverts, nullptr, - nullptr, nullptr); } @@ -688,17 +672,30 @@ void BKE_armature_deform_coords_with_mesh(const Object *ob_arm, const char *defgrp_name, const Mesh *me_target) { + const ListBase *defbase = BKE_id_defgroup_list_get(static_cast(ob_target->data)); + blender::Span dverts; + if (ob_target->type == OB_MESH) { + if (me_target == nullptr) { + me_target = static_cast(ob_target->data); + } + dverts = me_target->deform_verts(); + } + else if (ob_target->type == OB_LATTICE) { + const Lattice *lt = static_cast(ob_target->data); + dverts = blender::Span(lt->dvert, lt->pntsu * lt->pntsv * lt->pntsw); + } + armature_deform_coords_impl(ob_arm, ob_target, + defbase, vert_coords, vert_deform_mats, vert_coords_len, deformflag, vert_coords_prev, defgrp_name, - {}, + dverts, me_target, - nullptr, nullptr); } @@ -712,8 +709,10 @@ void BKE_armature_deform_coords_with_editmesh(const Object *ob_arm, const char *defgrp_name, const BMEditMesh *em_target) { + const ListBase *defbase = BKE_id_defgroup_list_get(static_cast(ob_target->data)); armature_deform_coords_impl(ob_arm, ob_target, + defbase, vert_coords, vert_deform_mats, vert_coords_len, @@ -722,8 +721,7 @@ void BKE_armature_deform_coords_with_editmesh(const Object *ob_arm, defgrp_name, {}, nullptr, - em_target, - nullptr); + em_target); } /** \} */ diff --git a/source/blender/modifiers/intern/MOD_grease_pencil_armature.cc b/source/blender/modifiers/intern/MOD_grease_pencil_armature.cc index 1960a323bc1..b8d3855fe2e 100644 --- a/source/blender/modifiers/intern/MOD_grease_pencil_armature.cc +++ b/source/blender/modifiers/intern/MOD_grease_pencil_armature.cc @@ -134,6 +134,7 @@ static void modify_curves(ModifierData &md, const ModifierEvalContext &ctx, Draw BKE_armature_deform_coords_with_curves(*amd.object, *ctx.object, + &curves.vertex_group_names, positions.slice(points), std::nullopt, std::nullopt,