Started pixel aligning hairlines (#166351)

fixes https://github.com/flutter/flutter/issues/162378
builds from: https://github.com/flutter/flutter/pull/165079
gets rid of the line artifacts in
https://github.com/flutter/flutter/issues/165689

## 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-04-02 14:28:58 -07:00
committed by GitHub
parent aeab137280
commit 25f7c15dd7
6 changed files with 174 additions and 5 deletions

View File

@@ -224,6 +224,16 @@ bool DlPath::IsOval(DlRect* bounds) const {
return GetSkPath().isOval(ToSkRect(bounds));
}
bool DlPath::IsLine(DlPoint* start, DlPoint* end) const {
SkPoint sk_points[2];
if (GetSkPath().isLine(sk_points)) {
*start = ToDlPoint(sk_points[0]);
*end = ToDlPoint(sk_points[1]);
return true;
}
return false;
}
bool DlPath::IsRoundRect(DlRoundRect* rrect) const {
SkRRect sk_rrect;
bool ret = GetSkPath().isRRect(rrect ? &sk_rrect : nullptr);

View File

@@ -118,6 +118,7 @@ class DlPath {
bool IsRect(DlRect* rect = nullptr, bool* is_closed = nullptr) const;
bool IsOval(DlRect* bounds = nullptr) const;
bool IsLine(DlPoint* start = nullptr, DlPoint* end = nullptr) const;
bool IsRoundRect(DlRoundRect* rrect = nullptr) const;
bool IsSkRect(SkRect* rect, bool* is_closed = nullptr) const;

View File

@@ -555,6 +555,45 @@ TEST(DisplayListPath, ConstructFromImpellerEqualsConstructFromSkia) {
EXPECT_EQ(DlPath(path_builder, DlPathFillType::kNonZero), DlPath(sk_path));
}
TEST(DisplayListPath, IsLineFromSkPath) {
SkPath sk_path;
sk_path.moveTo(SkPoint::Make(0, 0));
sk_path.lineTo(SkPoint::Make(100, 100));
DlPath path = DlPath(sk_path);
DlPoint start;
DlPoint end;
EXPECT_TRUE(path.IsLine(&start, &end));
EXPECT_EQ(start, DlPoint::MakeXY(0, 0));
EXPECT_EQ(end, DlPoint::MakeXY(100, 100));
EXPECT_FALSE(DlPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100))).IsLine());
}
TEST(DisplayListPath, IsLineFromImpellerPath) {
DlPathBuilder path_builder;
path_builder.MoveTo({0, 0});
path_builder.LineTo({100, 0});
DlPath path = DlPath(path_builder, DlPathFillType::kNonZero);
DlPoint start;
DlPoint end;
EXPECT_TRUE(path.IsLine(&start, &end));
EXPECT_EQ(start, DlPoint::MakeXY(0, 0));
EXPECT_EQ(end, DlPoint::MakeXY(100, 0));
{
DlPathBuilder path_builder;
path_builder.MoveTo({0, 0});
path_builder.LineTo({100, 0});
path_builder.LineTo({100, 100});
DlPath path = DlPath(path_builder, DlPathFillType::kNonZero);
EXPECT_FALSE(path.IsLine());
}
}
namespace {
class DlPathReceiverMock : public DlPathReceiver {
public:

View File

@@ -258,6 +258,88 @@ TEST_P(AiksTest, CanRenderStrokedConicPaths) {
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}
TEST_P(AiksTest, HairlinePath) {
Scalar scale = 1.f;
Scalar rotation = 0.f;
Scalar offset = 0.f;
auto callback = [&]() -> sk_sp<DisplayList> {
if (AiksTest::ImGuiBegin("Controls", nullptr,
ImGuiWindowFlags_AlwaysAutoResize)) {
ImGui::SliderFloat("Scale", &scale, 0, 6);
ImGui::SliderFloat("Rotate", &rotation, 0, 90);
ImGui::SliderFloat("Offset", &offset, 0, 2);
ImGui::End();
}
DisplayListBuilder builder;
builder.Scale(GetContentScale().x, GetContentScale().y);
builder.DrawPaint(DlPaint(DlColor(0xff111111)));
DlPaint paint;
paint.setStrokeWidth(0.f);
paint.setColor(DlColor::kWhite());
paint.setStrokeCap(DlStrokeCap::kRound);
paint.setStrokeJoin(DlStrokeJoin::kRound);
paint.setDrawStyle(DlDrawStyle::kStroke);
builder.Translate(512, 384);
builder.Scale(scale, scale);
builder.Rotate(rotation);
builder.Translate(-512, -384 + offset);
for (int i = 0; i < 5; ++i) {
Scalar yoffset = i * 25.25f + 300.f;
DlPathBuilder path_builder;
path_builder.MoveTo(DlPoint(100, yoffset));
path_builder.LineTo(DlPoint(924, yoffset));
builder.DrawPath(DlPath(path_builder), paint);
}
return builder.Build();
};
ASSERT_TRUE(OpenPlaygroundHere(callback));
}
TEST_P(AiksTest, HairlineDrawLine) {
Scalar scale = 1.f;
Scalar rotation = 0.f;
Scalar offset = 0.f;
auto callback = [&]() -> sk_sp<DisplayList> {
if (AiksTest::ImGuiBegin("Controls", nullptr,
ImGuiWindowFlags_AlwaysAutoResize)) {
ImGui::SliderFloat("Scale", &scale, 0, 6);
ImGui::SliderFloat("Rotate", &rotation, 0, 90);
ImGui::SliderFloat("Offset", &offset, 0, 2);
ImGui::End();
}
DisplayListBuilder builder;
builder.Scale(GetContentScale().x, GetContentScale().y);
builder.DrawPaint(DlPaint(DlColor(0xff111111)));
DlPaint paint;
paint.setStrokeWidth(0.f);
paint.setColor(DlColor::kWhite());
builder.Translate(512, 384);
builder.Scale(scale, scale);
builder.Rotate(rotation);
builder.Translate(-512, -384 + offset);
for (int i = 0; i < 5; ++i) {
Scalar yoffset = i * 25.25f + 300.f;
builder.DrawLine(DlPoint(100, yoffset), DlPoint(924, yoffset), paint);
}
return builder.Build();
};
ASSERT_TRUE(OpenPlaygroundHere(callback));
}
TEST_P(AiksTest, CanRenderTightConicPath) {
DisplayListBuilder builder;
builder.Scale(GetContentScale().x, GetContentScale().y);

View File

@@ -668,6 +668,13 @@ void DlDispatcherBase::SimplifyOrDrawPath(Canvas& canvas,
return;
}
DlPoint start;
DlPoint end;
if (path.IsLine(&start, &end)) {
canvas.DrawLine(start, end, paint);
return;
}
canvas.DrawPath(path.GetPath(), paint);
}

View File

@@ -78,22 +78,52 @@ Scalar LineGeometry::ComputeAlphaCoverage(const Matrix& entity) const {
return Geometry::ComputeStrokeAlphaCoverage(entity, width_);
}
namespace {
/// Minimizes the err when rounding to the closest 0.5 value.
/// If we round up, it drops down a half. If we round down it bumps up a half.
Scalar RoundToHalf(Scalar x) {
Scalar whole;
std::modf(x, &whole);
return whole + 0.5;
}
} // namespace
GeometryResult LineGeometry::GetPositionBuffer(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
using VT = SolidFillVertexShader::PerVertexData;
auto& transform = entity.GetTransform();
Matrix transform = entity.GetTransform();
auto radius = ComputePixelHalfWidth(transform, width_);
Point p0 = p0_;
Point p1 = p1_;
// Hairline pixel alignment.
if (width_ == 0.f && transform.IsTranslationScaleOnly()) {
p0 = transform * p0_;
p1 = transform * p1_;
transform = Matrix();
if (std::fabs(p0.x - p1.x) < kEhCloseEnough) {
p0.x = RoundToHalf(p0.x);
p1.x = p0.x;
} else if (std::fabs(p0.y - p1.y) < kEhCloseEnough) {
p0.y = RoundToHalf(p0.y);
p1.y = p0.y;
}
}
Entity fixed_transform = entity.Clone();
fixed_transform.SetTransform(transform);
if (cap_ == Cap::kRound) {
auto generator =
renderer.GetTessellator().RoundCapLine(transform, p0_, p1_, radius);
return ComputePositionGeometry(renderer, generator, entity, pass);
renderer.GetTessellator().RoundCapLine(transform, p0, p1, radius);
return ComputePositionGeometry(renderer, generator, fixed_transform, pass);
}
Point corners[4];
if (!ComputeCorners(corners, transform, cap_ == Cap::kSquare, p0_, p1_,
if (!ComputeCorners(corners, transform, cap_ == Cap::kSquare, p0, p1,
width_)) {
return kEmptyResult;
}
@@ -119,7 +149,7 @@ GeometryResult LineGeometry::GetPositionBuffer(const ContentContext& renderer,
.vertex_count = count,
.index_type = IndexType::kNone,
},
.transform = entity.GetShaderTransform(pass),
.transform = fixed_transform.GetShaderTransform(pass),
};
}