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

[Impeller] construct text frames on UI thread. #45418

Merged
merged 14 commits into from
Sep 6, 2023
1 change: 1 addition & 0 deletions display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ source_set("display_list") {
public_deps = [
"//flutter/fml",
"//flutter/impeller/runtime_stage",
"//flutter/impeller/typographer",
"//third_party/skia",
]

Expand Down
5 changes: 5 additions & 0 deletions display_list/benchmarking/dl_complexity_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions display_list/benchmarking/dl_complexity_gl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions display_list/benchmarking/dl_complexity_metal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions display_list/benchmarking/dl_complexity_metal.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,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,
Expand Down
1 change: 1 addition & 0 deletions display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ namespace flutter {
\
V(DrawDisplayList) \
V(DrawTextBlob) \
V(DrawTextFrame) \
\
V(DrawShadow) \
V(DrawShadowTransparentOccluder)
Expand Down
42 changes: 42 additions & 0 deletions display_list/dl_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1302,6 +1302,48 @@ void DisplayListBuilder::DrawTextBlob(const sk_sp<SkTextBlob>& blob,
SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawTextBlobFlags);
drawTextBlob(blob, x, y);
}

void DisplayListBuilder::drawTextFrame(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that drawTextBlob will go unused in the Impeller backend?

If so, would you want to (ifdef'd?) make sure it's not used, i.e. UNIMPLEMENTED, similar to the dispatcher?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we use DisplayListBuilder on Skia ever. FYI @flar

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do. That was its original purpose and is iteratively becoming its only purpose.

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(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised we don't have a conversion for this? If we don't, feel free to resolve.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do, its just all over the place.

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,
Expand Down
10 changes: 10 additions & 0 deletions display_list/dl_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 10 additions & 1 deletion display_list/dl_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -152,7 +154,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);
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions display_list/dl_op_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions display_list/dl_op_records.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1084,6 +1085,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 { \
Expand Down
8 changes: 8 additions & 0 deletions display_list/skia/dl_sk_canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions display_list/skia/dl_sk_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions display_list/skia/dl_sk_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions display_list/skia/dl_sk_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions display_list/utils/dl_receiver_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
}
}

Expand Down
10 changes: 10 additions & 0 deletions flow/layers/performance_overlay_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -50,7 +53,14 @@ 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;
}
}

Expand Down
16 changes: 3 additions & 13 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <array>
#include <cmath>
#include <cstdlib>
#include <iostream>
#include <memory>
#include <tuple>
#include <utility>
Expand Down Expand Up @@ -1278,11 +1277,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);
Expand Down Expand Up @@ -1471,7 +1466,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();
Expand Down Expand Up @@ -3094,12 +3089,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()));
Expand Down
4 changes: 2 additions & 2 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -533,15 +533,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() *
Expand Down
2 changes: 1 addition & 1 deletion impeller/aiks/canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,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);

Expand Down
Loading