GPv3: Armature Modifier #118752

Lukas Tönne · 2024-02-26T16:53:12+01:00

Lukas Tönne commented

2024-02-26 16:53:12 +01:00

Armature deformation modifier for Grease Pencil v3.

Changes compared to GPv2:

multi DNA field was unused and was removed.
vert_coords_prev array is unused and was removed (gets passed to armature functions but never gets allocated).
GPv3 modifier uses the common influence struct to store the vertex group name, for consistency. The GREASE_PENCIL_INFLUENCE_INVERT_VERTEX_GROUP flag is copied to deformflag as ARM_DEF_INVERT_VGROUP before evaluation, which is used internally by armature functions.
BKE_armature_deform_coords_with_curves is added as another variant of the deform function, but uses C++ parameter types (spans instead of raw pointers). It gets a Span<MDeformVert> directly instead of deducing it internally from the object type. This is because we want to do this curve-by-curve and already use arbitrary vector spans for positions.

Armature deformation modifier for Grease Pencil v3. Changes compared to GPv2: - `multi` DNA field was unused and was removed. - `vert_coords_prev` array is unused and was removed (gets passed to armature functions but never gets allocated). - GPv3 modifier uses the common `influence` struct to store the vertex group name, for consistency. The `GREASE_PENCIL_INFLUENCE_INVERT_VERTEX_GROUP` flag is copied to `deformflag` as `ARM_DEF_INVERT_VGROUP` before evaluation, which is used internally by armature functions. - `BKE_armature_deform_coords_with_curves` is added as another variant of the deform function, but uses C++ parameter types (spans instead of raw pointers). It gets a `Span<MDeformVert>` directly instead of deducing it internally from the object type. This is because we want to do this curve-by-curve and already use arbitrary vector spans for positions.

Lukas Tönne added 2 commits 2024-02-26 16:53:22 +01:00

529a4d5304 Implementation of armature deformation for GPv3.

b67d3e9c8d Merge branch 'main' into gp3-armature-modifier

Lukas Tönne added this to the Grease Pencil project 2024-02-26 16:53:25 +01:00

Hans Goudey reviewed 2024-02-26 16:58:20 +01:00

source/blender/blenkernel/intern/armature_deform.cc Outdated

						
				@ -660,0 +671,4 @@

				      vert_coords.size(),

				      deformflag,

				      vert_coords_prev ? reinterpret_cast<float(*)[3]>(vert_coords_prev->data()) : nullptr,

				      defgrp_name.data(),

Hans Goudey commented

2024-02-26 16:56:34 +01:00

Generally calling .data() for something that expects a C-string is wrong, since StringRef doesn't guarantee the null terminator. That probably just happened to work for the strings used so far. I'd use StringRefNull here.

Generally calling `.data()` for something that expects a C-string is wrong, since `StringRef` doesn't guarantee the null terminator. That probably just happened to work for the strings used so far. I'd use `StringRefNull` here.

LukasTonne marked this conversation as resolved

source/blender/modifiers/intern/MOD_grease_pencil_armature.cc

						
				@ -0,0 +120,4 @@

				  curves_mask.foreach_index(blender::GrainSize(128), [&](const int curve_i) {

				    const IndexRange points = points_by_curve[curve_i];

				    /* deform verts */

Hans Goudey commented

2024-02-26 16:57:25 +01:00

This comment could just be removed?

LukasTonne marked this conversation as resolved

Hans Goudey reviewed 2024-02-26 16:58:49 +01:00

source/blender/modifiers/intern/MOD_grease_pencil_armature.cc

						
				@ -0,0 +138,4 @@

				                                const ModifierEvalContext *ctx,

				                                bke::GeometrySet *geometry_set)

				{

				  auto *amd = reinterpret_cast<GreasePencilArmatureModifierData *>(md);