Breaking: Always use AttributedStringBox instead of AttributedString …

…in TextLayoutManager (#46104) Summary: Pull Request resolved: #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 Reviewed By: joevilches Differential Revision: D61484999 fbshipit-source-id: 07c5600cd917f2dab3d24559a25f27e0872ebddc
facebook · Aug 21, 2024 · ee597bf · ee597bf
1 parent 3cd6d18
commit ee597bf
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 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
@@ -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/...latform/android/react/renderer/components/androidtextinput/AndroidTextInputShadowNode.cpp b/...latform/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/...nts/textinput/platform/ios/react/renderer/components/iostextinput/TextInputShadowNode.cpp b/...nts/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/...textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.cpp b/...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,18 +228,22 @@ Float TextLayoutManager::baseline(
 }
 
 TextMeasurement TextLayoutManager::doMeasure(
-    AttributedString attributedString,
+    const AttributedString& attributedString,
     const ParagraphAttributes& paragraphAttributes,
-    LayoutConstraints layoutConstraints) const {
-  layoutConstraints.maximumSize.height = std::numeric_limits<Float>::infinity();
-
+    const LayoutConstraints& layoutConstraints) const {
   const int attachmentCount = countAttachments(attributedString);
   auto env = Environment::current();
   auto attachmentPositions = env->NewFloatArray(attachmentCount * 2);
 
   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<Float>::infinity();
+
   auto attributedStringMap = toMapBuffer(attributedString);
   auto paragraphAttributesMap = toMapBuffer(paragraphAttributes);
 

diff --git a/...r/textlayoutmanager/platform/android/react/renderer/textlayoutmanager/TextLayoutManager.h b/...r/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/...ct-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.cpp b/...ct-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/...eact-native/ReactCommon/react/renderer/textlayoutmanager/platform/cxx/TextLayoutManager.h b/...eact-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/...derer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.h b/...derer/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/...erer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm b/...erer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/TextLayoutManager.mm
@@ -25,10 +25,10 @@
 }
 
 TextMeasurement TextLayoutManager::measure(
-    AttributedStringBox attributedStringBox,
-    ParagraphAttributes paragraphAttributes,
+    const AttributedStringBox &attributedStringBox,
+    const ParagraphAttributes &paragraphAttributes,
     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 &paragraphAttributes,
+    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 &paragraphAttributes,
+    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;