From 576cdb40ff5112eec2fd88f9045132a1a8dc9ac1 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Mon, 31 Mar 2025 17:39:19 -0700 Subject: [PATCH] [impeller] fixes diagonal antialiased lines (#166298) fixes https://github.com/flutter/flutter/issues/166276 This fixes antialiased diagonal lines being a different width by doing all prefiltered math in unrotated space, then applying a rotation to the line. This avoids the oddity in the gpu gems 2 math. ## after photo ![IMG_7582](https://github.com/user-attachments/assets/26158d23-5416-420f-b092-18cbe00c7d57) ## without antialias ![IMG_7581](https://github.com/user-attachments/assets/fb913e56-15b1-46b4-af34-e4c7235eb21f) ## 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]. [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 --- .../display_list/aiks_dl_path_unittests.cc | 63 +++++++++++++++++++ .../impeller/entity/contents/line_contents.cc | 49 ++++++++++----- 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/engine/src/flutter/impeller/display_list/aiks_dl_path_unittests.cc b/engine/src/flutter/impeller/display_list/aiks_dl_path_unittests.cc index a450b907ea4..3ebb8283a3a 100644 --- a/engine/src/flutter/impeller/display_list/aiks_dl_path_unittests.cc +++ b/engine/src/flutter/impeller/display_list/aiks_dl_path_unittests.cc @@ -591,6 +591,69 @@ TEST_P(AiksTest, ScaleExperimentAntialiasLines) { ASSERT_TRUE(OpenPlaygroundHere(callback)); } +TEST_P(AiksTest, HexagonExperimentAntialiasLines) { + float scale = 5.0f; + float line_width = 10.f; + float rotation = 0.f; + + auto callback = [&]() -> sk_sp { + if (AiksTest::ImGuiBegin("Controls", nullptr, + ImGuiWindowFlags_AlwaysAutoResize)) { + // Use ImGui::SliderFloat for consistency + ImGui::SliderFloat("Scale", &scale, 0.001f, 5.0f); + ImGui::SliderFloat("Width", &line_width, 1.0f, 20.0f); + ImGui::SliderFloat("Rotation", &rotation, 0.0f, 180.0f); + + ImGui::End(); + } + DisplayListBuilder builder; + builder.Scale(static_cast(GetContentScale().x), + static_cast(GetContentScale().y)); + + builder.DrawPaint(DlPaint(DlColor(0xff111111))); // Background + + { + DlPaint hex_paint; + hex_paint.setColor( + DlColor::kGreen()); // Changed color to Red for visibility + hex_paint.setStrokeWidth(line_width); // Use the interactive width + + float cx = 512.0f; // Center X + float cy = 384.0f; // Center Y + float r = 80.0f; // Radius (distance from center to vertex) + + float r_sin60 = r * std::sqrt(3.0f) / 2.0f; + float r_cos60 = r / 2.0f; + + DlPoint v0 = DlPoint(cx + r, cy); // Right vertex + DlPoint v1 = DlPoint(cx + r_cos60, cy - r_sin60); // Top-right vertex + DlPoint v2 = DlPoint( + cx - r_cos60, + cy - r_sin60); // Top-left vertex (v1-v2 is top horizontal side) + DlPoint v3 = DlPoint(cx - r, cy); // Left vertex + DlPoint v4 = DlPoint(cx - r_cos60, cy + r_sin60); // Bottom-left vertex + DlPoint v5 = + DlPoint(cx + r_cos60, cy + r_sin60); // Bottom-right vertex (v4-v5 is + // bottom horizontal side) + + builder.Translate(cx, cy); + builder.Scale(scale, scale); + builder.Rotate(rotation); + builder.Translate(-cx, -cy); + + builder.DrawLine(v0, v1, hex_paint); + builder.DrawLine(v1, v2, hex_paint); // Top side + builder.DrawLine(v2, v3, hex_paint); + builder.DrawLine(v3, v4, hex_paint); + builder.DrawLine(v4, v5, hex_paint); // Bottom side + builder.DrawLine(v5, v0, hex_paint); // Close the hexagon + } + + return builder.Build(); + }; + ASSERT_TRUE(OpenPlaygroundHere(callback)); +} + TEST_P(AiksTest, SimpleExperimentAntialiasLines) { DisplayListBuilder builder; builder.Scale(GetContentScale().x, GetContentScale().y); diff --git a/engine/src/flutter/impeller/entity/contents/line_contents.cc b/engine/src/flutter/impeller/entity/contents/line_contents.cc index 4995c1aeade..949ad1eed39 100644 --- a/engine/src/flutter/impeller/entity/contents/line_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/line_contents.cc @@ -77,17 +77,29 @@ std::pair CreateGeometry( kEmptyResult); } - return std::make_pair(calculate_status.value(), - GeometryResult{ - .type = PrimitiveType::kTriangleStrip, - .vertex_buffer = - { - .vertex_buffer = vertex_buffer, - .vertex_count = count, - .index_type = IndexType::kNone, - }, - .transform = entity.GetShaderTransform(pass), - }); + // We do the math in CalculatePerVertex in unrotated space. This then applies + // the rotation to the line. + Point diff = line_geometry->GetP1() - line_geometry->GetP0(); + Scalar angle = std::atan2(diff.y, diff.x); + Entity rotated_entity = entity.Clone(); + Matrix matrix = entity.GetTransform(); + matrix = matrix * Matrix::MakeTranslation(line_geometry->GetP0()) * + Matrix::MakeRotationZ(Radians(angle)) * + Matrix::MakeTranslation(-1 * line_geometry->GetP0()); + rotated_entity.SetTransform(matrix); + + return std::make_pair( + calculate_status.value(), + GeometryResult{ + .type = PrimitiveType::kTriangleStrip, + .vertex_buffer = + { + .vertex_buffer = vertex_buffer, + .vertex_count = count, + .index_type = IndexType::kNone, + }, + .transform = rotated_entity.GetShaderTransform(pass), + }); } struct LineInfo { @@ -161,6 +173,7 @@ bool LineContents::Render(const ContentContext& renderer, [&renderer](ContentContextOptions options) { return renderer.GetLinePipeline(options); }; + return ColorSourceContents::DrawGeometry( this, geometry_.get(), renderer, entity, pass, pipeline_callback, frame_info, @@ -215,13 +228,21 @@ LineContents::CalculatePerVertex(LineVertexShader::PerVertexData* per_vertex, const LineGeometry* geometry, const Matrix& entity_transform) { Scalar scale = entity_transform.GetMaxBasisLengthXY(); + + // Transform the line into unrotated space by rotating p1 to be horizontal + // with p0. We do this because there seems to be a flaw in the eN calculations + // where they create thinner lines for diagonal lines. + Point diff = geometry->GetP1() - geometry->GetP0(); + Scalar magnitude = diff.GetLength(); + Point p1_prime = Point(geometry->GetP0().x + magnitude, geometry->GetP0().y); + std::array corners; // Make sure we get kSampleRadius pixels to sample from. Scalar expand_size = std::max(kSampleRadius / scale, kSampleRadius); if (!LineGeometry::ComputeCorners( corners.data(), entity_transform, /*extend_endpoints=*/geometry->GetCap() != Cap::kButt, - geometry->GetP0(), geometry->GetP1(), geometry->GetWidth())) { + geometry->GetP0(), p1_prime, geometry->GetWidth())) { return fml::Status(fml::StatusCode::kAborted, "No valid corners"); } Scalar effective_line_width = std::fabsf((corners[2] - corners[0]).y); @@ -230,8 +251,8 @@ LineContents::CalculatePerVertex(LineVertexShader::PerVertexData* per_vertex, Scalar effective_sample_radius = (padded_line_width - effective_line_width) / 2.f; LineInfo line_info = - CalculateLineInfo(geometry->GetP0(), geometry->GetP1(), - effective_line_width, effective_sample_radius); + CalculateLineInfo(geometry->GetP0(), p1_prime, effective_line_width, + effective_sample_radius); for (auto& corner : corners) { *per_vertex++ = { .position = corner,