bitmaps and pixelrefs no longer need lock/unlock

Test: covered by CtsGraphicsTestCases and hwui_unit_tests

Change-Id: I42a25961a2eb3755d2fd22a617aa21a24ec0994f

13 files changed, 2 insertions, 69 deletions

diff --git a/cmds/bootanimation/BootAnimation.cpp b/cmds/bootanimation/BootAnimation.cpp
index 7394490fc8d4..4f772c379098 100644
--- a/cmds/bootanimation/BootAnimation.cpp
+++ b/cmds/bootanimation/BootAnimation.cpp
@@ -158,10 +158,6 @@ status_t BootAnimation::initTexture(Texture* texture, AssetManager& assets,
     asset->close();
     delete asset;
 
-    // ensure we can call getPixels(). No need to call unlock, since the
-    // bitmap will go out of scope when we return from this method.
-    bitmap.lockPixels();
-
     const int w = bitmap.width();
     const int h = bitmap.height();
     const void* p = bitmap.getPixels();
@@ -216,10 +212,6 @@ status_t BootAnimation::initTexture(FileMap* map, int* width, int* height)
     // the packed resource can be released.
     delete map;
 
-    // ensure we can call getPixels(). No need to call unlock, since the
-    // bitmap will go out of scope when we return from this method.
-    bitmap.lockPixels();
-
     const int w = bitmap.width();
     const int h = bitmap.height();
     const void* p = bitmap.getPixels();
diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp
index bd9cc6430487..dd5632e49b72 100755
--- a/core/jni/android/graphics/Bitmap.cpp
+++ b/core/jni/android/graphics/Bitmap.cpp
@@ -278,9 +278,7 @@ void* lockPixels(JNIEnv* env, jobject bitmap) {
     if (!localBitmap->valid()) return nullptr;
 
     SkPixelRef& pixelRef = localBitmap->bitmap();
-    pixelRef.lockPixels();
     if (!pixelRef.pixels()) {
-        pixelRef.unlockPixels();
         return nullptr;
     }
     pixelRef.ref();
@@ -298,7 +296,6 @@ bool unlockPixels(JNIEnv* env, jobject bitmap) {
 
     SkPixelRef& pixelRef = localBitmap->bitmap();
     pixelRef.notifyPixelsChanged();
-    pixelRef.unlockPixels();
     pixelRef.unref();
     return true;
 }
@@ -438,7 +435,6 @@ static FromColorProc ChooseFromColorProc(const SkBitmap& bitmap) {
 
 bool GraphicsJNI::SetPixels(JNIEnv* env, jintArray srcColors, int srcOffset, int srcStride,
         int x, int y, int width, int height, const SkBitmap& dstBitmap) {
-    SkAutoLockPixels alp(dstBitmap);
     void* dst = dstBitmap.getPixels();
     FromColorProc proc = ChooseFromColorProc(dstBitmap);
 
@@ -758,11 +754,10 @@ static bool bitmapCopyTo(SkBitmap* dst, SkColorType dstCT, const SkBitmap& src,
     // Skia does not support copying from kAlpha8 to types that are not alpha only.
     // We will handle this case here.
     if (kAlpha_8_SkColorType == src.colorType() && kAlpha_8_SkColorType != dstCT) {
-        SkAutoPixmapUnlock srcUnlocker;
-        if (!src.requestLock(&srcUnlocker)) {
+        SkPixmap srcPixmap;
+        if (!src.peekPixels(&srcPixmap)) {
             return false;
         }
-        SkPixmap srcPixmap = srcUnlocker.pixmap();
 
         SkImageInfo dstInfo = src.info().makeColorType(dstCT);
         if (dstCT == kRGBA_F16_SkColorType) {
@@ -1138,9 +1133,7 @@ static jobject Bitmap_createFromParcel(JNIEnv* env, jobject, jobject parcel) {
             doThrowRE(env, "Could not allocate java pixel ref.");
             return NULL;
         }
-        bitmap->lockPixels();
         memcpy(bitmap->getPixels(), blob.data(), size);
-        bitmap->unlockPixels();
 
         // Release the blob handle.
         blob.release();
@@ -1186,7 +1179,6 @@ static jboolean Bitmap_writeToParcel(JNIEnv* env, jobject,
 
     if (bitmap.colorType() == kIndex_8_SkColorType) {
         // The bitmap needs to be locked to access its color table.
-        SkAutoLockPixels alp(bitmap);
         SkColorTable* ctable = bitmap.getColorTable();
         if (ctable != NULL) {
             int count = ctable->count();
@@ -1233,14 +1225,12 @@ static jboolean Bitmap_writeToParcel(JNIEnv* env, jobject,
         return JNI_FALSE;
     }
 
-    bitmap.lockPixels();
     const void* pSrc =  bitmap.getPixels();
     if (pSrc == NULL) {
         memset(blob.data(), 0, size);
     } else {
         memcpy(blob.data(), pSrc, size);
     }
-    bitmap.unlockPixels();
 
     blob.release();
     return JNI_TRUE;
@@ -1330,7 +1320,6 @@ static jint Bitmap_getPixel(JNIEnv* env, jobject, jlong bitmapHandle,
         jint x, jint y) {
     SkBitmap bitmap;
     reinterpret_cast<BitmapWrapper*>(bitmapHandle)->getSkBitmap(&bitmap);
-    SkAutoLockPixels alp(bitmap);
 
     ToColorProc proc = ChooseToColorProc(bitmap);
     if (NULL == proc) {
@@ -1362,7 +1351,6 @@ static void Bitmap_getPixels(JNIEnv* env, jobject, jlong bitmapHandle,
         jint x, jint y, jint width, jint height) {
     SkBitmap bitmap;
     reinterpret_cast<BitmapWrapper*>(bitmapHandle)->getSkBitmap(&bitmap);
-    SkAutoLockPixels alp(bitmap);
 
     ToColorProc proc = ChooseToColorProc(bitmap);
     if (NULL == proc) {
@@ -1411,7 +1399,6 @@ static void Bitmap_setPixel(JNIEnv* env, jobject, jlong bitmapHandle,
     SkBitmap bitmap;
     reinterpret_cast<BitmapWrapper*>(bitmapHandle)->getSkBitmap(&bitmap);
     SkColor color = static_cast<SkColor>(colorHandle);
-    SkAutoLockPixels alp(bitmap);
     if (NULL == bitmap.getPixels()) {
         return;
     }
@@ -1448,7 +1435,6 @@ static void Bitmap_copyPixelsToBuffer(JNIEnv* env, jobject,
                                       jlong bitmapHandle, jobject jbuffer) {
     SkBitmap bitmap;
     reinterpret_cast<BitmapWrapper*>(bitmapHandle)->getSkBitmap(&bitmap);
-    SkAutoLockPixels alp(bitmap);
     const void* src = bitmap.getPixels();
 
     if (NULL != src) {
@@ -1463,7 +1449,6 @@ static void Bitmap_copyPixelsFromBuffer(JNIEnv* env, jobject,
                                         jlong bitmapHandle, jobject jbuffer) {
     SkBitmap bitmap;
     reinterpret_cast<BitmapWrapper*>(bitmapHandle)->getSkBitmap(&bitmap);
-    SkAutoLockPixels alp(bitmap);
     void* dst = bitmap.getPixels();
 
     if (NULL != dst) {
@@ -1498,9 +1483,6 @@ static jboolean Bitmap_sameAs(JNIEnv* env, jobject, jlong bm0Handle, jlong bm1Ha
         return JNI_FALSE;
     }
 
-    SkAutoLockPixels alp0(bm0);
-    SkAutoLockPixels alp1(bm1);
-
     // if we can't load the pixels, return false
     if (NULL == bm0.getPixels() || NULL == bm1.getPixels()) {
         return JNI_FALSE;
diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp
index 6fc4dcb13c6c..5a25d5e06f07 100644
--- a/core/jni/android/graphics/BitmapFactory.cpp
+++ b/core/jni/android/graphics/BitmapFactory.cpp
@@ -197,10 +197,6 @@ public:
 
         mBitmap->reconfigure(info, bitmap->rowBytes(), sk_ref_sp(ctable));
         bitmap->setPixelRef(sk_ref_sp(mBitmap), 0, 0);
-
-        // since we're already allocated, we lockPixels right away
-        // HeapAllocator behaves this way too
-        bitmap->lockPixels();
         return true;
     }
 
diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp
index 2063ab4f4bfe..cbb7f9a7e9c3 100644
--- a/core/jni/android/graphics/Graphics.cpp
+++ b/core/jni/android/graphics/Graphics.cpp
@@ -461,10 +461,6 @@ android::Bitmap* GraphicsJNI::mapAshmemBitmap(JNIEnv* env, SkBitmap* bitmap,
     if (readOnly) {
         bitmap->pixelRef()->setImmutable();
     }
-    // since we're already allocated, we lockPixels right away
-    // HeapAllocator behaves this way too
-    bitmap->lockPixels();
-
     return wrapper;
 }
 
@@ -670,7 +666,6 @@ bool RecyclingClippingPixelAllocator::allocPixelRef(SkBitmap* bitmap, SkColorTab
         mRecycledBitmap->setAlphaType(bitmap->alphaType());
 
         bitmap->notifyPixelsChanged();
-        bitmap->lockPixels();
         mNeedsCopy = false;
 
         // TODO: If the dimensions of the SkBitmap are smaller than those of
diff --git a/core/jni/android/graphics/pdf/PdfRenderer.cpp b/core/jni/android/graphics/pdf/PdfRenderer.cpp
index 1001c05a891c..33bbdc4d8052 100644
--- a/core/jni/android/graphics/pdf/PdfRenderer.cpp
+++ b/core/jni/android/graphics/pdf/PdfRenderer.cpp
@@ -83,8 +83,6 @@ static void nativeRenderPage(JNIEnv* env, jclass thiz, jlong documentPtr, jlong
     SkBitmap skBitmap;
     GraphicsJNI::getSkBitmap(env, jbitmap, &skBitmap);
 
-    SkAutoLockPixels alp(skBitmap);
-
     const int stride = skBitmap.width() * 4;
 
     FPDF_BITMAP bitmap = FPDFBitmap_CreateEx(skBitmap.width(), skBitmap.height(),
diff --git a/core/jni/android/opengl/util.cpp b/core/jni/android/opengl/util.cpp
index d4735ec3c94f..1370e610398d 100644
--- a/core/jni/android/opengl/util.cpp
+++ b/core/jni/android/opengl/util.cpp
@@ -739,7 +739,6 @@ static jint util_texImage2D(JNIEnv *env, jclass clazz,
     int err = checkFormat(colorType, internalformat, type);
     if (err)
         return err;
-    bitmap.lockPixels();
     const int w = bitmap.width();
     const int h = bitmap.height();
     const void* p = bitmap.getPixels();
@@ -766,7 +765,6 @@ static jint util_texImage2D(JNIEnv *env, jclass clazz,
         glTexImage2D(target, level, internalformat, w, h, border, internalformat, type, p);
     }
 error:
-    bitmap.unlockPixels();
     return err;
 }
 
@@ -785,12 +783,10 @@ static jint util_texSubImage2D(JNIEnv *env, jclass clazz,
     int err = checkFormat(colorType, format, type);
     if (err)
         return err;
-    bitmap.lockPixels();
     const int w = bitmap.width();
     const int h = bitmap.height();
     const void* p = bitmap.getPixels();
     glTexSubImage2D(target, level, xoffset, yoffset, w, h, format, type, p);
-    bitmap.unlockPixels();
     return 0;
 }
 
diff --git a/core/jni/com_google_android_gles_jni_EGLImpl.cpp b/core/jni/com_google_android_gles_jni_EGLImpl.cpp
index 1ac05f91b3c0..d0ce1927b108 100644
--- a/core/jni/com_google_android_gles_jni_EGLImpl.cpp
+++ b/core/jni/com_google_android_gles_jni_EGLImpl.cpp
@@ -403,7 +403,6 @@ static jboolean jni_eglDestroySurface(JNIEnv *_env, jobject _this, jobject displ
         SkPixelRef* ref = (SkPixelRef*)(_env->GetLongField(surface,
                 gSurface_NativePixelRefFieldID));
         if (ref) {
-            ref->unlockPixels();
             SkSafeUnref(ref);
         }
     }
diff --git a/libs/hwui/OpenGLReadback.cpp b/libs/hwui/OpenGLReadback.cpp
index c460c0d2dfd4..d64804e480f5 100644
--- a/libs/hwui/OpenGLReadback.cpp
+++ b/libs/hwui/OpenGLReadback.cpp
@@ -144,8 +144,6 @@ inline CopyResult copyTextureInto(Caches& caches, RenderState& renderState,
         return CopyResult::UnknownError;
     }
 
-    SkAutoLockPixels alp(*bitmap);
-
     GLuint texture;
 
     GLenum format;
diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp
index a8e62cc68f9b..929d16067f4c 100644
--- a/libs/hwui/hwui/Bitmap.cpp
+++ b/libs/hwui/hwui/Bitmap.cpp
@@ -72,9 +72,6 @@ static sk_sp<Bitmap> allocateBitmap(SkBitmap* bitmap, sk_sp<SkColorTable> ctable
     auto wrapper = alloc(size, info, rowBytes, std::move(ctable));
     if (wrapper) {
         wrapper->getSkBitmap(bitmap);
-        // since we're already allocated, we lockPixels right away
-        // HeapAllocator behaves this way too
-        bitmap->lockPixels();
     }
     return wrapper;
 }
@@ -170,7 +167,6 @@ private:
 
 static bool uploadBitmapToGraphicBuffer(uirenderer::Caches& caches, SkBitmap& bitmap,
         GraphicBuffer& buffer, GLint format, GLint type) {
-    SkAutoLockPixels alp(bitmap);
     EGLDisplay display = eglGetCurrentDisplay();
     LOG_ALWAYS_FATAL_IF(display == EGL_NO_DISPLAY,
                 "Failed to get EGL_DEFAULT_DISPLAY! err=%s",
@@ -299,13 +295,11 @@ sk_sp<Bitmap> Bitmap::allocateAshmemBitmap(size_t size, const SkImageInfo& info,
 
 void FreePixelRef(void* addr, void* context) {
     auto pixelRef = (SkPixelRef*) context;
-    pixelRef->unlockPixels();
     pixelRef->unref();
 }
 
 sk_sp<Bitmap> Bitmap::createFrom(const SkImageInfo& info, SkPixelRef& pixelRef) {
     pixelRef.ref();
-    pixelRef.lockPixels();
     return sk_sp<Bitmap>(new Bitmap((void*) pixelRef.pixels(), (void*) &pixelRef, FreePixelRef,
             info, pixelRef.rowBytes(), sk_ref_sp(pixelRef.colorTable())));
 }
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp
index a18d26471a29..04aeb7c18c95 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLReadback.cpp
@@ -63,8 +63,6 @@ CopyResult SkiaOpenGLReadback::copyImageInto(EGLImageKHR eglImage, const Matrix4
     CopyResult copyResult = CopyResult::UnknownError;
     sk_sp<SkImage> image(SkImage::MakeFromAdoptedTexture(grContext.get(), textureDescription));
     if (image) {
-        SkAutoLockPixels alp(*bitmap);
-
         // convert to Skia data structures
         const SkRect bufferRect = SkRect::MakeIWH(imgWidth, imgHeight);
         SkRect skiaSrcRect = srcRect.toSkRect();
diff --git a/libs/hwui/tests/unit/SkiaCanvasTests.cpp b/libs/hwui/tests/unit/SkiaCanvasTests.cpp
index 0aecb8540e4e..c048dda4a2e9 100644
--- a/libs/hwui/tests/unit/SkiaCanvasTests.cpp
+++ b/libs/hwui/tests/unit/SkiaCanvasTests.cpp
@@ -89,7 +89,6 @@ TEST(SkiaCanvas, colorSpaceXform) {
     sk_sp<Bitmap> adobeBitmap = Bitmap::allocateHeapBitmap(adobeInfo);
     SkBitmap adobeSkBitmap;
     adobeBitmap->getSkBitmap(&adobeSkBitmap);
-    adobeSkBitmap.lockPixels();
     *adobeSkBitmap.getAddr32(0, 0) = 0xFF0000F0; // Opaque, almost fully-red
 
     SkImageInfo info = adobeInfo.makeColorSpace(nullptr);
@@ -101,7 +100,6 @@ TEST(SkiaCanvas, colorSpaceXform) {
     SkiaCanvas canvas(skBitmap);
     canvas.drawBitmap(*adobeBitmap, 0, 0, nullptr);
     // The result should be fully red, since we convert to sRGB at draw time.
-    skBitmap.lockPixels();
     ASSERT_EQ(0xFF0000FF, *skBitmap.getAddr32(0, 0));
 
     // Create a software canvas with an Adobe color space.
@@ -116,7 +114,6 @@ TEST(SkiaCanvas, colorSpaceXform) {
     SkiaCanvas deferCanvas(&skCanvas, Canvas::XformToSRGB::kDefer);
     deferCanvas.drawBitmap(*adobeBitmap, 0, 0, nullptr);
     // The result should be as before, since we deferred the conversion to sRGB.
-    skBitmap.lockPixels();
     ASSERT_EQ(0xFF0000DC, *skBitmap.getAddr32(0, 0));
 
     // Test picture recording.  We will kDefer the xform at recording time, but handle it when
diff --git a/media/jni/android_media_MediaMetadataRetriever.cpp b/media/jni/android_media_MediaMetadataRetriever.cpp
index f4e940d6cb21..0a3ce21d0549 100644
--- a/media/jni/android_media_MediaMetadataRetriever.cpp
+++ b/media/jni/android_media_MediaMetadataRetriever.cpp
@@ -302,13 +302,11 @@ static jobject android_media_MediaMetadataRetriever_getFrameAtTime(JNIEnv *env,
     SkBitmap bitmap;
     GraphicsJNI::getSkBitmap(env, jBitmap, &bitmap);
 
-    bitmap.lockPixels();
     rotate((uint16_t*)bitmap.getPixels(),
            (uint16_t*)((char*)videoFrame + sizeof(VideoFrame)),
            videoFrame->mWidth,
            videoFrame->mHeight,
            videoFrame->mRotationAngle);
-    bitmap.unlockPixels();
 
     if (videoFrame->mDisplayWidth  != videoFrame->mWidth ||
         videoFrame->mDisplayHeight != videoFrame->mHeight) {
diff --git a/rs/jni/android_renderscript_RenderScript.cpp b/rs/jni/android_renderscript_RenderScript.cpp
index 0acbb0258944..f6d3f4835381 100644
--- a/rs/jni/android_renderscript_RenderScript.cpp
+++ b/rs/jni/android_renderscript_RenderScript.cpp
@@ -1324,12 +1324,10 @@ nAllocationCreateFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong type,
     SkBitmap bitmap;
     GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
 
-    bitmap.lockPixels();
     const void* ptr = bitmap.getPixels();
     jlong id = (jlong)(uintptr_t)rsAllocationCreateFromBitmap((RsContext)con,
                                                   (RsType)type, (RsAllocationMipmapControl)mip,
                                                   ptr, bitmap.getSize(), usage);
-    bitmap.unlockPixels();
     return id;
 }
 
@@ -1340,12 +1338,10 @@ nAllocationCreateBitmapBackedAllocation(JNIEnv *_env, jobject _this, jlong con,
     SkBitmap bitmap;
     GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
 
-    bitmap.lockPixels();
     const void* ptr = bitmap.getPixels();
     jlong id = (jlong)(uintptr_t)rsAllocationCreateTyped((RsContext)con,
                                             (RsType)type, (RsAllocationMipmapControl)mip,
                                             (uint32_t)usage, (uintptr_t)ptr);
-    bitmap.unlockPixels();
     return id;
 }
 
@@ -1356,12 +1352,10 @@ nAllocationCubeCreateFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong ty
     SkBitmap bitmap;
     GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
 
-    bitmap.lockPixels();
     const void* ptr = bitmap.getPixels();
     jlong id = (jlong)(uintptr_t)rsAllocationCubeCreateFromBitmap((RsContext)con,
                                                       (RsType)type, (RsAllocationMipmapControl)mip,
                                                       ptr, bitmap.getSize(), usage);
-    bitmap.unlockPixels();
     return id;
 }
 
@@ -1373,12 +1367,10 @@ nAllocationCopyFromBitmap(JNIEnv *_env, jobject _this, jlong con, jlong alloc, j
     int w = bitmap.width();
     int h = bitmap.height();
 
-    bitmap.lockPixels();
     const void* ptr = bitmap.getPixels();
     rsAllocation2DData((RsContext)con, (RsAllocation)alloc, 0, 0,
                        0, RS_ALLOCATION_CUBEMAP_FACE_POSITIVE_X,
                        w, h, ptr, bitmap.getSize(), 0);
-    bitmap.unlockPixels();
 }
 
 static void
@@ -1387,10 +1379,8 @@ nAllocationCopyToBitmap(JNIEnv *_env, jobject _this, jlong con, jlong alloc, job
     SkBitmap bitmap;
     GraphicsJNI::getSkBitmap(_env, jbitmap, &bitmap);
 
-    bitmap.lockPixels();
     void* ptr = bitmap.getPixels();
     rsAllocationCopyToBitmap((RsContext)con, (RsAllocation)alloc, ptr, bitmap.getSize());
-    bitmap.unlockPixels();
     bitmap.notifyPixelsChanged();
 }