From 3426f1f53761953124f7e24f371ff24745333d89 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 15 Nov 2019 19:47:51 -0800 Subject: [PATCH] Fabric: YogaLayoutableShadowNode::isLeaf_ and using that in ParagraphShadowNode Summary: As soon as we have traits infra, it's pretty straight-forward to implement this kinda flag. Without the traits, it's challenging to build that in a performant and elegant way. The challenges of the dynamic_cast-involved approach are: * How to check for that feature in YogaLayoutableShadowNode? * Even if we use `dynamic_cast`, to which class to cast? * We also need to do that in constructor... and dynamic_cast-like checks for `this` don't work in constructors (C++ design limitation). * How to scale that if we have more classes like Paragraph (and we will have it for sure)? * Performance. * Relying on RTTI. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D18525347 fbshipit-source-id: b1915f43ff3fe4364ab6345fb9d1becc591b5a35 --- .../text/paragraph/ParagraphShadowNode.h | 6 ++++++ .../components/view/ConcreteViewShadowNode.h | 3 ++- .../view/yoga/YogaLayoutableShadowNode.cpp | 19 +++++++++++++++---- .../view/yoga/YogaLayoutableShadowNode.h | 15 +++++++++++---- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h index e1b30eb18b..37cac3e985 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h @@ -39,6 +39,12 @@ class ParagraphShadowNode : public ConcreteViewShadowNode< public: using ConcreteViewShadowNode::ConcreteViewShadowNode; + static ShadowNodeTraits BaseTraits() { + auto traits = ConcreteViewShadowNode::BaseTraits(); + traits.set(ShadowNodeTraits::Trait::LeafYogaNode); + return traits; + } + /* * Returns a `AttributedString` which represents text content of the node. */ diff --git a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h index de1a995b33..e3ff5cdbcf 100644 --- a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h @@ -58,7 +58,8 @@ class ConcreteViewShadowNode : public ConcreteShadowNode< ComponentDescriptor const &componentDescriptor, ShadowNodeTraits traits) : BaseShadowNode(fragment, componentDescriptor, traits), - YogaLayoutableShadowNode() { + YogaLayoutableShadowNode( + traits.check(ShadowNodeTraits::Trait::LeafYogaNode)) { YogaLayoutableShadowNode::setProps( *std::static_pointer_cast(fragment.props)); YogaLayoutableShadowNode::setChildren( diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index 27d3d62554..e9307b0a23 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -21,18 +21,21 @@ namespace facebook { namespace react { -YogaLayoutableShadowNode::YogaLayoutableShadowNode() - : yogaConfig_(nullptr), yogaNode_(&initializeYogaConfig(yogaConfig_)) { +YogaLayoutableShadowNode::YogaLayoutableShadowNode(bool isLeaf) + : yogaConfig_(nullptr), + yogaNode_(&initializeYogaConfig(yogaConfig_)), + isLeaf_(isLeaf) { yogaNode_.setContext(this); } YogaLayoutableShadowNode::YogaLayoutableShadowNode( - const YogaLayoutableShadowNode &layoutableShadowNode) + YogaLayoutableShadowNode const &layoutableShadowNode) : LayoutableShadowNode(layoutableShadowNode), yogaConfig_(nullptr), yogaNode_( layoutableShadowNode.yogaNode_, - &initializeYogaConfig(yogaConfig_)) { + &initializeYogaConfig(yogaConfig_)), + isLeaf_(layoutableShadowNode.isLeaf_) { yogaNode_.setContext(this); yogaNode_.setOwner(nullptr); @@ -70,6 +73,10 @@ void YogaLayoutableShadowNode::enableMeasurement() { } void YogaLayoutableShadowNode::appendChild(YogaLayoutableShadowNode *child) { + if (isLeaf_) { + return; + } + ensureUnsealed(); yogaNode_.setDirty(true); @@ -95,6 +102,10 @@ void YogaLayoutableShadowNode::appendChild(YogaLayoutableShadowNode *child) { void YogaLayoutableShadowNode::setChildren( YogaLayoutableShadowNode::UnsharedList children) { + if (isLeaf_) { + return; + } + ensureUnsealed(); // Optimization: diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h index f0842dfda1..026a3e9d5b 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h @@ -32,10 +32,10 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode, #pragma mark - Constructors - YogaLayoutableShadowNode(); + YogaLayoutableShadowNode(bool isLeaf); YogaLayoutableShadowNode( - const YogaLayoutableShadowNode &layoutableShadowNode); + YogaLayoutableShadowNode const &layoutableShadowNode); #pragma mark - Mutating Methods @@ -64,14 +64,14 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode, */ void setProps(const YogaStylableProps &props); - /** + /* * Sets layoutable size of node. */ void setSize(Size size) const; void setPadding(RectangleEdges padding) const; - /** + /* * Sets position type of Yoga node (relative, absolute). */ void setPositionType(YGPositionType positionType) const; @@ -108,6 +108,13 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode, */ mutable YGNode yogaNode_; + /* + * Forces associated YGNode to be a leaf. + * Adding a child `ShadowNode` will not add `YGNode` associated with it as a + * child to the stored `YGNode`. + */ + bool const isLeaf_; + private: static YGConfig &initializeYogaConfig(YGConfig &config); static YGNode *yogaNodeCloneCallbackConnector(