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

Commit 5d49c6d

Browse files
flarflar
authored
Reland "[Impeller] add support for Skia concept of RRect::isSimple needed for DL dispatching" (#47824)
Reverts #47821 The original PR was reverted because the Vulkan backends couldn't handle the extra tests, but they've since been patched to fix the problem.
1 parent 8a99224 commit 5d49c6d

File tree

11 files changed

+143
-62
lines changed

11 files changed

+143
-62
lines changed

‎impeller/aiks/aiks_unittests.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2407,9 +2407,9 @@ TEST_P(AiksTest, DrawRectAbsorbsClears) {
24072407

24082408
TEST_P(AiksTest, DrawRectAbsorbsClearsNegativeRRect) {
24092409
Canvas canvas;
2410-
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), 5.0,
2410+
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), {5.0, 5.0},
24112411
{.color = Color::Red(), .blend_mode = BlendMode::kSource});
2412-
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), 5.0,
2412+
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), {5.0, 5.0},
24132413
{.color = Color::CornflowerBlue().WithAlpha(0.75),
24142414
.blend_mode = BlendMode::kSourceOver});
24152415

@@ -3077,7 +3077,7 @@ TEST_P(AiksTest, CanRenderBackdropBlurInteractive) {
30773077
canvas.DrawCircle({300, 200}, 100, {.color = Color::GreenYellow()});
30783078
canvas.DrawCircle({140, 170}, 75, {.color = Color::DarkMagenta()});
30793079
canvas.DrawCircle({180, 120}, 100, {.color = Color::OrangeRed()});
3080-
canvas.ClipRRect(Rect::MakeLTRB(a.x, a.y, b.x, b.y), 20);
3080+
canvas.ClipRRect(Rect::MakeLTRB(a.x, a.y, b.x, b.y), {20, 20});
30813081
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
30823082
ImageFilter::MakeBlur(Sigma(20.0), Sigma(20.0),
30833083
FilterContents::BlurStyle::kNormal,
@@ -3096,7 +3096,7 @@ TEST_P(AiksTest, CanRenderBackdropBlur) {
30963096
canvas.DrawCircle({300, 200}, 100, {.color = Color::GreenYellow()});
30973097
canvas.DrawCircle({140, 170}, 75, {.color = Color::DarkMagenta()});
30983098
canvas.DrawCircle({180, 120}, 100, {.color = Color::OrangeRed()});
3099-
canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), 20);
3099+
canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), {20, 20});
31003100
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
31013101
ImageFilter::MakeBlur(Sigma(30.0), Sigma(30.0),
31023102
FilterContents::BlurStyle::kNormal,
@@ -3202,7 +3202,7 @@ TEST_P(AiksTest, CanRenderClippedRuntimeEffects) {
32023202

32033203
Canvas canvas;
32043204
canvas.Save();
3205-
canvas.ClipRRect(Rect::MakeXYWH(0, 0, 400, 400), 10.0,
3205+
canvas.ClipRRect(Rect::MakeXYWH(0, 0, 400, 400), {10.0, 10.0},
32063206
Entity::ClipOperation::kIntersect);
32073207
canvas.DrawRect(Rect::MakeXYWH(0, 0, 400, 400), paint);
32083208
canvas.Restore();
@@ -3480,7 +3480,7 @@ TEST_P(AiksTest, DrawPictureWithText) {
34803480

34813481
TEST_P(AiksTest, DrawPictureClipped) {
34823482
Canvas subcanvas;
3483-
subcanvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400), 15);
3483+
subcanvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400), {15, 15});
34843484
subcanvas.DrawPaint({.color = Color::Red()});
34853485
auto picture = subcanvas.EndRecordingAsPicture();
34863486

@@ -3492,7 +3492,7 @@ TEST_P(AiksTest, DrawPictureClipped) {
34923492

34933493
// Draw over the picture with a larger green rectangle, completely covering it
34943494
// up.
3495-
canvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400).Expand(20), 15);
3495+
canvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400).Expand(20), {15, 15});
34963496
canvas.DrawPaint({.color = Color::Green()});
34973497

34983498
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));

‎impeller/aiks/canvas.cc‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,14 @@ void Canvas::DrawRect(Rect rect, const Paint& paint) {
246246
GetCurrentPass().AddEntity(entity);
247247
}
248248

249-
void Canvas::DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint) {
250-
if (AttemptDrawBlurredRRect(rect, corner_radius, paint)) {
249+
void Canvas::DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
250+
if (corner_radii.x == corner_radii.y &&
251+
AttemptDrawBlurredRRect(rect, corner_radii.x, paint)) {
251252
return;
252253
}
253254
auto path = PathBuilder{}
254255
.SetConvexity(Convexity::kConvex)
255-
.AddRoundedRect(rect, corner_radius)
256+
.AddRoundedRect(rect, corner_radii)
256257
.SetBounds(rect)
257258
.TakePath();
258259
if (paint.style == Paint::Style::kFill) {
@@ -318,20 +319,20 @@ void Canvas::ClipRect(const Rect& rect, Entity::ClipOperation clip_op) {
318319
}
319320

320321
void Canvas::ClipRRect(const Rect& rect,
321-
Scalar corner_radius,
322+
Point corner_radii,
322323
Entity::ClipOperation clip_op) {
323324
auto path = PathBuilder{}
324325
.SetConvexity(Convexity::kConvex)
325-
.AddRoundedRect(rect, corner_radius)
326+
.AddRoundedRect(rect, corner_radii)
326327
.SetBounds(rect)
327328
.TakePath();
328329

329330
auto size = rect.GetSize();
330331
// Does the rounded rect have a flat part on the top/bottom or left/right?
331-
bool flat_on_TB = corner_radius * 2 < size.width;
332-
bool flat_on_LR = corner_radius * 2 < size.height;
332+
bool flat_on_TB = corner_radii.x * 2 < size.width;
333+
bool flat_on_LR = corner_radii.y * 2 < size.height;
333334
std::optional<Rect> inner_rect = (flat_on_LR && flat_on_TB)
334-
? rect.Expand(-corner_radius)
335+
? rect.Expand(-corner_radii)
335336
: std::make_optional<Rect>();
336337
auto geometry = Geometry::MakeFillPath(path, inner_rect);
337338
auto& cull_rect = xformation_stack_.back().cull_rect;
@@ -348,7 +349,7 @@ void Canvas::ClipRRect(const Rect& rect,
348349
IntersectCulling(rect);
349350
break;
350351
case Entity::ClipOperation::kDifference:
351-
if (corner_radius <= 0) {
352+
if (corner_radii.x <= 0.0 || corner_radii.y <= 0) {
352353
SubtractCulling(rect);
353354
} else {
354355
// We subtract the inner "tall" and "wide" rectangle pieces
@@ -357,14 +358,10 @@ void Canvas::ClipRRect(const Rect& rect,
357358
// Since this is a subtract operation, we can subtract each
358359
// rectangle piece individually without fear of interference.
359360
if (flat_on_TB) {
360-
SubtractCulling(Rect::MakeLTRB(
361-
rect.GetLeft() + corner_radius, rect.GetTop(),
362-
rect.GetRight() - corner_radius, rect.GetBottom()));
361+
SubtractCulling(rect.Expand({-corner_radii.x, 0.0}));
363362
}
364363
if (flat_on_LR) {
365-
SubtractCulling(Rect::MakeLTRB(
366-
rect.GetLeft(), rect.GetTop() + corner_radius, //
367-
rect.GetRight(), rect.GetBottom() - corner_radius));
364+
SubtractCulling(rect.Expand({0.0, -corner_radii.y}));
368365
}
369366
}
370367
break;

‎impeller/aiks/canvas.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class Canvas {
105105

106106
void DrawRect(Rect rect, const Paint& paint);
107107

108-
void DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint);
108+
void DrawRRect(Rect rect, Point corner_radii, const Paint& paint);
109109

110110
void DrawCircle(Point center, Scalar radius, const Paint& paint);
111111

@@ -135,7 +135,7 @@ class Canvas {
135135

136136
void ClipRRect(
137137
const Rect& rect,
138-
Scalar corner_radius,
138+
Point corner_radii,
139139
Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect);
140140

141141
void DrawPicture(const Picture& picture);

‎impeller/aiks/canvas_recorder.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,9 @@ class CanvasRecorder {
182182
paint);
183183
}
184184

185-
void DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint) {
185+
void DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
186186
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(DrawRRect), rect,
187-
corner_radius, paint);
187+
corner_radii, paint);
188188
}
189189

190190
void DrawCircle(Point center, Scalar radius, const Paint& paint) {
@@ -233,10 +233,10 @@ class CanvasRecorder {
233233

234234
void ClipRRect(
235235
const Rect& rect,
236-
Scalar corner_radius,
236+
Point corner_radii,
237237
Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect) {
238238
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(ClipRRect), rect,
239-
corner_radius, clip_op);
239+
corner_radii, clip_op);
240240
}
241241

242242
void DrawPicture(const Picture& picture) {

‎impeller/aiks/canvas_recorder_unittests.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ TEST(CanvasRecorder, DrawRect) {
160160

161161
TEST(CanvasRecorder, DrawRRect) {
162162
CanvasRecorder<Serializer> recorder;
163-
recorder.DrawRRect(Rect(), 0, Paint());
163+
recorder.DrawRRect(Rect(), {}, Paint());
164164
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::DrawRRect);
165165
}
166166

@@ -202,7 +202,7 @@ TEST(CanvasRecorder, ClipRect) {
202202

203203
TEST(CanvasRecorder, ClipRRect) {
204204
CanvasRecorder<Serializer> recorder;
205-
recorder.ClipRRect({}, 0);
205+
recorder.ClipRRect({}, {});
206206
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::ClipRRect);
207207
}
208208

‎impeller/aiks/canvas_unittests.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ TEST(AiksCanvasTest, RRectClipIntersectAgainstEmptyCullRect) {
170170
Rect rect_clip = Rect::MakeXYWH(5, 5, 10, 10);
171171

172172
Canvas canvas;
173-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kIntersect);
173+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kIntersect);
174174

175175
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
176176
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), rect_clip);
@@ -180,7 +180,7 @@ TEST(AiksCanvasTest, RRectClipDiffAgainstEmptyCullRect) {
180180
Rect rect_clip = Rect::MakeXYWH(5, 5, 10, 10);
181181

182182
Canvas canvas;
183-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
183+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
184184

185185
ASSERT_FALSE(canvas.GetCurrentLocalCullingBounds().has_value());
186186
}
@@ -191,7 +191,7 @@ TEST(AiksCanvasTest, RRectClipIntersectAgainstCullRect) {
191191
Rect result_cull = Rect::MakeXYWH(5, 5, 5, 5);
192192

193193
Canvas canvas(initial_cull);
194-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kIntersect);
194+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kIntersect);
195195

196196
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
197197
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -203,7 +203,7 @@ TEST(AiksCanvasTest, RRectClipDiffAgainstNonCoveredCullRect) {
203203
Rect result_cull = Rect::MakeXYWH(0, 0, 10, 10);
204204

205205
Canvas canvas(initial_cull);
206-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
206+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
207207

208208
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
209209
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -215,7 +215,7 @@ TEST(AiksCanvasTest, RRectClipDiffAgainstVPartiallyCoveredCullRect) {
215215
Rect result_cull = Rect::MakeXYWH(0, 0, 6, 10);
216216

217217
Canvas canvas(initial_cull);
218-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
218+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
219219

220220
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
221221
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -227,7 +227,7 @@ TEST(AiksCanvasTest, RRectClipDiffAgainstVFullyCoveredCullRect) {
227227
Rect result_cull = Rect::MakeXYWH(0, 0, 5, 10);
228228

229229
Canvas canvas(initial_cull);
230-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
230+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
231231

232232
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
233233
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -239,7 +239,7 @@ TEST(AiksCanvasTest, RRectClipDiffAgainstHPartiallyCoveredCullRect) {
239239
Rect result_cull = Rect::MakeXYWH(0, 0, 10, 6);
240240

241241
Canvas canvas(initial_cull);
242-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
242+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
243243

244244
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
245245
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
@@ -251,7 +251,7 @@ TEST(AiksCanvasTest, RRectClipDiffAgainstHFullyCoveredCullRect) {
251251
Rect result_cull = Rect::MakeXYWH(0, 0, 10, 5);
252252

253253
Canvas canvas(initial_cull);
254-
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
254+
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);
255255

256256
ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
257257
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);

‎impeller/display_list/dl_dispatcher.cc‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -726,9 +726,13 @@ void DlDispatcher::clipRect(const SkRect& rect, ClipOp clip_op, bool is_aa) {
726726

727727
// |flutter::DlOpReceiver|
728728
void DlDispatcher::clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) {
729-
if (rrect.isSimple()) {
729+
if (rrect.isRect()) {
730+
canvas_.ClipRect(skia_conversions::ToRect(rrect.rect()),
731+
ToClipOperation(clip_op));
732+
} else if (rrect.isSimple()) {
730733
canvas_.ClipRRect(skia_conversions::ToRect(rrect.rect()),
731-
rrect.getSimpleRadii().fX, ToClipOperation(clip_op));
734+
skia_conversions::ToPoint(rrect.getSimpleRadii()),
735+
ToClipOperation(clip_op));
732736
} else {
733737
canvas_.ClipPath(skia_conversions::ToPath(rrect), ToClipOperation(clip_op));
734738
}
@@ -793,7 +797,8 @@ void DlDispatcher::drawCircle(const SkPoint& center, SkScalar radius) {
793797
void DlDispatcher::drawRRect(const SkRRect& rrect) {
794798
if (rrect.isSimple()) {
795799
canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
796-
rrect.getSimpleRadii().fX, paint_);
800+
skia_conversions::ToPoint(rrect.getSimpleRadii()),
801+
paint_);
797802
} else {
798803
canvas_.DrawPath(skia_conversions::ToPath(rrect), paint_);
799804
}
@@ -809,30 +814,36 @@ void DlDispatcher::drawDRRect(const SkRRect& outer, const SkRRect& inner) {
809814

810815
// |flutter::DlOpReceiver|
811816
void DlDispatcher::drawPath(const SkPath& path) {
817+
SimplifyOrDrawPath(canvas_, path, paint_);
818+
}
819+
820+
void DlDispatcher::SimplifyOrDrawPath(CanvasType& canvas,
821+
const SkPath& path,
822+
const Paint& paint) {
812823
SkRect rect;
813824

814825
// We can't "optimize" a path into a rectangle if it's open.
815826
bool closed;
816827
if (path.isRect(&rect, &closed) && closed) {
817-
canvas_.DrawRect(skia_conversions::ToRect(rect), paint_);
828+
canvas.DrawRect(skia_conversions::ToRect(rect), paint);
818829
return;
819830
}
820831

821832
SkRRect rrect;
822833
if (path.isRRect(&rrect) && rrect.isSimple()) {
823-
canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
824-
rrect.getSimpleRadii().fX, paint_);
834+
canvas.DrawRRect(skia_conversions::ToRect(rrect.rect()),
835+
skia_conversions::ToPoint(rrect.getSimpleRadii()), paint);
825836
return;
826837
}
827838

828839
SkRect oval;
829840
if (path.isOval(&oval) && oval.width() == oval.height()) {
830-
canvas_.DrawCircle(skia_conversions::ToPoint(oval.center()),
831-
oval.width() * 0.5, paint_);
841+
canvas.DrawCircle(skia_conversions::ToPoint(oval.center()),
842+
oval.width() * 0.5, paint);
832843
return;
833844
}
834845

835-
canvas_.DrawPath(skia_conversions::ToPath(path), paint_);
846+
canvas.DrawPath(skia_conversions::ToPath(path), paint);
836847
}
837848

838849
// |flutter::DlOpReceiver|
@@ -1100,20 +1111,7 @@ void DlDispatcher::drawShadow(const SkPath& path,
11001111
canvas_.PreConcat(
11011112
Matrix::MakeTranslation(Vector2(0, -occluder_z * light_position.y)));
11021113

1103-
SkRect rect;
1104-
SkRRect rrect;
1105-
SkRect oval;
1106-
if (path.isRect(&rect)) {
1107-
canvas_.DrawRect(skia_conversions::ToRect(rect), paint);
1108-
} else if (path.isRRect(&rrect) && rrect.isSimple()) {
1109-
canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
1110-
rrect.getSimpleRadii().fX, paint);
1111-
} else if (path.isOval(&oval) && oval.width() == oval.height()) {
1112-
canvas_.DrawCircle(skia_conversions::ToPoint(oval.center()),
1113-
oval.width() * 0.5, paint);
1114-
} else {
1115-
canvas_.DrawPath(skia_conversions::ToPath(path), paint);
1116-
}
1114+
SimplifyOrDrawPath(canvas_, path, paint);
11171115

11181116
canvas_.Restore();
11191117
}

‎impeller/display_list/dl_dispatcher.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ class DlDispatcher final : public flutter::DlOpReceiver {
226226
CanvasType canvas_;
227227
Matrix initial_matrix_;
228228

229+
static void SimplifyOrDrawPath(CanvasType& canvas,
230+
const SkPath& path,
231+
const Paint& paint);
232+
229233
DlDispatcher(const DlDispatcher&) = delete;
230234

231235
DlDispatcher& operator=(const DlDispatcher&) = delete;

0 commit comments

Comments
 (0)