Revert of Make SkBlurImageFilter capable of cropping during blur (GPU path). (patchset #15 id:260001 of https://codereview.chromium.org/1431593002/ )

Reason for revert:
Causing valgrind error. Looks like the issue is in conolve_gaussiand_2d where bounds is not getting set if there is no srcBounds, but later on that bounds is being read in the creation of a TextureDomain.

Original issue's description:
> Make SkBlurImageFilter capable of cropping during blur (GPU path).
>
> This is the GPU equivalent of https://codereview.chromium.org/1415653003/.
>
> It requires passing down the bounds of the crop rect (srcBounds), and
> turning the blur 3-patch optimization in convolve_gaussian() into a 5-patch:
> clear above and below srcBounds, blur with bounds checks inside left and
> right rects, blur without bounds checks in middle rect.
>
> Note: this change causes minor pixels diffs in the
> imagefilterscropexpand GM: for odd crop positions relative to the
> dstBounds, we are now correctly resampling at an even pixel boundary.
>
> BUG=skia:4502, skia:4526
>
> Committed: https://skia.googlesource.com/skia/+/c57e0ded7d535523cfc6bf07c78e5f3479bb8c42

TBR=bsalomon@google.com,senorblanco@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:4502, skia:4526

Review URL: https://codereview.chromium.org/1407133019
This commit is contained in:
egdaniel 2015-11-09 10:04:20 -08:00 committed by Commit bot
parent b4df8b73d7
commit edce2a8576
4 changed files with 61 additions and 99 deletions

View File

@ -205,30 +205,28 @@ bool SkBlurImageFilter::filterImageGPU(Proxy* proxy, const SkBitmap& src, const
if (!this->filterInputGPU(0, proxy, src, ctx, &input, &srcOffset)) { if (!this->filterInputGPU(0, proxy, src, ctx, &input, &srcOffset)) {
return false; return false;
} }
SkIRect srcBounds, dstBounds; SkIRect rect;
if (!this->applyCropRect(ctx, input, srcOffset, &dstBounds, &srcBounds)) { if (!this->applyCropRect(ctx, proxy, input, &srcOffset, &rect, &input)) {
return false; return false;
} }
GrTexture* source = input.getTexture(); GrTexture* source = input.getTexture();
SkVector sigma = mapSigma(fSigma, ctx.ctm()); SkVector sigma = mapSigma(fSigma, ctx.ctm());
offset->fX = dstBounds.fLeft; offset->fX = rect.fLeft;
offset->fY = dstBounds.fTop; offset->fY = rect.fTop;
srcBounds.offset(-srcOffset); rect.offset(-srcOffset);
dstBounds.offset(-srcOffset);
SkRect srcBoundsF(SkRect::Make(srcBounds));
auto constraint = GrTextureProvider::FromImageFilter(ctx.sizeConstraint()); auto constraint = GrTextureProvider::FromImageFilter(ctx.sizeConstraint());
SkAutoTUnref<GrTexture> tex(SkGpuBlurUtils::GaussianBlur(source->getContext(), SkAutoTUnref<GrTexture> tex(SkGpuBlurUtils::GaussianBlur(source->getContext(),
source, source,
false, false,
SkRect::Make(dstBounds), SkRect::Make(rect),
&srcBoundsF, true,
sigma.x(), sigma.x(),
sigma.y(), sigma.y(),
constraint)); constraint));
if (!tex) { if (!tex) {
return false; return false;
} }
WrapTexture(tex, dstBounds.width(), dstBounds.height(), result); WrapTexture(tex, rect.width(), rect.height(), result);
return true; return true;
#else #else
SkDEBUGFAIL("Should not call in GPU-less build"); SkDEBUGFAIL("Should not call in GPU-less build");

View File

@ -1236,7 +1236,7 @@ bool SkBlurMaskFilterImpl::filterMaskGPU(GrTexture* src,
// gaussianBlur. Otherwise, we need to save it for later compositing. // gaussianBlur. Otherwise, we need to save it for later compositing.
bool isNormalBlur = (kNormal_SkBlurStyle == fBlurStyle); bool isNormalBlur = (kNormal_SkBlurStyle == fBlurStyle);
*result = SkGpuBlurUtils::GaussianBlur(context, src, isNormalBlur && canOverwriteSrc, *result = SkGpuBlurUtils::GaussianBlur(context, src, isNormalBlur && canOverwriteSrc,
clipRect, nullptr, xformedSigma, xformedSigma, clipRect, false, xformedSigma, xformedSigma,
GrTextureProvider::kApprox_SizeConstraint); GrTextureProvider::kApprox_SizeConstraint);
if (nullptr == *result) { if (nullptr == *result) {
return false; return false;

View File

@ -71,20 +71,16 @@ static void convolve_gaussian_2d(GrDrawContext* drawContext,
int radiusY, int radiusY,
SkScalar sigmaX, SkScalar sigmaX,
SkScalar sigmaY, SkScalar sigmaY,
const SkRect* srcBounds) { bool useBounds,
SkIRect bounds) {
SkRect dstRect = SkRect::MakeWH(srcRect.width(), srcRect.height()); SkRect dstRect = SkRect::MakeWH(srcRect.width(), srcRect.height());
SkMatrix localMatrix = SkMatrix::MakeTrans(srcRect.x(), srcRect.y()); SkMatrix localMatrix = SkMatrix::MakeTrans(srcRect.x(), srcRect.y());
SkISize size = SkISize::Make(2 * radiusX + 1, 2 * radiusY + 1); SkISize size = SkISize::Make(2 * radiusX + 1, 2 * radiusY + 1);
SkIPoint kernelOffset = SkIPoint::Make(radiusX, radiusY); SkIPoint kernelOffset = SkIPoint::Make(radiusX, radiusY);
GrPaint paint; GrPaint paint;
SkIRect bounds;
if (srcBounds) {
srcBounds->roundOut(&bounds);
}
SkAutoTUnref<GrFragmentProcessor> conv(GrMatrixConvolutionEffect::CreateGaussian( SkAutoTUnref<GrFragmentProcessor> conv(GrMatrixConvolutionEffect::CreateGaussian(
texture, bounds, size, 1.0, 0.0, kernelOffset, texture, bounds, size, 1.0, 0.0, kernelOffset,
srcBounds ? GrTextureDomain::kDecal_Mode : GrTextureDomain::kIgnore_Mode, useBounds ? GrTextureDomain::kClamp_Mode : GrTextureDomain::kIgnore_Mode,
true, sigmaX, sigmaY)); true, sigmaX, sigmaY));
paint.addColorFragmentProcessor(conv); paint.addColorFragmentProcessor(conv);
drawContext->fillRectWithLocalMatrix(clip, paint, SkMatrix::I(), dstRect, localMatrix); drawContext->fillRectWithLocalMatrix(clip, paint, SkMatrix::I(), dstRect, localMatrix);
@ -97,62 +93,46 @@ static void convolve_gaussian(GrDrawContext* drawContext,
Gr1DKernelEffect::Direction direction, Gr1DKernelEffect::Direction direction,
int radius, int radius,
float sigma, float sigma,
const SkRect* srcBounds, bool cropToSrcRect) {
const SkPoint& srcOffset) {
float bounds[2] = { 0.0f, 1.0f }; float bounds[2] = { 0.0f, 1.0f };
SkRect dstRect = SkRect::MakeWH(srcRect.width(), srcRect.height()); SkRect dstRect = SkRect::MakeWH(srcRect.width(), srcRect.height());
if (!srcBounds) { SkPoint srcOffset = SkPoint::Make(srcRect.x(), srcRect.y());
if (!cropToSrcRect) {
convolve_gaussian_1d(drawContext, clip, dstRect, srcOffset, texture, convolve_gaussian_1d(drawContext, clip, dstRect, srcOffset, texture,
direction, radius, sigma, false, bounds); direction, radius, sigma, false, bounds);
return; return;
} }
SkRect midRect = *srcBounds, leftRect, rightRect; SkRect lowerDstRect = dstRect;
midRect.offset(srcOffset); SkRect middleDstRect = dstRect;
SkIRect topRect, bottomRect; SkRect upperDstRect = dstRect;
SkScalar size;
SkScalar rad = SkIntToScalar(radius); SkScalar rad = SkIntToScalar(radius);
if (direction == Gr1DKernelEffect::kX_Direction) { if (direction == Gr1DKernelEffect::kX_Direction) {
bounds[0] = SkScalarToFloat(srcBounds->left()) / texture->width(); bounds[0] = SkScalarToFloat(srcRect.left()) / texture->width();
bounds[1] = SkScalarToFloat(srcBounds->right()) / texture->width(); bounds[1] = SkScalarToFloat(srcRect.right()) / texture->width();
SkRect::MakeLTRB(0, 0, dstRect.right(), midRect.top()).roundOut(&topRect); size = dstRect.width();
SkRect::MakeLTRB(0, midRect.bottom(), dstRect.right(), dstRect.bottom()) lowerDstRect.fRight = dstRect.left() + rad;
.roundOut(&bottomRect); upperDstRect.fLeft = dstRect.right() - rad;
midRect.inset(rad, 0); middleDstRect.inset(rad, 0);
leftRect = SkRect::MakeLTRB(0, midRect.top(), midRect.left(), midRect.bottom());
rightRect =
SkRect::MakeLTRB(midRect.right(), midRect.top(), dstRect.width(), midRect.bottom());
dstRect.fTop = midRect.top();
dstRect.fBottom = midRect.bottom();
} else { } else {
bounds[0] = SkScalarToFloat(srcBounds->top()) / texture->height(); bounds[0] = SkScalarToFloat(srcRect.top()) / texture->height();
bounds[1] = SkScalarToFloat(srcBounds->bottom()) / texture->height(); bounds[1] = SkScalarToFloat(srcRect.bottom()) / texture->height();
SkRect::MakeLTRB(0, 0, midRect.left(), dstRect.bottom()).roundOut(&topRect); size = dstRect.height();
SkRect::MakeLTRB(midRect.right(), 0, dstRect.right(), dstRect.bottom()) lowerDstRect.fBottom = dstRect.top() + rad;
.roundOut(&bottomRect);; upperDstRect.fTop = dstRect.bottom() - rad;
midRect.inset(0, rad); middleDstRect.inset(0, rad);
leftRect = SkRect::MakeLTRB(midRect.left(), 0, midRect.right(), midRect.top());
rightRect =
SkRect::MakeLTRB(midRect.left(), midRect.bottom(), midRect.right(), dstRect.height());
dstRect.fLeft = midRect.left();
dstRect.fRight = midRect.right();
} }
if (!topRect.isEmpty()) { if (radius >= size * SK_ScalarHalf) {
drawContext->clear(&topRect, 0, false); // Blur radius covers srcRect; use bounds over entire draw
} convolve_gaussian_1d(drawContext, clip, dstRect, srcOffset, texture,
if (!bottomRect.isEmpty()) {
drawContext->clear(&bottomRect, 0, false);
}
if (midRect.isEmpty()) {
// Blur radius covers srcBounds; use bounds over entire draw
convolve_gaussian_1d(drawContext, clip, dstRect, -srcOffset, texture,
direction, radius, sigma, true, bounds); direction, radius, sigma, true, bounds);
} else { } else {
// Draw right and left margins with bounds; middle without. // Draw upper and lower margins with bounds; middle without.
convolve_gaussian_1d(drawContext, clip, leftRect, -srcOffset, texture, convolve_gaussian_1d(drawContext, clip, lowerDstRect, srcOffset, texture,
direction, radius, sigma, true, bounds); direction, radius, sigma, true, bounds);
convolve_gaussian_1d(drawContext, clip, rightRect, -srcOffset, texture, convolve_gaussian_1d(drawContext, clip, upperDstRect, srcOffset, texture,
direction, radius, sigma, true, bounds); direction, radius, sigma, true, bounds);
convolve_gaussian_1d(drawContext, clip, midRect, -srcOffset, texture, convolve_gaussian_1d(drawContext, clip, middleDstRect, srcOffset, texture,
direction, radius, sigma, false, bounds); direction, radius, sigma, false, bounds);
} }
} }
@ -160,12 +140,13 @@ static void convolve_gaussian(GrDrawContext* drawContext,
GrTexture* GaussianBlur(GrContext* context, GrTexture* GaussianBlur(GrContext* context,
GrTexture* srcTexture, GrTexture* srcTexture,
bool canClobberSrc, bool canClobberSrc,
const SkRect& dstBounds, const SkRect& rect,
const SkRect* srcBounds, bool cropToRect,
float sigmaX, float sigmaX,
float sigmaY, float sigmaY,
GrTextureProvider::SizeConstraint constraint) { GrTextureProvider::SizeConstraint constraint) {
SkASSERT(context); SkASSERT(context);
SkIRect clearRect; SkIRect clearRect;
int scaleFactorX, radiusX; int scaleFactorX, radiusX;
int scaleFactorY, radiusY; int scaleFactorY, radiusY;
@ -173,25 +154,14 @@ GrTexture* GaussianBlur(GrContext* context,
sigmaX = adjust_sigma(sigmaX, maxTextureSize, &scaleFactorX, &radiusX); sigmaX = adjust_sigma(sigmaX, maxTextureSize, &scaleFactorX, &radiusX);
sigmaY = adjust_sigma(sigmaY, maxTextureSize, &scaleFactorY, &radiusY); sigmaY = adjust_sigma(sigmaY, maxTextureSize, &scaleFactorY, &radiusY);
SkPoint srcOffset = SkPoint::Make(-dstBounds.x(), -dstBounds.y()); SkRect srcRect(rect);
SkRect localDstBounds = SkRect::MakeWH(dstBounds.width(), dstBounds.height());
SkRect localSrcBounds;
SkRect srcRect;
if (srcBounds) {
srcRect = localSrcBounds = *srcBounds;
srcRect.offset(srcOffset);
srcBounds = &localSrcBounds;
} else {
srcRect = localDstBounds;
}
scale_rect(&srcRect, 1.0f / scaleFactorX, 1.0f / scaleFactorY); scale_rect(&srcRect, 1.0f / scaleFactorX, 1.0f / scaleFactorY);
srcRect.roundOut(&srcRect); srcRect.roundOut(&srcRect);
scale_rect(&srcRect, static_cast<float>(scaleFactorX), scale_rect(&srcRect, static_cast<float>(scaleFactorX),
static_cast<float>(scaleFactorY)); static_cast<float>(scaleFactorY));
// setup new clip // setup new clip
GrClip clip(localDstBounds); GrClip clip(SkRect::MakeWH(srcRect.width(), srcRect.height()));
SkASSERT(kBGRA_8888_GrPixelConfig == srcTexture->config() || SkASSERT(kBGRA_8888_GrPixelConfig == srcTexture->config() ||
kRGBA_8888_GrPixelConfig == srcTexture->config() || kRGBA_8888_GrPixelConfig == srcTexture->config() ||
@ -199,8 +169,8 @@ GrTexture* GaussianBlur(GrContext* context,
GrSurfaceDesc desc; GrSurfaceDesc desc;
desc.fFlags = kRenderTarget_GrSurfaceFlag; desc.fFlags = kRenderTarget_GrSurfaceFlag;
desc.fWidth = SkScalarFloorToInt(dstBounds.width()); desc.fWidth = SkScalarFloorToInt(srcRect.width());
desc.fHeight = SkScalarFloorToInt(dstBounds.height()); desc.fHeight = SkScalarFloorToInt(srcRect.height());
desc.fConfig = srcTexture->config(); desc.fConfig = srcTexture->config();
GrTexture* dstTexture; GrTexture* dstTexture;
@ -227,11 +197,12 @@ GrTexture* GaussianBlur(GrContext* context,
SkMatrix matrix; SkMatrix matrix;
matrix.setIDiv(srcTexture->width(), srcTexture->height()); matrix.setIDiv(srcTexture->width(), srcTexture->height());
SkRect dstRect(srcRect); SkRect dstRect(srcRect);
if (srcBounds && i == 1) { if (cropToRect && i == 1) {
dstRect.offset(-dstRect.fLeft, -dstRect.fTop);
SkRect domain; SkRect domain;
matrix.mapRect(&domain, *srcBounds); matrix.mapRect(&domain, rect);
domain.inset((i < scaleFactorX) ? SK_ScalarHalf / srcTexture->width() : 0.0f, domain.inset(i < scaleFactorX ? SK_ScalarHalf / srcTexture->width() : 0.0f,
(i < scaleFactorY) ? SK_ScalarHalf / srcTexture->height() : 0.0f); i < scaleFactorY ? SK_ScalarHalf / srcTexture->height() : 0.0f);
SkAutoTUnref<const GrFragmentProcessor> fp(GrTextureDomainEffect::Create( SkAutoTUnref<const GrFragmentProcessor> fp(GrTextureDomainEffect::Create(
srcTexture, srcTexture,
matrix, matrix,
@ -239,8 +210,6 @@ GrTexture* GaussianBlur(GrContext* context,
GrTextureDomain::kDecal_Mode, GrTextureDomain::kDecal_Mode,
GrTextureParams::kBilerp_FilterMode)); GrTextureParams::kBilerp_FilterMode));
paint.addColorFragmentProcessor(fp); paint.addColorFragmentProcessor(fp);
srcRect.offset(-srcOffset);
srcOffset.set(0, 0);
} else { } else {
GrTextureParams params(SkShader::kClamp_TileMode, GrTextureParams::kBilerp_FilterMode); GrTextureParams params(SkShader::kClamp_TileMode, GrTextureParams::kBilerp_FilterMode);
paint.addColorTextureProcessor(srcTexture, matrix, params); paint.addColorTextureProcessor(srcTexture, matrix, params);
@ -259,9 +228,10 @@ GrTexture* GaussianBlur(GrContext* context,
srcRect = dstRect; srcRect = dstRect;
srcTexture = dstTexture; srcTexture = dstTexture;
SkTSwap(dstTexture, tempTexture); SkTSwap(dstTexture, tempTexture);
localSrcBounds = srcRect;
} }
const SkIRect srcIRect = srcRect.roundOut();
// For really small blurs (certainly no wider than 5x5 on desktop gpus) it is faster to just // For really small blurs (certainly no wider than 5x5 on desktop gpus) it is faster to just
// launch a single non separable kernel vs two launches // launch a single non separable kernel vs two launches
if (sigmaX > 0.0f && sigmaY > 0.0f && if (sigmaX > 0.0f && sigmaY > 0.0f &&
@ -275,7 +245,7 @@ GrTexture* GaussianBlur(GrContext* context,
return nullptr; return nullptr;
} }
convolve_gaussian_2d(dstDrawContext, clip, srcRect, convolve_gaussian_2d(dstDrawContext, clip, srcRect,
srcTexture, radiusX, radiusY, sigmaX, sigmaY, srcBounds); srcTexture, radiusX, radiusY, sigmaX, sigmaY, cropToRect, srcIRect);
srcDrawContext.swap(dstDrawContext); srcDrawContext.swap(dstDrawContext);
srcRect.offsetTo(0, 0); srcRect.offsetTo(0, 0);
@ -283,10 +253,6 @@ GrTexture* GaussianBlur(GrContext* context,
SkTSwap(dstTexture, tempTexture); SkTSwap(dstTexture, tempTexture);
} else { } else {
srcRect = localDstBounds;
scale_rect(&srcRect, 1.0f / scaleFactorX, 1.0f / scaleFactorY);
srcRect.roundOut(&srcRect);
const SkIRect srcIRect = srcRect.roundOut();
if (sigmaX > 0.0f) { if (sigmaX > 0.0f) {
if (scaleFactorX > 1) { if (scaleFactorX > 1) {
// TODO: if we pass in the source draw context we don't need this here // TODO: if we pass in the source draw context we don't need this here
@ -311,13 +277,12 @@ GrTexture* GaussianBlur(GrContext* context,
} }
convolve_gaussian(dstDrawContext, clip, srcRect, convolve_gaussian(dstDrawContext, clip, srcRect,
srcTexture, Gr1DKernelEffect::kX_Direction, radiusX, sigmaX, srcTexture, Gr1DKernelEffect::kX_Direction, radiusX, sigmaX,
srcBounds, srcOffset); cropToRect);
srcDrawContext.swap(dstDrawContext); srcDrawContext.swap(dstDrawContext);
srcTexture = dstTexture; srcTexture = dstTexture;
srcRect.offsetTo(0, 0); srcRect.offsetTo(0, 0);
SkTSwap(dstTexture, tempTexture); SkTSwap(dstTexture, tempTexture);
localSrcBounds = srcRect;
srcOffset.set(0, 0);
} }
if (sigmaY > 0.0f) { if (sigmaY > 0.0f) {
@ -344,7 +309,7 @@ GrTexture* GaussianBlur(GrContext* context,
} }
convolve_gaussian(dstDrawContext, clip, srcRect, convolve_gaussian(dstDrawContext, clip, srcRect,
srcTexture, Gr1DKernelEffect::kY_Direction, radiusY, sigmaY, srcTexture, Gr1DKernelEffect::kY_Direction, radiusY, sigmaY,
srcBounds, srcOffset); cropToRect);
srcDrawContext.swap(dstDrawContext); srcDrawContext.swap(dstDrawContext);
srcTexture = dstTexture; srcTexture = dstTexture;
@ -352,7 +317,6 @@ GrTexture* GaussianBlur(GrContext* context,
SkTSwap(dstTexture, tempTexture); SkTSwap(dstTexture, tempTexture);
} }
} }
const SkIRect srcIRect = srcRect.roundOut();
if (scaleFactorX > 1 || scaleFactorY > 1) { if (scaleFactorX > 1 || scaleFactorY > 1) {
SkASSERT(srcDrawContext); SkASSERT(srcDrawContext);

View File

@ -26,9 +26,9 @@ namespace SkGpuBlurUtils {
* @param srcTexture The source texture to be blurred. * @param srcTexture The source texture to be blurred.
* @param canClobberSrc If true, srcTexture may be overwritten, and * @param canClobberSrc If true, srcTexture may be overwritten, and
* may be returned as the result. * may be returned as the result.
* @param dstBounds The destination bounds, relative to the source texture. * @param rect The destination rectangle.
* @param srcBounds The source bounds, relative to the source texture. If non-null, * @param cropToRect If true, do not sample any pixels outside the
* no pixels will be sampled outside of this rectangle. * source rect.
* @param sigmaX The blur's standard deviation in X. * @param sigmaX The blur's standard deviation in X.
* @param sigmaY The blur's standard deviation in Y. * @param sigmaY The blur's standard deviation in Y.
* @return the blurred texture, which may be srcTexture reffed, or a * @return the blurred texture, which may be srcTexture reffed, or a
@ -37,8 +37,8 @@ namespace SkGpuBlurUtils {
GrTexture* GaussianBlur(GrContext* context, GrTexture* GaussianBlur(GrContext* context,
GrTexture* srcTexture, GrTexture* srcTexture,
bool canClobberSrc, bool canClobberSrc,
const SkRect& dstBounds, const SkRect& rect,
const SkRect* srcBounds, bool cropToRect,
float sigmaX, float sigmaX,
float sigmaY, float sigmaY,
GrTextureProvider::SizeConstraint); GrTextureProvider::SizeConstraint);