From d73fc34a73abd344500dd3ecd309e4d75b9ff2d3 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Wed, 19 Feb 2025 20:49:11 -0800 Subject: [PATCH] [CP] all jittery glyph fixes (#163058) This is a cherry-pick for all the changes that went into fixing https://github.com/flutter/flutter/issues/149652 It looks like a lot but most of it is testing and refactoring. ## PRs included - https://github.com/flutter/flutter/pull/161625 - https://github.com/flutter/flutter/pull/162351 - https://github.com/flutter/flutter/pull/162415 - https://github.com/flutter/flutter/pull/162555 - https://github.com/flutter/flutter/pull/162824 ## Impacted Users All users of Impeller. ## Impact Description Animating text with translations and scales can cause: - jitter between glyphs - jitter between glyphs and the baseline - artifacts when rendering glyphs at non integer scales ## Workaround Use skia. ## Risk Since this edits how text is rendered, the risk is pretty high. The actual changes are small and there are unit tests for them. Golden test coverage for cherry-picks is not complete and text rendering golden coverage for android is problematic. ## Test Coverage Yes. ## Validation Steps The reproduction code in https://github.com/flutter/flutter/issues/149652 is good. --- CHANGELOG.md | 12 +- .../flutter/ci/licenses_golden/excluded_files | 2 + .../flutter/impeller/display_list/BUILD.gn | 4 + .../display_list/aiks_dl_text_unittests.cc | 35 ++- .../impeller/display_list/dl_dispatcher.cc | 7 +- .../display_list/dl_golden_blur_unittests.cc | 95 +------ .../display_list/dl_golden_unittests.cc | 255 ++++++++++++++++++ .../testing/render_text_in_canvas.cc | 46 ++++ .../testing/render_text_in_canvas.h | 35 +++ .../impeller/display_list/testing/rmse.cc | 47 ++++ .../impeller/display_list/testing/rmse.h | 16 ++ engine/src/flutter/impeller/entity/BUILD.gn | 2 + .../impeller/entity/contents/text_contents.cc | 255 +++++++++--------- .../impeller/entity/contents/text_contents.h | 10 + .../contents/text_contents_unittests.cc | 206 ++++++++++++++ engine/src/flutter/impeller/fixtures/BUILD.gn | 1 + .../backends/skia/typographer_context_skia.cc | 3 +- .../impeller/typographer/lazy_glyph_atlas.cc | 3 +- .../impeller/typographer/lazy_glyph_atlas.h | 1 + .../impeller/typographer/text_frame.cc | 20 +- .../flutter/impeller/typographer/text_frame.h | 7 +- .../typographer/typographer_unittests.cc | 10 +- 22 files changed, 835 insertions(+), 237 deletions(-) create mode 100644 engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.cc create mode 100644 engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.h create mode 100644 engine/src/flutter/impeller/display_list/testing/rmse.cc create mode 100644 engine/src/flutter/impeller/display_list/testing/rmse.h create mode 100644 engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 44a7df79039..c49e4c8cd9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,17 @@ INTERNAL NOTE ## Flutter 3.29 Changes ### [3.29.0](https://github.com/flutter/flutter/releases/tag/3.29.0) -Initial stable release. + +- [flutter/161625](https://github.com/flutter/flutter/pull/161625) - Adds text + tests. +- [flutter/162351](https://github.com/flutter/flutter/pull/162351) - Fixes + floating point math in text. +- [flutter/162415](https://github.com/flutter/flutter/pull/162415) - Fixes + aspect ratio of glyphs. +- [flutter/162555](https://github.com/flutter/flutter/pull/162555) - Increase + glyph atlas resolution. +- [flutter/162824](https://github.com/flutter/flutter/pull/162824) - Fixes + subpixel alignment of glyphs. - [flutter/163304](https://github.com/flutter/flutter/issues/163304) Fixes crash when using backdrop filter on GLES backend. - [flutter/161334](https://github.com/flutter/flutter/issues/161334) Disable Vulkan on certain Xclipse GPU models. diff --git a/engine/src/flutter/ci/licenses_golden/excluded_files b/engine/src/flutter/ci/licenses_golden/excluded_files index 417abf50b25..3d9a1de31f9 100644 --- a/engine/src/flutter/ci/licenses_golden/excluded_files +++ b/engine/src/flutter/ci/licenses_golden/excluded_files @@ -152,6 +152,7 @@ ../../../flutter/impeller/display_list/dl_golden_unittests.h ../../../flutter/impeller/display_list/dl_unittests.cc ../../../flutter/impeller/display_list/skia_conversions_unittests.cc +../../../flutter/impeller/display_list/testing ../../../flutter/impeller/docs ../../../flutter/impeller/entity/clip_stack_unittests.cc ../../../flutter/impeller/entity/contents/filters/blend_filter_contents_unittests.cc @@ -160,6 +161,7 @@ ../../../flutter/impeller/entity/contents/filters/matrix_filter_contents_unittests.cc ../../../flutter/impeller/entity/contents/host_buffer_unittests.cc ../../../flutter/impeller/entity/contents/test +../../../flutter/impeller/entity/contents/text_contents_unittests.cc ../../../flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc ../../../flutter/impeller/entity/draw_order_resolver_unittests.cc ../../../flutter/impeller/entity/entity_pass_target_unittests.cc diff --git a/engine/src/flutter/impeller/display_list/BUILD.gn b/engine/src/flutter/impeller/display_list/BUILD.gn index d52f73ea76a..3c4f1aefc8e 100644 --- a/engine/src/flutter/impeller/display_list/BUILD.gn +++ b/engine/src/flutter/impeller/display_list/BUILD.gn @@ -78,6 +78,10 @@ template("display_list_unittests_component") { "dl_playground.cc", "dl_playground.h", "dl_unittests.cc", + "testing/render_text_in_canvas.cc", + "testing/render_text_in_canvas.h", + "testing/rmse.cc", + "testing/rmse.h", ] additional_sources = [] if (defined(invoker.sources)) { diff --git a/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc b/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc index 9c7932c1642..8e911da902d 100644 --- a/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc +++ b/engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc @@ -33,6 +33,7 @@ struct TextRenderOptions { DlColor color = DlColor::kYellow(); SkPoint position = SkPoint::Make(100, 200); std::shared_ptr filter; + bool is_subpixel = false; }; bool RenderTextInCanvasSkia(const std::shared_ptr& context, @@ -59,6 +60,9 @@ bool RenderTextInCanvasSkia(const std::shared_ptr& context, } sk_sp font_mgr = txt::GetDefaultFontManager(); SkFont sk_font(font_mgr->makeFromData(mapping), options.font_size); + if (options.is_subpixel) { + sk_font.setSubpixel(true); + } auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font); if (!blob) { return false; @@ -154,17 +158,30 @@ TEST_P(AiksTest, CanRenderTextFrameWithHalfScaling) { } TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) { - DisplayListBuilder builder; + Scalar fine_scale = 0.f; + bool is_subpixel = false; + auto callback = [&]() -> sk_sp { + if (AiksTest::ImGuiBegin("Controls", nullptr, + ImGuiWindowFlags_AlwaysAutoResize)) { + ImGui::SliderFloat("Fine Scale", &fine_scale, -1, 1); + ImGui::Checkbox("subpixel", &is_subpixel); + ImGui::End(); + } - DlPaint paint; - paint.setColor(DlColor::ARGB(1, 0.1, 0.1, 0.1)); - builder.DrawPaint(paint); - builder.Scale(2.625, 2.625); + DisplayListBuilder builder; + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0.1, 0.1, 0.1)); + builder.DrawPaint(paint); + Scalar scale = 2.625 + fine_scale; + builder.Scale(scale, scale); + RenderTextInCanvasSkia(GetContext(), builder, + "the quick brown fox jumped over the lazy dog!.?", + "Roboto-Regular.ttf", + TextRenderOptions{.is_subpixel = is_subpixel}); + return builder.Build(); + }; - ASSERT_TRUE(RenderTextInCanvasSkia( - GetContext(), builder, "the quick brown fox jumped over the lazy dog!.?", - "Roboto-Regular.ttf")); - ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); + ASSERT_TRUE(OpenPlaygroundHere(callback)); } TEST_P(AiksTest, TextFrameSubpixelAlignment) { diff --git a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc index ce59d1f000d..361b5d8bc73 100644 --- a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc +++ b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc @@ -1128,9 +1128,10 @@ void FirstPassDispatcher::drawTextFrame( (matrix_ * Matrix::MakeTranslation(Point(x, y))).GetMaxBasisLengthXY()); renderer_.GetLazyGlyphAtlas()->AddTextFrame( - text_frame, // - scale, // - Point(x, y), // + text_frame, // + scale, // + Point(x, y), // + matrix_, (properties.stroke || text_frame->HasColor()) // ? std::optional(properties) // : std::nullopt // diff --git a/engine/src/flutter/impeller/display_list/dl_golden_blur_unittests.cc b/engine/src/flutter/impeller/display_list/dl_golden_blur_unittests.cc index 54784e69d9b..3f5b916328f 100644 --- a/engine/src/flutter/impeller/display_list/dl_golden_blur_unittests.cc +++ b/engine/src/flutter/impeller/display_list/dl_golden_blur_unittests.cc @@ -6,58 +6,18 @@ #include "flutter/display_list/dl_builder.h" #include "flutter/display_list/effects/dl_mask_filter.h" +#include "flutter/impeller/display_list/testing/render_text_in_canvas.h" +#include "flutter/impeller/display_list/testing/rmse.h" #include "flutter/impeller/geometry/round_rect.h" #include "flutter/impeller/golden_tests/screenshot.h" #include "flutter/testing/testing.h" #include "gtest/gtest.h" -#include "impeller/typographer/backends/skia/text_frame_skia.h" -#include "txt/platform.h" namespace flutter { namespace testing { using impeller::Font; -namespace { -struct TextRenderOptions { - bool stroke = false; - SkScalar font_size = 50; - DlColor color = DlColor::kYellow(); - std::shared_ptr mask_filter; -}; - -bool RenderTextInCanvasSkia(DlCanvas* canvas, - const std::string& text, - const std::string_view& font_fixture, - SkPoint position, - const TextRenderOptions& options = {}) { - auto c_font_fixture = std::string(font_fixture); - auto mapping = flutter::testing::OpenFixtureAsSkData(c_font_fixture.c_str()); - if (!mapping) { - return false; - } - sk_sp font_mgr = txt::GetDefaultFontManager(); - SkFont sk_font(font_mgr->makeFromData(mapping), options.font_size); - auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font); - if (!blob) { - return false; - } - - auto frame = impeller::MakeTextFrameFromTextBlobSkia(blob); - - DlPaint text_paint; - text_paint.setColor(options.color); - text_paint.setMaskFilter(options.mask_filter); - // text_paint.mask_blur_descriptor = options.mask_blur_descriptor; - // text_paint.stroke_width = 1; - // text_paint.style = - // options.stroke ? Paint::Style::kStroke : Paint::Style::kFill; - canvas->DrawTextFrame(frame, position.x(), position.y(), text_paint); - return true; -} - -} // namespace - TEST_P(DlGoldenTest, TextBlurMaskFilterRespectCTM) { impeller::Point content_scale = GetContentScale(); auto draw = [&](DlCanvas* canvas, @@ -70,13 +30,13 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterRespectCTM) { DlBlurMaskFilter::Make(DlBlurStyle::kNormal, /*sigma=*/10, /*respect_ctm=*/true); ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", - "Roboto-Regular.ttf", - SkPoint::Make(101, 101), options)); + "Roboto-Regular.ttf", DlPoint(101, 101), + options)); options.mask_filter = nullptr; options.color = DlColor::kRed(); ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", - "Roboto-Regular.ttf", - SkPoint::Make(100, 100), options)); + "Roboto-Regular.ttf", DlPoint(100, 100), + options)); }; DisplayListBuilder builder; @@ -97,13 +57,13 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterDisrespectCTM) { DlBlurMaskFilter::Make(DlBlurStyle::kNormal, /*sigma=*/10, /*respect_ctm=*/false); ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", - "Roboto-Regular.ttf", - SkPoint::Make(101, 101), options)); + "Roboto-Regular.ttf", DlPoint(101, 101), + options)); options.mask_filter = nullptr; options.color = DlColor::kRed(); ASSERT_TRUE(RenderTextInCanvasSkia(canvas, "hello world", - "Roboto-Regular.ttf", - SkPoint::Make(100, 100), options)); + "Roboto-Regular.ttf", DlPoint(100, 100), + options)); }; DisplayListBuilder builder; @@ -112,41 +72,6 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterDisrespectCTM) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } -namespace { -double CalculateDistance(const uint8_t* left, const uint8_t* right) { - double diff[4] = { - static_cast(left[0]) - right[0], // - static_cast(left[1]) - right[1], // - static_cast(left[2]) - right[2], // - static_cast(left[3]) - right[3] // - }; - return sqrt((diff[0] * diff[0]) + // - (diff[1] * diff[1]) + // - (diff[2] * diff[2]) + // - (diff[3] * diff[3])); -} - -double RMSE(const impeller::testing::Screenshot* left, - const impeller::testing::Screenshot* right) { - FML_CHECK(left); - FML_CHECK(right); - FML_CHECK(left->GetWidth() == right->GetWidth()); - FML_CHECK(left->GetHeight() == right->GetHeight()); - - int64_t samples = left->GetWidth() * left->GetHeight(); - double tally = 0; - - const uint8_t* left_ptr = left->GetBytes(); - const uint8_t* right_ptr = right->GetBytes(); - for (int64_t i = 0; i < samples; ++i, left_ptr += 4, right_ptr += 4) { - double distance = CalculateDistance(left_ptr, right_ptr); - tally += distance * distance; - } - - return sqrt(tally / static_cast(samples)); -} -} // namespace - // This is a test to make sure that we don't regress "shimmering" in the // gaussian blur. Shimmering is abrupt changes in signal when making tiny // changes to the blur parameters. diff --git a/engine/src/flutter/impeller/display_list/dl_golden_unittests.cc b/engine/src/flutter/impeller/display_list/dl_golden_unittests.cc index 8dc953d21b7..8758026cfac 100644 --- a/engine/src/flutter/impeller/display_list/dl_golden_unittests.cc +++ b/engine/src/flutter/impeller/display_list/dl_golden_unittests.cc @@ -8,6 +8,8 @@ #include "display_list/dl_paint.h" #include "display_list/geometry/dl_geometry_types.h" #include "flutter/display_list/dl_builder.h" +#include "flutter/impeller/display_list/testing/render_text_in_canvas.h" +#include "flutter/impeller/display_list/testing/rmse.h" #include "flutter/impeller/geometry/path_builder.h" #include "flutter/testing/testing.h" #include "gtest/gtest.h" @@ -335,5 +337,258 @@ TEST_P(DlGoldenTest, SaveLayerAtFractionalValue) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +namespace { +int32_t CalculateMaxY(const impeller::testing::Screenshot* img) { + const uint32_t* ptr = reinterpret_cast(img->GetBytes()); + int32_t max_y = 0; + for (uint32_t i = 0; i < img->GetHeight(); ++i) { + for (uint32_t j = 0; j < img->GetWidth(); ++j) { + uint32_t pixel = *ptr++; + if ((pixel & 0x00ffffff) != 0) { + max_y = std::max(max_y, static_cast(i)); + } + } + } + return max_y; +} + +int32_t CalculateSpaceBetweenUI(const impeller::testing::Screenshot* img) { + const uint32_t* ptr = reinterpret_cast(img->GetBytes()); + ptr += img->GetWidth() * static_cast(img->GetHeight() / 2.0); + std::vector boundaries; + uint32_t value = *ptr++; + for (size_t i = 1; i < img->GetWidth(); ++i) { + if (((*ptr & 0x00ffffff) != 0) != ((value & 0x00ffffff) != 0)) { + boundaries.push_back(i); + } + value = *ptr++; + } + + assert(boundaries.size() == 6); + return boundaries[4] - boundaries[3]; +} +} // namespace + +TEST_P(DlGoldenTest, BaselineHE) { + SetWindowSize(impeller::ISize(1024, 200)); + impeller::Scalar font_size = 300; + auto callback = [&](const char* text, + impeller::Scalar scale) -> sk_sp { + DisplayListBuilder builder; + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0, 0, 0)); + builder.DrawPaint(paint); + builder.Scale(scale, scale); + RenderTextInCanvasSkia(&builder, text, "Roboto-Regular.ttf", + DlPoint::MakeXY(10, 300), + TextRenderOptions{ + .font_size = font_size, + }); + return builder.Build(); + }; + + std::unique_ptr right = + MakeScreenshot(callback("h", 0.444)); + if (!right) { + GTEST_SKIP() << "making screenshots not supported."; + } + std::unique_ptr left = + MakeScreenshot(callback("e", 0.444)); + + int32_t left_max_y = CalculateMaxY(left.get()); + int32_t right_max_y = CalculateMaxY(right.get()); + int32_t y_diff = std::abs(left_max_y - right_max_y); + EXPECT_TRUE(y_diff <= 2) << "y diff: " << y_diff; +} + +TEST_P(DlGoldenTest, MaintainsSpace) { + SetWindowSize(impeller::ISize(1024, 200)); + impeller::Scalar font_size = 300; + auto callback = [&](const char* text, + impeller::Scalar scale) -> sk_sp { + DisplayListBuilder builder; + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0, 0, 0)); + builder.DrawPaint(paint); + builder.Scale(scale, scale); + RenderTextInCanvasSkia(&builder, text, "Roboto-Regular.ttf", + DlPoint::MakeXY(10, 300), + TextRenderOptions{ + .font_size = font_size, + }); + return builder.Build(); + }; + + std::optional last_space; + for (int i = 0; i <= 100; ++i) { + Scalar scale = 0.440 + i / 1000.0; + std::unique_ptr right = + MakeScreenshot(callback("ui", scale)); + if (!right) { + GTEST_SKIP() << "making screenshots not supported."; + } + + int32_t space = CalculateSpaceBetweenUI(right.get()); + if (last_space.has_value()) { + int32_t diff = abs(space - *last_space); + EXPECT_TRUE(diff <= 1) + << "i:" << i << " space:" << space << " last_space:" << *last_space; + } + last_space = space; + } +} + +namespace { +struct LeftmostIntensity { + int32_t x; + int32_t value; +}; + +/// Returns the highest value in the green channel for leftmost column that +/// isn't all black. +LeftmostIntensity CalculateLeftmostIntensity( + const impeller::testing::Screenshot* img) { + LeftmostIntensity result = {.x = static_cast(img->GetWidth()), + .value = 0}; + const uint32_t* ptr = reinterpret_cast(img->GetBytes()); + for (size_t i = 0; i < img->GetHeight(); ++i) { + for (int32_t j = 0; j < static_cast(img->GetWidth()); ++j) { + if (((*ptr & 0x00ffffff) != 0)) { + if (j < result.x) { + result.x = j; + result.value = (*ptr & 0xff00) >> 8; + } else if (j == result.x) { + result.value = + std::max(static_cast(*ptr & 0xff), result.value); + } + } + ptr++; + } + } + return result; +} +} // namespace + +// Checks that the left most edge of the glyph is fading out as we push +// it to the right by fractional pixels. +TEST_P(DlGoldenTest, Subpixel) { + SetWindowSize(impeller::ISize(1024, 200)); + impeller::Scalar font_size = 200; + auto callback = [&](Scalar offset_x) -> sk_sp { + DisplayListBuilder builder; + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0, 0, 0)); + builder.DrawPaint(paint); + RenderTextInCanvasSkia(&builder, "ui", "Roboto-Regular.ttf", + DlPoint::MakeXY(offset_x, 180), + TextRenderOptions{ + .font_size = font_size, + .is_subpixel = true, + }); + return builder.Build(); + }; + + LeftmostIntensity intensity[5]; + for (int i = 0; i <= 4; ++i) { + Scalar offset = 10 + (i / 4.0); + std::unique_ptr right = + MakeScreenshot(callback(offset)); + if (!right) { + GTEST_SKIP() << "making screenshots not supported."; + } + intensity[i] = CalculateLeftmostIntensity(right.get()); + ASSERT_NE(intensity[i].value, 0); + } + for (int i = 1; i < 5; ++i) { + EXPECT_TRUE(intensity[i].x - intensity[i - 1].x == 1 || + intensity[i].value < intensity[i - 1].value) + << i; + } + EXPECT_EQ(intensity[4].x - intensity[0].x, 1); +} + +// Checks that the left most edge of the glyph is fading out as we push +// it to the right by fractional pixels. +TEST_P(DlGoldenTest, SubpixelScaled) { + SetWindowSize(impeller::ISize(1024, 200)); + impeller::Scalar font_size = 200; + Scalar scalar = 0.75; + auto callback = [&](Scalar offset_x) -> sk_sp { + DisplayListBuilder builder; + builder.Scale(scalar, scalar); + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0, 0, 0)); + builder.DrawPaint(paint); + RenderTextInCanvasSkia(&builder, "ui", "Roboto-Regular.ttf", + DlPoint::MakeXY(offset_x, 180), + TextRenderOptions{ + .font_size = font_size, + .is_subpixel = true, + }); + return builder.Build(); + }; + + LeftmostIntensity intensity[5]; + Scalar offset_fraction = 0.25 / scalar; + for (int i = 0; i <= 4; ++i) { + Scalar offset = 10 + (offset_fraction * i); + std::unique_ptr right = + MakeScreenshot(callback(offset)); + if (!right) { + GTEST_SKIP() << "making screenshots not supported."; + } + intensity[i] = CalculateLeftmostIntensity(right.get()); + ASSERT_NE(intensity[i].value, 0); + } + for (int i = 1; i < 5; ++i) { + EXPECT_TRUE(intensity[i].x - intensity[i - 1].x == 1 || + intensity[i].value < intensity[i - 1].value) + << i; + } + EXPECT_EQ(intensity[4].x - intensity[0].x, 1); +} + +// Checks that the left most edge of the glyph is fading out as we push +// it to the right by fractional pixels. +TEST_P(DlGoldenTest, SubpixelScaledTranslated) { + SetWindowSize(impeller::ISize(1024, 200)); + impeller::Scalar font_size = 200; + Scalar scalar = 0.75; + auto callback = [&](Scalar offset_x) -> sk_sp { + DisplayListBuilder builder; + builder.Scale(scalar, scalar); + DlPaint paint; + paint.setColor(DlColor::ARGB(1, 0, 0, 0)); + builder.DrawPaint(paint); + builder.Translate(offset_x, 180); + RenderTextInCanvasSkia(&builder, "ui", "Roboto-Regular.ttf", + DlPoint::MakeXY(0, 0), + TextRenderOptions{ + .font_size = font_size, + .is_subpixel = true, + }); + return builder.Build(); + }; + + LeftmostIntensity intensity[5]; + Scalar offset_fraction = 0.25 / scalar; + for (int i = 0; i <= 4; ++i) { + Scalar offset = 10 + (offset_fraction * i); + std::unique_ptr right = + MakeScreenshot(callback(offset)); + if (!right) { + GTEST_SKIP() << "making screenshots not supported."; + } + intensity[i] = CalculateLeftmostIntensity(right.get()); + ASSERT_NE(intensity[i].value, 0); + } + for (int i = 1; i < 5; ++i) { + EXPECT_TRUE(intensity[i].x - intensity[i - 1].x == 1 || + intensity[i].value < intensity[i - 1].value) + << i; + } + EXPECT_EQ(intensity[4].x - intensity[0].x, 1); +} + } // namespace testing } // namespace flutter diff --git a/engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.cc b/engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.cc new file mode 100644 index 00000000000..d6b38635500 --- /dev/null +++ b/engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.cc @@ -0,0 +1,46 @@ +// 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 "impeller/display_list/testing/render_text_in_canvas.h" + +#include "flutter/testing/testing.h" +#include "txt/platform.h" + +namespace flutter { +namespace testing { + +bool RenderTextInCanvasSkia(DlCanvas* canvas, + const std::string& text, + const std::string_view& font_fixture, + DlPoint position, + const TextRenderOptions& options) { + auto c_font_fixture = std::string(font_fixture); + auto mapping = flutter::testing::OpenFixtureAsSkData(c_font_fixture.c_str()); + if (!mapping) { + return false; + } + sk_sp font_mgr = txt::GetDefaultFontManager(); + SkFont sk_font(font_mgr->makeFromData(mapping), options.font_size); + if (options.is_subpixel) { + sk_font.setSubpixel(true); + } + auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font); + if (!blob) { + return false; + } + + auto frame = impeller::MakeTextFrameFromTextBlobSkia(blob); + + DlPaint text_paint; + text_paint.setColor(options.color); + text_paint.setMaskFilter(options.mask_filter); + // text_paint.mask_blur_descriptor = options.mask_blur_descriptor; + // text_paint.stroke_width = 1; + // text_paint.style = + // options.stroke ? Paint::Style::kStroke : Paint::Style::kFill; + canvas->DrawTextFrame(frame, position.x, position.y, text_paint); + return true; +} +} // namespace testing +} // namespace flutter diff --git a/engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.h b/engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.h new file mode 100644 index 00000000000..e7493564b7b --- /dev/null +++ b/engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.h @@ -0,0 +1,35 @@ +// 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. + +#ifndef FLUTTER_IMPELLER_DISPLAY_LIST_TESTING_RENDER_TEXT_IN_CANVAS_H_ +#define FLUTTER_IMPELLER_DISPLAY_LIST_TESTING_RENDER_TEXT_IN_CANVAS_H_ + +#include + +#include "flutter/display_list/dl_canvas.h" +#include "flutter/display_list/dl_color.h" +#include "flutter/display_list/effects/dl_mask_filter.h" +#include "impeller/typographer/backends/skia/text_frame_skia.h" + +namespace flutter { +namespace testing { + +struct TextRenderOptions { + bool stroke = false; + SkScalar font_size = 50; + DlColor color = DlColor::kYellow(); + std::shared_ptr mask_filter; + bool is_subpixel = false; +}; + +bool RenderTextInCanvasSkia(DlCanvas* canvas, + const std::string& text, + const std::string_view& font_fixture, + DlPoint position, + const TextRenderOptions& options = {}); + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_IMPELLER_DISPLAY_LIST_TESTING_RENDER_TEXT_IN_CANVAS_H_ diff --git a/engine/src/flutter/impeller/display_list/testing/rmse.cc b/engine/src/flutter/impeller/display_list/testing/rmse.cc new file mode 100644 index 00000000000..45e19c80379 --- /dev/null +++ b/engine/src/flutter/impeller/display_list/testing/rmse.cc @@ -0,0 +1,47 @@ +// 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 "impeller/display_list/testing/rmse.h" + +#include "flutter/fml/logging.h" + +namespace flutter { +namespace testing { +namespace { +double CalculateDistance(const uint8_t* left, const uint8_t* right) { + double diff[4] = { + static_cast(left[0]) - right[0], // + static_cast(left[1]) - right[1], // + static_cast(left[2]) - right[2], // + static_cast(left[3]) - right[3] // + }; + return sqrt((diff[0] * diff[0]) + // + (diff[1] * diff[1]) + // + (diff[2] * diff[2]) + // + (diff[3] * diff[3])); +} +} // namespace + +double RMSE(const impeller::testing::Screenshot* left, + const impeller::testing::Screenshot* right) { + FML_CHECK(left); + FML_CHECK(right); + FML_CHECK(left->GetWidth() == right->GetWidth()); + FML_CHECK(left->GetHeight() == right->GetHeight()); + + int64_t samples = left->GetWidth() * left->GetHeight(); + double tally = 0; + + const uint8_t* left_ptr = left->GetBytes(); + const uint8_t* right_ptr = right->GetBytes(); + for (int64_t i = 0; i < samples; ++i, left_ptr += 4, right_ptr += 4) { + double distance = CalculateDistance(left_ptr, right_ptr); + tally += distance * distance; + } + + return sqrt(tally / static_cast(samples)); +} + +} // namespace testing +} // namespace flutter diff --git a/engine/src/flutter/impeller/display_list/testing/rmse.h b/engine/src/flutter/impeller/display_list/testing/rmse.h new file mode 100644 index 00000000000..aadb8302d85 --- /dev/null +++ b/engine/src/flutter/impeller/display_list/testing/rmse.h @@ -0,0 +1,16 @@ +// 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. + +#ifndef FLUTTER_IMPELLER_DISPLAY_LIST_TESTING_RMSE_H_ +#define FLUTTER_IMPELLER_DISPLAY_LIST_TESTING_RMSE_H_ + +#include "flutter/impeller/golden_tests/screenshot.h" + +namespace flutter { +namespace testing { +double RMSE(const impeller::testing::Screenshot* left, + const impeller::testing::Screenshot* right); +} +} // namespace flutter +#endif // FLUTTER_IMPELLER_DISPLAY_LIST_TESTING_RMSE_H_ diff --git a/engine/src/flutter/impeller/entity/BUILD.gn b/engine/src/flutter/impeller/entity/BUILD.gn index 735081d4f8d..0f256bb6788 100644 --- a/engine/src/flutter/impeller/entity/BUILD.gn +++ b/engine/src/flutter/impeller/entity/BUILD.gn @@ -248,6 +248,7 @@ impeller_component("entity_unittests") { "contents/filters/inputs/filter_input_unittests.cc", "contents/filters/matrix_filter_contents_unittests.cc", "contents/host_buffer_unittests.cc", + "contents/text_contents_unittests.cc", "contents/tiled_texture_contents_unittests.cc", "draw_order_resolver_unittests.cc", "entity_pass_target_unittests.cc", @@ -266,5 +267,6 @@ impeller_component("entity_unittests") { "../playground:playground_test", "//flutter/display_list/testing:display_list_testing", "//flutter/impeller/typographer/backends/skia:typographer_skia_backend", + "//flutter/third_party/txt", ] } diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.cc b/engine/src/flutter/impeller/entity/contents/text_contents.cc index 4d4a1304801..7c8be8c6843 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/text_contents.cc @@ -11,7 +11,6 @@ #include "impeller/core/buffer_view.h" #include "impeller/core/formats.h" #include "impeller/core/sampler_descriptor.h" -#include "impeller/entity/contents/content_context.h" #include "impeller/entity/entity.h" #include "impeller/geometry/color.h" #include "impeller/geometry/point.h" @@ -19,6 +18,12 @@ #include "impeller/typographer/glyph_atlas.h" namespace impeller { +Point SizeToPoint(Size size) { + return Point(size.width, size.height); +} + +using VS = GlyphAtlasPipeline::VertexShader; +using FS = GlyphAtlasPipeline::FragmentShader; TextContents::TextContents() = default; @@ -72,6 +77,131 @@ void TextContents::SetTextProperties(Color color, } } +void TextContents::ComputeVertexData( + VS::PerVertexData* vtx_contents, + const std::shared_ptr& frame, + Scalar scale, + const Matrix& entity_transform, + Vector2 offset, + std::optional glyph_properties, + const std::shared_ptr& atlas) { + // Common vertex information for all glyphs. + // All glyphs are given the same vertex information in the form of a + // unit-sized quad. The size of the glyph is specified in per instance data + // and the vertex shader uses this to size the glyph correctly. The + // interpolated vertex information is also used in the fragment shader to + // sample from the glyph atlas. + + constexpr std::array unit_points = {Point{0, 0}, Point{1, 0}, + Point{0, 1}, Point{1, 0}, + Point{0, 1}, Point{1, 1}}; + + ISize atlas_size = atlas->GetTexture()->GetSize(); + bool is_translation_scale = entity_transform.IsTranslationScaleOnly(); + Matrix basis_transform = entity_transform.Basis(); + + VS::PerVertexData vtx; + size_t i = 0u; + size_t bounds_offset = 0u; + for (const TextRun& run : frame->GetRuns()) { + const Font& font = run.GetFont(); + Scalar rounded_scale = TextFrame::RoundScaledFontSize(scale); + FontGlyphAtlas* font_atlas = nullptr; + + // Adjust glyph position based on the subpixel rounding + // used by the font. + Point subpixel_adjustment(0.5, 0.5); + switch (font.GetAxisAlignment()) { + case AxisAlignment::kNone: + break; + case AxisAlignment::kX: + subpixel_adjustment.x = 0.125; + break; + case AxisAlignment::kY: + subpixel_adjustment.y = 0.125; + break; + case AxisAlignment::kAll: + subpixel_adjustment.x = 0.125; + subpixel_adjustment.y = 0.125; + break; + } + + Point screen_offset = (entity_transform * Point(0, 0)); + for (const TextRun::GlyphPosition& glyph_position : + run.GetGlyphPositions()) { + const FrameBounds& frame_bounds = frame->GetFrameBounds(bounds_offset); + bounds_offset++; + auto atlas_glyph_bounds = frame_bounds.atlas_bounds; + auto glyph_bounds = frame_bounds.glyph_bounds; + + // If frame_bounds.is_placeholder is true, this is the first frame + // the glyph has been rendered and so its atlas position was not + // known when the glyph was recorded. Perform a slow lookup into the + // glyph atlas hash table. + if (frame_bounds.is_placeholder) { + if (!font_atlas) { + font_atlas = + atlas->GetOrCreateFontGlyphAtlas(ScaledFont{font, rounded_scale}); + } + + if (!font_atlas) { + VALIDATION_LOG << "Could not find font in the atlas."; + continue; + } + Point subpixel = TextFrame::ComputeSubpixelPosition( + glyph_position, font.GetAxisAlignment(), entity_transform); + + std::optional maybe_atlas_glyph_bounds = + font_atlas->FindGlyphBounds(SubpixelGlyph{ + glyph_position.glyph, // + subpixel, // + glyph_properties // + }); + if (!maybe_atlas_glyph_bounds.has_value()) { + VALIDATION_LOG << "Could not find glyph position in the atlas."; + continue; + } + atlas_glyph_bounds = maybe_atlas_glyph_bounds.value().atlas_bounds; + } + + Rect scaled_bounds = glyph_bounds.Scale(1.0 / rounded_scale); + // For each glyph, we compute two rectangles. One for the vertex + // positions and one for the texture coordinates (UVs). The atlas + // glyph bounds are used to compute UVs in cases where the + // destination and source sizes may differ due to clamping the sizes + // of large glyphs. + Point uv_origin = (atlas_glyph_bounds.GetLeftTop()) / atlas_size; + Point uv_size = SizeToPoint(atlas_glyph_bounds.GetSize()) / atlas_size; + + Point unrounded_glyph_position = + // This is for RTL text. + (basis_transform.m[0] < 0 ? Matrix::MakeScale({-1, 1, 1}) + : Matrix()) * + glyph_bounds.GetLeftTop() + + (basis_transform * glyph_position.position); + + Point screen_glyph_position = + (screen_offset + unrounded_glyph_position + subpixel_adjustment) + .Floor(); + for (const Point& point : unit_points) { + Point position; + if (is_translation_scale) { + position = (screen_glyph_position + + (basis_transform * point * scaled_bounds.GetSize())) + .Round(); + } else { + position = entity_transform * + (glyph_position.position + scaled_bounds.GetLeftTop() + + point * scaled_bounds.GetSize()); + } + vtx.uv = uv_origin + (uv_size * point); + vtx.position = position; + vtx_contents[i++] = vtx; + } + } + } +} + bool TextContents::Render(const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { @@ -100,17 +230,12 @@ bool TextContents::Render(const ContentContext& renderer, opts.primitive_type = PrimitiveType::kTriangle; pass.SetPipeline(renderer.GetGlyphAtlasPipeline(opts)); - using VS = GlyphAtlasPipeline::VertexShader; - using FS = GlyphAtlasPipeline::FragmentShader; - // Common vertex uniforms for all glyphs. VS::FrameInfo frame_info; frame_info.mvp = Entity::GetShaderTransform(entity.GetShaderClipDepth(), pass, Matrix()); - ISize atlas_size = atlas->GetTexture()->GetSize(); bool is_translation_scale = entity.GetTransform().IsTranslationScaleOnly(); Matrix entity_transform = entity.GetTransform(); - Matrix basis_transform = entity_transform.Basis(); VS::BindFrameInfo(pass, renderer.GetTransientsBuffer().EmplaceUniform(frame_info)); @@ -147,17 +272,6 @@ bool TextContents::Render(const ContentContext& renderer, sampler_desc) // sampler ); - // Common vertex information for all glyphs. - // All glyphs are given the same vertex information in the form of a - // unit-sized quad. The size of the glyph is specified in per instance data - // and the vertex shader uses this to size the glyph correctly. The - // interpolated vertex information is also used in the fragment shader to - // sample from the glyph atlas. - - constexpr std::array unit_points = {Point{0, 0}, Point{1, 0}, - Point{0, 1}, Point{1, 0}, - Point{0, 1}, Point{1, 1}}; - auto& host_buffer = renderer.GetTransientsBuffer(); size_t vertex_count = 0; for (const auto& run : frame_->GetRuns()) { @@ -168,112 +282,11 @@ bool TextContents::Render(const ContentContext& renderer, BufferView buffer_view = host_buffer.Emplace( vertex_count * sizeof(VS::PerVertexData), alignof(VS::PerVertexData), [&](uint8_t* contents) { - VS::PerVertexData vtx; VS::PerVertexData* vtx_contents = reinterpret_cast(contents); - size_t i = 0u; - size_t bounds_offset = 0u; - for (const TextRun& run : frame_->GetRuns()) { - const Font& font = run.GetFont(); - Scalar rounded_scale = TextFrame::RoundScaledFontSize(scale_); - FontGlyphAtlas* font_atlas = nullptr; - - // Adjust glyph position based on the subpixel rounding - // used by the font. - Point subpixel_adjustment(0.5, 0.5); - switch (font.GetAxisAlignment()) { - case AxisAlignment::kNone: - break; - case AxisAlignment::kX: - subpixel_adjustment.x = 0.125; - break; - case AxisAlignment::kY: - subpixel_adjustment.y = 0.125; - break; - case AxisAlignment::kAll: - subpixel_adjustment.x = 0.125; - subpixel_adjustment.y = 0.125; - break; - } - - Point screen_offset = (entity_transform * Point(0, 0)); - for (const TextRun::GlyphPosition& glyph_position : - run.GetGlyphPositions()) { - const FrameBounds& frame_bounds = - frame_->GetFrameBounds(bounds_offset); - bounds_offset++; - auto atlas_glyph_bounds = frame_bounds.atlas_bounds; - auto glyph_bounds = frame_bounds.glyph_bounds; - - // If frame_bounds.is_placeholder is true, this is the first frame - // the glyph has been rendered and so its atlas position was not - // known when the glyph was recorded. Perform a slow lookup into the - // glyph atlas hash table. - if (frame_bounds.is_placeholder) { - if (!font_atlas) { - font_atlas = atlas->GetOrCreateFontGlyphAtlas( - ScaledFont{font, rounded_scale}); - } - - if (!font_atlas) { - VALIDATION_LOG << "Could not find font in the atlas."; - continue; - } - Point subpixel = TextFrame::ComputeSubpixelPosition( - glyph_position, font.GetAxisAlignment(), offset_, - rounded_scale); - - std::optional maybe_atlas_glyph_bounds = - font_atlas->FindGlyphBounds(SubpixelGlyph{ - glyph_position.glyph, // - subpixel, // - GetGlyphProperties() // - }); - if (!maybe_atlas_glyph_bounds.has_value()) { - VALIDATION_LOG << "Could not find glyph position in the atlas."; - continue; - } - atlas_glyph_bounds = - maybe_atlas_glyph_bounds.value().atlas_bounds; - } - - Rect scaled_bounds = glyph_bounds.Scale(1.0 / rounded_scale); - // For each glyph, we compute two rectangles. One for the vertex - // positions and one for the texture coordinates (UVs). The atlas - // glyph bounds are used to compute UVs in cases where the - // destination and source sizes may differ due to clamping the sizes - // of large glyphs. - Point uv_origin = - (atlas_glyph_bounds.GetLeftTop() - Point(0.5, 0.5)) / - atlas_size; - Point uv_size = - (atlas_glyph_bounds.GetSize() + Point(1, 1)) / atlas_size; - - Point unrounded_glyph_position = - basis_transform * - (glyph_position.position + scaled_bounds.GetLeftTop()); - - Point screen_glyph_position = - (screen_offset + unrounded_glyph_position + subpixel_adjustment) - .Floor(); - - for (const Point& point : unit_points) { - Point position; - if (is_translation_scale) { - position = (screen_glyph_position + - (basis_transform * point * scaled_bounds.GetSize())) - .Round(); - } else { - position = entity_transform * (glyph_position.position + - scaled_bounds.GetLeftTop() + - point * scaled_bounds.GetSize()); - } - vtx.uv = uv_origin + (uv_size * point); - vtx.position = position; - vtx_contents[i++] = vtx; - } - } - } + ComputeVertexData(vtx_contents, frame_, scale_, + /*entity_transform=*/entity_transform, offset_, + GetGlyphProperties(), atlas); }); pass.SetVertexBuffer(std::move(buffer_view)); diff --git a/engine/src/flutter/impeller/entity/contents/text_contents.h b/engine/src/flutter/impeller/entity/contents/text_contents.h index 091da3d7488..6e5861f3713 100644 --- a/engine/src/flutter/impeller/entity/contents/text_contents.h +++ b/engine/src/flutter/impeller/entity/contents/text_contents.h @@ -7,6 +7,7 @@ #include +#include "impeller/entity/contents/content_context.h" #include "impeller/entity/contents/contents.h" #include "impeller/geometry/color.h" #include "impeller/typographer/font_glyph_pair.h" @@ -61,6 +62,15 @@ class TextContents final : public Contents { const Entity& entity, RenderPass& pass) const override; + static void ComputeVertexData( + GlyphAtlasPipeline::VertexShader::PerVertexData* vtx_contents, + const std::shared_ptr& frame, + Scalar scale, + const Matrix& entity_transform, + Vector2 offset, + std::optional glyph_properties, + const std::shared_ptr& atlas); + private: std::optional GetGlyphProperties() const; diff --git a/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc b/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc new file mode 100644 index 00000000000..217a20bdc0d --- /dev/null +++ b/engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc @@ -0,0 +1,206 @@ +// 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 "flutter/impeller/geometry/geometry_asserts.h" +#include "flutter/impeller/renderer/testing/mocks.h" +#include "flutter/testing/testing.h" +#include "impeller/entity/contents/text_contents.h" +#include "impeller/playground/playground_test.h" +#include "impeller/typographer/backends/skia/text_frame_skia.h" +#include "impeller/typographer/backends/skia/typographer_context_skia.h" +#include "third_party/googletest/googletest/include/gtest/gtest.h" +#include "txt/platform.h" + +#pragma GCC diagnostic ignored "-Wunreachable-code" + +namespace impeller { +namespace testing { + +using TextContentsTest = PlaygroundTest; +INSTANTIATE_PLAYGROUND_SUITE(TextContentsTest); + +using ::testing::Return; + +namespace { +std::shared_ptr MakeTextFrame(const std::string& text, + const std::string_view& font_fixture, + Scalar font_size) { + auto c_font_fixture = std::string(font_fixture); + auto mapping = flutter::testing::OpenFixtureAsSkData(c_font_fixture.c_str()); + if (!mapping) { + return nullptr; + } + sk_sp font_mgr = txt::GetDefaultFontManager(); + SkFont sk_font(font_mgr->makeFromData(mapping), font_size); + auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font); + if (!blob) { + return nullptr; + } + + return MakeTextFrameFromTextBlobSkia(blob); +} + +std::shared_ptr CreateGlyphAtlas( + Context& context, + const TypographerContext* typographer_context, + HostBuffer& host_buffer, + GlyphAtlas::Type type, + Scalar scale, + const std::shared_ptr& atlas_context, + const std::shared_ptr& frame) { + frame->SetPerFrameData(scale, /*offset=*/Point(0, 0), + /*transform=*/Matrix(), /*properties=*/std::nullopt); + return typographer_context->CreateGlyphAtlas(context, type, host_buffer, + atlas_context, {frame}); +} + +Rect PerVertexDataPositionToRect( + GlyphAtlasPipeline::VertexShader::PerVertexData data[6]) { + Scalar right = FLT_MIN; + Scalar left = FLT_MAX; + Scalar top = FLT_MAX; + Scalar bottom = FLT_MIN; + for (int i = 0; i < 6; ++i) { + right = std::max(right, data[i].position.x); + left = std::min(left, data[i].position.x); + top = std::min(top, data[i].position.y); + bottom = std::max(bottom, data[i].position.y); + } + + return Rect::MakeLTRB(left, top, right, bottom); +} + +Rect PerVertexDataUVToRect( + GlyphAtlasPipeline::VertexShader::PerVertexData data[6], + ISize texture_size) { + Scalar right = FLT_MIN; + Scalar left = FLT_MAX; + Scalar top = FLT_MAX; + Scalar bottom = FLT_MIN; + for (int i = 0; i < 6; ++i) { + right = std::max(right, data[i].uv.x * texture_size.width); + left = std::min(left, data[i].uv.x * texture_size.width); + top = std::min(top, data[i].uv.y * texture_size.height); + bottom = std::max(bottom, data[i].uv.y * texture_size.height); + } + + return Rect::MakeLTRB(left, top, right, bottom); +} + +double GetAspectRatio(Rect rect) { + return static_cast(rect.GetWidth()) / rect.GetHeight(); +} +} // namespace + +TEST_P(TextContentsTest, SimpleComputeVertexData) { +#ifndef FML_OS_MACOSX + GTEST_SKIP() << "Results aren't stable across linux and macos."; +#endif + + GlyphAtlasPipeline::VertexShader::PerVertexData data[6]; + + std::shared_ptr text_frame = + MakeTextFrame("1", "ahem.ttf", /*font_size=*/50); + + std::shared_ptr context = TypographerContextSkia::Make(); + std::shared_ptr atlas_context = + context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap); + std::shared_ptr host_buffer = HostBuffer::Create( + GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter()); + ASSERT_TRUE(context && context->IsValid()); + std::shared_ptr atlas = + CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, /*scale=*/1.0f, + atlas_context, text_frame); + + ISize texture_size = atlas->GetTexture()->GetSize(); + TextContents::ComputeVertexData(data, text_frame, /*scale=*/1.0, + /*entity_transform=*/Matrix(), + /*offset=*/Vector2(0, 0), + /*glyph_properties=*/std::nullopt, atlas); + + Rect position_rect = PerVertexDataPositionToRect(data); + Rect uv_rect = PerVertexDataUVToRect(data, texture_size); + // The -1 offset comes from Skia in `ComputeGlyphSize`. So since the font size + // is 50, the math appears to be to get back a 50x50 rect and apply 1 pixel + // of padding. + EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -41, 52, 52)); + EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 52, 52)); +} + +TEST_P(TextContentsTest, SimpleComputeVertexData2x) { +#ifndef FML_OS_MACOSX + GTEST_SKIP() << "Results aren't stable across linux and macos."; +#endif + + GlyphAtlasPipeline::VertexShader::PerVertexData data[6]; + + std::shared_ptr text_frame = + MakeTextFrame("1", "ahem.ttf", /*font_size=*/50); + + std::shared_ptr context = TypographerContextSkia::Make(); + std::shared_ptr atlas_context = + context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap); + std::shared_ptr host_buffer = HostBuffer::Create( + GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter()); + ASSERT_TRUE(context && context->IsValid()); + Scalar font_scale = 2.f; + std::shared_ptr atlas = CreateGlyphAtlas( + *GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, font_scale, atlas_context, text_frame); + + ISize texture_size = atlas->GetTexture()->GetSize(); + TextContents::ComputeVertexData( + data, text_frame, font_scale, + /*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}), + /*offset=*/Vector2(0, 0), + /*glyph_properties=*/std::nullopt, atlas); + + Rect position_rect = PerVertexDataPositionToRect(data); + Rect uv_rect = PerVertexDataUVToRect(data, texture_size); + EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -81, 102, 102)); + EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 102, 102)); +} + +TEST_P(TextContentsTest, MaintainsShape) { + std::shared_ptr text_frame = + MakeTextFrame("th", "ahem.ttf", /*font_size=*/50); + + std::shared_ptr context = TypographerContextSkia::Make(); + std::shared_ptr atlas_context = + context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap); + std::shared_ptr host_buffer = HostBuffer::Create( + GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter()); + ASSERT_TRUE(context && context->IsValid()); + for (int i = 0; i <= 1000; ++i) { + Scalar font_scale = 0.440 + (i / 1000.0); + Rect position_rect[2]; + Rect uv_rect[2]; + + { + GlyphAtlasPipeline::VertexShader::PerVertexData data[12]; + std::shared_ptr atlas = + CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer, + GlyphAtlas::Type::kAlphaBitmap, font_scale, + atlas_context, text_frame); + ISize texture_size = atlas->GetTexture()->GetSize(); + + TextContents::ComputeVertexData( + data, text_frame, font_scale, + /*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}), + /*offset=*/Vector2(0, 0), + /*glyph_properties=*/std::nullopt, atlas); + position_rect[0] = PerVertexDataPositionToRect(data); + uv_rect[0] = PerVertexDataUVToRect(data, texture_size); + position_rect[1] = PerVertexDataPositionToRect(data + 6); + uv_rect[1] = PerVertexDataUVToRect(data + 6, texture_size); + } + EXPECT_NEAR(GetAspectRatio(position_rect[1]), GetAspectRatio(uv_rect[1]), + 0.001) + << i; + } +} + +} // namespace testing +} // namespace impeller diff --git a/engine/src/flutter/impeller/fixtures/BUILD.gn b/engine/src/flutter/impeller/fixtures/BUILD.gn index 0e97078395d..f3a81d1cc64 100644 --- a/engine/src/flutter/impeller/fixtures/BUILD.gn +++ b/engine/src/flutter/impeller/fixtures/BUILD.gn @@ -102,6 +102,7 @@ impellerc("runtime_stages") { test_fixtures("file_fixtures") { fixtures = [ + "//flutter/third_party/txt/third_party/fonts/ahem.ttf", "//flutter/third_party/txt/third_party/fonts/HomemadeApple.ttf", "//flutter/third_party/txt/third_party/fonts/NotoColorEmoji.ttf", "//flutter/third_party/txt/third_party/fonts/Roboto-Medium.ttf", diff --git a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc index d4082760bf9..8f005d00fc7 100644 --- a/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc +++ b/engine/src/flutter/impeller/typographer/backends/skia/typographer_context_skia.cc @@ -452,7 +452,8 @@ TypographerContextSkia::CollectNewGlyphs( for (const auto& glyph_position : run.GetGlyphPositions()) { Point subpixel = TextFrame::ComputeSubpixelPosition( glyph_position, scaled_font.font.GetAxisAlignment(), - frame->GetOffset(), frame->GetScale()); + frame->GetTransform() * + Matrix::MakeTranslation(frame->GetOffset())); SubpixelGlyph subpixel_glyph(glyph_position.glyph, subpixel, frame->GetProperties()); const auto& font_glyph_bounds = diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc index 106ca538bf2..4a484109f47 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc +++ b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.cc @@ -34,8 +34,9 @@ LazyGlyphAtlas::~LazyGlyphAtlas() = default; void LazyGlyphAtlas::AddTextFrame(const std::shared_ptr& frame, Scalar scale, Point offset, + const Matrix& transform, std::optional properties) { - frame->SetPerFrameData(scale, offset, properties); + frame->SetPerFrameData(scale, offset, transform, properties); FML_DCHECK(alpha_atlas_ == nullptr && color_atlas_ == nullptr); if (frame->GetAtlasType() == GlyphAtlas::Type::kAlphaBitmap) { alpha_text_frames_.push_back(frame); diff --git a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h index 47c6d466df1..17b8767e270 100644 --- a/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h +++ b/engine/src/flutter/impeller/typographer/lazy_glyph_atlas.h @@ -22,6 +22,7 @@ class LazyGlyphAtlas { void AddTextFrame(const std::shared_ptr& frame, Scalar scale, Point offset, + const Matrix& transform, std::optional properties); void ResetTextFrames(); diff --git a/engine/src/flutter/impeller/typographer/text_frame.cc b/engine/src/flutter/impeller/typographer/text_frame.cc index 870ff17821e..870231a6043 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.cc +++ b/engine/src/flutter/impeller/typographer/text_frame.cc @@ -56,7 +56,7 @@ Scalar TextFrame::RoundScaledFontSize(Scalar scale) { // CTM, a glyph will fit in the atlas. If we clamp significantly, this may // reduce fidelity but is preferable to the alternative of failing to render. constexpr Scalar kMaximumTextScale = 48; - Scalar result = std::round(scale * 100) / 100; + Scalar result = std::round(scale * 200) / 200; return std::clamp(result, 0.0f, kMaximumTextScale); } @@ -82,26 +82,26 @@ static constexpr Scalar ComputeFractionalPosition(Scalar value) { Point TextFrame::ComputeSubpixelPosition( const TextRun::GlyphPosition& glyph_position, AxisAlignment alignment, - Point offset, - Scalar scale) { - Point pos = glyph_position.position + offset; + const Matrix& transform) { + Point pos = transform * glyph_position.position; switch (alignment) { case AxisAlignment::kNone: return Point(0, 0); case AxisAlignment::kX: - return Point(ComputeFractionalPosition(pos.x * scale), 0); + return Point(ComputeFractionalPosition(pos.x), 0); case AxisAlignment::kY: - return Point(0, ComputeFractionalPosition(pos.y * scale)); + return Point(0, ComputeFractionalPosition(pos.y)); case AxisAlignment::kAll: - return Point(ComputeFractionalPosition(pos.x * scale), - ComputeFractionalPosition(pos.y * scale)); + return Point(ComputeFractionalPosition(pos.x), + ComputeFractionalPosition(pos.y)); } } void TextFrame::SetPerFrameData(Scalar scale, Point offset, + const Matrix& transform, std::optional properties) { - if (!ScalarNearlyEqual(scale_, scale) || + if (!transform_.Equals(transform) || !ScalarNearlyEqual(scale_, scale) || !ScalarNearlyEqual(offset_.x, offset.x) || !ScalarNearlyEqual(offset_.y, offset.y) || !TextPropertiesEquals(properties_, properties)) { @@ -110,6 +110,7 @@ void TextFrame::SetPerFrameData(Scalar scale, scale_ = scale; offset_ = offset; properties_ = properties; + transform_ = transform; } Scalar TextFrame::GetScale() const { @@ -141,6 +142,7 @@ bool TextFrame::IsFrameComplete() const { } const FrameBounds& TextFrame::GetFrameBounds(size_t index) const { + FML_DCHECK(index < bound_values_.size()); return bound_values_[index]; } diff --git a/engine/src/flutter/impeller/typographer/text_frame.h b/engine/src/flutter/impeller/typographer/text_frame.h index 10a942cbeeb..86b03d6480f 100644 --- a/engine/src/flutter/impeller/typographer/text_frame.h +++ b/engine/src/flutter/impeller/typographer/text_frame.h @@ -30,8 +30,7 @@ class TextFrame { static Point ComputeSubpixelPosition( const TextRun::GlyphPosition& glyph_position, AxisAlignment alignment, - Point offset, - Scalar scale); + const Matrix& transform); static Scalar RoundScaledFontSize(Scalar scale); @@ -83,6 +82,7 @@ class TextFrame { /// glyph atlas. void SetPerFrameData(Scalar scale, Point offset, + const Matrix& transform, std::optional properties); // A generation id for the glyph atlas this text run was associated @@ -95,6 +95,8 @@ class TextFrame { TextFrame(const TextFrame& other) = default; + const Matrix& GetTransform() const { return transform_; } + private: friend class TypographerContextSkia; friend class LazyGlyphAtlas; @@ -123,6 +125,7 @@ class TextFrame { intptr_t atlas_id_ = 0; Point offset_; std::optional properties_; + Matrix transform_; }; } // namespace impeller diff --git a/engine/src/flutter/impeller/typographer/typographer_unittests.cc b/engine/src/flutter/impeller/typographer/typographer_unittests.cc index e677cd7c3c3..d12f55e01e9 100644 --- a/engine/src/flutter/impeller/typographer/typographer_unittests.cc +++ b/engine/src/flutter/impeller/typographer/typographer_unittests.cc @@ -37,7 +37,7 @@ static std::shared_ptr CreateGlyphAtlas( Scalar scale, const std::shared_ptr& atlas_context, const std::shared_ptr& frame) { - frame->SetPerFrameData(scale, {0, 0}, std::nullopt); + frame->SetPerFrameData(scale, {0, 0}, Matrix(), std::nullopt); return typographer_context->CreateGlyphAtlas(context, type, host_buffer, atlas_context, {frame}); } @@ -53,7 +53,7 @@ static std::shared_ptr CreateGlyphAtlas( const std::vector>& properties) { size_t offset = 0; for (auto& frame : frames) { - frame->SetPerFrameData(scale, {0, 0}, properties[offset++]); + frame->SetPerFrameData(scale, {0, 0}, Matrix(), properties[offset++]); } return typographer_context->CreateGlyphAtlas(context, type, host_buffer, atlas_context, frames); @@ -137,14 +137,14 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) { LazyGlyphAtlas lazy_atlas(TypographerContextSkia::Make()); - lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, {}); + lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, Matrix(), {}); frame = MakeTextFrameFromTextBlobSkia( SkTextBlob::MakeFromString("😀 ", emoji_font)); ASSERT_TRUE(frame->GetAtlasType() == GlyphAtlas::Type::kColorBitmap); - lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, {}); + lazy_atlas.AddTextFrame(frame, 1.0f, {0, 0}, Matrix(), {}); // Creates different atlases for color and red bitmap. auto color_atlas = lazy_atlas.CreateOrGetGlyphAtlas( @@ -227,7 +227,7 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { std::vector> frames; for (size_t index = 0; index < size_count; index += 1) { frames.push_back(MakeTextFrameFromTextBlobSkia(blob)); - frames.back()->SetPerFrameData(0.6 * index, {0, 0}, {}); + frames.back()->SetPerFrameData(0.6 * index, {0, 0}, Matrix(), {}); }; auto atlas = context->CreateGlyphAtlas(*GetContext(), GlyphAtlas::Type::kAlphaBitmap,