lottie: modifier chaining enhanced #3515
Conversation
@hermet please check now. I removed support for 'lineTo' everywhere I could. but since we have polygons and rectangles, it was not possible to remove all lineTo cases. |
Previously, the rounding modifier was always applied first, which led to incorrect results when a different modifier order was expected. This update removes that limitation and enables modifiers to be applied in any order. Additionally, the code now prevents an infinite loop that occurred when the rounded corners modifier was applied multiple times (a cycle where next was set to this).
Enables applying modifiers to rectangles in any order, with the exception that if roundness is the first modifier, it can be applied before the rectangle’s path is constructed.
Refactoring ellipse/polygon/star update to prepare for chained modifiers.
the above is outdated already. |
| Type type; | ||
| bool chained = false; |
There was a problem hiding this comment.
This variable looks an unnecessary complexity. doesn't mean a valid next chained?
There was a problem hiding this comment.
@hermet can you please rephrase this sentence: "doesn't mean a valid next chained?"?
I introduced it to handle cases a->b->...->a
| current chain | add | inside decorate |
|---|---|---|
| a | b | a->chaining = true; return b->a |
| b->a | c | b->chaining = true; return c->b->a |
| c->b->a | a | a->chaining == true; return c->b->a |
| without chaining check: | ||
| c->b->a | a | c->chaining = true; return a->c->b->a->.... - CYCLE |
There was a problem hiding this comment.
Got it, Do you have any sample where the exception occurs at line 53?
| @@ -391,9 +391,9 @@ static void _repeat(LottieGroup* parent, Shape* path, RenderContext* ctx) | |||
| } | |||
|
|
|||
|
|
|||
| void LottieBuilder::appendRect(Shape* shape, Point& pos, Point& size, float r, bool clockwise, RenderContext* ctx) | |||
| void LottieBuilder::appendRect(Shape* shape, Point& pos, Point& size, float r, bool clockwise, RenderContext* ctx, LottieModifier* modifier) | |||
There was a problem hiding this comment.
Please double-check if possible to use ctx rather than an additional parameter.
| @@ -417,38 +418,56 @@ void LottieBuilder::updateRect(LottieGroup* parent, LottieObject** child, float | |||
| auto size = rect->size(frameNo, tween, exps); | |||
| auto pos = rect->position(frameNo, tween, exps) - size * 0.5f; | |||
| auto r = rect->radius(frameNo, tween, exps); | |||
| auto modifier = ctx->modifier; | |||
|
|
||
| if (r == 0.0f) { | ||
| if (ctx->roundness) ctx->roundness->modifyRect(size, r); | ||
| //roundness is the first modifier -> it can be applied before the shape's path is established |
There was a problem hiding this comment.
I think this can be delegated to the relying modifier itself, allowing it to properly handle the logic internally by receiving the current shape target.
ie:
//modifies could query the target type (rect, ellipse, path, polystar, etc)
modifyPath(... LottieShape* target)
you could also improve all the if-else condition other places with the above approach.
|
|
||
| //just in the order. |
There was a problem hiding this comment.
Rather change the method concept, Please consider modifying the callers.
| Type type; | ||
| bool chained = false; |
There was a problem hiding this comment.
Got it, Do you have any sample where the exception occurs at line 53?
Previously, the rounding modifier was always applied
first, which led to incorrect results when a different
modifier order was expected.
This update removes that limitation and enables modifiers
to be applied in any order.
Additionally, the code now prevents an infinite loop that
occurred when the rounded corners modifier was applied
multiple times (a cycle where next was set to this).
&
Enables applying modifiers to rectangles in any order,
with the exception that if roundness is the first modifier,
it can be applied before the rectangle’s path is constructed.
&
Refactoring ellipse/polygon/star update to prepare for
chained modifiers.
before:
after (only after #3556 is applied):
samples:
star_or.json
star_ro.json
path_or.json
path_ro.json