Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 7a2bdf8

Browse files
matanlureymatanlurey
committed
Address comments.
1 parent 8fc1180 commit 7a2bdf8

File tree

8 files changed

+18
-80
lines changed

8 files changed

+18
-80
lines changed

‎display_list/display_list_unittests.cc‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,9 +386,6 @@ TEST_F(DisplayListTest, BuildRestoresAttributes) {
386386
builder.Build();
387387
check_defaults(builder, cull_rect);
388388

389-
builder.Build();
390-
check_defaults(builder, cull_rect);
391-
392389
receiver.setInvertColors(true);
393390
builder.Build();
394391
check_defaults(builder, cull_rect);

‎display_list/dl_op_flags.h‎

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,14 @@ class DisplayListFlags {
8181

8282
// clang-format off
8383
static constexpr int kUsesAntiAlias = 1 << 10;
84-
static constexpr int kUsesDither = 1 << 11;
85-
static constexpr int kUsesAlpha = 1 << 12;
86-
static constexpr int kUsesColor = 1 << 13;
87-
static constexpr int kUsesBlend = 1 << 14;
88-
static constexpr int kUsesShader = 1 << 15;
89-
static constexpr int kUsesColorFilter = 1 << 16;
90-
static constexpr int kUsesPathEffect = 1 << 17;
91-
static constexpr int kUsesMaskFilter = 1 << 18;
92-
static constexpr int kUsesImageFilter = 1 << 19;
84+
static constexpr int kUsesAlpha = 1 << 11;
85+
static constexpr int kUsesColor = 1 << 12;
86+
static constexpr int kUsesBlend = 1 << 13;
87+
static constexpr int kUsesShader = 1 << 14;
88+
static constexpr int kUsesColorFilter = 1 << 15;
89+
static constexpr int kUsesPathEffect = 1 << 16;
90+
static constexpr int kUsesMaskFilter = 1 << 17;
91+
static constexpr int kUsesImageFilter = 1 << 18;
9392

9493
// Some ops have an optional paint argument. If the version
9594
// stored in the DisplayList ignores the paint, but there
@@ -102,9 +101,8 @@ class DisplayListFlags {
102101
// clang-format on
103102

104103
static constexpr int kAnyAttributeMask = //
105-
kUsesAntiAlias | kUsesDither | kUsesAlpha | kUsesColor | kUsesBlend |
106-
kUsesShader | kUsesColorFilter | kUsesPathEffect | kUsesMaskFilter |
107-
kUsesImageFilter;
104+
kUsesAntiAlias | kUsesAlpha | kUsesColor | kUsesBlend | kUsesShader |
105+
kUsesColorFilter | kUsesPathEffect | kUsesMaskFilter | kUsesImageFilter;
108106
};
109107

110108
class DisplayListFlagsBase : protected DisplayListFlags {
@@ -173,7 +171,6 @@ class DisplayListAttributeFlags : DisplayListFlagsBase {
173171
constexpr bool ignores_paint() const { return has_any(kIgnoresPaint); }
174172

175173
constexpr bool applies_anti_alias() const { return has_any(kUsesAntiAlias); }
176-
constexpr bool applies_dither() const { return has_any(kUsesDither); }
177174
constexpr bool applies_color() const { return has_any(kUsesColor); }
178175
constexpr bool applies_alpha() const { return has_any(kUsesAlpha); }
179176
constexpr bool applies_alpha_or_color() const {
@@ -257,8 +254,7 @@ class DisplayListAttributeFlags : DisplayListFlagsBase {
257254
class DisplayListOpFlags : DisplayListFlags {
258255
private:
259256
// Flags common to all primitives that apply colors
260-
static constexpr int kBasePaintFlags = (kUsesDither | //
261-
kUsesColor | //
257+
static constexpr int kBasePaintFlags = (kUsesColor | //
262258
kUsesAlpha | //
263259
kUsesBlend | //
264260
kUsesShader | //
@@ -280,7 +276,6 @@ class DisplayListOpFlags : DisplayListFlags {
280276
// Flags common to primitives that render an image with paint attributes
281277
static constexpr int kBaseImageFlags = (kIsNonGeometric | //
282278
kUsesAlpha | //
283-
kUsesDither | //
284279
kUsesBlend | //
285280
kUsesColorFilter | //
286281
kUsesImageFilter);
@@ -376,7 +371,6 @@ class DisplayListOpFlags : DisplayListFlags {
376371
};
377372
static constexpr DisplayListAttributeFlags kDrawVerticesFlags{
378373
kIsNonGeometric | //
379-
kUsesDither | //
380374
kUsesAlpha | //
381375
kUsesShader | //
382376
kUsesBlend | //

‎display_list/dl_paint.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ DlPaint::DlPaint(DlColor color)
1212
stroke_cap_(static_cast<unsigned>(DlStrokeCap::kDefaultCap)),
1313
stroke_join_(static_cast<unsigned>(DlStrokeJoin::kDefaultJoin)),
1414
is_anti_alias_(false),
15-
is_dither_(false),
1615
is_invert_colors_(false),
1716
color_(color),
1817
stroke_width_(kDefaultWidth),
@@ -24,7 +23,6 @@ bool DlPaint::operator==(DlPaint const& other) const {
2423
stroke_cap_ == other.stroke_cap_ && //
2524
stroke_join_ == other.stroke_join_ && //
2625
is_anti_alias_ == other.is_anti_alias_ && //
27-
is_dither_ == other.is_dither_ && //
2826
is_invert_colors_ == other.is_invert_colors_ && //
2927
color_ == other.color_ && //
3028
stroke_width_ == other.stroke_width_ && //

‎display_list/dl_paint.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ class DlPaint {
216216
unsigned stroke_cap_ : kStrokeCapBits;
217217
unsigned stroke_join_ : kStrokeJoinBits;
218218
unsigned is_anti_alias_ : 1;
219-
unsigned is_dither_ : 1;
220219
unsigned is_invert_colors_ : 1;
221220
};
222221
};

‎display_list/skia/dl_sk_conversions.cc‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ SkPaint ToSk(const DlPaint& paint, bool force_stroke) {
5353
// See https://github.com/flutter/flutter/issues/112498.
5454
sk_paint.setDither(color_source->isGradient());
5555
sk_paint.setShader(ToSk(color_source));
56-
} else {
57-
sk_paint.setDither(false);
5856
}
5957

6058
sk_paint.setMaskFilter(ToSk(paint.getMaskFilterPtr()));

‎display_list/skia/dl_sk_paint_dispatcher.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,13 @@ void DlSkPaintDispatchHelper::setBlendMode(DlBlendMode mode) {
7070
paint_.setBlendMode(ToSk(mode));
7171
}
7272
void DlSkPaintDispatchHelper::setColorSource(const DlColorSource* source) {
73-
color_source_gradient_ = source && source->isGradient();
7473
paint_.setShader(ToSk(source));
74+
75+
// On the Impeller backend, we only support dithering of *gradients*, and so
76+
// by default, so this will force Skia to match that behavior (i.e. turning
77+
// dithering on for gradient paints, and off for everything else).
78+
auto const is_gradient = source && source->isGradient();
79+
paint_.setDither(is_gradient);
7580
}
7681
void DlSkPaintDispatchHelper::setImageFilter(const DlImageFilter* filter) {
7782
paint_.setImageFilter(ToSk(filter));

‎display_list/skia/dl_sk_paint_dispatcher.h‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,7 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
3737
void setMaskFilter(const DlMaskFilter* filter) override;
3838
void setImageFilter(const DlImageFilter* filter) override;
3939

40-
const SkPaint& paint() {
41-
// On the Impeller backend, we will only support dithering of *gradients*,
42-
// and it will be enabled by default (without the option to disable it).
43-
// Until Skia support is completely removed, we only want to dither
44-
// gradients (otherwise it will also apply to, for example, images, which is
45-
// not supported in Impeller).
46-
//
47-
// See https://github.com/flutter/flutter/issues/112498.
48-
paint_.setDither(color_source_gradient_);
49-
return paint_;
50-
}
40+
const SkPaint& paint() { return paint_; }
5141

5242
/// Returns the current opacity attribute which is used to reduce
5343
/// the alpha of all setColor calls encountered in the streeam
@@ -67,7 +57,6 @@ class DlSkPaintDispatchHelper : public virtual DlOpReceiver {
6757

6858
private:
6959
SkPaint paint_;
70-
bool color_source_gradient_ = false;
7160
bool invert_colors_ = false;
7261
sk_sp<SkColorFilter> sk_color_filter_;
7362

‎display_list/testing/dl_rendering_unittests.cc‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,48 +1781,6 @@ class CanvasCompareTester {
17811781
ctx.paint.setColorSource(dl_gradient);
17821782
}));
17831783
}
1784-
1785-
if (testP.uses_gradient()) {
1786-
// Dithering is only applied to gradients so we reuse the gradient
1787-
// created above in these setup methods. Also, thin stroked
1788-
// primitives (mainly drawLine and drawPoints) do not show much
1789-
// dithering so we use a non-trivial stroke width as well.
1790-
RenderEnvironment dither_env =
1791-
RenderEnvironment::Make565(env.provider());
1792-
if (!dither_env.valid()) {
1793-
// Currently only happens on Metal backend
1794-
static OncePerBackendWarning warnings("Skipping Dithering tests");
1795-
warnings.warn(dither_env.backend_name());
1796-
} else {
1797-
DlColor dither_bg = DlColor::kBlack();
1798-
SkSetup sk_dither_setup = [=](const SkSetupContext& ctx) {
1799-
ctx.paint.setShader(sk_gradient);
1800-
ctx.paint.setAlpha(0xf0);
1801-
ctx.paint.setStrokeWidth(5.0);
1802-
};
1803-
DlSetup dl_dither_setup = [=](const DlSetupContext& ctx) {
1804-
ctx.paint.setColorSource(dl_gradient);
1805-
ctx.paint.setAlpha(0xf0);
1806-
ctx.paint.setStrokeWidth(5.0);
1807-
};
1808-
dither_env.init_ref(sk_dither_setup, testP.sk_renderer(),
1809-
dl_dither_setup, testP.dl_renderer(),
1810-
testP.imp_renderer(), dither_bg);
1811-
quickCompareToReference(dither_env, "dither");
1812-
RenderWith(testP, dither_env, tolerance,
1813-
CaseParameters(
1814-
"Dither == True",
1815-
[=](const SkSetupContext& ctx) {
1816-
sk_dither_setup(ctx);
1817-
ctx.paint.setDither(true);
1818-
},
1819-
[=](const DlSetupContext& ctx) {
1820-
dl_dither_setup(ctx);
1821-
// Dithering is implicitly enabled for gradients.
1822-
})
1823-
.with_bg(dither_bg));
1824-
}
1825-
}
18261784
}
18271785
}
18281786

0 commit comments

Comments
 (0)