SkShader::asNewEffect Refactoring

src/gpu/SkGr.cpp

 /*
  * Copyright 2010 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkGr.h"
 #include "SkColorFilter.h"
 #include "SkConfig8888.h"
@@ skipping 412 matching lines @@
             dm = SkXfermode::kZero_Coeff;
         }
     } else {
         //SkDEBUGCODE(SkDebugf("Unsupported xfer mode.\n");)
         // Fall back to src-over
         sm = SkXfermode::kOne_Coeff;
         dm = SkXfermode::kISA_Coeff;
     }
     grPaint->setBlendFunc(sk_blend_to_grblend(sm), sk_blend_to_grblend(dm));
 
     if (justAlpha) {

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

Can we kill this?

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

I removed this by modifying the arguments that this method receives, instead of
a bool justAlpha now it gets a GrColor that is the one that is set for the
GrPaint.
The caller is in charge of creating and supplying the color.

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

         uint8_t alpha = skPaint.getAlpha();
         grPaint->setColor(GrColorPackRGBA(alpha, alpha, alpha, alpha));
         // justAlpha is currently set to true only if there is a texture,
         // so constantColor should not also be true.
         SkASSERT(!constantColor);
     } else {
         grPaint->setColor(SkColor2GrColor(skPaint.getColor()));
     }
 
     SkColorFilter* colorFilter = skPaint.getColorFilter();
@@ skipping 41 matching lines @@
     }
 
     // SkShader::asNewEffect() may do offscreen rendering. Save off the current RT, clip, and
     // matrix. We don't reset the matrix on the context because SkShader::asNewEffect may use
     // GrContext::getMatrix() to know the transformation from local coords to device space.
     GrContext::AutoRenderTarget art(context, NULL);
     GrContext::AutoClip ac(context, GrContext::AutoClip::kWideOpen_InitialClip);
     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.
         SkPaint2GrPaintNoShader(context, skPaint, true, false, grPaint);
     } else {
         // We still don't have SkColorShader::asNewEffect() implemented.
         SkShader::GradientInfo info;
         SkColor                color;
 
         info.fColors = &color;
         info.fColorOffsets = NULL;
         info.fColorCount = 1;
         if (SkShader::kColor_GradientType == shader->asAGradient(&info)) {
             SkPaint copy(skPaint);
             copy.setShader(NULL);
             // modulate the paint alpha by the shader's solid color alpha
             U8CPU newA = SkMulDiv255Round(SkColorGetA(color), copy.getAlpha());
             copy.setColor(SkColorSetA(color, newA));
             SkPaint2GrPaintNoShader(context, copy, false, constantColor, grPaint);
         } else {
             SkPaint2GrPaintNoShader(context, skPaint, false, constantColor, grPaint);
         }
     }
 }