make sure CanvasRecorder updates clip bounds methods (flutter#38325)

loic-sharma · Dec 15, 2022 · 353d694 · 353d694
1 parent 61e95ba
commit 353d694
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 3 deletions.
diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc
@@ -74,7 +74,9 @@ DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect,
     accumulator_ = std::make_unique<RectBoundsAccumulator>();
   }
 
-  layer_stack_.emplace_back(SkM44(), SkMatrix::I(), cull_rect);
+  // isEmpty protects us against NaN as we normalize any empty cull rects
+  SkRect cull = cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect;
+  layer_stack_.emplace_back(SkM44(), SkMatrix::I(), cull);
   current_layer_ = &layer_stack_.back();
 }
 

diff --git a/display_list/display_list_canvas_recorder.cc b/display_list/display_list_canvas_recorder.cc
@@ -21,8 +21,12 @@ namespace flutter {
 
 DisplayListCanvasRecorder::DisplayListCanvasRecorder(const SkRect& bounds,
                                                      bool prepare_rtree)
-    : SkCanvasVirtualEnforcer(bounds.width(), bounds.height()),
-      builder_(sk_make_sp<DisplayListBuilder>(bounds, prepare_rtree)) {}
+    : SkCanvasVirtualEnforcer(0, 0),
+      builder_(sk_make_sp<DisplayListBuilder>(bounds, prepare_rtree)) {
+  // isEmpty protects us against NaN as we normalize any empty cull rects
+  SkIRect cull = bounds.isEmpty() ? SkIRect::MakeEmpty() : bounds.roundOut();
+  SkCanvasVirtualEnforcer::resetCanvas(cull);
+}
 
 sk_sp<DisplayList> DisplayListCanvasRecorder::Build() {
   CHECK_DISPOSE(nullptr);
@@ -57,20 +61,23 @@ void DisplayListCanvasRecorder::onClipRect(const SkRect& rect,
   CHECK_DISPOSE();
   builder_->clipRect(rect, clip_op,
                      edge_style == ClipEdgeStyle::kSoft_ClipEdgeStyle);
+  SkCanvasVirtualEnforcer::onClipRect(rect, clip_op, edge_style);
 }
 void DisplayListCanvasRecorder::onClipRRect(const SkRRect& rrect,
                                             SkClipOp clip_op,
                                             ClipEdgeStyle edge_style) {
   CHECK_DISPOSE();
   builder_->clipRRect(rrect, clip_op,
                       edge_style == ClipEdgeStyle::kSoft_ClipEdgeStyle);
+  SkCanvasVirtualEnforcer::onClipRRect(rrect, clip_op, edge_style);
 }
 void DisplayListCanvasRecorder::onClipPath(const SkPath& path,
                                            SkClipOp clip_op,
                                            ClipEdgeStyle edge_style) {
   CHECK_DISPOSE();
   builder_->clipPath(path, clip_op,
                      edge_style == ClipEdgeStyle::kSoft_ClipEdgeStyle);
+  SkCanvasVirtualEnforcer::onClipPath(path, clip_op, edge_style);
 }
 
 void DisplayListCanvasRecorder::willSave() {

diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc
@@ -38,6 +38,112 @@ TEST(DisplayList, CallMethodAfterBuild) {
 }
 #endif  // NDEBUG
 
+TEST(DisplayList, RecorderInitialClipBounds) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkIRect clip_bounds = SkIRect::MakeWH(100, 100);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  SkCanvas* canvas = &recorder;
+  ASSERT_EQ(canvas->getDeviceClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, RecorderInitialClipBoundsNaN) {
+  SkRect cull_rect = SkRect::MakeWH(SK_ScalarNaN, SK_ScalarNaN);
+  SkIRect clip_bounds = SkIRect::MakeEmpty();
+  DisplayListCanvasRecorder recorder(cull_rect);
+  SkCanvas* canvas = &recorder;
+  ASSERT_EQ(canvas->getDeviceClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, RecorderClipBoundsAfterClipRect) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkRect clip_rect = SkRect::MakeLTRB(10, 10, 20, 20);
+  SkIRect clip_bounds = SkIRect::MakeLTRB(10, 10, 20, 20);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  SkCanvas* canvas = &recorder;
+  canvas->clipRect(clip_rect);
+  ASSERT_EQ(canvas->getDeviceClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, RecorderClipBoundsAfterClipRRect) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkRect clip_rect = SkRect::MakeLTRB(10, 10, 20, 20);
+  SkRRect clip_rrect = SkRRect::MakeRectXY(clip_rect, 2, 2);
+  SkIRect clip_bounds = SkIRect::MakeLTRB(10, 10, 20, 20);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  SkCanvas* canvas = &recorder;
+  canvas->clipRRect(clip_rrect);
+  ASSERT_EQ(canvas->getDeviceClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, RecorderClipBoundsAfterClipPath) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkPath clip_path = SkPath().addRect(10, 10, 15, 15).addRect(15, 15, 20, 20);
+  SkIRect clip_bounds = SkIRect::MakeLTRB(10, 10, 20, 20);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  SkCanvas* canvas = &recorder;
+  canvas->clipPath(clip_path);
+  ASSERT_EQ(canvas->getDeviceClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, RecorderInitialClipBoundsNonZero) {
+  SkRect cull_rect = SkRect::MakeLTRB(10, 10, 100, 100);
+  SkIRect clip_bounds = SkIRect::MakeLTRB(10, 10, 100, 100);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  SkCanvas* canvas = &recorder;
+  ASSERT_EQ(canvas->getDeviceClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, BuilderInitialClipBounds) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkRect clip_bounds = SkRect::MakeWH(100, 100);
+  DisplayListBuilder builder(cull_rect);
+  ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, BuilderInitialClipBoundsNaN) {
+  SkRect cull_rect = SkRect::MakeWH(SK_ScalarNaN, SK_ScalarNaN);
+  SkRect clip_bounds = SkRect::MakeEmpty();
+  DisplayListBuilder builder(cull_rect);
+  ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, BuilderClipBoundsAfterClipRect) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkRect clip_rect = SkRect::MakeLTRB(10, 10, 20, 20);
+  SkRect clip_bounds = SkRect::MakeLTRB(10, 10, 20, 20);
+  DisplayListBuilder builder(cull_rect);
+  builder.clipRect(clip_rect, SkClipOp::kIntersect, false);
+  ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, BuilderClipBoundsAfterClipRRect) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkRect clip_rect = SkRect::MakeLTRB(10, 10, 20, 20);
+  SkRRect clip_rrect = SkRRect::MakeRectXY(clip_rect, 2, 2);
+  SkRect clip_bounds = SkRect::MakeLTRB(10, 10, 20, 20);
+  DisplayListBuilder builder(cull_rect);
+  builder.clipRRect(clip_rrect, SkClipOp::kIntersect, false);
+  ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, BuilderClipBoundsAfterClipPath) {
+  SkRect cull_rect = SkRect::MakeWH(100, 100);
+  SkPath clip_path = SkPath().addRect(10, 10, 15, 15).addRect(15, 15, 20, 20);
+  SkRect clip_bounds = SkRect::MakeLTRB(10, 10, 20, 20);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  DisplayListBuilder builder(cull_rect);
+  builder.clipPath(clip_path, SkClipOp::kIntersect, false);
+  ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
+}
+
+TEST(DisplayList, BuilderInitialClipBoundsNonZero) {
+  SkRect cull_rect = SkRect::MakeLTRB(10, 10, 100, 100);
+  SkRect clip_bounds = SkRect::MakeLTRB(10, 10, 100, 100);
+  DisplayListCanvasRecorder recorder(cull_rect);
+  DisplayListBuilder builder(cull_rect);
+  ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds);
+}
+
 TEST(DisplayList, SingleOpSizes) {
   for (auto& group : allGroups) {
     for (size_t i = 0; i < group.variants.size(); i++) {