commit 855e88edfafe4b3892e99f932c38fa7433b2fcbe
author commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Mon Apr 21 19:33:12 2014 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org@2bbb7eff-a529-9590-31e7-b0007b416f81> Mon Apr 21 19:33:12 2014 +0000
tree 75b2ae89dfb43df24f10fa33cedf5bdadef398a1
parent 07b7462e1635c5f73ade09e3f6bf7ee57e7d1d6f

Fixes for SkPictureShader.

Update comment in header to make it more clear that the picture
should be unaltered after creating the shader. We want our shaders
to be immutable, and this supports that.

Make the factory return NULL if the shader would have never drawn
anyway i.e. for a null picture or picture with no width/height.

Addresses comments I brought up in
https://codereview.chromium.org/221923007/#msg16.

BUG=skia:1976
R=reed@google.com, fmalita@chromium.org, robertphillips@google.com

Author: scroggo@google.com

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

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

src/core/SkPictureShader.cpp

diff --git a/src/core/SkPictureShader.cpp b/src/core/SkPictureShader.cpp
index 15df3a3..bf31285 100644
--- a/src/core/SkPictureShader.cpp
+++ b/src/core/SkPictureShader.cpp
@@ -19,28 +19,25 @@
 #endif
 
 SkPictureShader::SkPictureShader(SkPicture* picture, TileMode tmx, TileMode tmy)
-    : fPicture(picture)
+    : fPicture(SkRef(picture))
     , fTmx(tmx)
-    , fTmy(tmy) {
-    SkSafeRef(fPicture);
-}
+    , fTmy(tmy) { }
 
 SkPictureShader::SkPictureShader(SkReadBuffer& buffer)
         : INHERITED(buffer) {
     fTmx = static_cast<SkShader::TileMode>(buffer.read32());
     fTmy = static_cast<SkShader::TileMode>(buffer.read32());
-    if (buffer.readBool()) {
-        fPicture = SkPicture::CreateFromBuffer(buffer);
-    } else {
-        fPicture = NULL;
-    }
+    fPicture = SkPicture::CreateFromBuffer(buffer);
 }
 
 SkPictureShader::~SkPictureShader() {
-    SkSafeUnref(fPicture);
+    fPicture->unref();
 }
 
 SkPictureShader* SkPictureShader::Create(SkPicture* picture, TileMode tmx, TileMode tmy) {
+    if (!picture || 0 == picture->width() || 0 == picture->height()) {
+        return NULL;
+    }
     return SkNEW_ARGS(SkPictureShader, (picture, tmx, tmy));
 }
 
@@ -49,16 +46,11 @@
 
     buffer.write32(fTmx);
     buffer.write32(fTmy);
-    buffer.writeBool(NULL != fPicture);
-    if (fPicture) {
-        fPicture->flatten(buffer);
-    }
+    fPicture->flatten(buffer);
 }
 
 bool SkPictureShader::buildBitmapShader(const SkMatrix& matrix) const {
-    if (!fPicture || (0 == fPicture->width() && 0 == fPicture->height())) {
-        return false;
-    }
+    SkASSERT(fPicture && fPicture->width() > 0 && fPicture->height() > 0);
 
     SkMatrix m;
     if (this->hasLocalMatrix()) {