Skip to content

Commit

Permalink
clone metrics path when a path is added
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
bodymovin and bodymovin committed Jan 4, 2024
1 parent bb903d5 commit 8af05e7
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .rive_head
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124a8f4e23baa003af40515d04686e2960cc5809
b58df4ba70205b17dbe8f4e8204d0a78becabeaa
3 changes: 2 additions & 1 deletion include/rive/shapes/metrics_path.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ class MetricsPath : public CommandPath
/// 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);
Expand Down
20 changes: 16 additions & 4 deletions src/shapes/metrics_path.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,36 @@
#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();
m_ComputedLengthTransform = Mat2D();
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
Expand Down Expand Up @@ -56,7 +69,7 @@ float MetricsPath::computeLength(const Mat2D& transform)
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())
Expand All @@ -73,8 +86,7 @@ void MetricsPath::trim(float startLength, float endLength, bool moveTo, RenderPa
// 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)) {}
Expand Down
9 changes: 6 additions & 3 deletions src/shapes/paint/trim_path.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ RenderPath* TrimPath::effectPath(MetricsPath* source, Factory* factory)
// Source is always a containing (shape) path.
const std::vector<MetricsPath*>& subPaths = source->paths();

RawPath rawTrimmed;

if (!m_TrimmedPath)
{
m_TrimmedPath = factory->makeEmptyRenderPath();
Expand Down Expand Up @@ -66,7 +68,7 @@ RenderPath* TrimPath::effectPath(MetricsPath* source, Factory* factory)

if (startLength < pathLength)
{
path->trim(startLength, endLength, true, m_TrimmedPath.get());
path->trim(startLength, endLength, true, &rawTrimmed);
endLength -= pathLength;
startLength = 0;
}
Expand Down Expand Up @@ -99,19 +101,20 @@ RenderPath* TrimPath::effectPath(MetricsPath* source, Factory* factory)
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);
}
}
}
break;
}

m_RenderPath = m_TrimmedPath.get();
rawTrimmed.addTo(m_RenderPath);
return m_RenderPath;
}

Expand Down

0 comments on commit 8af05e7

Please sign in to comment.