Revert "remove unused SkBitmap::copyPixelsTo"
This reverts commit 0f3fdfacf32261f943dcac5cdfd14475011f40db.
Reason for revert: Blink-headless in Google3 needs an update too.
Original change's description:
> remove unused SkBitmap::copyPixelsTo
>
> Needs https://codereview.chromium.org/2812853002/ to land first
>
> Bug: skia:6465
> Change-Id: I531e33b2848cd995f20844786ed1a8d34d63fb64
> Reviewed-on: https://skia-review.googlesource.com/13171
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Mike Reed <reed@google.com>
>
TBR=reed@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I5e7c4b0d05772e4948cb1dffdcc40e095fbdba41
Reviewed-on: https://skia-review.googlesource.com/13185
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index 0fcfa2b..34da542 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -345,6 +345,27 @@
*/
void setPixels(void* p, SkColorTable* ctable = NULL);
+ /** Copies the bitmap's pixels to the location pointed at by dst and returns
+ true if possible, returns false otherwise.
+
+ 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.
+
+ @param dst Location of destination buffer.
+ @param dstSize Size of destination buffer. Must be large enough to hold
+ pixels using indicated stride.
+ @param dstRowBytes Width of each line in the buffer. If 0, uses
+ bitmap's internal stride.
+ @param preserveDstPad Must we preserve padding in the dst
+ */
+ bool copyPixelsTo(void* const dst, size_t dstSize, size_t dstRowBytes = 0,
+ bool preserveDstPad = false) const;
+
/** Use the standard HeapAllocator to create the pixelref that manages the
pixel memory. It will be sized based on the current ImageInfo.
If this is called multiple times, a new pixelref object will be created
diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 30c06de..9b8ea7f 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -468,6 +468,61 @@
///////////////////////////////////////////////////////////////////////////////
+static bool copy_pixels_to(const SkPixmap& src, void* const dst, size_t dstSize,
+ size_t dstRowBytes, bool preserveDstPad) {
+ const SkImageInfo& info = src.info();
+
+ if (0 == dstRowBytes) {
+ dstRowBytes = src.rowBytes();
+ }
+ if (dstRowBytes < info.minRowBytes()) {
+ return false;
+ }
+
+ if (!preserveDstPad && static_cast<uint32_t>(dstRowBytes) == src.rowBytes()) {
+ size_t safeSize = src.getSafeSize();
+ if (safeSize > dstSize || safeSize == 0)
+ return false;
+ else {
+ // This implementation will write bytes beyond the end of each row,
+ // excluding the last row, if the bitmap's stride is greater than
+ // strictly required by the current config.
+ memcpy(dst, src.addr(), safeSize);
+ return true;
+ }
+ } else {
+ // If destination has different stride than us, then copy line by line.
+ if (info.getSafeSize(dstRowBytes) > dstSize) {
+ return false;
+ } else {
+ // Just copy what we need on each line.
+ size_t rowBytes = info.minRowBytes();
+ const uint8_t* srcP = reinterpret_cast<const uint8_t*>(src.addr());
+ uint8_t* dstP = reinterpret_cast<uint8_t*>(dst);
+ for (int row = 0; row < info.height(); ++row) {
+ memcpy(dstP, srcP, rowBytes);
+ srcP += src.rowBytes();
+ dstP += dstRowBytes;
+ }
+
+ return true;
+ }
+ }
+}
+
+bool SkBitmap::copyPixelsTo(void* dst, size_t dstSize, size_t dstRB, bool preserveDstPad) const {
+ if (nullptr == dst) {
+ return false;
+ }
+ SkAutoPixmapUnlock result;
+ if (!this->requestLock(&result)) {
+ return false;
+ }
+ return copy_pixels_to(result.pixmap(), dst, dstSize, dstRB, preserveDstPad);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
bool SkBitmap::isImmutable() const {
return fPixelRef ? fPixelRef->isImmutable() : false;
}
diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp
index d2fda9e..5eb9a2a9 100644
--- a/tests/BitmapCopyTest.cpp
+++ b/tests/BitmapCopyTest.cpp
@@ -10,6 +10,66 @@
#include "SkTemplates.h"
#include "Test.h"
+static const char* boolStr(bool value) {
+ return value ? "true" : "false";
+}
+
+static const char* color_type_name(SkColorType colorType) {
+ switch (colorType) {
+ case kUnknown_SkColorType:
+ return "None";
+ case kAlpha_8_SkColorType:
+ return "A8";
+ case kRGB_565_SkColorType:
+ return "565";
+ case kARGB_4444_SkColorType:
+ return "4444";
+ case kRGBA_8888_SkColorType:
+ return "RGBA";
+ case kBGRA_8888_SkColorType:
+ return "BGRA";
+ case kIndex_8_SkColorType:
+ return "Index8";
+ case kGray_8_SkColorType:
+ return "Gray8";
+ case kRGBA_F16_SkColorType:
+ return "F16";
+ }
+ return "";
+}
+
+static void report_opaqueness(skiatest::Reporter* reporter, const SkBitmap& src,
+ const SkBitmap& dst) {
+ ERRORF(reporter, "src %s opaque:%d, dst %s opaque:%d",
+ color_type_name(src.colorType()), src.isOpaque(),
+ color_type_name(dst.colorType()), dst.isOpaque());
+}
+
+static bool canHaveAlpha(SkColorType ct) {
+ return kRGB_565_SkColorType != ct;
+}
+
+// copyTo() should preserve isOpaque when it makes sense
+static void test_isOpaque(skiatest::Reporter* reporter,
+ const SkBitmap& srcOpaque, const SkBitmap& srcPremul,
+ SkColorType dstColorType) {
+ SkBitmap dst;
+
+ if (canHaveAlpha(srcPremul.colorType()) && canHaveAlpha(dstColorType)) {
+ REPORTER_ASSERT(reporter, srcPremul.copyTo(&dst, dstColorType));
+ REPORTER_ASSERT(reporter, dst.colorType() == dstColorType);
+ if (srcPremul.isOpaque() != dst.isOpaque()) {
+ report_opaqueness(reporter, srcPremul, dst);
+ }
+ }
+
+ REPORTER_ASSERT(reporter, srcOpaque.copyTo(&dst, dstColorType));
+ REPORTER_ASSERT(reporter, dst.colorType() == dstColorType);
+ if (srcOpaque.isOpaque() != dst.isOpaque()) {
+ report_opaqueness(reporter, srcOpaque, dst);
+ }
+}
+
static void init_src(const SkBitmap& bitmap) {
SkAutoLockPixels lock(bitmap);
if (bitmap.getPixels()) {
@@ -41,6 +101,60 @@
// reportCopyVerification()
// writeCoordPixels()
+// Utility function to read the value of a given pixel in bm. All
+// values converted to uint32_t for simplification of comparisons.
+static uint32_t getPixel(int x, int y, const SkBitmap& bm) {
+ uint32_t val = 0;
+ uint16_t val16;
+ uint8_t val8;
+ SkAutoLockPixels lock(bm);
+ const void* rawAddr = bm.getAddr(x,y);
+
+ switch (bm.bytesPerPixel()) {
+ case 4:
+ memcpy(&val, rawAddr, sizeof(uint32_t));
+ break;
+ case 2:
+ memcpy(&val16, rawAddr, sizeof(uint16_t));
+ val = val16;
+ break;
+ case 1:
+ memcpy(&val8, rawAddr, sizeof(uint8_t));
+ val = val8;
+ break;
+ default:
+ break;
+ }
+ return val;
+}
+
+// Utility function to set value of any pixel in bm.
+// bm.getConfig() specifies what format 'val' must be
+// converted to, but at present uint32_t can handle all formats.
+static void setPixel(int x, int y, uint32_t val, SkBitmap& bm) {
+ uint16_t val16;
+ uint8_t val8;
+ SkAutoLockPixels lock(bm);
+ void* rawAddr = bm.getAddr(x,y);
+
+ switch (bm.bytesPerPixel()) {
+ case 4:
+ memcpy(rawAddr, &val, sizeof(uint32_t));
+ break;
+ case 2:
+ val16 = val & 0xFFFF;
+ memcpy(rawAddr, &val16, sizeof(uint16_t));
+ break;
+ case 1:
+ val8 = val & 0xFF;
+ memcpy(rawAddr, &val8, sizeof(uint8_t));
+ break;
+ default:
+ // Ignore.
+ break;
+ }
+}
+
// Helper struct to contain pixel locations, while avoiding need for STL.
struct Coordinates {
@@ -60,6 +174,31 @@
}
};
+// A function to verify that two bitmaps contain the same pixel values
+// at all coordinates indicated by coords. Simplifies verification of
+// copied bitmaps.
+static void reportCopyVerification(const SkBitmap& bm1, const SkBitmap& bm2,
+ Coordinates& coords,
+ const char* msg,
+ skiatest::Reporter* reporter){
+ // Confirm all pixels in the list match.
+ for (int i = 0; i < coords.length; ++i) {
+ uint32_t p1 = getPixel(coords[i]->fX, coords[i]->fY, bm1);
+ uint32_t p2 = getPixel(coords[i]->fX, coords[i]->fY, bm2);
+// SkDebugf("[%d] (%d %d) p1=%x p2=%x\n", i, coords[i]->fX, coords[i]->fY, p1, p2);
+ if (p1 != p2) {
+ ERRORF(reporter, "%s [colortype = %s]", msg, color_type_name(bm1.colorType()));
+ break;
+ }
+ }
+}
+
+// Writes unique pixel values at locations specified by coords.
+static void writeCoordPixels(SkBitmap& bm, const Coordinates& coords) {
+ for (int i = 0; i < coords.length; ++i)
+ setPixel(coords[i]->fX, coords[i]->fY, i, bm);
+}
+
static const Pair gPairs[] = {
{ kUnknown_SkColorType, "0000000" },
{ kAlpha_8_SkColorType, "0100000" },
@@ -148,6 +287,283 @@
}
}
+DEF_TEST(BitmapCopy, reporter) {
+ static const bool isExtracted[] = {
+ false, true
+ };
+
+ for (size_t i = 0; i < SK_ARRAY_COUNT(gPairs); i++) {
+ SkBitmap srcOpaque, srcPremul;
+ setup_src_bitmaps(&srcOpaque, &srcPremul, gPairs[i].fColorType);
+
+ for (size_t j = 0; j < SK_ARRAY_COUNT(gPairs); j++) {
+ SkBitmap dst;
+
+ bool success = srcPremul.copyTo(&dst, gPairs[j].fColorType);
+ bool expected = gPairs[i].fValid[j] != '0';
+ if (success != expected) {
+ ERRORF(reporter, "SkBitmap::copyTo from %s to %s. expected %s "
+ "returned %s", color_type_name(gPairs[i].fColorType),
+ color_type_name(gPairs[j].fColorType),
+ boolStr(expected), boolStr(success));
+ }
+
+ bool canSucceed = srcPremul.canCopyTo(gPairs[j].fColorType);
+ if (success != canSucceed) {
+ ERRORF(reporter, "SkBitmap::copyTo from %s to %s. returned %s "
+ "canCopyTo %s", color_type_name(gPairs[i].fColorType),
+ color_type_name(gPairs[j].fColorType),
+ boolStr(success), boolStr(canSucceed));
+ }
+
+ if (success) {
+ REPORTER_ASSERT(reporter, srcPremul.width() == dst.width());
+ REPORTER_ASSERT(reporter, srcPremul.height() == dst.height());
+ REPORTER_ASSERT(reporter, dst.colorType() == gPairs[j].fColorType);
+ test_isOpaque(reporter, srcOpaque, srcPremul, dst.colorType());
+ if (srcPremul.colorType() == dst.colorType()) {
+ SkAutoLockPixels srcLock(srcPremul);
+ SkAutoLockPixels dstLock(dst);
+ REPORTER_ASSERT(reporter, srcPremul.readyToDraw());
+ REPORTER_ASSERT(reporter, dst.readyToDraw());
+ const char* srcP = (const char*)srcPremul.getAddr(0, 0);
+ const char* dstP = (const char*)dst.getAddr(0, 0);
+ REPORTER_ASSERT(reporter, srcP != dstP);
+ REPORTER_ASSERT(reporter, !memcmp(srcP, dstP,
+ srcPremul.getSize()));
+ REPORTER_ASSERT(reporter, srcPremul.getGenerationID() == dst.getGenerationID());
+ } else {
+ REPORTER_ASSERT(reporter, srcPremul.getGenerationID() != dst.getGenerationID());
+ }
+ } else {
+ // dst should be unchanged from its initial state
+ REPORTER_ASSERT(reporter, dst.colorType() == kUnknown_SkColorType);
+ REPORTER_ASSERT(reporter, dst.width() == 0);
+ REPORTER_ASSERT(reporter, dst.height() == 0);
+ }
+ } // for (size_t j = ...
+
+ // Tests for getSafeSize(), getSafeSize64(), copyPixelsTo(),
+ // copyPixelsFrom().
+ //
+ for (size_t copyCase = 0; copyCase < SK_ARRAY_COUNT(isExtracted);
+ ++copyCase) {
+ // Test copying to/from external buffer.
+ // Note: the tests below have hard-coded values ---
+ // Please take care if modifying.
+
+ // Tests for getSafeSize64().
+ // Test with a very large configuration without pixel buffer
+ // attached.
+ SkBitmap tstSafeSize;
+ tstSafeSize.setInfo(SkImageInfo::Make(100000000U, 100000000U,
+ gPairs[i].fColorType, kPremul_SkAlphaType));
+ int64_t safeSize = tstSafeSize.computeSafeSize64();
+ if (safeSize < 0) {
+ ERRORF(reporter, "getSafeSize64() negative: %s",
+ color_type_name(tstSafeSize.colorType()));
+ }
+ bool sizeFail = false;
+ // Compare against hand-computed values.
+ switch (gPairs[i].fColorType) {
+ case kUnknown_SkColorType:
+ break;
+
+ case kAlpha_8_SkColorType:
+ case kIndex_8_SkColorType:
+ if (safeSize != 0x2386F26FC10000LL) {
+ sizeFail = true;
+ }
+ break;
+
+ case kRGB_565_SkColorType:
+ case kARGB_4444_SkColorType:
+ if (safeSize != 0x470DE4DF820000LL) {
+ sizeFail = true;
+ }
+ break;
+
+ case kN32_SkColorType:
+ if (safeSize != 0x8E1BC9BF040000LL) {
+ sizeFail = true;
+ }
+ break;
+
+ default:
+ break;
+ }
+ if (sizeFail) {
+ ERRORF(reporter, "computeSafeSize64() wrong size: %s",
+ color_type_name(tstSafeSize.colorType()));
+ }
+
+ int subW = 2;
+ int subH = 2;
+
+ // Create bitmap to act as source for copies and subsets.
+ SkBitmap src, subset;
+ sk_sp<SkColorTable> ct;
+ if (kIndex_8_SkColorType == src.colorType()) {
+ ct = init_ctable();
+ }
+
+ int localSubW;
+ if (isExtracted[copyCase]) { // A larger image to extract from.
+ localSubW = 2 * subW + 1;
+ } else { // Tests expect a 2x2 bitmap, so make smaller.
+ localSubW = subW;
+ }
+ // could fail if we pass kIndex_8 for the colortype
+ if (src.tryAllocPixels(SkImageInfo::Make(localSubW, subH, gPairs[i].fColorType,
+ kPremul_SkAlphaType))) {
+ // failure is fine, as we will notice later on
+ }
+
+ // Either copy src or extract into 'subset', which is used
+ // for subsequent calls to copyPixelsTo/From.
+ bool srcReady = false;
+ // Test relies on older behavior that extractSubset will fail on
+ // kUnknown_SkColorType
+ if (kUnknown_SkColorType != src.colorType() &&
+ isExtracted[copyCase]) {
+ // The extractedSubset() test case allows us to test copy-
+ // ing when src and dst mave possibly different strides.
+ SkIRect r;
+ r.set(1, 0, 1 + subW, subH); // 2x2 extracted bitmap
+
+ srcReady = src.extractSubset(&subset, r);
+ } else {
+ srcReady = src.copyTo(&subset);
+ }
+
+ // Not all configurations will generate a valid 'subset'.
+ if (srcReady) {
+
+ // Allocate our target buffer 'buf' for all copies.
+ // To simplify verifying correctness of copies attach
+ // buf to a SkBitmap, but copies are done using the
+ // raw buffer pointer.
+ const size_t bufSize = subH *
+ SkColorTypeMinRowBytes(src.colorType(), subW) * 2;
+ SkAutoTMalloc<uint8_t> autoBuf (bufSize);
+ uint8_t* buf = autoBuf.get();
+
+ SkBitmap bufBm; // Attach buf to this bitmap.
+ bool successExpected;
+
+ // Set up values for each pixel being copied.
+ Coordinates coords(subW * subH);
+ for (int x = 0; x < subW; ++x)
+ for (int y = 0; y < subH; ++y)
+ {
+ int index = y * subW + x;
+ SkASSERT(index < coords.length);
+ coords[index]->fX = x;
+ coords[index]->fY = y;
+ }
+
+ writeCoordPixels(subset, coords);
+
+ // Test #1 ////////////////////////////////////////////
+
+ const SkImageInfo info = SkImageInfo::Make(subW, subH,
+ gPairs[i].fColorType,
+ kPremul_SkAlphaType);
+ // Before/after comparisons easier if we attach buf
+ // to an appropriately configured SkBitmap.
+ memset(buf, 0xFF, bufSize);
+ // Config with stride greater than src but that fits in buf.
+ bufBm.installPixels(info, buf, info.minRowBytes() * 2);
+ successExpected = false;
+ // Then attempt to copy with a stride that is too large
+ // to fit in the buffer.
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsTo(buf, bufSize, bufBm.rowBytes() * 3)
+ == successExpected);
+
+ if (successExpected)
+ reportCopyVerification(subset, bufBm, coords,
+ "copyPixelsTo(buf, bufSize, 1.5*maxRowBytes)",
+ reporter);
+
+ // Test #2 ////////////////////////////////////////////
+ // This test should always succeed, but in the case
+ // of extracted bitmaps only because we handle the
+ // issue of getSafeSize(). Without getSafeSize()
+ // buffer overrun/read would occur.
+ memset(buf, 0xFF, bufSize);
+ bufBm.installPixels(info, buf, subset.rowBytes());
+ successExpected = subset.getSafeSize() <= bufSize;
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsTo(buf, bufSize) ==
+ successExpected);
+ if (successExpected)
+ reportCopyVerification(subset, bufBm, coords,
+ "copyPixelsTo(buf, bufSize)", reporter);
+
+ // Test #3 ////////////////////////////////////////////
+ // Copy with different stride between src and dst.
+ memset(buf, 0xFF, bufSize);
+ bufBm.installPixels(info, buf, subset.rowBytes()+1);
+ successExpected = true; // Should always work.
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsTo(buf, bufSize,
+ subset.rowBytes()+1) == successExpected);
+ if (successExpected)
+ reportCopyVerification(subset, bufBm, coords,
+ "copyPixelsTo(buf, bufSize, rowBytes+1)", reporter);
+
+ // Test #4 ////////////////////////////////////////////
+ // Test copy with stride too small.
+ memset(buf, 0xFF, bufSize);
+ bufBm.installPixels(info, buf, info.minRowBytes());
+ successExpected = false;
+ // Request copy with stride too small.
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsTo(buf, bufSize, bufBm.rowBytes()-1)
+ == successExpected);
+ if (successExpected)
+ reportCopyVerification(subset, bufBm, coords,
+ "copyPixelsTo(buf, bufSize, rowBytes()-1)", reporter);
+
+#if 0 // copyPixelsFrom is gone
+ // Test #5 ////////////////////////////////////////////
+ // Tests the case where the source stride is too small
+ // for the source configuration.
+ memset(buf, 0xFF, bufSize);
+ bufBm.installPixels(info, buf, info.minRowBytes());
+ writeCoordPixels(bufBm, coords);
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsFrom(buf, bufSize, 1) == false);
+
+ // Test #6 ///////////////////////////////////////////
+ // Tests basic copy from an external buffer to the bitmap.
+ // If the bitmap is "extracted", this also tests the case
+ // where the source stride is different from the dest.
+ // stride.
+ // We've made the buffer large enough to always succeed.
+ bufBm.installPixels(info, buf, info.minRowBytes());
+ writeCoordPixels(bufBm, coords);
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsFrom(buf, bufSize, bufBm.rowBytes()) ==
+ true);
+ reportCopyVerification(bufBm, subset, coords,
+ "copyPixelsFrom(buf, bufSize)",
+ reporter);
+
+ // Test #7 ////////////////////////////////////////////
+ // Tests the case where the source buffer is too small
+ // for the transfer.
+ REPORTER_ASSERT(reporter,
+ subset.copyPixelsFrom(buf, 1, subset.rowBytes()) ==
+ false);
+
+#endif
+ }
+ } // for (size_t copyCase ...
+ }
+}
+
#include "SkColorPriv.h"
#include "SkUtils.h"