rename some methods, consolidate state, add attribute interaction unit tests

Author: flar

display_list/display_list_color_filter.h

@@ -60,7 +60,7 @@ class DlColorFilter
   // Return a boolean indicating whether the color filtering operation can
   // be applied either before or after modulating the pixels with an opacity
   // value without changing the operation.
-  virtual bool can_commute_with_alpha() const { return false; }
+  virtual bool can_commute_with_opacity() const { return false; }
 
   // Return a DlBlendColorFilter pointer to this object iff it is a Blend
   // type of ColorFilter, otherwise return nullptr.
@@ -155,7 +155,7 @@ class DlMatrixColorFilter final : public DlColorFilter {
            sk_filter->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT;
   }
 
-  bool can_commute_with_alpha() const override {
+  bool can_commute_with_opacity() const override {
     return matrix_[3] == 0 && matrix_[8] == 0 && matrix_[13] == 0 &&
            matrix_[15] == 0 && matrix_[16] == 0 && matrix_[17] == 0 &&
            (matrix_[18] >= 0.0 && matrix_[18] <= 1.0) && matrix_[19] == 0;
@@ -204,6 +204,7 @@ class DlSrgbToLinearGammaColorFilter final : public DlColorFilter {
   }
   size_t size() const override { return sizeof(*this); }
   bool modifies_transparent_black() const override { return false; }
+  bool can_commute_with_opacity() const override { return true; }
 
   std::shared_ptr<DlColorFilter> shared() const override { return instance; }
   sk_sp<SkColorFilter> skia_object() const override { return sk_filter_; }
@@ -236,6 +237,7 @@ class DlLinearToSrgbGammaColorFilter final : public DlColorFilter {
   }
   size_t size() const override { return sizeof(*this); }
   bool modifies_transparent_black() const override { return false; }
+  bool can_commute_with_opacity() const override { return true; }
 
   std::shared_ptr<DlColorFilter> shared() const override { return instance; }
   sk_sp<SkColorFilter> skia_object() const override { return sk_filter_; }

flow/layers/color_filter_layer.cc

@@ -55,9 +55,13 @@ void ColorFilterLayer::Preroll(PrerollContext* context,
   // However, some color filters can commute themselves with an opacity
   // modulation so in that case we can apply the opacity on behalf of our
   // ancestors - otherwise we can apply no attributes.
-  if (filter_ && filter_->can_commute_with_alpha()) {
-    context->renderable_state_flags = LayerStateStack::CALLER_CAN_APPLY_OPACITY;
+  if (filter_) {
+    context->renderable_state_flags =
+        filter_->can_commute_with_opacity()
+            ? LayerStateStack::CALLER_CAN_APPLY_OPACITY
+            : 0;
   }
+  // else - we can apply whatever our children can apply.
 }
 
 void ColorFilterLayer::Paint(PaintContext& context) const {

flow/layers/layer_state_stack.cc

@@ -11,30 +11,30 @@ namespace flutter {
 using AutoRestore = LayerStateStack::AutoRestore;
 using MutatorContext = LayerStateStack::MutatorContext;
 
-void LayerStateStack::clear_delegates() {
+void LayerStateStack::clear_delegate() {
   if (canvas_) {
-    canvas_->restoreToCount(canvas_restore_count_);
+    canvas_->restoreToCount(restore_count_);
     canvas_ = nullptr;
   }
   if (builder_) {
-    builder_->restoreToCount(builder_restore_count_);
+    builder_->restoreToCount(restore_count_);
     builder_ = nullptr;
   }
 }
 
-void LayerStateStack::set_canvas_delegate(SkCanvas* canvas) {
-  clear_delegates();
+void LayerStateStack::set_delegate(SkCanvas* canvas) {
+  clear_delegate();
   if (canvas) {
-    canvas_restore_count_ = canvas->getSaveCount();
+    restore_count_ = canvas->getSaveCount();
     canvas_ = canvas;
     reapply_all(canvas, nullptr);
   }
 }
 
-void LayerStateStack::set_builder_delegate(DisplayListBuilder* builder) {
-  clear_delegates();
+void LayerStateStack::set_delegate(DisplayListBuilder* builder) {
+  clear_delegate();
   if (builder) {
-    builder_restore_count_ = builder->getSaveCount();
+    restore_count_ = builder->getSaveCount();
     builder_ = builder;
     reapply_all(nullptr, builder);
   }
@@ -358,7 +358,7 @@ void LayerStateStack::maybe_save_layer(
     const std::shared_ptr<const DlColorFilter>& filter) {
   if (outstanding_.color_filter || outstanding_.image_filter ||
       (outstanding_.opacity < SK_Scalar1 &&
-       !filter->can_commute_with_alpha())) {
+       !filter->can_commute_with_opacity())) {
     // TBD: compose the 2 color filters together.
     save_layer(outstanding_.save_layer_bounds);
   }

flow/layers/layer_state_stack.h

@@ -20,24 +20,35 @@ class LayerStateStack {
   }
 
   // Clears out any old delegate to make room for a new one.
-  void clear_delegates();
+  void clear_delegate();
 
+  // Return the SkCanvas delegate if the state stack has such a delegate.
+  // The state stack will only have either an SkCanvas or a Builder
+  // delegate at any given time.
+  // See also |builder_delegate|.
   SkCanvas* canvas_delegate() { return canvas_; }
+
+  // Return the DisplayListBuilder delegate if the state stack has such a
+  // delegate.
+  // The state stack will only have either an SkCanvas or a Builder
+  // delegate at any given time.
+  // See also |builder_delegate|.
+  DisplayListBuilder* builder_delegate() { return builder_; }
+
   // Clears the old delegate and sets the canvas delegate to the indicated
   // canvas (if not nullptr). This ensures that only one delegate - either
   // a canvas or a builder - is present at any one time.
-  void set_canvas_delegate(SkCanvas* canvas);
+  void set_delegate(SkCanvas* canvas);
 
-  DisplayListBuilder* builder_delegate() { return builder_; }
   // Clears the old delegate and sets the builder delegate to the indicated
   // buider (if not nullptr). This ensures that only one delegate - either
   // a canvas or a builder - is present at any one time.
-  void set_builder_delegate(DisplayListBuilder* builder);
-  void set_builder_delegate(sk_sp<DisplayListBuilder> builder) {
-    set_builder_delegate(builder.get());
+  void set_delegate(DisplayListBuilder* builder);
+  void set_delegate(sk_sp<DisplayListBuilder> builder) {
+    set_delegate(builder.get());
   }
-  void set_builder_delegate(DisplayListCanvasRecorder& recorder) {
-    set_builder_delegate(recorder.builder().get());
+  void set_delegate(DisplayListCanvasRecorder& recorder) {
+    set_delegate(recorder.builder().get());
   }
 
   class AutoRestore {
@@ -484,9 +495,8 @@ class LayerStateStack {
   friend class MutatorContext;
 
   SkCanvas* canvas_ = nullptr;
-  int canvas_restore_count_ = 0.0;
+  int restore_count_ = 0;
   DisplayListBuilder* builder_ = nullptr;
-  int builder_restore_count_ = 0.0;
   RenderingAttributes outstanding_;
 
   bool do_checkerboard_ = false;