From b9850844a5fd3d3091c7e8d564652222b2149014 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 9 Oct 2018 16:25:01 -0700 Subject: [PATCH] Fabric: `constraintLayout` is now return boolean Summary: Setting the right expectations: setting layout constraints might fail. Nothing really changed. Implementing a reliable `constraintLayout` which locks instead of returning immediately requires some additional work and new/additional API. Reviewed By: mdvacca Differential Revision: D10159457 fbshipit-source-id: bb23c7de105629ef086ae0b04667ff32c6ffb81d --- React/Fabric/RCTScheduler.h | 2 +- React/Fabric/RCTScheduler.mm | 4 ++-- React/Fabric/RCTSurfacePresenter.h | 3 ++- React/Fabric/RCTSurfacePresenter.mm | 8 +++---- React/Fabric/Surface/RCTFabricSurface.mm | 6 ++--- ReactCommon/fabric/uimanager/Scheduler.cpp | 4 ++-- ReactCommon/fabric/uimanager/Scheduler.h | 10 +++++++- ReactCommon/fabric/uimanager/ShadowTree.cpp | 26 ++++++++++++--------- ReactCommon/fabric/uimanager/ShadowTree.h | 10 ++++---- 9 files changed, 43 insertions(+), 30 deletions(-) diff --git a/React/Fabric/RCTScheduler.h b/React/Fabric/RCTScheduler.h index d747032a1..b69e9207e 100644 --- a/React/Fabric/RCTScheduler.h +++ b/React/Fabric/RCTScheduler.h @@ -49,7 +49,7 @@ NS_ASSUME_NONNULL_BEGIN layoutContext:(facebook::react::LayoutContext)layoutContext surfaceId:(facebook::react::SurfaceId)surfaceId; -- (void)constraintSurfaceLayoutWithLayoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints +- (BOOL)constraintSurfaceLayoutWithLayoutConstraints:(facebook::react::LayoutConstraints)layoutConstraints layoutContext:(facebook::react::LayoutContext)layoutContext surfaceId:(facebook::react::SurfaceId)surfaceId; diff --git a/React/Fabric/RCTScheduler.mm b/React/Fabric/RCTScheduler.mm index 7a1822dd9..3755a0966 100644 --- a/React/Fabric/RCTScheduler.mm +++ b/React/Fabric/RCTScheduler.mm @@ -80,11 +80,11 @@ private: return RCTCGSizeFromSize(_scheduler->measureSurface(surfaceId, layoutConstraints, layoutContext)); } -- (void)constraintSurfaceLayoutWithLayoutConstraints:(LayoutConstraints)layoutConstraints +- (BOOL)constraintSurfaceLayoutWithLayoutConstraints:(LayoutConstraints)layoutConstraints layoutContext:(LayoutContext)layoutContext surfaceId:(SurfaceId)surfaceId { - _scheduler->constraintSurfaceLayout(surfaceId, layoutConstraints, layoutContext); + return _scheduler->constraintSurfaceLayout(surfaceId, layoutConstraints, layoutContext); } @end diff --git a/React/Fabric/RCTSurfacePresenter.h b/React/Fabric/RCTSurfacePresenter.h index 9e1931835..762066011 100644 --- a/React/Fabric/RCTSurfacePresenter.h +++ b/React/Fabric/RCTSurfacePresenter.h @@ -57,8 +57,9 @@ NS_ASSUME_NONNULL_BEGIN /** * Sets `minimumSize` and `maximumSize` layout constraints for the Surface. + * Returns `YES` if the operation finished successfully. */ -- (void)setMinimumSize:(CGSize)minimumSize +- (BOOL)setMinimumSize:(CGSize)minimumSize maximumSize:(CGSize)maximumSize surface:(RCTFabricSurface *)surface; diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index bf3344025..85c2ae0e6 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -119,7 +119,7 @@ using namespace facebook::react; surfaceId:surface.rootTag]; } -- (void)setMinimumSize:(CGSize)minimumSize +- (BOOL)setMinimumSize:(CGSize)minimumSize maximumSize:(CGSize)maximumSize surface:(RCTFabricSurface *)surface { @@ -129,9 +129,9 @@ using namespace facebook::react; layoutConstraints.minimumSize = RCTSizeFromCGSize(minimumSize); layoutConstraints.maximumSize = RCTSizeFromCGSize(maximumSize); - [self._scheduler constraintSurfaceLayoutWithLayoutConstraints:layoutConstraints - layoutContext:layoutContext - surfaceId:surface.rootTag]; + return [self._scheduler constraintSurfaceLayoutWithLayoutConstraints:layoutConstraints + layoutContext:layoutContext + surfaceId:surface.rootTag]; } #pragma mark - Private diff --git a/React/Fabric/Surface/RCTFabricSurface.mm b/React/Fabric/Surface/RCTFabricSurface.mm index fb238f31f..dd7e5f9c9 100644 --- a/React/Fabric/Surface/RCTFabricSurface.mm +++ b/React/Fabric/Surface/RCTFabricSurface.mm @@ -216,9 +216,9 @@ _minimumSize = minimumSize; } - return [_surfacePresenter setMinimumSize:minimumSize - maximumSize:maximumSize - surface:self]; + [_surfacePresenter setMinimumSize:minimumSize + maximumSize:maximumSize + surface:self]; } - (CGSize)minimumSize diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index 6f73f6314..84b8ef205 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -94,7 +94,7 @@ Size Scheduler::measureSurface( return shadowTree->measure(layoutConstraints, layoutContext); } -void Scheduler::constraintSurfaceLayout( +bool Scheduler::constraintSurfaceLayout( SurfaceId surfaceId, const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext @@ -135,7 +135,7 @@ void Scheduler::uiManagerDidFinishTransaction(Tag rootTag, const SharedShadowNod return; } - return iterator->second->complete(rootChildNodes); + iterator->second->complete(rootChildNodes); } void Scheduler::uiManagerDidCreateShadowNode(const SharedShadowNode &shadowNode) { diff --git a/ReactCommon/fabric/uimanager/Scheduler.h b/ReactCommon/fabric/uimanager/Scheduler.h index 87382a195..a7fcf5bd7 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.h +++ b/ReactCommon/fabric/uimanager/Scheduler.h @@ -48,7 +48,15 @@ public: const LayoutContext &layoutContext ) const; - void constraintSurfaceLayout( + /* + * Applies given `layoutConstraints` and `layoutContext` to a Surface. + * The user interface will be relaid out as a result. The operation will be + * performed synchronously (including mounting) if the method is called + * on the main thread. + * Returns `true` if the operation finished successfully. + * Can be called from any thread. + */ + bool constraintSurfaceLayout( SurfaceId surfaceId, const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext diff --git a/ReactCommon/fabric/uimanager/ShadowTree.cpp b/ReactCommon/fabric/uimanager/ShadowTree.cpp index f74c079e3..9bd4ce0a2 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.cpp +++ b/ReactCommon/fabric/uimanager/ShadowTree.cpp @@ -51,9 +51,9 @@ Size ShadowTree::measure(const LayoutConstraints &layoutConstraints, const Layou return newRootShadowNode->getLayoutMetrics().frame.size; } -void ShadowTree::constraintLayout(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const { +bool ShadowTree::constraintLayout(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const { auto newRootShadowNode = cloneRootShadowNode(layoutConstraints, layoutContext); - complete(newRootShadowNode); + return complete(newRootShadowNode); } #pragma mark - Commiting @@ -66,7 +66,7 @@ UnsharedRootShadowNode ShadowTree::cloneRootShadowNode(const LayoutConstraints & return newRootShadowNode; } -void ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) const { +bool ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) const { auto oldRootShadowNode = getRootShadowNode(); auto newRootShadowNode = std::make_shared( @@ -76,10 +76,10 @@ void ShadowTree::complete(const SharedShadowNodeUnsharedList &rootChildNodes) co } ); - complete(newRootShadowNode); + return complete(newRootShadowNode); } -void ShadowTree::complete(UnsharedRootShadowNode newRootShadowNode) const { +bool ShadowTree::complete(UnsharedRootShadowNode newRootShadowNode) const { SharedRootShadowNode oldRootShadowNode = getRootShadowNode(); newRootShadowNode->layout(); @@ -91,13 +91,17 @@ void ShadowTree::complete(UnsharedRootShadowNode newRootShadowNode) const { *newRootShadowNode ); - if (commit(oldRootShadowNode, newRootShadowNode, mutations)) { - emitLayoutEvents(mutations); - - if (delegate_) { - delegate_->shadowTreeDidCommit(*this, mutations); - } + if (!commit(oldRootShadowNode, newRootShadowNode, mutations)) { + return false; } + + emitLayoutEvents(mutations); + + if (delegate_) { + delegate_->shadowTreeDidCommit(*this, mutations); + } + + return true; } bool ShadowTree::commit( diff --git a/ReactCommon/fabric/uimanager/ShadowTree.h b/ReactCommon/fabric/uimanager/ShadowTree.h index 7ee60c82c..ff6cef9db 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.h +++ b/ReactCommon/fabric/uimanager/ShadowTree.h @@ -49,19 +49,19 @@ public: /* * Applies given `layoutConstraints` and `layoutContext` and commit * the new shadow tree. - * Returns `true` if the operation is finished successfully. + * Returns `true` if the operation finished successfully. * Can be called from any thread. */ - void constraintLayout(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const; + bool constraintLayout(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const; #pragma mark - Application /* * Create a new shadow tree with given `rootChildNodes` and commit. * Can be called from any thread. - * Returns `true` if the operation is finished successfully. + * Returns `true` if the operation finished successfully. */ - void complete(const SharedShadowNodeUnsharedList &rootChildNodes) const; + bool complete(const SharedShadowNodeUnsharedList &rootChildNodes) const; #pragma mark - Delegate @@ -76,7 +76,7 @@ public: private: UnsharedRootShadowNode cloneRootShadowNode(const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const; - void complete(UnsharedRootShadowNode newRootShadowNode) const; + bool complete(UnsharedRootShadowNode newRootShadowNode) const; bool commit( const SharedRootShadowNode &oldRootShadowNode, const SharedRootShadowNode &newRootShadowNode,