From 9395d15863ee1b9829dd1d7b0115aac3b7bbed8e Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 21 Sep 2022 12:59:09 -0700 Subject: [PATCH] Always flush in NativeAnimatedTurboModule Summary: ## Summary In the past, NativeAnimatedModule could animate **both** Paper **and** Fabric components. For Fabric nodes, we needed to manually flush NativeAnimatedModule's operations queue. So, we started tracking which nodes were Fabric owned in NativeAnimatedModule. ## Changes With bridgeless mode, all components must be Fabric-owned. So, should be able to remove this fabric ownership tracking logic and **always flush.** ## Is this safe? In the worst case, we over-flush. This doesn't seem bad. cc sammy-SC. ## Do we still need flushing? Arguably, all this manual flushing should be unnecessary, because we already migrated AnimatedModule's Paper integration to RCTSurfacePresenterObserver, here: D14336760 (https://github.com/facebook/react-native/commit/544d9fb10b5a73bd499feb18dab1a7dc11738748). So, do we still need this flushing? Yes. Here's what happens when you disable all the manual flushing in bridgeless mode: https://pxl.cl/2dqPf. Long-term, we need to re-think this operations queuing in NativeAnimatedTurboModule. I left my thoughts in T130668424 (Investigation - Day 5). Changelog: [Internal] Reviewed By: p-sun Differential Revision: D39592477 fbshipit-source-id: e971edc0d99661a37b5f430bce46c78acaa121c0 --- .../RCTNativeAnimatedTurboModule.mm | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm b/Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm index 9643b9f7d4e62c..2d41c1d0d79beb 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm +++ b/Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm @@ -26,9 +26,6 @@ @implementation RCTNativeAnimatedTurboModule NSMutableArray *_operations; // Operations called before views have been updated. NSMutableArray *_preOperations; - NSMutableDictionary *_animIdIsManagedByFabric; - // A set of nodeIDs managed by Fabric. - NSMutableSet *_nodeIDsManagedByFabric; } RCT_EXPORT_MODULE(); @@ -43,8 +40,6 @@ - (instancetype)init if (self = [super init]) { _operations = [NSMutableArray new]; _preOperations = [NSMutableArray new]; - _animIdIsManagedByFabric = [NSMutableDictionary new]; - _nodeIDsManagedByFabric = [NSMutableSet new]; } return self; } @@ -113,9 +108,6 @@ - (void)setSurfacePresenter:(id)surfacePresenter RCT_EXPORT_METHOD(connectAnimatedNodes:(double)parentTag childTag:(double)childTag) { - if ([_nodeIDsManagedByFabric containsObject:@(childTag)]) { - [_nodeIDsManagedByFabric addObject:@(parentTag)]; - } [self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) { [nodesManager connectAnimatedNodes:[NSNumber numberWithDouble:parentTag] childTag:[NSNumber numberWithDouble:childTag]]; }]; @@ -138,11 +130,7 @@ - (void)setSurfacePresenter:(id)surfacePresenter [nodesManager startAnimatingNode:[NSNumber numberWithDouble:animationId] nodeTag:[NSNumber numberWithDouble:nodeTag] config:config endCallback:callBack]; }]; - BOOL isNodeManagedByFabric = [_nodeIDsManagedByFabric containsObject:@(nodeTag)]; - if (isNodeManagedByFabric) { - self->_animIdIsManagedByFabric[[NSNumber numberWithDouble:animationId]] = @YES; - [self flushOperationQueues]; - } + [self flushOperationQueues]; } RCT_EXPORT_METHOD(stopAnimation:(double)animationId) @@ -150,9 +138,7 @@ - (void)setSurfacePresenter:(id)surfacePresenter [self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) { [nodesManager stopAnimation:[NSNumber numberWithDouble:animationId]]; }]; - if ([_animIdIsManagedByFabric[[NSNumber numberWithDouble:animationId]] boolValue]) { - [self flushOperationQueues]; - } + [self flushOperationQueues]; } RCT_EXPORT_METHOD(setAnimatedNodeValue:(double)nodeTag @@ -192,9 +178,6 @@ - (void)setSurfacePresenter:(id)surfacePresenter RCT_EXPORT_METHOD(connectAnimatedNodeToView:(double)nodeTag viewTag:(double)viewTag) { - if (RCTUIManagerTypeForTagIsFabric(@(viewTag))) { - [_nodeIDsManagedByFabric addObject:@(nodeTag)]; - } [self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) { // viewName is not used when node is managed by Fabric, and nodes are always managed by Fabric in Bridgeless. [nodesManager connectAnimatedNodeToView:[NSNumber numberWithDouble:nodeTag] viewTag:[NSNumber numberWithDouble:viewTag] viewName:nil]; @@ -296,7 +279,6 @@ - (void)flushOperationQueues _preOperations = [NSMutableArray new]; _operations = [NSMutableArray new]; - RCTExecuteOnMainQueue(^{ for (AnimatedOperation operation in preOperations) { operation(self->_nodesManager);