Make ContourMeasure more robust

Make assertions and operations inside ContourMeasure more robust with degenerate data and NaN.

Add a rive::math::clamp() method with better behavior surrounding NaN.

Change the ContourMeasureIter constructor to accept a "const RawPath*". (The former definition with "const RawPath&" accepted r-values, which was unsafe since the iterator stored raw pointers directly to objects owned by the RawPath.)

Delete RawPath operator*(). The way this operator was defined, the matrix had to go on the wrong side of the path relative to how it operated on the data.

Diffs=
cd6210f42 Make ContourMeasure more robust (#7294)

Co-authored-by: Chris Dalton <99840794+csmartdalton@users.noreply.github.com>

.rive_head

@@ -1 +1 @@
-405b8ef907d29cf480422b94d4717fdcdfad0824
+cd6210f42d644de3cc1c83fa65b3d4e1f790cece

include/rive/math/contour_measure.hpp

@@ -97,12 +97,12 @@ class ContourMeasureIter
     // approximation for the curves actual length.
     static constexpr float kDefaultTolerance = 0.5f;
 
-    ContourMeasureIter(const RawPath& path, float tol = kDefaultTolerance)
+    ContourMeasureIter(const RawPath* path, float tol = kDefaultTolerance)
     {
         this->rewind(path, tol);
     }
 
-    void rewind(const RawPath&, float = kDefaultTolerance);
+    void rewind(const RawPath*, float = kDefaultTolerance);
 
     // Returns a measure object for each contour in the path
     //   (contours with zero-length are skipped over)

include/rive/math/math_types.hpp

@@ -112,6 +112,17 @@ template <size_t N> RIVE_ALWAYS_INLINE constexpr size_t round_up_to_multiple_of(
                   "math::round_up_to_multiple_of<> only supports powers of 2.");
     return (x + (N - 1)) & ~(N - 1);
 }
+
+// Behaves better with NaN than std::clamp(). (Matching simd::clamp().)
+//
+//   Returns lo if x == NaN (but std::clamp() returns NaN).
+//   Returns hi if hi <= lo.
+//   Ignores hi and/or lo if they are NaN.
+//
+RIVE_ALWAYS_INLINE static float clamp(float x, float lo, float hi)
+{
+    return fminf(fmaxf(lo, x), hi);
+}
 } // namespace math
 
 template <typename T> T lerp(const T& a, const T& b, float t) { return a + (b - a) * t; }

include/rive/math/raw_path.hpp

@@ -49,7 +49,6 @@ class RawPath
 
     RawPath transform(const Mat2D&) const;
     void transformInPlace(const Mat2D&);
-    RawPath operator*(const Mat2D& mat) const { return this->transform(mat); }
 
     Span<const Vec2D> points() const { return m_Points; }
     Span<Vec2D> points() { return m_Points; }

src/constraints/follow_path_constraint.cpp

@@ -154,7 +154,7 @@ void FollowPathConstraint::update(ComponentDirt value)
             commandPath->addToRawPath(m_rawPath, path->pathTransform());
         }
 
-        auto measure = ContourMeasureIter(m_rawPath);
+        auto measure = ContourMeasureIter(&m_rawPath);
         for (auto contour = measure.next(); contour != nullptr; contour = measure.next())
         {
             m_contours.push_back(contour);

src/math/contour_measure.cpp

@@ -247,9 +247,12 @@ static float compute_t(Span<const ContourMeasure::Segment> segs, size_t index, f
         }
     }
 
-    assert(prevDist < seg.m_distance);
+    assert(prevDist <= seg.m_distance);
     const auto ratio = (distance - prevDist) / (seg.m_distance - prevDist);
-    return lerp(prevT, seg.getT(), ratio);
+    float t = lerp(prevT, seg.getT(), ratio);
+    t = math::clamp(t, prevT, seg.getT());
+    assert(prevT <= t && t <= seg.getT());
+    return t;
 }
 
 void ContourMeasure::getSegment(float startDist,
@@ -366,16 +369,16 @@ float ContourMeasureIter::addCubicSegs(ContourMeasure::Segment* segs,
     return distance;
 }
 
-void ContourMeasureIter::rewind(const RawPath& path, float tolerance)
+void ContourMeasureIter::rewind(const RawPath* path, float tolerance)
 {
-    m_iter = path.begin();
-    m_end = path.end();
-    m_srcPoints = path.points().data();
+    m_iter = path->begin();
+    m_end = path->end();
+    m_srcPoints = path->points().data();
 
     constexpr float kMinTolerance = 1.0f / 16;
     m_invTolerance = 1.0f / std::max(tolerance, kMinTolerance);
 
-    m_segmentCounts.resize(path.verbs().count());
+    m_segmentCounts.resize(path->verbs().count());
 }
 
 // Can return null if either it encountered an empty contour (length == 0)
@@ -515,12 +518,15 @@ rcp<ContourMeasure> ContourMeasureIter::tryNext()
 
     m_iter = endOfContour;
 
-    if (distance == 0 || pts.size() < 2)
+    if (distance > 0 && pts.size() >= 2)
     {
-        return nullptr;
+        assert(!std::isnan(distance));
+        return rcp<ContourMeasure>(
+            new ContourMeasure(std::move(segs), std::move(pts), distance, isClosed));
     }
-    return rcp<ContourMeasure>(
-        new ContourMeasure(std::move(segs), std::move(pts), distance, isClosed));
+
+    assert(distance == 0 || std::isnan(distance));
+    return nullptr;
 }
 
 rcp<ContourMeasure> ContourMeasureIter::next()
@@ -537,5 +543,6 @@ rcp<ContourMeasure> ContourMeasureIter::next()
             break;
         }
     }
+    assert(!cm || !std::isnan(cm->length()));
     return cm;
 }

src/shapes/metrics_path.cpp

@@ -63,7 +63,8 @@ float MetricsPath::computeLength(const Mat2D& transform)
     if (!m_Contour || transform != m_ComputedLengthTransform)
     {
         m_ComputedLengthTransform = transform;
-        m_Contour = ContourMeasureIter(m_RawPath * transform).next();
+        RawPath transformedPath = m_RawPath.transform(transform);
+        m_Contour = ContourMeasureIter(&transformedPath).next();
         m_ComputedLength = m_Contour ? m_Contour->length() : 0;
     }
     return m_ComputedLength;

test/contour_measure_test.cpp

@@ -39,15 +39,15 @@ TEST_CASE("contour-basics", "[contourmeasure]")
     const float tol = 0.000001f;
 
     RawPath path;
-    ContourMeasureIter iter(path, false);
+    ContourMeasureIter iter(&path);
     REQUIRE(iter.next() == nullptr);
 
     path.moveTo(1, 2);
-    iter.rewind(path, false);
+    iter.rewind(&path);
     REQUIRE(iter.next() == nullptr);
 
     path.lineTo(4, 6);
-    iter.rewind(path, false);
+    iter.rewind(&path);
     auto cm = iter.next();
     REQUIRE(cm);
     REQUIRE(nearly_eq(cm->length(), 5, tol));
@@ -58,7 +58,7 @@ TEST_CASE("contour-basics", "[contourmeasure]")
     path = RawPath();
     const float w = 4, h = 6;
     path.addRect({0, 0, w, h}, PathDirection::cw);
-    iter.rewind(path, false);
+    iter.rewind(&path);
     cm = iter.next();
     REQUIRE(cm);
     REQUIRE(nearly_eq(cm->length(), 2 * (w + h), tol));
@@ -118,7 +118,7 @@ TEST_CASE("multi-contours", "[contourmeasure]")
 
     path.addPoly(span, false); // len == 7
 
-    ContourMeasureIter iter(path, false);
+    ContourMeasureIter iter(&path);
     auto cm = iter.next();
     REQUIRE(cm->length() == 7);
     cm = iter.next();
@@ -136,7 +136,7 @@ TEST_CASE("contour-oval", "[contourmeasure]")
     const float r = 10;
     RawPath path;
     path.addOval({-r, -r, r, r}, PathDirection::cw);
-    ContourMeasureIter iter(path, false);
+    ContourMeasureIter iter(&path, tol);
 
     auto cm = iter.next();
     REQUIRE(nearly_eq(cm->length(), 2 * r * math::PI, tol));
@@ -152,3 +152,28 @@ TEST_CASE("bad contour", "[contourmeasure]")
     auto machine = artboard->defaultStateMachine();
     machine->advanceAndApply(0.0f);
 }
+
+// NaN paths don't return contours.
+TEST_CASE("nan-path", "[contourmeasure]")
+{
+    RawPath path;
+    path.lineTo(1, 2);
+    path.cubicTo(3, 4, 5, 6, 7, 8);
+    path.cubicTo(9, 10, 11, 12, 13, 14);
+    path.cubicTo(15, 16, 17, 18, 19, 20);
+
+    {
+        ContourMeasureIter iter(&path);
+        auto cm = iter.next();
+        CHECK(cm != nullptr);
+        CHECK(std::isfinite(cm->length()));
+        CHECK(iter.next() == nullptr);
+    }
+
+    {
+        auto nan = std::numeric_limits<float>::quiet_NaN();
+        RawPath path_ = path.transform(Mat2D(nan, nan, nan, nan, nan, nan));
+        ContourMeasureIter iter(&path_);
+        CHECK(iter.next() == nullptr);
+    }
+}

test/metrics_path_test.cpp

@@ -1,5 +1,11 @@
 #include <catch.hpp>
+#include <rive/math/math_types.hpp>
 #include <rive/shapes/metrics_path.hpp>
+#include <rive/shapes/paint/stroke.hpp>
+#include <rive/shapes/paint/trim_path.hpp>
+#include <utils/no_op_factory.hpp>
+
+using namespace rive;
 
 TEST_CASE("path metrics compute correctly", "[bezier]")
 {
@@ -25,4 +31,24 @@ TEST_CASE("path metrics compute correctly", "[bezier]")
 
     // float cubicLength = cubicPath.computeLength(identity);
     // REQUIRE(cubicLength == 238.38698f);
-}
+}
+
+// Regression test for a crash found by fuzzing.
+TEST_CASE("fuzz_issue_7295", "[MetricsPath]")
+{
+    NoOpFactory factory;
+
+    OnlyMetricsPath innerPath;
+    innerPath.moveTo(.0f, -20.5f);
+    innerPath.cubicTo(11.3218384f, -20.5f, 20.5f, -11.3218384f, 20.5f, .0f);
+    innerPath.cubicTo(20.5f, 11.3218384f, 11.3218384f, 20.5f, .0f, 20.5f);
+    innerPath.cubicTo(-11.3218384f, 20.5f, -20.5f, 11.3218384f, -20.5f, .0f);
+    innerPath.cubicTo(-20.5f, -11.3218384f, -11.3218384f, -20.5f, .0f, -20.5f);
+
+    OnlyMetricsPath outerPath;
+    outerPath.addPath(&innerPath, Mat2D(1.f, .0f, .0f, 1.f, -134217728.f, -134217728.f));
+
+    RawPath result;
+    outerPath.paths()[0]->trim(.0f, 168.389008f, true, &result);
+    CHECK(math::nearly_equal(outerPath.paths()[0]->length(), 168.389008f));
+}

test/simd_test.cpp

@@ -369,18 +369,30 @@ TEST_CASE("clamp", "[simd]")
 
     // Returns lo if x == NaN, but std::clamp() returns NaN.
     CHECK(simd::clamp<float, 1>(kNaN, 1, 2).x == 1);
+    // Matches math::clamp().
+    CHECK(simd::clamp<float, 1>(kNaN, 1, 2).x == math::clamp(kNaN, 1, 2));
 
     // Returns hi if hi <= lo.
     CHECK(simd::clamp<float, 1>(3, 2, 1).x == 1);
     CHECK(simd::clamp<float, 1>(kNaN, 2, 1).x == 1);
     CHECK(simd::clamp<float, 1>(kNaN, kNaN, 1).x == 1);
+    // Matches math::clamp().
+    CHECK(simd::clamp<float, 1>(3, 2, 1).x == math::clamp(3, 2, 1));
+    CHECK(simd::clamp<float, 1>(kNaN, 2, 1).x == math::clamp(kNaN, 2, 1));
+    CHECK(simd::clamp<float, 1>(kNaN, kNaN, 1).x == math::clamp(kNaN, kNaN, 1));
 
     // Ignores hi and/or lo if they are NaN.
     CHECK(simd::clamp<float, 1>(3, 4, kNaN).x == 4);
     CHECK(simd::clamp<float, 1>(3, 2, kNaN).x == 3);
     CHECK(simd::clamp<float, 1>(3, kNaN, 2).x == 2);
     CHECK(simd::clamp<float, 1>(3, kNaN, 4).x == 3);
     CHECK(simd::clamp<float, 1>(3, kNaN, kNaN).x == 3);
+    // Matches math::clamp().
+    CHECK(simd::clamp<float, 1>(3, 4, kNaN).x == math::clamp(3, 4, kNaN));
+    CHECK(simd::clamp<float, 1>(3, 2, kNaN).x == math::clamp(3, 2, kNaN));
+    CHECK(simd::clamp<float, 1>(3, kNaN, 2).x == math::clamp(3, kNaN, 2));
+    CHECK(simd::clamp<float, 1>(3, kNaN, 4).x == math::clamp(3, kNaN, 4));
+    CHECK(simd::clamp<float, 1>(3, kNaN, kNaN).x == math::clamp(3, kNaN, kNaN));
 }
 
 // Check simd::abs.

viewer/src/viewer_content/textpath_content.cpp

@@ -241,7 +241,7 @@ class TextPathContent : public ViewerContent
         RawPath warp = make_quad_path(m_pathpts);
         this->draw_warp(renderer, warp);
 
-        auto meas = ContourMeasureIter(warp).next();
+        auto meas = ContourMeasureIter(&warp).next();
 
         const float warpLength = meas->length();
         const float textLength = gruns.back().xpos.back();

viewer/src/viewer_content/trimpath_content.cpp

@@ -114,7 +114,7 @@ class TrimPathContent : public ViewerContent
         {
             renderer->save();
 
-            auto cm = ContourMeasureIter(*p, false).next();
+            auto cm = ContourMeasureIter(p, false).next();
             auto p1 = trim(cm.get(), m_trimFrom, m_trimTo);
             stroke_path(renderer, p1, 20, 0xFFFF0000);