[Bf-blender-cvs] [231d66d8ba3] master: Fix: Support swapped inputs properly in trim curve node

Hans Goudey noreply at git.blender.org
Sat Oct 23 00:32:27 CEST 2021


Commit: 231d66d8ba355cb7fa5332e9ea7d8921bbde82fe
Author: Hans Goudey
Date:   Fri Oct 22 17:32:21 2021 -0500
Branches: master
https://developer.blender.org/rB231d66d8ba355cb7fa5332e9ea7d8921bbde82fe

Fix: Support swapped inputs properly in trim curve node

Previously, when the start input was greater than the end input,
the spline was resized to a single point. That is correct, but the
single point wasn't placed properly along the spline. Now, it is
placed according to the "Start" value, as if the trim started, but
couldn't continue because the "End" value was smaller.

This behavior is handled with a separate code path to keep each
simpler and avoid special cases. Any cleanup to reduce duplication
should focus on making each code path shorter separately rather
than merging them.

Also included are some changes to `lookup_control_point_position`
to support cyclic splines, though this single-point method is still
disabled on cyclic splines for consistency.

===================================================================

M	source/blender/nodes/geometry/nodes/node_geo_curve_trim.cc

===================================================================

diff --git a/source/blender/nodes/geometry/nodes/node_geo_curve_trim.cc b/source/blender/nodes/geometry/nodes/node_geo_curve_trim.cc
index 82e2d7ad283..6b049b4d384 100644
--- a/source/blender/nodes/geometry/nodes/node_geo_curve_trim.cc
+++ b/source/blender/nodes/geometry/nodes/node_geo_curve_trim.cc
@@ -132,19 +132,19 @@ static void linear_trim_to_output_data(const TrimLocation &start,
 
 /* Look up the control points to the left and right of factor, and get the factor between them. */
 static TrimLocation lookup_control_point_position(const Spline::LookupResult &lookup,
-                                                  Span<int> control_point_offsets)
+                                                  const BezierSpline &spline)
 {
-  const int *left_offset = std::lower_bound(
-      control_point_offsets.begin(), control_point_offsets.end(), lookup.evaluated_index);
-  const int index = left_offset - control_point_offsets.begin();
-  const int left = control_point_offsets[index] > lookup.evaluated_index ? index - 1 : index;
-  const int right = left + 1;
-
-  const float factor = std::clamp(
-      (lookup.evaluated_index + lookup.factor - control_point_offsets[left]) /
-          (control_point_offsets[right] - control_point_offsets[left]),
-      0.0f,
-      1.0f);
+  Span<int> offsets = spline.control_point_offsets();
+
+  const int *offset = std::lower_bound(offsets.begin(), offsets.end(), lookup.evaluated_index);
+  const int index = offset - offsets.begin();
+
+  const int left = offsets[index] > lookup.evaluated_index ? index - 1 : index;
+  const int right = left == (spline.size() - 1) ? 0 : left + 1;
+
+  const float offset_in_segment = lookup.evaluated_index + lookup.factor - offsets[left];
+  const int segment_eval_size = offsets[left + 1] - offsets[left];
+  const float factor = std::clamp(offset_in_segment / segment_eval_size, 0.0f, 1.0f);
 
   return {left, right, factor};
 }
@@ -244,10 +244,11 @@ static void trim_bezier_spline(Spline &spline,
                                const Spline::LookupResult &end_lookup)
 {
   BezierSpline &bezier_spline = static_cast<BezierSpline &>(spline);
-  Span<int> control_offsets = bezier_spline.control_point_offsets();
 
-  const TrimLocation start = lookup_control_point_position(start_lookup, control_offsets);
-  TrimLocation end = lookup_control_point_position(end_lookup, control_offsets);
+  const TrimLocation start = lookup_control_point_position(start_lookup, bezier_spline);
+  TrimLocation end = lookup_control_point_position(end_lookup, bezier_spline);
+
+  const Span<int> control_offsets = bezier_spline.control_point_offsets();
 
   /* The number of control points in the resulting spline. */
   const int size = end.right_index - start.left_index + 1;
@@ -329,6 +330,133 @@ static void trim_bezier_spline(Spline &spline,
   bezier_spline.resize(size);
 }
 
+static void trim_spline(SplinePtr &spline,
+                        const Spline::LookupResult start,
+                        const Spline::LookupResult end)
+{
+  switch (spline->type()) {
+    case Spline::Type::Bezier:
+      trim_bezier_spline(*spline, start, end);
+      break;
+    case Spline::Type::Poly:
+      trim_poly_spline(*spline, start, end);
+      break;
+    case Spline::Type::NURBS:
+      spline = std::make_unique<PolySpline>(trim_nurbs_spline(*spline, start, end));
+      break;
+  }
+  spline->mark_cache_invalid();
+}
+
+template<typename T>
+static void to_single_point_data(const TrimLocation &trim, MutableSpan<T> data)
+{
+  data.first() = mix2<T>(trim.factor, data[trim.left_index], data[trim.right_index]);
+}
+template<typename T>
+static void to_single_point_data(const TrimLocation &trim, Span<T> src, MutableSpan<T> dst)
+{
+  dst.first() = mix2<T>(trim.factor, src[trim.left_index], src[trim.right_index]);
+}
+
+static void to_single_point_bezier(Spline &spline, const Spline::LookupResult &lookup)
+{
+  BezierSpline &bezier = static_cast<BezierSpline &>(spline);
+
+  const TrimLocation trim = lookup_control_point_position(lookup, bezier);
+
+  const BezierSpline::InsertResult new_point = bezier.calculate_segment_insertion(
+      trim.left_index, trim.right_index, trim.factor);
+  bezier.positions().first() = new_point.position;
+  bezier.handle_types_left().first() = BezierSpline::HandleType::Free;
+  bezier.handle_types_right().first() = BezierSpline::HandleType::Free;
+  bezier.handle_positions_left().first() = new_point.left_handle;
+  bezier.handle_positions_right().first() = new_point.right_handle;
+
+  to_single_point_data<float>(trim, bezier.radii());
+  to_single_point_data<float>(trim, bezier.tilts());
+  spline.attributes.foreach_attribute(
+      [&](const AttributeIDRef &attribute_id, const AttributeMetaData &UNUSED(meta_data)) {
+        std::optional<GMutableSpan> data = spline.attributes.get_for_write(attribute_id);
+        attribute_math::convert_to_static_type(data->type(), [&](auto dummy) {
+          using T = decltype(dummy);
+          to_single_point_data<T>(trim, data->typed<T>());
+        });
+        return true;
+      },
+      ATTR_DOMAIN_POINT);
+  spline.resize(1);
+}
+
+static void to_single_point_poly(Spline &spline, const Spline::LookupResult &lookup)
+{
+  const TrimLocation trim{lookup.evaluated_index, lookup.next_evaluated_index, lookup.factor};
+
+  to_single_point_data<float3>(trim, spline.positions());
+  to_single_point_data<float>(trim, spline.radii());
+  to_single_point_data<float>(trim, spline.tilts());
+  spline.attributes.foreach_attribute(
+      [&](const AttributeIDRef &attribute_id, const AttributeMetaData &UNUSED(meta_data)) {
+        std::optional<GMutableSpan> data = spline.attributes.get_for_write(attribute_id);
+        attribute_math::convert_to_static_type(data->type(), [&](auto dummy) {
+          using T = decltype(dummy);
+          to_single_point_data<T>(trim, data->typed<T>());
+        });
+        return true;
+      },
+      ATTR_DOMAIN_POINT);
+  spline.resize(1);
+}
+
+static PolySpline to_single_point_nurbs(const Spline &spline, const Spline::LookupResult &lookup)
+{
+  /* Since this outputs a poly spline, the evaluated indices are the control point indices. */
+  const TrimLocation trim{lookup.evaluated_index, lookup.next_evaluated_index, lookup.factor};
+
+  /* Create poly spline and copy trimmed data to it. */
+  PolySpline new_spline;
+  new_spline.resize(1);
+
+  spline.attributes.foreach_attribute(
+      [&](const AttributeIDRef &attribute_id, const AttributeMetaData &meta_data) {
+        new_spline.attributes.create(attribute_id, meta_data.data_type);
+        std::optional<GSpan> src = spline.attributes.get_for_read(attribute_id);
+        std::optional<GMutableSpan> dst = new_spline.attributes.get_for_write(attribute_id);
+        attribute_math::convert_to_static_type(src->type(), [&](auto dummy) {
+          using T = decltype(dummy);
+          GVArray_Typed<T> eval_data = spline.interpolate_to_evaluated<T>(src->typed<T>());
+          to_single_point_data<T>(trim, eval_data->get_internal_span(), dst->typed<T>());
+        });
+        return true;
+      },
+      ATTR_DOMAIN_POINT);
+
+  to_single_point_data<float3>(trim, spline.evaluated_positions(), new_spline.positions());
+
+  GVArray_Typed<float> evaluated_radii = spline.interpolate_to_evaluated(spline.radii());
+  to_single_point_data<float>(trim, evaluated_radii->get_internal_span(), new_spline.radii());
+
+  GVArray_Typed<float> evaluated_tilts = spline.interpolate_to_evaluated(spline.tilts());
+  to_single_point_data<float>(trim, evaluated_tilts->get_internal_span(), new_spline.tilts());
+
+  return new_spline;
+}
+
+static void to_single_point_spline(SplinePtr &spline, const Spline::LookupResult &lookup)
+{
+  switch (spline->type()) {
+    case Spline::Type::Bezier:
+      to_single_point_bezier(*spline, lookup);
+      break;
+    case Spline::Type::Poly:
+      to_single_point_poly(*spline, lookup);
+      break;
+    case Spline::Type::NURBS:
+      spline = std::make_unique<PolySpline>(to_single_point_nurbs(*spline, lookup));
+      break;
+  }
+}
+
 static void geometry_set_curve_trim(GeometrySet &geometry_set,
                                     const GeometryNodeCurveSampleMode mode,
                                     Field<float> &start_field,
@@ -354,46 +482,49 @@ static void geometry_set_curve_trim(GeometrySet &geometry_set,
 
   threading::parallel_for(splines.index_range(), 128, [&](IndexRange range) {
     for (const int i : range) {
-      Spline &spline = *splines[i];
+      SplinePtr &spline = splines[i];
 
-      /* Currently this node doesn't support cyclic splines, it could in the future though. */
-      if (spline.is_cyclic()) {
+      /* Currently trimming cyclic splines is not supported. It could be in the future though. */
+      if (spline->is_cyclic()) {
         continue;
       }
 
-      if (spline.evaluated_edges_size() == 0) {
+      if (spline->evaluated_edges_size() == 0) {
         continue;
       }
 
-      /* Return a spline with one point instead of implicitly
-       * reversing the spline or switching the parameters. */
-      if (ends[i] < starts[i]) {
-        spline.resize(1);
+      const float length = spline->length();
+      if (length == 0.0f) {
         continue;
       }
 
-      const Spline::LookupResult start_lookup =
-          (mode == GEO_NODE_CURVE_SAMPLE_LENGTH) ?
-              spline.lookup_evaluated_length(std::clamp(starts[i], 0.0f, spline.length())) :
-              spline.lookup_evaluated_factor(std::clamp(starts[i], 0.0f, 1.0f));
-      const Spline::LookupResult end_lookup =
-          (mode == GEO_NODE_CURVE_SAMPLE_LENGTH) ?
-              spline.lookup_evaluated_length(std::clamp(ends[i], 0.0f, spline.length())) :
-              spline.lookup_evaluated_factor(std::clamp(ends[i], 0.0f, 1.0f));
-
-      switch (spline.type()) {
-        case Spline::Type::Bezier:
-          trim_bezier_spline(spline, start_lookup, end_lookup);
-          break;
-        case Spline::Type::Poly:
-          trim_poly_spline(spline, start_lookup, end_lookup);
-          break;
-        case Spline::Type::NURBS:
-          splines[i] = std::make_unique<PolySpline>(
-              trim_nurbs_spline(spline, start_

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list