Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where trim offset could be calculated incorrectly #2053

Merged
merged 5 commits into from
Apr 27, 2023
Merged

Conversation

calda
Copy link
Member

@calda calda commented Apr 27, 2023

This PR fixes an issue where trim offsets could be calculated incorrectly, causing the animation to be rendered incorrectly. Fixes #1979.

The previous implementation from #1612 predated Keyframes.combined so was totally custom, and must have had a bug somewhere. Consolidating this to use the same interpolation code we use elsewhere also fixes some other minor issues in the regression suite where some stroke trims were rendered in a very-slightly different way between the two engines.

Before After
2023-04-27 12 31 33 2023-04-27 12 30 35

Copy link
Collaborator

@erichoracek erichoracek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-150 net LoC and a bugfix, nice!

Comment on lines +149 to +150
let offsetStart = start.cgFloatValue + (offset.cgFloatValue / 360 * 100)
let offsetEnd = end.cgFloatValue + (offset.cgFloatValue / 360 * 100)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe discuss why we have these values we're dividing and multiplying by

@calda calda enabled auto-merge (squash) April 27, 2023 22:24
@calda calda disabled auto-merge April 27, 2023 22:27
@calda calda merged commit 8558603 into master Apr 27, 2023
@calda calda deleted the cal--1979 branch April 27, 2023 22:27
@gsbernstein
Copy link

Thank you!

iago849 pushed a commit to atteamapps/lottie-ios that referenced this pull request Feb 8, 2024
MoroziOS pushed a commit to MoroziOS/tmg-lottie-ios that referenced this pull request May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Animation behavior changed in new render engine, breaking loop
3 participants