diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index 394fe092af0fe..11a141da9c905 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -119,6 +119,16 @@ class DlColorSource : public DlAttribute { /// 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 { diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc index 842f4593035ba..9299c9a6ff9d7 100644 --- a/display_list/skia/dl_sk_canvas.cc +++ b/display_list/skia/dl_sk_canvas.cc @@ -14,47 +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 - -static SkPaint ToSk(const DlPaint& paint, bool force_stroke = false) { - 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 - : 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.setShader(ToSk(paint.getColorSourcePtr())); - 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); - 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_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 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(mode); } diff --git a/display_list/skia/dl_sk_conversions_unittests.cc b/display_list/skia/dl_sk_conversions_unittests.cc index dae99690c91d5..1bae18a3ca1dd 100644 --- a/display_list/skia/dl_sk_conversions_unittests.cc +++ b/display_list/skia/dl_sk_conversions_unittests.cc @@ -2,13 +2,13 @@ // 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 "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_conversions.h" #include "gtest/gtest.h" #include "third_party/skia/include/core/SkSamplingOptions.h" #include "third_party/skia/include/core/SkTileMode.h" @@ -270,5 +270,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 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 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(