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 2e934259232..226385a6ec3 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -503,7 +503,8 @@ 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. + // ancestor isn't skipped by `calculateLayoutInternal` before reaching us. An + // owner that is already dirty stops it; the child loop below covers that. if (getTraits().check(ShadowNodeTraits::Trait::MeasurableYogaNode) && !floatEquality( getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier)) { @@ -546,6 +547,15 @@ 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. + const auto& updatedYogaNode = yogaLayoutableChildren_[i]->yogaNode_; + if (updatedYogaNode.isDirty() && !yogaNode_.isDirty() && + yogaNode_.style().display() != yoga::Display::None) { + yogaNode_.setDirty(true); + } } } @@ -768,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 ee71635f315..48e9f451f5a 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,362 @@ 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, + 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(affectedNodes)); + 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)); + SCOPED_TRACE(leaf.getComponentName()); + 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); +} + +// `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{}; + + // clang-format off + auto element = + Element() + .reference(rootShadowNode) + .tag(1) + .children({ + Element() + .tag(2) + .children({ + Element() + .tag(3) + .children({Element().tag(4)}) + }) + }); + // clang-format on + + builder_.build(element); + EXPECT_TRUE(rootShadowNode->layoutIfNeeded()); + + 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 largerRootShadowNode = layOutWithFontScale(*rootShadowNode, 1.5); + expectLaidOutWithFontScale(*largerRootShadowNode, 1.5); + + auto smallerRootShadowNode = layOutWithFontScale(*largerRootShadowNode, 1); + expectLaidOutWithFontScale(*smallerRootShadowNode, 1); +} + +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 = + 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& leaf = static_cast( + *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); +} + +// 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, + rtlDirectionOverrideIsConfiguredOnlyWhenTheSwapChanges) { + 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 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); + return std::pair{ + static_cast(*overrideNode) + .getLayoutMetrics() + .frame, + 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 = + 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}); + })); + affectedNodes.clear(); + EXPECT_TRUE(committedRootShadowNode->layoutIfNeeded(&affectedNodes)); + + EXPECT_EQ( + overrideSubtreeFrames(*committedRootShadowNode), framesBeforeCommit); + EXPECT_EQ(tagsOf(affectedNodes), (std::vector{2, 3, 4})); + EXPECT_EQ(leafOf(*committedRootShadowNode), leafOf(*rtlRootShadowNode)); +} +#endif + } // namespace facebook::react