Skip to content

Commit c9dcd64

Browse files
zeyapfacebook-github-bot
authored andcommitted
Clean up batchingControlledByJS in NativeAnimated kotlin (#53793)
Summary: Pull Request resolved: #53793 ## Changelog: [Android] [Deprecated] - Clean up batchingControlledByJS in NativeAnimated kotlin `start/finishOperationBatch` will no longer be called on kotlin NativeAnimated since D78005971 (#52521), so `batchingControlledByJS` will remain false. Cleaning up some logic and TODO comments there this feature was added in D23010844 Reviewed By: christophpurrer Differential Revision: D82461457 fbshipit-source-id: a1208720b83e614c2a5f994ec1a5005189c5f197
1 parent 15daa27 commit c9dcd64

File tree

2 files changed

+19
-28
lines changed

2 files changed

+19
-28
lines changed

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

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

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

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

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

205203
private var initializedForFabric = false
206204
private var initializedForNonFabric = false
@@ -282,22 +280,19 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) :
282280

283281
var batchNumber = currentBatchNumber - 1
284282

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-
}
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
301296
}
302297

303298
preOperations.executeBatch(batchNumber, nodesManager)
@@ -484,14 +479,14 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext) :
484479
}
485480
}
486481

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

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

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

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -774,12 +774,8 @@ 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
779777
ReactSoftExceptionLogger.logSoftException(TAG, ReactNoCrashSoftException(ex))
780778
} else if (eventListenerInitializedForFabric) {
781-
// TODO T71377544: investigate these SoftExceptions and see if we can remove entirely
782-
// or fix the root cause
783779
ReactSoftExceptionLogger.logSoftException(TAG, ReactNoCrashSoftException(ex))
784780
} else {
785781
throw ex

0 commit comments

Comments
 (0)