From 1071a2bef7a7f727b5bad4e084862b45f844a517 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 10:55:59 -0700 Subject: [PATCH 01/35] Adds DlColor wide gamut colors support. --- display_list/dl_builder.h | 2 +- display_list/dl_color.h | 98 ++++++++++++++++++-------- display_list/effects/dl_color_source.h | 9 ++- 3 files changed, 75 insertions(+), 34 deletions(-) diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h index 99e46143f32e6..29cc795aeb980 100644 --- a/display_list/dl_builder.h +++ b/display_list/dl_builder.h @@ -805,7 +805,7 @@ class DisplayListBuilder final : public virtual DlCanvas, // kAnyColor is a non-opaque and non-transparent color that will not // trigger any short-circuit tests about the results of a blend. - static constexpr DlColor kAnyColor = DlColor::kMidGrey().withAlpha(0x80); + static constexpr DlColor kAnyColor = DlColor::kMidGrey().withAlphaF(0.5f); static_assert(!kAnyColor.isOpaque()); static_assert(!kAnyColor.isTransparent()); static DlColor GetEffectiveColor(const DlPaint& paint, diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 9a823f813894b..3bc46028f6f03 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -9,10 +9,32 @@ namespace flutter { +enum class DlColorSpace { kSRGB = 0, kExtendedSRGB = 1, kDisplayP3 = 2 }; + struct DlColor { public: - constexpr DlColor() : argb_(0xFF000000) {} - constexpr explicit DlColor(uint32_t argb) : argb_(argb) {} + constexpr DlColor() + : alpha_(0.f), + red_(0.f), + green_(0.f), + blue_(0.f), + color_space_(DlColorSpace::kSRGB) {} + constexpr explicit DlColor(uint32_t argb) + : alpha_((argb >> 24) / 255.f), + red_((argb >> 16) / 255.f), + green_((argb >> 8) / 255.f), + blue_((argb >> 0) / 255.f), + color_space_(DlColorSpace::kSRGB) {} + constexpr explicit DlColor(float alpha, + float red, + float green, + float blue, + DlColorSpace colorspace) + : alpha_(alpha), + red_(red), + green_(green), + blue_(blue), + color_space_(colorspace) {} /// @brief Construct a 32 bit color from floating point R, G, B, and A color /// channels. @@ -29,10 +51,7 @@ struct DlColor { DlScalar r, DlScalar g, DlScalar b) { - return DlColor(toC(a) << 24 | // - toC(r) << 16 | // - toC(g) << 8 | // - toC(b)); + return DlColor(a, r, g, b, DlColorSpace::kSRGB); } static constexpr uint8_t toAlpha(DlScalar opacity) { return toC(opacity); } @@ -58,41 +77,55 @@ struct DlColor { static constexpr DlColor kCornflowerBlue() {return DlColor(0xFF6495ED);}; // clang-format on - constexpr bool isOpaque() const { return getAlpha() == 0xFF; } - constexpr bool isTransparent() const { return getAlpha() == 0; } + constexpr bool isOpaque() const { return alpha_ >= 1.f; } + constexpr bool isTransparent() const { return alpha_ <= 0.f; } + + constexpr int getAlpha() const { return toC(alpha_); } + constexpr int getRed() const { return toC(red_); } + constexpr int getGreen() const { return toC(green_); } + constexpr int getBlue() const { return toC(blue_); } - constexpr int getAlpha() const { return argb_ >> 24; } - constexpr int getRed() const { return (argb_ >> 16) & 0xFF; } - constexpr int getGreen() const { return (argb_ >> 8) & 0xFF; } - constexpr int getBlue() const { return argb_ & 0xFF; } + constexpr DlScalar getAlphaF() const { return alpha_; } + constexpr DlScalar getRedF() const { return red_; } + constexpr DlScalar getGreenF() const { return green_; } + constexpr DlScalar getBlueF() const { return blue_; } - constexpr DlScalar getAlphaF() const { return toF(getAlpha()); } - constexpr DlScalar getRedF() const { return toF(getRed()); } - constexpr DlScalar getGreenF() const { return toF(getGreen()); } - constexpr DlScalar getBlueF() const { return toF(getBlue()); } + constexpr DlColorSpace getColorSpace() const { return color_space_; } constexpr uint32_t premultipliedArgb() const { if (isOpaque()) { - return argb_; + return argb(); } DlScalar f = getAlphaF(); - return (argb_ & 0xFF000000) | // + return (argb() & 0xFF000000) | // toC(getRedF() * f) << 16 | // toC(getGreenF() * f) << 8 | // toC(getBlueF() * f); } constexpr DlColor withAlpha(uint8_t alpha) const { // - return DlColor((argb_ & 0x00FFFFFF) | (alpha << 24)); + return DlColor((argb() & 0x00FFFFFF) | (alpha << 24)); } constexpr DlColor withRed(uint8_t red) const { // - return DlColor((argb_ & 0xFF00FFFF) | (red << 16)); + return DlColor((argb() & 0xFF00FFFF) | (red << 16)); } constexpr DlColor withGreen(uint8_t green) const { // - return DlColor((argb_ & 0xFFFF00FF) | (green << 8)); + return DlColor((argb() & 0xFFFF00FF) | (green << 8)); } constexpr DlColor withBlue(uint8_t blue) const { // - return DlColor((argb_ & 0xFFFFFF00) | (blue << 0)); + return DlColor((argb() & 0xFFFFFF00) | (blue << 0)); + } + constexpr DlColor withAlphaF(float alpha) const { // + return DlColor(alpha, red_, green_, blue_, color_space_); + } + constexpr DlColor withRedF(float red) const { // + return DlColor(alpha_, red, green_, blue_, color_space_); + } + constexpr DlColor withGreenF(float green) const { // + return DlColor(alpha_, red_, green, blue_, color_space_); + } + constexpr DlColor withBlueF(float blue) const { // + return DlColor(alpha_, red_, green_, blue, color_space_); } constexpr DlColor modulateOpacity(DlScalar opacity) const { @@ -101,15 +134,24 @@ struct DlColor { : withAlpha(round(getAlpha() * opacity)); } - constexpr uint32_t argb() const { return argb_; } + constexpr uint32_t argb() const { + return toC(alpha_) << 24 | // + toC(red_) << 16 | // + toC(green_) << 8 | // + toC(blue_) << 0; + } - bool operator==(DlColor const& other) const { return argb_ == other.argb_; } - bool operator!=(DlColor const& other) const { return argb_ != other.argb_; } - bool operator==(uint32_t const& other) const { return argb_ == other; } - bool operator!=(uint32_t const& other) const { return argb_ != other; } + bool operator==(DlColor const& other) const { return argb() == other.argb(); } + bool operator!=(DlColor const& other) const { return argb() != other.argb(); } + bool operator==(uint32_t const& other) const { return argb() == other; } + bool operator!=(uint32_t const& other) const { return argb() != other; } private: - uint32_t argb_; + float alpha_; + float red_; + float green_; + float blue_; + DlColorSpace color_space_; static constexpr DlScalar toF(uint8_t comp) { return comp * (1.0f / 255); } static constexpr uint8_t toC(DlScalar fComp) { return round(fComp * 255); } diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index 39359d7996a06..b40b8804c4e69 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -327,11 +327,10 @@ class DlGradientColorSourceBase : public DlMatrixColorSourceBase { stop_count_ != other_base->stop_count_) { return false; } - static_assert(sizeof(colors()[0]) == 4); - static_assert(sizeof(stops()[0]) == 4); - int num_bytes = stop_count_ * 4; - return (memcmp(colors(), other_base->colors(), num_bytes) == 0 && - memcmp(stops(), other_base->stops(), num_bytes) == 0); + return (memcmp(colors(), other_base->colors(), + stop_count_ * sizeof(colors()[0])) == 0 && + memcmp(stops(), other_base->stops(), + stop_count_ * sizeof(stops()[0])) == 0); } void store_color_stops(void* pod, From 624e4c68ebf91cbba9f5443441953c48d06ddeb5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 11:27:42 -0700 Subject: [PATCH 02/35] fixed translation issue --- display_list/dl_color.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 3bc46028f6f03..a2327201e027d 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -20,10 +20,10 @@ struct DlColor { blue_(0.f), color_space_(DlColorSpace::kSRGB) {} constexpr explicit DlColor(uint32_t argb) - : alpha_((argb >> 24) / 255.f), - red_((argb >> 16) / 255.f), - green_((argb >> 8) / 255.f), - blue_((argb >> 0) / 255.f), + : alpha_(((argb >> 24) & 0xff) / 255.f), + red_(((argb >> 16) & 0xff) / 255.f), + green_(((argb >> 8) & 0xff) / 255.f), + blue_(((argb >> 0) & 0xff) / 255.f), color_space_(DlColorSpace::kSRGB) {} constexpr explicit DlColor(float alpha, float red, From 190b22a9728cfc835183c67282fa6bf060ad5257 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 15:30:52 -0700 Subject: [PATCH 03/35] fixed memory crashes in the dl tests --- display_list/effects/dl_color_source.cc | 8 ++++---- impeller/display_list/dl_dispatcher.cc | 14 ++------------ impeller/display_list/skia_conversions.cc | 1 + 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/display_list/effects/dl_color_source.cc b/display_list/effects/dl_color_source.cc index 5b1744f22f212..47a30d544a803 100644 --- a/display_list/effects/dl_color_source.cc +++ b/display_list/effects/dl_color_source.cc @@ -28,7 +28,7 @@ std::shared_ptr DlColorSource::MakeLinear( DlTileMode tile_mode, const SkMatrix* matrix) { size_t needed = sizeof(DlLinearGradientColorSource) + - (stop_count * (sizeof(uint32_t) + sizeof(float))); + (stop_count * (sizeof(DlColor) + sizeof(float))); void* storage = ::operator new(needed); @@ -49,7 +49,7 @@ std::shared_ptr DlColorSource::MakeRadial( DlTileMode tile_mode, const SkMatrix* matrix) { size_t needed = sizeof(DlRadialGradientColorSource) + - (stop_count * (sizeof(uint32_t) + sizeof(float))); + (stop_count * (sizeof(DlColor) + sizeof(float))); void* storage = ::operator new(needed); @@ -71,7 +71,7 @@ std::shared_ptr DlColorSource::MakeConical( DlTileMode tile_mode, const SkMatrix* matrix) { size_t needed = sizeof(DlConicalGradientColorSource) + - (stop_count * (sizeof(uint32_t) + sizeof(float))); + (stop_count * (sizeof(DlColor) + sizeof(float))); void* storage = ::operator new(needed); @@ -93,7 +93,7 @@ std::shared_ptr DlColorSource::MakeSweep( DlTileMode tile_mode, const SkMatrix* matrix) { size_t needed = sizeof(DlSweepGradientColorSource) + - (stop_count * (sizeof(uint32_t) + sizeof(float))); + (stop_count * (sizeof(DlColor) + sizeof(float))); void* storage = ::operator new(needed); diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 3ecd5ef2dcaba..f06b19c98912e 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -194,12 +194,7 @@ void DlDispatcherBase::setDrawStyle(flutter::DlDrawStyle style) { // |flutter::DlOpReceiver| void DlDispatcherBase::setColor(flutter::DlColor color) { - paint_.color = { - color.getRedF(), - color.getGreenF(), - color.getBlueF(), - color.getAlphaF(), - }; + paint_.color = skia_conversions::ToColor(color); } // |flutter::DlOpReceiver| @@ -1352,12 +1347,7 @@ void TextFrameDispatcher::setDrawStyle(flutter::DlDrawStyle style) { // |flutter::DlOpReceiver| void TextFrameDispatcher::setColor(flutter::DlColor color) { - paint_.color = { - color.getRedF(), - color.getGreenF(), - color.getBlueF(), - color.getAlphaF(), - }; + paint_.color = skia_conversions::ToColor(color); } // |flutter::DlOpReceiver| diff --git a/impeller/display_list/skia_conversions.cc b/impeller/display_list/skia_conversions.cc index f53f4b1bb7100..bef03963902c2 100644 --- a/impeller/display_list/skia_conversions.cc +++ b/impeller/display_list/skia_conversions.cc @@ -164,6 +164,7 @@ Size ToSize(const SkPoint& point) { } Color ToColor(const flutter::DlColor& color) { + FML_DCHECK(color.getColorSpace() == flutter::DlColorSpace::kExtendedSRGB); return { static_cast(color.getRedF()), // static_cast(color.getGreenF()), // From bb6f2ccac7e27bd14fa8a6f9fcebb6b426eef0fd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 16:03:43 -0700 Subject: [PATCH 04/35] fixed some test failures --- display_list/testing/dl_test_snippets.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index 5dc341cb703d9..e0b1c9a6f4b62 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -114,9 +114,9 @@ std::vector CreateAllAttributesOps() { }}, {"SetColor", { - {0, 8, 0, + {0, 4 + sizeof(DlColor), 0, [](DlOpReceiver& r) { r.setColor(DlColor(SK_ColorGREEN)); }}, - {0, 8, 0, + {0, 4 + sizeof(DlColor), 0, [](DlOpReceiver& r) { r.setColor(DlColor(SK_ColorBLUE)); }}, // Reset attribute to default as last entry From 6dfc01aae83039cf1bf5829e8dd9e6a0e5677de2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 16:19:11 -0700 Subject: [PATCH 05/35] updated more tests --- display_list/testing/dl_test_snippets.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index e0b1c9a6f4b62..846bdfc5ad175 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -137,14 +137,13 @@ std::vector CreateAllAttributesOps() { {"SetColorSource", { {0, 96, 0, [](DlOpReceiver& r) { r.setColorSource(&kTestSource1); }}, - // stop_count * (sizeof(float) + sizeof(uint32_t)) = 80 - {0, 80 + 6 * 4, 0, + {0, 152, 0, [](DlOpReceiver& r) { r.setColorSource(kTestSource2.get()); }}, - {0, 80 + 6 * 4, 0, + {0, 152, 0, [](DlOpReceiver& r) { r.setColorSource(kTestSource3.get()); }}, - {0, 88 + 6 * 4, 0, + {0, 160, 0, [](DlOpReceiver& r) { r.setColorSource(kTestSource4.get()); }}, - {0, 80 + 6 * 4, 0, + {0, 152, 0, [](DlOpReceiver& r) { r.setColorSource(kTestSource5.get()); }}, // Reset attribute to default as last entry From 3802bf3d49ab2b09ce91c13df49e1d03a9642389 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 16:51:39 -0700 Subject: [PATCH 06/35] updated more literals --- display_list/testing/dl_test_snippets.cc | 28 ++++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index 846bdfc5ad175..9dfadf47a71e5 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -218,11 +218,11 @@ std::vector CreateAllAttributesOps() { }}, {"SetColorFilter", { - {0, 24, 0, + {0, 40, 0, [](DlOpReceiver& r) { r.setColorFilter(&kTestBlendColorFilter1); }}, - {0, 24, 0, + {0, 40, 0, [](DlOpReceiver& r) { r.setColorFilter(&kTestBlendColorFilter2); }}, - {0, 24, 0, + {0, 40, 0, [](DlOpReceiver& r) { r.setColorFilter(&kTestBlendColorFilter3); }}, {0, 96, 0, [](DlOpReceiver& r) { @@ -550,15 +550,15 @@ std::vector CreateAllRenderingOps() { }}, {"DrawColor", { - {1, 16, 1, + {1, 32, 1, [](DlOpReceiver& r) { r.drawColor(DlColor(SK_ColorBLUE), DlBlendMode::kSrcIn); }}, - {1, 16, 1, + {1, 32, 1, [](DlOpReceiver& r) { r.drawColor(DlColor(SK_ColorBLUE), DlBlendMode::kDstOut); }}, - {1, 16, 1, + {1, 32, 1, [](DlOpReceiver& r) { r.drawColor(DlColor(SK_ColorCYAN), DlBlendMode::kSrcIn); }}, @@ -929,7 +929,7 @@ std::vector CreateAllRenderingOps() { DlBlendMode::kSrcIn, kNearestSampling, &cull_rect, false); }}, - {1, 48 + 32 + 8 + 8, 1, + {1, 128, 1, [](DlOpReceiver& r) { static SkRSXform xforms[] = {{1, 0, 0, 0}, {0, 1, 0, 0}}; static SkRect texs[] = {{10, 10, 20, 20}, {20, 20, 30, 30}}; @@ -938,7 +938,7 @@ std::vector CreateAllRenderingOps() { DlBlendMode::kSrcIn, kNearestSampling, nullptr, false); }}, - {1, 64 + 32 + 8 + 8, 1, + {1, 144, 1, [](DlOpReceiver& r) { static SkRSXform xforms[] = {{1, 0, 0, 0}, {0, 1, 0, 0}}; static SkRect texs[] = {{10, 10, 20, 20}, {20, 20, 30, 30}}; @@ -996,27 +996,27 @@ std::vector CreateAllRenderingOps() { }}, {"DrawShadow", { - {1, 48, 1, + {1, 64, 1, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 1.0, false, 1.0); }}, - {1, 48, 1, + {1, 64, 1, [](DlOpReceiver& r) { r.drawShadow(kTestPath2, DlColor(SK_ColorGREEN), 1.0, false, 1.0); }}, - {1, 48, 1, + {1, 64, 1, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorBLUE), 1.0, false, 1.0); }}, - {1, 48, 1, + {1, 64, 1, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 2.0, false, 1.0); }}, - {1, 48, 1, + {1, 64, 1, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 1.0, true, 1.0); }}, - {1, 48, 1, + {1, 64, 1, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 1.0, false, 2.5); }}, From 07d3a6ad0a8db72a54b4f8bac3ab70152bd9b4bb Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 17:05:03 -0700 Subject: [PATCH 07/35] ++ --- display_list/dl_color_unittests.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index 317901432131f..a286fa5362cd1 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -14,7 +14,7 @@ static void arraysEqual(const uint32_t* ints, const DlColor* colors, int count) { for (int i = 0; i < count; i++) { - EXPECT_TRUE(ints[i] == colors[i].argb()); + EXPECT_EQ(ints[i], colors[i].argb()) << " index:" << i; } } @@ -34,8 +34,6 @@ TEST(DisplayListColor, ArrayInterchangeableWithUint32) { DlColor(0xF1F2F3F4), }; arraysEqual(ints, colors, 5); - arraysEqual(reinterpret_cast(colors), - reinterpret_cast(ints), 5); } TEST(DisplayListColor, DlColorDirectlyComparesToSkColor) { From 468bc63e52d54fbd131187a56037bb30d85dfd14 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 17:09:48 -0700 Subject: [PATCH 08/35] more tests --- display_list/dl_color_unittests.cc | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index a286fa5362cd1..eb357ec24dd61 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -122,10 +122,10 @@ TEST(DisplayListColor, DlColorComponentGetters) { const DlScalar half = 127.0f * (1.0f / 255.0f); - EXPECT_EQ(test.getAlphaF(), half); - EXPECT_EQ(test.getRedF(), half); - EXPECT_EQ(test.getGreenF(), half); - EXPECT_EQ(test.getBlueF(), half); + EXPECT_NEAR(test.getAlphaF(), half, 0.00001); + EXPECT_NEAR(test.getRedF(), half, 0.00001); + EXPECT_NEAR(test.getGreenF(), half, 0.00001); + EXPECT_NEAR(test.getBlueF(), half, 0.00001); } { @@ -136,12 +136,10 @@ TEST(DisplayListColor, DlColorComponentGetters) { EXPECT_EQ(test.getGreen(), 0x80); EXPECT_EQ(test.getBlue(), 0x80); - const DlScalar half = 128.0f * (1.0f / 255.0f); - - EXPECT_EQ(test.getAlphaF(), half); - EXPECT_EQ(test.getRedF(), half); - EXPECT_EQ(test.getGreenF(), half); - EXPECT_EQ(test.getBlueF(), half); + EXPECT_EQ(test.getAlphaF(), 0.5); + EXPECT_EQ(test.getRedF(), 0.5); + EXPECT_EQ(test.getGreenF(), 0.5); + EXPECT_EQ(test.getBlueF(), 0.5); } { From fe103df8627cadf7a842064f26ccae4311377156 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 9 Aug 2024 17:13:36 -0700 Subject: [PATCH 09/35] tests --- display_list/dl_color_unittests.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index eb357ec24dd61..005a92619d40b 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -150,10 +150,10 @@ TEST(DisplayListColor, DlColorComponentGetters) { EXPECT_EQ(test.getGreen(), 0x3F); EXPECT_EQ(test.getBlue(), 0x4F); - EXPECT_EQ(test.getAlphaF(), 0x1f * (1.0f / 255.0f)); - EXPECT_EQ(test.getRedF(), 0x2f * (1.0f / 255.0f)); - EXPECT_EQ(test.getGreenF(), 0x3f * (1.0f / 255.0f)); - EXPECT_EQ(test.getBlueF(), 0x4f * (1.0f / 255.0f)); + EXPECT_NEAR(test.getAlphaF(), 0x1f * (1.0f / 255.0f), 0.00001); + EXPECT_NEAR(test.getRedF(), 0x2f * (1.0f / 255.0f), 0.00001); + EXPECT_NEAR(test.getGreenF(), 0x3f * (1.0f / 255.0f), 0.00001); + EXPECT_NEAR(test.getBlueF(), 0x4f * (1.0f / 255.0f), 0.00001); } { @@ -164,11 +164,10 @@ TEST(DisplayListColor, DlColorComponentGetters) { EXPECT_EQ(test.getGreen(), round(0.3f * 255)); EXPECT_EQ(test.getBlue(), round(0.4f * 255)); - // Unfortunately conversion from float to 8-bit back to float is lossy - EXPECT_EQ(test.getAlphaF(), round(0.1f * 255) * (1.0f / 255.0f)); - EXPECT_EQ(test.getRedF(), round(0.2f * 255) * (1.0f / 255.0f)); - EXPECT_EQ(test.getGreenF(), round(0.3f * 255) * (1.0f / 255.0f)); - EXPECT_EQ(test.getBlueF(), round(0.4f * 255) * (1.0f / 255.0f)); + EXPECT_EQ(test.getAlphaF(), 0.1f); + EXPECT_EQ(test.getRedF(), 0.2f); + EXPECT_EQ(test.getGreenF(), 0.3f); + EXPECT_EQ(test.getBlueF(), 0.4f); } { From 0c520819dde747a6fb31896301220241641f021b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 09:47:24 -0700 Subject: [PATCH 10/35] ++ --- display_list/display_list_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index c1a5f3cd7d93e..7a1f47b6493d8 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1751,7 +1751,7 @@ TEST_F(DisplayListTest, FlutterSvgIssue661BoundsWereEmpty) { // This is the more practical result. The bounds are "almost" 0,0,100x100 EXPECT_EQ(display_list->bounds().roundOut(), SkIRect::MakeWH(100, 100)); EXPECT_EQ(display_list->op_count(), 19u); - EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 408u); + EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 424u); EXPECT_EQ(display_list->total_depth(), 3u); } From fc37d075eefffddf249ee5281687de074301106d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 09:49:47 -0700 Subject: [PATCH 11/35] ++ --- display_list/dl_color_unittests.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index 005a92619d40b..0c43883e81091 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -178,11 +178,10 @@ TEST(DisplayListColor, DlColorComponentGetters) { EXPECT_EQ(test.getGreen(), round(0.3f * 255)); EXPECT_EQ(test.getBlue(), round(0.4f * 255)); - // Unfortunately conversion from float to 8-bit back to float is lossy - EXPECT_EQ(test.getAlphaF(), round(0.1f * 255) * (1.0f / 255.0f)); - EXPECT_EQ(test.getRedF(), round(0.2f * 255) * (1.0f / 255.0f)); - EXPECT_EQ(test.getGreenF(), round(0.3f * 255) * (1.0f / 255.0f)); - EXPECT_EQ(test.getBlueF(), round(0.4f * 255) * (1.0f / 255.0f)); + EXPECT_EQ(test.getAlphaF(), 0.1f); + EXPECT_EQ(test.getRedF(), 0.2f); + EXPECT_EQ(test.getGreenF(), 0.3f); + EXPECT_EQ(test.getBlueF(), 0.4f); } } From 3ce239c1e3072f8d36397aeba336c8f0b683e2d2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 10:01:38 -0700 Subject: [PATCH 12/35] ++ --- display_list/dl_vertices.cc | 9 +++++++++ display_list/dl_vertices.h | 4 +--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/display_list/dl_vertices.cc b/display_list/dl_vertices.cc index c338a02a97abc..2326baa36d291 100644 --- a/display_list/dl_vertices.cc +++ b/display_list/dl_vertices.cc @@ -267,6 +267,15 @@ void DlVertices::Builder::store_colors(const DlColor colors[]) { needs_colors_ = false; } +void DlVertices::Builder::store_colors(const uint32_t colors[]) { + std::vector dlcolors; + dlcolors.reserve(vertices_->vertex_count_); + for (int i = 0; i < vertices_->vertex_count_; ++i) { + dlcolors.emplace_back(DlColor(colors[i])); + } + store_colors(dlcolors.data()); +} + void DlVertices::Builder::store_indices(const uint16_t indices[]) { FML_CHECK(is_valid()); FML_CHECK(needs_indices_); diff --git a/display_list/dl_vertices.h b/display_list/dl_vertices.h index 582c02f0bf74e..ddbaa3f33de67 100644 --- a/display_list/dl_vertices.h +++ b/display_list/dl_vertices.h @@ -147,9 +147,7 @@ class DlVertices { /// /// fails if colors have already been supplied or if they were not /// promised by the flags.has_colors. - void store_colors(const uint32_t colors[]) { - store_colors(reinterpret_cast(colors)); - } + void store_colors(const uint32_t colors[]); /// @brief Copies the indicated list of 16-bit indices as vertex indices. /// From ac388e51327f9eb21213a8103e0f2c2f0d9f6b5d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 10:05:32 -0700 Subject: [PATCH 13/35] ++ --- display_list/skia/dl_sk_conversions.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/display_list/skia/dl_sk_conversions.h b/display_list/skia/dl_sk_conversions.h index ebe53882c990c..992876adf0b7c 100644 --- a/display_list/skia/dl_sk_conversions.h +++ b/display_list/skia/dl_sk_conversions.h @@ -21,7 +21,7 @@ inline SkBlendMode ToSk(DlBlendMode mode) { inline SkColor ToSk(DlColor color) { // This is safe because both SkColor and DlColor are backed by ARGB uint32_t. // See dl_sk_conversions_unittests.cc. - return reinterpret_cast(color); + return color.argb(); } inline SkPaint::Style ToSk(DlDrawStyle style) { From edd3dcd7cf75ae8314642f7ebdbc05d05476b705 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 13:50:41 -0700 Subject: [PATCH 14/35] ++ --- display_list/BUILD.gn | 1 + display_list/dl_color.cc | 47 +++++++++++++++++++++++ display_list/dl_color.h | 17 ++++++-- display_list/dl_color_unittests.cc | 34 ++++++++++++++++ impeller/display_list/skia_conversions.cc | 3 +- 5 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 display_list/dl_color.cc diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index ce48a7a35990f..781e5b028303b 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -35,6 +35,7 @@ source_set("display_list") { "dl_builder.h", "dl_canvas.cc", "dl_canvas.h", + "dl_color.cc", "dl_color.h", "dl_op_flags.cc", "dl_op_flags.h", diff --git a/display_list/dl_color.cc b/display_list/dl_color.cc new file mode 100644 index 0000000000000..cd0fb81a5d68c --- /dev/null +++ b/display_list/dl_color.cc @@ -0,0 +1,47 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/display_list/dl_color.h" + +namespace flutter { + +DlColor DlColor::withColorSpace(DlColorSpace color_space) const { + switch (color_space_) { + case DlColorSpace::kSRGB: + switch (color_space) { + case DlColorSpace::kSRGB: + return *this; + case DlColorSpace::kExtendedSRGB: + return DlColor(alpha_, red_, green_, blue_, + DlColorSpace::kExtendedSRGB); + case DlColorSpace::kDisplayP3: + FML_CHECK(false) << "not implemented"; + return *this; + } + case DlColorSpace::kExtendedSRGB: + switch (color_space) { + case DlColorSpace::kSRGB: + FML_CHECK(false) << "not implemented"; + return *this; + case DlColorSpace::kExtendedSRGB: + return *this; + case DlColorSpace::kDisplayP3: + FML_CHECK(false) << "not implemented"; + return *this; + } + case DlColorSpace::kDisplayP3: + switch (color_space) { + case DlColorSpace::kSRGB: + FML_CHECK(false) << "not implemented"; + return *this; + case DlColorSpace::kExtendedSRGB: + FML_CHECK(false) << "not implemented"; + return *this; + case DlColorSpace::kDisplayP3: + return *this; + } + } +} + +} // namespace flutter diff --git a/display_list/dl_color.h b/display_list/dl_color.h index a2327201e027d..4644deab38904 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -127,6 +127,7 @@ struct DlColor { constexpr DlColor withBlueF(float blue) const { // return DlColor(alpha_, red_, green_, blue, color_space_); } + DlColor withColorSpace(DlColorSpace color_space) const; constexpr DlColor modulateOpacity(DlScalar opacity) const { return opacity <= 0 ? withAlpha(0) @@ -141,10 +142,18 @@ struct DlColor { toC(blue_) << 0; } - bool operator==(DlColor const& other) const { return argb() == other.argb(); } - bool operator!=(DlColor const& other) const { return argb() != other.argb(); } - bool operator==(uint32_t const& other) const { return argb() == other; } - bool operator!=(uint32_t const& other) const { return argb() != other; } + bool operator==(DlColor const& other) const { + return argb() == other.argb() && color_space_ == other.color_space_; + } + bool operator!=(DlColor const& other) const { + return argb() != other.argb() || color_space_ != other.color_space_; + } + bool operator==(uint32_t const& other) const { + return argb() == other && color_space_ == DlColorSpace::kSRGB; + } + bool operator!=(uint32_t const& other) const { + return argb() != other || color_space_ != DlColorSpace::kSRGB; + } private: float alpha_; diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index 0c43883e81091..87c8fbd3b0c0c 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -224,5 +224,39 @@ TEST(DisplayListColor, DlColorOpaqueTransparent) { } } +TEST(DisplayListColor, EqualityWithColorspace) { + EXPECT_TRUE(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB) == + DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB)); + EXPECT_FALSE(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB) == + DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kExtendedSRGB)); + EXPECT_FALSE(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB) != + DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB)); + EXPECT_TRUE(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB) != + DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kExtendedSRGB)); +} + +TEST(DisplayListColor, ColorSpaceSRGBtoSRGB) { + DlColor srgb(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB); + EXPECT_EQ(srgb, srgb.withColorSpace(DlColorSpace::kSRGB)); +} + +TEST(DisplayListColor, ColorSpaceSRGBtoExtendedSRGB) { + DlColor srgb(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB); + EXPECT_EQ(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kExtendedSRGB), + srgb.withColorSpace(DlColorSpace::kExtendedSRGB)); +} + +TEST(DisplayListColor, ColorSpaceExtendedSRGBtoExtendedSRGB) { + DlColor xsrgb(0.9, 0.8, 0.7, 0.6, DlColorSpace::kExtendedSRGB); + EXPECT_EQ(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kExtendedSRGB), + xsrgb.withColorSpace(DlColorSpace::kExtendedSRGB)); +} + +TEST(DisplayListColor, ColorSpaceP3ToP3) { + DlColor p3(0.9, 0.8, 0.7, 0.6, DlColorSpace::kDisplayP3); + EXPECT_EQ(DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kDisplayP3), + p3.withColorSpace(DlColorSpace::kDisplayP3)); +} + } // namespace testing } // namespace flutter diff --git a/impeller/display_list/skia_conversions.cc b/impeller/display_list/skia_conversions.cc index bef03963902c2..4e3180a6e2074 100644 --- a/impeller/display_list/skia_conversions.cc +++ b/impeller/display_list/skia_conversions.cc @@ -164,7 +164,8 @@ Size ToSize(const SkPoint& point) { } Color ToColor(const flutter::DlColor& color) { - FML_DCHECK(color.getColorSpace() == flutter::DlColorSpace::kExtendedSRGB); + FML_DCHECK(color.getColorSpace() == flutter::DlColorSpace::kExtendedSRGB || + color.getColorSpace() == flutter::DlColorSpace::kSRGB); return { static_cast(color.getRedF()), // static_cast(color.getGreenF()), // From cbf1e99679e86e46803ee2903ab746e759c6e62d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 14:24:05 -0700 Subject: [PATCH 15/35] updated equality --- display_list/dl_color.h | 23 ++++++++++++++++------- display_list/dl_color_unittests.cc | 27 +++++++++++++++++++++++---- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 4644deab38904..21f7690faf940 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -142,24 +142,33 @@ struct DlColor { toC(blue_) << 0; } + bool isClose(DlColor const& other, DlScalar delta = 1.0f / 256.0f) { + return color_space_ == other.color_space_ && + std::abs(alpha_ - other.alpha_) < delta && + std::abs(red_ - other.red_) < delta && + std::abs(green_ - other.green_) < delta && + std::abs(blue_ - other.blue_) < delta; + } bool operator==(DlColor const& other) const { - return argb() == other.argb() && color_space_ == other.color_space_; + return alpha_ == other.alpha_ && red_ == other.red_ && + green_ == other.green_ && blue_ == other.blue_ && + color_space_ == other.color_space_; } bool operator!=(DlColor const& other) const { - return argb() != other.argb() || color_space_ != other.color_space_; + return !this->operator==(other); } bool operator==(uint32_t const& other) const { return argb() == other && color_space_ == DlColorSpace::kSRGB; } bool operator!=(uint32_t const& other) const { - return argb() != other || color_space_ != DlColorSpace::kSRGB; + return !this->operator==(other); } private: - float alpha_; - float red_; - float green_; - float blue_; + DlScalar alpha_; + DlScalar red_; + DlScalar green_; + DlScalar blue_; DlColorSpace color_space_; static constexpr DlScalar toF(uint8_t comp) { return comp * (1.0f / 255); } diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index 87c8fbd3b0c0c..d7f434aef96ba 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -46,13 +46,17 @@ TEST(DisplayListColor, DlColorDirectlyComparesToSkColor) { TEST(DisplayListColor, DlColorFloatConstructor) { EXPECT_EQ(DlColor::ARGB(1.0f, 1.0f, 1.0f, 1.0f), DlColor(0xFFFFFFFF)); EXPECT_EQ(DlColor::ARGB(0.0f, 0.0f, 0.0f, 0.0f), DlColor(0x00000000)); - EXPECT_EQ(DlColor::ARGB(0.5f, 0.5f, 0.5f, 0.5f), DlColor(0x80808080)); - EXPECT_EQ(DlColor::ARGB(1.0f, 0.0f, 0.5f, 1.0f), DlColor(0xFF0080FF)); + EXPECT_TRUE( + DlColor::ARGB(0.5f, 0.5f, 0.5f, 0.5f).isClose(DlColor(0x80808080))); + EXPECT_TRUE( + DlColor::ARGB(1.0f, 0.0f, 0.5f, 1.0f).isClose(DlColor(0xFF0080FF))); EXPECT_EQ(DlColor::RGBA(1.0f, 1.0f, 1.0f, 1.0f), DlColor(0xFFFFFFFF)); EXPECT_EQ(DlColor::RGBA(0.0f, 0.0f, 0.0f, 0.0f), DlColor(0x00000000)); - EXPECT_EQ(DlColor::RGBA(0.5f, 0.5f, 0.5f, 0.5f), DlColor(0x80808080)); - EXPECT_EQ(DlColor::RGBA(1.0f, 0.0f, 0.5f, 1.0f), DlColor(0xFFFF0080)); + EXPECT_TRUE( + DlColor::RGBA(0.5f, 0.5f, 0.5f, 0.5f).isClose(DlColor(0x80808080))); + EXPECT_TRUE( + DlColor::RGBA(1.0f, 0.0f, 0.5f, 1.0f).isClose(DlColor(0xFFFF0080))); } TEST(DisplayListColor, DlColorComponentGetters) { @@ -235,6 +239,13 @@ TEST(DisplayListColor, EqualityWithColorspace) { DlColor(0.9, 0.8, 0.7, 0.6, DlColorSpace::kExtendedSRGB)); } +TEST(DisplayListColor, EqualityWithExtendedSRGB) { + EXPECT_TRUE(DlColor(1.0, 1.1, -0.2, 0.1, DlColorSpace::kExtendedSRGB) == + DlColor(1.0, 1.1, -0.2, 0.1, DlColorSpace::kExtendedSRGB)); + EXPECT_FALSE(DlColor(1.0, 1.1, -0.2, 0.1, DlColorSpace::kExtendedSRGB) == + DlColor(1.0, 1.0, 0.0, 0.0, DlColorSpace::kExtendedSRGB)); +} + TEST(DisplayListColor, ColorSpaceSRGBtoSRGB) { DlColor srgb(0.9, 0.8, 0.7, 0.6, DlColorSpace::kSRGB); EXPECT_EQ(srgb, srgb.withColorSpace(DlColorSpace::kSRGB)); @@ -258,5 +269,13 @@ TEST(DisplayListColor, ColorSpaceP3ToP3) { p3.withColorSpace(DlColorSpace::kDisplayP3)); } +TEST(DisplayListColor, isClose) { + EXPECT_TRUE(DlColor(0xffaabbcc).isClose(DlColor(0xffaabbcc))); +} + +TEST(DisplayListColor, isNotClose) { + EXPECT_FALSE(DlColor(0xffaabbcc).isClose(DlColor(0xffaabbcd))); +} + } // namespace testing } // namespace flutter From cf35445578adfece14faf03ee1a3569bf6330bad Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 12 Aug 2024 15:02:39 -0700 Subject: [PATCH 16/35] added p3->srgb_xr transform --- display_list/dl_color.cc | 32 ++++++++++++++++++++++++++++-- display_list/dl_color_unittests.cc | 20 +++++++++++++++++++ testing/display_list_testing.cc | 19 +++++++++++++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/display_list/dl_color.cc b/display_list/dl_color.cc index cd0fb81a5d68c..24a60f74dd252 100644 --- a/display_list/dl_color.cc +++ b/display_list/dl_color.cc @@ -6,6 +6,35 @@ namespace flutter { +namespace { +const std::array kP3ToSrgb = { + 1.306671048092539, -0.298061942172353, + 0.213228303487995, -0.213580156254466, // + -0.117390025596251, 1.127722006101976, + 0.109727644608938, -0.109450321455370, // + 0.214813187718391, 0.054268702864647, + 1.406898424029350, -0.364892765879631}; + +DlColor transform(const DlColor& color, + const std::array& matrix, + DlColorSpace color_space) { + return DlColor(color.getAlphaF(), + matrix[0] * color.getRedF() + // + matrix[1] * color.getGreenF() + // + matrix[2] * color.getBlueF() + // + matrix[3], // + matrix[4] * color.getRedF() + // + matrix[5] * color.getGreenF() + // + matrix[6] * color.getBlueF() + // + matrix[7], // + matrix[8] * color.getRedF() + // + matrix[9] * color.getGreenF() + // + matrix[10] * color.getBlueF() + // + matrix[11], // + color_space); +} +} // namespace + DlColor DlColor::withColorSpace(DlColorSpace color_space) const { switch (color_space_) { case DlColorSpace::kSRGB: @@ -36,8 +65,7 @@ DlColor DlColor::withColorSpace(DlColorSpace color_space) const { FML_CHECK(false) << "not implemented"; return *this; case DlColorSpace::kExtendedSRGB: - FML_CHECK(false) << "not implemented"; - return *this; + return transform(*this, kP3ToSrgb, DlColorSpace::kExtendedSRGB); case DlColorSpace::kDisplayP3: return *this; } diff --git a/display_list/dl_color_unittests.cc b/display_list/dl_color_unittests.cc index d7f434aef96ba..ac29d89292c77 100644 --- a/display_list/dl_color_unittests.cc +++ b/display_list/dl_color_unittests.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/display_list/dl_color.h" +#include "flutter/testing/display_list_testing.h" #include "flutter/testing/testing.h" #include "third_party/skia/include/core/SkColor.h" @@ -269,6 +270,25 @@ TEST(DisplayListColor, ColorSpaceP3ToP3) { p3.withColorSpace(DlColorSpace::kDisplayP3)); } +TEST(DisplayListColor, ColorSpaceP3ToExtendedSRGB) { + DlColor red(0.9, 1.0, 0.0, 0.0, DlColorSpace::kDisplayP3); + EXPECT_TRUE( + DlColor(0.9, 1.0931, -0.2268, -0.1501, DlColorSpace::kExtendedSRGB) + .isClose(red.withColorSpace(DlColorSpace::kExtendedSRGB))) + << red.withColorSpace(DlColorSpace::kExtendedSRGB); + + DlColor green(0.9, 0.0, 1.0, 0.0, DlColorSpace::kDisplayP3); + EXPECT_TRUE( + DlColor(0.9, -0.5116, 1.0183, -0.3106, DlColorSpace::kExtendedSRGB) + .isClose(green.withColorSpace(DlColorSpace::kExtendedSRGB))) + << green.withColorSpace(DlColorSpace::kExtendedSRGB); + + DlColor blue(0.9, 0.0, 0.0, 1.0, DlColorSpace::kDisplayP3); + EXPECT_TRUE(DlColor(0.9, -0.0004, 0.0003, 1.0420, DlColorSpace::kExtendedSRGB) + .isClose(blue.withColorSpace(DlColorSpace::kExtendedSRGB))) + << blue.withColorSpace(DlColorSpace::kExtendedSRGB); +} + TEST(DisplayListColor, isClose) { EXPECT_TRUE(DlColor(0xffaabbcc).isClose(DlColor(0xffaabbcc))); } diff --git a/testing/display_list_testing.cc b/testing/display_list_testing.cc index 9b6a38b75d3ce..1ab19357f8139 100644 --- a/testing/display_list_testing.cc +++ b/testing/display_list_testing.cc @@ -284,7 +284,24 @@ std::ostream& operator<<(std::ostream& os, const DlFilterMode& mode) { } std::ostream& operator<<(std::ostream& os, const DlColor& color) { - return os << "DlColor(" << std::hex << color.argb() << std::dec << ")"; + const char* color_space; + switch(color.getColorSpace()) { + case flutter::DlColorSpace::kSRGB: + color_space = "srgb"; + break; + case flutter::DlColorSpace::kExtendedSRGB: + color_space = "srgb_xr"; + break; + case flutter::DlColorSpace::kDisplayP3: + color_space = "p3"; + break; + } + return os << "DlColor(" << // + color.getAlphaF() << ", " << // + color.getRedF() << ", " << // + color.getGreenF() << ", " << // + color.getBlueF() << ", " << // + color_space << ")"; } std::ostream& operator<<(std::ostream& os, DlImageSampling sampling) { From 6ecf978dd39c080b480f8364b770bc06f86bcf2a Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 15 Aug 2024 09:06:36 -0700 Subject: [PATCH 17/35] license update --- ci/licenses_golden/licenses_flutter | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index bd838fd198e0c..65d6269964ba8 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -41609,6 +41609,7 @@ ORIGIN: ../../../flutter/display_list/dl_builder.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_builder.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_canvas.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_canvas.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/dl_color.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_color.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_op_flags.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/dl_op_flags.h + ../../../flutter/LICENSE @@ -44491,6 +44492,7 @@ FILE: ../../../flutter/display_list/dl_builder.cc FILE: ../../../flutter/display_list/dl_builder.h FILE: ../../../flutter/display_list/dl_canvas.cc FILE: ../../../flutter/display_list/dl_canvas.h +FILE: ../../../flutter/display_list/dl_color.cc FILE: ../../../flutter/display_list/dl_color.h FILE: ../../../flutter/display_list/dl_op_flags.cc FILE: ../../../flutter/display_list/dl_op_flags.h From 961577c58977c6afc2176d8238f394f209b72baa Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 15 Aug 2024 09:54:00 -0700 Subject: [PATCH 18/35] fixed unsafe skia conversions --- display_list/skia/dl_sk_canvas.cc | 10 +++++++--- display_list/skia/dl_sk_conversions.cc | 27 +++++++++++++++++--------- display_list/skia/dl_sk_dispatcher.cc | 10 +++++++--- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc index 235b654f41bca..e74e4b532cf99 100644 --- a/display_list/skia/dl_sk_canvas.cc +++ b/display_list/skia/dl_sk_canvas.cc @@ -311,9 +311,13 @@ void DlSkCanvasAdapter::DrawAtlas(const sk_sp& atlas, const DlPaint* paint) { SkOptionalPaint sk_paint(paint); sk_sp sk_image = atlas->skia_image(); - const SkColor* sk_colors = reinterpret_cast(colors); - delegate_->drawAtlas(sk_image.get(), xform, tex, sk_colors, count, ToSk(mode), - ToSk(sampling), cullRect, sk_paint()); + std::vector sk_colors; + sk_colors.reserve(count); + for (int i = 0; i < count; ++i) { + sk_colors.push_back(colors[i].argb()); + } + delegate_->drawAtlas(sk_image.get(), xform, tex, sk_colors.data(), count, + ToSk(mode), ToSk(sampling), cullRect, sk_paint()); } void DlSkCanvasAdapter::DrawDisplayList(const sk_sp display_list, diff --git a/display_list/skia/dl_sk_conversions.cc b/display_list/skia/dl_sk_conversions.cc index a1747cfbb26bd..352449203bb69 100644 --- a/display_list/skia/dl_sk_conversions.cc +++ b/display_list/skia/dl_sk_conversions.cc @@ -69,8 +69,14 @@ sk_sp ToSk(const DlColorSource* source) { if (!source) { return nullptr; } - static auto ToSkColors = [](const DlGradientColorSourceBase* gradient) { - return reinterpret_cast(gradient->colors()); + static auto ToSkColors = + [](const DlGradientColorSourceBase* gradient) -> std::vector { + std::vector sk_colors; + sk_colors.reserve(gradient->stop_count()); + for (int i = 0; i < gradient->stop_count(); ++i) { + sk_colors.push_back(gradient->colors()[i].argb()); + } + return sk_colors; }; switch (source->type()) { case DlColorSourceType::kColor: { @@ -97,7 +103,7 @@ sk_sp ToSk(const DlColorSource* source) { SkPoint pts[] = {linear_source->start_point(), linear_source->end_point()}; return SkGradientShader::MakeLinear( - pts, ToSkColors(linear_source), linear_source->stops(), + pts, ToSkColors(linear_source).data(), linear_source->stops(), linear_source->stop_count(), ToSk(linear_source->tile_mode()), 0, linear_source->matrix_ptr()); } @@ -107,7 +113,7 @@ sk_sp ToSk(const DlColorSource* source) { FML_DCHECK(radial_source != nullptr); return SkGradientShader::MakeRadial( radial_source->center(), radial_source->radius(), - ToSkColors(radial_source), radial_source->stops(), + ToSkColors(radial_source).data(), radial_source->stops(), radial_source->stop_count(), ToSk(radial_source->tile_mode()), 0, radial_source->matrix_ptr()); } @@ -118,7 +124,7 @@ sk_sp ToSk(const DlColorSource* source) { return SkGradientShader::MakeTwoPointConical( conical_source->start_center(), conical_source->start_radius(), conical_source->end_center(), conical_source->end_radius(), - ToSkColors(conical_source), conical_source->stops(), + ToSkColors(conical_source).data(), conical_source->stops(), conical_source->stop_count(), ToSk(conical_source->tile_mode()), 0, conical_source->matrix_ptr()); } @@ -128,7 +134,7 @@ sk_sp ToSk(const DlColorSource* source) { FML_DCHECK(sweep_source != nullptr); return SkGradientShader::MakeSweep( sweep_source->center().x(), sweep_source->center().y(), - ToSkColors(sweep_source), sweep_source->stops(), + ToSkColors(sweep_source).data(), sweep_source->stops(), sweep_source->stop_count(), ToSk(sweep_source->tile_mode()), sweep_source->start(), sweep_source->end(), 0, sweep_source->matrix_ptr()); @@ -275,11 +281,14 @@ sk_sp ToSk(const DlMaskFilter* filter) { } sk_sp ToSk(const std::shared_ptr& vertices) { - const SkColor* sk_colors = - reinterpret_cast(vertices->colors()); + std::vector sk_colors; + sk_colors.reserve(vertices->vertex_count()); + for (int i = 0; i < vertices->vertex_count(); ++i) { + sk_colors.push_back(vertices->colors()[i].argb()); + } return SkVertices::MakeCopy(ToSk(vertices->mode()), vertices->vertex_count(), vertices->vertices(), - vertices->texture_coordinates(), sk_colors, + vertices->texture_coordinates(), sk_colors.data(), vertices->index_count(), vertices->indices()); } diff --git a/display_list/skia/dl_sk_dispatcher.cc b/display_list/skia/dl_sk_dispatcher.cc index 819860d3fb184..9dcd34758804d 100644 --- a/display_list/skia/dl_sk_dispatcher.cc +++ b/display_list/skia/dl_sk_dispatcher.cc @@ -250,9 +250,13 @@ void DlSkCanvasDispatcher::drawAtlas(const sk_sp atlas, if (!skia_atlas) { return; } - const SkColor* sk_colors = reinterpret_cast(colors); - canvas_->drawAtlas(skia_atlas.get(), xform, tex, sk_colors, count, ToSk(mode), - ToSk(sampling), cullRect, + std::vector sk_colors; + sk_colors.reserve(count); + for (int i = 0; i < count; ++i) { + sk_colors.push_back(colors[i].argb()); + } + canvas_->drawAtlas(skia_atlas.get(), xform, tex, sk_colors.data(), count, + ToSk(mode), ToSk(sampling), cullRect, safe_paint(render_with_attributes)); } void DlSkCanvasDispatcher::drawDisplayList( From 5cfa1754fe7beaf665efb80522d02cbd90937d26 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 15 Aug 2024 10:04:48 -0700 Subject: [PATCH 19/35] fixed vertices without color --- display_list/skia/dl_sk_conversions.cc | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/display_list/skia/dl_sk_conversions.cc b/display_list/skia/dl_sk_conversions.cc index 352449203bb69..8cbeae23c87d6 100644 --- a/display_list/skia/dl_sk_conversions.cc +++ b/display_list/skia/dl_sk_conversions.cc @@ -282,13 +282,17 @@ sk_sp ToSk(const DlMaskFilter* filter) { sk_sp ToSk(const std::shared_ptr& vertices) { std::vector sk_colors; - sk_colors.reserve(vertices->vertex_count()); - for (int i = 0; i < vertices->vertex_count(); ++i) { - sk_colors.push_back(vertices->colors()[i].argb()); + const SkColor* sk_colors_ptr = nullptr; + if (vertices->colors()) { + sk_colors.reserve(vertices->vertex_count()); + for (int i = 0; i < vertices->vertex_count(); ++i) { + sk_colors.push_back(vertices->colors()[i].argb()); + } + sk_colors_ptr = sk_colors.data(); } return SkVertices::MakeCopy(ToSk(vertices->mode()), vertices->vertex_count(), vertices->vertices(), - vertices->texture_coordinates(), sk_colors.data(), + vertices->texture_coordinates(), sk_colors_ptr, vertices->index_count(), vertices->indices()); } From 9866b13e35da06f90bbce8958afda859a093a804 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 16 Aug 2024 18:45:59 +0000 Subject: [PATCH 20/35] removed more reinterpret casts --- lib/ui/painting/gradient.cc | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/ui/painting/gradient.cc b/lib/ui/painting/gradient.cc index e54f39f41fadd..ca730cc1834e9 100644 --- a/lib/ui/painting/gradient.cc +++ b/lib/ui/painting/gradient.cc @@ -45,10 +45,14 @@ void CanvasGradient::initLinear(const tonic::Float32List& end_points, SkPoint p0 = SkPoint::Make(end_points[0], end_points[1]); SkPoint p1 = SkPoint::Make(end_points[2], end_points[3]); - const DlColor* colors_array = reinterpret_cast(colors.data()); + std::vector dl_colors; + dl_colors.reserve(color_stops.num_elements()); + for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.emplace_back(DlColor(colors[i])); + } dl_shader_ = DlColorSource::MakeLinear( - p0, p1, colors.num_elements(), colors_array, color_stops.data(), + p0, p1, colors.num_elements(), dl_colors.data(), color_stops.data(), tile_mode, has_matrix ? &sk_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); @@ -73,11 +77,15 @@ void CanvasGradient::initRadial(double center_x, sk_matrix = ToSkMatrix(matrix4); } - const DlColor* colors_array = reinterpret_cast(colors.data()); + std::vector dl_colors; + dl_colors.reserve(color_stops.num_elements()); + for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.emplace_back(DlColor(colors[i])); + } dl_shader_ = DlColorSource::MakeRadial( SkPoint::Make(SafeNarrow(center_x), SafeNarrow(center_y)), - SafeNarrow(radius), colors.num_elements(), colors_array, + SafeNarrow(radius), colors.num_elements(), dl_colors.data(), color_stops.data(), tile_mode, has_matrix ? &sk_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); @@ -103,13 +111,17 @@ void CanvasGradient::initSweep(double center_x, sk_matrix = ToSkMatrix(matrix4); } - const DlColor* colors_array = reinterpret_cast(colors.data()); + std::vector dl_colors; + dl_colors.reserve(color_stops.num_elements()); + for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.emplace_back(DlColor(colors[i])); + } dl_shader_ = DlColorSource::MakeSweep( SkPoint::Make(SafeNarrow(center_x), SafeNarrow(center_y)), SafeNarrow(start_angle) * 180.0f / static_cast(M_PI), SafeNarrow(end_angle) * 180.0f / static_cast(M_PI), - colors.num_elements(), colors_array, color_stops.data(), tile_mode, + colors.num_elements(), dl_colors.data(), color_stops.data(), tile_mode, has_matrix ? &sk_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); @@ -137,13 +149,17 @@ void CanvasGradient::initTwoPointConical(double start_x, sk_matrix = ToSkMatrix(matrix4); } - const DlColor* colors_array = reinterpret_cast(colors.data()); + std::vector dl_colors; + dl_colors.reserve(color_stops.num_elements()); + for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.emplace_back(DlColor(colors[i])); + } dl_shader_ = DlColorSource::MakeConical( SkPoint::Make(SafeNarrow(start_x), SafeNarrow(start_y)), SafeNarrow(start_radius), SkPoint::Make(SafeNarrow(end_x), SafeNarrow(end_y)), - SafeNarrow(end_radius), colors.num_elements(), colors_array, + SafeNarrow(end_radius), colors.num_elements(), dl_colors.data(), color_stops.data(), tile_mode, has_matrix ? &sk_matrix : nullptr); // Just a sanity check, all gradient shaders should be thread-safe FML_DCHECK(dl_shader_->isUIThreadSafe()); From 4b276e9a3e56823924ddcbadbbad5a72d9cfa8cd Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 16 Aug 2024 13:54:50 -0700 Subject: [PATCH 21/35] fixed conversion loops --- lib/ui/painting/gradient.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/ui/painting/gradient.cc b/lib/ui/painting/gradient.cc index ca730cc1834e9..12ebc5c498824 100644 --- a/lib/ui/painting/gradient.cc +++ b/lib/ui/painting/gradient.cc @@ -46,8 +46,8 @@ void CanvasGradient::initLinear(const tonic::Float32List& end_points, SkPoint p0 = SkPoint::Make(end_points[0], end_points[1]); SkPoint p1 = SkPoint::Make(end_points[2], end_points[3]); std::vector dl_colors; - dl_colors.reserve(color_stops.num_elements()); - for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.reserve(colors.num_elements()); + for (int i = 0; i < colors.num_elements(); ++i) { dl_colors.emplace_back(DlColor(colors[i])); } @@ -78,8 +78,8 @@ void CanvasGradient::initRadial(double center_x, } std::vector dl_colors; - dl_colors.reserve(color_stops.num_elements()); - for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.reserve(colors.num_elements()); + for (int i = 0; i < colors.num_elements(); ++i) { dl_colors.emplace_back(DlColor(colors[i])); } @@ -112,8 +112,8 @@ void CanvasGradient::initSweep(double center_x, } std::vector dl_colors; - dl_colors.reserve(color_stops.num_elements()); - for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.reserve(colors.num_elements()); + for (int i = 0; i < colors.num_elements(); ++i) { dl_colors.emplace_back(DlColor(colors[i])); } @@ -150,8 +150,8 @@ void CanvasGradient::initTwoPointConical(double start_x, } std::vector dl_colors; - dl_colors.reserve(color_stops.num_elements()); - for (int i = 0; i < color_stops.num_elements(); ++i) { + dl_colors.reserve(colors.num_elements()); + for (int i = 0; i < colors.num_elements(); ++i) { dl_colors.emplace_back(DlColor(colors[i])); } From 8753d858722fd1cd0cdbd222b4beabd7f5d4084e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 16 Aug 2024 15:20:36 -0700 Subject: [PATCH 22/35] jim feedback1 --- display_list/dl_color.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 21f7690faf940..75b77e4d6f9d7 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -20,16 +20,16 @@ struct DlColor { blue_(0.f), color_space_(DlColorSpace::kSRGB) {} constexpr explicit DlColor(uint32_t argb) - : alpha_(((argb >> 24) & 0xff) / 255.f), - red_(((argb >> 16) & 0xff) / 255.f), - green_(((argb >> 8) & 0xff) / 255.f), - blue_(((argb >> 0) & 0xff) / 255.f), + : alpha_(toF((argb >> 24) & 0xff)), + red_(toF((argb >> 16) & 0xff)), + green_(toF((argb >> 8) & 0xff)), + blue_(toF((argb >> 0) & 0xff)), color_space_(DlColorSpace::kSRGB) {} - constexpr explicit DlColor(float alpha, - float red, - float green, - float blue, - DlColorSpace colorspace) + constexpr DlColor(DlScalar alpha, + DlScalar red, + DlScalar green, + DlScalar blue, + DlColorSpace colorspace) : alpha_(alpha), red_(red), green_(green), From aa492075d778fdba4db64c12fe843422a1cca05f Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 16 Aug 2024 15:30:07 -0700 Subject: [PATCH 23/35] jim feedback2 --- display_list/dl_color.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 75b77e4d6f9d7..8ed94356d49d8 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -135,6 +135,8 @@ struct DlColor { : withAlpha(round(getAlpha() * opacity)); } + ///\deprecated Use floating point accessors to avoid data loss when using wide + /// gamut colors. constexpr uint32_t argb() const { return toC(alpha_) << 24 | // toC(red_) << 16 | // From 31623dfafa4c6bd32a588bf4a3b766d6a79a8725 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 16 Aug 2024 15:31:38 -0700 Subject: [PATCH 24/35] jim feedback3 --- display_list/dl_color.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 8ed94356d49d8..76adcbe054748 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -80,9 +80,17 @@ struct DlColor { constexpr bool isOpaque() const { return alpha_ >= 1.f; } constexpr bool isTransparent() const { return alpha_ <= 0.f; } + ///\deprecated Use floating point accessors to avoid data loss when using wide + /// gamut colors. constexpr int getAlpha() const { return toC(alpha_); } + ///\deprecated Use floating point accessors to avoid data loss when using wide + /// gamut colors. constexpr int getRed() const { return toC(red_); } + ///\deprecated Use floating point accessors to avoid data loss when using wide + /// gamut colors. constexpr int getGreen() const { return toC(green_); } + ///\deprecated Use floating point accessors to avoid data loss when using wide + /// gamut colors. constexpr int getBlue() const { return toC(blue_); } constexpr DlScalar getAlphaF() const { return alpha_; } From 93f5bd0fd7da9339bb7b9a5d836ef5622641eb49 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 16 Aug 2024 15:33:36 -0700 Subject: [PATCH 25/35] jim feedback4 --- display_list/dl_color.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 76adcbe054748..b5636d3f11143 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -100,6 +100,8 @@ struct DlColor { constexpr DlColorSpace getColorSpace() const { return color_space_; } + ///\deprecated Use floating point accessors to avoid data loss when using wide + /// gamut colors. constexpr uint32_t premultipliedArgb() const { if (isOpaque()) { return argb(); From 1c50f9da5e709dadb4218f7f05d6733fb904f134 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 08:57:40 -0700 Subject: [PATCH 26/35] removed premultipliedargb() --- display_list/dl_color.h | 13 ---------- .../testing/dl_rendering_unittests.cc | 25 +++++++++++++++++-- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index b5636d3f11143..17945d6d603eb 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -100,19 +100,6 @@ struct DlColor { constexpr DlColorSpace getColorSpace() const { return color_space_; } - ///\deprecated Use floating point accessors to avoid data loss when using wide - /// gamut colors. - constexpr uint32_t premultipliedArgb() const { - if (isOpaque()) { - return argb(); - } - DlScalar f = getAlphaF(); - return (argb() & 0xFF000000) | // - toC(getRedF() * f) << 16 | // - toC(getGreenF() * f) << 8 | // - toC(getBlueF() * f); - } - constexpr DlColor withAlpha(uint8_t alpha) const { // return DlColor((argb() & 0x00FFFFFF) | (alpha << 24)); } diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc index 1368f3e921fbe..b3b23659567dd 100644 --- a/display_list/testing/dl_rendering_unittests.cc +++ b/display_list/testing/dl_rendering_unittests.cc @@ -156,6 +156,27 @@ const int kHorizontalMiterDiamondPointCount = (sizeof(kHorizontalMiterDiamondPoints) / sizeof(kHorizontalMiterDiamondPoints[0])); +namespace { +constexpr DlScalar toF(uint8_t comp) { + return comp * (1.0f / 255); +} + +constexpr uint8_t toC(DlScalar fComp) { + return round(fComp * 255); +} + +constexpr uint32_t PremultipliedArgb(const DlColor& color) { + if (icolor.sOpaque()) { + return color.argb(); + } + DlScalar f = color.getAlphaF(); + return (argb() & 0xFF000000) | // + toC(color.getRedF() * f) << 16 | // + toC(color.getGreenF() * f) << 8 | // + toC(color.getBlueF() * f); +} +} // namespace + class SkImageSampling { public: static constexpr SkSamplingOptions kNearestNeighbor = @@ -2526,7 +2547,7 @@ class CanvasCompareTester { const SkRect ref_bounds, const std::string& info, const DlColor bg = DlColor::kTransparent()) { - uint32_t untouched = bg.premultipliedArgb(); + uint32_t untouched = PremultipliedArgb(bg); int pixels_touched = 0; int pixels_oob = 0; SkIRect i_bounds = ref_bounds.roundOut(); @@ -2611,7 +2632,7 @@ class CanvasCompareTester { int width = kTestWidth, int height = kTestHeight, bool printMismatches = false) { - uint32_t untouched = bg.premultipliedArgb(); + uint32_t untouched = PremultipliedArgb(bg); ASSERT_EQ(test_result->width(), width) << info; ASSERT_EQ(test_result->height(), height) << info; SkIRect i_bounds = From 7eca13111ad783a754116f4548cdb258e1f0bafa Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 09:07:31 -0700 Subject: [PATCH 27/35] added class docstring --- display_list/dl_color.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 17945d6d603eb..90b1ba8256306 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -11,6 +11,12 @@ namespace flutter { enum class DlColorSpace { kSRGB = 0, kExtendedSRGB = 1, kDisplayP3 = 2 }; +/// A representation of a color. +/// +/// The color belongs to a DlColorSpace. Using deprecated integer data accessors +/// on colors not in the kSRGB colorspace can lead to data loss. Using the +/// floating point accessors and constructors that were added for wide-gamut +/// support are preferred. struct DlColor { public: constexpr DlColor() From cf647605652d19a880e5a64646c65b98fcbd38cf Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 09:10:02 -0700 Subject: [PATCH 28/35] added withColorSpace docstring --- display_list/dl_color.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 90b1ba8256306..8ed5767a93aeb 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -130,6 +130,10 @@ struct DlColor { constexpr DlColor withBlueF(float blue) const { // return DlColor(alpha_, red_, green_, blue, color_space_); } + /// Performs a colorspace transformation. + /// + /// This isn't just a replacement of the color space field, the new color + /// components are calculated. DlColor withColorSpace(DlColorSpace color_space) const; constexpr DlColor modulateOpacity(DlScalar opacity) const { From eeed2edf9a2afe83cbc2189ed47980c9e765f364 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 09:17:13 -0700 Subject: [PATCH 29/35] added isclose docstring --- display_list/dl_color.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index 8ed5767a93aeb..ce1f97ca0dfec 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -151,6 +151,10 @@ struct DlColor { toC(blue_) << 0; } + /// Checks that no difference in color components exceeds the delta. + /// + /// This doesn't check against the actual distance between the colors in some + /// space. bool isClose(DlColor const& other, DlScalar delta = 1.0f / 256.0f) { return color_space_ == other.color_space_ && std::abs(alpha_ - other.alpha_) < delta && From 35cbd3147defdce6c2956f3da61cb57ddee96ab7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 09:19:31 -0700 Subject: [PATCH 30/35] removed comment --- display_list/skia/dl_sk_conversions.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/display_list/skia/dl_sk_conversions.h b/display_list/skia/dl_sk_conversions.h index 992876adf0b7c..808d9842efd43 100644 --- a/display_list/skia/dl_sk_conversions.h +++ b/display_list/skia/dl_sk_conversions.h @@ -19,8 +19,6 @@ inline SkBlendMode ToSk(DlBlendMode mode) { } inline SkColor ToSk(DlColor color) { - // This is safe because both SkColor and DlColor are backed by ARGB uint32_t. - // See dl_sk_conversions_unittests.cc. return color.argb(); } From 902e510c8b029b0016b09e7cb6ddf4dfc12908f3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 10:25:37 -0700 Subject: [PATCH 31/35] made store_colors faster --- display_list/dl_vertices.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/display_list/dl_vertices.cc b/display_list/dl_vertices.cc index 2326baa36d291..f0cf5ae8bd31d 100644 --- a/display_list/dl_vertices.cc +++ b/display_list/dl_vertices.cc @@ -268,12 +268,15 @@ void DlVertices::Builder::store_colors(const DlColor colors[]) { } void DlVertices::Builder::store_colors(const uint32_t colors[]) { - std::vector dlcolors; - dlcolors.reserve(vertices_->vertex_count_); + FML_CHECK(is_valid()); + FML_CHECK(needs_colors_); + char* pod = reinterpret_cast(vertices_.get()); + DlColor* dlcolors_ptr = + reinterpret_cast(pod + vertices_->colors_offset_); for (int i = 0; i < vertices_->vertex_count_; ++i) { - dlcolors.emplace_back(DlColor(colors[i])); + *dlcolors_ptr++ = DlColor(colors[i]); } - store_colors(dlcolors.data()); + needs_colors_ = false; } void DlVertices::Builder::store_indices(const uint16_t indices[]) { From d46e14085967a2bbafb337f6a8d991d89207034b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 10:28:03 -0700 Subject: [PATCH 32/35] added todo --- lib/ui/painting/gradient.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/painting/gradient.cc b/lib/ui/painting/gradient.cc index 12ebc5c498824..4be8d056e9a82 100644 --- a/lib/ui/painting/gradient.cc +++ b/lib/ui/painting/gradient.cc @@ -48,6 +48,7 @@ void CanvasGradient::initLinear(const tonic::Float32List& end_points, std::vector dl_colors; dl_colors.reserve(colors.num_elements()); for (int i = 0; i < colors.num_elements(); ++i) { + /// TODO(gaaclarke): Make this preserve wide gamut colors. dl_colors.emplace_back(DlColor(colors[i])); } From 10b1d6d9fea57a716ef245ed395a0f82599ae92e Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 10:38:25 -0700 Subject: [PATCH 33/35] pulled out rvalue --- display_list/skia/dl_sk_conversions.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/display_list/skia/dl_sk_conversions.cc b/display_list/skia/dl_sk_conversions.cc index 8cbeae23c87d6..9ed2d510ad7b4 100644 --- a/display_list/skia/dl_sk_conversions.cc +++ b/display_list/skia/dl_sk_conversions.cc @@ -102,8 +102,9 @@ sk_sp ToSk(const DlColorSource* source) { FML_DCHECK(linear_source != nullptr); SkPoint pts[] = {linear_source->start_point(), linear_source->end_point()}; + std::vector skcolors = ToSkColors(linear_source); return SkGradientShader::MakeLinear( - pts, ToSkColors(linear_source).data(), linear_source->stops(), + pts, skcolors.data(), linear_source->stops(), linear_source->stop_count(), ToSk(linear_source->tile_mode()), 0, linear_source->matrix_ptr()); } From b839e18ee454011dce0b9118a85baf444bef0d07 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 11:12:45 -0700 Subject: [PATCH 34/35] fixed typo --- display_list/testing/dl_rendering_unittests.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc index b3b23659567dd..8873e2f34f893 100644 --- a/display_list/testing/dl_rendering_unittests.cc +++ b/display_list/testing/dl_rendering_unittests.cc @@ -157,20 +157,16 @@ const int kHorizontalMiterDiamondPointCount = sizeof(kHorizontalMiterDiamondPoints[0])); namespace { -constexpr DlScalar toF(uint8_t comp) { - return comp * (1.0f / 255); -} - constexpr uint8_t toC(DlScalar fComp) { return round(fComp * 255); } constexpr uint32_t PremultipliedArgb(const DlColor& color) { - if (icolor.sOpaque()) { + if (color.isOpaque()) { return color.argb(); } DlScalar f = color.getAlphaF(); - return (argb() & 0xFF000000) | // + return (color.argb() & 0xFF000000) | // toC(color.getRedF() * f) << 16 | // toC(color.getGreenF() * f) << 8 | // toC(color.getBlueF() * f); From 2be2649e79769a6c498eebfe02a4703b9ee3ea63 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 19 Aug 2024 14:01:12 -0700 Subject: [PATCH 35/35] added colorspace comment --- display_list/dl_color.h | 1 + 1 file changed, 1 insertion(+) diff --git a/display_list/dl_color.h b/display_list/dl_color.h index ce1f97ca0dfec..e2d0e82ae09b4 100644 --- a/display_list/dl_color.h +++ b/display_list/dl_color.h @@ -9,6 +9,7 @@ namespace flutter { +// These should match the enumerations defined in //lib/ui/painting.dart. enum class DlColorSpace { kSRGB = 0, kExtendedSRGB = 1, kDisplayP3 = 2 }; /// A representation of a color.