SkPictureRecord: silently do nothing for non-drawable SkBitmaps.

SkPictureRecord: silently do nothing for non-drawable SkBitmaps.


BUG=skia:2135
Committed: http://code.google.com/p/skia/source/detail?r=13386

[reed1, 2014-02-05 16:03:00]

Small suggestions:

- perhaps a single method on bitmap to test for this?
- perhaps SkCanvas could perform the same tests as you're adding to
picturerecord
- once its reliably in canvas, replace SkDraw etc. checks with asserts
- perhaps augment your new test to exercise multiple backends (gpu, raster,
picture, surfaces) -- don't need to go overboard on this one

lgtm

[mtklein, 2014-02-05 16:36:30]

On 2014/02/05 16:03:00, reed1 wrote:
> Small suggestions:
> 
> - perhaps a single method on bitmap to test for this?
> - perhaps SkCanvas could perform the same tests as you're adding to
> picturerecord
> - once its reliably in canvas, replace SkDraw etc. checks with asserts
> - perhaps augment your new test to exercise multiple backends (gpu, raster,
> picture, surfaces) -- don't need to go overboard on this one
> 
> lgtm

Did some of this:

  - added drawsNothing() to bitmap
  - canvas does the same checks as picture record now
  - didn't replace anything in SkDraw yet
  - updated the tests to check canvas too

Going to follow this right up by seeing how many empty() and isNull() I can
sensibly replace with drawsNothing() globally.  I'm hoping this will lead me
down into converting SkDraw to asserts.

[mtklein, 2014-02-10 16:36:58]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-02-10 16:37:08]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/154083004/40003

[commit-bot: I haz the power, 2014-02-10 16:37:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-10 16:37:14]

Failed to apply patch for src/core/SkCanvas.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file src/core/SkCanvas.cpp
  Hunk #1 succeeded at 994 (offset 28 lines).
  Hunk #2 succeeded at 1061 (offset 28 lines).
  Hunk #3 succeeded at 1559 (offset 28 lines).
  Hunk #4 FAILED at 1543.
  Hunk #5 succeeded at 1780 (offset 17 lines).
  Hunk #6 succeeded at 1827 (offset 17 lines).
  1 out of 6 hunks FAILED -- saving rejects to file src/core/SkCanvas.cpp.rej

Patch:       src/core/SkCanvas.cpp
Index: src/core/SkCanvas.cpp
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index
08b09e8e2b6e970b06d93c0449b9af8859573173..7a1f5e8521f649c27298eca179fbed77e6b1464e
100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -966,14 +966,9 @@ bool SkCanvas::isDrawingToLayer() const {
 
 /////////////////////////////////////////////////////////////////////////////
 
-// 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;
     }
 
@@ -1038,12 +1033,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) {
@@ -1537,8 +1531,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,
@@ -1549,8 +1543,8 @@ SkBaseDevice* SkCanvas::createLayerDevice(SkBitmap::Config
config,
 }
 
 SkBaseDevice* SkCanvas::createCompatibleDevice(SkBitmap::Config config,
-                                           int width, int height,
-                                           bool isOpaque) {
+                                               int width, int height,
+                                               bool isOpaque) {
     SkBaseDevice* device = this->getDevice();
     if (device) {
         return device->createCompatibleDevice(config, width, height, isOpaque);
@@ -1769,7 +1763,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;
     }
 
@@ -1816,6 +1810,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;

[mtklein, 2014-02-10 18:12:37]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-02-10 18:12:51]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/154083004/260001

[commit-bot: I haz the power, 2014-02-10 18:29:13]

Change committed as 13386