From ec16c688dbb8808d446eddfd4af07fd0536bb0fc Mon Sep 17 00:00:00 2001 From: Mira Grudzinska Date: Wed, 18 Jun 2025 23:55:28 +0200 Subject: [PATCH] lottie: ensure proper shape closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Ensured proper closure of star and polygon shapes. The start and end points now match - in cases with degenerate bezier curves, the second-to-last point is also aligned. Proper shape closure is necessary for modifiers like offset (future pucker bloat). If the start and end points aren’t equal (within the comparison function’s precision), the shape will be closed with a straight line - and during offsetting, that line will become visible, even though it’s not intended. - Use of the internal _zero() function for point equality check in modifiers algs led to incorrect results when p2.x or p2.y was zero (division by zero). The intent was to treat nearly identical points as equal, but this approach was flawed - at the modifier stage, it’s no longer possible to tell if small gaps are intentional or just due to limited numerical precision (as seen for example in the difference between the start and end points of star/polygon shapes). --- src/loaders/lottie/tvgLottieBuilder.cpp | 11 +++++++++++ src/loaders/lottie/tvgLottieModifier.cpp | 13 +++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/loaders/lottie/tvgLottieBuilder.cpp b/src/loaders/lottie/tvgLottieBuilder.cpp index dd100833..5ceecff5 100644 --- a/src/loaders/lottie/tvgLottieBuilder.cpp +++ b/src/loaders/lottie/tvgLottieBuilder.cpp @@ -581,6 +581,13 @@ void LottieBuilder::updatePath(LottieGroup* parent, LottieObject** child, float } +static void _close(Array& pts, const Point& p, bool round) +{ + if (round && tvg::zero(pts.last() - pts[pts.count - 2])) pts[pts.count - 2] = p; + pts.last() = p; +} + + static void _updateStar(TVG_UNUSED LottieGroup* parent, LottiePolyStar* star, Matrix* transform, const LottieRoundnessModifier* roundness, const LottieOffsetModifier* offsetPath, float frameNo, Shape* merging, LottieExpressions* exps) { static constexpr auto POLYSTAR_MAGIC_NUMBER = 0.47829f / 0.28f; @@ -695,6 +702,8 @@ static void _updateStar(TVG_UNUSED LottieGroup* parent, LottiePolyStar* star, Ma angle += dTheta * direction; longSegment = !longSegment; } + //ensure proper shape closure - important for modifiers that behave differently for degenerate (linear) vs curved cubics + _close(P(shape)->rs.path.pts, in, hasRoundness); shape->close(); if (roundedCorner) { @@ -777,6 +786,8 @@ static void _updatePolygon(LottieGroup* parent, LottiePolyStar* star, Matrix* tr } angle += anglePerPoint * direction; } + //ensure proper shape closure - important for modifiers that behave differently for degenerate (linear) vs curved cubics + _close(P(shape)->rs.path.pts, in, hasRoundness); shape->close(); if (roundedCorner) { diff --git a/src/loaders/lottie/tvgLottieModifier.cpp b/src/loaders/lottie/tvgLottieModifier.cpp index 0fa00aa3..a5c01401 100644 --- a/src/loaders/lottie/tvgLottieModifier.cpp +++ b/src/loaders/lottie/tvgLottieModifier.cpp @@ -46,16 +46,9 @@ static void _roundCorner(Array& cmds, Array& pts, const Poin } -static bool _zero(const Point& p1, const Point& p2) -{ - constexpr float epsilon = 1e-3f; - return fabsf(p1.x / p2.x - 1.0f) < epsilon && fabsf(p1.y / p2.y - 1.0f) < epsilon; -} - - static bool _intersect(const Line& line1, const Line& line2, Point& intersection, bool& inside) { - if (_zero(line1.pt2, line2.pt1)) { + if (tvg::zero(line1.pt2 - line2.pt1)) { intersection = line1.pt2; inside = true; return true; @@ -162,7 +155,7 @@ void LottieOffsetModifier::line(const PathCommand* inCmds, uint32_t inCmdsCnt, c Line nextLine = state.firstLine; if (inCmds[currentCmd + 1] == PathCommand::LineTo) nextLine = _offset(inPts[currentPt + degenerated], inPts[currentPt + 1 + degenerated], offset); else if (inCmds[currentCmd + 1] == PathCommand::CubicTo) nextLine = _offset(inPts[currentPt + 1 + degenerated], inPts[currentPt + 2 + degenerated], offset); - else if (inCmds[currentCmd + 1] == PathCommand::Close && !_zero(inPts[currentPt + degenerated], inPts[state.movetoInIndex + degenerated])) + else if (inCmds[currentCmd + 1] == PathCommand::Close && !tvg::zero(inPts[currentPt + degenerated] - inPts[state.movetoInIndex + degenerated])) nextLine = _offset(inPts[currentPt + degenerated], inPts[state.movetoInIndex + degenerated], offset); corner(state.line, nextLine, state.movetoOutIndex, inCmds[currentCmd + 1] == PathCommand::Close, outCmds, outPts); @@ -368,7 +361,7 @@ bool LottieOffsetModifier::modifyPath(const PathCommand* inCmds, uint32_t inCmds iPt += 3; } else { - if (!_zero(inPts[iPt - 1], inPts[state.movetoInIndex])) { + if (!tvg::zero(inPts[iPt - 1] - inPts[state.movetoInIndex])) { outCmds.push(PathCommand::LineTo); corner(state.line, state.firstLine, state.movetoOutIndex, true, outCmds, outPts); }