Set the color space to sRGB on the Surface and remove colorFilter.
Also for a canvas wrapping a bitmap the colorspace of the bitmap
will be used to correctly blend content.
Test: CtsUiRenderingTestCases
Bug: 111436479
Change-Id: I63ad7a30605a7f725cc0ef4716d42ea978fb03e3
diff --git a/core/jni/android/graphics/Shader.cpp b/core/jni/android/graphics/Shader.cpp
index 68f5bef..ed6a84b 100644
--- a/core/jni/android/graphics/Shader.cpp
+++ b/core/jni/android/graphics/Shader.cpp
@@ -64,11 +64,10 @@
jint tileModeX, jint tileModeY) {
const SkMatrix* matrix = reinterpret_cast<const SkMatrix*>(matrixPtr);
sk_sp<SkImage> image;
- sk_sp<SkColorFilter> colorFilter;
if (jbitmap) {
// Only pass a valid SkBitmap object to the constructor if the Bitmap exists. Otherwise,
// we'll pass an empty SkBitmap to avoid crashing/excepting for compatibility.
- image = android::bitmap::toBitmap(env, jbitmap).makeImage(&colorFilter);
+ image = android::bitmap::toBitmap(env, jbitmap).makeImage();
}
if (!image.get()) {
@@ -81,9 +80,6 @@
if (matrix) {
shader = shader->makeWithLocalMatrix(*matrix);
}
- if(colorFilter) {
- shader = shader->makeWithColorFilter(colorFilter);
- }
ThrowIAE_IfNull(env, shader.get());
return reinterpret_cast<jlong>(shader.release());
diff --git a/libs/hwui/DeferredLayerUpdater.cpp b/libs/hwui/DeferredLayerUpdater.cpp
index b772e5b..3bee301 100644
--- a/libs/hwui/DeferredLayerUpdater.cpp
+++ b/libs/hwui/DeferredLayerUpdater.cpp
@@ -85,19 +85,18 @@
mUpdateTexImage = false;
sk_sp<SkImage> layerImage;
SkMatrix textureTransform;
- android_dataspace dataSpace;
bool queueEmpty = true;
// If the SurfaceTexture queue is in synchronous mode, need to discard all
// but latest frame. Since we can't tell which mode it is in,
// do this unconditionally.
do {
- layerImage = mSurfaceTexture->dequeueImage(textureTransform, dataSpace, &queueEmpty,
+ layerImage = mSurfaceTexture->dequeueImage(textureTransform, &queueEmpty,
mRenderState);
} while (layerImage.get() && (!queueEmpty));
if (layerImage.get()) {
// force filtration if buffer size != layer size
bool forceFilter = mWidth != layerImage->width() || mHeight != layerImage->height();
- updateLayer(forceFilter, textureTransform, dataSpace, layerImage);
+ updateLayer(forceFilter, textureTransform, layerImage);
}
}
@@ -109,12 +108,11 @@
}
void DeferredLayerUpdater::updateLayer(bool forceFilter, const SkMatrix& textureTransform,
- android_dataspace dataspace, const sk_sp<SkImage>& layerImage) {
+ const sk_sp<SkImage>& layerImage) {
mLayer->setBlend(mBlend);
mLayer->setForceFilter(forceFilter);
mLayer->setSize(mWidth, mHeight);
mLayer->getTexTransform() = textureTransform;
- mLayer->setDataSpace(dataspace);
mLayer->setImage(layerImage);
}
diff --git a/libs/hwui/DeferredLayerUpdater.h b/libs/hwui/DeferredLayerUpdater.h
index b2c5131..a91c111 100644
--- a/libs/hwui/DeferredLayerUpdater.h
+++ b/libs/hwui/DeferredLayerUpdater.h
@@ -95,7 +95,7 @@
void detachSurfaceTexture();
void updateLayer(bool forceFilter, const SkMatrix& textureTransform,
- android_dataspace dataspace, const sk_sp<SkImage>& layerImage);
+ const sk_sp<SkImage>& layerImage);
void destroyLayer();
diff --git a/libs/hwui/Layer.cpp b/libs/hwui/Layer.cpp
index 32aaa54..d0df200 100644
--- a/libs/hwui/Layer.cpp
+++ b/libs/hwui/Layer.cpp
@@ -33,7 +33,6 @@
// TODO: This is a violation of Android's typical ref counting, but it
// preserves the old inc/dec ref locations. This should be changed...
incStrong(nullptr);
- buildColorSpaceWithFilter();
renderState.registerLayer(this);
texTransform.setIdentity();
transform.setIdentity();
@@ -43,36 +42,6 @@
mRenderState.unregisterLayer(this);
}
-void Layer::setColorFilter(sk_sp<SkColorFilter> filter) {
- if (filter != mColorFilter) {
- mColorFilter = filter;
- buildColorSpaceWithFilter();
- }
-}
-
-void Layer::setDataSpace(android_dataspace dataspace) {
- if (dataspace != mCurrentDataspace) {
- mCurrentDataspace = dataspace;
- buildColorSpaceWithFilter();
- }
-}
-
-void Layer::buildColorSpaceWithFilter() {
- sk_sp<SkColorFilter> colorSpaceFilter;
- sk_sp<SkColorSpace> colorSpace = DataSpaceToColorSpace(mCurrentDataspace);
- if (colorSpace && !colorSpace->isSRGB()) {
- colorSpaceFilter = SkToSRGBColorFilter::Make(colorSpace);
- }
-
- if (mColorFilter && colorSpaceFilter) {
- mColorSpaceWithFilter = mColorFilter->makeComposed(colorSpaceFilter);
- } else if (colorSpaceFilter) {
- mColorSpaceWithFilter = colorSpaceFilter;
- } else {
- mColorSpaceWithFilter = mColorFilter;
- }
-}
-
void Layer::postDecStrong() {
mRenderState.postDecStrong(this);
}
diff --git a/libs/hwui/Layer.h b/libs/hwui/Layer.h
index e4f96e9..98600db 100644
--- a/libs/hwui/Layer.h
+++ b/libs/hwui/Layer.h
@@ -69,15 +69,9 @@
SkBlendMode getMode() const;
- inline SkColorFilter* getColorFilter() const { return mColorFilter.get(); }
+ inline sk_sp<SkColorFilter> getColorFilter() const { return mColorFilter; }
- void setColorFilter(sk_sp<SkColorFilter> filter);
-
- void setDataSpace(android_dataspace dataspace);
-
- void setColorSpace(sk_sp<SkColorSpace> colorSpace);
-
- inline sk_sp<SkColorFilter> getColorSpaceWithFilter() const { return mColorSpaceWithFilter; }
+ void setColorFilter(sk_sp<SkColorFilter> filter) { mColorFilter = filter; };
inline SkMatrix& getTexTransform() { return texTransform; }
@@ -98,24 +92,12 @@
RenderState& mRenderState;
private:
- void buildColorSpaceWithFilter();
-
/**
* Color filter used to draw this layer. Optional.
*/
sk_sp<SkColorFilter> mColorFilter;
/**
- * Colorspace of the contents of the layer. Optional.
- */
- android_dataspace mCurrentDataspace = HAL_DATASPACE_UNKNOWN;
-
- /**
- * A color filter that is the combination of the mColorFilter and mColorSpace. Optional.
- */
- sk_sp<SkColorFilter> mColorSpaceWithFilter;
-
- /**
* Indicates raster data backing the layer is scaled, requiring filtration.
*/
bool forceFilter = false;
diff --git a/libs/hwui/Readback.cpp b/libs/hwui/Readback.cpp
index 80f2b57..2a48837 100644
--- a/libs/hwui/Readback.cpp
+++ b/libs/hwui/Readback.cpp
@@ -66,13 +66,10 @@
sk_sp<SkColorSpace> colorSpace =
DataSpaceToColorSpace(static_cast<android_dataspace>(surface.getBuffersDataSpace()));
- sk_sp<SkColorFilter> colorSpaceFilter;
- if (colorSpace && !colorSpace->isSRGB()) {
- colorSpaceFilter = SkToSRGBColorFilter::Make(colorSpace);
- }
sk_sp<SkImage> image = SkImage::MakeFromAHardwareBuffer(
- reinterpret_cast<AHardwareBuffer*>(sourceBuffer.get()), kPremul_SkAlphaType);
- return copyImageInto(image, colorSpaceFilter, texTransform, srcRect, bitmap);
+ reinterpret_cast<AHardwareBuffer*>(sourceBuffer.get()),
+ kPremul_SkAlphaType, colorSpace);
+ return copyImageInto(image, texTransform, srcRect, bitmap);
}
CopyResult Readback::copyHWBitmapInto(Bitmap* hwBitmap, SkBitmap* bitmap) {
@@ -83,20 +80,7 @@
transform.loadScale(1, -1, 1);
transform.translate(0, -1);
- // TODO: Try to take and reuse the image inside HW bitmap with "hwBitmap->makeImage".
- // TODO: When this was attempted, it resulted in instability.
- sk_sp<SkColorFilter> colorSpaceFilter;
- sk_sp<SkColorSpace> colorSpace = hwBitmap->info().refColorSpace();
- if (colorSpace && !colorSpace->isSRGB()) {
- colorSpaceFilter = SkToSRGBColorFilter::Make(colorSpace);
- }
- sk_sp<SkImage> image = SkImage::MakeFromAHardwareBuffer(
- reinterpret_cast<AHardwareBuffer*>(hwBitmap->graphicBuffer()), kPremul_SkAlphaType);
-
- // HW Bitmap currently can only attach to a GraphicBuffer with PIXEL_FORMAT_RGBA_8888 format
- // and SRGB color space. ImageDecoder can create a new HW Bitmap with non-SRGB color space: for
- // example see android.graphics.cts.BitmapColorSpaceTest#testEncodeP3hardware test.
- return copyImageInto(image, colorSpaceFilter, transform, srcRect, bitmap);
+ return copyImageInto(hwBitmap->makeImage(), transform, srcRect, bitmap);
}
CopyResult Readback::copyLayerInto(DeferredLayerUpdater* deferredLayer, SkBitmap* bitmap) {
@@ -118,8 +102,7 @@
return copyResult;
}
-CopyResult Readback::copyImageInto(const sk_sp<SkImage>& image,
- sk_sp<SkColorFilter>& colorSpaceFilter, Matrix4& texTransform,
+CopyResult Readback::copyImageInto(const sk_sp<SkImage>& image, Matrix4& texTransform,
const Rect& srcRect, SkBitmap* bitmap) {
if (Properties::getRenderPipelineType() == RenderPipelineType::SkiaGL) {
mRenderThread.requireGlContext();
@@ -157,11 +140,7 @@
return copyResult;
}
- // See Readback::copyLayerInto for an overview of color space conversion.
- // HW Bitmap are allowed to be in a non-SRGB color space (for example coming from ImageDecoder).
- // For Surface and HW Bitmap readback flows we pass colorSpaceFilter, which does the conversion.
- // TextureView readback is using Layer::setDataSpace, which creates a SkColorFilter internally.
- Layer layer(mRenderThread.renderState(), colorSpaceFilter, 255, SkBlendMode::kSrc);
+ Layer layer(mRenderThread.renderState(), nullptr, 255, SkBlendMode::kSrc);
bool disableFilter = MathUtils::areEqual(skiaSrcRect.width(), skiaDestRect.width()) &&
MathUtils::areEqual(skiaSrcRect.height(), skiaDestRect.height());
layer.setForceFilter(!disableFilter);
@@ -177,38 +156,6 @@
bool Readback::copyLayerInto(Layer* layer, const SkRect* srcRect, const SkRect* dstRect,
SkBitmap* bitmap) {
- /*
- * In the past only TextureView readback was setting the temporary surface color space to null.
- * Now all 3 readback flows are drawing into a SkSurface with null color space.
- * At readback there are 3 options to convert the source image color space to the destination
- * color space requested in "bitmap->info().colorSpace()":
- * 1. Set color space for temporary surface render target to null (disables color management),
- * colorspace tag from source SkImage is ignored by Skia,
- * convert SkImage to SRGB at draw time with SkColorFilter/SkToSRGBColorFilter,
- * do a readback from temporary SkSurface to a temporary SRGB SkBitmap "bitmap2",
- * read back from SRGB "bitmap2" into non-SRGB "bitmap" which will do a CPU color conversion.
- *
- * 2. Set color space for temporary surface render target to SRGB (not nullptr),
- * colorspace tag on the source SkImage is used by Skia to enable conversion,
- * convert SkImage to SRGB at draw time with drawImage (no filters),
- * do a readback from temporary SkSurface, which will do a color conversion from SRGB to
- * bitmap->info().colorSpace() on the CPU.
- *
- * 3. Set color space for temporary surface render target to bitmap->info().colorSpace(),
- * colorspace tag on the source SkImage is used by Skia to enable conversion,
- * convert SkImage to bitmap->info().colorSpace() at draw time with drawImage (no filters),
- * do a readback from SkSurface, which will not do any color conversion, because
- * surface was created with the same color space as the "bitmap".
- *
- * Option 1 is used for all readback flows.
- * Options 2 and 3 are new, because skia added support for non-SRGB render targets without
- * linear blending.
- * TODO: evaluate if options 2 or 3 for color space conversion are better.
- */
-
- // drop the colorSpace from the temporary surface.
- SkImageInfo surfaceInfo = bitmap->info().makeColorSpace(nullptr);
-
/* This intermediate surface is present to work around a bug in SwiftShader that
* prevents us from reading the contents of the layer's texture directly. The
* workaround involves first rendering that texture into an intermediate buffer and
@@ -217,70 +164,44 @@
* with reading incorrect data from EGLImage backed SkImage (likely a driver bug).
*/
sk_sp<SkSurface> tmpSurface = SkSurface::MakeRenderTarget(mRenderThread.getGrContext(),
- SkBudgeted::kYes, surfaceInfo);
+ SkBudgeted::kYes, bitmap->info());
+ // if we can't generate a GPU surface that matches the destination bitmap (e.g. 565) then we
+ // attempt to do the intermediate rendering step in 8888
if (!tmpSurface.get()) {
- surfaceInfo = surfaceInfo.makeColorType(SkColorType::kN32_SkColorType);
+ SkImageInfo tmpInfo = bitmap->info().makeColorType(SkColorType::kN32_SkColorType);
tmpSurface = SkSurface::MakeRenderTarget(mRenderThread.getGrContext(), SkBudgeted::kYes,
- surfaceInfo);
+ tmpInfo);
if (!tmpSurface.get()) {
- ALOGW("Unable to readback GPU contents into the provided bitmap");
+ ALOGW("Unable to generate GPU buffer in a format compatible with the provided bitmap");
return false;
}
}
- if (skiapipeline::LayerDrawable::DrawLayer(mRenderThread.getGrContext(),
- tmpSurface->getCanvas(), layer, srcRect, dstRect,
- false)) {
- // If bitmap->info().colorSpace() is non-SRGB, convert the data from SRGB to non-SRGB on
- // CPU. We can't just pass bitmap->info() to SkSurface::readPixels, because "tmpSurface" has
- // disabled color conversion.
- SkColorSpace* destColorSpace = bitmap->info().colorSpace();
- SkBitmap tempSRGBBitmap;
- SkBitmap tmpN32Bitmap;
- SkBitmap* bitmapInSRGB;
- if (destColorSpace && !destColorSpace->isSRGB()) {
- tempSRGBBitmap.allocPixels(bitmap->info().makeColorSpace(SkColorSpace::MakeSRGB()));
- bitmapInSRGB = &tempSRGBBitmap; // Need to convert latter from SRGB to non-SRGB.
- } else {
- bitmapInSRGB = bitmap; // No need for color conversion - write directly into output.
- }
- bool success = false;
+ if (!skiapipeline::LayerDrawable::DrawLayer(mRenderThread.getGrContext(),
+ tmpSurface->getCanvas(), layer, srcRect, dstRect,
+ false)) {
+ ALOGW("Unable to draw content from GPU into the provided bitmap");
+ return false;
+ }
- // TODO: does any of the readbacks below clamp F16 exSRGB?
- // Readback into a SRGB SkBitmap.
- if (tmpSurface->readPixels(bitmapInSRGB->info(), bitmapInSRGB->getPixels(),
- bitmapInSRGB->rowBytes(), 0, 0)) {
- success = true;
- } else {
- // if we fail to readback from the GPU directly (e.g. 565) then we attempt to read into
- // 8888 and then convert that into the destination format before giving up.
- SkImageInfo bitmapInfo =
- SkImageInfo::MakeN32(bitmap->width(), bitmap->height(), bitmap->alphaType(),
- SkColorSpace::MakeSRGB());
- if (tmpN32Bitmap.tryAllocPixels(bitmapInfo) &&
- tmpSurface->readPixels(bitmapInfo, tmpN32Bitmap.getPixels(),
- tmpN32Bitmap.rowBytes(), 0, 0)) {
- success = true;
- bitmapInSRGB = &tmpN32Bitmap;
- }
- }
-
- if (success) {
- if (bitmapInSRGB != bitmap) {
- // Convert from SRGB to non-SRGB color space if needed. Convert from N32 to
- // destination bitmap color format if needed.
- if (!bitmapInSRGB->readPixels(bitmap->info(), bitmap->getPixels(),
- bitmap->rowBytes(), 0, 0)) {
- return false;
- }
- }
- bitmap->notifyPixelsChanged();
- return true;
+ if (!tmpSurface->readPixels(*bitmap, 0, 0)) {
+ // if we fail to readback from the GPU directly (e.g. 565) then we attempt to read into
+ // 8888 and then convert that into the destination format before giving up.
+ SkBitmap tmpBitmap;
+ SkImageInfo tmpInfo = bitmap->info().makeColorType(SkColorType::kN32_SkColorType);
+ if (bitmap->info().colorType() == SkColorType::kN32_SkColorType ||
+ !tmpBitmap.tryAllocPixels(tmpInfo) ||
+ !tmpSurface->readPixels(tmpBitmap, 0, 0) ||
+ !tmpBitmap.readPixels(bitmap->info(), bitmap->getPixels(),
+ bitmap->rowBytes(), 0, 0)) {
+ ALOGW("Unable to convert content into the provided bitmap");
+ return false;
}
}
- return false;
+ bitmap->notifyPixelsChanged();
+ return true;
}
} /* namespace uirenderer */
diff --git a/libs/hwui/Readback.h b/libs/hwui/Readback.h
index d9e10ce..e86a813 100644
--- a/libs/hwui/Readback.h
+++ b/libs/hwui/Readback.h
@@ -54,8 +54,8 @@
CopyResult copyLayerInto(DeferredLayerUpdater* layer, SkBitmap* bitmap);
private:
- CopyResult copyImageInto(const sk_sp<SkImage>& image, sk_sp<SkColorFilter>& colorSpaceFilter,
- Matrix4& texTransform, const Rect& srcRect, SkBitmap* bitmap);
+ CopyResult copyImageInto(const sk_sp<SkImage>& image, Matrix4& texTransform,
+ const Rect& srcRect, SkBitmap* bitmap);
bool copyLayerInto(Layer* layer, const SkRect* srcRect, const SkRect* dstRect,
SkBitmap* bitmap);
diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp
index 9c707ba..1543386 100644
--- a/libs/hwui/SkiaCanvas.cpp
+++ b/libs/hwui/SkiaCanvas.cpp
@@ -27,7 +27,6 @@
#include <SkAnimatedImage.h>
#include <SkCanvasStateUtils.h>
#include <SkColorFilter.h>
-#include <SkColorSpaceXformCanvas.h>
#include <SkDeque.h>
#include <SkDrawable.h>
#include <SkGraphics.h>
@@ -60,19 +59,8 @@
SkiaCanvas::SkiaCanvas(SkCanvas* canvas) : mCanvas(canvas) {}
SkiaCanvas::SkiaCanvas(const SkBitmap& bitmap) {
- sk_sp<SkColorSpace> cs = bitmap.refColorSpace();
- mCanvasOwned =
- std::unique_ptr<SkCanvas>(new SkCanvas(bitmap, SkCanvas::ColorBehavior::kLegacy));
- if (cs.get() == nullptr || cs->isSRGB()) {
- mCanvas = mCanvasOwned.get();
- } else {
- /** The wrapper is needed if we are drawing into a non-sRGB destination, since
- * we need to transform all colors (not just bitmaps via filters) into the
- * destination's colorspace.
- */
- mCanvasWrapper = SkCreateColorSpaceXformCanvas(mCanvasOwned.get(), std::move(cs));
- mCanvas = mCanvasWrapper.get();
- }
+ mCanvasOwned = std::unique_ptr<SkCanvas>(new SkCanvas(bitmap));
+ mCanvas = mCanvasOwned.get();
}
SkiaCanvas::~SkiaCanvas() {}
@@ -81,7 +69,6 @@
if (mCanvas != skiaCanvas) {
mCanvas = skiaCanvas;
mCanvasOwned.reset();
- mCanvasWrapper.reset();
}
mSaveStack.reset(nullptr);
}
@@ -91,18 +78,9 @@
// ----------------------------------------------------------------------------
void SkiaCanvas::setBitmap(const SkBitmap& bitmap) {
- sk_sp<SkColorSpace> cs = bitmap.refColorSpace();
- std::unique_ptr<SkCanvas> newCanvas =
- std::unique_ptr<SkCanvas>(new SkCanvas(bitmap, SkCanvas::ColorBehavior::kLegacy));
- std::unique_ptr<SkCanvas> newCanvasWrapper;
- if (cs.get() != nullptr && !cs->isSRGB()) {
- newCanvasWrapper = SkCreateColorSpaceXformCanvas(newCanvas.get(), std::move(cs));
- }
-
// deletes the previously owned canvas (if any)
- mCanvasOwned = std::move(newCanvas);
- mCanvasWrapper = std::move(newCanvasWrapper);
- mCanvas = mCanvasWrapper ? mCanvasWrapper.get() : mCanvasOwned.get();
+ mCanvasOwned.reset(new SkCanvas(bitmap));
+ mCanvas = mCanvasOwned.get();
// clean up the old save stack
mSaveStack.reset(nullptr);
@@ -547,40 +525,14 @@
// Canvas draw operations: Bitmaps
// ----------------------------------------------------------------------------
-SkiaCanvas::PaintCoW&& SkiaCanvas::filterBitmap(PaintCoW&& paint,
- sk_sp<SkColorFilter> colorSpaceFilter) const {
- /* We don't apply the colorSpace filter if this canvas is already wrapped with
- * a SkColorSpaceXformCanvas since it already takes care of converting the
- * contents of the bitmap into the appropriate colorspace. The mCanvasWrapper
- * should only be used if this canvas is backed by a surface/bitmap that is known
- * to have a non-sRGB colorspace.
- */
- if (!mCanvasWrapper && colorSpaceFilter) {
- SkPaint& tmpPaint = paint.writeable();
- if (tmpPaint.getColorFilter()) {
- tmpPaint.setColorFilter(SkColorFilter::MakeComposeFilter(tmpPaint.refColorFilter(),
- std::move(colorSpaceFilter)));
- LOG_ALWAYS_FATAL_IF(!tmpPaint.getColorFilter());
- } else {
- tmpPaint.setColorFilter(std::move(colorSpaceFilter));
- }
- }
- return filterPaint(std::move(paint));
-}
-
void SkiaCanvas::drawBitmap(Bitmap& bitmap, float left, float top, const SkPaint* paint) {
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mCanvas->drawImage(image, left, top, filterBitmap(paint, std::move(colorFilter)));
+ mCanvas->drawImage(bitmap.makeImage(), left, top, filterPaint(paint));
}
void SkiaCanvas::drawBitmap(Bitmap& bitmap, const SkMatrix& matrix, const SkPaint* paint) {
SkAutoCanvasRestore acr(mCanvas, true);
mCanvas->concat(matrix);
-
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mCanvas->drawImage(image, 0, 0, filterBitmap(paint, std::move(colorFilter)));
+ mCanvas->drawImage(bitmap.makeImage(), 0, 0, filterPaint(paint));
}
void SkiaCanvas::drawBitmap(Bitmap& bitmap, float srcLeft, float srcTop, float srcRight,
@@ -589,9 +541,7 @@
SkRect srcRect = SkRect::MakeLTRB(srcLeft, srcTop, srcRight, srcBottom);
SkRect dstRect = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom);
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mCanvas->drawImageRect(image, srcRect, dstRect, filterBitmap(paint, std::move(colorFilter)),
+ mCanvas->drawImageRect(bitmap.makeImage(), srcRect, dstRect, filterPaint(paint),
SkCanvas::kFast_SrcRectConstraint);
}
@@ -673,13 +623,9 @@
PaintCoW paintCoW(paint);
SkPaint& tmpPaint = paintCoW.writeable();
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
+ sk_sp<SkImage> image = bitmap.makeImage();
sk_sp<SkShader> shader =
image->makeShader(SkShader::kClamp_TileMode, SkShader::kClamp_TileMode);
- if (colorFilter) {
- shader = shader->makeWithColorFilter(std::move(colorFilter));
- }
tmpPaint.setShader(std::move(shader));
mCanvas->drawVertices(builder.detach(), SkBlendMode::kModulate,
@@ -710,10 +656,7 @@
lattice.fBounds = nullptr;
SkRect dst = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom);
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mCanvas->drawImageLattice(image.get(), lattice, dst,
- filterBitmap(paint, std::move(colorFilter)));
+ mCanvas->drawImageLattice(bitmap.makeImage().get(), lattice, dst, filterPaint(paint));
}
double SkiaCanvas::drawAnimatedImage(AnimatedImageDrawable* imgDrawable) {
diff --git a/libs/hwui/SkiaCanvas.h b/libs/hwui/SkiaCanvas.h
index 3a877cf..24d9c08 100644
--- a/libs/hwui/SkiaCanvas.h
+++ b/libs/hwui/SkiaCanvas.h
@@ -232,7 +232,6 @@
class Clip;
- std::unique_ptr<SkCanvas> mCanvasWrapper; // might own a wrapper on the canvas
std::unique_ptr<SkCanvas> mCanvasOwned; // might own a canvas we allocated
SkCanvas* mCanvas; // we do NOT own this canvas, it must survive us
// unless it is the same as mCanvasOwned.get()
diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp
index 75a6e72..6c77f9e 100644
--- a/libs/hwui/hwui/Bitmap.cpp
+++ b/libs/hwui/hwui/Bitmap.cpp
@@ -32,7 +32,6 @@
#include <SkCanvas.h>
#include <SkImagePriv.h>
-#include <SkToSRGBColorFilter.h>
#include <SkHighContrastFilter.h>
#include <limits>
@@ -287,14 +286,8 @@
void Bitmap::getSkBitmap(SkBitmap* outBitmap) {
if (isHardware()) {
- outBitmap->allocPixels(SkImageInfo::Make(info().width(), info().height(),
- info().colorType(), info().alphaType(), nullptr));
+ outBitmap->allocPixels(mInfo);
uirenderer::renderthread::RenderProxy::copyHWBitmapInto(this, outBitmap);
- if (mInfo.colorSpace()) {
- sk_sp<SkPixelRef> pixelRef = sk_ref_sp(outBitmap->pixelRef());
- outBitmap->setInfo(mInfo);
- outBitmap->setPixelRef(std::move(pixelRef), 0, 0);
- }
return;
}
outBitmap->setInfo(mInfo, rowBytes());
@@ -313,7 +306,7 @@
return nullptr;
}
-sk_sp<SkImage> Bitmap::makeImage(sk_sp<SkColorFilter>* outputColorFilter) {
+sk_sp<SkImage> Bitmap::makeImage() {
sk_sp<SkImage> image = mImage;
if (!image) {
SkASSERT(!isHardware());
@@ -325,9 +318,6 @@
// TODO: refactor Bitmap to not derive from SkPixelRef, which would allow caching here.
image = SkMakeImageFromRasterBitmap(skiaBitmap, kNever_SkCopyPixelsMode);
}
- if (image->colorSpace() != nullptr && !image->colorSpace()->isSRGB()) {
- *outputColorFilter = SkToSRGBColorFilter::Make(image->refColorSpace());
- }
return image;
}
diff --git a/libs/hwui/hwui/Bitmap.h b/libs/hwui/hwui/Bitmap.h
index 238c764..d446377 100644
--- a/libs/hwui/hwui/Bitmap.h
+++ b/libs/hwui/hwui/Bitmap.h
@@ -105,14 +105,8 @@
* Creates or returns a cached SkImage and is safe to be invoked from either
* the UI or RenderThread.
*
- * @param outputColorFilter is a required param that will be populated by
- * this function if the bitmap's colorspace is not sRGB. If populated the
- * filter will convert colors from the bitmaps colorspace into sRGB. It
- * is the callers responsibility to use this colorFilter when drawing
- * this image into any destination that is presumed to be sRGB (i.e. a
- * buffer that has no colorspace defined).
*/
- sk_sp<SkImage> makeImage(sk_sp<SkColorFilter>* outputColorFilter);
+ sk_sp<SkImage> makeImage();
static BitmapPalette computePalette(const SkImageInfo& info, const void* addr, size_t rowBytes);
diff --git a/libs/hwui/pipeline/skia/LayerDrawable.cpp b/libs/hwui/pipeline/skia/LayerDrawable.cpp
index 13d2dae..41788b6 100644
--- a/libs/hwui/pipeline/skia/LayerDrawable.cpp
+++ b/libs/hwui/pipeline/skia/LayerDrawable.cpp
@@ -70,7 +70,7 @@
SkPaint paint;
paint.setAlpha(layer->getAlpha());
paint.setBlendMode(layer->getMode());
- paint.setColorFilter(layer->getColorSpaceWithFilter());
+ paint.setColorFilter(layer->getColorFilter());
const bool nonIdentityMatrix = !matrix.isIdentity();
if (nonIdentityMatrix) {
canvas->save();
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
index d401b38..d6adaf8 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
@@ -97,7 +97,7 @@
SkASSERT(mRenderThread.getGrContext() != nullptr);
sk_sp<SkSurface> surface(SkSurface::MakeFromBackendRenderTarget(
mRenderThread.getGrContext(), backendRT, kBottomLeft_GrSurfaceOrigin, colorType,
- nullptr, &props));
+ mSurfaceColorSpace, &props));
SkiaPipeline::updateLighting(lightGeometry, lightInfo);
renderFrame(*layerUpdateQueue, dirty, renderNodes, opaque, contentDrawBounds, surface);
@@ -172,6 +172,7 @@
} else if (colorMode == ColorMode::WideColorGamut) {
mSurfaceColorType = SkColorType::kRGBA_F16_SkColorType;
}
+ mSurfaceColorSpace = SkColorSpace::MakeSRGB();
if (mEglSurface != EGL_NO_SURFACE) {
const bool preserveBuffer = (swapBehavior != SwapBehavior::kSwap_discardBuffer);
diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
index 2dfe7c7..7a255c1 100644
--- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
@@ -169,7 +169,7 @@
if (!layer || layer->width() != surfaceWidth || layer->height() != surfaceHeight) {
SkImageInfo info;
info = SkImageInfo::Make(surfaceWidth, surfaceHeight, getSurfaceColorType(),
- kPremul_SkAlphaType);
+ kPremul_SkAlphaType, getSurfaceColorSpace());
SkSurfaceProps props(0, kUnknown_SkPixelGeometry);
SkASSERT(mRenderThread.getGrContext() != nullptr);
node->setLayerSurface(SkSurface::MakeRenderTarget(mRenderThread.getGrContext(),
@@ -204,8 +204,7 @@
GrContext* context = thread.getGrContext();
if (context) {
ATRACE_FORMAT("Bitmap#prepareToDraw %dx%d", bitmap->width(), bitmap->height());
- sk_sp<SkColorFilter> colorFilter;
- auto image = bitmap->makeImage(&colorFilter);
+ auto image = bitmap->makeImage();
if (image.get() && !bitmap->isHardware()) {
SkImage_pinAsTexture(image.get(), context);
SkImage_unpinAsTexture(image.get(), context);
diff --git a/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp b/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp
index 596b8af..b66a843 100644
--- a/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp
+++ b/libs/hwui/pipeline/skia/SkiaRecordingCanvas.cpp
@@ -179,9 +179,8 @@
}
void SkiaRecordingCanvas::drawBitmap(Bitmap& bitmap, float left, float top, const SkPaint* paint) {
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mRecorder.drawImage(image, left, top, filterBitmap(paint, std::move(colorFilter)), bitmap.palette());
+ sk_sp<SkImage> image = bitmap.makeImage();
+ mRecorder.drawImage(image, left, top, filterPaint(paint), bitmap.palette());
// if image->unique() is true, then mRecorder.drawImage failed for some reason. It also means
// it is not safe to store a raw SkImage pointer, because the image object will be destroyed
// when this function ends.
@@ -194,9 +193,8 @@
SkAutoCanvasRestore acr(&mRecorder, true);
concat(matrix);
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mRecorder.drawImage(image, 0, 0, filterBitmap(paint, std::move(colorFilter)), bitmap.palette());
+ sk_sp<SkImage> image = bitmap.makeImage();
+ mRecorder.drawImage(image, 0, 0, filterPaint(paint), bitmap.palette());
if (!bitmap.isImmutable() && image.get() && !image->unique()) {
mDisplayList->mMutableImages.push_back(image.get());
}
@@ -208,9 +206,8 @@
SkRect srcRect = SkRect::MakeLTRB(srcLeft, srcTop, srcRight, srcBottom);
SkRect dstRect = SkRect::MakeLTRB(dstLeft, dstTop, dstRight, dstBottom);
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
- mRecorder.drawImageRect(image, srcRect, dstRect, filterBitmap(paint, std::move(colorFilter)),
+ sk_sp<SkImage> image = bitmap.makeImage();
+ mRecorder.drawImageRect(image, srcRect, dstRect, filterPaint(paint),
SkCanvas::kFast_SrcRectConstraint, bitmap.palette());
if (!bitmap.isImmutable() && image.get() && !image->unique() && !srcRect.isEmpty() &&
!dstRect.isEmpty()) {
@@ -247,10 +244,9 @@
if (!filteredPaint || filteredPaint->getFilterQuality() != kLow_SkFilterQuality) {
filteredPaint.writeable().setFilterQuality(kLow_SkFilterQuality);
}
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
+ sk_sp<SkImage> image = bitmap.makeImage();
mRecorder.drawImageLattice(image, lattice, dst,
- filterBitmap(std::move(filteredPaint), std::move(colorFilter)),
+ filterPaint(std::move(filteredPaint)),
bitmap.palette());
if (!bitmap.isImmutable() && image.get() && !image->unique() && !dst.isEmpty()) {
mDisplayList->mMutableImages.push_back(image.get());
diff --git a/libs/hwui/surfacetexture/ImageConsumer.cpp b/libs/hwui/surfacetexture/ImageConsumer.cpp
index 9ffccfb..15aec9f 100644
--- a/libs/hwui/surfacetexture/ImageConsumer.cpp
+++ b/libs/hwui/surfacetexture/ImageConsumer.cpp
@@ -22,6 +22,7 @@
#include "renderthread/EglManager.h"
#include "renderthread/RenderThread.h"
#include "renderthread/VulkanManager.h"
+#include "utils/Color.h"
// Macro for including the SurfaceTexture name in log messages
#define IMG_LOGE(x, ...) ALOGE("[%s] " x, st.mName.string(), ##__VA_ARGS__)
@@ -44,13 +45,16 @@
mImageSlots[buf].mEglFence = EGL_NO_SYNC_KHR;
}
-void ImageConsumer::ImageSlot::createIfNeeded(sp<GraphicBuffer> graphicBuffer) {
- if (!mImage.get()) {
+void ImageConsumer::ImageSlot::createIfNeeded(sp<GraphicBuffer> graphicBuffer,
+ android_dataspace dataspace) {
+ if (!mImage.get() || dataspace != mDataspace) {
mImage = graphicBuffer.get()
? SkImage::MakeFromAHardwareBuffer(
reinterpret_cast<AHardwareBuffer*>(graphicBuffer.get()),
- kPremul_SkAlphaType)
+ kPremul_SkAlphaType,
+ uirenderer::DataSpaceToColorSpace(dataspace))
: nullptr;
+ mDataspace = dataspace;
}
}
@@ -66,7 +70,7 @@
int slot = st.mCurrentTexture;
if (slot != BufferItem::INVALID_BUFFER_SLOT) {
*queueEmpty = true;
- mImageSlots[slot].createIfNeeded(st.mSlots[slot].mGraphicBuffer);
+ mImageSlots[slot].createIfNeeded(st.mSlots[slot].mGraphicBuffer, item.mDataSpace);
return mImageSlots[slot].mImage;
}
}
@@ -145,7 +149,7 @@
st.computeCurrentTransformMatrixLocked();
*queueEmpty = false;
- mImageSlots[slot].createIfNeeded(st.mSlots[slot].mGraphicBuffer);
+ mImageSlots[slot].createIfNeeded(st.mSlots[slot].mGraphicBuffer, item.mDataSpace);
return mImageSlots[slot].mImage;
}
diff --git a/libs/hwui/surfacetexture/ImageConsumer.h b/libs/hwui/surfacetexture/ImageConsumer.h
index 31ee8db..5bab0ef5 100644
--- a/libs/hwui/surfacetexture/ImageConsumer.h
+++ b/libs/hwui/surfacetexture/ImageConsumer.h
@@ -68,18 +68,21 @@
* ImageConsumer maintains about a BufferQueue buffer slot.
*/
struct ImageSlot {
- ImageSlot() : mEglFence(EGL_NO_SYNC_KHR) {}
+ ImageSlot() : mDataspace(HAL_DATASPACE_UNKNOWN), mEglFence(EGL_NO_SYNC_KHR) {}
// mImage is the SkImage created from mGraphicBuffer.
sk_sp<SkImage> mImage;
+ // the dataspace associated with the current image
+ android_dataspace mDataspace;
+
/**
* mEglFence is the EGL sync object that must signal before the buffer
* associated with this buffer slot may be dequeued.
*/
EGLSyncKHR mEglFence;
- void createIfNeeded(sp<GraphicBuffer> graphicBuffer);
+ void createIfNeeded(sp<GraphicBuffer> graphicBuffer, android_dataspace dataspace);
};
/**
diff --git a/libs/hwui/surfacetexture/SurfaceTexture.cpp b/libs/hwui/surfacetexture/SurfaceTexture.cpp
index 4bff715..90f8912 100644
--- a/libs/hwui/surfacetexture/SurfaceTexture.cpp
+++ b/libs/hwui/surfacetexture/SurfaceTexture.cpp
@@ -470,8 +470,7 @@
ConsumerBase::dumpLocked(result, prefix);
}
-sk_sp<SkImage> SurfaceTexture::dequeueImage(SkMatrix& transformMatrix, android_dataspace& dataSpace,
- bool* queueEmpty,
+sk_sp<SkImage> SurfaceTexture::dequeueImage(SkMatrix& transformMatrix, bool* queueEmpty,
uirenderer::RenderState& renderState) {
Mutex::Autolock _l(mMutex);
@@ -488,7 +487,6 @@
auto image = mImageConsumer.dequeueImage(queueEmpty, *this, renderState);
if (image.get()) {
uirenderer::mat4(mCurrentTransformMatrix).copyTo(transformMatrix);
- dataSpace = mCurrentDataSpace;
}
return image;
}
diff --git a/libs/hwui/surfacetexture/SurfaceTexture.h b/libs/hwui/surfacetexture/SurfaceTexture.h
index db392a9..96afd82 100644
--- a/libs/hwui/surfacetexture/SurfaceTexture.h
+++ b/libs/hwui/surfacetexture/SurfaceTexture.h
@@ -258,8 +258,8 @@
*/
status_t attachToContext(uint32_t tex);
- sk_sp<SkImage> dequeueImage(SkMatrix& transformMatrix, android_dataspace& dataSpace,
- bool* queueEmpty, uirenderer::RenderState& renderState);
+ sk_sp<SkImage> dequeueImage(SkMatrix& transformMatrix, bool* queueEmpty,
+ uirenderer::RenderState& renderState);
/**
* attachToView attaches a SurfaceTexture that is currently in the
diff --git a/libs/hwui/tests/common/TestUtils.cpp b/libs/hwui/tests/common/TestUtils.cpp
index 66b9b85..8a1bc4d 100644
--- a/libs/hwui/tests/common/TestUtils.cpp
+++ b/libs/hwui/tests/common/TestUtils.cpp
@@ -72,9 +72,7 @@
layerUpdater->setTransform(&transform);
// updateLayer so it's ready to draw
- SkMatrix identity;
- identity.setIdentity();
- layerUpdater->updateLayer(true, identity, HAL_DATASPACE_UNKNOWN, nullptr);
+ layerUpdater->updateLayer(true, SkMatrix::I(), nullptr);
return layerUpdater;
}
diff --git a/libs/hwui/tests/common/scenes/BitmapShaders.cpp b/libs/hwui/tests/common/scenes/BitmapShaders.cpp
index 15039b5..ad11a1d 100644
--- a/libs/hwui/tests/common/scenes/BitmapShaders.cpp
+++ b/libs/hwui/tests/common/scenes/BitmapShaders.cpp
@@ -44,8 +44,7 @@
});
SkPaint paint;
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = hwuiBitmap->makeImage(&colorFilter);
+ sk_sp<SkImage> image = hwuiBitmap->makeImage();
sk_sp<SkShader> repeatShader =
image->makeShader(SkShader::TileMode::kRepeat_TileMode,
SkShader::TileMode::kRepeat_TileMode, nullptr);
diff --git a/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp b/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp
index f137562..448408d 100644
--- a/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp
+++ b/libs/hwui/tests/common/scenes/HwBitmapInCompositeShader.cpp
@@ -72,8 +72,7 @@
void doFrame(int frameNr) override {}
sk_sp<SkShader> createBitmapShader(Bitmap& bitmap) {
- sk_sp<SkColorFilter> colorFilter;
- sk_sp<SkImage> image = bitmap.makeImage(&colorFilter);
+ sk_sp<SkImage> image = bitmap.makeImage();
return image->makeShader(SkShader::TileMode::kClamp_TileMode,
SkShader::TileMode::kClamp_TileMode);
}
diff --git a/libs/hwui/tests/unit/CacheManagerTests.cpp b/libs/hwui/tests/unit/CacheManagerTests.cpp
index c235715..210fced 100644
--- a/libs/hwui/tests/unit/CacheManagerTests.cpp
+++ b/libs/hwui/tests/unit/CacheManagerTests.cpp
@@ -54,8 +54,7 @@
// create an image and pin it so that we have something with a unique key in the cache
sk_sp<Bitmap> bitmap =
Bitmap::allocateHeapBitmap(SkImageInfo::MakeA8(displayInfo.w, displayInfo.h));
- sk_sp<SkColorFilter> filter;
- sk_sp<SkImage> image = bitmap->makeImage(&filter);
+ sk_sp<SkImage> image = bitmap->makeImage();
ASSERT_TRUE(SkImage_pinAsTexture(image.get(), grContext));
// attempt to trim all memory while we still hold strong refs
diff --git a/libs/hwui/tests/unit/DeferredLayerUpdaterTests.cpp b/libs/hwui/tests/unit/DeferredLayerUpdaterTests.cpp
index 6c8775b..a686979 100644
--- a/libs/hwui/tests/unit/DeferredLayerUpdaterTests.cpp
+++ b/libs/hwui/tests/unit/DeferredLayerUpdaterTests.cpp
@@ -43,7 +43,7 @@
SkBitmap bitmap;
bitmap.allocN32Pixels(16, 16);
sk_sp<SkImage> layerImage = SkImage::MakeFromBitmap(bitmap);
- layerUpdater->updateLayer(true, scaledMatrix, HAL_DATASPACE_UNKNOWN, layerImage);
+ layerUpdater->updateLayer(true, scaledMatrix, layerImage);
// the backing layer should now have all the properties applied.
EXPECT_EQ(100u, layerUpdater->backingLayer()->getWidth());
diff --git a/libs/hwui/tests/unit/SkiaCanvasTests.cpp b/libs/hwui/tests/unit/SkiaCanvasTests.cpp
index 634ceff..f3a7648 100644
--- a/libs/hwui/tests/unit/SkiaCanvasTests.cpp
+++ b/libs/hwui/tests/unit/SkiaCanvasTests.cpp
@@ -53,12 +53,12 @@
adobeBitmap->getSkBitmap(&adobeSkBitmap);
*adobeSkBitmap.getAddr32(0, 0) = 0xFF0000F0; // Opaque, almost fully-red
- SkImageInfo info = adobeInfo.makeColorSpace(nullptr);
+ SkImageInfo info = adobeInfo.makeColorSpace(SkColorSpace::MakeSRGB());
sk_sp<Bitmap> bitmap = Bitmap::allocateHeapBitmap(info);
SkBitmap skBitmap;
bitmap->getSkBitmap(&skBitmap);
- // Create a software canvas.
+ // Create a software sRGB canvas.
SkiaCanvas canvas(skBitmap);
canvas.drawBitmap(*adobeBitmap, 0, 0, nullptr);
// The result should be fully red, since we convert to sRGB at draw time.
@@ -77,7 +77,7 @@
picCanvas.drawBitmap(*adobeBitmap, 0, 0, nullptr);
sk_sp<SkPicture> picture = recorder.finishRecordingAsPicture();
- // Playback to an software canvas. The result should be fully red.
+ // Playback to a software sRGB canvas. The result should be fully red.
canvas.asSkCanvas()->drawPicture(picture);
ASSERT_EQ(0xFF0000FF, *skBitmap.getAddr32(0, 0));
}