griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #131573: Ensure vertex group attributes when subdividing Grease Pencil strokes

Browse Source 
In order to copy vertex group weights when subdividing strokes the
groups (`bDeformGroup`) must be created in advance, so that the attribute
wrappers correctly write to `CD_MDEFORMVERT` layer instead of creating simple
`CD_PROP_FLOAT` layers.

In addition the subdivision function must take care to fully write the
destination arrays, since initial values must be considered uninitialized (this
is obfuscated for simple CustomData arrays but breaks with more complex
attributes that use a buffer). In order to ensure fully defined destination
buffers without additional copies, the `finish` call to attribute writers is
postponed until the unselected attribute values have also been copied from input
buffers and all the values are properly defined.

Pull Request: https://projects.blender.org/blender/blender/pulls/132854
This commit is contained in:
Lukas Tönne
2025-01-13 09:45:11 +01:00
parent a4914b8972
commit 3bdc1bfd0a
5 changed files with 96 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
source/blender/blenkernel/BKE_attribute.hh
View File

@@ -870,6 +870,7 @@ class MutableAttributeAccessor : public AttributeAccessor {
struct AttributeTransferData {
/* Expect that if an attribute exists, it is stored as a contiguous array internally anyway. */
GVArraySpan src;
StringRef name;
AttributeMetaData meta_data;
GSpanAttributeWriter dst;
};

2
source/blender/blenkernel/intern/attribute_access.cc
View File

@@ -870,7 +870,7 @@ Vector<AttributeTransferData> retrieve_attributes_for_transfer(
GVArray src = *iter.get();
GSpanAttributeWriter dst = dst_attributes.lookup_or_add_for_write_only_span(
iter.name, iter.domain, iter.data_type);
attributes.append({std::move(src), {iter.domain, iter.data_type}, std::move(dst)});
attributes.append({std::move(src), iter.name, {iter.domain, iter.data_type}, std::move(dst)});
});
return attributes;
}

9
source/blender/geometry/intern/resample_curves.cc
View File

@@ -16,6 +16,7 @@
#include "BKE_attribute_math.hh"
#include "BKE_curves.hh"
#include "BKE_curves_utils.hh"
#include "BKE_deform.hh"
#include "BKE_geometry_fields.hh"
#include "GEO_resample_curves.hh"
@@ -411,6 +412,8 @@ static CurvesGeometry resample_to_uniform(const CurvesGeometry &src_curves,
const OffsetIndices src_points_by_curve = src_curves.points_by_curve();
CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
fn::FieldEvaluator evaluator{field_context, src_curves.curves_num()};
@@ -444,6 +447,8 @@ CurvesGeometry resample_to_count(const CurvesGeometry &src_curves,
const OffsetIndices src_points_by_curve = src_curves.points_by_curve();
CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
array_utils::copy(counts, selection, dst_offsets);
@@ -490,6 +495,8 @@ CurvesGeometry resample_to_length(const CurvesGeometry &src_curves,
const VArray<bool> curves_cyclic = src_curves.cyclic();
CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
src_curves.ensure_evaluated_lengths();
@@ -540,6 +547,8 @@ CurvesGeometry resample_to_evaluated(const CurvesGeometry &src_curves,
const IndexMask unselected = selection.complement(src_curves.curves_range(), memory);
CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
dst_curves.fill_curve_types(selection, CURVE_TYPE_POLY);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
offset_indices::copy_group_sizes(src_evaluated_points_by_curve, selection, dst_offsets);

116
source/blender/geometry/intern/set_curve_type.cc
View File

@@ -6,6 +6,7 @@
#include "BKE_attribute_math.hh"
#include "BKE_curves.hh"
#include "BKE_curves_utils.hh"
#include "BKE_deform.hh"
#include "BLI_array_utils.hh"
#include "BLI_task.hh"
@@ -279,6 +280,8 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
const IndexMask unselected = selection.complement(src_curves.curves_range(), memory);
bke::CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
dst_curves.fill_curve_types(selection, CURVE_TYPE_BEZIER);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
@@ -299,16 +302,13 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
MutableSpan<float3> dst_handles_r = dst_curves.handle_positions_right_for_write();
MutableSpan<int8_t> dst_types_l = dst_curves.handle_types_left_for_write();
MutableSpan<int8_t> dst_types_r = dst_curves.handle_types_right_for_write();
Set<std::string> attributes_to_skip = {
Vector<bke::AttributeTransferData> generic_attributes = bke::retrieve_attributes_for_transfer(
src_attributes, dst_attributes, ATTR_DOMAIN_MASK_POINT, attribute_filter);
Set<StringRef> attributes_to_skip = {
"position", "handle_type_left", "handle_type_right", "handle_right", "handle_left"};
if (!dst_curves.has_curve_with_type(CURVE_TYPE_NURBS)) {
attributes_to_skip.add_new("nurbs_weight");
}
Vector<bke::AttributeTransferData> generic_attributes = bke::retrieve_attributes_for_transfer(
src_attributes,
dst_attributes,
ATTR_DOMAIN_MASK_POINT,
bke::attribute_filter_with_skip_ref(attribute_filter, attributes_to_skip));
auto catmull_rom_to_bezier = [&](const IndexMask &selection) {
bke::curves::fill_points<int8_t>(
@@ -328,6 +328,9 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
});
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, attribute.src, attribute.dst.span);
}
@@ -342,6 +345,9 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
dst_points_by_curve, selection, BEZIER_HANDLE_VECTOR, dst_types_r);
dst_curves.calculate_bezier_auto_handles();
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, attribute.src, attribute.dst.span);
}
@@ -367,6 +373,9 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
dst_curves.calculate_bezier_auto_handles();
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, attribute.src, attribute.dst.span);
}
@@ -414,6 +423,9 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
});
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
selection.foreach_index(GrainSize(512), [&](const int i) {
const IndexRange src_points = src_points_by_curve[i];
const IndexRange dst_points = dst_points_by_curve[i];
@@ -433,18 +445,12 @@ static bke::CurvesGeometry convert_curves_to_bezier(const bke::CurvesGeometry &s
nurbs_to_bezier);
for (bke::AttributeTransferData &attribute : generic_attributes) {
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, unselected, attribute.src, attribute.dst.span);
attribute.dst.finish();
}
bke::copy_attributes_group_to_group(src_attributes,
bke::AttrDomain::Point,
bke::AttrDomain::Point,
attribute_filter,
src_points_by_curve,
dst_points_by_curve,
unselected,
dst_attributes);
return dst_curves;
}
@@ -461,6 +467,8 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
const IndexMask unselected = selection.complement(src_curves.curves_range(), memory);
bke::CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
dst_curves.fill_curve_types(selection, CURVE_TYPE_NURBS);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
@@ -475,16 +483,13 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
MutableSpan<float3> dst_positions = dst_curves.positions_for_write();
bke::MutableAttributeAccessor dst_attributes = dst_curves.attributes_for_write();
Vector<bke::AttributeTransferData> generic_attributes = bke::retrieve_attributes_for_transfer(
src_attributes,
dst_attributes,
ATTR_DOMAIN_MASK_POINT,
bke::attribute_filter_with_skip_ref(attribute_filter,
{"position",
"handle_type_left",
"handle_type_right",
"handle_right",
"handle_left",
"nurbs_weight"}));
src_attributes, dst_attributes, ATTR_DOMAIN_MASK_POINT, attribute_filter);
const Set<StringRef> attributes_to_skip = {"position",
"handle_type_left",
"handle_type_right",
"handle_right",
"handle_left",
"nurbs_weight"};
auto fill_weights_if_necessary = [&](const IndexMask &selection) {
if (src_attributes.contains("nurbs_weight")) {
@@ -509,6 +514,9 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
});
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
selection.foreach_index(GrainSize(512), [&](const int i) {
const IndexRange src_points = src_points_by_curve[i];
const IndexRange dst_points = dst_points_by_curve[i];
@@ -541,6 +549,9 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
}
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, attribute.src, attribute.dst.span);
}
@@ -565,6 +576,9 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
});
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
selection.foreach_index(GrainSize(512), [&](const int i) {
const IndexRange src_points = src_points_by_curve[i];
const IndexRange dst_points = dst_points_by_curve[i];
@@ -587,6 +601,9 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
}
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, attribute.src, attribute.dst.span);
}
@@ -601,18 +618,12 @@ static bke::CurvesGeometry convert_curves_to_nurbs(const bke::CurvesGeometry &sr
nurbs_to_nurbs);
for (bke::AttributeTransferData &attribute : generic_attributes) {
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, unselected, attribute.src, attribute.dst.span);
attribute.dst.finish();
}
bke::copy_attributes_group_to_group(src_attributes,
bke::AttrDomain::Point,
bke::AttrDomain::Point,
attribute_filter,
src_points_by_curve,
dst_points_by_curve,
unselected,
dst_attributes);
return dst_curves;
}
@@ -649,6 +660,8 @@ static bke::CurvesGeometry convert_curves_to_catmull_rom_or_poly(
const IndexMask unselected = selection.complement(src_curves.curves_range(), memory);
bke::CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
dst_curves.fill_curve_types(selection, dst_type);
MutableSpan<int> dst_offsets = dst_curves.offsets_for_write();
@@ -671,21 +684,21 @@ static bke::CurvesGeometry convert_curves_to_catmull_rom_or_poly(
MutableSpan<float3> dst_positions = dst_curves.positions_for_write();
bke::MutableAttributeAccessor dst_attributes = dst_curves.attributes_for_write();
Vector<bke::AttributeTransferData> generic_attributes = bke::retrieve_attributes_for_transfer(
src_attributes,
dst_attributes,
ATTR_DOMAIN_MASK_POINT,
bke::attribute_filter_with_skip_ref(attribute_filter,
{"position",
"handle_type_left",
"handle_type_right",
"handle_right",
"handle_left",
"nurbs_weight"}));
src_attributes, dst_attributes, ATTR_DOMAIN_MASK_POINT, attribute_filter);
const Set<StringRef> attributes_to_skip = {"position",
"handle_type_left",
"handle_type_right",
"handle_right",
"handle_left",
"nurbs_weight"};
auto convert_from_catmull_rom_or_poly_or_nurbs = [&](const IndexMask &selection) {
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, src_positions, dst_positions);
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, selection, attribute.src, attribute.dst.span);
}
@@ -710,6 +723,9 @@ static bke::CurvesGeometry convert_curves_to_catmull_rom_or_poly(
/* Transfer attributes. The handles the same attribute values as their corresponding control
* point. */
for (bke::AttributeTransferData &attribute : generic_attributes) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
const CPPType &cpp_type = attribute.src.type();
selection.foreach_index([&](const int curve_i) {
const IndexRange src_points = src_points_by_curve[curve_i];
@@ -733,18 +749,12 @@ static bke::CurvesGeometry convert_curves_to_catmull_rom_or_poly(
convert_from_catmull_rom_or_poly_or_nurbs);
for (bke::AttributeTransferData &attribute : generic_attributes) {
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, unselected, attribute.src, attribute.dst.span);
attribute.dst.finish();
}
bke::copy_attributes_group_to_group(src_attributes,
bke::AttrDomain::Point,
bke::AttrDomain::Point,
attribute_filter,
src_points_by_curve,
dst_points_by_curve,
unselected,
dst_attributes);
return dst_curves;
}

50
source/blender/geometry/intern/subdivide_curves.cc
View File

@@ -5,7 +5,9 @@
#include "BKE_attribute_math.hh"
#include "BKE_curves.hh"
#include "BKE_curves_utils.hh"
#include "BKE_deform.hh"
#include "BLI_array_utils.hh"
#include "BLI_task.hh"
#include "GEO_subdivide_curves.hh"
@@ -282,6 +284,8 @@ bke::CurvesGeometry subdivide_curves(const bke::CurvesGeometry &src_curves,
const IndexMask unselected = selection.complement(src_curves.curves_range(), memory);
bke::CurvesGeometry dst_curves = bke::curves::copy_only_curve_domain(src_curves);
/* Copy vertex groups from source curves to allow copying vertex group attributes. */
BKE_defgroup_copy_list(&dst_curves.vertex_group_names, &src_curves.vertex_group_names);
/* For each point, this contains the point offset in the corresponding result curve,
* starting at zero. For example for two curves with four points each, the values might
@@ -315,10 +319,12 @@ bke::CurvesGeometry subdivide_curves(const bke::CurvesGeometry &src_curves,
const bke::AttributeAccessor src_attributes = src_curves.attributes();
bke::MutableAttributeAccessor dst_attributes = dst_curves.attributes_for_write();
Vector<bke::AttributeTransferData> attributes_to_transfer =
bke::retrieve_attributes_for_transfer(
src_attributes, dst_attributes, ATTR_DOMAIN_MASK_POINT, attribute_filter);
auto subdivide_catmull_rom = [&](const IndexMask &selection) {
for (auto &attribute : bke::retrieve_attributes_for_transfer(
src_attributes, dst_attributes, ATTR_DOMAIN_MASK_POINT, attribute_filter))
{
for (auto &attribute : attributes_to_transfer) {
subdivide_attribute_catmull_rom(src_points_by_curve,
dst_points_by_curve,
selection,
@@ -326,21 +332,17 @@ bke::CurvesGeometry subdivide_curves(const bke::CurvesGeometry &src_curves,
cyclic,
attribute.src,
attribute.dst.span);
attribute.dst.finish();
}
};
auto subdivide_poly = [&](const IndexMask &selection) {
for (auto &attribute : bke::retrieve_attributes_for_transfer(
src_attributes, dst_attributes, ATTR_DOMAIN_MASK_POINT, attribute_filter))
{
for (auto &attribute : attributes_to_transfer) {
subdivide_attribute_linear(src_points_by_curve,
dst_points_by_curve,
selection,
all_point_offsets,
attribute.src,
attribute.dst.span);
attribute.dst.finish();
}
};
@@ -377,24 +379,19 @@ bke::CurvesGeometry subdivide_curves(const bke::CurvesGeometry &src_curves,
dst_handles_r.slice(dst_points));
});
for (auto &attribute :
bke::retrieve_attributes_for_transfer(src_attributes,
dst_attributes,
ATTR_DOMAIN_MASK_POINT,
attribute_filter_with_skip_ref(attribute_filter,
{"position",
"handle_type_left",
"handle_type_right",
"handle_right",
"handle_left"})))
{
/* Filter out positions and handles that are already interpolated. */
const Set<StringRef> attributes_to_skip = {
"position", "handle_type_left", "handle_type_right", "handle_right", "handle_left"};
for (auto &attribute : attributes_to_transfer) {
if (attributes_to_skip.contains(attribute.name)) {
continue;
}
subdivide_attribute_linear(src_points_by_curve,
dst_points_by_curve,
selection,
all_point_offsets,
attribute.src,
attribute.dst.span);
attribute.dst.finish();
}
};
@@ -410,14 +407,11 @@ bke::CurvesGeometry subdivide_curves(const bke::CurvesGeometry &src_curves,
subdivide_bezier,
subdivide_nurbs);
bke::copy_attributes_group_to_group(src_attributes,
bke::AttrDomain::Point,
bke::AttrDomain::Point,
attribute_filter,
src_points_by_curve,
dst_points_by_curve,
unselected,
dst_attributes);
for (auto &attribute : attributes_to_transfer) {
array_utils::copy_group_to_group(
src_points_by_curve, dst_points_by_curve, unselected, attribute.src, attribute.dst.span);
attribute.dst.finish();
}
return dst_curves;
}