Revert of third try at landing improved blur rect; this time with more correctness...

Reason for revert:
Failing layout test: https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux/32762/layout-test-results/virtual/gpu/fast/canvas/canvas-draw-canvas-on-canvas-shadow-pretty-diff.html

Original issue's description:
> third try at landing improved blur rect; this time with more correctness
>
> BUG=skia:2095
> R=bsalomon@google.com
> TBR=bsalomon
>
> Committed: https://skia.googlesource.com/skia/+/72abfc2b4e7caead660f6b6a05e60d05eaf1a66f

R=bsalomon@google.com, reed@google.com, humper@google.com
TBR=bsalomon@google.com, humper@google.com, reed@google.com
NOTREECHECKS=true
NOTRY=true
BUG=skia:2095

Author: scroggo@google.com

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

diff --git a/expectations/gm/ignored-tests.txt b/expectations/gm/ignored-tests.txt
index 9d6ce3e..cccd9bf 100644 (file)
--- a/expectations/gm/ignored-tests.txt
+++ b/expectations/gm/ignored-tests.txt
@@ -38,16 +38,6 @@
 # I will likely just delete the GM.
 canvas-layer-state
 
-# humper:
-# Needs rebaselining after faster GPU blur patch lands
-megalooper_0x0
-megalooper_1x4
-megalooper_4x1
-bleed
-blurquickreject
-blurrects
-bigblurs
-
 # These are part of picture-version 27 -- removal of SkUnitMapp
 # just need to be rebaselined
 scaled_tilemode_bitmap

diff --git a/src/effects/SkBlurMaskFilter.cpp b/src/effects/SkBlurMaskFilter.cpp
index 43324f2..e808885 100644 (file)
--- a/src/effects/SkBlurMaskFilter.cpp
+++ b/src/effects/SkBlurMaskFilter.cpp
@@ -557,38 +557,40 @@ public:
      */
     static GrEffectRef* Create(GrContext *context, const SkRect& rect,
                                float sigma) {
-        GrTexture *blurProfileTexture = NULL;
-        int doubleProfileSize = SkScalarCeilToInt(12*sigma);
-
-        if (doubleProfileSize >= rect.width() || doubleProfileSize >= rect.height()) {
-            // if the blur sigma is too large so the gaussian overlaps the whole
-            // rect in either direction, fall back to CPU path for now.
-
+        GrTexture *horizontalScanline = NULL, *verticalScanline = NULL;
+        bool createdScanlines = CreateScanlineTextures(context, sigma,
+                                                       SkScalarCeilToInt(rect.width()),
+                                                       SkScalarCeilToInt(rect.height()),
+                                                       &horizontalScanline, &verticalScanline);
+         SkAutoTUnref<GrTexture> hunref(horizontalScanline), vunref(verticalScanline);
+         if (!createdScanlines) {
             return NULL;
         }
-
-        bool createdBlurProfileTexture = CreateBlurProfileTexture(context, sigma, &blurProfileTexture);
-        SkAutoTUnref<GrTexture> hunref(blurProfileTexture);
-        if (!createdBlurProfileTexture) {
-           return NULL;
-        }
-        AutoEffectUnref effect(SkNEW_ARGS(GrRectBlurEffect, (rect, sigma, blurProfileTexture)));
+        AutoEffectUnref effect(SkNEW_ARGS(GrRectBlurEffect, (rect, sigma,
+                                                             horizontalScanline, verticalScanline)));
         return CreateEffectRef(effect);
     }
 
-    const SkRect& getRect() const { return fRect; }
+    unsigned int getWidth() const { return fWidth; }
+    unsigned int getHeight() const { return fHeight; }
     float getSigma() const { return fSigma; }
+    const GrCoordTransform& getTransform() const { return fTransform; }
 
 private:
-    GrRectBlurEffect(const SkRect& rect, float sigma, GrTexture *blur_profile);
+    GrRectBlurEffect(const SkRect& rect, float sigma,
+                     GrTexture *horizontal_scanline, GrTexture *vertical_scanline);
     virtual bool onIsEqual(const GrEffect&) const SK_OVERRIDE;
 
-    static bool CreateBlurProfileTexture(GrContext *context, float sigma,
-                                       GrTexture **blurProfileTexture);
+    static bool CreateScanlineTextures(GrContext *context, float sigma,
+                                       unsigned int width, unsigned int height,
+                                       GrTexture **horizontalScanline,
+                                       GrTexture **verticalScanline);
 
-    SkRect          fRect;
-    float           fSigma;
-    GrTextureAccess fBlurProfileAccess;
+    unsigned int fWidth, fHeight;
+    float fSigma;
+    GrTextureAccess  fHorizontalScanlineAccess;
+    GrTextureAccess  fVerticalScanlineAccess;
+    GrCoordTransform fTransform;
 
     GR_DECLARE_EFFECT_TEST;
 
@@ -612,31 +614,16 @@ public:
 private:
     typedef GrGLUniformManager::UniformHandle        UniformHandle;
 
-    UniformHandle       fProxyRectUniform;
-    UniformHandle       fProfileSizeUniform;
+    UniformHandle       fWidthUni;
+    UniformHandle       fHeightUni;
 
     typedef GrGLEffect INHERITED;
 };
 
-
-
 GrGLRectBlurEffect::GrGLRectBlurEffect(const GrBackendEffectFactory& factory, const GrDrawEffect&)
     : INHERITED(factory) {
 }
 
-void OutputRectBlurProfileLookup(GrGLShaderBuilder* builder,
-                                 const GrGLShaderBuilder::TextureSampler& sampler,
-                                 const char *output,
-                                 const char *profileSize, const char *loc,
-                                 const char *blurred_width,
-                                 const char *sharp_width) {
-    builder->fsCodeAppendf("\t\tfloat coord = (0.5 * (abs(2.0*%s - %s) - %s))/%s;\n",
-                           loc, blurred_width, sharp_width, profileSize);
-    builder->fsCodeAppendf("\t\t%s = ", output);
-    builder->fsAppendTextureLookup(sampler, "vec2(coord,0.5)");
-    builder->fsCodeAppend(".a;\n");
-}
-
 void GrGLRectBlurEffect::emitCode(GrGLShaderBuilder* builder,
                                  const GrDrawEffect&,
                                  EffectKey key,
@@ -645,19 +632,7 @@ void GrGLRectBlurEffect::emitCode(GrGLShaderBuilder* builder,
                                  const TransformedCoordsArray& coords,
                                  const TextureSamplerArray& samplers) {
 
-    const char *rectName;
-    const char *profileSizeName;
-
-    fProxyRectUniform = builder->addUniform(GrGLShaderBuilder::kFragment_Visibility,
-                                            kVec4f_GrSLType,
-                                            "proxyRect",
-                                            &rectName);
-    fProfileSizeUniform = builder->addUniform(GrGLShaderBuilder::kFragment_Visibility,
-                                            kFloat_GrSLType,
-                                            "profileSize",
-                                            &profileSizeName);
-
-    const char *fragmentPos = builder->fragmentPosition();
+    SkString texture_coords = builder->ensureFSCoords2D(coords, 0);
 
     if (inputColor) {
         builder->fsCodeAppendf("\tvec4 src=%s;\n", inputColor);
@@ -665,46 +640,31 @@ void GrGLRectBlurEffect::emitCode(GrGLShaderBuilder* builder,
         builder->fsCodeAppendf("\tvec4 src=vec4(1)\n;");
     }
 
-    builder->fsCodeAppendf("\tvec2 translatedPos = %s.xy - %s.xy;\n", fragmentPos, rectName );
-    builder->fsCodeAppendf("\tfloat width = %s.z - %s.x;\n", rectName, rectName);
-    builder->fsCodeAppendf("\tfloat height = %s.w - %s.y;\n", rectName, rectName);
-
-    builder->fsCodeAppendf("\tvec2 smallDims = vec2(width - %s, height-%s);\n", profileSizeName, profileSizeName);
-    builder->fsCodeAppendf("\tfloat center = 2.0 * floor(%s/2.0 + .25) - 1.0;\n", profileSizeName);
-    builder->fsCodeAppendf("\tvec2 wh = smallDims - vec2(center,center);\n");
-
-    builder->fsCodeAppendf("\tfloat horiz_lookup;\n");
-    builder->fsCodeAppendf("\tif (%s <= smallDims.x) {\n", profileSizeName);
-    OutputRectBlurProfileLookup(builder, samplers[0], "horiz_lookup", profileSizeName, "translatedPos.x", "width", "wh.x");
-    builder->fsCodeAppendf("\t}\n");
-
-    builder->fsCodeAppendf("\tfloat vert_lookup;\n");
-    builder->fsCodeAppendf("\tif (%s <= smallDims.y) {\n", profileSizeName);
-    OutputRectBlurProfileLookup(builder, samplers[0], "vert_lookup", profileSizeName, "translatedPos.y", "height", "wh.y");
-    builder->fsCodeAppendf("\t}\n");
+    builder->fsCodeAppendf("\tvec4 horiz = ");
+    builder->fsAppendTextureLookup( samplers[0], texture_coords.c_str() );
+    builder->fsCodeAppendf(";\n");
+    builder->fsCodeAppendf("\tvec4 vert = ");
+    builder->fsAppendTextureLookup( samplers[1], texture_coords.c_str() );
+    builder->fsCodeAppendf(";\n");
 
-    builder->fsCodeAppendf("\tfloat final = horiz_lookup * vert_lookup;\n");
-
-    builder->fsCodeAppendf("\t%s = src * vec4(final);\n", outputColor );
+    builder->fsCodeAppendf("\tfloat final = (horiz*vert).r;\n");
+    builder->fsCodeAppendf("\t%s = final*src;\n", outputColor);
 }
 
 void GrGLRectBlurEffect::setData(const GrGLUniformManager& uman,
-                                 const GrDrawEffect& drawEffect) {
-    const GrRectBlurEffect& rbe = drawEffect.castEffect<GrRectBlurEffect>();
-    SkRect rect = rbe.getRect();
-
-    uman.set4f(fProxyRectUniform, rect.fLeft, rect.fTop, rect.fRight, rect.fBottom);
-    uman.set1f(fProfileSizeUniform, SkScalarCeilToScalar(6*rbe.getSigma()));
+                                const GrDrawEffect& drawEffect) {
 }
 
-bool GrRectBlurEffect::CreateBlurProfileTexture(GrContext *context, float sigma,
-                                              GrTexture **blurProfileTexture) {
+bool GrRectBlurEffect::CreateScanlineTextures(GrContext *context, float sigma,
+                                              unsigned int width, unsigned int height,
+                                              GrTexture **horizontalScanline,
+                                              GrTexture **verticalScanline) {
     GrTextureParams params;
     GrTextureDesc texDesc;
 
-    unsigned int profile_size = SkScalarCeilToInt(6*sigma);
+    unsigned int profile_size = SkScalarFloorToInt(6*sigma);
 
-    texDesc.fWidth = profile_size;
+    texDesc.fWidth = width;
     texDesc.fHeight = 1;
     texDesc.fConfig = kAlpha_8_GrPixelConfig;
 
@@ -712,38 +672,73 @@ bool GrRectBlurEffect::CreateBlurProfileTexture(GrContext *context, float sigma,
     GrCacheID::Key key;
     memset(&key, 0, sizeof(key));
     key.fData32[0] = profile_size;
-    key.fData32[1] = 1;
-    GrCacheID blurProfileKey(gBlurProfileDomain, key);
+    key.fData32[1] = width;
+    key.fData32[2] = 1;
+    GrCacheID horizontalCacheID(gBlurProfileDomain, key);
 
     uint8_t *profile = NULL;
     SkAutoTDeleteArray<uint8_t> ada(NULL);
 
-    *blurProfileTexture = context->findAndRefTexture(texDesc, blurProfileKey, &params);
+    *horizontalScanline = context->findAndRefTexture(texDesc, horizontalCacheID, &params);
 
-    if (NULL == *blurProfileTexture) {
+    if (NULL == *horizontalScanline) {
 
         SkBlurMask::ComputeBlurProfile(sigma, &profile);
         ada.reset(profile);
 
-        *blurProfileTexture = context->createTexture(&params, texDesc, blurProfileKey,
-                                                     profile, 0);
+        SkAutoTMalloc<uint8_t> horizontalPixels(width);
+        SkBlurMask::ComputeBlurredScanline(horizontalPixels, profile, width, sigma);
+
+        *horizontalScanline = context->createTexture(&params, texDesc, horizontalCacheID,
+                                                     horizontalPixels, 0);
 
-        if (NULL == *blurProfileTexture) {
+        if (NULL == *horizontalScanline) {
             return false;
         }
     }
 
+    texDesc.fWidth = 1;
+    texDesc.fHeight = height;
+    key.fData32[1] = 1;
+    key.fData32[2] = height;
+    GrCacheID verticalCacheID(gBlurProfileDomain, key);
+
+    *verticalScanline = context->findAndRefTexture(texDesc, verticalCacheID, &params);
+    if (NULL == *verticalScanline) {
+        if (NULL == profile) {
+            SkBlurMask::ComputeBlurProfile(sigma, &profile);
+            ada.reset(profile);
+        }
+
+        SkAutoTMalloc<uint8_t> verticalPixels(height);
+        SkBlurMask::ComputeBlurredScanline(verticalPixels, profile, height, sigma);
+
+        *verticalScanline = context->createTexture(&params, texDesc, verticalCacheID,
+                                                   verticalPixels, 0);
+
+        if (NULL == *verticalScanline) {
+            SkSafeSetNull(*horizontalScanline);
+            return false;
+        }
+
+    }
     return true;
 }
 
 GrRectBlurEffect::GrRectBlurEffect(const SkRect& rect, float sigma,
-                                   GrTexture *blur_profile)
+                                   GrTexture *horizontal_scanline, GrTexture *vertical_scanline)
   : INHERITED(),
-    fRect(rect),
+    fWidth(horizontal_scanline->width()),
+    fHeight(vertical_scanline->width()),
     fSigma(sigma),
-    fBlurProfileAccess(blur_profile) {
-    this->addTextureAccess(&fBlurProfileAccess);
-    this->setWillReadFragmentPosition();
+    fHorizontalScanlineAccess(horizontal_scanline),
+    fVerticalScanlineAccess(vertical_scanline) {
+    SkMatrix mat;
+    mat.setRectToRect(rect, SkRect::MakeWH(1,1), SkMatrix::kFill_ScaleToFit);
+    fTransform.reset(kLocal_GrCoordSet, mat);
+    this->addTextureAccess(&fHorizontalScanlineAccess);
+    this->addTextureAccess(&fVerticalScanlineAccess);
+    this->addCoordTransform(&fTransform);
 }
 
 GrRectBlurEffect::~GrRectBlurEffect() {
@@ -755,7 +750,10 @@ const GrBackendEffectFactory& GrRectBlurEffect::getFactory() const {
 
 bool GrRectBlurEffect::onIsEqual(const GrEffect& sBase) const {
     const GrRectBlurEffect& s = CastEffect<GrRectBlurEffect>(sBase);
-    return this->getSigma() == s.getSigma() && this->getRect() == s.getRect();
+    return this->getWidth() == s.getWidth() &&
+           this->getHeight() == s.getHeight() &&
+           this->getSigma() == s.getSigma() &&
+           this->getTransform() == s.getTransform();
 }
 
 void GrRectBlurEffect::getConstantColorComponents(GrColor* color, uint32_t* validFlags) const {
@@ -795,9 +793,7 @@ bool SkBlurMaskFilterImpl::directFilterMaskGPU(GrContext* context,
 
     SkMatrix ctm = context->getMatrix();
     SkScalar xformedSigma = this->computeXformedSigma(ctm);
-
-    int pad=SkScalarCeilToInt(6*xformedSigma)/2;
-    rect.outset(SkIntToScalar(pad), SkIntToScalar(pad));
+    rect.outset(3*xformedSigma, 3*xformedSigma);
 
     SkAutoTUnref<GrEffectRef> effect(GrRectBlurEffect::Create(
             context, rect, xformedSigma));
@@ -810,6 +806,7 @@ bool SkBlurMaskFilterImpl::directFilterMaskGPU(GrContext* context,
        return false;
     }
 
+
     grp->addCoverageEffect(effect);
 
     context->drawRect(*grp, rect);

diff --git a/src/gpu/gl/GrGLShaderBuilder.cpp b/src/gpu/gl/GrGLShaderBuilder.cpp
index 4b2778c..f8f810f 100644 (file)
--- a/src/gpu/gl/GrGLShaderBuilder.cpp
+++ b/src/gpu/gl/GrGLShaderBuilder.cpp
@@ -216,7 +216,7 @@ bool GrGLShaderBuilder::genProgram(const GrEffectStage* colorStages[],
         if (GrGLProgramDesc::kSecondaryCoverageISA_CoverageOutput == header.fCoverageOutput) {
             // Get (1-A) into coeff
             coeff = GrGLSLExpr4::VectorCast(GrGLSLExpr1(1) - inputColor.a());
-        } else if (GrGLProgramDesc::kSecondaryCoverageISC_CoverageOutput ==
+        } else if (GrGLProgramDesc::kSecondaryCoverageISC_CoverageOutput == 
                    header.fCoverageOutput){
             // Get (1-RGBA) into coeff
             coeff = GrGLSLExpr4(1) - inputColor;

diff --git a/tests/BlurTest.cpp b/tests/BlurTest.cpp
index 143d777..c09a4ee 100644 (file)
--- a/tests/BlurTest.cpp
+++ b/tests/BlurTest.cpp
@@ -273,8 +273,6 @@ static void cpu_blur_path(const SkPath& path, SkScalar gaussianSigma,
 }
 
 #if SK_SUPPORT_GPU
-#if 0
-// temporary disable; see below for explanation
 static bool gpu_blur_path(GrContextFactory* factory, const SkPath& path,
                           SkScalar gaussianSigma,
                           int* result, int resultCount) {
@@ -300,7 +298,6 @@ static bool gpu_blur_path(GrContextFactory* factory, const SkPath& path,
     return true;
 }
 #endif
-#endif
 
 #if WRITE_CSV
 static void write_as_csv(const char* label, SkScalar scale, int* data, int count) {
@@ -346,6 +343,9 @@ static void test_sigma_range(skiatest::Reporter* reporter, GrContextFactory* fac
 
     int rectSpecialCaseResult[kSize];
     int generalCaseResult[kSize];
+#if SK_SUPPORT_GPU
+    int gpuResult[kSize];
+#endif
     int groundTruthResult[kSize];
     int bruteForce1DResult[kSize];
 
@@ -355,24 +355,20 @@ static void test_sigma_range(skiatest::Reporter* reporter, GrContextFactory* fac
 
         cpu_blur_path(rectPath, sigma, rectSpecialCaseResult, kSize);
         cpu_blur_path(polyPath, sigma, generalCaseResult, kSize);
-
+#if SK_SUPPORT_GPU
+        bool haveGPUResult = gpu_blur_path(factory, rectPath, sigma, gpuResult, kSize);
+#endif
         ground_truth_2d(100, 100, sigma, groundTruthResult, kSize);
         brute_force_1d(-50.0f, 50.0f, sigma, bruteForce1DResult, kSize);
 
         REPORTER_ASSERT(reporter, match(rectSpecialCaseResult, bruteForce1DResult, kSize, 5));
         REPORTER_ASSERT(reporter, match(generalCaseResult, bruteForce1DResult, kSize, 15));
 #if SK_SUPPORT_GPU
-#if 0
-        int gpuResult[kSize];
-        bool haveGPUResult = gpu_blur_path(factory, rectPath, sigma, gpuResult, kSize);
-        // Disabling this test for now -- I don't think it's a legit comparison.
-        // Will continue to investigate this.
         if (haveGPUResult) {
             // 1 works everywhere but: Ubuntu13 & Nexus4
             REPORTER_ASSERT(reporter, match(gpuResult, bruteForce1DResult, kSize, 10));
         }
 #endif
-#endif
         REPORTER_ASSERT(reporter, match(groundTruthResult, bruteForce1DResult, kSize, 1));
 
 #if WRITE_CSV