software-mansion / react-native-svg

SVG library for React Native, React Native Web, and plain React web projects.
MIT License
7.34k stars 1.11k forks source link

Text with animatedProps crashes on Android #1991

Closed matinzd closed 1 week ago

matinzd commented 1 year ago

Bug

NOTE: I am not sure if this issue is related to react-native-reanimated or react-native-svg

Passing animatedProps to AnimatedSvgText will crash on Android.

Environment info

React native info output:

System:
    OS: macOS 13.2
    CPU: (10) arm64 Apple M1 Pro
    Memory: 88.50 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.18.1 - /var/folders/q_/l5p2x67170n8qdtv2wlb3xmm0000gn/T/yarn--1677143801584-0.21076613196954197/node
    Yarn: 1.22.19 - /var/folders/q_/l5p2x67170n8qdtv2wlb3xmm0000gn/T/yarn--1677143801584-0.21076613196954197/yarn
    npm: 8.19.2 - ~/.nvm/versions/node/v16.18.1/bin/npm
    Watchman: Not Found
  Managers:
    CocoaPods: 1.11.3 - /Users/matin/.rvm/gems/ruby-2.7.6/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 22.2, iOS 16.2, macOS 13.1, tvOS 16.1, watchOS 9.1
    Android SDK: Not Found
  IDEs:
    Android Studio: 2022.1 AI-221.6008.13.2211.9477386
    Xcode: 14.2/14C18 - /usr/bin/xcodebuild
  Languages:
    Java: 11.0.17 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.2.0 => 18.2.0 
    react-native: 0.71.2 => 0.71.2 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found
✨  Done in 9.85s.

Library version: 13.8.0 Reanimated version: 2.14.4

Steps To Reproduce

  1. Create AnimatedSvgText component with Reanimated.createAnimatedComponent(Text)
  2. Pass in animatedProps
  3. It crashes on Android, works fine on iOS (Not sure if it is reanimated issue or svg)
Screenshot 2023-02-23 at 10 23 33

Describe what you expected to happen:

  1. Not to crash

Short, Self Contained, Correct (Compilable), Example

import {Button, View} from 'react-native'
import Animated, {
  useAnimatedProps,
  useSharedValue,
} from 'react-native-reanimated'
import {Text, Svg} from 'react-native-svg'

const AnimatedSvgText = Animated.createAnimatedComponent(Text)

const Example = ({progress = 0}) => {
  const position = useSharedValue(50)

  const animatedTextProps = useAnimatedProps(() => {
    return {
      x: position.value,
      y: position.value,
    }
  })

  return (
    <View style={{paddingTop: 120}}>
      <Svg
        fill={'red'}
        viewBox="0 0 100 100"
        style={{backgroundColor: 'red', width: 100, height: 100}}>
        <AnimatedSvgText fill="black" animatedProps={animatedTextProps}>
          {progress}
        </AnimatedSvgText>
      </Svg>
      <Button
        title={'Randomize position'}
        onPress={() => {
          position.value = Math.random() * 100
        }}
      />
    </View>
  )
}

export default Example
matinzd commented 1 year ago

Snack: https://snack.expo.dev/@ddappteam/reanimated-svg-crash

matinzd commented 1 year ago

UPDATE:

I passed number[] and the issue got resolved and now it works both on iOS and Android. But if I just pass number, the shared value will break the Android side. So it seems SVGText API is not consistent for reanimated in Android and iOS.

  const animatedTextProps = useAnimatedProps(() => {
    return {
      x: [position.value],
      y: [position.value],
    }
  })

iOS: https://github.com/software-mansion/react-native-svg/blob/2c59100e24853cbdd208f745ea64f366f7e00e6f/apple/Text/RNSVGText.mm#L158

Android: https://github.com/software-mansion/react-native-svg/blob/2c59100e24853cbdd208f745ea64f366f7e00e6f/android/src/paper/java/com/facebook/react/viewmanagers/RNSVGTSpanManagerInterface.java#L51

Maybe we need a cast check here:

https://github.com/software-mansion/react-native-svg/blob/2c59100e24853cbdd208f745ea64f366f7e00e6f/android/src/paper/java/com/facebook/react/viewmanagers/RNSVGTSpanManagerDelegate.java#LL135C24-L135C24

@tomekzaw

WoLewicki commented 1 year ago

Yeah, it is due to how codegen works. Unfortunately the native side can only accept one type for each prop, so we had to stick with passing an array for all the props that could be either number or number[]. So as long as the codegen is not upgraded to be able to pass different types for a prop, you have to pass array with one element instead of just element for all those props. Does it answer your question?

matinzd commented 1 year ago

Yeah, it is due to how codegen works. Unfortunately the native side can only accept one type for each prop, so we had to stick with passing an array for all the props that could be either number or number[]. So as long as the codegen is not upgraded to be able to pass different types for a prop, you have to pass array with one element instead of just element for all those props. Does it answer your question?

Yeah of course. I could fix it by just passing one element array. Isn't it better maybe to update types and document to just accept number[]? This was very misleading to me. But at the first place, why do we need array for positionX? Can't it be just double value? What is the reason behind it? Because I am not that familiar with the codebase.

WoLewicki commented 1 year ago

Isn't it better maybe to update types and document to just accept number[]?

You can pass a number as long as you don't use react-native-reanimated because we take care of the prop so it is parsed properly on the JS side, but reanimated skips this part.

But at the first place, why do we need array for positionX?

If you pass an array for the x prop in svg standard, it makes next elements start at the points passed in that array. You can play with it in your elements. Does it answer your questions?

matinzd commented 1 year ago

Isn't it better maybe to update types and document to just accept number[]?

You can pass a number as long as you don't use react-native-reanimated because we take care of the prop so it is parsed properly on the JS side, but reanimated skips this part.

But at the first place, why do we need array for positionX?

If you pass an array for the x prop in svg standard, it makes next elements start at the points passed in that array. You can play with it in your elements. Does it answer your questions?

Ah cool! Thank you. I see.

bessim-dev commented 8 months ago

I think i'm facing this same issue with a circle-fill animation, I'm using react-native-reanimated@3.5.4 and react-native-svg@13.14.0 my code is something like this: const animatedFill = useAnimatedProps(() => { return { fill: interpolateColor( color.value, [ UPLOAD_STATE.IDLE, UPLOAD_STATE.INITIALIZE, UPLOAD_STATE.UPLOADING, UPLOAD_STATE.SUCCESS, UPLOAD_STATE.ERROR, ], [mainColor, "#EEE683", "#D4A418", happyColor, deleteBtnColor] ), }; }); return ( <Svg height={svgSize} width={svgSize} style={{ position: "relative", shadowColor: mainColor, shadowOffset: { width: 0, height: 3, }, shadowOpacity: 0.17, shadowRadius: 3.05, elevation: 4, }}> <AnimatedCircle cx={svgSize / 2} cy={svgSize / 2} r={R} strokeWidth={0} animatedProps={animatedFill} /> </Svg> )}

tastydev commented 5 months ago

I think i'm facing this same issue with a circle-fill animation, I'm using react-native-reanimated@3.5.4 and react-native-svg@13.14.0 my code is something like this: const animatedFill = useAnimatedProps(() => { return { fill: interpolateColor( color.value, [ UPLOAD_STATE.IDLE, UPLOAD_STATE.INITIALIZE, UPLOAD_STATE.UPLOADING, UPLOAD_STATE.SUCCESS, UPLOAD_STATE.ERROR, ], [mainColor, "#EEE683", "#D4A418", happyColor, deleteBtnColor] ), }; }); return ( <Svg height={svgSize} width={svgSize} style={{ position: "relative", shadowColor: mainColor, shadowOffset: { width: 0, height: 3, }, shadowOpacity: 0.17, shadowRadius: 3.05, elevation: 4, }}> <AnimatedCircle cx={svgSize / 2} cy={svgSize / 2} r={R} strokeWidth={0} animatedProps={animatedFill} /> </Svg> )}

I managed to fix my issue like this, maybe it will help you in your case:

I had to pass every value natively thanks to createAnimatedPropAdapter to avoid cast/parse issues

import Animated, {
  createAnimatedPropAdapter,
  useAnimatedProps,
  useSharedValue,
  processColor
} from "react-native-reanimated";
import Svg, { Circle, Path } from "react-native-svg"
import { mixColor } from "react-native-redash";

const AnimatedHeart = Animated.createAnimatedComponent(Path);
const heartColorTransition = useSharedValue(0);

const animatedHeartProps = useAnimatedProps(
    () => {
      return {
        stroke: mixColor(heartColorTransition.value, "#131338", "#D9414E"),
        fill: mixColor(heartColorTransition.value, "transparent", "#D9414E"),
      }
    },
    [],
    createAnimatedPropAdapter(
      (props) => {
        if ("fill" in props) {
          // eslint-disable-next-line react/prop-types
          props.fill = { type: 0, payload: processColor(props.fill as string) }
        }
        if ("stroke" in props) {
          // eslint-disable-next-line react/prop-types
          props.stroke = { type: 0, payload: processColor(props.stroke as string) }
        }
      },
      ["fill", "stroke"]
    )
  );

<Svg width={24} height={24} viewBox="0 0 24 24">
            <AnimatedHeart
              d="M7.018 3.007a4.81 4.81 0 012.724.682 5.182 5.182 0 011.942 2.125c.133.27.499.27.633 0a5.166 5.166 0 011.942-2.125 4.804 4.804 0 012.723-.682c1.387.075 2.741.799 3.654 1.911C21.55 6.034 22 7.522 22 9.027c0 1.301-.448 2.58-1.08 3.732-.632 1.155-1.474 2.185-2.387 3.15A27.615 27.615 0 0112 21a27.612 27.612 0 01-6.533-5.091c-.913-.965-1.755-1.994-2.387-3.15C2.45 11.609 2 10.33 2 9.028c0-1.505.45-2.994 1.365-4.109.912-1.111 2.266-1.836 3.653-1.911z"
              strokeLinecap="round"
              strokeLinejoin="round"
              strokeWidth={1.5}
              animatedProps={animatedHeartProps}
            />
</Svg>