From 408324266fa30761cf5e80733bb24d2c7f9908b5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 27 Aug 2024 20:40:47 -0700 Subject: [PATCH 1/2] fall back to path rendering on thick paths. --- third_party/txt/src/skia/paragraph_skia.cc | 11 +++++++++-- third_party/txt/tests/paragraph_unittests.cc | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/third_party/txt/src/skia/paragraph_skia.cc b/third_party/txt/src/skia/paragraph_skia.cc index 4855e24c6d480..0ec3d1f686575 100644 --- a/third_party/txt/src/skia/paragraph_skia.cc +++ b/third_party/txt/src/skia/paragraph_skia.cc @@ -22,6 +22,7 @@ #include "fml/logging.h" #include "impeller/typographer/backends/skia/text_frame_skia.h" #include "include/core/SkMatrix.h" +#include "third_party/skia/src/core/SkTextBlobPriv.h" // nogncheck namespace txt { @@ -85,7 +86,9 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { #ifdef IMPELLER_SUPPORTS_RENDERING if (impeller_enabled_) { - if (ShouldRenderAsPath(dl_paints_[paint_id])) { + SkTextBlobRunIterator run(blob.get()); + if (ShouldRenderAsPath(dl_paints_[paint_id]) || + ShouldForcePathRendering(run.font(), /*dpr_guess=*/3.0)) { auto path = skia::textlayout::Paragraph::GetPath(blob.get()); // If there is no path, this is an emoji and should be drawn as is, // ignoring the color source. @@ -184,7 +187,11 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { // 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()); + // If the text is stroked and the stroke width is large enough, use path + // rendering anyway, as the fidelity problems won't be as noticable and + // rendering will be faster as it avoids software rasterization. + return (paint.getColorSource() && !paint.getColorSource()->asColor()) || + (paint.getDrawStyle() == DlDrawStyle::kStroke && paint.getStrokeWidth() > 16); } DlPaint toDlPaint(const DecorationStyle& decor_style, diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index f34b83b5331be..de24aa872a3b7 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -259,6 +259,23 @@ TEST_F(PainterTest, DrawStrokedTextImpeller) { EXPECT_EQ(recorder.pathCount(), 0); } +TEST_F(PainterTest, DrawStrokedTextWithLargeWidthImpeller) { + PretendImpellerIsEnabled(true); + + auto style = makeStyle(); + DlPaint foreground; + foreground.setDrawStyle(DlDrawStyle::kStroke); + foreground.setStrokeWidth(100); + style.foreground = foreground; + + auto recorder = DlOpRecorder(); + draw(style)->Dispatch(recorder); + + EXPECT_EQ(recorder.textFrameCount(), 0); + EXPECT_EQ(recorder.blobCount(), 0); + EXPECT_EQ(recorder.pathCount(), 1); +} + TEST_F(PainterTest, DrawTextWithGradientImpeller) { PretendImpellerIsEnabled(true); From ad893236bf5cf5bfb841681ce9402193543179d4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 27 Aug 2024 20:58:57 -0700 Subject: [PATCH 2/2] ++ --- third_party/txt/src/skia/paragraph_skia.cc | 10 ++++++---- third_party/txt/tests/paragraph_unittests.cc | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/third_party/txt/src/skia/paragraph_skia.cc b/third_party/txt/src/skia/paragraph_skia.cc index 0ec3d1f686575..b3521dd529515 100644 --- a/third_party/txt/src/skia/paragraph_skia.cc +++ b/third_party/txt/src/skia/paragraph_skia.cc @@ -87,8 +87,7 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { #ifdef IMPELLER_SUPPORTS_RENDERING if (impeller_enabled_) { SkTextBlobRunIterator run(blob.get()); - if (ShouldRenderAsPath(dl_paints_[paint_id]) || - ShouldForcePathRendering(run.font(), /*dpr_guess=*/3.0)) { + if (ShouldRenderAsPath(dl_paints_[paint_id])) { auto path = skia::textlayout::Paragraph::GetPath(blob.get()); // If there is no path, this is an emoji and should be drawn as is, // ignoring the color source. @@ -189,9 +188,12 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { // samples. // If the text is stroked and the stroke width is large enough, use path // rendering anyway, as the fidelity problems won't be as noticable and - // rendering will be faster as it avoids software rasterization. + // rendering will be faster as it avoids software rasterization. A stroke + // width of four was chosen by eyeballing the point at which the path + // text looks good enough, with some room for error. return (paint.getColorSource() && !paint.getColorSource()->asColor()) || - (paint.getDrawStyle() == DlDrawStyle::kStroke && paint.getStrokeWidth() > 16); + (paint.getDrawStyle() == DlDrawStyle::kStroke && + paint.getStrokeWidth() > 4); } DlPaint toDlPaint(const DecorationStyle& decor_style, diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index de24aa872a3b7..3bbc6250422f5 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -265,7 +265,7 @@ TEST_F(PainterTest, DrawStrokedTextWithLargeWidthImpeller) { auto style = makeStyle(); DlPaint foreground; foreground.setDrawStyle(DlDrawStyle::kStroke); - foreground.setStrokeWidth(100); + foreground.setStrokeWidth(10); style.foreground = foreground; auto recorder = DlOpRecorder();