Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit cdcbdcc

Browse files
jonahwilliamsjonahwilliams
authored
Revert "[Impeller] construct text frames on UI thread." (#45910)
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.
1 parent 3983b50 commit cdcbdcc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+122
-408
lines changed

‎display_list/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ source_set("display_list") {
8787
public_deps = [
8888
"//flutter/fml",
8989
"//flutter/impeller/runtime_stage",
90-
"//flutter/impeller/typographer",
9190
"//third_party/skia",
9291
]
9392

‎display_list/benchmarking/dl_complexity_gl.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -637,11 +637,6 @@ void DisplayListGLComplexityCalculator::GLHelper::drawTextBlob(
637637
draw_text_blob_count_++;
638638
}
639639

640-
void DisplayListGLComplexityCalculator::GLHelper::drawTextFrame(
641-
const std::shared_ptr<impeller::TextFrame>& text_frame,
642-
SkScalar x,
643-
SkScalar y) {}
644-
645640
void DisplayListGLComplexityCalculator::GLHelper::drawShadow(
646641
const SkPath& path,
647642
const DlColor color,

‎display_list/benchmarking/dl_complexity_gl.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ class DisplayListGLComplexityCalculator
7070
void drawTextBlob(const sk_sp<SkTextBlob> blob,
7171
SkScalar x,
7272
SkScalar y) override;
73-
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
74-
SkScalar x,
75-
SkScalar y) override;
7673
void drawShadow(const SkPath& path,
7774
const DlColor color,
7875
const SkScalar elevation,

‎display_list/benchmarking/dl_complexity_metal.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -581,11 +581,6 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawTextBlob(
581581
draw_text_blob_count_++;
582582
}
583583

584-
void DisplayListMetalComplexityCalculator::MetalHelper::drawTextFrame(
585-
const std::shared_ptr<impeller::TextFrame>& text_frame,
586-
SkScalar x,
587-
SkScalar y) {}
588-
589584
void DisplayListMetalComplexityCalculator::MetalHelper::drawShadow(
590585
const SkPath& path,
591586
const DlColor color,

‎display_list/benchmarking/dl_complexity_metal.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ class DisplayListMetalComplexityCalculator
7070
void drawTextBlob(const sk_sp<SkTextBlob> blob,
7171
SkScalar x,
7272
SkScalar y) override;
73-
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
74-
SkScalar x,
75-
SkScalar y) override;
7673
void drawShadow(const SkPath& path,
7774
const DlColor color,
7875
const SkScalar elevation,

‎display_list/display_list.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ namespace flutter {
136136
\
137137
V(DrawDisplayList) \
138138
V(DrawTextBlob) \
139-
V(DrawTextFrame) \
140139
\
141140
V(DrawShadow) \
142141
V(DrawShadowTransparentOccluder)

‎display_list/dl_builder.cc

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,48 +1302,6 @@ void DisplayListBuilder::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
13021302
SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawTextBlobFlags);
13031303
drawTextBlob(blob, x, y);
13041304
}
1305-
1306-
void DisplayListBuilder::drawTextFrame(
1307-
const std::shared_ptr<impeller::TextFrame>& text_frame,
1308-
SkScalar x,
1309-
SkScalar y) {
1310-
DisplayListAttributeFlags flags = kDrawTextBlobFlags;
1311-
OpResult result = PaintResult(current_, flags);
1312-
if (result == OpResult::kNoEffect) {
1313-
return;
1314-
}
1315-
impeller::Rect bounds = text_frame->GetBounds();
1316-
SkRect sk_bounds = SkRect::MakeLTRB(bounds.GetLeft(), bounds.GetTop(),
1317-
bounds.GetRight(), bounds.GetBottom());
1318-
bool unclipped = AccumulateOpBounds(sk_bounds.makeOffset(x, y), flags);
1319-
// TODO(https://github.com/flutter/flutter/issues/82202): Remove once the
1320-
// unit tests can use Fuchsia's font manager instead of the empty default.
1321-
// Until then we might encounter empty bounds for otherwise valid text and
1322-
// thus we ignore the results from AccumulateOpBounds.
1323-
#if defined(OS_FUCHSIA)
1324-
unclipped = true;
1325-
#endif // OS_FUCHSIA
1326-
if (unclipped) {
1327-
Push<DrawTextFrameOp>(0, 1, text_frame, x, y);
1328-
// There is no way to query if the glyphs of a text blob overlap and
1329-
// there are no current guarantees from either Skia or Impeller that
1330-
// they will protect overlapping glyphs from the effects of overdraw
1331-
// so we must make the conservative assessment that this DL layer is
1332-
// not compatible with group opacity inheritance.
1333-
UpdateLayerOpacityCompatibility(false);
1334-
UpdateLayerResult(result);
1335-
}
1336-
}
1337-
1338-
void DisplayListBuilder::DrawTextFrame(
1339-
const std::shared_ptr<impeller::TextFrame>& text_frame,
1340-
SkScalar x,
1341-
SkScalar y,
1342-
const DlPaint& paint) {
1343-
SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawTextBlobFlags);
1344-
drawTextFrame(text_frame, x, y);
1345-
}
1346-
13471305
void DisplayListBuilder::DrawShadow(const SkPath& path,
13481306
const DlColor color,
13491307
const SkScalar elevation,

‎display_list/dl_builder.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -224,16 +224,6 @@ class DisplayListBuilder final : public virtual DlCanvas,
224224
SkScalar x,
225225
SkScalar y,
226226
const DlPaint& paint) override;
227-
228-
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
229-
SkScalar x,
230-
SkScalar y) override;
231-
232-
void DrawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
233-
SkScalar x,
234-
SkScalar y,
235-
const DlPaint& paint) override;
236-
237227
// |DlCanvas|
238228
void DrawShadow(const SkPath& path,
239229
const DlColor color,

‎display_list/dl_canvas.h

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
#include "third_party/skia/include/core/SkRect.h"
1919
#include "third_party/skia/include/core/SkTextBlob.h"
2020

21-
#include "impeller/typographer/text_frame.h"
22-
2321
namespace flutter {
2422

2523
//------------------------------------------------------------------------------
@@ -154,7 +152,7 @@ class DlCanvas {
154152
virtual void DrawVertices(const DlVertices* vertices,
155153
DlBlendMode mode,
156154
const DlPaint& paint) = 0;
157-
void DrawVertices(const std::shared_ptr<const DlVertices>& vertices,
155+
void DrawVertices(const std::shared_ptr<const DlVertices> vertices,
158156
DlBlendMode mode,
159157
const DlPaint& paint) {
160158
DrawVertices(vertices.get(), mode, paint);
@@ -203,13 +201,6 @@ class DlCanvas {
203201
const DlPaint* paint = nullptr) = 0;
204202
virtual void DrawDisplayList(const sk_sp<DisplayList> display_list,
205203
SkScalar opacity = SK_Scalar1) = 0;
206-
207-
virtual void DrawTextFrame(
208-
const std::shared_ptr<impeller::TextFrame>& text_frame,
209-
SkScalar x,
210-
SkScalar y,
211-
const DlPaint& paint) = 0;
212-
213204
virtual void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
214205
SkScalar x,
215206
SkScalar y,

‎display_list/dl_op_receiver.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,10 +257,6 @@ class DlOpReceiver {
257257
virtual void drawTextBlob(const sk_sp<SkTextBlob> blob,
258258
SkScalar x,
259259
SkScalar y) = 0;
260-
virtual void drawTextFrame(
261-
const std::shared_ptr<impeller::TextFrame>& text_frame,
262-
SkScalar x,
263-
SkScalar y) = 0;
264260
virtual void drawShadow(const SkPath& path,
265261
const DlColor color,
266262
const SkScalar elevation,

‎display_list/dl_op_records.h

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "flutter/display_list/effects/dl_color_source.h"
1313
#include "flutter/fml/macros.h"
1414

15-
#include "impeller/typographer/text_frame.h"
1615
#include "third_party/skia/include/core/SkRSXform.h"
1716

1817
namespace flutter {
@@ -1085,25 +1084,6 @@ struct DrawTextBlobOp final : DrawOpBase {
10851084
}
10861085
};
10871086

1088-
struct DrawTextFrameOp final : DrawOpBase {
1089-
static const auto kType = DisplayListOpType::kDrawTextFrame;
1090-
1091-
DrawTextFrameOp(const std::shared_ptr<impeller::TextFrame>& text_frame,
1092-
SkScalar x,
1093-
SkScalar y)
1094-
: x(x), y(y), text_frame(text_frame) {}
1095-
1096-
const SkScalar x;
1097-
const SkScalar y;
1098-
const std::shared_ptr<impeller::TextFrame> text_frame;
1099-
1100-
void dispatch(DispatchContext& ctx) const {
1101-
if (op_needed(ctx)) {
1102-
ctx.receiver.drawTextFrame(text_frame, x, y);
1103-
}
1104-
}
1105-
};
1106-
11071087
// 4 byte header + 28 byte payload packs evenly into 32 bytes
11081088
#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \
11091089
struct Draw##name##Op final : DrawOpBase { \

‎display_list/skia/dl_sk_canvas.cc

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -325,14 +325,6 @@ void DlSkCanvasAdapter::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
325325
delegate_->drawTextBlob(blob, x, y, ToSk(paint));
326326
}
327327

328-
void DlSkCanvasAdapter::DrawTextFrame(
329-
const std::shared_ptr<impeller::TextFrame>& text_frame,
330-
SkScalar x,
331-
SkScalar y,
332-
const DlPaint& paint) {
333-
FML_CHECK(false);
334-
}
335-
336328
void DlSkCanvasAdapter::DrawShadow(const SkPath& path,
337329
const DlColor color,
338330
const SkScalar elevation,

‎display_list/skia/dl_sk_canvas.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
#include "flutter/display_list/dl_canvas.h"
99
#include "flutter/display_list/skia/dl_sk_types.h"
10-
#include "impeller/typographer/text_frame.h"
1110

1211
namespace flutter {
1312

@@ -145,10 +144,6 @@ class DlSkCanvasAdapter final : public virtual DlCanvas {
145144
SkScalar x,
146145
SkScalar y,
147146
const DlPaint& paint) override;
148-
void DrawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
149-
SkScalar x,
150-
SkScalar y,
151-
const DlPaint& paint) override;
152147
void DrawShadow(const SkPath& path,
153148
const DlColor color,
154149
const SkScalar elevation,

‎display_list/skia/dl_sk_dispatcher.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -270,13 +270,6 @@ void DlSkCanvasDispatcher::drawTextBlob(const sk_sp<SkTextBlob> blob,
270270
canvas_->drawTextBlob(blob, x, y, paint());
271271
}
272272

273-
void DlSkCanvasDispatcher::drawTextFrame(
274-
const std::shared_ptr<impeller::TextFrame>& text_frame,
275-
SkScalar x,
276-
SkScalar y) {
277-
FML_CHECK(false);
278-
}
279-
280273
void DlSkCanvasDispatcher::DrawShadow(SkCanvas* canvas,
281274
const SkPath& path,
282275
DlColor color,

‎display_list/skia/dl_sk_dispatcher.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,6 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver,
9898
void drawTextBlob(const sk_sp<SkTextBlob> blob,
9999
SkScalar x,
100100
SkScalar y) override;
101-
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
102-
SkScalar x,
103-
SkScalar y) override;
104101
void drawShadow(const SkPath& path,
105102
const DlColor color,
106103
const SkScalar elevation,

‎display_list/utils/dl_receiver_utils.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,6 @@ class IgnoreDrawDispatchHelper : public virtual DlOpReceiver {
129129
void drawTextBlob(const sk_sp<SkTextBlob> blob,
130130
SkScalar x,
131131
SkScalar y) override {}
132-
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
133-
SkScalar x,
134-
SkScalar y) override {}
135132
void drawShadow(const SkPath& path,
136133
const DlColor color,
137134
const SkScalar elevation,

‎flow/BUILD.gn

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ source_set("flow") {
9999
deps = [ "//third_party/skia" ]
100100

101101
if (impeller_supports_rendering) {
102-
deps += [
103-
"//flutter/impeller",
104-
"//flutter/impeller/typographer/backends/skia:typographer_skia_backend",
105-
]
102+
deps += [ "//flutter/impeller" ]
106103
}
107104
}
108105

‎flow/layers/performance_overlay_layer.cc

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414
#include "flow/stopwatch_sk.h"
1515
#include "third_party/skia/include/core/SkFont.h"
1616
#include "third_party/skia/include/core/SkTextBlob.h"
17-
#ifdef IMPELLER_SUPPORTS_RENDERING
18-
#include "impeller/typographer/backends/skia/text_frame_skia.h" // nogncheck
19-
#endif // IMPELLER_SUPPORTS_RENDERING
2017

2118
namespace flutter {
2219
namespace {
@@ -53,14 +50,7 @@ void VisualizeStopWatch(DlCanvas* canvas,
5350
stopwatch, label_prefix, font_path);
5451
// Historically SK_ColorGRAY (== 0xFF888888) was used here
5552
DlPaint paint(0xFF888888);
56-
#ifdef IMPELLER_SUPPORTS_RENDERING
57-
if (impeller_enabled) {
58-
canvas->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(text),
59-
x + label_x, y + height + label_y, paint);
60-
}
61-
#endif // IMPELLER_SUPPORTS_RENDERING
6253
canvas->DrawTextBlob(text, x + label_x, y + height + label_y, paint);
63-
return;
6454
}
6555
}
6656

‎impeller/aiks/aiks_unittests.cc

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <array>
66
#include <cmath>
77
#include <cstdlib>
8+
#include <iostream>
89
#include <memory>
910
#include <tuple>
1011
#include <utility>
@@ -1334,7 +1335,11 @@ bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
13341335
}
13351336

13361337
// Create the Impeller text frame and draw it at the designated baseline.
1337-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
1338+
auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob);
1339+
if (!maybe_frame.has_value()) {
1340+
return false;
1341+
}
1342+
auto frame = maybe_frame.value();
13381343

13391344
Paint text_paint;
13401345
text_paint.color = Color::Yellow().WithAlpha(options.alpha);
@@ -1523,7 +1528,7 @@ TEST_P(AiksTest, CanRenderTextOutsideBoundaries) {
15231528
{
15241529
auto blob = SkTextBlob::MakeFromString(t.text, sk_font);
15251530
ASSERT_NE(blob, nullptr);
1526-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
1531+
auto frame = MakeTextFrameFromTextBlobSkia(blob).value();
15271532
canvas.DrawTextFrame(frame, Point(), text_paint);
15281533
}
15291534
canvas.Restore();
@@ -3127,7 +3132,12 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) {
31273132

31283133
auto blob = SkTextBlob::MakeFromString("Hello", sk_font);
31293134
ASSERT_NE(blob, nullptr);
3130-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
3135+
auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob);
3136+
ASSERT_TRUE(maybe_frame.has_value());
3137+
if (!maybe_frame.has_value()) {
3138+
return;
3139+
}
3140+
auto frame = maybe_frame.value();
31313141
canvas.DrawTextFrame(frame, Point(), text_paint);
31323142

31333143
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));

‎impeller/aiks/canvas.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,15 +546,15 @@ void Canvas::SaveLayer(const Paint& paint,
546546
}
547547
}
548548

549-
void Canvas::DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
549+
void Canvas::DrawTextFrame(const TextFrame& text_frame,
550550
Point position,
551551
const Paint& paint) {
552552
Entity entity;
553553
entity.SetStencilDepth(GetStencilDepth());
554554
entity.SetBlendMode(paint.blend_mode);
555555

556556
auto text_contents = std::make_shared<TextContents>();
557-
text_contents->SetTextFrame(text_frame);
557+
text_contents->SetTextFrame(TextFrame(text_frame));
558558
text_contents->SetColor(paint.color);
559559

560560
entity.SetTransformation(GetCurrentTransformation() *

‎impeller/aiks/canvas.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class Canvas {
140140

141141
void DrawPicture(const Picture& picture);
142142

143-
void DrawTextFrame(const std::shared_ptr<TextFrame>& text_frame,
143+
void DrawTextFrame(const TextFrame& text_frame,
144144
Point position,
145145
const Paint& paint);
146146

0 commit comments

Comments
 (0)