From 42aaa548a1020be5d40b3dce9448d8004b1ef947 Mon Sep 17 00:00:00 2001 From: Brian Date: Sun, 25 Mar 2007 10:39:36 -0600 Subject: Fix some renderbuffer reference counting issues. Also fixes a mem leak. --- src/mesa/drivers/x11/xm_buffer.c | 12 --------- src/mesa/main/fbobject.c | 22 +++++++--------- src/mesa/main/framebuffer.c | 52 ++++++------------------------------- src/mesa/main/rbadaptors.c | 2 +- src/mesa/main/renderbuffer.c | 56 +++++++++++++++++++++++++--------------- src/mesa/main/renderbuffer.h | 3 ++- 6 files changed, 55 insertions(+), 92 deletions(-) diff --git a/src/mesa/drivers/x11/xm_buffer.c b/src/mesa/drivers/x11/xm_buffer.c index c1fa23328f..bb8fe31ce8 100644 --- a/src/mesa/drivers/x11/xm_buffer.c +++ b/src/mesa/drivers/x11/xm_buffer.c @@ -422,18 +422,6 @@ xmesa_delete_framebuffer(struct gl_framebuffer *fb) XMesaDestroyImage( b->rowimage ); } - /* Note that XMesaBuffer renderbuffers normally have a refcount of 2 - * (creation + binding) so we need to explicitly delete/unbind them here. - */ - if (b->frontxrb) { - _mesa_unreference_renderbuffer((struct gl_renderbuffer **) &b->frontxrb); - ASSERT(b->frontxrb == NULL); - } - if (b->backxrb) { - _mesa_unreference_renderbuffer((struct gl_renderbuffer **) &b->backxrb); - ASSERT(b->backxrb == NULL); - } - _mesa_free_framebuffer_data(fb); _mesa_free(fb); } diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index f7e870b49c..fefa14e503 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -559,7 +559,7 @@ _mesa_IsRenderbufferEXT(GLuint renderbuffer) void GLAPIENTRY _mesa_BindRenderbufferEXT(GLenum target, GLuint renderbuffer) { - struct gl_renderbuffer *newRb, *oldRb; + struct gl_renderbuffer *newRb; GET_CURRENT_CONTEXT(ctx); ASSERT_OUTSIDE_BEGIN_END(ctx); @@ -593,21 +593,16 @@ _mesa_BindRenderbufferEXT(GLenum target, GLuint renderbuffer) } ASSERT(newRb->AllocStorage); _mesa_HashInsert(ctx->Shared->RenderBuffers, renderbuffer, newRb); + newRb->RefCount = 1; /* referenced by hash table */ } - newRb->RefCount++; } else { newRb = NULL; } - oldRb = ctx->CurrentRenderbuffer; - if (oldRb) { - _mesa_unreference_renderbuffer(&oldRb); - } - ASSERT(newRb != &DummyRenderbuffer); - ctx->CurrentRenderbuffer = newRb; + _mesa_reference_renderbuffer(&ctx->CurrentRenderbuffer, newRb); } @@ -632,14 +627,15 @@ _mesa_DeleteRenderbuffersEXT(GLsizei n, const GLuint *renderbuffers) _mesa_BindRenderbufferEXT(GL_RENDERBUFFER_EXT, 0); } - /* remove from hash table immediately, to free the ID */ + /* Remove from hash table immediately, to free the ID. + * But the object will not be freed until it's no longer + * referenced anywhere else. + */ _mesa_HashRemove(ctx->Shared->RenderBuffers, renderbuffers[i]); if (rb != &DummyRenderbuffer) { - /* But the object will not be freed until it's no longer - * bound in any context. - */ - _mesa_unreference_renderbuffer(&rb); + /* no longer referenced by hash table */ + _mesa_reference_renderbuffer(&rb, NULL); } } } diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c index cd4f594aa2..1fd31a5321 100644 --- a/src/mesa/main/framebuffer.c +++ b/src/mesa/main/framebuffer.c @@ -69,42 +69,6 @@ compute_depth_max(struct gl_framebuffer *fb) } -/** - * Set the framebuffer's _DepthBuffer field, taking care of - * reference counts, etc. - */ -static void -set_depth_renderbuffer(struct gl_framebuffer *fb, - struct gl_renderbuffer *rb) -{ - if (fb->_DepthBuffer) { - _mesa_unreference_renderbuffer(&fb->_DepthBuffer); - } - fb->_DepthBuffer = rb; - if (rb) { - rb->RefCount++; - } -} - - -/** - * Set the framebuffer's _StencilBuffer field, taking care of - * reference counts, etc. - */ -static void -set_stencil_renderbuffer(struct gl_framebuffer *fb, - struct gl_renderbuffer *rb) -{ - if (fb->_StencilBuffer) { - _mesa_unreference_renderbuffer(&fb->_StencilBuffer); - } - fb->_StencilBuffer = rb; - if (rb) { - rb->RefCount++; - } -} - - /** * Create and initialize a gl_framebuffer object. * This is intended for creating _window_system_ framebuffers, not generic @@ -223,7 +187,7 @@ _mesa_free_framebuffer_data(struct gl_framebuffer *fb) for (i = 0; i < BUFFER_COUNT; i++) { struct gl_renderbuffer_attachment *att = &fb->Attachment[i]; if (att->Renderbuffer) { - _mesa_unreference_renderbuffer(&att->Renderbuffer); + _mesa_reference_renderbuffer(&att->Renderbuffer, NULL); } if (att->Texture) { /* render to texture */ @@ -239,9 +203,9 @@ _mesa_free_framebuffer_data(struct gl_framebuffer *fb) att->Texture = NULL; } - /* unbind depth/stencil to decr ref counts */ - set_depth_renderbuffer(fb, NULL); - set_stencil_renderbuffer(fb, NULL); + /* unbind _Depth/_StencilBuffer to decr ref counts */ + _mesa_reference_renderbuffer(&fb->_DepthBuffer, NULL); + _mesa_reference_renderbuffer(&fb->_StencilBuffer, NULL); } @@ -569,13 +533,13 @@ _mesa_update_depth_buffer(GLcontext *ctx, /* need to update wrapper */ struct gl_renderbuffer *wrapper = _mesa_new_z24_renderbuffer_wrapper(ctx, depthRb); - set_depth_renderbuffer(fb, wrapper); + _mesa_reference_renderbuffer(&fb->_DepthBuffer, wrapper); ASSERT(fb->_DepthBuffer->Wrapped == depthRb); } } else { /* depthRb may be null */ - set_depth_renderbuffer(fb, depthRb); + _mesa_reference_renderbuffer(&fb->_DepthBuffer, depthRb); } } @@ -610,13 +574,13 @@ _mesa_update_stencil_buffer(GLcontext *ctx, /* need to update wrapper */ struct gl_renderbuffer *wrapper = _mesa_new_s8_renderbuffer_wrapper(ctx, stencilRb); - set_stencil_renderbuffer(fb, wrapper); + _mesa_reference_renderbuffer(&fb->_StencilBuffer, wrapper); ASSERT(fb->_StencilBuffer->Wrapped == stencilRb); } } else { /* stencilRb may be null */ - set_stencil_renderbuffer(fb, stencilRb); + _mesa_reference_renderbuffer(&fb->_StencilBuffer, stencilRb); } } diff --git a/src/mesa/main/rbadaptors.c b/src/mesa/main/rbadaptors.c index 60f4948bec..c1ac0606c8 100644 --- a/src/mesa/main/rbadaptors.c +++ b/src/mesa/main/rbadaptors.c @@ -45,7 +45,7 @@ Delete_wrapper(struct gl_renderbuffer *rb) /* Decrement reference count on the buffer we're wrapping and delete * it if refcount hits zero. */ - _mesa_unreference_renderbuffer(&rb->Wrapped); + _mesa_reference_renderbuffer(&rb->Wrapped, NULL); /* delete myself */ _mesa_delete_renderbuffer(rb); diff --git a/src/mesa/main/renderbuffer.c b/src/mesa/main/renderbuffer.c index ded0063c73..49706b5251 100644 --- a/src/mesa/main/renderbuffer.c +++ b/src/mesa/main/renderbuffer.c @@ -1473,7 +1473,7 @@ _mesa_init_renderbuffer(struct gl_renderbuffer *rb, GLuint name) rb->ClassID = 0; rb->Name = name; - rb->RefCount = 1; + rb->RefCount = 0; rb->Delete = _mesa_delete_renderbuffer; /* The rest of these should be set later by the caller of this function or @@ -2105,9 +2105,7 @@ _mesa_add_renderbuffer(struct gl_framebuffer *fb, fb->Attachment[bufferName].Type = GL_RENDERBUFFER_EXT; fb->Attachment[bufferName].Complete = GL_TRUE; - fb->Attachment[bufferName].Renderbuffer = rb; - - rb->RefCount++; + _mesa_reference_renderbuffer(&fb->Attachment[bufferName].Renderbuffer, rb); } @@ -2125,38 +2123,55 @@ _mesa_remove_renderbuffer(struct gl_framebuffer *fb, GLuint bufferName) if (!rb) return; - _mesa_unreference_renderbuffer(&rb); + _mesa_reference_renderbuffer(&rb, NULL); fb->Attachment[bufferName].Renderbuffer = NULL; } /** - * Decrement a renderbuffer object's reference count and delete it when - * the refcount hits zero. - * Note: we pass the address of a pointer. + * Set *ptr to point to rb. If *ptr points to another renderbuffer, + * dereference that buffer first. The new renderbuffer's refcount will + * be incremented. The old renderbuffer's refcount will be decremented. */ void -_mesa_unreference_renderbuffer(struct gl_renderbuffer **rb) +_mesa_reference_renderbuffer(struct gl_renderbuffer **ptr, + struct gl_renderbuffer *rb) { - assert(rb); - if (*rb) { + assert(ptr); + if (*ptr == rb) { + /* no change */ + return; + } + + if (*ptr) { + /* Unreference the old renderbuffer */ GLboolean deleteFlag = GL_FALSE; + struct gl_renderbuffer *oldRb = *ptr; - _glthread_LOCK_MUTEX((*rb)->Mutex); - ASSERT((*rb)->RefCount > 0); - (*rb)->RefCount--; - deleteFlag = ((*rb)->RefCount == 0); - _glthread_UNLOCK_MUTEX((*rb)->Mutex); + _glthread_LOCK_MUTEX(oldRb->Mutex); + ASSERT(oldRb->RefCount > 0); + oldRb->RefCount--; + /*printf("RB DECR %p to %d\n", (void*) oldRb, oldRb->RefCount);*/ + deleteFlag = (oldRb->RefCount == 0); + _glthread_UNLOCK_MUTEX(oldRb->Mutex); if (deleteFlag) - (*rb)->Delete(*rb); + oldRb->Delete(oldRb); - *rb = NULL; + *ptr = NULL; } -} - + assert(!*ptr); + if (rb) { + /* reference new renderbuffer */ + _glthread_LOCK_MUTEX(rb->Mutex); + rb->RefCount++; + /*printf("RB REF %p to %d\n", (void*)rb, rb->RefCount);*/ + _glthread_UNLOCK_MUTEX(rb->Mutex); + *ptr = rb; + } +} /** @@ -2180,4 +2195,3 @@ _mesa_new_depthstencil_renderbuffer(GLcontext *ctx, GLuint name) return dsrb; } - diff --git a/src/mesa/main/renderbuffer.h b/src/mesa/main/renderbuffer.h index e5f1147b4a..c9bf888548 100644 --- a/src/mesa/main/renderbuffer.h +++ b/src/mesa/main/renderbuffer.h @@ -102,7 +102,8 @@ extern void _mesa_remove_renderbuffer(struct gl_framebuffer *fb, GLuint bufferName); extern void -_mesa_unreference_renderbuffer(struct gl_renderbuffer **rb); +_mesa_reference_renderbuffer(struct gl_renderbuffer **ptr, + struct gl_renderbuffer *rb); extern struct gl_renderbuffer * _mesa_new_depthstencil_renderbuffer(GLcontext *ctx, GLuint name); -- cgit v1.2.3