Reverts "[Impeller] add support for Skia concept of RRect::isSimple needed for DL dispatching" (#47821)
Reverts flutter/engine#47736
Initiated by: zanderso
This change reverts the following previous change:
Original Description:
Fixes https://github.com/flutter/flutter/issues/133793
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
index 3b34d94..3191257 100644
--- a/impeller/aiks/aiks_unittests.cc
+++ b/impeller/aiks/aiks_unittests.cc
@@ -2407,9 +2407,9 @@
TEST_P(AiksTest, DrawRectAbsorbsClearsNegativeRRect) {
Canvas canvas;
- canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), {5.0, 5.0},
+ canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), 5.0,
{.color = Color::Red(), .blend_mode = BlendMode::kSource});
- canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), {5.0, 5.0},
+ canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), 5.0,
{.color = Color::CornflowerBlue().WithAlpha(0.75),
.blend_mode = BlendMode::kSourceOver});
@@ -3077,7 +3077,7 @@
canvas.DrawCircle({300, 200}, 100, {.color = Color::GreenYellow()});
canvas.DrawCircle({140, 170}, 75, {.color = Color::DarkMagenta()});
canvas.DrawCircle({180, 120}, 100, {.color = Color::OrangeRed()});
- canvas.ClipRRect(Rect::MakeLTRB(a.x, a.y, b.x, b.y), {20, 20});
+ canvas.ClipRRect(Rect::MakeLTRB(a.x, a.y, b.x, b.y), 20);
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
ImageFilter::MakeBlur(Sigma(20.0), Sigma(20.0),
FilterContents::BlurStyle::kNormal,
@@ -3096,7 +3096,7 @@
canvas.DrawCircle({300, 200}, 100, {.color = Color::GreenYellow()});
canvas.DrawCircle({140, 170}, 75, {.color = Color::DarkMagenta()});
canvas.DrawCircle({180, 120}, 100, {.color = Color::OrangeRed()});
- canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), {20, 20});
+ canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), 20);
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
ImageFilter::MakeBlur(Sigma(30.0), Sigma(30.0),
FilterContents::BlurStyle::kNormal,
@@ -3202,7 +3202,7 @@
Canvas canvas;
canvas.Save();
- canvas.ClipRRect(Rect::MakeXYWH(0, 0, 400, 400), {10.0, 10.0},
+ canvas.ClipRRect(Rect::MakeXYWH(0, 0, 400, 400), 10.0,
Entity::ClipOperation::kIntersect);
canvas.DrawRect(Rect::MakeXYWH(0, 0, 400, 400), paint);
canvas.Restore();
@@ -3480,7 +3480,7 @@
TEST_P(AiksTest, DrawPictureClipped) {
Canvas subcanvas;
- subcanvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400), {15, 15});
+ subcanvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400), 15);
subcanvas.DrawPaint({.color = Color::Red()});
auto picture = subcanvas.EndRecordingAsPicture();
@@ -3492,7 +3492,7 @@
// Draw over the picture with a larger green rectangle, completely covering it
// up.
- canvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400).Expand(20), {15, 15});
+ canvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400).Expand(20), 15);
canvas.DrawPaint({.color = Color::Green()});
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
diff --git a/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc
index a0823f9..04852db 100644
--- a/impeller/aiks/canvas.cc
+++ b/impeller/aiks/canvas.cc
@@ -246,14 +246,13 @@
GetCurrentPass().AddEntity(entity);
}
-void Canvas::DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
- if (corner_radii.x == corner_radii.y &&
- AttemptDrawBlurredRRect(rect, corner_radii.x, paint)) {
+void Canvas::DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint) {
+ if (AttemptDrawBlurredRRect(rect, corner_radius, paint)) {
return;
}
auto path = PathBuilder{}
.SetConvexity(Convexity::kConvex)
- .AddRoundedRect(rect, corner_radii)
+ .AddRoundedRect(rect, corner_radius)
.SetBounds(rect)
.TakePath();
if (paint.style == Paint::Style::kFill) {
@@ -319,20 +318,20 @@
}
void Canvas::ClipRRect(const Rect& rect,
- Point corner_radii,
+ Scalar corner_radius,
Entity::ClipOperation clip_op) {
auto path = PathBuilder{}
.SetConvexity(Convexity::kConvex)
- .AddRoundedRect(rect, corner_radii)
+ .AddRoundedRect(rect, corner_radius)
.SetBounds(rect)
.TakePath();
auto size = rect.GetSize();
// Does the rounded rect have a flat part on the top/bottom or left/right?
- bool flat_on_TB = corner_radii.x * 2 < size.width;
- bool flat_on_LR = corner_radii.y * 2 < size.height;
+ bool flat_on_TB = corner_radius * 2 < size.width;
+ bool flat_on_LR = corner_radius * 2 < size.height;
std::optional<Rect> inner_rect = (flat_on_LR && flat_on_TB)
- ? rect.Expand(-corner_radii)
+ ? rect.Expand(-corner_radius)
: std::make_optional<Rect>();
auto geometry = Geometry::MakeFillPath(path, inner_rect);
auto& cull_rect = xformation_stack_.back().cull_rect;
@@ -349,7 +348,7 @@
IntersectCulling(rect);
break;
case Entity::ClipOperation::kDifference:
- if (corner_radii.x <= 0.0 || corner_radii.y <= 0) {
+ if (corner_radius <= 0) {
SubtractCulling(rect);
} else {
// We subtract the inner "tall" and "wide" rectangle pieces
@@ -358,10 +357,14 @@
// Since this is a subtract operation, we can subtract each
// rectangle piece individually without fear of interference.
if (flat_on_TB) {
- SubtractCulling(rect.Expand({-corner_radii.x, 0.0}));
+ SubtractCulling(Rect::MakeLTRB(
+ rect.GetLeft() + corner_radius, rect.GetTop(),
+ rect.GetRight() - corner_radius, rect.GetBottom()));
}
if (flat_on_LR) {
- SubtractCulling(rect.Expand({0.0, -corner_radii.y}));
+ SubtractCulling(Rect::MakeLTRB(
+ rect.GetLeft(), rect.GetTop() + corner_radius, //
+ rect.GetRight(), rect.GetBottom() - corner_radius));
}
}
break;
diff --git a/impeller/aiks/canvas.h b/impeller/aiks/canvas.h
index 61b1be4..285b7e4 100644
--- a/impeller/aiks/canvas.h
+++ b/impeller/aiks/canvas.h
@@ -105,7 +105,7 @@
void DrawRect(Rect rect, const Paint& paint);
- void DrawRRect(Rect rect, Point corner_radii, const Paint& paint);
+ void DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint);
void DrawCircle(Point center, Scalar radius, const Paint& paint);
@@ -135,7 +135,7 @@
void ClipRRect(
const Rect& rect,
- Point corner_radii,
+ Scalar corner_radius,
Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect);
void DrawPicture(const Picture& picture);
diff --git a/impeller/aiks/canvas_recorder.h b/impeller/aiks/canvas_recorder.h
index 360f39f..62711c1 100644
--- a/impeller/aiks/canvas_recorder.h
+++ b/impeller/aiks/canvas_recorder.h
@@ -182,9 +182,9 @@
paint);
}
- void DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
+ void DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint) {
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(DrawRRect), rect,
- corner_radii, paint);
+ corner_radius, paint);
}
void DrawCircle(Point center, Scalar radius, const Paint& paint) {
@@ -233,10 +233,10 @@
void ClipRRect(
const Rect& rect,
- Point corner_radii,
+ Scalar corner_radius,
Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect) {
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(ClipRRect), rect,
- corner_radii, clip_op);
+ corner_radius, clip_op);
}
void DrawPicture(const Picture& picture) {
diff --git a/impeller/aiks/canvas_recorder_unittests.cc b/impeller/aiks/canvas_recorder_unittests.cc
index fe2e457..041fc18 100644
--- a/impeller/aiks/canvas_recorder_unittests.cc
+++ b/impeller/aiks/canvas_recorder_unittests.cc
@@ -160,7 +160,7 @@
TEST(CanvasRecorder, DrawRRect) {
CanvasRecorder<Serializer> recorder;
- recorder.DrawRRect(Rect(), {}, Paint());
+ recorder.DrawRRect(Rect(), 0, Paint());
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::DrawRRect);
}
@@ -202,7 +202,7 @@
TEST(CanvasRecorder, ClipRRect) {
CanvasRecorder<Serializer> recorder;
- recorder.ClipRRect({}, {});
+ recorder.ClipRRect({}, 0);
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::ClipRRect);
}
diff --git a/impeller/aiks/canvas_unittests.cc b/impeller/aiks/canvas_unittests.cc
index 30be8f4..1f65bec 100644
--- a/impeller/aiks/canvas_unittests.cc
+++ b/impeller/aiks/canvas_unittests.cc
@@ -170,7 +170,7 @@
Rect rect_clip = Rect::MakeXYWH(5, 5, 10, 10);
Canvas canvas;
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kIntersect);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kIntersect);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), rect_clip);
@@ -180,7 +180,7 @@
Rect rect_clip = Rect::MakeXYWH(5, 5, 10, 10);
Canvas canvas;
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
ASSERT_FALSE(canvas.GetCurrentLocalCullingBounds().has_value());
}
@@ -191,7 +191,7 @@
Rect result_cull = Rect::MakeXYWH(5, 5, 5, 5);
Canvas canvas(initial_cull);
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kIntersect);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kIntersect);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -203,7 +203,7 @@
Rect result_cull = Rect::MakeXYWH(0, 0, 10, 10);
Canvas canvas(initial_cull);
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -215,7 +215,7 @@
Rect result_cull = Rect::MakeXYWH(0, 0, 6, 10);
Canvas canvas(initial_cull);
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -227,7 +227,7 @@
Rect result_cull = Rect::MakeXYWH(0, 0, 5, 10);
Canvas canvas(initial_cull);
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -239,7 +239,7 @@
Rect result_cull = Rect::MakeXYWH(0, 0, 10, 6);
Canvas canvas(initial_cull);
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -251,7 +251,7 @@
Rect result_cull = Rect::MakeXYWH(0, 0, 10, 5);
Canvas canvas(initial_cull);
- canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
+ canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc
index 2b3e02f..cbc4ee6 100644
--- a/impeller/display_list/dl_dispatcher.cc
+++ b/impeller/display_list/dl_dispatcher.cc
@@ -726,13 +726,9 @@
// |flutter::DlOpReceiver|
void DlDispatcher::clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) {
- if (rrect.isRect()) {
- canvas_.ClipRect(skia_conversions::ToRect(rrect.rect()),
- ToClipOperation(clip_op));
- } else if (rrect.isSimple()) {
+ if (rrect.isSimple()) {
canvas_.ClipRRect(skia_conversions::ToRect(rrect.rect()),
- skia_conversions::ToPoint(rrect.getSimpleRadii()),
- ToClipOperation(clip_op));
+ rrect.getSimpleRadii().fX, ToClipOperation(clip_op));
} else {
canvas_.ClipPath(skia_conversions::ToPath(rrect), ToClipOperation(clip_op));
}
@@ -797,8 +793,7 @@
void DlDispatcher::drawRRect(const SkRRect& rrect) {
if (rrect.isSimple()) {
canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
- skia_conversions::ToPoint(rrect.getSimpleRadii()),
- paint_);
+ rrect.getSimpleRadii().fX, paint_);
} else {
canvas_.DrawPath(skia_conversions::ToPath(rrect), paint_);
}
@@ -814,36 +809,30 @@
// |flutter::DlOpReceiver|
void DlDispatcher::drawPath(const SkPath& path) {
- SimplifyOrDrawPath(canvas_, path, paint_);
-}
-
-void DlDispatcher::SimplifyOrDrawPath(CanvasType& canvas,
- const SkPath& path,
- const Paint& paint) {
SkRect rect;
// We can't "optimize" a path into a rectangle if it's open.
bool closed;
if (path.isRect(&rect, &closed) && closed) {
- canvas.DrawRect(skia_conversions::ToRect(rect), paint);
+ canvas_.DrawRect(skia_conversions::ToRect(rect), paint_);
return;
}
SkRRect rrect;
if (path.isRRect(&rrect) && rrect.isSimple()) {
- canvas.DrawRRect(skia_conversions::ToRect(rrect.rect()),
- skia_conversions::ToPoint(rrect.getSimpleRadii()), paint);
+ canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
+ rrect.getSimpleRadii().fX, paint_);
return;
}
SkRect oval;
if (path.isOval(&oval) && oval.width() == oval.height()) {
- canvas.DrawCircle(skia_conversions::ToPoint(oval.center()),
- oval.width() * 0.5, paint);
+ canvas_.DrawCircle(skia_conversions::ToPoint(oval.center()),
+ oval.width() * 0.5, paint_);
return;
}
- canvas.DrawPath(skia_conversions::ToPath(path), paint);
+ canvas_.DrawPath(skia_conversions::ToPath(path), paint_);
}
// |flutter::DlOpReceiver|
@@ -1111,7 +1100,20 @@
canvas_.PreConcat(
Matrix::MakeTranslation(Vector2(0, -occluder_z * light_position.y)));
- SimplifyOrDrawPath(canvas_, path, paint);
+ SkRect rect;
+ SkRRect rrect;
+ SkRect oval;
+ if (path.isRect(&rect)) {
+ canvas_.DrawRect(skia_conversions::ToRect(rect), paint);
+ } else if (path.isRRect(&rrect) && rrect.isSimple()) {
+ canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
+ rrect.getSimpleRadii().fX, paint);
+ } else if (path.isOval(&oval) && oval.width() == oval.height()) {
+ canvas_.DrawCircle(skia_conversions::ToPoint(oval.center()),
+ oval.width() * 0.5, paint);
+ } else {
+ canvas_.DrawPath(skia_conversions::ToPath(path), paint);
+ }
canvas_.Restore();
}
diff --git a/impeller/display_list/dl_dispatcher.h b/impeller/display_list/dl_dispatcher.h
index 9e604dd..05d40cd 100644
--- a/impeller/display_list/dl_dispatcher.h
+++ b/impeller/display_list/dl_dispatcher.h
@@ -226,10 +226,6 @@
CanvasType canvas_;
Matrix initial_matrix_;
- static void SimplifyOrDrawPath(CanvasType& canvas,
- const SkPath& path,
- const Paint& paint);
-
DlDispatcher(const DlDispatcher&) = delete;
DlDispatcher& operator=(const DlDispatcher&) = delete;
diff --git a/impeller/display_list/dl_unittests.cc b/impeller/display_list/dl_unittests.cc
index 17e7aca..9e0456d 100644
--- a/impeller/display_list/dl_unittests.cc
+++ b/impeller/display_list/dl_unittests.cc
@@ -1591,68 +1591,6 @@
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
}
-TEST_P(DisplayListTest, ClipDrawRRectWithNonCircularRadii) {
- flutter::DisplayListBuilder builder;
-
- flutter::DlPaint fill_paint = //
- flutter::DlPaint() //
- .setColor(flutter::DlColor::kBlue()) //
- .setDrawStyle(flutter::DlDrawStyle::kFill) //
- .setStrokeWidth(10);
- flutter::DlPaint stroke_paint = //
- flutter::DlPaint() //
- .setColor(flutter::DlColor::kGreen()) //
- .setDrawStyle(flutter::DlDrawStyle::kStroke) //
- .setStrokeWidth(10);
-
- builder.DrawRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(500, 100, 300, 300), 120, 40),
- fill_paint);
- builder.DrawRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(500, 100, 300, 300), 120, 40),
- stroke_paint);
-
- builder.DrawRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(100, 500, 300, 300), 40, 120),
- fill_paint);
- builder.DrawRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(100, 500, 300, 300), 40, 120),
- stroke_paint);
-
- flutter::DlPaint reference_paint = //
- flutter::DlPaint() //
- .setColor(flutter::DlColor::kMidGrey()) //
- .setDrawStyle(flutter::DlDrawStyle::kFill) //
- .setStrokeWidth(10);
-
- builder.DrawRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(500, 500, 300, 300), 40, 40),
- reference_paint);
- builder.DrawRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(100, 100, 300, 300), 120, 120),
- reference_paint);
-
- flutter::DlPaint clip_fill_paint = //
- flutter::DlPaint() //
- .setColor(flutter::DlColor::kCyan()) //
- .setDrawStyle(flutter::DlDrawStyle::kFill) //
- .setStrokeWidth(10);
-
- builder.Save();
- builder.ClipRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(900, 100, 300, 300), 120, 40));
- builder.DrawPaint(clip_fill_paint);
- builder.Restore();
-
- builder.Save();
- builder.ClipRRect(
- SkRRect::MakeRectXY(SkRect::MakeXYWH(100, 900, 300, 300), 40, 120));
- builder.DrawPaint(clip_fill_paint);
- builder.Restore();
-
- ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
-}
-
TEST_P(DisplayListTest, DrawVerticesBlendModes) {
std::vector<const char*> blend_mode_names;
std::vector<flutter::DlBlendMode> blend_mode_values;
diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc
index bad11f1..9495cb5 100644
--- a/impeller/geometry/path_builder.cc
+++ b/impeller/geometry/path_builder.cc
@@ -201,13 +201,7 @@
PathBuilder& PathBuilder::AddRoundedRect(Rect rect, Scalar radius) {
return radius <= 0.0 ? AddRect(rect)
- : AddRoundedRect(rect, RoundingRadii(radius));
-}
-
-PathBuilder& PathBuilder::AddRoundedRect(Rect rect, Point radii) {
- return radii.x <= 0 || radii.y <= 0
- ? AddRect(rect)
- : AddRoundedRect(rect, RoundingRadii(radii));
+ : AddRoundedRect(rect, {radius, radius, radius, radius});
}
PathBuilder& PathBuilder::AddRoundedRect(Rect rect, RoundingRadii radii) {
diff --git a/impeller/geometry/path_builder.h b/impeller/geometry/path_builder.h
index fc72a50..342f091 100644
--- a/impeller/geometry/path_builder.h
+++ b/impeller/geometry/path_builder.h
@@ -122,18 +122,6 @@
top_right(p_top_right, p_top_right),
bottom_right(p_bottom_right, p_bottom_right) {}
- explicit RoundingRadii(Scalar radius)
- : top_left(radius, radius),
- bottom_left(radius, radius),
- top_right(radius, radius),
- bottom_right(radius, radius) {}
-
- explicit RoundingRadii(Point radii)
- : top_left(radii),
- bottom_left(radii),
- top_right(radii),
- bottom_right(radii) {}
-
bool AreAllZero() const {
return top_left.IsZero() && //
bottom_left.IsZero() && //
@@ -144,8 +132,6 @@
PathBuilder& AddRoundedRect(Rect rect, RoundingRadii radii);
- PathBuilder& AddRoundedRect(Rect rect, Point radii);
-
PathBuilder& AddRoundedRect(Rect rect, Scalar radius);
PathBuilder& AddPath(const Path& path);
