Skip to content

Commit

Permalink
Always flush in NativeAnimatedTurboModule
Browse files Browse the repository at this point in the history
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 (facebook@544d9fb).  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
  • Loading branch information
RSNara authored and OlimpiaZurek committed May 22, 2023
1 parent b8bb8c4 commit 9395d15
Showing 1 changed file with 2 additions and 20 deletions.
22 changes: 2 additions & 20 deletions Libraries/NativeAnimation/RCTNativeAnimatedTurboModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ @implementation RCTNativeAnimatedTurboModule
NSMutableArray<AnimatedOperation> *_operations;
// Operations called before views have been updated.
NSMutableArray<AnimatedOperation> *_preOperations;
NSMutableDictionary<NSNumber *, NSNumber *> *_animIdIsManagedByFabric;
// A set of nodeIDs managed by Fabric.
NSMutableSet<NSNumber *> *_nodeIDsManagedByFabric;
}

RCT_EXPORT_MODULE();
Expand All @@ -43,8 +40,6 @@ - (instancetype)init
if (self = [super init]) {
_operations = [NSMutableArray new];
_preOperations = [NSMutableArray new];
_animIdIsManagedByFabric = [NSMutableDictionary new];
_nodeIDsManagedByFabric = [NSMutableSet new];
}
return self;
}
Expand Down Expand Up @@ -113,9 +108,6 @@ - (void)setSurfacePresenter:(id<RCTSurfacePresenterStub>)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]];
}];
Expand All @@ -138,21 +130,15 @@ - (void)setSurfacePresenter:(id<RCTSurfacePresenterStub>)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)
{
[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
Expand Down Expand Up @@ -192,9 +178,6 @@ - (void)setSurfacePresenter:(id<RCTSurfacePresenterStub>)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];
Expand Down Expand Up @@ -296,7 +279,6 @@ - (void)flushOperationQueues
_preOperations = [NSMutableArray new];
_operations = [NSMutableArray new];


RCTExecuteOnMainQueue(^{
for (AnimatedOperation operation in preOperations) {
operation(self->_nodesManager);
Expand Down

0 comments on commit 9395d15

Please sign in to comment.