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 negative scale.x values didn't render correctly on iOS 14 #1894

Merged
merged 5 commits into from Jan 4, 2023

Conversation

calda
Copy link
Member

@calda calda commented Jan 4, 2023

This PR fixes an issue where negative scale.x values didn't render correctly on iOS 14 when using RenderingEngineOption.automatic. Fixes #1882.

I imagine there's a way to support this properly on iOS 14 and earlier, but I think it's better to just throw a compatibility error in this case:

  1. The existing code path for this doesn't work correctly in snapshot tests so any changes we make are risky and have the potential to cause invisible regressions
  2. It seems good to avoid having two complex code paths with substantially different behavior, especially considering we don't include CI tests for older OS versions.
Before After
Simulator Screen Shot - iPhone 6s (iOS 14) - 2023-01-04 at 11 34 18 Simulator Screen Shot - iPhone 6s (iOS 14) - 2023-01-04 at 11 38 38

context.logger.warn("""
Negative `scale.x` values are not displayed correctly in snapshot tests
""")
}
} else {
if !osSupportsNegativeScaleValues, hasNegativeXScaleValues {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused how this fixes the issue, isn't this just logging an extra warning?

Copy link
Member Author

@calda calda Jan 4, 2023

Choose a reason for hiding this comment

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

The logCompatibilityIssue and compatibilityAssert methods cause RenderingEngineOption.automatic to detect that the animation is unsupported by the core animation engine and should fall-back to using the main thread rendering engine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, got it!

@calda calda merged commit 93ba022 into master Jan 4, 2023
@calda calda deleted the cal--1882 branch January 4, 2023 22:29
iago849 pushed a commit to atteamapps/lottie-ios that referenced this pull request Feb 8, 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.

Old iOS versions have rendering issues [iOS 13]
2 participants