[Bf-blender-cvs] [24cab303f36] temp-T97352-3d-texturing-seam-bleeding-b2: Curves: Correct and improve Catmull Rom interpolation

Mattias Fredriksson noreply at git.blender.org
Tue Sep 20 10:32:13 CEST 2022


Commit: 24cab303f3600360a9a7b3d873a06800585cd29d
Author: Mattias Fredriksson
Date:   Sat Sep 17 21:53:58 2022 -0500
Branches: temp-T97352-3d-texturing-seam-bleeding-b2
https://developer.blender.org/rB24cab303f3600360a9a7b3d873a06800585cd29d

Curves: Correct and improve Catmull Rom interpolation

Correct interpolation of integer POD types for Catmull Rom
interpolation as implemented in eaf416693dcb.

**Problem description**
`attribute_math::DefaultMixer<T>::mix_in()` assumes/asserts positive
weights but the basis function for Catmull-Rom splines generates
negative weights (see image in revision). Passing negative weights will
yield correct result as sum(weights) = 1 (after multiplication by 0.5)
but the assert is still triggered in debug builds. This patch adjusts
the behavior by extending the mix functions with mix4(). The benefit
of using mix#() over a DefaultMixer is that the result no longer needs
to be divided by the weight sum, instead utilizing that the basis weight
sum is constant (see plot).

**Changes**
 * Added mix4() and updated catmull_rom::interpolate() to use it.
 * Removed TODOs from catmull_rom functions.
 * Moved mix definitions to be ordered as 2, 3, 4 in the header.

**Implementation specifics**
`catmull_rom::interpolate()` uses a constexpr to differentiate between
POD types which multiplies the result with 0.5 after weighting the
values, this reduces the number of multiplications for 1D, 2D, 3D
vectors (https://godbolt.org/z/8M1z9Pxx6). While this could be
considered unnecessary, I didn't want to change the original behavior
as it could influence performance (did not measure performance here
as this should ensure the logic is ~identical for FP types).

Differential Revision: https://developer.blender.org/D15997

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

M	source/blender/blenkernel/BKE_attribute_math.hh
M	source/blender/blenkernel/BKE_curves.hh
M	source/blender/blenkernel/intern/curve_catmull_rom.cc

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

diff --git a/source/blender/blenkernel/BKE_attribute_math.hh b/source/blender/blenkernel/BKE_attribute_math.hh
index 770937688d7..abd8b33b260 100644
--- a/source/blender/blenkernel/BKE_attribute_math.hh
+++ b/source/blender/blenkernel/BKE_attribute_math.hh
@@ -45,6 +45,58 @@ inline void convert_to_static_type(const eCustomDataType data_type, const Func &
   convert_to_static_type(cpp_type, func);
 }
 
+/* -------------------------------------------------------------------- */
+/** \name Mix two values of the same type.
+ *
+ * This is just basic linear interpolation.
+ * \{ */
+
+template<typename T> T mix2(float factor, const T &a, const T &b);
+
+template<> inline bool mix2(const float factor, const bool &a, const bool &b)
+{
+  return ((1.0f - factor) * a + factor * b) >= 0.5f;
+}
+
+template<> inline int8_t mix2(const float factor, const int8_t &a, const int8_t &b)
+{
+  return static_cast<int8_t>(std::round((1.0f - factor) * a + factor * b));
+}
+
+template<> inline int mix2(const float factor, const int &a, const int &b)
+{
+  return static_cast<int>(std::round((1.0f - factor) * a + factor * b));
+}
+
+template<> inline float mix2(const float factor, const float &a, const float &b)
+{
+  return (1.0f - factor) * a + factor * b;
+}
+
+template<> inline float2 mix2(const float factor, const float2 &a, const float2 &b)
+{
+  return math::interpolate(a, b, factor);
+}
+
+template<> inline float3 mix2(const float factor, const float3 &a, const float3 &b)
+{
+  return math::interpolate(a, b, factor);
+}
+
+template<>
+inline ColorGeometry4f mix2(const float factor, const ColorGeometry4f &a, const ColorGeometry4f &b)
+{
+  return math::interpolate(a, b, factor);
+}
+
+template<>
+inline ColorGeometry4b mix2(const float factor, const ColorGeometry4b &a, const ColorGeometry4b &b)
+{
+  return math::interpolate(a, b, factor);
+}
+
+/** \} */
+
 /* -------------------------------------------------------------------- */
 /** \name Mix three values of the same type.
  *
@@ -117,53 +169,85 @@ inline ColorGeometry4b mix3(const float3 &weights,
 /** \} */
 
 /* -------------------------------------------------------------------- */
-/** \name Mix two values of the same type.
+/** \name Mix four values of the same type.
  *
- * This is just basic linear interpolation.
  * \{ */
 
-template<typename T> T mix2(float factor, const T &a, const T &b);
+template<typename T>
+T mix4(const float4 &weights, const T &v0, const T &v1, const T &v2, const T &v3);
 
-template<> inline bool mix2(const float factor, const bool &a, const bool &b)
+template<>
+inline int8_t mix4(
+    const float4 &weights, const int8_t &v0, const int8_t &v1, const int8_t &v2, const int8_t &v3)
 {
-  return ((1.0f - factor) * a + factor * b) >= 0.5f;
+  return static_cast<int8_t>(
+      std::round(weights.x * v0 + weights.y * v1 + weights.z * v2 + weights.w * v3));
 }
 
-template<> inline int8_t mix2(const float factor, const int8_t &a, const int8_t &b)
+template<>
+inline bool mix4(
+    const float4 &weights, const bool &v0, const bool &v1, const bool &v2, const bool &v3)
 {
-  return static_cast<int8_t>(std::round((1.0f - factor) * a + factor * b));
+  return (weights.x * v0 + weights.y * v1 + weights.z * v2 + weights.w * v3) >= 0.5f;
 }
 
-template<> inline int mix2(const float factor, const int &a, const int &b)
+template<>
+inline int mix4(const float4 &weights, const int &v0, const int &v1, const int &v2, const int &v3)
 {
-  return static_cast<int>(std::round((1.0f - factor) * a + factor * b));
+  return static_cast<int>(
+      std::round(weights.x * v0 + weights.y * v1 + weights.z * v2 + weights.w * v3));
 }
 
-template<> inline float mix2(const float factor, const float &a, const float &b)
+template<>
+inline float mix4(
+    const float4 &weights, const float &v0, const float &v1, const float &v2, const float &v3)
 {
-  return (1.0f - factor) * a + factor * b;
+  return weights.x * v0 + weights.y * v1 + weights.z * v2 + weights.w * v3;
 }
 
-template<> inline float2 mix2(const float factor, const float2 &a, const float2 &b)
+template<>
+inline float2 mix4(
+    const float4 &weights, const float2 &v0, const float2 &v1, const float2 &v2, const float2 &v3)
 {
-  return math::interpolate(a, b, factor);
+  return weights.x * v0 + weights.y * v1 + weights.z * v2 + weights.w * v3;
 }
 
-template<> inline float3 mix2(const float factor, const float3 &a, const float3 &b)
+template<>
+inline float3 mix4(
+    const float4 &weights, const float3 &v0, const float3 &v1, const float3 &v2, const float3 &v3)
 {
-  return math::interpolate(a, b, factor);
+  return weights.x * v0 + weights.y * v1 + weights.z * v2 + weights.w * v3;
 }
 
 template<>
-inline ColorGeometry4f mix2(const float factor, const ColorGeometry4f &a, const ColorGeometry4f &b)
+inline ColorGeometry4f mix4(const float4 &weights,
+                            const ColorGeometry4f &v0,
+                            const ColorGeometry4f &v1,
+                            const ColorGeometry4f &v2,
+                            const ColorGeometry4f &v3)
 {
-  return math::interpolate(a, b, factor);
+  ColorGeometry4f result;
+  interp_v4_v4v4v4v4(result, v0, v1, v2, v3, weights);
+  return result;
 }
 
 template<>
-inline ColorGeometry4b mix2(const float factor, const ColorGeometry4b &a, const ColorGeometry4b &b)
+inline ColorGeometry4b mix4(const float4 &weights,
+                            const ColorGeometry4b &v0,
+                            const ColorGeometry4b &v1,
+                            const ColorGeometry4b &v2,
+                            const ColorGeometry4b &v3)
 {
-  return math::interpolate(a, b, factor);
+  const float4 v0_f{&v0.r};
+  const float4 v1_f{&v1.r};
+  const float4 v2_f{&v2.r};
+  const float4 v3_f{&v3.r};
+  float4 mixed;
+  interp_v4_v4v4v4v4(mixed, v0_f, v1_f, v2_f, v3_f, weights);
+  return ColorGeometry4b{static_cast<uint8_t>(mixed[0]),
+                         static_cast<uint8_t>(mixed[1]),
+                         static_cast<uint8_t>(mixed[2]),
+                         static_cast<uint8_t>(mixed[3])};
 }
 
 /** \} */
diff --git a/source/blender/blenkernel/BKE_curves.hh b/source/blender/blenkernel/BKE_curves.hh
index b1581e93491..0d67152dec8 100644
--- a/source/blender/blenkernel/BKE_curves.hh
+++ b/source/blender/blenkernel/BKE_curves.hh
@@ -709,7 +709,7 @@ void interpolate_to_evaluated(const GSpan src,
                               const Span<int> evaluated_offsets,
                               GMutableSpan dst);
 
-void calculate_basis(const float parameter, float r_weights[4]);
+void calculate_basis(const float parameter, float4 &r_weights);
 
 /**
  * Interpolate the control point values for the given parameter on the piecewise segment.
@@ -720,22 +720,15 @@ void calculate_basis(const float parameter, float r_weights[4]);
 template<typename T>
 T interpolate(const T &a, const T &b, const T &c, const T &d, const float parameter)
 {
-  float n[4];
+  BLI_assert(0.0f <= parameter && parameter <= 1.0f);
+  float4 n;
   calculate_basis(parameter, n);
-  /* TODO: Use DefaultMixer or other generic mixing in the basis evaluation function to simplify
-   * supporting more types. */
-  if constexpr (!is_same_any_v<T, float, float2, float3, float4, int8_t, int, int64_t>) {
-    T return_value;
-    attribute_math::DefaultMixer<T> mixer({&return_value, 1});
-    mixer.mix_in(0, a, n[0] * 0.5f);
-    mixer.mix_in(0, b, n[1] * 0.5f);
-    mixer.mix_in(0, c, n[2] * 0.5f);
-    mixer.mix_in(0, d, n[3] * 0.5f);
-    mixer.finalize();
-    return return_value;
+  if constexpr (is_same_any_v<T, float, float2, float3>) {
+    /* Save multiplications by adjusting weights after mix. */
+    return 0.5f * attribute_math::mix4<T>(n, a, b, c, d);
   }
   else {
-    return 0.5f * (a * n[0] + b * n[1] + c * n[2] + d * n[3]);
+    return attribute_math::mix4<T>(n * 0.5f, a, b, c, d);
   }
 }
 
diff --git a/source/blender/blenkernel/intern/curve_catmull_rom.cc b/source/blender/blenkernel/intern/curve_catmull_rom.cc
index dac88948036..b5f1a7cc450 100644
--- a/source/blender/blenkernel/intern/curve_catmull_rom.cc
+++ b/source/blender/blenkernel/intern/curve_catmull_rom.cc
@@ -17,7 +17,7 @@ int calculate_evaluated_num(const int points_num, const bool cyclic, const int r
 }
 
 /* Adapted from Cycles #catmull_rom_basis_eval function. */
-void calculate_basis(const float parameter, float r_weights[4])
+void calculate_basis(const float parameter, float4 &r_weights)
 {
   const float t = parameter;
   const float s = 1.0f - parameter;
@@ -139,11 +139,7 @@ void interpolate_to_evaluated(const GSpan src,
 {
   attribute_math::convert_to_static_type(src.type(), [&](auto dummy) {
     using T = decltype(dummy);
-    /* TODO: Use DefaultMixer or other generic mixing in the basis evaluation function to simplify
-     * supporting more types. */
-    if constexpr (is_same_any_v<T, float, float2, float3, float4, int8_t, int, int64_t>) {
-      interpolate_to_evaluated(src.typed<T>(), cyclic, resolution, dst.typed<T>());
-    }
+    interpolate_to_evaluated(src.typed<T>(), cyclic, resolution, dst.typed<T>());
   });
 }
 
@@ -154,11 +150,7 @@ void interpolate_to_evaluated(const GSpan src,
 {
   attribute_math::convert_to_static_type(src.type(), [&](auto dummy) {
     using T = decltype(dummy);
-    /* TODO: Use DefaultMixer or other generic mixing in the basis evaluation function to simplify
-     * supporting more types. */
-    if constexpr (is_same_any_v<T, float, float2, float3, float4, int8_t, int, int64_t>) {
-      interpolate_to_evaluated(src.typed<T>(), cyclic, evaluated_offsets, dst.typed<T>());
-    }
+    interpolate_to_evaluated(src.typed<T>(), cyclic, evaluated_offsets, dst.typed<T>());
   });
 }



More information about the Bf-blender-cvs mailing list