SkShader::asNewEffect Refactoring

include/core/SkShader.h

Index: include/core/SkShader.h
diff --git a/include/core/SkShader.h b/include/core/SkShader.h
index 82efab27410439b369d2186012354f480232fc0b..e492bd46ee70b2f7f980f960d9df2dd0ea1a1695 100644
--- a/include/core/SkShader.h
+++ b/include/core/SkShader.h
@@ -14,6 +14,7 @@
 #include "SkMask.h"
 #include "SkMatrix.h"
 #include "SkPaint.h"
+#include "../gpu/GrColor.h"
 
 class SkPath;
 class SkPicture;
@@ -373,14 +374,17 @@ public:
 
 
     /**
-     *  If the shader subclass has a GrEffect implementation, this resturns the effect to install.
+     *  Returns true if the shader subclass is able to set the grEffect output parameter to an
+     *  effect, in case it fails or the final *grEffect value is null it returns false.
+     *  It always sets the value of grColor, the default is the paint color.

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

It really always sets the color? Seems like the caller could set the skpaint
color on gr paint and then only override it if this returns true. Failing and
changing an out param feels weird to me.

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

Change contract of function, returns false if it fails at some point or if there
is no implementation of this method in the shader subclass. The caller is in
charge of defining what to do with the effect and grcolor if it fails.

On 2014/06/09 14:11:30, bsalomon wrote:

      *  The incoming color to the effect has r=g=b=a all extracted from the SkPaint's alpha.
      *  The output color should be the computed SkShader premul color modulated by the incoming
      *  color. The GrContext may be used by the effect to create textures. The GPU device does not
      *  call createContext. Instead we pass the SkPaint here in case the shader needs paint info.
      */
-    virtual GrEffectRef* asNewEffect(GrContext* context, const SkPaint& paint,
-                                     const SkMatrix* localMatrixOrNull) const;
+    virtual bool asNewEffect(GrContext* context, const SkPaint& paint,
+                             const SkMatrix* localMatrixOrNull, GrColor* grColor,
+                             GrEffectRef** grEffect) const;
 
 #ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
     /**
@@ -460,10 +464,15 @@ protected:
      *  Base class impl returns NULL.
      */
     virtual Context* onCreateContext(const ContextRec&, void* storage) const;
+    
+    /**
+     *  Returns a GrColor with all of its values set to the alpha of paint.
+     */
+    GrColor getColorAsAlpha(SkPaint paint) const;

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

function to get the SkPaint's alpha in the 4 channels of a GrColor

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

I don't think this should be a member of SkShader. It could be an inline in a Gr
header (e.g. GrColor.h or SkGr.h)

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

Done.

 
 private:
     SkMatrix fLocalMatrix;
-
+    
     typedef SkFlattenable INHERITED;
 };