Skip to content

Commit e407d87

Browse files
authored
[Impeller] Move skia_conversions towards retirement (#165408)
The skia_conversions sources and namespace have been filled with methods that are no longer used now that the DisplayList has its own complete set of APIs and is now mostly obsolete.
1 parent ce667ed commit e407d87

File tree

11 files changed

+237
-437
lines changed

11 files changed

+237
-437
lines changed

engine/src/flutter/ci/licenses_golden/excluded_files

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@
151151
../../../flutter/impeller/display_list/dl_golden_unittests.cc
152152
../../../flutter/impeller/display_list/dl_golden_unittests.h
153153
../../../flutter/impeller/display_list/dl_unittests.cc
154+
../../../flutter/impeller/display_list/paint_unittests.cc
154155
../../../flutter/impeller/display_list/skia_conversions_unittests.cc
155156
../../../flutter/impeller/display_list/testing
156157
../../../flutter/impeller/docs

engine/src/flutter/impeller/display_list/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ template("display_list_unittests_component") {
7878
"dl_playground.cc",
7979
"dl_playground.h",
8080
"dl_unittests.cc",
81+
"paint_unittests.cc",
8182
"testing/render_text_in_canvas.cc",
8283
"testing/render_text_in_canvas.h",
8384
"testing/rmse.cc",

engine/src/flutter/impeller/display_list/dl_dispatcher.cc

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,13 @@ static impeller::SamplerDescriptor ToSamplerDescriptor(
137137
return desc;
138138
}
139139

140+
static std::optional<const Rect> ToOptRect(const flutter::DlRect* rect) {
141+
if (rect == nullptr) {
142+
return std::nullopt;
143+
}
144+
return *rect;
145+
}
146+
140147
// |flutter::DlOpReceiver|
141148
void DlDispatcherBase::setAntiAlias(bool aa) {
142149
AUTO_DEPTH_WATCHER(0u);
@@ -506,8 +513,8 @@ void DlDispatcherBase::clipPath(const DlPath& path,
506513
} else {
507514
SkRRect rrect;
508515
if (path.IsSkRRect(&rrect) && rrect.isSimple()) {
509-
RoundRectGeometry geom(skia_conversions::ToRect(rrect.rect()),
510-
skia_conversions::ToSize(rrect.getSimpleRadii()));
516+
RoundRectGeometry geom(flutter::ToDlRect(rrect.rect()),
517+
flutter::ToDlSize(rrect.getSimpleRadii()));
511518
GetCanvas().ClipGeometry(geom, clip_op);
512519
} else {
513520
FillPathGeometry geom(path.GetPath());
@@ -820,7 +827,7 @@ void DlDispatcherBase::drawAtlas(const sk_sp<flutter::DlImage> atlas,
820827
static_cast<size_t>(count), //
821828
skia_conversions::ToBlendMode(mode), //
822829
skia_conversions::ToSamplerDescriptor(sampling), //
823-
skia_conversions::ToRect(cull_rect) //
830+
ToOptRect(cull_rect) //
824831
);
825832
auto atlas_contents = std::make_shared<AtlasContents>();
826833
atlas_contents->SetGeometry(&geometry);
@@ -854,10 +861,10 @@ void DlDispatcherBase::drawDisplayList(
854861
if (opacity < SK_Scalar1) {
855862
Paint save_paint;
856863
save_paint.color = Color(0, 0, 0, opacity);
857-
GetCanvas().SaveLayer(
858-
save_paint, skia_conversions::ToRect(display_list->bounds()), nullptr,
859-
ContentBoundsPromise::kContainsContents, display_list->total_depth(),
860-
display_list->can_apply_group_opacity());
864+
GetCanvas().SaveLayer(save_paint, display_list->GetBounds(), nullptr,
865+
ContentBoundsPromise::kContainsContents,
866+
display_list->total_depth(),
867+
display_list->can_apply_group_opacity());
861868
} else {
862869
// The display list may alter the clip, which must be restored to the
863870
// current clip at the end of playback.

engine/src/flutter/impeller/display_list/image_filter.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "impeller/display_list/image_filter.h"
66

7+
#include "flutter/display_list/effects/dl_color_sources.h"
78
#include "flutter/display_list/effects/dl_image_filters.h"
89
#include "fml/logging.h"
910
#include "impeller/display_list/color_filter.h"

engine/src/flutter/impeller/display_list/paint.cc

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,31 @@ using DlRect = flutter::DlRect;
3434
using DlIRect = flutter::DlIRect;
3535
using DlPath = flutter::DlPath;
3636

37+
void Paint::ConvertStops(const flutter::DlGradientColorSourceBase* gradient,
38+
std::vector<Color>& colors,
39+
std::vector<float>& stops) {
40+
FML_DCHECK(gradient->stop_count() >= 2)
41+
<< "stop_count:" << gradient->stop_count();
42+
43+
auto* dl_colors = gradient->colors();
44+
auto* dl_stops = gradient->stops();
45+
if (dl_stops[0] != 0.0) {
46+
colors.emplace_back(skia_conversions::ToColor(dl_colors[0]));
47+
stops.emplace_back(0);
48+
}
49+
for (auto i = 0; i < gradient->stop_count(); i++) {
50+
colors.emplace_back(skia_conversions::ToColor(dl_colors[i]));
51+
stops.emplace_back(std::clamp(dl_stops[i], 0.0f, 1.0f));
52+
}
53+
if (dl_stops[gradient->stop_count() - 1] != 1.0) {
54+
colors.emplace_back(colors.back());
55+
stops.emplace_back(1.0);
56+
}
57+
for (auto i = 1; i < gradient->stop_count(); i++) {
58+
stops[i] = std::clamp(stops[i], stops[i - 1], stops[i]);
59+
}
60+
}
61+
3762
std::shared_ptr<ColorSourceContents> Paint::CreateContents() const {
3863
if (color_source == nullptr) {
3964
auto contents = std::make_shared<SolidColorContents>();
@@ -50,7 +75,7 @@ std::shared_ptr<ColorSourceContents> Paint::CreateContents() const {
5075
auto end_point = linear->end_point();
5176
std::vector<Color> colors;
5277
std::vector<float> stops;
53-
skia_conversions::ConvertStops(linear, colors, stops);
78+
ConvertStops(linear, colors, stops);
5479

5580
auto tile_mode = static_cast<Entity::TileMode>(linear->tile_mode());
5681
auto effect_transform = linear->matrix();
@@ -78,7 +103,7 @@ std::shared_ptr<ColorSourceContents> Paint::CreateContents() const {
78103
auto radius = radialGradient->radius();
79104
std::vector<Color> colors;
80105
std::vector<float> stops;
81-
skia_conversions::ConvertStops(radialGradient, colors, stops);
106+
ConvertStops(radialGradient, colors, stops);
82107

83108
auto tile_mode =
84109
static_cast<Entity::TileMode>(radialGradient->tile_mode());
@@ -110,7 +135,7 @@ std::shared_ptr<ColorSourceContents> Paint::CreateContents() const {
110135
DlScalar focus_radius = conical_gradient->start_radius();
111136
std::vector<Color> colors;
112137
std::vector<float> stops;
113-
skia_conversions::ConvertStops(conical_gradient, colors, stops);
138+
ConvertStops(conical_gradient, colors, stops);
114139

115140
auto tile_mode =
116141
static_cast<Entity::TileMode>(conical_gradient->tile_mode());
@@ -144,7 +169,7 @@ std::shared_ptr<ColorSourceContents> Paint::CreateContents() const {
144169
auto end_angle = Degrees(sweepGradient->end());
145170
std::vector<Color> colors;
146171
std::vector<float> stops;
147-
skia_conversions::ConvertStops(sweepGradient, colors, stops);
172+
ConvertStops(sweepGradient, colors, stops);
148173

149174
auto tile_mode =
150175
static_cast<Entity::TileMode>(sweepGradient->tile_mode());

engine/src/flutter/impeller/display_list/paint.h

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include <memory>
99

1010
#include "display_list/effects/dl_color_filter.h"
11-
#include "display_list/effects/dl_color_source.h"
11+
#include "display_list/effects/dl_color_sources.h"
1212
#include "display_list/effects/dl_image_filter.h"
1313
#include "impeller/display_list/color_filter.h"
1414
#include "impeller/display_list/image_filter.h"
@@ -119,6 +119,25 @@ struct Paint {
119119
const Matrix& effect_transform,
120120
Entity::RenderingMode rendering_mode) const;
121121

122+
/// @brief Convert display list colors + stops into impeller colors and
123+
/// stops, taking care to ensure that the stops monotonically
124+
/// increase from 0.0 to 1.0.
125+
///
126+
/// The general process is:
127+
/// * Ensure that the first gradient stop value is 0.0. If not, insert a
128+
/// new stop with a value of 0.0 and use the first gradient color as this
129+
/// new stops color.
130+
/// * Ensure the last gradient stop value is 1.0. If not, insert a new stop
131+
/// with a value of 1.0 and use the last gradient color as this stops color.
132+
/// * Clamp all gradient values between the values of 0.0 and 1.0.
133+
/// * For all stop values, ensure that the values are monotonically
134+
/// increasing by clamping each value to a minimum of the previous stop
135+
/// value and itself. For example, with stop values of 0.0, 0.5, 0.4, 1.0,
136+
/// we would clamp such that the values were 0.0, 0.5, 0.5, 1.0.
137+
static void ConvertStops(const flutter::DlGradientColorSourceBase* gradient,
138+
std::vector<Color>& colors,
139+
std::vector<float>& stops);
140+
122141
private:
123142
std::shared_ptr<Contents> WithColorFilter(
124143
std::shared_ptr<Contents> input,
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/impeller/display_list/paint.h"
6+
7+
#include "gtest/gtest.h"
8+
9+
#include "flutter/display_list/dl_color.h"
10+
#include "flutter/display_list/dl_tile_mode.h"
11+
#include "flutter/display_list/effects/dl_color_source.h"
12+
#include "flutter/impeller/geometry/scalar.h"
13+
14+
namespace impeller {
15+
namespace testing {
16+
17+
TEST(PaintTest, GradientStopConversion) {
18+
// Typical gradient.
19+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
20+
flutter::DlColor::kRed(),
21+
flutter::DlColor::kGreen()};
22+
std::vector<float> stops = {0.0, 0.5, 1.0};
23+
const auto gradient =
24+
flutter::DlColorSource::MakeLinear(flutter::DlPoint(0, 0), //
25+
flutter::DlPoint(1.0, 1.0), //
26+
3, //
27+
colors.data(), //
28+
stops.data(), //
29+
flutter::DlTileMode::kClamp, //
30+
nullptr //
31+
);
32+
33+
std::vector<Color> converted_colors;
34+
std::vector<Scalar> converted_stops;
35+
Paint::ConvertStops(gradient->asLinearGradient(), converted_colors,
36+
converted_stops);
37+
38+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
39+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
40+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
41+
}
42+
43+
TEST(PaintTest, GradientMissing0) {
44+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
45+
flutter::DlColor::kRed()};
46+
std::vector<float> stops = {0.5, 1.0};
47+
const auto gradient =
48+
flutter::DlColorSource::MakeLinear(flutter::DlPoint(0, 0), //
49+
flutter::DlPoint(1.0, 1.0), //
50+
2, //
51+
colors.data(), //
52+
stops.data(), //
53+
flutter::DlTileMode::kClamp, //
54+
nullptr //
55+
);
56+
57+
std::vector<Color> converted_colors;
58+
std::vector<Scalar> converted_stops;
59+
Paint::ConvertStops(gradient->asLinearGradient(), converted_colors,
60+
converted_stops);
61+
62+
// First color is inserted as blue.
63+
ASSERT_TRUE(ScalarNearlyEqual(converted_colors[0].blue, 1.0f));
64+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
65+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
66+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
67+
}
68+
69+
TEST(PaintTest, GradientMissingLastValue) {
70+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
71+
flutter::DlColor::kRed()};
72+
std::vector<float> stops = {0.0, .5};
73+
const auto gradient =
74+
flutter::DlColorSource::MakeLinear(flutter::DlPoint(0, 0), //
75+
flutter::DlPoint(1.0, 1.0), //
76+
2, //
77+
colors.data(), //
78+
stops.data(), //
79+
flutter::DlTileMode::kClamp, //
80+
nullptr //
81+
);
82+
83+
std::vector<Color> converted_colors;
84+
std::vector<Scalar> converted_stops;
85+
Paint::ConvertStops(gradient->asLinearGradient(), converted_colors,
86+
converted_stops);
87+
88+
// Last color is inserted as red.
89+
ASSERT_TRUE(ScalarNearlyEqual(converted_colors[2].red, 1.0f));
90+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
91+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
92+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
93+
}
94+
95+
TEST(PaintTest, GradientStopGreaterThan1) {
96+
std::vector<flutter::DlColor> colors = {flutter::DlColor::kBlue(),
97+
flutter::DlColor::kGreen(),
98+
flutter::DlColor::kRed()};
99+
std::vector<float> stops = {0.0, 100, 1.0};
100+
const auto gradient =
101+
flutter::DlColorSource::MakeLinear(flutter::DlPoint(0, 0), //
102+
flutter::DlPoint(1.0, 1.0), //
103+
3, //
104+
colors.data(), //
105+
stops.data(), //
106+
flutter::DlTileMode::kClamp, //
107+
nullptr //
108+
);
109+
110+
std::vector<Color> converted_colors;
111+
std::vector<Scalar> converted_stops;
112+
Paint::ConvertStops(gradient->asLinearGradient(), converted_colors,
113+
converted_stops);
114+
115+
// Value is clamped to 1.0
116+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
117+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 1.0f));
118+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 1.0f));
119+
}
120+
121+
TEST(PaintTest, GradientConversionNonMonotonic) {
122+
std::vector<flutter::DlColor> colors = {
123+
flutter::DlColor::kBlue(), flutter::DlColor::kGreen(),
124+
flutter::DlColor::kGreen(), flutter::DlColor::kRed()};
125+
std::vector<float> stops = {0.0, 0.5, 0.4, 1.0};
126+
const auto gradient =
127+
flutter::DlColorSource::MakeLinear(flutter::DlPoint(0, 0), //
128+
flutter::DlPoint(1.0, 1.0), //
129+
4, //
130+
colors.data(), //
131+
stops.data(), //
132+
flutter::DlTileMode::kClamp, //
133+
nullptr //
134+
);
135+
136+
std::vector<Color> converted_colors;
137+
std::vector<Scalar> converted_stops;
138+
Paint::ConvertStops(gradient->asLinearGradient(), converted_colors,
139+
converted_stops);
140+
141+
// Value is clamped to 0.5
142+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[0], 0.0f));
143+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[1], 0.5f));
144+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[2], 0.5f));
145+
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[3], 1.0f));
146+
}
147+
148+
} // namespace testing
149+
} // namespace impeller

0 commit comments

Comments
 (0)