From 9176e2c159089458b1e2226a94fab1af0fba32ac Mon Sep 17 00:00:00 2001 From: joshualitt Date: Thu, 20 Nov 2014 07:28:52 -0800 Subject: [PATCH] dstCopy on optdrawstate BUG=skia: Review URL: https://codereview.chromium.org/735363003 --- src/gpu/GrDrawTarget.cpp | 20 +++++++++++--------- src/gpu/GrDrawTarget.h | 17 ++--------------- src/gpu/GrGpu.cpp | 16 ++++++---------- src/gpu/GrGpu.h | 11 +++-------- src/gpu/GrInOrderDrawBuffer.cpp | 19 +++++-------------- src/gpu/GrInOrderDrawBuffer.h | 6 ++---- src/gpu/GrOptDrawState.cpp | 11 +++++++++-- src/gpu/GrOptDrawState.h | 2 ++ src/gpu/GrTest.cpp | 5 +---- src/gpu/gl/GrGLProgram.cpp | 5 ++--- src/gpu/gl/GrGLProgram.h | 4 +--- src/gpu/gl/GrGLProgramDesc.cpp | 2 +- src/gpu/gl/GrGLProgramDesc.h | 1 - src/gpu/gl/GrGpuGL.h | 5 +---- src/gpu/gl/GrGpuGL_program.cpp | 9 +++------ 15 files changed, 49 insertions(+), 84 deletions(-) diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp index 4043eb8..2f395e6 100644 --- a/src/gpu/GrDrawTarget.cpp +++ b/src/gpu/GrDrawTarget.cpp @@ -41,8 +41,6 @@ GrDrawTarget::DrawInfo& GrDrawTarget::DrawInfo::operator =(const DrawInfo& di) { fDevBounds = NULL; } - fDstCopy = di.fDstCopy; - this->setVertexBuffer(di.vertexBuffer()); this->setIndexBuffer(di.indexBuffer()); @@ -468,14 +466,15 @@ void GrDrawTarget::drawIndexed(GrDrawState* ds, if (devBounds) { info.setDevBounds(*devBounds); } + // TODO: We should continue with incorrect blending. - if (!this->setupDstReadIfNecessary(ds, &info)) { + GrDeviceCoordTexture dstCopy; + if (!this->setupDstReadIfNecessary(ds, &dstCopy, devBounds)) { return; } - this->setDrawBuffers(&info); - this->onDraw(*ds, info, scissorState); + this->onDraw(*ds, info, scissorState, dstCopy.texture() ? &dstCopy : NULL); } } @@ -511,13 +510,14 @@ void GrDrawTarget::drawNonIndexed(GrDrawState* ds, } // TODO: We should continue with incorrect blending. - if (!this->setupDstReadIfNecessary(ds, &info)) { + GrDeviceCoordTexture dstCopy; + if (!this->setupDstReadIfNecessary(ds, &dstCopy, devBounds)) { return; } this->setDrawBuffers(&info); - this->onDraw(*ds, info, scissorState); + this->onDraw(*ds, info, scissorState, dstCopy.texture() ? &dstCopy : NULL); } } @@ -759,8 +759,10 @@ void GrDrawTarget::drawIndexedInstances(GrDrawState* ds, if (devBounds) { info.setDevBounds(*devBounds); } + // TODO: We should continue with incorrect blending. - if (!this->setupDstReadIfNecessary(ds, &info)) { + GrDeviceCoordTexture dstCopy; + if (!this->setupDstReadIfNecessary(ds, &dstCopy, devBounds)) { return; } @@ -777,7 +779,7 @@ void GrDrawTarget::drawIndexedInstances(GrDrawState* ds, info.fStartIndex, info.fVertexCount, info.fIndexCount)) { - this->onDraw(*ds, info, scissorState); + this->onDraw(*ds, info, scissorState, dstCopy.texture() ? &dstCopy : NULL); } info.fStartVertex += info.fVertexCount; instanceCount -= info.fInstanceCount; diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h index c5fff70..b86584d 100644 --- a/src/gpu/GrDrawTarget.h +++ b/src/gpu/GrDrawTarget.h @@ -569,15 +569,6 @@ public: } const SkRect* getDevBounds() const { return fDevBounds; } - // NULL if no copy of the dst is needed for the draw. - const GrDeviceCoordTexture* getDstCopy() const { - if (fDstCopy.texture()) { - return &fDstCopy; - } else { - return NULL; - } - } - private: DrawInfo() { fDevBounds = NULL; } @@ -599,8 +590,6 @@ public: GrPendingIOResource fVertexBuffer; GrPendingIOResource fIndexBuffer; - - GrDeviceCoordTexture fDstCopy; }; virtual void setDrawBuffers(DrawInfo*) = 0;; @@ -673,9 +662,6 @@ protected: // Makes a copy of the dst if it is necessary for the draw. Returns false if a copy is required // but couldn't be made. Otherwise, returns true. This method needs to be protected because it // needs to be accessed by GLPrograms to setup a correct drawstate - bool setupDstReadIfNecessary(GrDrawState* ds, DrawInfo* info) { - return this->setupDstReadIfNecessary(ds, &info->fDstCopy, info->getDevBounds()); - } bool setupDstReadIfNecessary(GrDrawState*, GrDeviceCoordTexture* dstCopy, const SkRect* drawBounds); @@ -699,7 +685,8 @@ private: // subclass called to perform drawing virtual void onDraw(const GrDrawState&, const DrawInfo&, - const GrClipMaskManager::ScissorState&) = 0; + const GrClipMaskManager::ScissorState&, + const GrDeviceCoordTexture* dstCopy) = 0; // TODO copy in order drawbuffer onDrawRect to here virtual void onDrawRect(GrDrawState*, const SkRect& rect, diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp index cdbb01d..2bda594 100644 --- a/src/gpu/GrGpu.cpp +++ b/src/gpu/GrGpu.cpp @@ -285,9 +285,7 @@ const GrIndexBuffer* GrGpu::getQuadIndexBuffer() const { void GrGpu::draw(const GrOptDrawState& ds, const GrDrawTarget::DrawInfo& info) { this->handleDirtyContext(); - if (!this->flushGraphicsState(ds, - PrimTypeToDrawType(info.primitiveType()), - info.getDstCopy())) { + if (!this->flushGraphicsState(ds, PrimTypeToDrawType(info.primitiveType()))) { return; } this->onDraw(ds, info); @@ -298,7 +296,7 @@ void GrGpu::stencilPath(const GrOptDrawState& ds, const GrStencilSettings& stencilSettings) { this->handleDirtyContext(); - if (!this->flushGraphicsState(ds, kStencilPath_DrawType, NULL)) { + if (!this->flushGraphicsState(ds, kStencilPath_DrawType)) { return; } @@ -308,11 +306,10 @@ void GrGpu::stencilPath(const GrOptDrawState& ds, void GrGpu::drawPath(const GrOptDrawState& ds, const GrPath* path, - const GrStencilSettings& stencilSettings, - const GrDeviceCoordTexture* dstCopy) { + const GrStencilSettings& stencilSettings) { this->handleDirtyContext(); - if (!this->flushGraphicsState(ds, kDrawPath_DrawType, dstCopy)) { + if (!this->flushGraphicsState(ds, kDrawPath_DrawType)) { return; } @@ -325,11 +322,10 @@ void GrGpu::drawPaths(const GrOptDrawState& ds, int count, const float transforms[], GrDrawTarget::PathTransformType transformsType, - const GrStencilSettings& stencilSettings, - const GrDeviceCoordTexture* dstCopy) { + const GrStencilSettings& stencilSettings) { this->handleDirtyContext(); - if (!this->flushGraphicsState(ds, kDrawPaths_DrawType, dstCopy)) { + if (!this->flushGraphicsState(ds, kDrawPaths_DrawType)) { return; } diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h index 987e067..956f083 100644 --- a/src/gpu/GrGpu.h +++ b/src/gpu/GrGpu.h @@ -316,7 +316,6 @@ public: virtual void buildProgramDesc(const GrOptDrawState&, const GrProgramDesc::DescInfo&, GrGpu::DrawType, - const GrDeviceCoordTexture* dstCopy, GrProgramDesc*) = 0; /** @@ -363,16 +362,14 @@ public: const GrStencilSettings&); virtual void drawPath(const GrOptDrawState&, const GrPath*, - const GrStencilSettings&, - const GrDeviceCoordTexture* dstCopy); + const GrStencilSettings&); virtual void drawPaths(const GrOptDrawState&, const GrPathRange*, const uint32_t indices[], int count, const float transforms[], GrDrawTarget::PathTransformType, - const GrStencilSettings&, - const GrDeviceCoordTexture*); + const GrStencilSettings&); static DrawType PrimTypeToDrawType(GrPrimitiveType type) { switch (type) { @@ -470,9 +467,7 @@ private: // deltas from previous state at draw time. This function does the // backend-specific flush of the state. // returns false if current state is unsupported. - virtual bool flushGraphicsState(const GrOptDrawState&, - DrawType, - const GrDeviceCoordTexture* dstCopy) = 0; + virtual bool flushGraphicsState(const GrOptDrawState&, DrawType) = 0; // clears target's entire stencil buffer to 0 virtual void clearStencil(GrRenderTarget* target) = 0; diff --git a/src/gpu/GrInOrderDrawBuffer.cpp b/src/gpu/GrInOrderDrawBuffer.cpp index e5d6795..cb44c2b 100644 --- a/src/gpu/GrInOrderDrawBuffer.cpp +++ b/src/gpu/GrInOrderDrawBuffer.cpp @@ -255,13 +255,14 @@ int GrInOrderDrawBuffer::concatInstancedDraw(const GrDrawState& ds, const DrawIn void GrInOrderDrawBuffer::onDraw(const GrDrawState& ds, const DrawInfo& info, - const ScissorState& scissorState) { + const ScissorState& scissorState, + const GrDeviceCoordTexture* dstCopy) { SkASSERT(info.vertexBuffer() && (!info.isIndexed() || info.indexBuffer())); GeometryPoolState& poolState = fGeoPoolStateStack.back(); if (!this->recordStateAndShouldDraw(ds, GrGpu::PrimTypeToDrawType(info.primitiveType()), - scissorState, info.getDstCopy())) { + scissorState, dstCopy)) { return; } @@ -317,9 +318,6 @@ void GrInOrderDrawBuffer::onDrawPath(const GrDrawState& ds, return; } DrawPath* dp = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, DrawPath, (path)); - if (dstCopy) { - dp->fDstCopy = *dstCopy; - } dp->fStencilSettings = stencilSettings; this->recordTraceMarkersIfNecessary(); } @@ -371,9 +369,6 @@ void GrInOrderDrawBuffer::onDrawPaths(const GrDrawState& ds, dp->fTransformsLocation = savedTransforms - fPathTransformBuffer.begin(); dp->fTransformsType = transformsType; dp->fStencilSettings = stencilSettings; - if (dstCopy) { - dp->fDstCopy = *dstCopy; - } this->recordTraceMarkersIfNecessary(); } @@ -517,8 +512,7 @@ void GrInOrderDrawBuffer::StencilPath::execute(GrInOrderDrawBuffer* buf, void GrInOrderDrawBuffer::DrawPath::execute(GrInOrderDrawBuffer* buf, const GrOptDrawState* optState) { - buf->fDstGpu->drawPath(*optState, this->path(), fStencilSettings, - fDstCopy.texture() ? &fDstCopy : NULL); + buf->fDstGpu->drawPath(*optState, this->path(), fStencilSettings); } void GrInOrderDrawBuffer::DrawPaths::execute(GrInOrderDrawBuffer* buf, @@ -526,7 +520,7 @@ void GrInOrderDrawBuffer::DrawPaths::execute(GrInOrderDrawBuffer* buf, buf->fDstGpu->drawPaths(*optState, this->pathRange(), &buf->fPathIndexBuffer[fIndicesLocation], fCount, &buf->fPathTransformBuffer[fTransformsLocation], fTransformsType, - fStencilSettings, fDstCopy.texture() ? &fDstCopy : NULL); + fStencilSettings); } void GrInOrderDrawBuffer::SetState::execute(GrInOrderDrawBuffer*, const GrOptDrawState*) { @@ -741,9 +735,6 @@ bool GrInOrderDrawBuffer::recordStateAndShouldDraw(const GrDrawState& ds, if (!fLastState || *optState != *fLastState) { SetState* ss = GrNEW_APPEND_TO_RECORDER(fCmdBuffer, SetState, (optState)); fLastState.reset(SkRef(optState.get())); - if (dstCopy) { - ss->fDstCopy = *dstCopy; - } ss->fDrawType = drawType; this->recordTraceMarkersIfNecessary(); } diff --git a/src/gpu/GrInOrderDrawBuffer.h b/src/gpu/GrInOrderDrawBuffer.h index ed5cdd4..ab7dd77 100644 --- a/src/gpu/GrInOrderDrawBuffer.h +++ b/src/gpu/GrInOrderDrawBuffer.h @@ -143,7 +143,6 @@ private: virtual void execute(GrInOrderDrawBuffer*, const GrOptDrawState*); - GrDeviceCoordTexture fDstCopy; GrStencilSettings fStencilSettings; private: @@ -161,7 +160,6 @@ private: size_t fCount; int fTransformsLocation; PathTransformType fTransformsType; - GrDeviceCoordTexture fDstCopy; GrStencilSettings fStencilSettings; private: @@ -222,7 +220,6 @@ private: SkAutoTUnref fState; GrGpu::DrawType fDrawType; - GrDeviceCoordTexture fDstCopy; }; typedef void* TCmdAlign; // This wouldn't be enough align if a command used long double. @@ -231,7 +228,8 @@ private: // overrides from GrDrawTarget void onDraw(const GrDrawState&, const DrawInfo&, - const ScissorState&) SK_OVERRIDE; + const ScissorState&, + const GrDeviceCoordTexture* dstCopy) SK_OVERRIDE; void onDrawRect(GrDrawState*, const SkRect& rect, const SkRect* localRect, diff --git a/src/gpu/GrOptDrawState.cpp b/src/gpu/GrOptDrawState.cpp index 93cc047..4f2d24f 100644 --- a/src/gpu/GrOptDrawState.cpp +++ b/src/gpu/GrOptDrawState.cpp @@ -32,6 +32,12 @@ GrOptDrawState::GrOptDrawState(const GrDrawState& drawState, fDrawFace = drawState.getDrawFace(); fSrcBlend = optSrcCoeff; fDstBlend = optDstCoeff; + + // TODO move this out of optDrawState + if (dstCopy) { + fDstCopy = *dstCopy; + } + GrProgramDesc::DescInfo descInfo; fFlags = 0; @@ -97,7 +103,7 @@ GrOptDrawState::GrOptDrawState(const GrDrawState& drawState, this->setOutputStateInfo(drawState, blendOpt, *gpu->caps(), &descInfo); // now create a key - gpu->buildProgramDesc(*this, descInfo, drawType, dstCopy, &fDesc); + gpu->buildProgramDesc(*this, descInfo, drawType, &fDesc); }; GrOptDrawState* GrOptDrawState::Create(const GrDrawState& drawState, @@ -278,7 +284,8 @@ bool GrOptDrawState::operator== (const GrOptDrawState& that) const { this->fVAStride != that.fVAStride || memcmp(this->fVAPtr, that.fVAPtr, this->fVACount * sizeof(GrVertexAttrib)) || this->fStencilSettings != that.fStencilSettings || - this->fDrawFace != that.fDrawFace) { + this->fDrawFace != that.fDrawFace || + this->fDstCopy.texture() != that.fDstCopy.texture()) { return false; } diff --git a/src/gpu/GrOptDrawState.h b/src/gpu/GrOptDrawState.h index 0bb3d07..b727cb5 100644 --- a/src/gpu/GrOptDrawState.h +++ b/src/gpu/GrOptDrawState.h @@ -196,6 +196,7 @@ public: /////////////////////////////////////////////////////////////////////////// + const GrDeviceCoordTexture* getDstCopy() const { return fDstCopy.texture() ? &fDstCopy : NULL; } const GrProgramDesc& programDesc() const { return fDesc; } @@ -275,6 +276,7 @@ private: GrStencilSettings fStencilSettings; uint8_t fCoverage; GrDrawState::DrawFace fDrawFace; + GrDeviceCoordTexture fDstCopy; GrBlendCoeff fSrcBlend; GrBlendCoeff fDstBlend; uint32_t fFlags; diff --git a/src/gpu/GrTest.cpp b/src/gpu/GrTest.cpp index 063a2bb..f861e42 100644 --- a/src/gpu/GrTest.cpp +++ b/src/gpu/GrTest.cpp @@ -67,7 +67,6 @@ public: virtual void buildProgramDesc(const GrOptDrawState&, const GrProgramDesc::DescInfo&, GrGpu::DrawType, - const GrDeviceCoordTexture* dstCopy, GrProgramDesc* desc) SK_OVERRIDE { } virtual void discard(GrRenderTarget*) SK_OVERRIDE { } @@ -148,9 +147,7 @@ private: return false; } - virtual bool flushGraphicsState(const GrOptDrawState&, - DrawType, - const GrDeviceCoordTexture* dstCopy) SK_OVERRIDE { + virtual bool flushGraphicsState(const GrOptDrawState&, DrawType) SK_OVERRIDE { return false; } diff --git a/src/gpu/gl/GrGLProgram.cpp b/src/gpu/gl/GrGLProgram.cpp index 0e67e81..3249793 100644 --- a/src/gpu/gl/GrGLProgram.cpp +++ b/src/gpu/gl/GrGLProgram.cpp @@ -126,9 +126,7 @@ void GrGLProgram::bindTextures(const GrGLInstalledProc* ip, const GrProcessor& p /////////////////////////////////////////////////////////////////////////////// -void GrGLProgram::setData(const GrOptDrawState& optState, - GrGpu::DrawType drawType, - const GrDeviceCoordTexture* dstCopy) { +void GrGLProgram::setData(const GrOptDrawState& optState, GrGpu::DrawType drawType) { GrColor color = optState.getColor(); uint8_t coverage = optState.getCoverage(); @@ -136,6 +134,7 @@ void GrGLProgram::setData(const GrOptDrawState& optState, this->setCoverage(optState, coverage); this->setMatrixAndRenderTargetHeight(drawType, optState); + const GrDeviceCoordTexture* dstCopy = optState.getDstCopy(); if (dstCopy) { if (fBuiltinUniformHandles.fDstCopyTopLeftUni.isValid()) { fProgramDataManager.set2f(fBuiltinUniformHandles.fDstCopyTopLeftUni, diff --git a/src/gpu/gl/GrGLProgram.h b/src/gpu/gl/GrGLProgram.h index c623977..a273a03 100644 --- a/src/gpu/gl/GrGLProgram.h +++ b/src/gpu/gl/GrGLProgram.h @@ -129,9 +129,7 @@ public: * GrGpuGL object to bind the textures required by the GrGLProcessors. The color and coverage * stages come from GrGLProgramDesc::Build(). */ - void setData(const GrOptDrawState&, - GrGpu::DrawType, - const GrDeviceCoordTexture* dstCopy /* can be NULL*/); + void setData(const GrOptDrawState&, GrGpu::DrawType); protected: typedef GrGLProgramDataManager::UniformHandle UniformHandle; diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp index f8510ff..954a482 100644 --- a/src/gpu/gl/GrGLProgramDesc.cpp +++ b/src/gpu/gl/GrGLProgramDesc.cpp @@ -204,7 +204,6 @@ bool GrGLProgramDescBuilder::Build(const GrOptDrawState& optState, const GrProgramDesc::DescInfo& descInfo, GrGpu::DrawType drawType, GrGpuGL* gpu, - const GrDeviceCoordTexture* dstCopy, GrProgramDesc* desc) { bool inputColorIsUsed = descInfo.fInputColorIsUsed; bool inputCoverageIsUsed = descInfo.fInputCoverageIsUsed; @@ -294,6 +293,7 @@ bool GrGLProgramDescBuilder::Build(const GrOptDrawState& optState, } if (descInfo.fReadsDst) { + const GrDeviceCoordTexture* dstCopy = optState.getDstCopy(); SkASSERT(dstCopy || gpu->caps()->dstReadInShaderSupport()); const GrTexture* dstCopyTexture = NULL; if (dstCopy) { diff --git a/src/gpu/gl/GrGLProgramDesc.h b/src/gpu/gl/GrGLProgramDesc.h index 8bfd506..8b032e0 100644 --- a/src/gpu/gl/GrGLProgramDesc.h +++ b/src/gpu/gl/GrGLProgramDesc.h @@ -62,7 +62,6 @@ public: const GrProgramDesc::DescInfo&, GrGpu::DrawType, GrGpuGL*, - const GrDeviceCoordTexture*, GrProgramDesc*); static const GLKeyHeader& GetHeader(const GrProgramDesc& desc) { diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h index 8212f9e..a176c95 100644 --- a/src/gpu/gl/GrGpuGL.h +++ b/src/gpu/gl/GrGpuGL.h @@ -109,7 +109,6 @@ protected: virtual void buildProgramDesc(const GrOptDrawState&, const GrProgramDesc::DescInfo&, GrGpu::DrawType, - const GrDeviceCoordTexture* dstCopy, GrProgramDesc*) SK_OVERRIDE; private: @@ -157,9 +156,7 @@ private: virtual void clearStencil(GrRenderTarget*) SK_OVERRIDE; - virtual bool flushGraphicsState(const GrOptDrawState&, - DrawType, - const GrDeviceCoordTexture* dstCopy) SK_OVERRIDE; + virtual bool flushGraphicsState(const GrOptDrawState&, DrawType) SK_OVERRIDE; // GrDrawTarget overrides virtual void didAddGpuTraceMarker() SK_OVERRIDE; diff --git a/src/gpu/gl/GrGpuGL_program.cpp b/src/gpu/gl/GrGpuGL_program.cpp index 864b453..176b7cd 100644 --- a/src/gpu/gl/GrGpuGL_program.cpp +++ b/src/gpu/gl/GrGpuGL_program.cpp @@ -201,9 +201,7 @@ GrGLProgram* GrGpuGL::ProgramCache::getProgram(const GrOptDrawState& optState, D #define GL_CALL(X) GR_GL_CALL(this->glInterface(), X) -bool GrGpuGL::flushGraphicsState(const GrOptDrawState& optState, - DrawType type, - const GrDeviceCoordTexture* dstCopy) { +bool GrGpuGL::flushGraphicsState(const GrOptDrawState& optState, DrawType type) { // GrGpu::setupClipAndFlushState should have already checked this and bailed if not true. SkASSERT(optState.getRenderTarget()); @@ -240,7 +238,7 @@ bool GrGpuGL::flushGraphicsState(const GrOptDrawState& optState, this->flushBlend(optState, kDrawLines_DrawType == type, srcCoeff, dstCoeff); - fCurrentProgram->setData(optState, type, dstCopy); + fCurrentProgram->setData(optState, type); } GrGLRenderTarget* glRT = static_cast(optState.getRenderTarget()); @@ -309,9 +307,8 @@ void GrGpuGL::setupGeometry(const GrOptDrawState& optState, void GrGpuGL::buildProgramDesc(const GrOptDrawState& optState, const GrProgramDesc::DescInfo& descInfo, GrGpu::DrawType drawType, - const GrDeviceCoordTexture* dstCopy, GrProgramDesc* desc) { - if (!GrGLProgramDescBuilder::Build(optState, descInfo, drawType, this, dstCopy, desc)) { + if (!GrGLProgramDescBuilder::Build(optState, descInfo, drawType, this, desc)) { SkDEBUGFAIL("Failed to generate GL program descriptor"); } } -- 2.7.4