Make SkAndroidCodec (optionally) respect origin

Bug: b/63909536

ImageDecoder will respect the origin, but BitmapFactory will maintain
its current behavior of not respecting it. Add an option to respect it.

In addition, add support for reading the EXIF data from a WEBP. This
seems to be an uncommon use case, but is occasionally used when
converting from a JPEG. Add 8 WEBPs, all converted (with cwebp) from
their analogous JPEG files already checked in.

Change-Id: I38afca58c86fa99ee9ab7d1dc83aaa4f23132c11
Reviewed-on: https://skia-review.googlesource.com/95300
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

include/codec/SkAndroidCodec.h

@@ -19,10 +19,31 @@
  */
 class SK_API SkAndroidCodec : SkNoncopyable {
 public:
+    enum class ExifOrientationBehavior {
+        /**
+         *  Ignore any exif orientation markers in the data.
+         *
+         *  getInfo's width and height will match the header of the image, and
+         *  no processing will be done to match the marker.
+         */
+        kIgnore,
+
+        /**
+         *  Respect the exif orientation marker.
+         *
+         *  getInfo's width and height will represent what they should be after
+         *  applying the orientation. For example, if the marker specifies a
+         *  rotation by 90 degrees, they will be swapped relative to the header.
+         *  getAndroidPixels will apply the orientation as well.
+         */
+        kRespect,
+    };
+
     /**
      *  Pass ownership of an SkCodec to a newly-created SkAndroidCodec.
      */
-    static std::unique_ptr<SkAndroidCodec> MakeFromCodec(std::unique_ptr<SkCodec>);
+    static std::unique_ptr<SkAndroidCodec> MakeFromCodec(std::unique_ptr<SkCodec>,
+            ExifOrientationBehavior = ExifOrientationBehavior::kIgnore);
 
     /**
      *  If this stream represents an encoded image that we know how to decode,
@@ -33,6 +54,8 @@ public:
      *
      *  If NULL is returned, the stream is deleted immediately. Otherwise, the
      *  SkCodec takes ownership of it, and will delete it when done with it.
+     *
+     *  ExifOrientationBehavior is set to kIgnore.
      */
     static std::unique_ptr<SkAndroidCodec> MakeFromStream(std::unique_ptr<SkStream>,
                                                           SkPngChunkReader* = nullptr);
@@ -43,6 +66,8 @@ public:
      *
      *  The SkPngChunkReader handles unknown chunks in PNGs.
      *  See SkCodec.h for more details.
+     *
+     *  ExifOrientationBehavior is set to kIgnore.
      */
     static std::unique_ptr<SkAndroidCodec> MakeFromData(sk_sp<SkData>, SkPngChunkReader* = nullptr);
 
@@ -245,8 +270,7 @@ public:
     SkCodec* codec() const { return fCodec.get(); }
 
 protected:
-
+    SkAndroidCodec(SkCodec*, ExifOrientationBehavior = ExifOrientationBehavior::kIgnore);
-    SkAndroidCodec(SkCodec*);
 
     virtual SkISize onGetSampledDimensions(int sampleSize) const = 0;
 
@@ -256,11 +280,8 @@ protected:
             size_t rowBytes, const AndroidOptions& options) = 0;
 
 private:
-
+    const SkImageInfo               fInfo;
-    // This will always be a reference to the info that is contained by the
+    const ExifOrientationBehavior   fOrientationBehavior;
-    // embedded SkCodec.
+    std::unique_ptr<SkCodec>        fCodec;
-    const SkImageInfo& fInfo;
-
-    std::unique_ptr<SkCodec> fCodec;
 };
 #endif // SkAndroidCodec_DEFINED

src/codec/SkAndroidCodec.cpp

@@ -9,6 +9,8 @@
 #include "SkCodec.h"
 #include "SkCodecPriv.h"
 #include "SkMakeUnique.h"
+#include "SkPixmap.h"
+#include "SkPixmapPriv.h"
 #include "SkRawAdapterCodec.h"
 #include "SkSampledCodec.h"
 #include "SkWebpAdapterCodec.h"
@@ -57,19 +59,32 @@ static bool is_wide_gamut(const SkColorSpace* colorSpace) {
     return false;
 }
 
-SkAndroidCodec::SkAndroidCodec(SkCodec* codec)
+static inline SkImageInfo adjust_info(SkCodec* codec,
-    : fInfo(codec->getInfo())
+        SkAndroidCodec::ExifOrientationBehavior orientationBehavior) {
+    auto info = codec->getInfo();
+    if (orientationBehavior == SkAndroidCodec::ExifOrientationBehavior::kIgnore
+            || !SkPixmapPriv::ShouldSwapWidthHeight(codec->getOrigin())) {
+        return info;
+    }
+    return SkPixmapPriv::SwapWidthHeight(info);
+}
+
+SkAndroidCodec::SkAndroidCodec(SkCodec* codec, ExifOrientationBehavior orientationBehavior)
+    : fInfo(adjust_info(codec, orientationBehavior))
+    , fOrientationBehavior(orientationBehavior)
     , fCodec(codec)
 {}
 
 SkAndroidCodec::~SkAndroidCodec() {}
 
-std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromStream(std::unique_ptr<SkStream> stream, SkPngChunkReader* chunkReader) {
+std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromStream(std::unique_ptr<SkStream> stream,
+                                                               SkPngChunkReader* chunkReader) {
     auto codec = SkCodec::MakeFromStream(std::move(stream), nullptr, chunkReader);
     return MakeFromCodec(std::move(codec));
 }
 
-std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<SkCodec> codec) {
+std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<SkCodec> codec,
+        ExifOrientationBehavior orientationBehavior) {
     if (nullptr == codec) {
         return nullptr;
     }
@@ -88,14 +103,16 @@ std::unique_ptr<SkAndroidCodec> SkAndroidCodec::MakeFromCodec(std::unique_ptr<Sk
 #ifdef SK_HAS_HEIF_LIBRARY
         case SkEncodedImageFormat::kHEIF:
 #endif
-            return skstd::make_unique<SkSampledCodec>(codec.release());
+            return skstd::make_unique<SkSampledCodec>(codec.release(), orientationBehavior);
 #ifdef SK_HAS_WEBP_LIBRARY
         case SkEncodedImageFormat::kWEBP:
-            return skstd::make_unique<SkWebpAdapterCodec>((SkWebpCodec*) codec.release());
+            return skstd::make_unique<SkWebpAdapterCodec>((SkWebpCodec*) codec.release(),
+                    orientationBehavior);
 #endif
 #ifdef SK_CODEC_DECODES_RAW
         case SkEncodedImageFormat::kDNG:
-            return skstd::make_unique<SkRawAdapterCodec>((SkRawCodec*)codec.release());
+            return skstd::make_unique<SkRawAdapterCodec>((SkRawCodec*)codec.release(),
+                    orientationBehavior);
 #endif
         default:
             return nullptr;
@@ -325,24 +342,43 @@ SkISize SkAndroidCodec::getSampledSubsetDimensions(int sampleSize, const SkIRect
             get_scaled_dimension(subset.height(), sampleSize)};
 }
 
-SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,
+static bool acceptable_result(SkCodec::Result result) {
-        size_t rowBytes, const AndroidOptions* options) {
+    switch (result) {
-    if (!pixels) {
+        // These results mean a partial or complete image. They should be considered
+        // a success by SkPixmapPriv.
+        case SkCodec::kSuccess:
+        case SkCodec::kIncompleteInput:
+        case SkCodec::kErrorInInput:
+            return true;
+        default:
+            return false;
+    }
+}
+
+SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& requestInfo,
+        void* requestPixels, size_t requestRowBytes, const AndroidOptions* options) {
+    if (!requestPixels) {
         return SkCodec::kInvalidParameters;
     }
-    if (rowBytes < info.minRowBytes()) {
+    if (requestRowBytes < requestInfo.minRowBytes()) {
         return SkCodec::kInvalidParameters;
     }
 
+    SkImageInfo adjustedInfo = fInfo;
+    if (ExifOrientationBehavior::kRespect == fOrientationBehavior
+            && SkPixmapPriv::ShouldSwapWidthHeight(fCodec->getOrigin())) {
+        adjustedInfo = SkPixmapPriv::SwapWidthHeight(adjustedInfo);
+    }
+
     AndroidOptions defaultOptions;
     if (!options) {
         options = &defaultOptions;
     } else if (options->fSubset) {
-        if (!is_valid_subset(*options->fSubset, fInfo.dimensions())) {
+        if (!is_valid_subset(*options->fSubset, adjustedInfo.dimensions())) {
             return SkCodec::kInvalidParameters;
         }
 
-        if (SkIRect::MakeSize(fInfo.dimensions()) == *options->fSubset) {
+        if (SkIRect::MakeSize(adjustedInfo.dimensions()) == *options->fSubset) {
             // The caller wants the whole thing, rather than a subset. Modify
             // the AndroidOptions passed to onGetAndroidPixels to not specify
             // a subset.
@@ -352,7 +388,27 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void*
         }
     }
 
-    return this->onGetAndroidPixels(info, pixels, rowBytes, *options);
+    if (ExifOrientationBehavior::kIgnore == fOrientationBehavior) {
+        return this->onGetAndroidPixels(requestInfo, requestPixels, requestRowBytes, *options);
+    }
+
+    SkCodec::Result result;
+    auto decode = [this, options, &result](const SkPixmap& pm) {
+        result = this->onGetAndroidPixels(pm.info(), pm.writable_addr(), pm.rowBytes(), *options);
+        return acceptable_result(result);
+    };
+
+    SkPixmap dst(requestInfo, requestPixels, requestRowBytes);
+    if (SkPixmapPriv::Orient(dst, fCodec->getOrigin(), decode)) {
+        return result;
+    }
+
+    // Orient returned false. If onGetAndroidPixels succeeded, then Orient failed internally.
+    if (acceptable_result(result)) {
+        return SkCodec::kInternalError;
+    }
+
+    return result;
 }
 
 SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,

src/codec/SkCodecPriv.h

@@ -13,6 +13,7 @@
 #include "SkColorSpaceXformPriv.h"
 #include "SkColorTable.h"
 #include "SkEncodedInfo.h"
+#include "SkEncodedOrigin.h"
 #include "SkImageInfo.h"
 #include "SkTypes.h"
 
@@ -295,4 +296,6 @@ static inline SkAlphaType select_xform_alpha(SkAlphaType dstAlphaType, SkAlphaTy
     return (kOpaque_SkAlphaType == srcAlphaType) ? kOpaque_SkAlphaType : dstAlphaType;
 }
 
+bool is_orientation_marker(const uint8_t* data, size_t data_length, SkEncodedOrigin* orientation);
+
 #endif // SkCodecPriv_DEFINED

src/codec/SkJpegCodec.cpp

@@ -49,32 +49,36 @@ static bool is_orientation_marker(jpeg_marker_struct* marker, SkEncodedOrigin* o
         return false;
     }
 
-    const uint8_t* data = marker->data;
     constexpr uint8_t kExifSig[] { 'E', 'x', 'i', 'f', '\0' };
-    if (memcmp(data, kExifSig, sizeof(kExifSig))) {
+    if (memcmp(marker->data, kExifSig, sizeof(kExifSig))) {
         return false;
     }
 
+    // Account for 'E', 'x', 'i', 'f', '\0', '<fill byte>'.
+    constexpr size_t kOffset = 6;
+    return is_orientation_marker(marker->data + kOffset, marker->data_length - kOffset,
+            orientation);
+}
+
+bool is_orientation_marker(const uint8_t* data, size_t data_length, SkEncodedOrigin* orientation) {
     bool littleEndian;
-    if (!is_valid_endian_marker(data + 6, &littleEndian)) {
+    if (!is_valid_endian_marker(data, &littleEndian)) {
         return false;
     }
 
     // Get the offset from the start of the marker.
-    // Account for 'E', 'x', 'i', 'f', '\0', '<fill byte>'.
     // Though this only reads four bytes, use a larger int in case it overflows.
-    uint64_t offset = get_endian_int(data + 10, littleEndian);
+    uint64_t offset = get_endian_int(data + 4, littleEndian);
-    offset += sizeof(kExifSig) + 1;
 
     // Require that the marker is at least large enough to contain the number of entries.
-    if (marker->data_length < offset + 2) {
+    if (data_length < offset + 2) {
         return false;
     }
     uint32_t numEntries = get_endian_short(data + offset, littleEndian);
 
     // Tag (2 bytes), Datatype (2 bytes), Number of elements (4 bytes), Data (4 bytes)
     const uint32_t kEntrySize = 12;
-    const auto max = SkTo<uint32_t>((marker->data_length - offset - 2) / kEntrySize);
+    const auto max = SkTo<uint32_t>((data_length - offset - 2) / kEntrySize);
     numEntries = SkTMin(numEntries, max);
 
     // Advance the data to the start of the entries.

src/codec/SkRawAdapterCodec.cpp

@@ -9,8 +9,8 @@
 #include "SkCodecPriv.h"
 #include "SkRawAdapterCodec.h"
 
-SkRawAdapterCodec::SkRawAdapterCodec(SkRawCodec* codec)
+SkRawAdapterCodec::SkRawAdapterCodec(SkRawCodec* codec, ExifOrientationBehavior behavior)
-    : INHERITED(codec)
+    : INHERITED(codec, behavior)
 {}
 
 SkISize SkRawAdapterCodec::onGetSampledDimensions(int sampleSize) const {

src/codec/SkRawAdapterCodec.h

@@ -21,8 +21,7 @@
  */
 class SkRawAdapterCodec : public SkAndroidCodec {
 public:
-
+    explicit SkRawAdapterCodec(SkRawCodec*, ExifOrientationBehavior);
-    explicit SkRawAdapterCodec(SkRawCodec*);
 
     ~SkRawAdapterCodec() override {}
 

src/codec/SkSampledCodec.cpp

@@ -12,8 +12,8 @@
 #include "SkSampler.h"
 #include "SkTemplates.h"
 
-SkSampledCodec::SkSampledCodec(SkCodec* codec)
+SkSampledCodec::SkSampledCodec(SkCodec* codec, ExifOrientationBehavior behavior)
-    : INHERITED(codec)
+    : INHERITED(codec, behavior)
 {}
 
 SkISize SkSampledCodec::accountForNativeScaling(int* sampleSizePtr, int* nativeSampleSize) const {

src/codec/SkSampledCodec.h

@@ -16,8 +16,7 @@
  */
 class SkSampledCodec : public SkAndroidCodec {
 public:
-
+    explicit SkSampledCodec(SkCodec*, ExifOrientationBehavior);
-    explicit SkSampledCodec(SkCodec*);
 
     ~SkSampledCodec() override {}
 

src/codec/SkWebpAdapterCodec.cpp

@@ -9,8 +9,8 @@
 #include "SkCodecPriv.h"
 #include "SkWebpAdapterCodec.h"
 
-SkWebpAdapterCodec::SkWebpAdapterCodec(SkWebpCodec* codec)
+SkWebpAdapterCodec::SkWebpAdapterCodec(SkWebpCodec* codec, ExifOrientationBehavior behavior)
-    : INHERITED(codec)
+    : INHERITED(codec, behavior)
 {}
 
 SkISize SkWebpAdapterCodec::onGetSampledDimensions(int sampleSize) const {

src/codec/SkWebpAdapterCodec.h

@@ -17,7 +17,7 @@
 class SkWebpAdapterCodec : public SkAndroidCodec {
 public:
 
-    explicit SkWebpAdapterCodec(SkWebpCodec*);
+    explicit SkWebpAdapterCodec(SkWebpCodec*, ExifOrientationBehavior);
 
     ~SkWebpAdapterCodec() override {}
 

src/codec/SkWebpCodec.cpp

@@ -87,14 +87,25 @@ std::unique_ptr<SkCodec> SkWebpCodec::MakeFromStream(std::unique_ptr<SkStream> s
         }
     }
 
-    WebPChunkIterator chunkIterator;
-    SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
     sk_sp<SkColorSpace> colorSpace = nullptr;
-    if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
+    {
-        colorSpace = SkColorSpace::MakeICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
+        WebPChunkIterator chunkIterator;
+        SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
+        if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
+            colorSpace = SkColorSpace::MakeICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
+        }
+        if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
+            colorSpace = SkColorSpace::MakeSRGB();
+        }
     }
-    if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
+
-        colorSpace = SkColorSpace::MakeSRGB();
+    SkEncodedOrigin origin = kDefault_SkEncodedOrigin;
+    {
+        WebPChunkIterator chunkIterator;
+        SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
+        if (WebPDemuxGetChunk(demux, "EXIF", 1, &chunkIterator)) {
+            is_orientation_marker(chunkIterator.chunk.bytes, chunkIterator.chunk.size, &origin);
+        }
     }
 
     // Get the first frame and its "features" to determine the color and alpha types.
@@ -156,10 +167,12 @@ std::unique_ptr<SkCodec> SkWebpCodec::MakeFromStream(std::unique_ptr<SkStream> s
             return nullptr;
     }
 
+
     *result = kSuccess;
     SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
     return std::unique_ptr<SkCodec>(new SkWebpCodec(width, height, info, std::move(colorSpace),
-                                           std::move(stream), demux.release(), std::move(data)));
+                                           std::move(stream), demux.release(), std::move(data),
+                                           origin));
 }
 
 SkISize SkWebpCodec::onGetScaledDimensions(float desiredScale) const {
@@ -646,9 +659,9 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
 
 SkWebpCodec::SkWebpCodec(int width, int height, const SkEncodedInfo& info,
                          sk_sp<SkColorSpace> colorSpace, std::unique_ptr<SkStream> stream,
-                         WebPDemuxer* demux, sk_sp<SkData> data)
+                         WebPDemuxer* demux, sk_sp<SkData> data, SkEncodedOrigin origin)
     : INHERITED(width, height, info, SkColorSpaceXform::kBGRA_8888_ColorFormat, std::move(stream),
-                std::move(colorSpace))
+                std::move(colorSpace), origin)
     , fDemux(demux)
     , fData(std::move(data))
     , fFailed(false)

src/codec/SkWebpCodec.h

@@ -48,7 +48,7 @@ protected:
 
 private:
     SkWebpCodec(int width, int height, const SkEncodedInfo&, sk_sp<SkColorSpace>,
-                std::unique_ptr<SkStream>, WebPDemuxer*, sk_sp<SkData>);
+                std::unique_ptr<SkStream>, WebPDemuxer*, sk_sp<SkData>, SkEncodedOrigin);
 
     SkAutoTCallVProc<WebPDemuxer, WebPDemuxDelete> fDemux;
 

tests/AndroidCodecTest.cpp

@@ -7,7 +7,9 @@
 
 #include "SkAndroidCodec.h"
 #include "SkCodec.h"
+#include "SkCodecImageGenerator.h"
 #include "SkEncodedImageFormat.h"
+#include "SkPixmapPriv.h"
 
 #include "Resources.h"
 #include "Test.h"
@@ -117,3 +119,69 @@ DEF_TEST(AndroidCodec_computeSampleSize, r) {
         }
     }
 }
+
+DEF_TEST(AndroidCodec_orientation, r) {
+    if (GetResourcePath().isEmpty()) {
+        return;
+    }
+
+    for (const char* ext : { "jpg", "webp" })
+    for (char i = '1'; i <= '8'; ++i) {
+        SkString path = SkStringPrintf("images/orientation/%c.%s", i, ext);
+        auto data = GetResourceAsData(path.c_str());
+        auto gen = SkCodecImageGenerator::MakeFromEncodedCodec(data);
+        if (!gen) {
+            ERRORF(r, "failed to decode %s", path.c_str());
+            return;
+        }
+
+        // Dimensions after adjusting for the origin.
+        const SkISize expectedDims = { 100, 80 };
+
+        // SkCodecImageGenerator automatically adjusts for the origin.
+        REPORTER_ASSERT(r, gen->getInfo().dimensions() == expectedDims);
+
+        auto androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(data));
+        if (!androidCodec) {
+            ERRORF(r, "failed to decode %s", path.c_str());
+            return;
+        }
+
+        // SkAndroidCodec does not adjust for the origin by default. Dimensions may be reversed.
+        if (SkPixmapPriv::ShouldSwapWidthHeight(androidCodec->codec()->getOrigin())) {
+            auto swappedDims = SkPixmapPriv::SwapWidthHeight(androidCodec->getInfo()).dimensions();
+            REPORTER_ASSERT(r, expectedDims == swappedDims);
+        } else {
+            REPORTER_ASSERT(r, expectedDims == androidCodec->getInfo().dimensions());
+        }
+
+        // Passing kRespect adjusts for the origin.
+        androidCodec = SkAndroidCodec::MakeFromCodec(SkCodec::MakeFromData(std::move(data)),
+                SkAndroidCodec::ExifOrientationBehavior::kRespect);
+        auto info = androidCodec->getInfo();
+        REPORTER_ASSERT(r, info.dimensions() == expectedDims);
+
+        SkBitmap fromGenerator;
+        fromGenerator.allocPixels(info);
+        REPORTER_ASSERT(r, gen->getPixels(info, fromGenerator.getPixels(),
+                                          fromGenerator.rowBytes()));
+
+        SkBitmap fromAndroidCodec;
+        fromAndroidCodec.allocPixels(info);
+        auto result = androidCodec->getPixels(info, fromAndroidCodec.getPixels(),
+                                              fromAndroidCodec.rowBytes());
+        REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+
+        for (int i = 0; i < info.width();  ++i)
+        for (int j = 0; j < info.height(); ++j) {
+            SkColor c1 = *fromGenerator   .getAddr32(i, j);
+            SkColor c2 = *fromAndroidCodec.getAddr32(i, j);
+            if (c1 != c2) {
+                ERRORF(r, "Bitmaps for %s do not match starting at position %i, %i\n"
+                          "\tfromGenerator: %x\tfromAndroidCodec: %x", path.c_str(), i, j,
+                          c1, c2);
+                return;
+            }
+        }
+    }
+}