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

Make ui.Canvas.getDestinationClipBounds works with matrix #34835

Merged
merged 8 commits into from
Jul 28, 2022
Merged

Make ui.Canvas.getDestinationClipBounds works with matrix #34835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
31d44f5
Make ui.Canvas.getDestinationClipBounds works with matrix
ColdPaleLight Jul 22, 2022
739bdfb
Add test to dart
ColdPaleLight Jul 22, 2022
56f9eb1
Add logic about clip path with inverse fill type
ColdPaleLight Jul 27, 2022
230f9d6
Clean code
ColdPaleLight Jul 27, 2022
fb1a872
Merge branch 'flutter:main' into clip_with_matrix
ColdPaleLight Jul 27, 2022
e0b1a0a
Clean code
ColdPaleLight Jul 27, 2022
1869882
Merge branch 'clip_with_matrix' of https://github.com/ColdPaleLight/e…
ColdPaleLight Jul 27, 2022
cab83b1
Add comment
ColdPaleLight Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions display_list/display_list_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,9 +602,7 @@ void DisplayListBuilder::clipRect(const SkRect& rect,
switch (clip_op) {
case SkClipOp::kIntersect:
Push<ClipIntersectRectOp>(0, 1, rect, is_aa);
if (!current_layer_->clip_bounds.intersect(rect)) {
current_layer_->clip_bounds.setEmpty();
}
intersect(rect);
break;
case SkClipOp::kDifference:
Push<ClipDifferenceRectOp>(0, 1, rect, is_aa);
Expand All @@ -620,9 +618,7 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect,
switch (clip_op) {
case SkClipOp::kIntersect:
Push<ClipIntersectRRectOp>(0, 1, rrect, is_aa);
if (!current_layer_->clip_bounds.intersect(rrect.getBounds())) {
current_layer_->clip_bounds.setEmpty();
}
intersect(rrect.getBounds());
break;
case SkClipOp::kDifference:
Push<ClipDifferenceRRectOp>(0, 1, rrect, is_aa);
Expand Down Expand Up @@ -653,15 +649,25 @@ void DisplayListBuilder::clipPath(const SkPath& path,
switch (clip_op) {
case SkClipOp::kIntersect:
Push<ClipIntersectPathOp>(0, 1, path, is_aa);
if (!current_layer_->clip_bounds.intersect(path.getBounds())) {
current_layer_->clip_bounds.setEmpty();
if (!path.isInverseFillType()) {
intersect(path.getBounds());
}
break;
case SkClipOp::kDifference:
Push<ClipDifferencePathOp>(0, 1, path, is_aa);
// Map "kDifference of inverse path" to "kIntersect of the original path".
if (path.isInverseFillType()) {
Copy link
Contributor

@flar flar Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a reminder comment here about how this works. Nothing too detailed, just reminding the reader (i.e. me?) that inverted difference is like intersection...

Copy link
Member Author

@ColdPaleLight ColdPaleLight Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

intersect(path.getBounds());
}
break;
}
}
void DisplayListBuilder::intersect(const SkRect& rect) {
SkRect devClipBounds = getTransform().mapRect(rect);
if (!current_layer_->clip_bounds.intersect(devClipBounds)) {
current_layer_->clip_bounds.setEmpty();
}
}
SkRect DisplayListBuilder::getLocalClipBounds() {
SkM44 inverse;
if (current_layer_->matrix.invert(&inverse)) {
Expand Down
1 change: 1 addition & 0 deletions display_list/display_list_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ class DisplayListBuilder final : public virtual Dispatcher,

void setAttributesFromDlPaint(const DlPaint& paint,
const DisplayListAttributeFlags flags);
void intersect(const SkRect& rect);

// kInvalidSigma is used to indicate that no MaskBlur is currently set.
static constexpr SkScalar kInvalidSigma = 0.0;
Expand Down
86 changes: 86 additions & 0 deletions display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2116,6 +2116,25 @@ TEST(DisplayList, ClipRectAffectsClipBounds) {
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
}

TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) {
DisplayListBuilder builder;
SkRect clipBounds1 = SkRect::MakeLTRB(0, 0, 10, 10);
SkRect clipBounds2 = SkRect::MakeLTRB(10, 10, 20, 20);
builder.save();
builder.clipRect(clipBounds1, SkClipOp::kIntersect, false);
builder.translate(10, 0);
builder.clipRect(clipBounds1, SkClipOp::kIntersect, false);
ASSERT_TRUE(builder.getDestinationClipBounds().isEmpty());
builder.restore();

builder.save();
builder.clipRect(clipBounds1, SkClipOp::kIntersect, false);
builder.translate(-10, -10);
builder.clipRect(clipBounds2, SkClipOp::kIntersect, false);
ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds1);
builder.restore();
}

TEST(DisplayList, ClipRRectAffectsClipBounds) {
DisplayListBuilder builder;
SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7);
Expand Down Expand Up @@ -2156,6 +2175,28 @@ TEST(DisplayList, ClipRRectAffectsClipBounds) {
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
}

TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) {
DisplayListBuilder builder;
SkRect clipBounds1 = SkRect::MakeLTRB(0, 0, 10, 10);
SkRect clipBounds2 = SkRect::MakeLTRB(10, 10, 20, 20);
SkRRect clip1 = SkRRect::MakeRectXY(clipBounds1, 3, 2);
SkRRect clip2 = SkRRect::MakeRectXY(clipBounds2, 3, 2);

builder.save();
builder.clipRRect(clip1, SkClipOp::kIntersect, false);
builder.translate(10, 0);
builder.clipRRect(clip1, SkClipOp::kIntersect, false);
ASSERT_TRUE(builder.getDestinationClipBounds().isEmpty());
builder.restore();

builder.save();
builder.clipRRect(clip1, SkClipOp::kIntersect, false);
builder.translate(-10, -10);
builder.clipRRect(clip2, SkClipOp::kIntersect, false);
ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds1);
builder.restore();
}

TEST(DisplayList, ClipPathAffectsClipBounds) {
DisplayListBuilder builder;
SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2);
Expand Down Expand Up @@ -2196,6 +2237,27 @@ TEST(DisplayList, ClipPathAffectsClipBounds) {
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
}

TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) {
DisplayListBuilder builder;
SkRect clipBounds = SkRect::MakeLTRB(0, 0, 10, 10);
SkPath clip1 = SkPath().addCircle(2.5, 2.5, 2.5).addCircle(7.5, 7.5, 2.5);
SkPath clip2 = SkPath().addCircle(12.5, 12.5, 2.5).addCircle(17.5, 17.5, 2.5);

builder.save();
builder.clipPath(clip1, SkClipOp::kIntersect, false);
builder.translate(10, 0);
builder.clipPath(clip1, SkClipOp::kIntersect, false);
ASSERT_TRUE(builder.getDestinationClipBounds().isEmpty());
builder.restore();

builder.save();
builder.clipPath(clip1, SkClipOp::kIntersect, false);
builder.translate(-10, -10);
builder.clipPath(clip2, SkClipOp::kIntersect, false);
ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds);
builder.restore();
}

TEST(DisplayList, DiffClipRectDoesNotAffectClipBounds) {
DisplayListBuilder builder;
SkRect diff_clip = SkRect::MakeLTRB(0, 0, 15, 15);
Expand Down Expand Up @@ -2252,6 +2314,30 @@ TEST(DisplayList, DiffClipPathDoesNotAffectClipBounds) {
ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds);
}

TEST(DisplayList, ClipPathWithInvertFillTypeDoesNotAffectClipBounds) {
SkRect cull_rect = SkRect::MakeLTRB(0, 0, 100.0, 100.0);
DisplayListBuilder builder(cull_rect);
SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2);
clip.setFillType(SkPathFillType::kInverseWinding);
builder.clipPath(clip, SkClipOp::kIntersect, false);

ASSERT_EQ(builder.getLocalClipBounds(), cull_rect);
ASSERT_EQ(builder.getDestinationClipBounds(), cull_rect);
}

TEST(DisplayList, DiffClipPathWithInvertFillTypeAffectsClipBounds) {
SkRect cull_rect = SkRect::MakeLTRB(0, 0, 100.0, 100.0);
DisplayListBuilder builder(cull_rect);
SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2);
clip.setFillType(SkPathFillType::kInverseWinding);
SkRect clip_bounds = SkRect::MakeLTRB(8.2, 9.3, 22.4, 27.7);
SkRect clip_expanded_bounds = SkRect::MakeLTRB(8, 9, 23, 28);
builder.clipPath(clip, SkClipOp::kDifference, false);

ASSERT_EQ(builder.getLocalClipBounds(), clip_expanded_bounds);
ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
}

TEST(DisplayList, FlatDrawPointsProducesBounds) {
SkPoint horizontal_points[2] = {{10, 10}, {20, 10}};
SkPoint vertical_points[2] = {{10, 10}, {10, 20}};
Expand Down
67 changes: 67 additions & 0 deletions testing/dart/canvas_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,27 @@ void main() {
expect(canvas.getDestinationClipBounds(), initialDestinationBounds);
});

test('Canvas.clipRect with matrix affects canvas.getClipBounds', () async {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
const Rect clipBounds1 = Rect.fromLTRB(0.0, 0.0, 10.0, 10.0);
const Rect clipBounds2 = Rect.fromLTRB(10.0, 10.0, 20.0, 20.0);

canvas.save();
canvas.clipRect(clipBounds1);
canvas.translate(0, 10.0);
canvas.clipRect(clipBounds1);
expect(canvas.getDestinationClipBounds().isEmpty, isTrue);
canvas.restore();

canvas.save();
canvas.clipRect(clipBounds1);
canvas.translate(-10.0, -10.0);
canvas.clipRect(clipBounds2);
expect(canvas.getDestinationClipBounds(), clipBounds1);
canvas.restore();
});

test('Canvas.clipRRect affects canvas.getClipBounds', () async {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
Expand Down Expand Up @@ -772,6 +793,29 @@ void main() {
expect(canvas.getDestinationClipBounds(), initialDestinationBounds);
});

test('Canvas.clipRRect with matrix affects canvas.getClipBounds', () async {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
const Rect clipBounds1 = Rect.fromLTRB(0.0, 0.0, 10.0, 10.0);
const Rect clipBounds2 = Rect.fromLTRB(10.0, 10.0, 20.0, 20.0);
final RRect clip1 = RRect.fromRectAndRadius(clipBounds1, const Radius.circular(3));
final RRect clip2 = RRect.fromRectAndRadius(clipBounds2, const Radius.circular(3));

canvas.save();
canvas.clipRRect(clip1);
canvas.translate(0, 10.0);
canvas.clipRRect(clip1);
expect(canvas.getDestinationClipBounds().isEmpty, isTrue);
canvas.restore();

canvas.save();
canvas.clipRRect(clip1);
canvas.translate(-10.0, -10.0);
canvas.clipRRect(clip2);
expect(canvas.getDestinationClipBounds(), clipBounds1);
canvas.restore();
});

test('Canvas.clipPath affects canvas.getClipBounds', () async {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
Expand Down Expand Up @@ -813,6 +857,29 @@ void main() {
expect(canvas.getDestinationClipBounds(), initialDestinationBounds);
});

test('Canvas.clipPath with matrix affects canvas.getClipBounds', () async {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
const Rect clipBounds1 = Rect.fromLTRB(0.0, 0.0, 10.0, 10.0);
const Rect clipBounds2 = Rect.fromLTRB(10.0, 10.0, 20.0, 20.0);
final Path clip1 = Path()..addRect(clipBounds1)..addOval(clipBounds1);
final Path clip2 = Path()..addRect(clipBounds2)..addOval(clipBounds2);

canvas.save();
canvas.clipPath(clip1);
canvas.translate(0, 10.0);
canvas.clipPath(clip1);
expect(canvas.getDestinationClipBounds().isEmpty, isTrue);
canvas.restore();

canvas.save();
canvas.clipPath(clip1);
canvas.translate(-10.0, -10.0);
canvas.clipPath(clip2);
expect(canvas.getDestinationClipBounds(), clipBounds1);
canvas.restore();
});

test('Canvas.clipRect(diff) does not affect canvas.getClipBounds', () async {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
Expand Down