Scrolling a 'moving' FlatList causes it to stutter or block scrolling completely on Android

tjzel commented 1 year ago

Description

When you create a FlatList that moves as it's being scrolled (change in offset for the FlatList is a change in top property of FlatList's style) it stutters heavily. This behavior is much less disruptive when movement is scaled down compared to scrolling and becomes app-breaking when upscaled. See provided videos:

Multiplier: 1.0 mult1.webm
Mutliplier: 2.0 mult2.webm
Multiplier: 0.5 mult0.5.webm

It was originally an issue in react-native-reanimated but after digging into it I narrowed it down to react-native and rewrote the code to not use react-native-reanimated. It seems to had been in the code for a long time, since this posts that dates 3 years back is about the same thing.

I checked it on versions 0.72, 0.71 and 0.70. On both Fabric and Paper and the result is always the same (although it feels a bit like Fabric behaviour is worse).

iOS works properly on each version too.

It seems like there is a problem when calculating layout and touch events - that's what I conducted due to multiplier making it better or worse. After moving a FlatList twice the distance it scrolled and having the component moved React Native seems to 'think' that the finger is now further than it should be and lowers the offset? That's just my guess.

React Native Version

0.72.3

Output of `npx react-native info`

System: OS: macOS 13.4.1 CPU: (10) arm64 Apple M2 Pro Memory: 49.31 MB / 16.00 GB Shell: version: "5.9" path: /bin/zsh Binaries: Node: version: 18.16.0 path: /var/folders/jg/m839qn593nn7w_h3n0r9k25c0000gn/T/yarn--1689593442476-0.9098900178237601/node Yarn: version: 1.22.19 path: /var/folders/jg/m839qn593nn7w_h3n0r9k25c0000gn/T/yarn--1689593442476-0.9098900178237601/yarn npm: version: 9.5.1 path: ~/.nvm/versions/node/v18.16.0/bin/npm Watchman: version: 2023.06.12.00 path: /opt/homebrew/bin/watchman Managers: CocoaPods: version: 1.12.1 path: /Users/user/.rbenv/shims/pod SDKs: iOS SDK: Platforms:

DriverKit 22.4
iOS 16.4
macOS 13.3
tvOS 16.4
watchOS 9.4 Android SDK: Not Found IDEs: Android Studio: 2022.1 AI-221.6008.13.2211.9619390 Xcode: version: 14.3.1/14E300c path: /usr/bin/xcodebuild Languages: Java: version: 11.0.19 path: /usr/bin/javac Ruby: version: 3.2.2 path: /Users/user/.rbenv/shims/ruby npmPackages: "@react-native-community/cli": Not Found react: installed: 18.2.0 wanted: 18.2.0 react-native: installed: 0.72.3 wanted: 0.72.3 react-native-macos: Not Found npmGlobalPackages: "react-native": Not Found Android: hermesEnabled: true newArchEnabled: true iOS: hermesEnabled: true newArchEnabled: false

Steps to reproduce

Just need to run the provided code snippet.

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

https://github.com/tjzel/ReactNativeMovingFlatList

import React from 'react';
import {Dimensions, FlatList, StyleSheet, Text, View} from 'react-native';

const {height: SCREEN_HEIGHT} = Dimensions.get('screen');

const data = Array.from({length: 30}, (_, index) => ({
  id: `${index + 1}`,
  text: `Item ${index + 1}`,
}));

const multiplier = 1.0;

export default function App() {
  const [animatedVerticalScroll, setAnimatedVerticalScroll] = React.useState(0);
  // used to detect stutters
  const previousValue = React.useRef(0);

  const listAnimatedStyle = {
    top:
      animatedVerticalScroll * multiplier > SCREEN_HEIGHT * 0.7
        ? 0
        : SCREEN_HEIGHT * 0.7 - animatedVerticalScroll * multiplier,
  };

  const renderItem = ({item}) => (
    <View style={styles.item}>
      <Text>{item.text}</Text>
    </View>
  );

  const scrollHandler = event => {
    // logger to detect if there was a stutter
    if (previousValue.current > event.nativeEvent.contentOffset.y) {
      console.log(
        `\nScrolling stuttered!\nPrevious offset was ${previousValue.current}\nCurrent offset is ${event.nativeEvent.contentOffset.y}\n`,
      );
    } else {
      console.log(event.nativeEvent.contentOffset.y);
    }
    setAnimatedVerticalScroll(event.nativeEvent.contentOffset.y);
    previousValue.current = event.nativeEvent.contentOffset.y;
  };

  return (
    <View style={styles.container}>
      <View style={styles.containerBehind}>
        <Text>Multiplier: {multiplier}</Text>
      </View>
      <FlatList
        data={data}
        renderItem={renderItem}
        style={[styles.listContainer, listAnimatedStyle]}
        onScroll={scrollHandler}
      />
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
  containerBehind: {
    width: '100%',
    height: '100%',
    flex: 1,
    backgroundColor: 'red',
    alignItems: 'center',
    justifyContent: 'center',
  },
  listContainer: {
    top: 0,
    left: 0,
    position: 'absolute',
    zIndex: 10,
    width: '100%',
    height: '100%',
    backgroundColor: 'green',
  },
  item: {
    height: 50,
  },
});

cortinico commented 1 year ago

Could you put your snippet in this repro template @tjzel and attach it to this issue? https://github.com/react-native-community/reproducer-react-native

tjzel commented 1 year ago

Sure, it's added!

efstathiosntonas commented 1 year ago

Just a sidenote, I’ve seen this happening with FlashList too.

efstathiosntonas commented 1 year ago

Hey folks, is this PR responsible for this?

https://github.com/facebook/react-native/pull/38475

efstathiosntonas commented 1 year ago

After building from source with the above PR included it's even worse @ryancat:

demo video

https://github.com/facebook/react-native/assets/717975/5237186c-4451-4a83-81d8-d69d5a26ffdf

ryancat commented 1 year ago

@efstathiosntonas #38475 is meant to support props that allow ScrollView to get throttled. If not used it should have no effect on ScrollView. I'll take a look to see why it's making any differences here.

efstathiosntonas commented 1 year ago

Thanks for the clarification @ryancat. Are there any updates on this one @cortinico?

cortinico commented 1 year ago

Thanks for the clarification @ryancat. Are there any updates on this one @cortinico?

No updates. We're happy to receive a PR that fixes this (so it's up for grab)

aureosouza commented 1 year ago

This is blocker for us as well. Since iOS works well, it could be connected with the way Android handles fractional pixel values for animation, described here and here.

ryancat commented 1 year ago

@efstathiosntonas #38475 is meant to support props that allow ScrollView to get throttled. If not used it should have no effect on ScrollView. I'll take a look to see why it's making any differences here.

VirtualizedList is defaulting scrollEventThrottle to 50 (See source), which is what FlatList using and got impacted.

I thought about dropping the default for Android, but on iOS we've been having the same default already. It's the right thing to do to align both platforms and to have a default so that we can improve native frame rate. If you feel the throttle is working against your use case, you may set a lower value (any value less than 17 should have no effect on throttling).

NickGerleman commented 1 year ago

I think we should remove this default, even though it was previously set on iOS. It is going to be incorrect for the vast majority of use-cases. Users shouldn't need to know on an RN update that they now need to set an obscure prop to a lower value. Time-based throttling has other issues as well (and VirtualizedList already has a mechanism to avoid starvation).

NickGerleman commented 1 year ago

any value less than 17 should have no effect on throttling

Separate from the throttling, we should not be adding new code to React Native which is hardcoded to assume 60fps. This is not correct on many devices, including even older or lower end Android devices.

NickGerleman commented 1 year ago

Anecdotally, I am aware of some code which implements a similar scenario and it seems to work okay on Android. Their implementation looks like:

Translating via transform, instead of top (top might cause relayout?)
Uses Animated.FlatList and the Animated.event APIs. Can natively accelerate the transforms without hitting JS I think

         onScroll={Animated.event(
            [{nativeEvent: {contentOffset: {x: this._scrollViewPos}}}],
            {useNativeDriver: true},
          )}

Animated.FlatList does override scrollEventThrottle to effectively disable it, as well.

efstathiosntonas commented 1 year ago

@NickGerleman I removed scrollEventThrottle on VirtualizedLists and it throws this:

"@react-native/virtualized-lists": "^0.72.6"

You specified `onScroll` on a <ScrollView> but not `scrollEventThrottle`. You will only receive one event. Using `16` you get all the events but be aware that it may cause frame drops, use a bigger number if you don't need as much precision.

tjzel commented 1 year ago

@NickGerleman Yeah there are several workarounds for this and by no means my code is made to be efficient - while using translate sounds like a smart move I think people actually want to use layout redefining prop in this scenario.

efstathiosntonas commented 1 year ago

translateY can be really tricky though since it “shifts” the entire list up leaving empty space at the bottom

NickGerleman commented 1 year ago

@NickGerleman I removed scrollEventThrottle on VirtualizedLists and it throws this:

"@react-native/virtualized-lists": "^0.72.6"
You specified `onScroll` on a <ScrollView> but not `scrollEventThrottle`. You will only receive one event. Using `16` you get all the events but be aware that it may cause frame drops, use a bigger number if you don't need as much precision.

I ended up running into this with the above PR. Some components like Animated.FlatList currently pass scrollEventThrottle={0.0001} as a value to cause iOS to emit scroll events, while not actually throttle, so I am changing the effective default to that to keep behavior of no throttling on Android, and to change before to not throttle on iOS by default.

This specific behavior and expectation is a bit quirky, and is probably some artifact of RN back in the day being written for iOS before Android, and the two diverging. I am kind of tempted to try to run an experiment to change this behavior in the new architecture (maybe not worth the churn to change behavior in Paper).

aureosouza commented 1 year ago

@NickGerleman thanks for support on this, we've tried using translateY as suggested, but we still see the stuttering in Android, as well we see issues with the footer, as not being able to scroll the whole list. Do you see any workaround for this with current RN?

Using demo, modified to:

import React from 'react';
import {Animated, Dimensions, StyleSheet, Text, View} from 'react-native';

const {height: SCREEN_HEIGHT} = Dimensions.get('screen');

const data = Array.from({length: 60}, (_, index) => ({
  id: `${index + 1}`,
  text: `Item ${index + 1}`,
}));

const App = () => {
  const scrollY = new Animated.Value(0);

  const translateY = scrollY.interpolate({
    inputRange: [0, SCREEN_HEIGHT],
    outputRange: [SCREEN_HEIGHT * 0.7, 0],
    extrapolate: 'extend',
  });

  const renderItem = ({item}) => (
    <View style={styles.item}>
      <Text>{item.text}</Text>
    </View>
  );

  return (
    <View style={styles.container}>
      <View style={styles.containerBehind}>
        <Text>Multiplier: 1.0</Text>
      </View>
      <Animated.FlatList
        data={data}
        renderItem={renderItem}
        style={[
          styles.listContainer,
          {
            transform: [{translateY}],
            paddingTop: scrollY,
          },
        ]}
        onScroll={Animated.event(
          [{nativeEvent: {contentOffset: {y: scrollY}}}],
          {useNativeDriver: false},
        )}
        ListFooterComponent={<Animated.View style={{height: scrollY}} />}
        scrollEventThrottle={16}
      />
    </View>
  );
};

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
  containerBehind: {
    width: '100%',
    height: '100%',
    flex: 1,
    backgroundColor: 'red',
    alignItems: 'center',
    justifyContent: 'center',
  },
  listContainer: {
    left: 0,
    position: 'absolute',
    zIndex: 10,
    width: '100%',
    height: '100%',
    backgroundColor: 'green',
  },
  item: {
    height: 50,
  },
});

export default App;

tjzel commented 10 months ago

Hey @NickGerleman, have you had the time to look into this? I built an app with nightly RN version and it seems the issue is still there.

guvenkaranfil commented 10 months ago

It still persist. Is there any update @NickGerleman

hattat-34 commented 8 months ago

I guess onScroll event has missing layout calculation, the list height is increase if another view height decrease which outside of the list(padding leads bug in this case). IOS side has no recalculation of the list height during scroll so there is no stuttering @tjzel @NickGerleman

facebook / react-native