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

Commit 689d403

Browse files
author
Jonah Williams
authored
[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
1 parent 92be04f commit 689d403

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

+407
-121
lines changed

display_list/BUILD.gn

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

display_list/benchmarking/dl_complexity_gl.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,11 @@ 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+
640645
void DisplayListGLComplexityCalculator::GLHelper::drawShadow(
641646
const SkPath& path,
642647
const DlColor color,

display_list/benchmarking/dl_complexity_gl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ 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;
7376
void drawShadow(const SkPath& path,
7477
const DlColor color,
7578
const SkScalar elevation,

display_list/benchmarking/dl_complexity_metal.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,11 @@ 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+
584589
void DisplayListMetalComplexityCalculator::MetalHelper::drawShadow(
585590
const SkPath& path,
586591
const DlColor color,

display_list/benchmarking/dl_complexity_metal.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ class DisplayListMetalComplexityCalculator
6868
void drawTextBlob(const sk_sp<SkTextBlob> blob,
6969
SkScalar x,
7070
SkScalar y) override;
71+
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
72+
SkScalar x,
73+
SkScalar y) override;
7174
void drawShadow(const SkPath& path,
7275
const DlColor color,
7376
const SkScalar elevation,

display_list/display_list.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ namespace flutter {
136136
\
137137
V(DrawDisplayList) \
138138
V(DrawTextBlob) \
139+
V(DrawTextFrame) \
139140
\
140141
V(DrawShadow) \
141142
V(DrawShadowTransparentOccluder)

display_list/dl_builder.cc

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,6 +1302,48 @@ 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+
13051347
void DisplayListBuilder::DrawShadow(const SkPath& path,
13061348
const DlColor color,
13071349
const SkScalar elevation,

display_list/dl_builder.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,16 @@ 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+
227237
// |DlCanvas|
228238
void DrawShadow(const SkPath& path,
229239
const DlColor color,

display_list/dl_canvas.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
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+
2123
namespace flutter {
2224

2325
//------------------------------------------------------------------------------
@@ -201,6 +203,13 @@ class DlCanvas {
201203
const DlPaint* paint = nullptr) = 0;
202204
virtual void DrawDisplayList(const sk_sp<DisplayList> display_list,
203205
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+
204213
virtual void DrawTextBlob(const sk_sp<SkTextBlob>& blob,
205214
SkScalar x,
206215
SkScalar y,

display_list/dl_op_receiver.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,10 @@ 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;
260264
virtual void drawShadow(const SkPath& path,
261265
const DlColor color,
262266
const SkScalar elevation,

0 commit comments

Comments
 (0)