lottie: fix offsetting partially degenerated cubics #3523
Conversation
Introduce the _colinear function - checks if a Bezier curve is degenerated to a line.
Until now, cases where a Bezier curve had `start == ctrl1` or `ctrl2 == end` were incorrectly treated as linear segments. This led to incorrect rendering, for example when offsetting a polystar with outer roundness > 0 and inner roundness == 0. Now such cases handled as proper curves with full cubic behavior.
|
is ready, but I want to test more (more complicated cases can be tested after modifiers chaining is applied - more combinations of lineTo/cubicTo possible) |
There was a problem hiding this comment.
Pull Request Overview
This PR improves how partially degenerated cubic Beziers (where start==ctrl1 or ctrl2==end) are detected and offset, fixing rendering artifacts for shapes like polystars with mixed roundness.
- Introduces a
_colinearhelper to detect degenerate cubic segments. - Updates
LottieRoundnessModifierto treat these as full cubics when rounding corners. - Enhances
LottieOffsetModifierto maintain continuity across degenerate lines and compute proper intersections.
Comments suppressed due to low confidence (1)
src/loaders/lottie/tvgLottieModifier.cpp:30
- [nitpick] Rename
_colinearto_collinearto correct the spelling of 'collinear' and improve readability.
static bool _colinear(const Point* p)
| @@ -328,6 +330,10 @@ bool LottieOffsetModifier::modifyPath(PathCommand* inCmds, uint32_t inCmdsCnt, P | |||
| auto offset = _clockwise(inPts, inPtsCnt) ? this->offset : -this->offset; | |||
| auto threshold = 1.0f / fabsf(offset) + 1.0f; | |||
|
|
|||
| bool inside{}; | |||
There was a problem hiding this comment.
The inside variable is declared but never used. Consider removing it or using it to validate intersection results.
| bool inside{}; |
Copilot uses AI. Check for mistakes.
| @@ -348,17 +354,27 @@ bool LottieOffsetModifier::modifyPath(PathCommand* inCmds, uint32_t inCmdsCnt, P | |||
| auto& bezier = stack.last(); | |||
| auto len = tvg::length(bezier.start - bezier.ctrl1) + tvg::length(bezier.ctrl1 - bezier.ctrl2) + tvg::length(bezier.ctrl2 - bezier.end); | |||
|
|
|||
| if (len > threshold * bezier.length()) { | |||
| if (len > threshold * bezier.length() && len > 1.0f) { | |||
There was a problem hiding this comment.
[nitpick] The literal 1.0f here is a magic number. Extract it into a named constant to clarify its purpose in the subdivision threshold check.
| if (len > threshold * bezier.length() && len > 1.0f) { | |
| if (len > threshold * bezier.length() && len > SUBDIVISION_THRESHOLD) { |
Copilot uses AI. Check for mistakes.
| Bezier next; | ||
| bezier.split(0.5f, next); | ||
| stack.push(next); | ||
| continue; | ||
| } | ||
| stack.pop(); | ||
|
|
||
| auto line1 = _offset(bezier.start, bezier.ctrl1, offset); | ||
| degeneratedLine1 = _zero(bezier.start, bezier.ctrl1); |
There was a problem hiding this comment.
The _zero helper divides coordinates and may trigger division by zero if any component is zero. Prefer using tvg::zero or a safer comparison to avoid potential runtime errors.
Copilot uses AI. Check for mistakes.
fcbdcc9 to
e20562c
Compare
Until now, cases where a Bezier curve had
start == ctrl1orctrl2 == endwere incorrectly treated as linear segments.This led to incorrect rendering, for example when offsetting
a polystar with outer roundness > 0 and inner roundness == 0.
Now such cases handled as proper curves with full cubic behavior.
before:
after:
AE (even here visible some artefacts):
samples:
sample1.json
sample2.json