From 92a4e5c3760cae979b19eca3018980f5c012b407 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 10:48:47 -0700
Subject: [PATCH 01/10] Initial try at gradient-only-dithering for the Sk
 backend.

---
 display_list/dl_paint.h                     | 14 +++++
 display_list/effects/dl_color_source.h      | 15 ++++++
 display_list/skia/dl_sk_canvas.cc           | 14 ++++-
 display_list/skia/dl_sk_canvas.h            |  2 +
 display_list/skia/dl_sk_canvas_unittests.cc | 58 +++++++++++++++++++++
 5 files changed, 101 insertions(+), 2 deletions(-)
 create mode 100644 display_list/skia/dl_sk_canvas_unittests.cc

diff --git a/display_list/dl_paint.h b/display_list/dl_paint.h
index 3d9220f57e3d6..430d557a9de79 100644
--- a/display_list/dl_paint.h
+++ b/display_list/dl_paint.h
@@ -66,6 +66,20 @@ class DlPaint {
     return *this;
   }
 
+  // In the Impeller backend, we'll *always* apply dithering to gradients, and
+  // *never* apply dithering otherwise, and there will be no user-facing feature
+  // to change this behavior.
+  //
+  // To temporarily match this behavior in the Skia backend, we tag gradients
+  // with a special flag, and then check for that flag when converting from
+  // DlPaint to SkPaint.
+  //
+  // TODO(matanl): Remove this flag when the Skia backend is removed,
+  // https://github.com/flutter/flutter/issues/112498.
+  bool isDitherHintForSkBackend() const {
+    return colorSource_ ? colorSource_->isDitherHintForSkBackend() : false;
+  }
+
   bool isInvertColors() const { return isInvertColors_; }
   DlPaint& setInvertColors(bool isInvertColors) {
     isInvertColors_ = isInvertColors;
diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h
index 394fe092af0fe..07eb89d95aaf6 100644
--- a/display_list/effects/dl_color_source.h
+++ b/display_list/effects/dl_color_source.h
@@ -119,6 +119,19 @@ class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
   ///
   virtual bool isUIThreadSafe() const = 0;
 
+  //----------------------------------------------------------------------------
+  /// @brief      If the underlying platform data should have dithering
+  ///             applied to it (if requested), this method returns true.
+  ///             In practice, this is only true for gradients and is only
+  ///             used by the Skia backend.
+  ///
+  ///             TODO(matanl): Remove this flag when the Skia backend is
+  ///             removed, https://github.com/flutter/flutter/issues/112498.
+  ///
+  /// @return     True if the class represents the output of a gradient.
+  ///
+  virtual bool isDitherHintForSkBackend() const { return false; }
+
   // Return a DlColorColorSource pointer to this object iff it is an Color
   // type of ColorSource, otherwise return nullptr.
   virtual const DlColorColorSource* asColor() const { return nullptr; }
@@ -287,6 +300,8 @@ class DlGradientColorSourceBase : public DlMatrixColorSourceBase {
     return true;
   }
 
+  bool isDitherHintForSkBackend() const override { return true; }
+
   DlTileMode tile_mode() const { return mode_; }
   int stop_count() const { return stop_count_; }
   const DlColor* colors() const {
diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index 842f4593035ba..18a4bd0bdb100 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -23,11 +23,21 @@ constexpr float kInvertColorMatrix[20] = {
 };
 // clang-format on
 
-static SkPaint ToSk(const DlPaint& paint, bool force_stroke = false) {
+static SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   SkPaint sk_paint;
 
   sk_paint.setAntiAlias(paint.isAntiAlias());
-  sk_paint.setDither(paint.isDither());
+
+  // On the Impeller backend, we will only support dithering of *gradients*,
+  // and it will be enabled by default (without the option to disable it). Until
+  // Skia support is completely removed, we only want to respect the dither flag
+  // for gradients (otherwise it will also apply to, for example, images, which
+  // is not supported in Impeller).
+  //
+  // See https://github.com/flutter/flutter/issues/112498.
+  if (paint.isDitherHintForSkBackend()) {
+    sk_paint.setDither(paint.isDither());
+  }
 
   sk_paint.setColor(paint.getColor());
   sk_paint.setBlendMode(ToSk(paint.getBlendMode()));
diff --git a/display_list/skia/dl_sk_canvas.h b/display_list/skia/dl_sk_canvas.h
index af774b74342f0..978449a134f62 100644
--- a/display_list/skia/dl_sk_canvas.h
+++ b/display_list/skia/dl_sk_canvas.h
@@ -10,6 +10,8 @@
 
 namespace flutter {
 
+static SkPaint ToSk(const DlPaint& paint, bool force_stroke = true);
+
 // An adapter to receive DlCanvas calls and dispatch them into
 // an SkCanvas.
 class DlSkCanvasAdapter final : public virtual DlCanvas {
diff --git a/display_list/skia/dl_sk_canvas_unittests.cc b/display_list/skia/dl_sk_canvas_unittests.cc
new file mode 100644
index 0000000000000..c393a2c9ff02c
--- /dev/null
+++ b/display_list/skia/dl_sk_canvas_unittests.cc
@@ -0,0 +1,58 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "display_list/dl_tile_mode.h"
+#include "display_list/effects/dl_color_source.h"
+#include "flutter/display_list/skia/dl_sk_canvas.h"
+
+#include "flutter/display_list/dl_paint.h"
+#include "gtest/gtest.h"
+
+namespace flutter {
+namespace testing {
+
+TEST(DisplayListSkCanvas, ToSkDitheringDisabled) {
+  // Test that when using the utility method "ToSk", the resulting SkPaint
+  // does not have "isDither" set to true, even if it's requested by the
+  // Flutter (dart:ui) paint, because it's not a supported feature in the
+  // Impeller backend.
+
+  // Create a new DlPaint with isDither set to true.
+  //
+  // This mimics the behavior of ui.Paint.enableDithering = true.
+  DlPaint dl_paint;
+  dl_paint.setDither(true);
+
+  // Convert to SkPaint.
+  SkPaint sk_paint = ToSk(dl_paint);
+
+  // Verify that isDither is false.
+  EXPECT_FALSE(sk_paint.isDither());
+}
+
+TEST(DisplayListSkCanvas, ToSkDitheringEnabledForGradients) {
+  // Test that when using the utility method "ToSk", the resulting SkPaint
+  // has "isDither" set to true, if the paint is a gradient, because it's
+  // a supported feature in the Impeller backend.
+
+  // Create a new DlPaint with isDither set to true.
+  //
+  // This mimics the behavior of ui.Paint.enableDithering = true.
+  DlPaint dl_paint;
+  dl_paint.setDither(true);
+
+  // Set the paint to be a gradient.
+  dl_paint.setColorSource(DlColorSource::MakeLinear(SkPoint::Make(0, 0),
+                                                    SkPoint::Make(100, 100), 0,
+                                                    0, 0, DlTileMode::kClamp));
+
+  // Convert to SkPaint.
+  SkPaint sk_paint = ToSk(dl_paint);
+
+  // Verify that isDither is true.
+  EXPECT_TRUE(sk_paint.isDither());
+}
+
+}  // namespace testing
+}  // namespace flutter

From 3fe2554c028a8c382a841de4e6f97d885b71b762 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 10:49:31 -0700
Subject: [PATCH 02/10] Licenses and GN.

---
 ci/licenses_golden/excluded_files | 1 +
 display_list/BUILD.gn             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
index e104b088334ba..1ad7772fbefc8 100644
--- a/ci/licenses_golden/excluded_files
+++ b/ci/licenses_golden/excluded_files
@@ -41,6 +41,7 @@
 ../../../flutter/display_list/effects/dl_path_effect_unittests.cc
 ../../../flutter/display_list/geometry/dl_region_unittests.cc
 ../../../flutter/display_list/geometry/dl_rtree_unittests.cc
+../../../flutter/display_list/skia/dl_sk_canvas_unittests.cc
 ../../../flutter/display_list/skia/dl_sk_conversions_unittests.cc
 ../../../flutter/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
 ../../../flutter/display_list/testing
diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn
index 97c9683511b61..a8aca47a49d48 100644
--- a/display_list/BUILD.gn
+++ b/display_list/BUILD.gn
@@ -116,6 +116,7 @@ if (enable_unittests) {
       "effects/dl_path_effect_unittests.cc",
       "geometry/dl_region_unittests.cc",
       "geometry/dl_rtree_unittests.cc",
+      "skia/dl_sk_canvas_unittests.cc",
       "skia/dl_sk_conversions_unittests.cc",
       "skia/dl_sk_paint_dispatcher_unittests.cc",
       "utils/dl_matrix_clip_tracker_unittests.cc",

From 839610d2b2712609642b60b795061533b0e35068 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 11:13:20 -0700
Subject: [PATCH 03/10] Allow calling ToSk from other files (i.e. unittests).

---
 display_list/skia/dl_sk_canvas.cc           | 2 +-
 display_list/skia/dl_sk_canvas.h            | 2 +-
 display_list/skia/dl_sk_canvas_unittests.cc | 6 +-----
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index 18a4bd0bdb100..b0caa7f71866f 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -23,7 +23,7 @@ constexpr float kInvertColorMatrix[20] = {
 };
 // clang-format on
 
-static SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
+SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   SkPaint sk_paint;
 
   sk_paint.setAntiAlias(paint.isAntiAlias());
diff --git a/display_list/skia/dl_sk_canvas.h b/display_list/skia/dl_sk_canvas.h
index 978449a134f62..b8b9d408d9d53 100644
--- a/display_list/skia/dl_sk_canvas.h
+++ b/display_list/skia/dl_sk_canvas.h
@@ -10,7 +10,7 @@
 
 namespace flutter {
 
-static SkPaint ToSk(const DlPaint& paint, bool force_stroke = true);
+SkPaint ToSk(const DlPaint& paint, bool force_stroke = false);
 
 // An adapter to receive DlCanvas calls and dispatch them into
 // an SkCanvas.
diff --git a/display_list/skia/dl_sk_canvas_unittests.cc b/display_list/skia/dl_sk_canvas_unittests.cc
index c393a2c9ff02c..8dd7d464848e1 100644
--- a/display_list/skia/dl_sk_canvas_unittests.cc
+++ b/display_list/skia/dl_sk_canvas_unittests.cc
@@ -5,8 +5,8 @@
 #include "display_list/dl_tile_mode.h"
 #include "display_list/effects/dl_color_source.h"
 #include "flutter/display_list/skia/dl_sk_canvas.h"
-
 #include "flutter/display_list/dl_paint.h"
+
 #include "gtest/gtest.h"
 
 namespace flutter {
@@ -24,10 +24,8 @@ TEST(DisplayListSkCanvas, ToSkDitheringDisabled) {
   DlPaint dl_paint;
   dl_paint.setDither(true);
 
-  // Convert to SkPaint.
   SkPaint sk_paint = ToSk(dl_paint);
 
-  // Verify that isDither is false.
   EXPECT_FALSE(sk_paint.isDither());
 }
 
@@ -47,10 +45,8 @@ TEST(DisplayListSkCanvas, ToSkDitheringEnabledForGradients) {
                                                     SkPoint::Make(100, 100), 0,
                                                     0, 0, DlTileMode::kClamp));
 
-  // Convert to SkPaint.
   SkPaint sk_paint = ToSk(dl_paint);
 
-  // Verify that isDither is true.
   EXPECT_TRUE(sk_paint.isDither());
 }
 

From 760944ff50337e0a49eb064cb28c69dc01732fb3 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 11:14:00 -0700
Subject: [PATCH 04/10] Clang format.

---
 display_list/skia/dl_sk_canvas_unittests.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/display_list/skia/dl_sk_canvas_unittests.cc b/display_list/skia/dl_sk_canvas_unittests.cc
index 8dd7d464848e1..4e665570d67c8 100644
--- a/display_list/skia/dl_sk_canvas_unittests.cc
+++ b/display_list/skia/dl_sk_canvas_unittests.cc
@@ -4,8 +4,8 @@
 
 #include "display_list/dl_tile_mode.h"
 #include "display_list/effects/dl_color_source.h"
-#include "flutter/display_list/skia/dl_sk_canvas.h"
 #include "flutter/display_list/dl_paint.h"
+#include "flutter/display_list/skia/dl_sk_canvas.h"
 
 #include "gtest/gtest.h"
 

From a1e1fe7de091c50b5e18ef51aee1f8dec13ccdc4 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 12:24:39 -0700
Subject: [PATCH 05/10] Remove unittests that test general dithering behavior.

---
 .../testing/dl_rendering_unittests.cc         | 57 -------------------
 1 file changed, 57 deletions(-)

diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc
index 491c82a0f9ffb..23a80c2f13d92 100644
--- a/display_list/testing/dl_rendering_unittests.cc
+++ b/display_list/testing/dl_rendering_unittests.cc
@@ -1247,63 +1247,6 @@ class CanvasCompareTester {
               [=](DlCanvas* cv, DlPaint& p) { dl_aa_setup(cv, p, false); }));
     }
 
-    {
-      // The CPU renderer does not always dither for solid colors and we
-      // need to use a non-default color (default is black) on an opaque
-      // surface, so we use a shader instead of a color. Also, thin stroked
-      // primitives (mainly drawLine and drawPoints) do not show much
-      // dithering so we use a non-trivial stroke width as well.
-      RenderEnvironment dither_env = RenderEnvironment::Make565(env.provider());
-      if (!dither_env.valid()) {
-        // Currently only happens on Metal backend
-        static std::set<std::string> warnings_sent;
-        std::string name = dither_env.backend_name();
-        if (warnings_sent.find(name) == warnings_sent.end()) {
-          warnings_sent.insert(name);
-          FML_LOG(INFO) << "Skipping Dithering tests on " << name;
-        }
-      } else {
-        DlColor dither_bg = DlColor::kBlack();
-        SkSetup sk_dither_setup = [=](SkCanvas*, SkPaint& p) {
-          p.setShader(kTestSkImageColorSource);
-          p.setAlpha(0xf0);
-          p.setStrokeWidth(5.0);
-        };
-        DlSetup dl_dither_setup = [=](DlCanvas*, DlPaint& p) {
-          p.setColorSource(&kTestDlImageColorSource);
-          p.setAlpha(0xf0);
-          p.setStrokeWidth(5.0);
-        };
-        dither_env.init_ref(sk_dither_setup, testP.sk_renderer(),
-                            dl_dither_setup, testP.dl_renderer(), dither_bg);
-        quickCompareToReference(dither_env, "dither");
-        RenderWith(testP, dither_env, tolerance,
-                   CaseParameters(
-                       "Dither == True",
-                       [=](SkCanvas* cv, SkPaint& p) {
-                         sk_dither_setup(cv, p);
-                         p.setDither(true);
-                       },
-                       [=](DlCanvas* cv, DlPaint& p) {
-                         dl_dither_setup(cv, p);
-                         p.setDither(true);
-                       })
-                       .with_bg(dither_bg));
-        RenderWith(testP, dither_env, tolerance,
-                   CaseParameters(
-                       "Dither = False",
-                       [=](SkCanvas* cv, SkPaint& p) {
-                         sk_dither_setup(cv, p);
-                         p.setDither(false);
-                       },
-                       [=](DlCanvas* cv, DlPaint& p) {
-                         dl_dither_setup(cv, p);
-                         p.setDither(false);
-                       })
-                       .with_bg(dither_bg));
-      }
-    }
-
     RenderWith(
         testP, env, tolerance,
         CaseParameters(

From af290e9b26ffc13fe96cd8c7f2166d4a43435598 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 12:55:19 -0700
Subject: [PATCH 06/10] Tweak based on Jim's suggestions.

---
 display_list/dl_paint.h                |  2 +-
 display_list/effects/dl_color_source.h |  9 +++------
 display_list/skia/dl_sk_canvas.cc      | 25 +++++++++++++------------
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/display_list/dl_paint.h b/display_list/dl_paint.h
index 430d557a9de79..e88c6cc532cc8 100644
--- a/display_list/dl_paint.h
+++ b/display_list/dl_paint.h
@@ -77,7 +77,7 @@ class DlPaint {
   // TODO(matanl): Remove this flag when the Skia backend is removed,
   // https://github.com/flutter/flutter/issues/112498.
   bool isDitherHintForSkBackend() const {
-    return colorSource_ ? colorSource_->isDitherHintForSkBackend() : false;
+    return colorSource_ ? colorSource_->isGradient() : false;
   }
 
   bool isInvertColors() const { return isInvertColors_; }
diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h
index 07eb89d95aaf6..11a141da9c905 100644
--- a/display_list/effects/dl_color_source.h
+++ b/display_list/effects/dl_color_source.h
@@ -120,17 +120,14 @@ class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
   virtual bool isUIThreadSafe() const = 0;
 
   //----------------------------------------------------------------------------
-  /// @brief      If the underlying platform data should have dithering
-  ///             applied to it (if requested), this method returns true.
-  ///             In practice, this is only true for gradients and is only
-  ///             used by the Skia backend.
+  /// @brief      If the underlying platform data represents a gradient.
   ///
   ///             TODO(matanl): Remove this flag when the Skia backend is
   ///             removed, https://github.com/flutter/flutter/issues/112498.
   ///
   /// @return     True if the class represents the output of a gradient.
   ///
-  virtual bool isDitherHintForSkBackend() const { return false; }
+  virtual bool isGradient() const { return false; }
 
   // Return a DlColorColorSource pointer to this object iff it is an Color
   // type of ColorSource, otherwise return nullptr.
@@ -300,7 +297,7 @@ class DlGradientColorSourceBase : public DlMatrixColorSourceBase {
     return true;
   }
 
-  bool isDitherHintForSkBackend() const override { return true; }
+  bool isGradient() const override { return true; }
 
   DlTileMode tile_mode() const { return mode_; }
   int stop_count() const { return stop_count_; }
diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index b0caa7f71866f..c2e84d3759335 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -27,18 +27,6 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   SkPaint sk_paint;
 
   sk_paint.setAntiAlias(paint.isAntiAlias());
-
-  // On the Impeller backend, we will only support dithering of *gradients*,
-  // and it will be enabled by default (without the option to disable it). Until
-  // Skia support is completely removed, we only want to respect the dither flag
-  // for gradients (otherwise it will also apply to, for example, images, which
-  // is not supported in Impeller).
-  //
-  // See https://github.com/flutter/flutter/issues/112498.
-  if (paint.isDitherHintForSkBackend()) {
-    sk_paint.setDither(paint.isDither());
-  }
-
   sk_paint.setColor(paint.getColor());
   sk_paint.setBlendMode(ToSk(paint.getBlendMode()));
   sk_paint.setStyle(force_stroke ? SkPaint::kStroke_Style
@@ -59,6 +47,19 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
     color_filter = invert_filter;
   }
   sk_paint.setColorFilter(color_filter);
+
+  // On the Impeller backend, we will only support dithering of *gradients*,
+  // and it will be enabled by default (without the option to disable it). Until
+  // Skia support is completely removed, we only want to respect the dither flag
+  // for gradients (otherwise it will also apply to, for example, images, which
+  // is not supported in Impeller).
+  //
+  // See https://github.com/flutter/flutter/issues/112498.
+  auto color_source = paint.getColorSourcePtr();
+  if (color_source && color_source->isGradient()) {
+    sk_paint.setDither(paint.isDither());
+  }
+
   sk_paint.setMaskFilter(ToSk(paint.getMaskFilterPtr()));
   sk_paint.setPathEffect(ToSk(paint.getPathEffectPtr()));
 

From bfefb9ae4aa79e562fc627d5fa9958ac64a21300 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 14:22:20 -0700
Subject: [PATCH 07/10] Remove dead code and duplication of code.

---
 display_list/dl_paint.h           | 14 --------------
 display_list/skia/dl_sk_canvas.cc |  3 +--
 2 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/display_list/dl_paint.h b/display_list/dl_paint.h
index e88c6cc532cc8..3d9220f57e3d6 100644
--- a/display_list/dl_paint.h
+++ b/display_list/dl_paint.h
@@ -66,20 +66,6 @@ class DlPaint {
     return *this;
   }
 
-  // In the Impeller backend, we'll *always* apply dithering to gradients, and
-  // *never* apply dithering otherwise, and there will be no user-facing feature
-  // to change this behavior.
-  //
-  // To temporarily match this behavior in the Skia backend, we tag gradients
-  // with a special flag, and then check for that flag when converting from
-  // DlPaint to SkPaint.
-  //
-  // TODO(matanl): Remove this flag when the Skia backend is removed,
-  // https://github.com/flutter/flutter/issues/112498.
-  bool isDitherHintForSkBackend() const {
-    return colorSource_ ? colorSource_->isGradient() : false;
-  }
-
   bool isInvertColors() const { return isInvertColors_; }
   DlPaint& setInvertColors(bool isInvertColors) {
     isInvertColors_ = isInvertColors;
diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index c2e84d3759335..700336225ab99 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -35,8 +35,6 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   sk_paint.setStrokeMiter(paint.getStrokeMiter());
   sk_paint.setStrokeCap(ToSk(paint.getStrokeCap()));
   sk_paint.setStrokeJoin(ToSk(paint.getStrokeJoin()));
-
-  sk_paint.setShader(ToSk(paint.getColorSourcePtr()));
   sk_paint.setImageFilter(ToSk(paint.getImageFilterPtr()));
   auto color_filter = ToSk(paint.getColorFilterPtr());
   if (paint.isInvertColors()) {
@@ -56,6 +54,7 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   //
   // See https://github.com/flutter/flutter/issues/112498.
   auto color_source = paint.getColorSourcePtr();
+  sk_paint.setShader(ToSk(color_source));
   if (color_source && color_source->isGradient()) {
     sk_paint.setDither(paint.isDither());
   }

From c01ac643aaf718ba2f925fcb50f126807c2b713a Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 15:26:02 -0700
Subject: [PATCH 08/10] Address comments.

---
 ci/licenses_golden/excluded_files             |  1 -
 display_list/BUILD.gn                         |  1 -
 display_list/skia/dl_sk_canvas.cc             | 22 ++++----
 display_list/skia/dl_sk_canvas_unittests.cc   | 54 -------------------
 .../skia/dl_sk_conversions_unittests.cc       | 43 ++++++++++++++-
 5 files changed, 53 insertions(+), 68 deletions(-)
 delete mode 100644 display_list/skia/dl_sk_canvas_unittests.cc

diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
index 1ad7772fbefc8..e104b088334ba 100644
--- a/ci/licenses_golden/excluded_files
+++ b/ci/licenses_golden/excluded_files
@@ -41,7 +41,6 @@
 ../../../flutter/display_list/effects/dl_path_effect_unittests.cc
 ../../../flutter/display_list/geometry/dl_region_unittests.cc
 ../../../flutter/display_list/geometry/dl_rtree_unittests.cc
-../../../flutter/display_list/skia/dl_sk_canvas_unittests.cc
 ../../../flutter/display_list/skia/dl_sk_conversions_unittests.cc
 ../../../flutter/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
 ../../../flutter/display_list/testing
diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn
index a8aca47a49d48..97c9683511b61 100644
--- a/display_list/BUILD.gn
+++ b/display_list/BUILD.gn
@@ -116,7 +116,6 @@ if (enable_unittests) {
       "effects/dl_path_effect_unittests.cc",
       "geometry/dl_region_unittests.cc",
       "geometry/dl_rtree_unittests.cc",
-      "skia/dl_sk_canvas_unittests.cc",
       "skia/dl_sk_conversions_unittests.cc",
       "skia/dl_sk_paint_dispatcher_unittests.cc",
       "utils/dl_matrix_clip_tracker_unittests.cc",
diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index 700336225ab99..9b5d07810f4ac 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -46,17 +46,19 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   }
   sk_paint.setColorFilter(color_filter);
 
-  // On the Impeller backend, we will only support dithering of *gradients*,
-  // and it will be enabled by default (without the option to disable it). Until
-  // Skia support is completely removed, we only want to respect the dither flag
-  // for gradients (otherwise it will also apply to, for example, images, which
-  // is not supported in Impeller).
-  //
-  // See https://github.com/flutter/flutter/issues/112498.
   auto color_source = paint.getColorSourcePtr();
-  sk_paint.setShader(ToSk(color_source));
-  if (color_source && color_source->isGradient()) {
-    sk_paint.setDither(paint.isDither());
+  if (color_source) {
+    // On the Impeller backend, we will only support dithering of *gradients*,
+    // and it will be enabled by default (without the option to disable it).
+    // Until Skia support is completely removed, we only want to respect the
+    // dither flag for gradients (otherwise it will also apply to, for example,
+    // images, which is not supported in Impeller).
+    //
+    // See https://github.com/flutter/flutter/issues/112498.
+    if (color_source->isGradient()) {
+      sk_paint.setDither(paint.isDither());
+    }
+    sk_paint.setShader(ToSk(color_source));
   }
 
   sk_paint.setMaskFilter(ToSk(paint.getMaskFilterPtr()));
diff --git a/display_list/skia/dl_sk_canvas_unittests.cc b/display_list/skia/dl_sk_canvas_unittests.cc
deleted file mode 100644
index 4e665570d67c8..0000000000000
--- a/display_list/skia/dl_sk_canvas_unittests.cc
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright 2013 The Flutter Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "display_list/dl_tile_mode.h"
-#include "display_list/effects/dl_color_source.h"
-#include "flutter/display_list/dl_paint.h"
-#include "flutter/display_list/skia/dl_sk_canvas.h"
-
-#include "gtest/gtest.h"
-
-namespace flutter {
-namespace testing {
-
-TEST(DisplayListSkCanvas, ToSkDitheringDisabled) {
-  // Test that when using the utility method "ToSk", the resulting SkPaint
-  // does not have "isDither" set to true, even if it's requested by the
-  // Flutter (dart:ui) paint, because it's not a supported feature in the
-  // Impeller backend.
-
-  // Create a new DlPaint with isDither set to true.
-  //
-  // This mimics the behavior of ui.Paint.enableDithering = true.
-  DlPaint dl_paint;
-  dl_paint.setDither(true);
-
-  SkPaint sk_paint = ToSk(dl_paint);
-
-  EXPECT_FALSE(sk_paint.isDither());
-}
-
-TEST(DisplayListSkCanvas, ToSkDitheringEnabledForGradients) {
-  // Test that when using the utility method "ToSk", the resulting SkPaint
-  // has "isDither" set to true, if the paint is a gradient, because it's
-  // a supported feature in the Impeller backend.
-
-  // Create a new DlPaint with isDither set to true.
-  //
-  // This mimics the behavior of ui.Paint.enableDithering = true.
-  DlPaint dl_paint;
-  dl_paint.setDither(true);
-
-  // Set the paint to be a gradient.
-  dl_paint.setColorSource(DlColorSource::MakeLinear(SkPoint::Make(0, 0),
-                                                    SkPoint::Make(100, 100), 0,
-                                                    0, 0, DlTileMode::kClamp));
-
-  SkPaint sk_paint = ToSk(dl_paint);
-
-  EXPECT_TRUE(sk_paint.isDither());
-}
-
-}  // namespace testing
-}  // namespace flutter
diff --git a/display_list/skia/dl_sk_conversions_unittests.cc b/display_list/skia/dl_sk_conversions_unittests.cc
index dae99690c91d5..8efaee7710463 100644
--- a/display_list/skia/dl_sk_conversions_unittests.cc
+++ b/display_list/skia/dl_sk_conversions_unittests.cc
@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "flutter/display_list/skia/dl_sk_conversions.h"
-
+#include "display_list/effects/dl_color_source.h"
 #include "flutter/display_list/dl_blend_mode.h"
 #include "flutter/display_list/dl_paint.h"
 #include "flutter/display_list/dl_sampling_options.h"
 #include "flutter/display_list/dl_tile_mode.h"
 #include "flutter/display_list/dl_vertices.h"
+#include "flutter/display_list/skia/dl_sk_canvas.h"
+#include "flutter/display_list/skia/dl_sk_conversions.h"
 #include "gtest/gtest.h"
 #include "third_party/skia/include/core/SkSamplingOptions.h"
 #include "third_party/skia/include/core/SkTileMode.h"
@@ -270,5 +271,43 @@ TEST(DisplayListSkConversions, MatrixColorFilterModifiesTransparency) {
   }
 }
 
+TEST(DisplayListSkConversions, ToSkDitheringDisabledForGradients) {
+  // Test that when using the utility method "ToSk", the resulting SkPaint
+  // does not have "isDither" set to true, even if it's requested by the
+  // Flutter (dart:ui) paint, because it's not a supported feature in the
+  // Impeller backend.
+
+  // Create a new DlPaint with isDither set to true.
+  //
+  // This mimics the behavior of ui.Paint.enableDithering = true.
+  DlPaint dl_paint;
+  dl_paint.setDither(true);
+
+  SkPaint sk_paint = ToSk(dl_paint);
+
+  EXPECT_FALSE(sk_paint.isDither());
+}
+
+TEST(DisplayListSkConversions, ToSkDitheringEnabledForGradients) {
+  // Test that when using the utility method "ToSk", the resulting SkPaint
+  // has "isDither" set to true, if the paint is a gradient, because it's
+  // a supported feature in the Impeller backend.
+
+  // Create a new DlPaint with isDither set to true.
+  //
+  // This mimics the behavior of ui.Paint.enableDithering = true.
+  DlPaint dl_paint;
+  dl_paint.setDither(true);
+
+  // Set the paint to be a gradient.
+  dl_paint.setColorSource(DlColorSource::MakeLinear(SkPoint::Make(0, 0),
+                                                    SkPoint::Make(100, 100), 0,
+                                                    0, 0, DlTileMode::kClamp));
+
+  SkPaint sk_paint = ToSk(dl_paint);
+
+  EXPECT_TRUE(sk_paint.isDither());
+}
+
 }  // namespace testing
 }  // namespace flutter

From 4903aa0ba1445045c8005a35beab006db37804f7 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 16:10:30 -0700
Subject: [PATCH 09/10] Address comments.

---
 display_list/skia/dl_sk_canvas.cc                | 4 +++-
 display_list/skia/dl_sk_conversions_unittests.cc | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index 9b5d07810f4ac..2858317a9ac0b 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -56,7 +56,9 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
     //
     // See https://github.com/flutter/flutter/issues/112498.
     if (color_source->isGradient()) {
-      sk_paint.setDither(paint.isDither());
+      // Originates from `dart:ui#Paint.enableDithering`.
+      auto user_specified_dither = paint.isDither();
+      sk_paint.setDither(user_specified_dither);
     }
     sk_paint.setShader(ToSk(color_source));
   }
diff --git a/display_list/skia/dl_sk_conversions_unittests.cc b/display_list/skia/dl_sk_conversions_unittests.cc
index 8efaee7710463..611251dcd843d 100644
--- a/display_list/skia/dl_sk_conversions_unittests.cc
+++ b/display_list/skia/dl_sk_conversions_unittests.cc
@@ -2,12 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "display_list/effects/dl_color_source.h"
 #include "flutter/display_list/dl_blend_mode.h"
 #include "flutter/display_list/dl_paint.h"
 #include "flutter/display_list/dl_sampling_options.h"
 #include "flutter/display_list/dl_tile_mode.h"
 #include "flutter/display_list/dl_vertices.h"
+#include "flutter/display_list/effects/dl_color_source.h"
 #include "flutter/display_list/skia/dl_sk_canvas.h"
 #include "flutter/display_list/skia/dl_sk_conversions.h"
 #include "gtest/gtest.h"

From 0156b57ba5ef8f6121653f33a057216a3f79329e Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 15 Aug 2023 16:40:37 -0700
Subject: [PATCH 10/10] Move SkPaint ToSk to dl_sk_conversions for consistency.

---
 display_list/skia/dl_sk_canvas.cc             | 55 -------------------
 display_list/skia/dl_sk_canvas.h              |  2 -
 display_list/skia/dl_sk_conversions.cc        | 55 +++++++++++++++++++
 display_list/skia/dl_sk_conversions.h         |  2 +
 .../skia/dl_sk_conversions_unittests.cc       |  1 -
 5 files changed, 57 insertions(+), 58 deletions(-)

diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc
index 2858317a9ac0b..9299c9a6ff9d7 100644
--- a/display_list/skia/dl_sk_canvas.cc
+++ b/display_list/skia/dl_sk_canvas.cc
@@ -14,61 +14,6 @@
 
 namespace flutter {
 
-// clang-format off
-constexpr float kInvertColorMatrix[20] = {
-  -1.0,    0,    0, 1.0, 0,
-     0, -1.0,    0, 1.0, 0,
-     0,    0, -1.0, 1.0, 0,
-   1.0,  1.0,  1.0, 1.0, 0
-};
-// clang-format on
-
-SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
-  SkPaint sk_paint;
-
-  sk_paint.setAntiAlias(paint.isAntiAlias());
-  sk_paint.setColor(paint.getColor());
-  sk_paint.setBlendMode(ToSk(paint.getBlendMode()));
-  sk_paint.setStyle(force_stroke ? SkPaint::kStroke_Style
-                                 : ToSk(paint.getDrawStyle()));
-  sk_paint.setStrokeWidth(paint.getStrokeWidth());
-  sk_paint.setStrokeMiter(paint.getStrokeMiter());
-  sk_paint.setStrokeCap(ToSk(paint.getStrokeCap()));
-  sk_paint.setStrokeJoin(ToSk(paint.getStrokeJoin()));
-  sk_paint.setImageFilter(ToSk(paint.getImageFilterPtr()));
-  auto color_filter = ToSk(paint.getColorFilterPtr());
-  if (paint.isInvertColors()) {
-    auto invert_filter = SkColorFilters::Matrix(kInvertColorMatrix);
-    if (color_filter) {
-      invert_filter = invert_filter->makeComposed(color_filter);
-    }
-    color_filter = invert_filter;
-  }
-  sk_paint.setColorFilter(color_filter);
-
-  auto color_source = paint.getColorSourcePtr();
-  if (color_source) {
-    // On the Impeller backend, we will only support dithering of *gradients*,
-    // and it will be enabled by default (without the option to disable it).
-    // Until Skia support is completely removed, we only want to respect the
-    // dither flag for gradients (otherwise it will also apply to, for example,
-    // images, which is not supported in Impeller).
-    //
-    // See https://github.com/flutter/flutter/issues/112498.
-    if (color_source->isGradient()) {
-      // Originates from `dart:ui#Paint.enableDithering`.
-      auto user_specified_dither = paint.isDither();
-      sk_paint.setDither(user_specified_dither);
-    }
-    sk_paint.setShader(ToSk(color_source));
-  }
-
-  sk_paint.setMaskFilter(ToSk(paint.getMaskFilterPtr()));
-  sk_paint.setPathEffect(ToSk(paint.getPathEffectPtr()));
-
-  return sk_paint;
-}
-
 class SkOptionalPaint {
  public:
   explicit SkOptionalPaint(const DlPaint* dl_paint) {
diff --git a/display_list/skia/dl_sk_canvas.h b/display_list/skia/dl_sk_canvas.h
index b8b9d408d9d53..af774b74342f0 100644
--- a/display_list/skia/dl_sk_canvas.h
+++ b/display_list/skia/dl_sk_canvas.h
@@ -10,8 +10,6 @@
 
 namespace flutter {
 
-SkPaint ToSk(const DlPaint& paint, bool force_stroke = false);
-
 // An adapter to receive DlCanvas calls and dispatch them into
 // an SkCanvas.
 class DlSkCanvasAdapter final : public virtual DlCanvas {
diff --git a/display_list/skia/dl_sk_conversions.cc b/display_list/skia/dl_sk_conversions.cc
index 4a4264269fd1f..16f5fc710a343 100644
--- a/display_list/skia/dl_sk_conversions.cc
+++ b/display_list/skia/dl_sk_conversions.cc
@@ -10,6 +10,61 @@
 
 namespace flutter {
 
+// clang-format off
+constexpr float kInvertColorMatrix[20] = {
+  -1.0,    0,    0, 1.0, 0,
+     0, -1.0,    0, 1.0, 0,
+     0,    0, -1.0, 1.0, 0,
+   1.0,  1.0,  1.0, 1.0, 0
+};
+// clang-format on
+
+SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
+  SkPaint sk_paint;
+
+  sk_paint.setAntiAlias(paint.isAntiAlias());
+  sk_paint.setColor(paint.getColor());
+  sk_paint.setBlendMode(ToSk(paint.getBlendMode()));
+  sk_paint.setStyle(force_stroke ? SkPaint::kStroke_Style
+                                 : ToSk(paint.getDrawStyle()));
+  sk_paint.setStrokeWidth(paint.getStrokeWidth());
+  sk_paint.setStrokeMiter(paint.getStrokeMiter());
+  sk_paint.setStrokeCap(ToSk(paint.getStrokeCap()));
+  sk_paint.setStrokeJoin(ToSk(paint.getStrokeJoin()));
+  sk_paint.setImageFilter(ToSk(paint.getImageFilterPtr()));
+  auto color_filter = ToSk(paint.getColorFilterPtr());
+  if (paint.isInvertColors()) {
+    auto invert_filter = SkColorFilters::Matrix(kInvertColorMatrix);
+    if (color_filter) {
+      invert_filter = invert_filter->makeComposed(color_filter);
+    }
+    color_filter = invert_filter;
+  }
+  sk_paint.setColorFilter(color_filter);
+
+  auto color_source = paint.getColorSourcePtr();
+  if (color_source) {
+    // On the Impeller backend, we will only support dithering of *gradients*,
+    // and it will be enabled by default (without the option to disable it).
+    // Until Skia support is completely removed, we only want to respect the
+    // dither flag for gradients (otherwise it will also apply to, for example,
+    // images, which is not supported in Impeller).
+    //
+    // See https://github.com/flutter/flutter/issues/112498.
+    if (color_source->isGradient()) {
+      // Originates from `dart:ui#Paint.enableDithering`.
+      auto user_specified_dither = paint.isDither();
+      sk_paint.setDither(user_specified_dither);
+    }
+    sk_paint.setShader(ToSk(color_source));
+  }
+
+  sk_paint.setMaskFilter(ToSk(paint.getMaskFilterPtr()));
+  sk_paint.setPathEffect(ToSk(paint.getPathEffectPtr()));
+
+  return sk_paint;
+}
+
 sk_sp<SkShader> ToSk(const DlColorSource* source) {
   if (!source) {
     return nullptr;
diff --git a/display_list/skia/dl_sk_conversions.h b/display_list/skia/dl_sk_conversions.h
index 2291b4afc4ddf..a1a9f6ce838b8 100644
--- a/display_list/skia/dl_sk_conversions.h
+++ b/display_list/skia/dl_sk_conversions.h
@@ -10,6 +10,8 @@
 
 namespace flutter {
 
+SkPaint ToSk(const DlPaint& paint, bool force_stroke = false);
+
 inline SkBlendMode ToSk(DlBlendMode mode) {
   return static_cast<SkBlendMode>(mode);
 }
diff --git a/display_list/skia/dl_sk_conversions_unittests.cc b/display_list/skia/dl_sk_conversions_unittests.cc
index 611251dcd843d..1bae18a3ca1dd 100644
--- a/display_list/skia/dl_sk_conversions_unittests.cc
+++ b/display_list/skia/dl_sk_conversions_unittests.cc
@@ -8,7 +8,6 @@
 #include "flutter/display_list/dl_tile_mode.h"
 #include "flutter/display_list/dl_vertices.h"
 #include "flutter/display_list/effects/dl_color_source.h"
-#include "flutter/display_list/skia/dl_sk_canvas.h"
 #include "flutter/display_list/skia/dl_sk_conversions.h"
 #include "gtest/gtest.h"
 #include "third_party/skia/include/core/SkSamplingOptions.h"