switch to virtual methods to clean up the code a bit at the cost of 8…

… bytes per record

ci/licenses_golden/licenses_flutter

@@ -52,6 +52,7 @@ FILE: ../../../flutter/display_list/display_list_canvas_dispatcher.h
 FILE: ../../../flutter/display_list/display_list_canvas_recorder.cc
 FILE: ../../../flutter/display_list/display_list_canvas_recorder.h
 FILE: ../../../flutter/display_list/display_list_canvas_unittests.cc
+FILE: ../../../flutter/display_list/display_list_color_filter_unittests.cc
 FILE: ../../../flutter/display_list/display_list_color_filter.cc
 FILE: ../../../flutter/display_list/display_list_color_filter.h
 FILE: ../../../flutter/display_list/display_list_complexity.cc

display_list/BUILD.gn

@@ -42,6 +42,7 @@ source_set("unittests") {
 
   sources = [
     "display_list_canvas_unittests.cc",
+    "display_list_color_filter_unittests.cc",
     "display_list_unittests.cc",
   ]
 

display_list/display_list_builder.cc

@@ -66,7 +66,6 @@ DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect)
     : cull_rect_(cull_rect) {
   layer_stack_.emplace_back();
   current_layer_ = &layer_stack_.back();
-  current_color_filter_.reset(new DlNoColorFilter());
 }
 
 DisplayListBuilder::~DisplayListBuilder() {
@@ -137,45 +136,46 @@ void DisplayListBuilder::onSetImageFilter(sk_sp<SkImageFilter> filter) {
 }
 void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) {
   if (filter == nullptr) {
-    filter = &DlNoColorFilter::instance;
-  }
-  void* pod;
-  switch (filter->type()) {
-    case DlColorFilter::kNone:
-      current_color_filter_.reset(new DlNoColorFilter());
-      Push<ClearColorFilterOp>(0, 0);
-      break;
-    case DlColorFilter::kBlend: {
-      const DlBlendColorFilter* blend_filter = filter->asABlendFilter();
-      FML_DCHECK(blend_filter);
-      current_color_filter_.reset(new DlBlendColorFilter(blend_filter));
-      pod = Push<SetColorFilterOp>(blend_filter->size(), 0);
-      new (pod) DlBlendColorFilter(blend_filter);
-      break;
+    if (!current_color_filter_) {
+      return;
     }
-    case DlColorFilter::kMatrix: {
-      const DlMatrixColorFilter* matrix_filter = filter->asAMatrixFilter();
-      FML_DCHECK(matrix_filter);
-      current_color_filter_.reset(new DlMatrixColorFilter(matrix_filter));
-      pod = Push<SetColorFilterOp>(matrix_filter->size(), 0);
-      new (pod) DlMatrixColorFilter(matrix_filter->asAMatrixFilter());
-      break;
+    current_color_filter_ = nullptr;
+    Push<ClearColorFilterOp>(0, 0);
+  } else {
+    if (current_color_filter_ && *current_color_filter_ == *filter) {
+      return;
     }
-    case DlColorFilter::kSrgbToLinearGamma:
-      current_color_filter_.reset(new DlSrgbToLinearGammaColorFilter());
-      pod = Push<SetColorFilterOp>(filter->size(), 0);
-      new (pod) DlSrgbToLinearGammaColorFilter();
-      break;
-    case DlColorFilter::kLinearToSrgbGamma:
-      current_color_filter_.reset(new DlLinearToSrgbGammaColorFilter());
-      pod = Push<SetColorFilterOp>(filter->size(), 0);
-      new (pod) DlLinearToSrgbGammaColorFilter();
-      break;
-    case DlColorFilter::kUnknown: {
-      const sk_sp<SkColorFilter> sk_filter = filter->sk_filter();
-      current_color_filter_.reset(new DlUnknownColorFilter(sk_filter));
-      Push<SetSkColorFilterOp>(0, 0, sk_filter);
-      break;
+    current_color_filter_ = filter->shared();
+    switch (filter->type()) {
+      case DlColorFilter::kBlend: {
+        const DlBlendColorFilter* blend_filter = filter->asBlend();
+        FML_DCHECK(blend_filter);
+        void* pod = Push<SetColorFilterOp>(blend_filter->size(), 0);
+        new (pod) DlBlendColorFilter(blend_filter);
+        break;
+      }
+      case DlColorFilter::kMatrix: {
+        const DlMatrixColorFilter* matrix_filter = filter->asMatrix();
+        FML_DCHECK(matrix_filter);
+        void* pod = Push<SetColorFilterOp>(matrix_filter->size(), 0);
+        new (pod) DlMatrixColorFilter(matrix_filter);
+        break;
+      }
+      case DlColorFilter::kSrgbToLinearGamma: {
+        void* pod = Push<SetColorFilterOp>(filter->size(), 0);
+        new (pod) DlSrgbToLinearGammaColorFilter();
+        break;
+      }
+      case DlColorFilter::kLinearToSrgbGamma: {
+        void* pod = Push<SetColorFilterOp>(filter->size(), 0);
+        new (pod) DlLinearToSrgbGammaColorFilter();
+        break;
+      }
+      case DlColorFilter::kUnknown: {
+        const sk_sp<SkColorFilter> sk_filter = filter->sk_filter();
+        Push<SetSkColorFilterOp>(0, 0, sk_filter);
+        break;
+      }
     }
   }
   UpdateCurrentOpacityCompatibility();
@@ -251,8 +251,12 @@ void DisplayListBuilder::setAttributesFromPaint(
     // we must clear it because it is a second potential color filter
     // that is composed with the paint's color filter.
     setInvertColors(false);
-    DlColorFilter filter = DlColorFilter::From(paint.getColorFilter());
-    setColorFilter(&filter);
+    SkColorFilter* color_filter = paint.getColorFilter();
+    if (color_filter) {
+      setColorFilter(DlColorFilter::From(color_filter).get());
+    } else {
+      setColorFilter(nullptr);
+    }
   }
   if (flags.applies_image_filter()) {
     setImageFilter(sk_ref_sp(paint.getImageFilter()));

display_list/display_list_builder.h

@@ -94,9 +94,8 @@ class DisplayListBuilder final : public virtual Dispatcher,
     }
   }
   void setColorFilter(const DlColorFilter* filter) override {
-    if (!DlColorFilter::Equals(current_color_filter_.get(), filter)) {
-      onSetColorFilter(filter);
-    }
+    // onSetColorFilter will deal with whether the filter is new
+    onSetColorFilter(filter);
   }
   void setPathEffect(sk_sp<SkPathEffect> effect) override {
     if (current_path_effect_ != effect) {
@@ -339,10 +338,10 @@ class DisplayListBuilder final : public virtual Dispatcher,
   }
 
   void UpdateCurrentOpacityCompatibility() {
-    current_opacity_compatibility_ =                              //
-        current_color_filter_->type() == DlColorFilter::kNone &&  //
-        !current_invert_colors_ &&                                //
-        current_blender_ == nullptr &&                            //
+    current_opacity_compatibility_ =         //
+        current_color_filter_ == nullptr &&  //
+        !current_invert_colors_ &&           //
+        current_blender_ == nullptr &&       //
         IsOpacityCompatible(current_blend_mode_);
   }
 
@@ -411,7 +410,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
   SkBlendMode current_blend_mode_ = SkBlendMode::kSrcOver;
   sk_sp<SkBlender> current_blender_;
   sk_sp<SkShader> current_shader_;
-  std::unique_ptr<DlColorFilter> current_color_filter_;
+  std::shared_ptr<const DlColorFilter> current_color_filter_;
   sk_sp<SkImageFilter> current_image_filter_;
   sk_sp<SkPathEffect> current_path_effect_;
   sk_sp<SkMaskFilter> current_mask_filter_;

display_list/display_list_canvas_unittests.cc

@@ -836,7 +836,7 @@ class CanvasCompareTester {
           0, 0, 0, 0.5, 0,
       };
       // clang-format on
-      DlColorFilter filter = DlMatrixColorFilter(rotate_alpha_color_matrix);
+      DlMatrixColorFilter filter(rotate_alpha_color_matrix);
       {
         RenderWith(testP, env, tolerance,
                    CaseParameters(
@@ -1141,7 +1141,7 @@ class CanvasCompareTester {
          1.0,  1.0,  1.0, 1.0,   0,
       };
       // clang-format on
-      DlColorFilter filter = DlMatrixColorFilter(rotate_color_matrix);
+      DlMatrixColorFilter filter(rotate_color_matrix);
       {
         SkColor bg = SK_ColorWHITE;
         RenderWith(testP, env, tolerance,

display_list/display_list_color_filter.cc

@@ -6,153 +6,43 @@
 
 namespace flutter {
 
-DlColorFilter DlColorFilter::From(SkColorFilter* sk_filter) {
+std::shared_ptr<DlColorFilter> DlColorFilter::From(SkColorFilter* sk_filter) {
   if (sk_filter == nullptr) {
-    return DlNoColorFilter::instance;
+    return nullptr;
+  }
+  if (sk_filter == DlSrgbToLinearGammaColorFilter::sk_filter_.get()) {
+    // Skia implements these filters as a singleton.
+    return DlSrgbToLinearGammaColorFilter::instance;
+  }
+  if (sk_filter == DlLinearToSrgbGammaColorFilter::sk_filter_.get()) {
+    // Skia implements these filters as a singleton.
+    return DlLinearToSrgbGammaColorFilter::instance;
   }
   {
     SkColor color;
     SkBlendMode mode;
     if (sk_filter->asAColorMode(&color, &mode)) {
-      return DlBlendColorFilter(color, mode);
+      return std::make_shared<DlBlendColorFilter>(color, mode);
     }
   }
   {
     float matrix[20];
     if (sk_filter->asAColorMatrix(matrix)) {
-      return DlMatrixColorFilter(matrix);
+      return std::make_shared<DlMatrixColorFilter>(matrix);
     }
   }
-  return DlUnknownColorFilter(sk_ref_sp(sk_filter));
-}
-
-DlColorFilter::~DlColorFilter() {
-  if (type_ == kUnknown) {
-    delete static_cast<const DlUnknownColorFilter*>(this);
-    type_ = kNone;
-  }
-}
-
-size_t DlColorFilter::size() const {
-  switch (type_) {
-    case kNone:
-      // This query is used for allocating raw storage within a buffer for
-      // storing these objects. Such a technique should not be used for
-      // the |kNone| version of these objects.
-      FML_DCHECK(false);
-      return 0;
-    case kBlend:
-      return static_cast<const DlBlendColorFilter*>(this)->size();
-    case kMatrix:
-      return static_cast<const DlMatrixColorFilter*>(this)->size();
-    case kSrgbToLinearGamma:
-      return static_cast<const DlSrgbToLinearGammaColorFilter*>(this)->size();
-    case kLinearToSrgbGamma:
-      return static_cast<const DlLinearToSrgbGammaColorFilter*>(this)->size();
-    case kUnknown:
-      return static_cast<const DlUnknownColorFilter*>(this)->size();
-  }
-}
-
-bool DlColorFilter::equals(const DlColorFilter* other) const {
-  return Equals(this, other);
-}
-
-bool DlColorFilter::Equals(const DlColorFilter* a, const DlColorFilter* b) {
-  if (a == b) {
-    return true;
-  }
-  if (a == nullptr || b == nullptr) {
-    return false;
-  }
-  if (a->type_ != b->type_) {
-    return false;
-  }
-  switch (a->type_) {
-    case kNone:
-      return true;
-    case kBlend:
-      return static_cast<const DlBlendColorFilter*>(a)->equals(
-          static_cast<const DlBlendColorFilter*>(b));
-    case kMatrix:
-      return static_cast<const DlMatrixColorFilter*>(a)->equals(
-          static_cast<const DlMatrixColorFilter*>(b));
-    case kSrgbToLinearGamma:
-    case kLinearToSrgbGamma:
-      return true;
-    case kUnknown:
-      return static_cast<const DlUnknownColorFilter*>(a)->equals(
-          static_cast<const DlUnknownColorFilter*>(b));
-  }
+  return std::make_shared<DlUnknownColorFilter>(sk_ref_sp(sk_filter));
 }
 
-sk_sp<SkColorFilter> DlColorFilter::sk_filter() const {
-  switch (type_) {
-    case kNone:
-      return nullptr;
-    case kBlend:
-      return static_cast<const DlBlendColorFilter*>(this)->sk_filter();
-    case kMatrix:
-      return static_cast<const DlMatrixColorFilter*>(this)->sk_filter();
-    case kSrgbToLinearGamma:
-      return static_cast<const DlSrgbToLinearGammaColorFilter*>(this)
-          ->sk_filter();
-    case kLinearToSrgbGamma:
-      return static_cast<const DlLinearToSrgbGammaColorFilter*>(this)
-          ->sk_filter();
-    case kUnknown:
-      return static_cast<const DlUnknownColorFilter*>(this)->sk_filter();
-  }
-}
-
-std::shared_ptr<const DlColorFilter> DlColorFilter::shared() const {
-  switch (type_) {
-    case kNone:
-      return std::make_shared<DlNoColorFilter>();
-    case kBlend:
-      return std::make_shared<DlBlendColorFilter>(
-          static_cast<const DlBlendColorFilter*>(this));
-    case kMatrix:
-      return std::make_shared<DlMatrixColorFilter>(
-          static_cast<const DlMatrixColorFilter*>(this));
-    case kSrgbToLinearGamma:
-      return std::make_shared<DlSrgbToLinearGammaColorFilter>(
-          static_cast<const DlSrgbToLinearGammaColorFilter*>(this));
-    case kLinearToSrgbGamma:
-      return std::make_shared<DlLinearToSrgbGammaColorFilter>(
-          static_cast<const DlLinearToSrgbGammaColorFilter*>(this));
-    case kUnknown:
-      return std::make_shared<DlUnknownColorFilter>(
-          static_cast<const DlUnknownColorFilter*>(this));
-  }
-}
-
-bool DlColorFilter::modifies_transparent_black() const {
-  switch (type_) {
-    case kNone:
-    case kSrgbToLinearGamma:
-    case kLinearToSrgbGamma:
-      return false;
-    case kBlend:
-      // Look at blend and color to make a faster determination?
-    case kMatrix:
-      // Look at the matrix to make a faster determination?
-      // Basically, are the translation components all 0?
-    case kUnknown:
-      return sk_filter()->filterColor(SK_ColorTRANSPARENT) !=
-             SK_ColorTRANSPARENT;
-  }
-}
-
-const DlNoColorFilter DlNoColorFilter::instance = DlNoColorFilter();
-
-const DlSrgbToLinearGammaColorFilter DlSrgbToLinearGammaColorFilter::instance =
-    DlSrgbToLinearGammaColorFilter();
+const std::shared_ptr<DlSrgbToLinearGammaColorFilter>
+    DlSrgbToLinearGammaColorFilter::instance =
+        std::make_shared<DlSrgbToLinearGammaColorFilter>();
 const sk_sp<SkColorFilter> DlSrgbToLinearGammaColorFilter::sk_filter_ =
     SkColorFilters::SRGBToLinearGamma();
 
-const DlLinearToSrgbGammaColorFilter DlLinearToSrgbGammaColorFilter::instance =
-    DlLinearToSrgbGammaColorFilter();
+const std::shared_ptr<DlLinearToSrgbGammaColorFilter>
+    DlLinearToSrgbGammaColorFilter::instance =
+        std::make_shared<DlLinearToSrgbGammaColorFilter>();
 const sk_sp<SkColorFilter> DlLinearToSrgbGammaColorFilter::sk_filter_ =
     SkColorFilters::LinearToSRGBGamma();