-
-
Notifications
You must be signed in to change notification settings - Fork 56
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: eliminate one frame delay in onMove
handler on iOS
#412
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
📊 Package size report
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
kirillzyusko
commented
Apr 16, 2024
4cde354
to
3218f3e
Compare
96d43c8
to
18c64be
Compare
…ation level, handle concurrent animations, remove willChange handler (since it doesn't give any benefits), repalce .duration with ONE_FRAME, rename displaylink to link, added TODO list
…ded because when we calculated duration as `targetTimestamp - beginTime (0.0)`, we were getting a very big value and because of that first animation frame was as a final one, so we had a jump. But since now we handle concurrent animations max(beginTime, initTime) we can remove if-statement
…removed unused code, renamed animation public methods)
…DO, fix slow animations in simulator, apply correction only if SpringAnimation indeed present
…ation because it introduces one frame lag again)
…c interval anymore)
344ffb1
to
5eb5765
Compare
This was referenced May 3, 2024
kirillzyusko
added a commit
that referenced
this pull request
May 4, 2024
…n driver (#431) ## 📜 Description Added `KeyboardAnimation` class (as common interface) which drives an animation of the keyboard. ## 💡 Motivation and Context Follow up for #412 Keyboard animation can be driven not only by `CASpringAnimation` class. What we want to achieve is to have a **single interface** that gives us all necessary properties (value for a particular timing, etc.), but will incapsulate calculations of these values internally. So in this PR I added `KeyboardAnimation` abstract class that uses some common properties for all animations (from/to value, speed, init time), has universal method for finding a particular timing for a given value (since we use a binary search) and defines other publicly available functions. And in this PR I also started to use this universal interface (`KeyboardAnimation`) in `KeyboardMovementObserver` (`SpringAnimation` inherits from `KeyboardAnimation`). ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### iOS - added `KeyboardAnimation` class and `KeyboardAnimationProtocol`; - moved all shared logic in `KeyboardAnimation` class; - started to use `KeyboardAnimation` as a common interface for `animation`. ## 🤔 How Has This Been Tested? Tested on iPhone 11 (iOS 17.4), real device. ## 📸 Screenshots (if appropriate): No visual changes. ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
kirillzyusko
added a commit
that referenced
this pull request
Jul 22, 2024
## 📜 Description A follow up PR for #412 - follow keyboard frame-in-frame if keyboard animation is **not** spring animation. ## 💡 Motivation and Context On iOS keyboard animation can be not only spring animation. It can be also `CABasicAnimation` (when keyboard goes up after interactive dismissal, for example). Before we were relying on `CADisplayLink` and coordinates pooling, but such approach doesn't give a good precision. Even after implementing custom `SpringAnimation` we still relied on this approach (we used this approach as a fallback if the animation is not `CASpringAnimation`). In this PR I implemented custom `TimingAnimation` class and if the animation is `CABasicAnimation` then we also can assure frame precision. All these math calculations were based on this [Desmos playground](https://www.desmos.com/calculator/eynenh1aga?lang=en) - I used concrete params of a particular animation config and concrete values produced by iOS. And then I wrote a code 🙃 > _As you can see some of points are not perfectly aligned to the curve - I think a similar behavior I also observed with `SpringAnimation` and later on I'll improve that as well_ For finding `t` for particular `x` (or `y`) I used [Newton-Raphson](https://en.wikipedia.org/wiki/Newton%27s_method) method. It has better convergence than a binary search and typically I can find a value with given precision within 6 iterations. Last but not least - the animation (`CABaicAnimation`) is not available straight in `keyboardWillShow` callback (it's `nil` there) and it's available only in `CADisplayLink` callback (so I added an attempt of animation initialization there as well). ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### iOS - added `TimingAnimation` class; - use `TimingAnimation` in `KeyboardMovementObserver`. ## 🤔 How Has This Been Tested? Tested manually on: - iPhone 6s (iOS 15.8) - iPhone 11 (iOS 17.4); - iPhone 14 Pro (iOS 17.4); - iPhone 15 Pro (iOS 17.5, simulator); ## 📸 Screenshots (if appropriate): |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/1c1bba40-c76e-468f-87aa-7e9105715ab7">|<video src="https://github.com/user-attachments/assets/55de0c4d-9d0a-4775-a4dd-4983acc30283">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
documentation
Improvements or additions to documentation
🍎 iOS
iOS specific
🚀 optimization
You optimize something and it becomes working faster
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.
📜 Description
Improved precision of
onMove
handler to drive animation frame-in-frame using this handler.💡 Motivation and Context
This problem was introduced since #87 was merged.
The thing is that we are reading
layer
properties usingCADisplayLink
. The problem is that in this case we read always out-of-date values, becauseCADisplayLink
fires a callback before the next frame - in this case we'll always read a value for a previous frame. Thus we have always one frame delay.The idea of this PR is to have an ability to read future values and use them. For that I had to replicate how
SpringAnimation
calculates its values (i. e. the math behind it, because defaultCASpringAnimation
class doesn't give us such ability).Now we know all variables - current timestamp, beginning of the animation, next scheduled timestamp etc. so we can calculate the next frame.
However it's also tricky, because in my observation:
So the formula like
targetTimestamp - beginTime + duration
may work good if XCode debugger is not attached, but will produce junky animation when debugger is attached.I struggled a lot and found that
CADisplayLink
can fire its updates with monotonic intervals (i. e. 16ms) but the keyboard position can be at this point of time in a different values (it can be 12ms or 20ms or any other value comparing to the previous frame) - see the image below as an example of values captured on simulator:So in the end I decided to stick to slightly different approach - when
CADisplayLink
callback is fired, then we read prev frame keyboard position, and instead of calculationduration
value based on absolute time intervals we are doing reversing of current keyboard frame, i. e. for a give keyboard position value we are calculating the duration (of animation), and only then calculate next frame asdurationFromKeyboardPosition + frameDuration
. Such approach gives a decent result on a real device (debug/release, debugger attached/detached etc.).Last but not least - this PR improves precision for plain keyboard show/hide movements. However on iOS in certain cases animation can be not only
CASpringAniamtion
- right now I handle it in a PR in a fallback way, i. e. if it's not a spring animation we fallback to the mechanism introduced in #87 and simply readlayer
properties.Later on I'll cover this case as well and will synchronize animations for all types of animations.
Warning
I can not test this PR onProMotion
device because I don't have it at the moment. However I'll get it back after 10.05.24, so I'll test it there and will see if any code adjustments are needed.Attached video proof, that on iPhone 14 Pro animations are running frame in frame.
📢 Changelog
Docs
onMove
handler;E2E
KeyboardAnimation
component;JS
gray
circle inKeyboardAnimation
component which is driven byonMove
handler;FlatList
example -> useonMove
handler to handle a case when layout animation is not schedule (when keyboard closed viakeyboardShouldPersistTaps
)iOS
core
folder;SpringAnimation
class;CASpringAnimation
;SpringAnimation
class is available, then calculate keyboard position using this class;onMove
will not be perfectly synchronized in any way (layout animation will override our calculations);🤔 How Has This Been Tested?
Tested manually on:
<- I tested it during development, however I made some changes in final code and can not test it again because this device is not with me at the moment - I'll test later when device comes back to me📸 Screenshots (if appropriate):
iPhone 11, iPhone 14, iPhone 6s
KeyboardAnimation
Screen.Recording.2024-05-01.at.17.23.22.mov
RPReplay_Final1714577657.MP4
RPReplay_Final1714664989.mp4
keyboard-animation.mov
RPReplay_Final1714555108.MP4
RPReplay_Final1714642652.mp4
RPReplay_Final1715175619.MP4
KeyboardAvoidingView
Screen.Recording.2024-05-01.at.17.27.24.mov
RPReplay_Final1714577704.MP4
RPReplay_Final1714665069.mp4
Screen.Recording.2024-05-01.at.14.22.48.mov
iphone11-kav.mp4
RPReplay_Final1714642836.mp4
RPReplay_Final1715175667.MP4
KeyboardAwareScrollView
Screen.Recording.2024-05-01.at.17.25.39.mov
RPReplay_Final1714577676.MP4
RPReplay_Final1714665039.mp4
Screen.Recording.2024-05-01.at.14.24.21.mov
RPReplay_Final1714555178.MP4
RPReplay_Final1714642685.mp4
RPReplay_Final1715175642.MP4
📝 Checklist