[Native Animated] Support for animated tracking in native driver

[kmagiera]

This PR adds support for Animated tracking to Animated Native Driver implementation on Android and iOS.

Motivation

Animated tracking allows for animation to be started with a "dynamic" end value. Instead of passing a fixed number as end value we can pass a reference to another Animated.Value. Then when that value changes, the animation will be reconfigured to drive the animation to the new destination point. What is important is that animation will keep its state in the process of updating "toValue". That is if it is a spring animation and the end value changes while the previous animation still hasn't settled the new animation will start from the current position and will inherit current velocity. This makes end value transitions very smooth.

Animated tracking is available in JS implementation of Animated library but not in the native implementation. Therefore until now, it wasn't possible to utilize native driver when using animated tracking. Offloading animation from JS thread turns out to be crucial for gesture driven animations. This PR is a step forward towards feature parity between JS and native implementations of Animated.

Here is a link to example video that shows how tracking can be used to implement chat heads effect: https://twitter.com/kzzzf/status/958362032650244101

In addition this PR fixes an issue with frames animation driver on Android that because of rounding issues was taking one extra frame to start. Because of that change I had to update a number of Android unit tests that were relying on that behavior and running that one additional animation step prior to performing checks.

Test Plan

As a part of this PR I'm adding three unit tests for each of the platforms that verifies most important aspects of this implementation. Please refer to the code and look at the test cases top level comments to learn what they do.

I'm also adding a section to "Native Animated Example" screen in RNTester app that provides a test case for tracking. In the example we have blue square that fallows the red line drawn on screen. Line uses Animated.Value for it's position while square is connected via tracking spring animation to that value. So it is ought to follow the line. When user taps in the area surrounding the button new position for the red line is selected at random and the value updates. Then we can watch blue screen animate to that position.

You can also refer to this video that I use to demonstrate how tracking can be linked with native gesture events using react-native-gesture-handler lib: https://twitter.com/kzzzf/status/958362032650244101

Release Notes

[GENERAL][FEATURE][Native Animated] - Added support for animated tracking to native driver. Now you can use useNativeDriver flag with animations that track other Animated.Values

[pull-bot]

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

@facebook-github-bot label Core Team

@facebook-github-bot large-pr

Attention: @janicduplessis

Generated by 🚫 dangerJS

[janicduplessis]

Awesome work, mostly just some small nits.

[janicduplessis]

Should we call this updateAnimationConfig instead since it doesn't "reset" the animation completely but only updates certain parameters.

[janicduplessis]

Nit: dictionary notation (a[@"b"]) for this and next line.

[kmagiera]

parentNodes is of type NSMapTable so that won't work. I can use it for the next line tho

[janicduplessis]

Maybe call this _toValueNodeTag, kind of confusing which is what because of _valueNodeTag.

[janicduplessis]

If we decide to update the name in the iOS impl update this too.

[janicduplessis]

Why was this change needed? Does it match the iOS / JS implementation better like this?

[kmagiera]

Tried to explain that in my PR description, you might have missed it as it is at the very end:

In addition this PR fixes an issue with frames animation driver on Android that because of rounding issues was taking one extra frame to start. Because of that change I had to update a number of Android unit tests that were relying on that behavior and running that one additional animation step prior to performing checks.

So basically because of unfortunate rounding here frames driver on android would always "play" first frame twice. This could also be seen in all the unit tests where we run update loop twice at the beginning for all the framedriver tests. I am also updating these in the PR.

[axemclion]

Looks like this line is starting to cause IllegalStateException in apps-

Caused by: java.lang.IllegalStateException: Calculated frame index should never be lower than 0
	at com.facebook.react.animated.FrameBasedAnimationDriver.runAnimationStep(FrameBasedAnimationDriver.java:60)
	at com.facebook.react.animated.NativeAnimatedNodesManager.runUpdates(NativeAnimatedNodesManager.java:444)
	at com.facebook.react.animated.NativeAnimatedModule$1.doFrameGuarded(NativeAnimatedModule.java:100)
	at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)

Is there a reason why timeFromStartMills will be negative ? The exception seems to have started with this PR merged in

[janicduplessis]

Let's name these ivars the same as the iOS implementation if possible.

[janicduplessis]

Were these broken before?

[kmagiera]

So there was a small inconsistency with the way getDouble and getInt work with JavaOnlyArray or Map vs NativeArray or Map. The problem was that in JS we only have "numbers" so both int and double map to the same type. Therefore NativeArray or Map is also casting the value to an appropriate type at the moment when getDouble or getInt is called. So even if we send a floating point from the JS, java code can call getInt and it will return a rounded integer. Same if we send an integer from JS.

This has became a problem since in animated driver tracking node I need to clone NativeArray/Map to JavaOnlyArray/Map, and when there are fields of type "Number" I can't tell if these are floating point numbers or not. Because of that I always read and write them as double. Then when I pass that JavaOnlyMap/Array down to the driver it knows about semantics of the fields and thus it may call getInt on some fields. In that case it was failing because the number has been set as a double.

[janicduplessis]

@facebook-github-bot shipit

[facebook-github-bot]

Something went wrong executing that command, @hramos could you take a look?

[facebook-github-bot]

@janicduplessis is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[hramos]

I looked into the failure and it looks like this is not passing some of our internal tests:

js/react-native-github/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m:987:18: error: unused variable 'frame' [-Werror,-Wunused-variable]
  for (NSNumber *frame in frames) {
                 ^
js/react-native-github/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m:1010:18: error: unused variable 'frame' [-Werror,-Wunused-variable]
  for (NSNumber *frame in frames) {
                 ^
stderr: js/react-native-github/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m:987:18: error: unused variable 'frame' [-Werror,-Wunused-variable]
  for (NSNumber *frame in frames) {
                 ^
js/react-native-github/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m:1010:18: error: unused variable 'frame' [-Werror,-Wunused-variable]
  for (NSNumber *frame in frames) {
                 ^

If you're surprised by this (Circle shows this PR as passing test_objc_ios), it's likely because I just found out yesterday that we haven't been running iOS tests at all since we moved them from Travis to Circle in #16354. On top of that, these tests were not running internally since December for an unrelated reason, but we fixed that internally last week.

The failure is not necessarily coming from this PR -- I haven't digged into that just yet -- but just wanted to let you know what's currently blocking it. I'll report back if it turns out this PR needs to be updated, but until then, assume that it's unrelated and that we need to fix our own internal breakage first.

[janicduplessis]

@hramos Sometimes this happens on iOS because OSS and FB use different clang warning flags when building (I think because fb build uses buck internally to build iOS which is not open source).

[janicduplessis]

@facebook-github-bot shipit

[facebook-github-bot]

Something went wrong executing that command, @hramos could you take a look?

[facebook-github-bot]

@janicduplessis is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[janicduplessis]

@axemclion @kmagiera I think this line could be the culprit. I guess it could be possible that mStartFrameTimeNanos is bigger than the curent clock time.

[sophiebits]

Maybe this would be better as mStartFrameTimeNanos = -1;.

[kmagiera]

I remember there was some other problem when I was setting it to -1 but can't remember what exactly. Will try that and send a new PR

[kmagiera]

Ah, ok I remember now, I didn't want mFromValue to be updated with the next loop and that happens when mStartFrameTimeNanos is -1. Will add another check for this case and send a PR

[kmagiera]

Submitted it here: #18061

[syaau]

@kmagiera Did you know that the native tracking doesn't work as it should when the tracked value is animated (rather than being set directly). The tracking starts only after the initiating animation is complete. Is this some issue with how the native animations pre-calculate and serialize the values when the animation is started? Here are the screen shots for animations without and with native driver -

Without useNativeDriver

With useNativeDriver

I have animated the red circle to move with timing of duration 500, the green circle tracks the red with spring animation and the blue circle tracks the green one.

Here's a code for your reference:

<View style={{ flex: 1 }}>
    <View style={{ flex: 1 }}>
      <Circle color="red" position={first} />
      <Circle color="green" position={second} />
      <Circle color="blue" position={third} />
    </View>
    <Button
      title="Start"
      onPress={() => {
        const useNativeDriver = true;
        Animated.spring(second, { toValue: first, duration: 0, friction: 3, useNativeDriver }).start();
        Animated.spring(third, { toValue: second, duration: 0, friction: 3, useNativeDriver }).start();


        Animated.timing(first, {
          toValue: { x: 300, y: 300 },
          duration: 500,
          useNativeDriver,
        }).start();
      }}
    />
</View>