Merge "Ensure all fields of AutoBufferPointer are initialized"

core/jni/android_nio_utils.cpp

@@ -18,42 +18,51 @@
 
 #include "core_jni_helpers.h"
 
-void* android::nio_getPointer(JNIEnv *_env, jobject buffer, jarray *array) {
-    assert(array);
+namespace {
 
+void* getPointer(JNIEnv *_env, jobject buffer, jarray *array, void** elements) {
+    assert(array);
     jint position;
     jint limit;
     jint elementSizeShift;
     jlong pointer = jniGetNioBufferFields(_env, buffer, &position, &limit, &elementSizeShift);
     if (pointer != 0L) {
+        *array = nullptr;
+        *elements = nullptr;
         pointer += position << elementSizeShift;
         return reinterpret_cast<void*>(pointer);
     }
-
     jint offset = jniGetNioBufferBaseArrayOffset(_env, buffer);
     *array = jniGetNioBufferBaseArray(_env, buffer);
-    void * data = _env->GetPrimitiveArrayCritical(*array, (jboolean *) 0);
-    return reinterpret_cast<void*>(reinterpret_cast<char*>(data) + offset);
+    *elements = _env->GetPrimitiveArrayCritical(*array, (jboolean *) 0);
+    return reinterpret_cast<void*>(reinterpret_cast<char*>(*elements) + offset);
 }
 
+void releasePointer(JNIEnv *_env, jarray array, void *elements, jboolean commit) {
+    _env->ReleasePrimitiveArrayCritical(array, elements, commit ? 0 : JNI_ABORT);
+}
 
-void android::nio_releasePointer(JNIEnv *_env, jarray array, void *data,
-                                jboolean commit) {
-    _env->ReleasePrimitiveArrayCritical(array, data,
-                                        commit ? 0 : JNI_ABORT);
+}  // namespace
+
+void* android::nio_getPointer(JNIEnv *_env, jobject buffer, jarray *array) {
+    void* elements;
+    return getPointer(_env, buffer, array, &elements);
+}
+
+void android::nio_releasePointer(JNIEnv *_env, jarray array, void *data, jboolean commit) {
+    releasePointer(_env, array, data, commit);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
-android::AutoBufferPointer::AutoBufferPointer(JNIEnv* env, jobject nioBuffer,
-                                              jboolean commit) {
+android::AutoBufferPointer::AutoBufferPointer(JNIEnv* env, jobject nioBuffer, jboolean commit) {
     fEnv = env;
     fCommit = commit;
-    fPointer = android::nio_getPointer(env, nioBuffer, &fArray);
+    fPointer = getPointer(env, nioBuffer, &fArray, &fElements);
 }
 
 android::AutoBufferPointer::~AutoBufferPointer() {
-    if (NULL != fArray) {
-        android::nio_releasePointer(fEnv, fArray, fPointer, fCommit);
+    if (nullptr != fArray) {
+        releasePointer(fEnv, fArray, fElements, fCommit);
     }
 }

core/jni/android_nio_utils.h

@@ -20,7 +20,7 @@
 #include <android_runtime/AndroidRuntime.h>
 
 namespace android {
-    
+
 /**
  * Given an nio.Buffer, return a pointer to it, beginning at its current
  * position. The returned pointer is only valid for the current JNI stack-frame.
@@ -63,9 +63,10 @@ public:
 
 private:
     JNIEnv* fEnv;
-    void*   fPointer;
-    jarray  fArray;
-    jboolean fCommit;
+    void*   fPointer;   // pointer to current buffer position.
+    void*   fElements;  // pointer to array element 0 (may be directly in fArray or a copy).
+    jarray  fArray;     // pointer to array on managed heap.
+    jboolean fCommit;   // commit data to source if required (when fElements is a copy of fArray).
 };
 
 }   /* namespace android */

core/tests/coretests/src/android/graphics/BitmapTest.java

@@ -22,6 +22,8 @@ import androidx.test.filters.SmallTest;
 
 import junit.framework.TestCase;
 
+import java.nio.ByteBuffer;
+
 public class BitmapTest extends TestCase {
 
     @SmallTest
@@ -262,4 +264,74 @@ public class BitmapTest extends TestCase {
         assertFalse(hardwareBitmap.isMutable());
         assertEquals(ColorSpace.get(ColorSpace.Named.DISPLAY_P3), hardwareBitmap.getColorSpace());
     }
+
+    @SmallTest
+    public void testCopyWithDirectBuffer() {
+        // Initialize Bitmap
+        final int width = 2;
+        final int height = 2;
+        Bitmap bm1 = Bitmap.createBitmap(width, height, Bitmap.Config.RGB_565);
+        bm1.setPixels(new int[] { 0xff, 0xeeee, 0xdddddd, 0xcccccccc }, 0, 2, 0, 0, 2, 2);
+
+        // Copy bytes to direct buffer, buffer is padded by fixed amount (pad bytes) either side
+        // of bitmap.
+        final int pad = 1;
+        final byte padValue = 0x5a;
+        final int bufferSize = pad + width * height * 2 + pad;
+        ByteBuffer directBuffer = ByteBuffer.allocateDirect(bufferSize);
+
+        // Write padding
+        directBuffer.put(0, padValue);
+        directBuffer.put(directBuffer.limit() - 1, padValue);
+
+        // Copy bitmap
+        directBuffer.position(pad);
+        bm1.copyPixelsToBuffer(directBuffer);
+        assertEquals(directBuffer.position(), pad + width * height * 2);
+
+        // Check padding
+        assertEquals(directBuffer.get(0), padValue);
+        assertEquals(directBuffer.get(directBuffer.limit() - 1), padValue);
+
+        // Create bitmap from direct buffer and check match.
+        directBuffer.position(pad);
+        Bitmap bm2 = Bitmap.createBitmap(width, height, Bitmap.Config.RGB_565);
+        bm2.copyPixelsFromBuffer(directBuffer);
+        assertTrue(bm2.sameAs(bm1));
+    }
+
+    @SmallTest
+    public void testCopyWithHeapBuffer() {
+        // Initialize Bitmap
+        final int width = 2;
+        final int height = 2;
+        Bitmap bm1 = Bitmap.createBitmap(width, height, Bitmap.Config.RGB_565);
+        bm1.setPixels(new int[] { 0xff, 0xeeee, 0xdddddd, 0xcccccccc }, 0, 2, 0, 0, 2, 2);
+
+        // Copy bytes to heap buffer, buffer is padded by fixed amount (pad bytes) either side
+        // of bitmap.
+        final int pad = 1;
+        final byte padValue = 0x5a;
+        final int bufferSize = pad + width * height * 2 + pad;
+        ByteBuffer heapBuffer = ByteBuffer.allocate(bufferSize);
+
+        // Write padding
+        heapBuffer.put(0, padValue);
+        heapBuffer.put(heapBuffer.limit() - 1, padValue);
+
+        // Copy bitmap
+        heapBuffer.position(pad);
+        bm1.copyPixelsToBuffer(heapBuffer);
+        assertEquals(heapBuffer.position(), pad + width * height * 2);
+
+        // Check padding
+        assertEquals(heapBuffer.get(0), padValue);
+        assertEquals(heapBuffer.get(heapBuffer.limit() - 1), padValue);
+
+        // Create bitmap from heap buffer and check match.
+        heapBuffer.position(pad);
+        Bitmap bm2 = Bitmap.createBitmap(width, height, Bitmap.Config.RGB_565);
+        bm2.copyPixelsFromBuffer(heapBuffer);
+        assertTrue(bm2.sameAs(bm1));
+    }
 }