[Impeller] Reland: construct text frames on UI thread. (#46115)

Due to flutter/flutter#127500 , we can get in a state where enable-impeller is true but we're using Skia. We need to either fall back completely to Skia, make this configuration fatal, or remote the check

----------------

Conversion of SkTextBlobs to impeller::TextFrame objects is one of the most expensive operations in display list dispatching. While the rest of the engine and framework makes a reasonable attempt to cache the SkTextBlobs generated during paragraph construction, the design of the dl dispatcher means that these the Impeller backend will always reconstruct all text frames on each frame - even if the display list/picture that contained those text frames was unchanged.

Removing this overhead is one of the goals of #45386 , however this patch is also fairly risky and will be difficult to land. As a more incremental solution, we can instead construct the impeller::TextFrame objects when performing paragraph painting and record them in the display list. This both moves the text frame construction to the UI thread and allows the framework/engine to cache unchanged text frames.

This also does not conflict with the dl_aiks_canvas patch directly, and is fine to land before or after it does. (though I'd argue we should land this first).

To compare the current performance levels, I ran the complex_layout_scroll perf test, since this is fairly text filled. On a Pixel 6 pro. Across several runs this is a fairly consistent ~1ms raster time improvement.

Fixes flutter/flutter#133204

Author: jonahwilliams

display_list/BUILD.gn

@@ -87,6 +87,7 @@ source_set("display_list") {
   public_deps = [
     "//flutter/fml",
     "//flutter/impeller/runtime_stage",
+    "//flutter/impeller/typographer",
     "//third_party/skia",
   ]
 

display_list/benchmarking/dl_complexity_gl.cc

@@ -637,6 +637,11 @@ void DisplayListGLComplexityCalculator::GLHelper::drawTextBlob(
   draw_text_blob_count_++;
 }
 
+void DisplayListGLComplexityCalculator::GLHelper::drawTextFrame(
+    const std::shared_ptr<impeller::TextFrame>& text_frame,
+    SkScalar x,
+    SkScalar y) {}
+
 void DisplayListGLComplexityCalculator::GLHelper::drawShadow(
     const SkPath& path,
     const DlColor color,

display_list/benchmarking/dl_complexity_gl.h

@@ -70,6 +70,9 @@ class DisplayListGLComplexityCalculator
     void drawTextBlob(const sk_sp<SkTextBlob> blob,
                       SkScalar x,
                       SkScalar y) override;
+    void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                       SkScalar x,
+                       SkScalar y) override;
     void drawShadow(const SkPath& path,
                     const DlColor color,
                     const SkScalar elevation,

display_list/benchmarking/dl_complexity_metal.cc

@@ -581,6 +581,11 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawTextBlob(
   draw_text_blob_count_++;
 }
 
+void DisplayListMetalComplexityCalculator::MetalHelper::drawTextFrame(
+    const std::shared_ptr<impeller::TextFrame>& text_frame,
+    SkScalar x,
+    SkScalar y) {}
+
 void DisplayListMetalComplexityCalculator::MetalHelper::drawShadow(
     const SkPath& path,
     const DlColor color,

display_list/benchmarking/dl_complexity_metal.h

@@ -68,6 +68,9 @@ class DisplayListMetalComplexityCalculator
     void drawTextBlob(const sk_sp<SkTextBlob> blob,
                       SkScalar x,
                       SkScalar y) override;
+    void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                       SkScalar x,
+                       SkScalar y) override;
     void drawShadow(const SkPath& path,
                     const DlColor color,
                     const SkScalar elevation,

display_list/display_list.h

@@ -136,6 +136,7 @@ namespace flutter {
                                     \
   V(DrawDisplayList)                \
   V(DrawTextBlob)                   \
+  V(DrawTextFrame)                  \
                                     \
   V(DrawShadow)                     \
   V(DrawShadowTransparentOccluder)

display_list/dl_builder.cc

@@ -1302,6 +1302,48 @@ void DisplayListBuilder::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
   SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawTextBlobFlags);
   drawTextBlob(blob, x, y);
 }
+
+void DisplayListBuilder::drawTextFrame(
+    const std::shared_ptr<impeller::TextFrame>& text_frame,
+    SkScalar x,
+    SkScalar y) {
+  DisplayListAttributeFlags flags = kDrawTextBlobFlags;
+  OpResult result = PaintResult(current_, flags);
+  if (result == OpResult::kNoEffect) {
+    return;
+  }
+  impeller::Rect bounds = text_frame->GetBounds();
+  SkRect sk_bounds = SkRect::MakeLTRB(bounds.GetLeft(), bounds.GetTop(),
+                                      bounds.GetRight(), bounds.GetBottom());
+  bool unclipped = AccumulateOpBounds(sk_bounds.makeOffset(x, y), flags);
+  // TODO(https://github.com/flutter/flutter/issues/82202): Remove once the
+  // unit tests can use Fuchsia's font manager instead of the empty default.
+  // Until then we might encounter empty bounds for otherwise valid text and
+  // thus we ignore the results from AccumulateOpBounds.
+#if defined(OS_FUCHSIA)
+  unclipped = true;
+#endif  // OS_FUCHSIA
+  if (unclipped) {
+    Push<DrawTextFrameOp>(0, 1, text_frame, x, y);
+    // There is no way to query if the glyphs of a text blob overlap and
+    // there are no current guarantees from either Skia or Impeller that
+    // they will protect overlapping glyphs from the effects of overdraw
+    // so we must make the conservative assessment that this DL layer is
+    // not compatible with group opacity inheritance.
+    UpdateLayerOpacityCompatibility(false);
+    UpdateLayerResult(result);
+  }
+}
+
+void DisplayListBuilder::DrawTextFrame(
+    const std::shared_ptr<impeller::TextFrame>& text_frame,
+    SkScalar x,
+    SkScalar y,
+    const DlPaint& paint) {
+  SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawTextBlobFlags);
+  drawTextFrame(text_frame, x, y);
+}
+
 void DisplayListBuilder::DrawShadow(const SkPath& path,
                                     const DlColor color,
                                     const SkScalar elevation,

display_list/dl_builder.h

@@ -224,6 +224,16 @@ class DisplayListBuilder final : public virtual DlCanvas,
                     SkScalar x,
                     SkScalar y,
                     const DlPaint& paint) override;
+
+  void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                     SkScalar x,
+                     SkScalar y) override;
+
+  void DrawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                     SkScalar x,
+                     SkScalar y,
+                     const DlPaint& paint) override;
+
   // |DlCanvas|
   void DrawShadow(const SkPath& path,
                   const DlColor color,

display_list/dl_canvas.h

@@ -18,6 +18,8 @@
 #include "third_party/skia/include/core/SkRect.h"
 #include "third_party/skia/include/core/SkTextBlob.h"
 
+#include "impeller/typographer/text_frame.h"
+
 namespace flutter {
 
 //------------------------------------------------------------------------------
@@ -201,6 +203,13 @@ class DlCanvas {
                          const DlPaint* paint = nullptr) = 0;
   virtual void DrawDisplayList(const sk_sp<DisplayList> display_list,
                                SkScalar opacity = SK_Scalar1) = 0;
+
+  virtual void DrawTextFrame(
+      const std::shared_ptr<impeller::TextFrame>& text_frame,
+      SkScalar x,
+      SkScalar y,
+      const DlPaint& paint) = 0;
+
   virtual void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
                             SkScalar x,
                             SkScalar y,

display_list/dl_op_receiver.h

@@ -257,6 +257,10 @@ class DlOpReceiver {
   virtual void drawTextBlob(const sk_sp<SkTextBlob> blob,
                             SkScalar x,
                             SkScalar y) = 0;
+  virtual void drawTextFrame(
+      const std::shared_ptr<impeller::TextFrame>& text_frame,
+      SkScalar x,
+      SkScalar y) = 0;
   virtual void drawShadow(const SkPath& path,
                           const DlColor color,
                           const SkScalar elevation,

display_list/dl_op_records.h

@@ -12,6 +12,7 @@
 #include "flutter/display_list/effects/dl_color_source.h"
 #include "flutter/fml/macros.h"
 
+#include "impeller/typographer/text_frame.h"
 #include "third_party/skia/include/core/SkRSXform.h"
 
 namespace flutter {
@@ -1085,6 +1086,25 @@ struct DrawTextBlobOp final : DrawOpBase {
   }
 };
 
+struct DrawTextFrameOp final : DrawOpBase {
+  static const auto kType = DisplayListOpType::kDrawTextFrame;
+
+  DrawTextFrameOp(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                  SkScalar x,
+                  SkScalar y)
+      : x(x), y(y), text_frame(text_frame) {}
+
+  const SkScalar x;
+  const SkScalar y;
+  const std::shared_ptr<impeller::TextFrame> text_frame;
+
+  void dispatch(DispatchContext& ctx) const {
+    if (op_needed(ctx)) {
+      ctx.receiver.drawTextFrame(text_frame, x, y);
+    }
+  }
+};
+
 // 4 byte header + 28 byte payload packs evenly into 32 bytes
 #define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder)                     \
   struct Draw##name##Op final : DrawOpBase {                                  \

display_list/skia/dl_sk_canvas.cc

@@ -325,6 +325,14 @@ void DlSkCanvasAdapter::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
   delegate_->drawTextBlob(blob, x, y, ToSk(paint));
 }
 
+void DlSkCanvasAdapter::DrawTextFrame(
+    const std::shared_ptr<impeller::TextFrame>& text_frame,
+    SkScalar x,
+    SkScalar y,
+    const DlPaint& paint) {
+  FML_CHECK(false);
+}
+
 void DlSkCanvasAdapter::DrawShadow(const SkPath& path,
                                    const DlColor color,
                                    const SkScalar elevation,

display_list/skia/dl_sk_canvas.h

@@ -7,6 +7,7 @@
 
 #include "flutter/display_list/dl_canvas.h"
 #include "flutter/display_list/skia/dl_sk_types.h"
+#include "impeller/typographer/text_frame.h"
 
 namespace flutter {
 
@@ -144,6 +145,10 @@ class DlSkCanvasAdapter final : public virtual DlCanvas {
                     SkScalar x,
                     SkScalar y,
                     const DlPaint& paint) override;
+  void DrawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                     SkScalar x,
+                     SkScalar y,
+                     const DlPaint& paint) override;
   void DrawShadow(const SkPath& path,
                   const DlColor color,
                   const SkScalar elevation,

display_list/skia/dl_sk_dispatcher.cc

@@ -270,6 +270,13 @@ void DlSkCanvasDispatcher::drawTextBlob(const sk_sp<SkTextBlob> blob,
   canvas_->drawTextBlob(blob, x, y, paint());
 }
 
+void DlSkCanvasDispatcher::drawTextFrame(
+    const std::shared_ptr<impeller::TextFrame>& text_frame,
+    SkScalar x,
+    SkScalar y) {
+  FML_CHECK(false);
+}
+
 void DlSkCanvasDispatcher::DrawShadow(SkCanvas* canvas,
                                       const SkPath& path,
                                       DlColor color,

display_list/skia/dl_sk_dispatcher.h

@@ -98,6 +98,9 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver,
   void drawTextBlob(const sk_sp<SkTextBlob> blob,
                     SkScalar x,
                     SkScalar y) override;
+  void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                     SkScalar x,
+                     SkScalar y) override;
   void drawShadow(const SkPath& path,
                   const DlColor color,
                   const SkScalar elevation,

display_list/utils/dl_receiver_utils.h

@@ -129,6 +129,9 @@ class IgnoreDrawDispatchHelper : public virtual DlOpReceiver {
   void drawTextBlob(const sk_sp<SkTextBlob> blob,
                     SkScalar x,
                     SkScalar y) override {}
+  void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
+                     SkScalar x,
+                     SkScalar y) override {}
   void drawShadow(const SkPath& path,
                   const DlColor color,
                   const SkScalar elevation,

flow/BUILD.gn

@@ -99,7 +99,10 @@ source_set("flow") {
   deps = [ "//third_party/skia" ]
 
   if (impeller_supports_rendering) {
-    deps += [ "//flutter/impeller" ]
+    deps += [
+      "//flutter/impeller",
+      "//flutter/impeller/typographer/backends/skia:typographer_skia_backend",
+    ]
   }
 }
 

flow/layers/performance_overlay_layer.cc

@@ -14,6 +14,9 @@
 #include "flow/stopwatch_sk.h"
 #include "third_party/skia/include/core/SkFont.h"
 #include "third_party/skia/include/core/SkTextBlob.h"
+#ifdef IMPELLER_SUPPORTS_RENDERING
+#include "impeller/typographer/backends/skia/text_frame_skia.h"  // nogncheck
+#endif  // IMPELLER_SUPPORTS_RENDERING
 
 namespace flutter {
 namespace {
@@ -50,7 +53,14 @@ void VisualizeStopWatch(DlCanvas* canvas,
         stopwatch, label_prefix, font_path);
     // Historically SK_ColorGRAY (== 0xFF888888) was used here
     DlPaint paint(DlColor(0xFF888888));
+#ifdef IMPELLER_SUPPORTS_RENDERING
+    if (impeller_enabled) {
+      canvas->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(text),
+                            x + label_x, y + height + label_y, paint);
+    }
+#endif  // IMPELLER_SUPPORTS_RENDERING
     canvas->DrawTextBlob(text, x + label_x, y + height + label_y, paint);
+    return;
   }
 }
 

impeller/aiks/aiks_unittests.cc

@@ -5,7 +5,6 @@
 #include <array>
 #include <cmath>
 #include <cstdlib>
-#include <iostream>
 #include <memory>
 #include <tuple>
 #include <utility>
@@ -1335,11 +1334,7 @@ bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
   }
 
   // Create the Impeller text frame and draw it at the designated baseline.
-  auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob);
-  if (!maybe_frame.has_value()) {
-    return false;
-  }
-  auto frame = maybe_frame.value();
+  auto frame = MakeTextFrameFromTextBlobSkia(blob);
 
   Paint text_paint;
   text_paint.color = Color::Yellow().WithAlpha(options.alpha);
@@ -1528,7 +1523,7 @@ TEST_P(AiksTest, CanRenderTextOutsideBoundaries) {
     {
       auto blob = SkTextBlob::MakeFromString(t.text, sk_font);
       ASSERT_NE(blob, nullptr);
-      auto frame = MakeTextFrameFromTextBlobSkia(blob).value();
+      auto frame = MakeTextFrameFromTextBlobSkia(blob);
       canvas.DrawTextFrame(frame, Point(), text_paint);
     }
     canvas.Restore();
@@ -3202,12 +3197,7 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) {
 
   auto blob = SkTextBlob::MakeFromString("Hello", sk_font);
   ASSERT_NE(blob, nullptr);
-  auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob);
-  ASSERT_TRUE(maybe_frame.has_value());
-  if (!maybe_frame.has_value()) {
-    return;
-  }
-  auto frame = maybe_frame.value();
+  auto frame = MakeTextFrameFromTextBlobSkia(blob);
   canvas.DrawTextFrame(frame, Point(), text_paint);
 
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));

impeller/aiks/canvas.cc

@@ -546,15 +546,15 @@ void Canvas::SaveLayer(const Paint& paint,
   }
 }
 
-void Canvas::DrawTextFrame(const TextFrame& text_frame,
+void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
                            Point position,
                            const Paint& paint) {
   Entity entity;
   entity.SetStencilDepth(GetStencilDepth());
   entity.SetBlendMode(paint.blend_mode);
 
   auto text_contents = std::make_shared<TextContents>();
-  text_contents->SetTextFrame(TextFrame(text_frame));
+  text_contents->SetTextFrame(text_frame);
   text_contents->SetColor(paint.color);
 
   entity.SetTransformation(GetCurrentTransformation() *

impeller/aiks/canvas.h

@@ -140,7 +140,7 @@ class Canvas {
 
   void DrawPicture(const Picture& picture);
 
-  void DrawTextFrame(const TextFrame& text_frame,
+  void DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
                      Point position,
                      const Paint& paint);