Do not make a synchronous call when canceling animation.

[Szymon20000]

Summary

I noticed in client's code that cancelAnimation called for few values takes almost a second.
That seems to be caused by the fact that it's used on every useShareValue hook on unmount and across multiple libraries build on the top of reanimated. Looks like getter is synchronous and blocks the js until we schedule a block on UI.
I suspect the idea was to make sure we use the same type as we can animate different types. I thought it can be possible to just take initial value if it's set. Please let me know if for some reason we have to make a synchronous call either way.

Test plan

• Start animation and then cancel it with cancelAnimation function

[arasrezaei]

I noticed that on navigation backs, does that related to this?
BTW, What is the profiler name?

[Szymon20000]

@tomekzaw Can I ask you for a bit of help here? :)

[joe-sam]

It was interesting to note that an animation is cancelled by updating its corresponding SharedValue.

What happens to all the DerivedValue(s) dependent on the SharedValue(s) on CancelAnimation.

Does a batched call for cancellation with reverse tree dependency chain of responsibility of the derived animations automatically fire ?

[Amurmurmur]

@tomekzaw Please please please can we get this merged

[tomekzaw]

Hey @Amurmurmur, right now we're in the process of releasing Reanimated 3.16.0 with support for React Native 0.76.

Once that's complete, we'll review any PRs that have been submitted in the meantime as our capacity is pretty limited.

I'd love to have this problem resolved but this PR is a significant change and we'll need to test it thoroughly prior to merging.

[tjzel]

This is a difficult problem, let me explain why:

There might be a situation, where:

1. cancelAnimation is called on React runtime
2. Slightly later another animation is started on UI runtime

This is fine with current implementation. cancelAnimation will synchronously stop the current one without interfering with the new animation. However, with your change we essentialy turn cancelAnimation to an asynchronous call.

Therefore, there might be a situation where another animation has started before previous got cancelled, resulting in the cancellation affecting the new animation. That's what I'm afraid of. Here's a video where I show this problem:

scary.mov

Reproduction snippet

import { StyleSheet, View, Button, Text } from 'react-native';

import React from 'react';

import Animated, {
useSharedValue,
useAnimatedStyle,
withTiming,
cancelAnimation,
runOnUI,
} from 'react-native-reanimated';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';

export default function EmptyExample() {
const rotation = useSharedValue(0);
const opacity = useSharedValue(1);

const animatedStyle = useAnimatedStyle(() => {
return {
transform: [{ rotate: rotation.value + 'rad' }],
};
});

const touchOpacity = useAnimatedStyle(() => {
return {
opacity: opacity.value,
};
});

const tap = Gesture.Tap()
.onBegin(() => {
opacity.value = 0.5;
rotation.value = withTiming(rotation.value + Math.PI, {
duration: 2000,
});
})
.onEnd(() => {
opacity.value = 1;
});

return (

<Button
title="Start from React"
onPress={() =>
runOnUI(() => {
rotation.value = withTiming(rotation.value + Math.PI, {
duration: 2000,
});
})()
}
/>
<Button
title="Stop from React"
onPress={() => {
// Let's pretend React/UI is busy:
setTimeout(() => cancelAnimation(rotation), 500);
}}
/>

<Animated.View style={touchOpacity}>
Start from UI
</Animated.View>

<Animated.View style={[animatedStyle, styles.box]} />

);
}

const styles = StyleSheet.create({
container: {
flex: 1,
alignItems: 'center',
justifyContent: 'center',
},
box: {
marginTop: 20,
width: 100,
height: 100,
backgroundColor: 'powderblue',
},
});

This behavior isn't necessarily bad, but we can't just make such change overnight - it should be well documented and we should provide an escape hatch for the edge case I presented.

I think a reasonable solution here would be to:

• Deprecate cancelAnimation.
• Add new stopAnimation function. stopAnimation would be async on React runtime and sync on UI runtime. It would be the default to replace stopAnimation.
• Add new stopAnimationSync function. It would be sync on both React and UI runtimes, therefore blocking. It would be the aforementioned escape hatch.

cc @tomekzaw @piaskowyk

[Szymon20000]

Thanks @tjzel for spending your time on it :)
We can just make runOnUI instead if you want to use the latest value to avoid setting the initial value. When I added it I assumed that people only use it on unmounting. Then we don't care much about the value.

runOnUI(() => {
'worklet' 
sv.value = sv.value;
})

I think the solution with sync and async version makes sense but to be honest I think the default one should be async. The main problem here is that we call cancelAnimation when useShareValue is being destroyed. In 99% of the cases we don't want to block the js thread. Imagine you have a component with 20 shared values (list with every item being animated). And every takes 20-100ms on unmount (in Release build).

[Szymon20000]

Actually with the current approach you can also cancel new animation instead of the previous one.

1. We synchronously read the old value
2. On UI thread we replace the animation we wanted to cancel with a new one
3. We schedule a async call to UI with setting a new value.

[tjzel]

@Szymon20000 you are right, I forgot we actually still have the write async in this scenario, since after the read the other runtime is unlocked.

[Szymon20000]

@Szymon20000 you are right, I forgot we actually still have the write async in this scenario, since after the read the other runtime is unlocked.

Those are complex stuff :) . So can we just wrap it in runOnUI if _WORKLET = false?

[joe-sam]

I don't know whether this is feasible to implement but perhaps the cancelAnimation actually needs to work through a PriorityQueue of Animations in a batched update. This may solve the asynchronous ordering problem, derived animations cancellations and mitigate the round robin blocking time.
In addition it should be also be quite possible to override the terminal value of the animation from the API on interrupt exception to either default to reset or finalize.

[Szymon20000]

I don't know whether this is feasible to implement but perhaps the cancelAnimation actually needs to work through a PriorityQueue of Animations in a batched update. This may solve the asynchronous ordering problem, derived animations cancellations and mitigate the round robin blocking time. In addition it should be also be quite possible to override the terminal value of the animation from the API on interrupt exception to either default to reset or finalize.

That's super rare problem which can be worked around and not sure if that's worth the overhead.

[tjzel]

@Szymon20000
Yes, synchronous call on a Worklet Runtime (_WORKLET === TRUE) and runOnUI on React Runtime should suffice.

There's another edge case where you can call cancelAnimation from another Worklet Runtime, not only UI one, but I don't even want to think about it... 🙈

[Szymon20000]

@tjzel I've just updated the pr to do exactly what we discussed.

[Amurmurmur]

@tjzel Can we please get this merged we really need this change by @Szymon20000 !
Thank you!

[tjzel]

@Szymon20000 I left only some minor corrections. Address them and we are good to merge. Please use yarn format in repo to fix CI.

I think this change is so important we should also port it back to 3.16, 3.15 and 3.14. Thoughts? @bartlomiejbloniarz @tomekzaw

[tjzel]

Sorry @Amurmurmur I was OOO for a week