From c82a8b7aa4ec19fba508c394920a9e88d3e5bd12 Mon Sep 17 00:00:00 2001 From: "robertphillips@google.com" Date: Thu, 21 Jun 2012 20:15:48 +0000 Subject: [PATCH] Fixed two bugs in SW-only clip mask generation http://codereview.appspot.com/6306086/ git-svn-id: http://skia.googlecode.com/svn/trunk@4290 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/gpu/GrContext.h | 5 ++++- src/gpu/GrClipMaskManager.cpp | 27 +++++++-------------------- src/gpu/GrContext.cpp | 6 ++++-- src/gpu/GrDrawTarget.h | 10 ++++++---- src/gpu/GrGpu.cpp | 15 ++++++++++++++- src/gpu/GrGpu.h | 4 +++- src/gpu/GrInOrderDrawBuffer.cpp | 17 +++++++++++++---- src/gpu/GrInOrderDrawBuffer.h | 26 ++++++++++++++++++++++---- src/gpu/GrSoftwarePathRenderer.cpp | 23 +++++++++++++++++++++-- src/gpu/GrSoftwarePathRenderer.h | 2 +- src/gpu/SkGpuDevice.cpp | 4 +--- 11 files changed, 96 insertions(+), 43 deletions(-) diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h index 9389499ec..71263d435 100644 --- a/include/gpu/GrContext.h +++ b/include/gpu/GrContext.h @@ -342,8 +342,11 @@ public: * Clear the entire or rect of the render target, ignoring any clips. * @param rect the rect to clear or the whole thing if rect is NULL. * @param color the color to clear to. + * @param target if non-NULL, the render target to clear otherwise clear + * the current render target */ - void clear(const GrIRect* rect, GrColor color); + void clear(const GrIRect* rect, GrColor color, + GrRenderTarget* target = NULL); /** * Draw everywhere (respecting the clip) with the paint. diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index c16b4106a..53221a3aa 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -427,21 +427,6 @@ void GrClipMaskManager::drawTexture(GrTexture* target, drawState->setTexture(0, NULL); } -namespace { - -void clear(GrGpu* gpu, - GrTexture* target, - GrColor color) { - GrDrawState* drawState = gpu->drawState(); - GrAssert(NULL != drawState); - - // zap entire target to specified color - drawState->setRenderTarget(target->asRenderTarget()); - gpu->clear(NULL, color); -} - -} - // get a texture to act as a temporary buffer for AA clip boolean operations // TODO: given the expense of createTexture we may want to just cache this too void GrClipMaskManager::getTemp(const GrIRect& bounds, @@ -577,7 +562,9 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, &clearToInside, &startOp); - clear(fGpu, accum, clearToInside ? 0xffffffff : 0x00000000); + fGpu->clear(NULL, + clearToInside ? 0xffffffff : 0x00000000, + accum->asRenderTarget()); GrAutoScratchTexture temp; @@ -592,7 +579,7 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, // replace ops and alter GrClip to allow them through // clear the accumulator and draw the new object directly into it - clear(fGpu, accum, 0x00000000); + fGpu->clear(NULL, 0x00000000, accum->asRenderTarget()); setup_boolean_blendcoeffs(drawState, op); this->drawClipShape(accum, clipIn, c); @@ -613,7 +600,7 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, } // clear the temp target & draw into it - clear(fGpu, temp.texture(), 0x00000000); + fGpu->clear(NULL, 0x00000000, temp.texture()->asRenderTarget()); setup_boolean_blendcoeffs(drawState, SkRegion::kReplace_Op); this->drawClipShape(temp.texture(), clipIn, c); @@ -1146,11 +1133,11 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, GrDrawState* drawState = fGpu->drawState(); GrAssert(NULL != drawState); GrRenderTarget* temp = drawState->getRenderTarget(); - clear(fGpu, accum, 0x00000000); + fGpu->clear(NULL, 0x00000000, accum->asRenderTarget()); // can't leave the accum bound as a rendertarget drawState->setRenderTarget(temp); - helper.toTexture(accum); + helper.toTexture(accum, clearToInside); *result = accum; diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index d2b312f7c..1fda26ba0 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -605,9 +605,11 @@ void GrContext::setClip(const GrIRect& rect) { //////////////////////////////////////////////////////////////////////////////// -void GrContext::clear(const GrIRect* rect, const GrColor color) { +void GrContext::clear(const GrIRect* rect, + const GrColor color, + GrRenderTarget* target) { this->flush(); - fGpu->clear(rect, color); + fGpu->clear(rect, color, target); } void GrContext::drawPaint(const GrPaint& paint) { diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h index 7bd330788..cdc5073af 100644 --- a/src/gpu/GrDrawTarget.h +++ b/src/gpu/GrDrawTarget.h @@ -546,11 +546,13 @@ public: } /** - * Clear the render target. Ignores the clip and all other draw state - * (blend mode, stages, etc). Clears the whole thing if rect is NULL, - * otherwise just the rect. + * Clear the current render target if one isn't passed in. Ignores the + * clip and all other draw state (blend mode, stages, etc). Clears the + * whole thing if rect is NULL, otherwise just the rect. */ - virtual void clear(const GrIRect* rect, GrColor color) = 0; + virtual void clear(const GrIRect* rect, + GrColor color, + GrRenderTarget* renderTarget = NULL) = 0; /** * Release any resources that are cached but not currently in use. This diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp index 132d2ed4d..7807324e0 100644 --- a/src/gpu/GrGpu.cpp +++ b/src/gpu/GrGpu.cpp @@ -233,12 +233,25 @@ GrPath* GrGpu::createPath(const SkPath& path) { return this->onCreatePath(path); } -void GrGpu::clear(const GrIRect* rect, GrColor color) { +void GrGpu::clear(const GrIRect* rect, + GrColor color, + GrRenderTarget* renderTarget) { + GrRenderTarget* oldRT = NULL; + if (NULL != renderTarget && + renderTarget != this->drawState()->getRenderTarget()) { + oldRT = this->drawState()->getRenderTarget(); + this->drawState()->setRenderTarget(renderTarget); + } + if (NULL == this->getDrawState().getRenderTarget()) { return; } this->handleDirtyContext(); this->onClear(rect, color); + + if (NULL != oldRT) { + this->drawState()->setRenderTarget(oldRT); + } } void GrGpu::forceRenderTargetFlush() { diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h index 53c74249d..83d20c649 100644 --- a/src/gpu/GrGpu.h +++ b/src/gpu/GrGpu.h @@ -290,7 +290,9 @@ public: void removeResource(GrResource* resource); // GrDrawTarget overrides - virtual void clear(const GrIRect* rect, GrColor color) SK_OVERRIDE; + virtual void clear(const GrIRect* rect, + GrColor color, + GrRenderTarget* renderTarget = NULL) SK_OVERRIDE; virtual void purgeResources() SK_OVERRIDE { // The clip mask manager can rebuild all its clip masks so just diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp index 907b4f124..9d8f887d8 100644 --- a/src/gpu/GrInOrderDrawBuffer.cpp +++ b/src/gpu/GrInOrderDrawBuffer.cpp @@ -25,7 +25,8 @@ GrInOrderDrawBuffer::GrInOrderDrawBuffer(const GrGpu* gpu, , fLastRectVertexLayout(0) , fQuadIndexBuffer(NULL) , fMaxQuads(0) - , fCurrQuad(0) { + , fCurrQuad(0) + , fFlushing(false) { fCaps = gpu->getCaps(); @@ -443,7 +444,9 @@ void GrInOrderDrawBuffer::onStencilPath(const GrPath&, GrPathFill) { GrCrash("Not implemented yet. Should not get here."); } -void GrInOrderDrawBuffer::clear(const GrIRect* rect, GrColor color) { +void GrInOrderDrawBuffer::clear(const GrIRect* rect, + GrColor color, + GrRenderTarget* renderTarget) { GrIRect r; if (NULL == rect) { // We could do something smart and remove previous draws and clears to @@ -458,6 +461,8 @@ void GrInOrderDrawBuffer::clear(const GrIRect* rect, GrColor color) { clr.fColor = color; clr.fBeforeDrawIdx = fDraws.count(); clr.fRect = *rect; + clr.fRenderTarget = renderTarget; + GrSafeRef(clr.fRenderTarget); } void GrInOrderDrawBuffer::reset() { @@ -522,7 +527,9 @@ void GrInOrderDrawBuffer::playback(GrDrawTarget* target) { for (int i = 0; i < numDraws; ++i) { while (currClear < fClears.count() && i == fClears[currClear].fBeforeDrawIdx) { - target->clear(&fClears[currClear].fRect, fClears[currClear].fColor); + target->clear(&fClears[currClear].fRect, + fClears[currClear].fColor, + fClears[currClear].fRenderTarget); ++currClear; } @@ -556,7 +563,9 @@ void GrInOrderDrawBuffer::playback(GrDrawTarget* target) { } while (currClear < fClears.count()) { GrAssert(fDraws.count() == fClears[currClear].fBeforeDrawIdx); - target->clear(&fClears[currClear].fRect, fClears[currClear].fColor); + target->clear(&fClears[currClear].fRect, + fClears[currClear].fColor, + fClears[currClear].fRenderTarget); ++currClear; } target->setDrawState(prevDrawState); diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h index ed7d97d06..9b5561a1f 100644 --- a/src/gpu/GrInOrderDrawBuffer.h +++ b/src/gpu/GrInOrderDrawBuffer.h @@ -81,8 +81,18 @@ public: * constraints and side-effects or playback() and reset apply(). */ void flushTo(GrDrawTarget* target) { + if (fFlushing) { + // When creating SW-only clip masks, the GrClipMaskManager can + // cause a GrContext::flush (when copying the mask results back + // to the GPU). Without a guard this results in a recursive call + // to this method. + return; + } + + fFlushing = true; this->playback(target); this->reset(); + fFlushing = false; } /** @@ -112,7 +122,9 @@ public: int* vertexCount, int* indexCount) const SK_OVERRIDE; - virtual void clear(const GrIRect* rect, GrColor color) SK_OVERRIDE; + virtual void clear(const GrIRect* rect, + GrColor color, + GrRenderTarget* renderTarget = NULL) SK_OVERRIDE; protected: virtual void willReserveVertexAndIndexSpace(GrVertexLayout vertexLayout, @@ -133,9 +145,13 @@ private: }; struct Clear { - int fBeforeDrawIdx; - GrIRect fRect; - GrColor fColor; + Clear() : fRenderTarget(NULL) {} + ~Clear() { GrSafeUnref(fRenderTarget); } + + int fBeforeDrawIdx; + GrIRect fRect; + GrColor fColor; + GrRenderTarget* fRenderTarget; }; // overrides from GrDrawTarget @@ -226,6 +242,8 @@ private: }; SkSTArray fGeoPoolStateStack; + bool fFlushing; + typedef GrDrawTarget INHERITED; }; diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp index 312216298..72ad54312 100644 --- a/src/gpu/GrSoftwarePathRenderer.cpp +++ b/src/gpu/GrSoftwarePathRenderer.cpp @@ -13,6 +13,7 @@ #include "GrContext.h" #include "SkDraw.h" #include "SkRasterClip.h" +#include "GrGpu.h" //////////////////////////////////////////////////////////////////////////////// bool GrSoftwarePathRenderer::canDrawPath(const SkPath& path, @@ -222,8 +223,26 @@ bool GrSWMaskHelper::getTexture(GrAutoScratchTexture* tex) { /** * Move the result of the software mask generation back to the gpu */ -void GrSWMaskHelper::toTexture(GrTexture *texture) { +void GrSWMaskHelper::toTexture(GrTexture *texture, bool clearToWhite) { SkAutoLockPixels alp(fBM); + + // The destination texture is almost always larger than "fBM". Clear + // it appropriately so we don't get mask artifacts outside of the path's + // bounding box + + // "texture" needs to be installed as the render target for the clear + // and the texture upload but cannot remain the render target upon + // returned. Callers typically use it as a texture and it would then + // be both source and dest. + GrDrawState::AutoRenderTargetRestore artr(fContext->getGpu()->drawState(), + texture->asRenderTarget()); + + if (clearToWhite) { + fContext->getGpu()->clear(NULL, SK_ColorWHITE); + } else { + fContext->getGpu()->clear(NULL, 0x00000000); + } + texture->writePixels(0, 0, fBM.width(), fBM.height(), kAlpha_8_GrPixelConfig, fBM.getPixels(), fBM.rowBytes()); @@ -255,7 +274,7 @@ bool sw_draw_path_to_mask_texture(const SkPath& clientPath, return false; } - helper.toTexture(tex->texture()); + helper.toTexture(tex->texture(), false); return true; } diff --git a/src/gpu/GrSoftwarePathRenderer.h b/src/gpu/GrSoftwarePathRenderer.h index 74715f681..cb84506d2 100644 --- a/src/gpu/GrSoftwarePathRenderer.h +++ b/src/gpu/GrSoftwarePathRenderer.h @@ -40,7 +40,7 @@ public: bool getTexture(GrAutoScratchTexture* tex); - void toTexture(GrTexture* texture); + void toTexture(GrTexture* texture, bool clearToWhite); void clear(GrColor color) { fBM.eraseColor(color); diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 628adc4c9..3ce70d15f 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -592,10 +592,8 @@ inline bool skPaint2GrPaintShader(SkGpuDevice* dev, } /////////////////////////////////////////////////////////////////////////////// - void SkGpuDevice::clear(SkColor color) { - fContext->setRenderTarget(fRenderTarget); - fContext->clear(NULL, color); + fContext->clear(NULL, color, fRenderTarget); } void SkGpuDevice::drawPaint(const SkDraw& draw, const SkPaint& paint) {