Offload NavigatorExperimental transition animations

[nickhudkins]

Motivation

Performing synchronous operations while a navigation transition are in progress, currently results in janky (http://n.hudk.in/hhwW) animations as the transitions are computed on the JS thread. Thanks to @janicduplessis work on NativeAnimated support, we can get silky smooth (http://n.hudk.in/hh66) transitions with limited effort!

Test plan

My current test plan has been minimal. I would love for some assistance here. However, what I have done is run a small application that performs the following operation immediately after a push transition begins:

      fetch('https://jsonplaceholder.typicode.com/photos').then(async (resp) => {
        const json = await resp.json();
        for (var i=0; i < 500; i++) {
          // Do something absurdly blocking.
          JSON.stringify(json);
        }
        console.log('Finished!');
      });

prior to the changes made in this PR the result looked like: http://n.hudk.in/hhwW
and after: http://n.hudk.in/hh66

This PR includes #9253 and #9598

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @buba447 and @janicduplessis to be potential reviewers.

[facebook-github-bot]

@nickhudkins updated the pull request - view changes

[nickhudkins]

The travis failure looks like it may be a timeout or something? I ran tests locally after 5b1711e and tests passed. Am I missing something?

[facebook-github-bot]

@nickhudkins updated the pull request - view changes

[dannycochran]

@nickhudkins any idea when this might make its way to a release?

@janicduplessis polite bump. pretty much all users of react-native-router-flux with an Android implementation are adversely affected by this.

[dannycochran]

@janicduplessis @nickhudkins @buba447

sorry to bug -- is there anyway I can escalate this? this will help to solve some of the most discussed performance issues in the RN docs (https://facebook.github.io/react-native/docs/performance.html) related to navigator transitions.

[janicduplessis]

@dannycochran AFAIK there is nothing blocking this, all important native animations PRs have been merged and I've been using it in an app for some time already and it works really well.

I think we just have to update the default transition config to use the native driver here https://github.com/facebook/react-native/blob/master/Libraries/NavigationExperimental/NavigationTransitioner.js#L52.

@ericvicenti Is that something we'd want to do soon?

[dannycochran]

@janicduplessis hmm maybe I don't understand, looking at RN Master none of updates from these files are present. how are you already using this functionality?

[janicduplessis]

Right now I'm using a fork of ex-navigation which is based off NatigationExperimental and add useNativeDriver to the transition config. It should be possible also to use configureTransition prop of NavigationTransitioner to add the native driver flag.

If you are using NavigationCardStack the simpler way is probably to just vendor that file in and add the flag here https://github.com/facebook/react-native/blob/master/Libraries/CustomComponents/NavigationExperimental/NavigationCardStack.js#L222

Looks like we are already adding it when there are no gestures. We could actually always add it now since I fixes it to work with gestures not so long ago.

[ericvicenti]

I think the reason this is off by default is because the native-driven animations are not compatible with the CardStack gestures. If you disable gestures, the native animations should happen by default.

[janicduplessis]

@ericvicenti It should work now with #10642 and #10643. Been using it with ex-nav + gestures in an app.

[dannycochran]

I'm not sure I follow. How do I enable native driven animations with what's on master?

[janicduplessis]

Right now the easiest way is to turn off gestures with enableGestures={false} on NavigationCardStack. If you want to keep gestures you can try copying https://github.com/facebook/react-native/blob/master/Libraries/CustomComponents/NavigationExperimental/NavigationCardStack.js somewhere in your project and import this instead of the one from react-native. Then you replace the content of this function to always return { useNativeDriver: true }

[dannycochran]

I see. And with the PRs you mentioned above, eventually gestures will be supported without having to locally modify RN. Thank you!

[nihgwu]

@janicduplessis since you have fixed the native animation when using gestures, it would be nice to send a PR to make it be driven by native all the time 😄

[janicduplessis]

There you go #11234

[maraujop]

Could anyone confirm me if useNativeDriver has Android support? I've been looking around in react-native code and I find Libraries/NativeAnimation for iOS implementation, but I haven't found a native driver implementation in java under ReactAndroid/src/main/java/com/facebook/react/.

I've also been looking around in react-native issues only finding iOS instructions and examples, thanks.

[nihgwu]

Support Android Check UIExplorer for instruments On 11 Jan 2017, at 19:07, Miguel Araujo <notifications@github.com<mailto:notifications@github.com>> wrote: Could anyone confirm me if useNativeDriver has Android support? I've been looking around in react-native code and I find Libraries/NativeAnimation for iOS implementation, but I haven't found a native driver implementation in java under ReactAndroid/src/main/java/com/facebook/react/. I've also been looking around in react-native issues only finding iOS instructions and examples, thanks. - You are receiving this because you commented. Reply to this email directly, view it on GitHub<#10289 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACeY8l_NV3z8n9PHCQUfXh8TvRiKUjoLks5rRLfogaJpZM4KRTEI>.

[maraujop]

Thanks @nihgwu you are right, it does have native support for Android using useNativeDriver: true in animation config.

[janicduplessis]

The code for android is here https://github.com/facebook/react-native/tree/master/ReactAndroid/src/main/java/com/facebook/react/animated.

[booboothefool]

Hey guys. I'm running into a lot of performance issues with https://github.com/aksonov/react-native-router-flux, and ended up stumbling upon this thread.

It seems like this might be just what I need, so before I refactor my app can someone confirm that:

1. This will give me better performance than react-native-router-flux, which sometimes gives me a 2-4 second delay before the transition happens even with in production mode on a device, no console.logs, InteractionManager optimizations, etc.
2. ExNavigator > NavigationExperimental > (react-native-router-flux)
3. And although I should use ExNavigator, it currently doesn't have this native driver functionality built-in?

@dannycochran could you chime in considering you first mentioned react-native-router-flux and that's currently what's giving me performance problems? 😝

Thanks. 👍

[janicduplessis]

You should be able to use native animations with react-native-router-flux. It doesn't seems like an option at the moment so you'd have to fork it. Looks like the animation is started here so you could try adding useNativeDriver: true in the config object and at least test if that helps with your perf issues, Also I recommend running RN 0.40+ when using native animations as previous version had quite a few bugs.

[janicduplessis]

You can probably just edit the file directly to test it out, it should be under node_modules/react-native-experimental-navigation

[booboothefool]

@janicduplessis I appreciate the help!

I think I did what you suggested:

function applyDefaultAnimation(
  position: NavigationAnimatedValue,
  navigationState: NavigationParentState,
): void {
  Animated.spring(
    position,
    {
      bounciness: 0,
      toValue: navigationState.index,
      useNativeDriver: true,  // added this
    }
  ).start();
}

Then my navigation started crashing with red screen saying NativeAnimation module not found, so I knew that it did something!!!, and I followed the steps here: #11094 (comment)

Got past the red screen, stuff started working, and noticed some performance improvements! Went from:
press Touchable -> 2-4 seconds later... -> then transitions
-to-
press Touchable -> 1 second later... -> then transitions

So it's not instant and perfect, but it went from unbearably & painfully slow to bearable, haha. This is testing on an iPhone 5 where it's needed the most, so it's already fast on iPhone 7 and whatnot.

However on some of my transitions, I am running into a red screen saying Style property 'left' is not supported by native animated module. I'm not sure what this is referring to considering the routes that mess up are pushed the same way / with the same options as the ones that work just fine. Any ideas what it means?

UPDATE: Nvm I lied. So the scenes that are failing are the ones where the NavBar is rendering e.g.

            <Scene key="filters"
                    component={Filters}
                    hideTabBar
                    hideNavBar={false}  // this will show the navbar
                    title="SEARCH FILTERS"  // this is the navbar's title
                    {...navBarProps}
                    panHandlers={null}
                  />

And it so turns out that parts of NavBar gets animated in from the left... Specifically I think it is referring to this title: https://github.com/aksonov/react-native-router-flux/blob/master/src/NavBar.js#L437
I don't know enough about React Native animations to know what to do next. 😝

UPDATE: I could get rid of the animation...

        <Text
          lineBreakMode="tail"
          numberOfLines={1}
          {...this.props.titleProps}
          style={[
            styles.title,
            this.props.titleStyle,
            this.props.navigationState.titleStyle,
            childState.titleStyle,
          ]}
        >
          {title}
        </Text>

And it works!

[janicduplessis]

@booboothefool Yea, native animated only works for non-layout properties, so things like colors, opacity, and transforms. The animation that uses left could probably be done using a translate transform instead.

[booboothefool]

@janicduplessis Cool! Only thing left is that the the swipe left -> right to go back gesture no longer works. When I try it, I get an error Attempted to use JS driven animation on node previously set with native: true. Is there something that also needs to be changed in: https://github.com/aksonov/react-native-experimental-navigation/blob/master/NavigationCardStackPanResponder.js ?

[janicduplessis]

Looks like it is an older version of navigation experimental. if add the changes from e173f14#diff-4bb680a7c25fe51a91ee47d2e34cb55a and ac19276 it should work with gestures.

[booboothefool]

@janicduplessis Adding those changes actually breaks all the transition animations (the animation goes away and scenes just appear without animation). 😢

https://github.com/aksonov/react-native-experimental-navigation/blob/master/NavigationCardStackStyleInterpolator.js
^ The position.interpolate didn't work with that other file, so am probably missing a lot of changes...

[dmhood]

I'm getting this same "Attempting to run JS driven animation on animated node..." error on Android. Even after adding the above changes to NavigationCardStackStyleInterpolator. Oh and I'm also using react-native-router-flux.

Edit: nevermind fixed it! Had to apply a "native animation" to my tab component that wrapped several scenes, even though it doesn't animate.

[janicduplessis]

Hmm, not sure about why it doesn't work, ideally react-native-router-flux would update to a more recent version of navigation experimental and it would probably fix it. At this point I suggest contacting the authors to see if it would be possible to add native animations support. I'm not using this library so I don't really know the internals but it should be relatively straightforward to get it working.

[booboothefool]

@dmhood Could you let me know exactly what you changed? I'm still unable to get it to work properly.

[dmhood]

@booboothefool I went about it in a bit of a different way. Instead of making the changes above to NavigatorExperimental, I just gave a custom animation and navbar renderer to all of my scenes:

function applyAnimation (pos, navState) {
    Animated.spring(
      pos, {
          bounciness: 0,
          toValue: navState.index,
          useNativeDriver: true  // added this
      }).start();
}

function renderTitle (navProps) {
    let title =  navProps.getTitle ? navProps.getTitle(navProps) : navProps.title;
    if (typeof (title) === 'function') {
        title = title(navProps);
    }
    return (
        <Animated.View
            style={[
                styles.titleWrapper,
                navProps.titleWrapperStyle
            ]}
      >
            <Animated.Text
                lineBreakMode="tail"
                numberOfLines={1}
                {...navProps.titleProps}
                style={[
                    styles.title,
                    navProps.titleStyle,
                    navProps.navigationState.titleStyle
                    // {
                    //     opacity: this.props.position.interpolate({
                    //         inputRange: [index - 1, index, index + 1],
                    //         outputRange: [0, this.props.titleOpacity, 0]
                    //     }),
                    //     left: this.props.position.interpolate({
                    //         inputRange: [index - 1, index + 1],
                    //         outputRange: [200, -200]
                    //     }),
                    //     right: this.props.position.interpolate({
                    //         inputRange: [index - 1, index + 1],
                    //         outputRange: [-200, 200]
                    //     })
                    // }
                ]}
            >
                {title}
            </Animated.Text>
        </Animated.View>
    );
}

// then every scene in my app looks like:
<Scene 
        key="sceneA" 
        ...some scene props
        renderTitle={renderTitle}
        applyAnimation={applyAnimation}
 />

I only tested this out on Android. Hope it helps!

[booboothefool]

Ah thanks, but I got that part working already. I thought you meant you got the swipe gestures working. 😝

Oddly enough, the swipe gestures in Android work fine with no changes other than what I mentioned in: #10289 (comment), but iOS is doing something different.

[booboothefool]

UPDATE: Alright... so after trying every optimization in the book, I decided to give Ex-navigation a whirl.

IT IS BUTTERY SMOOTH. No delay after a push, smooth animations, smooth swipe gestures. Runs beautifully on Android, iPhone 5, etc. It's a UX dream...

[dmhood]

@booboothefool do you mean you dropped react-native-router-flux and just used ex-navigator directly?

[booboothefool]

@dmhood Yeah, I threw in the towel with react-native-router-flux. After trying every optimization I could read about, there were only minor improvements. Made me feel like crap about my app honestly.

Switched to ExNavigation and everything now transitions before I can even blink. It's like Instagram smooth, haha. 2-4 second delay to no delay at all... Swipe gestures and everything too.

Seems to be more of a learning curve. Definitely isn't as easy to use as react-native-router-flux's slick API, but I'm finding it extremely worthwhile. It's amazingly fast.

It's similar to how I feel about LASIK. Wish I'd done it sooner.

UPDATE: While ExNavigation does seem smoother than react-native-router-flux, that wasn't my real performance bottleneck. ExNavigation started off super smooth as I was just refactoring my routes, but when I started to add all the data back, it began to slow down! So my 2-4 second delay before transitions isn't react-native-router-flux, but has something to do with having fetched too much data (no, not during transitions, but in general). My app uses GraphQL, and basically the bigger/more queries I have done, the more the app slows down. No idea why...

[deathemperor]

@booboothefool @dmhood just a dumb question: what are you referring to when you say "ExNavigation"? It's a RN built-in? Is it short of ExperimentalNavigation? I'm too having the same issues with router-flux on Android.