Cleanup of Bitmap.h entry points.

All Bitmap constructors have been made private and the only way to
create one is through the allocate or createFrom factories.

SkColorSpace is now explicitly passed in to all the factories and is
no longer assumed to be sRGB.

Test: atest CtsGraphicsTestCases
Change-Id: I92c1c5c59df6de7fdd90e9504a2c2717cb854588

10 files changed, 100 insertions, 100 deletions

diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp
index c32de0a5737e..12ca78a7ce92 100755
--- a/core/jni/android/graphics/Bitmap.cpp
+++ b/core/jni/android/graphics/Bitmap.cpp
@@ -725,9 +725,10 @@ static jobject Bitmap_createFromParcel(JNIEnv* env, jobject, jobject parcel) {
             return NULL;
         }
 
-        // Map the pixels in place and take ownership of the ashmem region.
-        nativeBitmap = sk_sp<Bitmap>(GraphicsJNI::mapAshmemBitmap(env, bitmap.get(),
-                dupFd, const_cast<void*>(blob.data()), size, !isMutable));
+        // Map the pixels in place and take ownership of the ashmem region. We must also respect the
+        // rowBytes value already set on the bitmap instead of attempting to compute our own.
+        nativeBitmap = Bitmap::createFrom(bitmap->info(), bitmap->rowBytes(), dupFd,
+                                          const_cast<void*>(blob.data()), size, !isMutable);
         if (!nativeBitmap) {
             close(dupFd);
             blob.release();
@@ -1097,21 +1098,20 @@ static jobject Bitmap_copyPreserveInternalConfig(JNIEnv* env, jobject, jlong bit
     SkBitmap src;
     hwuiBitmap.getSkBitmap(&src);
 
-    SkBitmap result;
-    HeapAllocator allocator;
-    if (!bitmapCopyTo(&result, hwuiBitmap.info().colorType(), src, &allocator)) {
+    if (src.pixelRef() == nullptr) {
         doThrowRE(env, "Could not copy a hardware bitmap.");
         return NULL;
     }
-    return createBitmap(env, allocator.getStorageObjAndReset(), getPremulBitmapCreateFlags(false));
+
+    sk_sp<Bitmap> bitmap = Bitmap::createFrom(src.info(), *src.pixelRef());
+    return createBitmap(env, bitmap.release(), getPremulBitmapCreateFlags(false));
 }
 
 static jobject Bitmap_createHardwareBitmap(JNIEnv* env, jobject, jobject graphicBuffer) {
     sp<GraphicBuffer> buffer(graphicBufferForJavaObject(env, graphicBuffer));
-    // Bitmap::createFrom currently assumes SRGB color space for RGBA images.
     // To support any color space, we need to pass an additional ColorSpace argument to
     // java Bitmap.createHardwareBitmap.
-    sk_sp<Bitmap> bitmap = Bitmap::createFrom(buffer);
+    sk_sp<Bitmap> bitmap = Bitmap::createFrom(buffer, SkColorSpace::MakeSRGB());
     if (!bitmap.get()) {
         ALOGW("failed to create hardware bitmap from graphic buffer");
         return NULL;
diff --git a/core/jni/android/graphics/Graphics.cpp b/core/jni/android/graphics/Graphics.cpp
index 26af15e79e2d..67d0c8aced61 100644
--- a/core/jni/android/graphics/Graphics.cpp
+++ b/core/jni/android/graphics/Graphics.cpp
@@ -424,36 +424,6 @@ jobject GraphicsJNI::createRegion(JNIEnv* env, SkRegion* region)
 
 ///////////////////////////////////////////////////////////////////////////////
 
-android::Bitmap* GraphicsJNI::mapAshmemBitmap(JNIEnv* env, SkBitmap* bitmap,
-        int fd, void* addr, size_t size, bool readOnly) {
-    const SkImageInfo& info = bitmap->info();
-    if (info.colorType() == kUnknown_SkColorType) {
-        doThrowIAE(env, "unknown bitmap configuration");
-        return nullptr;
-    }
-
-    if (!addr) {
-        // Map existing ashmem region if not already mapped.
-        int flags = readOnly ? (PROT_READ) : (PROT_READ | PROT_WRITE);
-        size = ashmem_get_size_region(fd);
-        addr = mmap(NULL, size, flags, MAP_SHARED, fd, 0);
-        if (addr == MAP_FAILED) {
-            return nullptr;
-        }
-    }
-
-    // we must respect the rowBytes value already set on the bitmap instead of
-    // attempting to compute our own.
-    const size_t rowBytes = bitmap->rowBytes();
-
-    auto wrapper = new android::Bitmap(addr, fd, size, info, rowBytes);
-    wrapper->getSkBitmap(bitmap);
-    if (readOnly) {
-        bitmap->pixelRef()->setImmutable();
-    }
-    return wrapper;
-}
-
 SkColorSpaceTransferFn GraphicsJNI::getNativeTransferParameters(JNIEnv* env, jobject transferParams) {
     SkColorSpaceTransferFn p;
     p.fA = (float) env->GetDoubleField(transferParams, gTransferParams_aFieldID);
diff --git a/core/jni/android/graphics/GraphicsJNI.h b/core/jni/android/graphics/GraphicsJNI.h
index cee3c46dd67f..b0bd68336e08 100644
--- a/core/jni/android/graphics/GraphicsJNI.h
+++ b/core/jni/android/graphics/GraphicsJNI.h
@@ -85,9 +85,6 @@ public:
 
     static jobject createBitmapRegionDecoder(JNIEnv* env, SkBitmapRegionDecoder* bitmap);
 
-    static android::Bitmap* mapAshmemBitmap(JNIEnv* env, SkBitmap* bitmap,
-            int fd, void* addr, size_t size, bool readOnly);
-
     /**
      * Given a bitmap we natively allocate a memory block to store the contents
      * of that bitmap.  The memory is then attached to the bitmap via an
diff --git a/core/jni/android_view_ThreadedRenderer.cpp b/core/jni/android_view_ThreadedRenderer.cpp
index 702741eb813c..5a8ab3c1bdc4 100644
--- a/core/jni/android_view_ThreadedRenderer.cpp
+++ b/core/jni/android_view_ThreadedRenderer.cpp
@@ -31,8 +31,6 @@
 #include <gui/BufferQueue.h>
 #include <gui/Surface.h>
 
-#include <EGL/egl.h>
-#include <EGL/eglext.h>
 #include <private/EGL/cache.h>
 
 #include <utils/Looper.h>
@@ -58,6 +56,7 @@
 #include <renderthread/RenderTask.h>
 #include <renderthread/RenderThread.h>
 #include <pipeline/skia/ShaderCache.h>
+#include <utils/Color.h>
 
 namespace android {
 
@@ -1011,10 +1010,9 @@ static jobject android_view_ThreadedRenderer_createHardwareBitmapFromRenderNode(
                 buffer->getWidth(), buffer->getHeight(), width, height);
         // Continue I guess?
     }
-    sk_sp<Bitmap> bitmap = Bitmap::createFrom(buffer);
-    // Bitmap::createFrom currently can only attach to a GraphicBuffer with PIXEL_FORMAT_RGBA_8888
-    // format and SRGB color space.
-    // To support any color space, we could extract it from BufferItem and pass it to Bitmap.
+
+    sk_sp<SkColorSpace> cs = uirenderer::DataSpaceToColorSpace(bufferItem.mDataSpace);
+    sk_sp<Bitmap> bitmap = Bitmap::createFrom(buffer, cs);
     return bitmap::createBitmap(env, bitmap.release(),
             android::bitmap::kBitmapCreateFlag_Premultiplied);
 }
diff --git a/libs/hwui/HardwareBitmapUploader.cpp b/libs/hwui/HardwareBitmapUploader.cpp
index a97c12cad9fd..b9860ada18fc 100644
--- a/libs/hwui/HardwareBitmapUploader.cpp
+++ b/libs/hwui/HardwareBitmapUploader.cpp
@@ -253,7 +253,8 @@ sk_sp<Bitmap> HardwareBitmapUploader::allocateHardwareBitmap(const SkBitmap& sou
         eglDestroySyncKHR(display, fence);
     }
 
-    return sk_sp<Bitmap>(new Bitmap(buffer.get(), bitmap.info(), Bitmap::computePalette(bitmap)));
+    return Bitmap::createFrom(buffer.get(), bitmap.refColorSpace(), bitmap.alphaType(),
+                              Bitmap::computePalette(bitmap));
 }
 
 }  // namespace android::uirenderer
diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp
index 6c77f9ee1845..6e0258c9ecb2 100644
--- a/libs/hwui/hwui/Bitmap.cpp
+++ b/libs/hwui/hwui/Bitmap.cpp
@@ -75,20 +75,33 @@ sk_sp<Bitmap> Bitmap::allocateAshmemBitmap(SkBitmap* bitmap) {
     return allocateBitmap(bitmap, &Bitmap::allocateAshmemBitmap);
 }
 
-static sk_sp<Bitmap> allocateHeapBitmap(size_t size, const SkImageInfo& info, size_t rowBytes) {
-    void* addr = calloc(size, 1);
-    if (!addr) {
+sk_sp<Bitmap> Bitmap::allocateAshmemBitmap(size_t size, const SkImageInfo& info, size_t rowBytes) {
+    // Create new ashmem region with read/write priv
+    int fd = ashmem_create_region("bitmap", size);
+    if (fd < 0) {
         return nullptr;
     }
-    return sk_sp<Bitmap>(new Bitmap(addr, size, info, rowBytes));
+
+    void* addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+    if (addr == MAP_FAILED) {
+        close(fd);
+        return nullptr;
+    }
+
+    if (ashmem_set_prot_region(fd, PROT_READ) < 0) {
+        munmap(addr, size);
+        close(fd);
+        return nullptr;
+    }
+    return sk_sp<Bitmap>(new Bitmap(addr, fd, size, info, rowBytes));
 }
 
-sk_sp<Bitmap> Bitmap::allocateHardwareBitmap(SkBitmap& bitmap) {
+sk_sp<Bitmap> Bitmap::allocateHardwareBitmap(const SkBitmap& bitmap) {
     return uirenderer::HardwareBitmapUploader::allocateHardwareBitmap(bitmap);
 }
 
 sk_sp<Bitmap> Bitmap::allocateHeapBitmap(SkBitmap* bitmap) {
-    return allocateBitmap(bitmap, &android::allocateHeapBitmap);
+    return allocateBitmap(bitmap, &Bitmap::allocateHeapBitmap);
 }
 
 sk_sp<Bitmap> Bitmap::allocateHeapBitmap(const SkImageInfo& info) {
@@ -97,28 +110,15 @@ sk_sp<Bitmap> Bitmap::allocateHeapBitmap(const SkImageInfo& info) {
         LOG_ALWAYS_FATAL("trying to allocate too large bitmap");
         return nullptr;
     }
-    return android::allocateHeapBitmap(size, info, info.minRowBytes());
+    return allocateHeapBitmap(size, info, info.minRowBytes());
 }
 
-sk_sp<Bitmap> Bitmap::allocateAshmemBitmap(size_t size, const SkImageInfo& info, size_t rowBytes) {
-    // Create new ashmem region with read/write priv
-    int fd = ashmem_create_region("bitmap", size);
-    if (fd < 0) {
-        return nullptr;
-    }
-
-    void* addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-    if (addr == MAP_FAILED) {
-        close(fd);
-        return nullptr;
-    }
-
-    if (ashmem_set_prot_region(fd, PROT_READ) < 0) {
-        munmap(addr, size);
-        close(fd);
+sk_sp<Bitmap> Bitmap::allocateHeapBitmap(size_t size, const SkImageInfo& info, size_t rowBytes) {
+    void* addr = calloc(size, 1);
+    if (!addr) {
         return nullptr;
     }
-    return sk_sp<Bitmap>(new Bitmap(addr, fd, size, info, rowBytes));
+    return sk_sp<Bitmap>(new Bitmap(addr, size, info, rowBytes));
 }
 
 void FreePixelRef(void* addr, void* context) {
@@ -132,17 +132,38 @@ sk_sp<Bitmap> Bitmap::createFrom(const SkImageInfo& info, SkPixelRef& pixelRef)
                                     pixelRef.rowBytes()));
 }
 
-sk_sp<Bitmap> Bitmap::createFrom(sp<GraphicBuffer> graphicBuffer) {
-    return createFrom(graphicBuffer, SkColorSpace::MakeSRGB());
-}
 
-sk_sp<Bitmap> Bitmap::createFrom(sp<GraphicBuffer> graphicBuffer, sk_sp<SkColorSpace> colorSpace) {
+sk_sp<Bitmap> Bitmap::createFrom(sp<GraphicBuffer> graphicBuffer, sk_sp<SkColorSpace> colorSpace,
+                                 SkAlphaType alphaType, BitmapPalette palette) {
     // As we will be effectively texture-sampling the buffer (using either EGL or Vulkan), we can
-    // view the colorspace as RGBA8888.
+    // view the format as RGBA8888.
     SkImageInfo info = SkImageInfo::Make(graphicBuffer->getWidth(), graphicBuffer->getHeight(),
-                                         kRGBA_8888_SkColorType, kPremul_SkAlphaType,
-                                         colorSpace);
-    return sk_sp<Bitmap>(new Bitmap(graphicBuffer.get(), info));
+                                         kRGBA_8888_SkColorType, alphaType, colorSpace);
+    return sk_sp<Bitmap>(new Bitmap(graphicBuffer.get(), info, palette));
+}
+
+sk_sp<Bitmap> Bitmap::createFrom(const SkImageInfo& info, size_t rowBytes, int fd, void* addr,
+                                 size_t size, bool readOnly) {
+    if (info.colorType() == kUnknown_SkColorType) {
+        LOG_ALWAYS_FATAL("unknown bitmap configuration");
+        return nullptr;
+    }
+
+    if (!addr) {
+        // Map existing ashmem region if not already mapped.
+        int flags = readOnly ? (PROT_READ) : (PROT_READ | PROT_WRITE);
+        size = ashmem_get_size_region(fd);
+        addr = mmap(NULL, size, flags, MAP_SHARED, fd, 0);
+        if (addr == MAP_FAILED) {
+            return nullptr;
+        }
+    }
+
+    sk_sp<Bitmap> bitmap(new Bitmap(addr, fd, size, info, rowBytes));
+    if (readOnly) {
+        bitmap->setImmutable();
+    }
+    return bitmap;
 }
 
 void Bitmap::setColorSpace(sk_sp<SkColorSpace> colorSpace) {
diff --git a/libs/hwui/hwui/Bitmap.h b/libs/hwui/hwui/Bitmap.h
index d446377ec1d9..2138040d9690 100644
--- a/libs/hwui/hwui/Bitmap.h
+++ b/libs/hwui/hwui/Bitmap.h
@@ -54,28 +54,31 @@ typedef void (*FreeFunc)(void* addr, void* context);
 
 class ANDROID_API Bitmap : public SkPixelRef {
 public:
+    /* The allocate factories not only construct the Bitmap object but also allocate the
+     * backing store whose type is determined by the specific method that is called.
+     *
+     * The factories that accept SkBitmap* as a param will modify those params by
+     * installing the returned bitmap as their SkPixelRef.
+     *
+     * The factories that accept const SkBitmap& as a param will copy the contents of the
+     * provided bitmap into the newly allocated buffer.
+     */
+    static sk_sp<Bitmap> allocateAshmemBitmap(SkBitmap* bitmap);
+    static sk_sp<Bitmap> allocateHardwareBitmap(const SkBitmap& bitmap);
     static sk_sp<Bitmap> allocateHeapBitmap(SkBitmap* bitmap);
     static sk_sp<Bitmap> allocateHeapBitmap(const SkImageInfo& info);
 
-    static sk_sp<Bitmap> allocateHardwareBitmap(SkBitmap& bitmap);
-
-    static sk_sp<Bitmap> allocateAshmemBitmap(SkBitmap* bitmap);
-    static sk_sp<Bitmap> allocateAshmemBitmap(size_t allocSize, const SkImageInfo& info,
-                                              size_t rowBytes);
-
-    static sk_sp<Bitmap> createFrom(sp<GraphicBuffer> graphicBuffer);
+    /* The createFrom factories construct a new Bitmap object by wrapping the already allocated
+     * memory that is provided as an input param.
+     */
     static sk_sp<Bitmap> createFrom(sp<GraphicBuffer> graphicBuffer,
-                                    sk_sp<SkColorSpace> colorSpace);
-
+                                    sk_sp<SkColorSpace> colorSpace,
+                                    SkAlphaType alphaType = kPremul_SkAlphaType,
+                                    BitmapPalette palette = BitmapPalette::Unknown);
+    static sk_sp<Bitmap> createFrom(const SkImageInfo& info, size_t rowBytes, int fd, void* addr,
+                                    size_t size, bool readOnly);
     static sk_sp<Bitmap> createFrom(const SkImageInfo&, SkPixelRef&);
 
-    Bitmap(void* address, size_t allocSize, const SkImageInfo& info, size_t rowBytes);
-    Bitmap(void* address, void* context, FreeFunc freeFunc, const SkImageInfo& info,
-           size_t rowBytes);
-    Bitmap(void* address, int fd, size_t mappedSize, const SkImageInfo& info, size_t rowBytes);
-    Bitmap(GraphicBuffer* buffer, const SkImageInfo& info,
-           BitmapPalette palette = BitmapPalette::Unknown);
-
     int rowBytesAsPixels() const { return rowBytes() >> mInfo.shiftPerPixel(); }
 
     void reconfigure(const SkImageInfo& info, size_t rowBytes);
@@ -123,6 +126,15 @@ public:
     }
 
 private:
+    static sk_sp<Bitmap> allocateAshmemBitmap(size_t size, const SkImageInfo& i, size_t rowBytes);
+    static sk_sp<Bitmap> allocateHeapBitmap(size_t size, const SkImageInfo& i, size_t rowBytes);
+
+    Bitmap(void* address, size_t allocSize, const SkImageInfo& info, size_t rowBytes);
+    Bitmap(void* address, void* context, FreeFunc freeFunc, const SkImageInfo& info,
+           size_t rowBytes);
+    Bitmap(void* address, int fd, size_t mappedSize, const SkImageInfo& info, size_t rowBytes);
+    Bitmap(GraphicBuffer* buffer, const SkImageInfo& info, BitmapPalette palette);
+
     virtual ~Bitmap();
     void* getStorage() const;
 
diff --git a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
index c67134cddb8a..1d3a24463057 100644
--- a/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaVulkanPipeline.cpp
@@ -158,7 +158,7 @@ sk_sp<Bitmap> SkiaVulkanPipeline::allocateHardwareBitmap(renderthread::RenderThr
         ALOGW("SkiaVulkanPipeline::allocateHardwareBitmap() failed in GraphicBuffer.create()");
         return nullptr;
     }
-    return sk_sp<Bitmap>(new Bitmap(buffer.get(), skBitmap.info()));
+    return Bitmap::createFrom(buffer, skBitmap.refColorSpace());
 }
 
 } /* namespace skiapipeline */
diff --git a/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp b/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp
index 448408d19eb1..ec81f629ee45 100644
--- a/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp
+++ b/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp
@@ -50,7 +50,7 @@ public:
             pixels[4000 + 4 * i + 3] = 255;
         }
         buffer->unlock();
-        sk_sp<Bitmap> hardwareBitmap(Bitmap::createFrom(buffer));
+        sk_sp<Bitmap> hardwareBitmap(Bitmap::createFrom(buffer, SkColorSpace::MakeSRGB()));
         sk_sp<SkShader> hardwareShader(createBitmapShader(*hardwareBitmap));
 
         SkPoint center;
diff --git a/libs/hwui/utils/Color.h b/libs/hwui/utils/Color.h
index 4daccda78e23..4473ce632b1b 100644
--- a/libs/hwui/utils/Color.h
+++ b/libs/hwui/utils/Color.h
@@ -17,6 +17,7 @@
 #define COLOR_H
 
 #include <math.h>
+#include <cutils/compiler.h>
 #include <system/graphics.h>
 #include <ui/PixelFormat.h>
 
@@ -117,7 +118,7 @@ bool transferFunctionCloseToSRGB(const SkColorSpace* colorSpace);
 
 android::PixelFormat ColorTypeToPixelFormat(SkColorType colorType);
 
-sk_sp<SkColorSpace> DataSpaceToColorSpace(android_dataspace dataspace);
+ANDROID_API sk_sp<SkColorSpace> DataSpaceToColorSpace(android_dataspace dataspace);
 
 struct Lab {
     float L;