diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index cca684c33824..1489a6f13688 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -347,6 +347,17 @@ void YogaLayoutableShadowNode::updateYogaChildrenOwnersIfNeeded() { } } +// Every clone gets its own Yoga config, and `setConfig()` re-aligns the +// layout's `configVersion` with it, so that field says nothing about whether +// two nodes carry the same layout. Everything else has to match. +static bool hasSameLayoutResults( + const yoga::Node& yogaNode, + const yoga::Node& otherYogaNode) { + auto layout = yogaNode.getLayout(); + layout.configVersion = otherYogaNode.getLayout().configVersion; + return layout == otherYogaNode.getLayout(); +} + void YogaLayoutableShadowNode::updateYogaChildren() { if (getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)) { return; @@ -386,8 +397,11 @@ void YogaLayoutableShadowNode::updateYogaChildren() { auto& newYogaChildNode = yogaLayoutableChildren_.at(yogaChildIndex)->yogaNode_; + // A clean child styled the same can still carry another layout, e.g. a + // revision zeroed out under a `display: none` clone of this node. isClean = isClean && !newYogaChildNode.isDirty() && - (newYogaChildNode.style() == oldYogaChildNode.style()); + (newYogaChildNode.style() == oldYogaChildNode.style()) && + hasSameLayoutResults(newYogaChildNode, oldYogaChildNode); } } } diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp index b5aa3bddb8d4..1e0a06b31da1 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp @@ -559,6 +559,59 @@ TEST_F(YogaCloneTest, newChildrenCloneReflectsNewList) { 100); } +// A clone that is handed its children must not keep its layout when a child, +// although clean and styled the same, carries a different computed layout. +// That is the case for a child revision zeroed out under a `display: none` +// clone of the parent, which React can hold through runtime shadow node +// reference updates while it keeps cloning from the visible parent. +TEST_F(YogaCloneTest, newChildrenWithDifferentLayoutDirtyTheClone) { + auto hiddenRoot = rootShadowNode_->cloneTree( + parentShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) { + auto hiddenProps = std::make_shared(); + auto& yogaStyle = hiddenProps->yogaStyle; + yogaStyle.setFlexDirection(yoga::FlexDirection::Row); + yogaStyle.setDimension( + yoga::Dimension::Width, yoga::StyleSizeLength::points(300)); + yogaStyle.setDimension( + yoga::Dimension::Height, yoga::StyleSizeLength::points(100)); + yogaStyle.setDisplay(yoga::Display::None); + return oldShadowNode.clone({.props = hiddenProps}); + }); + static_cast(*hiddenRoot).layoutIfNeeded(); + + auto zeroedChildA = hiddenRoot->getChildren()[0]->getChildren()[0]; + ASSERT_EQ( + yogaNodeOf(*zeroedChildA).getLayout().dimension(yoga::Dimension::Width), + 0); + ASSERT_FALSE(yogaNodeOf(*zeroedChildA).isDirty()); + + auto newChildren = + std::make_shared>>( + std::vector>{ + zeroedChildA->clone({}), + parentShadowNode_->getChildren()[1], + parentShadowNode_->getChildren()[2]}); + + auto clonedParent = parentShadowNode_->clone( + {.props = ShadowNodeFragment::propsPlaceholder(), + .children = newChildren}); + + EXPECT_FALSE( + static_cast(*clonedParent) + .getIsLayoutClean()); + + auto newRootShadowNode = rootShadowNode_->cloneTree( + parentShadowNode_->getFamily(), + [&](const ShadowNode& /*oldShadowNode*/) { return clonedParent; }); + static_cast(*newRootShadowNode).layoutIfNeeded(); + + const auto& laidOutChildA = + yogaNodeOf(*newRootShadowNode->getChildren()[0]->getChildren()[0]) + .getLayout(); + EXPECT_EQ(laidOutChildA.dimension(yoga::Dimension::Width), 100); + EXPECT_EQ(laidOutChildA.dimension(yoga::Dimension::Height), 100); +} + // Successive prop-only clones (the animation commit pattern) must each // produce a self-consistent layout. Validates that the fast path's vector // copy yields a vector the next clone can again copy from. diff --git a/packages/react-native/ReactCommon/react/renderer/core/__docs__/passChildrenWhenCloning.md b/packages/react-native/ReactCommon/react/renderer/core/__docs__/passChildrenWhenCloning.md index 194a86ce11ea..5b4f33c537fb 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/__docs__/passChildrenWhenCloning.md +++ b/packages/react-native/ReactCommon/react/renderer/core/__docs__/passChildrenWhenCloning.md @@ -48,5 +48,17 @@ of mutation. The clone implementation of the YogaLayoutableShadowNode will compare the previous set of children with the new set. If both sets have the same size and -each node has equivalent styling, the cloned node is not being dirtied for -layout. This allows for improving the layout data re-use on subsequent renders. +each new child is clean, has equivalent styling and carries the same layout +results (computed layout and layout cache) as the child it replaces, the cloned +node is not being dirtied for layout. This allows for improving the layout data +re-use on subsequent renders. + +The layout results have to be compared as well: a child can be clean and styled +the same while carrying a layout that the parent's cached layout was not +computed with. Runtime shadow node reference updates can point the renderer at +such a revision. For example, when `` hides a subtree the renderer +commits a `display: none` clone of the subtree's host root. Laying out that +clone zeroes out clones of its children, and those clones inherit the children's +runtime references, while the renderer keeps cloning from the visible host root. +Without the layout check, the next clone of that host root would keep its cached +layout and Yoga would never revisit the zeroed children. diff --git a/packages/react-native/ReactCommon/yoga/yoga/node/LayoutResults.h b/packages/react-native/ReactCommon/yoga/yoga/node/LayoutResults.h index 54960a443651..d427aea0385e 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/node/LayoutResults.h +++ b/packages/react-native/ReactCommon/yoga/yoga/node/LayoutResults.h @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -136,7 +137,7 @@ struct LayoutResults { padding_[yoga::to_underlying(physicalEdge)] = dimension; } - bool operator==(LayoutResults layout) const; + YG_EXPORT bool operator==(LayoutResults layout) const; private: Direction direction_ : bitCount() = Direction::Inherit; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api index 912089515add..921b4861dad4 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api @@ -7242,9 +7242,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api index 2567b4e72da9..29bb4ff776fa 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api @@ -11703,9 +11703,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api index 34b1da7689bb..4687eb5b751d 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api @@ -7091,9 +7091,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api index 34b1da7689bb..4687eb5b751d 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api @@ -7091,9 +7091,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api index 65b1426db47e..8360a9404bf0 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api @@ -8976,9 +8976,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api index ec76e48de457..10b1d184aae8 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api @@ -13463,9 +13463,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api index c8c92eb83ed1..5b242c3dae4c 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api @@ -8828,9 +8828,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api index 3ebc82e69ba0..28d77a4d0711 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api @@ -8835,9 +8835,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api index 2ef9e05072ac..1f658ff7f2d1 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api @@ -4457,9 +4457,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api index 58f5c38aa6a0..693da7d2e85d 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api @@ -8809,9 +8809,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api index 9279765a5038..c3f7f25e8261 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api @@ -4450,9 +4450,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api index 9279765a5038..c3f7f25e8261 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api @@ -4450,9 +4450,9 @@ struct facebook::yoga::LayoutData { } struct facebook::yoga::LayoutResults { + public YG_EXPORT bool operator==(facebook::yoga::LayoutResults layout) const; public bool computedAutoMinMainSizeResolved() const; public bool hadOverflow() const; - public bool operator==(facebook::yoga::LayoutResults layout) const; public facebook::yoga::CachedMeasurement cachedLayout; public facebook::yoga::Direction direction() const; public facebook::yoga::Direction lastOwnerDirection;