fix: contour bugs with new RectanglesToContour # Description - I caught a bug with multiple contours due to the inclusion setter always using the first rect, so it would fail on rects that would create multiple contours. - Also fixes an issue with the unique point sorter not using the stored vectors. I messed something up when rebasing that previous pr. Adds a test for the multiple contour issue. Diffs= 53131428a6 fix: contour bugs with new RectanglesToContour (#9450) Co-authored-by: Luigi Rosso <luigi-rosso@users.noreply.github.com>
diff --git a/.rive_head b/.rive_head index 7635b52..447e3c3 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -b4066558a126ad40b9049fe70b99f2ee6b43b3b7 +53131428a668ea7907d47de9fb36501dc7990dda
diff --git a/include/rive/math/rectangles_to_contour.hpp b/include/rive/math/rectangles_to_contour.hpp index 6a1581e..f87e944 100644 --- a/include/rive/math/rectangles_to_contour.hpp +++ b/include/rive/math/rectangles_to_contour.hpp
@@ -2,10 +2,28 @@ #define _RIVE_RECTANGLES_TO_CONTOUR_HPP_ #include <vector> #include <unordered_set> -#include <unordered_map> + #include "rive/math/mat2d.hpp" #include "rive/math/aabb.hpp" +#ifdef TESTING +// When building for testing we use an ordered map for the EdgeMap so we can get +// crossplatform stable order of contours. Visually this doesn't matter but for +// testing across results across different platforms, it does. +#include <map> +struct EdgeMapTesting +{ + bool operator()(const rive::Vec2D& a, const rive::Vec2D& b) const + { + return a.x < b.x || (a.x == b.x && a.y < b.y); + } +}; +using EdgeMap = std::map<rive::Vec2D, rive::Vec2D, EdgeMapTesting>; +#else +#include <unordered_map> +using EdgeMap = std::unordered_map<rive::Vec2D, rive::Vec2D>; +#endif + namespace rive { struct ContourPoint @@ -148,8 +166,8 @@ // around when you know you'll need to recompute the contour from a set of // rectangles often/in rapid succession. std::vector<RectEvent> m_rectEvents; - std::unordered_map<Vec2D, Vec2D> m_edgesH; - std::unordered_map<Vec2D, Vec2D> m_edgesV; + EdgeMap m_edgesH; + EdgeMap m_edgesV; std::unordered_set<Vec2D> m_uniquePoints; std::vector<AABB> m_rects; std::vector<AABB> m_subdividedRects;
diff --git a/src/math/rectangles_to_contour.cpp b/src/math/rectangles_to_contour.cpp index 51e3087..ba09e64 100644 --- a/src/math/rectangles_to_contour.cpp +++ b/src/math/rectangles_to_contour.cpp
@@ -1,8 +1,8 @@ #include "rive/math/rectangles_to_contour.hpp" #include <algorithm> -#include <unordered_map> using namespace rive; + using RectEvent = RectanglesToContour::RectEvent; class SpanOffset @@ -106,7 +106,7 @@ for (size_t i = 0; i < ev.size() - 1; ++i) { const auto& eventV = ev[i]; - markRectIncluded(ev[0].index, eventV.type == 0); + markRectIncluded(eventV.index, eventV.type == 0); const auto& next = ev[i + 1]; float beginX = eventV.x; float endX = next.x; @@ -171,21 +171,12 @@ m_rects.push_back(rect); } -template <typename Compare> -std::vector<Vec2D> sortPoints(const std::unordered_set<Vec2D>& points, - Compare comp) -{ - std::vector<Vec2D> sortedPoints(points.begin(), points.end()); - std::sort(sortedPoints.begin(), sortedPoints.end(), comp); - return sortedPoints; -} - // Build contours and append them into contourPoints delineated by // contourOffsets. void extractPolygons(std::vector<ContourPoint>& contourPoints, std::vector<size_t>& contourOffsets, - std::unordered_map<Vec2D, Vec2D>& edgesH, - std::unordered_map<Vec2D, Vec2D>& edgesV) + EdgeMap& edgesH, + EdgeMap& edgesV) { while (!edgesH.empty()) @@ -258,13 +249,23 @@ addUniquePoint(Vec2D(rect.minX, rect.maxY)); } - auto sortX = sortPoints(m_uniquePoints, [](const Vec2D& a, const Vec2D& b) { - return a.x < b.x || (a.x == b.x && a.y < b.y); - }); - - auto sortY = sortPoints(m_uniquePoints, [](const Vec2D& a, const Vec2D& b) { - return a.y < b.y || (a.y == b.y && a.x < b.x); - }); + m_sortedPointsX.clear(); + m_sortedPointsY.clear(); + for (auto pt : m_uniquePoints) + { + m_sortedPointsX.push_back(pt); + m_sortedPointsY.push_back(pt); + } + std::sort(m_sortedPointsX.begin(), + m_sortedPointsX.end(), + [](const Vec2D& a, const Vec2D& b) { + return a.x < b.x || (a.x == b.x && a.y < b.y); + }); + std::sort(m_sortedPointsY.begin(), + m_sortedPointsY.end(), + [](const Vec2D& a, const Vec2D& b) { + return a.y < b.y || (a.y == b.y && a.x < b.x); + }); // std::unordered_map isn't guaranteed to reserve memory, so this clear // is more in prep for when we allow using allocators. We could also @@ -274,25 +275,25 @@ m_edgesV.clear(); size_t i = 0; - while (i < sortY.size()) + while (i < m_sortedPointsY.size()) { - float currY = sortY[i].y; - while (i < sortY.size() && sortY[i].y == currY) + float currY = m_sortedPointsY[i].y; + while (i < m_sortedPointsY.size() && m_sortedPointsY[i].y == currY) { - m_edgesH[sortY[i]] = sortY[i + 1]; - m_edgesH[sortY[i + 1]] = sortY[i]; + m_edgesH[m_sortedPointsY[i]] = m_sortedPointsY[i + 1]; + m_edgesH[m_sortedPointsY[i + 1]] = m_sortedPointsY[i]; i += 2; } } i = 0; - while (i < sortX.size()) + while (i < m_sortedPointsX.size()) { - float currX = sortX[i].x; - while (i < sortX.size() && sortX[i].x == currX) + float currX = m_sortedPointsX[i].x; + while (i < m_sortedPointsX.size() && m_sortedPointsX[i].x == currX) { - m_edgesV[sortX[i]] = sortX[i + 1]; - m_edgesV[sortX[i + 1]] = sortX[i]; + m_edgesV[m_sortedPointsX[i]] = m_sortedPointsX[i + 1]; + m_edgesV[m_sortedPointsX[i + 1]] = m_sortedPointsX[i]; i += 2; } }
diff --git a/tests/unit_tests/runtime/rectangles_to_contour_test.cpp b/tests/unit_tests/runtime/rectangles_to_contour_test.cpp index 366053d..540bba8 100644 --- a/tests/unit_tests/runtime/rectangles_to_contour_test.cpp +++ b/tests/unit_tests/runtime/rectangles_to_contour_test.cpp
@@ -12,8 +12,27 @@ rectanglesToContour.computeContours(); CHECK(rectanglesToContour.contourCount() == 1); CHECK(rectanglesToContour.contour(0).size() == 4); - CHECK_VEC2D(rectanglesToContour.contour(0).point(0), Vec2D(30.0f, 20.0f)); - CHECK_VEC2D(rectanglesToContour.contour(0).point(1), Vec2D(30.0f, 10.0f)); - CHECK_VEC2D(rectanglesToContour.contour(0).point(2), Vec2D(10.0f, 10.0f)); - CHECK_VEC2D(rectanglesToContour.contour(0).point(3), Vec2D(10.0f, 20.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(0), Vec2D(10.0f, 10.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(1), Vec2D(10.0f, 20.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(2), Vec2D(30.0f, 20.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(3), Vec2D(30.0f, 10.0f)); + + rectanglesToContour.reset(); + rectanglesToContour.addRect(AABB(10.0f, 10.0f, 20.0f, 20.0f)); + rectanglesToContour.addRect(AABB(20.0f, 10.0f, 30.0f, 20.0f)); + rectanglesToContour.addRect(AABB(20.0f, 40.0f, 30.0f, 50.0f)); + + rectanglesToContour.computeContours(); + CHECK(rectanglesToContour.contourCount() == 2); + CHECK(rectanglesToContour.contour(0).size() == 4); + CHECK_VEC2D(rectanglesToContour.contour(0).point(0), Vec2D(10.0f, 10.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(1), Vec2D(10.0f, 20.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(2), Vec2D(30.0f, 20.0f)); + CHECK_VEC2D(rectanglesToContour.contour(0).point(3), Vec2D(30.0f, 10.0f)); + + CHECK(rectanglesToContour.contour(1).size() == 4); + CHECK_VEC2D(rectanglesToContour.contour(1).point(0), Vec2D(20.0f, 40.0f)); + CHECK_VEC2D(rectanglesToContour.contour(1).point(1), Vec2D(20.0f, 50.0f)); + CHECK_VEC2D(rectanglesToContour.contour(1).point(2), Vec2D(30.0f, 50.0f)); + CHECK_VEC2D(rectanglesToContour.contour(1).point(3), Vec2D(30.0f, 40.0f)); } \ No newline at end of file