Change how Java Bitmaps are accessed in a few places

Stop assuming that a Java Bitmap has a SkBitmap* that
has some externally managed lifecycle, and instead switch
a bunch of users to accessing the bitmap by providing
their own SkBitmap* on which to set the (ref counted!)
SkPixelRef* instead

Attempt #2 to land this, original issue was in getSkBitmap
and should be fixed

Change-Id: I0fd9e193968b41e5597784140d56b4885906864a
This commit is contained in:
John Reck
2015-04-10 13:52:57 -07:00
parent b77a755112
commit ed207b9274
14 changed files with 85 additions and 72 deletions

View File

@@ -261,7 +261,7 @@ static jobject doDecode(JNIEnv* env, SkStreamRewindable* stream, jobject padding
SkBitmap* outputBitmap = NULL;
unsigned int existingBufferSize = 0;
if (javaBitmap != NULL) {
outputBitmap = GraphicsJNI::getSkBitmap(env, javaBitmap);
outputBitmap = GraphicsJNI::getSkBitmapDeprecated(env, javaBitmap);
if (outputBitmap->isImmutable()) {
ALOGW("Unable to reuse an immutable bitmap as an image decoder target.");
javaBitmap = NULL;

View File

@@ -217,7 +217,7 @@ static jobject nativeDecodeRegion(JNIEnv* env, jobject, jlong brdHandle,
if (tileBitmap != NULL) {
// Re-use bitmap.
bitmap = GraphicsJNI::getSkBitmap(env, tileBitmap);
bitmap = GraphicsJNI::getSkBitmapDeprecated(env, tileBitmap);
}
if (bitmap == NULL) {
bitmap = new SkBitmap;

View File

@@ -338,7 +338,7 @@ SkColorType GraphicsJNI::legacyBitmapConfigToColorType(jint legacyConfig) {
return static_cast<SkColorType>(gConfig2ColorType[legacyConfig]);
}
SkBitmap* GraphicsJNI::getSkBitmap(JNIEnv* env, jobject bitmap) {
SkBitmap* GraphicsJNI::getSkBitmapDeprecated(JNIEnv* env, jobject bitmap) {
SkASSERT(env);
SkASSERT(bitmap);
SkASSERT(env->IsInstanceOf(bitmap, gBitmap_class));
@@ -348,6 +348,19 @@ SkBitmap* GraphicsJNI::getSkBitmap(JNIEnv* env, jobject bitmap) {
return b;
}
void GraphicsJNI::getSkBitmap(JNIEnv* env, jobject bitmap, SkBitmap* outBitmap) {
// TODO: We have to copy from the existing bitmap due to rowBytes not
// being updated on the SkPixelRef at reconfigure time. This is a short term
// problem that will be fixed with the specialized wrapper
*outBitmap = *getSkBitmapDeprecated(env, bitmap);
}
SkPixelRef* GraphicsJNI::getSkPixelRef(JNIEnv* env, jobject bitmap) {
jlong bitmapHandle = env->GetLongField(bitmap, gBitmap_skBitmapPtr);
SkBitmap* b = reinterpret_cast<SkBitmap*>(bitmapHandle);
return b->pixelRef();
}
SkColorType GraphicsJNI::getNativeBitmapColorType(JNIEnv* env, jobject jconfig) {
SkASSERT(env);
if (NULL == jconfig) {

View File

@@ -49,7 +49,9 @@ public:
static void point_to_jpointf(const SkPoint& point, JNIEnv*, jobject jpointf);
static android::Canvas* getNativeCanvas(JNIEnv*, jobject canvas);
static SkBitmap* getSkBitmap(JNIEnv*, jobject bitmap);
static SkBitmap* getSkBitmapDeprecated(JNIEnv*, jobject bitmap);
static void getSkBitmap(JNIEnv*, jobject bitmap, SkBitmap* outBitmap);
static SkPixelRef* getSkPixelRef(JNIEnv*, jobject bitmap);
static SkRegion* getNativeRegion(JNIEnv*, jobject region);
// Given the 'native' long held by the Rasterizer.java object, return a

View File

@@ -243,19 +243,21 @@ static void renderPageBitmap(FPDF_BITMAP bitmap, FPDF_PAGE page, int destLeft, i
}
static void nativeRenderPage(JNIEnv* env, jclass thiz, jlong documentPtr, jlong pagePtr,
jlong bitmapPtr, jint destLeft, jint destTop, jint destRight, jint destBottom,
jobject jbitmap, jint destLeft, jint destTop, jint destRight, jint destBottom,
jlong matrixPtr, jint renderMode) {
FPDF_PAGE page = reinterpret_cast<FPDF_PAGE>(pagePtr);
SkBitmap* skBitmap = reinterpret_cast<SkBitmap*>(bitmapPtr);
SkMatrix* skMatrix = reinterpret_cast<SkMatrix*>(matrixPtr);
skBitmap->lockPixels();
SkBitmap skBitmap;
GraphicsJNI::getSkBitmap(env, jbitmap, &skBitmap);
const int stride = skBitmap->width() * 4;
SkAutoLockPixels alp(skBitmap);
FPDF_BITMAP bitmap = FPDFBitmap_CreateEx(skBitmap->width(), skBitmap->height(),
FPDFBitmap_BGRA, skBitmap->getPixels(), stride);
const int stride = skBitmap.width() * 4;
FPDF_BITMAP bitmap = FPDFBitmap_CreateEx(skBitmap.width(), skBitmap.height(),
FPDFBitmap_BGRA, skBitmap.getPixels(), stride);
if (!bitmap) {
ALOGE("Erorr creating bitmap");
@@ -278,8 +280,7 @@ static void nativeRenderPage(JNIEnv* env, jclass thiz, jlong documentPtr, jlong
renderPageBitmap(bitmap, page, destLeft, destTop, destRight,
destBottom, skMatrix, renderFlags);
skBitmap->notifyPixelsChanged();
skBitmap->unlockPixels();
skBitmap.notifyPixelsChanged();
}
static JNINativeMethod gPdfRenderer_Methods[] = {
@@ -287,7 +288,7 @@ static JNINativeMethod gPdfRenderer_Methods[] = {
{"nativeClose", "(J)V", (void*) nativeClose},
{"nativeGetPageCount", "(J)I", (void*) nativeGetPageCount},
{"nativeScaleForPrinting", "(J)Z", (void*) nativeScaleForPrinting},
{"nativeRenderPage", "(JJJIIIIJI)V", (void*) nativeRenderPage},
{"nativeRenderPage", "(JJLandroid/graphics/Bitmap;IIIIJI)V", (void*) nativeRenderPage},
{"nativeOpenPageAndGetSize", "(JILandroid/graphics/Point;)J", (void*) nativeOpenPageAndGetSize},
{"nativeClosePage", "(J)V", (void*) nativeClosePage}
};

View File

@@ -618,23 +618,25 @@ static int getType(SkColorType colorType)
static jint util_getInternalFormat(JNIEnv *env, jclass clazz,
jobject jbitmap)
{
SkBitmap const * nativeBitmap = GraphicsJNI::getSkBitmap(env, jbitmap);
return getInternalFormat(nativeBitmap->colorType());
SkBitmap nativeBitmap;
GraphicsJNI::getSkBitmap(env, jbitmap, &nativeBitmap);
return getInternalFormat(nativeBitmap.colorType());
}
static jint util_getType(JNIEnv *env, jclass clazz,
jobject jbitmap)
{
SkBitmap const * nativeBitmap = GraphicsJNI::getSkBitmap(env, jbitmap);
return getType(nativeBitmap->colorType());
SkBitmap nativeBitmap;
GraphicsJNI::getSkBitmap(env, jbitmap, &nativeBitmap);
return getType(nativeBitmap.colorType());
}
static jint util_texImage2D(JNIEnv *env, jclass clazz,
jint target, jint level, jint internalformat,
jobject jbitmap, jint type, jint border)
{
SkBitmap const * nativeBitmap = GraphicsJNI::getSkBitmap(env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(env, jbitmap, &bitmap);
SkColorType colorType = bitmap.colorType();
if (internalformat < 0) {
internalformat = getInternalFormat(colorType);
@@ -680,8 +682,8 @@ static jint util_texSubImage2D(JNIEnv *env, jclass clazz,
jint target, jint level, jint xoffset, jint yoffset,
jobject jbitmap, jint format, jint type)
{
SkBitmap const * nativeBitmap = GraphicsJNI::getSkBitmap(env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(env, jbitmap, &bitmap);
SkColorType colorType = bitmap.colorType();
if (format < 0) {
format = getInternalFormat(colorType);

View File

@@ -42,7 +42,7 @@ static void finalizer(JNIEnv* env, jobject clazz, jlong canvasHandle) {
static jlong initRaster(JNIEnv* env, jobject, jobject jbitmap) {
SkBitmap* bitmap = nullptr;
if (jbitmap != NULL) {
bitmap = GraphicsJNI::getSkBitmap(env, jbitmap);
bitmap = GraphicsJNI::getSkBitmapDeprecated(env, jbitmap);
}
return reinterpret_cast<jlong>(Canvas::create_canvas(
bitmap ? *bitmap : SkBitmap()));
@@ -53,7 +53,7 @@ static jlong initRaster(JNIEnv* env, jobject, jobject jbitmap) {
static void setBitmap(JNIEnv* env, jobject, jlong canvasHandle, jobject jbitmap) {
SkBitmap* bitmap = nullptr;
if (jbitmap != NULL) {
bitmap = GraphicsJNI::getSkBitmap(env, jbitmap);
bitmap = GraphicsJNI::getSkBitmapDeprecated(env, jbitmap);
}
get_canvas(canvasHandle)->setBitmap(bitmap ? *bitmap : SkBitmap());
}

View File

@@ -80,10 +80,7 @@ status_t android_view_PointerIcon_load(JNIEnv* env, jobject pointerIconObj, jobj
jobject bitmapObj = env->GetObjectField(loadedPointerIconObj, gPointerIconClassInfo.mBitmap);
if (bitmapObj) {
SkBitmap* bitmap = GraphicsJNI::getSkBitmap(env, bitmapObj);
if (bitmap) {
outPointerIcon->bitmap = *bitmap; // use a shared pixel ref
}
GraphicsJNI::getSkBitmap(env, bitmapObj, &(outPointerIcon->bitmap));
env->DeleteLocalRef(bitmapObj);
}

View File

@@ -277,8 +277,9 @@ static void jni_eglCreatePixmapSurface(JNIEnv *_env, jobject _this, jobject out_
EGLConfig cnf = getConfig(_env, config);
jint* base = 0;
SkBitmap const * nativeBitmap = GraphicsJNI::getSkBitmap(_env, native_pixmap);
SkPixelRef* ref = nativeBitmap ? nativeBitmap->pixelRef() : 0;
SkBitmap nativeBitmap;
GraphicsJNI::getSkBitmap(_env, native_pixmap, &nativeBitmap);
SkPixelRef* ref = nativeBitmap.pixelRef();
if (ref == NULL) {
jniThrowException(_env, "java/lang/IllegalArgumentException", "Bitmap has no PixelRef");
return;
@@ -289,10 +290,10 @@ static void jni_eglCreatePixmapSurface(JNIEnv *_env, jobject _this, jobject out_
egl_native_pixmap_t pixmap;
pixmap.version = sizeof(pixmap);
pixmap.width = nativeBitmap->width();
pixmap.height = nativeBitmap->height();
pixmap.stride = nativeBitmap->rowBytes() / nativeBitmap->bytesPerPixel();
pixmap.format = convertPixelFormat(nativeBitmap->colorType());
pixmap.width = nativeBitmap.width();
pixmap.height = nativeBitmap.height();
pixmap.stride = nativeBitmap.rowBytes() / nativeBitmap.bytesPerPixel();
pixmap.format = convertPixelFormat(nativeBitmap.colorType());
pixmap.data = (uint8_t*)ref->pixels();
base = beginNativeAttribList(_env, attrib_list);

View File

@@ -380,7 +380,7 @@ public final class PdfRenderer implements AutoCloseable {
final long transformPtr = (transform != null) ? transform.native_instance : 0;
nativeRenderPage(mNativeDocument, mNativePage, destination.getSkBitmap(), contentLeft,
nativeRenderPage(mNativeDocument, mNativePage, destination, contentLeft,
contentTop, contentRight, contentBottom, transformPtr, renderMode);
}
@@ -425,7 +425,7 @@ public final class PdfRenderer implements AutoCloseable {
private static native void nativeClose(long documentPtr);
private static native int nativeGetPageCount(long documentPtr);
private static native boolean nativeScaleForPrinting(long documentPtr);
private static native void nativeRenderPage(long documentPtr, long pagePtr, long destPtr,
private static native void nativeRenderPage(long documentPtr, long pagePtr, Bitmap dest,
int destLeft, int destTop, int destRight, int destBottom, long matrixPtr, int renderMode);
private static native long nativeOpenPageAndGetSize(long documentPtr, int pageIndex,
Point outSize);

View File

@@ -299,15 +299,16 @@ static jobject android_media_MediaMetadataRetriever_getFrameAtTime(JNIEnv *env,
return NULL;
}
SkBitmap *bitmap = GraphicsJNI::getSkBitmap(env, jBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(env, jBitmap, &bitmap);
bitmap->lockPixels();
rotate((uint16_t*)bitmap->getPixels(),
bitmap.lockPixels();
rotate((uint16_t*)bitmap.getPixels(),
(uint16_t*)((char*)videoFrame + sizeof(VideoFrame)),
videoFrame->mWidth,
videoFrame->mHeight,
videoFrame->mRotationAngle);
bitmap->unlockPixels();
bitmap.unlockPixels();
if (videoFrame->mDisplayWidth != videoFrame->mWidth ||
videoFrame->mDisplayHeight != videoFrame->mHeight) {

View File

@@ -27,18 +27,16 @@ int AndroidBitmap_getInfo(JNIEnv* env, jobject jbitmap,
return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
}
SkBitmap* bm = GraphicsJNI::getSkBitmap(env, jbitmap);
if (NULL == bm) {
return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
}
SkBitmap bm;
GraphicsJNI::getSkBitmap(env, jbitmap, &bm);
if (info) {
info->width = bm->width();
info->height = bm->height();
info->stride = bm->rowBytes();
info->width = bm.width();
info->height = bm.height();
info->stride = bm.rowBytes();
info->flags = 0;
switch (bm->colorType()) {
switch (bm.colorType()) {
case kN32_SkColorType:
info->format = ANDROID_BITMAP_FORMAT_RGBA_8888;
break;
@@ -64,17 +62,18 @@ int AndroidBitmap_lockPixels(JNIEnv* env, jobject jbitmap, void** addrPtr) {
return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
}
SkBitmap* bm = GraphicsJNI::getSkBitmap(env, jbitmap);
if (NULL == bm) {
SkPixelRef* pixelRef = GraphicsJNI::getSkPixelRef(env, jbitmap);
if (!pixelRef) {
return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
}
bm->lockPixels();
void* addr = bm->getPixels();
pixelRef->lockPixels();
void* addr = pixelRef->pixels();
if (NULL == addr) {
bm->unlockPixels();
pixelRef->unlockPixels();
return ANDROID_BITMAP_RESULT_ALLOCATION_FAILED;
}
pixelRef->ref();
if (addrPtr) {
*addrPtr = addr;
@@ -87,8 +86,8 @@ int AndroidBitmap_unlockPixels(JNIEnv* env, jobject jbitmap) {
return ANDROID_BITMAP_RESULT_BAD_PARAMETER;
}
SkBitmap* bm = GraphicsJNI::getSkBitmap(env, jbitmap);
if (NULL == bm) {
SkPixelRef* pixelRef = GraphicsJNI::getSkPixelRef(env, jbitmap);
if (!pixelRef) {
return ANDROID_BITMAP_RESULT_JNI_EXCEPTION;
}
@@ -96,9 +95,11 @@ int AndroidBitmap_unlockPixels(JNIEnv* env, jobject jbitmap) {
// 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.
bm->notifyPixelsChanged();
pixelRef->notifyPixelsChanged();
pixelRef->unlockPixels();
pixelRef->unref();
bm->unlockPixels();
return ANDROID_BITMAP_RESULT_SUCCESS;
}

View File

@@ -1102,9 +1102,8 @@ static jlong
nAllocationCreateFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong type, jint mip,
jobject jbitmap, jint usage)
{
SkBitmap const * nativeBitmap =
GraphicsJNI::getSkBitmap(_env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
bitmap.lockPixels();
const void* ptr = bitmap.getPixels();
@@ -1119,9 +1118,8 @@ static jlong
nAllocationCreateBitmapBackedAllocation(JNIEnv *_env, jobject _this, jlong con, jlong type,
jint mip, jobject jbitmap, jint usage)
{
SkBitmap const * nativeBitmap =
GraphicsJNI::getSkBitmap(_env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
bitmap.lockPixels();
const void* ptr = bitmap.getPixels();
@@ -1136,9 +1134,8 @@ static jlong
nAllocationCubeCreateFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong type, jint mip,
jobject jbitmap, jint usage)
{
SkBitmap const * nativeBitmap =
GraphicsJNI::getSkBitmap(_env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
bitmap.lockPixels();
const void* ptr = bitmap.getPixels();
@@ -1152,9 +1149,8 @@ nAllocationCubeCreateFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong ty
static void
nAllocationCopyFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong alloc, jobject jbitmap)
{
SkBitmap const * nativeBitmap =
GraphicsJNI::getSkBitmap(_env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
int w = bitmap.width();
int h = bitmap.height();
@@ -1169,9 +1165,8 @@ nAllocationCopyFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong alloc, j
static void
nAllocationCopyToBitmap(JNIEnv *_env, jobject _this, jlong con, jlong alloc, jobject jbitmap)
{
SkBitmap const * nativeBitmap =
GraphicsJNI::getSkBitmap(_env, jbitmap);
const SkBitmap& bitmap(*nativeBitmap);
SkBitmap bitmap;
GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
bitmap.lockPixels();
void* ptr = bitmap.getPixels();

View File

@@ -64,7 +64,7 @@ namespace android {
static jboolean com_android_server_AssetAtlasService_upload(JNIEnv* env, jobject,
jobject graphicBuffer, jobject bitmapHandle) {
SkBitmap& bitmap = *GraphicsJNI::getSkBitmap(env, bitmapHandle);
SkBitmap& bitmap = *GraphicsJNI::getSkBitmapDeprecated(env, bitmapHandle);
SkAutoLockPixels alp(bitmap);
// The goal of this method is to copy the bitmap into the GraphicBuffer