From 7953c3e8543f124b2b9935dc0eb4e6473cc06440 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 26 Feb 2020 22:03:14 -0800 Subject: [PATCH] Fabric: Devirtualizing `LayoutableShadowNode::cloneAndReplaceChild` Summary: D19963353 mentioned the infrastructure that re-routes methods calls related to adding and cloning children between YogaLayoutableShadowNode and ShadowNode. `cloneAndReplaceChild` is exactly this. It was implemented as a virtual method that is called from `ConcreteViewShadowNode`. The whole process requires building a list of children of some class and passing that as a list of pointers. Now we don't need it all that because we can call directly and statically. That change will allow us to simplify that infra even more in the future diffs. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D20052022 fbshipit-source-id: ddf341c112edd8a2f79eaf74465a9a360a168541 --- .../components/view/ConcreteViewShadowNode.h | 26 ---------- .../view/yoga/YogaLayoutableShadowNode.cpp | 47 ++++++++++++++----- .../view/yoga/YogaLayoutableShadowNode.h | 18 ++++--- .../fabric/core/layout/LayoutableShadowNode.h | 9 +--- 4 files changed, 49 insertions(+), 51 deletions(-) diff --git a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h index de6e80d958e..f9618b77130 100644 --- a/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h +++ b/ReactCommon/fabric/components/view/ConcreteViewShadowNode.h @@ -89,32 +89,6 @@ class ConcreteViewShadowNode : public ConcreteShadowNode< return traits; } - void appendChild(const ShadowNode::Shared &child) { - BaseShadowNode::ensureUnsealed(); - - ShadowNode::appendChild(child); - - auto nonConstChild = const_cast(child.get()); - auto yogaLayoutableChild = - dynamic_cast(nonConstChild); - if (yogaLayoutableChild) { - YogaLayoutableShadowNode::appendChild(yogaLayoutableChild); - } - } - - LayoutableShadowNode *cloneAndReplaceChild( - LayoutableShadowNode *child, - int suggestedIndex = -1) override { - Sealable::ensureUnsealed(); - auto childShadowNode = static_cast(child); - auto clonedChildShadowNode = - std::static_pointer_cast( - childShadowNode->clone({})); - ShadowNode::replaceChild( - *childShadowNode, clonedChildShadowNode, suggestedIndex); - return clonedChildShadowNode.get(); - } - Transform getTransform() const override { return BaseShadowNode::getConcreteProps().transform; } diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index d5dd7b14dd3..75e512a1bce 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -78,28 +78,45 @@ void YogaLayoutableShadowNode::enableMeasurement() { YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector); } -void YogaLayoutableShadowNode::appendChild(YogaLayoutableShadowNode *child) { +void YogaLayoutableShadowNode::appendChild(ShadowNode::Shared const &child) { + ensureUnsealed(); + + LayoutableShadowNode::appendChild(child); + + auto yogaLayoutableChild = + traitCast(child.get()); + if (yogaLayoutableChild) { + appendChildYogaNode( + *const_cast(yogaLayoutableChild)); + } +} + +void YogaLayoutableShadowNode::appendChildYogaNode( + YogaLayoutableShadowNode &child) { + ensureUnsealed(); + if (getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)) { + // This node is a declared leaf, therefore we must not add the Yoga node as + // a child. return; } - ensureUnsealed(); - yogaNode_.setDirty(true); auto yogaNodeRawPtr = &yogaNode_; - auto childYogaNodeRawPtr = &child->yogaNode_; + auto childYogaNodeRawPtr = &child.yogaNode_; + auto childNodePtr = &child; if (childYogaNodeRawPtr->getOwner() != nullptr) { - child = static_cast( - cloneAndReplaceChild(child, yogaNode_.getChildren().size())); - childYogaNodeRawPtr = &child->yogaNode_; + childNodePtr = + &cloneAndReplaceChild(*childNodePtr, yogaNode_.getChildren().size()); + childYogaNodeRawPtr = &childNodePtr->yogaNode_; } // Inserted node must have a clear owner (must not be shared). assert(childYogaNodeRawPtr->getOwner() == nullptr); - child->ensureUnsealed(); + childNodePtr->ensureUnsealed(); childYogaNodeRawPtr->setOwner(yogaNodeRawPtr); yogaNodeRawPtr->insertChild( @@ -125,7 +142,7 @@ void YogaLayoutableShadowNode::setChildren( auto i = int{0}; for (auto const &child : children) { - appendChild(child); + appendChildYogaNode(*child); isClean = isClean && !child->yogaNode_.isDirty() && child->yogaNode_.getStyle() == oldChildren[i++]->getStyle(); @@ -246,6 +263,15 @@ YogaLayoutableShadowNode::getLayoutableChildNodes() const { return yogaLayoutableChildNodes; } +YogaLayoutableShadowNode &YogaLayoutableShadowNode::cloneAndReplaceChild( + YogaLayoutableShadowNode &child, + int suggestedIndex) { + auto clonedChildShadowNode = child.clone({}); + replaceChild(child, clonedChildShadowNode, suggestedIndex); + + return static_cast(*clonedChildShadowNode); +} + #pragma mark - Yoga Connectors YGNode *YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector( @@ -260,8 +286,7 @@ YGNode *YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector( static_cast(parentYogaNode->getContext()); auto oldNode = static_cast(oldYogaNode->getContext()); - auto clonedNode = static_cast( - parentNode->cloneAndReplaceChild(oldNode, childIndex)); + auto clonedNode = &parentNode->cloneAndReplaceChild(*oldNode, childIndex); return &clonedNode->yogaNode_; } diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h index bd58941c459..f75c56c4850 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h @@ -47,12 +47,7 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { */ void enableMeasurement(); - /* - * Appends `child`'s Yoga node to the own Yoga node. - * Complements `ShadowNode::appendChild(...)` functionality from Yoga - * perspective. - */ - void appendChild(YogaLayoutableShadowNode *child); + void appendChild(ShadowNode::Shared const &child); /* * Sets Yoga children based on collection of `YogaLayoutableShadowNode` @@ -111,6 +106,17 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { mutable YGNode yogaNode_; private: + /* + * Appends `child`'s Yoga node to the own Yoga node. + * Complements `ShadowNode::appendChild(...)` functionality from Yoga + * perspective. + */ + void appendChildYogaNode(YogaLayoutableShadowNode &child); + + YogaLayoutableShadowNode &cloneAndReplaceChild( + YogaLayoutableShadowNode &child, + int suggestedIndex); + static YGConfig &initializeYogaConfig(YGConfig &config); static YGNode *yogaNodeCloneCallbackConnector( YGNode *oldYogaNode, diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h index ec6e8e485e6..2625917c3a8 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -136,14 +137,6 @@ class LayoutableShadowNode : public ShadowNode { virtual LayoutableShadowNode::UnsharedList getLayoutableChildNodes() const = 0; - /* - * In case layout algorithm needs to mutate this (probably sealed) node, - * it has to clone and replace it in the hierarchy before to do so. - */ - virtual LayoutableShadowNode *cloneAndReplaceChild( - LayoutableShadowNode *child, - int suggestedIndex = -1) = 0; - /* * Sets layout metrics for the shadow node. * Returns true if the metrics are different from previous ones.