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

Commit d1d08d8

Browse files
flarflar
authored
Do not create incomplete DlColorSource objects from Skia gradients (#33131) (#33462)
1 parent d1b9a69 commit d1d08d8

File tree

2 files changed

+43
-136
lines changed

2 files changed

+43
-136
lines changed

‎display_list/display_list_color_source.cc

Lines changed: 5 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66

77
namespace flutter {
88

9-
static constexpr int kGradientStaticRecaptureCount = 24;
10-
119
std::shared_ptr<DlColorSource> DlColorSource::From(SkShader* sk_shader) {
1210
if (sk_shader == nullptr) {
1311
return nullptr;
@@ -27,61 +25,11 @@ std::shared_ptr<DlColorSource> DlColorSource::From(SkShader* sk_shader) {
2725
// of parameters which are missing, including the local matrix in every
2826
// gradient, and the sweep angles in the sweep gradients.
2927
//
30-
// Since the matrix is a rarely used property and since most sweep
31-
// gradients swing full circle, we will simply assume an Identity matrix
32-
// and 0,360 for the Sweep gradient.
33-
// Possibly the most likely "missing attribute" that might be different
34-
// would be the sweep gradients which might be a full circle, but might
35-
// have their starting angle in a custom direction.
36-
SkColor colors[kGradientStaticRecaptureCount];
37-
SkScalar stops[kGradientStaticRecaptureCount];
38-
SkShader::GradientInfo info = {};
39-
info.fColorCount = kGradientStaticRecaptureCount;
40-
info.fColors = colors;
41-
info.fColorOffsets = stops;
42-
SkShader::GradientType type = sk_shader->asAGradient(&info);
43-
if (type != SkShader::kNone_GradientType &&
44-
info.fColorCount > kGradientStaticRecaptureCount) {
45-
int count = info.fColorCount;
46-
info.fColors = new SkColor[count];
47-
info.fColorOffsets = new SkScalar[count];
48-
sk_shader->asAGradient(&info);
49-
FML_DCHECK(count == info.fColorCount);
50-
}
51-
DlTileMode mode = ToDl(info.fTileMode);
52-
std::shared_ptr<DlColorSource> source;
53-
switch (type) {
54-
case SkShader::kNone_GradientType:
55-
source = std::make_shared<DlUnknownColorSource>(sk_ref_sp(sk_shader));
56-
break;
57-
case SkShader::kColor_GradientType:
58-
source = std::make_shared<DlColorColorSource>(info.fColors[0]);
59-
break;
60-
case SkShader::kLinear_GradientType:
61-
source = MakeLinear(info.fPoint[0], info.fPoint[1], info.fColorCount,
62-
info.fColors, info.fColorOffsets, mode);
63-
break;
64-
case SkShader::kRadial_GradientType:
65-
source = MakeRadial(info.fPoint[0], info.fRadius[0], info.fColorCount,
66-
info.fColors, info.fColorOffsets, mode);
67-
break;
68-
case SkShader::kConical_GradientType:
69-
source = MakeConical(info.fPoint[0], info.fRadius[0], info.fPoint[1],
70-
info.fRadius[1], info.fColorCount, info.fColors,
71-
info.fColorOffsets, mode);
72-
break;
73-
case SkShader::kSweep_GradientType:
74-
source = MakeSweep(info.fPoint[0], 0, 360, info.fColorCount, info.fColors,
75-
info.fColorOffsets, mode);
76-
break;
77-
}
78-
if (info.fColors != colors) {
79-
delete info.fColors;
80-
}
81-
if (info.fColorOffsets != stops) {
82-
delete info.fColorOffsets;
83-
}
84-
return source;
28+
// Since we can't reproduce every Gradient, and customers rely on using
29+
// gradients with matrices in text code, we have to just use an Unknown
30+
// ColorSource to express all gradients.
31+
// (see: https://github.com/flutter/flutter/issues/102947)
32+
return std::make_shared<DlUnknownColorSource>(sk_ref_sp(sk_shader));
8533
}
8634

8735
static void DlGradientDeleter(void* p) {

‎display_list/display_list_color_source_unittests.cc

Lines changed: 38 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,20 @@ TEST(DisplayListColorSource, FromSkiaNullShader) {
9494
}
9595

9696
TEST(DisplayListColorSource, FromSkiaColorShader) {
97+
// We cannot read back the matrix parameter from a Skia LinearGradient
98+
// so we conservatively use an UnknownColorSource wrapper so as to not
99+
// lose any data. Note that the Skia Color shader end is read back from
100+
// the Skia asAGradient() method so while this type of color source
101+
// does not really need the matrix, we represent all of the gradient
102+
// sources using an unknown source.
103+
// Note that this shader should never really happen in practice as it
104+
// represents a degenerate gradient that collapsed to a single color.
97105
sk_sp<SkShader> shader = SkShaders::Color(SK_ColorBLUE);
98106
std::shared_ptr<DlColorSource> source = DlColorSource::From(shader);
99-
DlColorColorSource dl_source(SK_ColorBLUE);
100-
ASSERT_EQ(source->type(), DlColorSourceType::kColor);
101-
ASSERT_EQ(*source->asColor(), dl_source);
102-
ASSERT_EQ(source->asColor()->color(), SK_ColorBLUE);
107+
ASSERT_EQ(source->type(), DlColorSourceType::kUnknown);
108+
ASSERT_EQ(source->skia_object(), shader);
103109

110+
ASSERT_EQ(source->asColor(), nullptr);
104111
ASSERT_EQ(source->asImage(), nullptr);
105112
ASSERT_EQ(source->asLinearGradient(), nullptr);
106113
ASSERT_EQ(source->asRadialGradient(), nullptr);
@@ -131,126 +138,78 @@ TEST(DisplayListColorSource, FromSkiaImageShader) {
131138
}
132139

133140
TEST(DisplayListColorSource, FromSkiaLinearGradient) {
134-
// We can read back all of the parameters of a Linear gradient
135-
// except for matrix.
141+
// We cannot read back the matrix parameter from a Skia LinearGradient
142+
// so we conservatively use an UnknownColorSource wrapper so as to not
143+
// lose any data.
136144
sk_sp<SkShader> shader = SkGradientShader::MakeLinear(
137145
TestPoints, TestColors, TestStops, kTestStopCount, SkTileMode::kClamp);
138146
std::shared_ptr<DlColorSource> source = DlColorSource::From(shader);
139-
std::shared_ptr<DlColorSource> dl_source =
140-
DlColorSource::MakeLinear(TestPoints[0], TestPoints[1], kTestStopCount,
141-
TestColors, TestStops, DlTileMode::kClamp);
142-
ASSERT_EQ(source->type(), DlColorSourceType::kLinearGradient);
143-
EXPECT_TRUE(*source->asLinearGradient() == *dl_source->asLinearGradient());
144-
ASSERT_EQ(*source->asLinearGradient(), *dl_source->asLinearGradient());
145-
ASSERT_EQ(source->asLinearGradient()->start_point(), TestPoints[0]);
146-
ASSERT_EQ(source->asLinearGradient()->end_point(), TestPoints[1]);
147-
ASSERT_EQ(source->asLinearGradient()->stop_count(), kTestStopCount);
148-
for (int i = 0; i < kTestStopCount; i++) {
149-
ASSERT_EQ(source->asLinearGradient()->colors()[i], TestColors[i]);
150-
ASSERT_EQ(source->asLinearGradient()->stops()[i], TestStops[i]);
151-
}
152-
ASSERT_EQ(source->asLinearGradient()->tile_mode(), DlTileMode::kClamp);
153-
ASSERT_EQ(source->asLinearGradient()->matrix(), SkMatrix::I());
147+
ASSERT_EQ(source->type(), DlColorSourceType::kUnknown);
148+
ASSERT_EQ(source->skia_object(), shader);
154149

155150
ASSERT_EQ(source->asColor(), nullptr);
156151
ASSERT_EQ(source->asImage(), nullptr);
152+
ASSERT_EQ(source->asLinearGradient(), nullptr);
157153
ASSERT_EQ(source->asRadialGradient(), nullptr);
158154
ASSERT_EQ(source->asConicalGradient(), nullptr);
159155
ASSERT_EQ(source->asSweepGradient(), nullptr);
160156
}
161157

162158
TEST(DisplayListColorSource, FromSkiaRadialGradient) {
163-
// We can read back all of the parameters of a Radial gradient
164-
// except for matrix.
159+
// We cannot read back the matrix parameter from a Skia RadialGradient
160+
// so we conservatively use an UnknownColorSource wrapper so as to not
161+
// lose any data.
165162
sk_sp<SkShader> shader =
166163
SkGradientShader::MakeRadial(TestPoints[0], 10.0, TestColors, TestStops,
167164
kTestStopCount, SkTileMode::kClamp);
168165
std::shared_ptr<DlColorSource> source = DlColorSource::From(shader);
169-
std::shared_ptr<DlColorSource> dl_source =
170-
DlColorSource::MakeRadial(TestPoints[0], 10.0, kTestStopCount, TestColors,
171-
TestStops, DlTileMode::kClamp);
172-
ASSERT_EQ(source->type(), DlColorSourceType::kRadialGradient);
173-
EXPECT_TRUE(*source->asRadialGradient() == *dl_source->asRadialGradient());
174-
ASSERT_EQ(*source->asRadialGradient(), *dl_source->asRadialGradient());
175-
ASSERT_EQ(source->asRadialGradient()->center(), TestPoints[0]);
176-
ASSERT_EQ(source->asRadialGradient()->radius(), 10.0);
177-
ASSERT_EQ(source->asRadialGradient()->stop_count(), kTestStopCount);
178-
for (int i = 0; i < kTestStopCount; i++) {
179-
ASSERT_EQ(source->asRadialGradient()->colors()[i], TestColors[i]);
180-
ASSERT_EQ(source->asRadialGradient()->stops()[i], TestStops[i]);
181-
}
182-
ASSERT_EQ(source->asRadialGradient()->tile_mode(), DlTileMode::kClamp);
183-
ASSERT_EQ(source->asRadialGradient()->matrix(), SkMatrix::I());
166+
ASSERT_EQ(source->type(), DlColorSourceType::kUnknown);
167+
ASSERT_EQ(source->skia_object(), shader);
184168

185169
ASSERT_EQ(source->asColor(), nullptr);
186170
ASSERT_EQ(source->asImage(), nullptr);
187171
ASSERT_EQ(source->asLinearGradient(), nullptr);
172+
ASSERT_EQ(source->asRadialGradient(), nullptr);
188173
ASSERT_EQ(source->asConicalGradient(), nullptr);
189174
ASSERT_EQ(source->asSweepGradient(), nullptr);
190175
}
191176

192177
TEST(DisplayListColorSource, FromSkiaConicalGradient) {
193-
// We can read back all of the parameters of a Conical gradient
194-
// except for matrix.
178+
// We cannot read back the matrix parameter from a Skia ConicalGradient
179+
// so we conservatively use an UnknownColorSource wrapper so as to not
180+
// lose any data.
195181
sk_sp<SkShader> shader = SkGradientShader::MakeTwoPointConical(
196182
TestPoints[0], 10.0, TestPoints[1], 20.0, TestColors, TestStops,
197183
kTestStopCount, SkTileMode::kClamp);
198184
std::shared_ptr<DlColorSource> source = DlColorSource::From(shader);
199-
std::shared_ptr<DlColorSource> dl_source = DlColorSource::MakeConical(
200-
TestPoints[0], 10.0, TestPoints[1], 20.0, kTestStopCount, TestColors,
201-
TestStops, DlTileMode::kClamp);
202-
ASSERT_EQ(source->type(), DlColorSourceType::kConicalGradient);
203-
EXPECT_TRUE(*source->asConicalGradient() == *dl_source->asConicalGradient());
204-
ASSERT_EQ(*source->asConicalGradient(), *dl_source->asConicalGradient());
205-
ASSERT_EQ(source->asConicalGradient()->start_center(), TestPoints[0]);
206-
ASSERT_EQ(source->asConicalGradient()->start_radius(), 10.0);
207-
ASSERT_EQ(source->asConicalGradient()->end_center(), TestPoints[1]);
208-
ASSERT_EQ(source->asConicalGradient()->end_radius(), 20.0);
209-
ASSERT_EQ(source->asConicalGradient()->stop_count(), kTestStopCount);
210-
for (int i = 0; i < kTestStopCount; i++) {
211-
ASSERT_EQ(source->asConicalGradient()->colors()[i], TestColors[i]);
212-
ASSERT_EQ(source->asConicalGradient()->stops()[i], TestStops[i]);
213-
}
214-
ASSERT_EQ(source->asConicalGradient()->tile_mode(), DlTileMode::kClamp);
215-
ASSERT_EQ(source->asConicalGradient()->matrix(), SkMatrix::I());
185+
ASSERT_EQ(source->type(), DlColorSourceType::kUnknown);
186+
ASSERT_EQ(source->skia_object(), shader);
216187

217188
ASSERT_EQ(source->asColor(), nullptr);
218189
ASSERT_EQ(source->asImage(), nullptr);
219190
ASSERT_EQ(source->asLinearGradient(), nullptr);
220191
ASSERT_EQ(source->asRadialGradient(), nullptr);
192+
ASSERT_EQ(source->asConicalGradient(), nullptr);
221193
ASSERT_EQ(source->asSweepGradient(), nullptr);
222194
}
223195

224196
TEST(DisplayListColorSource, FromSkiaSweepGradient) {
225-
// We can read back all of the parameters of a Sweep gradient
226-
// except for matrix and the start/stop angles.
227-
sk_sp<SkShader> shader =
228-
SkGradientShader::MakeSweep(TestPoints[0].fX, TestPoints[0].fY,
229-
TestColors, TestStops, kTestStopCount);
197+
// We cannot read back the matrix parameter, nor the sweep parameters from a
198+
// Skia SweepGradient so we conservatively use an UnknownColorSource wrapper
199+
// so as to not lose any data.
200+
const SkColor* sk_colors = reinterpret_cast<const SkColor*>(TestColors);
201+
sk_sp<SkShader> shader = SkGradientShader::MakeSweep(
202+
TestPoints[0].fX, TestPoints[0].fY, sk_colors, TestStops, kTestStopCount);
230203
std::shared_ptr<DlColorSource> source = DlColorSource::From(shader);
231-
std::shared_ptr<DlColorSource> dl_source =
232-
DlColorSource::MakeSweep(TestPoints[0], 0, 360, kTestStopCount,
233-
TestColors, TestStops, DlTileMode::kClamp);
234-
ASSERT_EQ(source->type(), DlColorSourceType::kSweepGradient);
235-
EXPECT_TRUE(*source->asSweepGradient() == *dl_source->asSweepGradient());
236-
ASSERT_EQ(*source->asSweepGradient(), *dl_source->asSweepGradient());
237-
ASSERT_EQ(source->asSweepGradient()->center(), TestPoints[0]);
238-
ASSERT_EQ(source->asSweepGradient()->start(), 0);
239-
ASSERT_EQ(source->asSweepGradient()->end(), 360);
240-
ASSERT_EQ(source->asSweepGradient()->stop_count(), kTestStopCount);
241-
for (int i = 0; i < kTestStopCount; i++) {
242-
ASSERT_EQ(source->asSweepGradient()->colors()[i], TestColors[i]);
243-
ASSERT_EQ(source->asSweepGradient()->stops()[i], TestStops[i]);
244-
}
245-
ASSERT_EQ(source->asSweepGradient()->tile_mode(), DlTileMode::kClamp);
246-
ASSERT_EQ(source->asSweepGradient()->matrix(), SkMatrix::I());
204+
ASSERT_EQ(source->type(), DlColorSourceType::kUnknown);
205+
ASSERT_EQ(source->skia_object(), shader);
247206

248207
ASSERT_EQ(source->asColor(), nullptr);
249208
ASSERT_EQ(source->asImage(), nullptr);
250209
ASSERT_EQ(source->asLinearGradient(), nullptr);
251210
ASSERT_EQ(source->asRadialGradient(), nullptr);
252211
ASSERT_EQ(source->asConicalGradient(), nullptr);
253-
ASSERT_NE(source->asSweepGradient(), nullptr);
212+
ASSERT_EQ(source->asSweepGradient(), nullptr);
254213
}
255214

256215
TEST(DisplayListColorSource, FromSkiaUnrecognizedShader) {

0 commit comments

Comments
 (0)