From 164a30b3d8b3174a32ac7326782476f1188e6118 Mon Sep 17 00:00:00 2001 From: Freevatar Date: Mon, 3 Nov 2025 17:25:56 -0500 Subject: [PATCH] animation/bezier: Fix OOB in getYForPoint for non-monotonic 4-point curves (#81) --- CMakeLists.txt | 8 ++++ src/animation/BezierCurve.cpp | 52 ++++++++++++++-------- tests/beziercurve.cpp | 83 +++++++++++++++++++++++++++++++++++ tests/shared.hpp | 15 +++++++ 4 files changed, 139 insertions(+), 19 deletions(-) create mode 100644 tests/beziercurve.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 9ef581b..3aeb594 100644 --- a/CMakeLists.txt +++ b/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}) diff --git a/src/animation/BezierCurve.cpp b/src/animation/BezierCurve.cpp index 2c52a2c..463f0a3 100644 --- a/src/animation/BezierCurve.cpp +++ b/src/animation/BezierCurve.cpp @@ -26,20 +26,15 @@ void CBezierCurve::setup4(const std::array& pVec) { pVec[3], }; - if (m_vPoints.size() != 4) - std::abort(); - - // bake BAKEDPOINTS points for faster lookups - // T -> X ( / BAKEDPOINTS ) + // Pre-bake curve + // + // We start baking at t=(i+1)/n not at t=0 + // That means the first baked x can be > 0 if curve itself starts at x>0 for (int i = 0; i < BAKEDPOINTS; ++i) { - float const t = (i + 1) / sc(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 - const auto LOWERPOINT = &m_aPointsBaked[lowerIndex]; - const auto UPPERPOINT = &m_aPointsBaked[lowerIndex + 1]; + // Clamp final indices + if (lowerIndex < 0) + 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 - return 0.f; + const float dx = (UPPERPOINT.x - LOWERPOINT.x); + // 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& CBezierCurve::getControlPoints() const { diff --git a/tests/beziercurve.cpp b/tests/beziercurve.cpp new file mode 100644 index 0000000..1026bc5 --- /dev/null +++ b/tests/beziercurve.cpp @@ -0,0 +1,83 @@ +#include +#include +#include +#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 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::max())), true); + EXPECT(std::isfinite(curve.getYForPoint(-std::numeric_limits::max())), true); +} + +static void test_adjacent_baked_x_equal(int& ret) { + // Curve with flat tail (X=1, Y=1) + CBezierCurve curve; + std::array 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 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; +} diff --git a/tests/shared.hpp b/tests/shared.hpp index 33109f8..4b9aa32 100644 --- a/tests/shared.hpp +++ b/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)