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

The performance of the scrollTo function is very poor on Android, with severe jitter #1895

Closed
1 of 3 tasks
zyslife opened this issue Apr 1, 2021 · 9 comments
Closed
1 of 3 tasks
Labels
Area: Performance Bug bug-bash-jan22 Issues visited during Bug Bash Jan 2022 Close when stale This issue is going to be closed when there is no activity for a while Platform: Android This issue is specific to Android 🏠 Reanimated 2

Comments

@zyslife
Copy link

zyslife commented Apr 1, 2021

The performance of the scrollTo function is very poor on Android, with severe jitter

Description

When I operate the ScrollView to call the ScrollTo function at a certain frequency, I notice some page jitter, mostly on Android

Expected behavior

Hopefully the scrolling is smooth

Actual behavior & steps to reproduce

The scrolling is very uneven

Snack or minimal code example


import React, { useEffect } from 'react';
import {
    View,
    ScrollView,
} from 'react-native';
import {
    useAnimatedRef,
    useDerivedValue,
    useSharedValue,
    withTiming,
    scrollTo
} from 'react-native-reanimated'
const TestScrollTo = () => {
    const mRef = useAnimatedRef<ScrollView>()
    const testValue = useSharedValue(0)
    useEffect(() => {
        testValue.value = withTiming(2000, { duration: 10000 })
    }, [])
    useDerivedValue(() => {
        scrollTo(mRef, 0, testValue.value, false);
    })
    return <ScrollView ref={mRef}>
        {Array.from({ length: 200 }).map((item, index) => {
            return <View key={'Test_' + index} style={{ height: 200, borderColor: index % 2 ? 'red' : 'blue', borderWidth: 1 }} />
        })}
    </ScrollView>
}

export default TestScrollTo

Package versions

  • React Native:0.63.4
  • React Native Reanimated:2.0.0
  • NodeJS:v14.15.3
  • Xcode:12
  • Java & Gradle:6.1

Affected platforms

  • Android
  • iOS
  • Web
@github-actions
Copy link

github-actions bot commented Apr 1, 2021

Issue validator

The issue is valid!

@mrousavy
Copy link
Contributor

mrousavy commented Apr 2, 2021

Could you share a video of the lag?

@zyslife
Copy link
Author

zyslife commented Apr 2, 2021

Could you share a video of the lag?

Okay, I'll upload it later.

@zyslife
Copy link
Author

zyslife commented Apr 2, 2021

lag.mp4

@mrousavy Thank you for your attention. In this video, we should be able to see a clear sense of lag.
This video uses the code above.

@piaskowyk piaskowyk self-assigned this Apr 3, 2021
@vonovak
Copy link
Contributor

vonovak commented Jun 11, 2021

I know the repro code is taken from the example https://docs.swmansion.com/react-native-reanimated/docs/2.2.0/api/nativeMethods/scrollTo/ but personally it seems weird to me to call scrollTo inside useDerivedValue - useDerivedValue should not have any side effects but just derive a value from another one.

Seems to me that using useAnimatedReaction is more appropriate or perhaps you could try animate contentOffset prop (probably needs a call to addWhitelistedUIProps) - but I never tried this, just an idea

@zyslife
Copy link
Author

zyslife commented Jun 23, 2021

I know the repro code is taken from the example https://docs.swmansion.com/react-native-reanimated/docs/2.2.0/api/nativeMethods/scrollTo/ but personally it seems weird to me to call scrollTo inside useDerivedValue - useDerivedValue should not have any side effects but just derive a value from another one.

Seems to me that using useAnimatedReaction is more appropriate or perhaps you could try animate contentOffset prop (probably needs a call to addWhitelistedUIProps) - but I never tried this, just an idea

Thank you for your reply. I'm sorry I just saw it. We should really do these things in useAnimatedReaction.But that doesn't affect what we end up seeing. In addition, the contentOffset has not been added to addWhitelistedUIProps yet.

@jishuxx
Copy link

jishuxx commented Aug 5, 2021

validator
I've had this problem too, see when it can be resolved? Thank you very much @mrousavy

@tomekzaw tomekzaw added bug-bash-jan22 Issues visited during Bug Bash Jan 2022 Area: Performance Platform: Android This issue is specific to Android labels Jan 28, 2022
@tomekzaw
Copy link
Member

Hello! Thanks for reporting this issue. We'll definitely take a look on that.

I know the repro code is taken from the example https://docs.swmansion.com/react-native-reanimated/docs/2.2.0/api/nativeMethods/scrollTo/ but personally it seems weird to me to call scrollTo inside useDerivedValue - useDerivedValue should not have any side effects but just derive a value from another one.

That's right. We need to update our docs to call scrollTo within useAnimatedReaction instead of useDerivedValue.

@piaskowyk piaskowyk removed their assignment Jan 28, 2022
@bartlomiejbloniarz
Copy link
Contributor

Hi @zyslife, I tried to reproduce the issue with no success. Is it still happening for you?

@bartlomiejbloniarz bartlomiejbloniarz added the Close when stale This issue is going to be closed when there is no activity for a while label Dec 15, 2023
@github-actions github-actions bot closed this as completed Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Performance Bug bug-bash-jan22 Issues visited during Bug Bash Jan 2022 Close when stale This issue is going to be closed when there is no activity for a while Platform: Android This issue is specific to Android 🏠 Reanimated 2
Projects
None yet
Development

No branches or pull requests

7 participants