In which a series of things around attachToCanvas and writePixels are fixed

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

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

diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h
index 41e316c..da7842b 100644 (file)
--- a/include/core/SkDevice.h
+++ b/include/core/SkDevice.h
@@ -145,6 +145,7 @@ public:
      * devices to prepare for drawing (e.g., locking their pixels, etc.)
      */
     virtual void onAttachToCanvas(SkCanvas* canvas) {
+        SkASSERT(!fAttachedToCanvas);
         this->lockPixels();
 #ifdef SK_DEBUG
         fAttachedToCanvas = true;
@@ -158,6 +159,7 @@ public:
      * possibly from SkCanvas' dtor.
      */
     virtual void onDetachFromCanvas() {
+        SkASSERT(fAttachedToCanvas);
         this->unlockPixels();
 #ifdef SK_DEBUG
         fAttachedToCanvas = false;

diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index dc6da11..ba8042d 100644 (file)
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -644,7 +644,11 @@ void SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y,
                            Config8888 config8888) {
     SkDevice* device = this->getDevice();
     if (device) {
-        device->writePixels(bitmap, x, y, config8888);
+        if (SkIRect::Intersects(SkIRect::MakeSize(this->getDeviceSize()),
+                                SkIRect::MakeXYWH(x, y, bitmap.width(), bitmap.height()))) {
+            device->accessBitmap(true);
+            device->writePixels(bitmap, x, y, config8888);
+        }
     }
 }
 

diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index f8bed65..10d89d5 100644 (file)
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -9,6 +9,7 @@
 #include "SkDraw.h"
 #include "SkImageFilter.h"
 #include "SkMetaData.h"
+#include "SkRasterClip.h"
 #include "SkRect.h"
 
 SK_DEFINE_INST_COUNT(SkDevice)
@@ -299,8 +300,13 @@ void SkDevice::writePixels(const SkBitmap& bitmap,
 
     SkPaint paint;
     paint.setXfermodeMode(SkXfermode::kSrc_Mode);
-    SkCanvas canvas(this);
-    canvas.drawSprite(*sprite, x, y, &paint);
+    SkRasterClip clip(SkIRect::MakeWH(fBitmap.width(), fBitmap.height()));
+    SkDraw  draw;
+    draw.fRC = &clip;
+    draw.fClip = &clip.bwRgn();
+    draw.fBitmap = &fBitmap; // canvas should have already called accessBitmap
+    draw.fMatrix = &SkMatrix::I();
+    this->drawSprite(draw, *sprite, x, y, paint);
 }
 
 ///////////////////////////////////////////////////////////////////////////////

diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp
index 392c5ac..755841e 100644 (file)
--- a/tests/ReadPixelsTest.cpp
+++ b/tests/ReadPixelsTest.cpp
@@ -352,16 +352,19 @@ void ReadPixelsTest(skiatest::Reporter* reporter, GrContext* context) {
                     if (startsWithPixels) {
                         fillBitmap(&bmp);
                     }
-
+                    uint32_t idBefore = canvas.getDevice()->accessBitmap(false).getGenerationID();
                     bool success =
                         canvas.readPixels(&bmp, srcRect.fLeft,
                                           srcRect.fTop, config8888);
+                    uint32_t idAfter = canvas.getDevice()->accessBitmap(false).getGenerationID();
 
                     // we expect to succeed when the read isn't fully clipped
                     // out.
                     bool expectSuccess = SkIRect::Intersects(srcRect, DEV_RECT);
                     // determine whether we expected the read to succeed.
                     REPORTER_ASSERT(reporter, success == expectSuccess);
+                    // read pixels should never change the gen id
+                    REPORTER_ASSERT(reporter, idBefore == idAfter);
 
                     if (success || startsWithPixels) {
                         checkRead(reporter, bmp, srcRect.fLeft, srcRect.fTop,

diff --git a/tests/WritePixelsTest.cpp b/tests/WritePixelsTest.cpp
index 4f045ef..9c3a5fb 100644 (file)
--- a/tests/WritePixelsTest.cpp
+++ b/tests/WritePixelsTest.cpp
@@ -418,8 +418,17 @@ void WritePixelsTest(skiatest::Reporter* reporter, GrContext* context) {
                     SkCanvas::Config8888 config8888 = gSrcConfigs[c];
                     SkBitmap bmp;
                     REPORTER_ASSERT(reporter, setupBitmap(&bmp, config8888, rect.width(), rect.height(), SkToBool(tightBmp)));
+                    uint32_t idBefore = canvas.getDevice()->accessBitmap(false).getGenerationID();
                     canvas.writePixels(bmp, rect.fLeft, rect.fTop, config8888);
+                    uint32_t idAfter = canvas.getDevice()->accessBitmap(false).getGenerationID();
                     REPORTER_ASSERT(reporter, checkWrite(reporter, &canvas, bmp, rect.fLeft, rect.fTop, config8888));
+
+                    // we should change the genID iff pixels were actually written.
+                    SkIRect canvasRect = SkIRect::MakeSize(canvas.getDeviceSize());
+                    SkIRect writeRect = SkIRect::MakeXYWH(rect.fLeft, rect.fTop,
+                                                          bmp.width(), bmp.height());
+                    bool intersects = SkIRect::Intersects(canvasRect, writeRect) ;
+                    REPORTER_ASSERT(reporter, intersects == (idBefore != idAfter));
                 }
             }
         }