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

feat: Release [email protected] #6459

Closed
wants to merge 7 commits into from
Closed

feat: Release [email protected] #6459

wants to merge 7 commits into from

Conversation

tjzel
Copy link
Collaborator

@tjzel tjzel commented Aug 28, 2024

Summary

Not to be merged. Submitting the PR for better visibility of ran CIs etc.

PRs cherry picked into the patch:

I also had to cherry-pick a bunch of GitHub Actions PRs to be able to run them.

Test plan

bartlomiejbloniarz and others added 3 commits August 28, 2024 16:25
## Summary
Currently we schedule`rAF` flush when the first callback is added to the
list. However the flush method can also be called by an event, meaning
that sometimes we have a flush scheduled to run on a given frame, but
the callbacks array is empty. If then, another callback is requested,
the array will contain 1 element, triggering another flush request (even
though one is already scheduled). To prevent this, from causing
countless requests on a singe frame we check the frame timestamp and
abort if it's repeated.

This approach unfortunately causes some problems when video is playing
in the app. On iPhone devices sometimes the displayLink can fire its
callback twice in a single frame (with the same timestamp). This leads
to us cancelling the `rAF` flush, which means that until an event
triggers a flush, none updates from reanimated will come through.

This PR changes the way we request a flush. Instead of checking the
callbacks array size, we instead remember whether a flush was requested
and request a new one only when there was no request. This prevents us
from requesting unnecessary flushes when an event has caused the
callbacks array to be emptied, while also allowing for repeated frame
timestamps.

closes #6371 

## Test plan

Check for regressions in the example app and in this example:
https://gist.github.com/kmagiera/b2df85f9512951f5e6ceee7bc569f5f1
@tjzel tjzel changed the title feat: Release [email protected] feat: Release [email protected] Aug 28, 2024
@tjzel tjzel changed the base branch from main to @tjzel/release/3.15.1-target August 28, 2024 14:45
@tjzel tjzel changed the base branch from @tjzel/release/3.15.1-target to main August 28, 2024 14:47
@tjzel
Copy link
Collaborator Author

tjzel commented Aug 28, 2024

GitHub actions are beyond salvation here...

@tjzel tjzel closed this Aug 28, 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