From 353d6949e7e053952535ba8facaa6195ab66378d Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 15 Dec 2022 13:40:18 -0800 Subject: [PATCH] make sure CanvasRecorder updates clip bounds methods (#38325) --- display_list/display_list_builder.cc | 4 +- display_list/display_list_canvas_recorder.cc | 11 +- display_list/display_list_unittests.cc | 106 +++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index b1cdb843a5434..2e0c7bcce3bf9 100644 --- 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(); } - 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 index e905c26071417..be420bec89eab 100644 --- 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(bounds, prepare_rtree)) {} + : SkCanvasVirtualEnforcer(0, 0), + builder_(sk_make_sp(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 DisplayListCanvasRecorder::Build() { CHECK_DISPOSE(nullptr); @@ -57,6 +61,7 @@ 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, @@ -64,6 +69,7 @@ void DisplayListCanvasRecorder::onClipRRect(const SkRRect& rrect, 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, @@ -71,6 +77,7 @@ void DisplayListCanvasRecorder::onClipPath(const SkPath& path, 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 index 2eecf7baee585..4c5359d353439 100644 --- 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++) {