facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.34k stars 24.23k forks source link

Long text in inverted Flatlist causes huge performance drop on Android #30034

Closed divonelnc closed 4 months ago

divonelnc commented 3 years ago

Description

I recently upgraded to react 0.63.2 thanks to the latest Expo SDK 39. I immediately noticed that my most important FlatLists took a huge performance hit on Android (iOs seem unaffected).

After investigating, I found out that it happens when using inverted on a FlatList that contains items with a lot of text (see snack).

The same Flatlist, with the same data, is perfectly smooth when not inverted.

I have yet to test it in production, as the latest Expo SDK is causing trouble that stops me from building the app.

React Native version:

react-native: https://github.com/expo/react-native/archive/sdk-39.0.0.tar.gz => 0.63.2

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

  1. Create a Flatlist that renders items that contain a lot of text
  2. Set the Flatlist as inverted

Expected Results

The Flatlist should be as smooth when inverted as when normal.

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

https://snack.expo.io/@divone/4c8d68

divonelnc commented 3 years ago

Just checked it in production and the performance hit is still there, making the app feel laggy.

divonelnc commented 3 years ago

This issue appears as soon as you invert a ScrollView actually, as you can see on this snack: https://snack.expo.io/@divone/c0c1f3

Again, only visible on a physical Android device.

divonelnc commented 3 years ago

It seems that the issue only happens on some phones. For me, it happens on a Google Pixel 4 with Android 11.

SuryaL commented 3 years ago

I see this issue on Pixel4 XL running Android 11, which was working fine on React Native 0.61.5 with Android 10. I notice that when the keyboard is open it does not seem to lag.

safaiyeh commented 3 years ago

Hey @divonelnc thanks for the investigation! I noticed the same issue when running your example.

liangjs19 commented 3 years ago

Facing this issue on a Samsung Galaxy Note10 running Android 11 as well with Expo SDK 39. Didn't happen on Android 10.

Discovered that changing the verticallyInverted style prop from transform: [{scaleY: -1}] to scaleY: -1 in VirtualizedList fixes it for Android, however it does not invert on iOS.

chr-sk commented 3 years ago

Experienced this issue as well. I upgraded my Samsung Galaxy S10 from Android 10 to Android 11 and suddenly experienced a noticable lag on the inverted FlatList in my app. @cookiespam 's workaround fixed the problem, however.

TwanLuttik commented 3 years ago

Im experience the same issue on android with a Samsung Galaxy s10 Android 11, and i noticed after updated the same issue. And i even have it if i don't have long text (The text element goes full width with short text)

vinceplusplus commented 3 years ago

@cookiespam's workaround works. for a less intrusive fix, we could "invert" twice ourselves, once in the container and once in every cell

return (
  <FlatList
    // ...
    style={{scaleY: -1}}
    renderItem={({item}) => {
      return (
        <View style={{scaleY: -1}}>
          {/* cell content */}
        </View
      );
    }}
  >
);

for ios, could just use the official inverted prop. however, that scaleY outside of transform seems undocumented. @cookiespam how did you come up with it? great thanks though

liangjs19 commented 3 years ago

@vinceplusplus

I was just trying to invert the FlatList on my own without using the inverted prop and somehow accidentally used scaleY(was googling how to invert something using css) which had been deprecated.

vinceplusplus commented 3 years ago

@cookiespam then that was a good accident ๐Ÿ‘

pqkluan commented 3 years ago

I also encounter this issue with my chat screen. It's really hard to find to root cause.

It's only take 10 items in the list for the performance to be hit really bad.

I'm using @cookiespam bandaid + patch-package to resolve the issue for now now.

Here is the the content of the patch:

react-native+0.63.3.patch

diff --git a/node_modules/react-native/Libraries/Lists/VirtualizedList.js b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
index 9ec105f..a8f1d8c 100644
--- a/node_modules/react-native/Libraries/Lists/VirtualizedList.js
+++ b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
@@ -11,6 +11,7 @@
 'use strict';

 const Batchinator = require('../Interaction/Batchinator');
+const Platform = require('../Utilities/Platform');
 const FillRateHelper = require('./FillRateHelper');
 const PropTypes = require('prop-types');
 const React = require('react');
@@ -2185,9 +2186,14 @@ function describeNestedLists(childList: {
 }

 const styles = StyleSheet.create({
-  verticallyInverted: {
-    transform: [{scaleY: -1}],
-  },
+  verticallyInverted:
+    Platform.OS === 'android'
+      ? {
+          scaleY: -1,
+        }
+      : {
+          transform: [{scaleY: -1}]
+        },
   horizontallyInverted: {
     transform: [{scaleX: -1}],
   },
OlivierFreyssinet-old commented 3 years ago

Wow wow wow thank you guys this fix is a lifesaver !!! ๐Ÿ™

I was actually trying to replace my FlatList by a ScrollView to see where the issue was coming from and noticed then that without inverting the FlatList the performance was fine.

Now I can keep my FlatList inverted, it works exactly same as before without changing any other props and I'm going from horrible 40-50fps (since updating to Android 11) to butter smooth 90fps ๐Ÿ˜

the solution proposed by @vinceplusplus is what I will be using:

return (
  <FlatList
    // ...
    style={{scaleY: -1}}
    renderItem={({item}) => {
      return (
        <View style={{scaleY: -1}}>
          {/* cell content */}
        </View
      );
    }}
  >
);

Is someone considered making a pull request with @cookiespam and @pqkluan's patch? I could make one in a couple of days when I get some free time but I also don't want to steal your moment of glory haha

Jovan1998 commented 3 years ago

Facing this issue on a Samsung Galaxy Note10 running Android 11 as well with Expo SDK 39. Didn't happen on Android 10.

Discovered that changing the verticallyInverted style prop from transform: [{scaleY: -1}] to scaleY: -1 in VirtualizedList fixes it for Android, however it does not invert on iOS.

@cookiespam You are legend, worked for me too.

enahum commented 3 years ago

using the scaleY: -1 instead of transform: [{scaleY: -1}] indeed seems to fix the performance issue, BUT (as usual there is a but) the refresh control becomes an issue.

It renders on the top of the screen as you attempt to scroll down and in order to actually scroll you'll need to scroll up then down to "free" the pan responder somehow.

JamesMahy commented 3 years ago

@vinceplusplus

I was just trying to invert the FlatList on my own without using the inverted prop and somehow accidentally used scaleY(was googling how to invert something using css) which had been deprecated.

I think I love you, thank you!

KochMario commented 3 years ago

I'd just wanted to drop in and say thank you as well ๐Ÿ™๐Ÿผ๐Ÿ’™

I was severely confused when I upgraded to Android 11 (on a Redmi Note 8 Pro) and the performance on the chat screen in my app was suddenly horrible. Scrolling for a couple of seconds resulted in 1000+ dropped frames. It was so laggy and jerky that one got almost dizzy by watching it.

Based on your suggestions I've implemented a simple check (with react-native-device-info) so the fix is only applied for android & version >= 11.

const useAndroidInvertedFix = useMemo(
  () => Platform.OS === 'android' && Number(getSystemVersion()) >= 11,
  [],
);

I'm extremely grateful for this workaround, however the underlying issue should be officially addressed by react-native itself, right? Does anyone know the procedure for doing that or is it enough that this issue gets enough attention? ๐Ÿค”

safaiyeh commented 3 years ago

@KochMario the best way to get this fix in is by submitting a PR. Unfortunately the team canโ€™t address every issue as most are not a priority. But there is a massive effort right now to evaluate PrS submitted by contributors.

nero2009 commented 2 years ago

I have been struggling to understand why my chat was lagging when tested on a Samsung S20 ultra with 12gb of RAM while it works prefectly on my xiaomi 9 with just 3GB of RAM. And the comments in this thread helped me fix the issue. Thanks everyone๐ŸŽ‰

eggybot commented 2 years ago

resolved my issue on react-native-gifted-chat with the solution from @pqkluan

As @nero2009 mention it has issue with Android devices for lag scrolling (not smooth like in iOS). If you are using react-native-gifted-chat and experience this issue with the RN version you have. Apply the solution mention on this thread.

neeraj-nh commented 2 years ago

I am facing the same issue and have applied the solution here : https://github.com/facebook/react-native/issues/30034#issuecomment-780547496 . Hoping it does not break anything else. I'll post here if it does ๐Ÿคž .

sfuqua commented 2 years ago

Encountered this issue in a chat scenario (where inverting the list is desirable) and was pulling my hair out why adding text elements to the list was causing scroll perf to tank.

Can confirm that uninverting the list and applying the scaleY workaround fixes scroll perf, but breaks the pull-to-refresh functionality :( may need to do something custom there.

Does anyone have any idea at an Android level why the perf implodes with the default implementation?

React Native 0.66 on a Pixel 6 / Android 12.

edit: between this issue and https://github.com/facebook/react-native/issues/30373 I feel that inverted FlatLists are just completely unusable on Android out-of-the-box - my team will likely need to invest in a custom list control for our chat canvas.

JamesMahy commented 2 years ago

Encountered this issue in a chat scenario (where inverting the list is desirable) and was pulling my hair out why adding text elements to the list was causing scroll perf to tank.

Can confirm that uninverting the list and applying the scaleY workaround fixes scroll perf, but breaks the pull-to-refresh functionality :( may need to do something custom there.

Does anyone have any idea at an Android level why the perf implodes with the default implementation?

React Native 0.66 on a Pixel 6 / Android 12.

It was only something I saw on phones using One UI 3 so surprised it's now showing up on Android 12 too

enahum commented 2 years ago

Encountered this issue in a chat scenario (where inverting the list is desirable) and was pulling my hair out why adding text elements to the list was causing scroll perf to tank.

Can confirm that uninverting the list and applying the scaleY workaround fixes scroll perf, but breaks the pull-to-refresh functionality :( may need to do something custom there.

Does anyone have any idea at an Android level why the perf implodes with the default implementation?

React Native 0.66 on a Pixel 6 / Android 12.

edit: between this issue and https://github.com/facebook/react-native/issues/30373 I feel that inverted FlatLists are just completely unusable on Android out-of-the-box - my team will likely need to invest in a custom list control for our chat canvas.

@sfuqua you may want to take a peek at https://github.com/mattermost/mattermost-mobile/blob/master/app/components/post_list/post_list.tsx and https://github.com/mattermost/mattermost-mobile/blob/master/app/components/post_list/post_list_refresh_control.tsx for an inverted list with working refresh control using scaleY: -1

sfuqua commented 2 years ago

Encountered this issue in a chat scenario (where inverting the list is desirable) and was pulling my hair out why adding text elements to the list was causing scroll perf to tank. Can confirm that uninverting the list and applying the scaleY workaround fixes scroll perf, but breaks the pull-to-refresh functionality :( may need to do something custom there. Does anyone have any idea at an Android level why the perf implodes with the default implementation? React Native 0.66 on a Pixel 6 / Android 12. edit: between this issue and #30373 I feel that inverted FlatLists are just completely unusable on Android out-of-the-box - my team will likely need to invest in a custom list control for our chat canvas.

@sfuqua you may want to take a peek at https://github.com/mattermost/mattermost-mobile/blob/master/app/components/post_list/post_list.tsx and https://github.com/mattermost/mattermost-mobile/blob/master/app/components/post_list/post_list_refresh_control.tsx for an inverted list with working refresh control using scaleY: -1

Cheers, I'll give that a try - I thought I'd tried that yesterday but may have been applying the wrong style to the RefreshControl.

gorip1 commented 2 years ago

Bottom refreshControl

For those who want to keep the refresh control at the bottom, you can wrap your flatlist in a view with scaleY:-1 instead of putting the style into the flat list.

return (
<View style={{scaleY: -1, flex: 1}}>
  <FlatList
    // ...
    renderItem={({item}) => {
      return (
        <View style={{scaleY: -1}}>
          {/* cell content */}
        </View
      );
    }}
  />
</View>
);
adamgajzlerowicz commented 1 year ago

The scaleY style does not work on react 0.70.0

https://github.com/facebook/react-native/issues/34607

But transform: [{ rotate: '180deg' }], does the trick!

retrixe commented 1 year ago

Worth noting that the rotate transform isn't perfect, since the scroll indicator moves to the other side of the list (from the right to the left).

Currently, I disabled the vertical scroll indicator in my application by passing showsVerticalScrollIndicator={Platform.OS !== 'android'} to FlatList, but this doesn't make for a good user experience.

retrixe commented 1 year ago

For now, I have written a quick and dirty workaround to restore scaleY like so in my application's index.js file:

// Add scaleY back to work around its removal in React Native 0.70.
import ViewReactNativeStyleAttributes from 'react-native/Libraries/Components/View/ReactNativeStyleAttributes'
ViewReactNativeStyleAttributes.scaleY = true

This allows you to continue using scaleY and avoid the aforementioned issue with the scroll bar being moved to the other side of the FlatList when using rotate instead of scaleY. This works fine in React Native 0.70.x, but might break in the future if support for the property is removed from the native Android View code.

hotaryuzaki commented 1 year ago

any solution with RN 0.70?? because scaleY StyleSheet is already remove from the package.

holmesworcester commented 1 year ago

Just noting here that we observed that in Android 13 the impact of this is much, much worse than in previous versions of Android, at least in our case (a long list of messages in a chat interface.)

In Android 11 it was not so noticeable and in Android 13 it made the app completely unusable.

fabOnReact commented 1 year ago

Transform The implementation of transform includes:

The implementation of setScaleX https://github.com/fabriziobertoglio1987/react-native/blob/958ae951497c196fb7cab9b2754f22353555cb07/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java#L404

https://developer.android.com/reference/android/view/View#setScaleX(float)

fabOnReact commented 1 year ago
[
 Sx 0  0  0
 0  Sy 0  0
 0  0  Sz 0
 0  0  0  1
]

A negative value (scaleY: -1) results in a point reflection in that dimension. The value 1 has no effect. In geometry, a point reflection or inversion in a point (or inversion through a point, or central inversion) is a type of isometry of Euclidean space.

Original: Tomruenโ€‚Vector: KCVelaga

Related Transformation Matrix on Wikipedia, Scale Transformation, scale() on mdn docs https://github.com/facebook/react-native/issues/30373#issuecomment-1173649280

GuoXiaoyang commented 1 year ago

Related issue: https://github.com/facebook/react-native/issues/35350

Ashoat commented 1 year ago

In case it's helpful to anybody, here's the latest that's necessary for an inverted list to play nice on Android 13 with React Native 0.70: https://phab.comm.dev/D6193

swaroopbutala commented 1 year ago

In case it's helpful to anybody, here's the latest that's necessary for an inverted list to play nice on Android 13 with React Native 0.70: https://phab.comm.dev/D6193

Thank you! I have been struggling with this for the last few weeks! Thanks for putting in one place! It works great!

Strate commented 1 year ago

Also this issue could be fixed by also adding scaleX: -1 in addition to scaleY: -1 for container and every view.

https://github.com/facebook/react-native/issues/35350#issuecomment-1317465434

zeallat commented 1 year ago

Patch for RN 0.67.5 is here. Thanks for @pqkluan https://github.com/facebook/react-native/issues/30034#issuecomment-806396274

react-native+0.67.5.patch

diff --git a/node_modules/react-native/Libraries/Lists/VirtualizedList.js b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
index 2648cc3..a7e9f18 100644
--- a/node_modules/react-native/Libraries/Lists/VirtualizedList.js
+++ b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
@@ -9,6 +9,7 @@
  */

 const Batchinator = require('../Interaction/Batchinator');
+const Platform = require('../Utilities/Platform');
 const FillRateHelper = require('./FillRateHelper');
 const ReactNative = require('../Renderer/shims/ReactNative');
 const RefreshControl = require('../Components/RefreshControl/RefreshControl');
@@ -2118,9 +2119,14 @@ function describeNestedLists(childList: {
 }

 const styles = StyleSheet.create({
-  verticallyInverted: {
-    transform: [{scaleY: -1}],
-  },
+  verticallyInverted:
+    Platform.OS === 'android'
+      ? {
+        scaleY: -1,
+      }
+      : {
+        transform: [{scaleY: -1}]
+      },
   horizontallyInverted: {
     transform: [{scaleX: -1}],
   },
claudiozam commented 1 year ago

What about RN 0.71? Same issue?

Yes, same issue. Tested on samsung galaxy S20 Note - Android 13.

MayheMatan commented 1 year ago

any workarounds for react-native 0.71?

claudiozam commented 1 year ago

@MayheMatan try this

index.js (in your app)

import ViewReactNativeStyleAttributes from 'react-native/Libraries/Components/View/ReactNativeStyleAttributes';
ViewReactNativeStyleAttributes.scaleY = true;

Then patch RN 0.71 (react-native+0.71.1.patch)

diff --git a/node_modules/react-native/Libraries/Lists/VirtualizedList.js b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
index 2629142..4e60aad 100644
--- a/node_modules/react-native/Libraries/Lists/VirtualizedList.js
+++ b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
@@ -29,6 +29,7 @@ import {findNodeHandle} from '../ReactNative/RendererProxy';
 import flattenStyle from '../StyleSheet/flattenStyle';
 import StyleSheet from '../StyleSheet/StyleSheet';
 import clamp from '../Utilities/clamp';
+import Platform from '../Utilities/Platform';
 import infoLog from '../Utilities/infoLog';
 import {CellRenderMask} from './CellRenderMask';
 import ChildListCollection from './ChildListCollection';
@@ -1840,9 +1841,7 @@ export default class VirtualizedList extends StateSafePureComponent<
 }

 const styles = StyleSheet.create({
-  verticallyInverted: {
-    transform: [{scaleY: -1}],
-  },
+  verticallyInverted: Platform.OS === 'android' ? { scaleY: -1 } : { transform: [{scaleY: -1}] },
   horizontallyInverted: {
     transform: [{scaleX: -1}],
   },
devoren commented 1 year ago

Same problem for 0.71.2

devoren commented 1 year ago

@claudiozam it helps but only with a flat list (+JS drops to 30fps) but if using flashlist then UI and JS drops to 30fps :(

claudiozam commented 1 year ago

@devoren Flatlist is faster than Flashlist inverted.....

image

KoalaSat commented 1 year ago

For now, I have written a quick and dirty workaround to restore scaleY like so in my application's index.js file:

// Add scaleY back to work around its removal in React Native 0.70.
import ViewReactNativeStyleAttributes from 'react-native/Libraries/Components/View/ReactNativeStyleAttributes'
ViewReactNativeStyleAttributes.scaleY = true

This allows you to continue using scaleY and avoid the aforementioned issue with the scroll bar being moved to the other side of the FlatList when using rotate instead of scaleY. This works fine in React Native 0.70.x, but might break in the future if support for the property is removed from the native Android View code.

This saved my day

techyon7 commented 1 year ago

I also encounter this issue with my chat screen. It's really hard to find to root cause.

  • Only happen on Android S10e device.
  • With inverted FlatList.
  • FlatList Item contains a Text with any given length.

It's only take 10 items in the list for the performance to be hit really bad.

I'm using @cookiespam bandaid + patch-package to resolve the issue for now now.

Here is the the content of the patch:

react-native+0.63.3.patch

diff --git a/node_modules/react-native/Libraries/Lists/VirtualizedList.js b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
index 9ec105f..a8f1d8c 100644
--- a/node_modules/react-native/Libraries/Lists/VirtualizedList.js
+++ b/node_modules/react-native/Libraries/Lists/VirtualizedList.js
@@ -11,6 +11,7 @@
 'use strict';

 const Batchinator = require('../Interaction/Batchinator');
+const Platform = require('../Utilities/Platform');
 const FillRateHelper = require('./FillRateHelper');
 const PropTypes = require('prop-types');
 const React = require('react');
@@ -2185,9 +2186,14 @@ function describeNestedLists(childList: {
 }

 const styles = StyleSheet.create({
-  verticallyInverted: {
-    transform: [{scaleY: -1}],
-  },
+  verticallyInverted:
+    Platform.OS === 'android'
+      ? {
+          scaleY: -1,
+        }
+      : {
+          transform: [{scaleY: -1}]
+        },
   horizontallyInverted: {
     transform: [{scaleX: -1}],
   },

For me, it does solve the fps drop issue but the scrolling becomes quite janky. If I try to scroll fast, it would either stop abruptly, go in the opposite direction or scroll very unevenly. Is anyone else facing a similar issue?

rkyle35242 commented 1 year ago

Throwing a transform style on the flatlist and children of scaleY: -1 also had performance issues for us on Android. However, rotating the list and children 180deg didn't seem to have the same problem and kept the "inverted" functionality.

tj-mc commented 1 year ago

Still facing this in 0.71. This is an issue for all chat-like applications

Sondago commented 1 year ago

Still facing this in 0.71. This is an issue for all chat-like applications

+1

lmarques6 commented 1 year ago

Confirmed this solution works for RN 0.71.3 on Android 13 for me as well.

Any update on why this performance issue occurs?