commit 4b90b1122c93e6600ea352f4ccf1dfc54c8bb146
author scroggo@google.com <scroggo@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> Tue Dec 04 15:08:56 2012 +0000
committer scroggo@google.com <scroggo@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81> Tue Dec 04 15:08:56 2012 +0000
tree 16ee994e3b2293fde0d1265eb0d57c4164db3dea
parent ebce0301082cda9dc3e3298f6db91d46fe66298b

Handle recording a bitmap if copy fails.

If SkBitmapHeap::insert() returns INVALID_SLOT, assert at picture
record time so we can debug, but allow it to continue in release
mode, so that we can still capture a picture. At playback time,
print a message so we know that there was an error.

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

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

src/core/SkPicturePlayback.h

diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h
index 8a7ee0a..9e495fa 100644
--- a/src/core/SkPicturePlayback.h
+++ b/src/core/SkPicturePlayback.h
@@ -93,7 +93,11 @@
     };
 
     const SkBitmap& getBitmap(SkReader32& reader) {
-        int index = reader.readInt();
+        const int index = reader.readInt();
+        if (SkBitmapHeap::INVALID_SLOT == index) {
+            SkDebugf("An invalid bitmap was recorded!\n");
+            return fBadBitmap;
+        }
         return (*fBitmaps)[index];
     }
 
@@ -190,6 +194,10 @@
     void flattenToBuffer(SkOrderedWriteBuffer&) const;
 
 private:
+    // Only used by getBitmap() if the passed in index is SkBitmapHeap::INVALID_SLOT. This empty
+    // bitmap allows playback to draw nothing and move on.
+    SkBitmap fBadBitmap;
+
     SkAutoTUnref<SkBitmapHeap> fBitmapHeap;
     SkAutoTUnref<SkPathHeap> fPathHeap;
 

src/core/SkPictureRecord.cpp

diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index d6f51e3..270eee5 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -657,7 +657,12 @@
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkPictureRecord::addBitmap(const SkBitmap& bitmap) {
-    addInt(fBitmapHeap->insert(bitmap));
+    const int index = fBitmapHeap->insert(bitmap);
+    // In debug builds, a bad return value from insert() will crash, allowing for debugging. In
+    // release builds, the invalid value will be recorded so that the reader will know that there
+    // was a problem.
+    SkASSERT(index != SkBitmapHeap::INVALID_SLOT);
+    addInt(index);
 }
 
 void SkPictureRecord::addMatrix(const SkMatrix& matrix) {

tests/PictureTest.cpp

diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 79ca313..e62d5aa 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -154,7 +154,6 @@
 
     static const int N = 4;
     SkBitmap bm[N];
-    SkPMColor pmcolors[N];
     SkPixelRef* refs[N];
 
     const SkPoint pos[] = {
@@ -306,10 +305,30 @@
     }
 }
 
+#ifndef SK_DEBUG
+// Only test this is in release mode. We deliberately crash in debug mode, since a valid caller
+// should never do this.
+static void test_bad_bitmap() {
+    // This bitmap has a width and height but no pixels. As a result, attempting to record it will
+    // fail.
+    SkBitmap bm;
+    bm.setConfig(SkBitmap::kARGB_8888_Config, 100, 100);
+    SkPicture picture;
+    SkCanvas* recordingCanvas = picture.beginRecording(100, 100);
+    recordingCanvas->drawBitmap(bm, 0, 0);
+    picture.endRecording();
+
+    SkCanvas canvas;
+    canvas.drawPicture(picture);
+}
+#endif
+
 static void TestPicture(skiatest::Reporter* reporter) {
 #ifdef SK_DEBUG
     test_deleting_empty_playback();
     test_serializing_empty_picture();
+#else
+    test_bad_bitmap();
 #endif
     test_peephole(reporter);
     test_gatherpixelrefs(reporter);