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 (file)
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -345,6 +345,27 @@ public:
     */
     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 (file)
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -468,6 +468,61 @@ bool SkBitmap::HeapAllocator::allocPixelRef(SkBitmap* dst,
 
 ///////////////////////////////////////////////////////////////////////////////
 
+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..5eb9a2a 100644 (file)
--- 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 @@ struct Pair {
 // 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 @@ struct Coordinates {
     }
 };
 
+// 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_extractSubset, reporter) {
     }
 }
 
+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"