From d757fd805ca4028914a19f24041dc5b2754ca39c Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Thu, 28 Jul 2022 20:42:02 +0800 Subject: [PATCH 1/7] drafting the solution to optimize out unnecessary save restore pairs --- display_list/display_list_builder.cc | 31 +++++++++++- display_list/display_list_builder.h | 8 ++++ display_list/display_list_unittests.cc | 33 +++++++++++++ display_list/display_list_utils.cc | 65 +++++++++++++++++++++++++- display_list/display_list_utils.h | 19 ++++++++ 5 files changed, 154 insertions(+), 2 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 970e019ff56bf..d0e9582e856f9 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -4,6 +4,7 @@ #include "flutter/display_list/display_list_builder.h" +#include "flutter/display_list/display_list.h" #include "flutter/display_list/display_list_blend_mode.h" #include "flutter/display_list/display_list_ops.h" @@ -413,12 +414,29 @@ void DisplayListBuilder::setAttributesFromPaint( } } -void DisplayListBuilder::save() { +void DisplayListBuilder::checkForDeferredSave() { + if (current_layer_->deferred_save_count_ > 0) { + doSave(); + } +} + +void DisplayListBuilder::doSave() { Push(0, 1); + save_count_ += 1; + current_layer_->deferred_save_count_ -= 1; layer_stack_.emplace_back(current_layer_); current_layer_ = &layer_stack_.back(); } + +void DisplayListBuilder::save() { + current_layer_->deferred_save_count_ += 1; +} + void DisplayListBuilder::restore() { + if (current_layer_->deferred_save_count_ > 0) { + current_layer_->deferred_save_count_ -= 1; + return; + } if (layer_stack_.size() > 1) { // Grab the current layer info before we push the restore // on the stack. @@ -504,6 +522,7 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, void DisplayListBuilder::translate(SkScalar tx, SkScalar ty) { if (SkScalarIsFinite(tx) && SkScalarIsFinite(ty) && (tx != 0.0 || ty != 0.0)) { + checkForDeferredSave(); Push(0, 1, tx, ty); current_layer_->matrix.preTranslate(tx, ty); } @@ -511,12 +530,14 @@ void DisplayListBuilder::translate(SkScalar tx, SkScalar ty) { void DisplayListBuilder::scale(SkScalar sx, SkScalar sy) { if (SkScalarIsFinite(sx) && SkScalarIsFinite(sy) && (sx != 1.0 || sy != 1.0)) { + checkForDeferredSave(); Push(0, 1, sx, sy); current_layer_->matrix.preScale(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)); } @@ -524,6 +545,7 @@ void DisplayListBuilder::rotate(SkScalar degrees) { void DisplayListBuilder::skew(SkScalar sx, SkScalar sy) { if (SkScalarIsFinite(sx) && SkScalarIsFinite(sy) && (sx != 0.0 || sy != 0.0)) { + checkForDeferredSave(); Push(0, 1, sx, sy); current_layer_->matrix.preConcat(SkMatrix::Skew(sx, sy)); } @@ -540,6 +562,7 @@ void DisplayListBuilder::transform2DAffine( SkScalarsAreFinite(mxt, myt) && !(mxx == 1 && mxy == 0 && mxt == 0 && myx == 0 && myy == 1 && myt == 0)) { + checkForDeferredSave(); Push(0, 1, mxx, mxy, mxt, myx, myy, myt); @@ -599,6 +622,10 @@ void DisplayListBuilder::transform(const SkM44* m44) { void DisplayListBuilder::clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) { + if (!rect.isFinite()) { + return; + } + checkForDeferredSave(); switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, rect, is_aa); @@ -612,6 +639,7 @@ void DisplayListBuilder::clipRect(const SkRect& rect, void DisplayListBuilder::clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) { + checkForDeferredSave(); if (rrect.isRect()) { clipRect(rrect.rect(), clip_op, is_aa); } else { @@ -629,6 +657,7 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect, void DisplayListBuilder::clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) { + checkForDeferredSave(); if (!path.isInverseFillType()) { SkRect rect; if (path.isRect(&rect)) { diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index ce785c594b7ad..a081b7fa7d434 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -342,7 +342,12 @@ class DisplayListBuilder final : public virtual Dispatcher, sk_sp Build(); + unsigned save_count() const { return save_count_; } + private: + void doSave(); + void checkForDeferredSave(); + SkAutoTMalloc storage_; size_t used_ = 0; size_t allocated_ = 0; @@ -397,6 +402,8 @@ class DisplayListBuilder final : public virtual Dispatcher, // This offset is only valid if |has_layer| is true. size_t save_layer_offset; + unsigned deferred_save_count_ = 0; + bool has_layer; bool cannot_inherit_opacity; bool has_compatible_op; @@ -426,6 +433,7 @@ class DisplayListBuilder final : public virtual Dispatcher, std::vector layer_stack_; LayerInfo* current_layer_; + unsigned save_count_ = 0; // This flag indicates whether or not the current rendering attributes // are compatible with rendering ops applying an inherited opacity. diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index ea8dfc0cbee44..fa060d973327a 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1259,6 +1259,7 @@ TEST(DisplayList, ClipRectAffectsClipBounds) { // save/restore returned the values to their original values ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); + ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { @@ -1278,6 +1279,7 @@ TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { builder.clipRect(clipBounds2, SkClipOp::kIntersect, false); ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds1); builder.restore(); + ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipRRectAffectsClipBounds) { @@ -1318,6 +1320,7 @@ TEST(DisplayList, ClipRRectAffectsClipBounds) { // save/restore returned the values to their original values ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); + ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) { @@ -1340,6 +1343,7 @@ TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) { builder.clipRRect(clip2, SkClipOp::kIntersect, false); ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds1); builder.restore(); + ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipPathAffectsClipBounds) { @@ -1380,6 +1384,7 @@ TEST(DisplayList, ClipPathAffectsClipBounds) { // save/restore returned the values to their original values ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); + ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) { @@ -1401,6 +1406,7 @@ TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) { builder.clipPath(clip2, SkClipOp::kIntersect, false); ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds); builder.restore(); + ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, DiffClipRectDoesNotAffectClipBounds) { @@ -1586,6 +1592,7 @@ TEST(DisplayList, RTreeOfSaveRestoreScene) { builder.save(); builder.drawRect({50, 50, 60, 60}); builder.restore(); + ASSERT_EQ(builder.save_count(), 0u); auto display_list = builder.Build(); auto rtree = display_list->rtree(); std::vector rects = { @@ -1644,6 +1651,32 @@ TEST(DisplayList, RTreeOfSaveLayerFilterScene) { test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1}); } +TEST(DisplayList, RemoveUnnecessarySaveRestorePairs) { + { + DisplayListBuilder builder; + builder.drawRect({10, 10, 20, 20}); + builder.save(); // This save op is unnecessary + builder.drawRect({50, 50, 60, 60}); + builder.restore(); + ASSERT_EQ(builder.save_count(), 0u); + } + + { + DisplayListBuilder builder; + builder.drawRect({10, 10, 20, 20}); + builder.save(); + builder.translate(1.0, 1.0); + { + builder.save(); // unnecessary + builder.drawRect({50, 50, 60, 60}); + builder.restore(); + } + + builder.restore(); + ASSERT_EQ(builder.save_count(), 1u); + } +} + TEST(DisplayList, RTreeOfClippedSaveLayerFilterScene) { DisplayListBuilder builder; // blur filter with sigma=1 expands by 30 on all sides diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index 58ddba1425c46..9ea8af4a653f6 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -165,6 +165,7 @@ void SkMatrixDispatchHelper::transformReset() { void SkMatrixDispatchHelper::save() { saved_.push_back(matrix_); } + void SkMatrixDispatchHelper::restore() { if (saved_.empty()) { return; @@ -423,12 +424,24 @@ void DisplayListBoundsCalculator::setPathEffect(const DlPathEffect* effect) { void DisplayListBoundsCalculator::setMaskFilter(const DlMaskFilter* filter) { mask_filter_ = filter ? filter->shared() : nullptr; } -void DisplayListBoundsCalculator::save() { + +void DisplayListBoundsCalculator::doSave() { SkMatrixDispatchHelper::save(); ClipBoundsDispatchHelper::save(); + current_layer()->deferred_save_count_ -= 1; layer_infos_.emplace_back(std::make_unique(nullptr)); accumulator_.save(); } + +void DisplayListBoundsCalculator::checkForDeferredSave() { + if (current_layer()->deferred_save_count_ > 0) { + doSave(); + } +} + +void DisplayListBoundsCalculator::save() { + current_layer()->deferred_save_count_ += 1; +} void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { @@ -463,6 +476,10 @@ void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, } } void DisplayListBoundsCalculator::restore() { + if (current_layer()->deferred_save_count_ > 0) { + current_layer()->deferred_save_count_ -= 1; + return; + } if (layer_infos_.size() > 1) { SkMatrixDispatchHelper::restore(); ClipBoundsDispatchHelper::restore(); @@ -503,6 +520,52 @@ void DisplayListBoundsCalculator::restore() { } } +void DisplayListBoundsCalculator::transform2DAffine(SkScalar mxx, + SkScalar mxy, + SkScalar mxt, + SkScalar myx, + SkScalar myy, + SkScalar myt) { + checkForDeferredSave(); + SkMatrixDispatchHelper::transform2DAffine(mxx, mxy, mxt, myx, myy, myt); +} + +void DisplayListBoundsCalculator::translate(SkScalar tx, SkScalar ty) { + checkForDeferredSave(); + SkMatrixDispatchHelper::translate(tx, ty); +} + +void DisplayListBoundsCalculator::scale(SkScalar sx, SkScalar sy) { + checkForDeferredSave(); + SkMatrixDispatchHelper::scale(sx, sy); +} +void DisplayListBoundsCalculator::rotate(SkScalar degrees) { + checkForDeferredSave(); + SkMatrixDispatchHelper::rotate(degrees); +} +void DisplayListBoundsCalculator::skew(SkScalar sx, SkScalar sy) { + checkForDeferredSave(); + SkMatrixDispatchHelper::skew(sx, sy); +} +void DisplayListBoundsCalculator::clipRect(const SkRect& rect, + SkClipOp clip_op, + bool is_aa) { + checkForDeferredSave(); + ClipBoundsDispatchHelper::clipRect(rect, clip_op, is_aa); +} +void DisplayListBoundsCalculator::clipRRect(const SkRRect& rrect, + SkClipOp clip_op, + bool is_aa) { + checkForDeferredSave(); + ClipBoundsDispatchHelper::clipRRect(rrect, clip_op, is_aa); +} +void DisplayListBoundsCalculator::clipPath(const SkPath& path, + SkClipOp clip_op, + bool is_aa) { + checkForDeferredSave(); + ClipBoundsDispatchHelper::clipPath(path, clip_op, is_aa); +} + void DisplayListBoundsCalculator::drawPaint() { AccumulateUnbounded(); } diff --git a/display_list/display_list_utils.h b/display_list/display_list_utils.h index ff2bad1544fd7..550132a3375c5 100644 --- a/display_list/display_list_utils.h +++ b/display_list/display_list_utils.h @@ -559,6 +559,19 @@ class DisplayListBoundsCalculator final const SkScalar elevation, bool transparent_occluder, SkScalar dpr) override; + void transform2DAffine(SkScalar mxx, + SkScalar mxy, + SkScalar mxt, + SkScalar myx, + SkScalar myy, + SkScalar myt) override; + void translate(SkScalar tx, SkScalar ty) override; + void scale(SkScalar sx, SkScalar sy) override; + void rotate(SkScalar degrees) override; + void skew(SkScalar sx, SkScalar sy) override; + 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; // The DisplayList had an unbounded call with no cull rect or clip // to contain it. Should only be called after the stream is fully @@ -577,6 +590,9 @@ class DisplayListBoundsCalculator final private: BoundsAccumulator& accumulator_; + void doSave(); + void checkForDeferredSave(); + // A class that remembers the information kept for a single // |save| or |saveLayer|. // Each save or saveLayer will maintain its own bounds accumulator @@ -630,6 +646,7 @@ class DisplayListBoundsCalculator final *output = input; return true; } + unsigned deferred_save_count_ = 0; private: std::shared_ptr filter_; @@ -638,6 +655,8 @@ class DisplayListBoundsCalculator final FML_DISALLOW_COPY_AND_ASSIGN(LayerData); }; + auto* current_layer() const { return layer_infos_.back().get(); } + std::vector> layer_infos_; static constexpr SkScalar kMinStrokeWidth = 0.01; From f1f376b18b0bf1ad84912aa22d6ff5b5368d0fa8 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Fri, 29 Jul 2022 15:07:21 +0800 Subject: [PATCH 2/7] remove unnecessary save/restore pairs --- display_list/display_list_builder.cc | 21 +++----- display_list/display_list_builder.h | 6 +-- display_list/display_list_unittests.cc | 72 ++++++++++++++++++++++---- display_list/display_list_utils.cc | 11 ++-- display_list/display_list_utils.h | 5 +- 5 files changed, 80 insertions(+), 35 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index d0e9582e856f9..6df33b0736c58 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -415,27 +415,21 @@ void DisplayListBuilder::setAttributesFromPaint( } void DisplayListBuilder::checkForDeferredSave() { - if (current_layer_->deferred_save_count_ > 0) { - doSave(); + if (current_layer_->has_deferred_save_op_) { + Push(0, 1); + current_layer_->has_deferred_save_op_ = false; } } -void DisplayListBuilder::doSave() { - Push(0, 1); - save_count_ += 1; - current_layer_->deferred_save_count_ -= 1; +void DisplayListBuilder::save() { layer_stack_.emplace_back(current_layer_); current_layer_ = &layer_stack_.back(); -} - -void DisplayListBuilder::save() { - current_layer_->deferred_save_count_ += 1; + current_layer_->has_deferred_save_op_ = true; } void DisplayListBuilder::restore() { - if (current_layer_->deferred_save_count_ > 0) { - current_layer_->deferred_save_count_ -= 1; - return; + if (!current_layer_->has_deferred_save_op_) { + Push(0, 1); } if (layer_stack_.size() > 1) { // Grab the current layer info before we push the restore @@ -443,7 +437,6 @@ void DisplayListBuilder::restore() { LayerInfo layer_info = layer_stack_.back(); layer_stack_.pop_back(); current_layer_ = &layer_stack_.back(); - Push(0, 1); if (layer_info.has_layer) { if (layer_info.is_group_opacity_compatible()) { // We are now going to go back and modify the matching saveLayer diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index a081b7fa7d434..759b7280ed0f1 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -342,10 +342,7 @@ class DisplayListBuilder final : public virtual Dispatcher, sk_sp Build(); - unsigned save_count() const { return save_count_; } - private: - void doSave(); void checkForDeferredSave(); SkAutoTMalloc storage_; @@ -402,7 +399,7 @@ class DisplayListBuilder final : public virtual Dispatcher, // This offset is only valid if |has_layer| is true. size_t save_layer_offset; - unsigned deferred_save_count_ = 0; + bool has_deferred_save_op_ = false; bool has_layer; bool cannot_inherit_opacity; @@ -433,7 +430,6 @@ class DisplayListBuilder final : public virtual Dispatcher, std::vector layer_stack_; LayerInfo* current_layer_; - unsigned save_count_ = 0; // This flag indicates whether or not the current rendering attributes // are compatible with rendering ops applying an inherited opacity. diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index fa060d973327a..304e6aee6d394 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1259,7 +1259,6 @@ TEST(DisplayList, ClipRectAffectsClipBounds) { // save/restore returned the values to their original values ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); - ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { @@ -1279,7 +1278,6 @@ TEST(DisplayList, ClipRectAffectsClipBoundsWithMatrix) { builder.clipRect(clipBounds2, SkClipOp::kIntersect, false); ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds1); builder.restore(); - ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipRRectAffectsClipBounds) { @@ -1320,7 +1318,6 @@ TEST(DisplayList, ClipRRectAffectsClipBounds) { // save/restore returned the values to their original values ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); - ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) { @@ -1343,7 +1340,6 @@ TEST(DisplayList, ClipRRectAffectsClipBoundsWithMatrix) { builder.clipRRect(clip2, SkClipOp::kIntersect, false); ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds1); builder.restore(); - ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipPathAffectsClipBounds) { @@ -1384,7 +1380,6 @@ TEST(DisplayList, ClipPathAffectsClipBounds) { // save/restore returned the values to their original values ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); - ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) { @@ -1406,7 +1401,6 @@ TEST(DisplayList, ClipPathAffectsClipBoundsWithMatrix) { builder.clipPath(clip2, SkClipOp::kIntersect, false); ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds); builder.restore(); - ASSERT_EQ(builder.save_count(), 2u); } TEST(DisplayList, DiffClipRectDoesNotAffectClipBounds) { @@ -1592,7 +1586,6 @@ TEST(DisplayList, RTreeOfSaveRestoreScene) { builder.save(); builder.drawRect({50, 50, 60, 60}); builder.restore(); - ASSERT_EQ(builder.save_count(), 0u); auto display_list = builder.Build(); auto rtree = display_list->rtree(); std::vector rects = { @@ -1658,7 +1651,11 @@ TEST(DisplayList, RemoveUnnecessarySaveRestorePairs) { builder.save(); // This save op is unnecessary builder.drawRect({50, 50, 60, 60}); builder.restore(); - ASSERT_EQ(builder.save_count(), 0u); + + DisplayListBuilder builder2; + builder2.drawRect({10, 10, 20, 20}); + builder2.drawRect({50, 50, 60, 60}); + ASSERT_TRUE(DisplayListsEQ_Verbose(builder.Build(), builder2.Build())); } { @@ -1673,10 +1670,67 @@ TEST(DisplayList, RemoveUnnecessarySaveRestorePairs) { } builder.restore(); - ASSERT_EQ(builder.save_count(), 1u); + + DisplayListBuilder builder2; + builder2.drawRect({10, 10, 20, 20}); + builder2.save(); + builder2.translate(1.0, 1.0); + { builder2.drawRect({50, 50, 60, 60}); } + builder2.restore(); + ASSERT_TRUE(DisplayListsEQ_Verbose(builder.Build(), builder2.Build())); } } +TEST(DisplayList, CollapseMultipleNestedSaveRestore) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.save(); + builder1.translate(10, 10); + builder1.scale(2, 2); + builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.translate(10, 10); + builder2.scale(2, 2); + builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, CollapseNestedSaveAndSaveLayerRestore) { + DisplayListBuilder builder1; + builder1.save(); + builder1.saveLayer(nullptr, false); + builder1.translate(10, 10); + builder1.scale(2, 2); + builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.saveLayer(nullptr, false); + builder2.translate(10, 10); + builder2.scale(2, 2); + builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + TEST(DisplayList, RTreeOfClippedSaveLayerFilterScene) { DisplayListBuilder builder; // blur filter with sigma=1 expands by 30 on all sides diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index 9ea8af4a653f6..2ae2c9c2e93ca 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -428,20 +428,21 @@ void DisplayListBoundsCalculator::setMaskFilter(const DlMaskFilter* filter) { void DisplayListBoundsCalculator::doSave() { SkMatrixDispatchHelper::save(); ClipBoundsDispatchHelper::save(); - current_layer()->deferred_save_count_ -= 1; layer_infos_.emplace_back(std::make_unique(nullptr)); accumulator_.save(); } void DisplayListBoundsCalculator::checkForDeferredSave() { - if (current_layer()->deferred_save_count_ > 0) { + if (current_layer()->has_deferred_save_op_) { + current_layer()->has_deferred_save_op_ = false; doSave(); } } void DisplayListBoundsCalculator::save() { - current_layer()->deferred_save_count_ += 1; + current_layer()->has_deferred_save_op_ = true; } + void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { @@ -476,8 +477,8 @@ void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, } } void DisplayListBoundsCalculator::restore() { - if (current_layer()->deferred_save_count_ > 0) { - current_layer()->deferred_save_count_ -= 1; + if (current_layer()->has_deferred_save_op_) { + current_layer()->has_deferred_save_op_ = false; return; } if (layer_infos_.size() > 1) { diff --git a/display_list/display_list_utils.h b/display_list/display_list_utils.h index 550132a3375c5..f222136a0d3c8 100644 --- a/display_list/display_list_utils.h +++ b/display_list/display_list_utils.h @@ -606,7 +606,7 @@ class DisplayListBoundsCalculator final // |DlImageFilter| when they are restored, but for most // saveLayer (and all save) calls the filter will be null. explicit LayerData(std::shared_ptr filter = nullptr) - : filter_(filter), is_unbounded_(false) {} + : has_deferred_save_op_(false), filter_(filter), is_unbounded_(false) {} ~LayerData() = default; // The filter to apply to the layer bounds when it is restored @@ -646,7 +646,8 @@ class DisplayListBoundsCalculator final *output = input; return true; } - unsigned deferred_save_count_ = 0; + + bool has_deferred_save_op_ = true; private: std::shared_ptr filter_; From 0328e2e8e04d5b5404993eff14a59b98c9a08a68 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Fri, 29 Jul 2022 16:27:02 +0800 Subject: [PATCH 3/7] delete the calculator change; --- display_list/display_list_builder.cc | 6 +-- display_list/display_list_utils.cc | 66 +--------------------------- display_list/display_list_utils.h | 22 +--------- 3 files changed, 5 insertions(+), 89 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 6df33b0736c58..d25f8a0161330 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -428,10 +428,10 @@ void DisplayListBuilder::save() { } void DisplayListBuilder::restore() { - if (!current_layer_->has_deferred_save_op_) { - Push(0, 1); - } if (layer_stack_.size() > 1) { + if (!current_layer_->has_deferred_save_op_) { + Push(0, 1); + } // Grab the current layer info before we push the restore // on the stack. LayerInfo layer_info = layer_stack_.back(); diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index 2ae2c9c2e93ca..58ddba1425c46 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -165,7 +165,6 @@ void SkMatrixDispatchHelper::transformReset() { void SkMatrixDispatchHelper::save() { saved_.push_back(matrix_); } - void SkMatrixDispatchHelper::restore() { if (saved_.empty()) { return; @@ -424,25 +423,12 @@ void DisplayListBoundsCalculator::setPathEffect(const DlPathEffect* effect) { void DisplayListBoundsCalculator::setMaskFilter(const DlMaskFilter* filter) { mask_filter_ = filter ? filter->shared() : nullptr; } - -void DisplayListBoundsCalculator::doSave() { +void DisplayListBoundsCalculator::save() { SkMatrixDispatchHelper::save(); ClipBoundsDispatchHelper::save(); layer_infos_.emplace_back(std::make_unique(nullptr)); accumulator_.save(); } - -void DisplayListBoundsCalculator::checkForDeferredSave() { - if (current_layer()->has_deferred_save_op_) { - current_layer()->has_deferred_save_op_ = false; - doSave(); - } -} - -void DisplayListBoundsCalculator::save() { - current_layer()->has_deferred_save_op_ = true; -} - void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, const SaveLayerOptions options, const DlImageFilter* backdrop) { @@ -477,10 +463,6 @@ void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, } } void DisplayListBoundsCalculator::restore() { - if (current_layer()->has_deferred_save_op_) { - current_layer()->has_deferred_save_op_ = false; - return; - } if (layer_infos_.size() > 1) { SkMatrixDispatchHelper::restore(); ClipBoundsDispatchHelper::restore(); @@ -521,52 +503,6 @@ void DisplayListBoundsCalculator::restore() { } } -void DisplayListBoundsCalculator::transform2DAffine(SkScalar mxx, - SkScalar mxy, - SkScalar mxt, - SkScalar myx, - SkScalar myy, - SkScalar myt) { - checkForDeferredSave(); - SkMatrixDispatchHelper::transform2DAffine(mxx, mxy, mxt, myx, myy, myt); -} - -void DisplayListBoundsCalculator::translate(SkScalar tx, SkScalar ty) { - checkForDeferredSave(); - SkMatrixDispatchHelper::translate(tx, ty); -} - -void DisplayListBoundsCalculator::scale(SkScalar sx, SkScalar sy) { - checkForDeferredSave(); - SkMatrixDispatchHelper::scale(sx, sy); -} -void DisplayListBoundsCalculator::rotate(SkScalar degrees) { - checkForDeferredSave(); - SkMatrixDispatchHelper::rotate(degrees); -} -void DisplayListBoundsCalculator::skew(SkScalar sx, SkScalar sy) { - checkForDeferredSave(); - SkMatrixDispatchHelper::skew(sx, sy); -} -void DisplayListBoundsCalculator::clipRect(const SkRect& rect, - SkClipOp clip_op, - bool is_aa) { - checkForDeferredSave(); - ClipBoundsDispatchHelper::clipRect(rect, clip_op, is_aa); -} -void DisplayListBoundsCalculator::clipRRect(const SkRRect& rrect, - SkClipOp clip_op, - bool is_aa) { - checkForDeferredSave(); - ClipBoundsDispatchHelper::clipRRect(rrect, clip_op, is_aa); -} -void DisplayListBoundsCalculator::clipPath(const SkPath& path, - SkClipOp clip_op, - bool is_aa) { - checkForDeferredSave(); - ClipBoundsDispatchHelper::clipPath(path, clip_op, is_aa); -} - void DisplayListBoundsCalculator::drawPaint() { AccumulateUnbounded(); } diff --git a/display_list/display_list_utils.h b/display_list/display_list_utils.h index f222136a0d3c8..ff2bad1544fd7 100644 --- a/display_list/display_list_utils.h +++ b/display_list/display_list_utils.h @@ -559,19 +559,6 @@ class DisplayListBoundsCalculator final const SkScalar elevation, bool transparent_occluder, SkScalar dpr) override; - void transform2DAffine(SkScalar mxx, - SkScalar mxy, - SkScalar mxt, - SkScalar myx, - SkScalar myy, - SkScalar myt) override; - void translate(SkScalar tx, SkScalar ty) override; - void scale(SkScalar sx, SkScalar sy) override; - void rotate(SkScalar degrees) override; - void skew(SkScalar sx, SkScalar sy) override; - 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; // The DisplayList had an unbounded call with no cull rect or clip // to contain it. Should only be called after the stream is fully @@ -590,9 +577,6 @@ class DisplayListBoundsCalculator final private: BoundsAccumulator& accumulator_; - void doSave(); - void checkForDeferredSave(); - // A class that remembers the information kept for a single // |save| or |saveLayer|. // Each save or saveLayer will maintain its own bounds accumulator @@ -606,7 +590,7 @@ class DisplayListBoundsCalculator final // |DlImageFilter| when they are restored, but for most // saveLayer (and all save) calls the filter will be null. explicit LayerData(std::shared_ptr filter = nullptr) - : has_deferred_save_op_(false), filter_(filter), is_unbounded_(false) {} + : filter_(filter), is_unbounded_(false) {} ~LayerData() = default; // The filter to apply to the layer bounds when it is restored @@ -647,8 +631,6 @@ class DisplayListBoundsCalculator final return true; } - bool has_deferred_save_op_ = true; - private: std::shared_ptr filter_; bool is_unbounded_; @@ -656,8 +638,6 @@ class DisplayListBoundsCalculator final FML_DISALLOW_COPY_AND_ASSIGN(LayerData); }; - auto* current_layer() const { return layer_infos_.back().get(); } - std::vector> layer_infos_; static constexpr SkScalar kMinStrokeWidth = 0.01; From d7d374fde63139ac93f0fd2ef4c9d3f96cd2159b Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Wed, 3 Aug 2022 16:32:57 +0800 Subject: [PATCH 4/7] fix some logic; Add some testcases --- display_list/display_list_builder.cc | 6 +- display_list/display_list_unittests.cc | 189 ++++++++++++++++++++++--- 2 files changed, 174 insertions(+), 21 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index d25f8a0161330..ee4d5a3e49dc3 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -581,6 +581,7 @@ void DisplayListBuilder::transformFullPerspective( SkScalarsAreFinite(myx, myy) && SkScalarsAreFinite(myz, myt) && SkScalarsAreFinite(mzx, mzy) && SkScalarsAreFinite(mzz, mzt) && SkScalarsAreFinite(mwx, mwy) && SkScalarsAreFinite(mwz, mwt)) { + checkForDeferredSave(); Push(0, 1, mxx, mxy, mxz, mxt, myx, myy, myz, myt, @@ -594,6 +595,7 @@ void DisplayListBuilder::transformFullPerspective( } // clang-format on void DisplayListBuilder::transformReset() { + checkForDeferredSave(); Push(0, 0); current_layer_->matrix.setIdentity(); } @@ -632,10 +634,10 @@ void DisplayListBuilder::clipRect(const SkRect& rect, void DisplayListBuilder::clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) { - checkForDeferredSave(); if (rrect.isRect()) { clipRect(rrect.rect(), clip_op, is_aa); } else { + checkForDeferredSave(); switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, rrect, is_aa); @@ -650,7 +652,6 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect, void DisplayListBuilder::clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) { - checkForDeferredSave(); if (!path.isInverseFillType()) { SkRect rect; if (path.isRect(&rect)) { @@ -668,6 +669,7 @@ void DisplayListBuilder::clipPath(const SkPath& path, return; } } + checkForDeferredSave(); switch (clip_op) { case SkClipOp::kIntersect: Push(0, 1, path, is_aa); diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 304e6aee6d394..db8370e80c6e5 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -3,6 +3,9 @@ // found in the LICENSE file. #include +#include +#include +#include #include "flutter/display_list/display_list.h" #include "flutter/display_list/display_list_blend_mode.h" @@ -1708,27 +1711,175 @@ TEST(DisplayList, CollapseMultipleNestedSaveRestore) { } TEST(DisplayList, CollapseNestedSaveAndSaveLayerRestore) { - DisplayListBuilder builder1; - builder1.save(); - builder1.saveLayer(nullptr, false); - builder1.translate(10, 10); - builder1.scale(2, 2); - builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - builder1.restore(); - auto display_list1 = builder1.Build(); + // Translate + { + DisplayListBuilder builder1; + builder1.save(); + builder1.saveLayer(nullptr, false); + builder1.translate(10, 10); + builder1.scale(2, 2); + builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); - DisplayListBuilder builder2; - builder2.saveLayer(nullptr, false); - builder2.translate(10, 10); - builder2.scale(2, 2); - builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); + DisplayListBuilder builder2; + builder2.saveLayer(nullptr, false); + builder2.translate(10, 10); + builder2.scale(2, 2); + builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + // Transform + { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transform(SkM44::Translate(10, 100)); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44::Translate(10, 100)); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.transform(SkM44::Translate(10, 100)); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + builder2.save(); + builder2.transform(SkM44::Translate(10, 100)); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + // Skew + { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.skew(10, 10); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.skew(10, 10); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + + // Scale + { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.scale(0.5, 0.5); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.scale(0.5, 0.5); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + + { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + // Clip + { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.clipRect(SkRect::MakeLTRB(0, 0, 100, 100), SkClipOp::kIntersect, + true); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.clipRect(SkRect::MakeLTRB(0, 0, 100, 100), SkClipOp::kIntersect, + true); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + builder2.transform(SkM44()); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + + std::unordered_set function_names = {"Translate", + "Scale", + "Skew", + "Transform2d", + "TransformFullPerspective", + "ClipRect", + "ClipPath", + "ClipRRect"}; + + for (auto& group : allGroups) { + if (function_names.find(group.op_name) == function_names.cend()) { + continue; + } + for (size_t i = 0; i < group.variants.size(); i++) { + auto& invocation = group.variants[i]; + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + invocation.invoker(builder1); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + invocation.invoker(builder2); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + } } TEST(DisplayList, RTreeOfClippedSaveLayerFilterScene) { From 04f303f01069d1811fce60c3452896fc7246a3b3 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Mon, 8 Aug 2022 15:18:41 +0800 Subject: [PATCH 5/7] Add test for set DlPaint --- display_list/display_list_unittests.cc | 57 ++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index db8370e80c6e5..f8a9a201ca7f0 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -11,9 +11,11 @@ #include "flutter/display_list/display_list_blend_mode.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_canvas_recorder.h" +#include "flutter/display_list/display_list_paint.h" #include "flutter/display_list/display_list_rtree.h" #include "flutter/display_list/display_list_test_utils.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" @@ -1710,6 +1712,61 @@ TEST(DisplayList, CollapseMultipleNestedSaveRestore) { ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); } +TEST(DisplayList, RemoveUnnecessarySaveRestorePairsInSetPaint) { + SkRect build_bounds = SkRect::MakeLTRB(-100, -100, 200, 200); + SkRect rect = SkRect::MakeLTRB(30, 30, 70, 70); + // clang-format off + const float alpha_matrix[] = { + 0, 0, 0, 0, 0, + 0, 1, 0, 0, 0, + 0, 0, 1, 0, 0, + 0, 0, 0, 0, 1, + }; + // clang-format on + DlMatrixColorFilter alpha_color_filter(alpha_matrix); + // Making sure hiding a problematic ColorFilter as an ImageFilter + // will generate the same behavior as setting it as a ColorFilter + + DlColorFilterImageFilter color_filter_image_filter(alpha_color_filter); + { + DisplayListBuilder builder(build_bounds); + builder.save(); + DlPaint paint; + paint.setImageFilter(&color_filter_image_filter); + builder.drawRect(rect, paint); + builder.restore(); + sk_sp display_list1 = builder.Build(); + + DisplayListBuilder builder2(build_bounds); + DlPaint paint2; + paint2.setImageFilter(&color_filter_image_filter); + builder2.drawRect(rect, paint2); + sk_sp display_list2 = builder2.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } + + { + DisplayListBuilder builder(build_bounds); + builder.save(); + builder.saveLayer(&build_bounds, true); + DlPaint paint; + paint.setImageFilter(&color_filter_image_filter); + builder.drawRect(rect, paint); + builder.restore(); + builder.restore(); + sk_sp display_list1 = builder.Build(); + + DisplayListBuilder builder2(build_bounds); + builder2.saveLayer(&build_bounds, true); + DlPaint paint2; + paint2.setImageFilter(&color_filter_image_filter); + builder2.drawRect(rect, paint2); + builder2.restore(); + sk_sp display_list2 = builder2.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); + } +} + TEST(DisplayList, CollapseNestedSaveAndSaveLayerRestore) { // Translate { From d280061d7d2b1e2d21c4eb782eed1433312b1ab6 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Tue, 9 Aug 2022 11:20:08 +0800 Subject: [PATCH 6/7] update test cases --- display_list/display_list_unittests.cc | 501 +++++++++++++++++++------ 1 file changed, 379 insertions(+), 122 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index f8a9a201ca7f0..5c4fa1f3c1efb 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1712,6 +1712,26 @@ TEST(DisplayList, CollapseMultipleNestedSaveRestore) { ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); } +TEST(DisplayList, CollapseNestedSaveAndSaveLayerRestore) { + DisplayListBuilder builder1; + builder1.save(); + builder1.saveLayer(nullptr, false); + builder1.drawRect({0, 0, 100, 100}); + builder1.scale(2, 2); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.saveLayer(nullptr, false); + builder2.drawRect({0, 0, 100, 100}); + builder2.scale(2, 2); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + TEST(DisplayList, RemoveUnnecessarySaveRestorePairsInSetPaint) { SkRect build_bounds = SkRect::MakeLTRB(-100, -100, 200, 200); SkRect rect = SkRect::MakeLTRB(30, 30, 70, 70); @@ -1767,105 +1787,357 @@ TEST(DisplayList, RemoveUnnecessarySaveRestorePairsInSetPaint) { } } -TEST(DisplayList, CollapseNestedSaveAndSaveLayerRestore) { - // Translate - { - DisplayListBuilder builder1; - builder1.save(); - builder1.saveLayer(nullptr, false); - builder1.translate(10, 10); - builder1.scale(2, 2); - builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - builder1.restore(); - auto display_list1 = builder1.Build(); +TEST(DisplayList, TranslateTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.saveLayer(nullptr, false); + builder1.translate(10, 10); + builder1.scale(2, 2); + builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); - DisplayListBuilder builder2; - builder2.saveLayer(nullptr, false); - builder2.translate(10, 10); - builder2.scale(2, 2); - builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); + DisplayListBuilder builder2; + builder2.saveLayer(nullptr, false); + builder2.translate(10, 10); + builder2.scale(2, 2); + builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); - } - // Transform - { - DisplayListBuilder builder1; - builder1.save(); - builder1.save(); - builder1.transform(SkM44::Translate(10, 100)); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - builder1.transform(SkM44::Translate(10, 100)); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - auto display_list1 = builder1.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} - DisplayListBuilder builder2; - builder2.save(); - builder2.transform(SkM44::Translate(10, 100)); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - builder2.save(); - builder2.transform(SkM44::Translate(10, 100)); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); +TEST(DisplayList, TransformTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transform(SkM44::Translate(10, 100)); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44::Translate(10, 100)); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); - } - // Skew - { - DisplayListBuilder builder1; - builder1.save(); - builder1.save(); - builder1.skew(10, 10); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - auto display_list1 = builder1.Build(); + DisplayListBuilder builder2; + builder2.save(); + builder2.transform(SkM44::Translate(10, 100)); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + builder2.save(); + builder2.transform(SkM44::Translate(10, 100)); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); - DisplayListBuilder builder2; - builder2.save(); - builder2.skew(10, 10); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); - } +TEST(DisplayList, Transform2DTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transform2DAffine(0, 1, 12, 1, 0, 33); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); - // Scale - { - DisplayListBuilder builder1; - builder1.save(); - builder1.save(); - builder1.scale(0.5, 0.5); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - auto display_list1 = builder1.Build(); + DisplayListBuilder builder2; + builder2.save(); + builder2.transform2DAffine(0, 1, 12, 1, 0, 33); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); - DisplayListBuilder builder2; - builder2.save(); - builder2.scale(0.5, 0.5); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); - } +TEST(DisplayList, TransformPerspectiveTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transformFullPerspective(0, 1, 0, 12, 1, 0, 0, 33, 3, 2, 5, 29, 0, 0, + 0, 12); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.transformFullPerspective(0, 1, 0, 12, 1, 0, 0, 33, 3, 2, 5, 29, 0, 0, + 0, 12); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, ResetTransformTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transformReset(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.transformReset(); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, SkewTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.skew(10, 10); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.skew(10, 10); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} +TEST(DisplayList, ScaleTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.scale(0.5, 0.5); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.scale(0.5, 0.5); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, ClipRectTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.clipRect(SkRect::MakeLTRB(0, 0, 100, 100), SkClipOp::kIntersect, + true); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.clipRect(SkRect::MakeLTRB(0, 0, 100, 100), SkClipOp::kIntersect, + true); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + builder2.transform(SkM44()); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, ClipRRectTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.clipRRect(kTestRRect, SkClipOp::kIntersect, true); + + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.clipRRect(kTestRRect, SkClipOp::kIntersect, true); + + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + builder2.transform(SkM44()); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, ClipPathTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.clipPath(kTestPath1, SkClipOp::kIntersect, true); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.clipPath(kTestPath1, SkClipOp::kIntersect, true); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + builder2.transform(SkM44()); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPTranslateDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.translate(0, 0); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPScaleDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.scale(1.0, 1.0); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPRotationDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.rotate(360); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPSkewDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.skew(0, 0); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPTransformDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.transform(SkM44()); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPTransform2DDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.transform2DAffine(1, 0, 0, 0, 1, 0); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + +TEST(DisplayList, NOPTransformFullPerspectiveDoesNotTriggerDeferredSave) { { DisplayListBuilder builder1; builder1.save(); builder1.save(); - builder1.transform(SkM44()); + builder1.transformFullPerspective(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, + 0, 1); builder1.drawRect({0, 0, 100, 100}); builder1.restore(); - builder1.transform(SkM44()); builder1.drawRect({0, 0, 100, 100}); builder1.restore(); auto display_list1 = builder1.Build(); @@ -1877,66 +2149,51 @@ TEST(DisplayList, CollapseNestedSaveAndSaveLayerRestore) { ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); } - // Clip + { DisplayListBuilder builder1; builder1.save(); builder1.save(); - builder1.clipRect(SkRect::MakeLTRB(0, 0, 100, 100), SkClipOp::kIntersect, - true); + builder1.transformFullPerspective(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, + 0, 1); + builder1.transformReset(); builder1.drawRect({0, 0, 100, 100}); builder1.restore(); - builder1.transform(SkM44()); builder1.drawRect({0, 0, 100, 100}); builder1.restore(); auto display_list1 = builder1.Build(); DisplayListBuilder builder2; builder2.save(); - builder2.clipRect(SkRect::MakeLTRB(0, 0, 100, 100), SkClipOp::kIntersect, - true); + builder2.transformReset(); builder2.drawRect({0, 0, 100, 100}); builder2.restore(); - builder2.transform(SkM44()); builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); } +} - std::unordered_set function_names = {"Translate", - "Scale", - "Skew", - "Transform2d", - "TransformFullPerspective", - "ClipRect", - "ClipPath", - "ClipRRect"}; +TEST(DisplayList, NOPClipDoesNotTriggerDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.clipRect(SkRect::MakeLTRB(0, SK_ScalarNaN, SK_ScalarNaN, 0), + SkClipOp::kIntersect, true); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + auto display_list1 = builder1.Build(); - for (auto& group : allGroups) { - if (function_names.find(group.op_name) == function_names.cend()) { - continue; - } - for (size_t i = 0; i < group.variants.size(); i++) { - auto& invocation = group.variants[i]; - DisplayListBuilder builder1; - builder1.save(); - builder1.save(); - invocation.invoker(builder1); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - - auto display_list1 = builder1.Build(); - - DisplayListBuilder builder2; - builder2.save(); - invocation.invoker(builder2); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); - } - } + DisplayListBuilder builder2; + builder2.drawRect({0, 0, 100, 100}); + builder2.drawRect({0, 0, 100, 100}); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); } TEST(DisplayList, RTreeOfClippedSaveLayerFilterScene) { From 085100eac8f0dbce462213f719bf778343755317 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Thu, 11 Aug 2022 14:32:43 +0800 Subject: [PATCH 7/7] Prune TranslateTriggersDeferredSave unittest --- display_list/display_list_unittests.cc | 45 ++++++++++++-------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 5c4fa1f3c1efb..62155636c23de 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1787,30 +1787,6 @@ TEST(DisplayList, RemoveUnnecessarySaveRestorePairsInSetPaint) { } } -TEST(DisplayList, TranslateTriggersDeferredSave) { - DisplayListBuilder builder1; - builder1.save(); - builder1.saveLayer(nullptr, false); - builder1.translate(10, 10); - builder1.scale(2, 2); - builder1.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); - builder1.drawRect({0, 0, 100, 100}); - builder1.restore(); - builder1.restore(); - auto display_list1 = builder1.Build(); - - DisplayListBuilder builder2; - builder2.saveLayer(nullptr, false); - builder2.translate(10, 10); - builder2.scale(2, 2); - builder2.clipRect({10, 10, 20, 20}, SkClipOp::kIntersect, false); - builder2.drawRect({0, 0, 100, 100}); - builder2.restore(); - auto display_list2 = builder2.Build(); - - ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); -} - TEST(DisplayList, TransformTriggersDeferredSave) { DisplayListBuilder builder1; builder1.save(); @@ -1918,6 +1894,27 @@ TEST(DisplayList, SkewTriggersDeferredSave) { ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); } + +TEST(DisplayList, TranslateTriggersDeferredSave) { + DisplayListBuilder builder1; + builder1.save(); + builder1.save(); + builder1.translate(10, 10); + builder1.drawRect({0, 0, 100, 100}); + builder1.restore(); + builder1.restore(); + auto display_list1 = builder1.Build(); + + DisplayListBuilder builder2; + builder2.save(); + builder2.translate(10, 10); + builder2.drawRect({0, 0, 100, 100}); + builder2.restore(); + auto display_list2 = builder2.Build(); + + ASSERT_TRUE(DisplayListsEQ_Verbose(display_list1, display_list2)); +} + TEST(DisplayList, ScaleTriggersDeferredSave) { DisplayListBuilder builder1; builder1.save();