clone metrics path when a path is added When multiple strokes are added to the same path with trim path applied to them, and some are affected by transform, and some aren't, there's contention between the localPath and worldPath metrics. This PR solves it by storing a unique metrics path per instance added. It copies the values from the original path so they aren't calculated more than needed. There was the same contention with follow paths and trim paths, and it was solved in a slightly similar way, but there might be ramifications I'm missing from this approach. NOTE: it also addresses this [TODO](https://github.com/rive-app/rive/blob/master/packages/runtime/src/shapes/metrics_path.cpp#L72), which ended up not being needed for this fix but left it since there was a TODO for it. I can remove it if it makes things more clear. Diffs= b58df4ba7 clone metrics path when a path is added (#6394) Co-authored-by: hernan <hernan@rive.app>
diff --git a/.rive_head b/.rive_head index 94142ab..3eb61ea 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -124a8f4e23baa003af40515d04686e2960cc5809 +b58df4ba70205b17dbe8f4e8204d0a78becabeaa
diff --git a/include/rive/shapes/metrics_path.hpp b/include/rive/shapes/metrics_path.hpp index 29a7c98..7311b9c 100644 --- a/include/rive/shapes/metrics_path.hpp +++ b/include/rive/shapes/metrics_path.hpp
@@ -35,10 +35,11 @@ /// Add commands to the result RenderPath that will draw the segment /// from startLength to endLength of this MetricsPath. Requires /// computeLength be called prior to trimming. - void trim(float startLength, float endLength, bool moveTo, RenderPath* result); + void trim(float startLength, float endLength, bool moveTo, RawPath* result); /// Add this MetricsPath to a raw path with a transform. RawPath::Iter addToRawPath(RawPath& rawPath, const Mat2D& transform) const; + ~MetricsPath() override; private: float computeLength(const Mat2D& transform);
diff --git a/src/shapes/metrics_path.cpp b/src/shapes/metrics_path.cpp index 3a0dbf5..46fea41 100644 --- a/src/shapes/metrics_path.cpp +++ b/src/shapes/metrics_path.cpp
@@ -2,11 +2,16 @@ #include "rive/renderer.hpp" #include "rive/math/raw_path.hpp" #include "rive/math/contour_measure.hpp" +#include <iostream> using namespace rive; void MetricsPath::rewind() { + for (auto ptr : m_Paths) + { + delete ptr; + } m_Paths.clear(); m_Contour.reset(nullptr); m_RawPath.rewind(); @@ -14,11 +19,19 @@ m_ComputedLength = 0; } +MetricsPath::~MetricsPath() { rewind(); } + void MetricsPath::addPath(CommandPath* path, const Mat2D& transform) { MetricsPath* metricsPath = static_cast<MetricsPath*>(path); m_ComputedLength += metricsPath->computeLength(transform); - m_Paths.emplace_back(metricsPath); + // We need to copy the data to avoid contention between multiple uses of the same path + // for example when the same path is added as localPath and worldPath + auto metricsPathCopy = new OnlyMetricsPath(); + metricsPathCopy->m_Contour = metricsPath->m_Contour; + metricsPathCopy->m_RawPath = metricsPath->m_RawPath; + metricsPathCopy->m_ComputedLength = metricsPath->m_ComputedLength; + m_Paths.emplace_back(metricsPathCopy); } RawPath::Iter MetricsPath::addToRawPath(RawPath& rawPath, const Mat2D& transform) const @@ -56,7 +69,7 @@ return m_ComputedLength; } -void MetricsPath::trim(float startLength, float endLength, bool moveTo, RenderPath* result) +void MetricsPath::trim(float startLength, float endLength, bool moveTo, RawPath* result) { assert(endLength >= startLength); if (!m_Paths.empty()) @@ -73,8 +86,7 @@ // rawpaths, we wouldn't need this temporary copy (since ContourMeasure speaks // native rawpaths). RawPath tmp; - m_Contour->getSegment(startLength, endLength, &tmp, moveTo); - tmp.addTo(result); + m_Contour->getSegment(startLength, endLength, result, moveTo); } RenderMetricsPath::RenderMetricsPath(rcp<RenderPath> path) : m_RenderPath(std::move(path)) {}
diff --git a/src/shapes/paint/trim_path.cpp b/src/shapes/paint/trim_path.cpp index d099330..7fad542 100644 --- a/src/shapes/paint/trim_path.cpp +++ b/src/shapes/paint/trim_path.cpp
@@ -27,6 +27,8 @@ // Source is always a containing (shape) path. const std::vector<MetricsPath*>& subPaths = source->paths(); + RawPath rawTrimmed; + if (!m_TrimmedPath) { m_TrimmedPath = factory->makeEmptyRenderPath(); @@ -66,7 +68,7 @@ if (startLength < pathLength) { - path->trim(startLength, endLength, true, m_TrimmedPath.get()); + path->trim(startLength, endLength, true, &rawTrimmed); endLength -= pathLength; startLength = 0; } @@ -99,12 +101,12 @@ startLength -= pathLength; endLength -= pathLength; } - path->trim(startLength, endLength, true, m_TrimmedPath.get()); + path->trim(startLength, endLength, true, &rawTrimmed); while (endLength > pathLength) { startLength = 0; endLength -= pathLength; - path->trim(startLength, endLength, true, m_TrimmedPath.get()); + path->trim(startLength, endLength, true, &rawTrimmed); } } } @@ -112,6 +114,7 @@ } m_RenderPath = m_TrimmedPath.get(); + rawTrimmed.addTo(m_RenderPath); return m_RenderPath; }