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

Commit ae2e8b48 authored by John Reck's avatar John Reck
Browse files

Add warning if an in-use Bitmap is reconfigured 

Bug: 18928352

Also fix an issue around re-configure not properly handling
mPinnedCount in android::Bitmap

Change-Id: I1815b121f1474ad931060771bb1d52ef31d2aac7
parent fbb34dd8

core/jni/android/graphics/Bitmap.cpp

+35 −18
Original line number Diff line number Diff line
@@ -188,18 +188,43 @@ size_t Bitmap::rowBytes() const { 
    return mPixelRef->rowBytes();

}



SkPixelRef* Bitmap::pixelRef() const {

SkPixelRef* Bitmap::peekAtPixelRef() const {

    assertValid();

    return mPixelRef.get();

}



SkPixelRef* Bitmap::refPixelRef() {

    assertValid();

    android::AutoMutex _lock(mLock);

    return refPixelRefLocked();

}



SkPixelRef* Bitmap::refPixelRefLocked() {

    mPixelRef->ref();

    if (mPixelRef->unique()) {

        // We just restored this from 0, pin the pixels and inc the strong count

        // Note that there *might be* an incoming onStrongRefDestroyed from whatever

        // last unref'd

        pinPixelsLocked();

        mPinnedRefCount++;

    }

    return mPixelRef.get();

}



void Bitmap::reconfigure(const SkImageInfo& info, size_t rowBytes,

        SkColorTable* ctable) {

    {

        android::AutoMutex _lock(mLock);

        if (mPinnedRefCount) {

            ALOGW("Called reconfigure on a bitmap that is in use! This may"

                    " cause graphical corruption!");

        }

    }

    mPixelRef->reconfigure(info, rowBytes, ctable);

}



void Bitmap::reconfigure(const SkImageInfo& info) {

    mPixelRef->reconfigure(info, mPixelRef->rowBytes(), mPixelRef->colorTable());

    reconfigure(info, mPixelRef->rowBytes(), mPixelRef->colorTable());

}



void Bitmap::detachFromJava() {
@@ -287,18 +312,10 @@ void Bitmap::unpinPixelsLocked() { 
void Bitmap::getSkBitmap(SkBitmap* outBitmap) {

    assertValid();

    android::AutoMutex _lock(mLock);

    mPixelRef->ref();

    if (mPixelRef->unique()) {

        // We just restored this from 0, pin the pixels and inc the strong count

        // Note that there *might be* an incoming onStrongRefDestroyed from whatever

        // last unref'd

        pinPixelsLocked();

        mPinnedRefCount++;

    }

    // Safe because mPixelRef is a WrappedPixelRef type, otherwise rowBytes()

    // would require locking the pixels first.

    outBitmap->setInfo(mPixelRef->info(), mPixelRef->rowBytes());

    outBitmap->setPixelRef(mPixelRef.get())->unref();

    outBitmap->setPixelRef(refPixelRefLocked())->unref();

    outBitmap->setHasHardwareMipMap(hasHardwareMipMap());

}
@@ -323,7 +340,7 @@ public: 
    }



    void* pixels() {

        return mBitmap->pixelRef()->pixels();

        return mBitmap->peekAtPixelRef()->pixels();

    }



    bool valid() {
@@ -780,7 +797,7 @@ static jint Bitmap_config(JNIEnv* env, jobject, jlong bitmapHandle) { 


static jint Bitmap_getGenerationId(JNIEnv* env, jobject, jlong bitmapHandle) {

    LocalScopedBitmap bitmap(bitmapHandle);

    return static_cast<jint>(bitmap->pixelRef()->getGenerationID());

    return static_cast<jint>(bitmap->peekAtPixelRef()->getGenerationID());

}



static jboolean Bitmap_isPremultiplied(JNIEnv* env, jobject, jlong bitmapHandle) {
@@ -800,10 +817,10 @@ static void Bitmap_setHasAlpha(JNIEnv* env, jobject, jlong bitmapHandle, 
        jboolean hasAlpha, jboolean requestPremul) {

    LocalScopedBitmap bitmap(bitmapHandle);

    if (hasAlpha) {

        bitmap->pixelRef()->changeAlphaType(

        bitmap->peekAtPixelRef()->changeAlphaType(

                requestPremul ? kPremul_SkAlphaType : kUnpremul_SkAlphaType);

    } else {

        bitmap->pixelRef()->changeAlphaType(kOpaque_SkAlphaType);

        bitmap->peekAtPixelRef()->changeAlphaType(kOpaque_SkAlphaType);

    }

}
@@ -812,9 +829,9 @@ static void Bitmap_setPremultiplied(JNIEnv* env, jobject, jlong bitmapHandle, 
    LocalScopedBitmap bitmap(bitmapHandle);

    if (!bitmap->info().isOpaque()) {

        if (isPremul) {

            bitmap->pixelRef()->changeAlphaType(kPremul_SkAlphaType);

            bitmap->peekAtPixelRef()->changeAlphaType(kPremul_SkAlphaType);

        } else {

            bitmap->pixelRef()->changeAlphaType(kUnpremul_SkAlphaType);

            bitmap->peekAtPixelRef()->changeAlphaType(kUnpremul_SkAlphaType);

        }

    }

}
@@ -1164,7 +1181,7 @@ static jboolean Bitmap_sameAs(JNIEnv* env, jobject, jlong bm0Handle, 


static jlong Bitmap_refPixelRef(JNIEnv* env, jobject, jlong bitmapHandle) {

    LocalScopedBitmap bitmap(bitmapHandle);

    SkPixelRef* pixelRef = bitmap.valid() ? bitmap->pixelRef() : nullptr;

    SkPixelRef* pixelRef = bitmap.valid() ? bitmap->peekAtPixelRef() : nullptr;

    SkSafeRef(pixelRef);

    return reinterpret_cast<jlong>(pixelRef);

}

core/jni/android/graphics/Bitmap.h

+3 −1
Original line number Diff line number Diff line
@@ -62,7 +62,8 @@ public: 
    int width() const { return info().width(); }

    int height() const { return info().height(); }

    size_t rowBytes() const;

    SkPixelRef* pixelRef() const;

    SkPixelRef* peekAtPixelRef() const;

    SkPixelRef* refPixelRef();

    bool valid() const { return mPixelStorageType != PixelStorageType::Invalid; }



    void reconfigure(const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable);
@@ -88,6 +89,7 @@ private: 
    JNIEnv* jniEnv();

    bool shouldDisposeSelfLocked();

    void assertValid() const;

    SkPixelRef* refPixelRefLocked();



    android::Mutex mLock;

    int mPinnedRefCount = 0;

core/jni/android/graphics/BitmapFactory.cpp

+2 −2
Original line number Diff line number Diff line
@@ -184,7 +184,7 @@ public: 
        }



        mBitmap->reconfigure(info, bitmap->rowBytes(), ctable);

        bitmap->setPixelRef(mBitmap->pixelRef());

        bitmap->setPixelRef(mBitmap->refPixelRef())->unref();



        // since we're already allocated, we lockPixels right away

        // HeapAllocator/JavaPixelAllocator behaves this way too
@@ -258,7 +258,7 @@ static jobject doDecode(JNIEnv* env, SkStreamRewindable* stream, jobject padding 
    unsigned int existingBufferSize = 0;

    if (javaBitmap != NULL) {

        reuseBitmap = GraphicsJNI::getBitmap(env, javaBitmap);

        if (reuseBitmap->pixelRef()->isImmutable()) {

        if (reuseBitmap->peekAtPixelRef()->isImmutable()) {

            ALOGW("Unable to reuse an immutable bitmap as an image decoder target.");

            javaBitmap = NULL;

            reuseBitmap = nullptr;

core/jni/android/graphics/Graphics.cpp

+2 −2
Original line number Diff line number Diff line
@@ -352,8 +352,8 @@ void GraphicsJNI::getSkBitmap(JNIEnv* env, jobject bitmap, SkBitmap* outBitmap) 
    getBitmap(env, bitmap)->getSkBitmap(outBitmap);

}



SkPixelRef* GraphicsJNI::getSkPixelRef(JNIEnv* env, jobject bitmap) {

    return getBitmap(env, bitmap)->pixelRef();

SkPixelRef* GraphicsJNI::refSkPixelRef(JNIEnv* env, jobject bitmap) {

    return getBitmap(env, bitmap)->refPixelRef();

}



SkColorType GraphicsJNI::getNativeBitmapColorType(JNIEnv* env, jobject jconfig) {

core/jni/android/graphics/GraphicsJNI.h

+1 −1
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ public: 
    static android::Canvas* getNativeCanvas(JNIEnv*, jobject canvas);

    static android::Bitmap* getBitmap(JNIEnv*, jobject bitmap);

    static void getSkBitmap(JNIEnv*, jobject bitmap, SkBitmap* outBitmap);

    static SkPixelRef* getSkPixelRef(JNIEnv*, jobject bitmap);

    static SkPixelRef* refSkPixelRef(JNIEnv*, jobject bitmap);

    static SkRegion* getNativeRegion(JNIEnv*, jobject region);



    // Given the 'native' long held by the Rasterizer.java object, return a