Skip to content

Commit

Permalink
Do not send state updates for loop animations
Browse files Browse the repository at this point in the history
Summary:
**Motivation**

There is a special [code path](https://github.com/facebook/react-native/blob/bb23026daf1a853f4482be46d6f242712a6b7330/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/FrameBasedAnimationDriver.java#L98) for native driven looping animations that restarts the animation in native, never calling the "animation end" callback. This was designed for and works fine with the `Animated.loop(Animated.timing(...))` animations. Unfortunately, it doesn't work well with more complex animations.
Consider an `Animated.loop(Animated.sequence(Animated.timing(...)))` animation. It doesn't trigger this code path. Instead, its nested `Animated.timing` animations are continuously rescheduled from JS by the looping `Animated.sequence` as singular native driven animations. Each time they end, they fire their "animation end" callback.

This introduces a subtle breakage when those "animation end" callbacks trigger React to update its state. This in turn restarts the rendering. In case with long transitions such looping animations can restart the rendering multiple times. In worst cases it may render the app unresponsive.

**Solution**
We don't need to tell React to update its state when running looping animations. This diff introduces a mechanism, using which `Animated.loop` can tell its nested animations that they are in a loop, and and there's no need to send state updates when they finish. This is consistent with how `Animated.loop(Animated.timing(...))` behaves.

Changelog: [General][Breaking] - Looping animations will not send React state updates.

Facebook

This diff enables this new behaviour for IGVR and FBVR, it also set up the experiment for FBiOS and FB4a.

Reviewed By: javache

Differential Revision: D59970265
  • Loading branch information
dmytrorykun authored and facebook-github-bot committed Jul 25, 2024
1 parent eae2240 commit 53bc4d9
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import AnimatedValue from './nodes/AnimatedValue';
import AnimatedValueXY from './nodes/AnimatedValueXY';

export type CompositeAnimation = {
start: (callback?: ?EndCallback) => void,
start: (callback?: ?EndCallback, isLooping?: boolean) => void,
stop: () => void,
reset: () => void,
_startNativeLoop: (iterations?: number) => void,
Expand Down Expand Up @@ -234,8 +234,8 @@ const timing = function (

return (
maybeVectorAnim(value, config, timing) || {
start: function (callback?: ?EndCallback): void {
start(value, config, callback);
start: function (callback?: ?EndCallback, isLooping?: boolean): void {
start(value, {...config, isLooping}, callback);
},

stop: function (): void {
Expand Down Expand Up @@ -305,7 +305,7 @@ const sequence = function (
): CompositeAnimation {
let current = 0;
return {
start: function (callback?: ?EndCallback) {
start: function (callback?: ?EndCallback, isLooping?: boolean) {
const onComplete = function (result: EndResult) {
if (!result.finished) {
callback && callback(result);
Expand All @@ -321,13 +321,13 @@ const sequence = function (
return;
}

animations[current].start(onComplete);
animations[current].start(onComplete, isLooping);
};

if (animations.length === 0) {
callback && callback({finished: true});
} else {
animations[current].start(onComplete);
animations[current].start(onComplete, isLooping);
}
},

Expand Down Expand Up @@ -477,7 +477,7 @@ const loop = function (
} else {
iterationsSoFar++;
resetBeforeIteration && animation.reset();
animation.start(restart);
animation.start(restart, iterations === -1);
}
};
if (!animation || iterations === 0) {
Expand Down
10 changes: 10 additions & 0 deletions packages/react-native/Libraries/Animated/animations/Animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {PlatformConfig} from '../AnimatedPlatformConfig';
import type AnimatedNode from '../nodes/AnimatedNode';
import type AnimatedValue from '../nodes/AnimatedValue';

import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags';
import NativeAnimatedHelper from '../NativeAnimatedHelper';
import AnimatedProps from '../nodes/AnimatedProps';

Expand All @@ -26,6 +27,7 @@ export type AnimationConfig = {
platformConfig?: PlatformConfig,
onComplete?: ?EndCallback,
iterations?: number,
isLooping?: boolean,
};

let startNativeAnimationNextId = 1;
Expand All @@ -38,6 +40,7 @@ export default class Animation {
__isInteraction: boolean;
__onEnd: ?EndCallback;
__iterations: number;
__isLooping: ?boolean;

_nativeId: number;

Expand Down Expand Up @@ -107,6 +110,13 @@ export default class Animation {
if (value != null) {
animatedValue.__onAnimatedValueUpdateReceived(value);

if (
ReactNativeFeatureFlags.shouldSkipStateUpdatesForLoopingAnimations() &&
this.__isLooping
) {
return;
}

// Once the JS side node is synced with the updated values, trigger an
// update on the AnimatedProps nodes to call any registered callbacks.
this.__findAnimatedPropsNodes(animatedValue).forEach(node =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export default class TimingAnimation extends Animation {
this._useNativeDriver = NativeAnimatedHelper.shouldUseNativeDriver(config);
this._platformConfig = config.platformConfig;
this.__isInteraction = config.isInteraction ?? !this._useNativeDriver;
this.__isLooping = config.isLooping;
}

__getNativeAnimationConfig(): any {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ declare export class AnimatedEvent {

exports[`public API should not change unintentionally Libraries/Animated/AnimatedImplementation.js 1`] = `
"export type CompositeAnimation = {
start: (callback?: ?EndCallback) => void,
start: (callback?: ?EndCallback, isLooping?: boolean) => void,
stop: () => void,
reset: () => void,
_startNativeLoop: (iterations?: number) => void,
Expand Down Expand Up @@ -521,12 +521,14 @@ export type AnimationConfig = {
platformConfig?: PlatformConfig,
onComplete?: ?EndCallback,
iterations?: number,
isLooping?: boolean,
};
declare export default class Animation {
__active: boolean;
__isInteraction: boolean;
__onEnd: ?EndCallback;
__iterations: number;
__isLooping: ?boolean;
_nativeId: number;
start(
fromValue: number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ const definitions: FeatureFlagDefinitions = {
description:
'Function used to enable / disabled Layout Animations in React Native.',
},
shouldSkipStateUpdatesForLoopingAnimations: {
defaultValue: false,
description:
'If the animation is within Animated.loop, we do not send state updates to React.',
},
shouldUseAnimatedObjectForTransform: {
defaultValue: false,
description:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<f734c5d2afd065af224c5051ac3e737f>>
* @generated SignedSource<<b914ca36b2ab729f1262384d83b684b7>>
* @flow strict-local
*/

Expand All @@ -31,6 +31,7 @@ export type ReactNativeFeatureFlagsJsOnly = {
animatedShouldUseSingleOp: Getter<boolean>,
enableAccessToHostTreeInFabric: Getter<boolean>,
isLayoutAnimationEnabled: Getter<boolean>,
shouldSkipStateUpdatesForLoopingAnimations: Getter<boolean>,
shouldUseAnimatedObjectForTransform: Getter<boolean>,
shouldUseRemoveClippedSubviewsAsDefaultOnIOS: Getter<boolean>,
shouldUseSetNativePropsInFabric: Getter<boolean>,
Expand Down Expand Up @@ -107,6 +108,11 @@ export const enableAccessToHostTreeInFabric: Getter<boolean> = createJavaScriptF
*/
export const isLayoutAnimationEnabled: Getter<boolean> = createJavaScriptFlagGetter('isLayoutAnimationEnabled', true);

/**
* If the animation is within Animated.loop, we do not send state updates to React.
*/
export const shouldSkipStateUpdatesForLoopingAnimations: Getter<boolean> = createJavaScriptFlagGetter('shouldSkipStateUpdatesForLoopingAnimations', false);

/**
* Enables use of AnimatedObject for animating transform values.
*/
Expand Down

0 comments on commit 53bc4d9

Please sign in to comment.