From 7b5a579ccec367851ffebe4f27d7e2d2d9f5b686 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 13 Dec 2023 23:37:58 -0800 Subject: [PATCH] [Impeller] Round rects with circular ends should not generate ellipses --- impeller/aiks/aiks_unittests.cc | 43 +++++++-- impeller/tessellator/tessellator.cc | 2 +- impeller/tessellator/tessellator_unittests.cc | 95 +++++++++++++++++-- 3 files changed, 121 insertions(+), 19 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index c0f88a1d3bd41..9a550b0507eef 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -348,6 +348,18 @@ TEST_P(AiksTest, CanRenderSimpleClips) { canvas.DrawPaint(paint); canvas.Restore(); } + { + canvas.Save(); + canvas.ClipRRect(Rect::MakeLTRB(200, 230, 300, 270), {20, 20}); + canvas.DrawPaint(paint); + canvas.Restore(); + } + { + canvas.Save(); + canvas.ClipRRect(Rect::MakeLTRB(230, 200, 270, 300), {20, 20}); + canvas.DrawPaint(paint); + canvas.Restore(); + } canvas.Restore(); }; @@ -2355,6 +2367,10 @@ TEST_P(AiksTest, FilledRoundRectsRenderCorrectly) { Size(i * 5 + 10, j * 5 + 10), paint); } } + paint.color = colors[(c_index++) % color_count]; + canvas.DrawRRect(Rect::MakeXYWH(10, 420, 380, 80), Size(40, 40), paint); + paint.color = colors[(c_index++) % color_count]; + canvas.DrawRRect(Rect::MakeXYWH(410, 20, 80, 380), Size(40, 40), paint); std::vector gradient_colors = { Color{0x1f / 255.0, 0.0, 0x5c / 255.0, 1.0}, @@ -2377,26 +2393,37 @@ TEST_P(AiksTest, FilledRoundRectsRenderCorrectly) { /*enable_mipmapping=*/true); paint.color = Color::White().WithAlpha(0.1); - paint.color_source = ColorSource::MakeRadialGradient( - {500, 550}, 75, std::move(gradient_colors), std::move(stops), - Entity::TileMode::kMirror, {}); + {550, 550}, 75, gradient_colors, stops, Entity::TileMode::kMirror, {}); for (int i = 1; i <= 10; i++) { int j = 11 - i; - canvas.DrawRRect(Rect::MakeLTRB(500 - i * 20, 550 - j * 20, // - 500 + i * 20, 550 + j * 20), + canvas.DrawRRect(Rect::MakeLTRB(550 - i * 20, 550 - j * 20, // + 550 + i * 20, 550 + j * 20), Size(i * 10, j * 10), paint); } + paint.color = Color::White().WithAlpha(0.5); + paint.color_source = ColorSource::MakeRadialGradient( + {200, 650}, 75, std::move(gradient_colors), std::move(stops), + Entity::TileMode::kMirror, {}); + canvas.DrawRRect(Rect::MakeLTRB(100, 610, 300, 690), Size(40, 40), paint); + canvas.DrawRRect(Rect::MakeLTRB(160, 550, 240, 750), Size(40, 40), paint); + paint.color = Color::White().WithAlpha(0.1); paint.color_source = ColorSource::MakeImage( texture, Entity::TileMode::kRepeat, Entity::TileMode::kRepeat, {}, - Matrix::MakeTranslation({500, 20})); + Matrix::MakeTranslation({520, 20})); for (int i = 1; i <= 10; i++) { int j = 11 - i; - canvas.DrawRRect(Rect::MakeLTRB(700 - i * 20, 220 - j * 20, // - 700 + i * 20, 220 + j * 20), + canvas.DrawRRect(Rect::MakeLTRB(720 - i * 20, 220 - j * 20, // + 720 + i * 20, 220 + j * 20), Size(i * 10, j * 10), paint); } + paint.color = Color::White().WithAlpha(0.5); + paint.color_source = ColorSource::MakeImage( + texture, Entity::TileMode::kRepeat, Entity::TileMode::kRepeat, {}, + Matrix::MakeTranslation({800, 300})); + canvas.DrawRRect(Rect::MakeLTRB(800, 410, 1000, 490), Size(40, 40), paint); + canvas.DrawRRect(Rect::MakeLTRB(860, 350, 940, 550), Size(40, 40), paint); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index 35685c3f34479..c80d8738ead14 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -472,7 +472,7 @@ EllipticalVertexGenerator Tessellator::FilledRoundRect( const Matrix& view_transform, const Rect& bounds, const Size& radii) { - if (radii.width * 2 < bounds.GetSize().width && + if (radii.width * 2 < bounds.GetSize().width || radii.height * 2 < bounds.GetSize().height) { auto max_radius = radii.MaxDimension(); auto divisions = ComputeQuadrantDivisions( diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 10f64493d1e0e..ae4f9dcca47bf 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -397,18 +397,93 @@ TEST(TessellatorTest, FilledEllipseTessellationVertices) { // Square bounds should actually use the circle generator, but its // results should match the same math as the ellipse generator. - test({}, Rect::MakeLTRB(0, 0, 2, 2)); - - test({}, Rect::MakeLTRB(0, 0, 2, 3)); - test({}, Rect::MakeLTRB(0, 0, 3, 2)); - test({}, Rect::MakeLTRB(5, 10, 2, 3)); - test({}, Rect::MakeLTRB(16, 7, 3, 2)); - test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeLTRB(5, 10, 3, 2)); - test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeLTRB(5, 10, 2, 3)); + test({}, Rect::MakeXYWH(0, 0, 2, 2)); + + test({}, Rect::MakeXYWH(0, 0, 2, 3)); + test({}, Rect::MakeXYWH(0, 0, 3, 2)); + test({}, Rect::MakeXYWH(5, 10, 2, 3)); + test({}, Rect::MakeXYWH(16, 7, 3, 2)); + test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 3, 2)); + test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 2, 3)); test(Matrix::MakeScale({0.002, 0.002, 0.0}), - Rect::MakeLTRB(5000, 10000, 3000, 2000)); + Rect::MakeXYWH(5000, 10000, 3000, 2000)); test(Matrix::MakeScale({0.002, 0.002, 0.0}), - Rect::MakeLTRB(5000, 10000, 2000, 3000)); + Rect::MakeXYWH(5000, 10000, 2000, 3000)); +} + +TEST(TessellatorTest, FilledRoundRectTessellationVertices) { + auto tessellator = std::make_shared(); + + auto test = [&tessellator](const Matrix& transform, const Rect& bounds, + const Size& radii) { + FML_DCHECK(radii.width * 2 <= bounds.GetSize().width) << radii << bounds; + FML_DCHECK(radii.height * 2 <= bounds.GetSize().height) << radii << bounds; + + Scalar middle_left = bounds.GetOrigin().x + radii.width; + Scalar middle_top = bounds.GetOrigin().y + radii.height; + Scalar middle_right = + bounds.GetOrigin().x + bounds.GetSize().width - radii.width; + Scalar middle_bottom = + bounds.GetOrigin().y + bounds.GetSize().height - radii.height; + + auto generator = tessellator->FilledRoundRect(transform, bounds, radii); + EXPECT_EQ(generator.GetTriangleType(), PrimitiveType::kTriangleStrip); + + auto vertex_count = generator.GetVertexCount(); + auto vertices = std::vector(); + generator.GenerateVertices([&vertices](const Point& p) { // + vertices.push_back(p); + }); + EXPECT_EQ(vertices.size(), vertex_count); + ASSERT_EQ(vertex_count % 4, 0u); + + auto quadrant_count = vertex_count / 4; + for (size_t i = 0; i < quadrant_count; i++) { + double angle = kPiOver2 * i / (quadrant_count - 1); + double degrees = angle * 180.0 / kPi; + double rcos = cos(angle) * radii.width; + double rsin = sin(angle) * radii.height; + EXPECT_POINT_NEAR(vertices[i * 2], + Point(middle_left - rcos, middle_bottom + rsin)) + << "vertex " << i << ", angle = " << degrees << ", " // + << "bounds = " << bounds << std::endl; + EXPECT_POINT_NEAR(vertices[i * 2 + 1], + Point(middle_left - rcos, middle_top - rsin)) + << "vertex " << i << ", angle = " << degrees << ", " // + << "bounds = " << bounds << std::endl; + EXPECT_POINT_NEAR(vertices[vertex_count - i * 2 - 1], + Point(middle_right + rcos, middle_top - rsin)) + << "vertex " << i << ", angle = " << degrees << ", " // + << "bounds = " << bounds << std::endl; + EXPECT_POINT_NEAR(vertices[vertex_count - i * 2 - 2], + Point(middle_right + rcos, middle_bottom + rsin)) + << "vertex " << i << ", angle = " << degrees << ", " // + << "bounds = " << bounds << std::endl; + } + }; + + // Both radii spanning the bounds should actually use the circle/ellipse + // generator, but their results should match the same math as the round + // rect generator. + test({}, Rect::MakeXYWH(0, 0, 20, 20), {10, 10}); + + // One radius spanning the bounds, but not the other will not match the + // round rect math if the generator transfers to circle/ellipse + test({}, Rect::MakeXYWH(0, 0, 20, 20), {10, 5}); + test({}, Rect::MakeXYWH(0, 0, 20, 20), {5, 10}); + + test({}, Rect::MakeXYWH(0, 0, 20, 30), {2, 2}); + test({}, Rect::MakeXYWH(0, 0, 30, 20), {2, 2}); + test({}, Rect::MakeXYWH(5, 10, 20, 30), {2, 3}); + test({}, Rect::MakeXYWH(16, 7, 30, 20), {2, 3}); + test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 30, 20), + {2, 3}); + test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 20, 30), + {2, 3}); + test(Matrix::MakeScale({0.002, 0.002, 0.0}), + Rect::MakeXYWH(5000, 10000, 3000, 2000), {50, 70}); + test(Matrix::MakeScale({0.002, 0.002, 0.0}), + Rect::MakeXYWH(5000, 10000, 2000, 3000), {50, 70}); } } // namespace testing