SkShader::asNewEffect Refactoring

src/core/SkShader.cpp

Index: src/core/SkShader.cpp
diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index 9484db59c18837ac6feaa767cbff03a05cc78613..370159895d99d8a25d8aa29e13bd26f05791268e 100644
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -16,6 +16,7 @@
 #include "SkShader.h"
 #include "SkThread.h"
 #include "SkWriteBuffer.h"
+#include "SkGr.h"
 
 //#define SK_TRACK_SHADER_LIFETIME
 
@@ -208,8 +209,16 @@ 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 {
@@ -341,6 +350,16 @@ SkShader::GradientType SkColorShader::asAGradient(GradientInfo* info) const {
     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: (");