Skip to content

Commit b9c9bd1

Browse files
jehartzogfacebook-github-bot
jehartzog
authored and
facebook-github-bot
committed
Fix stale text measurement after display density change
Summary: Changelog: [General][Fixed] - Fix text measurements being incorrectly reused across pixel density changes Differential Revision: D107594582
1 parent 87184c8 commit b9c9bd1

13 files changed

Lines changed: 96 additions & 7 deletions

File tree

‎packages/react-native/ReactCommon/react/renderer/textlayoutmanager/TextMeasureCache.h‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ class TextMeasureCacheKey final {
6666
AttributedString attributedString{};
6767
ParagraphAttributes paragraphAttributes{};
6868
LayoutConstraints layoutConstraints{};
69+
// The measured size depends on the pixel scale factor because layout metrics
70+
// are rounded to the pixel grid. Two otherwise-identical measures at different
71+
// densities are not interchangeable, so the scale factor is part of the key.
72+
Float pointScaleFactor{};
6973
};
7074

7175
// The Key type that is used for Line Measure Cache.
@@ -87,6 +91,9 @@ class PreparedTextCacheKey final {
8791
AttributedString attributedString{};
8892
ParagraphAttributes paragraphAttributes{};
8993
LayoutConstraints layoutConstraints{};
94+
// A prepared layout is rounded to the pixel grid, so it is only reusable at
95+
// the pixel scale factor it was laid out at.
96+
Float pointScaleFactor{};
9097
};
9198

9299
/*
@@ -252,7 +259,8 @@ inline size_t attributedStringHashDisplayWise(const AttributedString &attributed
252259
inline bool operator==(const TextMeasureCacheKey &lhs, const TextMeasureCacheKey &rhs)
253260
{
254261
return areAttributedStringsEquivalentLayoutWise(lhs.attributedString, rhs.attributedString) &&
255-
lhs.paragraphAttributes == rhs.paragraphAttributes && lhs.layoutConstraints == rhs.layoutConstraints;
262+
lhs.paragraphAttributes == rhs.paragraphAttributes && lhs.layoutConstraints == rhs.layoutConstraints &&
263+
floatEquality(lhs.pointScaleFactor, rhs.pointScaleFactor);
256264
}
257265

258266
inline bool operator==(const LineMeasureCacheKey &lhs, const LineMeasureCacheKey &rhs)
@@ -264,7 +272,8 @@ inline bool operator==(const LineMeasureCacheKey &lhs, const LineMeasureCacheKey
264272
inline bool operator==(const PreparedTextCacheKey &lhs, const PreparedTextCacheKey &rhs)
265273
{
266274
return areAttributedStringsEquivalentDisplayWise(lhs.attributedString, rhs.attributedString) &&
267-
lhs.paragraphAttributes == rhs.paragraphAttributes && lhs.layoutConstraints == rhs.layoutConstraints;
275+
lhs.paragraphAttributes == rhs.paragraphAttributes && lhs.layoutConstraints == rhs.layoutConstraints &&
276+
floatEquality(lhs.pointScaleFactor, rhs.pointScaleFactor);
268277
}
269278

270279
} // namespace facebook::react
@@ -276,7 +285,10 @@ struct hash<facebook::react::TextMeasureCacheKey> {
276285
size_t operator()(const facebook::react::TextMeasureCacheKey &key) const
277286
{
278287
return facebook::react::hash_combine(
279-
attributedStringHashLayoutWise(key.attributedString), key.paragraphAttributes, key.layoutConstraints);
288+
attributedStringHashLayoutWise(key.attributedString),
289+
key.paragraphAttributes,
290+
key.layoutConstraints,
291+
key.pointScaleFactor);
280292
}
281293
};
282294

@@ -294,7 +306,10 @@ struct hash<facebook::react::PreparedTextCacheKey> {
294306
size_t operator()(const facebook::react::PreparedTextCacheKey &key) const
295307
{
296308
return facebook::react::hash_combine(
297-
attributedStringHashDisplayWise(key.attributedString), key.paragraphAttributes, key.layoutConstraints);
309+
attributedStringHashDisplayWise(key.attributedString),
310+
key.paragraphAttributes,
311+
key.layoutConstraints,
312+
key.pointScaleFactor);
298313
}
299314
};
300315

‎packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ TextMeasurement TextLayoutManager::measure(
193193
: textMeasureCache_.get(
194194
{.attributedString = attributedString,
195195
.paragraphAttributes = paragraphAttributes,
196-
.layoutConstraints = layoutConstraints},
196+
.layoutConstraints = layoutConstraints,
197+
.pointScaleFactor = layoutContext.pointScaleFactor},
197198
std::move(measureText));
198199

199200
measurement.size = layoutConstraints.clamp(measurement.size);
@@ -315,7 +316,8 @@ TextLayoutManager::PreparedTextLayout TextLayoutManager::prepareLayout(
315316
const auto [key, preparedText] = preparedTextCache_.getWithKey(
316317
{.attributedString = attributedString,
317318
.paragraphAttributes = paragraphAttributes,
318-
.layoutConstraints = layoutConstraints},
319+
.layoutConstraints = layoutConstraints,
320+
.pointScaleFactor = layoutContext.pointScaleFactor},
319321
[&]() {
320322
const auto& fabricUIManager =
321323
contextContainer_->at<jni::global_ref<jobject>>("FabricUIManager");

‎packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
measurement = textMeasureCache_.get(
4444
{.attributedString = attributedString,
4545
.paragraphAttributes = paragraphAttributes,
46-
.layoutConstraints = layoutConstraints},
46+
.layoutConstraints = layoutConstraints,
47+
.pointScaleFactor = layoutContext.pointScaleFactor},
4748
[&]() {
4849
auto telemetry = TransactionTelemetry::threadLocalTelemetry();
4950
if (telemetry) {

‎packages/react-native/ReactCommon/react/renderer/textlayoutmanager/tests/TextLayoutManagerTest.cpp‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,56 @@ TEST(TextLayoutManagerTest, maxFontSizeMultiplierAffectsLayoutCacheHash) {
4444
EXPECT_NE(
4545
textAttributesHashLayoutWise(lhs), textAttributesHashLayoutWise(rhs));
4646
}
47+
48+
// Measurements are rounded to the pixel grid, so a measurement cached at one
49+
// pixel scale factor must not satisfy a lookup at another. Keys that differ
50+
// only by pointScaleFactor must compare unequal.
51+
TEST(TextLayoutManagerTest, pointScaleFactorAffectsTextMeasureCacheEquality) {
52+
TextMeasureCacheKey lhs;
53+
TextMeasureCacheKey rhs;
54+
55+
lhs.pointScaleFactor = 2.0;
56+
rhs.pointScaleFactor = 1.6;
57+
EXPECT_FALSE(lhs == rhs);
58+
59+
rhs.pointScaleFactor = 2.0;
60+
EXPECT_TRUE(lhs == rhs);
61+
}
62+
63+
TEST(TextLayoutManagerTest, pointScaleFactorAffectsTextMeasureCacheHash) {
64+
TextMeasureCacheKey lhs;
65+
TextMeasureCacheKey rhs;
66+
67+
lhs.pointScaleFactor = 2.0;
68+
rhs.pointScaleFactor = 1.6;
69+
70+
EXPECT_NE(
71+
std::hash<TextMeasureCacheKey>{}(lhs),
72+
std::hash<TextMeasureCacheKey>{}(rhs));
73+
}
74+
75+
// Same invariant for the prepared-text cache: a prepared layout is pixel-grid
76+
// rounded and is only reusable at the pixel scale factor it was prepared at.
77+
TEST(TextLayoutManagerTest, pointScaleFactorAffectsPreparedTextCacheEquality) {
78+
PreparedTextCacheKey lhs;
79+
PreparedTextCacheKey rhs;
80+
81+
lhs.pointScaleFactor = 2.0;
82+
rhs.pointScaleFactor = 1.6;
83+
EXPECT_FALSE(lhs == rhs);
84+
85+
rhs.pointScaleFactor = 2.0;
86+
EXPECT_TRUE(lhs == rhs);
87+
}
88+
89+
TEST(TextLayoutManagerTest, pointScaleFactorAffectsPreparedTextCacheHash) {
90+
PreparedTextCacheKey lhs;
91+
PreparedTextCacheKey rhs;
92+
93+
lhs.pointScaleFactor = 2.0;
94+
rhs.pointScaleFactor = 1.6;
95+
96+
EXPECT_NE(
97+
std::hash<PreparedTextCacheKey>{}(lhs),
98+
std::hash<PreparedTextCacheKey>{}(rhs));
99+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4129,6 +4129,7 @@ class facebook::react::PointerHoverTracker {
41294129

41304130
class facebook::react::PreparedTextCacheKey {
41314131
public facebook::react::AttributedString attributedString;
4132+
public facebook::react::Float pointScaleFactor;
41324133
public facebook::react::LayoutConstraints layoutConstraints;
41334134
public facebook::react::ParagraphAttributes paragraphAttributes;
41344135
}
@@ -5064,6 +5065,7 @@ class facebook::react::TextLayoutManager {
50645065

50655066
class facebook::react::TextMeasureCacheKey {
50665067
public facebook::react::AttributedString attributedString;
5068+
public facebook::react::Float pointScaleFactor;
50675069
public facebook::react::LayoutConstraints layoutConstraints;
50685070
public facebook::react::ParagraphAttributes paragraphAttributes;
50695071
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3973,6 +3973,7 @@ class facebook::react::PointerHoverTracker {
39733973

39743974
class facebook::react::PreparedTextCacheKey {
39753975
public facebook::react::AttributedString attributedString;
3976+
public facebook::react::Float pointScaleFactor;
39763977
public facebook::react::LayoutConstraints layoutConstraints;
39773978
public facebook::react::ParagraphAttributes paragraphAttributes;
39783979
}
@@ -4890,6 +4891,7 @@ class facebook::react::TextLayoutManager {
48904891

48914892
class facebook::react::TextMeasureCacheKey {
48924893
public facebook::react::AttributedString attributedString;
4894+
public facebook::react::Float pointScaleFactor;
48934895
public facebook::react::LayoutConstraints layoutConstraints;
48944896
public facebook::react::ParagraphAttributes paragraphAttributes;
48954897
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4126,6 +4126,7 @@ class facebook::react::PointerHoverTracker {
41264126

41274127
class facebook::react::PreparedTextCacheKey {
41284128
public facebook::react::AttributedString attributedString;
4129+
public facebook::react::Float pointScaleFactor;
41294130
public facebook::react::LayoutConstraints layoutConstraints;
41304131
public facebook::react::ParagraphAttributes paragraphAttributes;
41314132
}
@@ -5055,6 +5056,7 @@ class facebook::react::TextLayoutManager {
50555056

50565057
class facebook::react::TextMeasureCacheKey {
50575058
public facebook::react::AttributedString attributedString;
5059+
public facebook::react::Float pointScaleFactor;
50585060
public facebook::react::LayoutConstraints layoutConstraints;
50595061
public facebook::react::ParagraphAttributes paragraphAttributes;
50605062
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6316,6 +6316,7 @@ class facebook::react::PointerHoverTracker {
63166316

63176317
class facebook::react::PreparedTextCacheKey {
63186318
public facebook::react::AttributedString attributedString;
6319+
public facebook::react::Float pointScaleFactor;
63196320
public facebook::react::LayoutConstraints layoutConstraints;
63206321
public facebook::react::ParagraphAttributes paragraphAttributes;
63216322
}
@@ -7283,6 +7284,7 @@ class facebook::react::TextLayoutManager {
72837284

72847285
class facebook::react::TextMeasureCacheKey {
72857286
public facebook::react::AttributedString attributedString;
7287+
public facebook::react::Float pointScaleFactor;
72867288
public facebook::react::LayoutConstraints layoutConstraints;
72877289
public facebook::react::ParagraphAttributes paragraphAttributes;
72887290
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6188,6 +6188,7 @@ class facebook::react::PointerHoverTracker {
61886188

61896189
class facebook::react::PreparedTextCacheKey {
61906190
public facebook::react::AttributedString attributedString;
6191+
public facebook::react::Float pointScaleFactor;
61916192
public facebook::react::LayoutConstraints layoutConstraints;
61926193
public facebook::react::ParagraphAttributes paragraphAttributes;
61936194
}
@@ -7137,6 +7138,7 @@ class facebook::react::TextLayoutManager {
71377138

71387139
class facebook::react::TextMeasureCacheKey {
71397140
public facebook::react::AttributedString attributedString;
7141+
public facebook::react::Float pointScaleFactor;
71407142
public facebook::react::LayoutConstraints layoutConstraints;
71417143
public facebook::react::ParagraphAttributes paragraphAttributes;
71427144
}

‎scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6313,6 +6313,7 @@ class facebook::react::PointerHoverTracker {
63136313

63146314
class facebook::react::PreparedTextCacheKey {
63156315
public facebook::react::AttributedString attributedString;
6316+
public facebook::react::Float pointScaleFactor;
63166317
public facebook::react::LayoutConstraints layoutConstraints;
63176318
public facebook::react::ParagraphAttributes paragraphAttributes;
63186319
}
@@ -7274,6 +7275,7 @@ class facebook::react::TextLayoutManager {
72747275

72757276
class facebook::react::TextMeasureCacheKey {
72767277
public facebook::react::AttributedString attributedString;
7278+
public facebook::react::Float pointScaleFactor;
72777279
public facebook::react::LayoutConstraints layoutConstraints;
72787280
public facebook::react::ParagraphAttributes paragraphAttributes;
72797281
}

0 commit comments

Comments
 (0)