From 11f0b519766dabed4ce882f3ee7c07a68d3ccb52 Mon Sep 17 00:00:00 2001 From: "bsalomon@google.com" Date: Tue, 29 Mar 2011 20:52:23 +0000 Subject: [PATCH] Fix ref leak on GrGpu. Review URL: http://codereview.appspot.com/4323043/ git-svn-id: http://skia.googlecode.com/svn/trunk@1015 2bbb7eff-a529-9590-31e7-b0007b416f81 --- gpu/src/GrBufferAllocPool.cpp | 16 ++++++-- gpu/src/GrBufferAllocPool.h | 72 ++++++++++++++++++--------------- gpu/src/GrGpu.cpp | 18 +++++---- samplecode/SampleApp.cpp | 19 ++++++--- xcode/gpu/gpu.xcodeproj/project.pbxproj | 8 ++++ 5 files changed, 83 insertions(+), 50 deletions(-) diff --git a/gpu/src/GrBufferAllocPool.cpp b/gpu/src/GrBufferAllocPool.cpp index 35f0c5e..0db12fe 100644 --- a/gpu/src/GrBufferAllocPool.cpp +++ b/gpu/src/GrBufferAllocPool.cpp @@ -34,11 +34,14 @@ GrBufferAllocPool::GrBufferAllocPool(GrGpu* gpu, size_t blockSize, int preallocBufferCnt) : fBlocks(GrMax(8, 2*preallocBufferCnt)) { + GrAssert(NULL != gpu); fGpu = gpu; + fGpu->ref(); + fGpuIsReffed = true; + fBufferType = bufferType; fFrequentResetHint = frequentResetHint; - fGpu->ref(); fBufferPtr = NULL; fMinBlockSize = GrMax(GrBufferAllocPool_MIN_BLOCK_SIZE, blockSize); @@ -61,11 +64,18 @@ GrBufferAllocPool::~GrBufferAllocPool() { buffer->unlock(); } } - fPreallocBuffers.unrefAll(); while (!fBlocks.empty()) { destroyBlock(); } - fGpu->unref(); + fPreallocBuffers.unrefAll(); + releaseGpuRef(); +} + +void GrBufferAllocPool::releaseGpuRef() { + if (fGpuIsReffed) { + fGpu->unref(); + fGpuIsReffed = false; + } } void GrBufferAllocPool::reset() { diff --git a/gpu/src/GrBufferAllocPool.h b/gpu/src/GrBufferAllocPool.h index 80f16ab..7d70ebb 100644 --- a/gpu/src/GrBufferAllocPool.h +++ b/gpu/src/GrBufferAllocPool.h @@ -39,38 +39,6 @@ class GrGpu; * be allocated at the min size and kept around until the pool is destroyed. */ class GrBufferAllocPool : GrNoncopyable { -protected: - - // We could make the createBuffer a virtual except that we want to use it - // in the cons for pre-allocated buffers. - enum BufferType { - kVertex_BufferType, - kIndex_BufferType, - }; - - /** - * Constructor - * - * @param gpu The GrGpu used to create the buffers. - * @param bufferType The type of buffers to create. - * @param frequentResetHint A hint that indicates that the pool - * should expect frequent unlock() calls - * (as opposed to many makeSpace / acquires - * between resets). - * @param bufferSize The minimum size of created buffers. - * This value will be clamped to some - * reasonable minimum. - * @param preallocBufferCnt The pool will allocate this number of - * buffers at bufferSize and keep them until it - * is destroyed. - */ - GrBufferAllocPool(GrGpu* gpu, - BufferType bufferType, - bool frequentResetHint, - size_t bufferSize = 0, - int preallocBufferCnt = 0); - - virtual ~GrBufferAllocPool(); public: /** @@ -94,7 +62,6 @@ public: */ int preallocatedBufferCount() const; - /** * Frees data from makeSpaces in LIFO order. */ @@ -107,6 +74,40 @@ public: protected: /** + * Used to determine what type of buffers to create. We could make the + * createBuffer a virtual except that we want to use it in the cons for + * pre-allocated buffers. + */ + enum BufferType { + kVertex_BufferType, + kIndex_BufferType, + }; + + /** + * Constructor + * + * @param gpu The GrGpu used to create the buffers. + * @param bufferType The type of buffers to create. + * @param frequentResetHint A hint that indicates that the pool + * should expect frequent unlock() calls + * (as opposed to many makeSpace / acquires + * between resets). + * @param bufferSize The minimum size of created buffers. + * This value will be clamped to some + * reasonable minimum. + * @param preallocBufferCnt The pool will allocate this number of + * buffers at bufferSize and keep them until it + * is destroyed. + */ + GrBufferAllocPool(GrGpu* gpu, + BufferType bufferType, + bool frequentResetHint, + size_t bufferSize = 0, + int preallocBufferCnt = 0); + + virtual ~GrBufferAllocPool(); + + /** * Gets the size of the preallocated buffers. * * @return the size of preallocated buffers. @@ -155,6 +156,10 @@ protected: private: + // The GrGpu must be able to clear the ref of pools it creates as members + friend class GrGpu; + void releaseGpuRef(); + struct BufferBlock { size_t fBytesFree; GrGeometryBuffer* fBuffer; @@ -168,6 +173,7 @@ private: #endif GrGpu* fGpu; + bool fGpuIsReffed; bool fFrequentResetHint; GrTDArray fPreallocBuffers; size_t fMinBlockSize; diff --git a/gpu/src/GrGpu.cpp b/gpu/src/GrGpu.cpp index 1db9252..378b881 100644 --- a/gpu/src/GrGpu.cpp +++ b/gpu/src/GrGpu.cpp @@ -432,7 +432,7 @@ bool GrGpu::setupClipAndFlushState(GrPrimitiveType type) { const GrPath& path = clip.getPath(c); pathIter.reset(path); pr = this->getClipPathRenderer(&pathIter, NonInvertedFill(fill)); - canRenderDirectToStencil = + canRenderDirectToStencil = !pr->requiresStencilPass(this, &pathIter, NonInvertedFill(fill)); } @@ -441,14 +441,14 @@ bool GrGpu::setupClipAndFlushState(GrPrimitiveType type) { int passes; GrStencilSettings stencilSettings[GrStencilSettings::kMaxStencilClipPasses]; - bool canDrawDirectToClip; // Given the renderer, the element, + bool canDrawDirectToClip; // Given the renderer, the element, // fill rule, and set operation can // we render the element directly to // stencil bit used for clipping. - canDrawDirectToClip = - GrStencilSettings::GetClipPasses(op, + canDrawDirectToClip = + GrStencilSettings::GetClipPasses(op, canRenderDirectToStencil, - clipBit, + clipBit, IsFillInverted(fill), &passes, stencilSettings); @@ -469,7 +469,7 @@ bool GrGpu::setupClipAndFlushState(GrPrimitiveType type) { } else { if (canRenderDirectToStencil) { this->setStencil(gDrawToStencil); - pr->drawPath(this, 0, + pr->drawPath(this, 0, &pathIter, NonInvertedFill(fill), NULL); @@ -519,12 +519,12 @@ bool GrGpu::setupClipAndFlushState(GrPrimitiveType type) { GrPathRenderer* GrGpu::getClipPathRenderer(GrPathIter* path, GrPathFill fill) { - if (NULL != fClientPathRenderer && + if (NULL != fClientPathRenderer && fClientPathRenderer->canDrawPath(this, path, fill)) { return fClientPathRenderer; } else { if (NULL == fDefaultPathRenderer) { - fDefaultPathRenderer = + fDefaultPathRenderer = new GrDefaultPathRenderer(this->supportsTwoSidedStencil(), this->supportsStencilWrapOps()); } @@ -603,6 +603,7 @@ void GrGpu::prepareVertexPool() { fVertexPool = new GrVertexBufferAllocPool(this, true, VERTEX_POOL_VB_SIZE, VERTEX_POOL_VB_COUNT); + fVertexPool->releaseGpuRef(); } else if (!fVertexPoolInUse) { // the client doesn't have valid data in the pool fVertexPool->reset(); @@ -612,6 +613,7 @@ void GrGpu::prepareVertexPool() { void GrGpu::prepareIndexPool() { if (NULL == fIndexPool) { fIndexPool = new GrIndexBufferAllocPool(this, true, 0, 1); + fIndexPool->releaseGpuRef(); } else if (!fIndexPoolInUse) { // the client doesn't have valid data in the pool fIndexPool->reset(); diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp index 0cc85fe..c71326b 100644 --- a/samplecode/SampleApp.cpp +++ b/samplecode/SampleApp.cpp @@ -342,6 +342,13 @@ bool SampleWindow::make3DReady() { #if defined(SK_SUPPORT_GL) if (attachGL()) { +#if 0 + if (NULL != fGrContext) { + GrAssert(1 == fGrContext->refcnt()); + fGrContext->unref(); + fGrContext = NULL; + } +#endif if (NULL == fGrContext) { #if defined(SK_USE_SHADERS) fGrContext = GrContext::Create(GrGpu::kOpenGL_Shaders_Engine, NULL); @@ -490,14 +497,14 @@ void SampleWindow::draw(SkCanvas* canvas) { m.mapXY(cx, cy, ¢er); cx = center.fX; cy = center.fY; - + m.setTranslate(-cx, -cy); m.postScale(fZoomScale, fZoomScale); m.postTranslate(cx, cy); - + canvas->concat(m); } - + // Apply any gesture matrix if (true) { const SkMatrix& localM = fGesture.localM(); @@ -506,12 +513,12 @@ void SampleWindow::draw(SkCanvas* canvas) { } canvas->concat(localM); canvas->concat(fGesture.globalM()); - + if (fGesture.isActive()) { this->inval(NULL); } } - + if (fNClip) { this->INHERITED::draw(canvas); SkBitmap orig = capture_bitmap(canvas); @@ -684,7 +691,7 @@ SkCanvas* SampleWindow::beforeChildren(SkCanvas* canvas) { bitmap.width(), bitmap.height(), false, false); fGpuCanvas->setDevice(device)->unref(); - + fGpuCanvas->concat(canvas->getTotalMatrix()); canvas = fGpuCanvas; diff --git a/xcode/gpu/gpu.xcodeproj/project.pbxproj b/xcode/gpu/gpu.xcodeproj/project.pbxproj index 5a44d54..d7a682d 100644 --- a/xcode/gpu/gpu.xcodeproj/project.pbxproj +++ b/xcode/gpu/gpu.xcodeproj/project.pbxproj @@ -91,6 +91,8 @@ 7D66934C132ABD8F003AC2F5 /* GrGLInterface.h in Headers */ = {isa = PBXBuildFile; fileRef = 7D66934B132ABD8F003AC2F5 /* GrGLInterface.h */; }; 7D66934E132ABDA7003AC2F5 /* GrGLPlatformIncludes.h in Headers */ = {isa = PBXBuildFile; fileRef = 7D66934D132ABDA7003AC2F5 /* GrGLPlatformIncludes.h */; }; 7D6EBF5A13330E8400AEAADD /* GrGLDefines.h in Headers */ = {isa = PBXBuildFile; fileRef = 7D6EBF5913330E8400AEAADD /* GrGLDefines.h */; }; + D51B4C3C1342649500718A57 /* GrPathUtils.cpp in Sources */ = {isa = PBXBuildFile; fileRef = D51B4C3A1342649500718A57 /* GrPathUtils.cpp */; }; + D51B4C3D1342649500718A57 /* GrPathUtils.h in Headers */ = {isa = PBXBuildFile; fileRef = D51B4C3B1342649500718A57 /* GrPathUtils.h */; }; D539049B12EA01E30025F3D6 /* GrContext_impl.h in Headers */ = {isa = PBXBuildFile; fileRef = D539049A12EA01E30025F3D6 /* GrContext_impl.h */; }; D53904A112EA026E0025F3D6 /* GrPaint.h in Headers */ = {isa = PBXBuildFile; fileRef = D53904A012EA026E0025F3D6 /* GrPaint.h */; }; D542EAAD131C87E90065FC9D /* GrStencil.h in Headers */ = {isa = PBXBuildFile; fileRef = D542EAAC131C87E90065FC9D /* GrStencil.h */; }; @@ -191,6 +193,8 @@ 7D66934D132ABDA7003AC2F5 /* GrGLPlatformIncludes.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = GrGLPlatformIncludes.h; path = ../../gpu/include/GrGLPlatformIncludes.h; sourceTree = SOURCE_ROOT; }; 7D6EBF5913330E8400AEAADD /* GrGLDefines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = GrGLDefines.h; path = ../../gpu/include/GrGLDefines.h; sourceTree = SOURCE_ROOT; }; D2AAC046055464E500DB518D /* libgpu.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libgpu.a; sourceTree = BUILT_PRODUCTS_DIR; }; + D51B4C3A1342649500718A57 /* GrPathUtils.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = GrPathUtils.cpp; path = ../../gpu/src/GrPathUtils.cpp; sourceTree = SOURCE_ROOT; }; + D51B4C3B1342649500718A57 /* GrPathUtils.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = GrPathUtils.h; path = ../../gpu/src/GrPathUtils.h; sourceTree = SOURCE_ROOT; }; D539049A12EA01E30025F3D6 /* GrContext_impl.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = GrContext_impl.h; path = ../../gpu/include/GrContext_impl.h; sourceTree = SOURCE_ROOT; }; D53904A012EA026E0025F3D6 /* GrPaint.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = GrPaint.h; path = ../../gpu/include/GrPaint.h; sourceTree = SOURCE_ROOT; }; D542EAAC131C87E90065FC9D /* GrStencil.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = GrStencil.h; path = ../../gpu/include/GrStencil.h; sourceTree = SOURCE_ROOT; }; @@ -297,6 +301,8 @@ 08FB7795FE84155DC02AAC07 /* Source */ = { isa = PBXGroup; children = ( + D51B4C3A1342649500718A57 /* GrPathUtils.cpp */, + D51B4C3B1342649500718A57 /* GrPathUtils.h */, D59BD412133BD384003B546A /* GrCreatePathRenderer_none.cpp */, 7D669345132ABD5D003AC2F5 /* GrGLInterface.cpp */, D5ED886E1313F92C00B98D64 /* GrRedBlackTree.h */, @@ -426,6 +432,7 @@ 7D66934E132ABDA7003AC2F5 /* GrGLPlatformIncludes.h in Headers */, 7D6EBF5A13330E8400AEAADD /* GrGLDefines.h in Headers */, D59BD3F4133BBB49003B546A /* GrPathRenderer.h in Headers */, + D51B4C3D1342649500718A57 /* GrPathUtils.h in Headers */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -507,6 +514,7 @@ D5558AE3131EB9BB00C71009 /* GrStencil.cpp in Sources */, 7D669346132ABD5D003AC2F5 /* GrGLInterface.cpp in Sources */, D59BD413133BD384003B546A /* GrCreatePathRenderer_none.cpp in Sources */, + D51B4C3C1342649500718A57 /* GrPathUtils.cpp in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; -- 2.7.4