Reland image encoder (#41632)

This is a reland of #41368 with fix for transparent images and a unit
test to verify it.

Skia would like to remove `SkImageGenerator::MakeFromEncoded` and this
appears to be the only remaining usage. It appears to be easily swapped
out for the `SkImages::DeferredFromEncodedData`. (skbug.com/13052)

This also removes the use of the internal `SkCodecImageGenerator` for
the public `SkCodec` API (which the image generator had just been
deferring to anyway).

While unbreaking some unit tests, I made a few assertions easier to
debug and produce nicer error messages.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat

Author: kjlubick

impeller/image/backends/skia/compressed_image_skia.cc

@@ -9,8 +9,9 @@
 #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/SkPixmap.h"
+#include "third_party/skia/include/core/SkRefCnt.h"
 
 namespace impeller {
 
@@ -46,12 +47,12 @@ DecompressedImage CompressedImageSkia::Decode() const {
       },
       src);
 
-  auto generator = SkImageGenerator::MakeFromEncoded(sk_data);
-  if (!generator) {
+  auto image = SkImages::DeferredFromEncodedData(sk_data);
+  if (!image) {
     return {};
   }
 
-  const auto dims = generator->getInfo().dimensions();
+  const auto dims = image->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 (!image->readPixels(nullptr, bitmap->pixmap(), 0, 0)) {
     VALIDATION_LOG << "Could not decompress image into arena.";
     return {};
   }

lib/ui/BUILD.gn

@@ -230,6 +230,7 @@ if (enable_unittests) {
       "fixtures/DisplayP3Logo.png",
       "fixtures/Horizontal.jpg",
       "fixtures/Horizontal.png",
+      "fixtures/heart_end.png",
       "fixtures/hello_loop_2.gif",
       "fixtures/hello_loop_2.webp",
       "fixtures/FontManifest.json",

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<ImageGenerator> generator =
@@ -847,27 +848,47 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) {
 
   auto descriptor = fml::MakeRefCounted<ImageDescriptor>(std::move(data),
                                                          std::move(generator));
-
-  ASSERT_EQ(ImageDecoderSkia::ImageFromCompressedData(
-                descriptor.get(), 6, 2, fml::tracing::TraceFlow(""))
-                ->dimensions(),
-            SkISize::Make(6, 2));
+  auto compressed_image = ImageDecoderSkia::ImageFromCompressedData(
+      descriptor.get(), 6, 2, fml::tracing::TraceFlow(""));
+  ASSERT_EQ(compressed_image->width(), 6);
+  ASSERT_EQ(compressed_image->height(), 2);
+  ASSERT_EQ(compressed_image->alphaType(), kOpaque_SkAlphaType);
 
 #if IMPELLER_SUPPORTS_RENDERING
   std::shared_ptr<impeller::Allocator> allocator =
       std::make_shared<impeller::TestImpellerAllocator>();
   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
 }
 
+TEST(ImageDecoderTest, ImagesWithTransparencyArePremulAlpha) {
+  auto data = OpenFixtureAsSkData("heart_end.png");
+  ASSERT_TRUE(data);
+  ImageGeneratorRegistry registry;
+  std::shared_ptr<ImageGenerator> generator =
+      registry.CreateCompatibleGenerator(data);
+  ASSERT_TRUE(generator);
+
+  auto descriptor = fml::MakeRefCounted<ImageDescriptor>(std::move(data),
+                                                         std::move(generator));
+  auto compressed_image = ImageDecoderSkia::ImageFromCompressedData(
+      descriptor.get(), 250, 250, fml::tracing::TraceFlow(""));
+  ASSERT_TRUE(compressed_image);
+  ASSERT_EQ(compressed_image->width(), 250);
+  ASSERT_EQ(compressed_image->height(), 250);
+  ASSERT_EQ(compressed_image->alphaType(), kPremul_SkAlphaType);
+}
+
 TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) {
   auto data = OpenFixtureAsSkData("Horizontal.jpg");
 
@@ -878,9 +899,15 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) {
   auto descriptor =
       fml::MakeRefCounted<ImageDescriptor>(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());
+
   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(
@@ -894,8 +921,9 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) {
 
   auto assert_image = [&](auto 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<SkData> encoded =
+        SkPngEncoder::Encode(nullptr, decoded_image.get(), {});
+    ASSERT_TRUE(encoded->equals(expected_data.get()));
   };
 
   assert_image(decode(300, 100));

lib/ui/painting/image_generator.cc

@@ -7,6 +7,8 @@
 #include <utility>
 
 #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"
 
@@ -81,34 +83,47 @@ std::unique_ptr<ImageGenerator> BuiltinSkiaImageGenerator::MakeFromGenerator(
 
 BuiltinSkiaCodecImageGenerator::~BuiltinSkiaCodecImageGenerator() = default;
 
+static SkImageInfo getInfoIncludingExif(SkCodec* codec) {
+  SkImageInfo info = codec->getInfo();
+  if (SkEncodedOriginSwapsWidthHeight(codec->getOrigin())) {
+    info = SkPixmapUtils::SwapWidthHeight(info);
+  }
+  if (kUnpremul_SkAlphaType == info.alphaType()) {
+    // Prefer premul over unpremul (this produces better filtering in general)
+    info = info.makeAlphaType(kPremul_SkAlphaType);
+  }
+  return info;
+}
+
 BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator(
     std::unique_ptr<SkCodec> codec)
-    : codec_generator_(static_cast<SkCodecImageGenerator*>(
-          SkCodecImageGenerator::MakeFromCodec(std::move(codec)).release())) {}
+    : codec_(std::move(codec)) {
+  image_info_ = getInfoIncludingExif(codec_.get());
+}
 
 BuiltinSkiaCodecImageGenerator::BuiltinSkiaCodecImageGenerator(
     sk_sp<SkData> buffer)
-    : codec_generator_(static_cast<SkCodecImageGenerator*>(
-          SkCodecImageGenerator::MakeFromEncodedCodec(std::move(buffer))
-              .release())) {}
+    : codec_(SkCodec::MakeFromData(std::move(buffer)).release()) {
+  image_info_ = getInfoIncludingExif(codec_.get());
+}
 
 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 +134,11 @@ const ImageGenerator::FrameInfo BuiltinSkiaCodecImageGenerator::GetFrameInfo(
 
 SkISize BuiltinSkiaCodecImageGenerator::GetScaledDimensions(
     float desired_scale) {
-  return codec_generator_->getScaledDimensions(desired_scale);
+  SkISize size = codec_->getScaledDimensions(desired_scale);
+  if (SkEncodedOriginSwapsWidthHeight(codec_->getOrigin())) {
+    std::swap(size.fWidth, size.fHeight);
+  }
+  return size;
 }
 
 bool BuiltinSkiaCodecImageGenerator::GetPixels(
@@ -133,7 +152,40 @@ bool BuiltinSkiaCodecImageGenerator::GetPixels(
   if (prior_frame.has_value()) {
     options.fPriorFrame = prior_frame.value();
   }
-  return codec_generator_->getPixels(info, pixels, row_bytes, &options);
+  SkEncodedOrigin origin = codec_->getOrigin();
+
+  SkPixmap output_pixmap(info, pixels, row_bytes);
+  SkPixmap temp_pixmap;
+  SkBitmap temp_bitmap;
+  if (origin == kTopLeft_SkEncodedOrigin) {
+    // 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 temp_info = output_pixmap.info();
+    if (SkEncodedOriginSwapsWidthHeight(origin)) {
+      // We'll be decoding into a buffer that has height and width swapped.
+      temp_info = SkPixmapUtils::SwapWidthHeight(temp_info);
+    }
+    if (!temp_bitmap.tryAllocPixels(temp_info)) {
+      FML_DLOG(ERROR) << "Failed to allocate memory for bitmap of size "
+                      << temp_info.computeMinByteSize() << "B";
+      return false;
+    }
+    temp_pixmap = temp_bitmap.pixmap();
+  }
+
+  SkCodec::Result result = codec_->getPixels(temp_pixmap, &options);
+  if (result != SkCodec::kSuccess) {
+    FML_DLOG(WARNING) << "codec could not get pixels. "
+                      << SkCodec::ResultToString(result);
+    return false;
+  }
+  if (origin == kTopLeft_SkEncodedOrigin) {
+    return true;
+  }
+  return SkPixmapUtils::Orient(output_pixmap, temp_pixmap, origin);
 }
 
 std::unique_ptr<ImageGenerator> BuiltinSkiaCodecImageGenerator::MakeFromData(

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/src/codec/SkCodecImageGenerator.h"  // nogncheck
 
 namespace flutter {
 
@@ -213,7 +213,8 @@ class BuiltinSkiaCodecImageGenerator : public ImageGenerator {
 
  private:
   FML_DISALLOW_COPY_ASSIGN_AND_MOVE(BuiltinSkiaCodecImageGenerator);
-  std::unique_ptr<SkCodecImageGenerator> codec_generator_;
+  std::unique_ptr<SkCodec> codec_;
+  SkImageInfo image_info_;
 };
 
 }  // namespace flutter