Skip to content

Commit 9c31f64

Browse files
flarflar
authored
[DisplayList] Delete (publicly) unused DlColorColorSource (flutter#56825)
While recently updating the DlColorSource sources I noticed some questionably implementation choices in the Color variant of the color sources. I then realized that there was no public use of these classes (other than mostly their own unit tests) and so they should be deleted to focus on implementing the variants that are actually used by Flutter.
1 parent 91f250b commit 9c31f64

23 files changed

+89
-236
lines changed

‎ci/licenses_golden/licenses_flutter‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42540,8 +42540,6 @@ ORIGIN: ../../../flutter/display_list/effects/color_filters/dl_matrix_color_filt
4254042540
ORIGIN: ../../../flutter/display_list/effects/color_filters/dl_matrix_color_filter.h + ../../../flutter/LICENSE
4254142541
ORIGIN: ../../../flutter/display_list/effects/color_filters/dl_srgb_to_linear_gamma_color_filter.cc + ../../../flutter/LICENSE
4254242542
ORIGIN: ../../../flutter/display_list/effects/color_filters/dl_srgb_to_linear_gamma_color_filter.h + ../../../flutter/LICENSE
42543-
ORIGIN: ../../../flutter/display_list/effects/color_sources/dl_color_color_source.cc + ../../../flutter/LICENSE
42544-
ORIGIN: ../../../flutter/display_list/effects/color_sources/dl_color_color_source.h + ../../../flutter/LICENSE
4254542543
ORIGIN: ../../../flutter/display_list/effects/color_sources/dl_conical_gradient_color_source.cc + ../../../flutter/LICENSE
4254642544
ORIGIN: ../../../flutter/display_list/effects/color_sources/dl_conical_gradient_color_source.h + ../../../flutter/LICENSE
4254742545
ORIGIN: ../../../flutter/display_list/effects/color_sources/dl_gradient_color_source_base.h + ../../../flutter/LICENSE
@@ -45467,8 +45465,6 @@ FILE: ../../../flutter/display_list/effects/color_filters/dl_matrix_color_filter
4546745465
FILE: ../../../flutter/display_list/effects/color_filters/dl_matrix_color_filter.h
4546845466
FILE: ../../../flutter/display_list/effects/color_filters/dl_srgb_to_linear_gamma_color_filter.cc
4546945467
FILE: ../../../flutter/display_list/effects/color_filters/dl_srgb_to_linear_gamma_color_filter.h
45470-
FILE: ../../../flutter/display_list/effects/color_sources/dl_color_color_source.cc
45471-
FILE: ../../../flutter/display_list/effects/color_sources/dl_color_color_source.h
4547245468
FILE: ../../../flutter/display_list/effects/color_sources/dl_conical_gradient_color_source.cc
4547345469
FILE: ../../../flutter/display_list/effects/color_sources/dl_conical_gradient_color_source.h
4547445470
FILE: ../../../flutter/display_list/effects/color_sources/dl_gradient_color_source_base.h

‎display_list/BUILD.gn‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ source_set("display_list") {
5555
"effects/color_filters/dl_matrix_color_filter.h",
5656
"effects/color_filters/dl_srgb_to_linear_gamma_color_filter.cc",
5757
"effects/color_filters/dl_srgb_to_linear_gamma_color_filter.h",
58-
"effects/color_sources/dl_color_color_source.cc",
59-
"effects/color_sources/dl_color_color_source.h",
6058
"effects/color_sources/dl_conical_gradient_color_source.cc",
6159
"effects/color_sources/dl_conical_gradient_color_source.h",
6260
"effects/color_sources/dl_image_color_source.cc",

‎display_list/dl_builder.cc‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,6 @@ void DisplayListBuilder::onSetColorSource(const DlColorSource* source) {
201201
current_.setColorSource(source->shared());
202202
is_ui_thread_safe_ = is_ui_thread_safe_ && source->isUIThreadSafe();
203203
switch (source->type()) {
204-
case DlColorSourceType::kColor: {
205-
const DlColorColorSource* color_source = source->asColor();
206-
current_.setColorSource(nullptr);
207-
setColor(color_source->color());
208-
break;
209-
}
210204
case DlColorSourceType::kImage: {
211205
const DlImageColorSource* image_source = source->asImage();
212206
FML_DCHECK(image_source);
@@ -1953,11 +1947,9 @@ DlColor DisplayListBuilder::GetEffectiveColor(const DlPaint& paint,
19531947
if (flags.applies_color()) {
19541948
const DlColorSource* source = paint.getColorSourcePtr();
19551949
if (source) {
1956-
if (source->asColor()) {
1957-
color = source->asColor()->color();
1958-
} else {
1959-
color = source->is_opaque() ? DlColor::kBlack() : kAnyColor;
1960-
}
1950+
// Suspecting that we need to modulate the ColorSource color by the
1951+
// color property, see https://github.com/flutter/flutter/issues/159507
1952+
color = source->is_opaque() ? DlColor::kBlack() : kAnyColor;
19611953
} else {
19621954
color = paint.getColor();
19631955
}

‎display_list/dl_paint_unittests.cc‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,16 @@ TEST(DisplayListPaint, ConstructorDefaults) {
5656
EXPECT_NE(paint, DlPaint().setStrokeWidth(6));
5757
EXPECT_NE(paint, DlPaint().setStrokeMiter(7));
5858

59-
auto color_source = DlColorSource::MakeColor(DlColor::kMagenta());
59+
DlColor colors[2] = {
60+
DlColor::kGreen(),
61+
DlColor::kBlue(),
62+
};
63+
float stops[2] = {
64+
0.0f,
65+
1.0f,
66+
};
67+
auto color_source = DlColorSource::MakeLinear({0, 0}, {10, 10}, 2, colors,
68+
stops, DlTileMode::kClamp);
6069
EXPECT_NE(paint, DlPaint().setColorSource(color_source));
6170

6271
auto color_filter =
@@ -95,19 +104,28 @@ TEST(DisplayListPaint, NullSharedPointerSetGet) {
95104
}
96105

97106
TEST(DisplayListPaint, ChainingConstructor) {
107+
DlColor colors[2] = {
108+
DlColor::kGreen(),
109+
DlColor::kBlue(),
110+
};
111+
float stops[2] = {
112+
0.0f,
113+
1.0f,
114+
};
98115
DlPaint paint =
99-
DlPaint() //
100-
.setAntiAlias(true) //
101-
.setInvertColors(true) //
102-
.setColor(DlColor::kGreen()) //
103-
.setAlpha(0x7F) //
104-
.setBlendMode(DlBlendMode::kLuminosity) //
105-
.setDrawStyle(DlDrawStyle::kStrokeAndFill) //
106-
.setStrokeCap(DlStrokeCap::kSquare) //
107-
.setStrokeJoin(DlStrokeJoin::kBevel) //
108-
.setStrokeWidth(42) //
109-
.setStrokeMiter(1.5) //
110-
.setColorSource(DlColorSource::MakeColor(DlColor::kMagenta())) //
116+
DlPaint() //
117+
.setAntiAlias(true) //
118+
.setInvertColors(true) //
119+
.setColor(DlColor::kGreen()) //
120+
.setAlpha(0x7F) //
121+
.setBlendMode(DlBlendMode::kLuminosity) //
122+
.setDrawStyle(DlDrawStyle::kStrokeAndFill) //
123+
.setStrokeCap(DlStrokeCap::kSquare) //
124+
.setStrokeJoin(DlStrokeJoin::kBevel) //
125+
.setStrokeWidth(42) //
126+
.setStrokeMiter(1.5) //
127+
.setColorSource(DlColorSource::MakeLinear( //
128+
{0, 0}, {10, 10}, 2, colors, stops, DlTileMode::kClamp)) //
111129
.setColorFilter(
112130
DlColorFilter::MakeBlend(DlColor::kYellow(), DlBlendMode::kDstIn))
113131
.setImageFilter(DlImageFilter::MakeBlur(1.3, 4.7, DlTileMode::kClamp))
@@ -123,7 +141,8 @@ TEST(DisplayListPaint, ChainingConstructor) {
123141
EXPECT_EQ(paint.getStrokeWidth(), 42);
124142
EXPECT_EQ(paint.getStrokeMiter(), 1.5);
125143
EXPECT_TRUE(Equals(paint.getColorSource(),
126-
DlColorSource::MakeColor(DlColor::kMagenta())));
144+
DlColorSource::MakeLinear({0, 0}, {10, 10}, 2, colors,
145+
stops, DlTileMode::kClamp)));
127146
EXPECT_TRUE(Equals(
128147
paint.getColorFilter(),
129148
DlColorFilter::MakeBlend(DlColor::kYellow(), DlBlendMode::kDstIn)));

‎display_list/effects/color_sources/dl_color_color_source.cc‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

‎display_list/effects/color_sources/dl_color_color_source.h‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎display_list/effects/dl_color_source.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@ static void DlGradientDeleter(void* p) {
2020
::operator delete(p);
2121
}
2222

23-
std::shared_ptr<DlColorSource> DlColorSource::MakeColor(DlColor color) {
24-
return std::make_shared<DlColorColorSource>(color);
25-
}
26-
2723
std::shared_ptr<DlColorSource> DlColorSource::MakeImage(
2824
const sk_sp<const DlImage>& image,
2925
DlTileMode horizontal_tile_mode,

‎display_list/effects/dl_color_source.h‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
namespace flutter {
1717

18-
class DlColorColorSource;
1918
class DlImageColorSource;
2019
class DlLinearGradientColorSource;
2120
class DlRadialGradientColorSource;
@@ -34,7 +33,6 @@ class DlRuntimeEffectColorSource;
3433
// attributes, and the final blend with the pixels in the destination.
3534

3635
enum class DlColorSourceType {
37-
kColor,
3836
kImage,
3937
kLinearGradient,
4038
kRadialGradient,
@@ -45,8 +43,6 @@ enum class DlColorSourceType {
4543

4644
class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
4745
public:
48-
static std::shared_ptr<DlColorSource> MakeColor(DlColor color);
49-
5046
static std::shared_ptr<DlColorSource> MakeImage(
5147
const sk_sp<const DlImage>& image,
5248
DlTileMode horizontal_tile_mode,
@@ -120,10 +116,6 @@ class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
120116
///
121117
virtual bool isGradient() const { return false; }
122118

123-
// Return a DlColorColorSource pointer to this object iff it is an Color
124-
// type of ColorSource, otherwise return nullptr.
125-
virtual const DlColorColorSource* asColor() const { return nullptr; }
126-
127119
// Return a DlImageColorSource pointer to this object iff it is an Image
128120
// type of ColorSource, otherwise return nullptr.
129121
virtual const DlImageColorSource* asImage() const { return nullptr; }

‎display_list/effects/dl_color_source_unittests.cc‎

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -88,53 +88,6 @@ static constexpr DlPoint kTestPoints2[2] = {
8888
DlPoint(107, 118),
8989
};
9090

91-
TEST(DisplayListColorSource, ColorConstructor) {
92-
DlColorColorSource source(DlColor::kRed());
93-
}
94-
95-
TEST(DisplayListColorSource, ColorShared) {
96-
DlColorColorSource source(DlColor::kRed());
97-
ASSERT_NE(source.shared().get(), &source);
98-
ASSERT_EQ(*source.shared(), source);
99-
}
100-
101-
TEST(DisplayListColorSource, ColorAsColor) {
102-
DlColorColorSource source(DlColor::kRed());
103-
ASSERT_NE(source.asColor(), nullptr);
104-
ASSERT_EQ(source.asColor(), &source);
105-
106-
ASSERT_EQ(source.asImage(), nullptr);
107-
ASSERT_EQ(source.asLinearGradient(), nullptr);
108-
ASSERT_EQ(source.asRadialGradient(), nullptr);
109-
ASSERT_EQ(source.asConicalGradient(), nullptr);
110-
ASSERT_EQ(source.asSweepGradient(), nullptr);
111-
ASSERT_EQ(source.asRuntimeEffect(), nullptr);
112-
}
113-
114-
TEST(DisplayListColorSource, ColorContents) {
115-
DlColorColorSource source(DlColor::kRed());
116-
ASSERT_EQ(source.color(), DlColor::kRed());
117-
ASSERT_EQ(source.is_opaque(), true);
118-
for (int i = 0; i < 255; i++) {
119-
SkColor alpha_color = SkColorSetA(SK_ColorRED, i);
120-
auto const alpha_source = DlColorColorSource(DlColor(alpha_color));
121-
ASSERT_EQ(alpha_source.color(), alpha_color);
122-
ASSERT_EQ(alpha_source.is_opaque(), false);
123-
}
124-
}
125-
126-
TEST(DisplayListColorSource, ColorEquals) {
127-
DlColorColorSource source1(DlColor::kRed());
128-
DlColorColorSource source2(DlColor::kRed());
129-
TestEquals(source1, source2);
130-
}
131-
132-
TEST(DisplayListColorSource, ColorNotEquals) {
133-
DlColorColorSource source1(DlColor::kRed());
134-
DlColorColorSource source2(DlColor::kBlue());
135-
TestNotEquals(source1, source2, "Color differs");
136-
}
137-
13891
TEST(DisplayListColorSource, ImageConstructor) {
13992
DlImageColorSource source(kTestImage1, DlTileMode::kClamp, DlTileMode::kClamp,
14093
DlImageSampling::kLinear, &kTestMatrix1);
@@ -153,11 +106,11 @@ TEST(DisplayListColorSource, ImageAsImage) {
153106
ASSERT_NE(source.asImage(), nullptr);
154107
ASSERT_EQ(source.asImage(), &source);
155108

156-
ASSERT_EQ(source.asColor(), nullptr);
157109
ASSERT_EQ(source.asLinearGradient(), nullptr);
158110
ASSERT_EQ(source.asRadialGradient(), nullptr);
159111
ASSERT_EQ(source.asConicalGradient(), nullptr);
160112
ASSERT_EQ(source.asSweepGradient(), nullptr);
113+
ASSERT_EQ(source.asRuntimeEffect(), nullptr);
161114
}
162115

163116
TEST(DisplayListColorSource, ImageContents) {
@@ -251,7 +204,6 @@ TEST(DisplayListColorSource, LinearGradientAsLinear) {
251204
ASSERT_NE(source->asLinearGradient(), nullptr);
252205
ASSERT_EQ(source->asLinearGradient(), source.get());
253206

254-
ASSERT_EQ(source->asColor(), nullptr);
255207
ASSERT_EQ(source->asImage(), nullptr);
256208
ASSERT_EQ(source->asRadialGradient(), nullptr);
257209
ASSERT_EQ(source->asConicalGradient(), nullptr);
@@ -370,7 +322,6 @@ TEST(DisplayListColorSource, RadialGradientAsRadial) {
370322
ASSERT_NE(source->asRadialGradient(), nullptr);
371323
ASSERT_EQ(source->asRadialGradient(), source.get());
372324

373-
ASSERT_EQ(source->asColor(), nullptr);
374325
ASSERT_EQ(source->asImage(), nullptr);
375326
ASSERT_EQ(source->asLinearGradient(), nullptr);
376327
ASSERT_EQ(source->asConicalGradient(), nullptr);
@@ -489,7 +440,6 @@ TEST(DisplayListColorSource, ConicalGradientAsConical) {
489440
ASSERT_NE(source->asConicalGradient(), nullptr);
490441
ASSERT_EQ(source->asConicalGradient(), source.get());
491442

492-
ASSERT_EQ(source->asColor(), nullptr);
493443
ASSERT_EQ(source->asImage(), nullptr);
494444
ASSERT_EQ(source->asLinearGradient(), nullptr);
495445
ASSERT_EQ(source->asRadialGradient(), nullptr);
@@ -624,7 +574,6 @@ TEST(DisplayListColorSource, SweepGradientAsSweep) {
624574
ASSERT_NE(source->asSweepGradient(), nullptr);
625575
ASSERT_EQ(source->asSweepGradient(), source.get());
626576

627-
ASSERT_EQ(source->asColor(), nullptr);
628577
ASSERT_EQ(source->asImage(), nullptr);
629578
ASSERT_EQ(source->asLinearGradient(), nullptr);
630579
ASSERT_EQ(source->asRadialGradient(), nullptr);
@@ -743,7 +692,6 @@ TEST(DisplayListColorSource, RuntimeEffect) {
743692
ASSERT_NE(source2->asRuntimeEffect(), source1.get());
744693

745694
ASSERT_EQ(source1->asImage(), nullptr);
746-
ASSERT_EQ(source1->asColor(), nullptr);
747695
ASSERT_EQ(source1->asLinearGradient(), nullptr);
748696
ASSERT_EQ(source1->asRadialGradient(), nullptr);
749697
ASSERT_EQ(source1->asConicalGradient(), nullptr);

‎display_list/effects/dl_color_sources.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
#ifndef FLUTTER_DISPLAY_LIST_EFFECTS_DL_COLOR_SOURCES_H_
66
#define FLUTTER_DISPLAY_LIST_EFFECTS_DL_COLOR_SOURCES_H_
77

8-
#include "flutter/display_list/effects/color_sources/dl_color_color_source.h"
98
#include "flutter/display_list/effects/color_sources/dl_conical_gradient_color_source.h"
109
#include "flutter/display_list/effects/color_sources/dl_image_color_source.h"
1110
#include "flutter/display_list/effects/color_sources/dl_linear_gradient_color_source.h"

0 commit comments

Comments
 (0)