From 1ade46d272f1279ce3d3dbe4867b7229ec170aff Mon Sep 17 00:00:00 2001 From: Leon Scroggins III <scroggo@google.com> Date: Wed, 15 Jan 2020 05:41:06 -0500 Subject: [PATCH] Replace setAlphaFlags with setUnpremultipliedRequired Bug: 135133301 Test: I48e49ee08ab1954eddf62ecae87942aeb128c10d As described in I3381582e27894e1072db9b8635f3762b801f5d69, this is a more sensible API. In addition, remove unused methods on ImageDecoder. Lastly, update AImageDecoder methods in the map to document which API level they were introduced in. Change-Id: I1aff544e8d6932b9ed0931a00da66a0aba6cd536 --- core/jni/android/graphics/ImageDecoder.cpp | 8 +++- libs/hwui/hwui/ImageDecoder.cpp | 47 +++++++++------------- libs/hwui/hwui/ImageDecoder.h | 8 ++-- native/graphics/jni/imagedecoder.cpp | 20 ++------- native/graphics/jni/libjnigraphics.map.txt | 34 ++++++++-------- 5 files changed, 48 insertions(+), 69 deletions(-) diff --git a/core/jni/android/graphics/ImageDecoder.cpp b/core/jni/android/graphics/ImageDecoder.cpp index a9002867ae10..e17e057d75c7 100644 --- a/core/jni/android/graphics/ImageDecoder.cpp +++ b/core/jni/android/graphics/ImageDecoder.cpp @@ -241,7 +241,7 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong doThrowISE(env, "Could not scale to target size!"); return nullptr; } - if (requireUnpremul && !decoder->setOutAlphaType(kUnpremul_SkAlphaType)) { + if (requireUnpremul && !decoder->setUnpremultipliedRequired(true)) { doThrowISE(env, "Cannot scale unpremultiplied pixels!"); return nullptr; } @@ -301,11 +301,15 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong } } - SkBitmap bm; SkImageInfo bitmapInfo = decoder->getOutputInfo(); + if (decoder->opaque()) { + bitmapInfo = bitmapInfo.makeAlphaType(kOpaque_SkAlphaType); + } if (asAlphaMask && colorType == kGray_8_SkColorType) { bitmapInfo = bitmapInfo.makeColorType(kAlpha_8_SkColorType); } + + SkBitmap bm; if (!bm.setInfo(bitmapInfo)) { doThrowIOE(env, "Failed to setInfo properly"); return nullptr; diff --git a/libs/hwui/hwui/ImageDecoder.cpp b/libs/hwui/hwui/ImageDecoder.cpp index 4f2027d978a1..a6c4e9db7280 100644 --- a/libs/hwui/hwui/ImageDecoder.cpp +++ b/libs/hwui/hwui/ImageDecoder.cpp @@ -30,19 +30,25 @@ ImageDecoder::ImageDecoder(std::unique_ptr<SkAndroidCodec> codec, sk_sp<SkPngChu , mTargetSize(mCodec->getInfo().dimensions()) , mDecodeSize(mTargetSize) , mOutColorType(mCodec->computeOutputColorType(kN32_SkColorType)) - , mOutAlphaType(mCodec->getInfo().isOpaque() ? - kOpaque_SkAlphaType : kPremul_SkAlphaType) + , mUnpremultipliedRequired(false) , mOutColorSpace(mCodec->getInfo().refColorSpace()) , mSampleSize(1) { } +SkAlphaType ImageDecoder::getOutAlphaType() const { + // While an SkBitmap may want to use kOpaque_SkAlphaType for a performance + // optimization, this class just outputs raw pixels. Using either + // premultiplication choice has no effect on decoding an opaque encoded image. + return mUnpremultipliedRequired ? kUnpremul_SkAlphaType : kPremul_SkAlphaType; +} + bool ImageDecoder::setTargetSize(int width, int height) { if (width <= 0 || height <= 0) { return false; } - auto info = SkImageInfo::Make(width, height, mOutColorType, mOutAlphaType); + auto info = SkImageInfo::Make(width, height, mOutColorType, getOutAlphaType()); size_t rowBytes = info.minRowBytes(); if (rowBytes == 0) { // This would have overflowed. @@ -63,7 +69,7 @@ bool ImageDecoder::setTargetSize(int width, int height) { SkISize targetSize = { width, height }, decodeSize = targetSize; int sampleSize = mCodec->computeSampleSize(&decodeSize); - if (decodeSize != targetSize && mOutAlphaType == kUnpremul_SkAlphaType + if (decodeSize != targetSize && mUnpremultipliedRequired && !mCodec->getInfo().isOpaque()) { return false; } @@ -119,29 +125,11 @@ bool ImageDecoder::setOutColorType(SkColorType colorType) { return true; } -bool ImageDecoder::setOutAlphaType(SkAlphaType alpha) { - switch (alpha) { - case kOpaque_SkAlphaType: - return opaque(); - case kPremul_SkAlphaType: - if (opaque()) { - // Opaque can be treated as premul. - return true; - } - break; - case kUnpremul_SkAlphaType: - if (opaque()) { - // Opaque can be treated as unpremul. - return true; - } - if (mDecodeSize != mTargetSize) { - return false; - } - break; - default: - return false; +bool ImageDecoder::setUnpremultipliedRequired(bool required) { + if (required && !opaque() && mDecodeSize != mTargetSize) { + return false; } - mOutAlphaType = alpha; + mUnpremultipliedRequired = required; return true; } @@ -151,11 +139,11 @@ void ImageDecoder::setOutColorSpace(sk_sp<SkColorSpace> colorSpace) { SkImageInfo ImageDecoder::getOutputInfo() const { SkISize size = mCropRect ? mCropRect->size() : mTargetSize; - return SkImageInfo::Make(size, mOutColorType, mOutAlphaType, mOutColorSpace); + return SkImageInfo::Make(size, mOutColorType, getOutAlphaType(), mOutColorSpace); } bool ImageDecoder::opaque() const { - return mOutAlphaType == kOpaque_SkAlphaType; + return mCodec->getInfo().alphaType() == kOpaque_SkAlphaType; } bool ImageDecoder::gray() const { @@ -165,7 +153,8 @@ bool ImageDecoder::gray() const { SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { void* decodePixels = pixels; size_t decodeRowBytes = rowBytes; - auto decodeInfo = SkImageInfo::Make(mDecodeSize, mOutColorType, mOutAlphaType, mOutColorSpace); + auto decodeInfo = SkImageInfo::Make(mDecodeSize, mOutColorType, getOutAlphaType(), + mOutColorSpace); // Used if we need a temporary before scaling or subsetting. // FIXME: Use scanline decoding on only a couple lines to save memory. b/70709380. SkBitmap tmp; diff --git a/libs/hwui/hwui/ImageDecoder.h b/libs/hwui/hwui/ImageDecoder.h index b956f4a77793..96f97e5421f3 100644 --- a/libs/hwui/hwui/ImageDecoder.h +++ b/libs/hwui/hwui/ImageDecoder.h @@ -41,14 +41,12 @@ public: bool setOutColorType(SkColorType outColorType); - bool setOutAlphaType(SkAlphaType outAlphaType); + bool setUnpremultipliedRequired(bool unpremultipliedRequired); void setOutColorSpace(sk_sp<SkColorSpace> cs); // The size is the final size after scaling and cropping. SkImageInfo getOutputInfo() const; - SkColorType getOutColorType() const { return mOutColorType; } - SkAlphaType getOutAlphaType() const { return mOutAlphaType; } bool opaque() const; bool gray() const; @@ -59,13 +57,15 @@ private: SkISize mTargetSize; SkISize mDecodeSize; SkColorType mOutColorType; - SkAlphaType mOutAlphaType; + bool mUnpremultipliedRequired; sk_sp<SkColorSpace> mOutColorSpace; int mSampleSize; std::optional<SkIRect> mCropRect; ImageDecoder(const ImageDecoder&) = delete; ImageDecoder& operator=(const ImageDecoder&) = delete; + + SkAlphaType getOutAlphaType() const; }; } // namespace android diff --git a/native/graphics/jni/imagedecoder.cpp b/native/graphics/jni/imagedecoder.cpp index 2ef203dd466f..e663534c7a01 100644 --- a/native/graphics/jni/imagedecoder.cpp +++ b/native/graphics/jni/imagedecoder.cpp @@ -241,26 +241,12 @@ int AImageDecoderHeaderInfo_getAlphaFlags(const AImageDecoderHeaderInfo* info) { } } -SkAlphaType toAlphaType(int androidBitmapFlags) { - switch (androidBitmapFlags) { - case ANDROID_BITMAP_FLAGS_ALPHA_PREMUL: - return kPremul_SkAlphaType; - case ANDROID_BITMAP_FLAGS_ALPHA_UNPREMUL: - return kUnpremul_SkAlphaType; - case ANDROID_BITMAP_FLAGS_ALPHA_OPAQUE: - return kOpaque_SkAlphaType; - default: - return kUnknown_SkAlphaType; - } -} - -int AImageDecoder_setAlphaFlags(AImageDecoder* decoder, int alphaFlag) { - if (!decoder || alphaFlag < ANDROID_BITMAP_FLAGS_ALPHA_PREMUL - || alphaFlag > ANDROID_BITMAP_FLAGS_ALPHA_UNPREMUL) { +int AImageDecoder_setUnpremultipliedRequired(AImageDecoder* decoder, bool required) { + if (!decoder) { return ANDROID_IMAGE_DECODER_BAD_PARAMETER; } - return toDecoder(decoder)->setOutAlphaType(toAlphaType(alphaFlag)) + return toDecoder(decoder)->setUnpremultipliedRequired(required) ? ANDROID_IMAGE_DECODER_SUCCESS : ANDROID_IMAGE_DECODER_INVALID_CONVERSION; } diff --git a/native/graphics/jni/libjnigraphics.map.txt b/native/graphics/jni/libjnigraphics.map.txt index 832770ffb97e..d5a3276a5f28 100644 --- a/native/graphics/jni/libjnigraphics.map.txt +++ b/native/graphics/jni/libjnigraphics.map.txt @@ -1,22 +1,22 @@ LIBJNIGRAPHICS { global: - AImageDecoder_createFromAAsset; - AImageDecoder_createFromFd; - AImageDecoder_createFromBuffer; - AImageDecoder_delete; - AImageDecoder_setAndroidBitmapFormat; - AImageDecoder_setAlphaFlags; - AImageDecoder_getHeaderInfo; - AImageDecoder_getMinimumStride; - AImageDecoder_decodeImage; - AImageDecoder_setTargetSize; - AImageDecoder_setCrop; - AImageDecoderHeaderInfo_getWidth; - AImageDecoderHeaderInfo_getHeight; - AImageDecoderHeaderInfo_getMimeType; - AImageDecoderHeaderInfo_getAlphaFlags; - AImageDecoderHeaderInfo_isAnimated; - AImageDecoderHeaderInfo_getAndroidBitmapFormat; + AImageDecoder_createFromAAsset; # introduced=30 + AImageDecoder_createFromFd; # introduced=30 + AImageDecoder_createFromBuffer; # introduced=30 + AImageDecoder_delete; # introduced=30 + AImageDecoder_setAndroidBitmapFormat; # introduced=30 + AImageDecoder_setUnpremultipliedRequired; # introduced=30 + AImageDecoder_getHeaderInfo; # introduced=30 + AImageDecoder_getMinimumStride; # introduced=30 + AImageDecoder_decodeImage; # introduced=30 + AImageDecoder_setTargetSize; # introduced=30 + AImageDecoder_setCrop; # introduced=30 + AImageDecoderHeaderInfo_getWidth; # introduced=30 + AImageDecoderHeaderInfo_getHeight; # introduced=30 + AImageDecoderHeaderInfo_getMimeType; # introduced=30 + AImageDecoderHeaderInfo_getAlphaFlags; # introduced=30 + AImageDecoderHeaderInfo_isAnimated; # introduced=30 + AImageDecoderHeaderInfo_getAndroidBitmapFormat; # introduced=30 AndroidBitmap_getInfo; AndroidBitmap_getDataSpace; AndroidBitmap_lockPixels; -- GitLab