diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index 249bf377589..dad43b29d79 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -88,16 +88,6 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec private abstract class UIThreadOperation { abstract void execute(NativeAnimatedNodesManager animatedNodesManager); - - long mFrameNumber = -1; - - public void setFrameNumber(long frameNumber) { - mFrameNumber = frameNumber; - } - - public long getFrameNumber() { - return mFrameNumber; - } } @NonNull private final GuardedFrameCallback mAnimatedFrameCallback; @@ -113,8 +103,6 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec private @Nullable NativeAnimatedNodesManager mNodesManager; - private volatile long mFrameNumber = 0; - private long mDispatchedFrameNumber = 0; private boolean mInitializedForFabric = false; private boolean mInitializedForNonFabric = false; private @UIManagerType int mUIManagerType = UIManagerType.DEFAULT; @@ -169,25 +157,13 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec } private void addOperation(UIThreadOperation operation) { - operation.setFrameNumber(mFrameNumber); mOperations.add(operation); } private void addPreOperation(UIThreadOperation operation) { - operation.setFrameNumber(mFrameNumber); mPreOperations.add(operation); } - // For FabricUIManager only - @Override - public void didScheduleMountItems(UIManager uiManager) { - if (mUIManagerType != UIManagerType.FABRIC) { - return; - } - - mFrameNumber++; - } - // For FabricUIManager only @Override @UiThread @@ -196,22 +172,6 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec return; } - // The problem we're trying to solve here: we could be in the middle of queueing - // a batch of related animation operations when Fabric flushes a batch of MountItems. - // It's visually bad if we execute half of the animation ops and then wait another frame - // (or more) to execute the rest. - // See mFrameNumber. If the dispatchedFrameNumber drifts too far - that - // is, if no MountItems are scheduled for a while, which can happen if a tree - // is committed but there are no changes - bring these counts back in sync and - // execute any queued operations. This number is arbitrary, but we want it low - // enough that the user shouldn't be able to see this delay in most cases. - mDispatchedFrameNumber++; - long currentFrameNo = mFrameNumber - 1; - if ((mDispatchedFrameNumber - mFrameNumber) > 2) { - mFrameNumber = mDispatchedFrameNumber; - currentFrameNo = mFrameNumber; - } - // This will execute all operations and preOperations queued // since the last time this was run, and will race with anything // being queued from the JS thread. That is, if the JS thread @@ -222,35 +182,13 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec // is that `scheduleMountItems` happens as close to the JS commit as // possible, whereas execution of those same items might happen sometime // later on the UI thread while the JS thread keeps plugging along. - executeAllOperations(mPreOperations, currentFrameNo); - executeAllOperations(mOperations, currentFrameNo); + executeAllOperations(mPreOperations); + executeAllOperations(mOperations); } - private void executeAllOperations(Queue operationQueue, long maxFrameNumber) { + private void executeAllOperations(Queue operationQueue) { NativeAnimatedNodesManager nodesManager = getNodesManager(); while (true) { - // There is a race condition where `peek` may return a non-null value and isEmpty() is false, - // but `poll` returns a null value - it's not clear why since we only peek and poll on the UI - // thread, but it might be something that happens during teardown or a crash. Regardless, the - // root cause is not currently known so we're extra cautious here. - // It happens equally in Fabric and non-Fabric. - UIThreadOperation peekedOperation = operationQueue.peek(); - - // This is the same as operationQueue.isEmpty() - if (peekedOperation == null) { - return; - } - // The rest of the operations are for the next frame. - if (peekedOperation.getFrameNumber() > maxFrameNumber) { - return; - } - - // Since we apparently can't guarantee that there is still an operation on the queue, - // much less the same operation, we do a poll and another null check. If this isn't - // the same operation as the peeked operation, we can't do anything about it - we still - // need to execute it, we have no mechanism to put it at the front of the queue, and it - // won't cause any errors to execute it earlier than expected (just a bit of UI jank at worst) - // so we just continue happily along. UIThreadOperation polledOperation = operationQueue.poll(); if (polledOperation == null) { return; @@ -270,13 +208,11 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec return; } - final long frameNo = mFrameNumber++; - UIBlock preOperationsUIBlock = new UIBlock() { @Override public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { - executeAllOperations(mPreOperations, frameNo); + executeAllOperations(mPreOperations); } }; @@ -284,7 +220,7 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec new UIBlock() { @Override public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { - executeAllOperations(mOperations, frameNo); + executeAllOperations(mOperations); } }; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManagerListener.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManagerListener.java index c190e0a8c02..d08ab69b62d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManagerListener.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/UIManagerListener.java @@ -16,6 +16,4 @@ public interface UIManagerListener { void willDispatchViewUpdates(UIManager uiManager); /* Called right after view updates are dispatched for a frame. */ void didDispatchMountItems(UIManager uiManager); - /* Called right after scheduleMountItems is called in Fabric, after a new tree is committed. */ - void didScheduleMountItems(UIManager uiManager); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 8538ce94c71..2cbaa95bf3e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -587,12 +587,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { boolean isBatchMountItem = mountItem instanceof BatchMountItem; boolean shouldSchedule = !(isBatchMountItem && ((BatchMountItem) mountItem).getSize() == 0); - // In case of sync rendering, this could be called on the UI thread. Otherwise, - // it should ~always be called on the JS thread. - for (UIManagerListener listener : mListeners) { - listener.didScheduleMountItems(this); - } - if (isBatchMountItem) { mCommitStartTime = commitStartTime; mLayoutTime = layoutEndTime - layoutStartTime;