diff options
| -rw-r--r-- | include/core/SkPixelRef.h | 17 | ||||
| -rw-r--r-- | include/gpu/GrContext.h | 8 | ||||
| -rw-r--r-- | include/gpu/SkGrPixelRef.h | 2 | ||||
| -rw-r--r-- | src/core/SkBitmap.cpp | 137 | ||||
| -rw-r--r-- | src/gpu/GrContext.cpp | 17 | ||||
| -rw-r--r-- | src/gpu/SkGrPixelRef.cpp | 26 | ||||
| -rw-r--r-- | tests/GpuBitmapCopyTest.cpp | 162 |
7 files changed, 296 insertions, 73 deletions
diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h index 74d32ac9ac..a2427abfac 100644 --- a/include/core/SkPixelRef.h +++ b/include/core/SkPixelRef.h @@ -132,11 +132,18 @@ public: bool readPixels(SkBitmap* dst, const SkIRect* subset = NULL); - /** Makes a deep copy of this PixelRef, respecting the requested config. - Returns NULL if either there is an error (e.g. the destination could - not be created with the given config), or this PixelRef does not - support deep copies. */ - virtual SkPixelRef* deepCopy(SkBitmap::Config config) { return NULL; } + /** + * Makes a deep copy of this PixelRef, respecting the requested config. + * @param config Desired config. + * @param subset Subset of this PixelRef to copy. Must be fully contained within the bounds of + * of this PixelRef. + * @return A new SkPixelRef, or NULL if either there is an error (e.g. the destination could + * not be created with the given config), or this PixelRef does not support deep + * copies. + */ + virtual SkPixelRef* deepCopy(SkBitmap::Config config, const SkIRect* subset = NULL) { + return NULL; + } #ifdef SK_BUILD_FOR_ANDROID /** diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h index 7b14fdfafe..9bd6f7df2d 100644 --- a/include/gpu/GrContext.h +++ b/include/gpu/GrContext.h @@ -591,11 +591,15 @@ public: /** - * Copies all texels from one texture to another. + * Copies a rectangle of texels from src to dst. The size of dst is the size of the rectangle + * copied and topLeft is the position of the rect in src. The rectangle is clipped to src's + * bounds. * @param src the texture to copy from. * @param dst the render target to copy to. + * @param topLeft the point in src that will be copied to the top-left of dst. If NULL, + * (0, 0) will be used. */ - void copyTexture(GrTexture* src, GrRenderTarget* dst); + void copyTexture(GrTexture* src, GrRenderTarget* dst, const SkIPoint* topLeft = NULL); /** * Resolves a render target that has MSAA. The intermediate MSAA buffer is diff --git a/include/gpu/SkGrPixelRef.h b/include/gpu/SkGrPixelRef.h index 4476a84a6f..0a32f21064 100644 --- a/include/gpu/SkGrPixelRef.h +++ b/include/gpu/SkGrPixelRef.h @@ -58,7 +58,7 @@ public: protected: // overrides from SkPixelRef virtual bool onReadPixels(SkBitmap* dst, const SkIRect* subset) SK_OVERRIDE; - virtual SkPixelRef* deepCopy(SkBitmap::Config dstConfig) SK_OVERRIDE; + virtual SkPixelRef* deepCopy(SkBitmap::Config dstConfig, const SkIRect* subset) SK_OVERRIDE; private: GrSurface* fSurface; diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 9d51f9b1cc..5554f46ce6 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -828,10 +828,18 @@ void SkBitmap::eraseARGB(U8CPU a, U8CPU r, U8CPU g, U8CPU b) const { #define SUB_OFFSET_FAILURE ((size_t)-1) -static size_t getSubOffset(const SkBitmap& bm, int x, int y) { - SkASSERT((unsigned)x < (unsigned)bm.width()); - SkASSERT((unsigned)y < (unsigned)bm.height()); - +// Declare these non-static so they can be tested by GpuBitmapCopyTest. +size_t getSubOffset(const SkBitmap& bm, int x, int y); +bool getUpperLeftFromOffset(const SkBitmap& bm, int* x, int* y); + +/** + * Based on the Config and rowBytes() of bm, return the offset into an SkPixelRef of the pixel at + * (x, y). + * Note that the SkPixelRef does not need to be set yet. deepCopyTo takes advantage of this fact. + * Also note that (x, y) may be outside the range of (0 - width(), 0 - height()), so long as it is + * within the bounds of the SkPixelRef being used. + */ +size_t getSubOffset(const SkBitmap& bm, int x, int y) { switch (bm.getConfig()) { case SkBitmap::kA8_Config: case SkBitmap:: kIndex8_Config: @@ -855,6 +863,49 @@ static size_t getSubOffset(const SkBitmap& bm, int x, int y) { return y * bm.rowBytes() + x; } +/** + * Using the pixelRefOffset(), rowBytes(), and Config of bm, determine the (x, y) coordinate of the + * upper left corner of bm relative to its SkPixelRef. + * x and y must be non-NULL. + */ +bool getUpperLeftFromOffset(const SkBitmap& bm, int* x, int* y) { + SkASSERT(x != NULL && y != NULL); + const size_t offset = bm.pixelRefOffset(); + if (0 == offset) { + *x = *y = 0; + return true; + } + // Use integer division to find the correct y position. + *y = offset / bm.rowBytes(); + // The remainder will be the x position, after we reverse getSubOffset. + *x = offset % bm.rowBytes(); + switch (bm.getConfig()) { + case SkBitmap::kA8_Config: + // Fall through. + case SkBitmap::kIndex8_Config: + // x is unmodified + break; + + case SkBitmap::kRGB_565_Config: + // Fall through. + case SkBitmap::kARGB_4444_Config: + *x >>= 1; + break; + + case SkBitmap::kARGB_8888_Config: + *x >>= 2; + break; + + case SkBitmap::kNo_Config: + // Fall through. + case SkBitmap::kA1_Config: + // Fall through. + default: + return false; + } + return true; +} + bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const { SkDEBUGCODE(this->validate();) @@ -868,6 +919,21 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const { return false; // r is empty (i.e. no intersection) } + if (fPixelRef->getTexture() != NULL) { + // Do a deep copy + SkPixelRef* pixelRef = fPixelRef->deepCopy(this->config(), &subset); + if (pixelRef != NULL) { + SkBitmap dst; + dst.setConfig(this->config(), subset.width(), subset.height()); + dst.setIsVolatile(this->isVolatile()); + dst.setIsOpaque(this->isOpaque()); + dst.setPixelRef(pixelRef)->unref(); + SkDEBUGCODE(dst.validate()); + result->swap(dst); + return true; + } + } + if (kRLE_Index8_Config == fConfig) { SkAutoLockPixels alp(*this); // don't call readyToDraw(), since we can operate w/o a colortable @@ -896,6 +962,11 @@ bool SkBitmap::extractSubset(SkBitmap* result, const SkIRect& subset) const { return true; } + // If the upper left of the rectangle was outside the bounds of this SkBitmap, we should have + // exited above. + SkASSERT(static_cast<unsigned>(r.fLeft) < static_cast<unsigned>(this->width())); + SkASSERT(static_cast<unsigned>(r.fTop) < static_cast<unsigned>(this->height())); + size_t offset = getSubOffset(*this, r.fLeft, r.fTop); if (SUB_OFFSET_FAILURE == offset) { return false; // config not supported @@ -961,21 +1032,28 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const { SkBitmap tmpSrc; const SkBitmap* src = this; - if (fPixelRef && fPixelRef->readPixels(&tmpSrc)) { - SkASSERT(tmpSrc.width() == this->width()); - SkASSERT(tmpSrc.height() == this->height()); + if (fPixelRef) { + SkIRect subset; + if (getUpperLeftFromOffset(*this, &subset.fLeft, &subset.fTop)) { + subset.fRight = subset.fLeft + fWidth; + subset.fBottom = subset.fTop + fHeight; + if (fPixelRef->readPixels(&tmpSrc, &subset)) { + SkASSERT(tmpSrc.width() == this->width()); + SkASSERT(tmpSrc.height() == this->height()); + + // did we get lucky and we can just return tmpSrc? + if (tmpSrc.config() == dstConfig && NULL == alloc) { + dst->swap(tmpSrc); + if (dst->pixelRef() && this->config() == dstConfig) { + dst->pixelRef()->fGenerationID = fPixelRef->getGenerationID(); + } + return true; + } - // did we get lucky and we can just return tmpSrc? - if (tmpSrc.config() == dstConfig && NULL == alloc) { - dst->swap(tmpSrc); - if (dst->pixelRef()) { - dst->pixelRef()->fGenerationID = fPixelRef->getGenerationID(); + // fall through to the raster case + src = &tmpSrc; } - return true; } - - // fall through to the raster case - src = &tmpSrc; } // we lock this now, since we may need its colortable @@ -1049,11 +1127,34 @@ bool SkBitmap::deepCopyTo(SkBitmap* dst, Config dstConfig) const { if (fPixelRef) { SkPixelRef* pixelRef = fPixelRef->deepCopy(dstConfig); if (pixelRef) { + uint32_t rowBytes; if (dstConfig == fConfig) { pixelRef->fGenerationID = fPixelRef->getGenerationID(); + // Use the same rowBytes as the original. + rowBytes = fRowBytes; + } else { + // With the new config, an appropriate fRowBytes will be computed by setConfig. + rowBytes = 0; + } + dst->setConfig(dstConfig, fWidth, fHeight, rowBytes); + + size_t pixelRefOffset; + if (0 == fPixelRefOffset || dstConfig == fConfig) { + // Use the same offset as the original. + pixelRefOffset = fPixelRefOffset; + } else { + // Find the correct offset in the new config. This needs to be done after calling + // setConfig so dst's fConfig and fRowBytes have been set properly. + int x, y; + if (!getUpperLeftFromOffset(*this, &x, &y)) { + return false; + } + pixelRefOffset = getSubOffset(*dst, x, y); + if (SUB_OFFSET_FAILURE == pixelRefOffset) { + return false; + } } - dst->setConfig(dstConfig, fWidth, fHeight); - dst->setPixelRef(pixelRef)->unref(); + dst->setPixelRef(pixelRef, pixelRefOffset)->unref(); return true; } } diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 9c209b3a62..d96a16e9aa 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -1437,7 +1437,7 @@ void GrContext::resolveRenderTarget(GrRenderTarget* target) { fGpu->resolveRenderTarget(target); } -void GrContext::copyTexture(GrTexture* src, GrRenderTarget* dst) { +void GrContext::copyTexture(GrTexture* src, GrRenderTarget* dst, const SkIPoint* topLeft) { if (NULL == src || NULL == dst) { return; } @@ -1454,11 +1454,18 @@ void GrContext::copyTexture(GrTexture* src, GrRenderTarget* dst) { drawState->setRenderTarget(dst); SkMatrix sampleM; sampleM.setIDiv(src->width(), src->height()); + SkIRect srcRect = SkIRect::MakeWH(dst->width(), dst->height()); + if (NULL != topLeft) { + srcRect.offset(*topLeft); + } + SkIRect srcBounds = SkIRect::MakeWH(src->width(), src->height()); + if (!srcRect.intersect(srcBounds)) { + return; + } + sampleM.preTranslate(SkIntToScalar(srcRect.fLeft), SkIntToScalar(srcRect.fTop)); drawState->createTextureEffect(0, src, sampleM); - SkRect rect = SkRect::MakeXYWH(0, 0, - SK_Scalar1 * src->width(), - SK_Scalar1 * src->height()); - fGpu->drawSimpleRect(rect, NULL); + SkRect dstR = SkRect::MakeWH(SkIntToScalar(srcRect.width()), SkIntToScalar(srcRect.height())); + fGpu->drawSimpleRect(dstR, NULL); } void GrContext::writeRenderTargetPixels(GrRenderTarget* target, diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp index e770ef09b6..255437e3a2 100644 --- a/src/gpu/SkGrPixelRef.cpp +++ b/src/gpu/SkGrPixelRef.cpp @@ -48,8 +48,8 @@ bool SkROLockPixelsPixelRef::onLockPixelsAreWritable() const { /////////////////////////////////////////////////////////////////////////////// -static SkGrPixelRef* copyToTexturePixelRef(GrTexture* texture, - SkBitmap::Config dstConfig) { +static SkGrPixelRef* copyToTexturePixelRef(GrTexture* texture, SkBitmap::Config dstConfig, + const SkIRect* subset) { if (NULL == texture) { return NULL; } @@ -59,8 +59,20 @@ static SkGrPixelRef* copyToTexturePixelRef(GrTexture* texture, } GrTextureDesc desc; - desc.fWidth = texture->width(); - desc.fHeight = texture->height(); + SkIPoint pointStorage; + SkIPoint* topLeft; + if (subset != NULL) { + SkASSERT(SkIRect::MakeWH(texture->width(), texture->height()).contains(*subset)); + // Create a new texture that is the size of subset. + desc.fWidth = subset->width(); + desc.fHeight = subset->height(); + pointStorage.set(subset->x(), subset->y()); + topLeft = &pointStorage; + } else { + desc.fWidth = texture->width(); + desc.fHeight = texture->height(); + topLeft = NULL; + } desc.fFlags = kRenderTarget_GrTextureFlagBit | kNoStencil_GrTextureFlagBit; desc.fConfig = SkBitmapConfig2GrPixelConfig(dstConfig); @@ -69,7 +81,7 @@ static SkGrPixelRef* copyToTexturePixelRef(GrTexture* texture, return NULL; } - context->copyTexture(texture, dst->asRenderTarget()); + context->copyTexture(texture, dst->asRenderTarget(), topLeft); // TODO: figure out if this is responsible for Chrome canvas errors #if 0 @@ -123,7 +135,7 @@ SkGpuTexture* SkGrPixelRef::getTexture() { return NULL; } -SkPixelRef* SkGrPixelRef::deepCopy(SkBitmap::Config dstConfig) { +SkPixelRef* SkGrPixelRef::deepCopy(SkBitmap::Config dstConfig, const SkIRect* subset) { if (NULL == fSurface) { return NULL; } @@ -134,7 +146,7 @@ SkPixelRef* SkGrPixelRef::deepCopy(SkBitmap::Config dstConfig) { // a GrTexture owned elsewhere (e.g., SkGpuDevice), and cannot live // independently of that texture. Texture-backed pixel refs, on the other // hand, own their GrTextures, and are thus self-contained. - return copyToTexturePixelRef(fSurface->asTexture(), dstConfig); + return copyToTexturePixelRef(fSurface->asTexture(), dstConfig, subset); } bool SkGrPixelRef::onReadPixels(SkBitmap* dst, const SkIRect* subset) { diff --git a/tests/GpuBitmapCopyTest.cpp b/tests/GpuBitmapCopyTest.cpp index 03890480c2..3e3e533746 100644 --- a/tests/GpuBitmapCopyTest.cpp +++ b/tests/GpuBitmapCopyTest.cpp @@ -10,7 +10,10 @@ #include "GrContext.h" #include "SkBitmap.h" +#include "SkCanvas.h" +#include "SkColor.h" #include "SkGpuDevice.h" +#include "SkPaint.h" #include "SkPixelRef.h" #include "SkRect.h" #include "Test.h" @@ -29,6 +32,89 @@ struct Pair { const char* fValid; }; +extern bool getUpperLeftFromOffset(const SkBitmap& bm, int* x, int* y); +extern size_t getSubOffset(const SkBitmap& bm, int x, int y); + +/** + * Tests that getUpperLeftFromOffset and getSubOffset agree with each other. + */ +static void TestSubsetHelpers(skiatest::Reporter* reporter, const SkBitmap& bitmap){ + int x, y; + bool upperLeft = getUpperLeftFromOffset(bitmap, &x, &y); + REPORTER_ASSERT(reporter, upperLeft); + REPORTER_ASSERT(reporter, getSubOffset(bitmap, x, y) == bitmap.pixelRefOffset()); +} + +/** + * Check to ensure that copying a GPU-backed SkBitmap behaved as expected. + * @param reporter Used to report failures. + * @param desiredConfig Config being copied to. If the copy succeeded, dst must have this Config. + * @param success True if the copy succeeded. + * @param src A GPU-backed SkBitmap that had copyTo or deepCopyTo called on it. + * @param dst SkBitmap that was copied to. + * @param deepCopy True if deepCopyTo was used; false if copyTo was used. + * @param subset Portion of src's SkPixelRef that src represents. dst should represent the same + * portion after the copy. Pass NULL for all pixels. + */ +static void TestIndividualCopy(skiatest::Reporter* reporter, const SkBitmap::Config desiredConfig, + const bool success, const SkBitmap& src, const SkBitmap& dst, + const bool deepCopy = true, const SkIRect* subset = NULL) { + if (success) { + REPORTER_ASSERT(reporter, src.width() == dst.width()); + REPORTER_ASSERT(reporter, src.height() == dst.height()); + REPORTER_ASSERT(reporter, dst.config() == desiredConfig); + if (src.config() == dst.config()) { + // FIXME: When calling copyTo (so deepCopy is false here), sometimes we copy the pixels + // exactly, in which case the IDs should be the same, but sometimes we do a bitmap draw, + // in which case the IDs should not be the same. Is there any way to determine which is + // the case at this point? + if (deepCopy) { + REPORTER_ASSERT(reporter, src.getGenerationID() == dst.getGenerationID()); + } + REPORTER_ASSERT(reporter, src.pixelRef() != NULL && dst.pixelRef() != NULL); + + // Do read backs and make sure that the two are the same. + SkBitmap srcReadBack, dstReadBack; + { + SkASSERT(src.getTexture() != NULL); + bool readBack = src.pixelRef()->readPixels(&srcReadBack, subset); + REPORTER_ASSERT(reporter, readBack); + } + if (dst.getTexture() != NULL) { + bool readBack = dst.pixelRef()->readPixels(&dstReadBack, subset); + REPORTER_ASSERT(reporter, readBack); + } else { + // If dst is not a texture, do a copy instead, to the same config as srcReadBack. + bool copy = dst.copyTo(&dstReadBack, srcReadBack.config()); + REPORTER_ASSERT(reporter, copy); + } + + SkAutoLockPixels srcLock(srcReadBack); + SkAutoLockPixels dstLock(dstReadBack); + REPORTER_ASSERT(reporter, srcReadBack.readyToDraw() && dstReadBack.readyToDraw()); + + const char* srcP = static_cast<const char*>(srcReadBack.getAddr(0, 0)); + const char* dstP = static_cast<const char*>(dstReadBack.getAddr(0, 0)); + REPORTER_ASSERT(reporter, srcP != dstP); + + REPORTER_ASSERT(reporter, !memcmp(srcP, dstP, srcReadBack.getSize())); + } else { + REPORTER_ASSERT(reporter, src.getGenerationID() != dst.getGenerationID()); + } + + // If the copy used a subset, test the pixel offset calculation functions. + if (subset != NULL) { + TestSubsetHelpers(reporter, dst); + } + } else { + // dst should be unchanged from its initial state + REPORTER_ASSERT(reporter, dst.config() == SkBitmap::kNo_Config); + REPORTER_ASSERT(reporter, dst.width() == 0); + REPORTER_ASSERT(reporter, dst.height() == 0); + } + +} + // Stripped down version of TestBitmapCopy that checks basic fields (width, height, config, genID) // to ensure that they were copied properly. static void TestGpuBitmapCopy(skiatest::Reporter* reporter, GrContext* grContext) { @@ -45,14 +131,30 @@ static void TestGpuBitmapCopy(skiatest::Reporter* reporter, GrContext* grContext const int H = 33; for (size_t i = 0; i < SK_ARRAY_COUNT(gPairs); i++) { - for (size_t j = 0; j < SK_ARRAY_COUNT(gPairs); j++) { - SkBitmap src, dst; + SkBitmap src, dst; + + SkGpuDevice* device = SkNEW_ARGS(SkGpuDevice, (grContext, gPairs[i].fConfig, W, H)); + SkAutoUnref aur(device); + src = device->accessBitmap(false); + device->clear(SK_ColorWHITE); - SkGpuDevice* device = SkNEW_ARGS(SkGpuDevice, (grContext, gPairs[i].fConfig, W, H)); - SkAutoUnref aur(device); - src = device->accessBitmap(false); - device->clear(SK_ColorWHITE); + // Draw something different to the same portion of the bitmap that we will extract as a + // subset, so that comparing the pixels of the subset will be meaningful. + SkIRect subsetRect = SkIRect::MakeLTRB(W/2, H/2, W, H); + SkCanvas drawingCanvas(device); + SkPaint paint; + paint.setColor(SK_ColorRED); + drawingCanvas.drawRect(SkRect::MakeFromIRect(subsetRect), paint); + // Extract a subset. If this succeeds we will test copying the subset. + SkBitmap subset; + const bool extracted = src.extractSubset(&subset, subsetRect); + if (extracted) { + TestSubsetHelpers(reporter, subset); + } + + for (size_t j = 0; j < SK_ARRAY_COUNT(gPairs); j++) { + dst.reset(); bool success = src.deepCopyTo(&dst, gPairs[j].fConfig); bool expected = gPairs[i].fValid[j] != '0'; if (success != expected) { @@ -73,37 +175,27 @@ static void TestGpuBitmapCopy(skiatest::Reporter* reporter, GrContext* grContext reporter->reportFailed(str); } - if (success) { - REPORTER_ASSERT(reporter, src.width() == dst.width()); - REPORTER_ASSERT(reporter, src.height() == dst.height()); - REPORTER_ASSERT(reporter, dst.config() == gPairs[j].fConfig); - if (src.config() == dst.config()) { - REPORTER_ASSERT(reporter, src.getGenerationID() == dst.getGenerationID()); - // Do read backs and make sure that the two are the same. - SkBitmap srcReadBack, dstReadBack; - REPORTER_ASSERT(reporter, src.pixelRef() != NULL - && dst.pixelRef() != NULL); - src.pixelRef()->readPixels(&srcReadBack); - dst.pixelRef()->readPixels(&dstReadBack); - SkAutoLockPixels srcLock(srcReadBack); - SkAutoLockPixels dstLock(dstReadBack); - REPORTER_ASSERT(reporter, srcReadBack.readyToDraw() - && dstReadBack.readyToDraw()); - const char* srcP = (const char*)srcReadBack.getAddr(0, 0); - const char* dstP = (const char*)dstReadBack.getAddr(0, 0); - REPORTER_ASSERT(reporter, srcP != dstP); - REPORTER_ASSERT(reporter, !memcmp(srcP, dstP, srcReadBack.getSize())); - } else { - REPORTER_ASSERT(reporter, src.getGenerationID() != dst.getGenerationID()); - } - } else { - // dst should be unchanged from its initial state - REPORTER_ASSERT(reporter, dst.config() == SkBitmap::kNo_Config); - REPORTER_ASSERT(reporter, dst.width() == 0); - REPORTER_ASSERT(reporter, dst.height() == 0); + TestIndividualCopy(reporter, gPairs[j].fConfig, success, src, dst); + + // Test copying the subset bitmap, using both copyTo and deepCopyTo. + if (extracted) { + SkBitmap subsetCopy; + success = subset.copyTo(&subsetCopy, gPairs[j].fConfig); + REPORTER_ASSERT(reporter, success == expected); + REPORTER_ASSERT(reporter, success == canSucceed); + TestIndividualCopy(reporter, gPairs[j].fConfig, success, subset, subsetCopy, false, + &subsetRect); + + // Reset the bitmap so that a failed copyTo will leave it in the expected state. + subsetCopy.reset(); + success = subset.deepCopyTo(&subsetCopy, gPairs[j].fConfig); + REPORTER_ASSERT(reporter, success == expected); + REPORTER_ASSERT(reporter, success == canSucceed); + TestIndividualCopy(reporter, gPairs[j].fConfig, success, subset, subsetCopy, true, + &subsetRect); } } // for (size_t j = ... - } + } // for (size_t i = ... } #include "TestClassDef.h" |