Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fabric] Poor animation performance when animating height property #6854

Open
kirillzyusko opened this issue Dec 26, 2024 · 6 comments
Open
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided

Comments

@kirillzyusko
Copy link
Contributor

Description

Whenever I try to animate height property I get a very poor animation performance:

Paper Fabric
animation-smooth.mp4
animation-fps-drop.mp4

It's really easy to notice on Android (I assume on iOS it may be laggy as well, but visually on iOS animation looks much better than on Android).

The same code on paper works well. Any help is highly appreciated! ❤️ 🙏

Steps to reproduce

  1. Open Chat FlatList screen
  2. Tap on input in the bottom of the screen
  3. Observe animation performance

Snack or a link to a repository

https://github.com/kirillzyusko/react-native-keyboard-controller/tree/main/FabricExample/src/screens/Examples/ReanimatedChatFlatList

Reanimated version

3.16.1

React Native version

0.76.2

Platforms

Android

JavaScript runtime

Hermes

Workflow

React Native

Architecture

Fabric (New Architecture)

Build type

Debug app & dev bundle

Device

Real device

Device model

Pixel 7 Pro

Acknowledgements

Yes

@github-actions github-actions bot added Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided labels Dec 26, 2024
@bartlomiejbloniarz
Copy link
Contributor

Hi @kirillzyusko! I checked your example. The main issue here is that when an inverted FlatList is used it invokes a setState in onLayout. This means that whenever you change the height via Reanimated it triggers a rerender. Because of that both the UI and JS thread try to update the Shadow Tree simultaneously and this results in worse performance. I'm not yet sure how we want to tackle this whole issue, but for your case you can as a workaround use invertStickyHeaders={false}. This will prevent those state updates, while still allowing you to have an inverted list.

I also checked why the issue is not present on iOS. It appears that onLayout is not triggered there when we run the animation. Don't really know why.

@mhoran
Copy link
Contributor

mhoran commented Jan 10, 2025

I have seen poor performance animating padding with an inverted FlatList on iOS when using Fabric as well -- as originally reported in #5685. This was with my own KeyboardAvoidingView using Reanimated useAnimatedKeyboard, but I also tried react-native-keyboard-controller and saw the same.

#5685 was initially opened for Paper, and the padding performance is also poor there when using an inverted FlatList and animating padding. So there may be something else going on there. However, Fabric performance with animated height, margin or padding is significantly degraded as compared to Paper, in my experience.

@kirillzyusko
Copy link
Contributor Author

@bartlomiejbloniarz interesting! I will try to specify invertStickyHeaders={false} and see whether it helps or not!

@mhoran yes, I also noticed that animating padding/height properties is not perfectly synchronized on Paper (you can even watch the attached videos in this issue frame by frame). But I'm just curios - is it possible to make padding/height animation fully synchronous on Android? From what I saw iOS doesn't have any delays on paper arch when I animate padding or height. So curious if it's an Android specific problem or maybe it's react-native + Android issue? 🤔

@bartlomiejbloniarz tagging you too, since maybe you know the answer on this question 😊

@mhoran
Copy link
Contributor

mhoran commented Jan 15, 2025

From what I saw iOS doesn't have any delays on paper arch when I animate padding or height. So curious if it's an Android specific problem or maybe it's react-native + Android issue? 🤔

I do see delays on iOS with padding when using an inverted FlatList, on both Paper and Fabric (though it's much worse on Fabric.) This is partly what led me to experiment with using the Keyboard Layout Guide to avoid the keyboard on iOS, after trying both Reanimated useAnimatedKeyboard and react-native-keyboard-controller.

My guess is that the system is already overloaded with rendering the inverted FlatList and subsequently trying to re-draw the FlatList due to the changing padding is too much for it to handle. If I use FlatList without inverted then it all works just fine (but that won't work for my purposes.) When I tried useAnimatedKeyboard I applied a transform to the Y-axis to offset from the keyboard, which had no delay. Unfortunately that approach introduces other issues.

I don't have much experience with React Native on Android -- I'm primarily using React Native on iOS. (Didn't mean to hijack your report, but just wanted to make sure it's known that this seems to be an issue on all platforms, and is particularly worse on Fabric.)

@kirillzyusko
Copy link
Contributor Author

When I tried useAnimatedKeyboard I applied a transform to the Y-axis to offset from the keyboard, which had no delay. Unfortunately that approach introduces other issues.

Yes, this approach is quite limited (while it works very smooth it still has too many limitations).

My guess is that the system is already overloaded with rendering the inverted FlatList and subsequently trying to re-draw the FlatList due to the changing padding is too much for it to handle. If I use FlatList without inverted then it all works just fine (but that won't work for my purposes.)

Here is an example how it works in react-native-keyboard-controller with inverted FlatList modifying height:

ScreenRecording_01-16-2025.10-56-25.AM_1.MP4

@mhoran Can you share your video with delays you are talking about?

@mhoran
Copy link
Contributor

mhoran commented Jan 16, 2025

Here is an example of jittery animated padding on Paper:

keyboard-controller-paper.mp4

And Fabric:

keyboard-controller-fabric.mp4

In practice, the Fabric animation is even worse, but is dependent on the content in the FlatList. The above recording is a contrived example; a channel with actual content is incredibly jittery (similar to Paper.)

kirillzyusko added a commit to kirillzyusko/react-native-keyboard-controller that referenced this issue Feb 24, 2025
…ew` (#830)

## 📜 Description

Added new `behavior="translate-with-padding"` for
`KeyboardAvoidingView`.

## 💡 Motivation and Context

The new mode ideally fits for chat-like applications and has very good
performance.

The implementation was mostly inspired by
[WindowInsetsAnimation](https://github.com/android/user-interface-samples/tree/main/WindowInsetsAnimation)
sample app. However when I tried to replace `translateY` to `0` and
apply `paddingBottom` which is equal to previous `translateY` I had
flashes on Android (mostly because of nature of Android - transform
properties (e.g., `translateY`) only trigger a **repaint** (no _reflow_)
and are applied **immediately**. Changes to **layout** properties like
**padding** force the Android rendering system to recalculate layout
(**reflow**), which can take an extra frame to resolve). So instead of
switching those properties I decided to keep `translateY` and add
`paddingTop` (in the end of animation to resize the container).

Since we change `paddingTop` only in the beginning or in the end of
animation the transition is unbelievable smooth, because we trigger
layout re-calculation only once (so in terms of complexity new approach
is `O(1)` vs `O(n)`).

Closes
#719 (comment)
software-mansion/react-native-reanimated#6854
#650 (comment)

## 🔢 Things to do
- write e2e tests for chat FlatList example screen (to cover new
functionality with e2e tests);
- update documentation page with detailed explanation of modes and when
to use each of them.

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- added new `"translate-with-padding"` behavior for
`KeyboardAvoidingView`
- added `useTranslateAnimation` hook;
- integrate `useTranslateAnimation` with `"translate-with-padding"`;
- update example app to more performant implementation;

### Docs
- mention new behavior;

## 🤔 How Has This Been Tested?

Tested both paper and fabric on:
- iPhone 15 Pro (iOS 17.5);
- Medium phone (API 35).

## 📸 Screenshots (if appropriate):

### Paper

|Android|iOS|
|--------|---|
|<video
src="https://github.com/user-attachments/assets/f8d35138-77ae-432e-a95b-97104a01b4fe">|<video
src="https://github.com/user-attachments/assets/93db0cdb-4ef4-4fb0-8cd4-9ada0312ff7d">|

### Fabric

|Android|iOS|
|--------|---|
|<video
src="https://github.com/user-attachments/assets/e2398edc-d6c0-41e8-8fdc-eb95e7741f74">|<video
src="https://github.com/user-attachments/assets/379567d3-5d7a-482c-81af-ab359356a11c">|

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided
Projects
None yet
Development

No branches or pull requests

3 participants