Fix some renderbuffer reference counting issues. Also fixes a mem leak.

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);
 }

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
+               /* no longer referenced by hash table */
-                * bound in any context.
+               _mesa_reference_renderbuffer(&rb, NULL);
-                */
-               _mesa_unreference_renderbuffer(&rb);
 	    }
 	 }
       }

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 */
+   /* unbind _Depth/_StencilBuffer to decr ref counts */
-   set_depth_renderbuffer(fb, NULL);
+   _mesa_reference_renderbuffer(&fb->_DepthBuffer, NULL);
-   set_stencil_renderbuffer(fb, 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);
    }
 }
 

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);

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;
+   _mesa_reference_renderbuffer(&fb->Attachment[bufferName].Renderbuffer, rb);
-
-   rb->RefCount++;
 }
 
 
@@ -2125,40 +2123,57 @@ _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
+ * Set *ptr to point to rb.  If *ptr points to another renderbuffer,
- * the refcount hits zero.
+ * dereference that buffer first.  The new renderbuffer's refcount will
- * Note: we pass the address of a pointer.
+ * 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);
+   assert(ptr);
-   if (*rb) {
+   if (*ptr == rb) {
-      GLboolean deleteFlag = GL_FALSE;
+      /* no change */
+      return;
+   }
 
-      _glthread_LOCK_MUTEX((*rb)->Mutex);
+   if (*ptr) {
-      ASSERT((*rb)->RefCount > 0);
+      /* Unreference the old renderbuffer */
-      (*rb)->RefCount--;
+      GLboolean deleteFlag = GL_FALSE;
-      deleteFlag = ((*rb)->RefCount == 0);
+      struct gl_renderbuffer *oldRb = *ptr;
-      _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;
    }
 }
 
 
-
-
 /**
  * Create a new combined depth/stencil renderbuffer for implementing
  * the GL_EXT_packed_depth_stencil extension.
@@ -2180,4 +2195,3 @@ _mesa_new_depthstencil_renderbuffer(GLcontext *ctx, GLuint name)
 
    return dsrb;
 }
-

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);