From 42b63c3f1d791917d5aa6d4c262ee3d95bf4d2da Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 21 Feb 2023 10:05:03 -0800 Subject: [PATCH 1/2] Updated the wide-gamut constant and added a unit test for it. --- lib/ui/painting/image_decoder_impeller.cc | 7 +++++-- lib/ui/painting/image_decoder_unittests.cc | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 4b6deac07166b..b412da4e19825 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -49,7 +49,9 @@ float CalculateArea(SkPoint abc[3]) { b.fX * a.fY); } -static constexpr float kSrgbD50GamutArea = 0.084f; +// Note: This is defined differently in Skia, in my testing this is the correct +// area and was calculated from SkColorSpace::MakeSRGB(). +static constexpr float kSrgbGamutArea = 0.0982f; // Source: // https://source.chromium.org/chromium/_/skia/skia.git/+/393fb1ec80f41d8ad7d104921b6920e69749fda1:src/codec/SkAndroidCodec.cpp;l=67;drc=46572b4d445f41943059d0e377afc6d6748cd5ca;bpv=1;bpt=0 @@ -58,7 +60,8 @@ bool IsWideGamut(const SkColorSpace& color_space) { color_space.toXYZD50(&xyzd50); SkPoint rgb[3]; LoadGamut(rgb, xyzd50); - return CalculateArea(rgb) > kSrgbD50GamutArea; + float area = CalculateArea(rgb); + return area > kSrgbGamutArea; } } // namespace diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index a9b09c559af08..947239321cf22 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -333,6 +333,28 @@ TEST_F(ImageDecoderFixtureTest, ImpellerWideGamutDisplayP3) { #endif // IMPELLER_SUPPORTS_RENDERING } +TEST_F(ImageDecoderFixtureTest, ImpellerNonWideGamut) { + auto data = OpenFixtureAsSkData("Horizontal.jpg"); + auto image = SkImage::MakeFromEncoded(data); + ASSERT_TRUE(image != nullptr); + ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); + + ImageGeneratorRegistry registry; + std::shared_ptr generator = + registry.CreateCompatibleGenerator(data); + ASSERT_TRUE(generator); + + auto descriptor = fml::MakeRefCounted(std::move(data), + std::move(generator)); + +#if IMPELLER_SUPPORTS_RENDERING + std::shared_ptr bitmap = ImageDecoderImpeller::DecompressTexture( + descriptor.get(), SkISize::Make(600, 200), {600, 200}, + /*supports_wide_gamut=*/true); + ASSERT_EQ(bitmap->colorType(), kRGBA_8888_SkColorType); +#endif // IMPELLER_SUPPORTS_RENDERING +} + TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) { auto loop = fml::ConcurrentMessageLoop::Create(); TaskRunners runners(GetCurrentTestName(), // label From 88862adce6f5c05bbb313316a4ee74da8434139b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 21 Feb 2023 15:23:44 -0800 Subject: [PATCH 2/2] dan feedback --- lib/ui/painting/image_decoder_impeller.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index b412da4e19825..73e0888108776 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -49,8 +49,7 @@ float CalculateArea(SkPoint abc[3]) { b.fX * a.fY); } -// Note: This is defined differently in Skia, in my testing this is the correct -// area and was calculated from SkColorSpace::MakeSRGB(). +// Note: This was calculated from SkColorSpace::MakeSRGB(). static constexpr float kSrgbGamutArea = 0.0982f; // Source: