Fixed some floating point inaccuracies in TextContents (#162351)

issue: https://github.com/flutter/flutter/issues/149652

## before
Animating from 0.444x to 0.445x

![before-pos](https://github.com/user-attachments/assets/3b561e35-68ca-49ca-aa3e-e2c224ae06c5)

## after
Animating from 0.444x to 0.445x

![after-pos](https://github.com/user-attachments/assets/3ef67ec9-29ce-481a-83ff-cb27fbe2ddca)

## diff
Diff at 0.444x

![444](https://github.com/user-attachments/assets/2a309f2d-65ad-4be2-bc35-7b9df5639c0f)


## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
gaaclarke 2025-01-30 12:42:39 -08:00 committed by GitHub
parent 006783101d
commit d0d4e9613e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 220 additions and 88 deletions

View File

@ -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

View File

@ -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)) {

View File

@ -152,17 +152,27 @@ TEST_P(AiksTest, CanRenderTextFrameWithHalfScaling) {
}
TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) {
DisplayListBuilder builder;
Scalar fine_scale = 0.f;
auto callback = [&]() -> sk_sp<DisplayList> {
if (AiksTest::ImGuiBegin("Controls", nullptr,
ImGuiWindowFlags_AlwaysAutoResize)) {
ImGui::SliderFloat("Fine Scale", &fine_scale, -1, 1);
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");
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) {

View File

@ -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;
DlScalar 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,
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<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,
@ -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.

View File

@ -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"
@ -350,5 +352,52 @@ TEST_P(DlGoldenTest, SaveLayerAtFractionalValue) {
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}
namespace {
int32_t CalculateMaxY(const impeller::testing::Screenshot* img) {
const uint32_t* ptr = reinterpret_cast<const uint32_t*>(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<int32_t>(i));
}
}
}
return max_y;
}
} // 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<DisplayList> {
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<impeller::testing::Screenshot> right =
MakeScreenshot(callback("h", 0.444));
if (!right) {
GTEST_SKIP() << "making screenshots not supported.";
}
std::unique_ptr<impeller::testing::Screenshot> 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;
}
} // namespace testing
} // namespace flutter

View File

@ -0,0 +1,43 @@
// 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<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 testing
} // namespace flutter

View File

@ -0,0 +1,34 @@
// 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 <memory>
#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<DlMaskFilter> mask_filter;
};
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_

View File

@ -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<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]));
}
} // 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<double>(samples));
}
} // namespace testing
} // namespace flutter

View File

@ -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_

View File

@ -172,8 +172,11 @@ void TextContents::ComputeVertexData(
Point uv_size = (atlas_glyph_bounds.GetSize() + Point(1, 1)) / atlas_size;
Point unrounded_glyph_position =
basis_transform *
(glyph_position.position + scaled_bounds.GetLeftTop());
// 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)