Fix NDK access to recycle'd bitmaps

Also kills off one user of GraphicsJNI.h!

Change-Id: Icbf979e485b3b6ec2f37e18ff654b8ff1e44fb35
Fixes: 34712423
Test: cts CtsGraphicsTestCases --test android.graphics.cts.BitmapTest#testNdkAccessAfterRecycle passes
This commit is contained in:
John Reck
2017-03-01 18:05:41 -08:00
parent d632305e37
commit 00799f760d
3 changed files with 81 additions and 58 deletions

View File

@@ -232,6 +232,73 @@ Bitmap& toBitmap(JNIEnv* env, jlong bitmapHandle) {
return localBitmap->bitmap();
}
void imageInfo(JNIEnv* env, jobject bitmap, AndroidBitmapInfo* info) {
SkASSERT(info);
SkASSERT(env);
SkASSERT(bitmap);
SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_nativePtr);
LocalScopedBitmap localBitmap(bitmapHandle);
const SkImageInfo& imageInfo = localBitmap->info();
info->width = imageInfo.width();
info->height = imageInfo.height();
info->stride = localBitmap->rowBytes();
info->flags = 0;
switch (imageInfo.colorType()) {
case kN32_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGBA_8888;
break;
case kRGB_565_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGB_565;
break;
case kARGB_4444_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGBA_4444;
break;
case kAlpha_8_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_A_8;
break;
default:
info->format = ANDROID_BITMAP_FORMAT_NONE;
break;
}
}
void* lockPixels(JNIEnv* env, jobject bitmap) {
SkASSERT(env);
SkASSERT(bitmap);
SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_nativePtr);
LocalScopedBitmap localBitmap(bitmapHandle);
if (!localBitmap->valid()) return nullptr;
SkPixelRef& pixelRef = localBitmap->bitmap();
pixelRef.lockPixels();
if (!pixelRef.pixels()) {
pixelRef.unlockPixels();
return nullptr;
}
pixelRef.ref();
return pixelRef.pixels();
}
bool unlockPixels(JNIEnv* env, jobject bitmap) {
SkASSERT(env);
SkASSERT(bitmap);
SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_nativePtr);
LocalScopedBitmap localBitmap(bitmapHandle);
if (!localBitmap->valid()) return false;
SkPixelRef& pixelRef = localBitmap->bitmap();
pixelRef.notifyPixelsChanged();
pixelRef.unlockPixels();
pixelRef.unref();
return true;
}
} // namespace bitmap
} // namespace android

View File

@@ -17,6 +17,7 @@
#define BITMAP_H_
#include <jni.h>
#include <android/bitmap.h>
#include <SkBitmap.h>
#include <SkColorTable.h>
#include <SkImageInfo.h>
@@ -44,6 +45,13 @@ void toSkBitmap(jlong bitmapHandle, SkBitmap* outBitmap);
Bitmap& toBitmap(JNIEnv* env, jobject bitmap);
Bitmap& toBitmap(JNIEnv* env, jlong bitmapHandle);
// NDK access
void imageInfo(JNIEnv* env, jobject bitmap, AndroidBitmapInfo* info);
// Returns a pointer to the pixels or nullptr if the bitmap is not valid
void* lockPixels(JNIEnv* env, jobject bitmap);
// Returns true if unlocked, false if the bitmap is no longer valid (destroyed)
bool unlockPixels(JNIEnv* env, jobject bitmap);
/** Reinitialize a bitmap. bitmap must already have its SkAlphaType set in
sync with isPremultiplied
*/

View File

@@ -15,11 +15,7 @@
*/
#include <android/bitmap.h>
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wunused-parameter"
#include <GraphicsJNI.h>
#pragma GCC diagnostic pop
#include <Bitmap.h>
int AndroidBitmap_getInfo(JNIEnv* env, jobject jbitmap,
AndroidBitmapInfo* info) {
@@ -27,32 +23,8 @@ int AndroidBitmap_getInfo(JNIEnv* env, jobject jbitmap,
return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
}
SkBitmap bm;
GraphicsJNI::getSkBitmap(env, jbitmap, &bm);
if (info) {
info->width = bm.width();
info->height = bm.height();
info->stride = bm.rowBytes();
info->flags = 0;
switch (bm.colorType()) {
case kN32_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGBA_8888;
break;
case kRGB_565_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGB_565;
break;
case kARGB_4444_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGBA_4444;
break;
case kAlpha_8_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_A_8;
break;
default:
info->format = ANDROID_BITMAP_FORMAT_NONE;
break;
}
android::bitmap::imageInfo(env, jbitmap, info);
}
return ANDROID_BITMAP_RESULT_SUCCESS;
}
@@ -62,19 +34,11 @@ int AndroidBitmap_lockPixels(JNIEnv* env, jobject jbitmap, void** addrPtr) {
return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
}
SkPixelRef* pixelRef = GraphicsJNI::refSkPixelRef(env, jbitmap);
if (!pixelRef) {
void* addr = android::bitmap::lockPixels(env, jbitmap);
if (!addr) {
return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
}
pixelRef->lockPixels();
void* addr = pixelRef->pixels();
if (NULL == addr) {
pixelRef->unlockPixels();
pixelRef->unref();
return ANDROID_BITMAP_RESULT_ALLOCATION_FAILED;
}
if (addrPtr) {
*addrPtr = addr;
}
@@ -86,26 +50,10 @@ int AndroidBitmap_unlockPixels(JNIEnv* env, jobject jbitmap) {
return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
}
SkPixelRef* pixelRef = GraphicsJNI::refSkPixelRef(env, jbitmap);
if (!pixelRef) {
bool unlocked = android::bitmap::unlockPixels(env, jbitmap);
if (!unlocked) {
return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
}
// notifyPixelsChanged() needs be called to apply writes to GL-backed
// bitmaps. Note that this will slow down read-only accesses to the
// bitmaps, but the NDK methods are primarily intended to be used for
// writes.
pixelRef->notifyPixelsChanged();
pixelRef->unlockPixels();
// Awkward in that we need to double-unref as the call to get the SkPixelRef
// did a ref(), so we need to unref() for the local ref and for the previous
// AndroidBitmap_lockPixels(). However this keeps GraphicsJNI a bit safer
// if others start using it without knowing about android::Bitmap's "fun"
// ref counting mechanism(s).
pixelRef->unref();
pixelRef->unref();
return ANDROID_BITMAP_RESULT_SUCCESS;
}