[Mesa-dev] [PATCH V3 1/9] mesa: create _mesa_attach_renderbuffer_without_ref() helper

[Brian Paul]

On 04/07/2017 09:21 PM, Timothy Arceri wrote:
> This will be used to take ownership of freashly created renderbuffers,
> avoiding the need to call the reference function which requires
> locking.
>
> V2: dereference any existing fb attachments and actually attach the
>      new rb.
>
> v3: split out validation and attachment type/complete setting into
>      a shared static function.
> ---
>   src/mesa/main/renderbuffer.c | 43 +++++++++++++++++++++++++++++++++++--------
>   src/mesa/main/renderbuffer.h |  5 +++++
>   2 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/main/renderbuffer.c b/src/mesa/main/renderbuffer.c
> index 4375b5b..627bdca 100644
> --- a/src/mesa/main/renderbuffer.c
> +++ b/src/mesa/main/renderbuffer.c
> @@ -99,28 +99,24 @@ _mesa_new_renderbuffer(struct gl_context *ctx, GLuint name)
>    * free the object in the end.
>    */
>   void
>   _mesa_delete_renderbuffer(struct gl_context *ctx, struct gl_renderbuffer *rb)
>   {
>      mtx_destroy(&rb->Mutex);
>      free(rb->Label);
>      free(rb);
>   }
>
> -
> -/**
> - * Attach a renderbuffer to a framebuffer.
> - * \param bufferName  one of the BUFFER_x tokens
> - */
> -void
> -_mesa_add_renderbuffer(struct gl_framebuffer *fb,
> -                       gl_buffer_index bufferName, struct gl_renderbuffer *rb)
> +static void
> +validate_and_init_renderbuffer_attachment(struct gl_framebuffer *fb,
> +                                          gl_buffer_index bufferName,
> +                                          struct gl_renderbuffer *rb)
>   {
>      assert(fb);
>      assert(rb);
>      assert(bufferName < BUFFER_COUNT);
>
>      /* There should be no previous renderbuffer on this attachment point,
>       * with the exception of depth/stencil since the same renderbuffer may
>       * be used for both.
>       */
>      assert(bufferName == BUFFER_DEPTH ||
> @@ -130,20 +126,51 @@ _mesa_add_renderbuffer(struct gl_framebuffer *fb,
>      /* winsys vs. user-created buffer cross check */
>      if (_mesa_is_user_fbo(fb)) {
>         assert(rb->Name);
>      }
>      else {
>         assert(!rb->Name);
>      }
>
>      fb->Attachment[bufferName].Type = GL_RENDERBUFFER_EXT;
>      fb->Attachment[bufferName].Complete = GL_TRUE;
> +}
> +
> +
> +/**
> + * Attach a renderbuffer to a framebuffer.
> + * \param bufferName  one of the BUFFER_x tokens
> + *
> + * This function avoids adding a reference and is therefore intended to be
> + * used with a freashly created renderbuffer.

"freshly"


> + */
> +void
> +_mesa_add_renderbuffer_without_ref(struct gl_framebuffer *fb,
> +                                   gl_buffer_index bufferName,
> +                                   struct gl_renderbuffer *rb)

I see you've already pushed this.  Still, I'd like to suggest a 
different name such as _mesa_own_renderbuffer() that stresses the 
transfer of ownership of the renderbuffer.


> +{

If this function should only be used with a "freshly created" 
renderbuffer, can we assert that its RefCount is one here?

-Brian

> +   validate_and_init_renderbuffer_attachment(fb, bufferName, rb);
> +
> +   _mesa_reference_renderbuffer(&fb->Attachment[bufferName].Renderbuffer,
> +                                NULL);
> +   fb->Attachment[bufferName].Renderbuffer = rb;
> +}
> +
> +/**
> + * Attach a renderbuffer to a framebuffer.
> + * \param bufferName  one of the BUFFER_x tokens
> + */
> +void
> +_mesa_add_renderbuffer(struct gl_framebuffer *fb,
> +                       gl_buffer_index bufferName, struct gl_renderbuffer *rb)
> +{
> +   validate_and_init_renderbuffer_attachment(fb, bufferName, rb);
>      _mesa_reference_renderbuffer(&fb->Attachment[bufferName].Renderbuffer, rb);
>   }
>
>
>   /**
>    * Remove the named renderbuffer from the given framebuffer.
>    * \param bufferName  one of the BUFFER_x tokens
>    */
>   void
>   _mesa_remove_renderbuffer(struct gl_framebuffer *fb,
> diff --git a/src/mesa/main/renderbuffer.h b/src/mesa/main/renderbuffer.h
> index aa83120..a6f1439 100644
> --- a/src/mesa/main/renderbuffer.h
> +++ b/src/mesa/main/renderbuffer.h
> @@ -40,20 +40,25 @@ struct gl_renderbuffer;
>   extern void
>   _mesa_init_renderbuffer(struct gl_renderbuffer *rb, GLuint name);
>
>   extern struct gl_renderbuffer *
>   _mesa_new_renderbuffer(struct gl_context *ctx, GLuint name);
>
>   extern void
>   _mesa_delete_renderbuffer(struct gl_context *ctx, struct gl_renderbuffer *rb);
>
>   extern void
> +_mesa_add_renderbuffer_without_ref(struct gl_framebuffer *fb,
> +                                   gl_buffer_index bufferName,
> +                                   struct gl_renderbuffer *rb);
> +
> +extern void
>   _mesa_add_renderbuffer(struct gl_framebuffer *fb,
>                          gl_buffer_index bufferName, struct gl_renderbuffer *rb);
>
>   extern void
>   _mesa_remove_renderbuffer(struct gl_framebuffer *fb,
>                             gl_buffer_index bufferName);
>
>   extern void
>   _mesa_reference_renderbuffer_(struct gl_renderbuffer **ptr,
>                                 struct gl_renderbuffer *rb);
>