From 81d4bf9f9ef7774717ace64d9bfd12a272ed5af4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 10:03:30 -0800 Subject: [PATCH 1/8] Removed heap allocations for radial, conical, sweep gradients --- .../dl_radial_gradient_color_source.cc | 14 ++++++++++ .../dl_radial_gradient_color_source.h | 8 ++++++ display_list/effects/dl_color_source.cc | 21 +++++++++++++++ display_list/effects/dl_color_source.h | 15 ++++++++++- .../effects/dl_color_source_unittests.cc | 26 +++++++++++++++++++ lib/ui/painting/gradient.cc | 12 +-------- 6 files changed, 84 insertions(+), 12 deletions(-) diff --git a/display_list/effects/color_sources/dl_radial_gradient_color_source.cc b/display_list/effects/color_sources/dl_radial_gradient_color_source.cc index 7c217f649fbd7..19ba780bffb78 100644 --- a/display_list/effects/color_sources/dl_radial_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_radial_gradient_color_source.cc @@ -19,6 +19,20 @@ DlRadialGradientColorSource::DlRadialGradientColorSource(DlPoint center, store_color_stops(this + 1, colors, stops); } +DlRadialGradientColorSource::DlRadialGradientColorSource( + DlPoint center, + DlScalar radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + center_(center), + radius_(radius) { + store_color_stops(this + 1, colors_argb, stops); +} + DlRadialGradientColorSource::DlRadialGradientColorSource( const DlRadialGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_radial_gradient_color_source.h b/display_list/effects/color_sources/dl_radial_gradient_color_source.h index 70b48d504578f..d1b2baecd7552 100644 --- a/display_list/effects/color_sources/dl_radial_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_radial_gradient_color_source.h @@ -41,6 +41,14 @@ class DlRadialGradientColorSource final : public DlGradientColorSourceBase { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + DlRadialGradientColorSource(DlPoint center, + DlScalar radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix = nullptr); + explicit DlRadialGradientColorSource( const DlRadialGradientColorSource* source); diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index 6d027ef822d59..ec6b80b9ed712 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -95,6 +95,27 @@ std::shared_ptr DlColorSource::MakeRadial( return ret; } +std::shared_ptr DlColorSource::MakeRadial( + DlPoint center, + DlScalar radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix) { + size_t needed = sizeof(DlRadialGradientColorSource) + + (stop_count * (sizeof(DlColor) + sizeof(float))); + + void* storage = ::operator new(needed); + + std::shared_ptr ret; + ret.reset( + new (storage) DlRadialGradientColorSource( + center, radius, stop_count, colors_argb, stops, tile_mode, matrix), + DlGradientDeleter); + return ret; +} + std::shared_ptr DlColorSource::MakeConical( DlPoint start_center, DlScalar start_radius, diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index 44aa88ef38924..6ff28d9ce30f0 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -60,7 +60,8 @@ class DlColorSource : public DlAttribute { const DlMatrix* matrix = nullptr); /// @brief Make a linear gradient. - /// @param colors_argb Array of DlScalars that represents colors in the ARGB. + /// @param colors_argb Array of DlScalars that represents colors in the ARGB + /// format. static std::shared_ptr MakeLinear( const DlPoint start_point, const DlPoint end_point, @@ -79,6 +80,18 @@ class DlColorSource : public DlAttribute { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + /// @brief Make a radial gradient. + /// @param colors_argb Array of DlScalars that represents colors in the ARGB + /// format. + static std::shared_ptr MakeRadial( + DlPoint center, + DlScalar radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix = nullptr); + static std::shared_ptr MakeConical( DlPoint start_center, DlScalar start_radius, diff --git a/display_list/effects/dl_color_source_unittests.cc b/display_list/effects/dl_color_source_unittests.cc index de2a9b97011b2..3bb7e488e33b9 100644 --- a/display_list/effects/dl_color_source_unittests.cc +++ b/display_list/effects/dl_color_source_unittests.cc @@ -333,6 +333,32 @@ TEST(DisplayListColorSource, RadialGradientConstructor) { DlTileMode::kClamp, &kTestMatrix1); } +TEST(DisplayListColorSource, RadialGradientARGBConstructor) { + std::array colors; + for (int i = 0; i < kTestStopCount; ++i) { + colors[i * 4 + 0] = kTestColors[i].getAlphaF(); // + colors[i * 4 + 1] = kTestColors[i].getRedF(); // + colors[i * 4 + 2] = kTestColors[i].getGreenF(); // + colors[i * 4 + 3] = kTestColors[i].getBlueF(); + } + std::shared_ptr source = DlColorSource::MakeRadial( + kTestPoints[0], 10.f, kTestStopCount, colors.data(), kTestStops, + DlTileMode::kClamp, &kTestMatrix1); + ASSERT_TRUE(source); + ASSERT_TRUE(source->asRadialGradient()); + EXPECT_EQ(source->asRadialGradient()->center(), kTestPoints[0]); + EXPECT_EQ(source->asRadialGradient()->radius(), 10.f); + EXPECT_EQ(source->asRadialGradient()->stop_count(), kTestStopCount); + for (int i = 0; i < kTestStopCount; i++) { + EXPECT_EQ(source->asRadialGradient()->colors()[i], + kTestColors[i].withColorSpace(DlColorSpace::kExtendedSRGB)); + EXPECT_EQ(source->asRadialGradient()->stops()[i], kTestStops[i]); + } + EXPECT_EQ(source->asRadialGradient()->tile_mode(), DlTileMode::kClamp); + EXPECT_EQ(source->asRadialGradient()->matrix(), kTestMatrix1); + EXPECT_EQ(source->is_opaque(), true); +} + TEST(DisplayListColorSource, RadialGradientShared) { std::shared_ptr source = DlColorSource::MakeRadial( kTestPoints[0], 10.0, kTestStopCount, kTestColors, kTestStops, diff --git a/lib/ui/painting/gradient.cc b/lib/ui/painting/gradient.cc index a7ecf3be6cfdb..a5084110b2b49 100644 --- a/lib/ui/painting/gradient.cc +++ b/lib/ui/painting/gradient.cc @@ -68,19 +68,9 @@ void CanvasGradient::initRadial(double center_x, dl_matrix = ToDlMatrix(matrix4); } - std::vector dl_colors; - dl_colors.reserve(num_colors); - for (int i = 0; i < colors.num_elements(); i += 4) { - DlScalar a = colors[i + 0]; - DlScalar r = colors[i + 1]; - DlScalar g = colors[i + 2]; - DlScalar b = colors[i + 3]; - dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB)); - } - dl_shader_ = DlColorSource::MakeRadial( DlPoint(SafeNarrow(center_x), SafeNarrow(center_y)), SafeNarrow(radius), - num_colors, dl_colors.data(), color_stops.data(), tile_mode, + num_colors, colors.data(), color_stops.data(), tile_mode, has_matrix ? &dl_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); From 2ba79c95575778404f5bd3c3fded4208b3b1a4c2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 10:27:46 -0800 Subject: [PATCH 2/8] added conical gradient --- .../dl_conical_gradient_color_source.cc | 18 ++++++++++++ .../dl_conical_gradient_color_source.h | 10 +++++++ display_list/effects/dl_color_source.cc | 23 +++++++++++++++ display_list/effects/dl_color_source.h | 14 ++++++++++ .../effects/dl_color_source_unittests.cc | 28 +++++++++++++++++++ lib/ui/painting/gradient.cc | 12 +------- 6 files changed, 94 insertions(+), 11 deletions(-) diff --git a/display_list/effects/color_sources/dl_conical_gradient_color_source.cc b/display_list/effects/color_sources/dl_conical_gradient_color_source.cc index 0e94d0c6078d3..dbd93d1cf7975 100644 --- a/display_list/effects/color_sources/dl_conical_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_conical_gradient_color_source.cc @@ -24,6 +24,24 @@ DlConicalGradientColorSource::DlConicalGradientColorSource( store_color_stops(this + 1, colors, stops); } +DlConicalGradientColorSource::DlConicalGradientColorSource( + DlPoint start_center, + DlScalar start_radius, + DlPoint end_center, + DlScalar end_radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + start_center_(start_center), + start_radius_(start_radius), + end_center_(end_center), + end_radius_(end_radius) { + store_color_stops(this + 1, colors_argb, stops); +} + DlConicalGradientColorSource::DlConicalGradientColorSource( const DlConicalGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_conical_gradient_color_source.h b/display_list/effects/color_sources/dl_conical_gradient_color_source.h index db523bc94c1c5..2fee5884892aa 100644 --- a/display_list/effects/color_sources/dl_conical_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_conical_gradient_color_source.h @@ -45,6 +45,16 @@ class DlConicalGradientColorSource final : public DlGradientColorSourceBase { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + DlConicalGradientColorSource(DlPoint start_center, + DlScalar start_radius, + DlPoint end_center, + DlScalar end_radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix = nullptr); + explicit DlConicalGradientColorSource( const DlConicalGradientColorSource* source); diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index ec6b80b9ed712..d937ed1b44666 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -139,6 +139,29 @@ std::shared_ptr DlColorSource::MakeConical( return ret; } +std::shared_ptr DlColorSource::MakeConical( + DlPoint start_center, + DlScalar start_radius, + DlPoint end_center, + DlScalar end_radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix) { + size_t needed = sizeof(DlConicalGradientColorSource) + + (stop_count * (sizeof(DlColor) + sizeof(float))); + + void* storage = ::operator new(needed); + + std::shared_ptr ret; + ret.reset(new (storage) DlConicalGradientColorSource( + start_center, start_radius, end_center, end_radius, stop_count, + colors_argb, stops, tile_mode, matrix), + DlGradientDeleter); + return ret; +} + std::shared_ptr DlColorSource::MakeSweep( DlPoint center, DlScalar start, diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index 6ff28d9ce30f0..e112f83efea90 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -103,6 +103,20 @@ class DlColorSource : public DlAttribute { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + /// @brief Make a conical gradient. + /// @param colors_argb colors_argb Array of DlScalars that represents colors + /// in the ARGB format. + static std::shared_ptr MakeConical( + DlPoint start_center, + DlScalar start_radius, + DlPoint end_center, + DlScalar end_radius, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix = nullptr); + static std::shared_ptr MakeSweep( DlPoint center, DlScalar start, diff --git a/display_list/effects/dl_color_source_unittests.cc b/display_list/effects/dl_color_source_unittests.cc index 3bb7e488e33b9..9400d9257405f 100644 --- a/display_list/effects/dl_color_source_unittests.cc +++ b/display_list/effects/dl_color_source_unittests.cc @@ -477,6 +477,34 @@ TEST(DisplayListColorSource, ConicalGradientConstructor) { kTestStops, DlTileMode::kClamp, &kTestMatrix1); } +TEST(DisplayListColorSource, ConicalGradientARGBConstructor) { + std::array colors; + for (int i = 0; i < kTestStopCount; ++i) { + colors[i * 4 + 0] = kTestColors[i].getAlphaF(); // + colors[i * 4 + 1] = kTestColors[i].getRedF(); // + colors[i * 4 + 2] = kTestColors[i].getGreenF(); // + colors[i * 4 + 3] = kTestColors[i].getBlueF(); + } + std::shared_ptr source = DlColorSource::MakeConical( + kTestPoints[0], 10.f, kTestPoints[1], 20.f, kTestStopCount, colors.data(), + kTestStops, DlTileMode::kClamp, &kTestMatrix1); + ASSERT_TRUE(source); + ASSERT_TRUE(source->asConicalGradient()); + EXPECT_EQ(source->asConicalGradient()->start_center(), kTestPoints[0]); + EXPECT_EQ(source->asConicalGradient()->start_radius(), 10.f); + EXPECT_EQ(source->asConicalGradient()->end_center(), kTestPoints[1]); + EXPECT_EQ(source->asConicalGradient()->end_radius(), 20.f); + EXPECT_EQ(source->asConicalGradient()->stop_count(), kTestStopCount); + for (int i = 0; i < kTestStopCount; i++) { + EXPECT_EQ(source->asConicalGradient()->colors()[i], + kTestColors[i].withColorSpace(DlColorSpace::kExtendedSRGB)); + EXPECT_EQ(source->asConicalGradient()->stops()[i], kTestStops[i]); + } + EXPECT_EQ(source->asConicalGradient()->tile_mode(), DlTileMode::kClamp); + EXPECT_EQ(source->asConicalGradient()->matrix(), kTestMatrix1); + EXPECT_EQ(source->is_opaque(), true); +} + TEST(DisplayListColorSource, ConicalGradientShared) { std::shared_ptr source = DlColorSource::MakeConical( kTestPoints[0], 10.0, kTestPoints[1], 20.0, kTestStopCount, kTestColors, diff --git a/lib/ui/painting/gradient.cc b/lib/ui/painting/gradient.cc index a5084110b2b49..7c0392590f561 100644 --- a/lib/ui/painting/gradient.cc +++ b/lib/ui/painting/gradient.cc @@ -134,20 +134,10 @@ void CanvasGradient::initTwoPointConical(double start_x, dl_matrix = ToDlMatrix(matrix4); } - std::vector dl_colors; - dl_colors.reserve(num_colors); - for (int i = 0; i < colors.num_elements(); i += 4) { - DlScalar a = colors[i + 0]; - DlScalar r = colors[i + 1]; - DlScalar g = colors[i + 2]; - DlScalar b = colors[i + 3]; - dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB)); - } - dl_shader_ = DlColorSource::MakeConical( DlPoint(SafeNarrow(start_x), SafeNarrow(start_y)), SafeNarrow(start_radius), DlPoint(SafeNarrow(end_x), SafeNarrow(end_y)), - SafeNarrow(end_radius), num_colors, dl_colors.data(), color_stops.data(), + SafeNarrow(end_radius), num_colors, colors.data(), color_stops.data(), tile_mode, has_matrix ? &dl_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); From 238ca2ee26082831211c242230f4e99a4782679a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 10:41:33 -0800 Subject: [PATCH 3/8] added sweep --- .../dl_sweep_gradient_color_source.cc | 16 +++++++++++ .../dl_sweep_gradient_color_source.h | 9 +++++++ display_list/effects/dl_color_source.cc | 22 +++++++++++++++ display_list/effects/dl_color_source.h | 10 +++++++ .../effects/dl_color_source_unittests.cc | 27 +++++++++++++++++++ lib/ui/painting/gradient.cc | 12 +-------- 6 files changed, 85 insertions(+), 11 deletions(-) diff --git a/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc b/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc index c575e72947a31..7a1a9c92c3f0d 100644 --- a/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc @@ -21,6 +21,22 @@ DlSweepGradientColorSource::DlSweepGradientColorSource(DlPoint center, store_color_stops(this + 1, colors, stops); } +DlSweepGradientColorSource::DlSweepGradientColorSource( + DlPoint center, + DlScalar start, + DlScalar end, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + center_(center), + start_(start), + end_(end) { + store_color_stops(this + 1, colors_argb, stops); +} + DlSweepGradientColorSource::DlSweepGradientColorSource( const DlSweepGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_sweep_gradient_color_source.h b/display_list/effects/color_sources/dl_sweep_gradient_color_source.h index b33753898ce31..0f123169fd73d 100644 --- a/display_list/effects/color_sources/dl_sweep_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_sweep_gradient_color_source.h @@ -43,6 +43,15 @@ class DlSweepGradientColorSource final : public DlGradientColorSourceBase { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + DlSweepGradientColorSource(DlPoint center, + DlScalar start, + DlScalar end, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix = nullptr); + explicit DlSweepGradientColorSource(const DlSweepGradientColorSource* source); DlPoint center_; diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index d937ed1b44666..2a339736abe0e 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -184,6 +184,28 @@ std::shared_ptr DlColorSource::MakeSweep( return ret; } +std::shared_ptr DlColorSource::MakeSweep( + DlPoint center, + DlScalar start, + DlScalar end, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix) { + size_t needed = sizeof(DlSweepGradientColorSource) + + (stop_count * (sizeof(DlColor) + sizeof(float))); + + void* storage = ::operator new(needed); + + std::shared_ptr ret; + ret.reset(new (storage) DlSweepGradientColorSource(center, start, end, + stop_count, colors_argb, + stops, tile_mode, matrix), + DlGradientDeleter); + return ret; +} + std::shared_ptr DlColorSource::MakeRuntimeEffect( sk_sp runtime_effect, std::vector> samplers, diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index e112f83efea90..d5f5acfd4881d 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -127,6 +127,16 @@ class DlColorSource : public DlAttribute { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + static std::shared_ptr MakeSweep( + DlPoint center, + DlScalar start, + DlScalar end, + uint32_t stop_count, + const DlScalar* colors_argb, + const float* stops, + DlTileMode tile_mode, + const DlMatrix* matrix = nullptr); + static std::shared_ptr MakeRuntimeEffect( sk_sp runtime_effect, std::vector> samplers, diff --git a/display_list/effects/dl_color_source_unittests.cc b/display_list/effects/dl_color_source_unittests.cc index 9400d9257405f..7be84896e49e3 100644 --- a/display_list/effects/dl_color_source_unittests.cc +++ b/display_list/effects/dl_color_source_unittests.cc @@ -639,6 +639,33 @@ TEST(DisplayListColorSource, SweepGradientConstructor) { DlTileMode::kClamp, &kTestMatrix1); } +TEST(DisplayListColorSource, SweepGradientARGBConstructor) { + std::array colors; + for (int i = 0; i < kTestStopCount; ++i) { + colors[i * 4 + 0] = kTestColors[i].getAlphaF(); // + colors[i * 4 + 1] = kTestColors[i].getRedF(); // + colors[i * 4 + 2] = kTestColors[i].getGreenF(); // + colors[i * 4 + 3] = kTestColors[i].getBlueF(); + } + std::shared_ptr source = DlColorSource::MakeSweep( + kTestPoints[0], 10.f, 20.f, kTestStopCount, kTestColors, kTestStops, + DlTileMode::kClamp, &kTestMatrix1); + ASSERT_TRUE(source); + ASSERT_TRUE(source->asSweepGradient()); + EXPECT_EQ(source->asSweepGradient()->center(), kTestPoints[0]); + EXPECT_EQ(source->asSweepGradient()->start(), 10.f); + EXPECT_EQ(source->asSweepGradient()->end(), 20.f); + EXPECT_EQ(source->asSweepGradient()->stop_count(), kTestStopCount); + for (int i = 0; i < kTestStopCount; i++) { + EXPECT_EQ(source->asConicalGradient()->colors()[i], + kTestColors[i].withColorSpace(DlColorSpace::kExtendedSRGB)); + EXPECT_EQ(source->asConicalGradient()->stops()[i], kTestStops[i]); + } + EXPECT_EQ(source->asSweepGradient()->tile_mode(), DlTileMode::kClamp); + EXPECT_EQ(source->asSweepGradient()->matrix(), kTestMatrix1); + EXPECT_EQ(source->is_opaque(), true); +} + TEST(DisplayListColorSource, SweepGradientShared) { std::shared_ptr source = DlColorSource::MakeSweep( kTestPoints[0], 10.0, 20.0, kTestStopCount, kTestColors, kTestStops, diff --git a/lib/ui/painting/gradient.cc b/lib/ui/painting/gradient.cc index 7c0392590f561..a4d783910afad 100644 --- a/lib/ui/painting/gradient.cc +++ b/lib/ui/painting/gradient.cc @@ -94,21 +94,11 @@ void CanvasGradient::initSweep(double center_x, dl_matrix = ToDlMatrix(matrix4); } - std::vector dl_colors; - dl_colors.reserve(num_colors); - for (int i = 0; i < colors.num_elements(); i += 4) { - DlScalar a = colors[i + 0]; - DlScalar r = colors[i + 1]; - DlScalar g = colors[i + 2]; - DlScalar b = colors[i + 3]; - dl_colors.emplace_back(DlColor(a, r, g, b, DlColorSpace::kExtendedSRGB)); - } - dl_shader_ = DlColorSource::MakeSweep( DlPoint(SafeNarrow(center_x), SafeNarrow(center_y)), SafeNarrow(start_angle) * 180.0f / static_cast(M_PI), SafeNarrow(end_angle) * 180.0f / static_cast(M_PI), num_colors, - dl_colors.data(), color_stops.data(), tile_mode, + colors.data(), color_stops.data(), tile_mode, has_matrix ? &dl_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); From df78f6a2090e20d8416725af13463f1a36be3fae Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 10:42:35 -0800 Subject: [PATCH 4/8] updated docstrings --- display_list/effects/dl_color_source.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index d5f5acfd4881d..4f3284c848ab2 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -61,7 +61,7 @@ class DlColorSource : public DlAttribute { /// @brief Make a linear gradient. /// @param colors_argb Array of DlScalars that represents colors in the ARGB - /// format. + /// format, in the extended srgb colorspace. static std::shared_ptr MakeLinear( const DlPoint start_point, const DlPoint end_point, @@ -82,7 +82,7 @@ class DlColorSource : public DlAttribute { /// @brief Make a radial gradient. /// @param colors_argb Array of DlScalars that represents colors in the ARGB - /// format. + /// format, in the extended srgb colorspace. static std::shared_ptr MakeRadial( DlPoint center, DlScalar radius, @@ -105,7 +105,7 @@ class DlColorSource : public DlAttribute { /// @brief Make a conical gradient. /// @param colors_argb colors_argb Array of DlScalars that represents colors - /// in the ARGB format. + /// in the ARGB format, in the extended srgb colorspace. static std::shared_ptr MakeConical( DlPoint start_center, DlScalar start_radius, @@ -127,6 +127,9 @@ class DlColorSource : public DlAttribute { DlTileMode tile_mode, const DlMatrix* matrix = nullptr); + /// @brief Make a sweep gradient. + /// @param colors_argb Array of DlScalars that represents colors in the ARGB + /// format, in the extended srgb colorspace. static std::shared_ptr MakeSweep( DlPoint center, DlScalar start, From 1277473525f4641e1f8ccffbad953787afd39e45 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 10:51:25 -0800 Subject: [PATCH 5/8] removed more duplication --- display_list/effects/dl_color_source.cc | 40 ++++++++++++------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index 2a339736abe0e..3d431309b14d6 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -31,8 +31,9 @@ std::shared_ptr DlColorSource::MakeImage( } namespace { -size_t CalculateLinearGradientSize(uint32_t stop_count) { - return sizeof(DlLinearGradientColorSource) + +template +size_t CalculateGradientSize(uint32_t stop_count) { + return sizeof(GradientColorSource) + (stop_count * (sizeof(DlColor) + sizeof(float))); } } // namespace @@ -45,7 +46,8 @@ std::shared_ptr DlColorSource::MakeLinear( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = CalculateLinearGradientSize(stop_count); + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -64,7 +66,8 @@ std::shared_ptr DlColorSource::MakeLinear( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = CalculateLinearGradientSize(stop_count); + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -83,9 +86,8 @@ std::shared_ptr DlColorSource::MakeRadial( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = sizeof(DlRadialGradientColorSource) + - (stop_count * (sizeof(DlColor) + sizeof(float))); - + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -103,9 +105,8 @@ std::shared_ptr DlColorSource::MakeRadial( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = sizeof(DlRadialGradientColorSource) + - (stop_count * (sizeof(DlColor) + sizeof(float))); - + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -126,9 +127,8 @@ std::shared_ptr DlColorSource::MakeConical( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = sizeof(DlConicalGradientColorSource) + - (stop_count * (sizeof(DlColor) + sizeof(float))); - + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -149,8 +149,8 @@ std::shared_ptr DlColorSource::MakeConical( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = sizeof(DlConicalGradientColorSource) + - (stop_count * (sizeof(DlColor) + sizeof(float))); + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); @@ -171,9 +171,8 @@ std::shared_ptr DlColorSource::MakeSweep( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = sizeof(DlSweepGradientColorSource) + - (stop_count * (sizeof(DlColor) + sizeof(float))); - + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -193,9 +192,8 @@ std::shared_ptr DlColorSource::MakeSweep( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = sizeof(DlSweepGradientColorSource) + - (stop_count * (sizeof(DlColor) + sizeof(float))); - + size_t needed = + CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; From ea32f3374ef409524573c50e0a464ee401cfb084 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 14:14:42 -0800 Subject: [PATCH 6/8] fixed typo in test --- display_list/effects/dl_color_source.cc | 6 ++---- display_list/effects/dl_color_source_unittests.cc | 6 +++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index 3d431309b14d6..3a0fd78c15bcb 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -171,8 +171,7 @@ std::shared_ptr DlColorSource::MakeSweep( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = - CalculateGradientSize(stop_count); + size_t needed = CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; @@ -192,8 +191,7 @@ std::shared_ptr DlColorSource::MakeSweep( const float* stops, DlTileMode tile_mode, const DlMatrix* matrix) { - size_t needed = - CalculateGradientSize(stop_count); + size_t needed = CalculateGradientSize(stop_count); void* storage = ::operator new(needed); std::shared_ptr ret; diff --git a/display_list/effects/dl_color_source_unittests.cc b/display_list/effects/dl_color_source_unittests.cc index 7be84896e49e3..4fd44469b1eaf 100644 --- a/display_list/effects/dl_color_source_unittests.cc +++ b/display_list/effects/dl_color_source_unittests.cc @@ -648,7 +648,7 @@ TEST(DisplayListColorSource, SweepGradientARGBConstructor) { colors[i * 4 + 3] = kTestColors[i].getBlueF(); } std::shared_ptr source = DlColorSource::MakeSweep( - kTestPoints[0], 10.f, 20.f, kTestStopCount, kTestColors, kTestStops, + kTestPoints[0], 10.f, 20.f, kTestStopCount, colors.data(), kTestStops, DlTileMode::kClamp, &kTestMatrix1); ASSERT_TRUE(source); ASSERT_TRUE(source->asSweepGradient()); @@ -657,9 +657,9 @@ TEST(DisplayListColorSource, SweepGradientARGBConstructor) { EXPECT_EQ(source->asSweepGradient()->end(), 20.f); EXPECT_EQ(source->asSweepGradient()->stop_count(), kTestStopCount); for (int i = 0; i < kTestStopCount; i++) { - EXPECT_EQ(source->asConicalGradient()->colors()[i], + EXPECT_EQ(source->asSweepGradient()->colors()[i], kTestColors[i].withColorSpace(DlColorSpace::kExtendedSRGB)); - EXPECT_EQ(source->asConicalGradient()->stops()[i], kTestStops[i]); + EXPECT_EQ(source->asSweepGradient()->stops()[i], kTestStops[i]); } EXPECT_EQ(source->asSweepGradient()->tile_mode(), DlTileMode::kClamp); EXPECT_EQ(source->asSweepGradient()->matrix(), kTestMatrix1); From 9a7613f4cf62319ff7914ef6033a9c54dffff71d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 14:22:43 -0800 Subject: [PATCH 7/8] removed constructor redundancy --- .../dl_conical_gradient_color_source.cc | 36 ------------------- .../dl_conical_gradient_color_source.h | 22 ++++++------ .../dl_linear_gradient_color_source.cc | 28 --------------- .../dl_linear_gradient_color_source.h | 18 +++++----- .../dl_radial_gradient_color_source.cc | 27 -------------- .../dl_radial_gradient_color_source.h | 18 +++++----- .../dl_sweep_gradient_color_source.cc | 31 ---------------- .../dl_sweep_gradient_color_source.h | 20 +++++------ display_list/effects/dl_color_source.cc | 13 +++++++ 9 files changed, 48 insertions(+), 165 deletions(-) diff --git a/display_list/effects/color_sources/dl_conical_gradient_color_source.cc b/display_list/effects/color_sources/dl_conical_gradient_color_source.cc index dbd93d1cf7975..2b930398c9565 100644 --- a/display_list/effects/color_sources/dl_conical_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_conical_gradient_color_source.cc @@ -6,42 +6,6 @@ namespace flutter { -DlConicalGradientColorSource::DlConicalGradientColorSource( - DlPoint start_center, - DlScalar start_radius, - DlPoint end_center, - DlScalar end_radius, - uint32_t stop_count, - const DlColor* colors, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - start_center_(start_center), - start_radius_(start_radius), - end_center_(end_center), - end_radius_(end_radius) { - store_color_stops(this + 1, colors, stops); -} - -DlConicalGradientColorSource::DlConicalGradientColorSource( - DlPoint start_center, - DlScalar start_radius, - DlPoint end_center, - DlScalar end_radius, - uint32_t stop_count, - const DlScalar* colors_argb, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - start_center_(start_center), - start_radius_(start_radius), - end_center_(end_center), - end_radius_(end_radius) { - store_color_stops(this + 1, colors_argb, stops); -} - DlConicalGradientColorSource::DlConicalGradientColorSource( const DlConicalGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_conical_gradient_color_source.h b/display_list/effects/color_sources/dl_conical_gradient_color_source.h index 2fee5884892aa..3efb60e698fef 100644 --- a/display_list/effects/color_sources/dl_conical_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_conical_gradient_color_source.h @@ -35,25 +35,23 @@ class DlConicalGradientColorSource final : public DlGradientColorSourceBase { bool equals_(DlColorSource const& other) const override; private: + template DlConicalGradientColorSource(DlPoint start_center, DlScalar start_radius, DlPoint end_center, DlScalar end_radius, uint32_t stop_count, - const DlColor* colors, + Colors colors, const float* stops, DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); - - DlConicalGradientColorSource(DlPoint start_center, - DlScalar start_radius, - DlPoint end_center, - DlScalar end_radius, - uint32_t stop_count, - const DlScalar* colors_argb, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); + const DlMatrix* matrix = nullptr) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + start_center_(start_center), + start_radius_(start_radius), + end_center_(end_center), + end_radius_(end_radius) { + store_color_stops(this + 1, colors, stops); + } explicit DlConicalGradientColorSource( const DlConicalGradientColorSource* source); diff --git a/display_list/effects/color_sources/dl_linear_gradient_color_source.cc b/display_list/effects/color_sources/dl_linear_gradient_color_source.cc index 6c9fa3b19a1ea..953cc82d84f04 100644 --- a/display_list/effects/color_sources/dl_linear_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_linear_gradient_color_source.cc @@ -6,34 +6,6 @@ namespace flutter { -DlLinearGradientColorSource::DlLinearGradientColorSource( - const DlPoint start_point, - const DlPoint end_point, - uint32_t stop_count, - const DlColor* colors, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - start_point_(start_point), - end_point_(end_point) { - store_color_stops(this + 1, colors, stops); -} - -DlLinearGradientColorSource::DlLinearGradientColorSource( - const DlPoint start_point, - const DlPoint end_point, - uint32_t stop_count, - const DlScalar* colors, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - start_point_(start_point), - end_point_(end_point) { - store_color_stops(this + 1, colors, stops); -} - DlLinearGradientColorSource::DlLinearGradientColorSource( const DlLinearGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_linear_gradient_color_source.h b/display_list/effects/color_sources/dl_linear_gradient_color_source.h index a229fbbd233c6..18ca74afe61f7 100644 --- a/display_list/effects/color_sources/dl_linear_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_linear_gradient_color_source.h @@ -33,21 +33,19 @@ class DlLinearGradientColorSource final : public DlGradientColorSourceBase { bool equals_(DlColorSource const& other) const override; private: + template DlLinearGradientColorSource(const DlPoint start_point, const DlPoint end_point, uint32_t stop_count, - const DlColor* colors, + Colors colors, const float* stops, DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); - - DlLinearGradientColorSource(const DlPoint start_point, - const DlPoint end_point, - uint32_t stop_count, - const DlScalar* colors, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); + const DlMatrix* matrix = nullptr) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + start_point_(start_point), + end_point_(end_point) { + store_color_stops(this + 1, colors, stops); + } explicit DlLinearGradientColorSource( const DlLinearGradientColorSource* source); diff --git a/display_list/effects/color_sources/dl_radial_gradient_color_source.cc b/display_list/effects/color_sources/dl_radial_gradient_color_source.cc index 19ba780bffb78..94b526a3cf60d 100644 --- a/display_list/effects/color_sources/dl_radial_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_radial_gradient_color_source.cc @@ -6,33 +6,6 @@ namespace flutter { -DlRadialGradientColorSource::DlRadialGradientColorSource(DlPoint center, - DlScalar radius, - uint32_t stop_count, - const DlColor* colors, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - center_(center), - radius_(radius) { - store_color_stops(this + 1, colors, stops); -} - -DlRadialGradientColorSource::DlRadialGradientColorSource( - DlPoint center, - DlScalar radius, - uint32_t stop_count, - const DlScalar* colors_argb, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - center_(center), - radius_(radius) { - store_color_stops(this + 1, colors_argb, stops); -} - DlRadialGradientColorSource::DlRadialGradientColorSource( const DlRadialGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_radial_gradient_color_source.h b/display_list/effects/color_sources/dl_radial_gradient_color_source.h index d1b2baecd7552..fe05c334c9e73 100644 --- a/display_list/effects/color_sources/dl_radial_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_radial_gradient_color_source.h @@ -33,21 +33,19 @@ class DlRadialGradientColorSource final : public DlGradientColorSourceBase { bool equals_(DlColorSource const& other) const override; private: + template DlRadialGradientColorSource(DlPoint center, DlScalar radius, uint32_t stop_count, - const DlColor* colors, + Colors colors, const float* stops, DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); - - DlRadialGradientColorSource(DlPoint center, - DlScalar radius, - uint32_t stop_count, - const DlScalar* colors_argb, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); + const DlMatrix* matrix = nullptr) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + center_(center), + radius_(radius) { + store_color_stops(this + 1, colors, stops); + } explicit DlRadialGradientColorSource( const DlRadialGradientColorSource* source); diff --git a/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc b/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc index 7a1a9c92c3f0d..4af8e04ba4e3f 100644 --- a/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc +++ b/display_list/effects/color_sources/dl_sweep_gradient_color_source.cc @@ -6,37 +6,6 @@ namespace flutter { -DlSweepGradientColorSource::DlSweepGradientColorSource(DlPoint center, - DlScalar start, - DlScalar end, - uint32_t stop_count, - const DlColor* colors, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - center_(center), - start_(start), - end_(end) { - store_color_stops(this + 1, colors, stops); -} - -DlSweepGradientColorSource::DlSweepGradientColorSource( - DlPoint center, - DlScalar start, - DlScalar end, - uint32_t stop_count, - const DlScalar* colors_argb, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix) - : DlGradientColorSourceBase(stop_count, tile_mode, matrix), - center_(center), - start_(start), - end_(end) { - store_color_stops(this + 1, colors_argb, stops); -} - DlSweepGradientColorSource::DlSweepGradientColorSource( const DlSweepGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), diff --git a/display_list/effects/color_sources/dl_sweep_gradient_color_source.h b/display_list/effects/color_sources/dl_sweep_gradient_color_source.h index 0f123169fd73d..4a01048deae15 100644 --- a/display_list/effects/color_sources/dl_sweep_gradient_color_source.h +++ b/display_list/effects/color_sources/dl_sweep_gradient_color_source.h @@ -34,23 +34,21 @@ class DlSweepGradientColorSource final : public DlGradientColorSourceBase { bool equals_(DlColorSource const& other) const override; private: + template DlSweepGradientColorSource(DlPoint center, DlScalar start, DlScalar end, uint32_t stop_count, - const DlColor* colors, + Colors colors, const float* stops, DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); - - DlSweepGradientColorSource(DlPoint center, - DlScalar start, - DlScalar end, - uint32_t stop_count, - const DlScalar* colors_argb, - const float* stops, - DlTileMode tile_mode, - const DlMatrix* matrix = nullptr); + const DlMatrix* matrix = nullptr) + : DlGradientColorSourceBase(stop_count, tile_mode, matrix), + center_(center), + start_(start), + end_(end) { + store_color_stops(this + 1, colors, stops); + } explicit DlSweepGradientColorSource(const DlSweepGradientColorSource* source); diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index 3a0fd78c15bcb..27be8f74dfa9b 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -36,6 +36,19 @@ size_t CalculateGradientSize(uint32_t stop_count) { return sizeof(GradientColorSource) + (stop_count * (sizeof(DlColor) + sizeof(float))); } + +template +std::shared_ptr MakeGradient(uint32_t stop_count, + Args&&... args) { + size_t needed = CalculateGradientSize(stop_count); + void* storage = ::operator new(needed); + + std::shared_ptr ret; + ret.reset(new (storage) GradientType(std::forward(args)...), + DlGradientDeleter); + + return ret; +} } // namespace std::shared_ptr DlColorSource::MakeLinear( From f36be66a0b785d253baed1530b928d18e74c6ff4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Dec 2024 14:39:10 -0800 Subject: [PATCH 8/8] oops, remove unused function --- display_list/effects/dl_color_source.cc | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index 27be8f74dfa9b..3a0fd78c15bcb 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -36,19 +36,6 @@ size_t CalculateGradientSize(uint32_t stop_count) { return sizeof(GradientColorSource) + (stop_count * (sizeof(DlColor) + sizeof(float))); } - -template -std::shared_ptr MakeGradient(uint32_t stop_count, - Args&&... args) { - size_t needed = CalculateGradientSize(stop_count); - void* storage = ::operator new(needed); - - std::shared_ptr ret; - ret.reset(new (storage) GradientType(std::forward(args)...), - DlGradientDeleter); - - return ret; -} } // namespace std::shared_ptr DlColorSource::MakeLinear(