firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Reverts "Started adjusting uvs to match pixel snapping. (#162049)" (#162392)

Browse Source 
<!-- start_original_pr_link -->
Reverts: flutter/flutter#162049
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: gaaclarke
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: Negatively affected Android rendering (
https://github.com/flutter/flutter/issues/162361)
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: gaaclarke
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {jonahwilliams}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
issue: https://github.com/flutter/flutter/issues/149652
doc: (currently google only)
https://docs.google.com/document/d/1Rulw_noQi0G8Glb47vk17uBbb6sySDxlQe-l-0kHn14/edit?tab=t.0

This increases the RMSE value in the test in
https://github.com/flutter/flutter/pull/161445 by a slight amount. I do
believe this reduces the time where we get non uniform scalars and
protects the integrity of relative spacing, thus being more what we
expect. There is still a bug that has to do with pixel alignment that
does give the illusion of stretching and shrinking though because of
hard/soft lines.

## Before


https://github.com/user-attachments/assets/e9b80b70-0961-4e02-9053-84d4457348e5


## After


https://github.com/user-attachments/assets/2494a2b1-497d-4a2b-afd7-23064acba293



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

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
This commit is contained in:
auto-submit[bot] 2025-01-29 15:35:55 -08:00 committed by GitHub
parent 4fc2f0b251
commit d048c771e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 95 additions and 254 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
engine/src/flutter/ci/licenses_golden/excluded_files
View File

@ -152,7 +152,6 @@
../../../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

4
engine/src/flutter/impeller/display_list/BUILD.gn
View File

@ -78,10 +78,6 @@ 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)) {

28
engine/src/flutter/impeller/display_list/aiks_dl_text_unittests.cc
View File

@ -152,27 +152,17 @@ TEST_P(AiksTest, CanRenderTextFrameWithHalfScaling) {
}
TEST_P(AiksTest, CanRenderTextFrameWithFractionScaling) {
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();
}
DisplayListBuilder builder;
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();
};
DlPaint paint;
paint.setColor(DlColor::ARGB(1, 0.1, 0.1, 0.1));
builder.DrawPaint(paint);
builder.Scale(2.625, 2.625);
ASSERT_TRUE(OpenPlaygroundHere(callback));
ASSERT_TRUE(RenderTextInCanvasSkia(
GetContext(), builder, "the quick brown fox jumped over the lazy dog!.?",
"Roboto-Regular.ttf"));
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}
TEST_P(AiksTest, TextFrameSubpixelAlignment) {

79
engine/src/flutter/impeller/display_list/dl_golden_blur_unittests.cc
View File

@ -6,18 +6,58 @@
#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,
@ -72,6 +112,41 @@ 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.

64
engine/src/flutter/impeller/display_list/dl_golden_unittests.cc
View File

@ -8,8 +8,6 @@
#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"
@ -352,67 +350,5 @@ 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
// This test makes sure that given a tiny change in scale, a glyph will not do a
// large jump in its drawn y position. This was noticed when performing
// animations as a problematic artifact. We tried to come up with a more
// holistic quantification of the problem but haven't yet been able to.
TEST_P(DlGoldenTest, TextJumpingTest) {
SetWindowSize(impeller::ISize(1024, 200));
impeller::Scalar font_size = 300;
auto callback = [&](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);
// If you move this code to a playgrounds test the RenderTextInCanvasSkia
// signature is a bit different there, it will look like this:
//
// RenderTextInCanvasSkia(GetContext(), builder,
// "the quick brown fox jumped over the lazy dog!.?",
// "Roboto-Regular.ttf",
// TextRenderOptions{
// .font_size = font_size,
// .position = SkPoint::Make(100, 300),
// });
// Note: The ahem font just has full blocks in it.
RenderTextInCanvasSkia(&builder, "h", "Roboto-Regular.ttf",
DlPoint::MakeXY(10, 300),
TextRenderOptions{
.font_size = font_size,
});
return builder.Build();
};
std::unique_ptr<impeller::testing::Screenshot> right =
MakeScreenshot(callback(0.445));
if (!right) {
GTEST_SKIP() << "making screenshots not supported.";
}
std::unique_ptr<impeller::testing::Screenshot> left =
MakeScreenshot(callback(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 <= 1) << "y diff: " << y_diff;
}
} // namespace testing
} // namespace flutter

43
engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.cc
View File

@ -1,43 +0,0 @@
// 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

34
engine/src/flutter/impeller/display_list/testing/render_text_in_canvas.h
View File

@ -1,34 +0,0 @@
// 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_

47
engine/src/flutter/impeller/display_list/testing/rmse.cc
View File

@ -1,47 +0,0 @@
// 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

16
engine/src/flutter/impeller/display_list/testing/rmse.h
View File

@ -1,16 +0,0 @@
// 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_

33
engine/src/flutter/impeller/entity/contents/text_contents.cc
View File

@ -74,16 +74,6 @@ void TextContents::SetTextProperties(Color color,
}
}
namespace {
Matrix MakeRectTransform(Rect from, Rect to) {
return Matrix::MakeTranslation({to.GetLeft(), to.GetTop(), 0}) *
Matrix::MakeScale(
/*s=*/{to.GetWidth() / from.GetWidth(),
to.GetHeight() / from.GetHeight(), 1}) *
Matrix::MakeTranslation({-from.GetLeft(), -from.GetTop(), 0});
}
} // namespace
void TextContents::ComputeVertexData(
VS::PerVertexData* vtx_contents,
const std::shared_ptr<TextFrame>& frame,
@ -188,25 +178,20 @@ void TextContents::ComputeVertexData(
Point screen_glyph_position =
(screen_offset + unrounded_glyph_position + subpixel_adjustment)
.Floor();
Vector3 screen_size = basis_transform * scaled_bounds.GetSize();
Matrix position_to_uv = MakeRectTransform(
Rect::MakeOriginSize(screen_glyph_position,
Size(screen_size.x, screen_size.y)),
Rect::MakeOriginSize(uv_origin, Size(uv_size.x, uv_size.y)));
for (const Point& point : unit_points) {
Point position;
if (is_translation_scale) {
vtx.position = (screen_glyph_position +
(basis_transform * point * scaled_bounds.GetSize()))
.Round();
vtx.uv = position_to_uv * vtx.position;
position = (screen_glyph_position +
(basis_transform * point * scaled_bounds.GetSize()))
.Round();
} else {
vtx.position = entity_transform *
(glyph_position.position + scaled_bounds.GetLeftTop() +
point * scaled_bounds.GetSize());
vtx.uv = uv_origin + (uv_size * point);
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;
}
}