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

Commit 4492be6

Browse files
authored
Removed linear gradient heap allocation for color conversions between dart and display list (#57108)
issue: flutter/flutter#154650 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/engine/blob/main/docs/testing/Testing-the-engine.md [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md
1 parent b59afdf commit 4492be6

8 files changed

+150
-23
lines changed

display_list/effects/color_sources/dl_gradient_color_source_base.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ class DlGradientColorSourceBase : public DlMatrixColorSourceBase {
4242
const DlColor* color_data,
4343
const float* stop_data);
4444

45+
void store_color_stops(void* pod,
46+
const DlScalar* color_data_argb,
47+
const float* stop_data);
48+
4549
private:
4650
DlTileMode mode_;
4751
uint32_t stop_count_;

display_list/effects/color_sources/dl_linear_gradient_color_source.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,20 @@ DlLinearGradientColorSource::DlLinearGradientColorSource(
2020
store_color_stops(this + 1, colors, stops);
2121
}
2222

23+
DlLinearGradientColorSource::DlLinearGradientColorSource(
24+
const DlPoint start_point,
25+
const DlPoint end_point,
26+
uint32_t stop_count,
27+
const DlScalar* colors,
28+
const float* stops,
29+
DlTileMode tile_mode,
30+
const DlMatrix* matrix)
31+
: DlGradientColorSourceBase(stop_count, tile_mode, matrix),
32+
start_point_(start_point),
33+
end_point_(end_point) {
34+
store_color_stops(this + 1, colors, stops);
35+
}
36+
2337
DlLinearGradientColorSource::DlLinearGradientColorSource(
2438
const DlLinearGradientColorSource* source)
2539
: DlGradientColorSourceBase(source->stop_count(),

display_list/effects/color_sources/dl_linear_gradient_color_source.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,14 @@ class DlLinearGradientColorSource final : public DlGradientColorSourceBase {
4141
DlTileMode tile_mode,
4242
const DlMatrix* matrix = nullptr);
4343

44+
DlLinearGradientColorSource(const DlPoint start_point,
45+
const DlPoint end_point,
46+
uint32_t stop_count,
47+
const DlScalar* colors,
48+
const float* stops,
49+
DlTileMode tile_mode,
50+
const DlMatrix* matrix = nullptr);
51+
4452
explicit DlLinearGradientColorSource(
4553
const DlLinearGradientColorSource* source);
4654

display_list/effects/dl_color_source.cc

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ std::shared_ptr<DlColorSource> DlColorSource::MakeImage(
3030
image, horizontal_tile_mode, vertical_tile_mode, sampling, matrix);
3131
}
3232

33+
namespace {
34+
size_t CalculateLinearGradientSize(uint32_t stop_count) {
35+
return sizeof(DlLinearGradientColorSource) +
36+
(stop_count * (sizeof(DlColor) + sizeof(float)));
37+
}
38+
} // namespace
39+
3340
std::shared_ptr<DlColorSource> DlColorSource::MakeLinear(
3441
const DlPoint start_point,
3542
const DlPoint end_point,
@@ -38,9 +45,7 @@ std::shared_ptr<DlColorSource> DlColorSource::MakeLinear(
3845
const float* stops,
3946
DlTileMode tile_mode,
4047
const DlMatrix* matrix) {
41-
size_t needed = sizeof(DlLinearGradientColorSource) +
42-
(stop_count * (sizeof(DlColor) + sizeof(float)));
43-
48+
size_t needed = CalculateLinearGradientSize(stop_count);
4449
void* storage = ::operator new(needed);
4550

4651
std::shared_ptr<DlLinearGradientColorSource> ret;
@@ -51,6 +56,25 @@ std::shared_ptr<DlColorSource> DlColorSource::MakeLinear(
5156
return ret;
5257
}
5358

59+
std::shared_ptr<DlColorSource> DlColorSource::MakeLinear(
60+
const DlPoint start_point,
61+
const DlPoint end_point,
62+
uint32_t stop_count,
63+
const DlScalar* colors_argb,
64+
const float* stops,
65+
DlTileMode tile_mode,
66+
const DlMatrix* matrix) {
67+
size_t needed = CalculateLinearGradientSize(stop_count);
68+
void* storage = ::operator new(needed);
69+
70+
std::shared_ptr<DlLinearGradientColorSource> ret;
71+
ret.reset(new (storage) DlLinearGradientColorSource(start_point, end_point,
72+
stop_count, colors_argb,
73+
stops, tile_mode, matrix),
74+
DlGradientDeleter);
75+
return ret;
76+
}
77+
5478
std::shared_ptr<DlColorSource> DlColorSource::MakeRadial(
5579
DlPoint center,
5680
DlScalar radius,
@@ -157,23 +181,72 @@ bool DlGradientColorSourceBase::base_equals_(
157181
stop_count_ * sizeof(stops()[0])) == 0);
158182
}
159183

160-
void DlGradientColorSourceBase::store_color_stops(void* pod,
161-
const DlColor* color_data,
162-
const float* stop_data) {
184+
namespace {
185+
template <typename DlColorIt>
186+
void do_store_color_stops(void* pod,
187+
DlColorIt color_data_argb_begin,
188+
DlColorIt color_data_argb_end,
189+
const float* stop_data) {
163190
DlColor* color_storage = reinterpret_cast<DlColor*>(pod);
164-
memcpy(color_storage, color_data, stop_count_ * sizeof(*color_data));
165-
float* stop_storage = reinterpret_cast<float*>(color_storage + stop_count_);
191+
uint32_t stop_count = 0;
192+
while (color_data_argb_begin < color_data_argb_end) {
193+
*color_storage++ = *color_data_argb_begin++;
194+
stop_count += 1;
195+
}
196+
float* stop_storage = reinterpret_cast<float*>(color_storage);
166197
if (stop_data) {
167-
memcpy(stop_storage, stop_data, stop_count_ * sizeof(*stop_data));
198+
memcpy(stop_storage, stop_data, stop_count * sizeof(*stop_data));
168199
} else {
169-
float div = stop_count_ - 1;
200+
float div = stop_count - 1;
170201
if (div <= 0) {
171202
div = 1;
172203
}
173-
for (uint32_t i = 0; i < stop_count_; i++) {
204+
for (uint32_t i = 0; i < stop_count; i++) {
174205
stop_storage[i] = i / div;
175206
}
176207
}
177208
}
178209

210+
class DlScalerToDlColorIterator {
211+
public:
212+
explicit DlScalerToDlColorIterator(const DlScalar* ptr) : ptr_(ptr) {}
213+
DlScalerToDlColorIterator(DlScalerToDlColorIterator&&) = default;
214+
DlScalerToDlColorIterator(const DlScalerToDlColorIterator&) = delete;
215+
DlScalerToDlColorIterator& operator=(const DlScalerToDlColorIterator&) =
216+
delete;
217+
218+
DlColor operator*() {
219+
return DlColor(ptr_[0], ptr_[1], ptr_[2], ptr_[3],
220+
DlColorSpace::kExtendedSRGB);
221+
}
222+
DlScalerToDlColorIterator operator++(int) {
223+
auto result = DlScalerToDlColorIterator(ptr_);
224+
ptr_ += 4;
225+
return result;
226+
}
227+
bool operator<(const DlScalerToDlColorIterator& that) const {
228+
return ptr_ < that.ptr_;
229+
}
230+
231+
private:
232+
const DlScalar* ptr_;
233+
};
234+
235+
} // namespace
236+
237+
void DlGradientColorSourceBase::store_color_stops(void* pod,
238+
const DlColor* color_data,
239+
const float* stop_data) {
240+
do_store_color_stops(pod, color_data, color_data + stop_count_, stop_data);
241+
}
242+
243+
void DlGradientColorSourceBase::store_color_stops(
244+
void* pod,
245+
const DlScalar* color_data_argb,
246+
const float* stop_data) {
247+
do_store_color_stops(
248+
pod, DlScalerToDlColorIterator(color_data_argb),
249+
DlScalerToDlColorIterator(color_data_argb + stop_count_ * 4), stop_data);
250+
}
251+
179252
} // namespace flutter

display_list/effects/dl_color_source.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ class DlColorSource : public DlAttribute<DlColorSource, DlColorSourceType> {
5959
DlTileMode tile_mode,
6060
const DlMatrix* matrix = nullptr);
6161

62+
/// @brief Make a linear gradient.
63+
/// @param colors_argb Array of DlScalars that represents colors in the ARGB.
64+
static std::shared_ptr<DlColorSource> MakeLinear(
65+
const DlPoint start_point,
66+
const DlPoint end_point,
67+
uint32_t stop_count,
68+
const DlScalar* colors_argb,
69+
const float* stops,
70+
DlTileMode tile_mode,
71+
const DlMatrix* matrix = nullptr);
72+
6273
static std::shared_ptr<DlColorSource> MakeRadial(
6374
DlPoint center,
6475
DlScalar radius,

display_list/effects/dl_color_source_unittests.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,32 @@ TEST(DisplayListColorSource, LinearGradientConstructor) {
189189
DlTileMode::kClamp, &kTestMatrix1);
190190
}
191191

192+
TEST(DisplayListColorSource, LinearGradientARGBConstructor) {
193+
std::array<DlScalar, kTestStopCount * 4> colors;
194+
for (int i = 0; i < kTestStopCount; ++i) {
195+
colors[i * 4 + 0] = kTestColors[i].getAlphaF(); //
196+
colors[i * 4 + 1] = kTestColors[i].getRedF(); //
197+
colors[i * 4 + 2] = kTestColors[i].getGreenF(); //
198+
colors[i * 4 + 3] = kTestColors[i].getBlueF();
199+
}
200+
std::shared_ptr<DlColorSource> source = DlColorSource::MakeLinear(
201+
kTestPoints[0], kTestPoints[1], kTestStopCount, colors.data(), kTestStops,
202+
DlTileMode::kClamp, &kTestMatrix1);
203+
ASSERT_TRUE(source);
204+
ASSERT_TRUE(source->asLinearGradient());
205+
EXPECT_EQ(source->asLinearGradient()->start_point(), kTestPoints[0]);
206+
EXPECT_EQ(source->asLinearGradient()->end_point(), kTestPoints[1]);
207+
EXPECT_EQ(source->asLinearGradient()->stop_count(), kTestStopCount);
208+
for (int i = 0; i < kTestStopCount; i++) {
209+
EXPECT_EQ(source->asLinearGradient()->colors()[i],
210+
kTestColors[i].withColorSpace(DlColorSpace::kExtendedSRGB));
211+
EXPECT_EQ(source->asLinearGradient()->stops()[i], kTestStops[i]);
212+
}
213+
EXPECT_EQ(source->asLinearGradient()->tile_mode(), DlTileMode::kClamp);
214+
EXPECT_EQ(source->asLinearGradient()->matrix(), kTestMatrix1);
215+
EXPECT_EQ(source->is_opaque(), true);
216+
}
217+
192218
TEST(DisplayListColorSource, LinearGradientShared) {
193219
std::shared_ptr<DlColorSource> source = DlColorSource::MakeLinear(
194220
kTestPoints[0], kTestPoints[1], kTestStopCount, kTestColors, kTestStops,

display_list/skia/dl_sk_conversions_unittests.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,8 @@ TEST(DisplayListSkConversions, ToSkDitheringEnabledForGradients) {
302302

303303
// Set the paint to be a gradient.
304304
dl_paint.setColorSource(DlColorSource::MakeLinear(
305-
DlPoint(0, 0), DlPoint(100, 100), 0, 0, 0, DlTileMode::kClamp));
305+
DlPoint(0, 0), DlPoint(100, 100), 0,
306+
std::array<DlColor, 1>{DlColor(0)}.data(), 0, DlTileMode::kClamp));
306307

307308
{
308309
SkPaint sk_paint = ToSk(dl_paint);

lib/ui/painting/gradient.cc

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,7 @@ void CanvasGradient::initLinear(const tonic::Float32List& end_points,
4444

4545
DlPoint p0 = DlPoint(end_points[0], end_points[1]);
4646
DlPoint p1 = DlPoint(end_points[2], end_points[3]);
47-
std::vector<DlColor> dl_colors;
48-
dl_colors.reserve(num_colors);
49-
for (int i = 0; i < colors.num_elements(); i += 4) {
50-
DlScalar a = colors[i + 0];
51-
DlScalar r = colors[i + 1];
52-
DlScalar g = colors[i + 2];
53-
DlScalar b = colors[i + 3];
54-
dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB));
55-
}
56-
57-
dl_shader_ = DlColorSource::MakeLinear(p0, p1, num_colors, dl_colors.data(),
47+
dl_shader_ = DlColorSource::MakeLinear(p0, p1, num_colors, colors.data(),
5848
color_stops.data(), tile_mode,
5949
has_matrix ? &dl_matrix : nullptr);
6050
// Just a sanity check, all gradient shaders should be thread-safe

0 commit comments

Comments
 (0)