From 3a0cfeb96185934c0a36f1313f21b96c57ca6341 Mon Sep 17 00:00:00 2001 From: joshualitt Date: Mon, 27 Oct 2014 07:38:01 -0700 Subject: [PATCH] Revert of Clip mask manager sets stencil on draw type (patchset #3 id:40001 of https://codereview.chromium.org/676983003/) Reason for revert: seems to cause a gm issue on windows. Original issue's description: > Clip mask manager sets stencil on draw type > > BUG=skia: > > Committed: https://skia.googlesource.com/skia/+/7afb5aa201e4b59397cbd8480e121d7501a227e7 TBR=bsalomon@google.com,joshualitt@chromium.org NOTREECHECKS=true NOTRY=true BUG=skia: Review URL: https://codereview.chromium.org/678843003 --- src/gpu/GrClipMaskManager.cpp | 36 +++++++++++++++++++----------------- src/gpu/GrClipMaskManager.h | 6 ++---- src/gpu/GrDrawState.h | 30 ------------------------------ src/gpu/GrGpu.cpp | 18 ++++++------------ src/gpu/GrGpu.h | 18 ++++++++++++++++-- src/gpu/gl/GrGpuGL.cpp | 16 ++++++++-------- src/gpu/gl/GrGpuGL.h | 2 +- src/gpu/gl/GrGpuGL_program.cpp | 2 +- 8 files changed, 53 insertions(+), 75 deletions(-) diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index 0738108..349d51e 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -17,12 +17,12 @@ #include "GrRenderTarget.h" #include "GrStencilBuffer.h" #include "GrSWMaskHelper.h" -#include "SkRasterClip.h" -#include "SkStrokeRec.h" -#include "SkTLazy.h" #include "effects/GrTextureDomain.h" #include "effects/GrConvexPolyEffect.h" #include "effects/GrRRectEffect.h" +#include "SkRasterClip.h" +#include "SkStrokeRec.h" +#include "SkTLazy.h" #define GR_AA_CLIP 1 @@ -212,7 +212,6 @@ bool GrClipMaskManager::installClipEffects(const GrReducedClip::ElementList& ele // scissor, or entirely software bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn, GrDrawState::AutoRestoreEffects* are, - GrDrawState::AutoRestoreStencil* asr, const SkRect* devBounds) { fCurrClipMaskType = kNone_ClipMaskType; @@ -229,6 +228,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn, SkASSERT(rt); bool ignoreClip = !drawState->isClipState() || clipDataIn->fClipStack->isWideOpen(); + if (!ignoreClip) { SkIRect clipSpaceRTIBounds = SkIRect::MakeWH(rt->width(), rt->height()); clipSpaceRTIBounds.offset(clipDataIn->fOrigin); @@ -250,7 +250,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn, if (ignoreClip) { fGpu->disableScissor(); - this->setDrawStateStencil(asr); + this->setGpuStencil(); return true; } @@ -275,7 +275,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn, } else { fGpu->disableScissor(); } - this->setDrawStateStencil(asr); + this->setGpuStencil(); return true; } } @@ -307,7 +307,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn, are->set(fGpu->drawState()); setup_drawstate_aaclip(fGpu, result, rtSpaceMaskBounds); fGpu->disableScissor(); - this->setDrawStateStencil(asr); + this->setGpuStencil(); return true; } // if alpha clip mask creation fails fall through to the non-AA code paths @@ -335,7 +335,7 @@ bool GrClipMaskManager::setupClipping(const GrClipData* clipDataIn, SkIRect scissorSpaceIBounds(clipSpaceIBounds); scissorSpaceIBounds.offset(clipSpaceToStencilSpaceOffset); fGpu->enableScissor(scissorSpaceIBounds); - this->setDrawStateStencil(asr); + this->setGpuStencil(); return true; } @@ -400,11 +400,11 @@ bool GrClipMaskManager::drawElement(GrTexture* target, // TODO: Do rects directly to the accumulator using a aa-rect GrProcessor that covers // the entire mask bounds and writes 0 outside the rect. if (element->isAA()) { - this->getContext()->getAARectRenderer()->fillAARect(fGpu, - fGpu, - element->getRect(), - SkMatrix::I(), - element->getRect()); + getContext()->getAARectRenderer()->fillAARect(fGpu, + fGpu, + element->getRect(), + SkMatrix::I(), + element->getRect()); } else { fGpu->drawSimpleRect(element->getRect()); } @@ -707,6 +707,7 @@ bool GrClipMaskManager::createStencilClipMask(int32_t elementsGenID, } if (stencilBuffer->mustRenderClip(elementsGenID, clipSpaceIBounds, clipSpaceToStencilOffset)) { + stencilBuffer->setLastClip(elementsGenID, clipSpaceIBounds, clipSpaceToStencilOffset); // Set the matrix so that rendered clip elements are transformed from clip to stencil space. @@ -902,7 +903,7 @@ const GrStencilSettings& basic_apply_stencil_clip_settings() { } } -void GrClipMaskManager::setDrawStateStencil(GrDrawState::AutoRestoreStencil* ars) { +void GrClipMaskManager::setGpuStencil() { // We make two copies of the StencilSettings here (except in the early // exit scenario. One copy from draw state to the stack var. Then another // from the stack var to the gpu. We could make this class hold a ptr to @@ -932,6 +933,7 @@ void GrClipMaskManager::setDrawStateStencil(GrDrawState::AutoRestoreStencil* ars if (GrClipMaskManager::kRespectClip_StencilClipMode == clipMode) { settings = basic_apply_stencil_clip_settings(); } else { + fGpu->disableStencil(); return; } } else { @@ -940,7 +942,8 @@ void GrClipMaskManager::setDrawStateStencil(GrDrawState::AutoRestoreStencil* ars // TODO: dynamically attach a stencil buffer int stencilBits = 0; - GrStencilBuffer* stencilBuffer = drawState.getRenderTarget()->getStencilBuffer(); + GrStencilBuffer* stencilBuffer = + drawState.getRenderTarget()->getStencilBuffer(); if (stencilBuffer) { stencilBits = stencilBuffer->bits(); } @@ -948,8 +951,7 @@ void GrClipMaskManager::setDrawStateStencil(GrDrawState::AutoRestoreStencil* ars SkASSERT(fGpu->caps()->stencilWrapOpsSupport() || !settings.usesWrapOp()); SkASSERT(fGpu->caps()->twoSidedStencilSupport() || !settings.isTwoSided()); this->adjustStencilParams(&settings, clipMode, stencilBits); - ars->set(fGpu->drawState()); - fGpu->drawState()->setStencil(settings); + fGpu->setStencilSettings(settings); } void GrClipMaskManager::adjustStencilParams(GrStencilSettings* settings, diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h index 8a4b45a..c98648b 100644 --- a/src/gpu/GrClipMaskManager.h +++ b/src/gpu/GrClipMaskManager.h @@ -50,9 +50,7 @@ public: * the manager when it must install additional effects to implement the * clip. devBounds is optional but can help optimize clipping. */ - bool setupClipping(const GrClipData* clipDataIn, - GrDrawState::AutoRestoreEffects*, - GrDrawState::AutoRestoreStencil*, + bool setupClipping(const GrClipData* clipDataIn, GrDrawState::AutoRestoreEffects*, const SkRect* devBounds); /** @@ -175,7 +173,7 @@ private: * updates the GrGpu with stencil settings that account stencil-based * clipping. */ - void setDrawStateStencil(GrDrawState::AutoRestoreStencil* asr); + void setGpuStencil(); /** * Adjusts the stencil settings to account for interaction with stencil diff --git a/src/gpu/GrDrawState.h b/src/gpu/GrDrawState.h index 71c44d4..3043fd7 100644 --- a/src/gpu/GrDrawState.h +++ b/src/gpu/GrDrawState.h @@ -346,36 +346,6 @@ public: int fCoverageEffectCnt; }; - /** - * AutoRestoreStencil - * - * This simple struct saves and restores the stencil settings - */ - class AutoRestoreStencil : public ::SkNoncopyable { - public: - AutoRestoreStencil() : fDrawState(NULL) {} - - AutoRestoreStencil(GrDrawState* ds) : fDrawState(NULL) { this->set(ds); } - - ~AutoRestoreStencil() { this->set(NULL); } - - void set(GrDrawState* ds) { - if (fDrawState) { - fDrawState->setStencil(fStencilSettings); - } - fDrawState = ds; - if (ds) { - fStencilSettings = ds->getStencil(); - } - } - - bool isSet() const { return SkToBool(fDrawState); } - - private: - GrDrawState* fDrawState; - GrStencilSettings fStencilSettings; - }; - /// @} /////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp index d8ffe67..6510048 100644 --- a/src/gpu/GrGpu.cpp +++ b/src/gpu/GrGpu.cpp @@ -298,12 +298,10 @@ const GrIndexBuffer* GrGpu::getQuadIndexBuffer() const { //////////////////////////////////////////////////////////////////////////////// -bool GrGpu::setupClipAndFlushState(DrawType type, - const GrDeviceCoordTexture* dstCopy, +bool GrGpu::setupClipAndFlushState(DrawType type, const GrDeviceCoordTexture* dstCopy, GrDrawState::AutoRestoreEffects* are, - GrDrawState::AutoRestoreStencil* ars, const SkRect* devBounds) { - if (!fClipMaskManager.setupClipping(this->getClip(), are, ars, devBounds)) { + if (!fClipMaskManager.setupClipping(this->getClip(), are, devBounds)) { return false; } @@ -346,9 +344,8 @@ void GrGpu::geometrySourceWillPop(const GeometrySrcState& restoredState) { void GrGpu::onDraw(const DrawInfo& info) { this->handleDirtyContext(); GrDrawState::AutoRestoreEffects are; - GrDrawState::AutoRestoreStencil asr; if (!this->setupClipAndFlushState(PrimTypeToDrawType(info.primitiveType()), - info.getDstCopy(), &are, &asr, info.getDevBounds())) { + info.getDstCopy(), &are, info.getDevBounds())) { return; } this->onGpuDraw(info); @@ -358,8 +355,7 @@ void GrGpu::onStencilPath(const GrPath* path, SkPath::FillType fill) { this->handleDirtyContext(); GrDrawState::AutoRestoreEffects are; - GrDrawState::AutoRestoreStencil asr; - if (!this->setupClipAndFlushState(kStencilPath_DrawType, NULL, &are, &asr, NULL)) { + if (!this->setupClipAndFlushState(kStencilPath_DrawType, NULL, &are, NULL)) { return; } @@ -374,8 +370,7 @@ void GrGpu::onDrawPath(const GrPath* path, SkPath::FillType fill, drawState()->setDefaultVertexAttribs(); GrDrawState::AutoRestoreEffects are; - GrDrawState::AutoRestoreStencil asr; - if (!this->setupClipAndFlushState(kDrawPath_DrawType, dstCopy, &are, &asr, NULL)) { + if (!this->setupClipAndFlushState(kDrawPath_DrawType, dstCopy, &are, NULL)) { return; } @@ -391,8 +386,7 @@ void GrGpu::onDrawPaths(const GrPathRange* pathRange, drawState()->setDefaultVertexAttribs(); GrDrawState::AutoRestoreEffects are; - GrDrawState::AutoRestoreStencil asr; - if (!this->setupClipAndFlushState(kDrawPaths_DrawType, dstCopy, &are, &asr, NULL)) { + if (!this->setupClipAndFlushState(kDrawPaths_DrawType, dstCopy, &are, NULL)) { return; } diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h index 5ce2528..e4669a2 100644 --- a/src/gpu/GrGpu.h +++ b/src/gpu/GrGpu.h @@ -306,6 +306,18 @@ public: } void disableScissor() { fScissorState.fEnabled = false; } + /** + * Like the scissor methods above this is called by setupClipping and + * should be flushed by the GrGpu subclass in flushGraphicsState. These + * stencil settings should be used in place of those on the GrDrawState. + * They have been adjusted to account for any interactions between the + * GrDrawState's stencil settings and stencil clipping. + */ + void setStencilSettings(const GrStencilSettings& settings) { + fStencilSettings = settings; + } + void disableStencil() { fStencilSettings.setDisabled(); } + // GrGpu subclass sets clip bit in the stencil buffer. The subclass is // free to clear the remaining bits to zero if masked clears are more // expensive than clearing all bits. @@ -357,8 +369,7 @@ protected: // prepares clip flushes gpu state before a draw bool setupClipAndFlushState(DrawType, const GrDeviceCoordTexture* dstCopy, - GrDrawState::AutoRestoreEffects*, - GrDrawState::AutoRestoreStencil*, + GrDrawState::AutoRestoreEffects* are, const SkRect* devBounds); // Functions used to map clip-respecting stencil tests into normal @@ -393,6 +404,9 @@ protected: SkIRect fRect; } fScissorState; + // The final stencil settings to use as determined by the clip manager. + GrStencilSettings fStencilSettings; + // Helpers for setting up geometry state void finalizeReservedVertices(); void finalizeReservedIndices(); diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp index 75c9b3a..c83a668 100644 --- a/src/gpu/gl/GrGpuGL.cpp +++ b/src/gpu/gl/GrGpuGL.cpp @@ -1904,9 +1904,9 @@ void set_gl_stencil(const GrGLInterface* gl, } } -void GrGpuGL::flushStencil(const GrStencilSettings& stencilSettings, DrawType type) { - if (kStencilPath_DrawType != type && fHWStencilSettings != stencilSettings) { - if (stencilSettings.isDisabled()) { +void GrGpuGL::flushStencil(DrawType type) { + if (kStencilPath_DrawType != type && fHWStencilSettings != fStencilSettings) { + if (fStencilSettings.isDisabled()) { if (kNo_TriState != fHWStencilTestEnabled) { GL_CALL(Disable(GR_GL_STENCIL_TEST)); fHWStencilTestEnabled = kNo_TriState; @@ -1917,24 +1917,24 @@ void GrGpuGL::flushStencil(const GrStencilSettings& stencilSettings, DrawType ty fHWStencilTestEnabled = kYes_TriState; } } - if (!stencilSettings.isDisabled()) { + if (!fStencilSettings.isDisabled()) { if (this->caps()->twoSidedStencilSupport()) { set_gl_stencil(this->glInterface(), - stencilSettings, + fStencilSettings, GR_GL_FRONT, GrStencilSettings::kFront_Face); set_gl_stencil(this->glInterface(), - stencilSettings, + fStencilSettings, GR_GL_BACK, GrStencilSettings::kBack_Face); } else { set_gl_stencil(this->glInterface(), - stencilSettings, + fStencilSettings, GR_GL_FRONT_AND_BACK, GrStencilSettings::kFront_Face); } } - fHWStencilSettings = stencilSettings; + fHWStencilSettings = fStencilSettings; } } diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h index f18962c..24ab4ec 100644 --- a/src/gpu/gl/GrGpuGL.h +++ b/src/gpu/gl/GrGpuGL.h @@ -235,7 +235,7 @@ private: // NULL means whole target. Can be an empty rect. void flushRenderTarget(GrGLRenderTarget*, const SkIRect* bounds); - void flushStencil(const GrStencilSettings&, DrawType); + void flushStencil(DrawType); void flushAAState(const GrOptDrawState&, DrawType); bool configToGLFormats(GrPixelConfig config, diff --git a/src/gpu/gl/GrGpuGL_program.cpp b/src/gpu/gl/GrGpuGL_program.cpp index ecfd813..6a09ebf 100644 --- a/src/gpu/gl/GrGpuGL_program.cpp +++ b/src/gpu/gl/GrGpuGL_program.cpp @@ -258,7 +258,7 @@ bool GrGpuGL::flushGraphicsState(DrawType type, const GrDeviceCoordTexture* dstC } GrGLRenderTarget* glRT = static_cast(optState->getRenderTarget()); - this->flushStencil(optState->getStencil(), type); + this->flushStencil(type); this->flushScissor(glRT->getViewport(), glRT->origin()); this->flushAAState(*optState.get(), type); -- 2.7.4