Skip to content

Commit 7f8b656

Browse files
NickGerlemanfacebook-github-bot
authored andcommitted
Resubmit: Ensure that ShadowNode measure functions respect constraints (#51299)
Summary: Pull Request resolved: #51299 ## Resubmit This change was backed out, due to the WinUI Fabric TextLayoutManager not respecting minimum size constraints, causing early debug assert. D74494087 was confirmed to fix this. For more safety, we limit fatal debug assertsions to `ParagraphShadowNode`, and only log a native error for other ShadowNodes. ## Original Android's TextLayoutManager may return widths greather than the max measure constraint. Yoga will clamp these, but this sort of issue points to a logic bug, and creates issues when we are looking at caching text measurements based on constraint reuse. Let's debug assert that we don't do that, and fix a case of rounding up at a pixel boundary, to ensure that it doesn't go above max width. This should theoretically be safe, since Yoga is already doing this clamping, which is what dictates final size of the TextView. Changelog: [Internal] Reviewed By: joevilches Differential Revision: D74674460 fbshipit-source-id: 807d6629bb799a3b1a55e378a3243065055ce219
1 parent 1fe3ff8 commit 7f8b656

4 files changed

Lines changed: 48 additions & 12 deletions

File tree

packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,23 @@
1717
#include <react/renderer/graphics/rounding.h>
1818
#include <react/renderer/telemetry/TransactionTelemetry.h>
1919
#include <react/renderer/textlayoutmanager/TextLayoutContext.h>
20-
#include <react/renderer/textlayoutmanager/TextLayoutManagerExtended.h>
20+
#include <react/utils/FloatComparison.h>
2121

2222
#include <react/renderer/components/text/ParagraphState.h>
2323

24-
namespace facebook::react {
24+
#define assert_valid_size(size, layoutConstraints) \
25+
react_native_assert( \
26+
!ReactNativeFeatureFlags::avoidCeilingAvailableAndroidTextWidth() || \
27+
((size).width + kDefaultEpsilon >= \
28+
(layoutConstraints).minimumSize.width && \
29+
(size).width - kDefaultEpsilon <= \
30+
(layoutConstraints).maximumSize.width && \
31+
(size).height + kDefaultEpsilon >= \
32+
(layoutConstraints).minimumSize.height && \
33+
(size).height - kDefaultEpsilon <= \
34+
(layoutConstraints).maximumSize.height))
2535

36+
namespace facebook::react {
2637
using Content = ParagraphShadowNode::Content;
2738

2839
const char ParagraphComponentName[] = "Paragraph";
@@ -212,17 +223,20 @@ Size ParagraphShadowNode::measureContent(
212223
// PreparedLayout is not trivially copyable on all platforms
213224
// NOLINTNEXTLINE(performance-move-const-arg)
214225
std::move(preparedLayout)});
226+
assert_valid_size(mesaurements.size, layoutConstraints);
215227
return mesaurements.size;
216228
}
217229
}
218230

219-
return textLayoutManager_
220-
->measure(
221-
AttributedStringBox{attributedString},
222-
content.paragraphAttributes,
223-
textLayoutContext,
224-
layoutConstraints)
225-
.size;
231+
auto size = textLayoutManager_
232+
->measure(
233+
AttributedStringBox{attributedString},
234+
content.paragraphAttributes,
235+
textLayoutContext,
236+
layoutConstraints)
237+
.size;
238+
assert_valid_size(size, layoutConstraints);
239+
return size;
226240
}
227241

228242
Float ParagraphShadowNode::baseline(

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
#include <react/renderer/components/view/ViewProps.h>
1616
#include <react/renderer/components/view/ViewShadowNode.h>
1717
#include <react/renderer/components/view/conversions.h>
18+
#include <react/renderer/core/ComponentDescriptor.h>
1819
#include <react/renderer/core/LayoutConstraints.h>
1920
#include <react/renderer/core/LayoutContext.h>
2021
#include <react/renderer/debug/DebugStringConvertibleItem.h>
22+
#include <react/utils/FloatComparison.h>
2123
#include <yoga/Yoga.h>
2224
#include <algorithm>
2325
#include <limits>
@@ -837,6 +839,21 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector(
837839
auto size = shadowNode.measureContent(
838840
threadLocalLayoutContext, {minimumSize, maximumSize});
839841

842+
#ifdef REACT_NATIVE_DEBUG
843+
bool widthInBounds = size.width + kDefaultEpsilon >= minimumSize.width &&
844+
size.width - kDefaultEpsilon <= maximumSize.width;
845+
bool heightInBounds = size.height + kDefaultEpsilon >= minimumSize.height &&
846+
size.height - kDefaultEpsilon <= maximumSize.height;
847+
848+
if (!widthInBounds || !heightInBounds) {
849+
LOG(ERROR) << shadowNode.getComponentDescriptor().getComponentName()
850+
<< " returned an invalid measurement. Min: ["
851+
<< minimumSize.width << "," << minimumSize.height << "] Max: ["
852+
<< maximumSize.width << "," << maximumSize.height
853+
<< "] Actual: [" << size.width << "," << size.height << "]";
854+
}
855+
#endif
856+
840857
return YGSize{
841858
yogaFloatFromFloat(size.width), yogaFloatFromFloat(size.height)};
842859
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,18 @@ TextMeasurement TextLayoutManager::measure(
1717
const AttributedStringBox& attributedStringBox,
1818
const ParagraphAttributes& /*paragraphAttributes*/,
1919
const TextLayoutContext& /*layoutContext*/,
20-
const LayoutConstraints& /*layoutConstraints*/) const {
20+
const LayoutConstraints& layoutConstraints) const {
2121
TextMeasurement::Attachments attachments;
2222
for (const auto& fragment : attributedStringBox.getValue().getFragments()) {
2323
if (fragment.isAttachment()) {
2424
attachments.push_back(
2525
TextMeasurement::Attachment{{{0, 0}, {0, 0}}, false});
2626
}
2727
}
28-
return TextMeasurement{{0, 0}, attachments};
28+
return TextMeasurement{
29+
{layoutConstraints.minimumSize.width,
30+
layoutConstraints.minimumSize.height},
31+
attachments};
2932
}
3033

3134
} // namespace facebook::react

packages/react-native/ReactCommon/react/utils/FloatComparison.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99

1010
namespace facebook::react {
1111

12-
inline bool floatEquality(float a, float b, float epsilon = 0.005f) {
12+
constexpr float kDefaultEpsilon = 0.005f;
13+
14+
inline bool floatEquality(float a, float b, float epsilon = kDefaultEpsilon) {
1315
return (std::isnan(a) && std::isnan(b)) ||
1416
(!std::isnan(a) && !std::isnan(b) && fabs(a - b) < epsilon);
1517
}

0 commit comments

Comments
 (0)