From 1c52eec2c6daa74af7a79e9d7239e2f666a64c6f Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 8 May 2025 14:41:50 -0700 Subject: [PATCH] Ensure that ShadowNode measure functions respect constraints (#51180) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51180 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: rshest Differential Revision: D74291373 fbshipit-source-id: 44166f2e47323384cb00f3cf4c32f398e298a63e --- .../react/views/text/TextLayoutManager.java | 2 +- .../view/YogaLayoutableShadowNode.cpp | 17 +++++++++++++++++ .../textlayoutmanager/TextLayoutManager.cpp | 7 +++++-- .../ReactCommon/react/utils/FloatComparison.h | 4 +++- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java index 6ab6673a178..e42d79c248d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java @@ -825,7 +825,7 @@ public class TextLayoutManager { // where the container is measured smaller than text. Math.ceil prevents it // See T136756103 for investigation if (android.os.Build.VERSION.SDK_INT > Build.VERSION_CODES.Q) { - calculatedWidth = (float) Math.ceil(calculatedWidth); + calculatedWidth = Math.min((float) Math.ceil(calculatedWidth), width); } return calculatedWidth; } 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 0abf41a8854..a0d9aba0f32 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -15,9 +15,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -837,6 +839,21 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector( auto size = shadowNode.measureContent( threadLocalLayoutContext, {minimumSize, maximumSize}); +#ifdef REACT_NATIVE_DEBUG + bool widthInBounds = size.width + kDefaultEpsilon >= minimumSize.width && + size.width - kDefaultEpsilon <= maximumSize.width; + bool heightInBounds = size.height + kDefaultEpsilon >= minimumSize.height && + size.height - kDefaultEpsilon <= maximumSize.height; + + if (!widthInBounds || !heightInBounds) { + LOG(FATAL) << shadowNode.getComponentDescriptor().getComponentName() + << " returned in invalid measurement. Min: [" + << minimumSize.width << "," << minimumSize.height << "] Max: [" + << maximumSize.width << "," << maximumSize.height + << "] Actual: [" << size.width << "," << size.height << "]"; + } +#endif + return YGSize{ yogaFloatFromFloat(size.width), yogaFloatFromFloat(size.height)}; } diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/react/renderer/textlayoutmanager/TextLayoutManager.cpp b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/react/renderer/textlayoutmanager/TextLayoutManager.cpp index 9fa1cec511f..cef69312beb 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/react/renderer/textlayoutmanager/TextLayoutManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/react/renderer/textlayoutmanager/TextLayoutManager.cpp @@ -17,7 +17,7 @@ TextMeasurement TextLayoutManager::measure( const AttributedStringBox& attributedStringBox, const ParagraphAttributes& /*paragraphAttributes*/, const TextLayoutContext& /*layoutContext*/, - const LayoutConstraints& /*layoutConstraints*/) const { + const LayoutConstraints& layoutConstraints) const { TextMeasurement::Attachments attachments; for (const auto& fragment : attributedStringBox.getValue().getFragments()) { if (fragment.isAttachment()) { @@ -25,7 +25,10 @@ TextMeasurement TextLayoutManager::measure( TextMeasurement::Attachment{{{0, 0}, {0, 0}}, false}); } } - return TextMeasurement{{0, 0}, attachments}; + return TextMeasurement{ + {layoutConstraints.minimumSize.width, + layoutConstraints.minimumSize.height}, + attachments}; } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/utils/FloatComparison.h b/packages/react-native/ReactCommon/react/utils/FloatComparison.h index d760cef1cdf..3e60643a9c3 100644 --- a/packages/react-native/ReactCommon/react/utils/FloatComparison.h +++ b/packages/react-native/ReactCommon/react/utils/FloatComparison.h @@ -9,7 +9,9 @@ namespace facebook::react { -inline bool floatEquality(float a, float b, float epsilon = 0.005f) { +constexpr float kDefaultEpsilon = 0.005f; + +inline bool floatEquality(float a, float b, float epsilon = kDefaultEpsilon) { return (std::isnan(a) && std::isnan(b)) || (!std::isnan(a) && !std::isnan(b) && fabs(a - b) < epsilon); }