From e85a32d4f8ce7fb9b6aaae89137dbf3766d833f2 Mon Sep 17 00:00:00 2001 From: robertphillips Date: Tue, 10 Feb 2015 08:16:55 -0800 Subject: Clean up clipping code a bit Review URL: https://codereview.chromium.org/913693002 --- gm/clipdrawdraw.cpp | 81 ++++++++++------------------ include/core/SkClipStack.h | 7 --- include/gpu/GrTypesPriv.h | 5 ++ src/core/SkClipStack.cpp | 25 --------- src/gpu/GrClipMaskManager.cpp | 5 +- src/gpu/GrSoftwarePathRenderer.cpp | 7 ++- src/gpu/GrTextContext.cpp | 10 +--- src/gpu/effects/GrConfigConversionEffect.cpp | 2 + src/gpu/gl/GrGLGpu.cpp | 30 +++++------ 9 files changed, 56 insertions(+), 116 deletions(-) diff --git a/gm/clipdrawdraw.cpp b/gm/clipdrawdraw.cpp index 0e471dd148..3846dc0b53 100644 --- a/gm/clipdrawdraw.cpp +++ b/gm/clipdrawdraw.cpp @@ -11,74 +11,51 @@ namespace skiagm { // This GM exercises the use case found in crbug.com/423834. // The following pattern: -// clipRect(r); -// drawRect(r, withAA); -// drawRect(r, noAA); +// save(); +// clipRect(rect, noAA); +// drawRect(bigRect, noAA); +// restore(); +// +// drawRect(rect, noAA); // can leave 1 pixel wide remnants of the first rect. class ClipDrawDrawGM : public GM { public: - ClipDrawDrawGM() { - this->setBGColor(0xFFCCCCCC); - } + ClipDrawDrawGM() { this->setBGColor(0xFFCCCCCC); } protected: - SkString onShortName() SK_OVERRIDE { - return SkString("clipdrawdraw"); - } + SkString onShortName() SK_OVERRIDE { return SkString("clipdrawdraw"); } - SkISize onISize() SK_OVERRIDE { - return SkISize::Make(512, 512); - } + SkISize onISize() SK_OVERRIDE { return SkISize::Make(512, 512); } - // Vertical remnant - static void draw1(SkCanvas* canvas) { + static void Draw(SkCanvas* canvas, const SkRect& rect) { SkPaint p; - p.setAntiAlias(true); - - const SkRect rect = SkRect::MakeXYWH(8, 9, 404, 313); - - canvas->save(); - - canvas->scale(0.5f, 0.5f); - canvas->translate(265, 265); - - canvas->save(); - canvas->clipRect(rect); - canvas->drawRect(rect, p); - canvas->restore(); - - p.setColor(SK_ColorWHITE); p.setAntiAlias(false); - canvas->drawRect(rect, p); - canvas->restore(); - } - - // Horizontal remnant - static void draw2(SkCanvas* canvas) { - SkPaint p; - p.setAntiAlias(true); - - const SkRect rect = SkRect::MakeXYWH(8, 9, 404, 313); - - canvas->save(); - canvas->translate(200.800003f, 172.299988f); - canvas->scale(0.8f, 0.8f); + const SkRect bigRect = SkRect::MakeWH(600, 600); canvas->save(); - canvas->clipRect(rect); - canvas->drawRect(rect, p); - canvas->restore(); - - p.setColor(SK_ColorWHITE); - p.setAntiAlias(false); - canvas->drawRect(rect, p); + // draw a black rect through the clip + canvas->save(); + canvas->clipRect(rect); + canvas->drawRect(bigRect, p); + canvas->restore(); + + // now draw the white rect on top + p.setColor(SK_ColorWHITE); + canvas->drawRect(rect, p); canvas->restore(); } void onDraw(SkCanvas* canvas) SK_OVERRIDE { - draw1(canvas); - draw2(canvas); + // Vertical remnant + const SkRect rect1 = SkRect::MakeLTRB(136.5f, 137.5f, 338.5f, 293.5f); + + // Horizontal remnant + // 179.488 rounds the right way (i.e., 179), 179.499 rounds the wrong way (i.e., 180) + const SkRect rect2 = SkRect::MakeLTRB(207.5f, 179.499f, 530.5f, 429.5f); + + Draw(canvas, rect1); + Draw(canvas, rect2); } private: diff --git a/include/core/SkClipStack.h b/include/core/SkClipStack.h index 3f79f92a5b..b74e47697f 100644 --- a/include/core/SkClipStack.h +++ b/include/core/SkClipStack.h @@ -309,13 +309,6 @@ public: BoundsType* boundType, bool* isIntersectionOfRects = NULL) const; - /** - * Takes an input rect in device space and conservatively clips it to the - * clip-stack. If false is returned then the rect does not intersect the - * clip and is unmodified. - */ - bool intersectRectWithClip(SkRect* devRect) const; - /** * Returns true if the input rect in device space is entirely contained * by the clip. A return value of false does not guarantee that the rect diff --git a/include/gpu/GrTypesPriv.h b/include/gpu/GrTypesPriv.h index b22f48026c..1f86197295 100644 --- a/include/gpu/GrTypesPriv.h +++ b/include/gpu/GrTypesPriv.h @@ -249,6 +249,11 @@ struct GrScissorState { (false == fEnabled || fRect == other.fRect); } bool operator!=(const GrScissorState& other) const { return !(*this == other); } + + bool enabled() const { return fEnabled; } + const SkIRect& rect() const { return fRect; } + +private: bool fEnabled; SkIRect fRect; }; diff --git a/src/core/SkClipStack.cpp b/src/core/SkClipStack.cpp index 2d8c94f0f7..e0c3db01ba 100644 --- a/src/core/SkClipStack.cpp +++ b/src/core/SkClipStack.cpp @@ -420,12 +420,6 @@ void SkClipStack::Element::updateBoundAndGenID(const Element* prior) { } if (!fDoAA) { - // Here we mimic a non-anti-aliased scanline system. If there is - // no anti-aliasing we can integerize the bounding box to exclude - // fractional parts that won't be rendered. - // Note: the left edge is handled slightly differently below. We - // are a bit more generous in the rounding since we don't want to - // risk missing the left pixels when fLeft is very close to .5 fFiniteBound.set(SkScalarFloorToScalar(fFiniteBound.fLeft+0.45f), SkScalarRoundToScalar(fFiniteBound.fTop), SkScalarRoundToScalar(fFiniteBound.fRight), @@ -622,25 +616,6 @@ void SkClipStack::getBounds(SkRect* canvFiniteBound, } } -bool SkClipStack::intersectRectWithClip(SkRect* rect) const { - SkASSERT(rect); - - SkRect bounds; - SkClipStack::BoundsType bt; - this->getBounds(&bounds, &bt); - if (bt == SkClipStack::kInsideOut_BoundsType) { - if (bounds.contains(*rect)) { - return false; - } else { - // If rect's x values are both within bound's x range we - // could clip here. Same for y. But we don't bother to check. - return true; - } - } else { - return rect->intersect(bounds); - } -} - bool SkClipStack::quickContains(const SkRect& rect) const { Iter iter(*this, Iter::kTop_IterStart); diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index 0226ac1b76..f69c8717f8 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -23,7 +23,6 @@ #include "effects/GrRRectEffect.h" #include "effects/GrTextureDomain.h" -#define GR_AA_CLIP 1 typedef SkClipStack::Element Element; //////////////////////////////////////////////////////////////////////////////// @@ -153,7 +152,7 @@ bool GrClipMaskManager::installClipEffects(GrPipelineBuilder* pipelineBuilder, if (!skip) { GrPrimitiveEdgeType edgeType; - if (GR_AA_CLIP && iter.get()->isAA()) { + if (iter.get()->isAA()) { if (rt->isMultisampled()) { // Coverage based AA clips don't place nicely with MSAA. failed = true; @@ -276,7 +275,6 @@ bool GrClipMaskManager::setupClipping(GrPipelineBuilder* pipelineBuilder, } } -#if GR_AA_CLIP // If MSAA is enabled we can do everything in the stencil buffer. if (0 == rt->numSamples() && requiresAA) { GrTexture* result = NULL; @@ -314,7 +312,6 @@ bool GrClipMaskManager::setupClipping(GrPipelineBuilder* pipelineBuilder, } // if alpha clip mask creation fails fall through to the non-AA code paths } -#endif // GR_AA_CLIP // Either a hard (stencil buffer) clip was explicitly requested or an anti-aliased clip couldn't // be created. In either case, free up the texture in the anti-aliased mask cache. diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp index 4fb7538b03..364c214a60 100644 --- a/src/gpu/GrSoftwarePathRenderer.cpp +++ b/src/gpu/GrSoftwarePathRenderer.cpp @@ -49,13 +49,11 @@ bool get_path_and_clip_bounds(const GrDrawTarget* target, if (NULL == rt) { return false; } - *devPathBounds = SkIRect::MakeWH(rt->width(), rt->height()); target->getClip()->getConservativeBounds(rt, devClipBounds); - // TODO: getConservativeBounds already intersects with the - // render target's bounding box. Remove this next line - if (!devPathBounds->intersect(*devClipBounds)) { + if (devClipBounds->isEmpty()) { + *devPathBounds = SkIRect::MakeWH(rt->width(), rt->height()); return false; } @@ -64,6 +62,7 @@ bool get_path_and_clip_bounds(const GrDrawTarget* target, matrix.mapRect(&pathSBounds, path.getBounds()); SkIRect pathIBounds; pathSBounds.roundOut(&pathIBounds); + *devPathBounds = *devClipBounds; if (!devPathBounds->intersect(pathIBounds)) { // set the correct path bounds, as this would be used later. *devPathBounds = pathIBounds; diff --git a/src/gpu/GrTextContext.cpp b/src/gpu/GrTextContext.cpp index e62136507c..bc544ad4a0 100644 --- a/src/gpu/GrTextContext.cpp +++ b/src/gpu/GrTextContext.cpp @@ -24,15 +24,9 @@ GrTextContext::~GrTextContext() { void GrTextContext::init(const GrPaint& grPaint, const SkPaint& skPaint) { const GrClipData* clipData = fContext->getClip(); - SkRect devConservativeBound; - clipData->fClipStack->getConservativeBounds( - -clipData->fOrigin.fX, - -clipData->fOrigin.fY, - fContext->getRenderTarget()->width(), + clipData->getConservativeBounds(fContext->getRenderTarget()->width(), fContext->getRenderTarget()->height(), - &devConservativeBound); - - devConservativeBound.roundOut(&fClipRect); + &fClipRect); fDrawTarget = fContext->getTextTarget(); diff --git a/src/gpu/effects/GrConfigConversionEffect.cpp b/src/gpu/effects/GrConfigConversionEffect.cpp index ec44d08b01..7aad50f5f8 100644 --- a/src/gpu/effects/GrConfigConversionEffect.cpp +++ b/src/gpu/effects/GrConfigConversionEffect.cpp @@ -156,6 +156,8 @@ GrGLFragmentProcessor* GrConfigConversionEffect::createGLInstance() const { return SkNEW_ARGS(GrGLConfigConversionEffect, (*this)); } + + void GrConfigConversionEffect::TestForPreservingPMConversions(GrContext* context, PMConversion* pmToUPMRule, PMConversion* upmToPMRule) { diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp index 6419aa6b52..21d2f6327b 100644 --- a/src/gpu/gl/GrGLGpu.cpp +++ b/src/gpu/gl/GrGLGpu.cpp @@ -1327,13 +1327,13 @@ GrIndexBuffer* GrGLGpu::onCreateIndexBuffer(size_t size, bool dynamic) { void GrGLGpu::flushScissor(const GrScissorState& scissorState, const GrGLIRect& rtViewport, GrSurfaceOrigin rtOrigin) { - if (scissorState.fEnabled) { + if (scissorState.enabled()) { GrGLIRect scissor; scissor.setRelativeTo(rtViewport, - scissorState.fRect.fLeft, - scissorState.fRect.fTop, - scissorState.fRect.width(), - scissorState.fRect.height(), + scissorState.rect().fLeft, + scissorState.rect().fTop, + scissorState.rect().width(), + scissorState.rect().height(), rtOrigin); // if the scissor fully contains the viewport then we fall through and // disable the scissor test. @@ -1489,9 +1489,8 @@ void GrGLGpu::onClear(GrRenderTarget* target, const SkIRect* rect, GrColor color this->flushRenderTarget(glRT, rect); GrScissorState scissorState; - scissorState.fEnabled = SkToBool(rect); - if (scissorState.fEnabled) { - scissorState.fRect = *rect; + if (rect) { + scissorState.set(*rect); } this->flushScissor(scissorState, glRT->getViewport(), glRT->origin()); @@ -1602,8 +1601,7 @@ void GrGLGpu::onClearStencilClip(GrRenderTarget* target, const SkIRect& rect, bo this->flushRenderTarget(glRT, &SkIRect::EmptyIRect()); GrScissorState scissorState; - scissorState.fEnabled = true; - scissorState.fRect = rect; + scissorState.set(rect); this->flushScissor(scissorState, glRT->getViewport(), glRT->origin()); GL_CALL(StencilMask((uint32_t) clipStencilMask)); @@ -1930,18 +1928,18 @@ void GrGLGpu::onResolveRenderTarget(GrRenderTarget* target) { fHWBoundRenderTargetUniqueID = SK_InvalidUniqueID; const GrGLIRect& vp = rt->getViewport(); const SkIRect dirtyRect = rt->getResolveRect(); - GrGLIRect r; - r.setRelativeTo(vp, dirtyRect.fLeft, dirtyRect.fTop, - dirtyRect.width(), dirtyRect.height(), target->origin()); if (GrGLCaps::kES_Apple_MSFBOType == this->glCaps().msFBOType()) { // Apple's extension uses the scissor as the blit bounds. GrScissorState scissorState; - scissorState.fEnabled = true; - scissorState.fRect = dirtyRect; - this->flushScissor(scissorState, rt->getViewport(), rt->origin()); + scissorState.set(dirtyRect); + this->flushScissor(scissorState, vp, rt->origin()); GL_CALL(ResolveMultisampleFramebuffer()); } else { + GrGLIRect r; + r.setRelativeTo(vp, dirtyRect.fLeft, dirtyRect.fTop, + dirtyRect.width(), dirtyRect.height(), target->origin()); + int right = r.fLeft + r.fWidth; int top = r.fBottom + r.fHeight; -- cgit v1.2.3