From ea54ceca1317033c6701ded5199100555cd8a711 Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Mon, 25 Feb 2019 12:22:24 -0800 Subject: [PATCH] basic useNativeDriver functionality Summary: Not super clean, but not terrible. Unfortunately this still relies on the old Paper UIManager calling delegate methods to flush the operations queues. This will work for Marketplace You since Paper will be active, but we need to fix this, along with Animated Events which don't work at all yet. Random aside: it seems like taps are less responsive in fabric vs. paper, at least on iOS. There is a sporadic delay between the touches event coming in nativly to the JS callback invoking the native module function to start the animation - this will need some debugging. Reviewed By: shergin Differential Revision: D14143331 fbshipit-source-id: 63a17eaafa1217d77a532a2716d9f886a96fae59 --- .../Nodes/RCTPropsAnimatedNode.h | 4 +- .../Nodes/RCTPropsAnimatedNode.m | 59 +++++++++++++++---- .../NativeAnimation/RCTNativeAnimatedModule.m | 6 +- .../RCTNativeAnimatedNodesManager.h | 2 +- .../RCTNativeAnimatedNodesManager.m | 8 +-- .../RCTNativeAnimatedNodesManagerTests.m | 4 +- React/Fabric/Mounting/RCTMountingManager.h | 3 + React/Fabric/Mounting/RCTMountingManager.mm | 11 ++++ React/Fabric/RCTScheduler.h | 3 + React/Fabric/RCTScheduler.mm | 5 ++ React/Fabric/RCTSurfacePresenter.h | 4 +- React/Fabric/RCTSurfacePresenter.mm | 31 +++++++++- ReactCommon/fabric/uimanager/Scheduler.cpp | 5 ++ ReactCommon/fabric/uimanager/Scheduler.h | 2 + 14 files changed, 119 insertions(+), 28 deletions(-) diff --git a/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h b/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h index 4bf39fe3e0..a8a5218a5e 100644 --- a/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h +++ b/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.h @@ -7,14 +7,14 @@ #import "RCTAnimatedNode.h" -@class RCTUIManager; +@class RCTBridge; @class RCTViewPropertyMapper; @interface RCTPropsAnimatedNode : RCTAnimatedNode - (void)connectToView:(NSNumber *)viewTag viewName:(NSString *)viewName - uiManager:(RCTUIManager *)uiManager; + bridge:(RCTBridge *)bridge; - (void)disconnectFromView:(NSNumber *)viewTag; diff --git a/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m b/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m index 473a921ba3..8677d2da1e 100644 --- a/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m +++ b/Libraries/NativeAnimation/Nodes/RCTPropsAnimatedNode.m @@ -7,6 +7,8 @@ #import "RCTPropsAnimatedNode.h" +#import + #import #import @@ -14,12 +16,33 @@ #import "RCTStyleAnimatedNode.h" #import "RCTValueAnimatedNode.h" +// TODO: Eventually we should just include RCTSurfacePresenter.h, but that pulls in all of fabric +// which doesn't compile in open source yet, so we mirror the protocol and duplicate the category +// here for now. + +@protocol SyncViewUpdater + +- (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictionary *)props; + +@end + +@implementation RCTBridge (SurfacePresenterShadow) + +- (id)surfacePresenter +{ + return objc_getAssociatedObject(self, @selector(surfacePresenter)); +} + +@end + + @implementation RCTPropsAnimatedNode { NSNumber *_connectedViewTag; + NSNumber *_rootTag; NSString *_connectedViewName; - __weak RCTUIManager *_uiManager; - NSMutableDictionary *_propsDictionary; + __weak RCTBridge *_bridge; + NSMutableDictionary *_propsDictionary; // TODO: use RawProps or folly::dynamic directly } - (instancetype)initWithTag:(NSNumber *)tag @@ -33,18 +56,32 @@ - (void)connectToView:(NSNumber *)viewTag viewName:(NSString *)viewName - uiManager:(RCTUIManager *)uiManager + bridge:(RCTBridge *)bridge { _connectedViewTag = viewTag; _connectedViewName = viewName; - _uiManager = uiManager; + _bridge = bridge; + _rootTag = nil; } - (void)disconnectFromView:(NSNumber *)viewTag { _connectedViewTag = nil; _connectedViewName = nil; - _uiManager = nil; + _bridge = nil; + _rootTag = nil; +} + +- (void)updateView +{ + BOOL fabricUpdateSuccess = [_bridge.surfacePresenter synchronouslyUpdateViewOnUIThread:_connectedViewTag + props:_propsDictionary]; + if (fabricUpdateSuccess) { + return; + } + [_bridge.uiManager synchronouslyUpdateViewOnUIThread:_connectedViewTag + viewName:_connectedViewName + props:_propsDictionary]; } - (void)restoreDefaultValues @@ -55,9 +92,7 @@ } if (_propsDictionary.count) { - [_uiManager synchronouslyUpdateViewOnUIThread:_connectedViewTag - viewName:_connectedViewName - props:_propsDictionary]; + [self updateView]; } } @@ -83,12 +118,12 @@ if (!_connectedViewTag) { return; } - + for (NSNumber *parentTag in self.parentNodes.keyEnumerator) { RCTAnimatedNode *parentNode = [self.parentNodes objectForKey:parentTag]; if ([parentNode isKindOfClass:[RCTStyleAnimatedNode class]]) { [self->_propsDictionary addEntriesFromDictionary:[(RCTStyleAnimatedNode *)parentNode propsDictionary]]; - + } else if ([parentNode isKindOfClass:[RCTValueAnimatedNode class]]) { NSString *property = [self propertyNameForParentTag:parentTag]; CGFloat value = [(RCTValueAnimatedNode *)parentNode value]; @@ -97,9 +132,7 @@ } if (_propsDictionary.count) { - [_uiManager synchronouslyUpdateViewOnUIThread:_connectedViewTag - viewName:_connectedViewName - props:_propsDictionary]; + [self updateView]; } } diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedModule.m b/Libraries/NativeAnimation/RCTNativeAnimatedModule.m index 57f08034e1..b00d95ff51 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedModule.m +++ b/Libraries/NativeAnimation/RCTNativeAnimatedModule.m @@ -41,12 +41,12 @@ RCT_EXPORT_MODULE(); { [super setBridge:bridge]; - _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithUIManager:self.bridge.uiManager]; + _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:self.bridge]; _operations = [NSMutableArray new]; _preOperations = [NSMutableArray new]; [bridge.eventDispatcher addDispatchObserver:self]; - [bridge.uiManager.observerCoordinator addObserver:self]; + [bridge.uiManager.observerCoordinator addObserver:self]; // TODO: add fabric equivalent? } #pragma mark -- API @@ -196,7 +196,7 @@ RCT_EXPORT_METHOD(removeAnimatedEventFromView:(nonnull NSNumber *)viewTag #pragma mark - RCTUIManagerObserver -- (void)uiManagerWillPerformMounting:(RCTUIManager *)uiManager +- (void)uiManagerWillPerformMounting:(RCTUIManager *)uiManager // TODO: need fabric equivalent { if (_preOperations.count == 0 && _operations.count == 0) { return; diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h index 6f30f96fab..1222050583 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h +++ b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h @@ -13,7 +13,7 @@ @interface RCTNativeAnimatedNodesManager : NSObject -- (nonnull instancetype)initWithUIManager:(nonnull RCTUIManager *)uiManager; +- (nonnull instancetype)initWithBridge:(nonnull RCTBridge *)bridge; - (void)updateAnimations; diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m index 30ec0e42da..03bded86af 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m +++ b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m @@ -30,7 +30,7 @@ @implementation RCTNativeAnimatedNodesManager { - __weak RCTUIManager *_uiManager; + __weak RCTBridge *_bridge; NSMutableDictionary *_animationNodes; // Mapping of a view tag and an event name to a list of event animation drivers. 99% of the time // there will be only one driver per mapping so all code code should be optimized around that. @@ -39,10 +39,10 @@ CADisplayLink *_displayLink; } -- (instancetype)initWithUIManager:(nonnull RCTUIManager *)uiManager +- (instancetype)initWithBridge:(nonnull RCTBridge *)bridge { if ((self = [super init])) { - _uiManager = uiManager; + _bridge = bridge; _animationNodes = [NSMutableDictionary new]; _eventDrivers = [NSMutableDictionary new]; _activeAnimations = [NSMutableSet new]; @@ -124,7 +124,7 @@ { RCTAnimatedNode *node = _animationNodes[nodeTag]; if ([node isKindOfClass:[RCTPropsAnimatedNode class]]) { - [(RCTPropsAnimatedNode *)node connectToView:viewTag viewName:viewName uiManager:_uiManager]; + [(RCTPropsAnimatedNode *)node connectToView:viewTag viewName:viewName bridge:_bridge]; } [node setNeedsUpdate]; } diff --git a/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m b/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m index eee726bf6b..38d64d864e 100644 --- a/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m +++ b/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m @@ -127,8 +127,10 @@ static id RCTPropChecker(NSString *prop, NSNumber *value) { [super setUp]; + RCTBridge *bridge = [OCMockObject niceMockForClass:[RCTBridge class]]; _uiManager = [OCMockObject niceMockForClass:[RCTUIManager class]]; - _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithUIManager:_uiManager]; + OCMStub([bridge uiManager]).andReturn(_uiManager); + _nodesManager = [[RCTNativeAnimatedNodesManager alloc] initWithBridge:bridge]; _displayLink = [RCTFakeDisplayLink new]; } diff --git a/React/Fabric/Mounting/RCTMountingManager.h b/React/Fabric/Mounting/RCTMountingManager.h index d4a64da800..0f65e39812 100644 --- a/React/Fabric/Mounting/RCTMountingManager.h +++ b/React/Fabric/Mounting/RCTMountingManager.h @@ -39,6 +39,9 @@ NS_ASSUME_NONNULL_BEGIN */ - (void)optimisticallyCreateComponentViewWithComponentHandle:(facebook::react::ComponentHandle)componentHandle; +- (void)synchronouslyUpdateViewOnUIThread:(ReactTag)reactTag + oldProps:(facebook::react::SharedProps)oldProps + newProps:(facebook::react::SharedProps)newProps; @end NS_ASSUME_NONNULL_END diff --git a/React/Fabric/Mounting/RCTMountingManager.mm b/React/Fabric/Mounting/RCTMountingManager.mm index 105f39919e..ed228f158b 100644 --- a/React/Fabric/Mounting/RCTMountingManager.mm +++ b/React/Fabric/Mounting/RCTMountingManager.mm @@ -176,6 +176,17 @@ using namespace facebook::react; [self.delegate mountingManager:self didMountComponentsWithRootTag:rootTag]; } +- (void)synchronouslyUpdateViewOnUIThread:(ReactTag)reactTag + oldProps:(SharedProps)oldProps + newProps:(SharedProps)newProps +{ + RCTUpdatePropsMountItem *mountItem = [[RCTUpdatePropsMountItem alloc] initWithTag:reactTag + oldProps:oldProps + newProps:newProps]; + RCTAssertMainQueue(); + [mountItem executeWithRegistry:self->_componentViewRegistry]; +} + - (void)optimisticallyCreateComponentViewWithComponentHandle:(ComponentHandle)componentHandle { if (RCTIsMainQueue()) { diff --git a/React/Fabric/RCTScheduler.h b/React/Fabric/RCTScheduler.h index a57c2cbf98..709250d39b 100644 --- a/React/Fabric/RCTScheduler.h +++ b/React/Fabric/RCTScheduler.h @@ -9,6 +9,7 @@ #import #import +#import #import #import #import @@ -54,6 +55,8 @@ NS_ASSUME_NONNULL_BEGIN layoutContext:(facebook::react::LayoutContext)layoutContext surfaceId:(facebook::react::SurfaceId)surfaceId; +- (const facebook::react::ComponentDescriptor &)getComponentDescriptor:(facebook::react::ComponentHandle)handle; + @end NS_ASSUME_NONNULL_END diff --git a/React/Fabric/RCTScheduler.mm b/React/Fabric/RCTScheduler.mm index 60752913c5..a7bc1cc305 100644 --- a/React/Fabric/RCTScheduler.mm +++ b/React/Fabric/RCTScheduler.mm @@ -107,4 +107,9 @@ private: _scheduler->constraintSurfaceLayout(surfaceId, layoutConstraints, layoutContext); } +- (const ComponentDescriptor &)getComponentDescriptor:(ComponentHandle)handle +{ + return _scheduler->getComponentDescriptor(handle); +} + @end diff --git a/React/Fabric/RCTSurfacePresenter.h b/React/Fabric/RCTSurfacePresenter.h index c4f1f98fbf..1e684dd5a4 100644 --- a/React/Fabric/RCTSurfacePresenter.h +++ b/React/Fabric/RCTSurfacePresenter.h @@ -10,9 +10,9 @@ #import #import -#import #import #import +#import NS_ASSUME_NONNULL_BEGIN @@ -66,6 +66,8 @@ NS_ASSUME_NONNULL_BEGIN maximumSize:(CGSize)maximumSize surface:(RCTFabricSurface *)surface; +- (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictionary *)props; + @end @interface RCTSurfacePresenter (Deprecated) diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index 3a18def1d2..c203638716 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -16,17 +16,18 @@ #import #import #import +#import #import #import #import #import #import -#import #import +#import #import -#import -#import #import +#import +#import #import #import @@ -59,6 +60,7 @@ using namespace facebook::react; if (self = [super init]) { _bridge = bridge; _batchedBridge = [_bridge batchedBridge] ?: _bridge; + [_batchedBridge setSurfacePresenter:self]; _surfaceRegistry = [[RCTSurfaceRegistry alloc] init]; @@ -161,6 +163,29 @@ using namespace facebook::react; surfaceId:surface.rootTag]; } +- (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictionary *)props +{ + ReactTag tag = [reactTag integerValue]; + UIView *componentView = [_mountingManager.componentViewRegistry componentViewByTag:tag]; + if (componentView == nil) { + return NO; // This view probably isn't managed by Fabric + } + ComponentHandle handle = [[componentView class] componentHandle]; + const facebook::react::ComponentDescriptor &componentDescriptor = [self._scheduler getComponentDescriptor:handle]; + + // Note: we use an empty object for `oldProps` to rely on the diffing algorithm internal to the + // RCTComponentViewProtocol::updateProps method. If there is a bug in that diffing, some props + // could get reset. One way around this would be to require all RCTComponentViewProtocol + // implementations to expose their current props so we could clone them, but that could be + // problematic for threading and other reasons. + facebook::react::SharedProps newProps = + componentDescriptor.cloneProps(nullptr, RawProps(convertIdToFollyDynamic(props))); + facebook::react::SharedProps oldProps = componentDescriptor.cloneProps(nullptr, RawProps(folly::dynamic::object())); + + [self->_mountingManager synchronouslyUpdateViewOnUIThread:tag oldProps:oldProps newProps:newProps]; + return YES; +} + #pragma mark - Private - (RCTScheduler *)_scheduler diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index c0212fd2d5..ca4f1489bf 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -190,6 +190,11 @@ void Scheduler::constraintSurfaceLayout( }); } +const ComponentDescriptor &Scheduler::getComponentDescriptor( + ComponentHandle handle) { + return componentDescriptorRegistry_->at(handle); +} + #pragma mark - Delegate void Scheduler::setDelegate(SchedulerDelegate *delegate) { diff --git a/ReactCommon/fabric/uimanager/Scheduler.h b/ReactCommon/fabric/uimanager/Scheduler.h index 1d2bc10f63..1eff117f32 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.h +++ b/ReactCommon/fabric/uimanager/Scheduler.h @@ -67,6 +67,8 @@ class Scheduler final : public UIManagerDelegate, public ShadowTreeDelegate { const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const; + const ComponentDescriptor &getComponentDescriptor(ComponentHandle handle); + #pragma mark - Delegate /*