From 06ee02b10b4a4c5f0f2b76dd3771c346419e4967 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Mon, 21 Aug 2023 10:45:56 -0700
Subject: [PATCH 1/8] Ignore and/or clear setDither if setColorSource isn't a
 gradient.

---
 display_list/skia/dl_sk_paint_dispatcher.cc   | 23 +++++++++++++++++++
 display_list/skia/dl_sk_paint_dispatcher.h    |  1 +
 .../skia/dl_sk_paint_dispatcher_unittests.cc  | 15 ++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.cc b/display_list/skia/dl_sk_paint_dispatcher.cc
index 80970cc9ee14b..418ebb878de4a 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher.cc
@@ -41,6 +41,17 @@ void DlSkPaintDispatchHelper::setAntiAlias(bool aa) {
   paint_.setAntiAlias(aa);
 }
 void DlSkPaintDispatchHelper::setDither(bool dither) {
+  // 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 (dither && !color_source_gradient_) {
+    return;
+  }
+
   paint_.setDither(dither);
 }
 void DlSkPaintDispatchHelper::setInvertColors(bool invert) {
@@ -73,6 +84,18 @@ void DlSkPaintDispatchHelper::setBlendMode(DlBlendMode mode) {
   paint_.setBlendMode(ToSk(mode));
 }
 void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* 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.
+  color_source_gradient_ = source->isGradient();
+  if (!color_source_gradient_) {
+    paint_.setDither(false);
+  }
+
   paint_.setShader(ToSk(source));
 }
 void DlSkPaintDispatchHelper::setImageFilter(const DlImageFilter* filter) {
diff --git a/display_list/skia/dl_sk_paint_dispatcher.h b/display_list/skia/dl_sk_paint_dispatcher.h
index 090d7ad9f0d8c..ca32172d88a95 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.h
+++ b/display_list/skia/dl_sk_paint_dispatcher.h
@@ -57,6 +57,7 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
 
  private:
   SkPaint paint_;
+  bool color_source_gradient_ = false;
   bool invert_colors_ = false;
   sk_sp<SkColorFilter> sk_color_filter_;
 
diff --git a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
index 402891e0ddd22..bf487f3b73def 100644
--- a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
@@ -31,5 +31,20 @@ TEST(DisplayListUtils, OverRestore) {
   helper.restore();
 }
 
+// https://github.com/flutter/flutter/issues/132860.
+TEST(DisplayListUtils, SetDitherIgnoredIfColorSourceNotGradient) {
+  MockDispatchHelper helper;
+  helper.setDither(true);
+  EXPECT_FALSE(helper.paint().isDither());
+}
+
+// https://github.com/flutter/flutter/issues/132860.
+TEST(DisplayListUtils, SetColorSourceClearsDitherIfNotGradient) {
+  MockDispatchHelper helper;
+  helper.setDither(true);
+  helper.setColorSource(nullptr);
+  EXPECT_FALSE(helper.paint().isDither());
+}
+
 }  // namespace testing
 }  // namespace flutter

From f7cc0137b83b50245ba7c40aae83edb8da0a6e66 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Mon, 21 Aug 2023 11:52:29 -0700
Subject: [PATCH 2/8] Avoid NPE.

---
 display_list/skia/dl_sk_paint_dispatcher.cc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.cc b/display_list/skia/dl_sk_paint_dispatcher.cc
index 418ebb878de4a..3881f789886e5 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher.cc
@@ -84,6 +84,10 @@ void DlSkPaintDispatchHelper::setBlendMode(DlBlendMode mode) {
   paint_.setBlendMode(ToSk(mode));
 }
 void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* source) {
+  if (!source) {
+    return;
+  }
+
   // 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

From 5f7cf607910d4de67e4395bd3bcb8be3056c9ce1 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Mon, 21 Aug 2023 12:06:47 -0700
Subject: [PATCH 3/8] Re-apply NPE tweak.

---
 display_list/skia/dl_sk_paint_dispatcher.cc | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.cc b/display_list/skia/dl_sk_paint_dispatcher.cc
index 3881f789886e5..404803473dd12 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher.cc
@@ -84,10 +84,6 @@ void DlSkPaintDispatchHelper::setBlendMode(DlBlendMode mode) {
   paint_.setBlendMode(ToSk(mode));
 }
 void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* source) {
-  if (!source) {
-    return;
-  }
-
   // 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
@@ -95,7 +91,7 @@ void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* source) {
   // images, which is not supported in Impeller).
   //
   // See https://github.com/flutter/flutter/issues/112498.
-  color_source_gradient_ = source->isGradient();
+  color_source_gradient_ = source && source->isGradient();
   if (!color_source_gradient_) {
     paint_.setDither(false);
   }

From 0784e316142d1e1255417454eed655cfad3cff06 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Mon, 21 Aug 2023 14:19:30 -0700
Subject: [PATCH 4/8] Simplify where dither checks occur.

---
 display_list/skia/dl_sk_paint_dispatcher.cc | 15 ---------------
 display_list/skia/dl_sk_paint_dispatcher.h  | 14 +++++++++++++-
 2 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.cc b/display_list/skia/dl_sk_paint_dispatcher.cc
index 404803473dd12..49b4255be6c38 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher.cc
@@ -41,17 +41,6 @@ void DlSkPaintDispatchHelper::setAntiAlias(bool aa) {
   paint_.setAntiAlias(aa);
 }
 void DlSkPaintDispatchHelper::setDither(bool dither) {
-  // 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 (dither && !color_source_gradient_) {
-    return;
-  }
-
   paint_.setDither(dither);
 }
 void DlSkPaintDispatchHelper::setInvertColors(bool invert) {
@@ -92,10 +81,6 @@ void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* source) {
   //
   // See https://github.com/flutter/flutter/issues/112498.
   color_source_gradient_ = source && source->isGradient();
-  if (!color_source_gradient_) {
-    paint_.setDither(false);
-  }
-
   paint_.setShader(ToSk(source));
 }
 void DlSkPaintDispatchHelper::setImageFilter(const DlImageFilter* filter) {
diff --git a/display_list/skia/dl_sk_paint_dispatcher.h b/display_list/skia/dl_sk_paint_dispatcher.h
index ca32172d88a95..c154338f8c403 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.h
+++ b/display_list/skia/dl_sk_paint_dispatcher.h
@@ -37,7 +37,19 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
   void setMaskFilter(const DlMaskFilter* filter) override;
   void setImageFilter(const DlImageFilter* filter) override;
 
-  const SkPaint& paint() { return paint_; }
+  const SkPaint& paint() {
+    // 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_gradient_) {
+      paint_.setDither(false);
+    }
+    return paint_;
+  }
 
   /// Returns the current opacity attribute which is used to reduce
   /// the alpha of all setColor calls encountered in the streeam

From 57ad22d87f21d9e966146f893b1602860915b091 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Mon, 21 Aug 2023 16:51:13 -0700
Subject: [PATCH 5/8] Add a regression test.

---
 .../skia/dl_sk_paint_dispatcher_unittests.cc  | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
index bf487f3b73def..be8693c238368 100644
--- a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
@@ -2,6 +2,7 @@
 // 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/skia/dl_sk_paint_dispatcher.h"
 
 #include "flutter/display_list/utils/dl_receiver_utils.h"
@@ -46,5 +47,23 @@ TEST(DisplayListUtils, SetColorSourceClearsDitherIfNotGradient) {
   EXPECT_FALSE(helper.paint().isDither());
 }
 
+// https://github.com/flutter/flutter/issues/132860.
+TEST(DisplayListUtils, SetDitherTrueThenSetColorSourceDithersIfGradient) {
+  // Create a simple linear gradient.
+  const DlColor colors[2] = {0xFF000000, 0xFFFFFFFF};
+  const float stops[2] = {0.0f, 1.0f};
+  const auto linear_gradient = DlColorSource::MakeLinear(
+      SkPoint::Make(0.0f, 0.0f), SkPoint::Make(100.0f, 100.0f), 2, colors,
+      stops, DlTileMode::kClamp, nullptr);
+
+  MockDispatchHelper helper;
+
+  // A naive implementation would ignore the dither flag here since the current
+  // color source is not a gradient.
+  helper.setDither(true);
+  helper.setColorSource(linear_gradient.get());
+  EXPECT_TRUE(helper.paint().isDither());
+}
+
 }  // namespace testing
 }  // namespace flutter

From f1ed380160af22223736684e1c93e715a9ce9a87 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Mon, 21 Aug 2023 18:32:59 -0700
Subject: [PATCH 6/8] Remove redundant comment.

---
 display_list/skia/dl_sk_paint_dispatcher.cc | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.cc b/display_list/skia/dl_sk_paint_dispatcher.cc
index 49b4255be6c38..4847a69319f80 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher.cc
@@ -73,13 +73,6 @@ void DlSkPaintDispatchHelper::setBlendMode(DlBlendMode mode) {
   paint_.setBlendMode(ToSk(mode));
 }
 void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* 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.
   color_source_gradient_ = source && source->isGradient();
   paint_.setShader(ToSk(source));
 }

From b61b7257c6dd39a16f9af55b62613298c43a9dcb Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 22 Aug 2023 09:20:42 -0700
Subject: [PATCH 7/8] Implement Jim's suggestions, thanks

---
 display_list/skia/dl_sk_paint_dispatcher.cc   |  2 +-
 display_list/skia/dl_sk_paint_dispatcher.h    |  5 +--
 .../skia/dl_sk_paint_dispatcher_unittests.cc  | 33 ++++++++++++++-----
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.cc b/display_list/skia/dl_sk_paint_dispatcher.cc
index 4847a69319f80..992b5146a6c13 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher.cc
@@ -41,7 +41,7 @@ void DlSkPaintDispatchHelper::setAntiAlias(bool aa) {
   paint_.setAntiAlias(aa);
 }
 void DlSkPaintDispatchHelper::setDither(bool dither) {
-  paint_.setDither(dither);
+  dither_ = dither;
 }
 void DlSkPaintDispatchHelper::setInvertColors(bool invert) {
   invert_colors_ = invert;
diff --git a/display_list/skia/dl_sk_paint_dispatcher.h b/display_list/skia/dl_sk_paint_dispatcher.h
index c154338f8c403..9182cd02cb7b2 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.h
+++ b/display_list/skia/dl_sk_paint_dispatcher.h
@@ -45,8 +45,8 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
     // example, images, which is not supported in Impeller).
     //
     // See https://github.com/flutter/flutter/issues/112498.
-    if (!color_source_gradient_) {
-      paint_.setDither(false);
+    if (color_source_gradient_ && dither_) {
+      paint_.setDither(true);
     }
     return paint_;
   }
@@ -70,6 +70,7 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
  private:
   SkPaint paint_;
   bool color_source_gradient_ = false;
+  bool dither_ = false;
   bool invert_colors_ = false;
   sk_sp<SkColorFilter> sk_color_filter_;
 
diff --git a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
index be8693c238368..33b4969c525f5 100644
--- a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
@@ -22,6 +22,17 @@ class MockDispatchHelper final : public virtual DlOpReceiver,
   void restore() override { DlSkPaintDispatchHelper::restore_opacity(); }
 };
 
+static const DlColor kTestColors[2] = {0xFF000000, 0xFFFFFFFF};
+static const float kTestStops[2] = {0.0f, 1.0f};
+static const auto kTestLinearGradient =
+    DlColorSource::MakeLinear(SkPoint::Make(0.0f, 0.0f),
+                              SkPoint::Make(100.0f, 100.0f),
+                              2,
+                              kTestColors,
+                              kTestStops,
+                              DlTileMode::kClamp,
+                              nullptr);
+
 // Regression test for https://github.com/flutter/flutter/issues/100176.
 TEST(DisplayListUtils, OverRestore) {
   MockDispatchHelper helper;
@@ -49,19 +60,25 @@ TEST(DisplayListUtils, SetColorSourceClearsDitherIfNotGradient) {
 
 // https://github.com/flutter/flutter/issues/132860.
 TEST(DisplayListUtils, SetDitherTrueThenSetColorSourceDithersIfGradient) {
-  // Create a simple linear gradient.
-  const DlColor colors[2] = {0xFF000000, 0xFFFFFFFF};
-  const float stops[2] = {0.0f, 1.0f};
-  const auto linear_gradient = DlColorSource::MakeLinear(
-      SkPoint::Make(0.0f, 0.0f), SkPoint::Make(100.0f, 100.0f), 2, colors,
-      stops, DlTileMode::kClamp, nullptr);
-
   MockDispatchHelper helper;
 
   // A naive implementation would ignore the dither flag here since the current
   // color source is not a gradient.
   helper.setDither(true);
-  helper.setColorSource(linear_gradient.get());
+  helper.setColorSource(kTestLinearGradient.get());
+  EXPECT_TRUE(helper.paint().isDither());
+}
+
+// https://github.com/flutter/flutter/issues/132860.
+TEST(DisplayListUtils, SetDitherTrueThenRenderNonGradientThenRenderGradient) {
+  MockDispatchHelper helper;
+
+  // "Render" a dithered non-gradient.
+  helper.setDither(true);
+  EXPECT_FALSE(helper.paint().isDither());
+
+  // "Render" a gradient.
+  helper.setColorSource(kTestLinearGradient.get());
   EXPECT_TRUE(helper.paint().isDither());
 }
 

From ff34b5e010d9151ecfe8d7dd3ec792ce53a03e70 Mon Sep 17 00:00:00 2001
From: Matan Lurey <matanlurey@users.noreply.github.com>
Date: Tue, 22 Aug 2023 14:21:20 -0700
Subject: [PATCH 8/8] Fix branch issue with persistent SkPaint.

---
 display_list/skia/dl_sk_paint_dispatcher.h     |  4 +---
 .../skia/dl_sk_paint_dispatcher_unittests.cc   | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/display_list/skia/dl_sk_paint_dispatcher.h b/display_list/skia/dl_sk_paint_dispatcher.h
index 9182cd02cb7b2..33c59ff29bb14 100644
--- a/display_list/skia/dl_sk_paint_dispatcher.h
+++ b/display_list/skia/dl_sk_paint_dispatcher.h
@@ -45,9 +45,7 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
     // example, images, which is not supported in Impeller).
     //
     // See https://github.com/flutter/flutter/issues/112498.
-    if (color_source_gradient_ && dither_) {
-      paint_.setDither(true);
-    }
+    paint_.setDither(color_source_gradient_ && dither_);
     return paint_;
   }
 
diff --git a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
index 33b4969c525f5..514a3a9d10018 100644
--- a/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
+++ b/display_list/skia/dl_sk_paint_dispatcher_unittests.cc
@@ -82,5 +82,23 @@ TEST(DisplayListUtils, SetDitherTrueThenRenderNonGradientThenRenderGradient) {
   EXPECT_TRUE(helper.paint().isDither());
 }
 
+// https://github.com/flutter/flutter/issues/132860.
+TEST(DisplayListUtils, SetDitherTrueThenGradientTHenNonGradientThenGradient) {
+  MockDispatchHelper helper;
+
+  // "Render" a dithered gradient.
+  helper.setDither(true);
+  helper.setColorSource(kTestLinearGradient.get());
+  EXPECT_TRUE(helper.paint().isDither());
+
+  // "Render" a non-gradient.
+  helper.setColorSource(nullptr);
+  EXPECT_FALSE(helper.paint().isDither());
+
+  // "Render" a gradient again.
+  helper.setColorSource(kTestLinearGradient.get());
+  EXPECT_TRUE(helper.paint().isDither());
+}
+
 }  // namespace testing
 }  // namespace flutter