From e1a5d2a9103140680d12c9ab50950ca7acf02307 Mon Sep 17 00:00:00 2001 From: "Simon G. Tsegay" <17194434+simongt@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:52:35 -0400 Subject: [PATCH 1/5] fix(fabric): lay out text under a dirty owner on font scale changes Text under a Modal, an InputAccessoryView or a react-native-screens screen kept its old size after a live font scale change on iOS. setSize(), which those descriptors call from adopt(), dirties a node without its ancestors, and markDirtyAndPropagate() stops at the first dirty owner. The clean parent above it then reuses its cached layout, so the subtree never gets the new metrics. Walk every owner in the font scale block of configureYogaTree instead, and stop at a display: none owner, which lays out no children. --- .../view/YogaLayoutableShadowNode.cpp | 12 +- .../components/view/tests/ViewTest.cpp | 207 ++++++++++++++++++ 2 files changed, 218 insertions(+), 1 deletion(-) 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 fb7c6e4ffe87..5c5dd55ec927 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -504,11 +504,21 @@ void YogaLayoutableShadowNode::configureYogaTree( // invalidate Yoga's layout cache. Dirty the node when it changes to force // re-measurement. We propagate up to the root so an unchanged, still-cached // ancestor isn't skipped by `calculateLayoutInternal` before reaching us. + // Unlike `markDirtyAndPropagate()`, keep going past an owner that is already + // dirty: `setSize()`, which Modal's `adopt()` calls, dirties a node without + // its ancestors. Stop at a `display: none` owner, which lays out no children. if (ReactNativeFeatureFlags::enableFontScaleChangesUpdatingLayout() && getTraits().check(ShadowNodeTraits::Trait::MeasurableYogaNode) && !floatEquality( getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier)) { - yogaNode_.markDirtyAndPropagate(); + for (auto* node = &yogaNode_; + node != nullptr && node->style().display() != yoga::Display::None; + node = node->getOwner()) { + if (!node->isDirty()) { + node->setDirty(true); + node->setLayoutComputedFlexBasis(yoga::FloatOptional()); + } + } } // TODO: `swapLeftAndRight` modified backing props and cannot be undone 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 ee71635f315c..6c67a7905cb0 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 @@ -11,8 +11,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -704,4 +706,209 @@ TEST_F(YogaCloneTest, cloneTreeClonesPathPlusSiblings) { << "Grandchild under unchanged sibling must be shared"; } +// NOLINTNEXTLINE(modernize-avoid-c-arrays) +static const char FontScaledTextComponentName[] = "FontScaledText"; +// NOLINTNEXTLINE(modernize-avoid-c-arrays) +static const char FontScaledInputComponentName[] = "FontScaledInput"; + +/* + * A measurable leaf whose height follows the font scale. With + * `keepsMeasurementOnClone` it keeps its measurement when cloned without new + * props, like `ParagraphShadowNode`; without it, every clone dirties it, like + * a text input. + */ +template +class FontScaledShadowNode final + : public ConcreteViewShadowNode { + public: + using Base = ConcreteViewShadowNode; + using Base::Base; + + static ShadowNodeTraits BaseTraits() { + auto traits = Base::BaseTraits(); + traits.set(ShadowNodeTraits::Trait::LeafYogaNode); + traits.set(ShadowNodeTraits::Trait::MeasurableYogaNode); + return traits; + } + + Size measureContent( + const LayoutContext& layoutContext, + const LayoutConstraints& /*layoutConstraints*/) const override { + return { + .width = 10 * layoutContext.fontSizeMultiplier, + .height = 10 * layoutContext.fontSizeMultiplier}; + } + + protected: + bool shouldNewRevisionDirtyMeasurement( + const ShadowNode& sourceShadowNode, + const ShadowNodeFragment& fragment) const override { + return keepsMeasurementOnClone + ? fragment.props != nullptr + : Base::shouldNewRevisionDirtyMeasurement(sourceShadowNode, fragment); + } +}; + +using FontScaledTextShadowNode = + FontScaledShadowNode; +using FontScaledInputShadowNode = + FontScaledShadowNode; + +// `ModalHostViewShadowNode` needs JNI to create its initial state on Android. +#ifndef ANDROID +class FontScaleLayoutTest : public ::testing::Test { + protected: + ComponentDescriptorProviderRegistry providerRegistry_{}; + ComponentDescriptorRegistry::Shared registry_; + ComponentBuilder builder_; + ContextContainer contextContainer_{}; + PropsParserContext parserContext_{-1, contextContainer_}; + + FontScaleLayoutTest() + : registry_(providerRegistry_.createComponentDescriptorRegistry( + ComponentDescriptorParameters{ + .eventDispatcher = EventDispatcher::Shared{}, + .contextContainer = nullptr, + .flavor = nullptr})), + builder_(registry_) { + providerRegistry_.add( + concreteComponentDescriptorProvider()); + providerRegistry_.add( + concreteComponentDescriptorProvider()); + providerRegistry_.add( + concreteComponentDescriptorProvider< + ModalHostViewComponentDescriptor>()); + providerRegistry_.add( + concreteComponentDescriptorProvider< + ConcreteComponentDescriptor>()); + providerRegistry_.add( + concreteComponentDescriptorProvider< + ConcreteComponentDescriptor>()); + } + + // Clones the root with a new font scale, as + // `SurfaceHandler::constraintLayout` does, and lays it out. + RootShadowNode::Unshared layOutWithFontScale( + const RootShadowNode& root, + Float fontSizeMultiplier) { + auto layoutContext = root.getConcreteProps().layoutContext; + layoutContext.fontSizeMultiplier = fontSizeMultiplier; + auto newRoot = root.clone( + parserContext_, + root.getConcreteProps().layoutConstraints, + layoutContext); + EXPECT_TRUE(newRoot->layoutIfNeeded()); + return newRoot; + } +}; + +TEST_F(FontScaleLayoutTest, fontScaleChangeLaysOutMeasurableNodesInsideModal) { + auto rootShadowNode = std::shared_ptr{}; + + // clang-format off + auto element = + Element() + .reference(rootShadowNode) + .tag(1) + .children({ + Element() + .tag(2) + .children({ + Element() + .tag(3) + .children({ + Element() + .tag(4) + .children({Element().tag(5)}), + Element() + .tag(6) + .children({Element().tag(7)}) + }) + }) + }); + // clang-format on + + builder_.build(element); + EXPECT_TRUE(rootShadowNode->layoutIfNeeded()); + + auto expectLaidOutWithFontScale = [](const RootShadowNode& root, + Float fontSizeMultiplier) { + const auto& modal = root.getChildren().at(0)->getChildren().at(0); + for (const auto& container : modal->getChildren()) { + const auto& leaf = static_cast( + *container->getChildren().at(0)); + EXPECT_EQ(leaf.getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier); + EXPECT_EQ( + leaf.getLayoutMetrics().frame.size.height, 10 * fontSizeMultiplier); + } + }; + + expectLaidOutWithFontScale(*rootShadowNode, 1); + + auto largerRootShadowNode = layOutWithFontScale(*rootShadowNode, 1.5); + expectLaidOutWithFontScale(*largerRootShadowNode, 1.5); + + auto smallerRootShadowNode = layOutWithFontScale(*largerRootShadowNode, 1); + expectLaidOutWithFontScale(*smallerRootShadowNode, 1); +} + +TEST_F(FontScaleLayoutTest, fontScaleChangeWhileHiddenLaysOutNodesOnceShown) { + auto rootShadowNode = std::shared_ptr{}; + auto containerShadowNode = std::shared_ptr{}; + + // clang-format off + auto element = + Element() + .reference(rootShadowNode) + .tag(1) + .children({ + Element() + .tag(2) + .children({ + Element() + .tag(3) + .children({ + Element() + .tag(4) + .reference(containerShadowNode) + .children({Element().tag(5)}) + }) + }) + }); + // clang-format on + + // Lays out the leaf while shown, so it has a measurement at the old scale. + builder_.build(element); + EXPECT_TRUE(rootShadowNode->layoutIfNeeded()); + + auto cloneWithContainerDisplay = [&](const ShadowNode& root, + yoga::Display display) { + return std::static_pointer_cast(root.cloneTree( + containerShadowNode->getFamily(), + [display](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDisplay(display); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + })); + }; + + auto hiddenRootShadowNode = + cloneWithContainerDisplay(*rootShadowNode, yoga::Display::None); + EXPECT_TRUE(hiddenRootShadowNode->layoutIfNeeded()); + + auto largerRootShadowNode = layOutWithFontScale(*hiddenRootShadowNode, 1.5); + + auto shownRootShadowNode = + cloneWithContainerDisplay(*largerRootShadowNode, yoga::Display::Flex); + EXPECT_TRUE(shownRootShadowNode->layoutIfNeeded()); + + const auto& modal = + shownRootShadowNode->getChildren().at(0)->getChildren().at(0); + const auto& leaf = static_cast( + *modal->getChildren().at(0)->getChildren().at(0)); + EXPECT_EQ(leaf.getLayoutMetrics().fontSizeMultiplier, 1.5); + EXPECT_EQ(leaf.getLayoutMetrics().frame.size.height, 15); +} +#endif + } // namespace facebook::react From c037ba429f9227f338f9ad073420f1879a0a10df Mon Sep 17 00:00:00 2001 From: "Simon G. Tsegay" <17194434+simongt@users.noreply.github.com> Date: Fri, 23 Oct 2026 09:23:41 -0400 Subject: [PATCH 2/5] fix(fabric): propagate dirty in-place clones in configureYogaTree configureYogaTree already walks the tree, so the extra walk over the owners is gone. Instead, apply the rule of updateYogaChildren() as the walk unwinds: after configuring a child, a node marks itself dirty when the child came back dirty, and display: none nodes are skipped. This covers clones dirtied by setSize() in a descriptor's adopt(), such as a Modal, and clones dirtied by completeClone(), such as a TextInput with no Modal above it. Replace the hidden-node test, which passed without the fix, and drop the unchanged-scale guard, which could not fail. Add a test for an input-like node without a Modal, and one that a hidden subtree is not laid out until it is shown. --- .../view/YogaLayoutableShadowNode.cpp | 25 ++--- .../components/view/tests/ViewTest.cpp | 100 +++++++++++++----- 2 files changed, 84 insertions(+), 41 deletions(-) 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 5c5dd55ec927..f99a911d6535 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -503,22 +503,13 @@ void YogaLayoutableShadowNode::configureYogaTree( // `pointScaleFactor` it is not part of the Yoga config, so a change does not // invalidate Yoga's layout cache. Dirty the node when it changes to force // re-measurement. We propagate up to the root so an unchanged, still-cached - // ancestor isn't skipped by `calculateLayoutInternal` before reaching us. - // Unlike `markDirtyAndPropagate()`, keep going past an owner that is already - // dirty: `setSize()`, which Modal's `adopt()` calls, dirties a node without - // its ancestors. Stop at a `display: none` owner, which lays out no children. + // ancestor isn't skipped by `calculateLayoutInternal` before reaching us. An + // owner that is already dirty stops it; the child loop below covers that. if (ReactNativeFeatureFlags::enableFontScaleChangesUpdatingLayout() && getTraits().check(ShadowNodeTraits::Trait::MeasurableYogaNode) && !floatEquality( getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier)) { - for (auto* node = &yogaNode_; - node != nullptr && node->style().display() != yoga::Display::None; - node = node->getOwner()) { - if (!node->isDirty()) { - node->setDirty(true); - node->setLayoutComputedFlexBasis(yoga::FloatOptional()); - } - } + yogaNode_.markDirtyAndPropagate(); } // TODO: `swapLeftAndRight` modified backing props and cannot be undone @@ -557,6 +548,16 @@ void YogaLayoutableShadowNode::configureYogaTree( cloneChildInPlace(i).configureYogaTree( pointScaleFactor, fontSizeMultiplier, errata, swapLeftAndRight); } + + // An in-place clone can come back dirty without dirtying this node, from + // `setSize()` in a descriptor's `adopt()` or from `completeClone()`. + // Apply the rule of `updateYogaChildren()` so Yoga doesn't skip it. + if (yogaLayoutableChildren_[i]->yogaNode_.isDirty() && + !yogaNode_.isDirty() && + yogaNode_.style().display() != yoga::Display::None && + ReactNativeFeatureFlags::enableFontScaleChangesUpdatingLayout()) { + yogaNode_.setDirty(true); + } } } 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 6c67a7905cb0..28afa199f57f 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 @@ -790,14 +790,15 @@ class FontScaleLayoutTest : public ::testing::Test { // `SurfaceHandler::constraintLayout` does, and lays it out. RootShadowNode::Unshared layOutWithFontScale( const RootShadowNode& root, - Float fontSizeMultiplier) { + Float fontSizeMultiplier, + std::vector* affectedNodes = nullptr) { auto layoutContext = root.getConcreteProps().layoutContext; layoutContext.fontSizeMultiplier = fontSizeMultiplier; auto newRoot = root.clone( parserContext_, root.getConcreteProps().layoutConstraints, layoutContext); - EXPECT_TRUE(newRoot->layoutIfNeeded()); + EXPECT_TRUE(newRoot->layoutIfNeeded(affectedNodes)); return newRoot; } }; @@ -837,6 +838,7 @@ TEST_F(FontScaleLayoutTest, fontScaleChangeLaysOutMeasurableNodesInsideModal) { for (const auto& container : modal->getChildren()) { const auto& leaf = static_cast( *container->getChildren().at(0)); + SCOPED_TRACE(leaf.getComponentName()); EXPECT_EQ(leaf.getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier); EXPECT_EQ( leaf.getLayoutMetrics().frame.size.height, 10 * fontSizeMultiplier); @@ -852,9 +854,10 @@ TEST_F(FontScaleLayoutTest, fontScaleChangeLaysOutMeasurableNodesInsideModal) { expectLaidOutWithFontScale(*smallerRootShadowNode, 1); } -TEST_F(FontScaleLayoutTest, fontScaleChangeWhileHiddenLaysOutNodesOnceShown) { +// `completeClone()` dirties an in-place clone of this leaf without its owner, +// as it does for a text input, so it needs no modal to go stale. +TEST_F(FontScaleLayoutTest, fontScaleChangeLaysOutInputLikeNodeWithoutModal) { auto rootShadowNode = std::shared_ptr{}; - auto containerShadowNode = std::shared_ptr{}; // clang-format off auto element = @@ -865,47 +868,86 @@ TEST_F(FontScaleLayoutTest, fontScaleChangeWhileHiddenLaysOutNodesOnceShown) { Element() .tag(2) .children({ - Element() + Element() .tag(3) - .children({ - Element() - .tag(4) - .reference(containerShadowNode) - .children({Element().tag(5)}) - }) + .children({Element().tag(4)}) }) }); // clang-format on - // Lays out the leaf while shown, so it has a measurement at the old scale. builder_.build(element); EXPECT_TRUE(rootShadowNode->layoutIfNeeded()); - auto cloneWithContainerDisplay = [&](const ShadowNode& root, - yoga::Display display) { - return std::static_pointer_cast(root.cloneTree( - containerShadowNode->getFamily(), - [display](const ShadowNode& oldShadowNode) { - auto viewProps = std::make_shared(); - viewProps->yogaStyle.setDisplay(display); - return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); - })); + auto expectLaidOutWithFontScale = [](const RootShadowNode& root, + Float fontSizeMultiplier) { + const auto& leaf = static_cast( + *root.getChildren().at(0)->getChildren().at(0)->getChildren().at(0)); + EXPECT_EQ(leaf.getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier); + EXPECT_EQ( + leaf.getLayoutMetrics().frame.size.height, 10 * fontSizeMultiplier); }; - auto hiddenRootShadowNode = - cloneWithContainerDisplay(*rootShadowNode, yoga::Display::None); - EXPECT_TRUE(hiddenRootShadowNode->layoutIfNeeded()); + auto largerRootShadowNode = layOutWithFontScale(*rootShadowNode, 1.5); + expectLaidOutWithFontScale(*largerRootShadowNode, 1.5); + + auto smallerRootShadowNode = layOutWithFontScale(*largerRootShadowNode, 1); + expectLaidOutWithFontScale(*smallerRootShadowNode, 1); +} - auto largerRootShadowNode = layOutWithFontScale(*hiddenRootShadowNode, 1.5); +TEST_F(FontScaleLayoutTest, fontScaleChangeSkipsHiddenSubtreeUntilShown) { + auto rootShadowNode = std::shared_ptr{}; + auto hiddenShadowNode = std::shared_ptr{}; + + // clang-format off + auto element = + Element() + .reference(rootShadowNode) + .tag(1) + .children({ + Element() + .tag(2) + .children({ + Element() + .tag(3) + .reference(hiddenShadowNode) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->yogaStyle.setDisplay(yoga::Display::None); + return sharedProps; + }) + .children({Element().tag(4)}) + }) + }); + // clang-format on + + builder_.build(element); + EXPECT_TRUE(rootShadowNode->layoutIfNeeded()); + + // A hidden node lays out no children, so a dirty clone below it must not + // make Yoga lay the hidden node out again. + auto affectedNodes = std::vector{}; + auto largerRootShadowNode = + layOutWithFontScale(*rootShadowNode, 1.5, &affectedNodes); + for (const auto* node : affectedNodes) { + EXPECT_NE(node->getTag(), hiddenShadowNode->getTag()); + } auto shownRootShadowNode = - cloneWithContainerDisplay(*largerRootShadowNode, yoga::Display::Flex); + std::static_pointer_cast(largerRootShadowNode->cloneTree( + hiddenShadowNode->getFamily(), [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDisplay(yoga::Display::Flex); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + })); EXPECT_TRUE(shownRootShadowNode->layoutIfNeeded()); - const auto& modal = - shownRootShadowNode->getChildren().at(0)->getChildren().at(0); const auto& leaf = static_cast( - *modal->getChildren().at(0)->getChildren().at(0)); + *shownRootShadowNode->getChildren() + .at(0) + ->getChildren() + .at(0) + ->getChildren() + .at(0)); EXPECT_EQ(leaf.getLayoutMetrics().fontSizeMultiplier, 1.5); EXPECT_EQ(leaf.getLayoutMetrics().frame.size.height, 15); } From 54c8337880b558365688004d77210fef376a0c7c Mon Sep 17 00:00:00 2001 From: "Simon G. Tsegay" <17194434+simongt@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:27:31 -0400 Subject: [PATCH 3/5] refactor(fabric): name the child node re-read after an in-place clone `cloneChildInPlace()` replaces `yogaLayoutableChildren_[i]`, so `child` still refers to the node from before the clone. Name the re-read node so the post-order dirty check visibly reads the configured clone. --- .../renderer/components/view/YogaLayoutableShadowNode.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 1704296c6e6f..154b59681b6e 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -551,8 +551,8 @@ void YogaLayoutableShadowNode::configureYogaTree( // An in-place clone can come back dirty without dirtying this node, from // `setSize()` in a descriptor's `adopt()` or from `completeClone()`. // Apply the rule of `updateYogaChildren()` so Yoga doesn't skip it. - if (yogaLayoutableChildren_[i]->yogaNode_.isDirty() && - !yogaNode_.isDirty() && + const auto& updatedYogaNode = yogaLayoutableChildren_[i]->yogaNode_; + if (updatedYogaNode.isDirty() && !yogaNode_.isDirty() && yogaNode_.style().display() != yoga::Display::None) { yogaNode_.setDirty(true); } From 7c521c6e602bb011428c9a696a4fc4ef95a5b103 Mon Sep 17 00:00:00 2001 From: "Simon G. Tsegay" <17194434+simongt@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:27:31 -0400 Subject: [PATCH 4/5] test(fabric): cover an RTL direction override on an unrelated commit Under an RTL root that swaps left and right, a `direction: ltr` subtree stores no swap in its layout metrics, so `configureYogaTree` configures it again on every pass and clones its children in place. An input-like leaf comes back dirty from that clone, and the post-order dirty check lays it out again. Pin that the geometry stays the same and which nodes are affected. --- .../components/view/tests/ViewTest.cpp | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) 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 28afa199f57f..d7e4238ec0e1 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 @@ -951,6 +951,97 @@ TEST_F(FontScaleLayoutTest, fontScaleChangeSkipsHiddenSubtreeUntilShown) { EXPECT_EQ(leaf.getLayoutMetrics().fontSizeMultiplier, 1.5); EXPECT_EQ(leaf.getLayoutMetrics().frame.size.height, 15); } + +// Under an RTL root that swaps left and right, a `direction: ltr` subtree +// stores no swap in its layout metrics, so `configureYogaTree` configures it +// again on every pass and clones its children in place. The input-like leaf +// comes back dirty from that clone, so its owners are dirtied and Yoga lays it +// out again at the same geometry. +TEST_F( + FontScaleLayoutTest, + rtlDirectionOverrideLaysOutDirtyCloneOnUnrelatedCommit) { + auto rootShadowNode = std::shared_ptr{}; + auto siblingShadowNode = std::shared_ptr{}; + + // clang-format off + auto element = + Element() + .reference(rootShadowNode) + .tag(1) + .children({ + Element() + .tag(2) + .children({ + Element() + .tag(3) + .reference(siblingShadowNode) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->yogaStyle.setDimension( + yoga::Dimension::Width, yoga::StyleSizeLength::points(10)); + sharedProps->yogaStyle.setDimension( + yoga::Dimension::Height, yoga::StyleSizeLength::points(10)); + return sharedProps; + }), + Element() + .tag(4) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->yogaStyle.setDirection(yoga::Direction::LTR); + return sharedProps; + }) + .children({Element().tag(5)}) + }) + }); + // clang-format on + + builder_.build(element); + + auto layoutConstraints = rootShadowNode->getConcreteProps().layoutConstraints; + layoutConstraints.layoutDirection = LayoutDirection::RightToLeft; + layoutConstraints.minimumSize = {.width = 100, .height = 100}; + layoutConstraints.maximumSize = {.width = 100, .height = 100}; + auto layoutContext = rootShadowNode->getConcreteProps().layoutContext; + layoutContext.swapLeftAndRightInRTL = true; + auto rtlRootShadowNode = + rootShadowNode->clone(parserContext_, layoutConstraints, layoutContext); + EXPECT_TRUE(rtlRootShadowNode->layoutIfNeeded()); + + auto overrideSubtreeFrames = [](const RootShadowNode& root) { + const auto& overrideNode = root.getChildren().at(0)->getChildren().at(1); + const auto& leaf = overrideNode->getChildren().at(0); + return std::pair{ + static_cast(*overrideNode) + .getLayoutMetrics() + .frame, + static_cast(*leaf) + .getLayoutMetrics() + .frame}; + }; + auto framesBeforeCommit = overrideSubtreeFrames(*rtlRootShadowNode); + + auto committedRootShadowNode = + std::static_pointer_cast(rtlRootShadowNode->cloneTree( + siblingShadowNode->getFamily(), [](const ShadowNode& oldShadowNode) { + auto viewProps = std::make_shared(); + viewProps->yogaStyle.setDimension( + yoga::Dimension::Width, yoga::StyleSizeLength::points(20)); + viewProps->yogaStyle.setDimension( + yoga::Dimension::Height, yoga::StyleSizeLength::points(10)); + return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); + })); + auto affectedNodes = std::vector{}; + EXPECT_TRUE(committedRootShadowNode->layoutIfNeeded(&affectedNodes)); + + EXPECT_EQ( + overrideSubtreeFrames(*committedRootShadowNode), framesBeforeCommit); + + auto affectedTags = std::vector{}; + for (const auto* node : affectedNodes) { + affectedTags.push_back(node->getTag()); + } + EXPECT_EQ(affectedTags, (std::vector{2, 3, 4, 5})); +} #endif } // namespace facebook::react From 4cd48f2afd4f5422873b1c844733f3c9d73d7c14 Mon Sep 17 00:00:00 2001 From: "Simon G. Tsegay" <17194434+simongt@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:38:20 -0400 Subject: [PATCH 5/5] fix(fabric): store the applied RTL swap for the configure skip check `configureYogaTree` skips a configured child when its stored `wasLeftAndRightSwapped` matches the swap passed down from the root. `layout()` stored the child's own direction instead, so a `direction` override under a root that swaps left and right never matched. That subtree was configured again and cloned in place on every pass, and a clone that came back dirty was laid out again. `wasLeftAndRightSwapped` has no other reader. Store the swap that was applied, taken from the parent, so the subtree is configured again only when the swap changes. The RTL test now flips the root to RTL, then asserts that an unrelated commit keeps the geometry, leaves the leaf out of `affectedNodes`, and does not clone it. --- .../view/YogaLayoutableShadowNode.cpp | 6 +- .../components/view/tests/ViewTest.cpp | 72 ++++++++++++------- 2 files changed, 50 insertions(+), 28 deletions(-) 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 154b59681b6e..226385a6ec31 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -778,9 +778,11 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) { auto newLayoutMetrics = layoutMetricsFromYogaNode(*childYogaNode); newLayoutMetrics.pointScaleFactor = layoutContext.pointScaleFactor; newLayoutMetrics.fontSizeMultiplier = layoutContext.fontSizeMultiplier; + // Store the swap that `configureYogaTree` applied, which comes from the + // root, not this node's direction. Otherwise a `direction` override never + // matches the skip check there and is configured again on every pass. newLayoutMetrics.wasLeftAndRightSwapped = - layoutContext.swapLeftAndRightInRTL && - newLayoutMetrics.layoutDirection == LayoutDirection::RightToLeft; + layoutMetrics_.wasLeftAndRightSwapped; // Child node's layout has changed. When a node is added to // `affectedNodes`, onLayout event is called on the component. Comparing 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 d7e4238ec0e1..48e9f451f5ae 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 @@ -952,14 +952,13 @@ TEST_F(FontScaleLayoutTest, fontScaleChangeSkipsHiddenSubtreeUntilShown) { EXPECT_EQ(leaf.getLayoutMetrics().frame.size.height, 15); } -// Under an RTL root that swaps left and right, a `direction: ltr` subtree -// stores no swap in its layout metrics, so `configureYogaTree` configures it -// again on every pass and clones its children in place. The input-like leaf -// comes back dirty from that clone, so its owners are dirtied and Yoga lays it -// out again at the same geometry. +// A `direction: ltr` subtree under an RTL root that swaps left and right must +// be configured again only when the swap passed down changes. Configuring it +// again clones the input-like leaf in place, which dirties it, so Yoga would +// lay it out again on every commit. TEST_F( FontScaleLayoutTest, - rtlDirectionOverrideLaysOutDirtyCloneOnUnrelatedCommit) { + rtlDirectionOverrideIsConfiguredOnlyWhenTheSwapChanges) { auto rootShadowNode = std::shared_ptr{}; auto siblingShadowNode = std::shared_ptr{}; @@ -997,27 +996,52 @@ TEST_F( builder_.build(element); - auto layoutConstraints = rootShadowNode->getConcreteProps().layoutConstraints; - layoutConstraints.layoutDirection = LayoutDirection::RightToLeft; - layoutConstraints.minimumSize = {.width = 100, .height = 100}; - layoutConstraints.maximumSize = {.width = 100, .height = 100}; - auto layoutContext = rootShadowNode->getConcreteProps().layoutContext; - layoutContext.swapLeftAndRightInRTL = true; - auto rtlRootShadowNode = - rootShadowNode->clone(parserContext_, layoutConstraints, layoutContext); - EXPECT_TRUE(rtlRootShadowNode->layoutIfNeeded()); - - auto overrideSubtreeFrames = [](const RootShadowNode& root) { + auto layOutWithDirection = + [this]( + const RootShadowNode& root, + LayoutDirection layoutDirection, + std::vector* affectedNodes) { + auto layoutConstraints = root.getConcreteProps().layoutConstraints; + layoutConstraints.layoutDirection = layoutDirection; + layoutConstraints.minimumSize = {.width = 100, .height = 100}; + layoutConstraints.maximumSize = {.width = 100, .height = 100}; + auto layoutContext = root.getConcreteProps().layoutContext; + layoutContext.swapLeftAndRightInRTL = true; + auto newRoot = + root.clone(parserContext_, layoutConstraints, layoutContext); + EXPECT_TRUE(newRoot->layoutIfNeeded(affectedNodes)); + return newRoot; + }; + auto tagsOf = [](const std::vector& nodes) { + auto tags = std::vector{}; + for (const auto* node : nodes) { + tags.push_back(node->getTag()); + } + return tags; + }; + auto leafOf = [](const RootShadowNode& root) { + return root.getChildren().at(0)->getChildren().at(1)->getChildren().at(0); + }; + auto overrideSubtreeFrames = [&](const RootShadowNode& root) { const auto& overrideNode = root.getChildren().at(0)->getChildren().at(1); - const auto& leaf = overrideNode->getChildren().at(0); return std::pair{ static_cast(*overrideNode) .getLayoutMetrics() .frame, - static_cast(*leaf) + static_cast(*leafOf(root)) .getLayoutMetrics() .frame}; }; + + auto ltrRootShadowNode = layOutWithDirection( + *rootShadowNode, LayoutDirection::LeftToRight, nullptr); + + // Turning the root RTL changes the swap passed down, so the subtree is + // configured again and the leaf is laid out again. + auto affectedNodes = std::vector{}; + auto rtlRootShadowNode = layOutWithDirection( + *ltrRootShadowNode, LayoutDirection::RightToLeft, &affectedNodes); + EXPECT_EQ(tagsOf(affectedNodes), (std::vector{2, 3, 4, 5})); auto framesBeforeCommit = overrideSubtreeFrames(*rtlRootShadowNode); auto committedRootShadowNode = @@ -1030,17 +1054,13 @@ TEST_F( yoga::Dimension::Height, yoga::StyleSizeLength::points(10)); return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps}); })); - auto affectedNodes = std::vector{}; + affectedNodes.clear(); EXPECT_TRUE(committedRootShadowNode->layoutIfNeeded(&affectedNodes)); EXPECT_EQ( overrideSubtreeFrames(*committedRootShadowNode), framesBeforeCommit); - - auto affectedTags = std::vector{}; - for (const auto* node : affectedNodes) { - affectedTags.push_back(node->getTag()); - } - EXPECT_EQ(affectedTags, (std::vector{2, 3, 4, 5})); + EXPECT_EQ(tagsOf(affectedNodes), (std::vector{2, 3, 4})); + EXPECT_EQ(leafOf(*committedRootShadowNode), leafOf(*rtlRootShadowNode)); } #endif