Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further filter/clear <SkPaint>.setDither(true), this time in DlSkPaintDispatchHelper #44912

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion display_list/skia/dl_sk_paint_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -73,6 +73,7 @@ void DlSkPaintDispatchHelper::setBlendMode(DlBlendMode mode) {
paint_.setBlendMode(ToSk(mode));
}
void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* source) {
color_source_gradient_ = source && source->isGradient();
paint_.setShader(ToSk(source));
}
void DlSkPaintDispatchHelper::setImageFilter(const DlImageFilter* filter) {
Expand Down
14 changes: 13 additions & 1 deletion display_list/skia/dl_sk_paint_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,17 @@ 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.
paint_.setDither(color_source_gradient_ && dither_);
return paint_;
}

/// Returns the current opacity attribute which is used to reduce
/// the alpha of all setColor calls encountered in the streeam
Expand All @@ -57,6 +67,8 @@ 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_;

Expand Down
69 changes: 69 additions & 0 deletions display_list/skia/dl_sk_paint_dispatcher_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -21,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;
Expand All @@ -31,5 +43,62 @@ 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());
}

// https://github.com/flutter/flutter/issues/132860.
TEST(DisplayListUtils, SetDitherTrueThenSetColorSourceDithersIfGradient) {
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(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());
}
matanlurey marked this conversation as resolved.
Show resolved Hide resolved

// 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