diff options
author | John Reck <jreck@google.com> | 2023-06-22 17:25:02 +0000 |
---|---|---|
committer | Android (Google) Code Review <android-gerrit@google.com> | 2023-06-22 17:25:02 +0000 |
commit | fae64dda461d9d76f3273ac0047adccc05218a8f (patch) | |
tree | 4d27ec40f743bc7a29ad3e18424f38597443aab9 | |
parent | 04c08fe37cef3bf2f11e8b4ff97032f9170521ba (diff) | |
parent | 40ffc1d77a674db21855878340a9f19079bb5967 (diff) | |
download | base-fae64dda461d9d76f3273ac0047adccc05218a8f.tar.gz |
Merge "Fix gainmap size for recycled inBitmap in BRD" into udc-dev
-rw-r--r-- | libs/hwui/jni/BitmapRegionDecoder.cpp | 57 | ||||
-rw-r--r-- | libs/hwui/jni/Graphics.cpp | 32 | ||||
-rw-r--r-- | libs/hwui/jni/GraphicsJNI.h | 4 |
3 files changed, 66 insertions, 27 deletions
diff --git a/libs/hwui/jni/BitmapRegionDecoder.cpp b/libs/hwui/jni/BitmapRegionDecoder.cpp index aeaa17198be5..4c9a23d3fde0 100644 --- a/libs/hwui/jni/BitmapRegionDecoder.cpp +++ b/libs/hwui/jni/BitmapRegionDecoder.cpp @@ -96,17 +96,33 @@ public: sk_sp<SkColorSpace> decodeColorSpace = mGainmapBRD->computeOutputColorSpace(decodeColorType, nullptr); SkBitmap bm; - HeapAllocator heapAlloc; - if (!mGainmapBRD->decodeRegion(&bm, &heapAlloc, desiredSubset, sampleSize, decodeColorType, - requireUnpremul, decodeColorSpace)) { - ALOGE("Error decoding Gainmap region"); - return false; - } - sk_sp<Bitmap> nativeBitmap(heapAlloc.getStorageObjAndReset()); + // Because we must match the dimensions of the base bitmap, we always use a + // recycling allocator even though we are allocating a new bitmap. This is to ensure + // that if a recycled bitmap was used for the base image that we match the relative + // dimensions of that base image. The behavior of BRD here is: + // if inBitmap is specified -> output dimensions are always equal to the inBitmap's + // if no bitmap is reused -> output dimensions are the intersect of the desiredSubset & + // the image bounds + // The handling of the above conditionals are baked into the desiredSubset, so we + // simply need to ensure that the resulting bitmap is the exact same width/height as + // the specified desiredSubset regardless of the intersection to the image bounds. + // kPremul_SkAlphaType is used just as a placeholder as it doesn't change the underlying + // allocation type. RecyclingClippingPixelAllocator will populate this with the + // actual alpha type in either allocPixelRef() or copyIfNecessary() + sk_sp<Bitmap> nativeBitmap = Bitmap::allocateHeapBitmap( + SkImageInfo::Make(desiredSubset.width(), desiredSubset.height(), decodeColorType, + kPremul_SkAlphaType, decodeColorSpace)); if (!nativeBitmap) { ALOGE("OOM allocating Bitmap for Gainmap"); return false; } + RecyclingClippingPixelAllocator allocator(nativeBitmap.get(), false); + if (!mGainmapBRD->decodeRegion(&bm, &allocator, desiredSubset, sampleSize, decodeColorType, + requireUnpremul, decodeColorSpace)) { + ALOGE("Error decoding Gainmap region"); + return false; + } + allocator.copyIfNecessary(); auto gainmap = sp<uirenderer::Gainmap>::make(); if (!gainmap) { ALOGE("OOM allocating Gainmap"); @@ -238,13 +254,11 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in // Recycle a bitmap if possible. android::Bitmap* recycledBitmap = nullptr; - size_t recycledBytes = 0; if (javaBitmap) { recycledBitmap = &bitmap::toBitmap(inBitmapHandle); if (recycledBitmap->isImmutable()) { ALOGW("Warning: Reusing an immutable bitmap as an image decoder target."); } - recycledBytes = recycledBitmap->getAllocationByteCount(); } auto* brd = reinterpret_cast<BitmapRegionDecoderWrapper*>(brdHandle); @@ -263,7 +277,7 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in // Set up the pixel allocator skia::BRDAllocator* allocator = nullptr; - RecyclingClippingPixelAllocator recycleAlloc(recycledBitmap, recycledBytes); + RecyclingClippingPixelAllocator recycleAlloc(recycledBitmap); HeapAllocator heapAlloc; if (javaBitmap) { allocator = &recycleAlloc; @@ -277,7 +291,7 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in decodeColorType, colorSpace); // Decode the region. - SkIRect subset = SkIRect::MakeXYWH(inputX, inputY, inputWidth, inputHeight); + const SkIRect subset = SkIRect::MakeXYWH(inputX, inputY, inputWidth, inputHeight); SkBitmap bitmap; if (!brd->decodeRegion(&bitmap, allocator, subset, sampleSize, decodeColorType, requireUnpremul, decodeColorSpace)) { @@ -307,10 +321,27 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in GraphicsJNI::getColorSpace(env, decodeColorSpace.get(), decodeColorType)); } + if (javaBitmap) { + recycleAlloc.copyIfNecessary(); + } + sp<uirenderer::Gainmap> gainmap; bool hasGainmap = brd->hasGainmap(); if (hasGainmap) { - SkIRect gainmapSubset = brd->calculateGainmapRegion(subset); + SkIRect adjustedSubset{}; + if (javaBitmap) { + // Clamp to the width/height of the recycled bitmap in case the reused bitmap + // was too small for the specified rectangle, in which case we need to clip + adjustedSubset = SkIRect::MakeXYWH(inputX, inputY, + std::min(subset.width(), recycledBitmap->width()), + std::min(subset.height(), recycledBitmap->height())); + } else { + // We are not recycling, so use the decoded width/height for calculating the gainmap + // subset instead to ensure the gainmap region proportionally matches + adjustedSubset = SkIRect::MakeXYWH(std::max(0, inputX), std::max(0, inputY), + bitmap.width(), bitmap.height()); + } + SkIRect gainmapSubset = brd->calculateGainmapRegion(adjustedSubset); if (!brd->decodeGainmapRegion(&gainmap, gainmapSubset, sampleSize, requireUnpremul)) { // If there is an error decoding Gainmap - we don't fail. We just don't provide Gainmap hasGainmap = false; @@ -319,7 +350,6 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in // If we may have reused a bitmap, we need to indicate that the pixels have changed. if (javaBitmap) { - recycleAlloc.copyIfNecessary(); if (hasGainmap) { recycledBitmap->setGainmap(std::move(gainmap)); } @@ -331,6 +361,7 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in if (!requireUnpremul) { bitmapCreateFlags |= android::bitmap::kBitmapCreateFlag_Premultiplied; } + if (isHardware) { sk_sp<Bitmap> hardwareBitmap = Bitmap::allocateHardwareBitmap(bitmap); if (hasGainmap) { diff --git a/libs/hwui/jni/Graphics.cpp b/libs/hwui/jni/Graphics.cpp index 914266de2753..78b4f7b7654d 100644 --- a/libs/hwui/jni/Graphics.cpp +++ b/libs/hwui/jni/Graphics.cpp @@ -620,13 +620,13 @@ bool HeapAllocator::allocPixelRef(SkBitmap* bitmap) { //////////////////////////////////////////////////////////////////////////////// -RecyclingClippingPixelAllocator::RecyclingClippingPixelAllocator( - android::Bitmap* recycledBitmap, size_t recycledBytes) - : mRecycledBitmap(recycledBitmap) - , mRecycledBytes(recycledBytes) - , mSkiaBitmap(nullptr) - , mNeedsCopy(false) -{} +RecyclingClippingPixelAllocator::RecyclingClippingPixelAllocator(android::Bitmap* recycledBitmap, + bool mustMatchColorType) + : mRecycledBitmap(recycledBitmap) + , mRecycledBytes(recycledBitmap ? recycledBitmap->getAllocationByteCount() : 0) + , mSkiaBitmap(nullptr) + , mNeedsCopy(false) + , mMustMatchColorType(mustMatchColorType) {} RecyclingClippingPixelAllocator::~RecyclingClippingPixelAllocator() {} @@ -637,10 +637,16 @@ bool RecyclingClippingPixelAllocator::allocPixelRef(SkBitmap* bitmap) { LOG_ALWAYS_FATAL_IF(!bitmap); mSkiaBitmap = bitmap; - // This behaves differently than the RecyclingPixelAllocator. For backwards - // compatibility, the original color type of the recycled bitmap must be maintained. - if (mRecycledBitmap->info().colorType() != bitmap->colorType()) { - return false; + if (mMustMatchColorType) { + // This behaves differently than the RecyclingPixelAllocator. For backwards + // compatibility, the original color type of the recycled bitmap must be maintained. + if (mRecycledBitmap->info().colorType() != bitmap->colorType()) { + ALOGW("recycled color type %d != bitmap color type %d", + mRecycledBitmap->info().colorType(), bitmap->colorType()); + return false; + } + } else { + mRecycledBitmap->reconfigure(mRecycledBitmap->info().makeColorType(bitmap->colorType())); } // The Skia bitmap specifies the width and height needed by the decoder. @@ -695,7 +701,7 @@ bool RecyclingClippingPixelAllocator::allocPixelRef(SkBitmap* bitmap) { void RecyclingClippingPixelAllocator::copyIfNecessary() { if (mNeedsCopy) { mRecycledBitmap->ref(); - SkPixelRef* recycledPixels = mRecycledBitmap; + android::Bitmap* recycledPixels = mRecycledBitmap; void* dst = recycledPixels->pixels(); const size_t dstRowBytes = mRecycledBitmap->rowBytes(); const size_t bytesToCopy = std::min(mRecycledBitmap->info().minRowBytes(), @@ -708,6 +714,8 @@ void RecyclingClippingPixelAllocator::copyIfNecessary() { dst = reinterpret_cast<void*>( reinterpret_cast<uint8_t*>(dst) + dstRowBytes); } + recycledPixels->setAlphaType(mSkiaBitmap->alphaType()); + recycledPixels->setColorSpace(mSkiaBitmap->refColorSpace()); recycledPixels->notifyPixelsChanged(); recycledPixels->unref(); } diff --git a/libs/hwui/jni/GraphicsJNI.h b/libs/hwui/jni/GraphicsJNI.h index 24f9e82b5340..23ab5dd38b1a 100644 --- a/libs/hwui/jni/GraphicsJNI.h +++ b/libs/hwui/jni/GraphicsJNI.h @@ -222,9 +222,8 @@ private: */ class RecyclingClippingPixelAllocator : public android::skia::BRDAllocator { public: - RecyclingClippingPixelAllocator(android::Bitmap* recycledBitmap, - size_t recycledBytes); + bool mustMatchColorType = true); ~RecyclingClippingPixelAllocator(); @@ -252,6 +251,7 @@ private: const size_t mRecycledBytes; SkBitmap* mSkiaBitmap; bool mNeedsCopy; + const bool mMustMatchColorType; }; class AshmemPixelAllocator : public SkBitmap::Allocator { |