lottie: fix repeater transformation #2280
Conversation
There was a problem hiding this comment.
@mgrudzinska Hello, it would be nice if you share the test resource here.
Unfortunately, this breaks the existing resources: in examples/lottie:
5317-fireworkds.json, abstract_circle.json
Need to confirm all works correctly.
mgrudzinska
force-pushed
the
mira/repeater_fix
branch
2 times, most recently
from
6d74348
to
552aa4d
Compare
|
It is a working solution, but I'm not entirely satisfied with the use of array repeaters. |
| @@ -59,7 +60,7 @@ struct RenderContext | |||
| Shape* propagator = nullptr; | |||
| Shape* merging = nullptr; //merging shapes if possible (if shapes have same properties) | |||
| LottieObject** begin = nullptr; //iteration entry point | |||
| RenderRepeater* repeater = nullptr; | |||
| Array<RenderRepeater*> repeaters; | |||
There was a problem hiding this comment.
Array<RenderRepeater> repeaters;
There was a problem hiding this comment.
I'll change it
| @@ -49,6 +49,7 @@ struct RenderRepeater | |||
| uint8_t endOpacity; | |||
| bool interpOpacity; | |||
| bool inorder; | |||
| bool external; | |||
There was a problem hiding this comment.
We strongly recommend adding a comment for this kind of ambiguous data.
There was a problem hiding this comment.
I changed the approach, since I've found nested examples. I need to know whether the level of repeater was greater or it was equal to the level of repeated shape
| @@ -1082,7 +1096,7 @@ static void _updateChildren(LottieGroup* parent, float frameNo, Inlist<RenderCon | |||
| break; | |||
| } | |||
| case LottieObject::Repeater: { | |||
| _updateRepeater(parent, child, frameNo, contexts, ctx, exps); | |||
| _updateRepeater(parent, child, frameNo, contexts, ctx, exps, groupLevel == level); | |||
There was a problem hiding this comment.
Here, the level looks over-programmed. You might wanna just confirm whether the repeater is requested after the group.
mgrudzinska
force-pushed
the
mira/repeater_fix
branch
from
674e56e
to
5036fc4
Compare
Depending on whether the repeater is an element of the shapes list or the shape layer, the repeater's transformation should be applied either before or after the shape's transformation, respectively.
mgrudzinska
force-pushed
the
mira/repeater_fix
branch
from
5036fc4
to
6a1ec07
Compare
|
samples: main: this pr: currently there are no differences regardless of whether the repeater is inside the group (before the transformation) or outside (after the transformation): Based on whether the repeater was called before or after the group, I cannot determine which transformation should be applied first. Only at the stage of updating the shape and cloning it, in the _repeat() can I decide, knowing the level values of both the shape and the repeater, the order in which to apply the transformations. regarding using an array to store all the repeaters: |
| if (rhs.repeater) { | ||
| repeater = new RenderRepeater(); | ||
| *repeater = *rhs.repeater; | ||
| if (!rhs.repeaters.empty()) { |
There was a problem hiding this comment.
if (!rhs.repeaters.empty()) {
...
}
->
repeaters.push(rhs.repeaters);
| @@ -40,6 +40,7 @@ | |||
| struct RenderRepeater | |||
| { | |||
| int cnt; | |||
| uint32_t level; | |||
There was a problem hiding this comment.
What do you think if we put this uint16_t level in the RenderContext. I guess the code could be more compact & clean.
|
combined with #2311 |
Matrix multiplication order changed.
before:
after:
AE:
