remove unneeded SkShader::validContext

src/core/SkPictureShader.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkPictureShader.h"
 
 #include "SkBitmap.h"
@@ skipping 92 matching lines @@
         fCachedBitmapShader.reset(CreateBitmapShader(bm, fTmx, fTmy, &shaderMatrix));
     }
 
     // Increment the ref counter inside the mutex to ensure the returned pointer is still valid.
     // Otherwise, the pointer may have been overwritten on a different thread before the object's
     // ref count was incremented.
     fCachedBitmapShader.get()->ref();
     return fCachedBitmapShader;
 }
 
-SkShader* SkPictureShader::validInternal(const ContextRec& rec, SkMatrix* totalInverse) const {
-    if (!this->INHERITED::validContext(rec, totalInverse)) {
-        return NULL;
-    }
-
-    SkAutoTUnref<SkShader> bitmapShader(this->refBitmapShader(*rec.fMatrix));
-    if (!bitmapShader || !bitmapShader->validContext(rec)) {
-        return NULL;
-    }
-
-    return bitmapShader.detach();
-}
-
-bool SkPictureShader::validContext(const ContextRec& rec, SkMatrix* totalInverse) const {
-    SkAutoTUnref<SkShader> shader(this->validInternal(rec, totalInverse));
-    return shader != NULL;
-}
-
-SkShader::Context* SkPictureShader::createContext(const ContextRec& rec, void* storage) const {
-    SkAutoTUnref<SkShader> bitmapShader(this->validInternal(rec, NULL));
-    if (!bitmapShader) {
-        return NULL;
-    }
-
-    return SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext, (*this, rec, bitmapShader.detach()));
-}
-
 size_t SkPictureShader::contextSize() const {
     return sizeof(PictureShaderContext);
 }
 
+SkShader::Context* SkPictureShader::onCreateContext(const ContextRec& rec, void* storage) const {
+    SkAutoTUnref<SkShader> bitmapShader(this->refBitmapShader(*rec.fMatrix));
+    if (NULL == bitmapShader.get()) {
+        return NULL;
+    }
+
+    PictureShaderContext* ctx = SkNEW_PLACEMENT_ARGS(storage, PictureShaderContext,
+                                                     (*this, rec, bitmapShader.detach()));
+    if (!ctx->isValid()) {

[scroggo, 2014/05/05 14:01:19]

Don't we have a general rule against creating an object that might be invalid
and then querying it?

This could be simplified by creating the BitmapShaderContext outside of
PictureShaderContext and passing it in.

[reed1, 2014/05/05 15:10:01]

Done.

+        ctx->~PictureShaderContext();
+        ctx = NULL;
+    }
+    return ctx;
+}
+
+/////////////////////////////////////////////////////////////////////////////////////////
+
 SkPictureShader::PictureShaderContext::PictureShaderContext(
         const SkPictureShader& shader, const ContextRec& rec, SkShader* bitmapShader)
     : INHERITED(shader, rec)
-    , fBitmapShader(bitmapShader)
+    , fBitmapShader(SkRef(bitmapShader))
 {
-    SkASSERT(fBitmapShader);
-    fBitmapShaderContextStorage = sk_malloc_throw(fBitmapShader->contextSize());
-    fBitmapShaderContext = fBitmapShader->createContext(rec, fBitmapShaderContextStorage);
-    SkASSERT(fBitmapShaderContext);
+    fBitmapShaderContextStorage = sk_malloc_throw(bitmapShader->contextSize());
+    fBitmapShaderContext = bitmapShader->createContext(rec, fBitmapShaderContextStorage);
+    //if fBitmapShaderContext is null, we are invalid
 }
 
 SkPictureShader::PictureShaderContext::~PictureShaderContext() {
-    fBitmapShaderContext->~Context();
+    if (fBitmapShaderContext) {
+        fBitmapShaderContext->~Context();
+    }
     sk_free(fBitmapShaderContextStorage);
 }
 
 uint32_t SkPictureShader::PictureShaderContext::getFlags() const {
+    SkASSERT(fBitmapShaderContext);
     return fBitmapShaderContext->getFlags();
 }
 
 SkShader::Context::ShadeProc SkPictureShader::PictureShaderContext::asAShadeProc(void** ctx) {
+    SkASSERT(fBitmapShaderContext);
     return fBitmapShaderContext->asAShadeProc(ctx);
 }
 
 void SkPictureShader::PictureShaderContext::shadeSpan(int x, int y, SkPMColor dstC[], int count) {
     SkASSERT(fBitmapShaderContext);
     fBitmapShaderContext->shadeSpan(x, y, dstC, count);
 }
 
 void SkPictureShader::PictureShaderContext::shadeSpan16(int x, int y, uint16_t dstC[], int count) {
     SkASSERT(fBitmapShaderContext);
@@ skipping 18 matching lines @@
 
 #if SK_SUPPORT_GPU
 GrEffectRef* SkPictureShader::asNewEffect(GrContext* context, const SkPaint& paint) const {
     SkAutoTUnref<SkShader> bitmapShader(this->refBitmapShader(context->getMatrix()));
     if (!bitmapShader) {
         return NULL;
     }
     return bitmapShader->asNewEffect(context, paint);
 }
 #endif