From e07c1ab40882078b505f2b0ceb7413c44af569e8 Mon Sep 17 00:00:00 2001 From: scroggo Date: Thu, 12 Jun 2014 12:10:24 -0700 Subject: [PATCH] Revert of third try at landing improved blur rect; this time with more correctness (https://codereview.chromium.org/331443003/) 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 --- expectations/gm/ignored-tests.txt | 10 -- src/effects/SkBlurMaskFilter.cpp | 191 +++++++++++++++--------------- src/gpu/gl/GrGLShaderBuilder.cpp | 2 +- tests/BlurTest.cpp | 16 +-- 4 files changed, 101 insertions(+), 118 deletions(-) diff --git a/expectations/gm/ignored-tests.txt b/expectations/gm/ignored-tests.txt index 9d6ce3ee31..cccd9bf823 100644 --- 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 43324f269d..e808885ddd 100644 --- 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 hunref(horizontalScanline), vunref(verticalScanline); + if (!createdScanlines) { return NULL; } - - bool createdBlurProfileTexture = CreateBlurProfileTexture(context, sigma, &blurProfileTexture); - SkAutoTUnref 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("\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("\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("\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(); - 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 ada(NULL); - *blurProfileTexture = context->findAndRefTexture(texDesc, blurProfileKey, ¶ms); + *horizontalScanline = context->findAndRefTexture(texDesc, horizontalCacheID, ¶ms); - if (NULL == *blurProfileTexture) { + if (NULL == *horizontalScanline) { SkBlurMask::ComputeBlurProfile(sigma, &profile); ada.reset(profile); - *blurProfileTexture = context->createTexture(¶ms, texDesc, blurProfileKey, - profile, 0); + SkAutoTMalloc horizontalPixels(width); + SkBlurMask::ComputeBlurredScanline(horizontalPixels, profile, width, sigma); - if (NULL == *blurProfileTexture) { + *horizontalScanline = context->createTexture(¶ms, texDesc, horizontalCacheID, + horizontalPixels, 0); + + 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, ¶ms); + if (NULL == *verticalScanline) { + if (NULL == profile) { + SkBlurMask::ComputeBlurProfile(sigma, &profile); + ada.reset(profile); + } + + SkAutoTMalloc verticalPixels(height); + SkBlurMask::ComputeBlurredScanline(verticalPixels, profile, height, sigma); + + *verticalScanline = context->createTexture(¶ms, 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(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 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 4b2778c503..f8f810ff78 100644 --- 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 143d777e88..c09a4ee1c6 100644 --- 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,23 +355,19 @@ 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));