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

Animations Broken After Upgrading To 0.1.146

Open enchorb opened this issue 1 year ago • 7 comments

Description

Head programmed a graph component for my app following along with the Wallet example. After upgrading to 0.1.146 from 0.1.142 the animations no longer seem to work (https://shopify.github.io/react-native-skia/docs/animations/animations/) for instance isActive triggering a change in opacity as so const opacity = useTiming(isActive.current ? 1 : 0, { duration: 250 }) is no longer working.

Occasionally the app will completely crash with the error ERROR The above error occurred in the <skDrawing> component. My code was working 100% fine prior to the upgrade so wondering what has changed to cause these issues?

Version

0.1.146

Steps to reproduce

Use the Wallet example, use isActive and useTiming to change the opacity of the cursor

Snack, code example, screenshot, or link to a repository

N/A

enchorb avatar Sep 11 '22 13:09 enchorb

Can you provide a small code snippet with the issue?

wcandillon avatar Sep 11 '22 13:09 wcandillon

In my main component I have specified isActive which is set based on the custom touch handler hook.

const isActive = useValue(false);

const onTouch = useGraphTouchHandler({
      snapTo: true,
      width,
      cursor: {
        x,
        y
      },
      onGraph: (on) => {
        isActive.current = on;
        onGraph?.(on);
      }
    });

I then pass isActive into my cursor component to change the opacity of the cursor when the user is interacting with the graph

const opacity = useTiming(isActive.current ? 1 : 0, { duration: 250 });

This flow was working fine in v0.1.142 but no changes to isActive are not animating and will occasionally crash the app. I have also posted my touch handler below for reference:

export interface GraphTouchHandlerProps {
  snapTo?: boolean;
  width: number;
  cursor: {
    x: SkiaMutableValue<number>;
    y: SkiaValue<number>;
  };
  onGraph?: (on: boolean) => void;
}

export const useGraphTouchHandler = ({
  snapTo = true,
  width,
  cursor,
  onGraph
}: GraphTouchHandlerProps) => {
  const offsetX = useValue(0);
  const isActive = useValue(false);

  const interpolatePoint = (x: number, offset: number) =>
    (cursor.x.current = clamp(x + offset, 0, width));

  return useTouchHandler({
    onStart: ({ x }) => {
      isActive.current = true;

      offsetX.current = cursor.x.current - x;
      interpolatePoint(x, snapTo ? 0 : offsetX.current);

      Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Medium);
      onGraph(true);
    },
    onActive: ({ x }) => {
      if (!isActive.current) return;
      interpolatePoint(x, snapTo ? 0 : offsetX.current);
    },
    onEnd: ({ velocityX }) => {
      if (!isActive.current) return;

      if (!snapTo)
        runDecay(cursor.x, {
          velocity: velocityX,
          clamp: [0, width]
        });

      isActive.current = false;
      onGraph(false);
    }
  });
};

enchorb avatar Sep 11 '22 14:09 enchorb

After rolling back to RN 0.69.5 (since only 0.1.146 works with 0.70) & Skia 0.1.143 my application is working again. After further testing any Skia version 0.1.145 and up causes issues.

enchorb avatar Sep 11 '22 16:09 enchorb

Hi @enchorb - This is definitely something we'd like to look into - could you create a simple example on Github (a full project reproducing the error) so that it is possible for us reproduce and investigate?

chrfalch avatar Sep 12 '22 06:09 chrfalch

Hi @enchorb - did you get the opportunity to create a reproduction?

chrfalch avatar Sep 16 '22 07:09 chrfalch

@chrfalch @wcandillon Created the repository with reproducible example here: https://github.com/enchorb/RNTest

Works on versions 0.1.143 and below, otherwise the error occurs

On one terminal window yarn:start will start the metro server and on another yarn start:ios will build the app to the emulator

enchorb avatar Sep 17 '22 16:09 enchorb

Hi @enchorb - I've now installed the project, but it is a bit too complex for us to debug and troubleshoot - both in terms of code but also when it comes to dependencies - it is not possible within a reasonable timeframe to get this to work in a way where I can debug the Skia related code.

What we need is a simple new React Native app with preferably no other dependencies other than Skia and react-native-gesture-handler showing the problem. The best way to do this is either to create a new Snack or to create a new React Native app and add a simple example to the App.tsx file.

chrfalch avatar Sep 19 '22 07:09 chrfalch

Closing as I found the issue (finally had time to look at this again lol)

For the snippets below isActive is a boolean useValue()

On versions 0.1.143 this used to work

  const scale = useSpring(isActive.current ? 1 : 0, {
    damping: isActive.current ? 5 : 100
  });
  const transformScale = useComputedValue(() => [{ scale: scale?.current ?? 0 }], [scale]);

 const opacity = useTiming(isActive.current ? 1 : 0, { duration: 250 });

However on versions above (ie. 0.1.157) it will either not work or in some cases cause an animation undefined error as mentions above. The fix is to do the animation inside a useComputedValue instead and using imperative animations.

  const scale = useValue(0);
  const opacity = useValue(0);
  useComputedValue(
    () => {
      runSpring(scale, isActive.current ? 1 : 0, {
        damping: isActive.current ? 5 : 100
      });

      runTiming(opacity, isActive.current ? 1 : 0, { duration: 250 })
    },
    [isActive]
  );

  const transformScale = useComputedValue(() => [{ scale: scale?.current ?? 0 }], [scale]);

@chrfalch / @william-candillon Just wanted to confirm this is the correct way to do this now?

enchorb avatar Nov 14 '22 02:11 enchorb