Skip to content

Commit

Permalink
Flush operations queue when animation starts in RCTNativeAnimatedModule
Browse files Browse the repository at this point in the history
Summary:
changelog: [internal]

If nodesManager has the information if animated node is managed by Fabric, we can't decide if the operation queue should be flushed before it is flushed. Therefore, keep the information about animated nodes inside a set instead of nodesManager.

For simplicity, I will refer to class `RCTNativeAnimatedTurboModule` as *NativeAnimated* and to `RCTNativeAnimatedNodesManager` as *NodesManager*

Notice that each call to *NativeAnimated* is queued up in `_operations` or `_preOperations`. When the queues are flushed, only then methods are called on `RCTNativeAnimatedNodesManager`.

There are two mechanisms that flush operations.
One is triggered by `RCTMountingManager` before mounting operations are applied and after they are applied. This works fine but is important to paint the picture.

The second mechanism is inside `[RCTNativeAnimatedTurboModule startAnimatingNode]`. It flushes the queues for Fabric nodes only (not sure why Fabric nodes only, I couldn't find any explanation in old diffs). It checks with *NativeAnimated* if a node is managed by Fabric. Keep in mind, *NodesManager* only knows about the nodes when the queues have been flushed.

Exampe:
JavaScript calls methods on *NativeAnimated*.
For example:
1. `createNode`
2. `connectAnimatedNodeToView`
3. `startAnimatingNode`. (here, the queues should be flushed, since we are in Fabric)

All of these operations are queued up and for as long as `RCTMountingManager` executes mounting, all proceeds as expected.
But if those operations happen after mounting phase, `startAnimatingNode` will not flush the operations queues, because it can't tell if nodeTag is managed by fabric or it isn't. This is because *NodesManager* hasn't been notified about any new nodes.

Reviewed By: RSNara

Differential Revision: D30099010

fbshipit-source-id: d3fc021dd4346d1cbbda3b49ecd9d982c543e705
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Aug 9, 2021
1 parent 91cac20 commit f3374d0
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 39 deletions.
2 changes: 0 additions & 2 deletions Libraries/NativeAnimation/Nodes/RCTAnimatedNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@

@property (nonatomic, readonly) BOOL needsUpdate;

-(BOOL)isManagedByFabric;

/**
* Marks a node and its children as needing update.
*/
Expand Down
10 changes: 0 additions & 10 deletions Libraries/NativeAnimation/Nodes/RCTAnimatedNode.m
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,4 @@ - (void)performUpdate
// during the current update loop
}

- (BOOL)isManagedByFabric
{
for (RCTAnimatedNode *child in _childNodes.objectEnumerator) {
if ([child isManagedByFabric]) {
return YES;
}
}
return NO;
}

@end
6 changes: 0 additions & 6 deletions Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ - (instancetype)initWithTag:(NSNumber *)tag
return self;
}

- (BOOL)isManagedByFabric
{
return _managedByFabric;
}

- (void)connectToView:(NSNumber *)viewTag
viewName:(NSString *)viewName
bridge:(RCTBridge *)bridge
Expand All @@ -47,7 +42,6 @@ - (void)connectToView:(NSNumber *)viewTag
_surfacePresenter = surfacePresenter;
_connectedViewTag = viewTag;
_connectedViewName = viewName;
_managedByFabric = RCTUIManagerTypeForTagIsFabric(viewTag);
_rootTag = nil;
}

Expand Down
26 changes: 16 additions & 10 deletions Libraries/NativeAnimation/RCTNativeAnimatedModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ @implementation RCTNativeAnimatedModule
// 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 @@ -40,6 +42,7 @@ - (instancetype)init
_operations = [NSMutableArray new];
_preOperations = [NSMutableArray new];
_animIdIsManagedByFabric = [NSMutableDictionary new];
_nodeIDsManagedByFabric = [NSMutableSet new];
}
return self;
}
Expand Down Expand Up @@ -96,6 +99,9 @@ - (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 @@ -117,17 +123,13 @@ - (void)setSurfacePresenter:(id<RCTSurfacePresenterStub>)surfacePresenter
[self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
[nodesManager startAnimatingNode:[NSNumber numberWithDouble:animationId] nodeTag:[NSNumber numberWithDouble:nodeTag] config:config endCallback:callBack];
}];

BOOL isNodeManagedByFabric = [_nodeIDsManagedByFabric containsObject:@(nodeTag)];

RCTExecuteOnMainQueue(^{
if (![self->_nodesManager isNodeManagedByFabric:[NSNumber numberWithDouble:nodeTag]]) {
return;
}

RCTExecuteOnUIManagerQueue(^{
self->_animIdIsManagedByFabric[[NSNumber numberWithDouble:animationId]] = @YES;
[self flushOperationQueues];
});
});
if (isNodeManagedByFabric) {
self->_animIdIsManagedByFabric[[NSNumber numberWithDouble:animationId]] = @YES;
[self flushOperationQueues];
}
}

RCT_EXPORT_METHOD(stopAnimation:(double)animationId)
Expand Down Expand Up @@ -173,6 +175,10 @@ - (void)setSurfacePresenter:(id<RCTSurfacePresenterStub>)surfacePresenter
RCT_EXPORT_METHOD(connectAnimatedNodeToView:(double)nodeTag
viewTag:(double)viewTag)
{
if (RCTUIManagerTypeForTagIsFabric(@(viewTag))) {
[_nodeIDsManagedByFabric addObject:@(nodeTag)];
}

NSString *viewName = [self.bridge.uiManager viewNameForReactTag:[NSNumber numberWithDouble:viewTag]];
[self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
[nodesManager connectAnimatedNodeToView:[NSNumber numberWithDouble:nodeTag] viewTag:[NSNumber numberWithDouble:viewTag] viewName:viewName];
Expand Down
2 changes: 0 additions & 2 deletions Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

- (void)stepAnimations:(nonnull CADisplayLink *)displaylink;

- (BOOL)isNodeManagedByFabric:(nonnull NSNumber *)tag;

- (void)getValue:(nonnull NSNumber *)nodeTag
saveCallback:(nullable RCTResponseSenderBlock)saveCallback;

Expand Down
9 changes: 0 additions & 9 deletions Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,6 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge surfacePresenter:(id<RCTSurfa
return self;
}

- (BOOL)isNodeManagedByFabric:(nonnull NSNumber *)tag
{
RCTAnimatedNode *node = _animationNodes[tag];
if (node) {
return [node isManagedByFabric];
}
return false;
}

#pragma mark -- Graph

- (void)createAnimatedNode:(nonnull NSNumber *)tag
Expand Down

0 comments on commit f3374d0

Please sign in to comment.