Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit b5350e1

Browse files
flarflar
authored
[Impeller] Round rects with circular ends should not generate ellipses (#49021)
Fixes flutter/flutter#140118 A bad test caused round rects with completely rounded sides to become ellipses.
1 parent 997d3df commit b5350e1

File tree

3 files changed

+121
-19
lines changed

3 files changed

+121
-19
lines changed

‎impeller/aiks/aiks_unittests.cc

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,18 @@ TEST_P(AiksTest, CanRenderSimpleClips) {
348348
canvas.DrawPaint(paint);
349349
canvas.Restore();
350350
}
351+
{
352+
canvas.Save();
353+
canvas.ClipRRect(Rect::MakeLTRB(200, 230, 300, 270), {20, 20});
354+
canvas.DrawPaint(paint);
355+
canvas.Restore();
356+
}
357+
{
358+
canvas.Save();
359+
canvas.ClipRRect(Rect::MakeLTRB(230, 200, 270, 300), {20, 20});
360+
canvas.DrawPaint(paint);
361+
canvas.Restore();
362+
}
351363
canvas.Restore();
352364
};
353365

@@ -2355,6 +2367,10 @@ TEST_P(AiksTest, FilledRoundRectsRenderCorrectly) {
23552367
Size(i * 5 + 10, j * 5 + 10), paint);
23562368
}
23572369
}
2370+
paint.color = colors[(c_index++) % color_count];
2371+
canvas.DrawRRect(Rect::MakeXYWH(10, 420, 380, 80), Size(40, 40), paint);
2372+
paint.color = colors[(c_index++) % color_count];
2373+
canvas.DrawRRect(Rect::MakeXYWH(410, 20, 80, 380), Size(40, 40), paint);
23582374

23592375
std::vector<Color> gradient_colors = {
23602376
Color{0x1f / 255.0, 0.0, 0x5c / 255.0, 1.0},
@@ -2377,26 +2393,37 @@ TEST_P(AiksTest, FilledRoundRectsRenderCorrectly) {
23772393
/*enable_mipmapping=*/true);
23782394

23792395
paint.color = Color::White().WithAlpha(0.1);
2380-
23812396
paint.color_source = ColorSource::MakeRadialGradient(
2382-
{500, 550}, 75, std::move(gradient_colors), std::move(stops),
2383-
Entity::TileMode::kMirror, {});
2397+
{550, 550}, 75, gradient_colors, stops, Entity::TileMode::kMirror, {});
23842398
for (int i = 1; i <= 10; i++) {
23852399
int j = 11 - i;
2386-
canvas.DrawRRect(Rect::MakeLTRB(500 - i * 20, 550 - j * 20, //
2387-
500 + i * 20, 550 + j * 20),
2400+
canvas.DrawRRect(Rect::MakeLTRB(550 - i * 20, 550 - j * 20, //
2401+
550 + i * 20, 550 + j * 20),
23882402
Size(i * 10, j * 10), paint);
23892403
}
2404+
paint.color = Color::White().WithAlpha(0.5);
2405+
paint.color_source = ColorSource::MakeRadialGradient(
2406+
{200, 650}, 75, std::move(gradient_colors), std::move(stops),
2407+
Entity::TileMode::kMirror, {});
2408+
canvas.DrawRRect(Rect::MakeLTRB(100, 610, 300, 690), Size(40, 40), paint);
2409+
canvas.DrawRRect(Rect::MakeLTRB(160, 550, 240, 750), Size(40, 40), paint);
23902410

2411+
paint.color = Color::White().WithAlpha(0.1);
23912412
paint.color_source = ColorSource::MakeImage(
23922413
texture, Entity::TileMode::kRepeat, Entity::TileMode::kRepeat, {},
2393-
Matrix::MakeTranslation({500, 20}));
2414+
Matrix::MakeTranslation({520, 20}));
23942415
for (int i = 1; i <= 10; i++) {
23952416
int j = 11 - i;
2396-
canvas.DrawRRect(Rect::MakeLTRB(700 - i * 20, 220 - j * 20, //
2397-
700 + i * 20, 220 + j * 20),
2417+
canvas.DrawRRect(Rect::MakeLTRB(720 - i * 20, 220 - j * 20, //
2418+
720 + i * 20, 220 + j * 20),
23982419
Size(i * 10, j * 10), paint);
23992420
}
2421+
paint.color = Color::White().WithAlpha(0.5);
2422+
paint.color_source = ColorSource::MakeImage(
2423+
texture, Entity::TileMode::kRepeat, Entity::TileMode::kRepeat, {},
2424+
Matrix::MakeTranslation({800, 300}));
2425+
canvas.DrawRRect(Rect::MakeLTRB(800, 410, 1000, 490), Size(40, 40), paint);
2426+
canvas.DrawRRect(Rect::MakeLTRB(860, 350, 940, 550), Size(40, 40), paint);
24002427

24012428
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
24022429
}

‎impeller/tessellator/tessellator.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ EllipticalVertexGenerator Tessellator::FilledRoundRect(
472472
const Matrix& view_transform,
473473
const Rect& bounds,
474474
const Size& radii) {
475-
if (radii.width * 2 < bounds.GetSize().width &&
475+
if (radii.width * 2 < bounds.GetSize().width ||
476476
radii.height * 2 < bounds.GetSize().height) {
477477
auto max_radius = radii.MaxDimension();
478478
auto divisions = ComputeQuadrantDivisions(

‎impeller/tessellator/tessellator_unittests.cc

Lines changed: 85 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -397,18 +397,93 @@ TEST(TessellatorTest, FilledEllipseTessellationVertices) {
397397

398398
// Square bounds should actually use the circle generator, but its
399399
// results should match the same math as the ellipse generator.
400-
test({}, Rect::MakeLTRB(0, 0, 2, 2));
401-
402-
test({}, Rect::MakeLTRB(0, 0, 2, 3));
403-
test({}, Rect::MakeLTRB(0, 0, 3, 2));
404-
test({}, Rect::MakeLTRB(5, 10, 2, 3));
405-
test({}, Rect::MakeLTRB(16, 7, 3, 2));
406-
test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeLTRB(5, 10, 3, 2));
407-
test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeLTRB(5, 10, 2, 3));
400+
test({}, Rect::MakeXYWH(0, 0, 2, 2));
401+
402+
test({}, Rect::MakeXYWH(0, 0, 2, 3));
403+
test({}, Rect::MakeXYWH(0, 0, 3, 2));
404+
test({}, Rect::MakeXYWH(5, 10, 2, 3));
405+
test({}, Rect::MakeXYWH(16, 7, 3, 2));
406+
test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 3, 2));
407+
test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 2, 3));
408408
test(Matrix::MakeScale({0.002, 0.002, 0.0}),
409-
Rect::MakeLTRB(5000, 10000, 3000, 2000));
409+
Rect::MakeXYWH(5000, 10000, 3000, 2000));
410410
test(Matrix::MakeScale({0.002, 0.002, 0.0}),
411-
Rect::MakeLTRB(5000, 10000, 2000, 3000));
411+
Rect::MakeXYWH(5000, 10000, 2000, 3000));
412+
}
413+
414+
TEST(TessellatorTest, FilledRoundRectTessellationVertices) {
415+
auto tessellator = std::make_shared<Tessellator>();
416+
417+
auto test = [&tessellator](const Matrix& transform, const Rect& bounds,
418+
const Size& radii) {
419+
FML_DCHECK(radii.width * 2 <= bounds.GetSize().width) << radii << bounds;
420+
FML_DCHECK(radii.height * 2 <= bounds.GetSize().height) << radii << bounds;
421+
422+
Scalar middle_left = bounds.GetOrigin().x + radii.width;
423+
Scalar middle_top = bounds.GetOrigin().y + radii.height;
424+
Scalar middle_right =
425+
bounds.GetOrigin().x + bounds.GetSize().width - radii.width;
426+
Scalar middle_bottom =
427+
bounds.GetOrigin().y + bounds.GetSize().height - radii.height;
428+
429+
auto generator = tessellator->FilledRoundRect(transform, bounds, radii);
430+
EXPECT_EQ(generator.GetTriangleType(), PrimitiveType::kTriangleStrip);
431+
432+
auto vertex_count = generator.GetVertexCount();
433+
auto vertices = std::vector<Point>();
434+
generator.GenerateVertices([&vertices](const Point& p) { //
435+
vertices.push_back(p);
436+
});
437+
EXPECT_EQ(vertices.size(), vertex_count);
438+
ASSERT_EQ(vertex_count % 4, 0u);
439+
440+
auto quadrant_count = vertex_count / 4;
441+
for (size_t i = 0; i < quadrant_count; i++) {
442+
double angle = kPiOver2 * i / (quadrant_count - 1);
443+
double degrees = angle * 180.0 / kPi;
444+
double rcos = cos(angle) * radii.width;
445+
double rsin = sin(angle) * radii.height;
446+
EXPECT_POINT_NEAR(vertices[i * 2],
447+
Point(middle_left - rcos, middle_bottom + rsin))
448+
<< "vertex " << i << ", angle = " << degrees << ", " //
449+
<< "bounds = " << bounds << std::endl;
450+
EXPECT_POINT_NEAR(vertices[i * 2 + 1],
451+
Point(middle_left - rcos, middle_top - rsin))
452+
<< "vertex " << i << ", angle = " << degrees << ", " //
453+
<< "bounds = " << bounds << std::endl;
454+
EXPECT_POINT_NEAR(vertices[vertex_count - i * 2 - 1],
455+
Point(middle_right + rcos, middle_top - rsin))
456+
<< "vertex " << i << ", angle = " << degrees << ", " //
457+
<< "bounds = " << bounds << std::endl;
458+
EXPECT_POINT_NEAR(vertices[vertex_count - i * 2 - 2],
459+
Point(middle_right + rcos, middle_bottom + rsin))
460+
<< "vertex " << i << ", angle = " << degrees << ", " //
461+
<< "bounds = " << bounds << std::endl;
462+
}
463+
};
464+
465+
// Both radii spanning the bounds should actually use the circle/ellipse
466+
// generator, but their results should match the same math as the round
467+
// rect generator.
468+
test({}, Rect::MakeXYWH(0, 0, 20, 20), {10, 10});
469+
470+
// One radius spanning the bounds, but not the other will not match the
471+
// round rect math if the generator transfers to circle/ellipse
472+
test({}, Rect::MakeXYWH(0, 0, 20, 20), {10, 5});
473+
test({}, Rect::MakeXYWH(0, 0, 20, 20), {5, 10});
474+
475+
test({}, Rect::MakeXYWH(0, 0, 20, 30), {2, 2});
476+
test({}, Rect::MakeXYWH(0, 0, 30, 20), {2, 2});
477+
test({}, Rect::MakeXYWH(5, 10, 20, 30), {2, 3});
478+
test({}, Rect::MakeXYWH(16, 7, 30, 20), {2, 3});
479+
test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 30, 20),
480+
{2, 3});
481+
test(Matrix::MakeScale({500.0, 500.0, 0.0}), Rect::MakeXYWH(5, 10, 20, 30),
482+
{2, 3});
483+
test(Matrix::MakeScale({0.002, 0.002, 0.0}),
484+
Rect::MakeXYWH(5000, 10000, 3000, 2000), {50, 70});
485+
test(Matrix::MakeScale({0.002, 0.002, 0.0}),
486+
Rect::MakeXYWH(5000, 10000, 2000, 3000), {50, 70});
412487
}
413488

414489
} // namespace testing

0 commit comments

Comments
 (0)