SkPictureRecord: silently do nothing for non-drawable SkBitmaps.

BUG=skia:2135
R=reed@google.com

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/154083004

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

4 files changed, 73 insertions, 19 deletions

diff --git a/include/core/SkBitmap.h b/include/core/SkBitmap.h
index 4951cf19cf..87d4649228 100644
--- a/include/core/SkBitmap.h
+++ b/include/core/SkBitmap.h
@@ -80,14 +80,20 @@ public:
     void swap(SkBitmap& other);
 
     /** Return true iff the bitmap has empty dimensions.
-    */
+     *  Hey!  Before you use this, see if you really want to know drawsNothing() instead.
+     */
     bool empty() const { return 0 == fWidth || 0 == fHeight; }
 
     /** Return true iff the bitmap has no pixelref. Note: this can return true even if the
-        dimensions of the bitmap are > 0 (see empty()).
-    */
+     *  dimensions of the bitmap are > 0 (see empty()).
+     *  Hey!  Before you use this, see if you really want to know drawsNothing() instead.
+     */
     bool isNull() const { return NULL == fPixelRef; }
 
+    /** Return true iff drawing this bitmap has no effect.
+     */
+    bool drawsNothing() const { return this->empty() || this->isNull(); }
+
     /** Return the config for the bitmap. */
     Config  config() const { return (Config)fConfig; }
 
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 20662d9af0..ed8274dc5d 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -994,14 +994,9 @@ SkSurface* SkCanvas::onNewSurface(const SkImageInfo& info) {
 
 /////////////////////////////////////////////////////////////////////////////
 
-// can't draw it if its empty, or its too big for a fixed-point width or height
-static bool reject_bitmap(const SkBitmap& bitmap) {
-    return  bitmap.width() <= 0 || bitmap.height() <= 0;
-}
-
 void SkCanvas::internalDrawBitmap(const SkBitmap& bitmap,
                                 const SkMatrix& matrix, const SkPaint* paint) {
-    if (reject_bitmap(bitmap)) {
+    if (bitmap.drawsNothing()) {
         return;
     }
 
@@ -1066,12 +1061,11 @@ void SkCanvas::internalDrawDevice(SkBaseDevice* srcDev, int x, int y,
 
 void SkCanvas::drawSprite(const SkBitmap& bitmap, int x, int y,
                           const SkPaint* paint) {
-    SkDEBUGCODE(bitmap.validate();)
-    CHECK_LOCKCOUNT_BALANCE(bitmap);
-
-    if (reject_bitmap(bitmap)) {
+    if (bitmap.drawsNothing()) {
         return;
     }
+    SkDEBUGCODE(bitmap.validate();)
+    CHECK_LOCKCOUNT_BALANCE(bitmap);
 
     SkPaint tmp;
     if (NULL == paint) {
@@ -1565,8 +1559,8 @@ const SkRegion& SkCanvas::getTotalClip() const {
 }
 
 SkBaseDevice* SkCanvas::createLayerDevice(SkBitmap::Config config,
-                                      int width, int height,
-                                      bool isOpaque) {
+                                          int width, int height,
+                                          bool isOpaque) {
     SkBaseDevice* device = this->getTopDevice();
     if (device) {
         return device->createCompatibleDeviceForSaveLayer(config, width, height,
@@ -1786,7 +1780,7 @@ void SkCanvas::drawBitmap(const SkBitmap& bitmap, SkScalar x, SkScalar y,
 void SkCanvas::internalDrawBitmapRect(const SkBitmap& bitmap, const SkRect* src,
                                       const SkRect& dst, const SkPaint* paint,
                                       DrawBitmapRectFlags flags) {
-    if (bitmap.width() == 0 || bitmap.height() == 0 || dst.isEmpty()) {
+    if (bitmap.drawsNothing() || dst.isEmpty()) {
         return;
     }
 
@@ -1833,6 +1827,9 @@ void SkCanvas::drawBitmapMatrix(const SkBitmap& bitmap, const SkMatrix& matrix,
 void SkCanvas::internalDrawBitmapNine(const SkBitmap& bitmap,
                                       const SkIRect& center, const SkRect& dst,
                                       const SkPaint* paint) {
+    if (bitmap.drawsNothing()) {
+        return;
+    }
     if (NULL == paint || paint->canComputeFastBounds()) {
         SkRect storage;
         const SkRect* bounds = &dst;
diff --git a/src/core/SkPictureRecord.cpp b/src/core/SkPictureRecord.cpp
index 5ce35d6fe8..fa50ced0df 100644
--- a/src/core/SkPictureRecord.cpp
+++ b/src/core/SkPictureRecord.cpp
@@ -949,7 +949,10 @@ void SkPictureRecord::drawPath(const SkPath& path, const SkPaint& paint) {
 }
 
 void SkPictureRecord::drawBitmap(const SkBitmap& bitmap, SkScalar left, SkScalar top,
-                        const SkPaint* paint = NULL) {
+                                 const SkPaint* paint = NULL) {
+    if (bitmap.drawsNothing()) {
+        return;
+    }
     // op + paint index + bitmap index + left + top
     uint32_t size = 3 * kUInt32Size + 2 * sizeof(SkScalar);
     size_t initialOffset = this->addDraw(DRAW_BITMAP, &size);
@@ -964,6 +967,9 @@ void SkPictureRecord::drawBitmap(const SkBitmap& bitmap, SkScalar left, SkScalar
 void SkPictureRecord::drawBitmapRectToRect(const SkBitmap& bitmap, const SkRect* src,
                                            const SkRect& dst, const SkPaint* paint,
                                            DrawBitmapRectFlags flags) {
+    if (bitmap.drawsNothing()) {
+        return;
+    }
     // id + paint index + bitmap index + bool for 'src' + flags
     uint32_t size = 5 * kUInt32Size;
     if (NULL != src) {
@@ -972,7 +978,8 @@ void SkPictureRecord::drawBitmapRectToRect(const SkBitmap& bitmap, const SkRect*
     size += sizeof(dst);        // + rect
 
     size_t initialOffset = this->addDraw(DRAW_BITMAP_RECT_TO_RECT, &size);
-    SkASSERT(initialOffset+getPaintOffset(DRAW_BITMAP_RECT_TO_RECT, size) == fWriter.bytesWritten());
+    SkASSERT(initialOffset+getPaintOffset(DRAW_BITMAP_RECT_TO_RECT, size)
+             == fWriter.bytesWritten());
     this->addPaintPtr(paint);
     this->addBitmap(bitmap);
     this->addRectPtr(src);  // may be null
@@ -983,6 +990,9 @@ void SkPictureRecord::drawBitmapRectToRect(const SkBitmap& bitmap, const SkRect*
 
 void SkPictureRecord::drawBitmapMatrix(const SkBitmap& bitmap, const SkMatrix& matrix,
                                        const SkPaint* paint) {
+    if (bitmap.drawsNothing()) {
+        return;
+    }
     // id + paint index + bitmap index + matrix
     uint32_t size = 3 * kUInt32Size + matrix.writeToMemory(NULL);
     size_t initialOffset = this->addDraw(DRAW_BITMAP_MATRIX, &size);
@@ -995,6 +1005,9 @@ void SkPictureRecord::drawBitmapMatrix(const SkBitmap& bitmap, const SkMatrix& m
 
 void SkPictureRecord::drawBitmapNine(const SkBitmap& bitmap, const SkIRect& center,
                                      const SkRect& dst, const SkPaint* paint) {
+    if (bitmap.drawsNothing()) {
+        return;
+    }
     // op + paint index + bitmap id + center + dst rect
     uint32_t size = 3 * kUInt32Size + sizeof(center) + sizeof(dst);
     size_t initialOffset = this->addDraw(DRAW_BITMAP_NINE, &size);
@@ -1007,7 +1020,10 @@ void SkPictureRecord::drawBitmapNine(const SkBitmap& bitmap, const SkIRect& cent
 }
 
 void SkPictureRecord::drawSprite(const SkBitmap& bitmap, int left, int top,
-                        const SkPaint* paint = NULL) {
+                                 const SkPaint* paint = NULL) {
+    if (bitmap.drawsNothing()) {
+        return;
+    }
     // op + paint index + bitmap index + left + top
     uint32_t size = 5 * kUInt32Size;
     size_t initialOffset = this->addDraw(DRAW_SPRITE, &size);
diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp
index 7665b13d7e..002fe8f441 100644
--- a/tests/PictureTest.cpp
+++ b/tests/PictureTest.cpp
@@ -1031,3 +1031,38 @@ DEF_TEST(Picture, reporter) {
     test_clip_expansion(reporter);
     test_hierarchical(reporter);
 }
+
+static void draw_bitmaps(const SkBitmap bitmap, SkCanvas* canvas) {
+    const SkPaint paint;
+    const SkRect rect = { 5.0f, 5.0f, 8.0f, 8.0f };
+    const SkIRect irect =  { 2, 2, 3, 3 };
+
+    // Don't care what these record, as long as they're legal.
+    canvas->drawBitmap(bitmap, 0.0f, 0.0f, &paint);
+    canvas->drawBitmapRectToRect(bitmap, &rect, rect, &paint, SkCanvas::kNone_DrawBitmapRectFlag);
+    canvas->drawBitmapMatrix(bitmap, SkMatrix::I(), &paint);
+    canvas->drawBitmapNine(bitmap, irect, rect, &paint);
+    canvas->drawSprite(bitmap, 1, 1);
+}
+
+static void test_draw_bitmaps(SkCanvas* canvas) {
+    SkBitmap empty;
+    draw_bitmaps(empty, canvas);
+    empty.setConfig(SkBitmap::kARGB_8888_Config, 10, 10);
+    draw_bitmaps(empty, canvas);
+}
+
+DEF_TEST(Picture_EmptyBitmap, r) {
+    SkPicture picture;
+    test_draw_bitmaps(picture.beginRecording(10, 10));
+    picture.endRecording();
+}
+
+DEF_TEST(Canvas_EmptyBitmap, r) {
+    SkBitmap dst;
+    dst.setConfig(SkBitmap::kARGB_8888_Config, 10, 10);
+    dst.allocPixels();
+    SkCanvas canvas(dst);
+
+    test_draw_bitmaps(&canvas);
+}