From 366f1c6a09f63c76e78145cb08028f66062f31fd Mon Sep 17 00:00:00 2001 From: "robertphillips@google.com" Date: Fri, 29 Jun 2012 21:38:47 +0000 Subject: [PATCH] Fixed lingering gpu-path AA clip mask generation bug http://codereview.appspot.com/6351055/ git-svn-id: http://skia.googlecode.com/svn/trunk@4416 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/gpu/GrClipMaskManager.cpp | 88 ++++++++++++++++++++++++++++-------- src/gpu/GrClipMaskManager.h | 3 +- src/gpu/GrSWMaskHelper.cpp | 93 ++++++++++++++++++++++---------------- src/gpu/GrSWMaskHelper.h | 53 +++++++++++++++++----- src/gpu/GrSoftwarePathRenderer.cpp | 65 ++++++++------------------ 5 files changed, 186 insertions(+), 116 deletions(-) diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index 30944d9..cdc13f0 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -353,15 +353,63 @@ void setup_boolean_blendcoeffs(GrDrawState* drawState, SkRegion::Op op) { } //////////////////////////////////////////////////////////////////////////////// +bool draw_path_in_software(GrContext* context, + GrGpu* gpu, + const SkPath& path, + GrPathFill fill, + bool doAA, + const GrIRect& resultBounds) { + + GrAutoScratchTexture ast; + + if (!GrSWMaskHelper::DrawToTexture(context, path, resultBounds, fill, + &ast, doAA, NULL)) { + return false; + } + + // TODO: merge this with the similar code in the GrSoftwarePathRenderer.cpp + SkAutoTUnref texture(ast.detach()); + GrAssert(NULL != texture); + + GrDrawState::StageMask stageMask = 0; + GrDrawTarget::AutoDeviceCoordDraw adcd(gpu, stageMask); + enum { + // the SW path renderer shares this stage with glyph + // rendering (kGlyphMaskStage in GrBatchedTextContext) + kPathMaskStage = GrPaint::kTotalStages, + }; + GrAssert(NULL == gpu->drawState()->getTexture(kPathMaskStage)); + gpu->drawState()->setTexture(kPathMaskStage, texture); + gpu->drawState()->sampler(kPathMaskStage)->reset(); + GrScalar w = GrIntToScalar(resultBounds.width()); + GrScalar h = GrIntToScalar(resultBounds.height()); + GrRect maskRect = GrRect::MakeWH(w / texture->width(), + h / texture->height()); + + const GrRect* srcRects[GrDrawState::kNumStages] = {NULL}; + srcRects[kPathMaskStage] = &maskRect; + stageMask |= 1 << kPathMaskStage; + GrRect dstRect = GrRect::MakeWH( + SK_Scalar1* resultBounds.width(), + SK_Scalar1* resultBounds.height()); + gpu->drawRect(dstRect, NULL, stageMask, srcRects, NULL); + gpu->drawState()->setTexture(kPathMaskStage, NULL); + GrAssert(!GrIsFillInverted(fill)); + return true; +} + + +//////////////////////////////////////////////////////////////////////////////// bool draw_path(GrContext* context, GrGpu* gpu, const SkPath& path, GrPathFill fill, - bool doAA) { + bool doAA, + const GrIRect& resultBounds) { - GrPathRenderer* pr = context->getPathRenderer(path, fill, gpu, doAA, true); + GrPathRenderer* pr = context->getPathRenderer(path, fill, gpu, doAA, false); if (NULL == pr) { - return false; + return draw_path_in_software(context, gpu, path, fill, doAA, resultBounds); } pr->drawPath(path, fill, NULL, gpu, 0, doAA); @@ -373,7 +421,8 @@ bool draw_path(GrContext* context, //////////////////////////////////////////////////////////////////////////////// bool GrClipMaskManager::drawClipShape(GrTexture* target, const GrClip& clipIn, - int index) { + int index, + const GrIRect& resultBounds) { GrDrawState* drawState = fGpu->drawState(); GrAssert(NULL != drawState); @@ -391,7 +440,8 @@ bool GrClipMaskManager::drawClipShape(GrTexture* target, return draw_path(this->getContext(), fGpu, clipIn.getPath(index), clipIn.getPathFill(index), - clipIn.getDoAA(index)); + clipIn.getDoAA(index), + resultBounds); } return true; } @@ -461,7 +511,7 @@ void GrClipMaskManager::setupCache(const GrClip& clipIn, // Returns true if there is no more work to be done (i.e., we got a cache hit) bool GrClipMaskManager::clipMaskPreamble(const GrClip& clipIn, GrTexture** result, - GrIRect *resultBounds) { + GrIRect* resultBounds) { GrDrawState* origDrawState = fGpu->drawState(); GrAssert(origDrawState->isClipState()); @@ -516,7 +566,7 @@ bool GrClipMaskManager::clipMaskPreamble(const GrClip& clipIn, bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, GrTexture** result, GrIRect *resultBounds) { - + GrAssert(NULL != resultBounds); GrAssert(kNone_ClipMaskType == fCurrClipMaskType); if (this->clipMaskPreamble(clipIn, result, resultBounds)) { @@ -575,7 +625,7 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, fGpu->clear(NULL, 0x00000000, accum->asRenderTarget()); setup_boolean_blendcoeffs(drawState, op); - this->drawClipShape(accum, clipIn, c); + this->drawClipShape(accum, clipIn, c, *resultBounds); } else if (SkRegion::kReverseDifference_Op == op || SkRegion::kIntersect_Op == op) { @@ -596,7 +646,7 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, fGpu->clear(NULL, 0x00000000, temp.texture()->asRenderTarget()); setup_boolean_blendcoeffs(drawState, SkRegion::kReplace_Op); - this->drawClipShape(temp.texture(), clipIn, c); + this->drawClipShape(temp.texture(), clipIn, c, *resultBounds); // TODO: rather than adding these two translations here // compute the bounding box needed to render the texture @@ -628,7 +678,7 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClip& clipIn, // all the remaining ops can just be directly draw into // the accumulation buffer setup_boolean_blendcoeffs(drawState, op); - this->drawClipShape(accum, clipIn, c); + this->drawClipShape(accum, clipIn, c, *resultBounds); } } @@ -1025,7 +1075,7 @@ GrPathFill invert_fill(GrPathFill fill) { bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, GrTexture** result, - GrIRect *resultBounds) { + GrIRect* resultBounds) { GrAssert(kNone_ClipMaskType == fCurrClipMaskType); if (this->clipMaskPreamble(clipIn, result, resultBounds)) { @@ -1040,7 +1090,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, GrSWMaskHelper helper(this->getContext()); - helper.init(*resultBounds, NULL, false); + helper.init(*resultBounds, NULL); int count = clipIn.getElementCount(); @@ -1051,7 +1101,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, &clearToInside, &startOp); - helper.clear(clearToInside ? SK_ColorWHITE : 0x00000000); + helper.clear(clearToInside ? 0xFF : 0x00); for (int i = start; i < count; ++i) { @@ -1073,7 +1123,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, SkIntToScalar(resultBounds->bottom())); // invert the entire scene - helper.draw(temp, SkRegion::kXOR_Op, false, SK_ColorWHITE); + helper.draw(temp, SkRegion::kXOR_Op, false, 0xFF); } if (kRect_ClipType == clipIn.getElementType(i)) { @@ -1084,7 +1134,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, helper.draw(temp, SkRegion::kReplace_Op, kInverseEvenOdd_GrPathFill, clipIn.getDoAA(i), - 0x00000000); + 0x00); } else { GrAssert(kPath_ClipType == clipIn.getElementType(i)); @@ -1092,7 +1142,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, SkRegion::kReplace_Op, invert_fill(clipIn.getPathFill(i)), clipIn.getDoAA(i), - 0x00000000); + 0x00); } continue; @@ -1104,7 +1154,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, helper.draw(clipIn.getRect(i), op, - clipIn.getDoAA(i), SK_ColorWHITE); + clipIn.getDoAA(i), 0xFF); } else { GrAssert(kPath_ClipType == clipIn.getElementType(i)); @@ -1112,7 +1162,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, helper.draw(clipIn.getPath(i), op, clipIn.getPathFill(i), - clipIn.getDoAA(i), SK_ColorWHITE); + clipIn.getDoAA(i), 0xFF); } } @@ -1129,7 +1179,7 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClip& clipIn, // can't leave the accum bound as a rendertarget drawState->setRenderTarget(temp); - helper.toTexture(accum, clearToInside); + helper.toTexture(accum, clearToInside ? 0xFF : 0x00); *result = accum; diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h index 144c042..00ca642 100644 --- a/src/gpu/GrClipMaskManager.h +++ b/src/gpu/GrClipMaskManager.h @@ -362,7 +362,8 @@ private: bool drawClipShape(GrTexture* target, const GrClip& clipIn, - int index); + int index, + const GrIRect& resultBounds); void drawTexture(GrTexture* target, GrTexture* texture); diff --git a/src/gpu/GrSWMaskHelper.cpp b/src/gpu/GrSWMaskHelper.cpp index f67336f..e34151c 100644 --- a/src/gpu/GrSWMaskHelper.cpp +++ b/src/gpu/GrSWMaskHelper.cpp @@ -52,17 +52,17 @@ SkPath::FillType gr_fill_to_sk_fill(GrPathFill fill) { /** * Draw a single rect element of the clip stack into the accumulation bitmap */ -void GrSWMaskHelper::draw(const GrRect& clientRect, SkRegion::Op op, - bool antiAlias, GrColor color) { +void GrSWMaskHelper::draw(const GrRect& rect, SkRegion::Op op, + bool antiAlias, uint8_t alpha) { SkPaint paint; SkXfermode* mode = SkXfermode::Create(op_to_mode(op)); paint.setXfermode(mode); paint.setAntiAlias(antiAlias); - paint.setColor(color); + paint.setColor(SkColorSetARGB(alpha, alpha, alpha, alpha)); - fDraw.drawRect(clientRect, paint); + fDraw.drawRect(rect, paint); SkSafeUnref(mode); } @@ -70,12 +70,12 @@ void GrSWMaskHelper::draw(const GrRect& clientRect, SkRegion::Op op, /** * Draw a single path element of the clip stack into the accumulation bitmap */ -void GrSWMaskHelper::draw(const SkPath& clientPath, SkRegion::Op op, - GrPathFill fill, bool antiAlias, GrColor color) { +void GrSWMaskHelper::draw(const SkPath& path, SkRegion::Op op, + GrPathFill fill, bool antiAlias, uint8_t alpha) { SkPaint paint; SkPath tmpPath; - const SkPath* pathToDraw = &clientPath; + const SkPath* pathToDraw = &path; if (kHairLine_GrPathFill == fill) { paint.setStyle(SkPaint::kStroke_Style); paint.setStrokeWidth(SK_Scalar1); @@ -92,30 +92,26 @@ void GrSWMaskHelper::draw(const SkPath& clientPath, SkRegion::Op op, paint.setXfermode(mode); paint.setAntiAlias(antiAlias); - paint.setColor(color); + paint.setColor(SkColorSetARGB(alpha, alpha, alpha, alpha)); fDraw.drawPath(*pathToDraw, paint); SkSafeUnref(mode); } -bool GrSWMaskHelper::init(const GrIRect& pathDevBounds, - const GrPoint* translate, - bool useMatrix) { - if (useMatrix) { - fMatrix = fContext->getMatrix(); +bool GrSWMaskHelper::init(const GrIRect& resultBounds, + const GrMatrix* matrix) { + if (NULL != matrix) { + fMatrix = *matrix; } else { fMatrix.setIdentity(); } - if (NULL != translate) { - fMatrix.postTranslate(translate->fX, translate->fY); - } - - fMatrix.postTranslate(-pathDevBounds.fLeft * SK_Scalar1, - -pathDevBounds.fTop * SK_Scalar1); - GrIRect bounds = GrIRect::MakeWH(pathDevBounds.width(), - pathDevBounds.height()); + // Now translate so the bound's UL corner is at the origin + fMatrix.postTranslate(-resultBounds.fLeft * SK_Scalar1, + -resultBounds.fTop * SK_Scalar1); + GrIRect bounds = GrIRect::MakeWH(resultBounds.width(), + resultBounds.height()); fBM.setConfig(SkBitmap::kA8_Config, bounds.fRight, bounds.fBottom); if (!fBM.allocPixels()) { @@ -133,28 +129,23 @@ bool GrSWMaskHelper::init(const GrIRect& pathDevBounds, } /** - * Get a texture (from the texture cache) of the correct size & format + * Get a texture (from the texture cache) of the correct size & format. + * Return true on success; false on failure. */ -bool GrSWMaskHelper::getTexture(GrAutoScratchTexture* tex) { +bool GrSWMaskHelper::getTexture(GrAutoScratchTexture* texture) { GrTextureDesc desc; desc.fWidth = fBM.width(); desc.fHeight = fBM.height(); desc.fConfig = kAlpha_8_GrPixelConfig; - tex->set(fContext, desc); - GrTexture* texture = tex->texture(); - - if (NULL == texture) { - return false; - } - - return true; + texture->set(fContext, desc); + return NULL != texture->texture(); } /** * Move the result of the software mask generation back to the gpu */ -void GrSWMaskHelper::toTexture(GrTexture *texture, bool clearToWhite) { +void GrSWMaskHelper::toTexture(GrTexture *texture, uint8_t alpha) { SkAutoLockPixels alp(fBM); // The destination texture is almost always larger than "fBM". Clear @@ -163,19 +154,45 @@ void GrSWMaskHelper::toTexture(GrTexture *texture, bool clearToWhite) { // "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 + // return. 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); - } + fContext->getGpu()->clear(NULL, SkColorSetARGB(alpha, alpha, alpha, alpha)); texture->writePixels(0, 0, fBM.width(), fBM.height(), kAlpha_8_GrPixelConfig, fBM.getPixels(), fBM.rowBytes()); } +//////////////////////////////////////////////////////////////////////////////// +/** + * Software rasterizes path to A8 mask (possibly using the context's matrix) + * and uploads the result to a scratch texture. Returns true on success; + * false on failure. + */ +bool GrSWMaskHelper::DrawToTexture(GrContext* context, + const SkPath& path, + const GrIRect& resultBounds, + GrPathFill fill, + GrAutoScratchTexture* result, + bool antiAlias, + GrMatrix* matrix) { + GrSWMaskHelper helper(context); + + if (!helper.init(resultBounds, matrix)) { + return false; + } + + helper.draw(path, SkRegion::kReplace_Op, fill, antiAlias, 0xFF); + + if (!helper.getTexture(result)) { + return false; + } + + helper.toTexture(result->texture(), 0x00); + + return true; +} + diff --git a/src/gpu/GrSWMaskHelper.h b/src/gpu/GrSWMaskHelper.h index 00bf323..54ae089 100644 --- a/src/gpu/GrSWMaskHelper.h +++ b/src/gpu/GrSWMaskHelper.h @@ -23,7 +23,17 @@ class SkPath; /** * The GrSWMaskHelper helps generate clip masks using the software rendering - * path. + * path. It is intended to be used as: + * + * GrSWMaskHelper helper(context); + * helper.init(...); + * + * draw one or more paths/rects specifying the required boolean ops + * + * toTexture(); // to get it from the internal bitmap to the GPU + * + * The result of this process will be the final mask (on the GPU) in the + * upper left hand corner of the texture. */ class GrSWMaskHelper : public GrNoncopyable { public: @@ -31,24 +41,43 @@ public: : fContext(context) { } - void draw(const GrRect& clientRect, SkRegion::Op op, - bool antiAlias, GrColor color); + // set up the internal state in preparation for draws. Since many masks + // may be accumulated in the helper during creation, "resultBounds" + // allows the caller to specify the region of interest - to limit the + // amount of work. + bool init(const GrIRect& resultBounds, const GrMatrix* matrix); - void draw(const SkPath& clientPath, SkRegion::Op op, - GrPathFill fill, bool antiAlias, GrColor color); + // Draw a single rect into the accumulation bitmap using the specified op + void draw(const GrRect& rect, SkRegion::Op op, + bool antiAlias, uint8_t alpha); - bool init(const GrIRect& pathDevBounds, - const GrPoint* translate, - bool useMatrix); + // Draw a single path into the accumuation bitmap using the specified op + void draw(const SkPath& path, SkRegion::Op op, + GrPathFill fill, bool antiAlias, uint8_t alpha); - bool getTexture(GrAutoScratchTexture* tex); + // Helper function to get a scratch texture suitable for capturing the + // result (i.e., right size & format) + bool getTexture(GrAutoScratchTexture* texture); - void toTexture(GrTexture* texture, bool clearToWhite); + // Move the mask generation results from the internal bitmap to the gpu. + // The space outside of the mask is cleared using "alpha" + void toTexture(GrTexture* texture, uint8_t alpha); - void clear(GrColor color) { - fBM.eraseColor(color); + // Reset the internal bitmap + void clear(uint8_t alpha) { + fBM.eraseColor(SkColorSetARGB(alpha, alpha, alpha, alpha)); } + // Canonical usage utility that draws a single path and uploads it + // to the GPU. The result is returned in "result". + static bool DrawToTexture(GrContext* context, + const SkPath& path, + const GrIRect& resultBounds, + GrPathFill fill, + GrAutoScratchTexture* result, + bool antiAlias, + GrMatrix* matrix); + protected: private: GrContext* fContext; diff --git a/src/gpu/GrSoftwarePathRenderer.cpp b/src/gpu/GrSoftwarePathRenderer.cpp index 2566a53..f1c87a4 100644 --- a/src/gpu/GrSoftwarePathRenderer.cpp +++ b/src/gpu/GrSoftwarePathRenderer.cpp @@ -36,7 +36,7 @@ namespace { // path bounds would be empty. bool get_path_and_clip_bounds(const GrDrawTarget* target, const SkPath& path, - const GrVec* translate, + const GrMatrix& matrix, GrIRect* pathBounds, GrIRect* clipBounds) { // compute bounds as intersection of rt size, clip, and path @@ -55,13 +55,9 @@ bool get_path_and_clip_bounds(const GrDrawTarget* target, // pathBounds is currently the rt extent, set clip bounds to that rect. *clipBounds = *pathBounds; } - GrRect pathSBounds = path.getBounds(); - if (!pathSBounds.isEmpty()) { - if (NULL != translate) { - pathSBounds.offset(*translate); - } - target->getDrawState().getViewMatrix().mapRect(&pathSBounds, - pathSBounds); + if (!path.getBounds().isEmpty()) { + GrRect pathSBounds; + matrix.mapRect(&pathSBounds, path.getBounds()); GrIRect pathIBounds; pathSBounds.roundOut(&pathIBounds); if (!pathBounds->intersect(pathIBounds)) { @@ -77,36 +73,6 @@ bool get_path_and_clip_bounds(const GrDrawTarget* target, } //////////////////////////////////////////////////////////////////////////////// -/** - * sw rasterizes path to A8 mask using the context's matrix and uploads to a - * scratch texture. - */ -bool sw_draw_path_to_mask_texture(const SkPath& clientPath, - const GrIRect& pathDevBounds, - GrPathFill fill, - GrContext* context, - const GrPoint* translate, - GrAutoScratchTexture* tex, - bool antiAlias) { - GrSWMaskHelper helper(context); - - if (!helper.init(pathDevBounds, translate, true)) { - return false; - } - - helper.draw(clientPath, SkRegion::kReplace_Op, - fill, antiAlias, SK_ColorWHITE); - - if (!helper.getTexture(tex)) { - return false; - } - - helper.toTexture(tex->texture(), false); - - return true; -} - -//////////////////////////////////////////////////////////////////////////////// void draw_around_inv_path(GrDrawTarget* target, GrDrawState::StageMask stageMask, const GrIRect& clipBounds, @@ -150,9 +116,16 @@ bool GrSoftwarePathRenderer::onDrawPath(const SkPath& path, return false; } + GrDrawState* drawState = target->drawState(); + + GrMatrix vm = drawState->getViewMatrix(); + if (NULL != translate) { + vm.postTranslate(translate->fX, translate->fY); + } + GrAutoScratchTexture ast; GrIRect pathBounds, clipBounds; - if (!get_path_and_clip_bounds(target, path, translate, + if (!get_path_and_clip_bounds(target, path, vm, &pathBounds, &clipBounds)) { if (GrIsFillInverted(fill)) { draw_around_inv_path(target, stageMask, @@ -160,9 +133,9 @@ bool GrSoftwarePathRenderer::onDrawPath(const SkPath& path, } return true; } - if (sw_draw_path_to_mask_texture(path, pathBounds, - fill, fContext, - translate, &ast, antiAlias)) { + + if (GrSWMaskHelper::DrawToTexture(fContext, path, pathBounds, fill, + &ast, antiAlias, &vm)) { SkAutoTUnref texture(ast.detach()); GrAssert(NULL != texture); GrDrawTarget::AutoDeviceCoordDraw adcd(target, stageMask); @@ -171,9 +144,9 @@ bool GrSoftwarePathRenderer::onDrawPath(const SkPath& path, // rendering (kGlyphMaskStage in GrBatchedTextContext) kPathMaskStage = GrPaint::kTotalStages, }; - GrAssert(NULL == target->drawState()->getTexture(kPathMaskStage)); - target->drawState()->setTexture(kPathMaskStage, texture); - target->drawState()->sampler(kPathMaskStage)->reset(); + GrAssert(NULL == drawState->getTexture(kPathMaskStage)); + drawState->setTexture(kPathMaskStage, texture); + drawState->sampler(kPathMaskStage)->reset(); GrScalar w = GrIntToScalar(pathBounds.width()); GrScalar h = GrIntToScalar(pathBounds.height()); GrRect maskRect = GrRect::MakeWH(w / texture->width(), @@ -188,7 +161,7 @@ bool GrSoftwarePathRenderer::onDrawPath(const SkPath& path, SK_Scalar1* pathBounds.fRight, SK_Scalar1* pathBounds.fBottom); target->drawRect(dstRect, NULL, stageMask, srcRects, NULL); - target->drawState()->setTexture(kPathMaskStage, NULL); + drawState->setTexture(kPathMaskStage, NULL); if (GrIsFillInverted(fill)) { draw_around_inv_path(target, stageMask, clipBounds, pathBounds); -- 2.7.4