From f8d904a7eed435b9de68fd2eef6d7f3c59fcc9cc Mon Sep 17 00:00:00 2001 From: "robertphillips@google.com" Date: Tue, 31 Jul 2012 12:18:16 +0000 Subject: [PATCH] GrClip no longer translates its clips (to better mimic SkClipStack's behavior) http://codereview.appspot.com/6445052/ git-svn-id: http://skia.googlecode.com/svn/trunk@4848 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/gpu/GrClip.h | 18 ++---- src/gpu/GrClip.cpp | 27 +++++--- src/gpu/GrClipMaskManager.cpp | 139 ++++++++++++++++++++++++++---------------- src/gpu/GrClipMaskManager.h | 12 +--- src/gpu/GrTextContext.cpp | 2 + src/gpu/SkGpuDevice.cpp | 8 +-- tests/ClipCacheTest.cpp | 2 +- 7 files changed, 117 insertions(+), 91 deletions(-) diff --git a/include/gpu/GrClip.h b/include/gpu/GrClip.h index d6655dd..bb07c28 100644 --- a/include/gpu/GrClip.h +++ b/include/gpu/GrClip.h @@ -23,11 +23,7 @@ class GrClip { public: GrClip(); GrClip(const GrClip& src); - /** - * The conservativeBounds parameter already takes (tx,ty) into account. - */ - GrClip(GrClipIterator* iter, GrScalar tx, GrScalar ty, - const GrRect& conservativeBounds); + GrClip(GrClipIterator* iter, const GrRect& conservativeBounds); explicit GrClip(const GrIRect& rect); explicit GrClip(const GrRect& rect); @@ -142,20 +138,16 @@ public: } } - // FIXME: This word "empty" is confusing. It means that the clip has no - // elements (it is the infinite plane) not that it has no area. - bool isEmpty() const { return 0 == fList.count(); } + // isWideOpen returns true if the clip has no elements (it is the + // infinite plane) not that it has no area. + bool isWideOpen() const { return 0 == fList.count(); } /** * Resets this clip to be empty */ void setEmpty(); - /** - * If specified, the bounds parameter already takes (tx,ty) into account. - */ - void setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty, - const GrRect& conservativeBounds); + void setFromIterator(GrClipIterator* iter, const GrRect& conservativeBounds); void setFromRect(const GrRect& rect); void setFromIRect(const GrIRect& rect); diff --git a/src/gpu/GrClip.cpp b/src/gpu/GrClip.cpp index 9ce89f3..a3905ba 100644 --- a/src/gpu/GrClip.cpp +++ b/src/gpu/GrClip.cpp @@ -30,9 +30,8 @@ GrClip::GrClip(const GrRect& rect) { this->setFromRect(rect); } -GrClip::GrClip(GrClipIterator* iter, GrScalar tx, GrScalar ty, - const GrRect& bounds) { - this->setFromIterator(iter, tx, ty, bounds); +GrClip::GrClip(GrClipIterator* iter, const GrRect& bounds) { + this->setFromIterator(iter, bounds); } GrClip::~GrClip() {} @@ -92,7 +91,7 @@ static void intersectWith(SkRect* dst, const SkRect& src) { } } -void GrClip::setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty, +void GrClip::setFromIterator(GrClipIterator* iter, const GrRect& conservativeBounds) { fList.reset(); fRequiresAA = false; @@ -116,9 +115,6 @@ void GrClip::setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty, switch (e.fType) { case kRect_ClipType: iter->getRect(&e.fRect); - if (tx || ty) { - e.fRect.offset(tx, ty); - } ++rectCount; if (isectRectValid) { if (SkRegion::kIntersect_Op == e.fOp) { @@ -137,9 +133,6 @@ void GrClip::setFromIterator(GrClipIterator* iter, GrScalar tx, GrScalar ty, break; case kPath_ClipType: e.fPath = *iter->getPath(); - if (tx || ty) { - e.fPath.offset(tx, ty); - } e.fPathFill = iter->getPathFill(); isectRectValid = false; break; @@ -252,6 +245,12 @@ void GrClip::Iter::reset(const GrClip& stack, IterStart startLoc) { /////////////////////////////////////////////////////////////////////////////// +/** + * getConservativeBounds returns the conservative bounding box of the clip + * in device (as opposed to canvas) coordinates. If the bounding box is + * the result of purely intersections of rects (with an initial replace) + * isIntersectionOfRects will be set to true. + */ void GrClipData::getConservativeBounds(const GrSurface* surface, GrIRect* result, bool* isIntersectionOfRects) const { @@ -266,10 +265,18 @@ void GrClipData::getConservativeBounds(const GrSurface* surface, SkIntToScalar(surface->height())); GrRect conservativeBounds = fClipStack->getConservativeBounds(); + + conservativeBounds.offset(SkIntToScalar(-fOrigin.fX), + SkIntToScalar(-fOrigin.fY)); + if (!conservativeBounds.intersect(temp)) { conservativeBounds.setEmpty(); } conservativeBounds.roundOut(result); + + if (NULL != isIntersectionOfRects) { + *isIntersectionOfRects = fClipStack->isRect(); + } } diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index ea5a2cd..97c102f 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -20,6 +20,18 @@ //#define GR_AA_CLIP 1 //#define GR_SW_CLIP 1 +GrClipMaskCache::GrClipMaskCache() + : fContext(NULL) + , fStack(sizeof(GrClipStackFrame)) { + // We need an initial frame to capture the clip state prior to + // any pushes + SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame); +} + +void GrClipMaskCache::push() { + SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame); +} + //////////////////////////////////////////////////////////////////////////////// namespace { // set up the draw state to enable the aa clipping mask. Besides setting up the @@ -136,7 +148,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) { fCurrClipMaskType = kNone_ClipMaskType; GrDrawState* drawState = fGpu->drawState(); - if (!drawState->isClipState() || clipDataIn->fClipStack->isEmpty()) { + if (!drawState->isClipState() || clipDataIn->fClipStack->isWideOpen()) { fGpu->disableScissor(); this->setGpuStencil(); return true; @@ -162,7 +174,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) { // Otherwise check if we should just create the entire clip mask // in software (this will only happen if the clip mask is anti-aliased // and too complex for the gpu to handle in its entirety) - if (0 == rt->numSamples() && + if (0 == rt->numSamples() && requiresAA && this->useSWOnlyPath(*clipDataIn->fClipStack)) { // The clip geometry is complex enough that it will be more @@ -214,14 +226,14 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) { // If the clip is a rectangle then just set the scissor. Otherwise, create // a stencil mask. - if (clipDataIn->fClipStack->isRect()) { + if (isIntersectionOfRects) { fGpu->enableScissor(bounds); this->setGpuStencil(); return true; } // use the stencil clip if we can't represent the clip as a rectangle. - bool useStencil = !clipDataIn->fClipStack->isEmpty() && !bounds.isEmpty(); + bool useStencil = !clipDataIn->fClipStack->isWideOpen() && !bounds.isEmpty(); if (useStencil) { this->createStencilClipMask(*clipDataIn, bounds); @@ -248,17 +260,20 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn) { namespace { /** * Does "container" contain "containee"? If either is empty then - * no containment is possible. + * no containment is possible. "container" is in canvas coordinates while + * "containee" is in device coordiates. "origin" provides the mapping between + * the two. */ -bool contains(const SkRect& container, const SkIRect& containee) { +bool contains(const SkRect& container, + const SkIRect& containee, + const SkIPoint& origin) { return !containee.isEmpty() && !container.isEmpty() && - container.fLeft <= SkIntToScalar(containee.fLeft) && - container.fTop <= SkIntToScalar(containee.fTop) && - container.fRight >= SkIntToScalar(containee.fRight) && - container.fBottom >= SkIntToScalar(containee.fBottom); + container.fLeft <= SkIntToScalar(containee.fLeft+origin.fX) && + container.fTop <= SkIntToScalar(containee.fTop+origin.fY) && + container.fRight >= SkIntToScalar(containee.fRight+origin.fX) && + container.fBottom >= SkIntToScalar(containee.fBottom+origin.fY); } - //////////////////////////////////////////////////////////////////////////////// // determines how many elements at the head of the clip can be skipped and // whether the initial clear should be to the inside- or outside-the-clip value, @@ -267,7 +282,8 @@ const GrClip::Iter::Clip* process_initial_clip_elements( GrClip::Iter* iter, const GrIRect& bounds, bool* clearToInside, - SkRegion::Op* firstOp) { + SkRegion::Op* firstOp, + const GrClipData& clipData) { GrAssert(NULL != iter && NULL != clearToInside && NULL != firstOp); @@ -294,7 +310,8 @@ const GrClip::Iter::Clip* process_initial_clip_elements( case SkRegion::kIntersect_Op: // if this element contains the entire bounds then we // can skip it. - if (NULL != clip->fRect && contains(*clip->fRect, bounds)) { + if (NULL != clip->fRect && + contains(*clip->fRect, bounds, clipData.fOrigin)) { break; } // if everything is initially clearToInside then intersect is @@ -360,7 +377,6 @@ const GrClip::Iter::Clip* process_initial_clip_elements( } - namespace { //////////////////////////////////////////////////////////////////////////////// @@ -450,7 +466,7 @@ bool GrClipMaskManager::drawClipShape(GrTexture* target, if (NULL != clip->fRect) { if (clip->fDoAA) { getContext()->getAARectRenderer()->fillAARect(fGpu, fGpu, - *clip->fRect, + *clip->fRect, true); } else { fGpu->drawSimpleRect(*clip->fRect, NULL); @@ -537,6 +553,7 @@ bool GrClipMaskManager::clipMaskPreamble(const GrClipData& clipDataIn, // unlike the stencil path the alpha path is not bound to the size of the // render target - determine the minimum size required for the mask + // Note: intBounds is in device (as opposed to canvas) coordinates GrIRect intBounds; clipDataIn.getConservativeBounds(rt, &intBounds); @@ -573,6 +590,8 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, return true; } + // Note: 'resultBounds' is in device (as opposed to canvas) coordinates + GrTexture* accum = fAACache.getLastMask(); if (NULL == accum) { fAACache.reset(); @@ -584,25 +603,25 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, GrDrawTarget::AutoGeometryPush agp(fGpu); - if (0 != resultBounds->fTop || 0 != resultBounds->fLeft) { + if (0 != resultBounds->fTop || 0 != resultBounds->fLeft || + 0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) { // if we were able to trim down the size of the mask we need to // offset the paths & rects that will be used to compute it - GrMatrix m; - - m.setTranslate(SkIntToScalar(-resultBounds->fLeft), - SkIntToScalar(-resultBounds->fTop)); - - drawState->setViewMatrix(m); + drawState->viewMatrix()->setTranslate( + SkIntToScalar(-resultBounds->fLeft-clipDataIn.fOrigin.fX), + SkIntToScalar(-resultBounds->fTop-clipDataIn.fOrigin.fY)); } bool clearToInside; SkRegion::Op firstOp = SkRegion::kReplace_Op; // suppress warning - GrClip::Iter iter(*clipDataIn.fClipStack, GrClip::Iter::kBottom_IterStart); + GrClip::Iter iter(*clipDataIn.fClipStack, + GrClip::Iter::kBottom_IterStart); const GrClip::Iter::Clip* clip = process_initial_clip_elements(&iter, *resultBounds, &clearToInside, - &firstOp); + &firstOp, + clipDataIn); fGpu->clear(NULL, clearToInside ? 0xffffffff : 0x00000000, @@ -633,9 +652,8 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, } else if (SkRegion::kReverseDifference_Op == op || SkRegion::kIntersect_Op == op) { // there is no point in intersecting a screen filling rectangle. - if (SkRegion::kIntersect_Op == op && - NULL != clip->fRect && - contains(*clip->fRect, *resultBounds)) { + if (SkRegion::kIntersect_Op == op && NULL != clip->fRect && + contains(*clip->fRect, *resultBounds, clipDataIn.fOrigin)) { continue; } @@ -654,13 +672,11 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, // TODO: rather than adding these two translations here // compute the bounding box needed to render the texture // into temp - if (0 != resultBounds->fTop || 0 != resultBounds->fLeft) { - GrMatrix m; - - m.setTranslate(SkIntToScalar(resultBounds->fLeft), - SkIntToScalar(resultBounds->fTop)); - - drawState->preConcatViewMatrix(m); + if (0 != resultBounds->fTop || 0 != resultBounds->fLeft || + 0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) { + // In order for the merge of the temp clip into the accumulator + // to work we need to disable the translation + drawState->viewMatrix()->reset(); } // Now draw into the accumulator using the real operation @@ -668,13 +684,11 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, setup_boolean_blendcoeffs(drawState, op); this->drawTexture(accum, temp.texture()); - if (0 != resultBounds->fTop || 0 != resultBounds->fLeft) { - GrMatrix m; - - m.setTranslate(SkIntToScalar(-resultBounds->fLeft), - SkIntToScalar(-resultBounds->fTop)); - - drawState->preConcatViewMatrix(m); + if (0 != resultBounds->fTop || 0 != resultBounds->fLeft || + 0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) { + drawState->viewMatrix()->setTranslate( + SkIntToScalar(-resultBounds->fLeft-clipDataIn.fOrigin.fX), + SkIntToScalar(-resultBounds->fTop-clipDataIn.fOrigin.fY)); } } else { @@ -691,7 +705,8 @@ bool GrClipMaskManager::createAlphaClipMask(const GrClipData& clipDataIn, } //////////////////////////////////////////////////////////////////////////////// -// Create a 1-bit clip mask in the stencil buffer +// Create a 1-bit clip mask in the stencil buffer. 'bounds' are in device +// (as opposed to canvas) coordinates bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, const GrIRect& bounds) { @@ -730,6 +745,17 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, drawState->setRenderTarget(rt); GrDrawTarget::AutoGeometryPush agp(fGpu); + if (0 != clipDataIn.fOrigin.fX || 0 != clipDataIn.fOrigin.fY) { + // Add the saveLayer's offset to the view matrix rather than + // offset each individual draw + GrMatrix m; + + m.setTranslate(SkIntToScalar(-clipDataIn.fOrigin.fX), + SkIntToScalar(-clipDataIn.fOrigin.fY)); + + drawState->setViewMatrix(m); + } + #if !VISUALIZE_COMPLEX_CLIP drawState->enableState(GrDrawState::kNoColorWrites_StateBit); #endif @@ -749,7 +775,8 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, const GrClip::Iter::Clip* clip = process_initial_clip_elements(&iter, rtRect, &clearToInside, - &firstOp); + &firstOp, + clipDataIn); fGpu->clearStencilClip(bounds, clearToInside); bool first = true; @@ -791,7 +818,7 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, // there is no point in intersecting a screen filling // rectangle. if (SkRegion::kIntersect_Op == op && - contains(*clip->fRect, rtRect)) { + contains(*clip->fRect, rtRect, oldClipData->fOrigin)) { continue; } } else if (NULL != clip->fPath) { @@ -862,11 +889,15 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, } } else { SET_RANDOM_COLOR + // 'bounds' is already in device coordinates so the + // translation in the view matrix is inappropriate. + // Apply the inverse translation so the drawn rect will + // be in the correct location GrRect rect = GrRect::MakeLTRB( - SkIntToScalar(bounds.fLeft), - SkIntToScalar(bounds.fTop), - SkIntToScalar(bounds.fRight), - SkIntToScalar(bounds.fBottom)); + SkIntToScalar(bounds.fLeft+clipDataIn.fOrigin.fX), + SkIntToScalar(bounds.fTop+clipDataIn.fOrigin.fY), + SkIntToScalar(bounds.fRight+clipDataIn.fOrigin.fX), + SkIntToScalar(bounds.fBottom+clipDataIn.fOrigin.fY)); fGpu->drawSimpleRect(rect, NULL); } } @@ -880,6 +911,7 @@ bool GrClipMaskManager::createStencilClipMask(const GrClipData& clipDataIn, return true; } + // mapping of clip-respecting stencil funcs to normal stencil funcs // mapping depends on whether stencil-clipping is in effect. static const GrStencilFunc @@ -1105,16 +1137,21 @@ bool GrClipMaskManager::createSoftwareClipMask(const GrClipData& clipDataIn, GrSWMaskHelper helper(this->getContext()); - helper.init(*resultBounds, NULL); + GrMatrix matrix; + matrix.setTranslate(SkIntToScalar(-clipDataIn.fOrigin.fX), + SkIntToScalar(-clipDataIn.fOrigin.fY)); + helper.init(*resultBounds, &matrix); bool clearToInside; SkRegion::Op firstOp = SkRegion::kReplace_Op; // suppress warning - GrClip::Iter iter(*clipDataIn.fClipStack, GrClip::Iter::kBottom_IterStart); + GrClip::Iter iter(*clipDataIn.fClipStack, + GrClip::Iter::kBottom_IterStart); const GrClip::Iter::Clip* clip = process_initial_clip_elements(&iter, *resultBounds, &clearToInside, - &firstOp); + &firstOp, + clipDataIn); helper.clear(clearToInside ? 0xFF : 0x00); diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h index 0551b8c..81be5ed 100644 --- a/src/gpu/GrClipMaskManager.h +++ b/src/gpu/GrClipMaskManager.h @@ -33,13 +33,7 @@ class GrDrawState; */ class GrClipMaskCache : public GrNoncopyable { public: - GrClipMaskCache() - : fContext(NULL) - , fStack(sizeof(GrClipStackFrame)) { - // We need an initial frame to capture the clip state prior to - // any pushes - SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame); - } + GrClipMaskCache(); ~GrClipMaskCache() { @@ -86,9 +80,7 @@ public: * TODO: can we take advantage of the nested nature of the clips to * reduce the mask creation cost? */ - void push() { - SkNEW_PLACEMENT(fStack.push_back(), GrClipStackFrame); - } + void push(); void pop() { //GrAssert(!fStack.empty()); diff --git a/src/gpu/GrTextContext.cpp b/src/gpu/GrTextContext.cpp index 36e6646..ab04bb4 100644 --- a/src/gpu/GrTextContext.cpp +++ b/src/gpu/GrTextContext.cpp @@ -88,6 +88,8 @@ GrTextContext::GrTextContext(GrContext* context, const GrClipData* clipData = context->getClip(); GrRect conservativeBound = clipData->fClipStack->getConservativeBounds(); + conservativeBound.offset(SkIntToScalar(-clipData->fOrigin.fX), + SkIntToScalar(-clipData->fOrigin.fY)); if (!fExtMatrix.isIdentity()) { GrMatrix inverse; diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 3e5bba0..dcfb57b 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -415,17 +415,13 @@ static void convert_matrixclip(GrContext* context, const SkMatrix& matrix, SkRect bounds; bool isIntersectionOfRects = false; - clipStack.getConservativeBounds(-origin.fX, - -origin.fY, + clipStack.getConservativeBounds(0, 0, renderTargetWidth, renderTargetHeight, &bounds, &isIntersectionOfRects); - result->setFromIterator(&iter, - GrIntToScalar(-origin.x()), - GrIntToScalar(-origin.y()), - bounds); + result->setFromIterator(&iter, bounds); GrAssert(result->isRect() == isIntersectionOfRects); diff --git a/tests/ClipCacheTest.cpp b/tests/ClipCacheTest.cpp index e3ff245..516a614 100644 --- a/tests/ClipCacheTest.cpp +++ b/tests/ClipCacheTest.cpp @@ -83,7 +83,7 @@ static void test_clip_bounds(skiatest::Reporter* reporter, GrContext* context) { iter.reset(stack); GrClip clip; - clip.setFromIterator(&iter, 0, 0, stackBounds); + clip.setFromIterator(&iter, stackBounds); const GrRect& grBound = clip.getConservativeBounds(); -- 2.7.4