From 61d3741461d61f11862044d38dcf5ebdde9ffc38 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Mon, 20 Feb 2017 05:49:32 -0800 Subject: [PATCH] Fix native animated event lag on Android Summary: Native animated events sometimes end up lagging a frame behind on android because we perform the update in the normal animation loop instead of doing it immediately when we receive the event. We had the same issue on iOS and was fixed in a similar way. Moved some code around to have a method that updates a list of node that we can use to update the node in the animated event handler and also use it in the animation update loop. **Test plan** Tested that it did fix sticky headers lagging a frame behind during momentum scrolling in my PR #11315 and also tested the native animations examples still work properly. Closes https://github.com/facebook/react-native/pull/11994 Reviewed By: mkonicek Differential Revision: D4488977 Pulled By: sahrens fbshipit-source-id: 831a1565bc7b8fa88cadd5a8c1be876fbdefbf66 --- .../animated/NativeAnimatedNodesManager.java | 101 +++++++++--------- 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index 0e53f59ef6e..12c7f640b95 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -29,6 +29,7 @@ import com.facebook.react.uimanager.events.EventDispatcherListener; import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -58,6 +59,8 @@ import javax.annotation.Nullable; private final Map> mCustomEventTypes; private final UIImplementation mUIImplementation; private int mAnimatedGraphBFSColor = 0; + // Used to avoid allocating a new array on every frame in runUpdates. + private final List mRunUpdateNodeList = new ArrayList<>(); public NativeAnimatedNodesManager(UIManagerModule uiManager) { mUIImplementation = uiManager.getUIImplementation(); @@ -334,7 +337,8 @@ import javax.annotation.Nullable; EventAnimationDriver eventDriver = mEventDrivers.get(event.getViewTag() + eventName); if (eventDriver != null) { event.dispatch(eventDriver); - mUpdatedNodes.put(eventDriver.mValueNode.mTag, eventDriver.mValueNode); + + updateNodes(Collections.singletonList((AnimatedNode) eventDriver.mValueNode)); } } } @@ -353,15 +357,52 @@ import javax.annotation.Nullable; */ public void runUpdates(long frameTimeNanos) { UiThreadUtil.assertOnUiThread(); - int activeNodesCount = 0; - int updatedNodesCount = 0; boolean hasFinishedAnimations = false; + for (int i = 0; i < mUpdatedNodes.size(); i++) { + AnimatedNode node = mUpdatedNodes.valueAt(i); + mRunUpdateNodeList.add(node); + } + + // Clean mUpdatedNodes queue + mUpdatedNodes.clear(); + + for (int i = 0; i < mActiveAnimations.size(); i++) { + AnimationDriver animation = mActiveAnimations.valueAt(i); + animation.runAnimationStep(frameTimeNanos); + AnimatedNode valueNode = animation.mAnimatedValue; + mRunUpdateNodeList.add(valueNode); + if (animation.mHasFinished) { + hasFinishedAnimations = true; + } + } + + updateNodes(mRunUpdateNodeList); + mRunUpdateNodeList.clear(); + + // Cleanup finished animations. Iterate over the array of animations and override ones that has + // finished, then resize `mActiveAnimations`. + if (hasFinishedAnimations) { + for (int i = mActiveAnimations.size() - 1; i >= 0; i--) { + AnimationDriver animation = mActiveAnimations.valueAt(i); + if (animation.mHasFinished) { + WritableMap endCallbackResponse = Arguments.createMap(); + endCallbackResponse.putBoolean("finished", true); + animation.mEndCallback.invoke(endCallbackResponse); + mActiveAnimations.removeAt(i); + } + } + } + } + + private void updateNodes(List nodes) { + int activeNodesCount = 0; + int updatedNodesCount = 0; + // STEP 1. - // BFS over graph of nodes starting from ones from `mUpdatedNodes` and ones that are attached to - // active animations (from `mActiveAnimations)`. Update `mIncomingNodes` attribute for each node - // during that BFS. Store number of visited nodes in `activeNodesCount`. We "execute" active - // animations as a part of this step. + // BFS over graph of nodes. Update `mIncomingNodes` attribute for each node during that BFS. + // Store number of visited nodes in `activeNodesCount`. We "execute" active animations as a part + // of this step. mAnimatedGraphBFSColor++; /* use new color */ if (mAnimatedGraphBFSColor == AnimatedNode.INITIAL_BFS_COLOR) { @@ -371,8 +412,7 @@ import javax.annotation.Nullable; } Queue nodesQueue = new ArrayDeque<>(); - for (int i = 0; i < mUpdatedNodes.size(); i++) { - AnimatedNode node = mUpdatedNodes.valueAt(i); + for (AnimatedNode node : nodes) { if (node.mBFSColor != mAnimatedGraphBFSColor) { node.mBFSColor = mAnimatedGraphBFSColor; activeNodesCount++; @@ -380,20 +420,6 @@ import javax.annotation.Nullable; } } - for (int i = 0; i < mActiveAnimations.size(); i++) { - AnimationDriver animation = mActiveAnimations.valueAt(i); - animation.runAnimationStep(frameTimeNanos); - AnimatedNode valueNode = animation.mAnimatedValue; - if (valueNode.mBFSColor != mAnimatedGraphBFSColor) { - valueNode.mBFSColor = mAnimatedGraphBFSColor; - activeNodesCount++; - nodesQueue.add(valueNode); - } - if (animation.mHasFinished) { - hasFinishedAnimations = true; - } - } - while (!nodesQueue.isEmpty()) { AnimatedNode nextNode = nodesQueue.poll(); if (nextNode.mChildren != null) { @@ -425,23 +451,13 @@ import javax.annotation.Nullable; // find nodes with zero "incoming nodes", those can be either nodes from `mUpdatedNodes` or // ones connected to active animations - for (int i = 0; i < mUpdatedNodes.size(); i++) { - AnimatedNode node = mUpdatedNodes.valueAt(i); + for (AnimatedNode node : nodes) { if (node.mActiveIncomingNodes == 0 && node.mBFSColor != mAnimatedGraphBFSColor) { node.mBFSColor = mAnimatedGraphBFSColor; updatedNodesCount++; nodesQueue.add(node); } } - for (int i = 0; i < mActiveAnimations.size(); i++) { - AnimationDriver animation = mActiveAnimations.valueAt(i); - AnimatedNode valueNode = animation.mAnimatedValue; - if (valueNode.mActiveIncomingNodes == 0 && valueNode.mBFSColor != mAnimatedGraphBFSColor) { - valueNode.mBFSColor = mAnimatedGraphBFSColor; - updatedNodesCount++; - nodesQueue.add(valueNode); - } - } // Run main "update" loop while (!nodesQueue.isEmpty()) { @@ -486,22 +502,5 @@ import javax.annotation.Nullable; throw new IllegalStateException("Looks like animated nodes graph has cycles, there are " + activeNodesCount + " but toposort visited only " + updatedNodesCount); } - - // Clean mUpdatedNodes queue - mUpdatedNodes.clear(); - - // Cleanup finished animations. Iterate over the array of animations and override ones that has - // finished, then resize `mActiveAnimations`. - if (hasFinishedAnimations) { - for (int i = mActiveAnimations.size() - 1; i >= 0; i--) { - AnimationDriver animation = mActiveAnimations.valueAt(i); - if (animation.mHasFinished) { - WritableMap endCallbackResponse = Arguments.createMap(); - endCallbackResponse.putBoolean("finished", true); - animation.mEndCallback.invoke(endCallbackResponse); - mActiveAnimations.removeAt(i); - } - } - } } }