SkShader::asNewEffect Refactoring

include/core/SkShader.h

 /*
  * 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.
  */
 
 
 #ifndef SkShader_DEFINED
 #define SkShader_DEFINED
 
 #include "SkBitmap.h"
 #include "SkFlattenable.h"
 #include "SkMask.h"
 #include "SkMatrix.h"
 #include "SkPaint.h"
+#include "../gpu/GrColor.h"
 
 class SkPath;
 class SkPicture;
 class SkXfermode;
 class GrContext;
 class GrEffectRef;
 
 /** \class SkShader
  *
  *  Shaders specify the source color(s) for what is being drawn. If a paint
@@ skipping 339 matching lines @@
     struct ComposeRec {
         const SkShader*     fShaderA;
         const SkShader*     fShaderB;
         const SkXfermode*   fMode;
     };
 
     virtual bool asACompose(ComposeRec* rec) const { return false; }
 
 
     /**
-     *  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
     /**
      *  If the shader is a custom shader which has data the caller might want, call this function
      *  to get that data.
      */
     virtual bool asACustomShader(void** customData) const { return false; }
 #endif
 
     //////////////////////////////////////////////////////////////////////////
@@ skipping 59 matching lines @@
     SkShader(SkReadBuffer& );
     virtual void flatten(SkWriteBuffer&) const SK_OVERRIDE;
 
     bool computeTotalInverse(const ContextRec&, SkMatrix* totalInverse) const;
 
     /**
      *  Your subclass must also override contextSize() if it overrides onCreateContext().
      *  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;
 };
 
 #endif