From c589b0b5c0235c3adedc574c8846fb62414ed93c Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Mon, 17 Apr 2017 07:53:07 -0400 Subject: [PATCH] Remove lastOpList capability from GrSurface This is split out of: https://skia-review.googlesource.com/c/11581/ (Split up opLists) TBR=bsalomon@google.com Change-Id: I80d589b42918ddd77538484c808b069576691da4 Reviewed-on: https://skia-review.googlesource.com/11793 Commit-Queue: Robert Phillips Reviewed-by: Greg Daniel --- include/gpu/GrSurface.h | 34 +++++++++++----------------------- include/private/GrSurfaceProxy.h | 1 + src/gpu/GrOpList.cpp | 7 +++---- src/gpu/GrOpList.h | 5 +++-- src/gpu/GrPipeline.cpp | 22 +++++++++++++++------- src/gpu/GrPipeline.h | 2 +- src/gpu/GrRenderTarget.cpp | 4 ---- src/gpu/GrRenderTargetOpList.h | 2 +- src/gpu/GrSurface.cpp | 17 ----------------- src/gpu/GrTextureOpList.cpp | 5 +++++ src/gpu/GrTextureOpList.h | 2 ++ 11 files changed, 42 insertions(+), 59 deletions(-) diff --git a/include/gpu/GrSurface.h b/include/gpu/GrSurface.h index 81bc215..65f1e0f 100644 --- a/include/gpu/GrSurface.h +++ b/include/gpu/GrSurface.h @@ -14,7 +14,6 @@ #include "SkImageInfo.h" #include "SkRect.h" -class GrOpList; class GrRenderTarget; class GrSurfacePriv; class GrTexture; @@ -37,7 +36,8 @@ public: SkRect getBoundsRect() const { return SkRect::MakeIWH(this->width(), this->height()); } GrSurfaceOrigin origin() const { - SkASSERT(kTopLeft_GrSurfaceOrigin == fDesc.fOrigin || kBottomLeft_GrSurfaceOrigin == fDesc.fOrigin); + SkASSERT(kTopLeft_GrSurfaceOrigin == fDesc.fOrigin || + kBottomLeft_GrSurfaceOrigin == fDesc.fOrigin); return fDesc.fOrigin; } @@ -55,16 +55,16 @@ public: const GrSurfaceDesc& desc() const { return fDesc; } /** - * @return the texture associated with the surface, may be NULL. + * @return the texture associated with the surface, may be null. */ - virtual GrTexture* asTexture() { return NULL; } - virtual const GrTexture* asTexture() const { return NULL; } + virtual GrTexture* asTexture() { return nullptr; } + virtual const GrTexture* asTexture() const { return nullptr; } /** - * @return the render target underlying this surface, may be NULL. + * @return the render target underlying this surface, may be null. */ - virtual GrRenderTarget* asRenderTarget() { return NULL; } - virtual const GrRenderTarget* asRenderTarget() const { return NULL; } + virtual GrRenderTarget* asRenderTarget() { return nullptr; } + virtual const GrRenderTarget* asRenderTarget() const { return nullptr; } /** Access methods that are only to be used within Skia code. */ inline GrSurfacePriv surfacePriv(); @@ -78,9 +78,6 @@ public: fReleaseCtx = ctx; } - void setLastOpList(GrOpList* opList); - GrOpList* getLastOpList() { return fLastOpList; } - static size_t WorstCaseSize(const GrSurfaceDesc& desc, bool useNextPow2 = false); static size_t ComputeSize(const GrSurfaceDesc& desc, int colorSamplesPerPixel, bool hasMIPMaps, bool useNextPow2 = false); @@ -97,9 +94,8 @@ protected: GrSurface(GrGpu* gpu, const GrSurfaceDesc& desc) : INHERITED(gpu) , fDesc(desc) - , fReleaseProc(NULL) - , fReleaseCtx(NULL) - , fLastOpList(nullptr) { + , fReleaseProc(nullptr) + , fReleaseCtx(nullptr) { } ~GrSurface() override; @@ -112,21 +108,13 @@ private: void invokeReleaseProc() { if (fReleaseProc) { fReleaseProc(fReleaseCtx); - fReleaseProc = NULL; + fReleaseProc = nullptr; } } ReleaseProc fReleaseProc; ReleaseCtx fReleaseCtx; - // The last opList that wrote to or is currently going to write to this surface - // The opList can be closed (e.g., no render target or texture context is currently bound - // to this renderTarget or texture). - // This back-pointer is required so that we can add a dependancy between - // the opList used to create the current contents of this surface - // and the opList of a destination surface to which this one is being drawn or copied. - GrOpList* fLastOpList; - typedef GrGpuResource INHERITED; }; diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h index 8d5a0ca..e438e66 100644 --- a/include/private/GrSurfaceProxy.h +++ b/include/private/GrSurfaceProxy.h @@ -14,6 +14,7 @@ #include "SkRect.h" class GrCaps; +class GrOpList; class GrRenderTargetOpList; class GrRenderTargetProxy; class GrResourceProvider; diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp index 28b972d..91c8c79 100644 --- a/src/gpu/GrOpList.cpp +++ b/src/gpu/GrOpList.cpp @@ -6,11 +6,10 @@ */ #include "GrOpList.h" - -#include "GrRenderTargetOpList.h" -#include "GrSurface.h" #include "GrSurfaceProxy.h" +#include "SkAtomics.h" + uint32_t GrOpList::CreateUniqueID() { static int32_t gUniqueID = SK_InvalidUniqueID; uint32_t id; @@ -49,7 +48,7 @@ void GrOpList::addDependency(GrOpList* dependedOn) { } // Convert from a GrSurface-based dependency to a GrOpList one -void GrOpList::addDependency(GrSurface* dependedOn) { +void GrOpList::addDependency(GrSurfaceProxy* dependedOn) { if (dependedOn->getLastOpList()) { // If it is still receiving dependencies, this GrOpList shouldn't be closed SkASSERT(!this->isClosed()); diff --git a/src/gpu/GrOpList.h b/src/gpu/GrOpList.h index 0139b6b..b7dd7fa 100644 --- a/src/gpu/GrOpList.h +++ b/src/gpu/GrOpList.h @@ -16,7 +16,6 @@ class GrAuditTrail; class GrOpFlushState; class GrRenderTargetOpList; -class GrSurface; class GrSurfaceProxy; class GrTextureOpList; @@ -54,7 +53,7 @@ public: /* * Notify this GrOpList that it relies on the contents of 'dependedOn' */ - void addDependency(GrSurface* dependedOn); + void addDependency(GrSurfaceProxy* dependedOn); /* * Does this opList depend on 'dependedOn'? @@ -80,6 +79,8 @@ public: */ SkDEBUGCODE(virtual void dump() const;) + SkDEBUGCODE(virtual void validateTargetsSingleRenderTarget() const = 0;) + protected: GrSurfaceProxy* fTarget; GrAuditTrail* fAuditTrail; diff --git a/src/gpu/GrPipeline.cpp b/src/gpu/GrPipeline.cpp index 193c320..9e04b86 100644 --- a/src/gpu/GrPipeline.cpp +++ b/src/gpu/GrPipeline.cpp @@ -83,22 +83,30 @@ void GrPipeline::init(const InitArgs& args) { } } -static void add_dependencies_for_processor(const GrFragmentProcessor* proc, GrRenderTarget* rt) { +// MDB TODO: re-enable when TextureSamplers store texture proxies +#if 0 +static void add_dependencies_for_processor(const GrFragmentProcessor* proc, + GrRenderTargetProxy* rtp) { GrFragmentProcessor::TextureAccessIter iter(proc); while (const GrResourceIOProcessor::TextureSampler* sampler = iter.next()) { - SkASSERT(rt->getLastOpList()); - rt->getLastOpList()->addDependency(sampler->texture()); + SkASSERT(rtp->getLastOpList()); + rtp->getLastOpList()->addDependency(sampler->texture()); } } +#endif -void GrPipeline::addDependenciesTo(GrRenderTarget* rt) const { +void GrPipeline::addDependenciesTo(GrRenderTargetProxy* rtp) const { + // MDB TODO: re-enable when TextureSamplers store texture proxies +#if 0 for (int i = 0; i < fFragmentProcessors.count(); ++i) { - add_dependencies_for_processor(fFragmentProcessors[i].get(), rt); + add_dependencies_for_processor(fFragmentProcessors[i].get(), rtp); } +#endif if (fDstTexture) { - SkASSERT(rt->getLastOpList()); - rt->getLastOpList()->addDependency(fDstTexture.get()); + SkASSERT(rtp->getLastOpList()); + // MDB TODO: re-enable when TextureSamplers store texture proxies + //rtp->getLastOpList()->addDependency(fDstTexture.get()); } } diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h index b08b4ba..76e9f2e 100644 --- a/src/gpu/GrPipeline.h +++ b/src/gpu/GrPipeline.h @@ -122,7 +122,7 @@ public: // Make the renderTarget's GrOpList (if it exists) be dependent on any // GrOpLists in this pipeline - void addDependenciesTo(GrRenderTarget* rt) const; + void addDependenciesTo(GrRenderTargetProxy*) const; int numColorFragmentProcessors() const { return fNumColorProcessors; } int numCoverageFragmentProcessors() const { diff --git a/src/gpu/GrRenderTarget.cpp b/src/gpu/GrRenderTarget.cpp index 9ea8596..61a6f92 100644 --- a/src/gpu/GrRenderTarget.cpp +++ b/src/gpu/GrRenderTarget.cpp @@ -61,10 +61,6 @@ void GrRenderTarget::onRelease() { void GrRenderTarget::onAbandon() { SkSafeSetNull(fStencilAttachment); - // The contents of this renderTarget are gone/invalid. It isn't useful to point back - // the creating opList. - this->setLastOpList(nullptr); - INHERITED::onAbandon(); } diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h index 1b6f086..5762e0a 100644 --- a/src/gpu/GrRenderTargetOpList.h +++ b/src/gpu/GrRenderTargetOpList.h @@ -110,7 +110,7 @@ public: SkDEBUGCODE(void dump() const override;) - SkDEBUGCODE(void validateTargetsSingleRenderTarget() const;) + SkDEBUGCODE(void validateTargetsSingleRenderTarget() const override;) private: friend class GrRenderTargetContextPriv; // for stencil clip state. TODO: this is invasive diff --git a/src/gpu/GrSurface.cpp b/src/gpu/GrSurface.cpp index 4c9e3b4..1762d00 100644 --- a/src/gpu/GrSurface.cpp +++ b/src/gpu/GrSurface.cpp @@ -15,11 +15,6 @@ #include "SkMathPriv.h" GrSurface::~GrSurface() { - if (fLastOpList) { - fLastOpList->clearTarget(); - } - SkSafeUnref(fLastOpList); - // check that invokeReleaseProc has been called (if needed) SkASSERT(NULL == fReleaseProc); } @@ -184,15 +179,3 @@ void GrSurface::onAbandon() { this->invokeReleaseProc(); this->INHERITED::onAbandon(); } - -void GrSurface::setLastOpList(GrOpList* opList) { - if (fLastOpList) { - // The non-MDB world never closes so we can't check this condition -#ifdef ENABLE_MDB - SkASSERT(fLastOpList->isClosed()); -#endif - fLastOpList->clearTarget(); - } - - SkRefCnt_SafeAssign(fLastOpList, opList); -} diff --git a/src/gpu/GrTextureOpList.cpp b/src/gpu/GrTextureOpList.cpp index d4becad..175f178 100644 --- a/src/gpu/GrTextureOpList.cpp +++ b/src/gpu/GrTextureOpList.cpp @@ -42,6 +42,11 @@ void GrTextureOpList::dump() const { clippedBounds.fBottom); } } + +void GrTextureOpList::validateTargetsSingleRenderTarget() const { + SkASSERT(1 == fRecordedOps.count() || 0 == fRecordedOps.count()); +} + #endif void GrTextureOpList::prepareOps(GrOpFlushState* flushState) { diff --git a/src/gpu/GrTextureOpList.h b/src/gpu/GrTextureOpList.h index ca5bda4..d42818d 100644 --- a/src/gpu/GrTextureOpList.h +++ b/src/gpu/GrTextureOpList.h @@ -62,6 +62,8 @@ public: SkDEBUGCODE(void dump() const override;) + SkDEBUGCODE(virtual void validateTargetsSingleRenderTarget() const override;) + private: // MDB TODO: The unique IDs are only needed for the audit trail. There should only be one // on the opList itself. -- 2.7.4