From 8d03e63265e8564e4baf38c0c3d8ca75638563e0 Mon Sep 17 00:00:00 2001 From: Zeya Peng Date: Tue, 16 Sep 2025 03:39:45 -0700 Subject: [PATCH] Revert D82461457: Clean up batchingControlledByJS in NativeAnimated kotlin Differential Revision: D82461457 Original commit changeset: a1208720b83e Original Phabricator Diff: D82461457 fbshipit-source-id: 99416873f66c45ccf61614d3e356e6a777bbad6b --- .../react/animated/NativeAnimatedModule.kt | 43 +++++++++++-------- .../animated/NativeAnimatedNodesManager.kt | 4 ++ 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt index 9d9ce251430..6c5a505c921 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt @@ -196,9 +196,11 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) : private val nodesManagerRef = AtomicReference() - @Volatile private var currentFrameNumber: Long = 0 + private var batchingControlledByJS = false // TODO T71377544: delete - @Volatile private var currentBatchNumber: Long = 0 // frame number at last operations dispatch + @Volatile private var currentFrameNumber: Long = 0 // TODO T71377544: delete + + @Volatile private var currentBatchNumber: Long = 0 private var initializedForFabric = false private var initializedForNonFabric = false @@ -280,19 +282,22 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) : var batchNumber = currentBatchNumber - 1 - // 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. - currentFrameNumber++ - if ((currentFrameNumber - currentBatchNumber) > 2) { - currentBatchNumber = currentFrameNumber - batchNumber = currentBatchNumber + // TODO T71377544: delete this when the JS method is confirmed safe + if (!batchingControlledByJS) { + // 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. + currentFrameNumber++ + if ((currentFrameNumber - currentBatchNumber) > 2) { + currentBatchNumber = currentFrameNumber + batchNumber = currentBatchNumber + } } preOperations.executeBatch(batchNumber, nodesManager) @@ -479,14 +484,14 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) : } } - @Suppress("DEPRECATION") override fun startOperationBatch() { - // no-op + batchingControlledByJS = true + currentBatchNumber++ } - @Suppress("DEPRECATION") override fun finishOperationBatch() { - // no-op + batchingControlledByJS = false + currentBatchNumber++ } override fun createAnimatedNode(tagDouble: Double, config: ReadableMap) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt index 4641503ac4f..f7237025ab3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt @@ -774,8 +774,12 @@ public class NativeAnimatedNodesManager( ("Looks like animated nodes graph has ${reason}, there are $activeNodesCount but toposort visited only $updatedNodesCount") ) if (eventListenerInitializedForFabric && cyclesDetected == 0) { + // TODO T71377544: investigate these SoftExceptions and see if we can remove entirely + // or fix the root cause ReactSoftExceptionLogger.logSoftException(TAG, ReactNoCrashSoftException(ex)) } else if (eventListenerInitializedForFabric) { + // TODO T71377544: investigate these SoftExceptions and see if we can remove entirely + // or fix the root cause ReactSoftExceptionLogger.logSoftException(TAG, ReactNoCrashSoftException(ex)) } else { throw ex