Respect EXIF information while decompressing images. (flutter#9905)

Adds a unit-test asserting this behavior.
johnsonmh · Jul 18, 2019 · fd2cb81 · fd2cb81
1 parent dd06cda
commit fd2cb81
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 53 deletions.
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -332,6 +332,7 @@ FILE: ../../../flutter/lib/ui/dart_ui.cc
 FILE: ../../../flutter/lib/ui/dart_ui.h
 FILE: ../../../flutter/lib/ui/dart_wrapper.h
 FILE: ../../../flutter/lib/ui/fixtures/DashInNooglerHat.jpg
+FILE: ../../../flutter/lib/ui/fixtures/Horizontal.jpg
 FILE: ../../../flutter/lib/ui/fixtures/ui_test.dart
 FILE: ../../../flutter/lib/ui/geometry.dart
 FILE: ../../../flutter/lib/ui/hash_codes.dart

diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn
@@ -151,7 +151,10 @@ source_set("ui") {
 if (current_toolchain == host_toolchain) {
   test_fixtures("ui_unittests_fixtures") {
     dart_main = "fixtures/ui_test.dart"
-    fixtures = [ "fixtures/DashInNooglerHat.jpg" ]
+    fixtures = [
+      "fixtures/DashInNooglerHat.jpg",
+      "fixtures/Horizontal.jpg",
+    ]
   }
 
   executable("ui_unittests") {

diff --git a/lib/ui/fixtures/Horizontal.jpg b/lib/ui/fixtures/Horizontal.jpg
diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc
@@ -133,62 +133,14 @@ static sk_sp<SkImage> ImageFromCompressedData(
   TRACE_EVENT0("flutter", __FUNCTION__);
   flow.Step(__FUNCTION__);
 
-  auto codec = SkCodec::MakeFromData(data);
+  auto decoded_image = SkImage::MakeFromEncoded(data);
 
-  if (!codec) {
-    return nullptr;
-  }
-
-  const auto encoded_info = codec->getInfo();
-
-  if (encoded_info.dimensions().isEmpty()) {
-    return nullptr;
-  }
-
-  const double desired_width =
-      target_width.value_or(encoded_info.dimensions().width());
-  const double desired_height =
-      target_height.value_or(encoded_info.dimensions().height());
-
-  const auto scale_x = desired_width / encoded_info.dimensions().width();
-  const auto scale_y = desired_height / encoded_info.dimensions().height();
-
-  const double scale = std::min({scale_x, scale_y, 1.0});
-
-  if (scale <= 0.0) {
-    return nullptr;
-  }
-
-  // We ask the codec for one of the natively supported dimensions. This may not
-  // be exactly what we need, but it will also be smaller than 1:1. We will
-  // still have to perform a resize, but from a smaller intermediate buffer. In
-  // case no resize needs to be performed, ResizeRasterImage will just return
-  // the original image.
-  const auto scaled_dimensions = codec->getScaledDimensions(scale);
-
-  if (scaled_dimensions.isEmpty()) {
-    return nullptr;
-  }
-
-  const auto decoded_info = encoded_info.makeWH(scaled_dimensions.width(),
-                                                scaled_dimensions.height());
-
-  SkBitmap decoded_bitmap;
-  if (!decoded_bitmap.tryAllocPixels(decoded_info)) {
-    FML_LOG(ERROR) << "Could not perform allocation for image decoding.";
-    return nullptr;
-  }
-
-  const auto decompression_result = codec->getPixels(decoded_bitmap.pixmap());
-  if (decompression_result != SkCodec::Result::kSuccess) {
-    FML_LOG(ERROR) << "Could not perform image decompression. Error: "
-                   << SkCodec::ResultToString(decompression_result);
+  if (!decoded_image) {
     return nullptr;
   }
 
-  decoded_bitmap.setImmutable();
-
-  auto decoded_image = SkImage::MakeFromBitmap(decoded_bitmap);
+  // Make sure to resolve all lazy images.
+  decoded_image = decoded_image->makeRasterImage();
 
   if (!decoded_image) {
     return nullptr;

diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc
@@ -195,6 +195,53 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) {
   latch.Wait();
 }
 
+TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) {
+  auto loop = fml::ConcurrentMessageLoop::Create();
+  TaskRunners runners(GetCurrentTestName(),    // label
+                      GetThreadTaskRunner(),   // platform
+                      CreateNewThread("gpu"),  // gpu
+                      CreateNewThread("ui"),   // ui
+                      CreateNewThread("io")    // io
+  );
+
+  fml::AutoResetWaitableEvent latch;
+
+  std::unique_ptr<IOManager> io_manager;
+  std::unique_ptr<ImageDecoder> image_decoder;
+
+  SkISize decoded_size = SkISize::MakeEmpty();
+  auto decode_image = [&]() {
+    image_decoder = std::make_unique<ImageDecoder>(
+        runners, loop->GetTaskRunner(), io_manager->GetWeakIOManager());
+
+    ImageDecoder::ImageDescriptor image_descriptor;
+    image_descriptor.data = OpenFixtureAsSkData("Horizontal.jpg");
+
+    ASSERT_TRUE(image_descriptor.data);
+    ASSERT_GE(image_descriptor.data->size(), 0u);
+
+    ImageDecoder::ImageResult callback = [&](SkiaGPUObject<SkImage> image) {
+      ASSERT_TRUE(runners.GetUITaskRunner()->RunsTasksOnCurrentThread());
+      ASSERT_TRUE(image.get());
+      decoded_size = image.get()->dimensions();
+      latch.Signal();
+    };
+    image_decoder->Decode(std::move(image_descriptor), callback);
+  };
+
+  auto setup_io_manager_and_decode = [&]() {
+    io_manager = std::make_unique<TestIOManager>(runners.GetIOTaskRunner());
+    runners.GetUITaskRunner()->PostTask(decode_image);
+  };
+
+  runners.GetIOTaskRunner()->PostTask(setup_io_manager_and_decode);
+
+  latch.Wait();
+
+  ASSERT_EQ(decoded_size.width(), 600);
+  ASSERT_EQ(decoded_size.height(), 200);
+}
+
 TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) {
   auto loop = fml::ConcurrentMessageLoop::Create();
   TaskRunners runners(GetCurrentTestName(),    // label