microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.13k stars 1.13k forks source link

Discrepancy in native driver animation behavior if AnimatedPlatformConfig::ShouldUseComposition() returns true #13253

Open kz-publisher opened 1 month ago

kz-publisher commented 1 month ago

Problem Description

If you set an initial non-zero Animated.Value() for an Animated component's animation value property, such as { translateY: initialValue }, future Animated.timing calls on that property with toValue: newValue will cause the animation to apply the initialValue translation again, additive on top of the initialValue (causing the animation to improperly jump to a new position, i.e. initialValue * 2). This is better explained through code/screenshots; see repro steps below for more details. The example is for translations but I believe the issue also repro's for opacity and possibly other animatable properties as well.

I'm not sure exactly what version this bug was introduced, but we were on 0.68.9 where it was working fine, and upgraded to 0.72.24 where the discrepancy in behavior was observed, so it was somewhere between these two versions. (The repro steps used the latest version 0.74 but it appears to repro on any version from 0.72 to 0.74 as well.)

This is not a blocker for us since we have a workaround. In AnimatedPlatformConfig.cpp we patched our custom RNW nuget to make it always return false for ShouldUseComposition(). It appears that if this function returns true, the discrepancy in behavior is observed, but if it is false, it works fine like before. Without this patch, we would have had to re-write large amounts of our codebase to circumvent the discrepancy in existing behavior which our app was built around (or change everywhere to use useNativeDriver: false which would make the animations much more choppy and unresponsive.)

Steps To Reproduce

  1. Create a sample rnw app using the steps here (https://microsoft.github.io/react-native-windows/docs/getting-started) and latest version of RNW (I was on 0.74.0-rc.9)
  2. Replace contents of App.tsx with this:
/**
 * Sample React Native App
 * https://github.com/facebook/react-native
 *
 * @format
 */

import React, { useState, useEffect } from 'react';
import type {PropsWithChildren} from 'react';
import {
  Animated,
  SafeAreaView,
  ScrollView,
  StatusBar,
  StyleSheet,
  Text,
  useColorScheme,
  View,
  Easing
} from 'react-native';

import {
  Colors,
  DebugInstructions,
  Header,
  LearnMoreLinks,
  ReloadInstructions,
} from 'react-native/Libraries/NewAppScreen';

type SectionProps = PropsWithChildren<{
  title: string;
}>;

function Section({children, title}: SectionProps): JSX.Element {
  const isDarkMode = useColorScheme() === 'dark';
  const [animated] = useState(new Animated.Value(100));

  const sectionStyle = [styles.sectionContainer, {
    transform: [
      { translateY: animated }
    ]
  }];

  const onPress = () => {
    Animated.timing(animated, {
      toValue: 200,
      easing: Easing.ease,
      duration: 500,
      useNativeDriver: true,
    }).start();
  };

  return (
    <Animated.View style={sectionStyle}>
      <Text
        onPress={onPress}
        style={[
          styles.sectionTitle,
          {
            color: isDarkMode ? Colors.white : Colors.black,
          },
        ]}>
        {title}
      </Text>
      <Text
        style={[
          styles.sectionDescription,
          {
            color: isDarkMode ? Colors.light : Colors.dark,
          },
        ]}>
        {children}
      </Text>
    </Animated.View>
  );
}

function App(): JSX.Element {
  const isDarkMode = useColorScheme() === 'dark';

  const backgroundStyle = {
    backgroundColor: isDarkMode ? Colors.darker : Colors.lighter,
  };

  return (
    <SafeAreaView style={backgroundStyle}>
      <StatusBar
        barStyle={isDarkMode ? 'light-content' : 'dark-content'}
        backgroundColor={backgroundStyle.backgroundColor}
      />
      <ScrollView
        contentInsetAdjustmentBehavior="automatic"
        style={backgroundStyle}>
        <Header />
        <View
          style={{
            backgroundColor: isDarkMode ? Colors.black : Colors.white,
          }}>
          <Section title="Step One">
            Edit <Text style={styles.highlight}>App.tsx</Text> to change this
            screen and then come back to see your edits.
          </Section>
          <Section title="See Your Changes">
            <ReloadInstructions />
          </Section>
          <Section title="Debug">
            <DebugInstructions />
          </Section>
          <Section title="Learn More">
            Read the docs to discover what to do next:
          </Section>
          <LearnMoreLinks />
        </View>
      </ScrollView>
    </SafeAreaView>
  );
}

const styles = StyleSheet.create({
  sectionContainer: {
    marginTop: 32,
    paddingHorizontal: 24,
  },
  sectionTitle: {
    fontSize: 24,
    fontWeight: '600',
  },
  sectionDescription: {
    marginTop: 8,
    fontSize: 18,
    fontWeight: '400',
  },
  highlight: {
    fontWeight: '700',
  },
});

export default App;
  1. Open the sample app with the change above. Try clicking "Step One" header element which has a click handler + animation trigger attached to it
  2. Observe the incorrect behavior, as demonstrated in this video:

https://github.com/microsoft/react-native-windows/assets/170368444/12f544e9-da24-4be4-86bd-c2f3bdba01ea

Expected Results

If you were to change line 49 to set useNativeDriver to false, you would observe the behavior in video below without native driver which is the expected, pre-existing behavior:

https://github.com/microsoft/react-native-windows/assets/170368444/45b387ac-cf8b-4918-9a1b-a50061738860

CLI version

13.6.4

Environment

info Fetching system and libraries information...
System:
  OS: Windows 10 10.0.19045
  CPU: (12) x64 Intel(R) Xeon(R) CPU E5-1650 v4 @ 3.60GHz
  Memory: 14.16 GB / 31.92 GB
Binaries:
  Node:
    version: 18.15.0
    path: C:\Program Files\nodejs\node.EXE
  Yarn:
    version: 1.22.22
    path: ~\AppData\Roaming\npm\yarn.CMD
  npm:
    version: 9.5.0
    path: C:\Program Files\nodejs\npm.CMD
  Watchman: Not Found
SDKs:
  Android SDK: Not Found
  Windows SDK:
    AllowDevelopmentWithoutDevLicense: Enabled
    AllowAllTrustedApps: Enabled
    Versions:
      - 10.0.18362.0
      - 10.0.19041.0
      - 10.0.22000.0
      - 10.0.22621.0
IDEs:
  Android Studio: Not Found
  Visual Studio:
    - 17.9.34902.65 (Visual Studio Enterprise 2022)
Languages:
  Java: Not Found
  Ruby: Not Found
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.0-rc.9
    wanted: 0.74.0-rc.9
  react-native-windows:
    installed: 0.74.0-preview.4
    wanted: 0.74.0-preview.4
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: false
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

info React Native v0.74.1 is now available (your project is running on v0.74.0-rc.9).
info Changelog: https://github.com/facebook/react-native/releases/tag/v0.74.1
info Diff: https://react-native-community.github.io/upgrade-helper/?from=0.74.0-rc.9
info For more info, check out "https://reactnative.dev/docs/upgrading?os=windows".

Target Platform Version

10.0.19041

Target Device(s)

Desktop, Xbox

Visual Studio Version

Visual Studio 2022

Build Configuration

Debug

Snack, code example, screenshot, or link to a repository

No response

chrisglein commented 1 month ago

Thanks for the detailed bug report. There are definitely some unresolved issues with the native animation diver. We're in the middle of the lift from the Paper renderer to the Fabric renderer, but as that Fabric is based on composition a lot of this carries over intact (including its bugs). So we'll need to do a proper review of the native animation driving.

Glad you have a workaround for this. We'll look into this as we review animations all up.