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

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: remove bogus assert 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..d3c0cf3fc140ec3cc1c6cd4a6816bb41f4e4202a 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -110,7 +110,10 @@ 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) {
+
+ SkASSERT(NULL != scissorRect);
GrDrawState* drawState = fGpu->drawState();
SkRect boundsInClipSpace;
@@ -121,7 +124,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 +164,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 +172,8 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
} else {
edgeType = invert ? kInverseFillBW_GrEffectEdgeType : kFillBW_GrEffectEdgeType;
}
+ // We don't want to exit if we convert a BW rect clip to a scissor.
+ bool failIfNoEffect = true;
SkAutoTUnref<GrEffectRef> effect;
switch (iter.get()->getType()) {
case SkClipStack::Element::kPath_Type:
@@ -180,7 +189,14 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
case SkClipStack::Element::kRect_Type: {
SkRect rect = iter.get()->getRect();
rect.offset(clipToRTOffset.fX, clipToRTOffset.fY);
- effect.reset(GrConvexPolyEffect::Create(edgeType, rect));
+ if (kFillBW_GrEffectEdgeType == edgeType && scissorRect->isEmpty()) {
+ // This is OK because we only allow clip operations that shrink the clip
+ // to be implemented as effects.
+ rect.roundOut(scissorRect);
+ failIfNoEffect = false;
+ } else {
+ effect.reset(GrConvexPolyEffect::Create(edgeType, rect));
+ }
break;
}
default:
@@ -192,12 +208,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) {
@@ -207,6 +223,12 @@ bool GrClipMaskManager::installClipEffects(const ElementList& elements,
return !failed;
}
+static inline bool rect_contains_irect(const SkIRect ir, const SkRect& r) {
+ SkASSERT(!ir.isEmpty());
+ return ir.fLeft <= r.fLeft && ir.fTop <= r.fTop &&
+ ir.fRight >= r.fRight && ir.fBottom >= r.fBottom;
+}
+
////////////////////////////////////////////////////////////////////////////////
// sort out what kind of clip mask needs to be created: alpha, stencil,
// scissor, or entirely software
@@ -263,17 +285,32 @@ 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 ||
+ !rect_contains_irect(scissorRect, *devBounds)) {
+ fGpu->enableScissor(scissorRect);
+ } else {
+ // When the vertices that will be rendered fit fully inside the clip's bounds
+ // then providing the scissor rect will not help the driver eliminate tiles
+ // from consideration for the draw, but changing the scissor will cause
+ // state changes between draws.
+ fGpu->disableScissor();
+ }
} else {
- 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