On 08/04/17 02:03, Brian Paul wrote:
On 04/06/2017 03:55 PM, Timothy Arceri wrote:
We don't need to call _mesa_reference_renderbuffer() for the first
assignment as refCount starts at 1. For swrast we work around the
fact we will indirectly call _mesa_reference_renderbuffer() by
resetting refCount to 0.
Fixes: 32141e53d1520 (mesa: tidy up renderbuffer RefCount initialisation)
---
src/mesa/main/fbobject.c | 2 +-
src/mesa/swrast/s_renderbuffer.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index e52c1e3..e46a7ca 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -441,21 +441,21 @@ _mesa_update_texture_renderbuffer(struct
gl_context *ctx,
texImage = att->Texture->Image[att->CubeMapFace][att->TextureLevel];
rb = att->Renderbuffer;
if (!rb) {
rb = ctx->Driver.NewRenderbuffer(ctx, ~0);
if (!rb) {
_mesa_error(ctx, GL_OUT_OF_MEMORY, "glFramebufferTexture()");
return;
}
- _mesa_reference_renderbuffer(&att->Renderbuffer, rb);
+ att->Renderbuffer = rb;
/* This can't get called on a texture renderbuffer, so set it
to NULL
* for clarity compared to user renderbuffers.
*/
rb->AllocStorage = NULL;
rb->NeedsFinishRenderTexture = ctx->Driver.FinishRenderTexture
!= NULL;
}
if (!texImage)
diff --git a/src/mesa/swrast/s_renderbuffer.c
b/src/mesa/swrast/s_renderbuffer.c
index d8c4ed3..af09955 100644
--- a/src/mesa/swrast/s_renderbuffer.c
+++ b/src/mesa/swrast/s_renderbuffer.c
@@ -264,20 +264,25 @@ add_color_renderbuffers(struct gl_context *ctx,
struct gl_framebuffer *fb,
continue;
assert(fb->Attachment[b].Renderbuffer == NULL);
rb = ctx->Driver.NewRenderbuffer(ctx, 0);
if (!rb) {
_mesa_error(ctx, GL_OUT_OF_MEMORY, "Allocating color buffer");
return GL_FALSE;
}
+ /* Set refcount to 0 to avoid a leak since the
_mesa_add_renderbuffer()
+ * call below will bump the initial refcount.
+ */
+ rb->RefCount = 0;
+
rb->InternalFormat = GL_RGBA;
rb->AllocStorage = soft_renderbuffer_storage;
_mesa_add_renderbuffer(fb, b, rb);
Perhaps another way of doing this which wouldn't look quite so hacky
would be to call _mesa_renderbuffer_reference(&rb, NULL) here, after the
_mesa_add_renderbuffer() call. The idea is we're transferring ownership
of the rb and the rb var is going out of scope.
I'd already pushed before seeing this. However it seems there were
additional leaks from internal renderbuffer creation that I hadn't
noticed, so I've sent a new series that fixes that (including updating
the changes from this patch) and in a less hacky looking way.
https://patchwork.freedesktop.org/series/22708/
In the series I avoid calling _mesa_renderbuffer_reference() when it's
not required which should be better as it creates an unnecessary lock.
Thanks,
Tim
Something similar could be done above. A comment indicating that we're
transferring ownership could be helpful too.
-Brian
}
return GL_TRUE;
}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
