From fcd72bf34a4789260815b4e76e701d438c3f3cdd Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sun, 15 Jul 2018 16:46:16 -0700 Subject: [PATCH] Fabric: Generalizing cloning of YogaLayoutable approach Summary: @public Non-null owner pointer in Yoga node indicates that this node is already being used by some other subtree, so it must be cloned in case of possible (re)layout. Theoretically, this node must/can be cloned by Yoga right before applying a new layout to this node, but Yoga has a special optimization that uses that fact that Yoga always cloning *all* children of a particular node altogether. This is not true for React; to meet React and Yoga worlds we double check the owner pointer in `addChild` and clone node preliminary if needed. See also the previous diff for more context. Reviewed By: mdvacca Differential Revision: D8709952 fbshipit-source-id: 84ef0faa0f1d9cc9a8136b550cf325bc20508d53 --- ReactCommon/fabric/uimanager/FabricUIManager.cpp | 10 ---------- .../fabric/view/yoga/YogaLayoutableShadowNode.cpp | 12 ++++++++---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/ReactCommon/fabric/uimanager/FabricUIManager.cpp b/ReactCommon/fabric/uimanager/FabricUIManager.cpp index 23dd8f8c6..9e20dd056 100644 --- a/ReactCommon/fabric/uimanager/FabricUIManager.cpp +++ b/ReactCommon/fabric/uimanager/FabricUIManager.cpp @@ -230,16 +230,6 @@ SharedShadowNode FabricUIManager::cloneNodeWithNewChildrenAndProps(const SharedS void FabricUIManager::appendChild(const SharedShadowNode &parentShadowNode, const SharedShadowNode &childShadowNode) { isLoggingEnabled && LOG(INFO) << "FabricUIManager::appendChild(parentShadowNode: " << parentShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ", childShadowNode: " << childShadowNode->getDebugDescription(DebugStringConvertibleOptions {.format = false}) << ")"; const SharedComponentDescriptor &componentDescriptor = (*componentDescriptorRegistry_)[parentShadowNode]; - - // TODO: Remove this after we move this to JS side. - if (childShadowNode->getSealed()) { - auto childComponentDescriptor = (*componentDescriptorRegistry_)[childShadowNode]; - auto clonedChildShadowNode = childComponentDescriptor->cloneShadowNode(childShadowNode); - auto nonConstClonedChildShadowNode = std::const_pointer_cast(clonedChildShadowNode); - componentDescriptor->appendChild(parentShadowNode, clonedChildShadowNode); - return; - } - componentDescriptor->appendChild(parentShadowNode, childShadowNode); } diff --git a/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp index 69ed31115..34b0f372f 100644 --- a/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp @@ -94,12 +94,16 @@ void YogaLayoutableShadowNode::appendChild(SharedYogaLayoutableShadowNode child) auto yogaNodeRawPtr = &yogaNode_; auto childYogaNodeRawPtr = &child->yogaNode_; - yogaNodeRawPtr->insertChild(childYogaNodeRawPtr, yogaNodeRawPtr->getChildren().size()); - if (childYogaNodeRawPtr->getOwner() == nullptr) { - child->ensureUnsealed(); - childYogaNodeRawPtr->setOwner(yogaNodeRawPtr); + if (childYogaNodeRawPtr->getOwner() != nullptr) { + child = std::static_pointer_cast(cloneAndReplaceChild(child)); + childYogaNodeRawPtr = &child->yogaNode_; } + + child->ensureUnsealed(); + childYogaNodeRawPtr->setOwner(yogaNodeRawPtr); + + yogaNodeRawPtr->insertChild(childYogaNodeRawPtr, yogaNodeRawPtr->getChildren().size()); } void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {