Skip to content

Commit

Permalink
Attempt to batch animation completions (#41052)
Browse files Browse the repository at this point in the history 
Summary:
Pull Request resolved: #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
  • Loading branch information
@javache @facebook-github-bot
javache authored and facebook-github-bot committed Oct 18, 2023
1 parent 66fbab2 commit 4b46788
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 36 deletions.
23 changes: 13 additions & 10 deletions packages/react-native/Libraries/Animated/NativeAnimatedHelper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -339,7 +339,7 @@ const API = {
function setupGlobalEventEmitterListeners() {
globalEventEmitterGetValueListener = RCTDeviceEventEmitter.addListener(
'onNativeAnimatedModuleGetValue',
function (params) {
params => {
const {tag} = params;
const callback = eventListenerGetValueCallbacks[tag];
if (!callback) {
Expand All @@ -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];
},
);
}
Expand Down
25 changes: 6 additions & 19 deletions packages/react-native/Libraries/Animated/animations/Animation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
}

Expand All @@ -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));
}
Expand All @@ -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 => {
Expand Down
33 changes: 26 additions & 7 deletions ...ve/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
}

Expand Down

0 comments on commit 4b46788

Please sign in to comment.