[Bf-blender-cvs] [e7c1702ccba] geometry-nodes-curve-support: Splines: Cleanup, add comments, formatting

Hans Goudey noreply at git.blender.org
Mon Apr 26 06:29:50 CEST 2021


Commit: e7c1702ccbad6f9e3169d4363eef7e85a277b174
Author: Hans Goudey
Date:   Sun Apr 25 23:29:28 2021 -0500
Branches: geometry-nodes-curve-support
https://developer.blender.org/rBe7c1702ccbad6f9e3169d4363eef7e85a277b174

Splines: Cleanup, add comments, formatting

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

M	source/blender/blenkernel/BKE_geometry_set.hh
M	source/blender/blenkernel/BKE_spline.hh
M	source/blender/blenkernel/intern/spline_base.cc
M	source/blender/blenkernel/intern/spline_bezier.cc
M	source/blender/blenkernel/intern/spline_group.cc
M	source/blender/blenkernel/intern/spline_nurbs.cc
M	source/blender/blenkernel/intern/spline_poly.cc
M	source/blender/nodes/geometry/nodes/node_geo_curve_to_mesh.cc

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

diff --git a/source/blender/blenkernel/BKE_geometry_set.hh b/source/blender/blenkernel/BKE_geometry_set.hh
index fc3adde0178..677d869666b 100644
--- a/source/blender/blenkernel/BKE_geometry_set.hh
+++ b/source/blender/blenkernel/BKE_geometry_set.hh
@@ -472,6 +472,7 @@ class PointCloudComponent : public GeometryComponent {
   const blender::bke::ComponentAttributeProviders *get_attribute_providers() const final;
 };
 
+/** A geometry component that stores curve data, in other words, a group of splines. */
 class CurveComponent : public GeometryComponent {
  private:
   SplineGroup *curve_ = nullptr;
diff --git a/source/blender/blenkernel/BKE_spline.hh b/source/blender/blenkernel/BKE_spline.hh
index 7f58794a1d3..4e4b6eed156 100644
--- a/source/blender/blenkernel/BKE_spline.hh
+++ b/source/blender/blenkernel/BKE_spline.hh
@@ -36,10 +36,18 @@ class Spline;
 using SplinePtr = std::unique_ptr<Spline>;
 
 /**
- * A spline is an abstraction of a curve section, its evaluation methods, and data.
- * The spline data itself is just control points and a set of attributes.
+ * A spline is an abstraction of a single branchless curve section, its evaluation methods,
+ * and data. The spline data itself is just control points and a set of attributes.
  *
- * Common evaluated data is stored in caches on the spline itself. This way operations on splines
+ * Any derived class of Spline has to manage two things:
+ *  1. Evaluating the positions based on the stored control point data.
+ *  2. Interpolating arbitrary attribute data from the control points to evaluated points.
+ *
+ * Beyond that, everything else is the bas class's responsibility, with minor exceptions. Futher
+ * evaluation happens in a layer on top of the evaluated points generated by the derived types.
+ * There are a few methods to evaluate a spline:
+ *
+ * Common evaluated data is stored in caches on the spline itself. That way operations on splines
  * don't need to worry about taking ownership of evaluated data when they don't need to.
  */
 class Spline {
@@ -158,6 +166,11 @@ class Spline {
   virtual void correct_end_tangents() const = 0;
 };
 
+/**
+ * A Bézier spline is made up of a many curve segments, possibly achieving continuity of curvature
+ * by constraining the alignment of curve handles. Evaluation stores the positions and a map of
+ * factors and indices in a list of floats, which is then used to interpolate any other data.
+ */
 class BezierSpline final : public Spline {
  public:
   enum HandleType {
@@ -269,6 +282,12 @@ class BezierSpline final : public Spline {
   void evaluate_bezier_position_and_mapping() const;
 };
 
+/**
+ * Data for Non-Uniform Rational B-Splines. The mapping from control points to evaluated points is
+ * influenced by a vector of knots, weights for each point, and the order of the spline. Every
+ * mapping of data to evaluated points is handled the same way, but the posistions are cached in
+ * the spline.
+ */
 class NURBSpline final : public Spline {
  public:
   enum KnotsMode {
@@ -360,6 +379,11 @@ class NURBSpline final : public Spline {
   void calculate_basis_cache() const;
 };
 
+/**
+ * A poly spline is like a bezier spline with a resolution of one. The main reason to distinguish
+ * the two is for reduced complexity and increased performance, since interpolating data to control
+ * points does not change it.
+ */
 class PolySpline final : public Spline {
  public:
   blender::Vector<blender::float3> positions_;
@@ -406,7 +430,10 @@ class PolySpline final : public Spline {
   void correct_end_tangents() const final;
 };
 
-/* Proposed name to be different from DNA type. */
+/**
+ * A #SplineGroup corresponds to the #Curve object data. The name is different for clarity, since
+ * more of the data is stored in the splines, but also just to be different than the name in DNA.
+ */
 class SplineGroup {
  public:
   blender::Vector<SplinePtr> splines;
diff --git a/source/blender/blenkernel/intern/spline_base.cc b/source/blender/blenkernel/intern/spline_base.cc
index 93535fd4036..576e6d2964f 100644
--- a/source/blender/blenkernel/intern/spline_base.cc
+++ b/source/blender/blenkernel/intern/spline_base.cc
@@ -15,20 +15,14 @@
  */
 
 #include "BLI_array.hh"
-#include "BLI_listbase.h"
 #include "BLI_span.hh"
 
-#include "DNA_curve_types.h"
-
 #include "BKE_spline.hh"
 
-using blender::Array;
 using blender::float3;
-using blender::float4x4;
 using blender::IndexRange;
 using blender::MutableSpan;
 using blender::Span;
-using blender::Vector;
 
 Spline::Type Spline::type() const
 {
@@ -95,7 +89,6 @@ Span<float> Spline::evaluated_lengths() const
   return evaluated_lengths_cache_;
 }
 
-/* TODO: Optimize this along with the function below. */
 static float3 direction_bisect(const float3 &prev, const float3 &middle, const float3 &next)
 {
   const float3 dir_prev = (middle - prev).normalized();
@@ -157,6 +150,7 @@ Span<float3> Spline::evaluated_tangents() const
   return evaluated_tangents_cache_;
 }
 
+#if 0 /* Not supported yet, has errors. */
 static float3 initial_normal(const float3 first_tangent)
 {
   /* TODO: Should be is "almost" zero. */
@@ -265,6 +259,7 @@ static void calculate_normals_minimum_twist(Span<float3> tangents,
     make_normals_cyclic(tangents, normals);
   }
 }
+#endif
 
 static void calculate_normals_z_up(Span<float3> tangents, MutableSpan<float3> normals)
 {
@@ -292,17 +287,20 @@ Span<float3> Spline::evaluated_normals() const
   this->evaluated_normals_cache_.resize(total);
 
   Span<float3> tangents = this->evaluated_tangents();
+
+#if 0 /* Not supported yet, has errors. */
   switch (this->normal_mode) {
     case NormalCalculationMode::Minimum:
       calculate_normals_minimum_twist(tangents, is_cyclic, this->evaluated_normals_cache_);
       break;
     case NormalCalculationMode::ZUp:
-      calculate_normals_z_up(tangents, this->evaluated_normals_cache_);
       break;
     case NormalCalculationMode::Tangent:
-      // calculate_normals_tangent(tangents, this->evaluated_normals_cache_);
+      calculate_normals_tangent(tangents, this->evaluated_normals_cache_);
       break;
   }
+#endif
+  calculate_normals_z_up(tangents, this->evaluated_normals_cache_);
 
   this->normal_cache_dirty_ = false;
   return evaluated_normals_cache_;
@@ -313,7 +311,9 @@ Spline::LookupResult Spline::lookup_evaluated_factor(const float factor) const
   return this->lookup_evaluated_length(this->length() * factor);
 }
 
-/* TODO: Support extrapolation somehow. */
+/**
+ * \note This does not support extrapolation currently.
+ */
 Spline::LookupResult Spline::lookup_evaluated_length(const float length) const
 {
   BLI_assert(length >= 0.0f && length <= this->length());
diff --git a/source/blender/blenkernel/intern/spline_bezier.cc b/source/blender/blenkernel/intern/spline_bezier.cc
index ef77ce085c4..dea91b2c0fd 100644
--- a/source/blender/blenkernel/intern/spline_bezier.cc
+++ b/source/blender/blenkernel/intern/spline_bezier.cc
@@ -15,18 +15,15 @@
  */
 
 #include "BLI_array.hh"
-#include "BLI_listbase.h"
 #include "BLI_span.hh"
 
 #include "BKE_spline.hh"
 
 using blender::Array;
 using blender::float3;
-using blender::float4x4;
 using blender::IndexRange;
 using blender::MutableSpan;
 using blender::Span;
-using blender::Vector;
 
 SplinePtr BezierSpline::copy() const
 {
@@ -175,7 +172,7 @@ bool BezierSpline::handle_end_is_automatic(const int index) const
   return ELEM(handle_types_end_[index], HandleType::Free, HandleType::Align);
 }
 
-void BezierSpline::move_control_point(const int index, const blender::float3 new_position)
+void BezierSpline::move_control_point(const int index, const float3 new_position)
 {
   const float3 position_delta = new_position - positions_[index];
   if (!this->handle_start_is_automatic(index)) {
@@ -211,9 +208,9 @@ int BezierSpline::evaluated_points_size() const
   BLI_assert(this->size() > 0);
 #ifndef DEBUG
   if (!this->base_cache_dirty_) {
-    /* In a non-debug build, assume that the cache's size has not changed, and that any operation
-     * that would cause the cache to change its length would also mark the cache dirty. This is
-     * checked at the end of this function in a debug build. */
+    /* In a non-debug build, assume that the cache size has not changed, and that any operation
+     * that would cause the cache to change its length would also mark the cache dirty. This
+     * assumption is checked at the end of this function in a debug build. */
     return this->evaluated_positions_cache_.size();
   }
 #endif
@@ -242,7 +239,6 @@ int BezierSpline::evaluated_points_size() const
     total_len++;
   }
 
-  /* Assert that the cache has the correct length in debug mode. */
   if (!this->base_cache_dirty_) {
     BLI_assert(this->evaluated_positions_cache_.size() == total_len);
   }
@@ -357,8 +353,7 @@ void BezierSpline::evaluate_bezier_position_and_mapping() const
   MutableSpan<float3> positions = this->evaluated_positions_cache_;
   MutableSpan<float> mappings = this->evaluated_mappings_cache_;
 
-  /* TODO: It would also be possible to store an array of offsets to facilitate parallelism here,
-   * maybe it is worth it? */
+  /* Note: It would also be possible to store an array of offsets to facilitate parallelism. */
   int offset = 0;
   for (const int i : IndexRange(this->size() - 1)) {
     this->evaluate_bezier_segment(i, i + 1, offset, positions, mappings);
@@ -425,6 +420,7 @@ static void interpolate_to_evaluated_points_impl(Span<float> mappings,
                                                  MutableSpan<T> result_data)
 {
   const int points_len = source_data.size();
+  /* TODO: Use a set of functions mix2 in attribute_math instead of DefaultMixer. */
   blender::attribute_math::DefaultMixer<T> mixer(result_data);
 
   for (const int i : result_data.index_range()) {
diff --git a/source/blender/blenkernel/intern/spline_group.cc b/source/blender/blenkernel/intern/spline_group.cc
index 9b80cc6bf47..0040ffbe4bd 100644
--- a/source/blender/blenkernel/intern/spline_group.cc
+++ b/source/blender/blenkernel/intern/spline_group.cc
@@ -23,13 +23,9 @@
 #include "BKE_curve.h"
 #include "BKE_spline.hh"
 
-using blender::Array;
 using blender::float3;
 using blender::float4x4;
-using blender::IndexRa

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list