SkShader::asNewEffect Refactoring

src/gpu/SkGr.cpp

Index: src/gpu/SkGr.cpp
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index 31372cd02a1816cf378ecae4f71070465983955d..b1977810fd30a8f198e9ba894e4adef6d56a9b6e 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -492,7 +492,11 @@ void SkPaint2GrPaintShader(GrContext* context, const SkPaint& skPaint,
     AutoMatrix am(context);
 
     // setup the shader as the first color effect on the paint
-    SkAutoTUnref<GrEffectRef> effect(shader->asNewEffect(context, skPaint, NULL));
+    GrColor grColor;
+    GrEffectRef* grEffect;
+    grEffect = NULL;
+    shader->asNewEffect(context, skPaint, &grColor, &grEffect, NULL);

[jvanverth1, 2014/06/05 17:03:07]

The bool return value isn't checked here. Also, in looking at it written out,
getting the ref * and then adding it to a SkAutoTUnref seems a little clunky.
I'm not sure if there's a better way to do this -- Brian?

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

I can't think of a better way other than sending down a SkAutoTUnref* but I'm
not sure I like that.

We should probably fall back to solid color painting if false is returned...
e.g. copy SkPaint's color to GrPaint.

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

If it does succeed, it should also use the returned value of grColor in some
way.

On 2014/06/05 17:32:44, bsalomon wrote:

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

When the call to asNewEffect returns true (if nothing wrong happens and if
grEffect is not NULL) it adds the effect to the GrPaint. The call to asNewEffect
also modifies the value so now we only need to setup the rest of the paint. 
Each shader determines the GrColor it needs and if asNewEffect is not
implemented it is just set to the SkPaint's color. 

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

+    SkAutoTUnref<GrEffectRef> effect(grEffect);
     if (NULL != effect.get()) {
         grPaint->addColorEffect(effect);
         // Now setup the rest of the paint.