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
diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp
index 623b496..0a642b6 100644
--- a/libs/hwui/SkiaCanvas.cpp
+++ b/libs/hwui/SkiaCanvas.cpp
@@ -530,33 +530,25 @@
// ----------------------------------------------------------------------------
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 @@
// 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 @@
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::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 @@
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 34dddd1..16a19ca 100644
--- a/libs/hwui/SkiaCanvasProxy.cpp
+++ b/libs/hwui/SkiaCanvasProxy.cpp
@@ -151,7 +151,8 @@
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::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 3a8914e..7995877 100644
--- a/libs/hwui/hwui/Bitmap.cpp
+++ b/libs/hwui/hwui/Bitmap.cpp
@@ -209,7 +209,9 @@
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 @@
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 @@
// 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 10e637a..925db30 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
@@ -303,6 +303,13 @@
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 @@
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);