Skip to content

Commit 45904c8

Browse files
j-piaseckimeta-codesync[bot]
authored andcommitted
Fix Fabric reusing nodes with a stale font scale (#57246)
Summary: Fixes #52895 The original solution to this issue approached this by comparing the font scale used to lay out respective root nodes and re-cloning measurable nodes in the entire tree when it was different. This didn't work when a node within the tree was cloned from a node with an obsolete font scale stored (with root having the correct one). The new node would use the obsolete value in that pass. This PR changes this approach - instead of comparing font scale in `SurfaceHandler::constraintLayout` (only when it changes), it moves the comparison to happen as part of `configureYogaTree`, similarly to `pointScaleFactor`. This way, nodes with an obsolete value stored get re-cloned using the correct one on each commit instead of only on the first one. ## Changelog: [GENERAL][FIXED] - Fixed Fabric reusing nodes with a stale font scale Pull Request resolved: #57246 Test Plan: Issue reproducer ||Before|After| |-|-|-| |Android|<video src="https://github.com/user-attachments/assets/0b30ca95-76e9-4ce9-8d8f-418f9d827bea" />|<video src="https://github.com/user-attachments/assets/009be13c-c3b5-4af0-8be1-b44c94ac0496" />| |iOS|<video src="https://github.com/user-attachments/assets/b861b622-a2ea-4cc4-a8db-0e3ad4cae5e7" />|<video src="https://github.com/user-attachments/assets/ec4d6f6e-7558-4430-813b-61a6e911c3b0" />| Reviewed By: javache Differential Revision: D108877387 Pulled By: j-piasecki fbshipit-source-id: f4c5335dc691d30a50db7e6f66744fa9c7295fb2
1 parent faf0951 commit 45904c8

14 files changed

Lines changed: 38 additions & 94 deletions

packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ void YogaLayoutableShadowNode::updateYogaProps() {
480480

481481
void YogaLayoutableShadowNode::configureYogaTree(
482482
float pointScaleFactor,
483+
Float fontSizeMultiplier,
483484
YGErrata defaultErrata,
484485
bool swapLeftAndRight) {
485486
ensureUnsealed();
@@ -489,6 +490,18 @@ void YogaLayoutableShadowNode::configureYogaTree(
489490
YGConfigSetErrata(&yogaConfig_, errata);
490491
YGConfigSetPointScaleFactor(&yogaConfig_, pointScaleFactor);
491492

493+
// A measurable node's measurement depends on `fontSizeMultiplier`, but unlike
494+
// `pointScaleFactor` it is not part of the Yoga config, so a change does not
495+
// invalidate Yoga's layout cache. Dirty the node when it changes to force
496+
// re-measurement. We propagate up to the root so an unchanged, still-cached
497+
// ancestor isn't skipped by `calculateLayoutInternal` before reaching us.
498+
if (ReactNativeFeatureFlags::enableFontScaleChangesUpdatingLayout() &&
499+
getTraits().check(ShadowNodeTraits::Trait::MeasurableYogaNode) &&
500+
!floatEquality(
501+
getLayoutMetrics().fontSizeMultiplier, fontSizeMultiplier)) {
502+
yogaNode_.markDirtyAndPropagate();
503+
}
504+
492505
// TODO: `swapLeftAndRight` modified backing props and cannot be undone
493506
if (swapLeftAndRight) {
494507
swapStyleLeftAndRight();
@@ -507,6 +520,8 @@ void YogaLayoutableShadowNode::configureYogaTree(
507520

508521
if (child.yogaTreeHasBeenConfigured_ &&
509522
childLayoutMetrics.pointScaleFactor == pointScaleFactor &&
523+
floatEquality(
524+
childLayoutMetrics.fontSizeMultiplier, fontSizeMultiplier) &&
510525
childLayoutMetrics.wasLeftAndRightSwapped == swapLeftAndRight &&
511526
childErrata == child.resolveErrata(errata)) {
512527
continue;
@@ -515,10 +530,13 @@ void YogaLayoutableShadowNode::configureYogaTree(
515530
if (doesOwn(child)) {
516531
auto& mutableChild = const_cast<YogaLayoutableShadowNode&>(child);
517532
mutableChild.configureYogaTree(
518-
pointScaleFactor, child.resolveErrata(errata), swapLeftAndRight);
533+
pointScaleFactor,
534+
fontSizeMultiplier,
535+
child.resolveErrata(errata),
536+
swapLeftAndRight);
519537
} else {
520538
cloneChildInPlace(i).configureYogaTree(
521-
pointScaleFactor, errata, swapLeftAndRight);
539+
pointScaleFactor, fontSizeMultiplier, errata, swapLeftAndRight);
522540
}
523541
}
524542
}
@@ -627,6 +645,7 @@ void YogaLayoutableShadowNode::layoutTree(
627645
TraceSection s2("YogaLayoutableShadowNode::configureYogaTree");
628646
configureYogaTree(
629647
layoutContext.pointScaleFactor,
648+
layoutContext.fontSizeMultiplier,
630649
YGErrataAll /*defaultErrata*/,
631650
swapLeftAndRight);
632651
}
@@ -692,6 +711,7 @@ void YogaLayoutableShadowNode::layoutTree(
692711
if (yogaNode_.getHasNewLayout()) {
693712
auto layoutMetrics = layoutMetricsFromYogaNode(yogaNode_);
694713
layoutMetrics.pointScaleFactor = layoutContext.pointScaleFactor;
714+
layoutMetrics.fontSizeMultiplier = layoutContext.fontSizeMultiplier;
695715
layoutMetrics.wasLeftAndRightSwapped = swapLeftAndRight;
696716
setLayoutMetrics(layoutMetrics);
697717
yogaNode_.setHasNewLayout(false);
@@ -739,6 +759,7 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {
739759

740760
auto newLayoutMetrics = layoutMetricsFromYogaNode(*childYogaNode);
741761
newLayoutMetrics.pointScaleFactor = layoutContext.pointScaleFactor;
762+
newLayoutMetrics.fontSizeMultiplier = layoutContext.fontSizeMultiplier;
742763
newLayoutMetrics.wasLeftAndRightSwapped =
743764
layoutContext.swapLeftAndRightInRTL &&
744765
newLayoutMetrics.layoutDirection == LayoutDirection::RightToLeft;

packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode {
134134
* ShadowTree has been constructed, but before it has been is laid out or
135135
* committed.
136136
*/
137-
void configureYogaTree(float pointScaleFactor, YGErrata defaultErrata, bool swapLeftAndRight);
137+
void
138+
configureYogaTree(float pointScaleFactor, Float fontSizeMultiplier, YGErrata defaultErrata, bool swapLeftAndRight);
138139

139140
/**
140141
* Return an errata based on a `layoutConformance` prop if given, otherwise

packages/react-native/ReactCommon/react/renderer/core/LayoutMetrics.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ struct LayoutMetrics {
4040
bool wasLeftAndRightSwapped{false};
4141
// Pixel density. Number of device pixels per density-independent pixel.
4242
Float pointScaleFactor{1.0};
43+
// Surface font scale this node was last laid out with.
44+
Float fontSizeMultiplier{1.0};
4345
// How much the children of the node actually overflow in each direction.
4446
// Positive values indicate that children are overflowing outside of the node.
4547
// Negative values indicate that children are clipped inside the node
@@ -120,6 +122,7 @@ struct hash<facebook::react::LayoutMetrics> {
120122
layoutMetrics.displayType,
121123
layoutMetrics.layoutDirection,
122124
layoutMetrics.pointScaleFactor,
125+
layoutMetrics.fontSizeMultiplier,
123126
layoutMetrics.overflowInset);
124127
}
125128
};

packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp

Lines changed: 1 addition & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
#include <cxxreact/TraceSection.h>
1111
#include <react/debug/react_native_assert.h>
12-
#include <react/featureflags/ReactNativeFeatureFlags.h>
1312
#include <react/renderer/uimanager/UIManager.h>
1413

1514
namespace facebook::react {
@@ -213,78 +212,6 @@ Size SurfaceHandler::measure(
213212
return rootShadowNode->getLayoutMetrics().frame.size;
214213
}
215214

216-
std::shared_ptr<const ShadowNode> SurfaceHandler::dirtyMeasurableNodesRecursive(
217-
std::shared_ptr<const ShadowNode> node) const {
218-
const auto nodeHasChildren = !node->getChildren().empty();
219-
const auto isMeasurableYogaNode =
220-
node->getTraits().check(ShadowNodeTraits::Trait::MeasurableYogaNode);
221-
222-
// Node is not measurable and has no children, its layout will not be affected
223-
if (!nodeHasChildren && !isMeasurableYogaNode) {
224-
return nullptr;
225-
}
226-
227-
std::shared_ptr<const std::vector<std::shared_ptr<const ShadowNode>>>
228-
newChildren = ShadowNodeFragment::childrenPlaceholder();
229-
230-
if (nodeHasChildren) {
231-
std::shared_ptr<std::vector<std::shared_ptr<const ShadowNode>>>
232-
newChildrenMutable = nullptr;
233-
for (size_t i = 0; i < node->getChildren().size(); i++) {
234-
const auto& child = node->getChildren()[i];
235-
236-
if (const auto& layoutableNode =
237-
std::dynamic_pointer_cast<const YogaLayoutableShadowNode>(
238-
child)) {
239-
auto newChild = dirtyMeasurableNodesRecursive(layoutableNode);
240-
241-
if (newChild != nullptr) {
242-
if (newChildrenMutable == nullptr) {
243-
newChildrenMutable = std::make_shared<
244-
std::vector<std::shared_ptr<const ShadowNode>>>(
245-
node->getChildren());
246-
newChildren = newChildrenMutable;
247-
}
248-
249-
(*newChildrenMutable)[i] = newChild;
250-
}
251-
}
252-
}
253-
254-
// Node is not measurable and its children were not dirtied, its layout will
255-
// not be affected
256-
if (!isMeasurableYogaNode && newChildrenMutable == nullptr) {
257-
return nullptr;
258-
}
259-
}
260-
261-
const auto newNode = node->getComponentDescriptor().cloneShadowNode(
262-
*node,
263-
{
264-
.children = newChildren,
265-
// Preserve the original state of the node
266-
.state = node->getState(),
267-
});
268-
269-
if (isMeasurableYogaNode) {
270-
std::static_pointer_cast<YogaLayoutableShadowNode>(newNode)->dirtyLayout();
271-
}
272-
273-
return newNode;
274-
}
275-
276-
void SurfaceHandler::dirtyMeasurableNodes(ShadowNode& root) const {
277-
for (const auto& child : root.getChildren()) {
278-
if (const auto& layoutableNode =
279-
std::dynamic_pointer_cast<const YogaLayoutableShadowNode>(child)) {
280-
const auto newChild = dirtyMeasurableNodesRecursive(layoutableNode);
281-
if (newChild != nullptr) {
282-
root.replaceChild(*child, newChild);
283-
}
284-
}
285-
}
286-
}
287-
288215
void SurfaceHandler::constraintLayout(
289216
const LayoutConstraints& layoutConstraints,
290217
const LayoutContext& layoutContext) const {
@@ -315,19 +242,8 @@ void SurfaceHandler::constraintLayout(
315242
link_.shadowTree && "`link_.shadowTree` must not be null.");
316243
link_.shadowTree->commit(
317244
[&](const RootShadowNode& oldRootShadowNode) {
318-
auto newRoot = oldRootShadowNode.clone(
245+
return oldRootShadowNode.clone(
319246
propsParserContext, layoutConstraints, layoutContext);
320-
321-
// Dirty all measurable nodes when the fontSizeMultiplier changes to
322-
// trigger re-measurement.
323-
if (ReactNativeFeatureFlags::enableFontScaleChangesUpdatingLayout() &&
324-
layoutContext.fontSizeMultiplier !=
325-
oldRootShadowNode.getConcreteProps()
326-
.layoutContext.fontSizeMultiplier) {
327-
dirtyMeasurableNodes(*newRoot);
328-
}
329-
330-
return newRoot;
331247
},
332248
{/* default commit options */});
333249
}

packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,6 @@ class SurfaceHandler {
151151

152152
void applyDisplayMode(DisplayMode displayMode) const;
153153

154-
/*
155-
* An utility for dirtying all measurable shadow nodes present in the tree.
156-
*/
157-
void dirtyMeasurableNodes(ShadowNode &root) const;
158-
std::shared_ptr<const ShadowNode> dirtyMeasurableNodesRecursive(std::shared_ptr<const ShadowNode> node) const;
159-
160154
#pragma mark - Link & Parameters
161155

162156
/*

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7415,6 +7415,7 @@ struct facebook::react::LayoutMetrics {
74157415
public facebook::react::EdgeInsets borderWidth;
74167416
public facebook::react::EdgeInsets contentInsets;
74177417
public facebook::react::EdgeInsets overflowInset;
7418+
public facebook::react::Float fontSizeMultiplier;
74187419
public facebook::react::Float pointScaleFactor;
74197420
public facebook::react::LayoutDirection layoutDirection;
74207421
public facebook::react::PositionType positionType;

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7198,6 +7198,7 @@ struct facebook::react::LayoutMetrics {
71987198
public facebook::react::EdgeInsets borderWidth;
71997199
public facebook::react::EdgeInsets contentInsets;
72007200
public facebook::react::EdgeInsets overflowInset;
7201+
public facebook::react::Float fontSizeMultiplier;
72017202
public facebook::react::Float pointScaleFactor;
72027203
public facebook::react::LayoutDirection layoutDirection;
72037204
public facebook::react::PositionType positionType;

scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7406,6 +7406,7 @@ struct facebook::react::LayoutMetrics {
74067406
public facebook::react::EdgeInsets borderWidth;
74077407
public facebook::react::EdgeInsets contentInsets;
74087408
public facebook::react::EdgeInsets overflowInset;
7409+
public facebook::react::Float fontSizeMultiplier;
74097410
public facebook::react::Float pointScaleFactor;
74107411
public facebook::react::LayoutDirection layoutDirection;
74117412
public facebook::react::PositionType positionType;

scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9496,6 +9496,7 @@ struct facebook::react::LayoutMetrics {
94969496
public facebook::react::EdgeInsets borderWidth;
94979497
public facebook::react::EdgeInsets contentInsets;
94989498
public facebook::react::EdgeInsets overflowInset;
9499+
public facebook::react::Float fontSizeMultiplier;
94999500
public facebook::react::Float pointScaleFactor;
95009501
public facebook::react::LayoutDirection layoutDirection;
95019502
public facebook::react::PositionType positionType;

scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9331,6 +9331,7 @@ struct facebook::react::LayoutMetrics {
93319331
public facebook::react::EdgeInsets borderWidth;
93329332
public facebook::react::EdgeInsets contentInsets;
93339333
public facebook::react::EdgeInsets overflowInset;
9334+
public facebook::react::Float fontSizeMultiplier;
93349335
public facebook::react::Float pointScaleFactor;
93359336
public facebook::react::LayoutDirection layoutDirection;
93369337
public facebook::react::PositionType positionType;

0 commit comments

Comments
 (0)