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

Blend mode support #1585

Merged
merged 10 commits into from
May 25, 2022
Merged

Blend mode support #1585

merged 10 commits into from
May 25, 2022

Conversation

k-o-d-e-n
Copy link
Contributor

No description provided.

@calda
Copy link
Member

calda commented May 16, 2022

Thanks for the contribution @k-o-d-e-n! I'm going to convert this to a draft for now since CI isn't passing, but feel free to continue working on this. Once CI passes, I think we can merge this.

  • I think the build is probably failing because the new BlendMode+Filter.swift file isn't referenced in the xcode project.
  • You'll need to run the snapshot test suite to create snapshot images for the sample asset you added

@calda calda marked this pull request as draft May 16, 2022 13:59
@k-o-d-e-n k-o-d-e-n marked this pull request as ready for review May 24, 2022 07:53
@calda
Copy link
Member

calda commented May 24, 2022

@k-o-d-e-n, some of the checks are failing. I think you need to:

  • regenerate the snapshot images for the samples you added, to resolve the Snapshot "Nonanimating/blend_mode_test (0%)" does not match reference error. You can do that by either deleting the file on disk and having it be recreated on the next run, or by adding isRecording = true to the testCoreAnimationEngine() test case and rerunning the test
  • run the Swift liner (bundle exec rake format:swift)

@calda
Copy link
Member

calda commented May 24, 2022

If the snapshot test case keeps failing, then the rendered output must be slightly nondeterministic. You an work around this type of issue by decreasing the comparison precision (example: https://github.com/airbnb/lottie-ios/blob/master/Tests/SnapshotConfiguration.swift#L43)

@k-o-d-e-n
Copy link
Contributor Author

k-o-d-e-n commented May 25, 2022

I have no failed tests with blend_mode_test.json. I don't know why it fails on CI.
Screenshot 2022-05-25 at 13 42 23

@calda
Copy link
Member

calda commented May 25, 2022

I pushed some commits that address the CI failures.

  • For the Typeface/B snapshots, I regenerated them using isRecording = true. Looks like that animation has a blend mode setup so its snapshots were affected by this PR.
  • For the blend_mode_test snapshots, it looks like those were being rendered nondeterministically based on the environment running the tests (we've hit this a few times before). I fixed this by decreasing the comparison precision for that test to 0.99

Will merge once checks pass. Thanks again for your contribution!

@calda calda enabled auto-merge (squash) May 25, 2022 15:53
@calda calda merged commit aabab7f into airbnb:master May 25, 2022
@k-o-d-e-n
Copy link
Contributor Author

Thanks for help!

calda added a commit that referenced this pull request Nov 28, 2022
Co-authored-by: Cal Stephens <[email protected]>
calda added a commit that referenced this pull request Dec 1, 2022
Co-authored-by: Cal Stephens <[email protected]>
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.

2 participants