Make SkBitmap::CopyTo respect requested dst color type when bitmap is texture backed.

BUG=chromium:550559
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1576983002

Review URL: https://codereview.chromium.org/1576983002
This commit is contained in:
bsalomon 2016-01-11 11:14:17 -08:00 committed by Commit bot
parent b4b42ed671
commit 9d22fd6e7b
7 changed files with 104 additions and 13 deletions

View File

@ -225,7 +225,8 @@ public:
return this->onGetYUV8Planes(sizes, planes, rowBytes, colorSpace);
}
bool readPixels(SkBitmap* dst, const SkIRect* subset = NULL);
/** Populates dst with the pixels of this pixelRef, converting them to colorType. */
bool readPixels(SkBitmap* dst, SkColorType colorType, const SkIRect* subset = NULL);
/**
* Makes a deep copy of this PixelRef, respecting the requested config.
@ -299,7 +300,7 @@ protected:
*
* The base class implementation returns false;
*/
virtual bool onReadPixels(SkBitmap* dst, const SkIRect* subsetOrNull);
virtual bool onReadPixels(SkBitmap* dst, SkColorType colorType, const SkIRect* subsetOrNull);
// default impl returns NULL.
virtual SkData* onRefEncodedData();

View File

@ -49,7 +49,7 @@ public:
protected:
// overrides from SkPixelRef
bool onReadPixels(SkBitmap* dst, const SkIRect* subset) override;
bool onReadPixels(SkBitmap* dst, SkColorType, const SkIRect* subset) override;
SkPixelRef* deepCopy(SkColorType, SkColorProfileType,
const SkIRect* subset) override;
void onNotifyPixelsChanged() override;

View File

@ -828,7 +828,7 @@ bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc)
SkIRect subset;
subset.setXYWH(fPixelRefOrigin.fX, fPixelRefOrigin.fY,
fInfo.width(), fInfo.height());
if (fPixelRef->readPixels(&tmpSrc, &subset)) {
if (fPixelRef->readPixels(&tmpSrc, dstColorType, &subset)) {
if (fPixelRef->info().alphaType() == kUnpremul_SkAlphaType) {
// FIXME: The only meaningful implementation of readPixels
// (GrPixelRef) assumes premultiplied pixels.

View File

@ -286,7 +286,7 @@ void SkBitmapDevice::drawBitmapRect(const SkDraw& draw, const SkBitmap& bitmap,
if(bitmap.pixelRef()->getTexture()) {
// Accelerated source canvas, don't use extractSubset but readPixels to get the subset.
// This way, the pixels are copied in CPU memory instead of GPU memory.
bitmap.pixelRef()->readPixels(&tmpBitmap, &srcIR);
bitmap.pixelRef()->readPixels(&tmpBitmap, kN32_SkColorType, &srcIR);
} else {
if (!bitmap.extractSubset(&tmpBitmap, srcIR)) {
return;

View File

@ -307,13 +307,13 @@ void SkPixelRef::restoreMutability() {
fMutability = kMutable;
}
bool SkPixelRef::readPixels(SkBitmap* dst, const SkIRect* subset) {
return this->onReadPixels(dst, subset);
bool SkPixelRef::readPixels(SkBitmap* dst, SkColorType ct, const SkIRect* subset) {
return this->onReadPixels(dst, ct, subset);
}
///////////////////////////////////////////////////////////////////////////////////////////////////
bool SkPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
bool SkPixelRef::onReadPixels(SkBitmap* dst, SkColorType, const SkIRect* subset) {
return false;
}

View File

@ -25,7 +25,7 @@ SkROLockPixelsPixelRef::~SkROLockPixelsPixelRef() {}
bool SkROLockPixelsPixelRef::onNewLockPixels(LockRec* rec) {
fBitmap.reset();
// SkDebugf("---------- calling readpixels in support of lockpixels\n");
if (!this->onReadPixels(&fBitmap, nullptr)) {
if (!this->onReadPixels(&fBitmap, this->info().colorType(), nullptr)) {
SkDebugf("SkROLockPixelsPixelRef::onLockPixels failed!\n");
return false;
}
@ -155,11 +155,20 @@ static bool tryAllocBitmapPixels(SkBitmap* bitmap) {
}
}
bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
bool SkGrPixelRef::onReadPixels(SkBitmap* dst, SkColorType colorType, const SkIRect* subset) {
if (nullptr == fSurface || fSurface->wasDestroyed()) {
return false;
}
GrPixelConfig config;
if (kRGBA_8888_SkColorType == colorType) {
config = kRGBA_8888_GrPixelConfig;
} else if (kBGRA_8888_SkColorType == colorType) {
config = kBGRA_8888_GrPixelConfig;
} else {
return false;
}
SkIRect bounds;
if (subset) {
bounds = *subset;
@ -172,7 +181,9 @@ bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
//Cache miss
SkBitmap cachedBitmap;
cachedBitmap.setInfo(this->info().makeWH(bounds.width(), bounds.height()));
cachedBitmap.setInfo(SkImageInfo::Make(bounds.width(), bounds.height(), colorType,
this->info().alphaType(),
this->info().profileType()));
// If we can't alloc the pixels, then fail
if (!tryAllocBitmapPixels(&cachedBitmap)) {
@ -183,8 +194,7 @@ bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) {
void* buffer = cachedBitmap.getPixels();
bool readPixelsOk = fSurface->readPixels(bounds.fLeft, bounds.fTop,
bounds.width(), bounds.height(),
kSkia8888_GrPixelConfig,
buffer, cachedBitmap.rowBytes());
config, buffer, cachedBitmap.rowBytes());
if (!readPixelsOk) {
return false;

View File

@ -632,3 +632,83 @@ DEF_TEST(BitmapReadPixels, reporter) {
}
}
#if SK_SUPPORT_GPU
#include "GrContext.h"
#include "SkGr.h"
#include "SkColorPriv.h"
/** Tests calling copyTo on a texture backed bitmap. Tests that all BGRA_8888/RGBA_8888 combinations
of src and dst work. This test should be removed when SkGrPixelRef is removed. */
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(BitmapCopy_Texture, reporter, ctx) {
static const SkPMColor kData[] = {
0xFF112233, 0xAF224499,
0xEF004466, 0x80773311
};
uint32_t swizData[SK_ARRAY_COUNT(kData)];
for (size_t i = 0; i < SK_ARRAY_COUNT(kData); ++i) {
swizData[i] = SkSwizzle_RB(kData[i]);
}
static const GrPixelConfig kSrcConfigs[] = {
kRGBA_8888_GrPixelConfig,
kBGRA_8888_GrPixelConfig,
};
for (size_t srcC = 0; srcC < SK_ARRAY_COUNT(kSrcConfigs); ++srcC) {
for (int rt = 0; rt < 2; ++rt) {
GrSurfaceDesc desc;
desc.fConfig = kSrcConfigs[srcC];
desc.fFlags = rt ? kRenderTarget_GrSurfaceFlag : kNone_GrSurfaceFlags;
desc.fWidth = 2;
desc.fHeight = 2;
desc.fOrigin = kTopLeft_GrSurfaceOrigin;
const void* srcData = (kSkia8888_GrPixelConfig == desc.fConfig) ? kData : swizData;
SkAutoTUnref<GrTexture> texture(
ctx->textureProvider()->createTexture(desc, false, srcData, 0));
SkBitmap srcBmp;
GrWrapTextureInBitmap(texture, 2, 2, false, &srcBmp);
if (srcBmp.isNull()) {
ERRORF(reporter, "Could not wrap texture in bitmap.");
continue;
}
static const SkColorType kDstCTs[] = { kRGBA_8888_SkColorType, kBGRA_8888_SkColorType };
for (size_t dCT = 0; dCT < SK_ARRAY_COUNT(kDstCTs); ++dCT) {
SkBitmap dstBmp;
if (!srcBmp.copyTo(&dstBmp, kDstCTs[dCT])) {
ERRORF(reporter, "CopyTo failed.");
}
if (dstBmp.colorType() != kDstCTs[dCT]) {
ERRORF(reporter, "SkBitmap::CopyTo did not respect passed in color type.");
}
SkAutoLockPixels alp(dstBmp);
uint8_t* dstBmpPixels = static_cast<uint8_t*>(dstBmp.getPixels());
const uint32_t* refData;
#if defined(SK_PMCOLOR_IS_RGBA)
refData = (kRGBA_8888_SkColorType == dstBmp.colorType()) ? kData : swizData;
#elif defined(SK_PMCOLOR_IS_BGRA)
refData = (kBGRA_8888_SkColorType == dstBmp.colorType()) ? kData : swizData;
#else
#error "PM Color must be BGRA or RGBA to use GPU backend."
#endif
bool foundError = false;
for (int y = 0; y < 2 && !foundError; ++y) {
uint32_t* dstBmpRow = reinterpret_cast<uint32_t*>(dstBmpPixels);
for (int x = 0; x < 2 && !foundError; ++x) {
if (refData[2 * y + x] != dstBmpRow[x]) {
ERRORF(reporter, "Expected pixel 0x%08x, found 0x%08x.",
refData[2 * y + x], dstBmpRow[x]);
foundError = true;
}
}
dstBmpPixels += dstBmp.rowBytes();
}
}
}
}
}
#endif