[flutter roll cp] Revert "[Impeller] construct text frames on UI thre…

…ad." (#45910) (#45958) Reverts #45418 Some google3 tests are hitting the CHECK I added in the DlSkCanvasDispatcher::drawTextFrame, which indicates that the SkParagraph code likely thinks impeller is enabled, whereas other code might be running with Skia. Perhaps this could happen if its software rendering? It should be a fatal error on startup so we can track this down. cherry pick for flutter roll Co-authored-by: Jonah Williams <[email protected]>
flutter · Sep 18, 2023 · aae3bc0 · aae3bc0
1 parent cd90cc8
commit aae3bc0
Show file tree

Hide file tree

Showing 41 changed files with 122 additions and 408 deletions.
diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn
@@ -87,7 +87,6 @@ source_set("display_list") {
   public_deps = [
     "//flutter/fml",
     "//flutter/impeller/runtime_stage",
-    "//flutter/impeller/typographer",
     "//third_party/skia",
   ]
 

diff --git a/display_list/benchmarking/dl_complexity_gl.cc b/display_list/benchmarking/dl_complexity_gl.cc
@@ -637,11 +637,6 @@ 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,

diff --git a/display_list/benchmarking/dl_complexity_gl.h b/display_list/benchmarking/dl_complexity_gl.h
@@ -70,9 +70,6 @@ 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,

diff --git a/display_list/benchmarking/dl_complexity_metal.cc b/display_list/benchmarking/dl_complexity_metal.cc
@@ -581,11 +581,6 @@ 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,

diff --git a/display_list/benchmarking/dl_complexity_metal.h b/display_list/benchmarking/dl_complexity_metal.h
@@ -70,9 +70,6 @@ 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,

diff --git a/display_list/display_list.h b/display_list/display_list.h
@@ -136,7 +136,6 @@ namespace flutter {
                                     \
   V(DrawDisplayList)                \
   V(DrawTextBlob)                   \
-  V(DrawTextFrame)                  \
                                     \
   V(DrawShadow)                     \
   V(DrawShadowTransparentOccluder)

diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc
@@ -1302,48 +1302,6 @@ 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,

diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h
@@ -224,16 +224,6 @@ 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,

diff --git a/display_list/dl_canvas.h b/display_list/dl_canvas.h
@@ -18,8 +18,6 @@
 #include "third_party/skia/include/core/SkRect.h"
 #include "third_party/skia/include/core/SkTextBlob.h"
 
-#include "impeller/typographer/text_frame.h"
-
 namespace flutter {
 
 //------------------------------------------------------------------------------
@@ -154,7 +152,7 @@ class DlCanvas {
   virtual void DrawVertices(const DlVertices* vertices,
                             DlBlendMode mode,
                             const DlPaint& paint) = 0;
-  void DrawVertices(const std::shared_ptr<const DlVertices>& vertices,
+  void DrawVertices(const std::shared_ptr<const DlVertices> vertices,
                     DlBlendMode mode,
                     const DlPaint& paint) {
     DrawVertices(vertices.get(), mode, paint);
@@ -203,13 +201,6 @@ 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,

diff --git a/display_list/dl_op_receiver.h b/display_list/dl_op_receiver.h
@@ -257,10 +257,6 @@ 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,

diff --git a/display_list/dl_op_records.h b/display_list/dl_op_records.h
@@ -12,7 +12,6 @@
 #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,25 +1084,6 @@ 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 {                                  \

diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
@@ -325,14 +325,6 @@ 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,

diff --git a/display_list/skia/dl_sk_canvas.h b/display_list/skia/dl_sk_canvas.h
@@ -7,7 +7,6 @@
 
 #include "flutter/display_list/dl_canvas.h"
 #include "flutter/display_list/skia/dl_sk_types.h"
-#include "impeller/typographer/text_frame.h"
 
 namespace flutter {
 
@@ -145,10 +144,6 @@ 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,

diff --git a/display_list/skia/dl_sk_dispatcher.cc b/display_list/skia/dl_sk_dispatcher.cc
@@ -270,13 +270,6 @@ 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,

diff --git a/display_list/skia/dl_sk_dispatcher.h b/display_list/skia/dl_sk_dispatcher.h
@@ -98,9 +98,6 @@ 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,

diff --git a/display_list/utils/dl_receiver_utils.h b/display_list/utils/dl_receiver_utils.h
@@ -129,9 +129,6 @@ 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,

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

diff --git a/flow/layers/performance_overlay_layer.cc b/flow/layers/performance_overlay_layer.cc
@@ -14,9 +14,6 @@
 #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 {
@@ -53,14 +50,7 @@ void VisualizeStopWatch(DlCanvas* canvas,
         stopwatch, label_prefix, font_path);
     // Historically SK_ColorGRAY (== 0xFF888888) was used here
     DlPaint paint(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;
   }
 }
 

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

diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc
@@ -545,15 +545,15 @@ void Canvas::SaveLayer(const Paint& paint,
   }
 }
 
-void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
+void Canvas::DrawTextFrame(const 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(text_frame);
+  text_contents->SetTextFrame(TextFrame(text_frame));
   text_contents->SetColor(paint.color);
 
   entity.SetTransformation(GetCurrentTransformation() *

diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h
@@ -140,7 +140,7 @@ class Canvas {
 
   void DrawPicture(const Picture& picture);
 
-  void DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
+  void DrawTextFrame(const TextFrame& text_frame,
                      Point position,
                      const Paint& paint);