commit cd0ba71aa942f35fcdb26808b86f20073b8aff92
author Chris Craik <ccraik@google.com> Fri Sep 06 14:40:30 2013 -0700
committer Chris Craik <ccraik@google.com> Mon Sep 09 14:08:11 2013 -0700
tree 204a43a0bb152f2712c51aab0bf717804d8c1a27
parent b8b017cb6879352aee015ba24e9bfd05cc094e74

Create a pixelref wrapper for reused bitmaps

Reused bitmaps may gain a color table when reused, so we wrap a new
AndroidPixelRef that holds the color table around the old.

bug:10608305
Change-Id: I35288edf3158cfda21c500360ad1abdf5654af8d

core/jni/android/graphics/BitmapFactory.cpp

diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp
index 16beb02..f9bb233 100644
--- a/core/jni/android/graphics/BitmapFactory.cpp
+++ b/core/jni/android/graphics/BitmapFactory.cpp
@@ -189,7 +189,13 @@
                     mSize, bitmap->getSize());
             return false;
         }
-        bitmap->setPixelRef(mPixelRef);
+
+        // Create a new pixelref with the new ctable that wraps the previous pixelref
+        SkPixelRef* pr = new AndroidPixelRef(*static_cast<AndroidPixelRef*>(mPixelRef), ctable);
+
+        bitmap->setPixelRef(pr)->unref();
+        // since we're already allocated, we lockPixels right away
+        // HeapAllocator/JavaPixelAllocator behaves this way too
         bitmap->lockPixels();
         return true;
     }

core/jni/android/graphics/Graphics.cpp

diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp
index ef5b7c9..1ff0d635 100644
--- a/core/jni/android/graphics/Graphics.cpp
+++ b/core/jni/android/graphics/Graphics.cpp
@@ -414,7 +414,8 @@
 ///////////////////////////////////////////////////////////////////////////////
 
 AndroidPixelRef::AndroidPixelRef(JNIEnv* env, void* storage, size_t size, jbyteArray storageObj,
-        SkColorTable* ctable) : SkMallocPixelRef(storage, size, ctable, (storageObj == NULL)) {
+        SkColorTable* ctable) : SkMallocPixelRef(storage, size, ctable, (storageObj == NULL)),
+        fWrappedPixelRef(NULL) {
     SkASSERT(storage);
     SkASSERT(env);
 
@@ -431,8 +432,25 @@
 
 }
 
+AndroidPixelRef::AndroidPixelRef(AndroidPixelRef& wrappedPixelRef, SkColorTable* ctable) :
+        SkMallocPixelRef(wrappedPixelRef.getAddr(), wrappedPixelRef.getSize(), ctable, false),
+        fWrappedPixelRef(wrappedPixelRef.fWrappedPixelRef ?
+            wrappedPixelRef.fWrappedPixelRef : &wrappedPixelRef)
+{
+    SkASSERT(fWrappedPixelRef);
+    SkSafeRef(fWrappedPixelRef);
+
+    // don't need to initialize these, as all the relevant logic delegates to the wrapped ref
+    fStorageObj = NULL;
+    fHasGlobalRef = false;
+    fGlobalRefCnt = 0;
+    fOnJavaHeap = false;
+}
+
 AndroidPixelRef::~AndroidPixelRef() {
-    if (fOnJavaHeap) {
+    if (fWrappedPixelRef) {
+        SkSafeUnref(fWrappedPixelRef);
+    } else if (fOnJavaHeap) {
         JNIEnv* env = vm2env(fVM);
 
         if (fStorageObj && fHasGlobalRef) {
@@ -441,15 +459,27 @@
         fStorageObj = NULL;
     }
 }
+jbyteArray AndroidPixelRef::getStorageObj() {
+    if (fWrappedPixelRef) {
+        return fWrappedPixelRef->fStorageObj;
+    }
+    return fStorageObj;
+}
 
 void AndroidPixelRef::setLocalJNIRef(jbyteArray arr) {
-    if (!fHasGlobalRef) {
+    if (fWrappedPixelRef) {
+        // delegate java obj management to the wrapped ref
+        fWrappedPixelRef->setLocalJNIRef(arr);
+    } else if (!fHasGlobalRef) {
         fStorageObj = arr;
     }
 }
 
 void AndroidPixelRef::globalRef(void* localref) {
-    if (fOnJavaHeap && sk_atomic_inc(&fGlobalRefCnt) == 0) {
+    if (fWrappedPixelRef) {
+        // delegate java obj management to the wrapped ref
+        fWrappedPixelRef->globalRef(localref);
+    } else if (fOnJavaHeap && sk_atomic_inc(&fGlobalRefCnt) == 0) {
         JNIEnv *env = vm2env(fVM);
 
         // If JNI ref was passed, it is always used
@@ -473,7 +503,10 @@
 }
 
 void AndroidPixelRef::globalUnref() {
-    if (fOnJavaHeap && sk_atomic_dec(&fGlobalRefCnt) == 1) {
+    if (fWrappedPixelRef) {
+        // delegate java obj management to the wrapped ref
+        fWrappedPixelRef->globalUnref();
+    } else if (fOnJavaHeap && sk_atomic_dec(&fGlobalRefCnt) == 1) {
         JNIEnv *env = vm2env(fVM);
         if (!fHasGlobalRef) {
             SkDebugf("We don't have a global ref!");

core/jni/android/graphics/GraphicsJNI.h

diff --git a/core/jni/android/graphics/GraphicsJNI.h b/core/jni/android/graphics/GraphicsJNI.h
index 5a2a6f8..f4590b9 100644
--- a/core/jni/android/graphics/GraphicsJNI.h
+++ b/core/jni/android/graphics/GraphicsJNI.h
@@ -91,9 +91,16 @@
     AndroidPixelRef(JNIEnv* env, void* storage, size_t size, jbyteArray storageObj,
                     SkColorTable* ctable);
 
+    /**
+     * Creates an AndroidPixelRef that wraps (and refs) another to reuse/share
+     * the same storage and java byte array refcounting, yet have a different
+     * color table.
+     */
+    AndroidPixelRef(AndroidPixelRef& wrappedPixelRef, SkColorTable* ctable);
+
     virtual ~AndroidPixelRef();
 
-    jbyteArray getStorageObj() { return fStorageObj; }
+    jbyteArray getStorageObj();
 
     void setLocalJNIRef(jbyteArray arr);
 
@@ -110,6 +117,8 @@
     virtual void globalUnref();
 
 private:
+    AndroidPixelRef* const fWrappedPixelRef; // if set, delegate memory management calls to this
+
     JavaVM* fVM;
     bool fOnJavaHeap; // If true, the memory was allocated on the Java heap