From ccf7da7d95d0e71f5a84f3de47ed4fc6549a8633 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Mon, 19 Aug 2024 16:41:08 -0700 Subject: [PATCH] Breaking: Always use AttributedStringBox instead of AttributedString in TextLayoutManager (#46104) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46104 We want to use `PrecomputedText` to store glyph-level measurements on underlying Android Spannable. This means we need to consistently reuse the same Spannable, instead of recreating them on measurement. We have an opaque cache ID used by Android, for spannables originating from uncontrolled TextInput on UI-thread side. We also have `AttributedStringBox`, for a kind of similar purpose on iOS, which allows passing opaque pointer to the `TextLayoutManager`. This is only used for the `measure` function. This change makes us consistently use `AttributedStringBox` at the TextLayoutManager boundary, to let us migrate calls across TextLayoutManager to all pass opaque handle to underlying Spannable we will store, instead of passing the AttributedString each time. For now, every place previously passing an AttributedString value still passes one. There were also some egregious cases of accepting very large structures by value, causing unneeded copies. I changed the APIs to accept anything potentially larger than two pointers to pass by reference instead. This change is technically breaking, to any 3p code calling into TextLayoutManager (IIRC live-markdown exposed prefabs for this, but should be able to adapt fairly easily). Changelog: [General][Breaking] - Always use AttributedStringBox instead of AttributedString in TextLayoutManager Differential Revision: D61484999 --- .../components/text/ParagraphShadowNode.cpp | 9 +++-- .../AndroidTextInputShadowNode.cpp | 5 ++- .../iostextinput/TextInputShadowNode.cpp | 3 +- .../textlayoutmanager/TextLayoutManager.cpp | 33 ++++++++++++------- .../textlayoutmanager/TextLayoutManager.h | 18 +++++----- .../platform/cxx/TextLayoutManager.cpp | 20 +++++------ .../platform/cxx/TextLayoutManager.h | 20 +++++------ .../textlayoutmanager/TextLayoutManager.h | 18 +++++----- .../textlayoutmanager/TextLayoutManager.mm | 23 ++++++++----- 9 files changed, 85 insertions(+), 64 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp index 49da5333107c96..95b2e2de19f71f 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/text/ParagraphShadowNode.cpp @@ -184,8 +184,9 @@ Float ParagraphShadowNode::baseline( attributedString.appendFragment({string, textAttributes, {}}); } + AttributedStringBox attributedStringBox{attributedString}; return textLayoutManager_->baseline( - attributedString, getConcreteProps().paragraphAttributes, size); + attributedStringBox, getConcreteProps().paragraphAttributes, size); } void ParagraphShadowNode::layout(LayoutContext layoutContext) { @@ -205,9 +206,11 @@ void ParagraphShadowNode::layout(LayoutContext layoutContext) { textLayoutContext.pointScaleFactor = layoutContext.pointScaleFactor; auto measurement = TextMeasurement{}; + AttributedStringBox attributedStringBox{content.attributedString}; + if (getConcreteProps().onTextLayout) { auto linesMeasurements = textLayoutManager_->measureLines( - content.attributedString, content.paragraphAttributes, size); + attributedStringBox, content.paragraphAttributes, size); getConcreteEventEmitter().onTextLayout(linesMeasurements); } @@ -218,7 +221,7 @@ void ParagraphShadowNode::layout(LayoutContext layoutContext) { // Only measure if attachments are not empty. measurement = textLayoutManager_->measure( - AttributedStringBox{content.attributedString}, + attributedStringBox, content.paragraphAttributes, textLayoutContext, layoutConstraints); diff --git a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp index 07ccccee1a710e..694b789ba17dfd 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp @@ -233,8 +233,11 @@ Float AndroidTextInputShadowNode::baseline( auto top = YGNodeLayoutGetBorder(&yogaNode_, YGEdgeTop) + YGNodeLayoutGetPadding(&yogaNode_, YGEdgeTop); + AttributedStringBox attributedStringBox{attributedString}; return textLayoutManager_->baseline( - attributedString, getConcreteProps().paragraphAttributes, size) + + attributedStringBox, + getConcreteProps().paragraphAttributes, + size) + top; } diff --git a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/ios/react/renderer/components/iostextinput/TextInputShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/ios/react/renderer/components/iostextinput/TextInputShadowNode.cpp index 27369209445a65..b6c7a889093f59 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/ios/react/renderer/components/iostextinput/TextInputShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/textinput/platform/ios/react/renderer/components/iostextinput/TextInputShadowNode.cpp @@ -161,8 +161,9 @@ Float TextInputShadowNode::baseline( auto top = YGNodeLayoutGetBorder(&yogaNode_, YGEdgeTop) + YGNodeLayoutGetPadding(&yogaNode_, YGEdgeTop); + AttributedStringBox attributedStringBox{std::move(attributedString)}; return textLayoutManager_->baseline( - attributedString, + attributedStringBox, getConcreteProps().getEffectiveParagraphAttributes(), size) + top; diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp index d7f4d49ac52e61..719c329d53357b 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp @@ -100,7 +100,7 @@ TextMeasurement TextLayoutManager::measure( const AttributedStringBox& attributedStringBox, const ParagraphAttributes& paragraphAttributes, const TextLayoutContext& layoutContext, - LayoutConstraints layoutConstraints) const { + const LayoutConstraints& layoutConstraints) const { auto& attributedString = attributedStringBox.getValue(); auto measurement = textMeasureCache_.get( @@ -128,7 +128,7 @@ TextMeasurement TextLayoutManager::measure( TextMeasurement TextLayoutManager::measureCachedSpannableById( int64_t cacheId, const ParagraphAttributes& paragraphAttributes, - LayoutConstraints layoutConstraints) const { + const LayoutConstraints& layoutConstraints) const { auto env = Environment::current(); auto attachmentPositions = env->NewFloatArray(0); auto minimumSize = layoutConstraints.minimumSize; @@ -162,9 +162,13 @@ TextMeasurement TextLayoutManager::measureCachedSpannableById( } LinesMeasurements TextLayoutManager::measureLines( - const AttributedString& attributedString, + const AttributedStringBox& attributedStringBox, const ParagraphAttributes& paragraphAttributes, - Size size) const { + const Size& size) const { + react_native_assert( + attributedStringBox.getMode() == AttributedStringBox::Mode::Value); + const auto& attributedString = attributedStringBox.getValue(); + auto lineMeasurements = lineMeasureCache_.get( {attributedString, paragraphAttributes, size}, [&](const LineMeasureCacheKey& /*key*/) { @@ -210,10 +214,11 @@ LinesMeasurements TextLayoutManager::measureLines( } Float TextLayoutManager::baseline( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const { - auto lines = this->measureLines(attributedString, paragraphAttributes, size); + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, + const Size& size) const { + auto lines = + this->measureLines(attributedStringBox, paragraphAttributes, size); if (!lines.empty()) { return lines[0].ascender; @@ -223,11 +228,9 @@ Float TextLayoutManager::baseline( } TextMeasurement TextLayoutManager::doMeasure( - AttributedString attributedString, + const AttributedString& attributedString, const ParagraphAttributes& paragraphAttributes, - LayoutConstraints layoutConstraints) const { - layoutConstraints.maximumSize.height = std::numeric_limits::infinity(); - + const LayoutConstraints& layoutConstraints) const { const int attachmentCount = countAttachments(attributedString); auto env = Environment::current(); auto attachmentPositions = env->NewFloatArray(attachmentCount * 2); @@ -235,6 +238,12 @@ TextMeasurement TextLayoutManager::doMeasure( auto minimumSize = layoutConstraints.minimumSize; auto maximumSize = layoutConstraints.maximumSize; + // We assume max height will have no effect on measurement, so we override it + // with a constant value with no constraints, to enable cache reuse later down + // in the stack. + // TODO: This is suss, and not at the right layer + maximumSize.height = std::numeric_limits::infinity(); + auto attributedStringMap = toMapBuffer(attributedString); auto paragraphAttributesMap = toMapBuffer(paragraphAttributes); diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.h index 103962aafab045..398230bd2e288a 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.h +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.h @@ -47,7 +47,7 @@ class TextLayoutManager { const AttributedStringBox& attributedStringBox, const ParagraphAttributes& paragraphAttributes, const TextLayoutContext& layoutContext, - LayoutConstraints layoutConstraints) const; + const LayoutConstraints& layoutConstraints) const; /** * Measures an AttributedString on the platform, as identified by some @@ -56,25 +56,25 @@ class TextLayoutManager { TextMeasurement measureCachedSpannableById( int64_t cacheId, const ParagraphAttributes& paragraphAttributes, - LayoutConstraints layoutConstraints) const; + const LayoutConstraints& layoutConstraints) const; /* * Measures lines of `attributedString` using native text rendering * infrastructure. */ LinesMeasurements measureLines( - const AttributedString& attributedString, + const AttributedStringBox& attributedStringBox, const ParagraphAttributes& paragraphAttributes, - Size size) const; + const Size& size) const; /* * Calculates baseline of `attributedString` using native text rendering * infrastructure. */ Float baseline( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const; + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, + const Size& size) const; /* * Returns an opaque pointer to platform-specific TextLayoutManager. @@ -84,9 +84,9 @@ class TextLayoutManager { private: TextMeasurement doMeasure( - AttributedString attributedString, + const AttributedString& attributedString, const ParagraphAttributes& paragraphAttributes, - LayoutConstraints layoutConstraints) const; + const LayoutConstraints& layoutConstraints) const; void* self_{}; ContextContainer::Shared contextContainer_; diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.cpp b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.cpp index 285c384e9f44d1..c5d867767817ba 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.cpp @@ -14,10 +14,10 @@ void* TextLayoutManager::getNativeTextLayoutManager() const { } TextMeasurement TextLayoutManager::measure( - AttributedStringBox attributedStringBox, - ParagraphAttributes paragraphAttributes, + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& /*paragraphAttributes*/, const TextLayoutContext& /*layoutContext*/, - LayoutConstraints layoutConstraints) const { + const LayoutConstraints& /*layoutConstraints*/) const { TextMeasurement::Attachments attachments; for (const auto& fragment : attributedStringBox.getValue().getFragments()) { if (fragment.isAttachment()) { @@ -31,21 +31,21 @@ TextMeasurement TextLayoutManager::measure( TextMeasurement TextLayoutManager::measureCachedSpannableById( int64_t /*cacheId*/, const ParagraphAttributes& /*paragraphAttributes*/, - LayoutConstraints /*layoutConstraints*/) const { + const LayoutConstraints& /*layoutConstraints*/) const { return {}; } LinesMeasurements TextLayoutManager::measureLines( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const { + const AttributedStringBox& /*attributedStringBox*/, + const ParagraphAttributes& /*paragraphAttributes*/, + const Size& /*size*/) const { return {}; }; Float TextLayoutManager::baseline( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const { + const AttributedStringBox& /*attributedStringBox*/, + const ParagraphAttributes& /*paragraphAttributes*/, + const Size& /*size*/) const { return 0; } diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.h index 180605602ccb99..8a7bb824380d87 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.h +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.h @@ -36,10 +36,10 @@ class TextLayoutManager { * Measures `attributedStringBox` using native text rendering infrastructure. */ virtual TextMeasurement measure( - AttributedStringBox attributedStringBox, - ParagraphAttributes paragraphAttributes, + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, const TextLayoutContext& layoutContext, - LayoutConstraints layoutConstraints) const; + const LayoutConstraints& layoutConstraints) const; /** * Measures an AttributedString on the platform, as identified by some @@ -48,25 +48,25 @@ class TextLayoutManager { virtual TextMeasurement measureCachedSpannableById( int64_t cacheId, const ParagraphAttributes& paragraphAttributes, - LayoutConstraints layoutConstraints) const; + const LayoutConstraints& layoutConstraints) const; /* * Measures lines of `attributedString` using native text rendering * infrastructure. */ virtual LinesMeasurements measureLines( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const; + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, + const Size& size) const; /* * Calculates baseline of `attributedString` using native text rendering * infrastructure. */ virtual Float baseline( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const; + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, + const Size& size) const; /* * Returns an opaque pointer to platform-specific TextLayoutManager. diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.h index 5c380ab41c7a0a..c12277093f2d0d 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.h +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.h @@ -31,28 +31,28 @@ class TextLayoutManager { * Measures `attributedString` using native text rendering infrastructure. */ TextMeasurement measure( - AttributedStringBox attributedStringBox, - ParagraphAttributes paragraphAttributes, + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, const TextLayoutContext& layoutContext, - LayoutConstraints layoutConstraints) const; + const LayoutConstraints& layoutConstraints) const; /* * Measures lines of `attributedString` using native text rendering * infrastructure. */ LinesMeasurements measureLines( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const; + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, + const Size& size) const; /* * Calculates baseline of `attributedString` using native text rendering * infrastructure. */ Float baseline( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const; + const AttributedStringBox& attributedStringBox, + const ParagraphAttributes& paragraphAttributes, + const Size& size) const; /* * Returns an opaque pointer to platform-specific TextLayoutManager. diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm index a36f094d46074f..a34667060b08c6 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm @@ -25,10 +25,10 @@ } TextMeasurement TextLayoutManager::measure( - AttributedStringBox attributedStringBox, - ParagraphAttributes paragraphAttributes, + const AttributedStringBox &attributedStringBox, + const ParagraphAttributes ¶graphAttributes, const TextLayoutContext &layoutContext, - LayoutConstraints layoutConstraints) const + const LayoutConstraints &layoutConstraints) const { RCTTextLayoutManager *textLayoutManager = (RCTTextLayoutManager *)unwrapManagedObject(self_); @@ -85,10 +85,13 @@ } LinesMeasurements TextLayoutManager::measureLines( - AttributedString attributedString, - ParagraphAttributes paragraphAttributes, - Size size) const + const AttributedStringBox &attributedStringBox, + const ParagraphAttributes ¶graphAttributes, + const Size &size) const { + react_native_assert(attributedStringBox.getMode() == AttributedStringBox::Mode::Value); + const auto &attributedString = attributedStringBox.getValue(); + RCTTextLayoutManager *textLayoutManager = (RCTTextLayoutManager *)unwrapManagedObject(self_); auto measurement = @@ -102,10 +105,12 @@ return measurement; } -Float TextLayoutManager::baseline(AttributedString attributedString, ParagraphAttributes paragraphAttributes, Size size) - const +Float TextLayoutManager::baseline( + const AttributedStringBox &attributedStringBox, + const ParagraphAttributes ¶graphAttributes, + const Size &size) const { - auto lines = this->measureLines(attributedString, paragraphAttributes, size); + auto lines = this->measureLines(attributedStringBox, paragraphAttributes, size); if (!lines.empty()) { return lines[0].ascender;