From aae3bc0b73b4e5d7db081d97d180ff67c0772d27 Mon Sep 17 00:00:00 2001 From: Xilai Zhang Date: Sun, 17 Sep 2023 22:02:47 -0700 Subject: [PATCH] [flutter roll cp] Revert "[Impeller] construct text frames on UI thread." (#45910) (#45958) Reverts flutter/engine#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 --- display_list/BUILD.gn | 1 - display_list/benchmarking/dl_complexity_gl.cc | 5 - display_list/benchmarking/dl_complexity_gl.h | 3 - .../benchmarking/dl_complexity_metal.cc | 5 - .../benchmarking/dl_complexity_metal.h | 3 - display_list/display_list.h | 1 - display_list/dl_builder.cc | 42 ----- display_list/dl_builder.h | 10 -- display_list/dl_canvas.h | 11 +- display_list/dl_op_receiver.h | 4 - display_list/dl_op_records.h | 20 --- display_list/skia/dl_sk_canvas.cc | 8 - display_list/skia/dl_sk_canvas.h | 5 - display_list/skia/dl_sk_dispatcher.cc | 7 - display_list/skia/dl_sk_dispatcher.h | 3 - display_list/utils/dl_receiver_utils.h | 3 - flow/BUILD.gn | 5 +- flow/layers/performance_overlay_layer.cc | 10 -- impeller/aiks/aiks_unittests.cc | 16 +- impeller/aiks/canvas.cc | 4 +- impeller/aiks/canvas.h | 2 +- impeller/display_list/dl_dispatcher.cc | 21 ++- impeller/display_list/dl_dispatcher.h | 5 - impeller/entity/contents/text_contents.cc | 16 +- impeller/entity/contents/text_contents.h | 4 +- impeller/entity/entity_unittests.cc | 6 +- .../backends/skia/text_frame_skia.cc | 13 +- .../backends/skia/text_frame_skia.h | 3 +- .../backends/stb/text_frame_stb.cc | 10 +- .../typographer/backends/stb/text_frame_stb.h | 7 +- impeller/typographer/typographer_context.h | 1 + impeller/typographer/typographer_unittests.cc | 47 +++--- shell/common/dl_op_spy.cc | 8 - shell/common/dl_op_spy.h | 3 - testing/display_list_testing.cc | 9 -- testing/display_list_testing.h | 3 - testing/mock_canvas.cc | 8 - testing/mock_canvas.h | 4 - third_party/txt/BUILD.gn | 1 - third_party/txt/src/skia/paragraph_skia.cc | 46 +----- third_party/txt/tests/paragraph_unittests.cc | 147 ++++-------------- 41 files changed, 122 insertions(+), 408 deletions(-) diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 736e13dc5c7bb..97c9683511b61 100644 --- 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 index 53b70e2c24e47..f8dd1c52a1260 100644 --- 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& 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 index 5115bb4d6ecd6..9fc7596687051 100644 --- 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 blob, SkScalar x, SkScalar y) override; - void drawTextFrame(const std::shared_ptr& 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 index c6b6547afee56..56d4f3b406ad9 100644 --- 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& 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 index dd068e2fa3243..aa63863fa4d05 100644 --- 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 blob, SkScalar x, SkScalar y) override; - void drawTextFrame(const std::shared_ptr& 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 index 3d4a7acff5896..668a247d5596f 100644 --- 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 index f20b8fe82ec47..2906f9a85fda7 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -1302,48 +1302,6 @@ void DisplayListBuilder::DrawTextBlob(const sk_sp& blob, SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawTextBlobFlags); drawTextBlob(blob, x, y); } - -void DisplayListBuilder::drawTextFrame( - const std::shared_ptr& 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(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& 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 index 3a65b8fad5328..ab0bce0db3c38 100644 --- 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& text_frame, - SkScalar x, - SkScalar y) override; - - void DrawTextFrame(const std::shared_ptr& 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 index 5cd06146c78c3..f04645c157788 100644 --- 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& vertices, + void DrawVertices(const std::shared_ptr 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 display_list, SkScalar opacity = SK_Scalar1) = 0; - - virtual void DrawTextFrame( - const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y, - const DlPaint& paint) = 0; - virtual void DrawTextBlob(const sk_sp& blob, SkScalar x, SkScalar y, diff --git a/display_list/dl_op_receiver.h b/display_list/dl_op_receiver.h index d9bbc7b7f4f3f..23a0aaf6fd97c 100644 --- 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 blob, SkScalar x, SkScalar y) = 0; - virtual void drawTextFrame( - const std::shared_ptr& 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 index a1c7835e43df9..47cdeb9d0e6fb 100644 --- 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& text_frame, - SkScalar x, - SkScalar y) - : x(x), y(y), text_frame(text_frame) {} - - const SkScalar x; - const SkScalar y; - const std::shared_ptr 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 index 34270bd88018d..9299c9a6ff9d7 100644 --- 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& blob, delegate_->drawTextBlob(blob, x, y, ToSk(paint)); } -void DlSkCanvasAdapter::DrawTextFrame( - const std::shared_ptr& 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 index f085d35ae8c9d..0377b7de75e30 100644 --- 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& 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 index f8cc16d1151c1..def08fa8886a3 100644 --- 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 blob, canvas_->drawTextBlob(blob, x, y, paint()); } -void DlSkCanvasDispatcher::drawTextFrame( - const std::shared_ptr& 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 index ee6101291e76a..a7fc73bbe2a54 100644 --- 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 blob, SkScalar x, SkScalar y) override; - void drawTextFrame(const std::shared_ptr& 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 index 2a1f9cb7efff4..e2d07310368ae 100644 --- 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 blob, SkScalar x, SkScalar y) override {} - void drawTextFrame(const std::shared_ptr& 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 index a75605749c7aa..53de13474e316 100644 --- 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 index 1c1ebb4566798..911131fd0cfbc 100644 --- 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 index 6af0cbc9283a9..116fe055ba273 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -1309,7 +1310,11 @@ bool RenderTextInCanvasSkia(const std::shared_ptr& 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 index f50dd36af7ad9..6db3b209f11d2 100644 --- a/impeller/aiks/canvas.cc +++ b/impeller/aiks/canvas.cc @@ -545,7 +545,7 @@ void Canvas::SaveLayer(const Paint& paint, } } -void Canvas::DrawTextFrame(const std::shared_ptr& text_frame, +void Canvas::DrawTextFrame(const TextFrame& text_frame, Point position, const Paint& paint) { Entity entity; @@ -553,7 +553,7 @@ void Canvas::DrawTextFrame(const std::shared_ptr& text_frame, entity.SetBlendMode(paint.blend_mode); auto text_contents = std::make_shared(); - 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 index 0ea187c39a5ad..8015555f92b6c 100644 --- 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& text_frame, + void DrawTextFrame(const TextFrame& text_frame, Point position, const Paint& paint); diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index edcd363a7e45c..aaa2185f839cc 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -34,6 +34,7 @@ #include "impeller/geometry/path_builder.h" #include "impeller/geometry/scalar.h" #include "impeller/geometry/sigma.h" +#include "impeller/typographer/backends/skia/text_frame_skia.h" #if IMPELLER_ENABLE_3D #include "impeller/entity/contents/scene_contents.h" @@ -1060,14 +1061,20 @@ void DlDispatcher::drawDisplayList( void DlDispatcher::drawTextBlob(const sk_sp blob, SkScalar x, SkScalar y) { - // When running with Impeller enabled Skia text blobs are converted to - // Impeller text frames in paragraph_skia.cc - UNIMPLEMENTED; -} + const auto maybe_text_frame = MakeTextFrameFromTextBlobSkia(blob); + if (!maybe_text_frame.has_value()) { + return; + } + const auto text_frame = maybe_text_frame.value(); + if (paint_.style == Paint::Style::kStroke || + paint_.color_source.GetType() != ColorSource::Type::kColor) { + auto bounds = blob->bounds(); + auto path = skia_conversions::PathDataFromTextBlob( + blob, Point(x + bounds.left(), y + bounds.top())); + canvas_.DrawPath(path, paint_); + return; + } -void DlDispatcher::drawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) { canvas_.DrawTextFrame(text_frame, // impeller::Point{x, y}, // paint_ // diff --git a/impeller/display_list/dl_dispatcher.h b/impeller/display_list/dl_dispatcher.h index 0dc3b7ce72461..d9a36e2239980 100644 --- a/impeller/display_list/dl_dispatcher.h +++ b/impeller/display_list/dl_dispatcher.h @@ -212,11 +212,6 @@ class DlDispatcher final : public flutter::DlOpReceiver { SkScalar x, SkScalar y) override; - // |flutter::DlOpReceiver| - void drawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) override; - // |flutter::DlOpReceiver| void drawShadow(const SkPath& path, const flutter::DlColor color, diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index c17c47ae20b31..fceab90a6ed26 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -24,8 +24,8 @@ TextContents::TextContents() = default; TextContents::~TextContents() = default; -void TextContents::SetTextFrame(const std::shared_ptr& frame) { - frame_ = frame; +void TextContents::SetTextFrame(TextFrame&& frame) { + frame_ = std::move(frame); } std::shared_ptr TextContents::ResolveAtlas( @@ -49,7 +49,7 @@ Color TextContents::GetColor() const { } bool TextContents::CanInheritOpacity(const Entity& entity) const { - return !frame_->MaybeHasOverlapping(); + return !frame_.MaybeHasOverlapping(); } void TextContents::SetInheritedOpacity(Scalar opacity) { @@ -61,13 +61,13 @@ void TextContents::SetOffset(Vector2 offset) { } std::optional TextContents::GetCoverage(const Entity& entity) const { - return frame_->GetBounds().TransformBounds(entity.GetTransformation()); + return frame_.GetBounds().TransformBounds(entity.GetTransformation()); } void TextContents::PopulateGlyphAtlas( const std::shared_ptr& lazy_glyph_atlas, Scalar scale) { - lazy_glyph_atlas->AddTextFrame(*frame_, scale); + lazy_glyph_atlas->AddTextFrame(frame_, scale); scale_ = scale; } @@ -79,7 +79,7 @@ bool TextContents::Render(const ContentContext& renderer, return true; } - auto type = frame_->GetAtlasType(); + auto type = frame_.GetAtlasType(); auto atlas = ResolveAtlas(*renderer.GetContext(), type, renderer.GetLazyGlyphAtlas()); @@ -152,7 +152,7 @@ bool TextContents::Render(const ContentContext& renderer, auto& host_buffer = pass.GetTransientsBuffer(); size_t vertex_count = 0; - for (const auto& run : frame_->GetRuns()) { + for (const auto& run : frame_.GetRuns()) { vertex_count += run.GetGlyphPositions().size(); } vertex_count *= 6; @@ -163,7 +163,7 @@ bool TextContents::Render(const ContentContext& renderer, VS::PerVertexData vtx; VS::PerVertexData* vtx_contents = reinterpret_cast(contents); - for (const TextRun& run : frame_->GetRuns()) { + for (const TextRun& run : frame_.GetRuns()) { const Font& font = run.GetFont(); Scalar rounded_scale = TextFrame::RoundScaledFontSize( scale_, font.GetMetrics().point_size); diff --git a/impeller/entity/contents/text_contents.h b/impeller/entity/contents/text_contents.h index 07d191ef3caad..6e3b89d558e61 100644 --- a/impeller/entity/contents/text_contents.h +++ b/impeller/entity/contents/text_contents.h @@ -26,7 +26,7 @@ class TextContents final : public Contents { ~TextContents(); - void SetTextFrame(const std::shared_ptr& frame); + void SetTextFrame(TextFrame&& frame); void SetColor(Color color); @@ -56,7 +56,7 @@ class TextContents final : public Contents { RenderPass& pass) const override; private: - std::shared_ptr frame_; + TextFrame frame_; Scalar scale_ = 1.0; Color color_; Scalar inherited_opacity_ = 1.0; diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index b5d36ca2abad8..c01a4ae11c7c1 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2173,13 +2173,13 @@ TEST_P(EntityTest, InheritOpacityTest) { SkFont font; font.setSize(30); auto blob = SkTextBlob::MakeFromString("A", font); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); auto lazy_glyph_atlas = std::make_shared(TypographerContextSkia::Make()); - lazy_glyph_atlas->AddTextFrame(*frame, 1.0f); + lazy_glyph_atlas->AddTextFrame(frame, 1.0f); auto text_contents = std::make_shared(); - text_contents->SetTextFrame(frame); + text_contents->SetTextFrame(std::move(frame)); text_contents->SetColor(Color::Blue().WithAlpha(0.5)); ASSERT_TRUE(text_contents->CanInheritOpacity(entity)); diff --git a/impeller/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc index b5d7ee8c468e5..11c256a8af918 100644 --- a/impeller/typographer/backends/skia/text_frame_skia.cc +++ b/impeller/typographer/backends/skia/text_frame_skia.cc @@ -8,6 +8,7 @@ #include "flutter/fml/logging.h" #include "impeller/typographer/backends/skia/typeface_skia.h" +#include "include/core/SkFontTypes.h" #include "include/core/SkRect.h" #include "third_party/skia/include/core/SkFont.h" #include "third_party/skia/include/core/SkFontMetrics.h" @@ -38,10 +39,16 @@ static Rect ToRect(const SkRect& rect) { static constexpr Scalar kScaleSize = 100000.0f; -std::shared_ptr MakeTextFrameFromTextBlobSkia( +std::optional MakeTextFrameFromTextBlobSkia( const sk_sp& blob) { - bool has_color = false; + // Handling nullptr text blobs feels overly defensive here, as I don't + // actually know if this happens. + if (!blob) { + return {}; + } + std::vector runs; + bool has_color = false; for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) { // TODO(jonahwilliams): ask Skia for a public API to look this up. // https://github.com/flutter/flutter/issues/112005 @@ -88,7 +95,7 @@ std::shared_ptr MakeTextFrameFromTextBlobSkia( continue; } } - return std::make_shared(runs, ToRect(blob->bounds()), has_color); + return TextFrame(runs, ToRect(blob->bounds()), has_color); } } // namespace impeller diff --git a/impeller/typographer/backends/skia/text_frame_skia.h b/impeller/typographer/backends/skia/text_frame_skia.h index e7721a0c6e9c7..53170a248d3a1 100644 --- a/impeller/typographer/backends/skia/text_frame_skia.h +++ b/impeller/typographer/backends/skia/text_frame_skia.h @@ -4,12 +4,13 @@ #pragma once +#include "flutter/fml/macros.h" #include "impeller/typographer/text_frame.h" #include "third_party/skia/include/core/SkTextBlob.h" namespace impeller { -std::shared_ptr MakeTextFrameFromTextBlobSkia( +std::optional MakeTextFrameFromTextBlobSkia( const sk_sp& blob); } // namespace impeller diff --git a/impeller/typographer/backends/stb/text_frame_stb.cc b/impeller/typographer/backends/stb/text_frame_stb.cc index e1d1dd0d1c662..5e6060ba32551 100644 --- a/impeller/typographer/backends/stb/text_frame_stb.cc +++ b/impeller/typographer/backends/stb/text_frame_stb.cc @@ -8,10 +8,9 @@ namespace impeller { -std::shared_ptr MakeTextFrameSTB( - const std::shared_ptr& typeface_stb, - Font::Metrics metrics, - const std::string& text) { +TextFrame MakeTextFrameSTB(const std::shared_ptr& typeface_stb, + Font::Metrics metrics, + const std::string& text) { TextRun run(Font(typeface_stb, metrics)); // Shape the text run using STB. The glyph positions could also be resolved @@ -62,8 +61,7 @@ std::shared_ptr MakeTextFrameSTB( } std::vector runs = {run}; - return std::make_shared( - runs, result.value_or(Rect::MakeLTRB(0, 0, 0, 0)), false); + return TextFrame(runs, result.value_or(Rect::MakeLTRB(0, 0, 0, 0)), false); } } // namespace impeller diff --git a/impeller/typographer/backends/stb/text_frame_stb.h b/impeller/typographer/backends/stb/text_frame_stb.h index 39f8cccc7688a..622cb1f3b72fd 100644 --- a/impeller/typographer/backends/stb/text_frame_stb.h +++ b/impeller/typographer/backends/stb/text_frame_stb.h @@ -10,9 +10,8 @@ namespace impeller { -std::shared_ptr MakeTextFrameSTB( - const std::shared_ptr& typeface_stb, - Font::Metrics metrics, - const std::string& text); +TextFrame MakeTextFrameSTB(const std::shared_ptr& typeface_stb, + Font::Metrics metrics, + const std::string& text); } // namespace impeller diff --git a/impeller/typographer/typographer_context.h b/impeller/typographer/typographer_context.h index 3714dd804070e..1ca4d4a7687c3 100644 --- a/impeller/typographer/typographer_context.h +++ b/impeller/typographer/typographer_context.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include "flutter/fml/macros.h" diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index eb3581d750f0b..a996bdf98d98d 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -40,9 +40,9 @@ TEST_P(TypographerTest, CanConvertTextBlob) { auto blob = SkTextBlob::MakeFromString( "the quick brown fox jumped over the lazy dog.", font); ASSERT_TRUE(blob); - auto frame = MakeTextFrameFromTextBlobSkia(blob); - ASSERT_EQ(frame->GetRunCount(), 1u); - for (const auto& run : frame->GetRuns()) { + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); + ASSERT_EQ(frame.GetRunCount(), 1u); + for (const auto& run : frame.GetRuns()) { ASSERT_TRUE(run.IsValid()); ASSERT_EQ(run.GetGlyphCount(), 45u); } @@ -62,7 +62,7 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas->GetType(), GlyphAtlas::Type::kAlphaBitmap); @@ -113,20 +113,21 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { auto blob = SkTextBlob::MakeFromString("hello", sk_font); ASSERT_TRUE(blob); - auto frame = MakeTextFrameFromTextBlobSkia(blob); + auto frame = MakeTextFrameFromTextBlobSkia(blob).value(); - ASSERT_FALSE(frame->GetAtlasType() == GlyphAtlas::Type::kColorBitmap); + ASSERT_FALSE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); LazyGlyphAtlas lazy_atlas(TypographerContextSkia::Make()); - lazy_atlas.AddTextFrame(*frame, 1.0f); + lazy_atlas.AddTextFrame(frame, 1.0f); frame = MakeTextFrameFromTextBlobSkia( - SkTextBlob::MakeFromString("😀 ", emoji_font)); + SkTextBlob::MakeFromString("😀 ", emoji_font)) + .value(); - ASSERT_TRUE(frame->GetAtlasType() == GlyphAtlas::Type::kColorBitmap); + ASSERT_TRUE(frame.GetAtlasType() == GlyphAtlas::Type::kColorBitmap); - lazy_atlas.AddTextFrame(*frame, 1.0f); + lazy_atlas.AddTextFrame(frame, 1.0f); // Creates different atlases for color and alpha bitmap. auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas( @@ -147,7 +148,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); @@ -164,7 +165,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_NE(atlas, nullptr); ASSERT_NE(atlas->GetTexture(), nullptr); ASSERT_EQ(atlas, atlas_context->GetGlyphAtlas()); @@ -173,7 +174,7 @@ TEST_P(TypographerTest, GlyphAtlasIsRecycledIfUnchanged) { auto next_atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); ASSERT_EQ(atlas, next_atlas); ASSERT_EQ(atlas_context->GetGlyphAtlas(), atlas); } @@ -196,7 +197,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { FontGlyphMap font_glyph_map; size_t size_count = 8; for (size_t index = 0; index < size_count; index += 1) { - MakeTextFrameFromTextBlobSkia(blob)->CollectUniqueFontGlyphPairs( + MakeTextFrameFromTextBlobSkia(blob).value().CollectUniqueFontGlyphPairs( font_glyph_map, 0.6 * index); }; auto atlas = @@ -231,7 +232,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -245,7 +246,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) { auto blob2 = SkTextBlob::MakeFromString("spooky 2", sk_font); auto next_atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob2)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob2).value()); ASSERT_EQ(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -264,7 +265,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { ASSERT_TRUE(blob); auto atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kAlphaBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob).value()); auto old_packer = atlas_context->GetRectPacker(); ASSERT_NE(atlas, nullptr); @@ -279,7 +280,7 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecreatedIfTypeChanges) { auto blob2 = SkTextBlob::MakeFromString("spooky 1", sk_font); auto next_atlas = CreateGlyphAtlas( *GetContext(), context.get(), GlyphAtlas::Type::kColorBitmap, 1.0f, - atlas_context, *MakeTextFrameFromTextBlobSkia(blob2)); + atlas_context, MakeTextFrameFromTextBlobSkia(blob2).value()); ASSERT_NE(atlas, next_atlas); auto* second_texture = next_atlas->GetTexture().get(); @@ -296,13 +297,15 @@ TEST_P(TypographerTest, MaybeHasOverlapping) { SkFont sk_font(typeface, 0.5f); auto frame = - MakeTextFrameFromTextBlobSkia(SkTextBlob::MakeFromString("1", sk_font)); + MakeTextFrameFromTextBlobSkia(SkTextBlob::MakeFromString("1", sk_font)) + .value(); // Single character has no overlapping - ASSERT_FALSE(frame->MaybeHasOverlapping()); + ASSERT_FALSE(frame.MaybeHasOverlapping()); auto frame_2 = MakeTextFrameFromTextBlobSkia( - SkTextBlob::MakeFromString("123456789", sk_font)); - ASSERT_FALSE(frame_2->MaybeHasOverlapping()); + SkTextBlob::MakeFromString("123456789", sk_font)) + .value(); + ASSERT_FALSE(frame_2.MaybeHasOverlapping()); } TEST_P(TypographerTest, RectanglePackerAddsNonoverlapingRectangles) { diff --git a/shell/common/dl_op_spy.cc b/shell/common/dl_op_spy.cc index bcf213bffa8e9..15910fc2e85de 100644 --- a/shell/common/dl_op_spy.cc +++ b/shell/common/dl_op_spy.cc @@ -127,14 +127,6 @@ void DlOpSpy::drawTextBlob(const sk_sp blob, SkScalar y) { did_draw_ |= will_draw_; } - -void DlOpSpy::drawTextFrame( - const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) { - did_draw_ |= will_draw_; -} - void DlOpSpy::drawShadow(const SkPath& path, const DlColor color, const SkScalar elevation, diff --git a/shell/common/dl_op_spy.h b/shell/common/dl_op_spy.h index 1bcf08c7a84c5..1cb29ff7219aa 100644 --- a/shell/common/dl_op_spy.h +++ b/shell/common/dl_op_spy.h @@ -90,9 +90,6 @@ class DlOpSpy final : public virtual DlOpReceiver, void drawTextBlob(const sk_sp blob, SkScalar x, SkScalar y) override; - void drawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) override; void drawShadow(const SkPath& path, const DlColor color, const SkScalar elevation, diff --git a/testing/display_list_testing.cc b/testing/display_list_testing.cc index 3ddeeac6cd92c..f54a90c192d19 100644 --- a/testing/display_list_testing.cc +++ b/testing/display_list_testing.cc @@ -859,15 +859,6 @@ void DisplayListStreamDispatcher::drawTextBlob(const sk_sp blob, << blob.get() << ", " << x << ", " << y << ");" << std::endl; } - -void DisplayListStreamDispatcher::drawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) { - startl() << "drawTextFrame(" - << text_frame.get() << ", " - << x << ", " << y << ");" << std::endl; -} - void DisplayListStreamDispatcher::drawShadow(const SkPath& path, const DlColor color, const SkScalar elevation, diff --git a/testing/display_list_testing.h b/testing/display_list_testing.h index a468deda878e2..4f3830a204b44 100644 --- a/testing/display_list_testing.h +++ b/testing/display_list_testing.h @@ -144,9 +144,6 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void drawTextBlob(const sk_sp blob, SkScalar x, SkScalar y) override; - void drawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) override; void drawShadow(const SkPath& path, const DlColor color, const SkScalar elevation, diff --git a/testing/mock_canvas.cc b/testing/mock_canvas.cc index 8d5a51fd1b6c6..75b03107e2a75 100644 --- a/testing/mock_canvas.cc +++ b/testing/mock_canvas.cc @@ -160,14 +160,6 @@ void MockCanvas::DrawTextBlob(const sk_sp& text, paint, SkPoint::Make(x, y)}}); } -void MockCanvas::DrawTextFrame( - const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y, - const DlPaint& paint) { - FML_DCHECK(false); -} - void MockCanvas::DrawRect(const SkRect& rect, const DlPaint& paint) { draw_calls_.emplace_back(DrawCall{current_layer_, DrawRectData{rect, paint}}); } diff --git a/testing/mock_canvas.h b/testing/mock_canvas.h index 591f1c908e9fb..71260d5b8755f 100644 --- a/testing/mock_canvas.h +++ b/testing/mock_canvas.h @@ -273,10 +273,6 @@ class MockCanvas final : public DlCanvas { SkScalar x, SkScalar y, const DlPaint& paint) override; - void DrawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y, - const DlPaint& paint) override; void DrawShadow(const SkPath& path, const DlColor color, const SkScalar elevation, diff --git a/third_party/txt/BUILD.gn b/third_party/txt/BUILD.gn index a61438b72c5eb..b799bc5dd46d5 100644 --- a/third_party/txt/BUILD.gn +++ b/third_party/txt/BUILD.gn @@ -81,7 +81,6 @@ source_set("txt") { public_deps = [ "//flutter/display_list", "//flutter/fml", - "//flutter/impeller/typographer/backends/skia:typographer_skia_backend", "//third_party/harfbuzz", "//third_party/icu", "//third_party/skia", diff --git a/third_party/txt/src/skia/paragraph_skia.cc b/third_party/txt/src/skia/paragraph_skia.cc index a688720cfced9..76d737224aa79 100644 --- a/third_party/txt/src/skia/paragraph_skia.cc +++ b/third_party/txt/src/skia/paragraph_skia.cc @@ -18,10 +18,7 @@ #include #include -#include "display_list/dl_paint.h" #include "fml/logging.h" -#include "impeller/typographer/backends/skia/text_frame_skia.h" -#include "include/core/SkMatrix.h" namespace txt { @@ -68,10 +65,10 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { /// decision (i.e. with `#ifdef`) instead of a runtime option. DisplayListParagraphPainter(DisplayListBuilder* builder, const std::vector& dl_paints, - bool impeller_enabled) + bool draw_path_effect) : builder_(builder), dl_paints_(dl_paints), - impeller_enabled_(impeller_enabled) {} + draw_path_effect_(draw_path_effect) {} void drawTextBlob(const sk_sp& blob, SkScalar x, @@ -82,22 +79,6 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { } size_t paint_id = std::get(paint); FML_DCHECK(paint_id < dl_paints_.size()); - -#ifdef IMPELLER_SUPPORTS_RENDERING - if (impeller_enabled_) { - if (ShouldRenderAsPath(dl_paints_[paint_id])) { - auto path = skia::textlayout::Paragraph::GetPath(blob.get()); - auto transformed = path.makeTransform(SkMatrix::Translate( - x + blob->bounds().left(), y + blob->bounds().top())); - builder_->DrawPath(transformed, dl_paints_[paint_id]); - return; - } - - builder_->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(blob), x, - y, dl_paints_[paint_id]); - return; - } -#endif // IMPELLER_SUPPORTS_RENDERING builder_->DrawTextBlob(blob, x, y, dl_paints_[paint_id]); } @@ -115,11 +96,6 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { DlBlurMaskFilter filter(DlBlurStyle::kNormal, blur_sigma, false); paint.setMaskFilter(&filter); } - if (impeller_enabled_) { - builder_->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(blob), x, - y, paint); - return; - } builder_->DrawTextBlob(blob, x, y, paint); } @@ -153,13 +129,11 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { // the line directly using the `drawLine` API instead of using a path effect // (because Impeller does not support path effects). auto dash_path_effect = decor_style.getDashPathEffect(); -#ifdef IMPELLER_SUPPORTS_RENDERING - if (impeller_enabled_ && dash_path_effect) { + if (draw_path_effect_ && dash_path_effect) { auto path = dashedLine(x0, x1, y0, *dash_path_effect); builder_->DrawPath(path, toDlPaint(decor_style)); return; } -#endif // IMPELLER_SUPPORTS_RENDERING auto paint = toDlPaint(decor_style); if (dash_path_effect) { @@ -206,16 +180,6 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { return path; } - bool ShouldRenderAsPath(const DlPaint& paint) const { - FML_DCHECK(impeller_enabled_); - // Text with non-trivial color sources or stroke paint mode should be - // rendered as a path when running on Impeller for correctness. These - // filters rely on having the glyph coverage, whereas regular text is - // drawn as rectangular texture samples. - return ((paint.getColorSource() && !paint.getColorSource()->asColor()) || - paint.getDrawStyle() == DlDrawStyle::kStroke); - } - DlPaint toDlPaint(const DecorationStyle& decor_style, DlDrawStyle draw_style = DlDrawStyle::kStroke) { DlPaint paint; @@ -228,7 +192,7 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { void setPathEffect(DlPaint& paint, const DashPathEffect& dash_path_effect) { // Impeller does not support path effects, so we should never be setting. - FML_DCHECK(!impeller_enabled_); + FML_DCHECK(!draw_path_effect_); std::array intervals{dash_path_effect.fOnLength, dash_path_effect.fOffLength}; @@ -238,7 +202,7 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { DisplayListBuilder* builder_; const std::vector& dl_paints_; - const bool impeller_enabled_; + bool draw_path_effect_; }; } // anonymous namespace diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index 7eaf771c6cde8..ec06ce702512e 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -3,13 +3,8 @@ // found in the LICENSE file. #include -#include "display_list/dl_color.h" -#include "display_list/dl_paint.h" -#include "display_list/dl_tile_mode.h" -#include "display_list/effects/dl_color_source.h" #include "display_list/utils/dl_receiver_utils.h" #include "gtest/gtest.h" -#include "include/core/SkScalar.h" #include "runtime/test_font_data.h" #include "skia/paragraph_builder_skia.h" #include "testing/canvas_test.h" @@ -28,8 +23,6 @@ class DlOpRecorder final : public virtual DlOpReceiver, int lineCount() const { return lines_.size(); } int rectCount() const { return rects_.size(); } int pathCount() const { return paths_.size(); } - int textFrameCount() const { return text_frames_.size(); } - int blobCount() const { return blobs_.size(); } bool hasPathEffect() const { return path_effect_ != nullptr; } private: @@ -37,18 +30,6 @@ class DlOpRecorder final : public virtual DlOpReceiver, lines_.emplace_back(p0, p1); } - void drawTextFrame(const std::shared_ptr& text_frame, - SkScalar x, - SkScalar y) override { - text_frames_.push_back(text_frame); - } - - void drawTextBlob(const sk_sp blob, - SkScalar x, - SkScalar y) override { - blobs_.push_back(blob); - } - void drawRect(const SkRect& rect) override { rects_.push_back(rect); } void drawPath(const SkPath& path) override { paths_.push_back(path); } @@ -57,8 +38,6 @@ class DlOpRecorder final : public virtual DlOpReceiver, path_effect_ = effect; } - std::vector> text_frames_; - std::vector> blobs_; std::vector> lines_; std::vector rects_; std::vector paths_; @@ -73,30 +52,10 @@ class PainterTestBase : public CanvasTestBase { void PretendImpellerIsEnabled(bool impeller) { impeller_ = impeller; } protected: - txt::TextStyle makeDecoratedStyle(txt::TextDecorationStyle style) { - auto t_style = txt::TextStyle(); - t_style.color = SK_ColorBLACK; // default - t_style.font_weight = txt::FontWeight::w400; // normal - t_style.font_size = 14; // default - t_style.decoration = txt::TextDecoration::kUnderline; - t_style.decoration_style = style; - t_style.decoration_color = SK_ColorBLACK; - t_style.font_families.push_back("ahem"); - return t_style; - } - - txt::TextStyle makeStyle() { - auto t_style = txt::TextStyle(); - t_style.color = SK_ColorBLACK; // default - t_style.font_weight = txt::FontWeight::w400; // normal - t_style.font_size = 14; // default - t_style.font_families.push_back("ahem"); - return t_style; - } - - sk_sp draw(txt::TextStyle style) const { + sk_sp draw(txt::TextDecorationStyle style) const { + auto t_style = makeDecoratedStyle(style); auto pb_skia = makeParagraphBuilder(); - pb_skia.PushStyle(style); + pb_skia.PushStyle(t_style); pb_skia.AddText(u"Hello World!"); pb_skia.Pop(); @@ -126,6 +85,18 @@ class PainterTestBase : public CanvasTestBase { return txt::ParagraphBuilderSkia(p_style, f_collection, impeller_); } + txt::TextStyle makeDecoratedStyle(txt::TextDecorationStyle style) const { + auto t_style = txt::TextStyle(); + t_style.color = SK_ColorBLACK; // default + t_style.font_weight = txt::FontWeight::w400; // normal + t_style.font_size = 14; // default + t_style.decoration = txt::TextDecoration::kUnderline; + t_style.decoration_style = style; + t_style.decoration_color = SK_ColorBLACK; + t_style.font_families.push_back("ahem"); + return t_style; + } + bool impeller_ = false; }; @@ -135,8 +106,7 @@ TEST_F(PainterTest, DrawsSolidLineSkia) { PretendImpellerIsEnabled(false); auto recorder = DlOpRecorder(); - draw(makeDecoratedStyle(txt::TextDecorationStyle::kSolid)) - ->Dispatch(recorder); + draw(txt::TextDecorationStyle::kSolid)->Dispatch(recorder); // Skia may draw a solid underline as a filled rectangle: // https://skia.googlesource.com/skia/+/refs/heads/main/modules/skparagraph/src/Decorations.cpp#91 @@ -144,25 +114,11 @@ TEST_F(PainterTest, DrawsSolidLineSkia) { EXPECT_FALSE(recorder.hasPathEffect()); } -TEST_F(PainterTest, DrawDashedLineSkia) { - PretendImpellerIsEnabled(false); - - auto recorder = DlOpRecorder(); - draw(makeDecoratedStyle(txt::TextDecorationStyle::kDashed)) - ->Dispatch(recorder); - - // Skia draws a dashed underline as a filled rectangle with a path effect. - EXPECT_EQ(recorder.lineCount(), 1); - EXPECT_TRUE(recorder.hasPathEffect()); -} - -#ifdef IMPELLER_SUPPORTS_RENDERING TEST_F(PainterTest, DrawsSolidLineImpeller) { PretendImpellerIsEnabled(true); auto recorder = DlOpRecorder(); - draw(makeDecoratedStyle(txt::TextDecorationStyle::kSolid)) - ->Dispatch(recorder); + draw(txt::TextDecorationStyle::kSolid)->Dispatch(recorder); // Skia may draw a solid underline as a filled rectangle: // https://skia.googlesource.com/skia/+/refs/heads/main/modules/skparagraph/src/Decorations.cpp#91 @@ -170,76 +126,27 @@ TEST_F(PainterTest, DrawsSolidLineImpeller) { EXPECT_FALSE(recorder.hasPathEffect()); } -TEST_F(PainterTest, DrawDashedLineImpeller) { - PretendImpellerIsEnabled(true); - - auto recorder = DlOpRecorder(); - draw(makeDecoratedStyle(txt::TextDecorationStyle::kDashed)) - ->Dispatch(recorder); - - // Impeller draws a dashed underline as a path. - EXPECT_EQ(recorder.pathCount(), 1); - EXPECT_FALSE(recorder.hasPathEffect()); -} - -TEST_F(PainterTest, DrawTextFrameImpeller) { - PretendImpellerIsEnabled(true); - - auto recorder = DlOpRecorder(); - draw(makeStyle())->Dispatch(recorder); - - EXPECT_EQ(recorder.textFrameCount(), 1); - EXPECT_EQ(recorder.blobCount(), 0); -} - -TEST_F(PainterTest, DrawStrokedTextImpeller) { - PretendImpellerIsEnabled(true); - - auto style = makeStyle(); - // What is your shtyle? - DlPaint foreground; - foreground.setDrawStyle(DlDrawStyle::kStroke); - style.foreground = foreground; +TEST_F(PainterTest, DrawDashedLineSkia) { + PretendImpellerIsEnabled(false); auto recorder = DlOpRecorder(); - draw(style)->Dispatch(recorder); + draw(txt::TextDecorationStyle::kDashed)->Dispatch(recorder); - EXPECT_EQ(recorder.textFrameCount(), 0); - EXPECT_EQ(recorder.blobCount(), 0); - EXPECT_EQ(recorder.pathCount(), 1); + // Skia draws a dashed underline as a filled rectangle with a path effect. + EXPECT_EQ(recorder.lineCount(), 1); + EXPECT_TRUE(recorder.hasPathEffect()); } -TEST_F(PainterTest, DrawTextWithGradientImpeller) { +TEST_F(PainterTest, DrawDashedLineImpeller) { PretendImpellerIsEnabled(true); - auto style = makeStyle(); - // how do you like my shtyle? - DlPaint foreground; - std::vector colors = {DlColor::kRed(), DlColor::kCyan()}; - std::vector stops = {0.0, 1.0}; - foreground.setColorSource(DlColorSource::MakeLinear( - SkPoint::Make(0, 0), SkPoint::Make(100, 100), 2, colors.data(), - stops.data(), DlTileMode::kClamp)); - style.foreground = foreground; - auto recorder = DlOpRecorder(); - draw(style)->Dispatch(recorder); + draw(txt::TextDecorationStyle::kDashed)->Dispatch(recorder); - EXPECT_EQ(recorder.textFrameCount(), 0); - EXPECT_EQ(recorder.blobCount(), 0); + // Impeller draws a dashed underline as a path. EXPECT_EQ(recorder.pathCount(), 1); + EXPECT_FALSE(recorder.hasPathEffect()); } -TEST_F(PainterTest, DrawTextBlobNoImpeller) { - PretendImpellerIsEnabled(false); - - auto recorder = DlOpRecorder(); - draw(makeStyle())->Dispatch(recorder); - - EXPECT_EQ(recorder.textFrameCount(), 0); - EXPECT_EQ(recorder.blobCount(), 1); -} -#endif // IMPELLER_SUPPORTS_RENDERING - } // namespace testing } // namespace flutter