Make all events on iOS flush updates immediately #4097
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
piaskowyk
approved these changes
Feb 24, 2023
fluiddot
pushed a commit
to wordpress-mobile/react-native-reanimated
that referenced
this pull request
Jun 5, 2023
## Summary Before this change we'd only call `performOperations` for a subset of filtered events (initially introduced in software-mansion#312). The event's that weren't considered "direct" would require additional animation frame for having their updates flushed. This change makes it so that we call `performOperations` for all types of events therefore matching the behavior [on Android](https://github.com/software-mansion/react-native-reanimated/blob/99b8b3ed56e36ca615cce7164ccaf04d154571b1/android/src/main/java/com/swmansion/reanimated/NodesManager.java#L283) A consequence of this change was that for some event types, the updates reanimated was performing weren't getting flushed onto screen. Namely, we noticed this problem in Pager example where a view pager component with some custom set of events is used and event handlers update shared values. Even though such event would trigger shared value update, and these updates would trigger the style to recalculate and we'd even call updateProps method to apply the updated props, we'd still see no result as in that example the changes require layout run. Without `performOperation` call the layout would not be executed unless react would rerender or other time-based animation would run. The problem became apparent after software-mansion#3970 where we changed the place where updates are performed from requestAnimationFrame to setImmediate. Before this change, since we were running the updates in "animation frame" the `performOperation` method was being run by the frame scheduler. We, however were getting these updates delayed by one frame because of that. This issue also wasn't noticed prior to shareable rewrite from software-mansion#3722 because before, we were always starting frame updater for every single update happening to shared value even if it was due to an event. As a result, we were getting the stuff updated on screen but again, with a delay of one frame. ## Test plan Run pager example on iOS.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Before this change we'd only call
performOperationsfor a subset of filtered events (initially introduced in #312). The event's that weren't considered "direct" would require additional animation frame for having their updates flushed. This change makes it so that we callperformOperationsfor all types of events therefore matching the behavior on AndroidA consequence of this change was that for some event types, the updates reanimated was performing weren't getting flushed onto screen. Namely, we noticed this problem in Pager example where a view pager component with some custom set of events is used and event handlers update shared values. Even though such event would trigger shared value update, and these updates would trigger the style to recalculate and we'd even call updateProps method to apply the updated props, we'd still see no result as in that example the changes require layout run. Without
performOperationcall the layout would not be executed unless react would rerender or other time-based animation would run.The problem became apparent after #3970 where we changed the place where updates are performed from requestAnimationFrame to setImmediate. Before this change, since we were running the updates in "animation frame" the
performOperationmethod was being run by the frame scheduler. We, however were getting these updates delayed by one frame because of that. This issue also wasn't noticed prior to shareable rewrite from #3722 because before, we were always starting frame updater for every single update happening to shared value even if it was due to an event. As a result, we were getting the stuff updated on screen but again, with a delay of one frame.Test plan
Run pager example on iOS.