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

Commit 82c78ad

Browse files
jonahwilliamsjonahwilliams
authored
[Impeller] Match Skia gradient clamping behavior (and document). (#44825)
Skia clamps any gradient stops to values of 0.0 to 1.0. Implement this behavior in Impeller and document it in dart:ui (Framework also needs to be documented). This also matches the w3c gradient behavior: https://www.w3.org/TR/2000/CR-SVG-20000802/pservers.html - almost. We might be slightly off with how we're inserting additional stops for 0.0 and 1.0, but at least its closer. Fixes flutter/flutter#132792
1 parent 8ea2612 commit 82c78ad

File tree

5 files changed

+195
-31
lines changed

5 files changed

+195
-31
lines changed

‎impeller/display_list/dl_dispatcher.cc

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -271,30 +271,6 @@ static std::vector<Color> ToColors(const flutter::DlColor colors[], int count) {
271271
return result;
272272
}
273273

274-
// Convert display list colors + stops into impeller colors and stops, taking
275-
// care to ensure that the stops always start with 0.0 and end with 1.0.
276-
template <typename T>
277-
static void ConvertStops(T* gradient,
278-
std::vector<Color>* colors,
279-
std::vector<float>* stops) {
280-
FML_DCHECK(gradient->stop_count() >= 2);
281-
282-
auto* dl_colors = gradient->colors();
283-
auto* dl_stops = gradient->stops();
284-
if (dl_stops[0] != 0.0) {
285-
colors->emplace_back(skia_conversions::ToColor(dl_colors[0]));
286-
stops->emplace_back(0);
287-
}
288-
for (auto i = 0; i < gradient->stop_count(); i++) {
289-
colors->emplace_back(skia_conversions::ToColor(dl_colors[i]));
290-
stops->emplace_back(dl_stops[i]);
291-
}
292-
if (stops->back() != 1.0) {
293-
colors->emplace_back(colors->back());
294-
stops->emplace_back(1.0);
295-
}
296-
}
297-
298274
static std::optional<ColorSource::Type> ToColorSourceType(
299275
flutter::DlColorSourceType type) {
300276
switch (type) {
@@ -351,7 +327,7 @@ void DlDispatcher::setColorSource(const flutter::DlColorSource* source) {
351327
auto end_point = skia_conversions::ToPoint(linear->end_point());
352328
std::vector<Color> colors;
353329
std::vector<float> stops;
354-
ConvertStops(linear, &colors, &stops);
330+
skia_conversions::ConvertStops(linear, colors, stops);
355331

356332
auto tile_mode = ToTileMode(linear->tile_mode());
357333
auto matrix = ToMatrix(linear->matrix());
@@ -372,7 +348,7 @@ void DlDispatcher::setColorSource(const flutter::DlColorSource* source) {
372348
SkScalar focus_radius = conical_gradient->start_radius();
373349
std::vector<Color> colors;
374350
std::vector<float> stops;
375-
ConvertStops(conical_gradient, &colors, &stops);
351+
skia_conversions::ConvertStops(conical_gradient, colors, stops);
376352

377353
auto tile_mode = ToTileMode(conical_gradient->tile_mode());
378354
auto matrix = ToMatrix(conical_gradient->matrix());
@@ -390,7 +366,7 @@ void DlDispatcher::setColorSource(const flutter::DlColorSource* source) {
390366
auto radius = radialGradient->radius();
391367
std::vector<Color> colors;
392368
std::vector<float> stops;
393-
ConvertStops(radialGradient, &colors, &stops);
369+
skia_conversions::ConvertStops(radialGradient, colors, stops);
394370

395371
auto tile_mode = ToTileMode(radialGradient->tile_mode());
396372
auto matrix = ToMatrix(radialGradient->matrix());
@@ -409,7 +385,7 @@ void DlDispatcher::setColorSource(const flutter::DlColorSource* source) {
409385
auto end_angle = Degrees(sweepGradient->end());
410386
std::vector<Color> colors;
411387
std::vector<float> stops;
412-
ConvertStops(sweepGradient, &colors, &stops);
388+
skia_conversions::ConvertStops(sweepGradient, colors, stops);
413389

414390
auto tile_mode = ToTileMode(sweepGradient->tile_mode());
415391
auto matrix = ToMatrix(sweepGradient->matrix());

‎impeller/display_list/skia_conversions.cc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,5 +189,29 @@ std::optional<impeller::PixelFormat> ToPixelFormat(SkColorType type) {
189189
return std::nullopt;
190190
}
191191

192+
void ConvertStops(const flutter::DlGradientColorSourceBase* gradient,
193+
std::vector<Color>& colors,
194+
std::vector<float>& stops) {
195+
FML_DCHECK(gradient->stop_count() >= 2);
196+
197+
auto* dl_colors = gradient->colors();
198+
auto* dl_stops = gradient->stops();
199+
if (dl_stops[0] != 0.0) {
200+
colors.emplace_back(skia_conversions::ToColor(dl_colors[0]));
201+
stops.emplace_back(0);
202+
}
203+
for (auto i = 0; i < gradient->stop_count(); i++) {
204+
colors.emplace_back(skia_conversions::ToColor(dl_colors[i]));
205+
stops.emplace_back(std::clamp(dl_stops[i], 0.0f, 1.0f));
206+
}
207+
if (dl_stops[gradient->stop_count() - 1] != 1.0) {
208+
colors.emplace_back(colors.back());
209+
stops.emplace_back(1.0);
210+
}
211+
for (auto i = 1; i < gradient->stop_count(); i++) {
212+
stops[i] = std::clamp(stops[i], stops[i - 1], stops[i]);
213+
}
214+
}
215+
192216
} // namespace skia_conversions
193217
} // namespace impeller

‎impeller/display_list/skia_conversions.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#pragma once
66

77
#include "display_list/dl_color.h"
8+
#include "display_list/effects/dl_color_source.h"
89
#include "impeller/core/formats.h"
910
#include "impeller/geometry/color.h"
1011
#include "impeller/geometry/path.h"
@@ -47,5 +48,23 @@ Path PathDataFromTextBlob(const sk_sp<SkTextBlob>& blob,
4748

4849
std::optional<impeller::PixelFormat> ToPixelFormat(SkColorType type);
4950

51+
/// @brief Convert display list colors + stops into impeller colors and stops,
52+
/// taking care to ensure that the stops monotonically increase from 0.0 to 1.0.
53+
///
54+
/// The general process is:
55+
/// * Ensure that the first gradient stop value is 0.0. If not, insert a new
56+
/// stop with a value of 0.0 and use the first gradient color as this new
57+
/// stops color.
58+
/// * Ensure the last gradient stop value is 1.0. If not, insert a new stop
59+
/// with a value of 1.0 and use the last gradient color as this stops color.
60+
/// * Clamp all gradient values between the values of 0.0 and 1.0.
61+
/// * For all stop values, ensure that the values are monotonically increasing
62+
/// by clamping each value to a minimum of the previous stop value and itself.
63+
/// For example, with stop values of 0.0, 0.5, 0.4, 1.0, we would clamp such
64+
/// that the values were 0.0, 0.5, 0.5, 1.0.
65+
void ConvertStops(const flutter::DlGradientColorSourceBase* gradient,
66+
std::vector<Color>& colors,
67+
std::vector<float>& stops);
68+
5069
} // namespace skia_conversions
5170
} // namespace impeller

‎impeller/display_list/skia_conversions_unittests.cc

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
#include "display_list/dl_color.h"
6+
#include "display_list/dl_tile_mode.h"
57
#include "flutter/testing/testing.h"
68
#include "impeller/display_list/skia_conversions.h"
79
#include "impeller/geometry/scalar.h"
@@ -23,5 +25,136 @@ TEST(SkiaConversionsTest, ToColor) {
2325
ASSERT_TRUE(ScalarNearlyEqual(converted_color.blue, 0x20 * (1.0f / 255)));
2426
}
2527

28+
TEST(SkiaConversionsTest, GradientStopConversion) {
29+
// Typical gradient.
30+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
31+
flutter::DlColor::kRed(),
32+
flutter::DlColor::kGreen()};
33+
std::vector<float> stops = {0.0, 0.5, 1.0};
34+
const auto gradient =
35+
flutter::DlColorSource::MakeLinear(SkPoint::Make(0, 0), //
36+
SkPoint::Make(1.0, 1.0), //
37+
3, //
38+
colors.data(), //
39+
stops.data(), //
40+
flutter::DlTileMode::kClamp, //
41+
nullptr //
42+
);
43+
44+
std::vector<Color> converted_colors;
45+
std::vector<Scalar> converted_stops;
46+
skia_conversions::ConvertStops(gradient.get(), converted_colors,
47+
converted_stops);
48+
49+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
50+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
51+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
52+
}
53+
54+
TEST(SkiaConversionsTest, GradientMissing0) {
55+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
56+
flutter::DlColor::kRed()};
57+
std::vector<float> stops = {0.5, 1.0};
58+
const auto gradient =
59+
flutter::DlColorSource::MakeLinear(SkPoint::Make(0, 0), //
60+
SkPoint::Make(1.0, 1.0), //
61+
2, //
62+
colors.data(), //
63+
stops.data(), //
64+
flutter::DlTileMode::kClamp, //
65+
nullptr //
66+
);
67+
68+
std::vector<Color> converted_colors;
69+
std::vector<Scalar> converted_stops;
70+
skia_conversions::ConvertStops(gradient.get(), converted_colors,
71+
converted_stops);
72+
73+
// First color is inserted as blue.
74+
ASSERT_TRUE(ScalarNearlyEqual(converted_colors[0].blue, 1.0f));
75+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
76+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
77+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
78+
}
79+
80+
TEST(SkiaConversionsTest, GradientMissingLastValue) {
81+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
82+
flutter::DlColor::kRed()};
83+
std::vector<float> stops = {0.0, .5};
84+
const auto gradient =
85+
flutter::DlColorSource::MakeLinear(SkPoint::Make(0, 0), //
86+
SkPoint::Make(1.0, 1.0), //
87+
2, //
88+
colors.data(), //
89+
stops.data(), //
90+
flutter::DlTileMode::kClamp, //
91+
nullptr //
92+
);
93+
94+
std::vector<Color> converted_colors;
95+
std::vector<Scalar> converted_stops;
96+
skia_conversions::ConvertStops(gradient.get(), converted_colors,
97+
converted_stops);
98+
99+
// Last color is inserted as red.
100+
ASSERT_TRUE(ScalarNearlyEqual(converted_colors[2].red, 1.0f));
101+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
102+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
103+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
104+
}
105+
106+
TEST(SkiaConversionsTest, GradientStopGreaterThan1) {
107+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
108+
flutter::DlColor::kGreen(),
109+
flutter::DlColor::kRed()};
110+
std::vector<float> stops = {0.0, 100, 1.0};
111+
const auto gradient =
112+
flutter::DlColorSource::MakeLinear(SkPoint::Make(0, 0), //
113+
SkPoint::Make(1.0, 1.0), //
114+
3, //
115+
colors.data(), //
116+
stops.data(), //
117+
flutter::DlTileMode::kClamp, //
118+
nullptr //
119+
);
120+
121+
std::vector<Color> converted_colors;
122+
std::vector<Scalar> converted_stops;
123+
skia_conversions::ConvertStops(gradient.get(), converted_colors,
124+
converted_stops);
125+
126+
// Value is clamped to 1.0
127+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
128+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 1.0f));
129+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
130+
}
131+
132+
TEST(SkiaConversionsTest, GradientConversionNonMonotonic) {
133+
std::vector<flutter::DlColor> colors = {
134+
flutter::DlColor::kBlue(), flutter::DlColor::kGreen(),
135+
flutter::DlColor::kGreen(), flutter::DlColor::kRed()};
136+
std::vector<float> stops = {0.0, 0.5, 0.4, 1.0};
137+
const auto gradient =
138+
flutter::DlColorSource::MakeLinear(SkPoint::Make(0, 0), //
139+
SkPoint::Make(1.0, 1.0), //
140+
4, //
141+
colors.data(), //
142+
stops.data(), //
143+
flutter::DlTileMode::kClamp, //
144+
nullptr //
145+
);
146+
147+
std::vector<Color> converted_colors;
148+
std::vector<Scalar> converted_stops;
149+
skia_conversions::ConvertStops(gradient.get(), converted_colors,
150+
converted_stops);
151+
152+
// Value is clamped to 0.5
153+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
154+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
155+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 0.5f));
156+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[3], 1.0f));
157+
}
158+
26159
} // namespace testing
27160
} // namespace impeller

‎lib/ui/painting.dart

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4191,7 +4191,11 @@ base class Gradient extends Shader {
41914191
/// If `colorStops` is provided, `colorStops[i]` is a number from 0.0 to 1.0
41924192
/// that specifies where `color[i]` begins in the gradient. If `colorStops` is
41934193
/// not provided, then only two stops, at 0.0 and 1.0, are implied (and
4194-
/// `color` must therefore only have two entries).
4194+
/// `color` must therefore only have two entries). Stop values less than 0.0
4195+
/// will be rounded up to 0.0 and stop values greater than 1.0 will be rounded
4196+
/// down to 1.0. Each stop value must be greater than or equal to the previous
4197+
/// stop value. Stop values that do not meet this criteria will be rounded up
4198+
/// to the previous stop value.
41954199
///
41964200
/// The behavior before `from` and after `to` is described by the `tileMode`
41974201
/// argument. For details, see the [TileMode] enum.
@@ -4233,7 +4237,11 @@ base class Gradient extends Shader {
42334237
/// If `colorStops` is provided, `colorStops[i]` is a number from 0.0 to 1.0
42344238
/// that specifies where `color[i]` begins in the gradient. If `colorStops` is
42354239
/// not provided, then only two stops, at 0.0 and 1.0, are implied (and
4236-
/// `color` must therefore only have two entries).
4240+
/// `color` must therefore only have two entries). Stop values less than 0.0
4241+
/// will be rounded up to 0.0 and stop values greater than 1.0 will be rounded
4242+
/// down to 1.0. Each stop value must be greater than or equal to the previous
4243+
/// stop value. Stop values that do not meet this criteria will be rounded up
4244+
/// to the previous stop value.
42374245
///
42384246
/// The behavior before and after the radius is described by the `tileMode`
42394247
/// argument. For details, see the [TileMode] enum.
@@ -4295,7 +4303,11 @@ base class Gradient extends Shader {
42954303
/// If `colorStops` is provided, `colorStops[i]` is a number from 0.0 to 1.0
42964304
/// that specifies where `color[i]` begins in the gradient. If `colorStops` is
42974305
/// not provided, then only two stops, at 0.0 and 1.0, are implied (and
4298-
/// `color` must therefore only have two entries).
4306+
/// `color` must therefore only have two entries). Stop values less than 0.0
4307+
/// will be rounded up to 0.0 and stop values greater than 1.0 will be rounded
4308+
/// down to 1.0. Each stop value must be greater than or equal to the previous
4309+
/// stop value. Stop values that do not meet this criteria will be rounded up
4310+
/// to the previous stop value.
42994311
///
43004312
/// The behavior before `startAngle` and after `endAngle` is described by the
43014313
/// `tileMode` argument. For details, see the [TileMode] enum.

0 commit comments

Comments
 (0)