[rust png] Parse orientation from the eXIf chunk.

This CL parses the `eXIf` data reported by the `png` crate and exposes the image orientation (`SkEncodedOrigin`) through `SkCodec`'s public API. This CL depends on `png` crate populating/reporting the decoded `eXIf` chunk's data - this is being implemented in image-rs/image-png#568 Bug: chromium:390707316 Change-Id: Ieaf05c772e7a74492f5312f103b132bc888806d4 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/937702 Reviewed-by: Florin Malita <[email protected]> Commit-Queue: Łukasz Anforowicz <[email protected]>
google · Jan 22, 2025 · 4ff2e3a · 4ff2e3a
1 parent 73a5aec
commit 4ff2e3a
Show file tree

Hide file tree

Showing 7 changed files with 56 additions and 5 deletions.
diff --git a/experimental/rust_png/decoder/impl/SkPngRustCodec.cpp b/experimental/rust_png/decoder/impl/SkPngRustCodec.cpp
@@ -23,6 +23,7 @@
 #include "src/base/SkAutoMalloc.h"
 #include "src/base/SkSafeMath.h"
 #include "src/codec/SkFrameHolder.h"
+#include "src/codec/SkParseEncodedOrigin.h"
 #include "src/codec/SkSwizzler.h"
 #include "src/core/SkRasterPipeline.h"
 #include "src/core/SkRasterPipelineOpList.h"
@@ -326,6 +327,18 @@ void blendAllRows(SkSpan<uint8_t> dstFrame,
     }
 }
 
+SkEncodedOrigin GetEncodedOrigin(const rust_png::Reader& reader) {
+    if (reader.has_exif_chunk()) {
+        rust::Slice<const uint8_t> rust_slice = reader.get_exif_chunk();
+        SkEncodedOrigin origin;
+        if (SkParseEncodedOrigin(rust_slice.data(), rust_slice.size(), &origin)) {
+            return origin;
+        }
+    }
+
+    return kTopLeft_SkEncodedOrigin;
+}
+
 }  // namespace
 
 // static
@@ -354,7 +367,8 @@ SkPngRustCodec::SkPngRustCodec(SkEncodedInfo&& encodedInfo,
                          // TODO(https://crbug.com/370522089): If/when `SkCodec` can
                          // avoid unnecessary rewinding, then stop "hiding" our stream
                          // from it.
-                         /* stream = */ nullptr)
+                         /* stream = */ nullptr,
+                         GetEncodedOrigin(*reader))
         , fReader(std::move(reader))
         , fPrivStream(std::move(stream))
         , fFrameHolder(encodedInfo.width(), encodedInfo.height()) {

diff --git a/experimental/rust_png/ffi/FFI.rs b/experimental/rust_png/ffi/FFI.rs
@@ -118,6 +118,8 @@ mod ffi {
             is_full_range: &mut bool,
         ) -> bool;
         fn try_get_gama(self: &Reader, gamma: &mut f32) -> bool;
+        fn has_exif_chunk(self: &Reader) -> bool;
+        fn get_exif_chunk(self: &Reader) -> &[u8];
         fn has_iccp_chunk(self: &Reader) -> bool;
         fn get_iccp_chunk(self: &Reader) -> &[u8];
         fn has_trns_chunk(self: &Reader) -> bool;
@@ -460,6 +462,17 @@ impl Reader {
         }
     }
 
+    /// Returns whether the `eXIf` chunk exists.
+    fn has_exif_chunk(&self) -> bool {
+        self.reader.info().exif_metadata.is_some()
+    }
+
+    /// Returns contents of the `eXIf` chunk.  Panics if there is no `eXIf`
+    /// chunk.
+    fn get_exif_chunk(&self) -> &[u8] {
+        self.reader.info().exif_metadata.as_ref().unwrap().as_ref()
+    }
+
     /// Returns whether the `iCCP` chunk exists.
     fn has_iccp_chunk(&self) -> bool {
         self.reader.info().icc_profile.is_some()

diff --git a/resources/images/F-exif-chunk-early.png b/resources/images/F-exif-chunk-early.png
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
@@ -7,6 +7,7 @@
 
 #include "src/codec/SkPngCodec.h"
 
+#include "include/codec/SkEncodedOrigin.h"
 #include "include/codec/SkPngChunkReader.h"
 #include "include/codec/SkPngDecoder.h"
 #include "include/core/SkData.h"
@@ -904,14 +905,18 @@ void AutoCleanPng::infoCallback(size_t idatLength) {
     this->releasePngPtrs();
 }
 
+// TODO(https://crbug.com/390707316): Consider adding handling of eXIF chunks
+// for parity with Blink.
+constexpr SkEncodedOrigin kDefaultEncodedOrigin = kTopLeft_SkEncodedOrigin;
+
 SkPngCodec::SkPngCodec(SkEncodedInfo&& encodedInfo,
                        std::unique_ptr<SkStream> stream,
                        sk_sp<SkPngCompositeChunkReader> chunkReader,
                        void* png_ptr,
                        void* info_ptr,
                        std::unique_ptr<SkStream> gainmapStream,
                        std::optional<SkGainmapInfo> gainmapInfo)
-        : SkPngCodecBase(std::move(encodedInfo), std::move(stream))
+        : SkPngCodecBase(std::move(encodedInfo), std::move(stream), kDefaultEncodedOrigin)
         , fPngChunkReader(std::move(chunkReader))
         , fPng_ptr(png_ptr)
         , fInfo_ptr(info_ptr)

diff --git a/src/codec/SkPngCodecBase.cpp b/src/codec/SkPngCodecBase.cpp
@@ -77,8 +77,10 @@ bool SkPngCodecBase::isCompatibleColorProfileAndType(const SkEncodedInfo::ICCPro
     return true;
 }
 
-SkPngCodecBase::SkPngCodecBase(SkEncodedInfo&& encodedInfo, std::unique_ptr<SkStream> stream)
-        : SkCodec(std::move(encodedInfo), ToPixelFormat(encodedInfo), std::move(stream)) {}
+SkPngCodecBase::SkPngCodecBase(SkEncodedInfo&& encodedInfo,
+                               std::unique_ptr<SkStream> stream,
+                               SkEncodedOrigin origin)
+        : SkCodec(std::move(encodedInfo), ToPixelFormat(encodedInfo), std::move(stream), origin) {}
 
 SkEncodedImageFormat SkPngCodecBase::onGetEncodedFormat() const {
     return SkEncodedImageFormat::kPNG;

diff --git a/src/codec/SkPngCodecBase.h b/src/codec/SkPngCodecBase.h
@@ -13,6 +13,7 @@
 #include <optional>
 
 #include "include/codec/SkCodec.h"
+#include "include/codec/SkEncodedOrigin.h"
 #include "include/core/SkImageInfo.h"
 #include "include/core/SkRefCnt.h"
 #include "include/private/SkEncodedInfo.h"
@@ -35,7 +36,7 @@ class SkPngCodecBase : public SkCodec {
     static bool isCompatibleColorProfileAndType(const SkEncodedInfo::ICCProfile* profile,
                                                 SkEncodedInfo::Color color);
 protected:
-    SkPngCodecBase(SkEncodedInfo&&, std::unique_ptr<SkStream>);
+    SkPngCodecBase(SkEncodedInfo&&, std::unique_ptr<SkStream>, SkEncodedOrigin origin);
 
     // Initialize most fields needed by `applyXformRow`.
     //

diff --git a/tests/SkPngRustDecoderTest.cpp b/tests/SkPngRustDecoderTest.cpp
@@ -660,3 +660,19 @@ DEF_TEST(RustPngCodec_green15x15, r) {
     const SkColor kExpectedColor = SkColorSetARGB(0xFF, 0x00, 0x80, 0x00);
     AssertPixelColor(r, pixmap, 0, 0, kExpectedColor, "Expecting a dark green pixel");
 }
+
+DEF_TEST(RustPngCodec_exif_orientation, r) {
+    std::unique_ptr<SkCodec> codec = SkPngRustDecoderDecode(r, "images/F-exif-chunk-early.png");
+    if (!codec) {
+        return;
+    }
+
+    // TODO(https://crbug.com/390707316): After rolling the `png` crate's
+    // version `in skia/WORKSPACE` we need to change the test expectations below
+    // to the ones that are really correct: `kRightTop_SkEncodedOrigin`.  The
+    // assertion below checks for the current, incorrect behavior
+    // (`SkPngRustCodec` plumbs the `exif_metadata` from the `png` crate,
+    // but `png` crate's decoder doesn't populate the `exit_metadata` until
+    // https://github.com/image-rs/image-png/pull/568
+    REPORTER_ASSERT(r, codec->getOrigin() == kTopLeft_SkEncodedOrigin);
+}