When copying a bitmap, copy the generation ID.

Review URL: https://codereview.appspot.com/6462084

git-svn-id: http://skia.googlecode.com/svn/trunk@5227 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/gyp/tests.gyp b/gyp/tests.gyp
index efb41f81e4c64b60c61356fcd34ec050a311b062..fa04dcecd47260b6c0e6bc911addc41059d6eb79 100644 (file)
--- a/gyp/tests.gyp
+++ b/gyp/tests.gyp
@@ -47,6 +47,7 @@
         '../tests/GeometryTest.cpp',
         '../tests/GLInterfaceValidation.cpp',
         '../tests/GLProgramsTest.cpp',
+        '../tests/GpuBitmapCopyTest.cpp',
         '../tests/GrContextFactoryTest.cpp',
         '../tests/GradientTest.cpp',
         '../tests/GrMemoryPoolTest.cpp',

diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index c8e1b2cc5bac50e7b930b0b0f2930a8fd43a9abd..1b632d36e892ccb240029e3566ef243282170580 100644 (file)
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -193,6 +193,10 @@ private:
 
     mutable uint32_t fGenerationID;
 
+    // SkBitmap is only a friend so that when copying, it can modify the new SkPixelRef to have the
+    // same fGenerationID as the original.
+    friend class SkBitmap;
+
     SkString    fURI;
 
     // can go from false to true, but never from true to false

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index e59c5069834be2246725b895b161298c3293d069..382d6a1dceff8cf0cad67dbc1faac86eb6b7a3a1 100644 (file)
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -892,6 +892,9 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const {
         // 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();
+            }
             return true;
         }
 
@@ -926,6 +929,10 @@ bool SkBitmap::copyTo(SkBitmap* dst, Config dstConfig, Allocator* alloc) const {
     if (src->config() == dstConfig) {
         if (tmpDst.getSize() == src->getSize()) {
             memcpy(tmpDst.getPixels(), src->getPixels(), src->getSafeSize());
+            SkPixelRef* pixelRef = tmpDst.pixelRef();
+            if (pixelRef != NULL) {
+                pixelRef->fGenerationID = this->getGenerationID();
+            }
         } else {
             const char* srcP = reinterpret_cast<const char*>(src->getPixels());
             char* dstP = reinterpret_cast<char*>(tmpDst.getPixels());
@@ -966,6 +973,9 @@ bool SkBitmap::deepCopyTo(SkBitmap* dst, Config dstConfig) const {
     if (fPixelRef) {
         SkPixelRef* pixelRef = fPixelRef->deepCopy(dstConfig);
         if (pixelRef) {
+            if (dstConfig == fConfig) {
+                pixelRef->fGenerationID = fPixelRef->getGenerationID();
+            }
             dst->setConfig(dstConfig, fWidth, fHeight);
             dst->setPixelRef(pixelRef)->unref();
             return true;

diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp
index 690497a55e2e56d035597c7dac6c58ab5a3f84b9..0366068426da1f9b0613a038bf08b8dbeb29ae7e 100644 (file)
--- a/tests/BitmapCopyTest.cpp
+++ b/tests/BitmapCopyTest.cpp
@@ -218,10 +218,11 @@ static void reportCopyVerification(const SkBitmap& bm1, const SkBitmap& bm2,
     bool success = true;
 
     // Confirm all pixels in the list match.
-    for (int i = 0; i < coords.length; ++i)
+    for (int i = 0; i < coords.length; ++i) {
         success = success &&
                   (getPixel(coords[i]->fX, coords[i]->fY, bm1) ==
                    getPixel(coords[i]->fX, coords[i]->fY, bm2));
+    }
 
     if (!success) {
         SkString str;
@@ -305,6 +306,9 @@ static void TestBitmapCopy(skiatest::Reporter* reporter) {
                     REPORTER_ASSERT(reporter, srcP != dstP);
                     REPORTER_ASSERT(reporter, !memcmp(srcP, dstP,
                                                       src.getSize()));
+                    REPORTER_ASSERT(reporter, src.getGenerationID() == dst.getGenerationID());
+                } else {
+                    REPORTER_ASSERT(reporter, src.getGenerationID() != dst.getGenerationID());
                 }
                 // test extractSubset
                 {

diff --git a/tests/GpuBitmapCopyTest.cpp b/tests/GpuBitmapCopyTest.cpp
new file mode 100644 (file)
index 0000000..df05f40
--- /dev/null
+++ b/tests/GpuBitmapCopyTest.cpp
@@ -0,0 +1,108 @@
+
+/*
+ * Copyright 2012 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "GrContext.h"
+#include "SkBitmap.h"
+#include "SkGpuDevice.h"
+#include "SkPixelRef.h"
+#include "SkRect.h"
+#include "Test.h"
+
+static const char* boolStr(bool value) {
+    return value ? "true" : "false";
+}
+
+// these are in the same order as the SkBitmap::Config enum
+static const char* gConfigName[] = {
+    "None", "4444", "8888"
+};
+
+struct Pair {
+    SkBitmap::Config    fConfig;
+    const char*         fValid;
+};
+
+// 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) {
+    if (NULL == grContext) {
+        return;
+    }
+    static const Pair gPairs[] = {
+        { SkBitmap::kNo_Config,         "000"  },
+        { SkBitmap::kARGB_4444_Config,  "011"  },
+        { SkBitmap::kARGB_8888_Config,  "011"  },
+    };
+
+    const int W = 20;
+    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;
+
+            SkGpuDevice* device = SkNEW_ARGS(SkGpuDevice, (grContext, gPairs[i].fConfig, W, H));
+            SkAutoUnref aur(device);
+            src = device->accessBitmap(false);
+            device->clear(SK_ColorWHITE);
+
+            bool success = src.deepCopyTo(&dst, gPairs[j].fConfig);
+            bool expected = gPairs[i].fValid[j] != '0';
+            if (success != expected) {
+                SkString str;
+                str.printf("SkBitmap::deepCopyTo from %s to %s. expected %s returned %s",
+                           gConfigName[i], gConfigName[j], boolStr(expected),
+                           boolStr(success));
+                reporter->reportFailed(str);
+            }
+
+            bool canSucceed = src.canCopyTo(gPairs[j].fConfig);
+            if (success != canSucceed) {
+                SkString str;
+                str.printf("SkBitmap::deepCopyTo from %s to %s returned %s,"
+                           "but canCopyTo returned %s",
+                           gConfigName[i], gConfigName[j], boolStr(success),
+                           boolStr(canSucceed));
+                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);
+            }
+        } // for (size_t j = ...
+    }
+}
+
+#include "TestClassDef.h"
+DEFINE_GPUTESTCLASS("GpuBitmapCopy", TestGpuBitmapCopyClass, TestGpuBitmapCopy)