Skip to content

Commit 8d03e63

Browse files
Zeya Pengfacebook-github-bot
authored andcommitted
Revert D82461457: Clean up batchingControlledByJS in NativeAnimated kotlin
Differential Revision: D82461457 Original commit changeset: a1208720b83e Original Phabricator Diff: D82461457 fbshipit-source-id: 99416873f66c45ccf61614d3e356e6a777bbad6b
1 parent fae11d5 commit 8d03e63

File tree

2 files changed

+28
-19
lines changed

2 files changed

+28
-19
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,11 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) :
196196

197197
private val nodesManagerRef = AtomicReference<NativeAnimatedNodesManager?>()
198198

199-
@Volatile private var currentFrameNumber: Long = 0
199+
private var batchingControlledByJS = false // TODO T71377544: delete
200200

201-
@Volatile private var currentBatchNumber: Long = 0 // frame number at last operations dispatch
201+
@Volatile private var currentFrameNumber: Long = 0 // TODO T71377544: delete
202+
203+
@Volatile private var currentBatchNumber: Long = 0
202204

203205
private var initializedForFabric = false
204206
private var initializedForNonFabric = false
@@ -280,19 +282,22 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) :
280282

281283
var batchNumber = currentBatchNumber - 1
282284

283-
// The problem we're trying to solve here: we could be in the middle of queueing
284-
// a batch of related animation operations when Fabric flushes a batch of MountItems.
285-
// It's visually bad if we execute half of the animation ops and then wait another frame
286-
// (or more) to execute the rest.
287-
// See mFrameNumber. If the dispatchedFrameNumber drifts too far - that
288-
// is, if no MountItems are scheduled for a while, which can happen if a tree
289-
// is committed but there are no changes - bring these counts back in sync and
290-
// execute any queued operations. This number is arbitrary, but we want it low
291-
// enough that the user shouldn't be able to see this delay in most cases.
292-
currentFrameNumber++
293-
if ((currentFrameNumber - currentBatchNumber) > 2) {
294-
currentBatchNumber = currentFrameNumber
295-
batchNumber = currentBatchNumber
285+
// TODO T71377544: delete this when the JS method is confirmed safe
286+
if (!batchingControlledByJS) {
287+
// The problem we're trying to solve here: we could be in the middle of queueing
288+
// a batch of related animation operations when Fabric flushes a batch of MountItems.
289+
// It's visually bad if we execute half of the animation ops and then wait another frame
290+
// (or more) to execute the rest.
291+
// See mFrameNumber. If the dispatchedFrameNumber drifts too far - that
292+
// is, if no MountItems are scheduled for a while, which can happen if a tree
293+
// is committed but there are no changes - bring these counts back in sync and
294+
// execute any queued operations. This number is arbitrary, but we want it low
295+
// enough that the user shouldn't be able to see this delay in most cases.
296+
currentFrameNumber++
297+
if ((currentFrameNumber - currentBatchNumber) > 2) {
298+
currentBatchNumber = currentFrameNumber
299+
batchNumber = currentBatchNumber
300+
}
296301
}
297302

298303
preOperations.executeBatch(batchNumber, nodesManager)
@@ -479,14 +484,14 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) :
479484
}
480485
}
481486

482-
@Suppress("DEPRECATION")
483487
override fun startOperationBatch() {
484-
// no-op
488+
batchingControlledByJS = true
489+
currentBatchNumber++
485490
}
486491

487-
@Suppress("DEPRECATION")
488492
override fun finishOperationBatch() {
489-
// no-op
493+
batchingControlledByJS = false
494+
currentBatchNumber++
490495
}
491496

492497
override fun createAnimatedNode(tagDouble: Double, config: ReadableMap) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,8 +774,12 @@ public class NativeAnimatedNodesManager(
774774
("Looks like animated nodes graph has ${reason}, there are $activeNodesCount but toposort visited only $updatedNodesCount")
775775
)
776776
if (eventListenerInitializedForFabric && cyclesDetected == 0) {
777+
// TODO T71377544: investigate these SoftExceptions and see if we can remove entirely
778+
// or fix the root cause
777779
ReactSoftExceptionLogger.logSoftException(TAG, ReactNoCrashSoftException(ex))
778780
} else if (eventListenerInitializedForFabric) {
781+
// TODO T71377544: investigate these SoftExceptions and see if we can remove entirely
782+
// or fix the root cause
779783
ReactSoftExceptionLogger.logSoftException(TAG, ReactNoCrashSoftException(ex))
780784
} else {
781785
throw ex

0 commit comments

Comments
 (0)