diff --git a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index 57758be73c6ab7..ef72d45011fed7 100644 --- a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -115,6 +115,12 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode( .yogaNode_.isDirty() == yogaNode_.isDirty() && "Yoga node must inherit dirty flag."); + for (auto &child : getChildren()) { + if (auto layoutableChild = traitCast(child)) { + yogaLayoutableChildren_.push_back(layoutableChild); + } + } + yogaNode_.setContext(this); yogaNode_.setOwner(nullptr); updateYogaChildrenOwnersIfNeeded(); @@ -159,17 +165,17 @@ void YogaLayoutableShadowNode::enableMeasurement() { YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector); } -void YogaLayoutableShadowNode::appendYogaChild(ShadowNode const &childNode) { +void YogaLayoutableShadowNode::appendYogaChild( + YogaLayoutableShadowNode::Shared const &childNode) { // The caller must check this before calling this method. react_native_assert( !getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)); ensureYogaChildrenLookFine(); - auto &layoutableChildNode = - traitCast(childNode); + yogaLayoutableChildren_.push_back(childNode); yogaNode_.insertChild( - &layoutableChildNode.yogaNode_, + &childNode->yogaNode_, static_cast(yogaNode_.getChildren().size())); ensureYogaChildrenLookFine(); @@ -183,48 +189,21 @@ void YogaLayoutableShadowNode::adoptYogaChild(size_t index) { react_native_assert( !getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)); - auto &children = getChildren(); - - // Overflow checks. - react_native_assert(children.size() > index); - react_native_assert(children.size() >= yogaNode_.getChildren().size()); - - auto &childNode = *children.at(index); + auto &childNode = + traitCast(*getChildren().at(index)); - auto &layoutableChildNode = - traitCast(childNode); - - // Note, the following (commented out) assert is conceptually valid but still - // might produce false-positive signals because of the ABA problem (different - // objects with non-interleaving life-times being allocated on the same - // address). react_native_assert(layoutableChildNode.yogaNode_.getOwner() != - // &yogaNode_); - - if (layoutableChildNode.yogaNode_.getOwner() == nullptr) { + if (childNode.yogaNode_.getOwner() == nullptr) { // The child node is not owned. - layoutableChildNode.yogaNode_.setOwner(&yogaNode_); + childNode.yogaNode_.setOwner(&yogaNode_); // At this point the child yoga node must be already inserted by the caller. // react_native_assert(layoutableChildNode.yogaNode_.isDirty()); } else { // The child is owned by some other node, we need to clone that. // TODO: At this point, React has wrong reference to the node. (T138668036) auto clonedChildNode = childNode.clone({}); - auto &layoutableClonedChildNode = - traitCast(*clonedChildNode); - - // The owner must be nullptr for a newly cloned node. - react_native_assert( - layoutableClonedChildNode.yogaNode_.getOwner() == nullptr); - - // Establishing ownership. - layoutableClonedChildNode.yogaNode_.setOwner(&yogaNode_); // Replace the child node with a newly cloned one in the children list. replaceChild(childNode, clonedChildNode, static_cast(index)); - - // Replace the Yoga node inside the Yoga node children list. - yogaNode_.replaceChild( - &layoutableClonedChildNode.yogaNode_, static_cast(index)); } ensureYogaChildrenLookFine(); @@ -243,33 +222,79 @@ void YogaLayoutableShadowNode::appendChild( return; } - // Here we don't have information about the previous structure of the node (if - // it that existed before), so we don't have anything to compare the Yoga node - // with (like a previous version of this node). Therefore we must dirty the - // node. - yogaNode_.setDirty(true); + if (auto yogaLayoutableChild = + traitCast(childNode)) { + // Here we don't have information about the previous structure of the node + // (if it that existed before), so we don't have anything to compare the + // Yoga node with (like a previous version of this node). Therefore we must + // dirty the node. + yogaNode_.setDirty(true); - // All children of a non-leaf `YogaLayoutableShadowNode` must be a - // `YogaLayoutableShadowNode`s to be appended. This happens when invalid - // string/numeric child is passed which is not YogaLayoutableShadowNode - // (e.g. RCTRawText). This used to throw an error, but we are ignoring it - // because we want core library components to be fault-tolerant and degrade - // gracefully. A soft error will be emitted from JavaScript. - if (traitCast(childNode.get()) != nullptr) { // Appending the Yoga node. - appendYogaChild(*childNode); + appendYogaChild(yogaLayoutableChild); ensureYogaChildrenLookFine(); - ensureYogaChildrenAlighment(); + ensureYogaChildrenAlignment(); // Adopting the Yoga node. adoptYogaChild(getChildren().size() - 1); ensureConsistency(); + } +} + +void YogaLayoutableShadowNode::replaceChild( + ShadowNode const &oldChild, + ShadowNode::Shared const &newChild, + size_t suggestedIndex) { + LayoutableShadowNode::replaceChild(oldChild, newChild, suggestedIndex); + + ensureUnsealed(); + ensureYogaChildrenLookFine(); + + auto layoutableOldChild = + traitCast(&oldChild); + auto layoutableNewChild = traitCast(newChild); + + if (layoutableOldChild == nullptr && layoutableNewChild == nullptr) { + // No need to mutate yogaLayoutableChildren_ + return; + } + + bool suggestedIndexAccurate = suggestedIndex >= 0 && + suggestedIndex < yogaLayoutableChildren_.size() && + yogaLayoutableChildren_[suggestedIndex].get() == layoutableOldChild; + + auto oldChildIter = suggestedIndexAccurate + ? yogaLayoutableChildren_.begin() + suggestedIndex + : std::find_if( + yogaLayoutableChildren_.begin(), + yogaLayoutableChildren_.end(), + [&](YogaLayoutableShadowNode::Shared const &layoutableChild) { + return layoutableChild.get() == layoutableOldChild; + }); + auto oldChildIndex = + static_cast(oldChildIter - yogaLayoutableChildren_.begin()); + + if (oldChildIter == yogaLayoutableChildren_.end()) { + // oldChild does not exist as part of our node + return; + } + + if (layoutableNewChild) { + // Both children are layoutable, replace the old one with the new one + react_native_assert(layoutableNewChild->yogaNode_.getOwner() == nullptr); + layoutableNewChild->yogaNode_.setOwner(&yogaNode_); + *oldChildIter = layoutableNewChild; + yogaNode_.replaceChild(&layoutableNewChild->yogaNode_, oldChildIndex); } else { - react_native_log_error( - "Text strings must be rendered within a component."); + // Layoutable child replaced with non layoutable child. Remove the previous + // child from the layoutable children list. + yogaLayoutableChildren_.erase(oldChildIter); + yogaNode_.removeChild(oldChildIndex); } + + ensureYogaChildrenLookFine(); } bool YogaLayoutableShadowNode::doesOwn( @@ -297,23 +322,28 @@ void YogaLayoutableShadowNode::updateYogaChildren() { auto oldYogaChildren = isClean ? yogaNode_.getChildren() : YGVector{}; yogaNode_.setChildren({}); + yogaLayoutableChildren_.clear(); for (size_t i = 0; i < getChildren().size(); i++) { - appendYogaChild(*getChildren().at(i)); - adoptYogaChild(i); - - if (isClean) { - auto &oldYogaChildNode = *oldYogaChildren.at(i); - auto &newYogaChildNode = - traitCast(*getChildren().at(i)) - .yogaNode_; - - isClean = isClean && !newYogaChildNode.isDirty() && - (newYogaChildNode.getStyle() == oldYogaChildNode.getStyle()); + if (auto yogaLayoutableChild = + traitCast(getChildren()[i])) { + appendYogaChild(yogaLayoutableChild); + adoptYogaChild(i); + + if (isClean) { + auto yogaChildIndex = yogaLayoutableChildren_.size() - 1; + auto &oldYogaChildNode = *oldYogaChildren.at(yogaChildIndex); + auto &newYogaChildNode = + yogaLayoutableChildren_.at(yogaChildIndex)->yogaNode_; + + isClean = isClean && !newYogaChildNode.isDirty() && + (newYogaChildNode.getStyle() == oldYogaChildNode.getStyle()); + } } } - react_native_assert(getChildren().size() == yogaNode_.getChildren().size()); + react_native_assert( + yogaLayoutableChildren_.size() == yogaNode_.getChildren().size()); yogaNode_.setDirty(!isClean); } @@ -720,12 +750,9 @@ void YogaLayoutableShadowNode::swapLeftAndRightInTree( swapLeftAndRightInYogaStyleProps(shadowNode); swapLeftAndRightInViewProps(shadowNode); - for (auto &child : shadowNode.getChildren()) { - auto const yogaLayoutableChild = - traitCast(child.get()); - if ((yogaLayoutableChild != nullptr) && - !yogaLayoutableChild->doesOwn(shadowNode)) { - swapLeftAndRightInTree(*yogaLayoutableChild); + for (auto &child : shadowNode.yogaLayoutableChildren_) { + if (!child->doesOwn(shadowNode)) { + swapLeftAndRightInTree(*child); } } } @@ -837,7 +864,7 @@ void YogaLayoutableShadowNode::swapLeftAndRightInViewProps( void YogaLayoutableShadowNode::ensureConsistency() const { ensureYogaChildrenLookFine(); - ensureYogaChildrenAlighment(); + ensureYogaChildrenAlignment(); ensureYogaChildrenOwnersConsistency(); } @@ -874,7 +901,7 @@ void YogaLayoutableShadowNode::ensureYogaChildrenLookFine() const { #endif } -void YogaLayoutableShadowNode::ensureYogaChildrenAlighment() const { +void YogaLayoutableShadowNode::ensureYogaChildrenAlignment() const { #ifdef REACT_NATIVE_DEBUG // If the node is not a leaf node, checking that: // - All children are `YogaLayoutableShadowNode` subclasses. @@ -882,7 +909,7 @@ void YogaLayoutableShadowNode::ensureYogaChildrenAlighment() const { // this node. auto &yogaChildren = yogaNode_.getChildren(); - auto &children = getChildren(); + auto &children = yogaLayoutableChildren_; if (getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)) { react_native_assert(yogaChildren.empty()); diff --git a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h index e76f593a1622e6..7dcd333ae2c5e4 100644 --- a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h +++ b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h @@ -26,9 +26,9 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { using CompactValue = facebook::yoga::detail::CompactValue; public: - using UnsharedList = butter::small_vector< - YogaLayoutableShadowNode *, - kShadowNodeChildrenSmallVectorSize>; + using Shared = std::shared_ptr; + using ListOfShared = + butter::small_vector; static ShadowNodeTraits BaseTraits(); static ShadowNodeTraits::Trait IdentifierTrait(); @@ -52,7 +52,11 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { */ void enableMeasurement(); - void appendChild(ShadowNode::Shared const &child); + void appendChild(ShadowNode::Shared const &child) override; + void replaceChild( + ShadowNode const &oldChild, + ShadowNode::Shared const &newChild, + size_t suggestedIndex = -1) override; void updateYogaChildren(); @@ -121,7 +125,7 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { * The method does *not* do anything besides that (no cloning or `owner` field * adjustment). */ - void appendYogaChild(ShadowNode const &childNode); + void appendYogaChild(YogaLayoutableShadowNode::Shared const &childNode); /* * Makes the child node with a given `index` (and Yoga node associated with) a @@ -187,9 +191,15 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { #pragma mark - Consistency Ensuring Helpers void ensureConsistency() const; - void ensureYogaChildrenAlighment() const; + void ensureYogaChildrenAlignment() const; void ensureYogaChildrenOwnersConsistency() const; void ensureYogaChildrenLookFine() const; + +#pragma mark - Private member variables + /* + * List of children which derive from YogaLayoutableShadowNode + */ + ListOfShared yogaLayoutableChildren_; }; } // namespace react diff --git a/ReactCommon/react/renderer/core/ShadowNode.cpp b/ReactCommon/react/renderer/core/ShadowNode.cpp index aab1b72cf8193b..6068eb3e822aec 100644 --- a/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -228,7 +228,7 @@ void ShadowNode::appendChild(const ShadowNode::Shared &child) { void ShadowNode::replaceChild( ShadowNode const &oldChild, ShadowNode::Shared const &newChild, - int suggestedIndex) { + size_t suggestedIndex) { ensureUnsealed(); cloneChildrenIfShared(); @@ -239,7 +239,7 @@ void ShadowNode::replaceChild( *std::const_pointer_cast(children_); auto size = children.size(); - if (suggestedIndex != -1 && static_cast(suggestedIndex) < size) { + if (suggestedIndex != -1 && suggestedIndex < size) { // If provided `suggestedIndex` is accurate, // replacing in place using the index. if (children.at(suggestedIndex).get() == &oldChild) { diff --git a/ReactCommon/react/renderer/core/ShadowNode.h b/ReactCommon/react/renderer/core/ShadowNode.h index c3bbe40140b8a6..e1483bb08caafd 100644 --- a/ReactCommon/react/renderer/core/ShadowNode.h +++ b/ReactCommon/react/renderer/core/ShadowNode.h @@ -161,11 +161,11 @@ class ShadowNode : public Sealable, public DebugStringConvertible { #pragma mark - Mutating Methods - void appendChild(Shared const &child); - void replaceChild( + virtual void appendChild(Shared const &child); + virtual void replaceChild( ShadowNode const &oldChild, Shared const &newChild, - int suggestedIndex = -1); + size_t suggestedIndex = -1); /* * Performs all side effects associated with mounting/unmounting in one place.