Revert of Calculate inverse scale for distance fields in vertex shader (patchset...
authorjvanverth <jvanverth@google.com>
Mon, 6 Apr 2015 20:58:59 +0000 (13:58 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 6 Apr 2015 20:58:59 +0000 (13:58 -0700)
Reason for revert:
This appears to be breaking large text on Nexus 7.

Original issue's description:
> Calculate inverse scale for distance field text in vertex shader
>
> This is for the uniform scale case only. Using the dFdx() function on certain
> Mali GPUs causes issues because the precision is too low, so we have to
> compute 1/scale from the view matrix instead.
>
> BUG=skia:3528
>
> Committed: https://skia.googlesource.com/skia/+/5b143038cb47763974d2750ed78d436eb6c38bea

TBR=bsalomon@google.com,joshualitt@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:3528

Review URL: https://codereview.chromium.org/1066613003

src/gpu/GrDistanceFieldTextContext.cpp
src/gpu/effects/GrDistanceFieldTextureEffect.cpp
src/gpu/effects/GrDistanceFieldTextureEffect.h

index 66c97c7..1fe4320 100755 (executable)
@@ -271,12 +271,6 @@ inline void GrDistanceFieldTextContext::init(GrRenderTarget* rt, const GrClip& c
     fSkPaint.setAutohinted(false);
     fSkPaint.setHinting(SkPaint::kNormal_Hinting);
     fSkPaint.setSubpixelText(true);
-    
-    // fix for skia:3528
-    // if we're scaling up, include any scaling to match text size in the view matrix
-    if (fTextRatio > 1.0f) {
-        fViewMatrix.preScale(fTextRatio, fTextRatio);
-    }
 }
 
 void GrDistanceFieldTextContext::onDrawText(GrRenderTarget* rt, const GrClip& clip,
@@ -489,24 +483,13 @@ void GrDistanceFieldTextContext::setupCoverageEffect(const SkColor& filteredColo
     kRectToRect_DistanceFieldEffectFlag : 0;
     bool useBGR = SkPixelGeometryIsBGR(fDeviceProperties.pixelGeometry());
     flags |= fUseLCDText && useBGR ? kBGR_DistanceFieldEffectFlag : 0;
-
-    // fix for skia:3528
-    // set the local matrix to correct any text size scaling for gradients et al.
-    SkMatrix localMatrix;
-    if (fTextRatio > 1.0f) {
-        localMatrix.setScale(fTextRatio, fTextRatio);
-    } else {
-        localMatrix.reset();
-    }
-
+    
     // see if we need to create a new effect
     if (textureUniqueID != fEffectTextureUniqueID ||
         filteredColor != fEffectColor ||
         flags != fEffectFlags ||
-        !fCachedGeometryProcessor->viewMatrix().cheapEqualTo(fViewMatrix) ||
-        !fCachedGeometryProcessor->localMatrix().cheapEqualTo(localMatrix)) {
+        !fCachedGeometryProcessor->viewMatrix().cheapEqualTo(fViewMatrix)) {
         GrColor color = fPaint.getColor();
-
         if (fUseLCDText) {
             GrColor colorNoPreMul = skcolor_to_grcolor_nopremultiply(filteredColor);
 
@@ -522,7 +505,6 @@ void GrDistanceFieldTextContext::setupCoverageEffect(const SkColor& filteredColo
                                                                    blueCorrection);
             fCachedGeometryProcessor.reset(GrDistanceFieldLCDTextureEffect::Create(color,
                                                                                    fViewMatrix,
-                                                                                   localMatrix,
                                                                                    fCurrTexture,
                                                                                    params,
                                                                                    widthAdjust,
@@ -536,7 +518,6 @@ void GrDistanceFieldTextContext::setupCoverageEffect(const SkColor& filteredColo
             float correction = fDistanceAdjustTable[lum >> kDistanceAdjustLumShift];
             fCachedGeometryProcessor.reset(GrDistanceFieldTextureEffect::Create(color,
                                                                                 fViewMatrix,
-                                                                                localMatrix,
                                                                                 fCurrTexture,
                                                                                 params,
                                                                                 correction,
@@ -545,7 +526,6 @@ void GrDistanceFieldTextContext::setupCoverageEffect(const SkColor& filteredColo
 #else
             fCachedGeometryProcessor.reset(GrDistanceFieldTextureEffect::Create(color,
                                                                                 fViewMatrix,
-                                                                                localMatrix,
                                                                                 fCurrTexture,
                                                                                 params,
                                                                                 flags,
@@ -624,18 +604,12 @@ bool GrDistanceFieldTextContext::appendGlyph(GrGlyph::PackedID packed,
     SkScalar height = SkIntToScalar(glyph->fBounds.height() - 2*SK_DistanceFieldInset);
 
     SkScalar scale = fTextRatio;
-    // if we're scaling up, using fix for skia:3528
-    if (scale > 1.0f) {
-        sx /= scale;
-        sy /= scale;
-    } else {
-        dx *= scale;
-        dy *= scale;
-        width *= scale;
-        height *= scale;
-    }
+    dx *= scale;
+    dy *= scale;
     sx += dx;
     sy += dy;
+    width *= scale;
+    height *= scale;
     SkRect glyphRect = SkRect::MakeXYWH(sx, sy, width, height);
 
     // check if we clipped out
@@ -669,6 +643,7 @@ bool GrDistanceFieldTextContext::appendGlyph(GrGlyph::PackedID packed,
             this->flush();
 
             SkMatrix ctm;
+            ctm.setScale(fTextRatio, fTextRatio);
             ctm.postTranslate(sx - dx, sy - dy);
 
             SkPath tmpPath(*glyph->fPath);
index 13ca052..d44c193 100755 (executable)
@@ -51,6 +51,16 @@ public:
         // emit attributes
         vsBuilder->emitAttributes(dfTexEffect);
 
+        GrGLVertToFrag st(kVec2f_GrSLType);
+        args.fPB->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
+        vsBuilder->codeAppendf("%s = %s;", st.vsOut(), dfTexEffect.inTextureCoords()->fName);
+        
+        GrGLVertToFrag uv(kVec2f_GrSLType);
+        args.fPB->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
+        // this is only used with text, so our texture bounds always match the glyph atlas
+        vsBuilder->codeAppendf("%s = vec2(" GR_FONT_ATLAS_A8_RECIP_WIDTH ", "
+                               GR_FONT_ATLAS_RECIP_HEIGHT ")*%s;", uv.vsOut(),
+                               dfTexEffect.inTextureCoords()->fName);
 #ifdef SK_GAMMA_APPLY_TO_A8
         // adjust based on gamma
         const char* distanceAdjustUniName = NULL;
@@ -68,36 +78,9 @@ public:
         this->setupPosition(pb, gpArgs, dfTexEffect.inPosition()->fName, dfTexEffect.viewMatrix());
 
         // emit transforms
-        const SkMatrix& localMatrix = dfTexEffect.localMatrix();
         this->emitTransforms(args.fPB, gpArgs->fPositionVar, dfTexEffect.inPosition()->fName,
-                             localMatrix, args.fTransformsIn, args.fTransformsOut);
-
-        // add varyings
-        GrGLVertToFrag recipScale(kFloat_GrSLType);
-        GrGLVertToFrag st(kVec2f_GrSLType);
-        bool isSimilarity = SkToBool(dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag);
-        const char* viewMatrixName = this->uViewM();
-        // view matrix name is NULL if identity matrix
-        bool useInverseScale = !localMatrix.isIdentity() && viewMatrixName;
-        if (isSimilarity && useInverseScale) {
-            args.fPB->addVarying("RecipScale", &recipScale, kHigh_GrSLPrecision);
-            vsBuilder->codeAppendf("vec2 tx = vec2(%s[0][0], %s[1][0]);",
-                                   viewMatrixName, viewMatrixName);
-            vsBuilder->codeAppend("float tx2 = dot(tx, tx);");
-            vsBuilder->codeAppendf("%s = inversesqrt(tx2);", recipScale.vsOut());
-        } else {
-            args.fPB->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
-            vsBuilder->codeAppendf("%s = %s;", st.vsOut(), dfTexEffect.inTextureCoords()->fName);
-        }
+                             dfTexEffect.localMatrix(), args.fTransformsIn, args.fTransformsOut);
 
-        GrGLVertToFrag uv(kVec2f_GrSLType);
-        args.fPB->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
-        // this is only used with text, so our texture bounds always match the glyph atlas
-        vsBuilder->codeAppendf("%s = vec2(" GR_FONT_ATLAS_A8_RECIP_WIDTH ", "
-                               GR_FONT_ATLAS_RECIP_HEIGHT ")*%s;", uv.vsOut(),
-                               dfTexEffect.inTextureCoords()->fName);
-        
-        
         // Use highp to work around aliasing issues
         fsBuilder->codeAppend(GrGLShaderVar::PrecisionString(kHigh_GrSLPrecision,
                                                              pb->ctxInfo().standard()));
@@ -117,23 +100,16 @@ public:
 
         fsBuilder->codeAppend(GrGLShaderVar::PrecisionString(kHigh_GrSLPrecision,
                                                              pb->ctxInfo().standard()));
+        fsBuilder->codeAppendf("vec2 st = %s;", st.fsIn());
         fsBuilder->codeAppend("float afwidth;");
-        if (isSimilarity) {
+        if (dfTexEffect.getFlags() & kSimilarity_DistanceFieldEffectFlag) {
             // For uniform scale, we adjust for the effect of the transformation on the distance
-            // either by using the inverse scale in the view matrix, or (if there is no view matrix)
             // by using the length of the gradient of the texture coordinates. We use st coordinates
-            // with the latter to ensure we're mapping 1:1 from texel space to pixel space.
+            // to ensure we're mapping 1:1 from texel space to pixel space.
 
             // this gives us a smooth step across approximately one fragment
-            if (useInverseScale) {
-                fsBuilder->codeAppendf("afwidth = abs(" SK_DistanceFieldAAFactor "*%s);",
-                                       recipScale.fsIn());
-            } else {
-                fsBuilder->codeAppendf("afwidth = abs(" SK_DistanceFieldAAFactor "*dFdx(%s.x));",
-                                       st.fsIn());
-            }
+            fsBuilder->codeAppend("afwidth = abs(" SK_DistanceFieldAAFactor "*dFdx(st.x));");
         } else {
-            fsBuilder->codeAppendf("vec2 st = %s;", st.fsIn());
             // For general transforms, to determine the amount of correction we multiply a unit
             // vector pointing along the SDF gradient direction by the Jacobian of the st coords
             // (which is the inverse transform for this fragment) and take the length of the result.
@@ -194,7 +170,6 @@ public:
         key |= local.fInputColorType << 16;
         key |= local.fUsesLocalCoords && gp.localMatrix().hasPerspective() ? 0x1 << 24: 0x0;
         key |= ComputePosKey(gp.viewMatrix()) << 25;
-        key |= (!gp.viewMatrix().isIdentity() && !gp.localMatrix().isIdentity()) ? 0x1 << 27 : 0x0;
         b->add32(key);
     }
 
@@ -213,14 +188,13 @@ private:
 
 GrDistanceFieldTextureEffect::GrDistanceFieldTextureEffect(GrColor color,
                                                            const SkMatrix& viewMatrix,
-                                                           const SkMatrix& localMatrix,
                                                            GrTexture* texture,
                                                            const GrTextureParams& params,
 #ifdef SK_GAMMA_APPLY_TO_A8
                                                            float distanceAdjust,
 #endif
                                                            uint32_t flags, bool opaqueVertexColors)
-    : INHERITED(color, viewMatrix, localMatrix, opaqueVertexColors)
+    : INHERITED(color, viewMatrix, SkMatrix::I(), opaqueVertexColors)
     , fTextureAccess(texture, params)
 #ifdef SK_GAMMA_APPLY_TO_A8
     , fDistanceAdjust(distanceAdjust)
@@ -307,7 +281,6 @@ GrGeometryProcessor* GrDistanceFieldTextureEffect::TestCreate(SkRandom* random,
 
     return GrDistanceFieldTextureEffect::Create(GrRandomColor(random),
                                                 GrProcessorUnitTest::TestMatrix(random),
-                                                GrProcessorUnitTest::TestMatrix(random),
                                                 textures[texIdx], params,
 #ifdef SK_GAMMA_APPLY_TO_A8
                                                 random->nextF(),
@@ -591,6 +564,17 @@ public:
         // emit attributes
         vsBuilder->emitAttributes(dfTexEffect);
 
+        GrGLVertToFrag st(kVec2f_GrSLType);
+        args.fPB->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
+        vsBuilder->codeAppendf("%s = %s;", st.vsOut(), dfTexEffect.inTextureCoords()->fName);
+        
+        GrGLVertToFrag uv(kVec2f_GrSLType);
+        args.fPB->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
+        // this is only used with text, so our texture bounds always match the glyph atlas
+        vsBuilder->codeAppendf("%s = vec2(" GR_FONT_ATLAS_A8_RECIP_WIDTH ", "
+                               GR_FONT_ATLAS_RECIP_HEIGHT ")*%s;", uv.vsOut(),
+                               dfTexEffect.inTextureCoords()->fName);
+        
         // setup pass through color
         this->setupColorPassThrough(pb, local.fInputColorType, args.fOutputColor, NULL,
                                     &fColorUniform);
@@ -599,36 +583,9 @@ public:
         this->setupPosition(pb, gpArgs, dfTexEffect.inPosition()->fName, dfTexEffect.viewMatrix());
 
         // emit transforms
-        const SkMatrix& localMatrix = dfTexEffect.localMatrix();
         this->emitTransforms(args.fPB, gpArgs->fPositionVar, dfTexEffect.inPosition()->fName,
-                             localMatrix, args.fTransformsIn, args.fTransformsOut);
-
-        // set up varyings
-        bool isUniformScale = SkToBool(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
-        GrGLVertToFrag recipScale(kFloat_GrSLType);
-        GrGLVertToFrag st(kVec2f_GrSLType);
-        const char* viewMatrixName = this->uViewM();
-        // view matrix name is NULL if identity matrix
-        bool useInverseScale = !localMatrix.isIdentity() && viewMatrixName;
-        if (isUniformScale && useInverseScale) {
-            args.fPB->addVarying("RecipScale", &recipScale, kHigh_GrSLPrecision);
-            vsBuilder->codeAppendf("vec2 tx = vec2(%s[0][0], %s[1][0]);",
-                                   viewMatrixName, viewMatrixName);
-            vsBuilder->codeAppend("float tx2 = dot(tx, tx);");
-            vsBuilder->codeAppendf("%s = inversesqrt(tx2);", recipScale.vsOut());
-        } else {
-            args.fPB->addVarying("IntTextureCoords", &st, kHigh_GrSLPrecision);
-            vsBuilder->codeAppendf("%s = %s;", st.vsOut(), dfTexEffect.inTextureCoords()->fName);
-        }
-
-        GrGLVertToFrag uv(kVec2f_GrSLType);
-        args.fPB->addVarying("TextureCoords", &uv, kHigh_GrSLPrecision);
-        // this is only used with text, so our texture bounds always match the glyph atlas
-        vsBuilder->codeAppendf("%s = vec2(" GR_FONT_ATLAS_A8_RECIP_WIDTH ", "
-                               GR_FONT_ATLAS_RECIP_HEIGHT ")*%s;", uv.vsOut(),
-                               dfTexEffect.inTextureCoords()->fName);
+                             dfTexEffect.localMatrix(), args.fTransformsIn, args.fTransformsOut);
 
-        // add frag shader code
         GrGLGPFragmentBuilder* fsBuilder = args.fPB->getFragmentShaderBuilder();
 
         SkAssertResult(fsBuilder->enableFeature(
@@ -641,24 +598,21 @@ public:
         fsBuilder->codeAppendf("vec2 uv = %s;\n", uv.fsIn());
         fsBuilder->codeAppend(GrGLShaderVar::PrecisionString(kHigh_GrSLPrecision,
                                                              pb->ctxInfo().standard()));
+        fsBuilder->codeAppendf("vec2 st = %s;\n", st.fsIn());
+        bool isUniformScale = !!(dfTexEffect.getFlags() & kUniformScale_DistanceFieldEffectMask);
+        
         if (dfTexEffect.getFlags() & kBGR_DistanceFieldEffectFlag) {
             fsBuilder->codeAppend("float delta = -" GR_FONT_ATLAS_LCD_DELTA ";\n");
         } else {
             fsBuilder->codeAppend("float delta = " GR_FONT_ATLAS_LCD_DELTA ";\n");
         }
         if (isUniformScale) {
-            if (useInverseScale) {
-                fsBuilder->codeAppendf("float dx = %s;", recipScale.fsIn());
-            } else {
-                fsBuilder->codeAppendf("float dx = dFdx(%s.x);", st.fsIn());
-            }
-            fsBuilder->codeAppend("vec2 offset = vec2(dx*delta, 0.0);");
+            fsBuilder->codeAppend("\tfloat dx = dFdx(st.x);\n");
+            fsBuilder->codeAppend("\tvec2 offset = vec2(dx*delta, 0.0);\n");
         } else {
-            fsBuilder->codeAppendf("vec2 st = %s;\n", st.fsIn());
-
-            fsBuilder->codeAppend("vec2 Jdx = dFdx(st);");
-            fsBuilder->codeAppend("vec2 Jdy = dFdy(st);");
-            fsBuilder->codeAppend("vec2 offset = delta*Jdx;");
+            fsBuilder->codeAppend("\tvec2 Jdx = dFdx(st);\n");
+            fsBuilder->codeAppend("\tvec2 Jdy = dFdy(st);\n");
+            fsBuilder->codeAppend("\tvec2 offset = delta*Jdx;\n");
         }
 
         // green is distance to uv center
@@ -767,7 +721,6 @@ public:
         key |= local.fInputColorType << 16;
         key |= local.fUsesLocalCoords && gp.localMatrix().hasPerspective() ? 0x1 << 24: 0x0;
         key |= ComputePosKey(gp.viewMatrix()) << 25;
-        key |= (!gp.viewMatrix().isIdentity() && !gp.localMatrix().isIdentity()) ? 0x1 << 27 : 0x0;
         b->add32(key);
     }
 
@@ -784,11 +737,10 @@ private:
 
 GrDistanceFieldLCDTextureEffect::GrDistanceFieldLCDTextureEffect(
                                                   GrColor color, const SkMatrix& viewMatrix,
-                                                  const SkMatrix& localMatrix,
                                                   GrTexture* texture, const GrTextureParams& params,
                                                   DistanceAdjust distanceAdjust,
                                                   uint32_t flags)
-    : INHERITED(color, viewMatrix, localMatrix)
+    : INHERITED(color, viewMatrix, SkMatrix::I())
     , fTextureAccess(texture, params)
     , fDistanceAdjust(distanceAdjust)
     , fFlags(flags & kLCD_DistanceFieldEffectMask){
@@ -869,7 +821,6 @@ GrGeometryProcessor* GrDistanceFieldLCDTextureEffect::TestCreate(SkRandom* rando
     flags |= random->nextBool() ? kBGR_DistanceFieldEffectFlag : 0;
     return GrDistanceFieldLCDTextureEffect::Create(GrRandomColor(random),
                                                    GrProcessorUnitTest::TestMatrix(random),
-                                                   GrProcessorUnitTest::TestMatrix(random),
                                                    textures[texIdx], params,
                                                    wa,
                                                    flags);
index 58d1b77..6be7f9e 100644 (file)
@@ -47,20 +47,18 @@ enum GrDistanceFieldEffectFlags {
 class GrDistanceFieldTextureEffect : public GrGeometryProcessor {
 public:
 #ifdef SK_GAMMA_APPLY_TO_A8
-    static GrGeometryProcessor* Create(GrColor color, const SkMatrix& viewMatrix,
-                                       const SkMatrix& localMatrix,
-                                       GrTexture* tex, const GrTextureParams& params,
+    static GrGeometryProcessor* Create(GrColor color, const SkMatrix& viewMatrix, GrTexture* tex,
+                                       const GrTextureParams& params,
                                        float lum, uint32_t flags, bool opaqueVertexColors) {
-        return SkNEW_ARGS(GrDistanceFieldTextureEffect, (color, viewMatrix, localMatrix, tex,
-                                                         params, lum, flags, opaqueVertexColors));
+       return SkNEW_ARGS(GrDistanceFieldTextureEffect, (color, viewMatrix, tex, params, lum,
+                                                        flags, opaqueVertexColors));
     }
 #else
-    static GrGeometryProcessor* Create(GrColor color, const SkMatrix& viewMatrix,
-                                       const SkMatrix& localMatrix,
-                                       GrTexture* tex, const GrTextureParams& params,
+    static GrGeometryProcessor* Create(GrColor color, const SkMatrix& viewMatrix, GrTexture* tex,
+                                       const GrTextureParams& params,
                                        uint32_t flags, bool opaqueVertexColors) {
-        return SkNEW_ARGS(GrDistanceFieldTextureEffect, (color, viewMatrix, localMatrix, tex,
-                                                         params, flags, opaqueVertexColors));
+        return  SkNEW_ARGS(GrDistanceFieldTextureEffect, (color, viewMatrix, tex, params, flags,
+                                                          opaqueVertexColors));
     }
 #endif
 
@@ -90,8 +88,8 @@ public:
                         const GrBatchTracker&) const override;
 
 private:
-    GrDistanceFieldTextureEffect(GrColor, const SkMatrix& viewMatrix, const SkMatrix& localMatrix,
-                                 GrTexture* texture, const GrTextureParams& params,
+    GrDistanceFieldTextureEffect(GrColor, const SkMatrix& viewMatrix, GrTexture* texture,
+                                 const GrTextureParams& params,
 #ifdef SK_GAMMA_APPLY_TO_A8
                                  float distanceAdjust,
 #endif
@@ -196,12 +194,11 @@ public:
         }
     };
 
-    static GrGeometryProcessor* Create(GrColor color, const SkMatrix& viewMatrix,
-                                       const SkMatrix& localMatrix,
-                                       GrTexture* tex, const GrTextureParams& params,
+    static GrGeometryProcessor* Create(GrColor color, const SkMatrix& viewMatrix, GrTexture* tex,
+                                       const GrTextureParams& params, 
                                        DistanceAdjust distanceAdjust, uint32_t flags) {
         return SkNEW_ARGS(GrDistanceFieldLCDTextureEffect,
-                          (color, viewMatrix, localMatrix, tex, params, distanceAdjust, flags));
+                          (color, viewMatrix, tex, params, distanceAdjust, flags));
     }
 
     virtual ~GrDistanceFieldLCDTextureEffect() {}
@@ -227,9 +224,8 @@ public:
                         const GrBatchTracker&) const override;
 
 private:
-    GrDistanceFieldLCDTextureEffect(GrColor, const SkMatrix& viewMatrix,
-                                    const SkMatrix& localMatrix,
-                                    GrTexture* texture, const GrTextureParams& params,
+    GrDistanceFieldLCDTextureEffect(GrColor, const SkMatrix& viewMatrix, GrTexture* texture,
+                                    const GrTextureParams& params,
                                     DistanceAdjust wa, uint32_t flags);
 
     bool onIsEqual(const GrGeometryProcessor& other) const override;