[Skia] Only respect ui.Paint.dither when the colorSource is a gradient

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

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",

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_->isGradient() : false;
+  }
+
   bool isInvertColors() const { return isInvertColors_; }
   DlPaint& setInvertColors(bool isInvertColors) {
     isInvertColors_ = isInvertColors;

display_list/effects/dl_color_source.h

@@ -119,6 +119,16 @@ class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
   ///
   virtual bool isUIThreadSafe() const = 0;
 
+  //----------------------------------------------------------------------------
+  /// @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 isGradient() 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 +297,8 @@ class DlGradientColorSourceBase : public DlMatrixColorSourceBase {
     return true;
   }
 
+  bool isGradient() const override { return true; }
+
   DlTileMode tile_mode() const { return mode_; }
   int stop_count() const { return stop_count_; }
   const DlColor* colors() const {

display_list/skia/dl_sk_canvas.cc

@@ -23,12 +23,10 @@ constexpr float kInvertColorMatrix[20] = {
 };
 // clang-format on
 
-static SkPaint ToSk(const DlPaint& paint, bool force_stroke = false) {
+SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
   SkPaint sk_paint;
 
   sk_paint.setAntiAlias(paint.isAntiAlias());
-  sk_paint.setDither(paint.isDither());
-
   sk_paint.setColor(paint.getColor());
   sk_paint.setBlendMode(ToSk(paint.getBlendMode()));
   sk_paint.setStyle(force_stroke ? SkPaint::kStroke_Style
@@ -49,6 +47,19 @@ static SkPaint ToSk(const DlPaint& paint, bool force_stroke = false) {
     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()));
 

display_list/skia/dl_sk_canvas.h

@@ -10,6 +10,8 @@
 
 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 {

display_list/skia/dl_sk_canvas_unittests.cc

@@ -0,0 +1,54 @@
+// 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

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(