Skip to content

Commit

Permalink
Fix native animated event lag on Android
Browse files Browse the repository at this point in the history
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 #11994

Reviewed By: mkonicek

Differential Revision: D4488977

Pulled By: sahrens

fbshipit-source-id: 831a1565bc7b8fa88cadd5a8c1be876fbdefbf66
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Feb 20, 2017
1 parent bdd27f4 commit 61d3741
Showing 1 changed file with 49 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -58,6 +59,8 @@
private final Map<String, Map<String, String>> mCustomEventTypes;
private final UIImplementation mUIImplementation;
private int mAnimatedGraphBFSColor = 0;
// Used to avoid allocating a new array on every frame in runUpdates.
private final List<AnimatedNode> mRunUpdateNodeList = new ArrayList<>();

public NativeAnimatedNodesManager(UIManagerModule uiManager) {
mUIImplementation = uiManager.getUIImplementation();
Expand Down Expand Up @@ -334,7 +337,8 @@ public void onEventDispatch(Event event) {
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));
}
}
}
Expand All @@ -353,15 +357,52 @@ public void onEventDispatch(Event event) {
*/
public void runUpdates(long frameTimeNanos) {
UiThreadUtil.assertOnUiThread();
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<AnimatedNode> nodes) {
int activeNodesCount = 0;
int updatedNodesCount = 0;
boolean hasFinishedAnimations = false;

// 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) {
Expand All @@ -371,29 +412,14 @@ public void runUpdates(long frameTimeNanos) {
}

Queue<AnimatedNode> 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++;
nodesQueue.add(node);
}
}

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) {
Expand Down Expand Up @@ -425,23 +451,13 @@ public void runUpdates(long frameTimeNanos) {

// 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()) {
Expand Down Expand Up @@ -486,22 +502,5 @@ public void runUpdates(long frameTimeNanos) {
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);
}
}
}
}
}

0 comments on commit 61d3741

Please sign in to comment.