From 31d44f5ea62f2283ea634d9d816d8418479c1210 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 22 Jul 2022 15:32:12 +0800 Subject: [PATCH 1/6] Make ui.Canvas.getDestinationClipBounds works with matrix --- display_list/display_list_builder.cc | 18 ++++---- display_list/display_list_builder.h | 2 +- display_list/display_list_unittests.cc | 62 ++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index fc7a8586f38ee..881edf817709e 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -602,9 +602,7 @@ void DisplayListBuilder::clipRect(const SkRect& rect, switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, rect, is_aa); - if (!current_layer_->clip_bounds.intersect(rect)) { - current_layer_->clip_bounds.setEmpty(); - } + intersect(rect); break; case SkClipOp::kDifference: Push(0, 1, rect, is_aa); @@ -620,9 +618,7 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect, switch (clip_op) { case SkClipOp::kIntersect: Push(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(0, 1, rrect, is_aa); @@ -653,15 +649,19 @@ void DisplayListBuilder::clipPath(const SkPath& path, switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, path, is_aa); - if (!current_layer_->clip_bounds.intersect(path.getBounds())) { - current_layer_->clip_bounds.setEmpty(); - } + intersect(path.getBounds()); break; case SkClipOp::kDifference: Push(0, 1, path, is_aa); 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)) { diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index 152c0eb64af3f..f7065561c01b5 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -211,7 +211,7 @@ class DisplayListBuilder final : public virtual Dispatcher, void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override; void clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) override; - + void intersect(const SkRect& rect); /// Conservative estimate of the bounds of all outstanding clip operations /// measured in the coordinate space within which this DisplayList will /// be rendered. diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 2a6b01d396b6a..7897fb6877b7e 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -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_EQ(builder.getDestinationClipBounds(), SkRect::MakeEmpty()); + 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); @@ -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_EQ(builder.getDestinationClipBounds(), SkRect::MakeEmpty()); + 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); @@ -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_EQ(builder.getDestinationClipBounds(), SkRect::MakeEmpty()); + 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); From 739bdfbaf616f200e5cc7deaac4132f8840c792f Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 22 Jul 2022 16:24:22 +0800 Subject: [PATCH 2/6] Add test to dart --- testing/dart/canvas_test.dart | 67 +++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 4de620f9750cd..f0abab40fe79c 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -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(), Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)); + 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); @@ -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(), Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)); + 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); @@ -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(), Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)); + 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); From 56f9eb1cb3c5536d6e8b535cc5b8777fe9c97346 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 27 Jul 2022 13:05:17 +0800 Subject: [PATCH 3/6] Add logic about clip path with inverse fill type --- display_list/display_list_builder.cc | 7 +++++- display_list/display_list_builder.h | 3 ++- display_list/display_list_unittests.cc | 31 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 881edf817709e..a7b9e6edf0a95 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -649,10 +649,15 @@ void DisplayListBuilder::clipPath(const SkPath& path, switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, path, is_aa); - intersect(path.getBounds()); + if (!path.isInverseFillType()) { + intersect(path.getBounds()); + } break; case SkClipOp::kDifference: Push(0, 1, path, is_aa); + if (path.isInverseFillType()) { + intersect(path.getBounds()); + } break; } } diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index f7065561c01b5..ce785c594b7ad 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -211,7 +211,7 @@ class DisplayListBuilder final : public virtual Dispatcher, void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override; void clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) override; - void intersect(const SkRect& rect); + /// Conservative estimate of the bounds of all outstanding clip operations /// measured in the coordinate space within which this DisplayList will /// be rendered. @@ -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; diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 7897fb6877b7e..b3099e2a5308e 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -10,9 +10,12 @@ #include "flutter/display_list/display_list_canvas_recorder.h" #include "flutter/display_list/display_list_rtree.h" #include "flutter/display_list/display_list_utils.h" +#include "flutter/fml/logging.h" #include "flutter/fml/math.h" #include "flutter/testing/display_list_testing.h" #include "flutter/testing/testing.h" +#include "include/core/SkPathTypes.h" +#include "include/core/SkRect.h" #include "third_party/skia/include/core/SkPictureRecorder.h" #include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/effects/SkBlenders.h" @@ -2314,6 +2317,34 @@ 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); + + SkRect initial_local_bounds = builder.getLocalClipBounds(); + SkRect initial_destination_bounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initial_local_bounds, cull_rect); + ASSERT_EQ(initial_destination_bounds, 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); + + SkRect initial_local_bounds = builder.getLocalClipBounds(); + SkRect initial_destination_bounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_destination_bounds, clip_bounds); +} + TEST(DisplayList, FlatDrawPointsProducesBounds) { SkPoint horizontal_points[2] = {{10, 10}, {20, 10}}; SkPoint vertical_points[2] = {{10, 10}, {10, 20}}; From 230f9d6d291f957708bc9d02a8ecdb40629cfc53 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 27 Jul 2022 13:14:49 +0800 Subject: [PATCH 4/6] Clean code --- display_list/display_list_unittests.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index b3099e2a5308e..804cc1986980d 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -10,12 +10,9 @@ #include "flutter/display_list/display_list_canvas_recorder.h" #include "flutter/display_list/display_list_rtree.h" #include "flutter/display_list/display_list_utils.h" -#include "flutter/fml/logging.h" #include "flutter/fml/math.h" #include "flutter/testing/display_list_testing.h" #include "flutter/testing/testing.h" -#include "include/core/SkPathTypes.h" -#include "include/core/SkRect.h" #include "third_party/skia/include/core/SkPictureRecorder.h" #include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/effects/SkBlenders.h" From e0b1a0a2b81c0974dd7092339b4f84e60856f4e1 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Wed, 27 Jul 2022 14:22:14 +0800 Subject: [PATCH 5/6] Clean code --- display_list/display_list_unittests.cc | 18 +++++++----------- testing/dart/canvas_test.dart | 6 +++--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 804cc1986980d..a3db273606562 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -2124,7 +2124,7 @@ TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { builder.clipRect(clipBounds1, SkClipOp::kIntersect, false); builder.translate(10, 0); builder.clipRect(clipBounds1, SkClipOp::kIntersect, false); - ASSERT_EQ(builder.getDestinationClipBounds(), SkRect::MakeEmpty()); + ASSERT_TRUE(builder.getDestinationClipBounds().isEmpty()); builder.restore(); builder.save(); @@ -2186,7 +2186,7 @@ TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) { builder.clipRRect(clip1, SkClipOp::kIntersect, false); builder.translate(10, 0); builder.clipRRect(clip1, SkClipOp::kIntersect, false); - ASSERT_EQ(builder.getDestinationClipBounds(), SkRect::MakeEmpty()); + ASSERT_TRUE(builder.getDestinationClipBounds().isEmpty()); builder.restore(); builder.save(); @@ -2247,7 +2247,7 @@ TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) { builder.clipPath(clip1, SkClipOp::kIntersect, false); builder.translate(10, 0); builder.clipPath(clip1, SkClipOp::kIntersect, false); - ASSERT_EQ(builder.getDestinationClipBounds(), SkRect::MakeEmpty()); + ASSERT_TRUE(builder.getDestinationClipBounds().isEmpty()); builder.restore(); builder.save(); @@ -2321,10 +2321,8 @@ TEST(DisplayList, ClipPathWithInvertFillTypeDoesNotAffectClipBounds) { clip.setFillType(SkPathFillType::kInverseWinding); builder.clipPath(clip, SkClipOp::kIntersect, false); - SkRect initial_local_bounds = builder.getLocalClipBounds(); - SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, cull_rect); - ASSERT_EQ(initial_destination_bounds, cull_rect); + ASSERT_EQ(builder.getLocalClipBounds(), cull_rect); + ASSERT_EQ(builder.getDestinationClipBounds(), cull_rect); } TEST(DisplayList, DiffClipPathWithInvertFillTypeAffectsClipBounds) { @@ -2336,10 +2334,8 @@ TEST(DisplayList, DiffClipPathWithInvertFillTypeAffectsClipBounds) { SkRect clip_expanded_bounds = SkRect::MakeLTRB(8, 9, 23, 28); builder.clipPath(clip, SkClipOp::kDifference, false); - SkRect initial_local_bounds = builder.getLocalClipBounds(); - SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); - ASSERT_EQ(initial_destination_bounds, clip_bounds); + ASSERT_EQ(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds); } TEST(DisplayList, FlatDrawPointsProducesBounds) { diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index f0abab40fe79c..0e3b880c277bb 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -741,7 +741,7 @@ void main() { canvas.clipRect(clipBounds1); canvas.translate(0, 10.0); canvas.clipRect(clipBounds1); - expect(canvas.getDestinationClipBounds(), Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)); + expect(canvas.getDestinationClipBounds().isEmpty, isTrue); canvas.restore(); canvas.save(); @@ -805,7 +805,7 @@ void main() { canvas.clipRRect(clip1); canvas.translate(0, 10.0); canvas.clipRRect(clip1); - expect(canvas.getDestinationClipBounds(), Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)); + expect(canvas.getDestinationClipBounds().isEmpty, isTrue); canvas.restore(); canvas.save(); @@ -869,7 +869,7 @@ void main() { canvas.clipPath(clip1); canvas.translate(0, 10.0); canvas.clipPath(clip1); - expect(canvas.getDestinationClipBounds(), Rect.fromLTRB(0.0, 0.0, 0.0, 0.0)); + expect(canvas.getDestinationClipBounds().isEmpty, isTrue); canvas.restore(); canvas.save(); From cab83b1f8f7073d6ffc75a5220fe458ef674e491 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Thu, 28 Jul 2022 10:31:47 +0800 Subject: [PATCH 6/6] Add comment --- display_list/display_list_builder.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index a7b9e6edf0a95..970e019ff56bf 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -655,6 +655,7 @@ void DisplayListBuilder::clipPath(const SkPath& path, break; case SkClipOp::kDifference: Push(0, 1, path, is_aa); + // Map "kDifference of inverse path" to "kIntersect of the original path". if (path.isInverseFillType()) { intersect(path.getBounds()); }