Revert of Add GrResourceCache2. (patchset #4 of https://codereview.chromium.org/481443002/)
Reason for revert:
Likely caused a leak detected in Chromium after last Skia roll.
Original issue's description:
> Add GrResourceCache2.
>
> Currently it just replaces GrGpu as the owner of the linked list of resources.
>
> Committed: https://skia.googlesource.com/skia/+/94ce9ac8624dbb45656b8f5c992fad9c9ff3ee5f
R=mtklein@google.com, robertphillips@google.com
TBR=mtklein@google.com, robertphillips@google.com
NOTREECHECKS=true
NOTRY=true
Author: bsalomon@google.com
Review URL: https://codereview.chromium.org/477323006
diff --git a/gyp/gpu.gypi b/gyp/gpu.gypi
index 3613928..589cc51 100644
--- a/gyp/gpu.gypi
+++ b/gyp/gpu.gypi
@@ -113,8 +113,6 @@
'<(skia_src_path)/gpu/GrReducedClip.h',
'<(skia_src_path)/gpu/GrResourceCache.cpp',
'<(skia_src_path)/gpu/GrResourceCache.h',
- '<(skia_src_path)/gpu/GrResourceCache2.cpp',
- '<(skia_src_path)/gpu/GrResourceCache2.h',
'<(skia_src_path)/gpu/GrStencil.cpp',
'<(skia_src_path)/gpu/GrStencil.h',
'<(skia_src_path)/gpu/GrStencilAndCoverPathRenderer.cpp',
diff --git a/include/gpu/GrContext.h b/include/gpu/GrContext.h
index 4f9b642..d624510 100644
--- a/include/gpu/GrContext.h
+++ b/include/gpu/GrContext.h
@@ -35,7 +35,6 @@
class GrPathRenderer;
class GrResourceEntry;
class GrResourceCache;
-class GrResourceCache2;
class GrStencilBuffer;
class GrTestTarget;
class GrTextContext;
@@ -927,7 +926,6 @@
GrDrawTarget* getTextTarget();
const GrIndexBuffer* getQuadIndexBuffer() const;
GrAARectRenderer* getAARectRenderer() { return fAARectRenderer; }
- GrResourceCache2* getResourceCache2() { return fResourceCache2; }
// Called by tests that draw directly to the context via GrDrawTarget
void getTestTarget(GrTestTarget*);
@@ -975,7 +973,6 @@
GrDrawState* fDrawState;
GrResourceCache* fResourceCache;
- GrResourceCache2* fResourceCache2;
GrFontCache* fFontCache;
SkAutoTDelete<GrLayerCache> fLayerCache;
diff --git a/include/gpu/GrGpuResource.h b/include/gpu/GrGpuResource.h
index 8b16282..127f64b 100644
--- a/include/gpu/GrGpuResource.h
+++ b/include/gpu/GrGpuResource.h
@@ -12,7 +12,6 @@
#include "SkTInternalLList.h"
class GrResourceCacheEntry;
-class GrResourceCache2;
class GrGpu;
class GrContext;
@@ -119,13 +118,12 @@
static uint32_t CreateUniqueID();
- // We're in an internal doubly linked list owned by GrResourceCache2
+ // We're in an internal doubly linked list
SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrGpuResource);
- // This is not ref'ed but abandon() or release() will be called before the GrGpu object
- // is destroyed. Those calls set will this to NULL.
- GrGpu* fGpu;
-
+ GrGpu* fGpu; // not reffed. The GrGpu can be deleted while there
+ // are still live GrGpuResources. It will call
+ // release() on all such objects in its destructor.
enum Flags {
/**
* This object wraps a GPU object given to us by the user.
diff --git a/src/gpu/GrClipMaskCache.h b/src/gpu/GrClipMaskCache.h
index 6b484e8..b332c7b 100644
--- a/src/gpu/GrClipMaskCache.h
+++ b/src/gpu/GrClipMaskCache.h
@@ -41,7 +41,6 @@
// We could reuse the mask if bounds is a subset of last bounds. We'd have to communicate
// an offset to the caller.
if (back->fLastMask.texture() &&
- !back->fLastMask.texture()->wasDestroyed() &&
back->fLastBound == bounds &&
back->fLastClipGenID == clipGenID) {
return true;
@@ -180,8 +179,8 @@
return fContext;
}
- // TODO: Remove this when we hold cache keys instead of refs to textures.
- void purgeResources() {
+ void releaseResources() {
+
SkDeque::F2BIter iter(fStack);
for (GrClipStackFrame* frame = (GrClipStackFrame*) iter.next();
frame != NULL;
@@ -220,8 +219,7 @@
int32_t fLastClipGenID;
// The mask's width & height values are used by GrClipMaskManager to correctly scale the
- // texture coords for the geometry drawn with this mask. TODO: This should be a cache key
- // and not a hard ref to a texture.
+ // texture coords for the geometry drawn with this mask.
GrAutoScratchTexture fLastMask;
// fLastBound stores the bounding box of the clip mask in clip-stack space. This rect is
// used by GrClipMaskManager to position a rect and compute texture coords for the mask.
diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp
index d4ab0b7..7cafdf7 100644
--- a/src/gpu/GrClipMaskManager.cpp
+++ b/src/gpu/GrClipMaskManager.cpp
@@ -1112,8 +1112,8 @@
}
////////////////////////////////////////////////////////////////////////////////
-void GrClipMaskManager::purgeResources() {
- fAACache.purgeResources();
+void GrClipMaskManager::releaseResources() {
+ fAACache.releaseResources();
}
void GrClipMaskManager::setGpu(GrGpu* gpu) {
diff --git a/src/gpu/GrClipMaskManager.h b/src/gpu/GrClipMaskManager.h
index ea16fc8..c3a21fd 100644
--- a/src/gpu/GrClipMaskManager.h
+++ b/src/gpu/GrClipMaskManager.h
@@ -53,11 +53,7 @@
bool setupClipping(const GrClipData* clipDataIn, GrDrawState::AutoRestoreEffects*,
const SkRect* devBounds);
- /**
- * Purge resources to free up memory. TODO: This class shouldn't hold any long lived refs
- * which will allow ResourceCache2 to automatically purge anything this class has created.
- */
- void purgeResources();
+ void releaseResources();
bool isClipInStencil() const {
return kStencil_ClipMaskType == fCurrClipMaskType;
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index df26be6..091c4a8 100755
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -25,7 +25,6 @@
#include "GrPathRenderer.h"
#include "GrPathUtils.h"
#include "GrResourceCache.h"
-#include "GrResourceCache2.h"
#include "GrSoftwarePathRenderer.h"
#include "GrStencilBuffer.h"
#include "GrStencilAndCoverTextContext.h"
@@ -109,7 +108,6 @@
fPathRendererChain = NULL;
fSoftwarePathRenderer = NULL;
fResourceCache = NULL;
- fResourceCache2 = NULL;
fFontCache = NULL;
fDrawBuffer = NULL;
fDrawBufferVBAllocPool = NULL;
@@ -135,7 +133,6 @@
fResourceCache = SkNEW_ARGS(GrResourceCache, (MAX_RESOURCE_CACHE_COUNT,
MAX_RESOURCE_CACHE_BYTES));
fResourceCache->setOverbudgetCallback(OverbudgetCB, this);
- fResourceCache2 = SkNEW(GrResourceCache2);
fFontCache = SkNEW_ARGS(GrFontCache, (fGpu));
@@ -164,8 +161,10 @@
(*fCleanUpData[i].fFunc)(this, fCleanUpData[i].fInfo);
}
- delete fResourceCache2;
- fResourceCache2 = NULL;
+ // Since the gpu can hold scratch textures, give it a chance to let go
+ // of them before freeing the texture cache
+ fGpu->purgeResources();
+
delete fResourceCache;
fResourceCache = NULL;
delete fFontCache;
@@ -185,9 +184,7 @@
void GrContext::abandonContext() {
// abandon first to so destructors
// don't try to free the resources in the API.
- fResourceCache2->abandonAll();
-
- fGpu->contextAbandonded();
+ fGpu->abandonResources();
// a path renderer may be holding onto resources that
// are now unusable
@@ -210,6 +207,7 @@
fFontCache->freeAll();
fLayerCache->freeAll();
+ fGpu->markContextDirty();
}
void GrContext::resetContext(uint32_t state) {
@@ -220,9 +218,6 @@
this->flush();
fGpu->purgeResources();
- if (NULL != fDrawBuffer) {
- fDrawBuffer->purgeResources();
- }
fAARectRenderer->reset();
fOvalRenderer->reset();
@@ -545,10 +540,6 @@
}
void GrContext::unlockScratchTexture(GrTexture* texture) {
- if (texture->wasDestroyed()) {
- return;
- }
-
ASSERT_OWNED_RESOURCE(texture);
SkASSERT(NULL != texture->getCacheEntry());
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index bdf6772..0b49e74 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -49,9 +49,62 @@
#endif
}
-GrGpu::~GrGpu() {}
+GrGpu::~GrGpu() {
+ this->releaseResources();
+}
-void GrGpu::contextAbandonded() {}
+void GrGpu::abandonResources() {
+
+ fClipMaskManager.releaseResources();
+
+ while (NULL != fObjectList.head()) {
+ fObjectList.head()->abandon();
+ }
+
+ SkASSERT(NULL == fQuadIndexBuffer || fQuadIndexBuffer->wasDestroyed());
+ SkSafeSetNull(fQuadIndexBuffer);
+ delete fVertexPool;
+ fVertexPool = NULL;
+ delete fIndexPool;
+ fIndexPool = NULL;
+}
+
+void GrGpu::releaseResources() {
+
+ fClipMaskManager.releaseResources();
+
+ while (NULL != fObjectList.head()) {
+ fObjectList.head()->release();
+ }
+
+ SkASSERT(NULL == fQuadIndexBuffer || fQuadIndexBuffer->wasDestroyed());
+ SkSafeSetNull(fQuadIndexBuffer);
+ delete fVertexPool;
+ fVertexPool = NULL;
+ delete fIndexPool;
+ fIndexPool = NULL;
+}
+
+void GrGpu::insertObject(GrGpuResource* object) {
+ SkASSERT(NULL != object);
+ SkASSERT(this == object->getGpu());
+
+ fObjectList.addToHead(object);
+}
+
+void GrGpu::removeObject(GrGpuResource* object) {
+ SkASSERT(NULL != object);
+ SkASSERT(this == object->getGpu());
+
+ fObjectList.remove(object);
+}
+
+
+void GrGpu::unimpl(const char msg[]) {
+#ifdef SK_DEBUG
+ GrPrintf("--- GrGpu unimplemented(\"%s\")\n", msg);
+#endif
+}
////////////////////////////////////////////////////////////////////////////////
@@ -266,8 +319,7 @@
}
const GrIndexBuffer* GrGpu::getQuadIndexBuffer() const {
- if (NULL == fQuadIndexBuffer || fQuadIndexBuffer->wasDestroyed()) {
- SkSafeUnref(fQuadIndexBuffer);
+ if (NULL == fQuadIndexBuffer) {
static const int SIZE = sizeof(uint16_t) * 6 * MAX_QUADS;
GrGpu* me = const_cast<GrGpu*>(this);
fQuadIndexBuffer = me->createIndexBuffer(SIZE, false);
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 360e889..b752f7c 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -54,12 +54,6 @@
GrContext* getContext() { return this->INHERITED::getContext(); }
const GrContext* getContext() const { return this->INHERITED::getContext(); }
- // Called by GrContext when the underlying backend context has been destroyed.
- // GrGpu should use this to ensure that no backend API calls will be made from
- // here onward, including in its destructor. Subclasses should call
- // INHERITED::contextAbandonded() if they override this.
- virtual void contextAbandonded();
-
/**
* The GrGpu object normally assumes that no outsider is setting state
* within the underlying 3D API's context/device/whatever. This call informs
@@ -254,6 +248,30 @@
GrPixelConfig config, const void* buffer,
size_t rowBytes);
+ /**
+ * Called to tell GrGpu that all GrGpuResources have been lost and should
+ * be abandoned. Overrides must call INHERITED::abandonResources().
+ */
+ virtual void abandonResources();
+
+ /**
+ * Called to tell GrGpu to release all GrGpuResources. Overrides must call
+ * INHERITED::releaseResources().
+ */
+ void releaseResources();
+
+ /**
+ * Add object to list of objects. Should only be called by GrGpuResource.
+ * @param resource the resource to add.
+ */
+ void insertObject(GrGpuResource* object);
+
+ /**
+ * Remove object from list of objects. Should only be called by GrGpuResource.
+ * @param resource the resource to remove.
+ */
+ void removeObject(GrGpuResource* object);
+
// GrDrawTarget overrides
virtual void clear(const SkIRect* rect,
GrColor color,
@@ -263,7 +281,7 @@
virtual void purgeResources() SK_OVERRIDE {
// The clip mask manager can rebuild all its clip masks so just
// get rid of them all.
- fClipMaskManager.purgeResources();
+ fClipMaskManager.releaseResources();
}
// After the client interacts directly with the 3D context state the GrGpu
@@ -508,6 +526,7 @@
enum {
kPreallocGeomPoolStateStackCnt = 4,
};
+ typedef SkTInternalLList<GrGpuResource> ObjectList;
SkSTArray<kPreallocGeomPoolStateStackCnt, GeometryPoolState, true> fGeomPoolStateStack;
ResetTimestamp fResetTimestamp;
uint32_t fResetBits;
@@ -518,6 +537,9 @@
int fIndexPoolUseCnt;
// these are mutable so they can be created on-demand
mutable GrIndexBuffer* fQuadIndexBuffer;
+ // Used to abandon/release all resources created by this GrGpu. TODO: Move this
+ // functionality to GrResourceCache.
+ ObjectList fObjectList;
typedef GrDrawTarget INHERITED;
};
diff --git a/src/gpu/GrGpuResource.cpp b/src/gpu/GrGpuResource.cpp
index 7a19c25..8c2dc45 100644
--- a/src/gpu/GrGpuResource.cpp
+++ b/src/gpu/GrGpuResource.cpp
@@ -8,27 +8,19 @@
#include "GrGpuResource.h"
-#include "GrResourceCache2.h"
#include "GrGpu.h"
-static inline GrResourceCache2* get_resource_cache2(GrGpu* gpu) {
- SkASSERT(NULL != gpu);
- SkASSERT(NULL != gpu->getContext());
- SkASSERT(NULL != gpu->getContext()->getResourceCache2());
- return gpu->getContext()->getResourceCache2();
-}
-
GrGpuResource::GrGpuResource(GrGpu* gpu, bool isWrapped)
- : fGpu(gpu)
- , fRefCnt(1)
+ : fRefCnt(1)
, fCacheEntry(NULL)
, fUniqueID(CreateUniqueID()) {
+ fGpu = gpu;
if (isWrapped) {
fFlags = kWrapped_FlagBit;
} else {
fFlags = 0;
}
- get_resource_cache2(fGpu)->insertResource(this);
+ fGpu->insertObject(this);
}
GrGpuResource::~GrGpuResource() {
@@ -40,7 +32,7 @@
void GrGpuResource::release() {
if (NULL != fGpu) {
this->onRelease();
- get_resource_cache2(fGpu)->removeResource(this);
+ fGpu->removeObject(this);
fGpu = NULL;
}
}
@@ -48,7 +40,7 @@
void GrGpuResource::abandon() {
if (NULL != fGpu) {
this->onAbandon();
- get_resource_cache2(fGpu)->removeResource(this);
+ fGpu->removeObject(this);
fGpu = NULL;
}
}
diff --git a/src/gpu/GrLayerCache.cpp b/src/gpu/GrLayerCache.cpp
index 9325497..c0510c0 100644
--- a/src/gpu/GrLayerCache.cpp
+++ b/src/gpu/GrLayerCache.cpp
@@ -112,7 +112,7 @@
fAtlas.free();
// GrLayerCache always assumes an atlas exists so recreate it. The atlas
// lazily allocates a replacement texture so reallocating a new
- // atlas here won't disrupt a GrContext::abandonContext or freeGpuResources.
+ // atlas here won't disrupt a GrContext::contextDestroyed or freeGpuResources.
// TODO: Make GrLayerCache lazily allocate the atlas manager?
this->initAtlas();
}
diff --git a/src/gpu/GrResourceCache.cpp b/src/gpu/GrResourceCache.cpp
index 8a21002..e3a17f0 100644
--- a/src/gpu/GrResourceCache.cpp
+++ b/src/gpu/GrResourceCache.cpp
@@ -267,7 +267,7 @@
void GrResourceCache::removeInvalidResource(GrResourceCacheEntry* entry) {
// If the resource went invalid while it was detached then purge it
// This can happen when a 3D context was lost,
- // the client called GrContext::abandonContext() to notify Gr,
+ // the client called GrContext::contextDestroyed() to notify Gr,
// and then later an SkGpuDevice's destructor releases its backing
// texture (which was invalidated at contextDestroyed time).
// TODO: Safely delete the GrResourceCacheEntry as well.
diff --git a/src/gpu/GrResourceCache2.cpp b/src/gpu/GrResourceCache2.cpp
deleted file mode 100644
index 0b6944e..0000000
--- a/src/gpu/GrResourceCache2.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-
-/*
- * Copyright 2014 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-
-#include "GrResourceCache2.h"
-#include "GrGpuResource.h"
-
-GrResourceCache2::~GrResourceCache2() {
- this->releaseAll();
-}
-
-void GrResourceCache2::insertResource(GrGpuResource* resource) {
- SkASSERT(NULL != resource);
- SkASSERT(!resource->wasDestroyed());
- fResources.addToHead(resource);
- ++fCount;
-}
-
-void GrResourceCache2::removeResource(GrGpuResource* resource) {
- fResources.remove(resource);
- --fCount;
-}
-
-void GrResourceCache2::abandonAll() {
- while (GrGpuResource* head = fResources.head()) {
- SkASSERT(!head->wasDestroyed());
- head->abandon();
- // abandon should have already removed this from the list.
- SkASSERT(head != fResources.head());
- }
- SkASSERT(!fCount);
-}
-
-void GrResourceCache2::releaseAll() {
- while (GrGpuResource* head = fResources.head()) {
- SkASSERT(!head->wasDestroyed());
- head->release();
- // release should have already removed this from the list.
- SkASSERT(head != fResources.head());
- }
- SkASSERT(!fCount);
-}
diff --git a/src/gpu/GrResourceCache2.h b/src/gpu/GrResourceCache2.h
deleted file mode 100644
index 1262c80..0000000
--- a/src/gpu/GrResourceCache2.h
+++ /dev/null
@@ -1,40 +0,0 @@
-
-/*
- * Copyright 2014 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#ifndef GrResourceCache2_DEFINED
-#define GrResourceCache2_DEFINED
-
-#include "GrTypes.h"
-#include "SkTInternalLList.h"
-
-class GrGpuResource;
-
-/**
- * Eventual replacement for GrResourceCache. Currently it simply holds a list
- * of all GrGpuResource objects for a GrContext. It is used to invalidate all
- * the resources when necessary.
- */
-class GrResourceCache2 {
-public:
- GrResourceCache2() : fCount(0) {};
- ~GrResourceCache2();
-
- void insertResource(GrGpuResource* resource);
-
- void removeResource(GrGpuResource* resource);
-
- void abandonAll();
-
- void releaseAll();
-
-private:
- int fCount;
- SkTInternalLList<GrGpuResource> fResources;
-};
-
-#endif
diff --git a/src/gpu/gl/GrGLPathRendering.cpp b/src/gpu/gl/GrGLPathRendering.cpp
index 6204deb..249d981 100644
--- a/src/gpu/gl/GrGLPathRendering.cpp
+++ b/src/gpu/gl/GrGLPathRendering.cpp
@@ -73,6 +73,7 @@
fPathNameAllocator.reset(NULL);
}
+
// NV_path_rendering
GrGLuint GrGLPathRendering::genPaths(GrGLsizei range) {
if (range > 1) {
diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp
index b975fd1..7131cfa 100644
--- a/src/gpu/gl/GrGpuGL.cpp
+++ b/src/gpu/gl/GrGpuGL.cpp
@@ -182,15 +182,9 @@
// This must be called by before the GrDrawTarget destructor
this->releaseGeometry();
-}
-
-void GrGpuGL::contextAbandonded() {
- INHERITED::contextAbandonded();
- fProgramCache->abandon();
- fHWProgramID = 0;
- if (this->glCaps().pathRenderingSupport()) {
- fPathRendering->abandonGpuResources();
- }
+ // This subclass must do this before the base class destructor runs
+ // since we will unref the GrGLInterface.
+ this->releaseResources();
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h
index 9270219..b39aedb 100644
--- a/src/gpu/gl/GrGpuGL.h
+++ b/src/gpu/gl/GrGpuGL.h
@@ -31,8 +31,6 @@
GrGpuGL(const GrGLContext& ctx, GrContext* context);
virtual ~GrGpuGL();
- virtual void contextAbandonded() SK_OVERRIDE;
-
const GrGLContext& glContext() const { return fGLContext; }
const GrGLInterface* glInterface() const { return fGLContext.interface(); }
@@ -85,6 +83,8 @@
virtual void initCopySurfaceDstDesc(const GrSurface* src, GrTextureDesc* desc) SK_OVERRIDE;
+ virtual void abandonResources() SK_OVERRIDE;
+
// These functions should be used to bind GL objects. They track the GL state and skip redundant
// bindings. Making the equivalent glBind calls directly will confuse the state tracking.
void bindVertexArray(GrGLuint id) {
diff --git a/src/gpu/gl/GrGpuGL_program.cpp b/src/gpu/gl/GrGpuGL_program.cpp
index b4fd464..beaaa90 100644
--- a/src/gpu/gl/GrGpuGL_program.cpp
+++ b/src/gpu/gl/GrGpuGL_program.cpp
@@ -199,6 +199,17 @@
////////////////////////////////////////////////////////////////////////////////
+void GrGpuGL::abandonResources(){
+ INHERITED::abandonResources();
+ fProgramCache->abandon();
+ fHWProgramID = 0;
+ if (this->glCaps().pathRenderingSupport()) {
+ fPathRendering->abandonGpuResources();
+ }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
#define GL_CALL(X) GR_GL_CALL(this->glInterface(), X)
bool GrGpuGL::flushGraphicsState(DrawType type, const GrDeviceCoordTexture* dstCopy) {