SkShader::asNewEffect Refactoring

src/core/SkBitmapProcShader.cpp

 
 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 #include "SkColorPriv.h"
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
@@ skipping 364 matching lines @@
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #if SK_SUPPORT_GPU
 
 #include "GrTextureAccess.h"
 #include "effects/GrSimpleTextureEffect.h"
 #include "SkGr.h"
 
-GrEffectRef* SkBitmapProcShader::asNewEffect(GrContext* context, const SkPaint& paint,
-                                             const SkMatrix* localMatrix) const {
+bool SkBitmapProcShader::asNewEffect(GrContext* context, const SkPaint& paint, GrColor* grColor,
+                                     GrEffectRef** grEffect, const SkMatrix* localMatrix) const {

[bsalomon, 2014/06/05 17:32:44]

Doesn't this function need to set grColor?

[jvanverth1, 2014/06/05 17:36:30]

grColor never gets set by this method

[dandov, 2014/06/06 21:50:43]

Done.

[dandov, 2014/06/06 21:50:43]

Done.

[dandov, 2014/06/06 21:50:43]

The grColor is set the alpha of the SkPaint's alpha

On 2014/06/05 17:36:30, jvanverth1 wrote:

     SkMatrix matrix;
     matrix.setIDiv(fRawBitmap.width(), fRawBitmap.height());
 
     SkMatrix lmInverse;
     if (!this->getLocalMatrix().invert(&lmInverse)) {
-        return NULL;
+        return false;
     }
     if (localMatrix) {
         SkMatrix inv;
         if (!localMatrix->invert(&inv)) {
-            return NULL;
+            return false;
         }
         lmInverse.postConcat(inv);
     }
     matrix.preConcat(lmInverse);
 
     SkShader::TileMode tm[] = {
         (TileMode)fTileModeX,
         (TileMode)fTileModeY,
     };
 
@@ skipping 35 matching lines @@
             textureFilterMode = GrTextureParams::kMipMap_FilterMode;
             break;
 
     }
     GrTextureParams params(tm, textureFilterMode);
     GrTexture* texture = GrLockAndRefCachedBitmapTexture(context, fRawBitmap, &params);
 
     if (NULL == texture) {
         SkErrorInternals::SetError( kInternalError_SkError,
                                     "Couldn't convert bitmap to texture.");
-        return NULL;
+        return false;
     }
 
-    GrEffectRef* effect = NULL;
+    *grEffect = NULL;

[bsalomon, 2014/06/05 17:32:44]

I don't think we should require this. The passed in ptr should already point to
NULL.

[dandov, 2014/06/06 21:50:43]

Done.

     if (useBicubic) {
-        effect = GrBicubicEffect::Create(texture, matrix, tm);
+        *grEffect = GrBicubicEffect::Create(texture, matrix, tm);
     } else {
-        effect = GrSimpleTextureEffect::Create(texture, matrix, params);
+        *grEffect = GrSimpleTextureEffect::Create(texture, matrix, params);
     }
     GrUnlockAndUnrefCachedBitmapTexture(texture);
-    return effect;
+    return true;
 }
 #endif