Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7dcb7d2d authored by Alec Mouri's avatar Alec Mouri
Browse files

Resample gainmaps during region decoding.

Implicitly, region decoding is a cropping operation, which previously
was introducing quality issues when decoding a gainmapped image. For
instance:

(a) Consider a 48x48 image with a 12x12 gainmap.
(b) Consider decoding a 10x10 region of the image. The decoder must choose
  to either decode a 2x2 or 3x3 region of the gainmap, which does not
  match the 4x scale ratio of the source, so either we decode too much
  or too little of the image
(c) When displaying the image, we bilinearly scale both the image and
the gainmap to the destination, then apply the gainmap. But, because of
(b), the gainmap is misaligned with the base image, introducing haloing
artifacts.

To fix this, we change (b) by always decoding a slightly larger
region (in the examplar case, either a decode a 3x3 or 4x4 region), but
retain information about the "logical" region we intended. Then, we
resample from the "logical" region up to the decoded bounds.

In the above example, this means that we decode a 3x3 or 4x4 bitmap,
that holds the logical 2.5x2.5 region of the gainmap, ensuring that (0, 0)
in the resulting bitmap maps to (0, 0) in the bitmap for the decoded region
from the base image, so that the gainmap content lines up with the base
image after the gainmap is upsampled during rendering.

We do the actual resampling inside of the recycling allocator, since we
sometimes perform a bitmap copy there anyways, so we can resample during
the copy.

Hide this behind a flag, since I broke decoding in about 5 different
ways before settling on this.

Bug: 352847821
Flag: com.android.graphics.hwui.flags.resample_gainmap_regions
Test: Decoding works in Photos, Files, and modified SilkFX
Change-Id: Ic21d44011858619273b11c20ee746614a1749a73
parent 06d5b870
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -42,6 +42,9 @@ constexpr bool hdr_10bit_plus() {
constexpr bool initialize_gl_always() {
    return false;
}
constexpr bool resample_gainmap_regions() {
    return false;
}
}  // namespace hwui_flags
#endif

@@ -100,6 +103,7 @@ float Properties::maxHdrHeadroomOn8bit = 5.f; // TODO: Refine this number

bool Properties::clipSurfaceViews = false;
bool Properties::hdr10bitPlus = false;
bool Properties::resampleGainmapRegions = false;

int Properties::timeoutMultiplier = 1;

@@ -175,6 +179,8 @@ bool Properties::load() {
    clipSurfaceViews =
            base::GetBoolProperty("debug.hwui.clip_surfaceviews", hwui_flags::clip_surfaceviews());
    hdr10bitPlus = hwui_flags::hdr_10bit_plus();
    resampleGainmapRegions = base::GetBoolProperty("debug.hwui.resample_gainmap_regions",
                                                   hwui_flags::resample_gainmap_regions());

    timeoutMultiplier = android::base::GetIntProperty("ro.hw_timeout_multiplier", 1);

+1 −0
Original line number Diff line number Diff line
@@ -342,6 +342,7 @@ public:

    static bool clipSurfaceViews;
    static bool hdr10bitPlus;
    static bool resampleGainmapRegions;

    static int timeoutMultiplier;

+10 −0
Original line number Diff line number Diff line
@@ -97,3 +97,13 @@ flag {
  description: "Initialize GL even when HWUI is set to use Vulkan. This improves app startup time for apps using GL."
  bug: "335172671"
}

flag {
  name: "resample_gainmap_regions"
  namespace: "core_graphics"
  description: "Resample gainmaps when decoding regions, to improve visual quality"
  bug: "352847821"
  metadata {
    purpose: PURPOSE_BUGFIX
  }
}
+61 −20
Original line number Diff line number Diff line
@@ -87,8 +87,17 @@ public:
                                           requireUnpremul, prefColorSpace);
    }

    bool decodeGainmapRegion(sp<uirenderer::Gainmap>* outGainmap, int outWidth, int outHeight,
                             const SkIRect& desiredSubset, int sampleSize, bool requireUnpremul) {
    // Decodes the gainmap region. If decoding succeeded, returns true and
    // populate outGainmap with the decoded gainmap. Otherwise, returns false.
    //
    // Note that the desiredSubset is the logical region within the source
    // gainmap that we want to decode. This is used for scaling into the final
    // bitmap, since we do not want to include portions of the gainmap outside
    // of this region. desiredSubset is also _not_ guaranteed to be
    // pixel-aligned, so it's not possible to simply resize the resulting
    // bitmap to accomplish this.
    bool decodeGainmapRegion(sp<uirenderer::Gainmap>* outGainmap, SkISize bitmapDimensions,
                             const SkRect& desiredSubset, int sampleSize, bool requireUnpremul) {
        SkColorType decodeColorType = mGainmapBRD->computeOutputColorType(kN32_SkColorType);
        sk_sp<SkColorSpace> decodeColorSpace =
                mGainmapBRD->computeOutputColorSpace(decodeColorType, nullptr);
@@ -107,13 +116,30 @@ public:
        // allocation type. RecyclingClippingPixelAllocator will populate this with the
        // actual alpha type in either allocPixelRef() or copyIfNecessary()
        sk_sp<Bitmap> nativeBitmap = Bitmap::allocateHeapBitmap(SkImageInfo::Make(
                outWidth, outHeight, decodeColorType, kPremul_SkAlphaType, decodeColorSpace));
                bitmapDimensions, 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,

        // Round out the subset so that we decode a slightly larger region, in
        // case the subset has fractional components.
        SkIRect roundedSubset = desiredSubset.roundOut();

        // Map the desired subset to the space of the decoded gainmap. The
        // subset is repositioned relative to the resulting bitmap, and then
        // scaled to respect the sampleSize.
        // This assumes that the subset will not be modified by the decoder, which is true
        // for existing gainmap formats.
        SkRect logicalSubset = desiredSubset.makeOffset(-std::floorf(desiredSubset.left()),
                                                        -std::floorf(desiredSubset.top()));
        logicalSubset.fLeft /= sampleSize;
        logicalSubset.fTop /= sampleSize;
        logicalSubset.fRight /= sampleSize;
        logicalSubset.fBottom /= sampleSize;

        RecyclingClippingPixelAllocator allocator(nativeBitmap.get(), false, logicalSubset);
        if (!mGainmapBRD->decodeRegion(&bm, &allocator, roundedSubset, sampleSize, decodeColorType,
                                       requireUnpremul, decodeColorSpace)) {
            ALOGE("Error decoding Gainmap region");
            return false;
@@ -130,16 +156,31 @@ public:
        return true;
    }

    SkIRect calculateGainmapRegion(const SkIRect& mainImageRegion, int* inOutWidth,
                                   int* inOutHeight) {
    struct Projection {
        SkRect srcRect;
        SkISize destSize;
    };
    Projection calculateGainmapRegion(const SkIRect& mainImageRegion, SkISize dimensions) {
        const float scaleX = ((float)mGainmapBRD->width()) / mMainImageBRD->width();
        const float scaleY = ((float)mGainmapBRD->height()) / mMainImageBRD->height();
        *inOutWidth *= scaleX;
        *inOutHeight *= scaleY;
        // TODO: Account for rounding error?
        return SkIRect::MakeLTRB(mainImageRegion.left() * scaleX, mainImageRegion.top() * scaleY,
                                 mainImageRegion.right() * scaleX,
                                 mainImageRegion.bottom() * scaleY);

        if (uirenderer::Properties::resampleGainmapRegions) {
            const auto srcRect = SkRect::MakeLTRB(
                    mainImageRegion.left() * scaleX, mainImageRegion.top() * scaleY,
                    mainImageRegion.right() * scaleX, mainImageRegion.bottom() * scaleY);
            // Request a slightly larger destination size so that the gainmap
            // subset we want fits entirely in this size.
            const auto destSize = SkISize::Make(std::ceil(dimensions.width() * scaleX),
                                                std::ceil(dimensions.height() * scaleY));
            return Projection{.srcRect = srcRect, .destSize = destSize};
        } else {
            const auto srcRect = SkRect::Make(SkIRect::MakeLTRB(
                    mainImageRegion.left() * scaleX, mainImageRegion.top() * scaleY,
                    mainImageRegion.right() * scaleX, mainImageRegion.bottom() * scaleY));
            const auto destSize =
                    SkISize::Make(dimensions.width() * scaleX, dimensions.height() * scaleY);
            return Projection{.srcRect = srcRect, .destSize = destSize};
        }
    }

    bool hasGainmap() { return mGainmapBRD != nullptr; }
@@ -327,16 +368,16 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle, jint in
    sp<uirenderer::Gainmap> gainmap;
    bool hasGainmap = brd->hasGainmap();
    if (hasGainmap) {
        int gainmapWidth = bitmap.width();
        int gainmapHeight = bitmap.height();
        SkISize gainmapDims = SkISize::Make(bitmap.width(), bitmap.height());
        if (javaBitmap) {
            // If we are recycling we must match the inBitmap's relative dimensions
            gainmapWidth = recycledBitmap->width();
            gainmapHeight = recycledBitmap->height();
            gainmapDims.fWidth = recycledBitmap->width();
            gainmapDims.fHeight = recycledBitmap->height();
        }
        SkIRect gainmapSubset = brd->calculateGainmapRegion(subset, &gainmapWidth, &gainmapHeight);
        if (!brd->decodeGainmapRegion(&gainmap, gainmapWidth, gainmapHeight, gainmapSubset,
                                      sampleSize, requireUnpremul)) {
        BitmapRegionDecoderWrapper::Projection gainmapProjection =
                brd->calculateGainmapRegion(subset, gainmapDims);
        if (!brd->decodeGainmapRegion(&gainmap, gainmapProjection.destSize,
                                      gainmapProjection.srcRect, sampleSize, requireUnpremul)) {
            // If there is an error decoding Gainmap - we don't fail. We just don't provide Gainmap
            hasGainmap = false;
        }
+52 −23
Original line number Diff line number Diff line
#include <assert.h>
#include <cutils/ashmem.h>
#include <hwui/Canvas.h>
#include <log/log.h>
#include <nativehelper/JNIHelp.h>
#include <unistd.h>

#include "jni.h"
#include <nativehelper/JNIHelp.h>
#include "GraphicsJNI.h"

#include "SkBitmap.h"
#include "SkCanvas.h"
#include "SkColor.h"
#include "SkColorSpace.h"
#include "SkFontMetrics.h"
#include "SkImageInfo.h"
@@ -14,10 +16,9 @@
#include "SkPoint.h"
#include "SkRect.h"
#include "SkRegion.h"
#include "SkSamplingOptions.h"
#include "SkTypes.h"
#include <cutils/ashmem.h>
#include <hwui/Canvas.h>
#include <log/log.h>
#include "jni.h"

using namespace android;

@@ -630,13 +631,15 @@ bool HeapAllocator::allocPixelRef(SkBitmap* bitmap) {

////////////////////////////////////////////////////////////////////////////////

RecyclingClippingPixelAllocator::RecyclingClippingPixelAllocator(android::Bitmap* recycledBitmap,
                                                                 bool mustMatchColorType)
RecyclingClippingPixelAllocator::RecyclingClippingPixelAllocator(
        android::Bitmap* recycledBitmap, bool mustMatchColorType,
        std::optional<SkRect> desiredSubset)
        : mRecycledBitmap(recycledBitmap)
        , mRecycledBytes(recycledBitmap ? recycledBitmap->getAllocationByteCount() : 0)
        , mSkiaBitmap(nullptr)
        , mNeedsCopy(false)
        , mMustMatchColorType(mustMatchColorType) {}
        , mMustMatchColorType(mustMatchColorType)
        , mDesiredSubset(getSourceBoundsForUpsample(desiredSubset)) {}

RecyclingClippingPixelAllocator::~RecyclingClippingPixelAllocator() {}

@@ -668,7 +671,8 @@ bool RecyclingClippingPixelAllocator::allocPixelRef(SkBitmap* bitmap) {
    const SkImageInfo maxInfo = bitmap->info().makeWH(maxWidth, maxHeight);
    const size_t rowBytes = maxInfo.minRowBytes();
    const size_t bytesNeeded = maxInfo.computeByteSize(rowBytes);
    if (bytesNeeded <= mRecycledBytes) {

    if (!mDesiredSubset && bytesNeeded <= mRecycledBytes) {
        // Here we take advantage of reconfigure() to reset the rowBytes
        // of mRecycledBitmap.  It is very important that we pass in
        // mRecycledBitmap->info() for the SkImageInfo.  According to the
@@ -712,20 +716,31 @@ void RecyclingClippingPixelAllocator::copyIfNecessary() {
    if (mNeedsCopy) {
        mRecycledBitmap->ref();
        android::Bitmap* recycledPixels = mRecycledBitmap;
        if (mDesiredSubset) {
            recycledPixels->setAlphaType(mSkiaBitmap->alphaType());
            recycledPixels->setColorSpace(mSkiaBitmap->refColorSpace());

            auto canvas = SkCanvas(recycledPixels->getSkBitmap());
            SkRect destination = SkRect::Make(recycledPixels->info().bounds());
            destination.intersect(SkRect::Make(mSkiaBitmap->info().bounds()));
            canvas.drawImageRect(mSkiaBitmap->asImage(), *mDesiredSubset, destination,
                                 SkSamplingOptions(SkFilterMode::kLinear), nullptr,
                                 SkCanvas::kFast_SrcRectConstraint);
        } else {
            void* dst = recycledPixels->pixels();
            const size_t dstRowBytes = mRecycledBitmap->rowBytes();
            const size_t bytesToCopy = std::min(mRecycledBitmap->info().minRowBytes(),
                                                mSkiaBitmap->info().minRowBytes());
        const int rowsToCopy = std::min(mRecycledBitmap->info().height(),
                mSkiaBitmap->info().height());
            const int rowsToCopy =
                    std::min(mRecycledBitmap->info().height(), mSkiaBitmap->info().height());
            for (int y = 0; y < rowsToCopy; y++) {
                memcpy(dst, mSkiaBitmap->getAddr(0, y), bytesToCopy);
                // Cast to bytes in order to apply the dstRowBytes offset correctly.
            dst = reinterpret_cast<void*>(
                    reinterpret_cast<uint8_t*>(dst) + dstRowBytes);
                dst = reinterpret_cast<void*>(reinterpret_cast<uint8_t*>(dst) + dstRowBytes);
            }
            recycledPixels->setAlphaType(mSkiaBitmap->alphaType());
            recycledPixels->setColorSpace(mSkiaBitmap->refColorSpace());
        }
        recycledPixels->notifyPixelsChanged();
        recycledPixels->unref();
    }
@@ -733,6 +748,20 @@ void RecyclingClippingPixelAllocator::copyIfNecessary() {
    mSkiaBitmap = nullptr;
}

std::optional<SkRect> RecyclingClippingPixelAllocator::getSourceBoundsForUpsample(
        std::optional<SkRect> subset) {
    if (!uirenderer::Properties::resampleGainmapRegions || !subset || subset->isEmpty()) {
        return std::nullopt;
    }

    if (subset->left() == floor(subset->left()) && subset->top() == floor(subset->top()) &&
        subset->right() == floor(subset->right()) && subset->bottom() == floor(subset->bottom())) {
        return std::nullopt;
    }

    return subset;
}

////////////////////////////////////////////////////////////////////////////////

AshmemPixelAllocator::AshmemPixelAllocator(JNIEnv *env) {
Loading