Skip to content

[CP] all jittery glyph fixes #163058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 7 commits into from
Feb 20, 2025
Merged

[CP] all jittery glyph fixes #163058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7700dca
[Impeller] backfilling TextContents unit tests (#161625)
gaaclarke Jan 21, 2025
742fe68
Fixed some floating point inaccuracies in TextContents (#162351)
gaaclarke Jan 30, 2025
9671502
Fixed the text aspect ratio (#162415)
gaaclarke Jan 31, 2025
ef1d68c
Increased the glyph atlas resolution 2x (#162555)
gaaclarke Feb 3, 2025
6ea6c18
Fixed translated text's subpixel alignment. (#162824)
gaaclarke Feb 7, 2025
e2c6d8d
fixed bad merge
gaaclarke Feb 11, 2025
6e6009a
updated changelog
gaaclarke Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 3.27 Changes

Expand Down
2 changes: 2 additions & 0 deletions engine/src/flutter/ci/licenses_golden/excluded_files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions engine/src/flutter/impeller/display_list/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
35 changes: 26 additions & 9 deletions engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct TextRenderOptions {
DlColor color = DlColor::kYellow();
SkPoint position = SkPoint::Make(100, 200);
std::shared_ptr<DlMaskFilter> filter;
bool is_subpixel = false;
};

bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
Expand All @@ -59,6 +60,9 @@ bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
}
sk_sp<SkFontMgr> 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;
Expand Down Expand Up @@ -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<DisplayList> {
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) {
Expand Down
7 changes: 4 additions & 3 deletions engine/src/flutter/impeller/display_list/dl_dispatcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<GlyphProperties>(properties) //
: std::nullopt //
Expand Down
95 changes: 10 additions & 85 deletions engine/src/flutter/impeller/display_list/dl_golden_blur_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<DlMaskFilter> 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<SkFontMgr> 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,
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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<double>(left[0]) - right[0], //
static_cast<double>(left[1]) - right[1], //
static_cast<double>(left[2]) - right[2], //
static_cast<double>(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<double>(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.
Expand Down
Loading