Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(241)

Unified Diff: src/gpu/GrClipMaskManager.cpp

Issue 367013003: Remove the AA requirement for selecting GrEffect-based clipping. (Closed) Base URL: https://skia.googlesource.com/skia.git@master
Patch Set: update Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/gpu/GrClipMaskManager.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/gpu/GrClipMaskManager.cpp
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index a878894c59949410cf2c0522e766657f23c06336..23b6197f92e5c94156ed3bc132902e4bed07b608 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -110,7 +110,8 @@ bool GrClipMaskManager::useSWOnlyPath(const ElementList& elements) {
bool GrClipMaskManager::installClipEffects(const ElementList& elements,
GrDrawState::AutoRestoreEffects* are,
const SkVector& clipToRTOffset,
- const SkRect* drawBounds) {
+ const SkRect* drawBounds,
+ SkIRect* scissorRect) {
robertphillips 2014/07/02 14:44:34 SkASSERT(NULL != scissorRect && scissorRect->isEmp
bsalomon 2014/07/02 16:36:46 Added the NULL assertion, I'm not sure this functi
robertphillips 2014/07/02 16:48:28 Fair enough.
GrDrawState* drawState = fGpu->drawState();
SkRect boundsInClipSpace;
@@ -121,7 +122,11 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
are->set(drawState);
GrRenderTarget* rt = drawState->getRenderTarget();
- ElementList::Iter iter(elements);
+ // We iterate from the top of the stack to the bottom. We do this because we select the first
+ // BW rectangle as the scissor. Clients performing hierarchical rendering tend to use smaller
+ // clips towards the top of the clip stack. Smaller scissor rects can help tiled architectures
+ // skip processing tiles for draws.
+ ElementList::Iter iter(elements, ElementList::Iter::kTail_IterStart);
bool setARE = false;
bool failed = false;
@@ -157,7 +162,7 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
GrEffectEdgeType edgeType;
if (GR_AA_CLIP && iter.get()->isAA()) {
if (rt->isMultisampled()) {
- // Coverage based AA clips don't place nicely with MSAA.
+ // Coverage based AA clips don't play nicely with MSAA.
failed = true;
break;
}
@@ -165,6 +170,7 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
} else {
edgeType = invert ? kInverseFillBW_GrEffectEdgeType : kFillBW_GrEffectEdgeType;
}
robertphillips 2014/07/02 14:44:34 // We don't want to exit if we convert a BW rect c
bsalomon 2014/07/02 16:36:47 Done.
+ bool failIfNoEffect = true;
SkAutoTUnref<GrEffectRef> effect;
switch (iter.get()->getType()) {
case SkClipStack::Element::kPath_Type:
@@ -180,7 +186,12 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
case SkClipStack::Element::kRect_Type: {
SkRect rect = iter.get()->getRect();
rect.offset(clipToRTOffset.fX, clipToRTOffset.fY);
robertphillips 2014/07/02 14:44:34 Kind of hand wavy here but what about something li
bsalomon 2014/07/02 16:36:47 Expanding how? The round out? I think that's what
- effect.reset(GrConvexPolyEffect::Create(edgeType, rect));
+ if (kFillBW_GrEffectEdgeType == edgeType && scissorRect->isEmpty()) {
+ rect.roundOut(scissorRect);
+ failIfNoEffect = false;
+ } else {
+ effect.reset(GrConvexPolyEffect::Create(edgeType, rect));
+ }
break;
}
default:
@@ -192,12 +203,12 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
setARE = true;
}
fGpu->drawState()->addCoverageEffect(effect);
- } else {
+ } else if (failIfNoEffect) {
failed = true;
break;
}
}
- iter.next();
+ iter.prev();
}
if (failed) {
@@ -263,17 +274,28 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn,
// configuration's relative costs of switching RTs to generate a mask vs
// longer shaders.
if (elements.count() <= 4) {
+ SkIRect scissorRect;
+ scissorRect.setEmpty();
SkVector clipToRTOffset = { SkIntToScalar(-clipDataIn->fOrigin.fX),
SkIntToScalar(-clipDataIn->fOrigin.fY) };
if (elements.isEmpty() ||
- (requiresAA && this->installClipEffects(elements, are, clipToRTOffset, devBounds))) {
- SkIRect scissorSpaceIBounds(clipSpaceIBounds);
- scissorSpaceIBounds.offset(-clipDataIn->fOrigin);
- if (NULL == devBounds ||
- !SkRect::Make(scissorSpaceIBounds).contains(*devBounds)) {
- fGpu->enableScissor(scissorSpaceIBounds);
+ this->installClipEffects(elements, are, clipToRTOffset, devBounds, &scissorRect)) {
+ if (scissorRect.isEmpty()) {
+ // We may still want to use a scissor, especially on tiled architectures.
+ scissorRect = clipSpaceIBounds;
+ scissorRect.offset(-clipDataIn->fOrigin);
+ if (NULL == devBounds ||
robertphillips 2014/07/02 14:44:34 Add magical SkIRect contains Rect method ?
bsalomon 2014/07/02 16:36:47 This current code is actually overkill because con
+ !SkRect::Make(scissorRect).contains(*devBounds)) {
+ fGpu->enableScissor(scissorRect);
+ } else {
robertphillips 2014/07/02 14:44:34 Why not use devBounds intersected with scissorRect
bsalomon 2014/07/02 16:36:47 Scissor changes prevent batching. If we don't see
robertphillips 2014/07/02 16:48:28 That would make a good comment.
bsalomon 2014/07/02 17:28:46 Done, and added a comment about effects always shr
+ fGpu->disableScissor();
+ }
} else {
robertphillips 2014/07/02 14:44:34 Add SkIRect::intersectWH or, failing that, use SkI
bsalomon 2014/07/02 16:36:47 The intersect() methods on SkIRect all do empty ch
- fGpu->disableScissor();
+ scissorRect.fLeft = SkTMax(0, scissorRect.fLeft);
+ scissorRect.fTop = SkTMax(0, scissorRect.fTop);
+ scissorRect.fRight = SkTMin(rt->width(), scissorRect.fRight);
+ scissorRect.fBottom = SkTMin(rt->height(), scissorRect.fBottom);
+ fGpu->enableScissor(scissorRect);
}
this->setGpuStencil();
return true;
« no previous file with comments | « src/gpu/GrClipMaskManager.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698