Improve color correctness for drawing bitmaps with Skia pipeline

Fix drawing of bitmaps with color profiles. This CL is making
ColorSpaceTests and Rgba16fTests CTS tests to pass with Skia
pipeline. Drawing bitmaps withs pixels outside SRGB range
may need more work (ColorSpaceTests#testDrawDisplayP3 test use a
a wider gamut, but the actual pixels fall into the SRGB range).

Test: Ran CtsUiRenderingTestCases with HWUI and Skia pipeline.
Bug: 62347704
Change-Id: I8d318076bb38f7d32bfde7e5492ae7a61f4731a5

4 files changed, 29 insertions, 25 deletions

diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp
index 623b496f5f09..0a642b60d4c7 100644
--- a/libs/hwui/SkiaCanvas.cpp
+++ b/libs/hwui/SkiaCanvas.cpp
@@ -530,33 +530,25 @@ void SkiaCanvas::drawVertices(const SkVertices* vertices, SkBlendMode mode, cons
 // ----------------------------------------------------------------------------
 
 void SkiaCanvas::drawBitmap(Bitmap& bitmap, float left, float top, const SkPaint* paint) {
-    SkBitmap skBitmap;
-    bitmap.getSkBitmap(&skBitmap);
-    mCanvas->drawBitmap(skBitmap, left, top, paint);
+    mCanvas->drawImage(bitmap.makeImage(), left, top, paint);
 }
 
 void SkiaCanvas::drawBitmap(Bitmap& hwuiBitmap, const SkMatrix& matrix, const SkPaint* paint) {
-    SkBitmap bitmap;
-    hwuiBitmap.getSkBitmap(&bitmap);
     SkAutoCanvasRestore acr(mCanvas, true);
     mCanvas->concat(matrix);
-    mCanvas->drawBitmap(bitmap, 0, 0, paint);
+    mCanvas->drawImage(hwuiBitmap.makeImage(), 0, 0, paint);
 }
 
 void SkiaCanvas::drawBitmap(Bitmap& hwuiBitmap, float srcLeft, float srcTop,
                             float srcRight, float srcBottom, float dstLeft, float dstTop,
                             float dstRight, float dstBottom, const SkPaint* paint) {
-    SkBitmap bitmap;
-    hwuiBitmap.getSkBitmap(&bitmap);
     SkRect srcRect = SkRect::MakeLTRB(srcLeft, srcTop, srcRight, srcBottom);
     SkRect dstRect = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom);
-    mCanvas->drawBitmapRect(bitmap, srcRect, dstRect, paint);
+    mCanvas->drawImageRect(hwuiBitmap.makeImage(), srcRect, dstRect, paint);
 }
 
 void SkiaCanvas::drawBitmapMesh(Bitmap& hwuiBitmap, int meshWidth, int meshHeight,
         const float* vertices, const int* colors, const SkPaint* paint) {
-    SkBitmap bitmap;
-    hwuiBitmap.getSkBitmap(&bitmap);
     const int ptCount = (meshWidth + 1) * (meshHeight + 1);
     const int indexCount = meshWidth * meshHeight * 6;
     uint32_t flags = SkVertices::kHasTexCoords_BuilderFlag;
@@ -573,8 +565,8 @@ void SkiaCanvas::drawBitmapMesh(Bitmap& hwuiBitmap, int meshWidth, int meshHeigh
 
     // cons up texture coordinates and indices
     {
-        const SkScalar w = SkIntToScalar(bitmap.width());
-        const SkScalar h = SkIntToScalar(bitmap.height());
+        const SkScalar w = SkIntToScalar(hwuiBitmap.width());
+        const SkScalar h = SkIntToScalar(hwuiBitmap.height());
         const SkScalar dx = w / meshWidth;
         const SkScalar dy = h / meshHeight;
 
@@ -635,7 +627,7 @@ void SkiaCanvas::drawBitmapMesh(Bitmap& hwuiBitmap, int meshWidth, int meshHeigh
         tmpPaint = *paint;
     }
 
-    sk_sp<SkImage> image = SkMakeImageFromRasterBitmap(bitmap, kNever_SkCopyPixelsMode);
+    sk_sp<SkImage> image = hwuiBitmap.makeImage();
     tmpPaint.setShader(image->makeShader(SkShader::kClamp_TileMode, SkShader::kClamp_TileMode));
 
     mCanvas->drawVertices(builder.detach(), SkBlendMode::kModulate, tmpPaint);
@@ -644,11 +636,8 @@ void SkiaCanvas::drawBitmapMesh(Bitmap& hwuiBitmap, int meshWidth, int meshHeigh
 void SkiaCanvas::drawNinePatch(Bitmap& hwuiBitmap, const Res_png_9patch& chunk,
         float dstLeft, float dstTop, float dstRight, float dstBottom, const SkPaint* paint) {
 
-    SkBitmap bitmap;
-    hwuiBitmap.getSkBitmap(&bitmap);
-
     SkCanvas::Lattice lattice;
-    NinePatchUtils::SetLatticeDivs(&lattice, chunk, bitmap.width(), bitmap.height());
+    NinePatchUtils::SetLatticeDivs(&lattice, chunk, hwuiBitmap.width(), hwuiBitmap.height());
 
     lattice.fFlags = nullptr;
     int numFlags = 0;
@@ -665,7 +654,7 @@ void SkiaCanvas::drawNinePatch(Bitmap& hwuiBitmap, const Res_png_9patch& chunk,
 
     lattice.fBounds = nullptr;
     SkRect dst = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom);
-    mCanvas->drawBitmapLattice(bitmap, lattice, dst, paint);
+    mCanvas->drawImageLattice(hwuiBitmap.makeImage().get(), lattice, dst, paint);
 }
 
 void SkiaCanvas::drawVectorDrawable(VectorDrawableRoot* vectorDrawable) {
diff --git a/libs/hwui/SkiaCanvasProxy.cpp b/libs/hwui/SkiaCanvasProxy.cpp
index 34dddd169e96..16a19ca4d36a 100644
--- a/libs/hwui/SkiaCanvasProxy.cpp
+++ b/libs/hwui/SkiaCanvasProxy.cpp
@@ -151,7 +151,8 @@ void SkiaCanvasProxy::onDrawBitmapNine(const SkBitmap& bitmap, const SkIRect& ce
 void SkiaCanvasProxy::onDrawImage(const SkImage* image, SkScalar left, SkScalar top,
         const SkPaint* paint) {
     SkBitmap skiaBitmap;
-    if (image->asLegacyBitmap(&skiaBitmap, SkImage::kRO_LegacyBitmapMode)) {
+    SkPixmap pixmap;
+    if (image->peekPixels(&pixmap) && skiaBitmap.installPixels(pixmap)) {
         onDrawBitmap(skiaBitmap, left, top, paint);
     }
 }
@@ -159,7 +160,8 @@ void SkiaCanvasProxy::onDrawImage(const SkImage* image, SkScalar left, SkScalar
 void SkiaCanvasProxy::onDrawImageRect(const SkImage* image, const SkRect* srcPtr, const SkRect& dst,
         const SkPaint* paint, SrcRectConstraint constraint) {
     SkBitmap skiaBitmap;
-    if (image->asLegacyBitmap(&skiaBitmap, SkImage::kRO_LegacyBitmapMode)) {
+    SkPixmap pixmap;
+    if (image->peekPixels(&pixmap) && skiaBitmap.installPixels(pixmap)) {
         sk_sp<Bitmap> bitmap = Bitmap::createFrom(skiaBitmap.info(), *skiaBitmap.pixelRef());
         SkRect src = (srcPtr) ? *srcPtr : SkRect::MakeWH(image->width(), image->height());
         mCanvas->drawBitmap(*bitmap, src.fLeft, src.fTop, src.fRight, src.fBottom,
diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp
index 3a8914ecf9d6..79958773b21f 100644
--- a/libs/hwui/hwui/Bitmap.cpp
+++ b/libs/hwui/hwui/Bitmap.cpp
@@ -209,7 +209,9 @@ Bitmap::Bitmap(GraphicBuffer* buffer, const SkImageInfo& info)
     buffer->incStrong(buffer);
     setImmutable(); // HW bitmaps are always immutable
     if (uirenderer::Properties::isSkiaEnabled()) {
-        // TODO: add color correctness for Skia pipeline - pass null color space for now
+        // GraphicBuffer should be in the display color space (Bitmap::createFrom is always
+        // passing SRGB). The code that uploads into a GraphicBuffer should do color conversion if
+        // needed.
         mImage = SkImage::MakeFromAHardwareBuffer(reinterpret_cast<AHardwareBuffer*>(buffer),
                 mInfo.alphaType(), nullptr);
     }
@@ -294,7 +296,6 @@ void Bitmap::getSkBitmap(SkBitmap* outBitmap) {
     outBitmap->setHasHardwareMipMap(mHasHardwareMipMap);
     if (isHardware()) {
         if (uirenderer::Properties::isSkiaEnabled()) {
-            // TODO: add color correctness for Skia pipeline - pass null color space for now
             outBitmap->allocPixels(SkImageInfo::Make(info().width(), info().height(),
                     info().colorType(), info().alphaType(), nullptr));
         } else {
@@ -330,7 +331,12 @@ sk_sp<SkImage> Bitmap::makeImage() {
         // Note we don't cache in this case, because the raster image holds a pointer to this Bitmap
         // internally and ~Bitmap won't be invoked.
         // TODO: refactor Bitmap to not derive from SkPixelRef, which would allow caching here.
-        image = SkMakeImageFromRasterBitmap(skiaBitmap, kNever_SkCopyPixelsMode);
+        if (uirenderer::Properties::isSkiaEnabled()) {
+            image = SkMakeImageInColorSpace(skiaBitmap, SkColorSpace::MakeSRGB(),
+                    skiaBitmap.getGenerationID(), kNever_SkCopyPixelsMode);
+        } else {
+            image = SkMakeImageFromRasterBitmap(skiaBitmap, kNever_SkCopyPixelsMode);
+        }
     }
     return image;
 }
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
index 10e637a65f57..925db303461f 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
@@ -303,6 +303,13 @@ sk_sp<Bitmap> SkiaOpenGLPipeline::allocateHardwareBitmap(renderthread::RenderThr
         return nullptr;
     }
 
+    auto colorSpace = info.colorSpace();
+    bool convertToSRGB = false;
+    if (colorSpace && (!colorSpace->isSRGB())) {
+        isSupported = false;
+        convertToSRGB = true;
+    }
+
     SkBitmap bitmap;
     if (isSupported) {
         bitmap = skBitmap;
@@ -310,7 +317,7 @@ sk_sp<Bitmap> SkiaOpenGLPipeline::allocateHardwareBitmap(renderthread::RenderThr
         bitmap.allocPixels(SkImageInfo::MakeN32(info.width(), info.height(), info.alphaType(),
                 nullptr));
         bitmap.eraseColor(0);
-        if (info.colorType() == kRGBA_F16_SkColorType) {
+        if (info.colorType() == kRGBA_F16_SkColorType || convertToSRGB) {
             // Drawing RGBA_F16 onto ARGB_8888 is not supported
             skBitmap.readPixels(bitmap.info().makeColorSpace(SkColorSpace::MakeSRGB()),
                     bitmap.getPixels(), bitmap.rowBytes(), 0, 0);