Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 8 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
1 change: 0 additions & 1 deletion display_list/benchmarking/dl_complexity_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ class ComplexityCalculatorHelper

virtual ~ComplexityCalculatorHelper() = default;

void setDither(bool dither) override {}
void setInvertColors(bool invert) override {}
void setStrokeCap(DlStrokeCap cap) override {}
void setStrokeJoin(DlStrokeJoin join) override {}
Expand Down
1 change: 0 additions & 1 deletion display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ namespace flutter {

#define FOR_EACH_DISPLAY_LIST_OP(V) \
V(SetAntiAlias) \
V(SetDither) \
V(SetInvertColors) \
\
V(SetStrokeCap) \
Expand Down
8 changes: 2 additions & 6 deletions display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ class DisplayListTestBase : public BaseT {
DlPaint defaults;

EXPECT_EQ(builder_paint.isAntiAlias(), defaults.isAntiAlias());
EXPECT_EQ(builder_paint.isDither(), defaults.isDither());
EXPECT_EQ(builder_paint.isInvertColors(), defaults.isInvertColors());
EXPECT_EQ(builder_paint.getColor(), defaults.getColor());
EXPECT_EQ(builder_paint.getBlendMode(), defaults.getBlendMode());
Expand Down Expand Up @@ -387,7 +386,6 @@ TEST_F(DisplayListTest, BuildRestoresAttributes) {
builder.Build();
check_defaults(builder, cull_rect);

receiver.setDither(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two lines following that should also be removed. They have nothing to check if the receiver wasn't modified.

builder.Build();
check_defaults(builder, cull_rect);

Expand Down Expand Up @@ -1060,11 +1058,9 @@ TEST_F(DisplayListTest, SingleOpsMightSupportGroupOpacityBlendMode) {
};

#define RUN_TESTS(body) \
run_tests( \
#body, [](DlOpReceiver& receiver) { body }, true, false)
run_tests(#body, [](DlOpReceiver& receiver) { body }, true, false)
#define RUN_TESTS2(body, expect) \
run_tests( \
#body, [](DlOpReceiver& receiver) { body }, expect, expect)
run_tests(#body, [](DlOpReceiver& receiver) { body }, expect, expect)

RUN_TESTS(receiver.drawPaint(););
RUN_TESTS2(receiver.drawColor(DlColor(SK_ColorRED), DlBlendMode::kSrcOver);
Expand Down
7 changes: 0 additions & 7 deletions display_list/dl_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,6 @@ void DisplayListBuilder::onSetAntiAlias(bool aa) {
current_.setAntiAlias(aa);
Push<SetAntiAliasOp>(0, 0, aa);
}
void DisplayListBuilder::onSetDither(bool dither) {
current_.setDither(dither);
Push<SetDitherOp>(0, 0, dither);
}
void DisplayListBuilder::onSetInvertColors(bool invert) {
current_.setInvertColors(invert);
Push<SetInvertColorsOp>(0, 0, invert);
Expand Down Expand Up @@ -347,9 +343,6 @@ void DisplayListBuilder::SetAttributesFromPaint(
if (flags.applies_anti_alias()) {
setAntiAlias(paint.isAntiAlias());
}
if (flags.applies_dither()) {
setDither(paint.isDither());
}
if (flags.applies_alpha_or_color()) {
setColor(paint.getColor());
}
Expand Down
7 changes: 0 additions & 7 deletions display_list/dl_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,6 @@ class DisplayListBuilder final : public virtual DlCanvas,
}
}
// |DlOpReceiver|
void setDither(bool dither) override {
if (current_.isDither() != dither) {
onSetDither(dither);
}
}
// |DlOpReceiver|
void setInvertColors(bool invert) override {
if (current_.isInvertColors() != invert) {
onSetInvertColors(invert);
Expand Down Expand Up @@ -673,7 +667,6 @@ class DisplayListBuilder final : public virtual DlCanvas,
}

void onSetAntiAlias(bool aa);
void onSetDither(bool dither);
void onSetInvertColors(bool invert);
void onSetStrokeCap(DlStrokeCap cap);
void onSetStrokeJoin(DlStrokeJoin join);
Expand Down
1 change: 0 additions & 1 deletion display_list/dl_op_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ class DlOpReceiver {
// another method that changes the same attribute. The current set of
// attributes is not affected by |save| and |restore|.
virtual void setAntiAlias(bool aa) = 0;
virtual void setDither(bool dither) = 0;
virtual void setDrawStyle(DlDrawStyle style) = 0;
virtual void setColor(DlColor color) = 0;
virtual void setStrokeWidth(float width) = 0;
Expand Down
1 change: 0 additions & 1 deletion display_list/dl_op_records.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ struct DLOp {
} \
};
DEFINE_SET_BOOL_OP(AntiAlias)
DEFINE_SET_BOOL_OP(Dither)
DEFINE_SET_BOOL_OP(InvertColors)
#undef DEFINE_SET_BOOL_OP

Expand Down
6 changes: 0 additions & 6 deletions display_list/dl_paint.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,6 @@ class DlPaint {
return *this;
}

bool isDither() const { return is_dither_; }
DlPaint& setDither(bool isDither) {
is_dither_ = isDither;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The field can be deleted as well.

return *this;
}

bool isInvertColors() const { return is_invert_colors_; }
DlPaint& setInvertColors(bool isInvertColors) {
is_invert_colors_ = isInvertColors;
Expand Down
4 changes: 0 additions & 4 deletions display_list/dl_paint_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ namespace testing {
TEST(DisplayListPaint, ConstructorDefaults) {
DlPaint paint;
EXPECT_FALSE(paint.isAntiAlias());
EXPECT_FALSE(paint.isDither());
EXPECT_FALSE(paint.isInvertColors());
EXPECT_EQ(paint.getColor(), DlPaint::kDefaultColor);
EXPECT_EQ(paint.getAlpha(), 0xFF);
Expand Down Expand Up @@ -45,7 +44,6 @@ TEST(DisplayListPaint, ConstructorDefaults) {
EXPECT_EQ(paint, DlPaint(DlColor(0xFF000000)));

EXPECT_NE(paint, DlPaint().setAntiAlias(true));
EXPECT_NE(paint, DlPaint().setDither(true));
EXPECT_NE(paint, DlPaint().setInvertColors(true));
EXPECT_NE(paint, DlPaint().setColor(DlColor::kGreen()));
EXPECT_NE(paint, DlPaint(DlColor::kGreen()));
Expand Down Expand Up @@ -110,7 +108,6 @@ TEST(DisplayListPaint, ChainingConstructor) {
DlPaint paint =
DlPaint() //
.setAntiAlias(true) //
.setDither(true) //
.setInvertColors(true) //
.setColor(DlColor::kGreen()) //
.setAlpha(0x7F) //
Expand All @@ -129,7 +126,6 @@ TEST(DisplayListPaint, ChainingConstructor) {
.setMaskFilter(DlBlurMaskFilter(DlBlurStyle::kInner, 3.14).shared())
.setPathEffect(path_effect);
EXPECT_TRUE(paint.isAntiAlias());
EXPECT_TRUE(paint.isDither());
EXPECT_TRUE(paint.isInvertColors());
EXPECT_EQ(paint.getColor(), DlColor::kGreen().withAlpha(0x7F));
EXPECT_EQ(paint.getAlpha(), 0x7F);
Expand Down
8 changes: 3 additions & 5 deletions display_list/skia/dl_sk_conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,10 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
// 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.setDither(color_source->isGradient());
sk_paint.setShader(ToSk(color_source));
} else {
sk_paint.setDither(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sk_paint is a brand new object allocated above with defaults (dither is false by default).

}

sk_paint.setMaskFilter(ToSk(paint.getMaskFilterPtr()));
Expand Down
22 changes: 0 additions & 22 deletions display_list/skia/dl_sk_conversions_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -286,41 +286,19 @@ 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());
}

Expand Down
3 changes: 0 additions & 3 deletions display_list/skia/dl_sk_paint_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ void DlSkPaintDispatchHelper::restore_opacity() {
void DlSkPaintDispatchHelper::setAntiAlias(bool aa) {
paint_.setAntiAlias(aa);
}
void DlSkPaintDispatchHelper::setDither(bool dither) {
dither_ = dither;
}
void DlSkPaintDispatchHelper::setInvertColors(bool invert) {
invert_colors_ = invert;
paint_.setColorFilter(makeColorFilter());
Expand Down
10 changes: 4 additions & 6 deletions display_list/skia/dl_sk_paint_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
}

void setAntiAlias(bool aa) override;
void setDither(bool dither) override;
void setDrawStyle(DlDrawStyle style) override;
void setColor(DlColor color) override;
void setStrokeWidth(SkScalar width) override;
Expand All @@ -41,12 +40,12 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
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).
// Until Skia support is completely removed, we only want to dither
// 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_);
paint_.setDither(color_source_gradient_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be handled in setColorSource directly now without having to pass a flag to this method. (color_source_gradient_ field is obsolete compared to just setting the dither flag on the paint_ object)

return paint_;
}

Expand All @@ -69,7 +68,6 @@ 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
44 changes: 2 additions & 42 deletions display_list/skia/dl_sk_paint_dispatcher_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,60 +45,20 @@ TEST(DisplayListUtils, OverRestore) {
}

// https://github.com/flutter/flutter/issues/132860.
TEST(DisplayListUtils, SetDitherIgnoredIfColorSourceNotGradient) {
TEST(DisplayListUtils, SetColorSourceDithersIfGradient) {
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) {
TEST(DisplayListUtils, SetColorSourceDoesNotDitherIfNotGradient) {
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());
}

// 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
Expand Down
18 changes: 1 addition & 17 deletions display_list/testing/dl_rendering_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -872,10 +872,6 @@ class TestParameters {
return false;
}
}
if (flags_.applies_dither() && //
ref_attr.isDither() != attr.isDither()) {
return false;
}
if (flags_.applies_color() && //
ref_attr.getColor() != attr.getColor()) {
return false;
Expand Down Expand Up @@ -1822,19 +1818,7 @@ class CanvasCompareTester {
},
[=](const DlSetupContext& ctx) {
dl_dither_setup(ctx);
ctx.paint.setDither(true);
})
.with_bg(dither_bg));
RenderWith(testP, dither_env, tolerance,
CaseParameters(
"Dither = False",
[=](const SkSetupContext& ctx) {
sk_dither_setup(ctx);
ctx.paint.setDither(false);
},
[=](const DlSetupContext& ctx) {
dl_dither_setup(ctx);
ctx.paint.setDither(false);
// Dithering is implicitly enabled for gradients.
})
.with_bg(dither_bg));
}
Expand Down
5 changes: 0 additions & 5 deletions display_list/testing/dl_test_snippets.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ std::vector<DisplayListInvocationGroup> CreateAllAttributesOps() {
{0, 8, 0, 0, [](DlOpReceiver& r) { r.setAntiAlias(true); }},
{0, 0, 0, 0, [](DlOpReceiver& r) { r.setAntiAlias(false); }},
}},
{"SetDither",
{
{0, 8, 0, 0, [](DlOpReceiver& r) { r.setDither(true); }},
{0, 0, 0, 0, [](DlOpReceiver& r) { r.setDither(false); }},
}},
{"SetInvertColors",
{
{0, 8, 0, 0, [](DlOpReceiver& r) { r.setInvertColors(true); }},
Expand Down
1 change: 0 additions & 1 deletion display_list/utils/dl_receiver_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ namespace flutter {
class IgnoreAttributeDispatchHelper : public virtual DlOpReceiver {
public:
void setAntiAlias(bool aa) override {}
void setDither(bool dither) override {}
void setInvertColors(bool invert) override {}
void setStrokeCap(DlStrokeCap cap) override {}
void setStrokeJoin(DlStrokeJoin join) override {}
Expand Down
5 changes: 0 additions & 5 deletions impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,6 @@ void DlDispatcher::setAntiAlias(bool aa) {
// Nothing to do because AA is implicit.
}

// |flutter::DlOpReceiver|
void DlDispatcher::setDither(bool dither) {
paint_.dither = dither;
}

static Paint::Style ToStyle(flutter::DlDrawStyle style) {
switch (style) {
case flutter::DlDrawStyle::kFill:
Expand Down
3 changes: 0 additions & 3 deletions impeller/display_list/dl_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ class DlDispatcher final : public flutter::DlOpReceiver {
// |flutter::DlOpReceiver|
void setAntiAlias(bool aa) override;

// |flutter::DlOpReceiver|
void setDither(bool dither) override;

// |flutter::DlOpReceiver|
void setDrawStyle(flutter::DlDrawStyle style) override;

Expand Down
Loading