Skip to content

Commit

Permalink
[Impeller] add support for Skia concept of RRect::isSimple needed for…
Browse files Browse the repository at this point in the history
… DL dispatching (flutter#47736)

Fixes flutter/flutter#133793
  • Loading branch information
flar authored Nov 8, 2023
1 parent 5306213 commit 2366ce6
Show file tree
Hide file tree
Showing 11 changed files with 143 additions and 62 deletions.
14 changes: 7 additions & 7 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2407,9 +2407,9 @@ TEST_P(AiksTest, DrawRectAbsorbsClears) {

TEST_P(AiksTest, DrawRectAbsorbsClearsNegativeRRect) {
Canvas canvas;
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), 5.0,
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), {5.0, 5.0},
{.color = Color::Red(), .blend_mode = BlendMode::kSource});
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), 5.0,
canvas.DrawRRect(Rect::MakeXYWH(0, 0, 300, 300), {5.0, 5.0},
{.color = Color::CornflowerBlue().WithAlpha(0.75),
.blend_mode = BlendMode::kSourceOver});

Expand Down Expand Up @@ -3077,7 +3077,7 @@ TEST_P(AiksTest, CanRenderBackdropBlurInteractive) {
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);
canvas.ClipRRect(Rect::MakeLTRB(a.x, a.y, b.x, b.y), {20, 20});
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
ImageFilter::MakeBlur(Sigma(20.0), Sigma(20.0),
FilterContents::BlurStyle::kNormal,
Expand All @@ -3096,7 +3096,7 @@ TEST_P(AiksTest, CanRenderBackdropBlur) {
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);
canvas.ClipRRect(Rect::MakeLTRB(75, 50, 375, 275), {20, 20});
canvas.SaveLayer({.blend_mode = BlendMode::kSource}, std::nullopt,
ImageFilter::MakeBlur(Sigma(30.0), Sigma(30.0),
FilterContents::BlurStyle::kNormal,
Expand Down Expand Up @@ -3202,7 +3202,7 @@ TEST_P(AiksTest, CanRenderClippedRuntimeEffects) {

Canvas canvas;
canvas.Save();
canvas.ClipRRect(Rect::MakeXYWH(0, 0, 400, 400), 10.0,
canvas.ClipRRect(Rect::MakeXYWH(0, 0, 400, 400), {10.0, 10.0},
Entity::ClipOperation::kIntersect);
canvas.DrawRect(Rect::MakeXYWH(0, 0, 400, 400), paint);
canvas.Restore();
Expand Down Expand Up @@ -3480,7 +3480,7 @@ TEST_P(AiksTest, DrawPictureWithText) {

TEST_P(AiksTest, DrawPictureClipped) {
Canvas subcanvas;
subcanvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400), 15);
subcanvas.ClipRRect(Rect::MakeLTRB(100, 100, 400, 400), {15, 15});
subcanvas.DrawPaint({.color = Color::Red()});
auto picture = subcanvas.EndRecordingAsPicture();

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

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

ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
Expand Down
27 changes: 12 additions & 15 deletions impeller/aiks/canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,14 @@ void Canvas::DrawRect(Rect rect, const Paint& paint) {
GetCurrentPass().AddEntity(entity);
}

void Canvas::DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint) {
if (AttemptDrawBlurredRRect(rect, corner_radius, paint)) {
void Canvas::DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
if (corner_radii.x == corner_radii.y &&
AttemptDrawBlurredRRect(rect, corner_radii.x, paint)) {
return;
}
auto path = PathBuilder{}
.SetConvexity(Convexity::kConvex)
.AddRoundedRect(rect, corner_radius)
.AddRoundedRect(rect, corner_radii)
.SetBounds(rect)
.TakePath();
if (paint.style == Paint::Style::kFill) {
Expand Down Expand Up @@ -318,20 +319,20 @@ void Canvas::ClipRect(const Rect& rect, Entity::ClipOperation clip_op) {
}

void Canvas::ClipRRect(const Rect& rect,
Scalar corner_radius,
Point corner_radii,
Entity::ClipOperation clip_op) {
auto path = PathBuilder{}
.SetConvexity(Convexity::kConvex)
.AddRoundedRect(rect, corner_radius)
.AddRoundedRect(rect, corner_radii)
.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_radius * 2 < size.width;
bool flat_on_LR = corner_radius * 2 < size.height;
bool flat_on_TB = corner_radii.x * 2 < size.width;
bool flat_on_LR = corner_radii.y * 2 < size.height;
std::optional<Rect> inner_rect = (flat_on_LR && flat_on_TB)
? rect.Expand(-corner_radius)
? rect.Expand(-corner_radii)
: std::make_optional<Rect>();
auto geometry = Geometry::MakeFillPath(path, inner_rect);
auto& cull_rect = xformation_stack_.back().cull_rect;
Expand All @@ -348,7 +349,7 @@ void Canvas::ClipRRect(const Rect& rect,
IntersectCulling(rect);
break;
case Entity::ClipOperation::kDifference:
if (corner_radius <= 0) {
if (corner_radii.x <= 0.0 || corner_radii.y <= 0) {
SubtractCulling(rect);
} else {
// We subtract the inner "tall" and "wide" rectangle pieces
Expand All @@ -357,14 +358,10 @@ void Canvas::ClipRRect(const Rect& rect,
// Since this is a subtract operation, we can subtract each
// rectangle piece individually without fear of interference.
if (flat_on_TB) {
SubtractCulling(Rect::MakeLTRB(
rect.GetLeft() + corner_radius, rect.GetTop(),
rect.GetRight() - corner_radius, rect.GetBottom()));
SubtractCulling(rect.Expand({-corner_radii.x, 0.0}));
}
if (flat_on_LR) {
SubtractCulling(Rect::MakeLTRB(
rect.GetLeft(), rect.GetTop() + corner_radius, //
rect.GetRight(), rect.GetBottom() - corner_radius));
SubtractCulling(rect.Expand({0.0, -corner_radii.y}));
}
}
break;
Expand Down
4 changes: 2 additions & 2 deletions impeller/aiks/canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Canvas {

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

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

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

Expand Down Expand Up @@ -135,7 +135,7 @@ class Canvas {

void ClipRRect(
const Rect& rect,
Scalar corner_radius,
Point corner_radii,
Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect);

void DrawPicture(const Picture& picture);
Expand Down
8 changes: 4 additions & 4 deletions impeller/aiks/canvas_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,9 @@ class CanvasRecorder {
paint);
}

void DrawRRect(Rect rect, Scalar corner_radius, const Paint& paint) {
void DrawRRect(Rect rect, Point corner_radii, const Paint& paint) {
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(DrawRRect), rect,
corner_radius, paint);
corner_radii, paint);
}

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

void ClipRRect(
const Rect& rect,
Scalar corner_radius,
Point corner_radii,
Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect) {
return ExecuteAndSerialize(FLT_CANVAS_RECORDER_OP_ARG(ClipRRect), rect,
corner_radius, clip_op);
corner_radii, clip_op);
}

void DrawPicture(const Picture& picture) {
Expand Down
4 changes: 2 additions & 2 deletions impeller/aiks/canvas_recorder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ TEST(CanvasRecorder, DrawRect) {

TEST(CanvasRecorder, DrawRRect) {
CanvasRecorder<Serializer> recorder;
recorder.DrawRRect(Rect(), 0, Paint());
recorder.DrawRRect(Rect(), {}, Paint());
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::DrawRRect);
}

Expand Down Expand Up @@ -202,7 +202,7 @@ TEST(CanvasRecorder, ClipRect) {

TEST(CanvasRecorder, ClipRRect) {
CanvasRecorder<Serializer> recorder;
recorder.ClipRRect({}, 0);
recorder.ClipRRect({}, {});
ASSERT_EQ(recorder.GetSerializer().last_op_, CanvasRecorderOp::ClipRRect);
}

Expand Down
16 changes: 8 additions & 8 deletions impeller/aiks/canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ TEST(AiksCanvasTest, RRectClipIntersectAgainstEmptyCullRect) {
Rect rect_clip = Rect::MakeXYWH(5, 5, 10, 10);

Canvas canvas;
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kIntersect);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kIntersect);

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

Canvas canvas;
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);

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

Canvas canvas(initial_cull);
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kIntersect);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kIntersect);

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

Canvas canvas(initial_cull);
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);

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

Canvas canvas(initial_cull);
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);

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

Canvas canvas(initial_cull);
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);

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

Canvas canvas(initial_cull);
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);

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

Canvas canvas(initial_cull);
canvas.ClipRRect(rect_clip, 1, Entity::ClipOperation::kDifference);
canvas.ClipRRect(rect_clip, {1, 1}, Entity::ClipOperation::kDifference);

ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value());
ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull);
Expand Down
44 changes: 21 additions & 23 deletions impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -726,9 +726,13 @@ void DlDispatcher::clipRect(const SkRect& rect, ClipOp clip_op, bool is_aa) {

// |flutter::DlOpReceiver|
void DlDispatcher::clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) {
if (rrect.isSimple()) {
if (rrect.isRect()) {
canvas_.ClipRect(skia_conversions::ToRect(rrect.rect()),
ToClipOperation(clip_op));
} else if (rrect.isSimple()) {
canvas_.ClipRRect(skia_conversions::ToRect(rrect.rect()),
rrect.getSimpleRadii().fX, ToClipOperation(clip_op));
skia_conversions::ToPoint(rrect.getSimpleRadii()),
ToClipOperation(clip_op));
} else {
canvas_.ClipPath(skia_conversions::ToPath(rrect), ToClipOperation(clip_op));
}
Expand Down Expand Up @@ -793,7 +797,8 @@ void DlDispatcher::drawCircle(const SkPoint& center, SkScalar radius) {
void DlDispatcher::drawRRect(const SkRRect& rrect) {
if (rrect.isSimple()) {
canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
rrect.getSimpleRadii().fX, paint_);
skia_conversions::ToPoint(rrect.getSimpleRadii()),
paint_);
} else {
canvas_.DrawPath(skia_conversions::ToPath(rrect), paint_);
}
Expand All @@ -809,30 +814,36 @@ void DlDispatcher::drawDRRect(const SkRRect& outer, const SkRRect& inner) {

// |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()),
rrect.getSimpleRadii().fX, paint_);
canvas.DrawRRect(skia_conversions::ToRect(rrect.rect()),
skia_conversions::ToPoint(rrect.getSimpleRadii()), 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|
Expand Down Expand Up @@ -1100,20 +1111,7 @@ void DlDispatcher::drawShadow(const SkPath& path,
canvas_.PreConcat(
Matrix::MakeTranslation(Vector2(0, -occluder_z * light_position.y)));

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);
}
SimplifyOrDrawPath(canvas_, path, paint);

canvas_.Restore();
}
Expand Down
4 changes: 4 additions & 0 deletions impeller/display_list/dl_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ class DlDispatcher final : public flutter::DlOpReceiver {
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;
Expand Down
Loading

0 comments on commit 2366ce6

Please sign in to comment.