From 1a8ddf0a35bfb6c21a1184f81d2fdd50053acf31 Mon Sep 17 00:00:00 2001 From: "bsalomon@google.com" Date: Wed, 2 Nov 2011 19:34:16 +0000 Subject: Changes the SkCanvas::readPixels API. Allows caller to read into prealloced bitmap pixels. Changes how clipping to device bounds is handled. Review URL: http://codereview.appspot.com/5307077/ git-svn-id: http://skia.googlecode.com/svn/trunk@2584 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gm/gmmain.cpp | 4 +- gyp/tests.gyp | 1 + include/core/SkBitmap.h | 15 +-- include/core/SkCanvas.h | 41 +++++- include/core/SkDevice.h | 31 ++++- include/device/xps/SkXPSDevice.h | 11 +- include/gpu/GrContext.h | 5 +- include/gpu/GrGLDefines.h | 4 +- include/gpu/SkGpuDevice.h | 6 +- include/pdf/SkPDFDevice.h | 11 +- src/core/SkBitmap.cpp | 6 +- src/core/SkCanvas.cpp | 26 ++-- src/core/SkDevice.cpp | 67 ++++++++-- src/gpu/GrContext.cpp | 9 +- src/gpu/GrGpu.cpp | 6 +- src/gpu/GrGpu.h | 6 +- src/gpu/GrGpuGL.cpp | 72 ++++++++--- src/gpu/GrGpuGL.h | 6 +- src/gpu/SkGpuDevice.cpp | 43 ++----- tests/ReadPixelsTest.cpp | 260 +++++++++++++++++++++++++++++++++++++++ 20 files changed, 521 insertions(+), 109 deletions(-) create mode 100644 tests/ReadPixelsTest.cpp diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp index 5269b3405e..f2ab7033fc 100644 --- a/gm/gmmain.cpp +++ b/gm/gmmain.cpp @@ -251,7 +251,9 @@ static bool generate_image(GM* gm, const ConfigData& gRec, // the device is as large as the current rendertarget, so we explicitly // only readback the amount we expect (in size) // overwrite our previous allocation - gc.readPixels(SkIRect::MakeSize(size), bitmap); + bitmap->setConfig(SkBitmap::kARGB_8888_Config, size.fWidth, + size.fHeight); + gc.readPixels(bitmap, 0, 0); } return true; } diff --git a/gyp/tests.gyp b/gyp/tests.gyp index d1904876f0..92aba93a03 100644 --- a/gyp/tests.gyp +++ b/gyp/tests.gyp @@ -47,6 +47,7 @@ '../tests/PDFPrimitivesTest.cpp', '../tests/PointTest.cpp', '../tests/Reader32Test.cpp', + '../tests/ReadPixelsTest.cpp', '../tests/RefDictTest.cpp', '../tests/RegionTest.cpp', '../tests/Sk64Test.cpp', diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h index e98a294bde..ae56739c5f 100644 --- a/include/core/SkBitmap.h +++ b/include/core/SkBitmap.h @@ -225,12 +225,11 @@ public: /** Copies the bitmap's pixels to the location pointed at by dst and returns true if possible, returns false otherwise. - In the event that the bitmap's stride is equal to dstRowBytes, and if - it is greater than strictly required by the bitmap's current config - (this may happen if the bitmap is an extracted subset of another), then - this function will copy bytes past the eand of each row, excluding the - last row. No copies are made outside of the declared size of dst, - however. + In the case when the dstRowBytes matches the bitmap's rowBytes, the copy + may be made faster by copying over the dst's per-row padding (for all + rows but the last). By setting preserveDstPad to true the caller can + disable this optimization and ensure that pixels in the padding are not + overwritten. Always returns false for RLE formats. @@ -239,8 +238,10 @@ public: pixels using indicated stride. @param dstRowBytes Width of each line in the buffer. If -1, uses bitmap's internal stride. + @param preserveDstPad Must we preserve padding in the dst */ - bool copyPixelsTo(void* const dst, size_t dstSize, int dstRowBytes = -1) + bool copyPixelsTo(void* const dst, size_t dstSize, int dstRowBytes = -1, + bool preserveDstPad = false) const; /** Use the standard HeapAllocator to create the pixelref that manages the diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index b2bdafe738..90bc1f433e 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -105,14 +105,45 @@ public: /////////////////////////////////////////////////////////////////////////// + /** + * On success (returns true), copy the canvas pixels into the bitmap. + * On failure, the bitmap parameter is left unchanged and false is + * returned. + * + * If the canvas is backed by a non-raster device (e.g. PDF) then + * readPixels will fail. + * + * If the bitmap has pixels already allocated, the canvas pixels will be + * written there. If not, bitmap->allocPixels() will be called + * automatically. If the bitmap is backed by a texture readPixels will + * fail. + * + * The canvas' pixels are converted to the bitmap's config. The only + * supported config is kARGB_8888_Config, though this may be relaxed in + * future. + * + * The actual pixels written is the intersection of the canvas' bounds, and + * the rectangle formed by the bitmap's width,height and the specified x,y. + * If bitmap pixels extend outside of that intersection, they will not be + * modified. + * + * Example that reads the entire canvas into a bitmap: + * SkISize size = canvas->getDeviceSize(); + * bitmap->setConfig(SkBitmap::kARGB_8888_Config, size.fWidth, + * size.fHeight); + * if (canvas->readPixels(bitmap, 0, 0)) { + * // use the pixels + * } + */ + bool readPixels(SkBitmap* bitmap, int x, int y); + /** - * Copy the pixels from the device into bitmap. Returns true on success. - * If false is returned, then the bitmap parameter is left unchanged. - * The bitmap parameter is treated as output-only, and will be completely - * overwritten (if the method returns true). + * DEPRECATED: This will be removed as soon as webkit is no longer relying + * on it. The bitmap is resized to the intersection of srcRect and the + * canvas bounds. New pixels are always allocated on success. Bitmap is + * unmodified on failure. */ bool readPixels(const SkIRect& srcRect, SkBitmap* bitmap); - bool readPixels(SkBitmap* bitmap); /** * Similar to draw sprite, this method will copy the pixels in bitmap onto diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h index 95b63892ae..5d184e478f 100644 --- a/include/core/SkDevice.h +++ b/include/core/SkDevice.h @@ -108,11 +108,23 @@ public: /** * Copy the pixels from the device into bitmap. Returns true on success. - * If false is returned, then the bitmap parameter is left unchanged. - * The bitmap parameter is treated as output-only, and will be completely - * overwritten (if the method returns true). + * If false is returned, then the bitmap parameter is left unchanged. The + * rectangle read is defined by x, y and the bitmap's width and height. + * + * If the bitmap has pixels allocated the canvas will write directly to + * into that memory (if the call succeeds). + * + * The read is clipped to the device bounds. If bitmap pixels were + * preallocated then pixels outside the clip are left unmodified. If the + * call allocates bitmap pixels then pixels outside the clip will be + * uninitialized. + * + * Currently bitmap must have kARGB_8888_Config or readPixels will fail. + * This will likely be relaxed in the future. + * + * The bitmap parameter is not modified if the call fails. */ - virtual bool readPixels(const SkIRect& srcRect, SkBitmap* bitmap); + bool readPixels(SkBitmap* bitmap, int x, int y); /** * Similar to draw sprite, this method will copy the pixels in bitmap onto @@ -256,6 +268,17 @@ protected: fBitmap.setPixelRef(pr, offset); return pr; } + + /** + * Implements readPixels API. The caller will ensure that: + * 1. bitmap has pixel config kARGB_8888_Config. + * 2. bitmap has pixels. + * 3. The rectangle (x, y, x + bitmap->width(), y + bitmap->height()) is + * contained in the device bounds. + * 4. the bitmap struct is safe to partially overwrite in case of failure + */ + virtual bool onReadPixels(const SkBitmap* bitmap, int x, int y); + /** Called when this device is installed into a Canvas. Balanaced by a call to unlockPixels() when the device is removed from a Canvas. diff --git a/include/device/xps/SkXPSDevice.h b/include/device/xps/SkXPSDevice.h index 1fb9220d15..ed61ced8d0 100644 --- a/include/device/xps/SkXPSDevice.h +++ b/include/device/xps/SkXPSDevice.h @@ -71,11 +71,6 @@ public: return kVector_Capability; } - virtual bool readPixels(const SkIRect& srcRect, - SkBitmap* bitmap) SK_OVERRIDE { - return false; - } - protected: virtual void clear(SkColor color) SK_OVERRIDE; @@ -146,6 +141,12 @@ protected: int x, int y, const SkPaint& paint) SK_OVERRIDE; + virtual bool onReadPixels(const SkBitmap* bitmap, + int x, + int y) SK_OVERRIDE { + return false; + } + private: class TypefaceUse : ::SkNoncopyable { public: diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h index f980702dd6..d61b8bb5a0 100644 --- a/include/gpu/GrContext.h +++ b/include/gpu/GrContext.h @@ -432,6 +432,8 @@ public: * @param height height of rectangle to read in pixels. * @param config the pixel config of the destination buffer * @param buffer memory to read the rectangle into. + * @param rowBytes number of bytes bewtween consecueive rows. Zero + * means rows are tightly packed. * * @return true if the read succeeded, false if not. The read can fail * because of a unsupported pixel config or because no render @@ -439,7 +441,8 @@ public: */ bool readRenderTargetPixels(GrRenderTarget* target, int left, int top, int width, int height, - GrPixelConfig config, void* buffer); + GrPixelConfig config, void* buffer, + size_t rowBytes = 0); /** * Reads a rectangle of pixels from a texture. diff --git a/include/gpu/GrGLDefines.h b/include/gpu/GrGLDefines.h index 7a3d6767fa..5d11b9f27e 100644 --- a/include/gpu/GrGLDefines.h +++ b/include/gpu/GrGLDefines.h @@ -348,7 +348,9 @@ #define GR_GL_EXTENSIONS 0x1F03 /* Pixel Mode / Transfer */ -#define GR_GL_UNPACK_ROW_LENGTH 0x0CF2 +#define GR_GL_UNPACK_ROW_LENGTH 0x0CF2 +#define GR_GL_PACK_ROW_LENGTH 0x0D02 + /* TextureMagFilter */ #define GR_GL_NEAREST 0x2600 diff --git a/include/gpu/SkGpuDevice.h b/include/gpu/SkGpuDevice.h index f5613a7fd8..047fd0740c 100644 --- a/include/gpu/SkGpuDevice.h +++ b/include/gpu/SkGpuDevice.h @@ -68,7 +68,6 @@ public: // overrides from SkDevice virtual void clear(SkColor color); - virtual bool readPixels(const SkIRect& srcRect, SkBitmap* bitmap); virtual void writePixels(const SkBitmap& bitmap, int x, int y); virtual void setMatrixClip(const SkMatrix& matrix, const SkRegion& clip, @@ -140,6 +139,11 @@ protected: TexCache fTex; }; friend class SkAutoTexCache; + + // overrides from SkDevice + virtual bool onReadPixels(const SkBitmap* bitmap, + int x, int y) SK_OVERRIDE; + private: GrContext* fContext; diff --git a/include/pdf/SkPDFDevice.h b/include/pdf/SkPDFDevice.h index 9e985e7702..b25e39a7d0 100644 --- a/include/pdf/SkPDFDevice.h +++ b/include/pdf/SkPDFDevice.h @@ -66,10 +66,6 @@ public: virtual void clear(SkColor color); - virtual bool readPixels(const SkIRect& srcRect, SkBitmap* bitmap) { - return false; - } - /** These are called inside the per-device-layer loop for each draw call. When these are called, we have already applied any saveLayer operations, and are handling any looping from the paint, and any effects from the @@ -160,6 +156,13 @@ public: const SkPDFGlyphSetMap& getFontGlyphUsage() const { return *(fFontGlyphUsage.get()); } + +protected: + virtual bool onReadPixels(const SkBitmap* bitmap, + int x, int y) SK_OVERRIDE { + return false; + } + private: // TODO(vandebo): push most of SkPDFDevice's state into a core object in diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp index 9683654db6..760bab7d8a 100644 --- a/src/core/SkBitmap.cpp +++ b/src/core/SkBitmap.cpp @@ -456,8 +456,8 @@ Sk64 SkBitmap::getSafeSize64() const { return ComputeSafeSize64(getConfig(), fWidth, fHeight, fRowBytes); } -bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize, int dstRowBytes) - const { +bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize, + int dstRowBytes, bool preserveDstPad) const { if (dstRowBytes == -1) dstRowBytes = fRowBytes; @@ -468,7 +468,7 @@ bool SkBitmap::copyPixelsTo(void* const dst, size_t dstSize, int dstRowBytes) dst == NULL || (getPixels() == NULL && pixelRef() == NULL)) return false; - if (static_cast(dstRowBytes) == fRowBytes) { + if (!preserveDstPad && static_cast(dstRowBytes) == fRowBytes) { size_t safeSize = getSafeSize(); if (safeSize > dstSize || safeSize == 0) return false; diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 3ea9a9c36b..da7aeb971a 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -550,24 +550,32 @@ SkDevice* SkCanvas::setBitmapDevice(const SkBitmap& bitmap) { return device; } -bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) { +bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) { SkDevice* device = this->getDevice(); if (!device) { return false; } - return device->readPixels(srcRect, bitmap); + return device->readPixels(bitmap, x, y); } -////////////////////////////////////////////////////////////////////////////// - -bool SkCanvas::readPixels(SkBitmap* bitmap) { +bool SkCanvas::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) { SkDevice* device = this->getDevice(); - if (!device) { + + SkIRect bounds; + bounds.set(0, 0, device->width(), device->height()); + if (!bounds.intersect(srcRect)) { + return false; + } + + SkBitmap tmp; + tmp.setConfig(SkBitmap::kARGB_8888_Config, bounds.width(), + bounds.height()); + if (this->readPixels(&tmp, bounds.fLeft, bounds.fTop)) { + bitmap->swap(tmp); + return true; + } else { return false; } - SkIRect bounds; - bounds.set(0, 0, device->width(), device->height()); - return this->readPixels(bounds, bitmap); } void SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) { diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp index 18087ae04d..f5523bd9db 100644 --- a/src/core/SkDevice.cpp +++ b/src/core/SkDevice.cpp @@ -102,27 +102,70 @@ void SkDevice::setMatrixClip(const SkMatrix& matrix, const SkRegion& region, /////////////////////////////////////////////////////////////////////////////// -bool SkDevice::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) { - const SkBitmap& src = this->accessBitmap(false); - - SkIRect bounds; - bounds.set(0, 0, src.width(), src.height()); - if (!bounds.intersect(srcRect)) { +bool SkDevice::readPixels(SkBitmap* bitmap, int x, int y) { + if (SkBitmap::kARGB_8888_Config != bitmap->config() || + NULL != bitmap->getTexture()) { return false; } - SkBitmap subset; - if (!src.extractSubset(&subset, bounds)) { + const SkBitmap& src = this->accessBitmap(false); + + SkIRect srcRect = SkIRect::MakeXYWH(x, y, bitmap->width(), + bitmap->height()); + SkIRect devbounds = SkIRect::MakeWH(src.width(), src.height()); + if (!srcRect.intersect(devbounds)) { return false; } SkBitmap tmp; - if (!subset.copyTo(&tmp, SkBitmap::kARGB_8888_Config)) { - return false; + SkBitmap* bmp; + if (bitmap->isNull()) { + tmp.setConfig(SkBitmap::kARGB_8888_Config, bitmap->width(), + bitmap->height()); + if (!tmp.allocPixels()) { + return false; + } + bmp = &tmp; + } else { + bmp = bitmap; + } + + SkIRect subrect = srcRect; + subrect.offset(-x, -y); + SkBitmap bmpSubset; + bmp->extractSubset(&bmpSubset, subrect); + + bool result = this->onReadPixels(&bmpSubset, srcRect.fLeft, srcRect.fTop); + if (result && bmp == &tmp) { + tmp.swap(*bitmap); } + return result; +} + +bool SkDevice::onReadPixels(const SkBitmap* bitmap, int x, int y) { + SkASSERT(SkBitmap::kARGB_8888_Config == bitmap->config()); + SkASSERT(!bitmap->isNull()); + SkASSERT(SkIRect::MakeWH(this->width(), this->height()).contains(SkIRect::MakeXYWH(x, y, bitmap->width(), bitmap->height()))); + + SkIRect srcRect = SkIRect::MakeXYWH(x, y, bitmap->width(), + bitmap->height()); + const SkBitmap& src = this->accessBitmap(false); - tmp.swap(*bitmap); - return true; + SkBitmap subset; + if (!src.extractSubset(&subset, srcRect)) { + return false; + } + if (SkBitmap::kARGB_8888_Config != subset.config()) { + // It'd be preferable to do this directly to bitmap. + // We'd need a SkBitmap::copyPixelsTo that takes a config + // or make copyTo lazily allocate. + subset.copyTo(&subset, SkBitmap::kARGB_8888_Config); + } + SkAutoLockPixels alp(*bitmap); + return subset.copyPixelsTo(bitmap->getPixels(), + bitmap->getSize(), + bitmap->rowBytes(), + true); } void SkDevice::writePixels(const SkBitmap& bitmap, int x, int y) { diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 7028c91147..3fc5d7d773 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -1637,15 +1637,16 @@ bool GrContext::readTexturePixels(GrTexture* texture, if (NULL != target) { return fGpu->readPixels(target, left, top, width, height, - config, buffer); + config, buffer, 0); } else { return false; } } bool GrContext::readRenderTargetPixels(GrRenderTarget* target, - int left, int top, int width, int height, - GrPixelConfig config, void* buffer) { + int left, int top, int width, int height, + GrPixelConfig config, void* buffer, + size_t rowBytes) { SK_TRACE_EVENT0("GrContext::readRenderTargetPixels"); uint32_t flushFlags = 0; if (NULL == target) { @@ -1655,7 +1656,7 @@ bool GrContext::readRenderTargetPixels(GrRenderTarget* target, this->flush(flushFlags); return fGpu->readPixels(target, left, top, width, height, - config, buffer); + config, buffer, rowBytes); } void GrContext::writePixels(int left, int top, int width, int height, diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp index f0808d394f..1fc8d47339 100644 --- a/src/gpu/GrGpu.cpp +++ b/src/gpu/GrGpu.cpp @@ -221,10 +221,12 @@ void GrGpu::forceRenderTargetFlush() { bool GrGpu::readPixels(GrRenderTarget* target, int left, int top, int width, int height, - GrPixelConfig config, void* buffer) { + GrPixelConfig config, void* buffer, + size_t rowBytes) { this->handleDirtyContext(); - return this->onReadPixels(target, left, top, width, height, config, buffer); + return this->onReadPixels(target, left, top, width, height, + config, buffer, rowBytes); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h index 9107554cd6..cf29ed7555 100644 --- a/src/gpu/GrGpu.h +++ b/src/gpu/GrGpu.h @@ -180,6 +180,8 @@ public: * @param height height of rectangle to read in pixels. * @param config the pixel config of the destination buffer * @param buffer memory to read the rectangle into. + * @param rowBytes the number of bytes between consecutive rows. Zero + * means rows are tightly packed. * * @return true if the read succeeded, false if not. The read can fail * because of a unsupported pixel config or because no render @@ -187,7 +189,7 @@ public: */ bool readPixels(GrRenderTarget* renderTarget, int left, int top, int width, int height, - GrPixelConfig config, void* buffer); + GrPixelConfig config, void* buffer, size_t rowBytes); const GrGpuStats& getStats() const; void resetStats(); @@ -321,7 +323,7 @@ protected: // overridden by API-specific derived class to perform the read pixels. virtual bool onReadPixels(GrRenderTarget* target, int left, int top, int width, int height, - GrPixelConfig, void* buffer) = 0; + GrPixelConfig, void* buffer, size_t rowBytes) = 0; // called to program the vertex data, indexCount will be 0 if drawing non- // indexed geometry. The subclass may adjust the startVertex and/or diff --git a/src/gpu/GrGpuGL.cpp b/src/gpu/GrGpuGL.cpp index 78655929f3..dc4d78ac43 100644 --- a/src/gpu/GrGpuGL.cpp +++ b/src/gpu/GrGpuGL.cpp @@ -1384,14 +1384,18 @@ void GrGpuGL::onForceRenderTargetFlush() { } bool GrGpuGL::onReadPixels(GrRenderTarget* target, - int left, int top, int width, int height, - GrPixelConfig config, void* buffer) { + int left, int top, + int width, int height, + GrPixelConfig config, + void* buffer, size_t rowBytes) { GrGLenum internalFormat; // we don't use this for glReadPixels GrGLenum format; GrGLenum type; if (!this->canBeTexture(config, &internalFormat, &format, &type)) { return false; - } + } + + // resolve the render target if necessary GrGLRenderTarget* tgt = static_cast(target); GrAutoTPtrValueRestore autoTargetRestore; switch (tgt->getResolveType()) { @@ -1417,26 +1421,62 @@ bool GrGpuGL::onReadPixels(GrRenderTarget* target, // the read rect is viewport-relative GrGLIRect readRect; readRect.setRelativeTo(glvp, left, top, width, height); + + size_t tightRowBytes = GrBytesPerPixel(config) * width; + if (0 == rowBytes) { + rowBytes = tightRowBytes; + } + size_t readDstRowBytes = tightRowBytes; + void* readDst = buffer; + + // determine if GL can read using the passed rowBytes or if we need + // a scratch buffer. + SkAutoSMalloc<32 * sizeof(GrColor)> scratch; + if (rowBytes != tightRowBytes) { + if (kDesktop_GrGLBinding == this->glBinding()) { + GrAssert(!(rowBytes % sizeof(GrColor))); + GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, rowBytes / sizeof(GrColor))); + readDstRowBytes = rowBytes; + } else { + scratch.reset(tightRowBytes * height); + readDst = scratch.get(); + } + } GL_CALL(ReadPixels(readRect.fLeft, readRect.fBottom, readRect.fWidth, readRect.fHeight, - format, type, buffer)); + format, type, readDst)); + if (readDstRowBytes != tightRowBytes) { + GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, 0)); + } // now reverse the order of the rows, since GL's are bottom-to-top, but our - // API presents top-to-bottom - { - size_t stride = width * GrBytesPerPixel(config); - SkAutoMalloc rowStorage(stride); - void* tmp = rowStorage.get(); - + // API presents top-to-bottom. We must preserve the padding contents. Note + // that the above readPixels did not overwrite the padding. + if (readDst == buffer) { + GrAssert(rowBytes == readDstRowBytes); + scratch.reset(tightRowBytes); + void* tmpRow = scratch.get(); + // flip y in-place by rows const int halfY = height >> 1; char* top = reinterpret_cast(buffer); - char* bottom = top + (height - 1) * stride; + char* bottom = top + (height - 1) * rowBytes; for (int y = 0; y < halfY; y++) { - memcpy(tmp, top, stride); - memcpy(top, bottom, stride); - memcpy(bottom, tmp, stride); - top += stride; - bottom -= stride; + memcpy(tmpRow, top, tightRowBytes); + memcpy(top, bottom, tightRowBytes); + memcpy(bottom, tmpRow, tightRowBytes); + top += rowBytes; + bottom -= rowBytes; + } + } else { + GrAssert(readDst != buffer); + // copy from readDst to buffer while flipping y + const int halfY = height >> 1; + const char* src = reinterpret_cast(readDst); + char* dst = reinterpret_cast(buffer) + (height-1) * rowBytes; + for (int y = 0; y < height; y++) { + memcpy(dst, src, tightRowBytes); + src += readDstRowBytes; + dst -= rowBytes; } } return true; diff --git a/src/gpu/GrGpuGL.h b/src/gpu/GrGpuGL.h index 2d246e8479..51eb4aed02 100644 --- a/src/gpu/GrGpuGL.h +++ b/src/gpu/GrGpuGL.h @@ -90,8 +90,10 @@ protected: virtual void onForceRenderTargetFlush(); virtual bool onReadPixels(GrRenderTarget* target, - int left, int top, int width, int height, - GrPixelConfig, void* buffer); + int left, int top, + int width, int height, + GrPixelConfig, + void* buffer, size_t rowBytes) SK_OVERRIDE; virtual void onGpuDrawIndexed(GrPrimitiveType type, uint32_t startVertex, diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index d75838350e..790cf6d09e 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -256,36 +256,19 @@ void SkGpuDevice::makeRenderTargetCurrent() { /////////////////////////////////////////////////////////////////////////////// -bool SkGpuDevice::readPixels(const SkIRect& srcRect, SkBitmap* bitmap) { - SkIRect bounds; - bounds.set(0, 0, this->width(), this->height()); - if (!bounds.intersect(srcRect)) { - return false; - } - - const int w = bounds.width(); - const int h = bounds.height(); - SkBitmap tmp; - // note we explicitly specify our rowBytes to be snug (no gap between rows) - tmp.setConfig(SkBitmap::kARGB_8888_Config, w, h, w * 4); - if (!tmp.allocPixels()) { - return false; - } - - tmp.lockPixels(); - - bool read = fContext->readRenderTargetPixels(fRenderTarget, - bounds.fLeft, bounds.fTop, - bounds.width(), bounds.height(), - kRGBA_8888_GrPixelConfig, - tmp.getPixels()); - tmp.unlockPixels(); - if (!read) { - return false; - } - - tmp.swap(*bitmap); - return true; +bool SkGpuDevice::onReadPixels(const SkBitmap* bitmap, int x, int y) { + SkASSERT(SkBitmap::kARGB_8888_Config == bitmap->config()); + SkASSERT(!bitmap->isNull()); + SkASSERT(SkIRect::MakeWH(this->width(), this->height()).contains(SkIRect::MakeXYWH(x, y, bitmap->width(), bitmap->height()))); + + SkAutoLockPixels alp(*bitmap); + return fContext->readRenderTargetPixels(fRenderTarget, + x, y, + bitmap->width(), + bitmap->height(), + kRGBA_8888_GrPixelConfig, + bitmap->getPixels(), + bitmap->rowBytes()); } void SkGpuDevice::writePixels(const SkBitmap& bitmap, int x, int y) { diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp new file mode 100644 index 0000000000..a7083bb5a6 --- /dev/null +++ b/tests/ReadPixelsTest.cpp @@ -0,0 +1,260 @@ + +/* + * Copyright 2011 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "Test.h" +#include "SkCanvas.h" +#include "SkRegion.h" +#include "SkGpuDevice.h" + + +static const int DEV_W = 100, DEV_H = 100; +static const SkIRect DEV_RECT = SkIRect::MakeWH(DEV_W, DEV_H); +static const SkRect DEV_RECT_S = SkRect::MakeWH(DEV_W * SK_Scalar1, + DEV_H * SK_Scalar1); + +namespace { +SkPMColor getCanvasColor(int x, int y) { + SkASSERT(x >= 0 && x < DEV_W); + SkASSERT(y >= 0 && y < DEV_H); + return SkPackARGB32(0xff, x, y, 0x0); +} + +SkPMColor getBitmapColor(int x, int y, int w, int h) { + int n = y * w + x; + + U8CPU b = n & 0xff; + U8CPU g = (n >> 8) & 0xff; + U8CPU r = (n >> 16) & 0xff; + return SkPackARGB32(0xff, r, g , b); +} + +void fillCanvas(SkCanvas* canvas) { + static SkBitmap bmp; + if (bmp.isNull()) { + bmp.setConfig(SkBitmap::kARGB_8888_Config, DEV_W, DEV_H); + bool alloc = bmp.allocPixels(); + SkASSERT(alloc); + SkAutoLockPixels alp(bmp); + intptr_t pixels = reinterpret_cast(bmp.getPixels()); + for (int y = 0; y < DEV_H; ++y) { + for (int x = 0; x < DEV_W; ++x) { + SkPMColor* pixel = reinterpret_cast(pixels + y * bmp.rowBytes() + x * bmp.bytesPerPixel()); + *pixel = getCanvasColor(x, y); + } + } + } + canvas->save(); + canvas->setMatrix(SkMatrix::I()); + canvas->clipRect(DEV_RECT_S, SkRegion::kReplace_Op); + SkPaint paint; + paint.setXfermodeMode(SkXfermode::kSrc_Mode); + canvas->drawBitmap(bmp, 0, 0, &paint); + canvas->restore(); +} + +void fillBitmap(SkBitmap* bitmap) { + SkASSERT(bitmap->lockPixelsAreWritable()); + SkAutoLockPixels alp(*bitmap); + int w = bitmap->width(); + int h = bitmap->height(); + intptr_t pixels = reinterpret_cast(bitmap->getPixels()); + for (int y = 0; y < h; ++y) { + for (int x = 0; x < w; ++x) { + SkPMColor* pixel = reinterpret_cast(pixels + y * bitmap->rowBytes() + x * bitmap->bytesPerPixel()); + *pixel = getBitmapColor(x, y, w, h); + } + } +} + +// checks the bitmap contains correct pixels after the readPixels +// if the bitmap was prefilled with pixels it checks that these weren't +// overwritten in the area outside the readPixels. +bool checkRead(skiatest::Reporter* reporter, + const SkBitmap& bitmap, + int x, int y, + bool preFilledBmp) { + SkASSERT(SkBitmap::kARGB_8888_Config == bitmap.config()); + SkASSERT(!bitmap.isNull()); + + int bw = bitmap.width(); + int bh = bitmap.height(); + + SkIRect srcRect = SkIRect::MakeXYWH(x, y, bw, bh); + SkIRect clippedSrcRect = DEV_RECT; + if (!clippedSrcRect.intersect(srcRect)) { + clippedSrcRect.setEmpty(); + } + + SkAutoLockPixels alp(bitmap); + intptr_t pixels = reinterpret_cast(bitmap.getPixels()); + for (int by = 0; by < bh; ++by) { + for (int bx = 0; bx < bw; ++bx) { + int devx = bx + srcRect.fLeft; + int devy = by + srcRect.fTop; + + SkPMColor pixel = *reinterpret_cast(pixels + by * bitmap.rowBytes() + bx * bitmap.bytesPerPixel()); + + if (clippedSrcRect.contains(devx, devy)) { + REPORTER_ASSERT(reporter, getCanvasColor(devx, devy) == pixel); + if (getCanvasColor(devx, devy) != pixel) { + return false; + } + } else if (preFilledBmp) { + REPORTER_ASSERT(reporter, getBitmapColor(bx, by, bw, bh) == pixel); + if (getBitmapColor(bx, by, bw, bh) != pixel) { + return false; + } + } + } + } + return true; +} + +enum BitmapInit { + kFirstBitmapInit = 0, + + kNoPixels_BitmapInit = kFirstBitmapInit, + kTight_BitmapInit, + kRowBytes_BitmapInit, + + kBitmapInitCnt +}; + +BitmapInit nextBMI(BitmapInit bmi) { + int x = bmi; + return static_cast(++x); +} + + +void init_bitmap(SkBitmap* bitmap, const SkIRect& rect, BitmapInit init) { + int w = rect.width(); + int h = rect.height(); + int rowBytes = 0; + bool alloc = true; + switch (init) { + case kNoPixels_BitmapInit: + alloc = false; + case kTight_BitmapInit: + break; + case kRowBytes_BitmapInit: + rowBytes = w * sizeof(SkPMColor) + 16 * sizeof(SkPMColor); + break; + default: + SkASSERT(0); + break; + } + bitmap->setConfig(SkBitmap::kARGB_8888_Config, w, h, rowBytes); + if (alloc) { + bitmap->allocPixels(); + } +} + +void ReadPixelsTest(skiatest::Reporter* reporter, GrContext* context) { + SkCanvas canvas; + + const SkIRect testRects[] = { + // entire thing + DEV_RECT, + // larger on all sides + SkIRect::MakeLTRB(-10, -10, DEV_W + 10, DEV_H + 10), + // fully contained + SkIRect::MakeLTRB(DEV_W / 4, DEV_H / 4, 3 * DEV_W / 4, 3 * DEV_H / 4), + // outside top left + SkIRect::MakeLTRB(-10, -10, -1, -1), + // touching top left corner + SkIRect::MakeLTRB(-10, -10, 0, 0), + // overlapping top left corner + SkIRect::MakeLTRB(-10, -10, DEV_W / 4, DEV_H / 4), + // overlapping top left and top right corners + SkIRect::MakeLTRB(-10, -10, DEV_W + 10, DEV_H / 4), + // touching entire top edge + SkIRect::MakeLTRB(-10, -10, DEV_W + 10, 0), + // overlapping top right corner + SkIRect::MakeLTRB(3 * DEV_W / 4, -10, DEV_W + 10, DEV_H / 4), + // contained in x, overlapping top edge + SkIRect::MakeLTRB(DEV_W / 4, -10, 3 * DEV_W / 4, DEV_H / 4), + // outside top right corner + SkIRect::MakeLTRB(DEV_W + 1, -10, DEV_W + 10, -1), + // touching top right corner + SkIRect::MakeLTRB(DEV_W, -10, DEV_W + 10, 0), + // overlapping top left and bottom left corners + SkIRect::MakeLTRB(-10, -10, DEV_W / 4, DEV_H + 10), + // touching entire left edge + SkIRect::MakeLTRB(-10, -10, 0, DEV_H + 10), + // overlapping bottom left corner + SkIRect::MakeLTRB(-10, 3 * DEV_H / 4, DEV_W / 4, DEV_H + 10), + // contained in y, overlapping left edge + SkIRect::MakeLTRB(-10, DEV_H / 4, DEV_W / 4, 3 * DEV_H / 4), + // outside bottom left corner + SkIRect::MakeLTRB(-10, DEV_H + 1, -1, DEV_H + 10), + // touching bottom left corner + SkIRect::MakeLTRB(-10, DEV_H, 0, DEV_H + 10), + // overlapping bottom left and bottom right corners + SkIRect::MakeLTRB(-10, 3 * DEV_H / 4, DEV_W + 10, DEV_H + 10), + // touching entire left edge + SkIRect::MakeLTRB(0, DEV_H, DEV_W, DEV_H + 10), + // overlapping bottom right corner + SkIRect::MakeLTRB(3 * DEV_W / 4, 3 * DEV_H / 4, DEV_W + 10, DEV_H + 10), + // overlapping top right and bottom right corners + SkIRect::MakeLTRB(3 * DEV_W / 4, -10, DEV_W + 10, DEV_H + 10), + }; + for (int dtype = 0; dtype < 2; ++dtype) { + if (0 == dtype) { + canvas.setDevice(new SkDevice(SkBitmap::kARGB_8888_Config, DEV_W, DEV_H, false))->unref(); + } else { + canvas.setDevice(new SkGpuDevice(context, SkBitmap::kARGB_8888_Config, DEV_W, DEV_H))->unref(); + } + fillCanvas(&canvas); + + for (int rect = 0; rect < SK_ARRAY_COUNT(testRects); ++rect) { + SkBitmap bmp; + for (BitmapInit bmi = kFirstBitmapInit; bmi < kBitmapInitCnt; bmi = nextBMI(bmi)) { + + const SkIRect& srcRect = testRects[rect]; + + init_bitmap(&bmp, srcRect, bmi); + + // if the bitmap has pixels allocated before the readPixels, note + // that and fill them with pattern + bool startsWithPixels = !bmp.isNull(); + if (startsWithPixels) { + fillBitmap(&bmp); + } + + bool success = canvas.readPixels(&bmp, srcRect.fLeft, srcRect.fTop); + + // determine whether we expected the read to succeed. + REPORTER_ASSERT(reporter, success == SkIRect::Intersects(srcRect, DEV_RECT)); + + if (success || startsWithPixels) { + checkRead(reporter, bmp, srcRect.fLeft, srcRect.fTop, startsWithPixels); + } else { + // if we had no pixels beforehand and the readPixels failed then + // our bitmap should still not have any pixels + REPORTER_ASSERT(reporter, bmp.isNull()); + } + + // check the old webkit version of readPixels that clips the bitmap size + SkBitmap wkbmp; + success = canvas.readPixels(srcRect, &wkbmp); + SkIRect clippedRect = DEV_RECT; + if (clippedRect.intersect(srcRect)) { + REPORTER_ASSERT(reporter, success); + checkRead(reporter, wkbmp, clippedRect.fLeft, clippedRect.fTop, false); + } else { + REPORTER_ASSERT(reporter, !success); + } + } + } + } +} +} + +#include "TestClassDef.h" +DEFINE_GPUTESTCLASS("ReadPixels", ReadPixelsTestClass, ReadPixelsTest) + -- cgit v1.2.3