From dbe4cc89ba335057d8492d9a2583cac8ffd9ce12 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 4 Feb 2022 00:45:10 -0800 Subject: [PATCH] DisplayList savelayer opacity peephole optimization (#30957) --- display_list/display_list.cc | 4 + display_list/display_list.h | 46 ++++ display_list/display_list_builder.cc | 43 ++- display_list/display_list_builder.h | 25 +- .../display_list_canvas_dispatcher.cc | 35 ++- display_list/display_list_canvas_dispatcher.h | 2 +- display_list/display_list_canvas_unittests.cc | 90 +++++-- display_list/display_list_dispatcher.h | 20 +- display_list/display_list_ops.h | 14 +- display_list/display_list_unittests.cc | 254 +++++++++++++++++- display_list/display_list_utils.cc | 22 +- display_list/display_list_utils.h | 92 ++++++- testing/dart/observatory/tracing_test.dart | 2 + 13 files changed, 571 insertions(+), 78 deletions(-) diff --git a/display_list/display_list.cc b/display_list/display_list.cc index 8ba380f19d10d..31b3c5637034d 100644 --- a/display_list/display_list.cc +++ b/display_list/display_list.cc @@ -11,6 +11,10 @@ namespace flutter { +const SaveLayerOptions SaveLayerOptions::kNoAttributes = SaveLayerOptions(); +const SaveLayerOptions SaveLayerOptions::kWithAttributes = + kNoAttributes.with_renders_with_attributes(); + const SkSamplingOptions DisplayList::NearestSampling = SkSamplingOptions(SkFilterMode::kNearest, SkMipmapMode::kNone); const SkSamplingOptions DisplayList::LinearSampling = diff --git a/display_list/display_list.h b/display_list/display_list.h index a7aa0cc648b47..46bbb03a67125 100644 --- a/display_list/display_list.h +++ b/display_list/display_list.h @@ -150,6 +150,52 @@ enum class DisplayListOpType { FOR_EACH_DISPLAY_LIST_OP(DL_OP_TO_ENUM_VALUE) }; class Dispatcher; class DisplayListBuilder; +class SaveLayerOptions { + public: + static const SaveLayerOptions kWithAttributes; + static const SaveLayerOptions kNoAttributes; + + SaveLayerOptions() : flags_(0) {} + SaveLayerOptions(const SaveLayerOptions& options) : flags_(options.flags_) {} + SaveLayerOptions(const SaveLayerOptions* options) : flags_(options->flags_) {} + + SaveLayerOptions without_optimizations() const { + SaveLayerOptions options; + options.fRendersWithAttributes = fRendersWithAttributes; + return options; + } + + bool renders_with_attributes() const { return fRendersWithAttributes; } + SaveLayerOptions with_renders_with_attributes() const { + SaveLayerOptions options(this); + options.fRendersWithAttributes = true; + return options; + } + + bool can_distribute_opacity() const { return fCanDistributeOpacity; } + SaveLayerOptions with_can_distribute_opacity() const { + SaveLayerOptions options(this); + options.fCanDistributeOpacity = true; + return options; + } + + bool operator==(const SaveLayerOptions& other) const { + return flags_ == other.flags_; + } + bool operator!=(const SaveLayerOptions& other) const { + return flags_ != other.flags_; + } + + private: + union { + struct { + unsigned fRendersWithAttributes : 1; + unsigned fCanDistributeOpacity : 1; + }; + uint32_t flags_; + }; +}; + // The base class that contains a sequence of rendering operations // for dispatch to a Dispatcher. These objects must be instantiated // through an instance of DisplayListBuilder::build(). diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 39558be3cbb62..0bae1483ea724 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -237,7 +237,25 @@ void DisplayListBuilder::restore() { layer_stack_.pop_back(); current_layer_ = &layer_stack_.back(); Push(0, 1); - if (!layer_info.has_layer) { + if (layer_info.has_layer) { + if (layer_info.is_group_opacity_compatible()) { + // We are now going to go back and modify the matching saveLayer + // call to add the option indicating it can distribute an opacity + // value to its children. + // + // Note that this operation cannot and does not change the size + // or structure of the SaveLayerOp record. It only sets an option + // flag on an existing field. + // + // Note that these kinds of modification operations on data already + // in the DisplayList are only allowed *during* the build phase. + // Once built, the DisplayList records must remain read only to + // ensure consistency of rendering and |Equals()| behavior. + SaveLayerOp* op = reinterpret_cast( + storage_.get() + layer_info.save_layer_offset); + op->options = op->options.with_can_distribute_opacity(); + } + } else { // For regular save() ops there was no protecting layer so we have to // accumulate the values into the enclosing layer. if (layer_info.cannot_inherit_opacity) { @@ -249,13 +267,24 @@ void DisplayListBuilder::restore() { } } void DisplayListBuilder::saveLayer(const SkRect* bounds, - bool restore_with_paint) { + const SaveLayerOptions in_options) { + SaveLayerOptions options = in_options.without_optimizations(); + size_t save_layer_offset = used_; bounds // - ? Push(0, 1, *bounds, restore_with_paint) - : Push(0, 1, restore_with_paint); - CheckLayerOpacityCompatibility(restore_with_paint); - layer_stack_.emplace_back(true); + ? Push(0, 1, *bounds, options) + : Push(0, 1, options); + CheckLayerOpacityCompatibility(options.renders_with_attributes()); + layer_stack_.emplace_back(save_layer_offset, true); current_layer_ = &layer_stack_.back(); + if (options.renders_with_attributes()) { + // |current_opacity_compatibility_| does not take an ImageFilter into + // account because an individual primitive with an ImageFilter can apply + // opacity on top of it. But, if the layer is applying the ImageFilter + // then it cannot pass the opacity on. + if (!current_opacity_compatibility_ || current_image_filter_ != nullptr) { + UpdateLayerOpacityCompatibility(false); + } + } } void DisplayListBuilder::translate(SkScalar tx, SkScalar ty) { @@ -455,6 +484,8 @@ void DisplayListBuilder::drawVertices(const sk_sp vertices, Push(0, 1, std::move(vertices), mode); // DrawVertices applies its colors to the paint so we have no way // of controlling opacity using the current paint attributes. + // Although, examination of the |mode| might find some predictable + // cases. UpdateLayerOpacityCompatibility(false); } diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index 0dc3417a4a7a2..580ab4ecec2ca 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -153,7 +153,17 @@ class DisplayListBuilder final : public virtual Dispatcher, sk_sp getImageFilter() const { return current_image_filter_; } void save() override; - void saveLayer(const SkRect* bounds, bool restore_with_paint) override; + // Only the |renders_with_attributes()| option will be accepted here. Any + // other flags will be ignored and calculated anew as the DisplayList is + // built. Alternatively, use the |saveLayer(SkRect, bool)| method. + void saveLayer(const SkRect* bounds, const SaveLayerOptions options) override; + // Convenience method with just a boolean to indicate whether the saveLayer + // should apply the rendering attributes. + void saveLayer(const SkRect* bounds, bool renders_with_attributes) { + saveLayer(bounds, renders_with_attributes + ? SaveLayerOptions::kWithAttributes + : SaveLayerOptions::kNoAttributes); + } void restore() override; int getSaveCount() { return layer_stack_.size(); } @@ -271,11 +281,20 @@ class DisplayListBuilder final : public virtual Dispatcher, } struct LayerInfo { - LayerInfo(bool has_layer = false) - : has_layer(has_layer), + LayerInfo(size_t save_layer_offset = 0, bool has_layer = false) + : save_layer_offset(save_layer_offset), + has_layer(has_layer), cannot_inherit_opacity(false), has_compatible_op(false) {} + // 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 + // the records inside the saveLayer that may impact how the saveLayer + // is handled (e.g., |cannot_inherit_opacity| == false). + // This offset is only valid if |has_layer| is true. + size_t save_layer_offset; + bool has_layer; bool cannot_inherit_opacity; bool has_compatible_op; diff --git a/display_list/display_list_canvas_dispatcher.cc b/display_list/display_list_canvas_dispatcher.cc index ce4007f7cda6a..e31d919c4a692 100644 --- a/display_list/display_list_canvas_dispatcher.cc +++ b/display_list/display_list_canvas_dispatcher.cc @@ -26,17 +26,42 @@ const SkPaint* DisplayListCanvasDispatcher::safe_paint(bool use_attributes) { void DisplayListCanvasDispatcher::save() { canvas_->save(); - save_opacity(false); + // save has no impact on attributes, but it needs to register a record + // on the restore stack so that the eventual call to restore() will + // know what to do at that time. We could annotate the restore record + // with a flag that the record came from a save call, but it is simpler + // to just pass in the current opacity value as the value to be used by + // the children and let the utility calls notice that it didn't change. + save_opacity(opacity()); } void DisplayListCanvasDispatcher::restore() { canvas_->restore(); restore_opacity(); } void DisplayListCanvasDispatcher::saveLayer(const SkRect* bounds, - bool restore_with_paint) { - TRACE_EVENT0("flutter", "Canvas::saveLayer"); - canvas_->saveLayer(bounds, safe_paint(restore_with_paint)); - save_opacity(true); + const SaveLayerOptions options) { + if (bounds == nullptr && options.can_distribute_opacity()) { + // We know that: + // - no bounds is needed for clipping here + // - the current attributes only have an alpha + // - the children are compatible with individually rendering with + // an inherited opacity + // Therefore we can just use a save instead of a saveLayer and pass the + // intended opacity to the children. + canvas_->save(); + // If the saveLayer does not use attributes, the children should continue + // to render with the inherited opacity unmodified. If attributes are to + // be applied, the children should render with the combination of the + // inherited opacity combined with the alpha from the current color. + save_opacity(options.renders_with_attributes() ? combined_opacity() + : opacity()); + } else { + TRACE_EVENT0("flutter", "Canvas::saveLayer"); + canvas_->saveLayer(bounds, safe_paint(options.renders_with_attributes())); + // saveLayer will apply the current opacity on behalf of the children + // so they will inherit an opaque opacity. + save_opacity(SK_Scalar1); + } } void DisplayListCanvasDispatcher::translate(SkScalar tx, SkScalar ty) { diff --git a/display_list/display_list_canvas_dispatcher.h b/display_list/display_list_canvas_dispatcher.h index 753fb6467eb0c..3595114ba3069 100644 --- a/display_list/display_list_canvas_dispatcher.h +++ b/display_list/display_list_canvas_dispatcher.h @@ -29,7 +29,7 @@ class DisplayListCanvasDispatcher : public virtual Dispatcher, void save() override; void restore() override; - void saveLayer(const SkRect* bounds, bool restore_with_paint) override; + void saveLayer(const SkRect* bounds, const SaveLayerOptions options) override; void translate(SkScalar tx, SkScalar ty) override; void scale(SkScalar sx, SkScalar sy) override; diff --git a/display_list/display_list_canvas_unittests.cc b/display_list/display_list_canvas_unittests.cc index c7eb5400c3ece..f8c9eb5bfa3ea 100644 --- a/display_list/display_list_canvas_unittests.cc +++ b/display_list/display_list_canvas_unittests.cc @@ -618,6 +618,7 @@ class CaseParameters { EmptyDlRenderer, SK_ColorTRANSPARENT, false, + false, false) {} CaseParameters(std::string info, @@ -627,7 +628,8 @@ class CaseParameters { DlRenderer dl_restore, SkColor bg, bool has_diff_clip, - bool has_mutating_save_layer) + bool has_mutating_save_layer, + bool fuzzy_compare_components) : info_(info), bg_(bg), cv_setup_(cv_setup), @@ -635,29 +637,35 @@ class CaseParameters { cv_restore_(cv_restore), dl_restore_(dl_restore), has_diff_clip_(has_diff_clip), - has_mutating_save_layer_(has_mutating_save_layer) {} + has_mutating_save_layer_(has_mutating_save_layer), + fuzzy_compare_components_(fuzzy_compare_components) {} CaseParameters with_restore(CvRenderer cv_restore, DlRenderer dl_restore, - bool mutating_layer) { + bool mutating_layer, + bool fuzzy_compare_components = false) { return CaseParameters(info_, cv_setup_, dl_setup_, cv_restore, dl_restore, - bg_, has_diff_clip_, mutating_layer); + bg_, has_diff_clip_, mutating_layer, + fuzzy_compare_components); } CaseParameters with_bg(SkColor bg) { return CaseParameters(info_, cv_setup_, dl_setup_, cv_restore_, dl_restore_, - bg, has_diff_clip_, has_mutating_save_layer_); + bg, has_diff_clip_, has_mutating_save_layer_, + fuzzy_compare_components_); } CaseParameters with_diff_clip() { return CaseParameters(info_, cv_setup_, dl_setup_, cv_restore_, dl_restore_, - bg_, true, has_mutating_save_layer_); + bg_, true, has_mutating_save_layer_, + fuzzy_compare_components_); } std::string info() const { return info_; } SkColor bg() const { return bg_; } bool has_diff_clip() const { return has_diff_clip_; } bool has_mutating_save_layer() const { return has_mutating_save_layer_; } + bool fuzzy_compare_components() const { return fuzzy_compare_components_; } CvSetup cv_setup() const { return cv_setup_; } DlRenderer dl_setup() const { return dl_setup_; } @@ -689,6 +697,7 @@ class CaseParameters { const DlRenderer dl_restore_; const bool has_diff_clip_; const bool has_mutating_save_layer_; + const bool fuzzy_compare_components_; }; class CanvasCompareTester { @@ -715,16 +724,24 @@ class CanvasCompareTester { SkRect rect = SkRect::MakeXYWH(RenderCenterX, RenderCenterY, 10, 10); SkColor alpha_layer_color = SkColorSetARGB(0x7f, 0x00, 0xff, 0xff); SkColor default_color = SkPaint().getColor(); - CvRenderer cv_restore = [=](SkCanvas* cv, const SkPaint& p) { + CvRenderer cv_safe_restore = [=](SkCanvas* cv, const SkPaint& p) { // Draw another primitive to disable peephole optimizations cv->drawRect(RenderBounds.makeOffset(500, 500), p); cv->restore(); }; - DlRenderer dl_restore = [=](DisplayListBuilder& b) { + DlRenderer dl_safe_restore = [=](DisplayListBuilder& b) { // Draw another primitive to disable peephole optimizations b.drawRect(RenderBounds.makeOffset(500, 500)); b.restore(); }; + CvRenderer cv_opt_restore = [=](SkCanvas* cv, const SkPaint& p) { + // Just a simple restore to allow peephole optimizations to occur + cv->restore(); + }; + DlRenderer dl_opt_restore = [=](DisplayListBuilder& b) { + // Just a simple restore to allow peephole optimizations to occur + b.restore(); + }; SkRect layer_bounds = RenderBounds.makeInset(15, 15); RenderWith(testP, env, tolerance, CaseParameters( @@ -756,7 +773,7 @@ class CanvasCompareTester { [=](DisplayListBuilder& b) { // b.saveLayer(nullptr, false); }) - .with_restore(cv_restore, dl_restore, false)); + .with_restore(cv_safe_restore, dl_safe_restore, false)); RenderWith(testP, env, tolerance, CaseParameters( "saveLayer no paint, with bounds", @@ -766,7 +783,7 @@ class CanvasCompareTester { [=](DisplayListBuilder& b) { // b.saveLayer(&layer_bounds, false); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); RenderWith(testP, env, tolerance, CaseParameters( "saveLayer with alpha, no bounds", @@ -780,7 +797,21 @@ class CanvasCompareTester { b.saveLayer(nullptr, true); b.setColor(default_color); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); + RenderWith(testP, env, tolerance, + CaseParameters( + "saveLayer with peephole alpha, no bounds", + [=](SkCanvas* cv, SkPaint& p) { + SkPaint save_p; + save_p.setColor(alpha_layer_color); + cv->saveLayer(nullptr, &save_p); + }, + [=](DisplayListBuilder& b) { + b.setColor(alpha_layer_color); + b.saveLayer(nullptr, true); + b.setColor(default_color); + }) + .with_restore(cv_opt_restore, dl_opt_restore, true, true)); RenderWith(testP, env, tolerance, CaseParameters( "saveLayer with alpha and bounds", @@ -794,7 +825,7 @@ class CanvasCompareTester { b.saveLayer(&layer_bounds, true); b.setColor(default_color); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); { // clang-format off @@ -823,7 +854,7 @@ class CanvasCompareTester { b.setColorFilter(nullptr); b.setStrokeWidth(5.0); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); } EXPECT_TRUE(filter->unique()) << "saveLayer ColorFilter, no bounds Cleanup"; @@ -843,7 +874,7 @@ class CanvasCompareTester { b.setColorFilter(nullptr); b.setStrokeWidth(5.0); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); } EXPECT_TRUE(filter->unique()) << "saveLayer ColorFilter and bounds Cleanup"; @@ -867,7 +898,7 @@ class CanvasCompareTester { b.setImageFilter(nullptr); b.setStrokeWidth(5.0); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); } EXPECT_TRUE(filter->unique()) << "saveLayer ImageFilter, no bounds Cleanup"; @@ -887,7 +918,7 @@ class CanvasCompareTester { b.setImageFilter(nullptr); b.setStrokeWidth(5.0); }) - .with_restore(cv_restore, dl_restore, true)); + .with_restore(cv_safe_restore, dl_safe_restore, true)); } EXPECT_TRUE(filter->unique()) << "saveLayer ImageFilter and bounds Cleanup"; @@ -1741,7 +1772,8 @@ class CanvasCompareTester { display_list->RenderTo(dl_surface->canvas()); compareToReference(dl_surface->pixmap(), sk_pixels, info + " (DisplayList built directly -> surface)", - &dl_bounds, &tolerance, bg); + &dl_bounds, &tolerance, bg, + caseP.fuzzy_compare_components()); if (display_list->can_apply_group_opacity()) { checkGroupOpacity(env, display_list, dl_surface->pixmap(), @@ -1761,7 +1793,8 @@ class CanvasCompareTester { dl_recorder.builder()->Build()->RenderTo(cv_dl_surface->canvas()); compareToReference(cv_dl_surface->pixmap(), sk_pixels, info + " (Skia calls -> DisplayList -> surface)", - nullptr, nullptr, bg); + nullptr, nullptr, bg, + caseP.fuzzy_compare_components()); } { @@ -1800,10 +1833,22 @@ class CanvasCompareTester { display_list_x2->RenderTo(test_x2_canvas); compareToReference(test_x2_surface->pixmap(), ref_x2_pixels, info + " (Both rendered scaled 2x)", nullptr, nullptr, - bg, TestWidth2, TestHeight2, false); + bg, caseP.fuzzy_compare_components(), // + TestWidth2, TestHeight2, false); } } + static bool fuzzyCompare(uint32_t pixel_a, uint32_t pixel_b, int fudge) { + for (int i = 0; i < 32; i += 8) { + int comp_a = (pixel_a >> i) & 0xff; + int comp_b = (pixel_b >> i) & 0xff; + if (std::abs(comp_a - comp_b) > fudge) { + return false; + } + } + return true; + } + static void checkGroupOpacity(const RenderEnvironment& env, sk_sp display_list, const SkPixmap* ref_pixmap, @@ -1914,9 +1959,10 @@ class CanvasCompareTester { SkRect* bounds, const BoundsTolerance* tolerance, const SkColor bg, + bool fuzzyCompares = false, int width = TestWidth, int height = TestHeight, - bool printMismatches = false) { + bool printMismatches = true) { SkPMColor untouched = SkPreMultiplyColor(bg); ASSERT_EQ(test_pixels->width(), width) << info; ASSERT_EQ(test_pixels->height(), height) << info; @@ -1952,7 +1998,9 @@ class CanvasCompareTester { pixels_oob++; } } - if (test_row[x] != ref_row[x]) { + bool match = fuzzyCompares ? fuzzyCompare(test_row[x], ref_row[x], 1) + : test_row[x] == ref_row[x]; + if (!match) { if (printMismatches) { FML_LOG(ERROR) << "pix[" << x << ", " << y << "] mismatch: " << std::hex << test_row[x] diff --git a/display_list/display_list_dispatcher.h b/display_list/display_list_dispatcher.h index e938eec6ebe91..9b4edd0eee0e0 100644 --- a/display_list/display_list_dispatcher.h +++ b/display_list/display_list_dispatcher.h @@ -5,8 +5,7 @@ #ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_DISPATCHER_H_ #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_DISPATCHER_H_ -#include "flutter/display_list/types.h" -#include "flutter/fml/macros.h" +#include "flutter/display_list/display_list.h" namespace flutter { @@ -57,12 +56,17 @@ class Dispatcher { // All of the following methods are nearly 1:1 with their counterparts // in |SkCanvas| and have the same behavior and output. virtual void save() = 0; - // The |restore_with_paint| parameter determines whether the existing - // rendering attributes will be applied to the save layer surface while - // rendering it back to the current surface. If the parameter is false - // then this method is equivalent to |SkCanvas::saveLayer| with a null - // paint object. - virtual void saveLayer(const SkRect* bounds, bool restore_with_paint) = 0; + // The |options| parameter can specify whether the existing rendering + // attributes will be applied to the save layer surface while rendering + // it back to the current surface. If the flag is false then this method + // is equivalent to |SkCanvas::saveLayer| with a null paint object. + // The |options| parameter may contain other options that indicate some + // specific optimizations may be made by the underlying implementation + // to avoid creating a temporary layer, these optimization options will + // be determined as the |DisplayList| is constructed and should not be + // specified in calling a |DisplayListBuilder| as they will be ignored. + virtual void saveLayer(const SkRect* bounds, + const SaveLayerOptions options) = 0; virtual void restore() = 0; virtual void translate(SkScalar tx, SkScalar ty) = 0; diff --git a/display_list/display_list_ops.h b/display_list/display_list_ops.h index f98c17006e3ef..574637a283d71 100644 --- a/display_list/display_list_ops.h +++ b/display_list/display_list_ops.h @@ -215,26 +215,26 @@ struct SaveOp final : DLOp { struct SaveLayerOp final : DLOp { static const auto kType = DisplayListOpType::kSaveLayer; - explicit SaveLayerOp(bool with_paint) : with_paint(with_paint) {} + explicit SaveLayerOp(const SaveLayerOptions options) : options(options) {} - bool with_paint; + SaveLayerOptions options; void dispatch(Dispatcher& dispatcher) const { - dispatcher.saveLayer(nullptr, with_paint); + dispatcher.saveLayer(nullptr, options); } }; // 4 byte header + 20 byte payload packs evenly into 24 bytes struct SaveLayerBoundsOp final : DLOp { static const auto kType = DisplayListOpType::kSaveLayerBounds; - SaveLayerBoundsOp(SkRect rect, bool with_paint) - : with_paint(with_paint), rect(rect) {} + SaveLayerBoundsOp(SkRect rect, const SaveLayerOptions options) + : options(options), rect(rect) {} - bool with_paint; + SaveLayerOptions options; const SkRect rect; void dispatch(Dispatcher& dispatcher) const { - dispatcher.saveLayer(&rect, with_paint); + dispatcher.saveLayer(&rect, options); } }; // 4 byte header + no payload uses minimum 8 bytes (4 bytes unused) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 9f8a7c06657a2..5a858e6631fb0 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -5,6 +5,7 @@ #include "flutter/display_list/display_list.h" #include "flutter/display_list/display_list_builder.h" #include "flutter/display_list/display_list_canvas_recorder.h" +#include "flutter/display_list/display_list_utils.h" #include "flutter/fml/math.h" #include "flutter/testing/testing.h" #include "third_party/skia/include/core/SkPictureRecorder.h" @@ -363,12 +364,47 @@ std::vector allGroups = { } }, { "Save(Layer)+Restore", { - // cv.save/restore are ignored if there are no draw calls between them - {2, 16, 0, 0, [](DisplayListBuilder& b) {b.save(); b.restore();}}, - {2, 16, 2, 16, [](DisplayListBuilder& b) {b.saveLayer(nullptr, false); b.restore(); }}, - {2, 16, 2, 16, [](DisplayListBuilder& b) {b.saveLayer(nullptr, true); b.restore(); }}, - {2, 32, 2, 32, [](DisplayListBuilder& b) {b.saveLayer(&TestBounds, false); b.restore(); }}, - {2, 32, 2, 32, [](DisplayListBuilder& b) {b.saveLayer(&TestBounds, true); b.restore(); }}, + // There are many reasons that save and restore can elide content, including + // whether or not there are any draw operations between them, whether or not + // there are any state changes to restore, and whether group rendering (opacity) + // optimizations can allow attributes to be distributed to the children. + // To prevent those cases we include at least one clip operation and 2 overlapping + // rendering primitives between each save/restore pair. + {5, 88, 5, 88, [](DisplayListBuilder& b) { + b.save(); + b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); + b.drawRect({5, 5, 15, 15}); + b.drawRect({10, 10, 20, 20}); + b.restore(); + }}, + {5, 88, 5, 88, [](DisplayListBuilder& b) { + b.saveLayer(nullptr, false); + b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); + b.drawRect({5, 5, 15, 15}); + b.drawRect({10, 10, 20, 20}); + b.restore(); + }}, + {5, 88, 5, 88, [](DisplayListBuilder& b) { + b.saveLayer(nullptr, true); + b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); + b.drawRect({5, 5, 15, 15}); + b.drawRect({10, 10, 20, 20}); + b.restore(); + }}, + {5, 104, 5, 104, [](DisplayListBuilder& b) { + b.saveLayer(&TestBounds, false); + b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); + b.drawRect({5, 5, 15, 15}); + b.drawRect({10, 10, 20, 20}); + b.restore(); + }}, + {5, 104, 5, 104, [](DisplayListBuilder& b) { + b.saveLayer(&TestBounds, true); + b.clipRect({0, 0, 25, 25}, SkClipOp::kIntersect, true); + b.drawRect({5, 5, 15, 15}); + b.drawRect({10, 10, 20, 20}); + b.restore(); + }}, } }, { "Translate", { @@ -1584,5 +1620,211 @@ TEST(DisplayList, SaveLayerBoundsSnapshotsImageFilter) { EXPECT_EQ(bounds, SkRect::MakeLTRB(50, 50, 100, 100)); } +class SaveLayerOptionsExpector : public virtual Dispatcher, + public IgnoreAttributeDispatchHelper, + public IgnoreClipDispatchHelper, + public IgnoreTransformDispatchHelper, + public IgnoreDrawDispatchHelper { + public: + explicit SaveLayerOptionsExpector(SaveLayerOptions expected) { + expected_.push_back(expected); + } + + explicit SaveLayerOptionsExpector(std::vector expected) + : expected_(expected) {} + + void saveLayer(const SkRect* bounds, + const SaveLayerOptions options) override { + EXPECT_EQ(options, expected_[save_layer_count_]); + save_layer_count_++; + } + + int save_layer_count() { return save_layer_count_; } + + private: + std::vector expected_; + int save_layer_count_ = 0; +}; + +TEST(DisplayList, SaveLayerOneSimpleOpSupportsOpacityOptimization) { + SaveLayerOptions expected = + SaveLayerOptions::kWithAttributes.with_can_distribute_opacity(); + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerNoAttributesSupportsOpacityOptimization) { + SaveLayerOptions expected = + SaveLayerOptions::kNoAttributes.with_can_distribute_opacity(); + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.saveLayer(nullptr, false); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerTwoOverlappingOpsPreventsOpacityOptimization) { + SaveLayerOptions expected = SaveLayerOptions::kWithAttributes; + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.drawRect({10, 10, 20, 20}); + builder.drawRect({15, 15, 25, 25}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, NestedSaveLayersMightSupportOpacityOptimization) { + SaveLayerOptions expected1 = + SaveLayerOptions::kWithAttributes.with_can_distribute_opacity(); + SaveLayerOptions expected2 = SaveLayerOptions::kWithAttributes; + SaveLayerOptions expected3 = + SaveLayerOptions::kWithAttributes.with_can_distribute_opacity(); + SaveLayerOptionsExpector expector({expected1, expected2, expected3}); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.saveLayer(nullptr, true); + builder.drawRect({10, 10, 20, 20}); + builder.saveLayer(nullptr, true); + builder.drawRect({15, 15, 25, 25}); + builder.restore(); + builder.restore(); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 3); +} + +TEST(DisplayList, NestedSaveLayersCanBothSupportOpacityOptimization) { + SaveLayerOptions expected1 = + SaveLayerOptions::kWithAttributes.with_can_distribute_opacity(); + SaveLayerOptions expected2 = + SaveLayerOptions::kNoAttributes.with_can_distribute_opacity(); + SaveLayerOptionsExpector expector({expected1, expected2}); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.saveLayer(nullptr, false); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 2); +} + +TEST(DisplayList, SaveLayerImageFilterPreventsOpacityOptimization) { + SaveLayerOptions expected = SaveLayerOptions::kWithAttributes; + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.setImageFilter(TestImageFilter1); + builder.saveLayer(nullptr, true); + builder.setImageFilter(nullptr); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerColorFilterPreventsOpacityOptimization) { + SaveLayerOptions expected = SaveLayerOptions::kWithAttributes; + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.setColorFilter(TestColorFilter1); + builder.saveLayer(nullptr, true); + builder.setColorFilter(nullptr); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerSrcBlendPreventsOpacityOptimization) { + SaveLayerOptions expected = SaveLayerOptions::kWithAttributes; + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.setBlendMode(SkBlendMode::kSrc); + builder.saveLayer(nullptr, true); + builder.setBlendMode(SkBlendMode::kSrcOver); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerImageFilterOnChildSupportsOpacityOptimization) { + SaveLayerOptions expected = + SaveLayerOptions::kWithAttributes.with_can_distribute_opacity(); + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.setImageFilter(TestImageFilter1); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerColorFilterOnChildPreventsOpacityOptimization) { + SaveLayerOptions expected = SaveLayerOptions::kWithAttributes; + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.setColorFilter(TestColorFilter1); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + +TEST(DisplayList, SaveLayerSrcBlendOnChildPreventsOpacityOptimization) { + SaveLayerOptions expected = SaveLayerOptions::kWithAttributes; + SaveLayerOptionsExpector expector(expected); + + DisplayListBuilder builder; + builder.setColor(SkColorSetARGB(127, 255, 255, 255)); + builder.saveLayer(nullptr, true); + builder.setBlendMode(SkBlendMode::kSrc); + builder.drawRect({10, 10, 20, 20}); + builder.restore(); + + builder.Build()->Dispatch(expector); + EXPECT_EQ(expector.save_layer_count(), 1); +} + } // namespace testing } // namespace flutter diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index 24ffa869e1997..9fac45b3e8ec0 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -26,22 +26,12 @@ constexpr float invert_color_matrix[20] = { }; // clang-format on -void SkPaintDispatchHelper::save_opacity(bool reset_and_restore) { - if (opacity_ >= SK_Scalar1) { - reset_and_restore = false; - } - save_stack_.emplace_back(opacity_, reset_and_restore); - if (reset_and_restore) { - opacity_ = SK_Scalar1; - setColor(current_color_); - } +void SkPaintDispatchHelper::save_opacity(SkScalar child_opacity) { + save_stack_.emplace_back(opacity_); + set_opacity(child_opacity); } void SkPaintDispatchHelper::restore_opacity() { - SaveInfo& info = save_stack_.back(); - if (info.restore_opacity) { - opacity_ = info.opacity; - setColor(current_color_); - } + set_opacity(save_stack_.back().opacity); save_stack_.pop_back(); } @@ -298,10 +288,10 @@ void DisplayListBoundsCalculator::save() { accumulator_ = layer_infos_.back()->layer_accumulator(); } void DisplayListBoundsCalculator::saveLayer(const SkRect* bounds, - bool with_paint) { + const SaveLayerOptions options) { SkMatrixDispatchHelper::save(); ClipBoundsDispatchHelper::save(); - if (with_paint) { + if (options.renders_with_attributes()) { layer_infos_.emplace_back(std::make_unique( accumulator_, image_filter_, paint_nops_on_transparency())); } else { diff --git a/display_list/display_list_utils.h b/display_list/display_list_utils.h index 7089aebb72de9..e76b3d4b049a2 100644 --- a/display_list/display_list_utils.h +++ b/display_list/display_list_utils.h @@ -91,6 +91,73 @@ class IgnoreTransformDispatchHelper : public virtual Dispatcher { // clang-format on }; +class IgnoreDrawDispatchHelper : public virtual Dispatcher { + public: + void save() override {} + void saveLayer(const SkRect* bounds, + const SaveLayerOptions options) override {} + void restore() override {} + void drawColor(SkColor color, SkBlendMode mode) override {} + void drawPaint() override {} + void drawLine(const SkPoint& p0, const SkPoint& p1) override {} + void drawRect(const SkRect& rect) override {} + void drawOval(const SkRect& bounds) override {} + void drawCircle(const SkPoint& center, SkScalar radius) override {} + void drawRRect(const SkRRect& rrect) override {} + void drawDRRect(const SkRRect& outer, const SkRRect& inner) override {} + void drawPath(const SkPath& path) override {} + void drawArc(const SkRect& oval_bounds, + SkScalar start_degrees, + SkScalar sweep_degrees, + bool use_center) override {} + void drawPoints(SkCanvas::PointMode mode, + uint32_t count, + const SkPoint points[]) override {} + void drawVertices(const sk_sp vertices, + SkBlendMode mode) override {} + void drawImage(const sk_sp image, + const SkPoint point, + const SkSamplingOptions& sampling, + bool render_with_attributes) override {} + void drawImageRect(const sk_sp image, + const SkRect& src, + const SkRect& dst, + const SkSamplingOptions& sampling, + bool render_with_attributes, + SkCanvas::SrcRectConstraint constraint) override {} + void drawImageNine(const sk_sp image, + const SkIRect& center, + const SkRect& dst, + SkFilterMode filter, + bool render_with_attributes) override {} + void drawImageLattice(const sk_sp image, + const SkCanvas::Lattice& lattice, + const SkRect& dst, + SkFilterMode filter, + bool render_with_attributes) override {} + void drawAtlas(const sk_sp atlas, + const SkRSXform xform[], + const SkRect tex[], + const SkColor colors[], + int count, + SkBlendMode mode, + const SkSamplingOptions& sampling, + const SkRect* cull_rect, + bool render_with_attributes) override {} + void drawPicture(const sk_sp picture, + const SkMatrix* matrix, + bool render_with_attributes) override {} + void drawDisplayList(const sk_sp display_list) override {} + void drawTextBlob(const sk_sp blob, + SkScalar x, + SkScalar y) override {} + void drawShadow(const SkPath& path, + const SkColor color, + const SkScalar elevation, + bool transparent_occluder, + SkScalar dpr) override {} +}; + // A utility class that will monitor the Dispatcher methods relating // to the rendering attributes and accumulate them into an SkPaint // which can be accessed at any time via paint(). @@ -122,11 +189,21 @@ class SkPaintDispatchHelper : public virtual Dispatcher { void setImageFilter(sk_sp filter) override; const SkPaint& paint() { return paint_; } + + /// Returns the current opacity attribute which is used to reduce + /// the alpha of all setColor calls encountered in the streeam SkScalar opacity() { return opacity_; } + /// Returns the combined opacity that includes both the current + /// opacity attribute and the alpha of the most recent color. + /// The most recently set color will have combined the two and + /// stored the combined value in the alpha of the paint. + SkScalar combined_opacity() { return paint_.getAlphaf(); } + /// Returns true iff the current opacity attribute is not opaque, + /// irrespective of the alpha of the current color bool has_opacity() { return opacity_ < SK_Scalar1; } protected: - void save_opacity(bool reset_and_restore); + void save_opacity(SkScalar opacity_for_children); void restore_opacity(); private: @@ -137,14 +214,19 @@ class SkPaintDispatchHelper : public virtual Dispatcher { sk_sp makeColorFilter(); struct SaveInfo { - SaveInfo(SkScalar opacity, bool restore_opacity) - : opacity(opacity), restore_opacity(restore_opacity) {} + SaveInfo(SkScalar opacity) : opacity(opacity) {} SkScalar opacity; - bool restore_opacity; }; std::vector save_stack_; + void set_opacity(SkScalar opacity) { + if (opacity_ != opacity) { + opacity_ = opacity; + setColor(current_color_); + } + } + SkColor current_color_; SkScalar opacity_; }; @@ -324,7 +406,7 @@ class DisplayListBoundsCalculator final void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override; void save() override; - void saveLayer(const SkRect* bounds, bool with_paint) override; + void saveLayer(const SkRect* bounds, const SaveLayerOptions options) override; void restore() override; void drawPaint() override; diff --git a/testing/dart/observatory/tracing_test.dart b/testing/dart/observatory/tracing_test.dart index 597d436d2eb16..3257a8ef06e3c 100644 --- a/testing/dart/observatory/tracing_test.dart +++ b/testing/dart/observatory/tracing_test.dart @@ -29,8 +29,10 @@ void main() { canvas.drawColor(const Color(0xff0000ff), BlendMode.srcOut); canvas.drawPaint(Paint()..imageFilter = ImageFilter.blur(sigmaX: 2, sigmaY: 3)); canvas.saveLayer(null, Paint()); + canvas.drawRect(const Rect.fromLTRB(10, 10, 20, 20), Paint()); canvas.saveLayer(const Rect.fromLTWH(0, 0, 100, 100), Paint()); canvas.drawRect(const Rect.fromLTRB(10, 10, 20, 20), Paint()); + canvas.drawRect(const Rect.fromLTRB(15, 15, 25, 25), Paint()); canvas.restore(); canvas.restore(); final Picture picture = recorder.endRecording();