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. |
the above is outdated already. |
| LottieModifier* next = nullptr; | ||
| RenderPath* buffer; | ||
| 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?
|
|
||
| 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.
| } | ||
| this->chained = true; | ||
|
|
||
| //just in the order. |
There was a problem hiding this comment.
Rather change the method concept, Please consider modifying the callers.
| LottieModifier* next = nullptr; | ||
| RenderPath* buffer; | ||
| 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?
9b6c8bc to
a6ffb7a
Compare
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.
Refactoring ellipse/polygon/star update to prepare for chained modifiers.
7ceb794 to
43923a5
Compare
9ddde24 to
dac61de
Compare
30c82d7 to
0c3e8ff
Compare
7b0fe6f to
811aac3
Compare
d471af2 to
0aaa738
Compare
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