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

Commit 7d940ba4 authored by Leon Scroggins III's avatar Leon Scroggins III
Browse files

Never scale nine-patches in ImageDecoder 

Bug: 76448902
Bug: 70889348
Test: Manual + CtsThemeHostTestCases
(Ica5e7e81848c3880accee922ee6f1cc9e26262ca)

Scaling a nine-patch requires scaling its divs. When the scale factor is
not an integer, we have to round. This gets out of sync with the way the
decoder scaled the image, resulting in stretching or keeping fixed the
wrong portions of the image. Making this worse, when we scale down, we
end up with divs colliding with each other, and we have to arbitrarily
adjust them further so they do not collide.

NinePatchDrawable and the drawing code already know how to handle
drawing from the originally-sized image and do a better job stretching
appropriately, so allow them to do their job.

We already do something similar for Bitmaps created by ImageDecoder on
apps targeting P and above - instead of scaling them up, we allow the
BitmapDrawable's scaling code to handle density differences. We
preserved the old behavior (scale up) on apps targeting pre-P because
those apps may rely on the size of the Bitmap contained in a
BitmapDrawable without accounting for its density (see Bug: 74061412).
But that is not an issue for NinePatchDrawables, which do not allow
peeking at their internal Bitmaps.

Rewrite ImageDecoder.computeDensity. There is no need for it to be
static, since it takes an ImageDecoder as a parameter and reads its
fields, including the new field mIsNinePatch. Set mIsNinePatch in the
constructor to avoid another down call into native. Split up the
conditions that result in returning srcDensity without calling
setTargetSize for clarity.

Remove ImageDecoder constructor from the graylist. It was accidentally
added due to the fact that it is called transitively from public APIs.

Change-Id: I3c5ddd67f3352c991515f30ce1c477c9a608833f
parent ac959199

config/hiddenapi-light-greylist.txt

+0 −1
Original line number Diff line number Diff line
@@ -760,7 +760,6 @@ Landroid/graphics/GraphicBuffer;->createFromExisting(IIIIJ)Landroid/graphics/Gra 
Landroid/graphics/GraphicBuffer;->CREATOR:Landroid/os/Parcelable$Creator;

Landroid/graphics/GraphicBuffer;-><init>(IIIIJ)V

Landroid/graphics/GraphicBuffer;->mNativeObject:J

Landroid/graphics/ImageDecoder;-><init>(JIIZ)V

Landroid/graphics/ImageDecoder;->postProcessAndRelease(Landroid/graphics/Canvas;)I

Landroid/graphics/LinearGradient;->mColors:[I

Landroid/graphics/Matrix;->native_instance:J

core/jni/android/graphics/ImageDecoder.cpp

+9 −13
Original line number Diff line number Diff line
@@ -116,9 +116,10 @@ static jobject native_create(JNIEnv* env, std::unique_ptr<SkStream> stream, jobj 
    const auto& info = decoder->mCodec->getInfo();

    const int width = info.width();

    const int height = info.height();

    const bool isNinePatch = decoder->mPeeker->mPatch != nullptr;

    return env->NewObject(gImageDecoder_class, gImageDecoder_constructorMethodID,

                          reinterpret_cast<jlong>(decoder.release()), width, height,

                          animated);

                          animated, isNinePatch);

}



static jobject ImageDecoder_nCreateFd(JNIEnv* env, jobject /*clazz*/,
@@ -332,13 +333,6 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong 
        }

    }



    float scaleX = 1.0f;

    float scaleY = 1.0f;

    if (scale) {

        scaleX = (float) desiredWidth  / decodeInfo.width();

        scaleY = (float) desiredHeight / decodeInfo.height();

    }



    jbyteArray ninePatchChunk = nullptr;

    jobject ninePatchInsets = nullptr;
@@ -346,9 +340,6 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong 
    if (!jpostProcess) {

        // FIXME: Share more code with BitmapFactory.cpp.

        if (decoder->mPeeker->mPatch != nullptr) {

            if (scale) {

                decoder->mPeeker->scale(scaleX, scaleY, desiredWidth, desiredHeight);

            }

            size_t ninePatchArraySize = decoder->mPeeker->mPatch->serializedSize();

            ninePatchChunk = env->NewByteArray(ninePatchArraySize);

            if (ninePatchChunk == nullptr) {
@@ -408,7 +399,12 @@ static jobject ImageDecoder_nDecodeBitmap(JNIEnv* env, jobject /*clazz*/, jlong 


        SkCanvas canvas(scaledBm, SkCanvas::ColorBehavior::kLegacy);

        canvas.translate(translateX, translateY);

        if (scale) {

            float scaleX = (float) desiredWidth  / decodeInfo.width();

            float scaleY = (float) desiredHeight / decodeInfo.height();

            canvas.scale(scaleX, scaleY);

        }



        canvas.drawBitmap(bm, 0.0f, 0.0f, &paint);



        bm.swap(scaledBm);
@@ -532,7 +528,7 @@ static const JNINativeMethod gImageDecoderMethods[] = { 


int register_android_graphics_ImageDecoder(JNIEnv* env) {

    gImageDecoder_class = MakeGlobalRefOrDie(env, FindClassOrDie(env, "android/graphics/ImageDecoder"));

    gImageDecoder_constructorMethodID = GetMethodIDOrDie(env, gImageDecoder_class, "<init>", "(JIIZ)V");

    gImageDecoder_constructorMethodID = GetMethodIDOrDie(env, gImageDecoder_class, "<init>", "(JIIZZ)V");

    gImageDecoder_postProcessMethodID = GetMethodIDOrDie(env, gImageDecoder_class, "postProcessAndRelease", "(Landroid/graphics/Canvas;)I");



    gSize_class = MakeGlobalRefOrDie(env, FindClassOrDie(env, "android/util/Size"));

graphics/java/android/graphics/ImageDecoder.java

+30 −12
Original line number Diff line number Diff line
@@ -748,6 +748,7 @@ public final class ImageDecoder implements AutoCloseable { 
    private final int     mWidth;

    private final int     mHeight;

    private final boolean mAnimated;

    private final boolean mIsNinePatch;



    private int        mDesiredWidth;

    private int        mDesiredHeight;
@@ -778,13 +779,14 @@ public final class ImageDecoder implements AutoCloseable { 
     */

    @SuppressWarnings("unused")

    private ImageDecoder(long nativePtr, int width, int height,

            boolean animated) {

            boolean animated, boolean isNinePatch) {

        mNativePtr = nativePtr;

        mWidth = width;

        mHeight = height;

        mDesiredWidth = width;

        mDesiredHeight = height;

        mAnimated = animated;

        mIsNinePatch = isNinePatch;

        mCloseGuard.open("close");

    }
@@ -1665,7 +1667,7 @@ public final class ImageDecoder implements AutoCloseable { 


            // this call potentially manipulates the decoder so it must be performed prior to

            // decoding the bitmap and after decode set the density on the resulting bitmap

            final int srcDensity = computeDensity(src, decoder);

            final int srcDensity = decoder.computeDensity(src);

            if (decoder.mAnimated) {

                // AnimatedImageDrawable calls postProcessAndRelease only if

                // mPostProcessor exists.
@@ -1755,7 +1757,7 @@ public final class ImageDecoder implements AutoCloseable { 


            // this call potentially manipulates the decoder so it must be performed prior to

            // decoding the bitmap

            final int srcDensity = computeDensity(src, decoder);

            final int srcDensity = decoder.computeDensity(src);

            Bitmap bm = decoder.decodeBitmapInternal();

            bm.setDensity(srcDensity);
@@ -1772,12 +1774,26 @@ public final class ImageDecoder implements AutoCloseable { 
    }



    // This method may modify the decoder so it must be called prior to performing the decode

    private static int computeDensity(@NonNull Source src, @NonNull ImageDecoder decoder) {

    private int computeDensity(@NonNull Source src) {

        // if the caller changed the size then we treat the density as unknown

        if (decoder.requestedResize()) {

        if (this.requestedResize()) {

            return Bitmap.DENSITY_NONE;

        }



        final int srcDensity = src.getDensity();

        if (srcDensity == Bitmap.DENSITY_NONE) {

            return srcDensity;

        }



        // Scaling up nine-patch divs is imprecise and is better handled

        // at draw time. An app won't be relying on the internal Bitmap's

        // size, so it is safe to let NinePatchDrawable handle scaling.

        // mPostProcessor disables nine-patching, so behave normally if

        // it is present.

        if (mIsNinePatch && mPostProcessor == null) {

            return srcDensity;

        }



        // Special stuff for compatibility mode: if the target density is not

        // the same as the display density, but the resource -is- the same as

        // the display density, then don't scale it down to the target density.
@@ -1786,23 +1802,25 @@ public final class ImageDecoder implements AutoCloseable { 
        // to the compatibility density only to have them scaled back up when

        // drawn to the screen.

        Resources res = src.getResources();

        final int srcDensity = src.getDensity();

        if (res != null && res.getDisplayMetrics().noncompatDensityDpi == srcDensity) {

            return srcDensity;

        }



        final int dstDensity = src.computeDstDensity();

        if (srcDensity == dstDensity) {

            return srcDensity;

        }



        // For P and above, only resize if it would be a downscale. Scale up prior

        // to P in case the app relies on the Bitmap's size without considering density.

        final int dstDensity = src.computeDstDensity();

        if (srcDensity == Bitmap.DENSITY_NONE || srcDensity == dstDensity

                || (srcDensity < dstDensity && sApiLevel >= Build.VERSION_CODES.P)) {

        if (srcDensity < dstDensity && sApiLevel >= Build.VERSION_CODES.P) {

            return srcDensity;

        }



        float scale = (float) dstDensity / srcDensity;

        int scaledWidth = (int) (decoder.mWidth * scale + 0.5f);

        int scaledHeight = (int) (decoder.mHeight * scale + 0.5f);

        decoder.setTargetSize(scaledWidth, scaledHeight);

        int scaledWidth = (int) (mWidth * scale + 0.5f);

        int scaledHeight = (int) (mHeight * scale + 0.5f);

        this.setTargetSize(scaledWidth, scaledHeight);

        return dstDensity;

    }