Different approach to fixing gpu blurs on platforms that "useDrawInsteadOfClear"
This CL reverts https://skia-review.googlesource.com/c/5148/ (Fix gpu blurring on platforms that "useDrawInsteadOfClear") (all the worstCaseWidth/Height stuff) and adds a new GrRenderTargetContext entry point (absClear) to specify clears that can't be discarded or altered.
BUG=skia:
Change-Id: I18b1373ecf4a153ca8c0f290ab8b1d00770426da
Reviewed-on: https://skia-review.googlesource.com/5484
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/include/gpu/GrRenderTargetContext.h b/include/gpu/GrRenderTargetContext.h
index 93a3282..7ef572a 100644
--- a/include/gpu/GrRenderTargetContext.h
+++ b/include/gpu/GrRenderTargetContext.h
@@ -382,13 +382,6 @@
bool isWrapped_ForTesting() const;
- // These two methods return the worst case size of the backing GPU resource when it is
- // finally allocated. In the approx-match case the allocated size could be smaller than
- // what is reported by these entry points (i.e., Ganesh could, optionally, return an
- // exact match)
- int worstCaseWidth() const { return fRenderTargetProxy->worstCaseWidth(); }
- int worstCaseHeight() const { return fRenderTargetProxy->worstCaseHeight(); }
-
protected:
GrRenderTargetContext(GrContext*, GrDrawingManager*, sk_sp<GrRenderTargetProxy>,
sk_sp<SkColorSpace>, const SkSurfaceProps* surfaceProps, GrAuditTrail*,
diff --git a/include/gpu/GrTextureProvider.h b/include/gpu/GrTextureProvider.h
index f3ecfed..cda42a8 100644
--- a/include/gpu/GrTextureProvider.h
+++ b/include/gpu/GrTextureProvider.h
@@ -112,6 +112,8 @@
*/
sk_sp<GrRenderTarget> wrapBackendRenderTarget(const GrBackendRenderTargetDesc& desc);
+ static const int kMinScratchTextureSize;
+
protected:
GrTextureProvider(GrGpu* gpu, GrResourceCache* cache, GrSingleOwner* singleOwner);
diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h
index e60dc28..731603b 100644
--- a/include/private/GrSurfaceProxy.h
+++ b/include/private/GrSurfaceProxy.h
@@ -224,9 +224,9 @@
* Helper that gets the width and height of the surface as a bounding rectangle.
*/
SkRect getBoundsRect() const { return SkRect::MakeIWH(this->width(), this->height()); }
-
- int worstCaseWidth() const;
- int worstCaseHeight() const;
+
+ int worstCaseWidth(const GrCaps& caps) const;
+ int worstCaseHeight(const GrCaps& caps) const;
/**
* @return the texture proxy associated with the surface proxy, may be NULL.
diff --git a/src/core/SkGpuBlurUtils.cpp b/src/core/SkGpuBlurUtils.cpp
index f210178..2ae398d 100644
--- a/src/core/SkGpuBlurUtils.cpp
+++ b/src/core/SkGpuBlurUtils.cpp
@@ -15,6 +15,7 @@
#include "GrContext.h"
#include "GrCaps.h"
#include "GrRenderTargetContext.h"
+#include "GrRenderTargetContextPriv.h"
#include "GrFixedClip.h"
#define MAX_BLUR_SIGMA 4.0f
@@ -311,7 +312,7 @@
// X convolution from reading garbage.
clearRect = SkIRect::MakeXYWH(srcRect.fRight, srcRect.fTop,
radiusX, srcRect.height());
- srcRenderTargetContext->clear(&clearRect, 0x0, false);
+ srcRenderTargetContext->priv().absClear(&clearRect, 0x0);
}
convolve_gaussian(dstRenderTargetContext.get(), clip, srcRect,
@@ -336,7 +337,7 @@
// convolution from reading garbage.
clearRect = SkIRect::MakeXYWH(srcRect.fLeft, srcRect.fBottom,
srcRect.width(), radiusY);
- srcRenderTargetContext->clear(&clearRect, 0x0, false);
+ srcRenderTargetContext->priv().absClear(&clearRect, 0x0);
}
convolve_gaussian(dstRenderTargetContext.get(), clip, srcRect,
@@ -353,10 +354,12 @@
if (scaleFactorX > 1 || scaleFactorY > 1) {
// Clear one pixel to the right and below, to accommodate bilinear upsampling.
+ // TODO: it seems like we should actually be clamping here rather than darkening
+ // the bottom right edges.
clearRect = SkIRect::MakeXYWH(srcRect.fLeft, srcRect.fBottom, srcRect.width() + 1, 1);
- srcRenderTargetContext->clear(&clearRect, 0x0, false);
+ srcRenderTargetContext->priv().absClear(&clearRect, 0x0);
clearRect = SkIRect::MakeXYWH(srcRect.fRight, srcRect.fTop, 1, srcRect.height());
- srcRenderTargetContext->clear(&clearRect, 0x0, false);
+ srcRenderTargetContext->priv().absClear(&clearRect, 0x0);
GrPaint paint;
paint.setGammaCorrect(dstRenderTargetContext->isGammaCorrect());
diff --git a/src/gpu/GrBlurUtils.cpp b/src/gpu/GrBlurUtils.cpp
index 8f14bf7..4d89bef 100644
--- a/src/gpu/GrBlurUtils.cpp
+++ b/src/gpu/GrBlurUtils.cpp
@@ -10,6 +10,7 @@
#include "GrCaps.h"
#include "GrContext.h"
#include "GrFixedClip.h"
+#include "GrRenderTargetContextPriv.h"
#include "effects/GrSimpleTextureEffect.h"
#include "GrStyle.h"
#include "GrTexture.h"
@@ -111,7 +112,7 @@
return nullptr;
}
- rtContext->clear(nullptr, 0x0, true);
+ rtContext->priv().absClear(nullptr, 0x0);
GrPaint tempPaint;
tempPaint.setAntiAlias(doAA);
@@ -141,8 +142,8 @@
SkASSERT(maskFilter);
SkIRect clipBounds;
- clip.getConservativeBounds(renderTargetContext->worstCaseWidth(),
- renderTargetContext->worstCaseHeight(),
+ clip.getConservativeBounds(renderTargetContext->width(),
+ renderTargetContext->height(),
&clipBounds);
SkTLazy<SkPath> tmpPath;
SkStrokeRec::InitStyle fillOrHairline;
diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp
index 13e6024..e2ce046 100644
--- a/src/gpu/GrClipStackClip.cpp
+++ b/src/gpu/GrClipStackClip.cpp
@@ -268,8 +268,8 @@
return true;
}
- SkRect devBounds = SkRect::MakeIWH(renderTargetContext->worstCaseWidth(),
- renderTargetContext->worstCaseHeight());
+ SkRect devBounds = SkRect::MakeIWH(renderTargetContext->width(),
+ renderTargetContext->height());
if (!devBounds.intersect(out->clippedDrawBounds())) {
return false;
}
@@ -299,8 +299,8 @@
#ifdef SK_DEBUG
SkASSERT(reducedClip.hasIBounds());
- SkIRect rtIBounds = SkIRect::MakeWH(renderTargetContext->worstCaseWidth(),
- renderTargetContext->worstCaseHeight());
+ SkIRect rtIBounds = SkIRect::MakeWH(renderTargetContext->width(),
+ renderTargetContext->height());
SkIRect clipIBounds = reducedClip.ibounds().makeOffset(-fOrigin.x(), -fOrigin.y());
SkASSERT(rtIBounds.contains(clipIBounds)); // Mask shouldn't be larger than the RT.
#endif
diff --git a/src/gpu/GrFixedClip.cpp b/src/gpu/GrFixedClip.cpp
index 9e87588..79e2662 100644
--- a/src/gpu/GrFixedClip.cpp
+++ b/src/gpu/GrFixedClip.cpp
@@ -48,7 +48,7 @@
bool GrFixedClip::apply(GrContext*, GrRenderTargetContext* rtc,
bool, bool, GrAppliedClip* out) const {
if (fScissorState.enabled()) {
- SkIRect tightScissor = SkIRect::MakeWH(rtc->worstCaseWidth(), rtc->worstCaseHeight());
+ SkIRect tightScissor = SkIRect::MakeWH(rtc->width(), rtc->height());
if (!tightScissor.intersect(fScissorState.rect())) {
return false;
}
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index bc29a70..ebdf810 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -227,6 +227,65 @@
this->internalClear(rect ? GrFixedClip(*rect) : GrFixedClip::Disabled(), color, canIgnoreRect);
}
+void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const GrColor color) {
+ ASSERT_SINGLE_OWNER_PRIV
+ RETURN_IF_ABANDONED_PRIV
+ SkDEBUGCODE(fRenderTargetContext->validate();)
+ GR_AUDIT_TRAIL_AUTO_FRAME(fRenderTargetContext->fAuditTrail,
+ "GrRenderTargetContext::absClear");
+
+ AutoCheckFlush acf(fRenderTargetContext->fDrawingManager);
+
+ SkIRect rtRect = SkIRect::MakeWH(fRenderTargetContext->fRenderTargetProxy->worstCaseWidth(
+ *fRenderTargetContext->caps()),
+ fRenderTargetContext->fRenderTargetProxy->worstCaseHeight(
+ *fRenderTargetContext->caps()));
+
+ if (clearRect) {
+ if (clearRect->contains(rtRect)) {
+ clearRect = nullptr; // full screen
+ } else {
+ if (!rtRect.intersect(*clearRect)) {
+ return;
+ }
+ }
+ }
+
+ // TODO: in a post-MDB world this should be handled at the OpList level.
+ // An op-list that is initially cleared and has no other ops should receive an
+ // extra draw.
+ if (fRenderTargetContext->fContext->caps()->useDrawInsteadOfClear()) {
+ // This works around a driver bug with clear by drawing a rect instead.
+ // The driver will ignore a clear if it is the only thing rendered to a
+ // target before the target is read.
+ GrPaint paint;
+ paint.setColor4f(GrColor4f::FromGrColor(color));
+ paint.setXPFactory(GrPorterDuffXPFactory::Make(SkBlendMode::kSrc));
+
+ // We don't call drawRect() here to avoid the cropping to the, possibly smaller,
+ // RenderTargetProxy bounds
+ fRenderTargetContext->drawNonAAFilledRect(GrNoClip(), paint, SkMatrix::I(),
+ SkRect::Make(rtRect),
+ nullptr,nullptr, nullptr, false);
+
+ } else {
+ if (!fRenderTargetContext->accessRenderTarget()) {
+ return;
+ }
+
+ // This path doesn't handle coalescing of full screen clears b.c. it
+ // has to clear the entire render target - not just the content area.
+ // It could be done but will take more finagling.
+ sk_sp<GrOp> batch(GrClearBatch::Make(rtRect, color,
+ fRenderTargetContext->accessRenderTarget(),
+ !clearRect));
+ if (!batch) {
+ return;
+ }
+ fRenderTargetContext->getOpList()->addOp(std::move(batch));
+ }
+}
+
void GrRenderTargetContextPriv::clear(const GrFixedClip& clip,
const GrColor color,
bool canIgnoreClip) {
@@ -254,7 +313,7 @@
// This works around a driver bug with clear by drawing a rect instead.
// The driver will ignore a clear if it is the only thing rendered to a
// target before the target is read.
- SkIRect clearRect = SkIRect::MakeWH(this->worstCaseWidth(), this->worstCaseHeight());
+ SkIRect clearRect = SkIRect::MakeWH(this->width(), this->height());
if (isFull) {
this->discard();
} else if (!clearRect.intersect(clip.scissorRect())) {
@@ -415,8 +474,7 @@
const SkRect& rect,
const GrUserStencilSettings* ss) {
SkRect croppedRect = rect;
- if (!crop_filled_rect(this->worstCaseWidth(), this->worstCaseHeight(),
- clip, viewMatrix, &croppedRect)) {
+ if (!crop_filled_rect(this->width(), this->height(), clip, viewMatrix, &croppedRect)) {
return true;
}
diff --git a/src/gpu/GrRenderTargetContextPriv.h b/src/gpu/GrRenderTargetContextPriv.h
index ff17505..b2e647d 100644
--- a/src/gpu/GrRenderTargetContextPriv.h
+++ b/src/gpu/GrRenderTargetContextPriv.h
@@ -51,6 +51,19 @@
void clearStencilClip(const GrFixedClip&, bool insideStencilMask);
+ /*
+ * Some portions of the code, which use approximate-match rendertargets (i.e., ImageFilters),
+ * rely on clears that lie outside of the content region to still have an effect.
+ * For example, when sampling a decimated blurred image back up to full size, the GaussianBlur
+ * code draws 1-pixel rects along the left and bottom edges to be able to use bilerp for
+ * upsampling. The "absClear" entry point ignores the content bounds but does use the
+ * worst case (instantiated) bounds.
+ *
+ * @param rect if (!null) the rect to clear, otherwise it is a full screen clear
+ * @param color the color to clear to
+ */
+ void absClear(const SkIRect* rect, const GrColor color);
+
void stencilRect(const GrClip& clip,
const GrUserStencilSettings* ss,
bool useHWAA,
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 35e505c..4a0c348 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -7,6 +7,7 @@
#include "GrSurfaceProxy.h"
+#include "GrCaps.h"
#include "GrGpuResourcePriv.h"
#include "GrOpList.h"
#include "GrTextureProvider.h"
@@ -49,14 +50,14 @@
#ifdef SK_DEBUG
if (kInvalidGpuMemorySize != this->getRawGpuMemorySize_debugOnly()) {
- SkASSERT(fTarget->gpuMemorySize() <= this->getRawGpuMemorySize_debugOnly());
+ SkASSERT(fTarget->gpuMemorySize() <= this->getRawGpuMemorySize_debugOnly());
}
#endif
return fTarget;
}
-int GrSurfaceProxy::worstCaseWidth() const {
+int GrSurfaceProxy::worstCaseWidth(const GrCaps& caps) const {
if (fTarget) {
return fTarget->width();
}
@@ -65,10 +66,14 @@
return fDesc.fWidth;
}
- return GrNextPow2(fDesc.fWidth);
+ if (caps.reuseScratchTextures() || fDesc.fFlags & kRenderTarget_GrSurfaceFlag) {
+ return SkTMax(GrTextureProvider::kMinScratchTextureSize, GrNextPow2(fDesc.fWidth));
+ }
+
+ return fDesc.fWidth;
}
-int GrSurfaceProxy::worstCaseHeight() const {
+int GrSurfaceProxy::worstCaseHeight(const GrCaps& caps) const {
if (fTarget) {
return fTarget->height();
}
@@ -77,7 +82,11 @@
return fDesc.fHeight;
}
- return GrNextPow2(fDesc.fHeight);
+ if (caps.reuseScratchTextures() || fDesc.fFlags & kRenderTarget_GrSurfaceFlag) {
+ return SkTMax(GrTextureProvider::kMinScratchTextureSize, GrNextPow2(fDesc.fHeight));
+ }
+
+ return fDesc.fHeight;
}
void GrSurfaceProxy::setLastOpList(GrOpList* opList) {
diff --git a/src/gpu/GrTextureProvider.cpp b/src/gpu/GrTextureProvider.cpp
index 843bc0c..993f4b8 100644
--- a/src/gpu/GrTextureProvider.cpp
+++ b/src/gpu/GrTextureProvider.cpp
@@ -16,6 +16,8 @@
#include "SkTArray.h"
#include "SkTLazy.h"
+const int GrTextureProvider::kMinScratchTextureSize = 16;
+
#define ASSERT_SINGLE_OWNER \
SkDEBUGCODE(GrSingleOwner::AutoEnforce debug_SingleOwner(fSingleOwner);)
@@ -119,10 +121,9 @@
if (fGpu->caps()->reuseScratchTextures() || (desc->fFlags & kRenderTarget_GrSurfaceFlag)) {
if (!(kExact_ScratchTextureFlag & flags)) {
// bin by pow2 with a reasonable min
- const int kMinSize = 16;
GrSurfaceDesc* wdesc = desc.writable();
- wdesc->fWidth = SkTMax(kMinSize, GrNextPow2(desc->fWidth));
- wdesc->fHeight = SkTMax(kMinSize, GrNextPow2(desc->fHeight));
+ wdesc->fWidth = SkTMax(kMinScratchTextureSize, GrNextPow2(desc->fWidth));
+ wdesc->fHeight = SkTMax(kMinScratchTextureSize, GrNextPow2(desc->fHeight));
}
GrScratchKey key;
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index ff66177..4e91f4a 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -236,10 +236,10 @@
void SkGpuDevice::clearAll() {
ASSERT_SINGLE_OWNER
- GrColor color = 0;
GR_CREATE_TRACE_MARKER_CONTEXT("SkGpuDevice", "clearAll", fContext.get());
+
SkIRect rect = SkIRect::MakeWH(this->width(), this->height());
- fRenderTargetContext->clear(&rect, color, true);
+ fRenderTargetContext->clear(&rect, 0x0, true);
}
void SkGpuDevice::replaceRenderTargetContext(bool shouldRetainContent) {
diff --git a/src/gpu/batches/GrAAHairLinePathRenderer.cpp b/src/gpu/batches/GrAAHairLinePathRenderer.cpp
index 70d0d4c..662010f 100644
--- a/src/gpu/batches/GrAAHairLinePathRenderer.cpp
+++ b/src/gpu/batches/GrAAHairLinePathRenderer.cpp
@@ -971,8 +971,8 @@
SkASSERT(!args.fRenderTargetContext->isUnifiedMultisampled());
SkIRect devClipBounds;
- args.fClip->getConservativeBounds(args.fRenderTargetContext->worstCaseWidth(),
- args.fRenderTargetContext->worstCaseHeight(),
+ args.fClip->getConservativeBounds(args.fRenderTargetContext->width(),
+ args.fRenderTargetContext->height(),
&devClipBounds);
SkPath path;
diff --git a/src/gpu/batches/GrClearBatch.h b/src/gpu/batches/GrClearBatch.h
index 2571c26..f80605f 100644
--- a/src/gpu/batches/GrClearBatch.h
+++ b/src/gpu/batches/GrClearBatch.h
@@ -27,6 +27,12 @@
return batch;
}
+ static sk_sp<GrClearBatch> Make(const SkIRect& rect, GrColor color, GrRenderTarget* rt,
+ bool fullScreen) {
+ sk_sp<GrClearBatch> batch(new GrClearBatch(rect, color, rt, fullScreen));
+ return batch;
+ }
+
const char* name() const override { return "Clear"; }
// TODO: this needs to be updated to return GrSurfaceProxy::UniqueID
@@ -68,6 +74,17 @@
fRenderTarget.reset(rt);
}
+ GrClearBatch(const SkIRect& rect, GrColor color, GrRenderTarget* rt, bool fullScreen)
+ : INHERITED(ClassID())
+ , fClip(GrFixedClip(rect))
+ , fColor(color)
+ , fRenderTarget(rt) {
+ if (fullScreen) {
+ fClip.disableScissor();
+ }
+ this->setBounds(SkRect::Make(rect), HasAABloat::kNo, IsZeroArea::kNo);
+ }
+
bool onCombineIfPossible(GrOp* t, const GrCaps& caps) override {
// This could be much more complicated. Currently we look at cases where the new clear
// contains the old clear, or when the new clear is a subset of the old clear and is the
diff --git a/src/gpu/batches/GrTessellatingPathRenderer.cpp b/src/gpu/batches/GrTessellatingPathRenderer.cpp
index 86cf883..afa9fe7 100644
--- a/src/gpu/batches/GrTessellatingPathRenderer.cpp
+++ b/src/gpu/batches/GrTessellatingPathRenderer.cpp
@@ -357,8 +357,8 @@
GR_AUDIT_TRAIL_AUTO_FRAME(args.fRenderTargetContext->auditTrail(),
"GrTessellatingPathRenderer::onDrawPath");
SkIRect clipBoundsI;
- args.fClip->getConservativeBounds(args.fRenderTargetContext->worstCaseWidth(),
- args.fRenderTargetContext->worstCaseHeight(),
+ args.fClip->getConservativeBounds(args.fRenderTargetContext->width(),
+ args.fRenderTargetContext->height(),
&clipBoundsI);
sk_sp<GrDrawOp> batch(TessellatingPathBatch::Create(args.fPaint->getColor(),
*args.fShape,