animation/bezier: Fix OOB in getYForPoint for non-monotonic 4-point curves (#81)

CMakeLists.txt

@@ -112,6 +112,14 @@ add_test(
   COMMAND hyprutils_animation "utils")
 add_dependencies(tests hyprutils_animation)
 
+add_executable(hyprutils_beziercurve "tests/beziercurve.cpp")
+target_link_libraries(hyprutils_beziercurve PRIVATE hyprutils PkgConfig::deps)
+add_test(
+  NAME "BezierCurve"
+  WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests
+  COMMAND hyprutils_beziercurve "beziercurve")
+add_dependencies(tests hyprutils_beziercurve)
+
 # Installation
 install(TARGETS hyprutils)
 install(DIRECTORY "include/hyprutils" DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})

src/animation/BezierCurve.cpp

@@ -26,20 +26,15 @@ void CBezierCurve::setup4(const std::array<Vector2D, 4>& pVec) {
         pVec[3],
     };
 
-    if (m_vPoints.size() != 4)
+    // Pre-bake curve
-        std::abort();
+    //
-
+    // We start baking at t=(i+1)/n not at t=0
-    // bake BAKEDPOINTS points for faster lookups
+    // That means the first baked x can be > 0 if curve itself starts at x>0
-    // T -> X ( / BAKEDPOINTS )
     for (int i = 0; i < BAKEDPOINTS; ++i) {
-        float const t     = (i + 1) / sc<float>(BAKEDPOINTS);
+        // When i=0 -> t=1/255
+        const float t     = (i + 1) * INVBAKEDPOINTS;
         m_aPointsBaked[i] = Vector2D(getXForT(t), getYForT(t));
     }
-
-    for (int j = 1; j < 10; ++j) {
-        float i = j / 10.0f;
-        getYForPoint(i);
-    }
 }
 
 float CBezierCurve::getXForT(float const& t) const {
@@ -53,7 +48,7 @@ float CBezierCurve::getYForT(float const& t) const {
     float t2 = t * t;
     float t3 = t2 * t;
 
-    return ((1 - t) * (1 - t) * (1 - t) * m_vPoints[0].y) +(3 * t * (1 - t) * (1 - t) * m_vPoints[1].y) + (3 * t2 * (1 - t) * m_vPoints[2].y) + (t3 * m_vPoints[3].y);
+    return ((1 - t) * (1 - t) * (1 - t) * m_vPoints[0].y) + (3 * t * (1 - t) * (1 - t) * m_vPoints[1].y) + (3 * t2 * (1 - t) * m_vPoints[2].y) + (t3 * m_vPoints[3].y);
 }
 
 // Todo: this probably can be done better and faster
@@ -71,21 +66,40 @@ float CBezierCurve::getYForPoint(float const& x) const {
         else
             index -= step;
 
+        // Clamp to avoid index walking off
+        if (index < 0)
+            index = 0;
+        else if (index > BAKEDPOINTS - 1)
+            index = BAKEDPOINTS - 1;
+
         below = m_aPointsBaked[index].x < x;
     }
 
     int lowerIndex = index - (!below || index == BAKEDPOINTS - 1);
 
-    // in the name of performance i shall make a hack
+    // Clamp final indices
-    const auto LOWERPOINT = &m_aPointsBaked[lowerIndex];
+    if (lowerIndex < 0)
-    const auto UPPERPOINT = &m_aPointsBaked[lowerIndex + 1];
+        lowerIndex = 0;
+    else if (lowerIndex > BAKEDPOINTS - 2)
+        lowerIndex = BAKEDPOINTS - 2;
 
-    const auto PERCINDELTA = (x - LOWERPOINT->x) / (UPPERPOINT->x - LOWERPOINT->x);
+    // In the name of performance I shall make a hack
+    const auto& LOWERPOINT = m_aPointsBaked[lowerIndex];
+    const auto& UPPERPOINT = m_aPointsBaked[lowerIndex + 1];
 
-    if (std::isnan(PERCINDELTA) || std::isinf(PERCINDELTA)) // can sometimes happen for VERY small x
+    const float dx = (UPPERPOINT.x - LOWERPOINT.x);
-        return 0.f;
+    // If two baked points have almost the same x
+    //  just return the lower one
+    if (dx <= 1e-6f)
+        return LOWERPOINT.y;
 
-    return LOWERPOINT->y + ((UPPERPOINT->y - LOWERPOINT->y) * PERCINDELTA);
+    const auto PERCINDELTA = (x - LOWERPOINT.x) / dx;
+
+    // Can sometimes happen for VERY small x
+    if (std::isnan(PERCINDELTA) || std::isinf(PERCINDELTA))
+        return LOWERPOINT.y;
+
+    return LOWERPOINT.y + ((UPPERPOINT.y - LOWERPOINT.y) * PERCINDELTA);
 }
 
 const std::vector<Hyprutils::Math::Vector2D>& CBezierCurve::getControlPoints() const {

tests/beziercurve.cpp

@@ -0,0 +1,83 @@
+#include <cmath>
+#include <hyprutils/animation/BezierCurve.hpp>
+#include <hyprutils/math/Vector2D.hpp>
+#include "shared.hpp"
+
+using Hyprutils::Animation::CBezierCurve;
+using Hyprutils::Math::Vector2D;
+
+static void test_nonmonotonic4_clamps_out_of_range(int& ret) {
+    // Non-monotonic curve in X
+    // This used to drive the step-halving search to OOB. It should now clamp
+    CBezierCurve            curve;
+    std::array<Vector2D, 4> pts = {
+        Vector2D{0.5f, 1.0f}, // P0
+        Vector2D{1.0f, 1.0f}, // P1
+        Vector2D{0.0f, 0.0f}, // P2
+        Vector2D{0.5f, 0.0f}  // P3
+    };
+    curve.setup4(pts);
+
+    // x > last baked x
+    EXPECT(std::isfinite(curve.getYForPoint(0.6f)), true);
+    // Far beyond range
+    EXPECT(std::isfinite(curve.getYForPoint(std::numeric_limits<float>::max())), true);
+    EXPECT(std::isfinite(curve.getYForPoint(-std::numeric_limits<float>::max())), true);
+}
+
+static void test_adjacent_baked_x_equal(int& ret) {
+    // Curve with flat tail (X=1, Y=1)
+    CBezierCurve            curve;
+    std::array<Vector2D, 4> pts = {
+        Vector2D{0.0f, 0.0f}, // P0
+        Vector2D{0.2f, 0.2f}, // P1
+        Vector2D{1.0f, 1.0f}, // P2
+        Vector2D{1.0f, 1.0f}  // P3
+    };
+    curve.setup4(pts);
+
+    // Exactly at last baked X
+    const float y_at_end = curve.getYForPoint(1.0f);
+    // Slightly beyond last baked X
+    const float y_past_end = curve.getYForPoint(1.0001f);
+
+    EXPECT(y_at_end, 1.0f);
+    EXPECT(y_past_end, y_at_end);
+}
+
+static void test_all_baked_x_equal(int& ret) {
+    // Extreme case: X is constant along the whole curve
+    CBezierCurve            curve;
+    std::array<Vector2D, 4> pts = {
+        Vector2D{0.0f, 0.0f}, // P0
+        Vector2D{0.0f, 0.3f}, // P1
+        Vector2D{0.0f, 0.7f}, // P2
+        Vector2D{0.0f, 1.0f}  // P3
+    };
+    curve.setup4(pts);
+
+    // Below any baked X
+    const float y_lo = curve.getYForPoint(-100.0f);
+    const float y_0  = curve.getYForPoint(0.0f);
+    // Above any baked X
+    const float y_hi = curve.getYForPoint(100.0f);
+
+    EXPECT(std::isfinite(y_lo), true);
+    EXPECT(std::isfinite(y_0), true);
+    EXPECT(std::isfinite(y_hi), true);
+
+    // For this curve Y should stay within [0,1]
+    EXPECT((y_lo >= 0.0f && y_lo <= 1.0f), true);
+    EXPECT((y_0 >= 0.0f && y_0 <= 1.0f), true);
+    EXPECT((y_hi >= 0.0f && y_hi <= 1.0f), true);
+}
+
+int main() {
+    int ret = 0;
+
+    test_nonmonotonic4_clamps_out_of_range(ret);
+    test_adjacent_baked_x_equal(ret);
+    test_all_baked_x_equal(ret);
+
+    return ret;
+}

tests/shared.hpp

@@ -18,6 +18,7 @@ namespace Colors {
     } else {                                                                                                                                                                       \
         std::cout << Colors::GREEN << "Passed " << Colors::RESET << #expr << ". Got " << val << "\n";                                                                              \
     }
+
 #define EXPECT_VECTOR2D(expr, val)                                                                                                                                                 \
     do {                                                                                                                                                                           \
         const auto& RESULT   = expr;                                                                                                                                               \
@@ -30,3 +31,17 @@ namespace Colors {
             std::cout << Colors::GREEN << "Passed " << Colors::RESET << #expr << ". Got (" << RESULT.x << ", " << RESULT.y << ")\n";                                               \
         }                                                                                                                                                                          \
     } while (0)
+
+#define EXPECT_NEAR(actual, expected, tolerance)                                                                                                                                   \
+    do {                                                                                                                                                                           \
+        auto _a = (actual);                                                                                                                                                        \
+        auto _e = (expected);                                                                                                                                                      \
+        auto _t = (tolerance);                                                                                                                                                     \
+        if (!(std::fabs((_a) - (_e)) <= (_t))) {                                                                                                                                   \
+            std::cout << Colors::RED << "Failed: " << Colors::RESET << " EXPECT_NEAR(" #actual ", " #expected ", " #tolerance ")  got=" << _a << " expected=" << _e << " ± " << _t \
+                      << "\n";                                                                                                                                                     \
+            ret = 1;                                                                                                                                                               \
+        } else {                                                                                                                                                                   \
+            std::cout << Colors::GREEN << "Passed " << Colors::RESET << " |" #actual " - " #expected "| <= " #tolerance "\n";                                                      \
+        }                                                                                                                                                                          \
+    } while (0)