From f5adfa6acdd5b64ff97dc2239c3a89af778814c1 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Sat, 16 Dec 2023 12:52:22 -0500 Subject: [PATCH] Fix: New generic attributes uninitialized after curves draw tool Add a utility to set attribute values to their default, use it in a few places that have already done this samething. Also: - Don't create resolution or cyclic attributes unnecessarily - Use API function to set new curve's type - Always create the new selection on the curve domain - Remove selection before resize to avoid unnecessary work --- source/blender/blenkernel/BKE_attribute.hh | 5 + .../blenkernel/intern/attribute_access.cc | 21 ++++ .../editors/curves/intern/curves_draw.cc | 99 +++++++++++-------- .../sculpt_paint/grease_pencil_paint.cc | 45 +++------ .../geometry/intern/add_curves_on_mesh.cc | 22 ++--- 5 files changed, 100 insertions(+), 92 deletions(-) diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh index e86f749e319..c5e9d933257 100644 --- a/source/blender/blenkernel/BKE_attribute.hh +++ b/source/blender/blenkernel/BKE_attribute.hh @@ -914,4 +914,9 @@ void copy_attributes_group_to_group(AttributeAccessor src_attributes, const IndexMask &selection, MutableAttributeAccessor dst_attributes); +void fill_attribute_range_default(MutableAttributeAccessor dst_attributes, + eAttrDomain domain, + const Set &skip, + IndexRange range); + } // namespace blender::bke diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc index 16eedad8fb8..8c9500041c6 100644 --- a/source/blender/blenkernel/intern/attribute_access.cc +++ b/source/blender/blenkernel/intern/attribute_access.cc @@ -1052,4 +1052,25 @@ void copy_attributes_group_to_group(const AttributeAccessor src_attributes, }); } +void fill_attribute_range_default(MutableAttributeAccessor attributes, + const eAttrDomain domain, + const Set &skip, + const IndexRange range) +{ + attributes.for_all([&](const AttributeIDRef &id, const AttributeMetaData meta_data) { + if (meta_data.domain != domain) { + return true; + } + if (skip.contains(id.name())) { + return true; + } + GSpanAttributeWriter attribute = attributes.lookup_for_write_span(id); + const CPPType &type = attribute.span.type(); + GMutableSpan data = attribute.span.slice(range); + type.fill_assign_n(type.default_value(), data.data(), data.size()); + attribute.finish(); + return true; + }); +} + } // namespace blender::bke diff --git a/source/blender/editors/curves/intern/curves_draw.cc b/source/blender/editors/curves/intern/curves_draw.cc index bd21abb0de7..ea23c1ffd10 100644 --- a/source/blender/editors/curves/intern/curves_draw.cc +++ b/source/blender/editors/curves/intern/curves_draw.cc @@ -655,13 +655,16 @@ static int curves_draw_exec(bContext *C, wmOperator *op) Curves *curves_id = static_cast(obedit->data); bke::CurvesGeometry &curves = curves_id->geometry.wrap(); - const eAttrDomain selection_domain = eAttrDomain(curves_id->selection_domain); const int curve_index = curves.curves_num(); const bool use_pressure_radius = (cps->flag & CURVE_PAINT_FLAG_PRESSURE_RADIUS) || ((cps->radius_taper_start != 0.0f) || (cps->radius_taper_end != 0.0f)); + bke::MutableAttributeAccessor attributes = curves.attributes_for_write(); + + attributes.remove(".selection"); + if (cdd->curve_type == CU_BEZIER) { /* Allow to interpolate multiple channels */ int dims = 3; @@ -761,30 +764,18 @@ static int curves_draw_exec(bContext *C, wmOperator *op) if (result == 0) { curves.resize(curves.points_num() + cubic_spline_len, curve_index + 1); - MutableSpan curve_types = curves.curve_types_for_write(); - curve_types[curve_index] = CURVE_TYPE_BEZIER; - curves.update_curve_types(); + curves.fill_curve_types(IndexRange(curve_index, 1), CURVE_TYPE_BEZIER); + MutableSpan positions = curves.positions_for_write(); MutableSpan handle_positions_l = curves.handle_positions_left_for_write(); MutableSpan handle_positions_r = curves.handle_positions_right_for_write(); MutableSpan handle_types_l = curves.handle_types_left_for_write(); MutableSpan handle_types_r = curves.handle_types_right_for_write(); - MutableSpan cyclic = curves.cyclic_for_write(); - MutableSpan resolution = curves.resolution_for_write(); - resolution[curve_index] = 12; - IndexRange new_points = curves.points_by_curve()[curve_index]; - MutableSpan positions = curves.positions_for_write(); + const IndexRange new_points = curves.points_by_curve()[curve_index]; - bke::MutableAttributeAccessor curves_attributes = curves.attributes_for_write(); - bke::SpanAttributeWriter radius_attribute = - curves_attributes.lookup_or_add_for_write_only_span("radius", ATTR_DOMAIN_POINT); - MutableSpan radii = radius_attribute.span; - - bke::GSpanAttributeWriter selection = ensure_selection_attribute( - curves, selection_domain, CD_PROP_BOOL); - - curves::fill_selection_false(selection.span); + bke::SpanAttributeWriter radii = attributes.lookup_or_add_for_write_only_span( + "radius", ATTR_DOMAIN_POINT); float *co = cubic_spline; @@ -800,18 +791,14 @@ static int curves_draw_exec(bContext *C, wmOperator *op) const float radius = (radius_index != -1) ? (pt[radius_index] * cdd->radius.range) + cdd->radius.min : radius_max; - radii[i] = radius; + radii.span[i] = radius; handle_types_l[i] = BEZIER_HANDLE_ALIGN; handle_types_r[i] = BEZIER_HANDLE_ALIGN; co += (dims * 3); } - curves::fill_selection_true( - selection.span, - selection.domain == ATTR_DOMAIN_POINT ? new_points : IndexRange(curve_index, 1)); - selection.finish(); - // bezt->f1 = bezt->f2 = bezt->f3 = SELECT; + // bezt->f1 = bezt->f2 = bezt->f3 = SELECT; if (corners_index) { /* ignore the first and last */ @@ -829,9 +816,35 @@ static int curves_draw_exec(bContext *C, wmOperator *op) } } - cyclic[curve_index] = bool(calc_flag & CURVE_FIT_CALC_CYCLIC); + radii.finish(); - radius_attribute.finish(); + bke::AttributeWriter selection = attributes.lookup_or_add_for_write( + ".selection", ATTR_DOMAIN_CURVE); + selection.varray.set(curve_index, true); + selection.finish(); + + if (attributes.contains("resolution")) { + curves.resolution_for_write()[curve_index] = 12; + } + + if (attributes.contains("cyclic") || bool(calc_flag & CURVE_FIT_CALC_CYCLIC)) { + curves.cyclic_for_write()[curve_index] = true; + } + + bke::fill_attribute_range_default(attributes, + ATTR_DOMAIN_POINT, + {"position", + "radius", + "handle_left", + "handle_right", + "handle_type_left", + "handle_type_right", + ".selection"}, + new_points); + bke::fill_attribute_range_default(attributes, + ATTR_DOMAIN_CURVE, + {"curve_type", "resolution", "cyclic", ".selection"}, + IndexRange(curve_index, 1)); } if (corners_index) { @@ -844,20 +857,13 @@ static int curves_draw_exec(bContext *C, wmOperator *op) } else { /* CU_POLY */ curves.resize(curves.points_num() + stroke_len, curve_index + 1); - MutableSpan curve_types = curves.curve_types_for_write(); - curve_types[curve_index] = CURVE_TYPE_POLY; - curves.update_curve_types(); + curves.fill_curve_types(IndexRange(curve_index, 1), CURVE_TYPE_POLY); - IndexRange new_points = curves.points_by_curve()[curve_index]; MutableSpan positions = curves.positions_for_write(); - bke::MutableAttributeAccessor curves_attributes = curves.attributes_for_write(); - bke::SpanAttributeWriter radius_attribute = - curves_attributes.lookup_or_add_for_write_only_span("radius", ATTR_DOMAIN_POINT); - MutableSpan radii = radius_attribute.span; + bke::SpanAttributeWriter radii = attributes.lookup_or_add_for_write_only_span( + "radius", ATTR_DOMAIN_POINT); - bke::GSpanAttributeWriter selection = ensure_selection_attribute( - curves, selection_domain, CD_PROP_BOOL); - curves::fill_selection_false(selection.span); + const IndexRange new_points = curves.points_by_curve()[curve_index]; IndexRange::Iterator points_iter = new_points.begin(); @@ -872,14 +878,21 @@ static int curves_draw_exec(bContext *C, wmOperator *op) positions[i][2] = 0.0f; } - radii[i] = use_pressure_radius ? (selem->pressure * radius_range) + radius_min : - cps->radius_max; + radii.span[i] = use_pressure_radius ? (selem->pressure * radius_range) + radius_min : + cps->radius_max; } - curves::fill_selection_true( - selection.span, - selection.domain == ATTR_DOMAIN_POINT ? new_points : IndexRange(curve_index, 1)); + + radii.finish(); + + bke::AttributeWriter selection = attributes.lookup_or_add_for_write( + ".selection", ATTR_DOMAIN_CURVE); + selection.varray.set(curve_index, true); selection.finish(); - radius_attribute.finish(); + + bke::fill_attribute_range_default( + attributes, ATTR_DOMAIN_POINT, {"position", "radius", ".selection"}, new_points); + bke::fill_attribute_range_default( + attributes, ATTR_DOMAIN_CURVE, {"curve_type", ".selection"}, IndexRange(curve_index, 1)); } WM_event_add_notifier(C, NC_GEOM | ND_DATA, obedit->data); diff --git a/source/blender/editors/sculpt_paint/grease_pencil_paint.cc b/source/blender/editors/sculpt_paint/grease_pencil_paint.cc index 6a16eb2b7ad..14a4ceee05a 100644 --- a/source/blender/editors/sculpt_paint/grease_pencil_paint.cc +++ b/source/blender/editors/sculpt_paint/grease_pencil_paint.cc @@ -238,27 +238,14 @@ struct PaintOperationExecutor { curves.update_curve_types(); /* Initialize the rest of the attributes with default values. */ - Set attributes_to_skip{{"position", - "curve_type", - "material_index", - "cyclic", - "radius", - "opacity", - "vertex_color"}}; - attributes.for_all( - [&](const bke::AttributeIDRef &id, const bke::AttributeMetaData /*meta_data*/) { - if (attributes_to_skip.contains(id.name())) { - return true; - } - bke::GSpanAttributeWriter attribute = attributes.lookup_for_write_span(id); - const CPPType &type = attribute.span.type(); - GMutableSpan new_data = attribute.span.slice(attribute.domain == ATTR_DOMAIN_POINT ? - curves.points_range().take_back(1) : - curves.curves_range().take_back(1)); - type.fill_assign_n(type.default_value(), new_data.data(), new_data.size()); - attribute.finish(); - return true; - }); + bke::fill_attribute_range_default(attributes, + ATTR_DOMAIN_POINT, + {"position", "radius", "opacity", "vertex_color"}, + curves.points_range().take_back(1)); + bke::fill_attribute_range_default(attributes, + ATTR_DOMAIN_CURVE, + {"curve_type", "material_index", "cyclic"}, + curves.curves_range().take_back(1)); drawing_->tag_topology_changed(); } @@ -417,18 +404,10 @@ struct PaintOperationExecutor { } /* Initialize the rest of the attributes with default values. */ - Set attributes_to_skip{{"position", "radius", "opacity", "vertex_color"}}; - attributes.for_all([&](const bke::AttributeIDRef &id, const bke::AttributeMetaData meta_data) { - if (attributes_to_skip.contains(id.name()) || meta_data.domain != ATTR_DOMAIN_POINT) { - return true; - } - bke::GSpanAttributeWriter attribute = attributes.lookup_for_write_span(id); - const CPPType &type = attribute.span.type(); - GMutableSpan new_data = attribute.span.slice(new_points); - type.fill_assign_n(type.default_value(), new_data.data(), new_data.size()); - attribute.finish(); - return true; - }); + bke::fill_attribute_range_default(attributes, + ATTR_DOMAIN_POINT, + {"position", "radius", "opacity", "vertex_color"}, + curves.points_range().take_back(1)); } void execute(PaintOperation &self, const bContext &C, const InputSample &extension_sample) diff --git a/source/blender/geometry/intern/add_curves_on_mesh.cc b/source/blender/geometry/intern/add_curves_on_mesh.cc index f0431e99b77..4163947088d 100644 --- a/source/blender/geometry/intern/add_curves_on_mesh.cc +++ b/source/blender/geometry/intern/add_curves_on_mesh.cc @@ -397,22 +397,12 @@ AddCurvesOnMeshOutputs add_curves_on_mesh(CurvesGeometry &curves, } /* Explicitly set all other attributes besides those processed above to default values. */ - Set attributes_to_skip{ - {"position", "curve_type", "surface_uv_coordinate", "resolution"}}; - attributes.for_all( - [&](const bke::AttributeIDRef &id, const bke::AttributeMetaData /*meta_data*/) { - if (attributes_to_skip.contains(id.name())) { - return true; - } - bke::GSpanAttributeWriter attribute = attributes.lookup_for_write_span(id); - const CPPType &type = attribute.span.type(); - GMutableSpan new_data = attribute.span.slice(attribute.domain == ATTR_DOMAIN_POINT ? - outputs.new_points_range : - outputs.new_curves_range); - type.fill_assign_n(type.default_value(), new_data.data(), new_data.size()); - attribute.finish(); - return true; - }); + bke::fill_attribute_range_default( + attributes, ATTR_DOMAIN_POINT, {"position"}, outputs.new_points_range); + bke::fill_attribute_range_default(attributes, + ATTR_DOMAIN_CURVE, + {"curve_type", "surface_uv_coordinate", "resolution"}, + outputs.new_curves_range); return outputs; }