From cbe36deb82151c2a06115a138444bafd9e0c182f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 31 May 2024 15:14:23 -0700 Subject: [PATCH 1/4] [display_list] allow applying opacity peephole to single glyph. --- display_list/dl_builder.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index 944219c974cf1..bacacb2454945 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -1698,7 +1698,11 @@ void DisplayListBuilder::drawTextFrame( // 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); + // A single glyph can still have the opacity peephole applied (this is + // likely a glyph used as an Icon) + const bool is_single_glyph = text_frame->GetRunCount() == 1u && + text_frame->GetRuns()[0].GetGlyphCount() == 1u; + UpdateLayerOpacityCompatibility(is_single_glyph); UpdateLayerResult(result); } } From ebcdf6eb3383385fadbcb85b9d7dd2d8548c9af5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 3 Jun 2024 13:46:55 -0700 Subject: [PATCH 2/4] add unittests --- display_list/display_list_unittests.cc | 33 ++++++++++++++++++++++++ display_list/testing/dl_test_snippets.cc | 7 +++++ display_list/testing/dl_test_snippets.h | 2 ++ 3 files changed, 42 insertions(+) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index ef423aa7f29f8..55b5443b13e92 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -22,6 +22,7 @@ #include "flutter/testing/display_list_testing.h" #include "flutter/testing/testing.h" +#include "impeller/typographer/backends/skia/text_frame_skia.h" #include "third_party/skia/include/core/SkBBHFactory.h" #include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/core/SkPictureRecorder.h" @@ -4331,5 +4332,37 @@ TEST_F(DisplayListTest, DrawDisplayListForwardsBackdropFlag) { EXPECT_TRUE(parent_dl->root_has_backdrop_filter()); } +TEST_F(DisplayListTest, TextFrameOpacityPeephole) { + // TODO(https://github.com/flutter/flutter/issues/82202): Remove once the + // performance overlay can use Fuchsia's font manager instead of the empty + // default. +#if defined(OS_FUCHSIA) || !defined(IMPELLER_SUPPORTS_RENDERING) + GTEST_SKIP() << "Rendering comparisons require a valid default font manager"; +#else + // Single character can have opacity peephole applied. + { + std::string message = "A"; + sk_sp blob = CreateTextBlob(message); + auto frame = impeller::MakeTextFrameFromTextBlobSkia(blob); + DisplayListBuilder builder; + builder.DrawTextFrame(frame, 0, 0, {}); + auto dl = builder.Build(); + EXPECT_TRUE(dl->can_apply_group_opacity()); + } + + // Multiple characters cannot have opacity peephole applied. + { + std::string message = "ABC"; + sk_sp blob = CreateTextBlob(message); + + auto frame = impeller::MakeTextFrameFromTextBlobSkia(blob); + DisplayListBuilder builder; + builder.DrawTextFrame(frame, 0, 0, {}); + auto dl = builder.Build(); + EXPECT_FALSE(dl->can_apply_group_opacity()); + } +#endif // defined(OS_FUCHSIA) || !defined(IMPELLER_SUPPORTS_RENDERING) +} + } // namespace testing } // namespace flutter diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index 56b36804f6a0b..6cbab1415881b 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -980,5 +980,12 @@ sk_sp GetTestTextBlob(int index) { return blob; } +sk_sp CreateTextBlob(std::string& message) { + sk_sp blob = + SkTextBlob::MakeFromText(message.c_str(), message.size(), + CreateTestFontOfSize(20), SkTextEncoding::kUTF8); + return blob; +} + } // namespace testing } // namespace flutter diff --git a/display_list/testing/dl_test_snippets.h b/display_list/testing/dl_test_snippets.h index e8a8631a2f0e8..393b1217b3988 100644 --- a/display_list/testing/dl_test_snippets.h +++ b/display_list/testing/dl_test_snippets.h @@ -227,6 +227,8 @@ SkFont CreateTestFontOfSize(SkScalar scalar); sk_sp GetTestTextBlob(int index); +sk_sp CreateTextBlob(std::string& message); + struct DisplayListInvocation { // ---------------------------------- // Required fields for initialization From 1381635ba274859da880d0c84fc7653a9427113b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 3 Jun 2024 13:49:28 -0700 Subject: [PATCH 3/4] ++ --- display_list/display_list_unittests.cc | 7 ------- 1 file changed, 7 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 55b5443b13e92..3d27a6a378837 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -4333,12 +4333,6 @@ TEST_F(DisplayListTest, DrawDisplayListForwardsBackdropFlag) { } TEST_F(DisplayListTest, TextFrameOpacityPeephole) { - // TODO(https://github.com/flutter/flutter/issues/82202): Remove once the - // performance overlay can use Fuchsia's font manager instead of the empty - // default. -#if defined(OS_FUCHSIA) || !defined(IMPELLER_SUPPORTS_RENDERING) - GTEST_SKIP() << "Rendering comparisons require a valid default font manager"; -#else // Single character can have opacity peephole applied. { std::string message = "A"; @@ -4361,7 +4355,6 @@ TEST_F(DisplayListTest, TextFrameOpacityPeephole) { auto dl = builder.Build(); EXPECT_FALSE(dl->can_apply_group_opacity()); } -#endif // defined(OS_FUCHSIA) || !defined(IMPELLER_SUPPORTS_RENDERING) } } // namespace testing From 1025c6115b284b4a061e4353d650cb71ac9dffa7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 3 Jun 2024 13:59:57 -0700 Subject: [PATCH 4/4] add include. --- display_list/BUILD.gn | 1 + 1 file changed, 1 insertion(+) diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index dd6aba8d3814b..614ff1260f65b 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -131,6 +131,7 @@ if (enable_unittests) { ":display_list", ":display_list_fixtures", "//flutter/display_list/testing:display_list_testing", + "//flutter/impeller/typographer/backends/skia:typographer_skia_backend", "//flutter/testing", "//flutter/testing:skia", ]