From 0b94e621ccc0360099961a13be10dbf024c022be Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 16 Dec 2022 22:11:51 +0800 Subject: [PATCH 1/8] Use DisplayListMatrixClipTracker in DisplayListBuilder --- display_list/display_list_builder.cc | 83 +++++++------------ display_list/display_list_builder.h | 33 ++------ .../display_list_matrix_clip_tracker.cc | 17 +++- ...play_list_matrix_clip_tracker_unittests.cc | 26 ++++++ 4 files changed, 79 insertions(+), 80 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 2e0c7bcce3bf9..7097990100901 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -67,16 +67,16 @@ sk_sp DisplayListBuilder::Build() { } DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect, - bool prepare_rtree) { + bool prepare_rtree) + : tracker_(cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect, + SkMatrix::I()) { if (prepare_rtree) { accumulator_ = std::make_unique(); } else { accumulator_ = std::make_unique(); } - // 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); + layer_stack_.emplace_back(LayerInfo()); current_layer_ = &layer_stack_.back(); } @@ -443,6 +443,7 @@ void DisplayListBuilder::save() { layer_stack_.emplace_back(current_layer_); current_layer_ = &layer_stack_.back(); current_layer_->has_deferred_save_op_ = true; + tracker_.save(); accumulator()->save(); } @@ -455,6 +456,7 @@ void DisplayListBuilder::restore() { // on the stack. LayerInfo layer_info = layer_stack_.back(); + tracker_.restore(); layer_stack_.pop_back(); current_layer_ = &layer_stack_.back(); bool is_unbounded = layer_info.is_unbounded(); @@ -463,7 +465,7 @@ void DisplayListBuilder::restore() { // current accumulator and adjust it as required based on the filter. std::shared_ptr filter = layer_info.filter(); if (filter) { - const SkRect* clip = ¤t_layer_->clip_bounds(); + const SkRect clip = tracker_.device_cull_rect(); if (!accumulator()->restore( [filter = filter, matrix = getTransform()](const SkRect& input, SkRect& output) { @@ -473,7 +475,7 @@ void DisplayListBuilder::restore() { output.set(output_bounds); return ret; }, - clip)) { + &clip)) { is_unbounded = true; } } else { @@ -549,6 +551,7 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, } else { layer_stack_.emplace_back(current_layer_, save_layer_offset, true, nullptr); } + tracker_.save(); accumulator()->save(); current_layer_ = &layer_stack_.back(); if (options.renders_with_attributes()) { @@ -566,7 +569,7 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, // use them as the temporary layer bounds during rendering the layer, so // we set them as if a clip operation were performed. if (bounds) { - intersect(*bounds); + tracker_.clipRect(*bounds, SkClipOp::kIntersect, false); } if (backdrop) { // A backdrop will affect up to the entire surface, bounded by the clip @@ -590,8 +593,7 @@ void DisplayListBuilder::translate(SkScalar tx, SkScalar ty) { (tx != 0.0 || ty != 0.0)) { checkForDeferredSave(); Push(0, 1, tx, ty); - current_layer_->matrix().preTranslate(tx, ty); - current_layer_->update_matrix33(); + tracker_.translate(tx, ty); } } void DisplayListBuilder::scale(SkScalar sx, SkScalar sy) { @@ -599,16 +601,14 @@ void DisplayListBuilder::scale(SkScalar sx, SkScalar sy) { (sx != 1.0 || sy != 1.0)) { checkForDeferredSave(); Push(0, 1, sx, sy); - current_layer_->matrix().preScale(sx, sy); - current_layer_->update_matrix33(); + tracker_.scale(sx, sy); } } void DisplayListBuilder::rotate(SkScalar degrees) { if (SkScalarMod(degrees, 360.0) != 0.0) { checkForDeferredSave(); Push(0, 1, degrees); - current_layer_->matrix().preConcat(SkMatrix::RotateDeg(degrees)); - current_layer_->update_matrix33(); + tracker_.rotate(degrees); } } void DisplayListBuilder::skew(SkScalar sx, SkScalar sy) { @@ -616,8 +616,7 @@ void DisplayListBuilder::skew(SkScalar sx, SkScalar sy) { (sx != 0.0 || sy != 0.0)) { checkForDeferredSave(); Push(0, 1, sx, sy); - current_layer_->matrix().preConcat(SkMatrix::Skew(sx, sy)); - current_layer_->update_matrix33(); + tracker_.skew(sx, sy); } } @@ -636,11 +635,10 @@ void DisplayListBuilder::transform2DAffine( Push(0, 1, mxx, mxy, mxt, myx, myy, myt); - current_layer_->matrix().preConcat(SkM44(mxx, mxy, 0, mxt, - myx, myy, 0, myt, - 0, 0, 1, 0, - 0, 0, 0, 1)); - current_layer_->update_matrix33(); + tracker_.transform(SkM44(mxx, mxy, 0, mxt, + myx, myy, 0, myt, + 0, 0, 1, 0, + 0, 0, 0, 1)); } } // full 4x4 transform in row major order @@ -665,19 +663,17 @@ void DisplayListBuilder::transformFullPerspective( myx, myy, myz, myt, mzx, mzy, mzz, mzt, mwx, mwy, mwz, mwt); - current_layer_->matrix().preConcat(SkM44(mxx, mxy, mxz, mxt, - myx, myy, myz, myt, - mzx, mzy, mzz, mzt, - mwx, mwy, mwz, mwt)); - current_layer_->update_matrix33(); + tracker_.transform(SkM44(mxx, mxy, mxz, mxt, + myx, myy, myz, myt, + mzx, mzy, mzz, mzt, + mwx, mwy, mwz, mwt)); } } // clang-format on void DisplayListBuilder::transformReset() { checkForDeferredSave(); Push(0, 0); - current_layer_->matrix().setIdentity(); - current_layer_->update_matrix33(); + tracker_.setIdentity(); } void DisplayListBuilder::transform(const SkMatrix* matrix) { if (matrix != nullptr) { @@ -704,12 +700,12 @@ void DisplayListBuilder::clipRect(const SkRect& rect, switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, rect, is_aa); - intersect(rect); break; case SkClipOp::kDifference: Push(0, 1, rect, is_aa); break; } + tracker_.clipRect(rect, clip_op, is_aa); } void DisplayListBuilder::clipRRect(const SkRRect& rrect, SkClipOp clip_op, @@ -721,12 +717,12 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect, switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, rrect, is_aa); - intersect(rrect.getBounds()); break; case SkClipOp::kDifference: Push(0, 1, rrect, is_aa); break; } + tracker_.clipRRect(rrect, clip_op, is_aa); } } void DisplayListBuilder::clipPath(const SkPath& path, @@ -753,33 +749,12 @@ void DisplayListBuilder::clipPath(const SkPath& path, switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, path, is_aa); - if (!path.isInverseFillType()) { - intersect(path.getBounds()); - } 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()); - } break; } -} -void DisplayListBuilder::intersect(const SkRect& rect) { - SkRect dev_clip_bounds = getTransform().mapRect(rect); - if (!current_layer_->clip_bounds().intersect(dev_clip_bounds)) { - current_layer_->clip_bounds().setEmpty(); - } -} -SkRect DisplayListBuilder::getLocalClipBounds() { - SkM44 inverse; - if (current_layer_->matrix().invert(&inverse)) { - SkRect dev_bounds; - current_layer_->clip_bounds().roundOut(&dev_bounds); - return inverse.asM33().mapRect(dev_bounds); - } - return kMaxCullRect; + tracker_.clipPath(path, clip_op, is_aa); } bool DisplayListBuilder::quickReject(const SkRect& bounds) const { @@ -794,7 +769,7 @@ bool DisplayListBuilder::quickReject(const SkRect& bounds) const { } SkRect dev_bounds; matrix.mapRect(bounds).roundOut(&dev_bounds); - return !current_layer_->clip_bounds().intersects(dev_bounds); + return !tracker_.device_cull_rect().intersects(dev_bounds); } void DisplayListBuilder::drawPaint() { @@ -1357,7 +1332,7 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds, } void DisplayListBuilder::AccumulateUnbounded() { - accumulator()->accumulate(current_layer_->clip_bounds()); + accumulator()->accumulate(tracker_.device_cull_rect()); } void DisplayListBuilder::AccumulateOpBounds(SkRect& bounds, @@ -1370,7 +1345,7 @@ void DisplayListBuilder::AccumulateOpBounds(SkRect& bounds, } void DisplayListBuilder::AccumulateBounds(SkRect& bounds) { getTransform().mapRect(&bounds); - if (bounds.intersect(current_layer_->clip_bounds())) { + if (bounds.intersect(tracker_.device_cull_rect())) { accumulator()->accumulate(bounds); } } diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index b16e8b37ea012..5d35238349f3b 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -11,6 +11,7 @@ #include "flutter/display_list/display_list_dispatcher.h" #include "flutter/display_list/display_list_flags.h" #include "flutter/display_list/display_list_image.h" +#include "flutter/display_list/display_list_matrix_clip_tracker.h" #include "flutter/display_list/display_list_paint.h" #include "flutter/display_list/display_list_path_effect.h" #include "flutter/display_list/display_list_sampling_options.h" @@ -210,11 +211,11 @@ class DisplayListBuilder final : public virtual Dispatcher, /// Returns the 4x4 full perspective transform representing all transform /// operations executed so far in this DisplayList within the enclosing /// save stack. - SkM44 getTransformFullPerspective() const { return current_layer_->matrix(); } + SkM44 getTransformFullPerspective() const { return tracker_.matrix_4x4(); } /// Returns the 3x3 partial perspective transform representing all transform /// operations executed so far in this DisplayList within the enclosing /// save stack. - SkMatrix getTransform() const { return current_layer_->matrix33(); } + SkMatrix getTransform() const { return tracker_.matrix_3x3(); } void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override; @@ -223,11 +224,11 @@ class DisplayListBuilder final : public virtual Dispatcher, /// Conservative estimate of the bounds of all outstanding clip operations /// measured in the coordinate space within which this DisplayList will /// be rendered. - SkRect getDestinationClipBounds() { return current_layer_->clip_bounds(); } + SkRect getDestinationClipBounds() { return tracker_.device_cull_rect(); } /// Conservative estimate of the bounds of all outstanding clip operations /// transformed into the local coordinate space in which currently /// recorded rendering operations are interpreted. - SkRect getLocalClipBounds(); + SkRect getLocalClipBounds() { return tracker_.local_cull_rect(); } /// Return true iff the supplied bounds are easily shown to be outside /// of the current clip bounds. This method may conservatively return @@ -386,19 +387,13 @@ class DisplayListBuilder final : public virtual Dispatcher, class LayerInfo { public: - explicit LayerInfo(const SkM44& matrix, - const SkMatrix& matrix33, - const SkRect& clip_bounds, - size_t save_layer_offset = 0, + explicit LayerInfo(size_t save_layer_offset = 0, bool has_layer = false, std::shared_ptr filter = nullptr) : save_layer_offset_(save_layer_offset), has_layer_(has_layer), cannot_inherit_opacity_(false), has_compatible_op_(false), - matrix_(matrix), - matrix33_(matrix33), - clip_bounds_(clip_bounds), filter_(filter), is_unbounded_(false) {} @@ -406,12 +401,7 @@ class DisplayListBuilder final : public virtual Dispatcher, size_t save_layer_offset = 0, bool has_layer = false, std::shared_ptr filter = nullptr) - : LayerInfo(current_layer->matrix_, - current_layer->matrix33_, - current_layer->clip_bounds_, - save_layer_offset, - has_layer, - filter) {} + : LayerInfo(save_layer_offset, has_layer, filter) {} // The offset into the memory buffer where the saveLayer DLOp record // for this saveLayer() call is placed. This may be needed if the @@ -424,11 +414,6 @@ class DisplayListBuilder final : public virtual Dispatcher, bool has_layer() const { return has_layer_; } bool cannot_inherit_opacity() const { return cannot_inherit_opacity_; } bool has_compatible_op() const { return cannot_inherit_opacity_; } - SkM44& matrix() { return matrix_; } - SkMatrix& matrix33() { return matrix33_; } - SkRect& clip_bounds() { return clip_bounds_; } - - void update_matrix33() { matrix33_ = matrix_.asM33(); } bool is_group_opacity_compatible() const { return !cannot_inherit_opacity_; @@ -486,9 +471,6 @@ class DisplayListBuilder final : public virtual Dispatcher, bool has_layer_; bool cannot_inherit_opacity_; bool has_compatible_op_; - SkM44 matrix_; - SkMatrix matrix33_; - SkRect clip_bounds_; std::shared_ptr filter_; bool is_unbounded_; bool has_deferred_save_op_ = false; @@ -640,6 +622,7 @@ class DisplayListBuilder final : public virtual Dispatcher, DlPaint current_; // If |current_blender_| is set then ignore |current_.getBlendMode()| sk_sp current_blender_; + DisplayListMatrixClipTracker tracker_; }; } // namespace flutter diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index 49f653f396c41..febb1e59c6cef 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -183,6 +183,19 @@ void DisplayListMatrixClipTracker::clipRRect(const SkRRect& rrect, void DisplayListMatrixClipTracker::clipPath(const SkPath& path, SkClipOp op, bool is_aa) { + // Map "kDifference of inverse path" to "kIntersect of the original path" and + // map "kIntersect of inverse path" to "kDifference of the original path" + if (path.isInverseFillType()) { + switch (op) { + case SkClipOp::kIntersect: + op = SkClipOp::kDifference; + break; + case SkClipOp::kDifference: + op = SkClipOp::kIntersect; + break; + } + } + SkRect bounds; switch (op) { case SkClipOp::kIntersect: @@ -323,7 +336,9 @@ SkRect Data3x3::local_cull_rect() const { // cull rect. return DisplayListBuilder::kMaxCullRect; } - return inverse.mapRect(cull_rect_); + SkRect expended_rect; + cull_rect_.roundOut(&expended_rect); + return inverse.mapRect(expended_rect); } } // namespace flutter diff --git a/display_list/display_list_matrix_clip_tracker_unittests.cc b/display_list/display_list_matrix_clip_tracker_unittests.cc index c4ffb18310505..89480ab28a5fc 100644 --- a/display_list/display_list_matrix_clip_tracker_unittests.cc +++ b/display_list/display_list_matrix_clip_tracker_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/display_list/display_list_matrix_clip_tracker.h" #include "gtest/gtest.h" +#include "third_party/skia/include/core/SkPath.h" namespace flutter { namespace testing { @@ -225,5 +226,30 @@ TEST(DisplayListMatrixClipTracker, Rotate) { ASSERT_EQ(tracker2.matrix_4x4(), rotated_m44); } +TEST(DisplayListMatrixClipTracker, ClipPathWithInvertFillType) { + SkRect cull_rect = SkRect::MakeLTRB(0, 0, 100.0, 100.0); + DisplayListMatrixClipTracker builder(cull_rect, SkMatrix::I()); + 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.local_cull_rect(), cull_rect); + ASSERT_EQ(builder.device_cull_rect(), cull_rect); +} + +TEST(DisplayListMatrixClipTracker, DiffClipPathWithInvertFillType) { + SkRect cull_rect = SkRect::MakeLTRB(0, 0, 100.0, 100.0); + DisplayListMatrixClipTracker tracker(cull_rect, SkMatrix::I()); + + 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); + tracker.clipPath(clip, SkClipOp::kDifference, false); + + ASSERT_EQ(tracker.local_cull_rect(), clip_expanded_bounds); + ASSERT_EQ(tracker.device_cull_rect(), clip_bounds); +} + } // namespace testing } // namespace flutter From b201dadc773f8e726ec68ed88114df9be7b5a9b0 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 16 Dec 2022 23:03:09 +0800 Subject: [PATCH 2/8] Ignore is_aa --- display_list/display_list_matrix_clip_tracker.cc | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index febb1e59c6cef..218c113314ac0 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -242,9 +242,6 @@ void DisplayListMatrixClipTracker::Data::clipBounds(const SkRect& clip, } SkRect rect; map_rect(clip, &rect); - if (is_aa) { - rect.roundOut(&rect); - } if (!cull_rect_.intersect(rect)) { cull_rect_.setEmpty(); } @@ -256,15 +253,7 @@ void DisplayListMatrixClipTracker::Data::clipBounds(const SkRect& clip, } SkRect rect; if (map_rect(clip, &rect)) { - // This technique only works if it is rect -> rect - if (is_aa) { - SkIRect rounded; - rect.round(&rounded); - if (rounded.isEmpty()) { - break; - } - rect.set(rounded); - } + // This technique only works if it is rect -> rect} if (rect.fLeft <= cull_rect_.fLeft && rect.fRight >= cull_rect_.fRight) { // bounds spans entire width of cull_rect_ From d71c31cd9e598cbb14453be6cccdcb77936ed6b5 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Fri, 16 Dec 2022 23:23:33 +0800 Subject: [PATCH 3/8] Revert "Ignore is_aa" This reverts commit b201dadc773f8e726ec68ed88114df9be7b5a9b0. --- display_list/display_list_matrix_clip_tracker.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index 218c113314ac0..febb1e59c6cef 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -242,6 +242,9 @@ void DisplayListMatrixClipTracker::Data::clipBounds(const SkRect& clip, } SkRect rect; map_rect(clip, &rect); + if (is_aa) { + rect.roundOut(&rect); + } if (!cull_rect_.intersect(rect)) { cull_rect_.setEmpty(); } @@ -253,7 +256,15 @@ void DisplayListMatrixClipTracker::Data::clipBounds(const SkRect& clip, } SkRect rect; if (map_rect(clip, &rect)) { - // This technique only works if it is rect -> rect} + // This technique only works if it is rect -> rect + if (is_aa) { + SkIRect rounded; + rect.round(&rounded); + if (rounded.isEmpty()) { + break; + } + rect.set(rounded); + } if (rect.fLeft <= cull_rect_.fLeft && rect.fRight >= cull_rect_.fRight) { // bounds spans entire width of cull_rect_ From 88aa3ceee1a0a9abffb0e1822e8ae53b40cc0580 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Mon, 19 Dec 2022 17:00:47 +0800 Subject: [PATCH 4/8] Tweak code --- display_list/display_list_builder.cc | 23 ++--- display_list/display_list_builder.h | 8 +- .../display_list_matrix_clip_tracker.cc | 49 +++++++++- .../display_list_matrix_clip_tracker.h | 10 ++ ...play_list_matrix_clip_tracker_unittests.cc | 98 +++++++++++++++++++ testing/dart/canvas_test.dart | 38 +++---- 6 files changed, 184 insertions(+), 42 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 7097990100901..927cde2965bfe 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -68,8 +68,7 @@ sk_sp DisplayListBuilder::Build() { DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect, bool prepare_rtree) - : tracker_(cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect, - SkMatrix::I()) { + : tracker_(cull_rect, SkMatrix::I()) { if (prepare_rtree) { accumulator_ = std::make_unique(); } else { @@ -440,7 +439,7 @@ void DisplayListBuilder::checkForDeferredSave() { } void DisplayListBuilder::save() { - layer_stack_.emplace_back(current_layer_); + layer_stack_.emplace_back(LayerInfo()); current_layer_ = &layer_stack_.back(); current_layer_->has_deferred_save_op_ = true; tracker_.save(); @@ -546,10 +545,10 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, // We will fill the clip of the outer layer when we restore AccumulateUnbounded(); } - layer_stack_.emplace_back(current_layer_, save_layer_offset, true, + layer_stack_.emplace_back(save_layer_offset, true, current_.getImageFilter()); } else { - layer_stack_.emplace_back(current_layer_, save_layer_offset, true, nullptr); + layer_stack_.emplace_back(save_layer_offset, true, nullptr); } tracker_.save(); accumulator()->save(); @@ -635,10 +634,8 @@ void DisplayListBuilder::transform2DAffine( Push(0, 1, mxx, mxy, mxt, myx, myy, myt); - tracker_.transform(SkM44(mxx, mxy, 0, mxt, - myx, myy, 0, myt, - 0, 0, 1, 0, - 0, 0, 0, 1)); + tracker_.transform2DAffine(mxx, mxy, mxt, + myx, myy, myt); } } // full 4x4 transform in row major order @@ -663,10 +660,10 @@ void DisplayListBuilder::transformFullPerspective( myx, myy, myz, myt, mzx, mzy, mzz, mzt, mwx, mwy, mwz, mwt); - tracker_.transform(SkM44(mxx, mxy, mxz, mxt, - myx, myy, myz, myt, - mzx, mzy, mzz, mzt, - mwx, mwy, mwz, mwt)); + tracker_.transformFullPerspective(mxx, mxy, mxz, mxt, + myx, myy, myz, myt, + mzx, mzy, mzz, mzt, + mwx, mwy, mwz, mwt); } } // clang-format on diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index 5d35238349f3b..d08055dc1193f 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -397,12 +397,6 @@ class DisplayListBuilder final : public virtual Dispatcher, filter_(filter), is_unbounded_(false) {} - explicit LayerInfo(const LayerInfo* current_layer, - size_t save_layer_offset = 0, - bool has_layer = false, - std::shared_ptr filter = nullptr) - : LayerInfo(save_layer_offset, has_layer, filter) {} - // The offset into the memory buffer where the saveLayer DLOp record // for this saveLayer() call is placed. This may be needed if the // eventual restore() call has discovered important information about @@ -480,6 +474,7 @@ class DisplayListBuilder final : public virtual Dispatcher, std::vector layer_stack_; LayerInfo* current_layer_; + DisplayListMatrixClipTracker tracker_; std::unique_ptr accumulator_; BoundsAccumulator* accumulator() { return accumulator_.get(); } @@ -622,7 +617,6 @@ class DisplayListBuilder final : public virtual Dispatcher, DlPaint current_; // If |current_blender_| is set then ignore |current_.getBlendMode()| sk_sp current_blender_; - DisplayListMatrixClipTracker tracker_; }; } // namespace flutter diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index febb1e59c6cef..a05308c84a3ae 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -103,21 +103,64 @@ static bool is_3x3(const SkM44& m) { DisplayListMatrixClipTracker::DisplayListMatrixClipTracker( const SkRect& cull_rect, const SkMatrix& matrix) { - saved_.emplace_back(std::make_unique(matrix, cull_rect)); + // isEmpty protects us against NaN as we normalize any empty cull rects + SkRect cull = cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect; + saved_.emplace_back(std::make_unique(matrix, cull)); current_ = saved_.back().get(); } DisplayListMatrixClipTracker::DisplayListMatrixClipTracker( const SkRect& cull_rect, const SkM44& m44) { + // isEmpty protects us against NaN as we normalize any empty cull rects + SkRect cull = cull_rect.isEmpty() ? SkRect::MakeEmpty() : cull_rect; if (is_3x3(m44)) { - saved_.emplace_back(std::make_unique(m44.asM33(), cull_rect)); + saved_.emplace_back(std::make_unique(m44.asM33(), cull)); } else { - saved_.emplace_back(std::make_unique(m44, cull_rect)); + saved_.emplace_back(std::make_unique(m44, cull)); } current_ = saved_.back().get(); } +// clang-format off +void DisplayListMatrixClipTracker::transform2DAffine( + SkScalar mxx, SkScalar mxy, SkScalar mxt, + SkScalar myx, SkScalar myy, SkScalar myt) { + if (!current_->is_4x4()) { + transform(SkMatrix::MakeAll(mxx, mxy, mxt, + myx, myy, myt, + 0, 0, 1)); + } else { + transform(SkM44(mxx, mxy, 0, mxt, + myx, myy, 0, myt, + 0, 0, 1, 0, + 0, 0, 0, 1)); + } +} +void DisplayListMatrixClipTracker::transformFullPerspective( + SkScalar mxx, SkScalar mxy, SkScalar mxz, SkScalar mxt, + SkScalar myx, SkScalar myy, SkScalar myz, SkScalar myt, + SkScalar mzx, SkScalar mzy, SkScalar mzz, SkScalar mzt, + SkScalar mwx, SkScalar mwy, SkScalar mwz, SkScalar mwt) { + if (!current_->is_4x4()) { + if ( mxz == 0 && + myz == 0 && + mzx == 0 && mzy == 0 && mzz == 1 && mzt == 0 && + mwz == 0) { + transform(SkMatrix::MakeAll(mxx, mxy, mxt, + myx, myy, myt, + mwx, mwy, mwt)); + return; + } + } + + transform(SkM44(mxx, mxy, mxz, mxt, + myx, myy, myz, myt, + mzx, mzy, mzz, mzt, + mwx, mwy, mwz, mwt)); +} +// clang-format on + void DisplayListMatrixClipTracker::save() { if (current_->is_4x4()) { saved_.emplace_back(std::make_unique(current_)); diff --git a/display_list/display_list_matrix_clip_tracker.h b/display_list/display_list_matrix_clip_tracker.h index 5e82f624a9ea9..15d2a9b57601a 100644 --- a/display_list/display_list_matrix_clip_tracker.h +++ b/display_list/display_list_matrix_clip_tracker.h @@ -40,6 +40,16 @@ class DisplayListMatrixClipTracker { void rotate(SkScalar degrees) { current_->rotate(degrees); } void transform(const SkM44& m44); void transform(const SkMatrix& matrix) { current_->transform(matrix); } + // clang-format off + void transform2DAffine( + SkScalar mxx, SkScalar mxy, SkScalar mxt, + SkScalar myx, SkScalar myy, SkScalar myt); + void transformFullPerspective( + SkScalar mxx, SkScalar mxy, SkScalar mxz, SkScalar mxt, + SkScalar myx, SkScalar myy, SkScalar myz, SkScalar myt, + SkScalar mzx, SkScalar mzy, SkScalar mzz, SkScalar mzt, + SkScalar mwx, SkScalar mwy, SkScalar mwz, SkScalar mwt); + // clang-format on void setTransform(const SkMatrix& matrix) { current_->setTransform(matrix); } void setTransform(const SkM44& m44); void setIdentity() { current_->setIdentity(); } diff --git a/display_list/display_list_matrix_clip_tracker_unittests.cc b/display_list/display_list_matrix_clip_tracker_unittests.cc index 89480ab28a5fc..bcaa8dcf166bf 100644 --- a/display_list/display_list_matrix_clip_tracker_unittests.cc +++ b/display_list/display_list_matrix_clip_tracker_unittests.cc @@ -226,6 +226,104 @@ TEST(DisplayListMatrixClipTracker, Rotate) { ASSERT_EQ(tracker2.matrix_4x4(), rotated_m44); } +TEST(DisplayListMatrixClipTracker, Transform2DAffine) { + const SkRect cull_rect = SkRect::MakeLTRB(20, 20, 60, 60); + const SkMatrix matrix = SkMatrix::Scale(4, 4); + const SkM44 m44 = SkM44::Scale(4, 4); + + const SkMatrix transformed_matrix = + SkMatrix::Concat(matrix, SkMatrix::MakeAll(2, 0, 5, // + 0, 2, 6, // + 0, 0, 1)); + const SkM44 transformed_m44 = SkM44(transformed_matrix); + const SkRect local_cull_rect = SkRect::MakeLTRB(0, -0.5, 5, 4.5); + + DisplayListMatrixClipTracker tracker1(cull_rect, matrix); + DisplayListMatrixClipTracker tracker2(cull_rect, m44); + tracker1.transform2DAffine(2, 0, 5, // + 0, 2, 6); + tracker2.transform2DAffine(2, 0, 5, // + 0, 2, 6); + ASSERT_FALSE(tracker1.using_4x4_matrix()); + ASSERT_EQ(tracker1.device_cull_rect(), cull_rect); + ASSERT_EQ(tracker1.local_cull_rect(), local_cull_rect); + ASSERT_EQ(tracker1.matrix_3x3(), transformed_matrix); + ASSERT_EQ(tracker1.matrix_4x4(), transformed_m44); + + ASSERT_FALSE(tracker2.using_4x4_matrix()); + ASSERT_EQ(tracker2.device_cull_rect(), cull_rect); + ASSERT_EQ(tracker2.local_cull_rect(), local_cull_rect); + ASSERT_EQ(tracker2.matrix_3x3(), transformed_matrix); + ASSERT_EQ(tracker2.matrix_4x4(), transformed_m44); +} + +TEST(DisplayListMatrixClipTracker, TransformFullPerspectiveUsing3x3Matrix) { + const SkRect cull_rect = SkRect::MakeLTRB(20, 20, 60, 60); + const SkMatrix matrix = SkMatrix::Scale(4, 4); + const SkM44 m44 = SkM44::Scale(4, 4); + + const SkMatrix transformed_matrix = + SkMatrix::Concat(matrix, SkMatrix::MakeAll(2, 0, 5, // + 0, 2, 6, // + 0, 0, 1)); + const SkM44 transformed_m44 = SkM44(transformed_matrix); + const SkRect local_cull_rect = SkRect::MakeLTRB(0, -0.5, 5, 4.5); + + DisplayListMatrixClipTracker tracker1(cull_rect, matrix); + DisplayListMatrixClipTracker tracker2(cull_rect, m44); + tracker1.transformFullPerspective(2, 0, 0, 5, // + 0, 2, 0, 6, // + 0, 0, 1, 0, // + 0, 0, 0, 1); + tracker2.transformFullPerspective(2, 0, 0, 5, // + 0, 2, 0, 6, // + 0, 0, 1, 0, // + 0, 0, 0, 1); + ASSERT_FALSE(tracker1.using_4x4_matrix()); + ASSERT_EQ(tracker1.device_cull_rect(), cull_rect); + ASSERT_EQ(tracker1.local_cull_rect(), local_cull_rect); + ASSERT_EQ(tracker1.matrix_3x3(), transformed_matrix); + ASSERT_EQ(tracker1.matrix_4x4(), transformed_m44); + + ASSERT_FALSE(tracker2.using_4x4_matrix()); + ASSERT_EQ(tracker2.device_cull_rect(), cull_rect); + ASSERT_EQ(tracker2.local_cull_rect(), local_cull_rect); + ASSERT_EQ(tracker2.matrix_3x3(), transformed_matrix); + ASSERT_EQ(tracker2.matrix_4x4(), transformed_m44); +} + +TEST(DisplayListMatrixClipTracker, TransformFullPerspectiveUsing4x4Matrix) { + const SkRect cull_rect = SkRect::MakeLTRB(20, 20, 60, 60); + const SkMatrix matrix = SkMatrix::Scale(4, 4); + const SkM44 m44 = SkM44::Scale(4, 4); + + const SkM44 transformed_m44 = SkM44(m44, SkM44(2, 0, 0, 5, // + 0, 2, 0, 6, // + 0, 0, 1, 7, // + 0, 0, 0, 1)); + const SkRect local_cull_rect = SkRect::MakeLTRB(0, -0.5, 5, 4.5); + + DisplayListMatrixClipTracker tracker1(cull_rect, matrix); + DisplayListMatrixClipTracker tracker2(cull_rect, m44); + tracker1.transformFullPerspective(2, 0, 0, 5, // + 0, 2, 0, 6, // + 0, 0, 1, 7, // + 0, 0, 0, 1); + tracker2.transformFullPerspective(2, 0, 0, 5, // + 0, 2, 0, 6, // + 0, 0, 1, 7, // + 0, 0, 0, 1); + ASSERT_TRUE(tracker1.using_4x4_matrix()); + ASSERT_EQ(tracker1.device_cull_rect(), cull_rect); + ASSERT_EQ(tracker1.local_cull_rect(), local_cull_rect); + ASSERT_EQ(tracker1.matrix_4x4(), transformed_m44); + + ASSERT_TRUE(tracker2.using_4x4_matrix()); + ASSERT_EQ(tracker2.device_cull_rect(), cull_rect); + ASSERT_EQ(tracker2.local_cull_rect(), local_cull_rect); + ASSERT_EQ(tracker2.matrix_4x4(), transformed_m44); +} + TEST(DisplayListMatrixClipTracker, ClipPathWithInvertFillType) { SkRect cull_rect = SkRect::MakeLTRB(0, 0, 100.0, 100.0); DisplayListMatrixClipTracker builder(cull_rect, SkMatrix::I()); diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 5870692166767..b40756eb5e021 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -731,7 +731,7 @@ void main() { final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); - canvas.clipRect(clipBounds); + canvas.clipRect(clipBounds, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); @@ -773,16 +773,16 @@ void main() { const Rect clipBounds2 = Rect.fromLTRB(10.0, 10.0, 20.0, 20.0); canvas.save(); - canvas.clipRect(clipBounds1); + canvas.clipRect(clipBounds1, doAntiAlias: false); canvas.translate(0, 10.0); - canvas.clipRect(clipBounds1); + canvas.clipRect(clipBounds1, doAntiAlias: false); expect(canvas.getDestinationClipBounds().isEmpty, isTrue); canvas.restore(); canvas.save(); - canvas.clipRect(clipBounds1); + canvas.clipRect(clipBounds1, doAntiAlias: false); canvas.translate(-10.0, -10.0); - canvas.clipRect(clipBounds2); + canvas.clipRect(clipBounds2, doAntiAlias: false); expect(canvas.getDestinationClipBounds(), clipBounds1); canvas.restore(); }); @@ -793,7 +793,7 @@ void main() { const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); final RRect clip = RRect.fromRectAndRadius(clipBounds, const Radius.circular(3)); - canvas.clipRRect(clip); + canvas.clipRRect(clip, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); @@ -802,7 +802,7 @@ void main() { expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.save(); - canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), doAntiAlias: false); // Both clip bounds have changed expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); @@ -837,16 +837,16 @@ void main() { final RRect clip2 = RRect.fromRectAndRadius(clipBounds2, const Radius.circular(3)); canvas.save(); - canvas.clipRRect(clip1); + canvas.clipRRect(clip1, doAntiAlias: false); canvas.translate(0, 10.0); - canvas.clipRRect(clip1); + canvas.clipRRect(clip1, doAntiAlias: false); expect(canvas.getDestinationClipBounds().isEmpty, isTrue); canvas.restore(); canvas.save(); - canvas.clipRRect(clip1); + canvas.clipRRect(clip1, doAntiAlias: false); canvas.translate(-10.0, -10.0); - canvas.clipRRect(clip2); + canvas.clipRRect(clip2, doAntiAlias: false); expect(canvas.getDestinationClipBounds(), clipBounds1); canvas.restore(); }); @@ -857,7 +857,7 @@ void main() { const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); final Path clip = Path()..addRect(clipBounds)..addOval(clipBounds); - canvas.clipPath(clip); + canvas.clipPath(clip, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); @@ -866,7 +866,7 @@ void main() { expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.save(); - canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), doAntiAlias: false); // Both clip bounds have changed expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); @@ -901,16 +901,16 @@ void main() { final Path clip2 = Path()..addRect(clipBounds2)..addOval(clipBounds2); canvas.save(); - canvas.clipPath(clip1); + canvas.clipPath(clip1, doAntiAlias: false); canvas.translate(0, 10.0); - canvas.clipPath(clip1); + canvas.clipPath(clip1, doAntiAlias: false); expect(canvas.getDestinationClipBounds().isEmpty, isTrue); canvas.restore(); canvas.save(); - canvas.clipPath(clip1); + canvas.clipPath(clip1, doAntiAlias: false); canvas.translate(-10.0, -10.0); - canvas.clipPath(clip2); + canvas.clipPath(clip2, doAntiAlias: false); expect(canvas.getDestinationClipBounds(), clipBounds1); canvas.restore(); }); @@ -920,7 +920,7 @@ void main() { final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); - canvas.clipRect(clipBounds); + canvas.clipRect(clipBounds, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); @@ -928,7 +928,7 @@ void main() { expect(initialLocalBounds, closeToRect(clipExpandedBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); - canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), clipOp: ClipOp.difference); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), clipOp: ClipOp.difference, doAntiAlias: false); expect(canvas.getLocalClipBounds(), initialLocalBounds); expect(canvas.getDestinationClipBounds(), initialDestinationBounds); }); From 24c72bd15fdad6793f5038f8e248a9a6cb2af73b Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Mon, 19 Dec 2022 17:20:38 +0800 Subject: [PATCH 5/8] Use content_culled --- display_list/display_list_builder.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 927cde2965bfe..76727f6380c18 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -764,9 +764,8 @@ bool DisplayListBuilder::quickReject(const SkRect& bounds) const { if (!matrix.invert(nullptr)) { return true; } - SkRect dev_bounds; - matrix.mapRect(bounds).roundOut(&dev_bounds); - return !tracker_.device_cull_rect().intersects(dev_bounds); + + return tracker_.content_culled(bounds); } void DisplayListBuilder::drawPaint() { From 0810362e74906aab4fcf74617ed18d2e86731613 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Mon, 19 Dec 2022 18:17:28 +0800 Subject: [PATCH 6/8] getLocalClipBounds without device clip bounds roundsOut --- .../display_list_matrix_clip_tracker.cc | 4 +- ...play_list_matrix_clip_tracker_unittests.cc | 3 +- display_list/display_list_unittests.cc | 45 ++++++++----------- testing/dart/canvas_test.dart | 36 +++++++-------- 4 files changed, 37 insertions(+), 51 deletions(-) diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index a05308c84a3ae..16508f7a888a0 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -379,9 +379,7 @@ SkRect Data3x3::local_cull_rect() const { // cull rect. return DisplayListBuilder::kMaxCullRect; } - SkRect expended_rect; - cull_rect_.roundOut(&expended_rect); - return inverse.mapRect(expended_rect); + return inverse.mapRect(cull_rect_); } } // namespace flutter diff --git a/display_list/display_list_matrix_clip_tracker_unittests.cc b/display_list/display_list_matrix_clip_tracker_unittests.cc index bcaa8dcf166bf..eff7e9094ad3c 100644 --- a/display_list/display_list_matrix_clip_tracker_unittests.cc +++ b/display_list/display_list_matrix_clip_tracker_unittests.cc @@ -342,10 +342,9 @@ TEST(DisplayListMatrixClipTracker, DiffClipPathWithInvertFillType) { 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); tracker.clipPath(clip, SkClipOp::kDifference, false); - ASSERT_EQ(tracker.local_cull_rect(), clip_expanded_bounds); + ASSERT_EQ(tracker.local_cull_rect(), clip_bounds); ASSERT_EQ(tracker.device_cull_rect(), clip_bounds); } diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 4c5359d353439..85b40259ea4c2 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1361,22 +1361,21 @@ TEST(DisplayList, FullTransformAffectsCurrentTransform) { TEST(DisplayList, ClipRectAffectsClipBounds) { DisplayListBuilder builder; SkRect clip_bounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); - SkRect clip_expanded_bounds = SkRect::MakeLTRB(10, 11, 21, 26); builder.clipRect(clip_bounds, SkClipOp::kIntersect, false); // Save initial return values for testing restored values SkRect initial_local_bounds = builder.getLocalClipBounds(); SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.save(); builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); // Both clip bounds have changed - ASSERT_NE(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_NE(builder.getLocalClipBounds(), clip_bounds); ASSERT_NE(builder.getDestinationClipBounds(), clip_bounds); // Previous return values have not changed - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.restore(); @@ -1386,8 +1385,8 @@ TEST(DisplayList, ClipRectAffectsClipBounds) { builder.save(); builder.scale(2, 2); - SkRect scaled_expanded_bounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13); - ASSERT_EQ(builder.getLocalClipBounds(), scaled_expanded_bounds); + SkRect scaled_clip_bounds = SkRect::MakeLTRB(5.1, 5.65, 10.2, 12.85); + ASSERT_EQ(builder.getLocalClipBounds(), scaled_clip_bounds); // Destination bounds are unaffected by transform ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds); builder.restore(); @@ -1419,23 +1418,22 @@ TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { TEST(DisplayList, ClipRRectAffectsClipBounds) { DisplayListBuilder builder; SkRect clip_bounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); - SkRect clip_expanded_bounds = SkRect::MakeLTRB(10, 11, 21, 26); SkRRect clip = SkRRect::MakeRectXY(clip_bounds, 3, 2); builder.clipRRect(clip, SkClipOp::kIntersect, false); // Save initial return values for testing restored values SkRect initial_local_bounds = builder.getLocalClipBounds(); SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.save(); builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); // Both clip bounds have changed - ASSERT_NE(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_NE(builder.getLocalClipBounds(), clip_bounds); ASSERT_NE(builder.getDestinationClipBounds(), clip_bounds); // Previous return values have not changed - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.restore(); @@ -1445,8 +1443,8 @@ TEST(DisplayList, ClipRRectAffectsClipBounds) { builder.save(); builder.scale(2, 2); - SkRect scaled_expanded_bounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13); - ASSERT_EQ(builder.getLocalClipBounds(), scaled_expanded_bounds); + SkRect scaled_clip_bounds = SkRect::MakeLTRB(5.1, 5.65, 10.2, 12.85); + ASSERT_EQ(builder.getLocalClipBounds(), scaled_clip_bounds); // Destination bounds are unaffected by transform ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds); builder.restore(); @@ -1482,22 +1480,21 @@ TEST(DisplayList, ClipPathAffectsClipBounds) { DisplayListBuilder builder; SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); 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::kIntersect, false); // Save initial return values for testing restored values SkRect initial_local_bounds = builder.getLocalClipBounds(); SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.save(); builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); // Both clip bounds have changed - ASSERT_NE(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_NE(builder.getLocalClipBounds(), clip_bounds); ASSERT_NE(builder.getDestinationClipBounds(), clip_bounds); // Previous return values have not changed - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.restore(); @@ -1507,8 +1504,8 @@ TEST(DisplayList, ClipPathAffectsClipBounds) { builder.save(); builder.scale(2, 2); - SkRect scaled_expanded_bounds = SkRect::MakeLTRB(4, 4.5, 11.5, 14); - ASSERT_EQ(builder.getLocalClipBounds(), scaled_expanded_bounds); + SkRect scaled_clip_bounds = SkRect::MakeLTRB(4.1, 4.65, 11.2, 13.85); + ASSERT_EQ(builder.getLocalClipBounds(), scaled_clip_bounds); // Destination bounds are unaffected by transform ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds); builder.restore(); @@ -1543,13 +1540,12 @@ TEST(DisplayList, DiffClipRectDoesNotAffectClipBounds) { DisplayListBuilder builder; SkRect diff_clip = SkRect::MakeLTRB(0, 0, 15, 15); SkRect clip_bounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); - SkRect clip_expanded_bounds = SkRect::MakeLTRB(10, 11, 21, 26); builder.clipRect(clip_bounds, SkClipOp::kIntersect, false); // Save initial return values for testing after kDifference clip SkRect initial_local_bounds = builder.getLocalClipBounds(); SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.clipRect(diff_clip, SkClipOp::kDifference, false); @@ -1561,14 +1557,13 @@ TEST(DisplayList, DiffClipRRectDoesNotAffectClipBounds) { DisplayListBuilder builder; SkRRect diff_clip = SkRRect::MakeRectXY({0, 0, 15, 15}, 1, 1); SkRect clip_bounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); - SkRect clip_expanded_bounds = SkRect::MakeLTRB(10, 11, 21, 26); SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2); builder.clipRRect(clip, SkClipOp::kIntersect, false); // Save initial return values for testing after kDifference clip SkRect initial_local_bounds = builder.getLocalClipBounds(); SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.clipRRect(diff_clip, SkClipOp::kDifference, false); @@ -1581,13 +1576,12 @@ TEST(DisplayList, DiffClipPathDoesNotAffectClipBounds) { SkPath diff_clip = SkPath().addRect({0, 0, 15, 15}); SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); 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::kIntersect, false); // Save initial return values for testing after kDifference clip SkRect initial_local_bounds = builder.getLocalClipBounds(); SkRect initial_destination_bounds = builder.getDestinationClipBounds(); - ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_local_bounds, clip_bounds); ASSERT_EQ(initial_destination_bounds, clip_bounds); builder.clipPath(diff_clip, SkClipOp::kDifference, false); @@ -1612,10 +1606,9 @@ TEST(DisplayList, DiffClipPathWithInvertFillTypeAffectsClipBounds) { 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.getLocalClipBounds(), clip_bounds); ASSERT_EQ(builder.getDestinationClipBounds(), clip_bounds); } diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index b40756eb5e021..0b2476b670975 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -730,22 +730,21 @@ void main() { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); - const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); canvas.clipRect(clipBounds, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.save(); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); // Both clip bounds have changed - expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getLocalClipBounds(), notCloseToRect(clipBounds)); expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); // Previous return values have not changed - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.restore(); @@ -755,8 +754,8 @@ void main() { canvas.save(); canvas.scale(2, 2); - const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); - expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + const Rect scaledClipBounds = Rect.fromLTRB(5.1, 5.65, 10.2, 12.85); + expect(canvas.getLocalClipBounds(), closeToRect(scaledClipBounds)); // Destination bounds are unaffected by transform expect(canvas.getDestinationClipBounds(), closeToRect(clipBounds)); canvas.restore(); @@ -791,23 +790,22 @@ void main() { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); - const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); final RRect clip = RRect.fromRectAndRadius(clipBounds, const Radius.circular(3)); canvas.clipRRect(clip, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.save(); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), doAntiAlias: false); // Both clip bounds have changed - expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getLocalClipBounds(), notCloseToRect(clipBounds)); expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); // Previous return values have not changed - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.restore(); @@ -817,8 +815,8 @@ void main() { canvas.save(); canvas.scale(2, 2); - const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); - expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + const Rect scaledClipBounds = Rect.fromLTRB(5.1, 5.65, 10.2, 12.85); + expect(canvas.getLocalClipBounds(), closeToRect(scaledClipBounds)); // Destination bounds are unaffected by transform expect(canvas.getDestinationClipBounds(), closeToRect(clipBounds)); canvas.restore(); @@ -855,23 +853,22 @@ void main() { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); - const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); final Path clip = Path()..addRect(clipBounds)..addOval(clipBounds); canvas.clipPath(clip, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.save(); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), doAntiAlias: false); // Both clip bounds have changed - expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getLocalClipBounds(), notCloseToRect(clipBounds)); expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); // Previous return values have not changed - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.restore(); @@ -881,8 +878,8 @@ void main() { canvas.save(); canvas.scale(2, 2); - const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); - expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + const Rect scaledClipBounds = Rect.fromLTRB(5.1, 5.65, 10.2, 12.85); + expect(canvas.getLocalClipBounds(), closeToRect(scaledClipBounds)); // Destination bounds are unaffected by transform expect(canvas.getDestinationClipBounds(), closeToRect(clipBounds)); canvas.restore(); @@ -919,13 +916,12 @@ void main() { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); - const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); canvas.clipRect(clipBounds, doAntiAlias: false); // Save initial return values for testing restored values final Rect initialLocalBounds = canvas.getLocalClipBounds(); final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); - expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialLocalBounds, closeToRect(clipBounds)); expect(initialDestinationBounds, closeToRect(clipBounds)); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), clipOp: ClipOp.difference, doAntiAlias: false); From 0aee025223b8121b7ad2bc7f55be062d9b46a3b5 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 20 Dec 2022 14:27:20 +0800 Subject: [PATCH 7/8] Tweak code and add more tests --- display_list/display_list_builder.cc | 16 +-- .../display_list_matrix_clip_tracker.cc | 25 ++-- .../display_list_matrix_clip_tracker.h | 4 +- display_list/display_list_unittests.cc | 118 ++++++++++++++++ testing/dart/canvas_test.dart | 128 +++++++++++++++++- 5 files changed, 265 insertions(+), 26 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 76727f6380c18..fc0f4da90df19 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -75,7 +75,7 @@ DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect, accumulator_ = std::make_unique(); } - layer_stack_.emplace_back(LayerInfo()); + layer_stack_.emplace_back(); current_layer_ = &layer_stack_.back(); } @@ -439,7 +439,7 @@ void DisplayListBuilder::checkForDeferredSave() { } void DisplayListBuilder::save() { - layer_stack_.emplace_back(LayerInfo()); + layer_stack_.emplace_back(); current_layer_ = &layer_stack_.back(); current_layer_->has_deferred_save_op_ = true; tracker_.save(); @@ -755,16 +755,6 @@ void DisplayListBuilder::clipPath(const SkPath& path, } bool DisplayListBuilder::quickReject(const SkRect& bounds) const { - if (bounds.isEmpty()) { - return true; - } - SkMatrix matrix = getTransform(); - // We don't need the inverse, but this method tells us if the matrix - // is singular in which case we can reject all rendering. - if (!matrix.invert(nullptr)) { - return true; - } - return tracker_.content_culled(bounds); } @@ -1340,7 +1330,7 @@ void DisplayListBuilder::AccumulateOpBounds(SkRect& bounds, } } void DisplayListBuilder::AccumulateBounds(SkRect& bounds) { - getTransform().mapRect(&bounds); + tracker_.mapRect(&bounds); if (bounds.intersect(tracker_.device_cull_rect())) { accumulator()->accumulate(bounds); } diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index 16508f7a888a0..fea6ba780c778 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -39,12 +39,13 @@ class Data4x4 : public DisplayListMatrixClipTracker::Data { void setTransform(const SkMatrix& matrix) override { m44_ = SkM44(matrix); } void setTransform(const SkM44& m44) override { m44_ = m44; } void setIdentity() override { m44_.setIdentity(); } + bool mapRect(const SkRect& rect, SkRect* mapped) const override { + return m44_.asM33().mapRect(mapped, rect); + } + bool canBeInverted() const override { return m44_.asM33().invert(nullptr); } protected: bool has_perspective() const override; - bool map_rect(const SkRect& rect, SkRect* mapped) const override { - return m44_.asM33().mapRect(mapped, rect); - } private: SkM44 m44_; @@ -80,12 +81,15 @@ class Data3x3 : public DisplayListMatrixClipTracker::Data { FML_CHECK(false) << "SkM44 was set without upgrading Data"; } void setIdentity() override { matrix_.setIdentity(); } + bool mapRect(const SkRect& rect, SkRect* mapped) const override { + return matrix_.mapRect(mapped, rect); + } + virtual bool canBeInverted() const override { + return matrix_.invert(nullptr); + } protected: bool has_perspective() const override { return matrix_.hasPerspective(); } - bool map_rect(const SkRect& rect, SkRect* mapped) const override { - return matrix_.mapRect(mapped, rect); - } private: SkMatrix matrix_; @@ -258,11 +262,14 @@ bool DisplayListMatrixClipTracker::Data::content_culled( if (cull_rect_.isEmpty() || content_bounds.isEmpty()) { return true; } + if (!canBeInverted()) { + return true; + } if (has_perspective()) { return false; } SkRect mapped; - map_rect(content_bounds, &mapped); + mapRect(content_bounds, &mapped); return !mapped.intersects(cull_rect_); } @@ -284,7 +291,7 @@ void DisplayListMatrixClipTracker::Data::clipBounds(const SkRect& clip, break; } SkRect rect; - map_rect(clip, &rect); + mapRect(clip, &rect); if (is_aa) { rect.roundOut(&rect); } @@ -298,7 +305,7 @@ void DisplayListMatrixClipTracker::Data::clipBounds(const SkRect& clip, break; } SkRect rect; - if (map_rect(clip, &rect)) { + if (mapRect(clip, &rect)) { // This technique only works if it is rect -> rect if (is_aa) { SkIRect rounded; diff --git a/display_list/display_list_matrix_clip_tracker.h b/display_list/display_list_matrix_clip_tracker.h index 15d2a9b57601a..c160b20443a6d 100644 --- a/display_list/display_list_matrix_clip_tracker.h +++ b/display_list/display_list_matrix_clip_tracker.h @@ -53,6 +53,7 @@ class DisplayListMatrixClipTracker { void setTransform(const SkMatrix& matrix) { current_->setTransform(matrix); } void setTransform(const SkM44& m44); void setIdentity() { current_->setIdentity(); } + bool mapRect(SkRect* rect) const { return current_->mapRect(*rect, rect); } void clipRect(const SkRect& rect, SkClipOp op, bool is_aa) { current_->clipBounds(rect, op, is_aa); @@ -83,6 +84,8 @@ class DisplayListMatrixClipTracker { virtual void setTransform(const SkMatrix& matrix) = 0; virtual void setTransform(const SkM44& m44) = 0; virtual void setIdentity() = 0; + virtual bool mapRect(const SkRect& rect, SkRect* mapped) const = 0; + virtual bool canBeInverted() const = 0; virtual void clipBounds(const SkRect& clip, SkClipOp op, bool is_aa); @@ -90,7 +93,6 @@ class DisplayListMatrixClipTracker { Data(const SkRect& rect) : cull_rect_(rect) {} virtual bool has_perspective() const = 0; - virtual bool map_rect(const SkRect& rect, SkRect* mapped) const = 0; SkRect cull_rect_; }; diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 85b40259ea4c2..d0cd15d038560 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1396,6 +1396,45 @@ TEST(DisplayList, ClipRectAffectsClipBounds) { ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); } +TEST(DisplayList, ClipRectDoAAAffectsClipBounds) { + DisplayListBuilder builder; + SkRect clip_bounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + SkRect clip_expanded_bounds = SkRect::MakeLTRB(10, 11, 21, 26); + builder.clipRect(clip_bounds, SkClipOp::kIntersect, true); + + // Save initial return values for testing restored values + 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_expanded_bounds); + + builder.save(); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, true); + // Both clip bounds have changed + ASSERT_NE(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_NE(builder.getDestinationClipBounds(), clip_expanded_bounds); + // Previous return values have not changed + ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_destination_bounds, clip_expanded_bounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initial_local_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); + + builder.save(); + builder.scale(2, 2); + SkRect scaled_expanded_bounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13); + ASSERT_EQ(builder.getLocalClipBounds(), scaled_expanded_bounds); + // Destination bounds are unaffected by transform + ASSERT_EQ(builder.getDestinationClipBounds(), clip_expanded_bounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initial_local_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); +} + TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { DisplayListBuilder builder; SkRect clip_bounds_1 = SkRect::MakeLTRB(0, 0, 10, 10); @@ -1454,6 +1493,46 @@ TEST(DisplayList, ClipRRectAffectsClipBounds) { ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); } +TEST(DisplayList, ClipRRectDoAAAffectsClipBounds) { + DisplayListBuilder builder; + SkRect clip_bounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + SkRect clip_expanded_bounds = SkRect::MakeLTRB(10, 11, 21, 26); + SkRRect clip = SkRRect::MakeRectXY(clip_bounds, 3, 2); + builder.clipRRect(clip, SkClipOp::kIntersect, true); + + // Save initial return values for testing restored values + 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_expanded_bounds); + + builder.save(); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, true); + // Both clip bounds have changed + ASSERT_NE(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_NE(builder.getDestinationClipBounds(), clip_expanded_bounds); + // Previous return values have not changed + ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_destination_bounds, clip_expanded_bounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initial_local_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); + + builder.save(); + builder.scale(2, 2); + SkRect scaled_expanded_bounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13); + ASSERT_EQ(builder.getLocalClipBounds(), scaled_expanded_bounds); + // Destination bounds are unaffected by transform + ASSERT_EQ(builder.getDestinationClipBounds(), clip_expanded_bounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initial_local_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); +} + TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) { DisplayListBuilder builder; SkRect clip_bounds_1 = SkRect::MakeLTRB(0, 0, 10, 10); @@ -1515,6 +1594,45 @@ TEST(DisplayList, ClipPathAffectsClipBounds) { ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); } +TEST(DisplayList, ClipPathDoAAAffectsClipBounds) { + DisplayListBuilder builder; + SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); + SkRect clip_expanded_bounds = SkRect::MakeLTRB(8, 9, 23, 28); + builder.clipPath(clip, SkClipOp::kIntersect, true); + + // Save initial return values for testing restored values + 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_expanded_bounds); + + builder.save(); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, true); + // Both clip bounds have changed + ASSERT_NE(builder.getLocalClipBounds(), clip_expanded_bounds); + ASSERT_NE(builder.getDestinationClipBounds(), clip_expanded_bounds); + // Previous return values have not changed + ASSERT_EQ(initial_local_bounds, clip_expanded_bounds); + ASSERT_EQ(initial_destination_bounds, clip_expanded_bounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initial_local_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); + + builder.save(); + builder.scale(2, 2); + SkRect scaled_expanded_bounds = SkRect::MakeLTRB(4, 4.5, 11.5, 14); + ASSERT_EQ(builder.getLocalClipBounds(), scaled_expanded_bounds); + // Destination bounds are unaffected by transform + ASSERT_EQ(builder.getDestinationClipBounds(), clip_expanded_bounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initial_local_bounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initial_destination_bounds); +} + TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) { DisplayListBuilder builder; SkRect clip_bounds = SkRect::MakeLTRB(0, 0, 10, 10); diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 0b2476b670975..69bcbc77c2c68 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -726,7 +726,47 @@ void main() { Expect.fail('$value is too close to $expected'); }; - test('Canvas.clipRect affects canvas.getClipBounds', () async { + test('Canvas.clipRect(doAA=true) affects canvas.getClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); + canvas.clipRect(clipBounds); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipExpandedBounds)); + + canvas.save(); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + // Both clip bounds have changed + expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getDestinationClipBounds(), notCloseToRect(clipExpandedBounds)); + // Previous return values have not changed + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipExpandedBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); + expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + // Destination bounds are unaffected by transform + expect(canvas.getDestinationClipBounds(), closeToRect(clipExpandedBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + + test('Canvas.clipRect(doAA=false) affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); @@ -786,7 +826,48 @@ void main() { canvas.restore(); }); - test('Canvas.clipRRect affects canvas.getClipBounds', () async { + test('Canvas.clipRRect(doAA=true) affects canvas.getClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); + final RRect clip = RRect.fromRectAndRadius(clipBounds, const Radius.circular(3)); + canvas.clipRRect(clip); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipExpandedBounds)); + + canvas.save(); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + // Both clip bounds have changed + expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getDestinationClipBounds(), notCloseToRect(clipExpandedBounds)); + // Previous return values have not changed + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipExpandedBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); + expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + // Destination bounds are unaffected by transform + expect(canvas.getDestinationClipBounds(), closeToRect(clipExpandedBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + + test('Canvas.clipRRect(doAA=false) affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); @@ -849,7 +930,48 @@ void main() { canvas.restore(); }); - test('Canvas.clipPath affects canvas.getClipBounds', () async { + test('Canvas.clipPath(doAA=true) affects canvas.getClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); + final Path clip = Path()..addRect(clipBounds)..addOval(clipBounds); + canvas.clipPath(clip); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipExpandedBounds)); + + canvas.save(); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + // Both clip bounds have changed + expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getDestinationClipBounds(), notCloseToRect(clipExpandedBounds)); + // Previous return values have not changed + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipExpandedBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); + expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + // Destination bounds are unaffected by transform + expect(canvas.getDestinationClipBounds(), closeToRect(clipExpandedBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + + test('Canvas.clipPath(doAA=false) affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); From c6f674c902975cef8dd028ecb61e158a2af22228 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 20 Dec 2022 17:21:08 +0800 Subject: [PATCH 8/8] remove virtual --- display_list/display_list_matrix_clip_tracker.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/display_list/display_list_matrix_clip_tracker.cc b/display_list/display_list_matrix_clip_tracker.cc index fea6ba780c778..d79f6933aa0c1 100644 --- a/display_list/display_list_matrix_clip_tracker.cc +++ b/display_list/display_list_matrix_clip_tracker.cc @@ -84,9 +84,7 @@ class Data3x3 : public DisplayListMatrixClipTracker::Data { bool mapRect(const SkRect& rect, SkRect* mapped) const override { return matrix_.mapRect(mapped, rect); } - virtual bool canBeInverted() const override { - return matrix_.invert(nullptr); - } + bool canBeInverted() const override { return matrix_.invert(nullptr); } protected: bool has_perspective() const override { return matrix_.hasPerspective(); }