diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index eb01511772d49..1748bf91ff9da 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -300,8 +300,9 @@ ParagraphBuilder::ParagraphBuilder( ->client() ->GetFontCollection(); + auto impeller_enabled = UIDartState::Current()->IsImpellerEnabled(); m_paragraphBuilder = txt::ParagraphBuilder::CreateSkiaBuilder( - style, font_collection.GetFontCollection()); + style, font_collection.GetFontCollection(), impeller_enabled); } ParagraphBuilder::~ParagraphBuilder() = default; diff --git a/third_party/txt/BUILD.gn b/third_party/txt/BUILD.gn index 1f57ef313a84e..b799bc5dd46d5 100644 --- a/third_party/txt/BUILD.gn +++ b/third_party/txt/BUILD.gn @@ -143,6 +143,7 @@ if (enable_unittests) { sources = [ "tests/font_collection_tests.cc", + "tests/paragraph_unittests.cc", "tests/txt_run_all_unittests.cc", ] @@ -154,7 +155,10 @@ if (enable_unittests) { ":txt", ":txt_fixtures", "//flutter/fml", + "//flutter/runtime:test_font", + "//flutter/testing:skia", "//flutter/testing:testing_lib", + "//third_party/skia/modules/skparagraph:skparagraph", ] # This is needed for //third_party/googletest for linking zircon symbols. diff --git a/third_party/txt/src/skia/paragraph_builder_skia.cc b/third_party/txt/src/skia/paragraph_builder_skia.cc index 641725ec6ccfb..2028e80353c50 100644 --- a/third_party/txt/src/skia/paragraph_builder_skia.cc +++ b/third_party/txt/src/skia/paragraph_builder_skia.cc @@ -46,8 +46,9 @@ SkFontStyle MakeSkFontStyle(txt::FontWeight font_weight, ParagraphBuilderSkia::ParagraphBuilderSkia( const ParagraphStyle& style, - std::shared_ptr font_collection) - : base_style_(style.GetTextStyle()) { + std::shared_ptr font_collection, + const bool impeller_enabled) + : base_style_(style.GetTextStyle()), impeller_enabled_(impeller_enabled) { builder_ = skt::ParagraphBuilder::make( TxtToSkia(style), font_collection->CreateSktFontCollection()); } @@ -85,8 +86,8 @@ void ParagraphBuilderSkia::AddPlaceholder(PlaceholderRun& span) { } std::unique_ptr ParagraphBuilderSkia::Build() { - return std::make_unique(builder_->Build(), - std::move(dl_paints_)); + return std::make_unique( + builder_->Build(), std::move(dl_paints_), impeller_enabled_); } skt::ParagraphPainter::PaintID ParagraphBuilderSkia::CreatePaintID( diff --git a/third_party/txt/src/skia/paragraph_builder_skia.h b/third_party/txt/src/skia/paragraph_builder_skia.h index e08e4610f42af..f14f7fe41f591 100644 --- a/third_party/txt/src/skia/paragraph_builder_skia.h +++ b/third_party/txt/src/skia/paragraph_builder_skia.h @@ -24,11 +24,16 @@ namespace txt { -// Implementation of ParagraphBuilder based on Skia's text layout module. +//------------------------------------------------------------------------------ +/// @brief ParagraphBuilder implementation using Skia's text layout module. +/// +/// @note Despite the suffix "Skia", this class is not specific to Skia +/// and is also used with the Impeller backend. class ParagraphBuilderSkia : public ParagraphBuilder { public: ParagraphBuilderSkia(const ParagraphStyle& style, - std::shared_ptr font_collection); + std::shared_ptr font_collection, + const bool impeller_enabled); virtual ~ParagraphBuilderSkia(); @@ -47,6 +52,14 @@ class ParagraphBuilderSkia : public ParagraphBuilder { std::shared_ptr builder_; TextStyle base_style_; + + /// @brief Whether Impeller is enabled in the runtime. + /// + /// @note As of the time of this writing, this is used to draw text + /// decorations (i.e. dashed and dotted lines) directly using the + /// `drawLine` API, because Impeller's path rendering does not + /// support dashed and dotted lines (but Skia's does). + const bool impeller_enabled_; std::stack txt_style_stack_; std::vector dl_paints_; }; diff --git a/third_party/txt/src/skia/paragraph_skia.cc b/third_party/txt/src/skia/paragraph_skia.cc index c909a34daef6a..76d737224aa79 100644 --- a/third_party/txt/src/skia/paragraph_skia.cc +++ b/third_party/txt/src/skia/paragraph_skia.cc @@ -18,6 +18,7 @@ #include #include +#include "fml/logging.h" namespace txt { @@ -45,27 +46,29 @@ txt::FontStyle GetTxtFontStyle(SkFontStyle::Slant font_slant) { class DisplayListParagraphPainter : public skt::ParagraphPainter { public: + //---------------------------------------------------------------------------- + /// @brief Creates a |skt::ParagraphPainter| that draws to DisplayList. + /// + /// @param builder The display list builder. + /// @param[in] dl_paints The paints referenced by ID in the `drawX` methods. + /// @param[in] draw_path_effect If true, draw path effects directly by + /// drawing multiple lines instead of providing + // a path effect to the paint. + /// + /// @note Impeller does not (and will not) support path effects, but the + /// Skia backend does. That means that if we want to draw dashed + /// and dotted lines, we need to draw them directly using the + /// `drawLine` API instead of using a path effect. + /// + /// See https://github.com/flutter/flutter/issues/126673. It + /// probably makes sense to eventually make this a compile-time + /// decision (i.e. with `#ifdef`) instead of a runtime option. DisplayListParagraphPainter(DisplayListBuilder* builder, - const std::vector& dl_paints) - : builder_(builder), dl_paints_(dl_paints) {} - - DlPaint toDlPaint(const DecorationStyle& decor_style, - DlDrawStyle draw_style = DlDrawStyle::kStroke) { - DlPaint paint; - paint.setDrawStyle(draw_style); - paint.setAntiAlias(true); - paint.setColor(decor_style.getColor()); - paint.setStrokeWidth(decor_style.getStrokeWidth()); - std::optional dash_path_effect = - decor_style.getDashPathEffect(); - if (dash_path_effect) { - std::array intervals{dash_path_effect->fOnLength, - dash_path_effect->fOffLength}; - paint.setPathEffect( - DlDashPathEffect::Make(intervals.data(), intervals.size(), 0)); - } - return paint; - } + const std::vector& dl_paints, + bool draw_path_effect) + : builder_(builder), + dl_paints_(dl_paints), + draw_path_effect_(draw_path_effect) {} void drawTextBlob(const sk_sp& blob, SkScalar x, @@ -118,8 +121,25 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { SkScalar x1, SkScalar y1, const DecorationStyle& decor_style) override { - builder_->DrawLine(SkPoint::Make(x0, y0), SkPoint::Make(x1, y1), - toDlPaint(decor_style)); + // We only support horizontal lines. + FML_DCHECK(y0 == y1); + + // This function is called for both solid and dashed lines. If we're drawing + // a dashed line, and we're using the Impeller backend, then we need to draw + // 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(); + if (draw_path_effect_ && dash_path_effect) { + auto path = dashedLine(x0, x1, y0, *dash_path_effect); + builder_->DrawPath(path, toDlPaint(decor_style)); + return; + } + + auto paint = toDlPaint(decor_style); + if (dash_path_effect) { + setPathEffect(paint, *dash_path_effect); + } + builder_->DrawLine(SkPoint::Make(x0, y0), SkPoint::Make(x1, y1), paint); } void clipRect(const SkRect& rect) override { @@ -135,15 +155,64 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { void restore() override { builder_->Restore(); } private: + SkPath dashedLine(SkScalar x0, + SkScalar x1, + SkScalar y0, + const DashPathEffect& dash_path_effect) { + auto dx = 0.0; + auto path = SkPath(); + auto on = true; + auto length = x1 - x0; + while (dx < length) { + if (on) { + // Draw the on part of the dash. + path.moveTo(x0 + dx, y0); + dx += dash_path_effect.fOnLength; + path.lineTo(x0 + dx, y0); + } else { + // Skip the off part of the dash. + dx += dash_path_effect.fOffLength; + } + on = !on; + } + + path.close(); + return path; + } + + DlPaint toDlPaint(const DecorationStyle& decor_style, + DlDrawStyle draw_style = DlDrawStyle::kStroke) { + DlPaint paint; + paint.setDrawStyle(draw_style); + paint.setAntiAlias(true); + paint.setColor(decor_style.getColor()); + paint.setStrokeWidth(decor_style.getStrokeWidth()); + return paint; + } + + void setPathEffect(DlPaint& paint, const DashPathEffect& dash_path_effect) { + // Impeller does not support path effects, so we should never be setting. + FML_DCHECK(!draw_path_effect_); + + std::array intervals{dash_path_effect.fOnLength, + dash_path_effect.fOffLength}; + auto effect = DlDashPathEffect::Make(intervals.data(), intervals.size(), 0); + paint.setPathEffect(effect); + } + DisplayListBuilder* builder_; const std::vector& dl_paints_; + bool draw_path_effect_; }; } // anonymous namespace ParagraphSkia::ParagraphSkia(std::unique_ptr paragraph, - std::vector&& dl_paints) - : paragraph_(std::move(paragraph)), dl_paints_(dl_paints) {} + std::vector&& dl_paints, + bool impeller_enabled) + : paragraph_(std::move(paragraph)), + dl_paints_(dl_paints), + impeller_enabled_(impeller_enabled) {} double ParagraphSkia::GetMaxWidth() { return SkScalarToDouble(paragraph_->getMaxWidth()); @@ -223,7 +292,7 @@ void ParagraphSkia::Layout(double width) { } bool ParagraphSkia::Paint(DisplayListBuilder* builder, double x, double y) { - DisplayListParagraphPainter painter(builder, dl_paints_); + DisplayListParagraphPainter painter(builder, dl_paints_, impeller_enabled_); paragraph_->paint(&painter, x, y); return true; } diff --git a/third_party/txt/src/skia/paragraph_skia.h b/third_party/txt/src/skia/paragraph_skia.h index 8fa1314209049..5779445e09a71 100644 --- a/third_party/txt/src/skia/paragraph_skia.h +++ b/third_party/txt/src/skia/paragraph_skia.h @@ -29,7 +29,8 @@ namespace txt { class ParagraphSkia : public Paragraph { public: ParagraphSkia(std::unique_ptr paragraph, - std::vector&& dl_paints); + std::vector&& dl_paints, + bool impeller_enabled); virtual ~ParagraphSkia() = default; @@ -75,6 +76,7 @@ class ParagraphSkia : public Paragraph { std::vector dl_paints_; std::optional> line_metrics_; std::vector line_metrics_styles_; + const bool impeller_enabled_; }; } // namespace txt diff --git a/third_party/txt/src/txt/paragraph_builder.cc b/third_party/txt/src/txt/paragraph_builder.cc index ac1f7dc1b859f..41dd9b1a8958a 100644 --- a/third_party/txt/src/txt/paragraph_builder.cc +++ b/third_party/txt/src/txt/paragraph_builder.cc @@ -22,10 +22,18 @@ namespace txt { +//------------------------------------------------------------------------------ +/// @brief Creates a |ParagraphBuilder| based on Skia's text layout module. +/// +/// @param[in] style The style to use for the paragraph. +/// @param[in] font_collection The font collection to use for the paragraph. +/// @param[in] impeller_enabled Whether Impeller is enabled in the runtime. std::unique_ptr ParagraphBuilder::CreateSkiaBuilder( const ParagraphStyle& style, - std::shared_ptr font_collection) { - return std::make_unique(style, font_collection); + std::shared_ptr font_collection, + const bool impeller_enabled) { + return std::make_unique(style, font_collection, + impeller_enabled); } } // namespace txt diff --git a/third_party/txt/src/txt/paragraph_builder.h b/third_party/txt/src/txt/paragraph_builder.h index 67376940031e0..acd5e952c2fce 100644 --- a/third_party/txt/src/txt/paragraph_builder.h +++ b/third_party/txt/src/txt/paragraph_builder.h @@ -33,7 +33,8 @@ class ParagraphBuilder { public: static std::unique_ptr CreateSkiaBuilder( const ParagraphStyle& style, - std::shared_ptr font_collection); + std::shared_ptr font_collection, + const bool impeller_enabled); virtual ~ParagraphBuilder() = default; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc new file mode 100644 index 0000000000000..ec06ce702512e --- /dev/null +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -0,0 +1,152 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include +#include "display_list/utils/dl_receiver_utils.h" +#include "gtest/gtest.h" +#include "runtime/test_font_data.h" +#include "skia/paragraph_builder_skia.h" +#include "testing/canvas_test.h" + +namespace flutter { +namespace testing { + +//------------------------------------------------------------------------------ +/// @brief A custom |DlOpReceiver| that records some |DlOps| it receives. +class DlOpRecorder final : public virtual DlOpReceiver, + private IgnoreAttributeDispatchHelper, + private IgnoreClipDispatchHelper, + private IgnoreDrawDispatchHelper, + private IgnoreTransformDispatchHelper { + public: + int lineCount() const { return lines_.size(); } + int rectCount() const { return rects_.size(); } + int pathCount() const { return paths_.size(); } + bool hasPathEffect() const { return path_effect_ != nullptr; } + + private: + void drawLine(const SkPoint& p0, const SkPoint& p1) override { + lines_.emplace_back(p0, p1); + } + + void drawRect(const SkRect& rect) override { rects_.push_back(rect); } + + void drawPath(const SkPath& path) override { paths_.push_back(path); } + + void setPathEffect(const DlPathEffect* effect) override { + path_effect_ = effect; + } + + std::vector> lines_; + std::vector rects_; + std::vector paths_; + const DlPathEffect* path_effect_; +}; + +template +class PainterTestBase : public CanvasTestBase { + public: + PainterTestBase() = default; + + void PretendImpellerIsEnabled(bool impeller) { impeller_ = impeller; } + + protected: + sk_sp draw(txt::TextDecorationStyle style) const { + auto t_style = makeDecoratedStyle(style); + auto pb_skia = makeParagraphBuilder(); + pb_skia.PushStyle(t_style); + pb_skia.AddText(u"Hello World!"); + pb_skia.Pop(); + + auto builder = DisplayListBuilder(); + auto paragraph = pb_skia.Build(); + paragraph->Layout(10000); + paragraph->Paint(&builder, 0, 0); + + return builder.Build(); + } + + private: + std::shared_ptr makeFontCollection() const { + auto f_collection = std::make_shared(); + auto font_provider = std::make_unique(); + for (auto& font : GetTestFontData()) { + font_provider->RegisterTypeface(font); + } + auto manager = sk_make_sp(std::move(font_provider)); + f_collection->SetAssetFontManager(manager); + return f_collection; + } + + txt::ParagraphBuilderSkia makeParagraphBuilder() const { + auto p_style = txt::ParagraphStyle(); + auto f_collection = makeFontCollection(); + 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; +}; + +using PainterTest = PainterTestBase<::testing::Test>; + +TEST_F(PainterTest, DrawsSolidLineSkia) { + PretendImpellerIsEnabled(false); + + auto recorder = DlOpRecorder(); + 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 + EXPECT_EQ(recorder.rectCount(), 1); + EXPECT_FALSE(recorder.hasPathEffect()); +} + +TEST_F(PainterTest, DrawsSolidLineImpeller) { + PretendImpellerIsEnabled(true); + + auto recorder = DlOpRecorder(); + 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 + EXPECT_EQ(recorder.rectCount(), 1); + EXPECT_FALSE(recorder.hasPathEffect()); +} + +TEST_F(PainterTest, DrawDashedLineSkia) { + PretendImpellerIsEnabled(false); + + auto recorder = DlOpRecorder(); + draw(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()); +} + +TEST_F(PainterTest, DrawDashedLineImpeller) { + PretendImpellerIsEnabled(true); + + auto recorder = DlOpRecorder(); + draw(txt::TextDecorationStyle::kDashed)->Dispatch(recorder); + + // Impeller draws a dashed underline as a path. + EXPECT_EQ(recorder.pathCount(), 1); + EXPECT_FALSE(recorder.hasPathEffect()); +} + +} // namespace testing +} // namespace flutter diff --git a/third_party/txt/tests/txt_run_all_unittests.cc b/third_party/txt/tests/txt_run_all_unittests.cc index bc1629fe42ba9..7a20fa6b99f9a 100644 --- a/third_party/txt/tests/txt_run_all_unittests.cc +++ b/third_party/txt/tests/txt_run_all_unittests.cc @@ -15,13 +15,10 @@ */ #include "flutter/fml/backtrace.h" -#include "flutter/fml/command_line.h" -#include "flutter/fml/logging.h" #include "flutter/testing/testing.h" int main(int argc, char** argv) { fml::InstallCrashHandler(); - fml::CommandLine cmd = fml::CommandLineFromPlatformOrArgcArgv(argc, argv); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); }