From 139145be8082f0606045b89feebc9126a9a376d8 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Thu, 17 Dec 2020 15:43:54 -0500 Subject: [PATCH] Handle EXIF orientation in hwui/ImageDecoder Bug: 160984428 Test: AImageDecoderTest#testRespectOrientation ImageDecoderTest#testRespectOrientation Ieda439910ae52e609f0710d424503616d99ae5c7 I23caef26b4c82173c758dd0ce7fb6f04e4154588 I345a13d20776a007052d32e74fa42865b42f726d It is possible to create an animated image with an exif orientation. Using kRespect, there is no clean way to handle the orientation plus compositing frames. Switch ImageDecoder to use kIgnore (the default). Depends on a change in Skia (https://review.skia.org/344762) to make SkAnimatedImage handle the orientation even with kIgnore. Change-Id: Ib93b0ced09fa3cca4a6681745406355c48158fae --- libs/hwui/hwui/ImageDecoder.cpp | 54 ++++++++++++++++++++++------ libs/hwui/hwui/ImageDecoder.h | 5 +++ libs/hwui/jni/ImageDecoder.cpp | 8 ++--- native/graphics/jni/imagedecoder.cpp | 15 ++++---- 4 files changed, 58 insertions(+), 24 deletions(-) diff --git a/libs/hwui/hwui/ImageDecoder.cpp b/libs/hwui/hwui/ImageDecoder.cpp index 43cc4f244f711..6889134719138 100644 --- a/libs/hwui/hwui/ImageDecoder.cpp +++ b/libs/hwui/hwui/ImageDecoder.cpp @@ -40,13 +40,14 @@ sk_sp ImageDecoder::getDefaultColorSpace() const { ImageDecoder::ImageDecoder(std::unique_ptr codec, sk_sp peeker) : mCodec(std::move(codec)) , mPeeker(std::move(peeker)) - , mTargetSize(mCodec->getInfo().dimensions()) - , mDecodeSize(mTargetSize) + , mDecodeSize(mCodec->codec()->dimensions()) , mOutColorType(mCodec->computeOutputColorType(kN32_SkColorType)) , mUnpremultipliedRequired(false) , mOutColorSpace(getDefaultColorSpace()) , mSampleSize(1) { + mTargetSize = swapWidthHeight() ? SkISize { mDecodeSize.height(), mDecodeSize.width() } + : mDecodeSize; } SkAlphaType ImageDecoder::getOutAlphaType() const { @@ -77,7 +78,8 @@ bool ImageDecoder::setTargetSize(int width, int height) { } } - SkISize targetSize = { width, height }, decodeSize = targetSize; + SkISize targetSize = { width, height }; + SkISize decodeSize = swapWidthHeight() ? SkISize { height, width } : targetSize; int sampleSize = mCodec->computeSampleSize(&decodeSize); if (decodeSize != targetSize && mUnpremultipliedRequired && !opaque()) { @@ -157,6 +159,22 @@ SkImageInfo ImageDecoder::getOutputInfo() const { return SkImageInfo::Make(size, mOutColorType, getOutAlphaType(), getOutputColorSpace()); } +bool ImageDecoder::swapWidthHeight() const { + return SkEncodedOriginSwapsWidthHeight(mCodec->codec()->getOrigin()); +} + +int ImageDecoder::width() const { + return swapWidthHeight() + ? mCodec->codec()->dimensions().height() + : mCodec->codec()->dimensions().width(); +} + +int ImageDecoder::height() const { + return swapWidthHeight() + ? mCodec->codec()->dimensions().width() + : mCodec->codec()->dimensions().height(); +} + bool ImageDecoder::opaque() const { return mCodec->getInfo().alphaType() == kOpaque_SkAlphaType; } @@ -174,7 +192,9 @@ SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { // FIXME: Use scanline decoding on only a couple lines to save memory. b/70709380. SkBitmap tmp; const bool scale = mDecodeSize != mTargetSize; - if (scale || mCropRect) { + const auto origin = mCodec->codec()->getOrigin(); + const bool handleOrigin = origin != kDefault_SkEncodedOrigin; + if (scale || handleOrigin || mCropRect) { if (!tmp.setInfo(decodeInfo)) { return SkCodec::kInternalError; } @@ -189,7 +209,7 @@ SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { options.fSampleSize = mSampleSize; auto result = mCodec->getAndroidPixels(decodeInfo, decodePixels, decodeRowBytes, &options); - if (scale || mCropRect) { + if (scale || handleOrigin || mCropRect) { SkBitmap scaledBm; if (!scaledBm.installPixels(getOutputInfo(), pixels, rowBytes)) { return SkCodec::kInternalError; @@ -200,15 +220,27 @@ SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { paint.setFilterQuality(kLow_SkFilterQuality); // bilinear filtering SkCanvas canvas(scaledBm, SkCanvas::ColorBehavior::kLegacy); + SkMatrix outputMatrix; if (mCropRect) { - canvas.translate(-mCropRect->fLeft, -mCropRect->fTop); - } - if (scale) { - float scaleX = (float) mTargetSize.width() / mDecodeSize.width(); - float scaleY = (float) mTargetSize.height() / mDecodeSize.height(); - canvas.scale(scaleX, scaleY); + outputMatrix.setTranslate(-mCropRect->fLeft, -mCropRect->fTop); } + int targetWidth = mTargetSize.width(); + int targetHeight = mTargetSize.height(); + if (handleOrigin) { + outputMatrix.preConcat(SkEncodedOriginToMatrix(origin, targetWidth, targetHeight)); + if (SkEncodedOriginSwapsWidthHeight(origin)) { + std::swap(targetWidth, targetHeight); + } + } + + if (scale) { + float scaleX = (float) targetWidth / mDecodeSize.width(); + float scaleY = (float) targetHeight / mDecodeSize.height(); + outputMatrix.preScale(scaleX, scaleY); + } + + canvas.setMatrix(outputMatrix); canvas.drawBitmap(tmp, 0.0f, 0.0f, &paint); } diff --git a/libs/hwui/hwui/ImageDecoder.h b/libs/hwui/hwui/ImageDecoder.h index a1b51573db3f5..a08e92478fb0a 100644 --- a/libs/hwui/hwui/ImageDecoder.h +++ b/libs/hwui/hwui/ImageDecoder.h @@ -49,7 +49,11 @@ public: // The size is the final size after scaling and cropping. SkImageInfo getOutputInfo() const; + int width() const; + int height() const; + bool opaque() const; + bool gray() const; SkCodec::Result decode(void* pixels, size_t rowBytes); @@ -68,6 +72,7 @@ private: SkAlphaType getOutAlphaType() const; sk_sp getOutputColorSpace() const; + bool swapWidthHeight() const; }; } // namespace android diff --git a/libs/hwui/jni/ImageDecoder.cpp b/libs/hwui/jni/ImageDecoder.cpp index da91d46b0738f..96e912fd9f26a 100644 --- a/libs/hwui/jni/ImageDecoder.cpp +++ b/libs/hwui/jni/ImageDecoder.cpp @@ -135,19 +135,15 @@ static jobject native_create(JNIEnv* env, std::unique_ptr stream, return throw_exception(env, kSourceException, "", jexception, source); } - auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec), - SkAndroidCodec::ExifOrientationBehavior::kRespect); + auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec)); if (!androidCodec.get()) { return throw_exception(env, kSourceMalformedData, "", nullptr, source); } - const auto& info = androidCodec->getInfo(); - const int width = info.width(); - const int height = info.height(); const bool isNinePatch = peeker->mPatch != nullptr; ImageDecoder* decoder = new ImageDecoder(std::move(androidCodec), std::move(peeker)); return env->NewObject(gImageDecoder_class, gImageDecoder_constructorMethodID, - reinterpret_cast(decoder), width, height, + reinterpret_cast(decoder), decoder->width(), decoder->height(), animated, isNinePatch); } diff --git a/native/graphics/jni/imagedecoder.cpp b/native/graphics/jni/imagedecoder.cpp index ac4c16a4199bb..4aeebe47f1aef 100644 --- a/native/graphics/jni/imagedecoder.cpp +++ b/native/graphics/jni/imagedecoder.cpp @@ -65,17 +65,18 @@ static int createFromStream(std::unique_ptr stream, AImageDe SkCodec::Result result; auto codec = SkCodec::MakeFromStream(std::move(stream), &result, nullptr, SkCodec::SelectionPolicy::kPreferAnimation); - auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec), - SkAndroidCodec::ExifOrientationBehavior::kRespect); + // These may be swapped due to the SkEncodedOrigin, but we're just checking + // them to make sure they fit in int32_t. + auto dimensions = codec->dimensions(); + auto androidCodec = SkAndroidCodec::MakeFromCodec(std::move(codec)); if (!androidCodec) { return ResultToErrorCode(result); } // AImageDecoderHeaderInfo_getWidth/Height return an int32_t. Ensure that // the conversion is safe. - const auto& info = androidCodec->getInfo(); - if (info.width() > std::numeric_limits::max() - || info.height() > std::numeric_limits::max()) { + if (dimensions.width() > std::numeric_limits::max() || + dimensions.height() > std::numeric_limits::max()) { return ANDROID_IMAGE_DECODER_INVALID_INPUT; } @@ -200,14 +201,14 @@ int32_t AImageDecoderHeaderInfo_getWidth(const AImageDecoderHeaderInfo* info) { if (!info) { return 0; } - return toDecoder(info)->mCodec->getInfo().width(); + return toDecoder(info)->width(); } int32_t AImageDecoderHeaderInfo_getHeight(const AImageDecoderHeaderInfo* info) { if (!info) { return 0; } - return toDecoder(info)->mCodec->getInfo().height(); + return toDecoder(info)->height(); } const char* AImageDecoderHeaderInfo_getMimeType(const AImageDecoderHeaderInfo* info) {