SkShader::asNewEffect Refactoring

src/core/SkShader.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBitmapProcShader.h"
 #include "SkEmptyShader.h"
 #include "SkReadBuffer.h"
 #include "SkMallocPixelRef.h"
 #include "SkPaint.h"
 #include "SkPicture.h"
 #include "SkPictureShader.h"
 #include "SkScalar.h"
 #include "SkShader.h"
 #include "SkThread.h"
 #include "SkWriteBuffer.h"
+#include "SkGr.h"
 
 //#define SK_TRACK_SHADER_LIFETIME
 
 #ifdef SK_TRACK_SHADER_LIFETIME
     static int32_t gShaderCounter;
 #endif
 
 static inline void inc_shader_counter() {
 #ifdef SK_TRACK_SHADER_LIFETIME
     int32_t prev = sk_atomic_inc(&gShaderCounter);
@@ skipping 172 matching lines @@
 //////////////////////////////////////////////////////////////////////////////
 
 SkShader::BitmapType SkShader::asABitmap(SkBitmap*, SkMatrix*, TileMode*) const {
     return kNone_BitmapType;
 }
 
 SkShader::GradientType SkShader::asAGradient(GradientInfo* info) const {
     return kNone_GradientType;
 }
 
-GrEffectRef* SkShader::asNewEffect(GrContext*, const SkPaint&, const SkMatrix*) const {
-    return NULL;
+bool SkShader::asNewEffect(GrContext* context, const SkPaint& paint,
+                           const SkMatrix* localMatrixOrNull, GrColor* grColor,
+                           GrEffectRef** grEffect)  const {
+    *grEffect = NULL;

[bsalomon, 2014/06/09 14:11:30]

I'd feel better if this did nothing other than return false. It seems like a
cleaner contract for the caller to be able to assume that nothing was
initialized if the function fails.

[dandov, 2014/06/09 19:10:55]

Done.

+    *grColor = SkColor2GrColor(paint.getColor());

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

default grColor value is the paint's color

[bsalomon, 2014/06/09 14:11:31]

I think this default should be implemented by the calling function.

+    return false;
+}
+
+GrColor SkShader::getColorAsAlpha(SkPaint paint) const {

[bsalomon, 2014/06/09 14:11:31]

Having this as a function doesn't seem to add much (and could add cost to a
single operation).

[dandov, 2014/06/09 19:10:55]

Done.

+    return SkColor2GrColorJustAlpha(paint.getColor());
 }
 
 SkShader* SkShader::refAsALocalMatrixShader(SkMatrix*) const {
     return NULL;
 }
 
 SkShader* SkShader::CreateEmptyShader() {
     return SkNEW(SkEmptyShader);
 }
 
@@ skipping 111 matching lines @@
     if (info) {
         if (info->fColors && info->fColorCount >= 1) {
             info->fColors[0] = fColor;
         }
         info->fColorCount = 1;
         info->fTileMode = SkShader::kRepeat_TileMode;
     }
     return kColor_GradientType;
 }
 
+bool SkColorShader::asNewEffect(GrContext* context, const SkPaint& paint,
+                                const SkMatrix* localMatrix, GrColor* grColor,
+                                GrEffectRef** grEffect) const {

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

Use the SkColorShader's color as the grColor, modulated by the SkPaint's alpha.

+    *grEffect = NULL;
+    SkColor skColor = fColor;
+    U8CPU newA = SkMulDiv255Round(SkColorGetA(fColor), paint.getAlpha());
+    *grColor = SkColor2GrColor(SkColorSetA(skColor, newA));
+    return false;

[bsalomon, 2014/06/09 14:11:31]

This should return true since it succeeded. Otherwise why have a return value?

[dandov, 2014/06/09 19:10:55]

Done.

+}
+
 #ifndef SK_IGNORE_TO_STRING
 void SkColorShader::toString(SkString* str) const {
     str->append("SkColorShader: (");
 
     str->append("Color: ");
     str->appendHex(fColor);
 
     this->INHERITED::toString(str);
 
     str->append(")");
 }
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifndef SK_IGNORE_TO_STRING
 #include "SkEmptyShader.h"
 
 void SkEmptyShader::toString(SkString* str) const {
     str->append("SkEmptyShader: (");
 
     this->INHERITED::toString(str);
 
     str->append(")");
 }
 #endif