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

New Architecture - "Sticky" list header flickering when using useAnimatedScrollHandler/useAnimatedStyle #6992

Open
Flewp opened this issue Feb 6, 2025 · 14 comments
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snippet of code, snack or repo is provided Reproducible 🎉

Comments

@Flewp
Copy link

Flewp commented Feb 6, 2025

Description

I have a ScrollView that contains an item that I want to "stick" to the top. Historically we've given this sticky item a useAnimatedStyle where its transformY is driven by a shared value that is updated by a useAnimatedScrollHandler attached to the onScroll property of the Animated.ScrollView. This works using the old architecture, but in the new architecture the sticky item lags behind the scroll view's offset, causing the sticky item to flicker.

Old Architecture New Architecture
old-arch-video.mov
new-arch-video.mov

Steps to reproduce

Reproducer link is below, or use this component in a sandbox app.

import React from 'react';
import {SafeAreaView, StyleSheet, Text, View} from 'react-native';
import Animated, {
  makeMutable,
  useAnimatedScrollHandler,
  useAnimatedStyle,
  useFrameCallback,
  type SharedValue,
} from 'react-native-reanimated';
import {ReanimatedScrollEvent} from 'react-native-reanimated/lib/typescript/hook/commonTypes';

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
  },
  stickyItemContainer: {
    zIndex: 1,
    padding: 32,
    backgroundColor: 'yellow',
  },
  itemContainer: {
    padding: 16,
    backgroundColor: '#f2f2f2',
    marginVertical: 4,
    marginHorizontal: 8,
    borderRadius: 8,
  },
  itemText: {
    fontSize: 16,
    fontWeight: '500',
  },
});

const scrollPosition: SharedValue<number> = makeMutable(0);

function StickyItem() {
  const stickyStyle = useAnimatedStyle(() => {
    return {
      transform: [
        {
          translateY: scrollPosition.get(),
        },
      ],
    };
  });

  return (
    <Animated.View
      key={'sticky'}
      style={[styles.stickyItemContainer, stickyStyle]}>
      <Text style={styles.itemText}>{'STICKY'}</Text>
    </Animated.View>
  );
}

const items = Array.from({length: 50}, (_, i) => `Item ${i + 1}`);
const App = () => {

  const handleScroll = useAnimatedScrollHandler(
    (event: ReanimatedScrollEvent) => {
      scrollPosition.set(event.contentOffset.y);
    },
    [],
  );

  return (
    <SafeAreaView style={styles.container}>
      <Animated.ScrollView onScroll={handleScroll}>
        <StickyItem />
        {items.map((item, index) => (
          <View key={index} style={styles.itemContainer}>
            <Text style={styles.itemText}>{item}</Text>
          </View>
        ))}
      </Animated.ScrollView>
    </SafeAreaView>
  );
};

export default App;

Snack or a link to a repository

https://github.com/Flewp/reanimated-scroll-reproducer

Reanimated version

3.16.7

React Native version

0.77.0

Platforms

Android

JavaScript runtime

Hermes

Workflow

React Native

Architecture

Fabric (New Architecture)

Build type

Release app & production 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 Feb 6, 2025
@Flewp
Copy link
Author

Flewp commented Feb 6, 2025

We've tried patching this change in, but still experiencing the repro.

This issue looks like it could be related to #6915, #6708

@tomekzaw
Copy link
Member

tomekzaw commented Feb 6, 2025

Here's what I found out so far:

  • This is broken only on Android, iOS works fine.
  • This is broken on both 0.76.6 and 0.77.0
  • Still happens after changing makeMutable to useSharedValue
  • Still happens after adding margin: Math.random() to force updates via ShadowTree instead of via synchronouslyUpdateUIProps
  • Still happens after replacing transform with top
  • Still happens even if I block the JS thread for 100 ms every 200 ms
  • Happens also with useScrollViewOffset instead of useAnimatedScrollHandler

@tomekzaw
Copy link
Member

tomekzaw commented Feb 7, 2025

After some digging with @bartlomiejbloniarz, we found out that the solution is to enable ReactNativeFeatureFlags::enableSynchronousStateUpdates feature flag.

@Nodonisko
Copy link
Contributor

Nodonisko commented Feb 8, 2025

@tomekzaw How can I enable this feature flag? Also what does it do? I am not able to find any documentation about it.

Also for me there is significant framedrop on both threads when doing similar animation when scrolling fast. Since it drops frames also on UI thread it will cause scroll itself to be janky.

@tomekzaw
Copy link
Member

tomekzaw commented Feb 8, 2025

How can I enable this feature flag?

Probably the easiest way is to just return true in NativeReactNativeFeatureFlags::enableSynchronousStateUpdates in NativeReactNativeFeatureFlags.cpp and build React Native from source.

Also what does it do?

When ScrollView is scrolled on Android, it does two things here

  1. Dispatch state update
  2. Emit onScroll event

Reanimated intercepts onScroll event, runs useAnimatedScrollHandler worklets, runs useAnimatedStyle to calculate the new style of the sticky header and commits the changes to the C++ ShadowTree synchronously on the UI thread.

As for state update, React Native synchronously calls EventDispatcher::dispatchStateUpdate method in order to update the scrollOffset in ScrollView's State object also in C++ ShadowTree.

EventDispatcher::dispatchStateUpdate calls eventQueue_.enqueueStateUpdate which adds the job to the queue which is then flushed on the JS thread because for some reason React Native commits to the ShadowTree only on the JS thread. This makes the process asynchronous and thus prone to inconsistencies.

We've noticed that there's a feature flag that changes the behavior EventDispatcher::dispatchStateUpdate and makes a synchronous call to statePipe_ which will commit the new state synchronously on the UI thread, without jumping to the JS thread, which fixes the problem:

https://github.com/facebook/react-native/blob/7b7c45030ba6ae196b89f0fb8615d184a261cd4a/packages/react-native/ReactCommon/react/renderer/core/EventDispatcher.cpp#L46-L50

@Nodonisko
Copy link
Contributor

Nodonisko commented Feb 8, 2025

@tomekzaw Thanks for exhausting explanation!

Do you know what's the reason that this is not enabled by default? Using own modified version of RN isn't really comfortable.

@tomekzaw
Copy link
Member

tomekzaw commented Feb 9, 2025

@Nodonisko I don't know the exact reason, I suspect that some of the changes in the rendering pipeline are hidden behind a feature flag so the community can test it on a small group of users and prevent unwanted regressions.

@Nodonisko
Copy link
Contributor

Thanks, so to sum it up we either can use our local version of RN with feature flag turned on or wait until it will be on by default in RN.

github-merge-queue bot pushed a commit that referenced this issue Feb 10, 2025
## Summary

This PR adds an example with sticky header implemented using
`useAnimatedScrollHandler` (although `useScrollViewOffset` is also
recommended) and `useAnimatedStyle` to align the vertical translation of
a view with `ScrollView` content offset.

Source code has been adapted from the repro in
#6992
by @Flewp.

## Test plan
@jonatan-zoominfo
Copy link

@Nodonisko Did this solution work for you? Unfortunately didn't help in my case. Tried on RN Versions: 0.76.7, 0.77.0

@Nodonisko
Copy link
Contributor

@jonatan-zoominfo Sadly no, currently I only disabled animation for Android because it's not critical for us.

@yadormad
Copy link

This issue reproduces also with sticky headers in @shopify/flash-list which are implemented without reanimated and greatly affects scrolling performance (on production builds as well). Reproducing also on on ios.
Repro repo

withReanimated.mov
withoutReanimated.mov
iosWithReanimated.mov

@janicduplessis
Copy link
Contributor

I've done further investigation here to try to find the root cause of the issue. It seems to boil down to how animated props are added in ReanimatedCommitHook.cpp (https://github.com/software-mansion/react-native-reanimated/blob/main/packages/react-native-reanimated/Common/cpp/reanimated/Fabric/ReanimatedCommitHook.cpp#L79-L82) and how the fabric renderer on Android implements a "push" model instead of a "pull" model like on iOS.

Here's more details about what happens exactly:

On iOS, the renderer implements what we will call here a "pull" model. What this means is that when react is done mutating the shadow tree it notifies the mounting layer, which will then post a new task to be executed on the UI thread. This task will then "pull" the mutations from the shadow tree and commit them to the host views synchronously. This is very important since this guarantees that what is committed represents the latest state of the shadow tree.

Image

You can see here that since the operations are pulled from the UI thread, they do include updates that were committed from the ui thread.

On android, the renderer implements what we will call here a "push" model. What this means is when react is done mutating the shadow tree it immediately pulls the mutations and posts a new task to be executed on the UI thread containing the mutations. The UI thread will then commit these updates to the host views. Note that if an update is committed from the UI thread after the mutations were pulled from JS, it is possible that stale values will be committed.

Image

In this case, here's what happens:

Image

It also happens a lot more because scroll events trigger native state updates, which are also dispatched from the JS thread (without the synchronous state updates feature flags) and trigger the commit hook. However just using sync state updates doesn't fully solve the issue since JS updates from react renders will also trigger the same problem.

I think the ideal fix is to implement the same pull model on Android as we have on iOS, but this might require a lot for work. I am curious if there is any workarounds we could implement in reanimated in the meantime.

@yadormad
Copy link

yadormad commented Mar 1, 2025

@janicduplessis Thank you for the explanation. Do your investigation results relevant considering that this issue also reproduces on ios? See my previous comment.

@janicduplessis
Copy link
Contributor

In our case we didn’t see any problems on iOS, and also not using flashlist, I suspect this might be a different issue.

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 Reproducible 🎉
Projects
None yet
Development

No branches or pull requests

6 participants