From c2ebc2bcf37d1ba67bd3c1ec66a65aa947ac086e Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Wed, 13 Jan 2021 09:46:06 -0500 Subject: [PATCH] Implement AImageDecoder_setInternallyHandleDisposePrevious Bug: 160984428 Test: I00682f201a52f894b0e1335c00c4368ce675a805 Also fix a bug caught by the new test. If the current frame is the first in a series of one or more RestorePrevious frames, fPriorFrame should be set to |currentFrame - 1|. Otherwise SkCodec will decode the required frame. This is wasted work, since the prior frame should already be prepared (either by AImageDecoder or by the client). Change-Id: I1fb9f91dc66fd3121f187b9a91c15f625eb17f8d --- libs/hwui/hwui/ImageDecoder.cpp | 51 +++++++++++++++------- libs/hwui/hwui/ImageDecoder.h | 6 +++ native/graphics/jni/imagedecoder.cpp | 6 +++ native/graphics/jni/libjnigraphics.map.txt | 1 + 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/libs/hwui/hwui/ImageDecoder.cpp b/libs/hwui/hwui/ImageDecoder.cpp index b3f7627b1cf7a..764bc4c7ff0ac 100644 --- a/libs/hwui/hwui/ImageDecoder.cpp +++ b/libs/hwui/hwui/ImageDecoder.cpp @@ -52,6 +52,7 @@ ImageDecoder::ImageDecoder(std::unique_ptr codec, sk_spcomputeOutputColorType(kN32_SkColorType)) , mUnpremultipliedRequired(false) , mOutColorSpace(getDefaultColorSpace()) + , mHandleRestorePrevious(true) { mTargetSize = swapWidthHeight() ? SkISize { mDecodeSize.height(), mDecodeSize.width() } : mDecodeSize; @@ -230,6 +231,13 @@ bool ImageDecoder::rewind() { return true; } +void ImageDecoder::setHandleRestorePrevious(bool handle) { + mHandleRestorePrevious = handle; + if (!handle) { + mRestoreFrame = nullptr; + } +} + bool ImageDecoder::advanceFrame() { const int frameIndex = ++mOptions.fFrameIndex; const int frameCount = mCodec->codec()->getFrameCount(); @@ -255,6 +263,7 @@ bool ImageDecoder::advanceFrame() { case RestoreState::kDoNothing: case RestoreState::kNeedsRestore: mRestoreState = RestoreState::kFirstRPFrame; + mOptions.fPriorFrame = frameIndex - 1; break; case RestoreState::kFirstRPFrame: mRestoreState = RestoreState::kRPFrame; @@ -315,29 +324,19 @@ bool ImageDecoder::finished() const { return mOptions.fFrameIndex >= mCodec->codec()->getFrameCount(); } -SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { - // This was checked inside setTargetSize, but it's possible the first frame - // was opaque, so that method succeeded, but after calling advanceFrame, the - // current frame is not opaque. - if (mUnpremultipliedRequired && !opaque()) { - // Allow using a matrix to handle orientation, but not scaling. - if (requires_matrix_scaling(swapWidthHeight(), mDecodeSize, mTargetSize)) { - return SkCodec::kInvalidScale; - } +bool ImageDecoder::handleRestorePrevious(const SkImageInfo& outputInfo, void* pixels, + size_t rowBytes) { + if (!mHandleRestorePrevious) { + return true; } - void* decodePixels = pixels; - size_t decodeRowBytes = rowBytes; - const auto decodeInfo = SkImageInfo::Make(mDecodeSize, mOutColorType, getOutAlphaType(), - getOutputColorSpace()); - const auto outputInfo = getOutputInfo(); switch (mRestoreState) { case RestoreState::kFirstRPFrame:{ // This frame is marked kRestorePrevious. The prior frame should be in // |pixels|, and it is what we'll restore after each consecutive // kRestorePrevious frame. Cache it now. if (!(mRestoreFrame = Bitmap::allocateHeapBitmap(outputInfo))) { - return SkCodec::kInternalError; + return false; } const uint8_t* srcRow = static_cast(pixels); @@ -366,7 +365,29 @@ SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { case RestoreState::kDoNothing: break; } + return true; +} +SkCodec::Result ImageDecoder::decode(void* pixels, size_t rowBytes) { + // This was checked inside setTargetSize, but it's possible the first frame + // was opaque, so that method succeeded, but after calling advanceFrame, the + // current frame is not opaque. + if (mUnpremultipliedRequired && !opaque()) { + // Allow using a matrix to handle orientation, but not scaling. + if (requires_matrix_scaling(swapWidthHeight(), mDecodeSize, mTargetSize)) { + return SkCodec::kInvalidScale; + } + } + + const auto outputInfo = getOutputInfo(); + if (!handleRestorePrevious(outputInfo, pixels, rowBytes)) { + return SkCodec::kInternalError; + } + + void* decodePixels = pixels; + size_t decodeRowBytes = rowBytes; + const auto decodeInfo = SkImageInfo::Make(mDecodeSize, mOutColorType, getOutAlphaType(), + getOutputColorSpace()); // 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 90261b16e57ef..1b309bcc7bf08 100644 --- a/libs/hwui/hwui/ImageDecoder.h +++ b/libs/hwui/hwui/ImageDecoder.h @@ -73,6 +73,9 @@ public: SkCodec::FrameInfo getCurrentFrameInfo(); + // Set whether the ImageDecoder should handle RestorePrevious frames. + void setHandleRestorePrevious(bool handle); + private: // State machine for keeping track of how to handle RestorePrevious (RP) // frames in decode(). @@ -105,6 +108,7 @@ private: SkAndroidCodec::AndroidOptions mOptions; bool mCurrentFrameIsIndependent; bool mCurrentFrameIsOpaque; + bool mHandleRestorePrevious; RestoreState mRestoreState; sk_sp mRestoreFrame; std::optional mCropRect; @@ -115,6 +119,8 @@ private: SkAlphaType getOutAlphaType() const; sk_sp getOutputColorSpace() const; bool swapWidthHeight() const; + // Store/restore a frame if necessary. Returns false on error. + bool handleRestorePrevious(const SkImageInfo&, void* pixels, size_t rowBytes); }; } // namespace android diff --git a/native/graphics/jni/imagedecoder.cpp b/native/graphics/jni/imagedecoder.cpp index 0f6190722aa32..5973790e48fdf 100644 --- a/native/graphics/jni/imagedecoder.cpp +++ b/native/graphics/jni/imagedecoder.cpp @@ -535,3 +535,9 @@ int32_t AImageDecoderFrameInfo_getBlendOp(const AImageDecoderFrameInfo* info) { return ANDROID_IMAGE_DECODER_BLEND_OP_SRC_OVER; } } + +void AImageDecoder_setInternallyHandleDisposePrevious(AImageDecoder* decoder, bool handle) { + if (decoder) { + toDecoder(decoder)->setHandleRestorePrevious(handle); + } +} diff --git a/native/graphics/jni/libjnigraphics.map.txt b/native/graphics/jni/libjnigraphics.map.txt index d8c3cefd30ca9..e0df7945ab0ef 100644 --- a/native/graphics/jni/libjnigraphics.map.txt +++ b/native/graphics/jni/libjnigraphics.map.txt @@ -19,6 +19,7 @@ LIBJNIGRAPHICS { AImageDecoder_advanceFrame; # introduced=31 AImageDecoder_rewind; # introduced=31 AImageDecoder_getFrameInfo; # introduced = 31 + AImageDecoder_setInternallyHandleDisposePrevious; # introduced = 31 AImageDecoderHeaderInfo_getWidth; # introduced=30 AImageDecoderHeaderInfo_getHeight; # introduced=30 AImageDecoderHeaderInfo_getMimeType; # introduced=30