From 37979245d1f7535aa974d63a259755a9b2c2b7c2 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Thu, 20 Apr 2023 11:04:46 -0400 Subject: [PATCH 01/12] Remove use of internal SkCodecImageGenerator and SkImageGenerator::MakeFromEncoded --- .../backends/skia/compressed_image_skia.cc | 11 +++++---- lib/ui/painting/image_generator.cc | 23 ++++++++++--------- lib/ui/painting/image_generator.h | 5 ++-- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/impeller/image/backends/skia/compressed_image_skia.cc b/impeller/image/backends/skia/compressed_image_skia.cc index 49630583446f2..87bad49ac8749 100644 --- a/impeller/image/backends/skia/compressed_image_skia.cc +++ b/impeller/image/backends/skia/compressed_image_skia.cc @@ -9,7 +9,8 @@ #include "impeller/base/validation.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkData.h" -#include "third_party/skia/include/core/SkImageGenerator.h" +#include "third_party/skia/include/core/SkImage.h" +#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkPixmap.h" namespace impeller { @@ -46,12 +47,12 @@ DecompressedImage CompressedImageSkia::Decode() const { }, src); - auto generator = SkImageGenerator::MakeFromEncoded(sk_data); - if (!generator) { + auto img = SkImages::DeferredFromEncodedData(sk_data); + if (!img) { return {}; } - const auto dims = generator->getInfo().dimensions(); + const auto dims = img->imageInfo().dimensions(); auto info = SkImageInfo::Make(dims.width(), dims.height(), kRGBA_8888_SkColorType, kPremul_SkAlphaType); @@ -61,7 +62,7 @@ DecompressedImage CompressedImageSkia::Decode() const { return {}; } - if (!generator->getPixels(bitmap->pixmap())) { + if (!img->readPixels(nullptr, bitmap->pixmap(), 0, 0)) { VALIDATION_LOG << "Could not decompress image into arena."; return {}; } diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index c86e53ac4fd97..52dbc303f3873 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -83,32 +83,33 @@ BuiltinSkiaCodecImageGenerator::~BuiltinSkiaCodecImageGenerator() = default; BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( std::unique_ptr codec) - : codec_generator_(static_cast( - SkCodecImageGenerator::MakeFromCodec(std::move(codec)).release())) {} + : codec_(std::move(codec)) { + image_info_ = codec_->getInfo(); +} BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( sk_sp buffer) - : codec_generator_(static_cast( - SkCodecImageGenerator::MakeFromEncodedCodec(std::move(buffer)) - .release())) {} + : codec_(SkCodec::MakeFromData(std::move(buffer)).release()) { + image_info_ = codec_->getInfo(); +} const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() { - return codec_generator_->getInfo(); + return image_info_; } unsigned int BuiltinSkiaCodecImageGenerator::GetFrameCount() const { - return codec_generator_->getFrameCount(); + return codec_->getFrameCount(); } unsigned int BuiltinSkiaCodecImageGenerator::GetPlayCount() const { - auto repetition_count = codec_generator_->getRepetitionCount(); + auto repetition_count = codec_->getRepetitionCount(); return repetition_count < 0 ? kInfinitePlayCount : repetition_count + 1; } const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo( unsigned int frame_index) { SkCodec::FrameInfo info = {}; - codec_generator_->getFrameInfo(frame_index, &info); + codec_->getFrameInfo(frame_index, &info); return { .required_frame = info.fRequiredFrame == SkCodec::kNoFrame ? std::nullopt @@ -119,7 +120,7 @@ const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo( SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( float desired_scale) { - return codec_generator_->getScaledDimensions(desired_scale); + return codec_->getScaledDimensions(desired_scale); } bool BuiltinSkiaCodecImageGenerator::GetPixels( @@ -133,7 +134,7 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( if (prior_frame.has_value()) { options.fPriorFrame = prior_frame.value(); } - return codec_generator_->getPixels(info, pixels, row_bytes, &options); + return codec_->getPixels(info, pixels, row_bytes, &options); } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h index a5515c8057319..d88d3a3356864 100644 --- a/lib/ui/painting/image_generator.h +++ b/lib/ui/painting/image_generator.h @@ -13,7 +13,7 @@ #include "third_party/skia/include/core/SkImage.h" #include "third_party/skia/include/core/SkImageInfo.h" #include "third_party/skia/include/core/SkSize.h" -#include "third_party/skia/src/codec/SkCodecImageGenerator.h" // nogncheck +#include "third_party/skia/include/core/SkImageGenerator.h" namespace flutter { @@ -213,7 +213,8 @@ class BuiltinSkiaCodecImageGenerator : public ImageGenerator { private: FML_DISALLOW_COPY_ASSIGN_AND_MOVE(BuiltinSkiaCodecImageGenerator); - std::unique_ptr codec_generator_; + std::unique_ptr codec_; + SkImageInfo image_info_; }; } // namespace flutter From 314a2c7029a312011d32d95945d5bd779ac85b88 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Thu, 20 Apr 2023 11:05:21 -0400 Subject: [PATCH 02/12] format --- impeller/image/backends/skia/compressed_image_skia.cc | 2 +- lib/ui/painting/image_generator.cc | 4 ++-- lib/ui/painting/image_generator.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/impeller/image/backends/skia/compressed_image_skia.cc b/impeller/image/backends/skia/compressed_image_skia.cc index 87bad49ac8749..ecaa6f9440dbe 100644 --- a/impeller/image/backends/skia/compressed_image_skia.cc +++ b/impeller/image/backends/skia/compressed_image_skia.cc @@ -10,8 +10,8 @@ #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkData.h" #include "third_party/skia/include/core/SkImage.h" -#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/skia/include/core/SkPixmap.h" +#include "third_party/skia/include/core/SkRefCnt.h" namespace impeller { diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 52dbc303f3873..bf5127ce50bb9 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -84,13 +84,13 @@ BuiltinSkiaCodecImageGenerator::~BuiltinSkiaCodecImageGenerator() = default; BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( std::unique_ptr codec) : codec_(std::move(codec)) { - image_info_ = codec_->getInfo(); + image_info_ = codec_->getInfo(); } BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( sk_sp buffer) : codec_(SkCodec::MakeFromData(std::move(buffer)).release()) { - image_info_ = codec_->getInfo(); + image_info_ = codec_->getInfo(); } const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() { diff --git a/lib/ui/painting/image_generator.h b/lib/ui/painting/image_generator.h index d88d3a3356864..dcfe4b1f069c7 100644 --- a/lib/ui/painting/image_generator.h +++ b/lib/ui/painting/image_generator.h @@ -11,9 +11,9 @@ #include "third_party/skia/include/codec/SkCodecAnimation.h" #include "third_party/skia/include/core/SkData.h" #include "third_party/skia/include/core/SkImage.h" +#include "third_party/skia/include/core/SkImageGenerator.h" #include "third_party/skia/include/core/SkImageInfo.h" #include "third_party/skia/include/core/SkSize.h" -#include "third_party/skia/include/core/SkImageGenerator.h" namespace flutter { From 81610472ff8c53f4fde419beca336c3c3bfc097d Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Thu, 20 Apr 2023 13:12:54 -0400 Subject: [PATCH 03/12] handle success better --- lib/ui/painting/image_generator.cc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index bf5127ce50bb9..e77e79d9a1861 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -134,7 +134,15 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( if (prior_frame.has_value()) { options.fPriorFrame = prior_frame.value(); } - return codec_->getPixels(info, pixels, row_bytes, &options); + SkCodec::Result result = codec_->getPixels(info, pixels, row_bytes, &options); + switch (result) { + case SkCodec::kSuccess: + case SkCodec::kIncompleteInput: + case SkCodec::kErrorInInput: + return true; + default: + return false; + } } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( From 3f277da7d2f97eedd15ff80bc11ba7ddabd4dde0 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 24 Apr 2023 08:45:55 -0400 Subject: [PATCH 04/12] trying skpixmaputils --- lib/ui/painting/image_generator.cc | 33 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index e77e79d9a1861..3dce646fa01c7 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -7,8 +7,10 @@ #include #include "flutter/fml/logging.h" +#include "third_party/skia/include/codec/SkEncodedOrigin.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkImage.h" +#include "third_party/skia/include/core/SkPixmapUtils.h" namespace flutter { @@ -134,15 +136,32 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( if (prior_frame.has_value()) { options.fPriorFrame = prior_frame.value(); } - SkCodec::Result result = codec_->getPixels(info, pixels, row_bytes, &options); - switch (result) { - case SkCodec::kSuccess: - case SkCodec::kIncompleteInput: - case SkCodec::kErrorInInput: - return true; - default: + SkEncodedOrigin origin = codec_->getOrigin(); + + SkPixmap dst(info, pixels, row_bytes); + SkPixmap tmp; + SkBitmap tmpBitmap; + if (origin == kTopLeft_SkEncodedOrigin) { + tmp = dst; // we can decode directly into the output buffer + } else { + // We need to decode into a different buffer so we can re-orient + // the pixels later. + if (!tmpBitmap.tryAllocPixels(info)) { + FML_DLOG(ERROR) << "Failed to allocate memory for bitmap of size " + << info.computeMinByteSize() << "B"; return false; + } + tmp = tmpBitmap.pixmap(); } + + SkCodec::Result result = codec_->getPixels(tmp, &options); + if (result != SkCodec::kSuccess) { + return false; + } + if (origin == kTopLeft_SkEncodedOrigin) { + return true; + } + return SkPixmapUtils::Orient(dst, tmp, origin); } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( From 361587c238ebebdf1433e5b0f7cffe27a62348d3 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 24 Apr 2023 09:01:51 -0400 Subject: [PATCH 05/12] Use SkPixmapUtils to orient image if necessary --- lib/ui/painting/image_generator.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 3dce646fa01c7..30264a111fd42 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -8,9 +8,9 @@ #include "flutter/fml/logging.h" #include "third_party/skia/include/codec/SkEncodedOrigin.h" +#include "third_party/skia/include/codec/SkPixmapUtils.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkImage.h" -#include "third_party/skia/include/core/SkPixmapUtils.h" namespace flutter { @@ -142,7 +142,7 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( SkPixmap tmp; SkBitmap tmpBitmap; if (origin == kTopLeft_SkEncodedOrigin) { - tmp = dst; // we can decode directly into the output buffer + tmp = dst; // we can decode directly into the output buffer } else { // We need to decode into a different buffer so we can re-orient // the pixels later. @@ -156,7 +156,7 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( SkCodec::Result result = codec_->getPixels(tmp, &options); if (result != SkCodec::kSuccess) { - return false; + return false; } if (origin == kTopLeft_SkEncodedOrigin) { return true; From 598f3a329e876f41d56967544f77d05cfde43649 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 24 Apr 2023 14:53:22 -0400 Subject: [PATCH 06/12] debugging --- lib/ui/painting/image_decoder_unittests.cc | 7 ++++--- lib/ui/painting/image_generator.cc | 14 +++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 571b96c7d4a84..05e79323ab1c6 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -892,10 +892,11 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { ASSERT_TRUE(expected_data != nullptr); ASSERT_FALSE(expected_data->isEmpty()); - auto assert_image = [&](auto decoded_image) { + auto assert_image = [&](sk_sp decoded_image) { ASSERT_EQ(decoded_image->dimensions(), SkISize::Make(300, 100)); - ASSERT_TRUE(SkPngEncoder::Encode(nullptr, decoded_image.get(), {}) - ->equals(expected_data.get())); + sk_sp encoded = SkPngEncoder::Encode(nullptr, decoded_image.get(), + {}); + ASSERT_TRUE(encoded->equals(expected_data.get())); }; assert_image(decode(300, 100)); diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 30264a111fd42..e0d8fd5b32f88 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -146,9 +146,14 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( } else { // We need to decode into a different buffer so we can re-orient // the pixels later. - if (!tmpBitmap.tryAllocPixels(info)) { + SkImageInfo tmpInfo = dst.info(); + // if (SkEncodedOriginSwapsWidthHeight(origin)) { + // // We'll be decoding into a buffer that has height and width swapped. + // tmpInfo = SkPixmapUtils::SwapWidthHeight(tmpInfo); + // } + if (!tmpBitmap.tryAllocPixels(tmpInfo)) { FML_DLOG(ERROR) << "Failed to allocate memory for bitmap of size " - << info.computeMinByteSize() << "B"; + << tmpInfo.computeMinByteSize() << "B"; return false; } tmp = tmpBitmap.pixmap(); @@ -156,12 +161,15 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( SkCodec::Result result = codec_->getPixels(tmp, &options); if (result != SkCodec::kSuccess) { + FML_DLOG(WARNING) << "codec could not get pixels. status " << result; return false; } if (origin == kTopLeft_SkEncodedOrigin) { return true; } - return SkPixmapUtils::Orient(dst, tmp, origin); + FML_DLOG(WARNING) << "Origin " << origin; + return true; + //return SkPixmapUtils::Orient(dst, tmp, origin); } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( From 671860e1105eb8980df5cd837f4f345052e300bf Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 24 Apr 2023 15:40:18 -0400 Subject: [PATCH 07/12] more asserts --- lib/ui/painting/image_decoder_unittests.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 05e79323ab1c6..b5e6ae3aade3b 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -878,9 +878,13 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { auto descriptor = fml::MakeRefCounted(data, std::move(generator)); + ASSERT_EQ(600, descriptor->width()); + ASSERT_EQ(200, descriptor->height()); + auto image = SkImages::DeferredFromEncodedData(data); ASSERT_TRUE(image != nullptr); - ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); + ASSERT_EQ(600, image->width()); + ASSERT_EQ(200, image->height()); auto decode = [descriptor](uint32_t target_width, uint32_t target_height) { return ImageDecoderSkia::ImageFromCompressedData( From b9926080a7f1da3ba53bd84df248405b7c8d0d28 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 24 Apr 2023 16:14:28 -0400 Subject: [PATCH 08/12] try swapping --- lib/ui/painting/image_decoder_unittests.cc | 19 ++++++++++------- lib/ui/painting/image_generator.cc | 24 ++++++++++++++-------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index b5e6ae3aade3b..11440e73b013a 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -838,7 +838,8 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto data = OpenFixtureAsSkData("Horizontal.jpg"); auto image = SkImages::DeferredFromEncodedData(data); ASSERT_TRUE(image != nullptr); - ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); + ASSERT_EQ(600, image->width()); + ASSERT_EQ(200, image->height()); ImageGeneratorRegistry registry; std::shared_ptr generator = @@ -848,10 +849,10 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto descriptor = fml::MakeRefCounted(std::move(data), std::move(generator)); - ASSERT_EQ(ImageDecoderSkia::ImageFromCompressedData( - descriptor.get(), 6, 2, fml::tracing::TraceFlow("")) - ->dimensions(), - SkISize::Make(6, 2)); + auto compressed_img = ImageDecoderSkia::ImageFromCompressedData( + descriptor.get(), 6, 2, fml::tracing::TraceFlow("")); + ASSERT_EQ(compressed_img->width(), 6); + ASSERT_EQ(compressed_img->height(), 2); #if IMPELLER_SUPPORTS_RENDERING std::shared_ptr allocator = @@ -859,12 +860,14 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto result_1 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(6, 2), {100, 100}, /*supports_wide_gamut=*/false, allocator); - ASSERT_EQ(result_1->sk_bitmap->dimensions(), SkISize::Make(6, 2)); + ASSERT_EQ(result_1->sk_bitmap->width(), 6); + ASSERT_EQ(result_1->sk_bitmap->height(), 2); auto result_2 = ImageDecoderImpeller::DecompressTexture( descriptor.get(), SkISize::Make(60, 20), {10, 10}, /*supports_wide_gamut=*/false, allocator); - ASSERT_EQ(result_2->sk_bitmap->dimensions(), SkISize::Make(10, 10)); + ASSERT_EQ(result_2->sk_bitmap->width(), 10); + ASSERT_EQ(result_2->sk_bitmap->height(), 10); #endif // IMPELLER_SUPPORTS_RENDERING } @@ -878,6 +881,8 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { auto descriptor = fml::MakeRefCounted(data, std::move(generator)); + // If Exif metadata is ignored, the height and width will be swapped because + // "Rotate 90 CW" is what is encoded there. ASSERT_EQ(600, descriptor->width()); ASSERT_EQ(200, descriptor->height()); diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index e0d8fd5b32f88..4dd671a3bc897 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -83,16 +83,24 @@ std::unique_ptr BuiltinSkiaImageGenerator::MakeFromGenerator( BuiltinSkiaCodecImageGenerator::~BuiltinSkiaCodecImageGenerator() = default; +static SkImageInfo getInfoIncludingExif(SkCodec* codec) { + SkImageInfo info = codec->getInfo(); + if (SkEncodedOriginSwapsWidthHeight(codec->getOrigin())) { + info = SkPixmapUtils::SwapWidthHeight(info); + } + return info; +} + BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( std::unique_ptr codec) : codec_(std::move(codec)) { - image_info_ = codec_->getInfo(); + image_info_ = getInfoIncludingExif(codec_.get()); } BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator( sk_sp buffer) : codec_(SkCodec::MakeFromData(std::move(buffer)).release()) { - image_info_ = codec_->getInfo(); + image_info_ = getInfoIncludingExif(codec_.get()); } const SkImageInfo& BuiltinSkiaCodecImageGenerator::GetInfo() { @@ -147,10 +155,10 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( // We need to decode into a different buffer so we can re-orient // the pixels later. SkImageInfo tmpInfo = dst.info(); - // if (SkEncodedOriginSwapsWidthHeight(origin)) { - // // We'll be decoding into a buffer that has height and width swapped. - // tmpInfo = SkPixmapUtils::SwapWidthHeight(tmpInfo); - // } + if (SkEncodedOriginSwapsWidthHeight(origin)) { + // We'll be decoding into a buffer that has height and width swapped. + tmpInfo = SkPixmapUtils::SwapWidthHeight(tmpInfo); + } if (!tmpBitmap.tryAllocPixels(tmpInfo)) { FML_DLOG(ERROR) << "Failed to allocate memory for bitmap of size " << tmpInfo.computeMinByteSize() << "B"; @@ -167,9 +175,7 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( if (origin == kTopLeft_SkEncodedOrigin) { return true; } - FML_DLOG(WARNING) << "Origin " << origin; - return true; - //return SkPixmapUtils::Orient(dst, tmp, origin); + return SkPixmapUtils::Orient(dst, tmp, origin); } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( From c6f1166daf4ea1d91be326cf30662ea2f2278b6e Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Mon, 24 Apr 2023 16:29:18 -0400 Subject: [PATCH 09/12] tests pass --- lib/ui/painting/image_decoder_unittests.cc | 1 - lib/ui/painting/image_generator.cc | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 11440e73b013a..52f35227e06a5 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -848,7 +848,6 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto descriptor = fml::MakeRefCounted(std::move(data), std::move(generator)); - auto compressed_img = ImageDecoderSkia::ImageFromCompressedData( descriptor.get(), 6, 2, fml::tracing::TraceFlow("")); ASSERT_EQ(compressed_img->width(), 6); diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 4dd671a3bc897..6beb7bb0ee946 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -130,7 +130,11 @@ const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo( SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions( float desired_scale) { - return codec_->getScaledDimensions(desired_scale); + SkISize size = codec_->getScaledDimensions(desired_scale); + if (SkEncodedOriginSwapsWidthHeight(codec_->getOrigin())) { + std::swap(size.fWidth, size.fHeight); + } + return size; } bool BuiltinSkiaCodecImageGenerator::GetPixels( @@ -169,7 +173,7 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( SkCodec::Result result = codec_->getPixels(tmp, &options); if (result != SkCodec::kSuccess) { - FML_DLOG(WARNING) << "codec could not get pixels. status " << result; + FML_DLOG(WARNING) << "codec could not get pixels. " << SkCodec::ResultToString(result); return false; } if (origin == kTopLeft_SkEncodedOrigin) { From 1a0be70e3c8302293704f834a9684dd2b188ca32 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Tue, 25 Apr 2023 07:26:33 -0400 Subject: [PATCH 10/12] clang tidy --- lib/ui/painting/image_decoder_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 596e217519eb8..a3b2ff92fcd60 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -900,7 +900,7 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { ASSERT_TRUE(expected_data != nullptr); ASSERT_FALSE(expected_data->isEmpty()); - auto assert_image = [&](sk_sp decoded_image) { + auto assert_image = [&](auto decoded_image) { ASSERT_EQ(decoded_image->dimensions(), SkISize::Make(300, 100)); sk_sp encoded = SkPngEncoder::Encode(nullptr, decoded_image.get(), {}); From 7696cd698ac28ee91b72f3ddac10347b882683cb Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Thu, 27 Apr 2023 13:33:08 -0400 Subject: [PATCH 11/12] address feedback --- .../backends/skia/compressed_image_skia.cc | 8 +++---- lib/ui/painting/image_generator.cc | 23 ++++++++++--------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/impeller/image/backends/skia/compressed_image_skia.cc b/impeller/image/backends/skia/compressed_image_skia.cc index ecaa6f9440dbe..66d917e4ab8b7 100644 --- a/impeller/image/backends/skia/compressed_image_skia.cc +++ b/impeller/image/backends/skia/compressed_image_skia.cc @@ -47,12 +47,12 @@ DecompressedImage CompressedImageSkia::Decode() const { }, src); - auto img = SkImages::DeferredFromEncodedData(sk_data); - if (!img) { + auto image = SkImages::DeferredFromEncodedData(sk_data); + if (!image) { return {}; } - const auto dims = img->imageInfo().dimensions(); + const auto dims = image->imageInfo().dimensions(); auto info = SkImageInfo::Make(dims.width(), dims.height(), kRGBA_8888_SkColorType, kPremul_SkAlphaType); @@ -62,7 +62,7 @@ DecompressedImage CompressedImageSkia::Decode() const { return {}; } - if (!img->readPixels(nullptr, bitmap->pixmap(), 0, 0)) { + if (!image->readPixels(nullptr, bitmap->pixmap(), 0, 0)) { VALIDATION_LOG << "Could not decompress image into arena."; return {}; } diff --git a/lib/ui/painting/image_generator.cc b/lib/ui/painting/image_generator.cc index 1c71067671793..949cfc9648605 100644 --- a/lib/ui/painting/image_generator.cc +++ b/lib/ui/painting/image_generator.cc @@ -150,28 +150,29 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( } SkEncodedOrigin origin = codec_->getOrigin(); - SkPixmap dst(info, pixels, row_bytes); - SkPixmap tmp; - SkBitmap tmpBitmap; + SkPixmap output_pixmap(info, pixels, row_bytes); + SkPixmap temp_pixmap; + SkBitmap temp_bitmap; if (origin == kTopLeft_SkEncodedOrigin) { - tmp = dst; // we can decode directly into the output buffer + // We can decode directly into the output buffer. + temp_pixmap = output_pixmap; } else { // We need to decode into a different buffer so we can re-orient // the pixels later. - SkImageInfo tmpInfo = dst.info(); + SkImageInfo temp_info = output_pixmap.info(); if (SkEncodedOriginSwapsWidthHeight(origin)) { // We'll be decoding into a buffer that has height and width swapped. - tmpInfo = SkPixmapUtils::SwapWidthHeight(tmpInfo); + temp_info = SkPixmapUtils::SwapWidthHeight(temp_info); } - if (!tmpBitmap.tryAllocPixels(tmpInfo)) { + if (!temp_bitmap.tryAllocPixels(temp_info)) { FML_DLOG(ERROR) << "Failed to allocate memory for bitmap of size " - << tmpInfo.computeMinByteSize() << "B"; + << temp_info.computeMinByteSize() << "B"; return false; } - tmp = tmpBitmap.pixmap(); + temp_pixmap = temp_bitmap.pixmap(); } - SkCodec::Result result = codec_->getPixels(tmp, &options); + SkCodec::Result result = codec_->getPixels(temp_pixmap, &options); if (result != SkCodec::kSuccess) { FML_DLOG(WARNING) << "codec could not get pixels. " << SkCodec::ResultToString(result); @@ -180,7 +181,7 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels( if (origin == kTopLeft_SkEncodedOrigin) { return true; } - return SkPixmapUtils::Orient(dst, tmp, origin); + return SkPixmapUtils::Orient(output_pixmap, temp_pixmap, origin); } std::unique_ptr BuiltinSkiaCodecImageGenerator::MakeFromData( From db7c21a36289fdc9edfab724e59772790e1bcb1f Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Thu, 27 Apr 2023 13:36:11 -0400 Subject: [PATCH 12/12] one more img --- lib/ui/painting/image_decoder_unittests.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index a3b2ff92fcd60..5ddee5b49bd11 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -848,10 +848,10 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { auto descriptor = fml::MakeRefCounted(std::move(data), std::move(generator)); - auto compressed_img = ImageDecoderSkia::ImageFromCompressedData( + auto compressed_image = ImageDecoderSkia::ImageFromCompressedData( descriptor.get(), 6, 2, fml::tracing::TraceFlow("")); - ASSERT_EQ(compressed_img->width(), 6); - ASSERT_EQ(compressed_img->height(), 2); + ASSERT_EQ(compressed_image->width(), 6); + ASSERT_EQ(compressed_image->height(), 2); #if IMPELLER_SUPPORTS_RENDERING std::shared_ptr allocator =