react-native-reanimated icon indicating copy to clipboard operation
react-native-reanimated copied to clipboard

Using `interpolateColor` inside `useDerivedValue` creates colors glitches on iOS

Open swansontec opened this issue 1 year ago • 4 comments
trafficstars

Description

Returning an interpolateColor value from a useDerivedValue will cause incorrect colors to appear on iOS. Here is the setup:

const blue = '#2080c0';
const derivedColor = useDerivedValue(() =>
  interpolateColor(0, [0, 1], [blue, blue]),
);

This should always render as blue if we put it in an animated style:

const animatedStyle = useAnimatedStyle(() => ({
  color: derivedColor.value,
}));

However, on iOS specifically, the text will glitch to pink after the first button press:

  // Cause a re-render on press:
  const [count, setCount] = React.useState(0);
  const handleCount = () => setCount(count + 1);

  return (
    <>
      <Animated.Text style={animatedStyle}>
        {count}
      </Animated.Text>
      <Button onPress={handleCount} title="Count" />
    </>
  );

Steps to reproduce

Here is what seems to be happening:

  1. First render a. JS thread: useDerivedValue returns the string "rgba(32, 128, 192, 1)" b. JS thread: useAnimatedStyle returns the style { color: "rgba(32, 128, 192, 1)" } c. The text renders as blue d. UI thread: useDerivedValue runs again, and returns the integer 0xff2080c0. This seems to be some sort of optimization, since this differs from what the JS thread returns.
  2. User presses the button
  3. Second render a. JS thread: useDerivedValue does not run, but returns the saved 0xff2080c0 from the UI thread. b. JS thread: useAnimatedStyle returns the style { color: 0xff2080c0 } c. The text renders as pink

Directly rendering <Text style={{ color: 0xff2080c0 }}>Hello</Text> produces the same pink color (although TypeScript complains that numbers aren't valid colors). Messing around with the values, it seems that useAnimatedStyle on the JS thread interprets integers as 0xrrggbbaa, whereas the Reanimated UI thread interprets integers as 0xaarrggbb.

Snack or a link to a repository

https://github.com/swansontec/PinkEyeTest

Reanimated version

3.12.0

React Native version

0.74.2

Platforms

iOS

JavaScript runtime

Hermes

Workflow

React Native

Architecture

Paper (Old Architecture)

Build type

Any

Device

Any

Device model

No response

Acknowledgements

Yes

swansontec avatar Jun 13 '24 16:06 swansontec

As a hacky work-around (since I don't know Reanimated well enough to fix this myself), we are using patch-package to edit node_modules/react-native-reanimated/src/reanimated2/Colors.ts, around line 520:

-  if (IS_WEB || !_WORKLET) {
+  if (!IS_ANDROID || !_WORKLET) {
     return `rgba(${r}, ${g}, ${b}, ${alpha})`;
   }

This removes a performance optimization, but it does solve the bug.

swansontec avatar Jun 13 '24 17:06 swansontec

@swansontec Seems like a fully fledged bug but I have one question: why use interpolateColor inside of useDerivedValue, if it doesn't use any other shared value? (useDerivedValue is specifically used for creating shared values based on other shared values).

szydlovsky avatar Jun 14 '24 12:06 szydlovsky

@szydlovsky Yeah, this code doesn't make any sense because it's a minimal repo case. Animations aren't really necessary to trigger the bug.

In our actual app, we do use animations to control the derived value where we found the bug:

const iconColor = useDerivedValue(() => interpolateIconColor(focusAnimation, disableAnimation))

swansontec avatar Jun 17 '24 21:06 swansontec

Hey @swansontec I think I found a solution for you! It required some digging, but the performance optimisation you mentioned was indeed the culprit. It is there to speed up the way we pass colors on UI thread (hence _WORKLET check). Unfortunately, useDerivedValue runs some of its code on UI, which ends up giving us processed colors as numbers (meant for UI thread only). I found a kinda hacky workaround for you and in general I will discuss the topic with the team.

What I did is: made sure interpolation function runs on JS using runOnJS. This forces us to define the interpolation function on JS thread, but you mentioned that in your app it uses shared values as well, so I parametrised it. I also added a shared value that gets the return of mentioned interpolation and makes sure that useDerivedValue updates. Also added some small color change to test animations along with it. The changed code is here:

import * as React from 'react';
import { SafeAreaView, Button } from 'react-native';
import Animated, {
  interpolateColor,
  runOnJS,
  useAnimatedStyle,
  useDerivedValue,
  useSharedValue,
  withTiming,
} from 'react-native-reanimated';

const blue = '#2080c0';
const yellow = '#e0e63e';

export function RepoCase(): React.JSX.Element {
  // Shared values
  const changingValue = useSharedValue(0); // for animation
  const colorRef = useSharedValue(''); // for keeping the color

  // interpolate function defined on JS thread
  function interpolateFunc(someNumber: number) {
    colorRef.value = interpolateColor(someNumber, [0, 1], [yellow, blue]);
  }

  const derivedColor = useDerivedValue(() => {
    runOnJS(interpolateFunc)(changingValue.value);
    return colorRef.value;
  });

  const animatedStyle = useAnimatedStyle(() => {
    console.log(derivedColor.value);
    return {
      color: derivedColor.value,
      fontSize: 100,
      fontWeight: '700',
    };
  });

  // Re-render on press:
  const [count, setCount] = React.useState(0);
  const handleCount = () => setCount(count + 1);

  const toggleColors = () => {
    changingValue.value =
      changingValue.value > 0
        ? (changingValue.value = withTiming(0, { duration: 2000 }))
        : (changingValue.value = withTiming(1, { duration: 2000 }));
  };

  return (
    <SafeAreaView>
      <Animated.Text style={animatedStyle}>{count}</Animated.Text>
      <Button onPress={handleCount} title="Count" />
      <Button onPress={toggleColors} title="Toggle colors" />
    </SafeAreaView>
  );
}

function App(): React.JSX.Element {
  const [id, setId] = React.useState(0);
  const handleReset = () => setId(id + 1);

  return (
    <SafeAreaView>
      <RepoCase key={`id${id}`} />
      <Button onPress={handleReset} title="Reset" />
    </SafeAreaView>
  );
}

export default App;

Let me know what you think!

szydlovsky avatar Jul 01 '24 15:07 szydlovsky

Hey @swansontec turns out this "optmialisation" you mentioned was unneeded whatsoever haha. Once the PR goes through review I'll send you a patch to apply 👍

szydlovsky avatar Jul 04 '24 14:07 szydlovsky

Together with merge of the mentioned PR, the issue should be gone in the nearest nightly version (probably tomorrow morning) or in a next stable one - 3.14 or next iteration of 3.13.x

szydlovsky avatar Jul 05 '24 13:07 szydlovsky

We have updated to version 3.14.0, and everything works correctly! Thanks a bunch for getting this fix upstreamed.

swansontec avatar Jul 25 '24 22:07 swansontec