Add effect caching to distance field text.

Add effect caching to distance field text.

This also is a step towards unifying GrDistanceFieldTextureEffect and GrDistanceFieldLCDTextureEffect.

Committed: https://skia.googlesource.com/skia/+/137bac067306c5446bc4f9797bedc3bbaf302822

Committed: https://skia.googlesource.com/skia/+/78f0718f4dac8bdcc0df43a3280cf8a89d8cf87a

[robertphillips, 2014-07-29 14:01:06]

lgtm + suggestions

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
File src/gpu/GrDistanceFieldTextContext.cpp (right):

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:69: fEffectTextureUniqueID =
SK_InvalidUniqueID;
fEffectColor = XXX; ?

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:132: 
textureUniqueID ?

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:142: }
Could this be a helper method like "setupCoverageEffect" or "updateCachedEffect"
?

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
File src/gpu/GrDistanceFieldTextContext.h (right):

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.h:36: bool                   
fEnableDFRendering;
Weak suggestion: wrap this in a nameless struct?

https://codereview.chromium.org/424103002/diff/20001/src/gpu/effects/GrDistan...
File src/gpu/effects/GrDistanceFieldTextureEffect.h (right):

https://codereview.chromium.org/424103002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.h:25:
kRectToRect_DistanceFieldEffectFlag,
// The subset of the flags relevant to GrDistanceFieldTextureEffect ?

https://codereview.chromium.org/424103002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.h:26:
kNonLCD_DistanceFieldEffectMask       = kSimilarity_DistanceFieldEffectFlag,
// The subset of the flags relevant to GrDistanceFieldLCDTextureEffect ?

[jvanverth1, 2014-07-29 14:43:42]

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
File src/gpu/GrDistanceFieldTextContext.cpp (right):

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:69: fEffectTextureUniqueID =
SK_InvalidUniqueID;
On 2014/07/29 14:01:06, robertphillips wrote:
> fEffectColor = XXX; ?

Done.

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:132: 
On 2014/07/29 14:01:06, robertphillips wrote:
> textureUniqueID ?

Done.

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.cpp:142: }
On 2014/07/29 14:01:06, robertphillips wrote:
> Could this be a helper method like "setupCoverageEffect" or
"updateCachedEffect"
> ?

Done.

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
File src/gpu/GrDistanceFieldTextContext.h (right):

https://codereview.chromium.org/424103002/diff/20001/src/gpu/GrDistanceFieldT...
src/gpu/GrDistanceFieldTextContext.h:36: bool                   
fEnableDFRendering;
On 2014/07/29 14:01:06, robertphillips wrote:
> Weak suggestion: wrap this in a nameless struct?

Weakly ignored. :)

https://codereview.chromium.org/424103002/diff/20001/src/gpu/effects/GrDistan...
File src/gpu/effects/GrDistanceFieldTextureEffect.h (right):

https://codereview.chromium.org/424103002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.h:25:
kRectToRect_DistanceFieldEffectFlag,
On 2014/07/29 14:01:06, robertphillips wrote:
> // The subset of the flags relevant to GrDistanceFieldTextureEffect ?

Done.

https://codereview.chromium.org/424103002/diff/20001/src/gpu/effects/GrDistan...
src/gpu/effects/GrDistanceFieldTextureEffect.h:26:
kNonLCD_DistanceFieldEffectMask       = kSimilarity_DistanceFieldEffectFlag,
On 2014/07/29 14:01:06, robertphillips wrote:
> // The subset of the flags relevant to GrDistanceFieldLCDTextureEffect ?

Done.

[jvanverth1, 2014-07-29 14:43:45]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-07-29 14:43:58]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/jvanverth@google.com/424103002/40001

[jvanverth1, 2014-07-29 15:13:00]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-07-29 15:14:02]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/jvanverth@google.com/424103002/60001

[commit-bot: I haz the power, 2014-07-29 15:29:54]

Change committed as 137bac067306c5446bc4f9797bedc3bbaf302822

[benchen, 2014-07-29 17:35:29]

A revert of this CL has been created in
https://codereview.chromium.org/424173002/ by bensong@google.com.

The reason for reverting is: breaking Chrome canary..

[jvanverth1, 2014-07-29 19:47:03]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-07-29 19:47:45]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/jvanverth@google.com/424103002/80001

[jvanverth1, 2014-07-29 19:53:49]

The CQ bit was unchecked by jvanverth@google.com

[jvanverth1, 2014-07-30 12:54:06]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-07-30 12:55:06]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/jvanverth@google.com/424103002/80001

[commit-bot: I haz the power, 2014-07-30 12:55:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-30 12:55:08]

Failed to apply patch for src/gpu/effects/GrDistanceFieldTextureEffect.cpp:
While running git apply --index -p1;
  <stdin>:115: trailing whitespace.
          
  error: patch failed: src/gpu/effects/GrDistanceFieldTextureEffect.cpp:393
  error: src/gpu/effects/GrDistanceFieldTextureEffect.cpp: patch does not apply

Patch:       src/gpu/effects/GrDistanceFieldTextureEffect.cpp
Index: src/gpu/effects/GrDistanceFieldTextureEffect.cpp
diff --git a/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
b/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
index
0c0c1afc769ac15f2355e85c46deb83427b19c20..0ea6c677200566355d9920616c36cf9c27ecbe55
100755
--- a/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
+++ b/src/gpu/effects/GrDistanceFieldTextureEffect.cpp
@@ -79,7 +79,7 @@ public:
         builder->fsCodeAppendf("\tvec2 uv = %s;\n", fsCoordName.c_str());
         builder->fsCodeAppendf("\tvec2 st = uv*%s;\n", textureSizeUniName);
         builder->fsCodeAppend("\tfloat afwidth;\n");
-        if (dfTexEffect.isSimilarity()) {
+        if (dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag) {
             // this gives us a smooth step across approximately one fragment
             builder->fsCodeAppend("\tafwidth = " SK_DistanceFieldAAFactor
"*dFdx(st.x);\n");
         } else {
@@ -153,7 +153,7 @@ public:
         const GrDistanceFieldTextureEffect& dfTexEffect =
                                              
drawEffect.castEffect<GrDistanceFieldTextureEffect>();
 
-        b->add32(dfTexEffect.isSimilarity());
+        b->add32(dfTexEffect.getFlags());
     }
 
 private:
@@ -174,13 +174,14 @@
GrDistanceFieldTextureEffect::GrDistanceFieldTextureEffect(GrTexture* texture,
                                                            const
GrTextureParams& gammaParams,
                                                            float luminance,
 #endif
-                                                           bool similarity)
+                                                           uint32_t flags)
     : fTextureAccess(texture, params)
 #ifdef SK_GAMMA_APPLY_TO_A8
     , fGammaTextureAccess(gamma, gammaParams)
     , fLuminance(luminance)
 #endif
-    , fIsSimilarity(similarity) {
+    , fFlags(flags & kNonLCD_DistanceFieldEffectMask) {
+    SkASSERT(!(flags & ~kNonLCD_DistanceFieldEffectMask));
     this->addTextureAccess(&fTextureAccess);
 #ifdef SK_GAMMA_APPLY_TO_A8
     this->addTextureAccess(&fGammaTextureAccess);
@@ -190,7 +191,12 @@
GrDistanceFieldTextureEffect::GrDistanceFieldTextureEffect(GrTexture* texture,
 
 bool GrDistanceFieldTextureEffect::onIsEqual(const GrEffect& other) const {
     const GrDistanceFieldTextureEffect& cte =
CastEffect<GrDistanceFieldTextureEffect>(other);
-    return fTextureAccess == cte.fTextureAccess;
+    return fTextureAccess == cte.fTextureAccess &&
+#ifdef SK_GAMMA_APPLY_TO_A8
+           fGammaTextureAccess == cte.fGammaTextureAccess &&
+           fLuminance == cte.fLuminance &&
+#endif
+           fFlags == cte.fFlags;
 }
 
 void GrDistanceFieldTextureEffect::getConstantColorComponents(GrColor* color,
@@ -242,7 +248,8 @@ GrEffect* GrDistanceFieldTextureEffect::TestCreate(SkRandom*
random,
                                                 textures[texIdx2], params2,
                                                 random->nextF(),
 #endif
-                                                random->nextBool());
+                                                random->nextBool() ?
+                                                   
kSimilarity_DistanceFieldEffectFlag : 0);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -286,7 +293,8 @@ public:
         // create LCD offset adjusted by inverse of transform
         builder->fsCodeAppendf("\tvec2 uv = %s;\n", fsCoordName.c_str());
         builder->fsCodeAppendf("\tvec2 st = uv*%s.xy;\n", textureSizeUniName);
-        if (dfTexEffect.isUniformScale()) {
+        bool isUniformScale = !!(dfTexEffect.getFlags() &
kUniformScale_DistanceFieldEffectMask);
+        if (isUniformScale) {
             builder->fsCodeAppend("\tfloat dx = dFdx(st.x);\n");
             builder->fsCodeAppendf("\tvec2 offset = vec2(dx*%s.z, 0.0);\n",
textureSizeUniName);
         } else {
@@ -327,7 +335,7 @@ public:
         // transformations, and even then using a single factor seems like a
reasonable
         // trade-off between quality and speed.
         builder->fsCodeAppend("\tfloat afwidth;\n");
-        if (dfTexEffect.isUniformScale()) {
+        if (isUniformScale) {
             // this gives us a smooth step across approximately one fragment
             builder->fsCodeAppend("\tafwidth = " SK_DistanceFieldAAFactor
"*dx;\n");
         } else {
@@ -393,7 +401,7 @@ public:
             texture->height() != fTextureSize.height()) {
             fTextureSize = SkISize::Make(texture->width(), texture->height());
             float delta = 1.0f/(3.0f*texture->width());
-            if (dfTexEffect.useBGR()) {
+            if (dfTexEffect.getFlags() & kBGR_DistanceFieldEffectFlag) {
                 delta = -delta;
             }
             uman.set3f(fTextureSizeUni,
@@ -418,7 +426,7 @@ public:
         const GrDistanceFieldLCDTextureEffect& dfTexEffect =
                                           
drawEffect.castEffect<GrDistanceFieldLCDTextureEffect>();
 
-        b->add32(dfTexEffect.isUniformScale());
+        b->add32(dfTexEffect.getFlags());
     }
 
 private:
@@ -436,12 +444,13 @@
GrDistanceFieldLCDTextureEffect::GrDistanceFieldLCDTextureEffect(
                                                   GrTexture* texture, const
GrTextureParams& params,
                                                   GrTexture* gamma, const
GrTextureParams& gParams,
                                                   SkColor textColor,
-                                                  bool uniformScale, bool
useBGR)
+                                                  uint32_t flags)
     : fTextureAccess(texture, params)
     , fGammaTextureAccess(gamma, gParams)
     , fTextColor(textColor)
-    , fUniformScale(uniformScale)
-    , fUseBGR(useBGR) {
+    , fFlags(flags & kLCD_DistanceFieldEffectMask) {
+    SkASSERT(!(flags & ~kLCD_DistanceFieldEffectMask) && (flags &
kUseLCD_DistanceFieldEffectFlag));
+        
     this->addTextureAccess(&fTextureAccess);
     this->addTextureAccess(&fGammaTextureAccess);
     this->addVertexAttrib(kVec2f_GrSLType);
@@ -450,7 +459,10 @@
GrDistanceFieldLCDTextureEffect::GrDistanceFieldLCDTextureEffect(
 bool GrDistanceFieldLCDTextureEffect::onIsEqual(const GrEffect& other) const {
     const GrDistanceFieldLCDTextureEffect& cte = 
                                            
CastEffect<GrDistanceFieldLCDTextureEffect>(other);
-    return (fTextureAccess == cte.fTextureAccess && fGammaTextureAccess ==
cte.fGammaTextureAccess);
+    return (fTextureAccess == cte.fTextureAccess &&
+            fGammaTextureAccess == cte.fGammaTextureAccess &&
+            fTextColor == cte.fTextColor &&
+            fFlags == cte.fFlags);
 }
 
 void GrDistanceFieldLCDTextureEffect::getConstantColorComponents(GrColor*
color,
@@ -496,8 +508,11 @@ GrEffect*
GrDistanceFieldLCDTextureEffect::TestCreate(SkRandom* random,
                                         random->nextULessThan(256),
                                         random->nextULessThan(256),
                                         random->nextULessThan(256));
+    uint32_t flags = kUseLCD_DistanceFieldEffectFlag;
+    flags |= random->nextBool() ? kUniformScale_DistanceFieldEffectMask : 0;
+    flags |= random->nextBool() ? kBGR_DistanceFieldEffectFlag : 0;
     return GrDistanceFieldLCDTextureEffect::Create(textures[texIdx], params,
                                                    textures[texIdx2], params2,
                                                    textColor,
-                                                   random->nextBool(),
random->nextBool());
+                                                   flags);
 }

[jvanverth1, 2014-07-30 13:05:59]

The CQ bit was checked by jvanverth@google.com

[commit-bot: I haz the power, 2014-07-30 13:06:06]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/jvanverth@google.com/424103002/100001

[commit-bot: I haz the power, 2014-07-30 13:18:04]

Change committed as 78f0718f4dac8bdcc0df43a3280cf8a89d8cf87a