From 4b467887dd650c4b6ee86d0c3e0433e67c83027e Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 18 Oct 2023 10:56:41 -0700 Subject: [PATCH] Attempt to batch animation completions (#41052) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/41052 When we animate an Animated.Value that has multiple components (eg Animated.Color, AnimatedXY), we have no guarantee that the animation callbacks will be processed in a single React commit. Previously, we attempted to work around this by ignoring these updates in `__findAnimatedPropsNodes`, but that leads to other issues. Instead, force all animation completions that happen in a single frame to be processed together by emitting them as a single event (Note: this only works when using the singleOpBatching flag for Animated which hasn't been rolled out) Changelog: [Internal] Reviewed By: rubennorte Differential Revision: D50366676 fbshipit-source-id: 613920056113b6515792e80e06254b92061bc335 --- .../Animated/NativeAnimatedHelper.js | 23 +++++++------ .../Animated/animations/Animation.js | 25 ++++---------- .../animated/NativeAnimatedNodesManager.java | 33 +++++++++++++++---- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/packages/react-native/Libraries/Animated/NativeAnimatedHelper.js b/packages/react-native/Libraries/Animated/NativeAnimatedHelper.js index a96287de26ddd8..22126703d2b280 100644 --- a/packages/react-native/Libraries/Animated/NativeAnimatedHelper.js +++ b/packages/react-native/Libraries/Animated/NativeAnimatedHelper.js @@ -48,10 +48,10 @@ const useSingleOpBatching = let flushQueueTimeout = null; const eventListenerGetValueCallbacks: { - [$FlowFixMe | number]: ((value: number) => void) | void, + [number]: (value: number) => void, } = {}; const eventListenerAnimationFinishedCallbacks: { - [$FlowFixMe | number]: EndCallback | void, + [number]: EndCallback, } = {}; let globalEventEmitterGetValueListener: ?EventSubscription = null; let globalEventEmitterAnimationFinishedListener: ?EventSubscription = null; @@ -339,7 +339,7 @@ const API = { function setupGlobalEventEmitterListeners() { globalEventEmitterGetValueListener = RCTDeviceEventEmitter.addListener( 'onNativeAnimatedModuleGetValue', - function (params) { + params => { const {tag} = params; const callback = eventListenerGetValueCallbacks[tag]; if (!callback) { @@ -352,14 +352,17 @@ function setupGlobalEventEmitterListeners() { globalEventEmitterAnimationFinishedListener = RCTDeviceEventEmitter.addListener( 'onNativeAnimatedModuleAnimationFinished', - function (params) { - const {animationId} = params; - const callback = eventListenerAnimationFinishedCallbacks[animationId]; - if (!callback) { - return; + params => { + // TODO: remove Array.isArray once native changes have propagated + const animations = Array.isArray(params) ? params : [params]; + for (const animation of animations) { + const {animationId} = animation; + const callback = eventListenerAnimationFinishedCallbacks[animationId]; + if (callback) { + callback(animation); + delete eventListenerAnimationFinishedCallbacks[animationId]; + } } - callback(params); - delete eventListenerAnimationFinishedCallbacks[animationId]; }, ); } diff --git a/packages/react-native/Libraries/Animated/animations/Animation.js b/packages/react-native/Libraries/Animated/animations/Animation.js index d8e955f9fda49b..635055ce402d76 100644 --- a/packages/react-native/Libraries/Animated/animations/Animation.js +++ b/packages/react-native/Libraries/Animated/animations/Animation.js @@ -39,10 +39,11 @@ let startNativeAnimationNextId = 1; export default class Animation { __active: boolean; __isInteraction: boolean; - __nativeId: number; __onEnd: ?EndCallback; __iterations: number; + _nativeId: number; + start( fromValue: number, onUpdate: (value: number) => void, @@ -52,8 +53,8 @@ export default class Animation { ): void {} stop(): void { - if (this.__nativeId) { - NativeAnimatedHelper.API.stopAnimation(this.__nativeId); + if (this._nativeId) { + NativeAnimatedHelper.API.stopAnimation(this._nativeId); } } @@ -78,20 +79,6 @@ export default class Animation { return result; } - // Vectorized animations (animations on AnimatedValueXY, AnimatedColor nodes) - // are split into multiple animations for each component that execute in parallel. - // Calling update() on AnimatedProps when each animation completes results in - // potential flickering as all animations that are part of the vectorized animation - // may not have completed yet. For example, only the animation for the red channel of - // an animating color may have been completed, resulting in a temporary red color - // being rendered. So, for now, ignore AnimatedProps that use a vectorized animation. - if ( - Platform.OS === 'ios' && - (node instanceof AnimatedValueXY || node instanceof AnimatedColor) - ) { - return result; - } - for (const child of node.__getChildren()) { result.push(...this.__findAnimatedPropsNodes(child)); } @@ -108,9 +95,9 @@ export default class Animation { try { const config = this.__getNativeAnimationConfig(); animatedValue.__makeNative(config.platformConfig); - this.__nativeId = NativeAnimatedHelper.generateNewAnimationId(); + this._nativeId = NativeAnimatedHelper.generateNewAnimationId(); NativeAnimatedHelper.API.startAnimatingNode( - this.__nativeId, + this._nativeId, animatedValue.__getNativeTag(), config, result => { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index 79805825767cb2..efe2872b9c599a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -22,6 +22,7 @@ import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.UIManager; import com.facebook.react.bridge.UiThreadUtil; +import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; import com.facebook.react.uimanager.UIManagerHelper; import com.facebook.react.uimanager.common.UIManagerType; @@ -295,6 +296,7 @@ private void stopAnimationsForNode(AnimatedNode animatedNode) { // same time. Therefore it does not make much sense to create an animationId -> animation // object map that would require additional memory just to support the use-case of stopping // an animation + WritableArray events = null; for (int i = 0; i < mActiveAnimations.size(); i++) { AnimationDriver animation = mActiveAnimations.valueAt(i); if (animatedNode.equals(animation.mAnimatedValue)) { @@ -312,13 +314,18 @@ private void stopAnimationsForNode(AnimatedNode animatedNode) { params.putInt("animationId", animation.mId); params.putBoolean("finished", false); params.putDouble("value", animation.mAnimatedValue.mValue); - mReactApplicationContext.emitDeviceEvent( - "onNativeAnimatedModuleAnimationFinished", params); + if (events == null) { + events = Arguments.createArray(); + } + events.pushMap(params); } mActiveAnimations.removeAt(i); i--; } } + if (events != null) { + mReactApplicationContext.emitDeviceEvent("onNativeAnimatedModuleAnimationFinished", events); + } } @UiThread @@ -327,6 +334,7 @@ public void stopAnimation(int animationId) { // same time. Therefore it does not make much sense to create an animationId -> animation // object map that would require additional memory just to support the use-case of stopping // an animation + WritableArray events = null; for (int i = 0; i < mActiveAnimations.size(); i++) { AnimationDriver animation = mActiveAnimations.valueAt(i); if (animation.mId == animationId) { @@ -344,13 +352,18 @@ public void stopAnimation(int animationId) { params.putInt("animationId", animation.mId); params.putBoolean("finished", false); params.putDouble("value", animation.mAnimatedValue.mValue); - mReactApplicationContext.emitDeviceEvent( - "onNativeAnimatedModuleAnimationFinished", params); + if (events == null) { + events = Arguments.createArray(); + } + events.pushMap(params); } mActiveAnimations.removeAt(i); - return; + break; } } + if (events != null) { + mReactApplicationContext.emitDeviceEvent("onNativeAnimatedModuleAnimationFinished", events); + } // Do not throw an error in the case animation could not be found. We only keep "active" // animations in the registry and there is a chance that Animated.js will enqueue a // stopAnimation call after the animation has ended or the call will reach native thread only @@ -643,6 +656,7 @@ public void runUpdates(long frameTimeNanos) { // Cleanup finished animations. Iterate over the array of animations and override ones that has // finished, then resize `mActiveAnimations`. if (hasFinishedAnimations) { + WritableArray events = null; for (int i = mActiveAnimations.size() - 1; i >= 0; i--) { AnimationDriver animation = mActiveAnimations.valueAt(i); if (animation.mHasFinished) { @@ -659,12 +673,17 @@ public void runUpdates(long frameTimeNanos) { params.putInt("animationId", animation.mId); params.putBoolean("finished", true); params.putDouble("value", animation.mAnimatedValue.mValue); - mReactApplicationContext.emitDeviceEvent( - "onNativeAnimatedModuleAnimationFinished", params); + if (events == null) { + events = Arguments.createArray(); + } + events.pushMap(params); } mActiveAnimations.removeAt(i); } } + if (events != null) { + mReactApplicationContext.emitDeviceEvent("onNativeAnimatedModuleAnimationFinished", events); + } } }