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

MotiPressable has big performance hit vs animated TouchableWithoutFeedback in flash list #322

Open
2 of 3 tasks
xzilja opened this issue Oct 29, 2023 · 33 comments
Open
2 of 3 tasks

Comments

@xzilja
Copy link

xzilja commented Oct 29, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Do you want this issue prioritized?

  • Yes, I have sponsored
  • Not urgent

Current Behavior

I have made 2 implementations for a Pressable component, one is using <MotiPressable /> other is using TouchableWithoutFeedback and Animated (react-native-gesture-handler + react-native-reanimated respectively).

I have a modal which renders list of emojis within @shopify/flash-list where each of the items is that <Pressable> component with a text node inside of it.

For some reason there is significant lag with <MotiPressable /> implementation, see Screenshots section below for videos.

Expected Behavior

Should have same performance as animated touchable?

Steps To Reproduce

Pressable implementation with moti (laggy)

import type { MotiPressableProps } from 'moti/interactions'
import { MotiPressable } from 'moti/interactions'

// -- Types ---------------------------------------------------------------------
export interface Props extends Omit<MotiPressableProps, 'animate' | 'transition' | 'hitSlop'> {
  pressedScale?: number
  pressedOpacity?: number
}

// -- Component -----------------------------------------------------------------
export function MotiPressableExample({ children, pressedOpacity, pressedScale, ...props }: Props) {
  const scale = pressedScale ?? 0.9
  const opacity = pressedOpacity ?? 0.7
  
  return (
    <MotiPressable
      {...props}
      hitSlop={{ top: 12, left: 12, bottom: 12, right: 12 }}
      animate={({ pressed }) => {
        'worklet'
        return {
          scale: pressed ? scale : 1,
          opacity: pressed ? opacity : 1
        }
      }}
    >
      {children}
    </MotiPressable>
  )
}

Pressable implementation with touchable and reanimated (performant)

import { TouchableWithoutFeedback } from 'react-native-gesture-handler'
import type { TouchableOpacityProps } from 'react-native-gesture-handler'
import Animated, {
  interpolate,
  useAnimatedStyle,
  useSharedValue,
  withSpring
} from 'react-native-reanimated'

// -- Helpers -------------------------------------------------------------------
const AnimatedTouchable = Animated.createAnimatedComponent(TouchableWithoutFeedback)

// -- Types ---------------------------------------------------------------------
export interface Props extends Omit<TouchableOpacityProps, 'hitSlop' | 'onPressIn' | 'onPressOut'> {
  pressedScale?: number
  pressedOpacity?: number
}

// -- Component -----------------------------------------------------------------
export function Pressable({ children, pressedOpacity, pressedScale, style, ...props }: Props) {
  const animation = useSharedValue(0)
  const scale = pressedScale ?? 0.9
  const opacity = pressedOpacity ?? 0.7

  const animatedStyle = useAnimatedStyle(() => ({
    transform: [{ scale: interpolate(animation.value, [0, 1], [1, scale]) }],
    opacity: interpolate(animation.value, [0, 1], [1, opacity])
  }))

  function onPressIn() {
    animation.value = withSpring(1)
  }

  function onPressOut() {
    animation.value = withSpring(0)
  }

  return (
    <AnimatedTouchable
      {...props}
      onPressIn={onPressIn}
      onPressOut={onPressOut}
      hitSlop={{ top: 12, left: 12, bottom: 12, right: 12 }}
      style={[animatedStyle, style]}
    >
      {children}
    </AnimatedTouchable>
  )
}

Usage / Test example for both of these

       <FlashList
          horizontal={false}
          numColumns={5}
          data={emojis}
          showsVerticalScrollIndicator={false}
          estimatedItemSize={56}
          renderItem={({ item }) => (
            <Pressable>
              <Text>
                {item}
              </Text>
            </Pressable>
          )}
        />

Versions

- Moti: 0.27.0
- Reanimated: 3.5.4
- React Native: 0.72.6

Screenshots

Test with moti (laggy)

Screen.Recording.2023-10-29.at.14.43.48.mov

Test with animated touchable (performant)

Screen.Recording.2023-10-29.at.14.44.14.mov

Reproduction

https://stackblitz.com/edit/nextjs-v5vkju?file=pages%2Findex.tsx
Tried to recreate this in StackBlitz but adding flashlist broke it on web, let me know if above code snippets are not enough.

@nandorojo
Copy link
Owner

Very odd. A few things:

can you follow the flashlist rules, like useCallback for renderItem and returning an actual item component that is memoized? Also, can you test in production (not dev mode) and send a video there?

lastly, what if you put a plain MotiView instead of MotiPressable? Same issue? Want to make sure Moti isn’t the bottleneck.

@alex-lanclos
Copy link

@nandorojo hey! I wanted to flag this issue as being an actual issue. So I've been using Moti and Dripsy for the basis of a component library for a production app for a company I work for. When we migrated to Reanimated 3 and React Native .72, we noticed a huge performance drop on pages that were using the new component library. I had thought it was something with Dripsy, but under further investigation, it seems to be more from Moti. For context, if I even have just a few of our buttons that were just using a MotiView with a standard Pressable wrapped around it, we'd see dropped frames on navigating to those pages.

So the tldr is that, just one MotiView doesn't really affect performance too bad, but as you start to stack more and more on a page, the performance hits become more noticeable.

@nandorojo
Copy link
Owner

Which reanimated version?

It's interesting because moti is just reanimated together with RNGH.

I typically avoid using any expensive components at all in lists (including reanimated and RNGH often)

For context, if I even have just a few of our buttons that were just using a MotiView with a standard Pressable wrapped around it, we'd see dropped frames on navigating to those pages.

Each moti view calls useAnimatedStyle, it's possible this is why?

I wonder where the cost is coming from...

@alex-lanclos
Copy link

We are on 3.6.1 for Reanimated. We saw better performance on our current master when we were on the following lib combos:

"moti": "^0.25.3",
"react-native-reanimated": "~2.17.0",
"react-native": "0.70.11",

I'm using https://docs.flashlight.dev/ to monitor dropped frames and cpu usage. Even when were on the above lib versions, we were seeing performance hits on pages where we had more buttons or text inputs ( our text inputs have custom translation based animations) but as soon as we migrated to

"moti": "^0.27.2",
"react-native-reanimated": "~3.6.1",
"react-native": "0.72.9",

we saw a considerable drop in performance on those pages.

@alex-lanclos
Copy link

To add some extra information from some testing. I swapped out MotiView that I was using for just a base level Animated.View with a useAnimatedStyle, and I'm not seeing performance hits. I haven't tried using RNGH for this, so it's possible that has performance issues

@nandorojo
Copy link
Owner

Only other thing I could think of is our use of react context?

https://github.com/nandorojo/moti/blob/master/packages/moti/src/interactions/pressable/pressable.tsx#L243

Though every RN text node uses this so I'm not sure if that's it...

It could maybe be the usage of other hooks in useMotify?

To clarify -- MotiView has this issue, not just MotiPressable, right?

Perhaps there was some regression I missed. Might be an easy fix.

What happens if you call useMotify and pass the result to an animated view from Reanimated?

Thanks for digging into this!

@nandorojo
Copy link
Owner

The overhead moti adds is 2 useEffects (could be moved into one), one shared value (tracking mounted state) and framer motion presence hooks.

If useMotify alone works, then I'd blame framer, and we can come up with a fix.

@alex-lanclos
Copy link

alex-lanclos commented Jan 11, 2024

Correct this is just for MotiView.

So I'll start posting a few comments with my testing journey. So let's use our much simpler Fab component. Here's a quick snippet.

import React from 'react'
import { PressableProps } from 'react-native'
import { SxProp, useSx, Pressable, View, styled } from 'dripsy'
import MaterialIcon from 'react-native-vector-icons/MaterialIcons'
import MaterialIconOutlined from 'react-native-vector-icons/MaterialIconsOutlined'

import { MotiView } from 'moti'
import { baseColors } from '@styles/theme'

export interface FabProps extends PressableProps {
  /**
   * An optional style override for the "default" state.
   */
  viewStyleOverride?: SxProp
  backgroundColorOverride?: string
  pressedColorOverride?: string
  /**
   * An optional property to disable the button.
   */
  disabled?: boolean
  /**
   * An optional property to provide the icon.
   */
  materialIcon?: string
  /**
   * An optional property to provide the icon.
   */
  materialIconOutlined?: string
  /**
   * An optional property to override the icon color.
   */
  iconColorOverride?: string
}

export function Fab(props: FabProps) {
  const {
    viewStyleOverride: $viewStyleOverride,
    backgroundColorOverride,
    pressedColorOverride,
    disabled,
    materialIcon,
    materialIconOutlined,
    iconColorOverride,
    ...rest
  } = props

  const sx = useSx()

  const $viewStyle = () => {
    const disabledStyle = disabled ? $disabledView : {}
    return sx({
      ...$viewStyleOverride,
      ...disabledStyle,
    } as SxProp)
  }

  const $backgroundAnimatedView = ({ pressed }) => {
    const color = pressedColorOverride || $pressedColor
    const backgroundColor = backgroundColorOverride || $backgroundColor

    return sx({
      backgroundColor: pressed ? color : backgroundColor,
    })
  }

  const IconComponent = materialIcon ? MaterialIcon : MaterialIconOutlined
  const iconColor: string = disabled
    ? baseColors.$disabledWhiteText
    : iconColorOverride || baseColors.$primaryWhiteText

  const StyledMotiView = styled(MotiView, {
    themeKey: 'buttons',
    defaultVariant: 'fab',
  })()

  return (
    <Pressable accessibilityRole="button" disabled={disabled} {...rest}>
      {(state) => (
        <StyledMotiView
          animate={$backgroundAnimatedView(state)}
          transition={{ type: 'timing', duration: 300 }}
          style={$viewStyle()}
        >
          <IconComponent size={32} name={materialIcon || materialIconOutlined} color={iconColor} />
        </StyledMotiView>
      )}
    </Pressable>
  )
}

const $backgroundColor = '$blue100'

const $pressedColor = '$blue120'

const $pressedView: SxProp = {
  shadowOpacity: 0,
  elevation: 0,
}

const $disabledView: SxProp = {
  backgroundColor: '$blue40',
}

When I navigate to a page on our .72 react native build that has 20 Fab components, we see a really noticeable frame drop.
Screenshot 2024-01-11 at 2 57 10 PM

@nandorojo
Copy link
Owner

Also -- how are you measuring performance here?

@alex-lanclos
Copy link

Also -- how are you measuring performance here?

https://docs.flashlight.dev/

@alex-lanclos
Copy link

alex-lanclos commented Jan 11, 2024

If I swap over to using reanimated straight with some hardcoded colors instead of pulling from sx,

import React from 'react'
import { PressableProps } from 'react-native'
import { SxProp, useSx, Pressable, View, styled } from 'dripsy'
import MaterialIcon from 'react-native-vector-icons/MaterialIcons'
import MaterialIconOutlined from 'react-native-vector-icons/MaterialIconsOutlined'

import { MotiView } from 'moti'
import { baseColors } from '@styles/theme'
import Animated, { useAnimatedStyle, useSharedValue, withTiming } from 'react-native-reanimated'

export interface FabProps extends PressableProps {
  /**
   * An optional style override for the "default" state.
   */
  viewStyleOverride?: SxProp
  backgroundColorOverride?: string
  pressedColorOverride?: string
  /**
   * An optional property to disable the button.
   */
  disabled?: boolean
  /**
   * An optional property to provide the icon.
   */
  materialIcon?: string
  /**
   * An optional property to provide the icon.
   */
  materialIconOutlined?: string
  /**
   * An optional property to override the icon color.
   */
  iconColorOverride?: string
}

export function Fab(props: FabProps) {
  const {
    viewStyleOverride: $viewStyleOverride,
    backgroundColorOverride,
    pressedColorOverride,
    disabled,
    materialIcon,
    materialIconOutlined,
    iconColorOverride,
    ...rest
  } = props

  const sx = useSx()
  const pressed = useSharedValue(false);

  const $viewStyle = () => {
    const disabledStyle = disabled ? $disabledView : {}
    return sx({
      ...$viewStyleOverride,
      ...disabledStyle,
    } as SxProp)
  }

  const animatedStyles = useAnimatedStyle(() => {
    const color = pressedColorOverride || $pressedColor
    const backgroundColor = backgroundColorOverride || $backgroundColor
    return {
    backgroundColor: withTiming(pressed.value ? color : backgroundColor, { duration: 300 }),
  }})

  const IconComponent = materialIcon ? MaterialIcon : MaterialIconOutlined
  const iconColor: string = disabled
    ? baseColors.$disabledWhiteText
    : iconColorOverride || baseColors.$primaryWhiteText


  return (
    <Pressable onPressIn={() => {
      pressed.value = true
    }} 
    onPressOut={() => {
      pressed.value = false
    }} accessibilityRole="button" disabled={disabled} {...rest}>
      {(state) => (
        <Animated.View
          style={[sx({variant: 'buttons.fab'}), $viewStyle(), animatedStyles]}
        >
          <IconComponent size={32} name={materialIcon || materialIconOutlined} color={iconColor} />
        </Animated.View>
      )}
    </Pressable>
  )
}

const $backgroundColor = baseColors.$blue100

const $pressedColor = baseColors.$blue120

const $pressedView: SxProp = {
  shadowOpacity: 0,
  elevation: 0,
}

const $disabledView: SxProp = {
  backgroundColor: '$blue40',
}

I'm not seeing any dropped frames when navigating. Also the styling is a little weird, my prettier crashed on my vscode and didn't feel like restarting vscode 😂

@alex-lanclos
Copy link

alex-lanclos commented Jan 11, 2024

@nandorojo so it's 100% the inclusion of useMotify. Without even using it's value anywhere. Just by adding

const animation = useMotify({ animate: {
    backgroundColor: pressed ? $pressedColor : $backgroundColor
  },  transition: { type: 'timing', duration: 300 }})

inside of my Fab Component causes the exact same frame drops. Also I've never used useMotify before, so I was actually running into some weird issues actually getting Animated.View to even accept the style that animation was spitting out. Very possible I was missing something, but regardless just having the hook in the component causes the issue.

Scratch that, I was just being dumb and didn't return animation.style

@alex-lanclos
Copy link

Also I've been following facebook/react-native#36296 pretty closely for the past few days. It's very possible that there was maybe a regression in React Native with how effects themselves cause the component tree to re-render?

@nandorojo
Copy link
Owner

so it's 100% the inclusion of useMotify. Without even using it's value anywhere. Just by adding

Maybe you can copy the source into your app and comment things out like effects etc to see if it gets fixed?

Maybe comment things that are outside of the style hook?

https://github.com/nandorojo/moti/blob/de894634c7b8205c1f3e2b393a45fc61d73b3068/packages/moti/src/core/use-motify.ts

@alex-lanclos
Copy link

Doing that right now, it's definitely not the effects. It stems from the useAnimatedStyle itself. If I just replace it with a very simple useAnimatedStyle, there are no issues.

@alex-lanclos
Copy link

Seems to be within the logic for Object.keys(mergedStyles as any).forEach((key) => {. Going to see if I can find the exact section that has the issue.

@alex-lanclos
Copy link

Running

const { animation, config, shouldRepeat, repeatCount, repeatReverse } = animationConfig(
        key,
        transition
      )

Causes a minor FPS drop just by itself. The dips on the right side are when I uncomment animationConfig.
Screenshot 2024-01-11 at 4 00 56 PM

@alex-lanclos
Copy link

alex-lanclos commented Jan 11, 2024

uncommenting

let finalValue = animation(value, config, callback)
      if (shouldRepeat) {
        finalValue = withRepeat(finalValue, repeatCount, repeatReverse)
      }

      if (delayMs != null && typeof delayMs === 'number') {
        final[key] = withDelay(delayMs, finalValue)
      } else {
        final[key] = finalValue
      }

which actually sets up my very simple animation of just adjusting the background color, I'm seeing further frame drops as seen on the right most side.
Screenshot 2024-01-11 at 4 05 14 PM

@alex-lanclos
Copy link

alex-lanclos commented Jan 11, 2024

Screenshot 2024-01-11 at 4 06 22 PM

Also here are some CPU specs that are inline with the above FPS graph.

@nandorojo
Copy link
Owner

Good to know...that function should be pretty lightweight, I wonder what it could be. Let me know any other findings.

How are you recording the performance btw?

@alex-lanclos
Copy link

How are you recording the performance btw?

Linked a few times but it must have gotten lost in the conversation. https://github.com/bamlab/flashlight It's an android only tool for now, but aligns pretty well with what I'm seeing in person with the dropped frames when navigating to a page that contains this test scenario.

@nandorojo
Copy link
Owner

Oh sorry missed that comment

@alex-lanclos
Copy link

Oh sorry missed that comment

No worries. I'll have to pick this up again tomorrow or next week. Let me know if you find anything else out from your side.

@nandorojo
Copy link
Owner

I opened a new issue at #336 to address general performance issues. I think there may have been a regression with worklets + reanimated 3 causing issues for Moti and I'd like to solve it generally. If there is a simple step-by-step way to measure performance differences I would please ask that you comment it there. I would like to debug it but don't have experience on perf measurement so any help there is appreciated.

Thanks a bunch!

@nandorojo
Copy link
Owner

PS the notes from @alex-lanclos already help a lot. I'm wondering if calling withTiming early in the worklet is causing any issues, for example. Is it possible Reanimated is executing these functions early causing a performance problem? We'll have to see.

@alex-lanclos
Copy link

@nandorojo as a heads up, I should be able to look back into this again in a week or two, got put on a pretty large refactor that was time sensitive, but finishing that up soon. In the short run we unblocked ourselves by migrating our components from Moti to pure reanimated v3 and that completely removed all of our frame drops. From what I've gathered worst offenders are when we have a large amount of Moti based views on a screen and we have 1 of the 3 cases: 1) They are in a list of a reasonable size, 2) On a page that re-renders a good bit (an example from our app was a page where you could draw on a page with Skia, 3) When navigating to a page that has a reasonable amount of Moti Views. All three of these cases correlate to large spikes in the CPU, and depending on the amount of items, could lead to full multi second 0 FPS frame drops.

For context we are using react navigation v6 as our library of choice for navigation and v3 react native screens. As a refresher, I've been using Flashlight.dev and measuring performance on local android devices between multiple iterations. Also our current version of reanimated is "react-native-reanimated": "~3.6.1",

Also as some extra context, this seems to have been a long standing performance issue with Moti, but it was just less perceptible for quite some time. When we were on "react-native-reanimated": "~2.17.0" and "moti": "^0.25.3", we were still seeing performance hits when encountering one of the three above cases, but the frame drops were closer to just 42 FPS lows.

@nandorojo
Copy link
Owner

Okay got it. My guess is there is some one-line fix somewhere causing this, since moti is just useAnimatedStyle that loops over object keys. So it must be a benign issue somewhere.

@nandorojo
Copy link
Owner

One small idea, should we switch from Pressable to TouchableWithoutFeedback?

@marcel-happyfloat
Copy link

I also noticed problems with the performance of Moti. The first 2-3 repeats went smooth but after some scrolling or open/closing modals (can't remember exactly) it went bad fast.
So I replaced the same animation with my own implementation and it worked smoothly without stutter and lag. Can't remember exactly what I used with Moti, but was very basic.

function AnimatedDrift({children, startValue, endValue, duration}) {
  const sv = useSharedValue(endValue || 20);

  useEffect(() => {
    sv.value = withRepeat(
      withTiming(startValue != null ? startValue : -sv.value, {
        duration: duration || 2000,
      }),
      -1,
      true,
    );
  }, []);

  const animatedStyle = useAnimatedStyle(() => ({
    transform: [{translateX: sv.value}],
  }));

  return <Animated.View style={animatedStyle}>{children}</Animated.View>;
}

@alex-lanclos
Copy link

Using pure react-native-reanimated caused no issues on my side either. I'm not sure why Pressable vs TouchableWithoutFeedback would cause a big change, but worth a test?

@nandorojo
Copy link
Owner

Sorry to hear about the perf issues you guys faced. I assume you’re on SDK 50? Can you share exact reanimated version so I can try reproducing properly?

I haven’t had time to get to it quite yet but I will when I have the chance

@nandorojo
Copy link
Owner

Also did you guys notice a difference on iOS or just Android? And is it all measured or just noticing it qualitatively?

@efstathiosntonas
Copy link
Contributor

@nandorojo long shot but keep in mind that there are some regressions on reanimated > 3.6.1, if you check this issue and follow the connected/mentioned issues there is probably a memory leak with useSharedValue:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants