From 873ea0c93f202600ec2591bc1e2e5d7a1e05f59d Mon Sep 17 00:00:00 2001 From: "bsalomon@google.com" Date: Fri, 30 Mar 2012 15:55:32 +0000 Subject: [PATCH] Make fewer copies when using GrDrawTarget::AutoStateRestore Review URL: http://codereview.appspot.com/5938043/ git-svn-id: http://skia.googlecode.com/svn/trunk@3557 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/gpu/GrAAConvexPathRenderer.cpp | 4 +- src/gpu/GrAAHairLinePathRenderer.cpp | 10 ++-- src/gpu/GrContext.cpp | 26 ++++----- src/gpu/GrDefaultPathRenderer.cpp | 4 +- src/gpu/GrDrawTarget.cpp | 47 +++++++--------- src/gpu/GrDrawTarget.h | 100 ++++++++++++++++------------------- src/gpu/GrGpu.cpp | 10 ++-- src/gpu/GrInOrderDrawBuffer.cpp | 13 ++--- src/gpu/GrInOrderDrawBuffer.h | 2 +- src/gpu/GrTesselatedPathRenderer.cpp | 2 +- 10 files changed, 101 insertions(+), 117 deletions(-) diff --git a/src/gpu/GrAAConvexPathRenderer.cpp b/src/gpu/GrAAConvexPathRenderer.cpp index c539060..28aeef1 100644 --- a/src/gpu/GrAAConvexPathRenderer.cpp +++ b/src/gpu/GrAAConvexPathRenderer.cpp @@ -454,14 +454,14 @@ bool GrAAConvexPathRenderer::onDrawPath(const SkPath& origPath, if (path->isEmpty()) { return true; } + GrDrawTarget::AutoStateRestore asr(target, + GrDrawTarget::kPreserve_ASRInit); GrDrawState* drawState = target->drawState(); - GrDrawTarget::AutoStateRestore asr; GrMatrix vm = drawState->getViewMatrix(); if (NULL != translate) { vm.postTranslate(translate->fX, translate->fY); } - asr.set(target); GrMatrix ivm; if (vm.invert(&ivm)) { drawState->preConcatSamplerMatrices(stageMask, ivm); diff --git a/src/gpu/GrAAHairLinePathRenderer.cpp b/src/gpu/GrAAHairLinePathRenderer.cpp index 3e5a1fa..3015674 100644 --- a/src/gpu/GrAAHairLinePathRenderer.cpp +++ b/src/gpu/GrAAHairLinePathRenderer.cpp @@ -609,17 +609,21 @@ bool GrAAHairLinePathRenderer::onDrawPath(const SkPath& path, return false; } - GrDrawState* drawState = target->drawState(); - GrDrawTarget::AutoStateRestore asr; + GrDrawState* drawState = target->drawState(); if (!drawState->getViewMatrix().hasPerspective()) { - asr.set(target); + // we are going to whack the view matrix to identity to remove + // perspective. + asr.set(target, + GrDrawTarget::kPreserve_ASRInit); + drawState = target->drawState(); GrMatrix ivm; if (drawState->getViewInverse(&ivm)) { drawState->preConcatSamplerMatrices(stageMask, ivm); } drawState->viewMatrix()->reset(); } + // TODO: See whether rendering lines as degenerate quads improves perf // when we have a mix diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index ca4df24..4ef0f5b 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -268,10 +268,9 @@ void apply_morphology(GrGpu* gpu, GrAssert(filter == GrSamplerState::kErode_Filter || filter == GrSamplerState::kDilate_Filter); - GrDrawTarget::AutoStateRestore asr(gpu); + GrRenderTarget* target = gpu->drawState()->getRenderTarget(); + GrDrawTarget::AutoStateRestore asr(gpu, GrDrawTarget::kReset_ASRInit); GrDrawState* drawState = gpu->drawState(); - GrRenderTarget* target = drawState->getRenderTarget(); - drawState->reset(); drawState->setRenderTarget(target); GrMatrix sampleM; sampleM.setIDiv(texture->width(), texture->height()); @@ -289,10 +288,9 @@ void convolve(GrGpu* gpu, const float* kernel, int kernelWidth, GrSamplerState::FilterDirection direction) { - GrDrawTarget::AutoStateRestore asr(gpu); + GrRenderTarget* target = gpu->drawState()->getRenderTarget(); + GrDrawTarget::AutoStateRestore asr(gpu, GrDrawTarget::kReset_ASRInit); GrDrawState* drawState = gpu->drawState(); - GrRenderTarget* target = drawState->getRenderTarget(); - drawState->reset(); drawState->setRenderTarget(target); GrMatrix sampleM; sampleM.setIDiv(texture->width(), texture->height()); @@ -428,9 +426,9 @@ GrContext::TextureCacheEntry GrContext::createAndLockTexture( GrTexture* texture = fGpu->createTexture(rtDesc, NULL, 0); if (NULL != texture) { - GrDrawTarget::AutoStateRestore asr(fGpu); + GrDrawTarget::AutoStateRestore asr(fGpu, + GrDrawTarget::kReset_ASRInit); GrDrawState* drawState = fGpu->drawState(); - drawState->reset(); drawState->setRenderTarget(texture->asRenderTarget()); drawState->setTexture(0, clampEntry.texture()); @@ -1729,9 +1727,9 @@ bool GrContext::internalReadRenderTargetPixels(GrRenderTarget* target, target = texture->asRenderTarget(); GrAssert(NULL != target); - GrDrawTarget::AutoStateRestore asr(fGpu); + GrDrawTarget::AutoStateRestore asr(fGpu, + GrDrawTarget::kReset_ASRInit); GrDrawState* drawState = fGpu->drawState(); - drawState->reset(); drawState->setRenderTarget(target); GrMatrix matrix; @@ -1773,9 +1771,8 @@ void GrContext::copyTexture(GrTexture* src, GrRenderTarget* dst) { } ASSERT_OWNED_RESOURCE(src); - GrDrawTarget::AutoStateRestore asr(fGpu); + GrDrawTarget::AutoStateRestore asr(fGpu, GrDrawTarget::kReset_ASRInit); GrDrawState* drawState = fGpu->drawState(); - drawState->reset(); drawState->setRenderTarget(dst); GrMatrix sampleM; sampleM.setIDiv(src->width(), src->height()); @@ -1798,7 +1795,7 @@ void GrContext::internalWriteRenderTargetPixels(GrRenderTarget* target, ASSERT_OWNED_RESOURCE(target); if (NULL == target) { - target = fGpu->drawState()->getRenderTarget(); + target = fDrawState->getRenderTarget(); if (NULL == target) { return; } @@ -1865,9 +1862,8 @@ void GrContext::internalWriteRenderTargetPixels(GrRenderTarget* target, this->internalWriteTexturePixels(texture, 0, 0, width, height, config, buffer, rowBytes, flags); - GrDrawTarget::AutoStateRestore asr(fGpu); + GrDrawTarget::AutoStateRestore asr(fGpu, GrDrawTarget::kReset_ASRInit); GrDrawState* drawState = fGpu->drawState(); - drawState->reset(); GrMatrix matrix; matrix.setTranslate(GrIntToScalar(left), GrIntToScalar(top)); diff --git a/src/gpu/GrDefaultPathRenderer.cpp b/src/gpu/GrDefaultPathRenderer.cpp index 791dc68..b164131 100644 --- a/src/gpu/GrDefaultPathRenderer.cpp +++ b/src/gpu/GrDefaultPathRenderer.cpp @@ -340,7 +340,6 @@ bool GrDefaultPathRenderer::internalDrawPath(const SkPath& path, GrMatrix viewM = target->getDrawState().getViewMatrix(); GrScalar tol = GR_Scalar1; tol = GrPathUtils::scaleToleranceToSrc(tol, viewM, path.getBounds()); - GrDrawState* drawState = target->drawState(); int vertexCnt; int indexCnt; @@ -360,7 +359,8 @@ bool GrDefaultPathRenderer::internalDrawPath(const SkPath& path, } GrAssert(NULL != target); - GrDrawTarget::AutoStateRestore asr(target); + GrDrawTarget::AutoStateRestore asr(target, GrDrawTarget::kPreserve_ASRInit); + GrDrawState* drawState = target->drawState(); bool colorWritesWereDisabled = drawState->isColorWriteDisabled(); // face culling doesn't make sense here GrAssert(GrDrawState::kBoth_DrawFace == drawState->getDrawFace()); diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp index ef03a61..730450b 100644 --- a/src/gpu/GrDrawTarget.cpp +++ b/src/gpu/GrDrawTarget.cpp @@ -527,18 +527,6 @@ void GrDrawTarget::setDrawState(GrDrawState* drawState) { } } -void GrDrawTarget::saveCurrentDrawState(SavedDrawState* state) const { - state->fState.set(this->getDrawState()); -} - -void GrDrawTarget::restoreDrawState(const SavedDrawState& state) { - *fDrawState = *state.fState.get(); -} - -void GrDrawTarget::copyDrawState(const GrDrawTarget& srcTarget) { - *fDrawState = srcTarget.getDrawState(); -} - bool GrDrawTarget::reserveVertexSpace(GrVertexLayout vertexLayout, int vertexCount, void** vertices) { @@ -1155,29 +1143,34 @@ GrDrawTarget::AutoStateRestore::AutoStateRestore() { fDrawTarget = NULL; } -GrDrawTarget::AutoStateRestore::AutoStateRestore(GrDrawTarget* target) { - fDrawTarget = target; - if (NULL != fDrawTarget) { - fDrawTarget->saveCurrentDrawState(&fDrawState); - } +GrDrawTarget::AutoStateRestore::AutoStateRestore(GrDrawTarget* target, + ASRInit init) { + fDrawTarget = NULL; + this->set(target, init); } GrDrawTarget::AutoStateRestore::~AutoStateRestore() { if (NULL != fDrawTarget) { - fDrawTarget->restoreDrawState(fDrawState); + fDrawTarget->setDrawState(fSavedState); + fSavedState->unref(); } } -void GrDrawTarget::AutoStateRestore::set(GrDrawTarget* target) { - if (target != fDrawTarget) { - if (NULL != fDrawTarget) { - fDrawTarget->restoreDrawState(fDrawState); - } - if (NULL != target) { - target->saveCurrentDrawState(&fDrawState); - } - fDrawTarget = target; +void GrDrawTarget::AutoStateRestore::set(GrDrawTarget* target, ASRInit init) { + GrAssert(NULL == fDrawTarget); + fDrawTarget = target; + fSavedState = target->drawState(); + GrAssert(fSavedState); + fSavedState->ref(); + if (kReset_ASRInit == init) { + // calls the default cons + fTempState.init(); + } else { + GrAssert(kPreserve_ASRInit == init); + // calls the copy cons + fTempState.set(*fSavedState); } + target->setDrawState(fTempState.get()); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h index 8f5be3c..ca499db 100644 --- a/src/gpu/GrDrawTarget.h +++ b/src/gpu/GrDrawTarget.h @@ -154,43 +154,6 @@ public: bool willUseHWAALines() const; /** - * Used to save and restore the GrGpu's drawing state - */ - struct SavedDrawState { - private: - SkTLazy fState; - friend class GrDrawTarget; - }; - - /** - * Saves the current draw state. The state can be restored at a later time - * with restoreDrawState. - * - * See also AutoStateRestore class. - * - * @param state will hold the state after the function returns. - */ - void saveCurrentDrawState(SavedDrawState* state) const; - - /** - * Restores previously saved draw state. The client guarantees that state - * was previously passed to saveCurrentDrawState and that the rendertarget - * and texture set at save are still valid. - * - * See also AutoStateRestore class. - * - * @param state the previously saved state to restore. - */ - void restoreDrawState(const SavedDrawState& state); - - /** - * Copies the draw state from another target to this target. - * - * @param srcTarget draw target used as src of the draw state. - */ - void copyDrawState(const GrDrawTarget& srcTarget); - - /** * The format of vertices is represented as a bitfield of flags. * Flags that indicate the layout of vertex data. Vertices always contain * positions and may also contain up to GrDrawState::kMaxTexCoords sets @@ -580,22 +543,62 @@ public: //////////////////////////////////////////////////////////////////////////// + /** + * See AutoStateRestore below. + */ + enum ASRInit { + kPreserve_ASRInit, + kReset_ASRInit + }; + + /** + * Saves off the current state and restores it in the destructor. It will + * install a new GrDrawState object on the target (setDrawState) and restore + * the previous one in the destructor. The caller should call drawState() to + * get the new draw state after the ASR is installed. + * + * GrDrawState* state = target->drawState(); + * AutoStateRestore asr(target, GrDrawTarget::kReset_ASRInit). + * state->setRenderTarget(rt); // state refers to the GrDrawState set on + * // target before asr was initialized. + * // Therefore, rt is set on the GrDrawState + * // that will be restored after asr's + * // destructor rather than target's current + * // GrDrawState. + */ class AutoStateRestore : ::GrNoncopyable { public: + /** + * Default ASR will have no effect unless set() is subsequently called. + */ AutoStateRestore(); - AutoStateRestore(GrDrawTarget* target); + + /** + * Saves the state on target. The state will be restored when the ASR + * is destroyed. If this constructor is used do not call set(). + * + * @param init Should the newly installed GrDrawState be a copy of the + * previous state or a default-initialized GrDrawState. + */ + AutoStateRestore(GrDrawTarget* target, ASRInit init); + ~AutoStateRestore(); /** - * if this object is already saving state for param target then - * this does nothing. Otherise, it restores previously saved state on - * previous target (if any) and saves current state on param target. + * Saves the state on target. The state will be restored when the ASR + * is destroyed. This should only be called once per ASR object and only + * when the default constructor was used. For nested saves use multiple + * ASR objects. + * + * @param init Should the newly installed GrDrawState be a copy of the + * previous state or a default-initialized GrDrawState. */ - void set(GrDrawTarget* target); + void set(GrDrawTarget* target, ASRInit init); private: - GrDrawTarget* fDrawTarget; - SavedDrawState fDrawState; + GrDrawTarget* fDrawTarget; + SkTLazy fTempState; + GrDrawState* fSavedState; }; //////////////////////////////////////////////////////////////////////////// @@ -990,15 +993,6 @@ protected: return mask; } - // Helpers for GrDrawTarget subclasses that won't have private access to - // SavedDrawState but need to peek at the state values. - static GrDrawState& accessSavedDrawState(SavedDrawState& sds) { - return *sds.fState.get(); - } - static const GrDrawState& accessSavedDrawState(const SavedDrawState& sds){ - return *sds.fState.get(); - } - // A sublcass can optionally overload this function to be notified before // vertex and index space is reserved. virtual void willReserveVertexAndIndexSpace(GrVertexLayout vertexLayout, diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp index 6863630..aeb104c 100644 --- a/src/gpu/GrGpu.cpp +++ b/src/gpu/GrGpu.cpp @@ -544,7 +544,7 @@ bool GrGpu::setupClipAndFlushState(GrPrimitiveType type) { GrIRect clipRect; GrDrawState* drawState = this->drawState(); - const GrRenderTarget* rt = drawState->getRenderTarget(); + GrRenderTarget* rt = drawState->getRenderTarget(); // GrDrawTarget should have filtered this for us GrAssert(NULL != rt); @@ -592,16 +592,16 @@ bool GrGpu::setupClipAndFlushState(GrPrimitiveType type) { const GrClip& clip = stencilBuffer->getLastClip(); fClip.setFromRect(bounds); - AutoStateRestore asr(this); + AutoStateRestore asr(this, GrDrawTarget::kReset_ASRInit); + drawState = this->drawState(); + drawState->setRenderTarget(rt); AutoGeometryPush agp(this); - drawState->viewMatrix()->reset(); this->flushScissor(NULL); #if !VISUALIZE_COMPLEX_CLIP drawState->enableState(GrDrawState::kNoColorWrites_StateBit); -#else - drawState->disableState(GrDrawState::kNoColorWrites_StateBit); #endif + int count = clip.getElementCount(); int clipBit = stencilBuffer->bits(); SkASSERT((clipBit <= 16) && diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp index ab46a47..e6b3ca8 100644 --- a/src/gpu/GrInOrderDrawBuffer.cpp +++ b/src/gpu/GrInOrderDrawBuffer.cpp @@ -462,11 +462,10 @@ void GrInOrderDrawBuffer::reset() { this->resetIndexSource(); uint32_t numStates = fStates.count(); for (uint32_t i = 0; i < numStates; ++i) { - const GrDrawState& dstate = this->accessSavedDrawState(fStates[i]); for (int s = 0; s < GrDrawState::kNumStages; ++s) { - GrSafeUnref(dstate.getTexture(s)); + GrSafeUnref(fStates[i].getTexture(s)); } - GrSafeUnref(dstate.getRenderTarget()); + GrSafeUnref(fStates[i].getRenderTarget()); } int numDraws = fDraws.count(); for (int d = 0; d < numDraws; ++d) { @@ -522,8 +521,7 @@ void GrInOrderDrawBuffer::playback(GrDrawTarget* target) { const Draw& draw = fDraws[i]; if (draw.fStateChanged) { ++currState; - GrDrawState* ds = &GrDrawTarget::accessSavedDrawState(fStates[currState]); - target->setDrawState(ds); + target->setDrawState(&fStates[currState]); } if (draw.fClipChanged) { ++currClip; @@ -773,8 +771,7 @@ bool GrInOrderDrawBuffer::needsNewState() const { if (fStates.empty()) { return true; } else { - const GrDrawState& old = this->accessSavedDrawState(fStates.back()); - return old != this->getDrawState(); + return fStates.back() != this->getDrawState(); } } @@ -784,7 +781,7 @@ void GrInOrderDrawBuffer::pushState() { GrSafeRef(drawState.getTexture(s)); } GrSafeRef(drawState.getRenderTarget()); - this->saveCurrentDrawState(&fStates.push_back()); + fStates.push_back(this->getDrawState()); } bool GrInOrderDrawBuffer::needsNewClip() const { diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h index e334520..3b27d70 100644 --- a/src/gpu/GrInOrderDrawBuffer.h +++ b/src/gpu/GrInOrderDrawBuffer.h @@ -182,7 +182,7 @@ private: }; GrSTAllocator fDraws; - GrSTAllocator fStates; + GrSTAllocator fStates; GrSTAllocator fClears; GrSTAllocator fClips; diff --git a/src/gpu/GrTesselatedPathRenderer.cpp b/src/gpu/GrTesselatedPathRenderer.cpp index 3823bbd..23074d9 100644 --- a/src/gpu/GrTesselatedPathRenderer.cpp +++ b/src/gpu/GrTesselatedPathRenderer.cpp @@ -354,7 +354,7 @@ bool GrTesselatedPathRenderer::onDrawPath(const SkPath& path, GrDrawState::StageMask stageMask, bool antiAlias) { - GrDrawTarget::AutoStateRestore asr(target); + GrDrawTarget::AutoStateRestore asr(target, GrDrawTarget::kPreserve_ASRInit); GrDrawState* drawState = target->drawState(); // face culling doesn't make sense here GrAssert(GrDrawState::kBoth_DrawFace == drawState->getDrawFace()); -- 2.7.4