Add closed-form damped harmonic oscillator algorithm to Animated.spring by skevy · Pull Request #15322 · react/react-native

skevy · 2017-08-02T00:41:30Z

Motivation

As I was working on mimicking iOS animations for my ongoing work with react-navigation, one task I had was to match the "push from right" animation that is common in UINavigationController.

I was able to grab the exact animation values for this animation with some LLDB magic, and found that the screen is animated using a CASpringAnimation with the parameters:

stiffness: 1000
damping: 500
mass: 3

After spending a considerable amount of time attempting to replicate the spring created with these values by CASpringAnimation by specifying values for tension and friction in the current Animated.spring implementation, I was unable to come up with mathematically equivalent values that could replicate the spring exactly.

After doing some research, I ended up disassembling the QuartzCore framework, reading the assembly, and determined that Apple's implementation of CASpringAnimation does not use an integrated, numerical animation model as we do in Animated.spring, but instead solved for the closed form of the equations that govern damped harmonic oscillation (the differential equations themselves are here, and a paper describing the math to arrive at the closed-form solution to the second-order ODE that describes the DHO is here).

Though we can get the currently implemented RK4 integration close by tweaking some values, it is, the current model is at it's core, an approximation. It seemed that if I wanted to implement the CASpringAnimation behavior exactly, I needed to implement the analytical model (as is implemented in CASpringAnimation) in Animated.

Implementation

We add three new optional parameters to Animated.spring (to both the JS and native implementations):

stiffness, a value describing the spring's stiffness coefficient
damping, a value defining how the spring's motion should be damped due to the forces of friction (technically called the viscous damping coefficient).
mass, a value describing the mass of the object attached to the end of the simulated spring

Just like if a developer were to specify bounciness/speed and tension/friction in the same config, specifying any of these new parameters while also specifying the aforementioned config values will cause an error to be thrown.

~~Defaults for Animated.spring across all three implementations (JS/iOS/Android) stay the same, so this is intended to be a non-breaking change.~~

~~If stiffness, damping, or mass are provided in the config, we switch to animating the spring with the new damped harmonic oscillator model (DHO as described in the code).~~

We replace the old RK4 integration implementation with our new analytic implementation. Tension/friction nicely correspond directly to stiffness/damping with the mass of the spring locked at 1. This is intended to be a non-breaking change, but there may be very slight differences in people's springs (maybe not even noticeable to the naked eye), given the fact that this implementation is more accurate.

The DHO animation algorithm will calculate the position of the spring at time t explicitly and in an analytical fashion, and use this calculation to update the animation's value. It will also analytically calculate the velocity at time t, so as to allow animated value tracking to continue to work as expected.

Also, docs have been updated to cover the new configuration options (and also I added docs for Animated configuration options that were missing, such as restDisplacementThreshold, etc).

Test Plan

Run tests. Run "Animated Gratuitous App" and "NativeAnimation" example in RNTester.

pull-bot · 2017-08-02T00:49:31Z

	Warnings
⚠️	❗ Big PR - This PR is extremely unlikely to get reviewed because it touches 804 lines.

@facebook-github-bot label Core Team

@facebook-github-bot large-pr

Attention: @janicduplessis

Generated by 🚫 dangerJS

janicduplessis · 2017-08-02T02:44:38Z

Nice work! I'll have a closer look tomorrow, could you also add a test for the native implementations? Here are the tests for the other spring implementation: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java#L265 and https://github.com/facebook/react-native/blob/master/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m#L269.

skevy · 2017-08-02T02:56:25Z

@janicduplessis yah for sure. Sorry I missed them!

kmagiera · 2017-08-02T09:56:11Z

Awesome work! Just wanted to discuss a few things loosely related to the code:

As you have an analytic formula for x(t) maybe it would be better to also calculate velocity analytically instead of using an approximation here:

var velocity = deltaTime > 0 ? (position - this._lastPosition) / deltaTime : 0;

Calculating it analytically will yield more precise results and I wonder if they could be noticeable. One example where this can be noted is when you use animated tracking with spring (like in "chatheads"), in which case parameters of spring changes with the motion of the finger and the velocity from the previous spring is passed to the next spring.
Calculating velocity analytically is just a matter of calculating time derivative for the equation you have for x(t).

I understand your concern about not making this a breaking change but I wonder what would it take to make this new implementation compatible with the old one so that we can replace RK4 completely. I think this would help in maintaining this quite complicated codebase as we already have spring implementation in three places (js, android, iOS) and now we multiply it by two.
From my understanding the difficulty here is to translate properties of RK4 impl spring (tension and friction) into DHO properties (stiffness, damping and mass). I understand that even if we find a proper mapping the two implementation would yield slightly different results.

From the RK4 implementation it appears like the tension parameter is actually used as a "stiffness" param and friction seems to play the same role as "damping constant":

var aAcceleration = this._tension *
        (this._toValue - tempPosition) - this._friction * tempVelocity;

And this is from the paper you linked:

Assuming mass = 1, this._tensions plays the same role as k (stiffness) and this._friction corresponds to c (damping constant)

I was wandering how CASpringAnimation handles frame drops. The RK4 implementation has a limit on time delta that can be animated in a single animation loop. So for example when loop is stuck for like 10 frames then we advance only by four frames. This should supposedly provide a better experience as the distance between the frames where we update position is going to be shorter and therefore harder to notice but not sure how this works in practice. I can imagine that adding this to DHO impl shouldn't be very difficult (e.g we add another variable that keeps track of the time offset and as long as the time delta between loop calls is small enough we just add that to the time offset and if it's longer than we add some constant number of milliseconds). Even though that would not be very difficult to add I'm not sure if it's worth it, so would like to hear your thoughts.

skevy · 2017-08-02T19:24:24Z

Thanks @kmagiera for the thoughts.

On analytic solution for velocity:

I'll take a look at this further today. Just have to do the math. I think it's a reasonable ask.

On replacing the current RK4 solution with DHO:

I can play around with this. You're right that it theoretically should work, although, I also thought that I would be able to get the RK4 method to take mass into account by doing as you say -- treating tension as k, friction as c, and dividing k and c by m. But that didn't work. I'll investigate this further.

On frame drops:

I'm not sure how CASpringAnimation handles frame drops (probably not very common for it to ever encounter any, given that core animation runs out of process in a higher priority thread than the app). Re this particular implementation however, I've already added it to the JS and Android implementations: https://github.com/facebook/react-native/pull/15322/files#diff-f048d92ca0be679bc38d38147b311100R682 (JS) and https://github.com/facebook/react-native/pull/15322/files#diff-3205235300ac0a6c2dbcf7d2a7264117R161 (Android). This matches the behavior in the RK4 implementation. I didn't add it the iOS implementation, as I didn't see that we were accounting for frame drops anywhere else in the native Animated implementations, but maybe I should add the same logic here.

janicduplessis · 2017-08-03T04:00:44Z

I'll let you figure out the maths :P
I think a small change in the spring curve should not be noticeable so if we are able to convert the RK4 parameters to something similar with DHO we should get rid of the RK4 implementation. Might as well deprecate some parameters to keep the config simpler since there will now be 3 different ways to pass the spring config.
Unless we can figure out how CASpringAnimation handles frame drops I think we should add the same logic on iOS for both DHO and RK4 (if we decide to keep that implementation). It was probably forgotten when it was implemented on iOS, I'd like to keep the 3 implementations as close as possible.

skevy · 2017-08-03T04:03:10Z

@janicduplessis all sounds good. Worked on this a bit today. Hope to have a final version tomorrow

hramos · 2017-08-03T14:53:49Z

This looks great. Quick note, I removed the "Core Team" label as we're currently using this label to track PRs that have already been assigned internally to a Facebook employee for further review. I wouldn't want to set the expectation that adding the label will automatically flag it for internal review - that process is still done manually. That said, if you want to get this reviewed by a FB employee, let me know!

skevy · 2017-08-03T19:03:40Z

@janicduplessis @kmagiera this is ready for re-review.

I've taken all of @kmagiera's comments and implemented them.

Velocity is now calculated analytically (that derivative for the underdamped spring is NARLY).
The analytic DHO model is now the only model across the three platforms. Tension/friction/bounciness/speed can still be specified as before. Tension/friction does indeed correspond directly to stiffness/damping with a mass of 1, as @kmagiera posited.
I added the "max delta time" functionality that existed in Android/JS to the iOS implementation

skevy · 2017-08-03T19:04:29Z

@hramos sorry for adding the label! Originally I was looking for internal review, but generally, that's probably not necessary

...unless @vjeux wants to comment since he wrote a bunch of this initially. Hi @vjeux!

vjeux · 2017-08-04T16:05:25Z

Awesome detective work :) @willbailey will likely be interested as he implemented rebound and the RK4 algorithm, I just copy pasted his implementation in RN

appsforartists · 2017-08-04T16:54:54Z

As the others have said, fantastic work and sleuthing, @skevy!

Would you be interested in breaking this out into its own library? Rebound is great, but it's encumbered by the PATENTS clause (for better or worse, many companies refuse to use any dependencies with a PATENTS clause). It's also 12K minified.

If there was an accurate and unencumbered springs microlibrary, the JavaScript ecosystem would be better for it. Animated could wrap it for React Native, and we'd wrap it for material-motion.

Please consider it. 😃

ericvicenti · 2017-08-04T19:01:15Z

Nice work! (such math!)

Out of curiosity, what is the use case for isInteraction?

janicduplessis · 2017-08-04T19:04:11Z

I think it is if you want your animation to not block InteractionManager.runAfterInteractions, could be useful for a looping animation.

skevy · 2017-08-04T21:50:27Z

@appsforartists yah, I could take a crack at this. I'll DM you on twitter and we can chat about it.

janicduplessis · 2017-08-23T02:22:12Z

@skevy Sorry 5590b1b landed so you'll need to rebase :)

The code is now in animations/SpringAnimation.js. Could you also just let/const + prettier since I did it for most files in Animated now.

appsforartists · 2017-08-23T02:46:46Z

Not sure what your policy is on introducing external dependencies, but this algorithm now exists in its own repo: https://github.com/skevy/wobble/ Would be nice to have it in one canonical place, rather than two.

skevy · 2017-09-03T23:36:16Z

@janicduplessis this is rebased now and ready for final review.

Thanks for your patience!

cc @kmagiera

skevy · 2017-09-03T23:50:56Z

@appsforartists fwiw I decided to keep the implementation implemented separately here for now, given the fact that RN also contains Obj-C/Java implementations of the same algorithm, and I think it's reasonable to keep them colocated. Maybe we can change this in the future, but I think it's easier for now.

janicduplessis · 2017-09-04T00:21:01Z


-  @Test
-  public void testSpringAnimation() {
+  public void performSpringAnimationTestWithConfig(JavaOnlyMap config, Boolean testForCriticallyDamped) {


Nit: boolean instead of Boolean

janicduplessis

Awesome work, just some small implementation details comments.

janicduplessis · 2017-09-04T00:31:03Z

-      position += dxdt * step;
-      velocity += dvdt * step;
+    let deltaTime = 0.0;
+    if (now > this._lastTime) {


Is this necesary? I don't see any case where now could be smaller than _lastTime.

janicduplessis · 2017-09-04T00:34:09Z

  _toValue: any;
-  _tension: number;
-  _friction: number;
+  _stiffness: ?number;


Can we make _stiffness, _damping, _mass and _initialVelocity non-nullable now? Looks like they are initialized in every case.

janicduplessis · 2017-09-04T00:36:28Z

+    }
+    this._frameTime += deltaTime;
+
+    const c: number = this._damping || 0;


If we make those non-nullable we can remove the ||

janicduplessis · 2017-09-04T00:41:33Z


 @end

+const CGFloat MAX_DELTA_TIME = 0.064;


Let's use NSTimeInterval for time values

janicduplessis · 2017-09-04T00:41:43Z

  NSInteger _iterations;
  NSInteger _currentLoop;
+
+  CGFloat _t; // Current time (startTime + dt)


NSTimeInterval

janicduplessis · 2017-09-04T00:42:27Z

-    _animationStartTime = _animationCurrentTime = currentTime;
+
+  // calculate delta time
+  CFTimeInterval deltaTime;


NSTimeInterval

janicduplessis · 2017-09-04T00:45:09Z

+    const k: number = this._stiffness || 0;
+    const v0: number = -(this._initialVelocity || 0);
+
+    invariant(m > 0, 'Mass value must be greater than 0');


Can we assert this in the ctor instead? If we do that we could also remove those asserts from the native implementations.

janicduplessis · 2017-09-04T00:47:03Z

            <Animated.Image
              {...handlers}
              key={i}
-              source={{uri: CHAIN_IMGS[j]}}


Are these changes on purpose?

Yah, but I can remove them if you want. It just puts some images in there so you can see the effect. The images that are currently in this list are broken links.

Sure we can keep it, I assume the images were already there?

Yes they were.

hramos · 2017-09-08T17:00:45Z

Thanks for working on this. I'll import it and let some internal tests run, and if all goes well, I'll land it on Monday.

facebook-github-bot · 2017-09-08T17:01:00Z

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

hramos · 2017-09-11T14:50:46Z

This is almost ready to land. I need to perform some manual steps before doing so. Stay tuned.

hramos · 2017-09-12T01:01:51Z

+   *
+   * We use the closed form of the second order differential equation:
+   *
+   * x'' + (2ζ⍵_0)x' + ⍵^2x = 0


Our linter is complaining about these unicode characters (⍵, ζ, √). It may not be a big deal to import, as this is a comment and not executable code, but the check is probably there for a reason and may prevent issues with our current tooling.

Do you have any preference on how to format this instead? I could replace these with html entities, but that affects readability when looking at the code.

No need to edit your PR, I already have a copy of this PR checked out internally and I can apply any patch prior to landing.

Hmm.

x'' + (2[zeta][omega]_0)x' + [omega]^2x = 0?

facebook-github-bot · 2017-09-20T22:10:26Z

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

skevy requested a review from janicduplessis as a code owner August 2, 2017 00:41

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Aug 2, 2017

skevy added the Core Team label Aug 2, 2017

skevy force-pushed the @skevy/new-spring-algo-upstream branch from 3a852a1 to 0573ae8 Compare August 2, 2017 03:31

hramos removed the Core Team label Aug 3, 2017

mattgperry mentioned this pull request Aug 5, 2017

Adding dedicated spring action Popmotion/popmotion#189

Merged

hramos added the Core Team label Aug 24, 2017

matthamil mentioned this pull request Aug 28, 2017

Is this project still being maintained? react-navigation/react-navigation#2476

Closed

skevy force-pushed the @skevy/new-spring-algo-upstream branch 2 times, most recently from 957c77d to 67d47ed Compare September 3, 2017 23:34

janicduplessis reviewed Sep 4, 2017

View reviewed changes

janicduplessis suggested changes Sep 4, 2017

View reviewed changes

Adam Miskiewicz added 3 commits September 7, 2017 15:15

Add closed-form damped harmonic oscillator algorithm to Animated.spring

5411bb6

Fix native nits

277a243

Fix JS nits

02e6356

skevy force-pushed the @skevy/new-spring-algo-upstream branch from 67d47ed to 02e6356 Compare September 7, 2017 22:33

janicduplessis approved these changes Sep 8, 2017

View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Sep 8, 2017

hramos reviewed Sep 12, 2017

View reviewed changes

hramos added 3 commits September 20, 2017 14:47

Merge branch 'master' into @skevy/new-spring-algo-upstream

77ba309

Fix line too long lint warnings

81b12bc

Fix line too long lint warnings

56cf14e

facebook-github-bot added GH Review: accepted and removed GH Review: review-needed labels Sep 20, 2017

facebook-github-bot closed this in 26133be Sep 21, 2017

kelset mentioned this pull request Nov 2, 2017

StackNavigator renders nothing on first time in Android react-navigation/react-navigation#2887

Closed

ide deleted the @skevy/new-spring-algo-upstream branch April 25, 2020 08:34

Conversation

skevy commented Aug 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Implementation

Test Plan

Uh oh!

pull-bot commented Aug 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janicduplessis commented Aug 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skevy commented Aug 2, 2017

Uh oh!

kmagiera commented Aug 2, 2017

Uh oh!

skevy commented Aug 2, 2017

Uh oh!

janicduplessis commented Aug 3, 2017

Uh oh!

skevy commented Aug 3, 2017

Uh oh!

hramos commented Aug 3, 2017

Uh oh!

skevy commented Aug 3, 2017

Uh oh!

skevy commented Aug 3, 2017

Uh oh!

vjeux commented Aug 4, 2017

Uh oh!

appsforartists commented Aug 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericvicenti commented Aug 4, 2017

Uh oh!

janicduplessis commented Aug 4, 2017

Uh oh!

skevy commented Aug 4, 2017

Uh oh!

janicduplessis commented Aug 23, 2017

Uh oh!

appsforartists commented Aug 23, 2017

Uh oh!

skevy commented Sep 3, 2017

Uh oh!

skevy commented Sep 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janicduplessis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hramos commented Sep 8, 2017

Uh oh!

facebook-github-bot commented Sep 8, 2017

Uh oh!

hramos commented Sep 11, 2017

skevy commented Aug 2, 2017 •

edited

Loading

pull-bot commented Aug 2, 2017 •

edited

Loading

janicduplessis commented Aug 2, 2017 •

edited

Loading

appsforartists commented Aug 4, 2017 •

edited

Loading