[PATCH weston 2/8] compositor: add and export weston_buffer_reference()

[Kristian Høgsberg]

On Fri, Nov 23, 2012 at 03:56:51PM +0200, Pekka Paalanen wrote:
> The wl_buffer reference counting API has been inconsistent. You would
> manually increment the refcount and register a destroy listener, as
> opposed to calling weston_buffer_post_release(), which internally
> decremented the refcount, and then removing a list item.
> 
> Replace both cases with a single function:
> weston_buffer_reference(wl_buffer **ref, wl_buffer *buffer, listener)
> 
> Buffer is assigned to *ref, while taking care of all the refcounting and
> release posting. You take a reference by passing a non-NULL buffer, and
> release a reference by passing NULL as buffer. Registering and
> de-registering the destroy listener is included.
> 
> This is inspired by the pipe_resource_reference() of Mesa.
> 
> Additionally, when a surface gets destroyed, the associated wl_buffer
> will send a release event. Usually the buffer is already destroyed on
> client side, so the event will be discarded by libwayland-client.
> 
> Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>

I think this is fine, but can we make it a struct
weston_buffer_reference instead, that includes the struct wl_listener
and buffer pointer?  All users just set up a listener that NULLs the
buffer pointers so we should pull that logic into the reference and
reduce the required boilerplate code.

Kristian

> ---
>  src/compositor.c |   31 +++++++++++++++++++++----------
>  src/compositor.h |    4 ++++
>  2 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 3399ad4..c1fc287 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -779,8 +779,8 @@ destroy_surface(struct wl_resource *resource)
>  	if (surface->pending.buffer)
>  		wl_list_remove(&surface->pending.buffer_destroy_listener.link);
>  
> -	if (surface->buffer)
> -		wl_list_remove(&surface->buffer_destroy_listener.link);
> +	weston_buffer_reference(&surface->buffer, NULL,
> +				&surface->buffer_destroy_listener);
>  
>  	pixman_region32_fini(&surface->texture_damage);
>  	compositor->renderer->destroy_surface(surface);
> @@ -808,27 +808,36 @@ weston_surface_destroy(struct weston_surface *surface)
>  	destroy_surface(&surface->surface.resource);
>  }
>  
> -static void
> -weston_surface_attach(struct weston_surface *surface, struct wl_buffer *buffer)
> +WL_EXPORT void
> +weston_buffer_reference(struct wl_buffer **ref, struct wl_buffer *buffer,
> +			struct wl_listener *buffer_destroy_listener)
>  {
> -	if (surface->buffer && buffer != surface->buffer) {
> -		weston_buffer_post_release(surface->buffer);
> -		wl_list_remove(&surface->buffer_destroy_listener.link);
> +	if (*ref && buffer != *ref) {
> +		weston_buffer_post_release(*ref);
> +		wl_list_remove(&buffer_destroy_listener->link);
>  	}
>  
> -	if (buffer && buffer != surface->buffer) {
> +	if (buffer && buffer != *ref) {
>  		buffer->busy_count++;
>  		wl_signal_add(&buffer->resource.destroy_signal,
> -			      &surface->buffer_destroy_listener);
> +			      buffer_destroy_listener);
>  	}
>  
> +	*ref = buffer;
> +}
> +
> +static void
> +weston_surface_attach(struct weston_surface *surface, struct wl_buffer *buffer)
> +{
> +	weston_buffer_reference(&surface->buffer, buffer,
> +				&surface->buffer_destroy_listener);
> +
>  	if (!buffer) {
>  		if (weston_surface_is_mapped(surface))
>  			weston_surface_unmap(surface);
>  	}
>  
>  	surface->compositor->renderer->attach(surface, buffer);
> -	surface->buffer = buffer;
>  }
>  
>  WL_EXPORT void
> @@ -1147,6 +1156,8 @@ surface_attach(struct wl_client *client,
>  	if (buffer_resource)
>  		buffer = buffer_resource->data;
>  
> +	/* Attach, attach, without commit in between does not send
> +	 * wl_buffer.release. */
>  	if (surface->pending.buffer)
>  		wl_list_remove(&surface->pending.buffer_destroy_listener.link);
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index e770664..9809238 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -669,6 +669,10 @@ weston_surface_unmap(struct weston_surface *surface);
>  void
>  weston_buffer_post_release(struct wl_buffer *buffer);
>  
> +void
> +weston_buffer_reference(struct wl_buffer **ref, struct wl_buffer *buffer,
> +			struct wl_listener *buffer_destroy_listener);
> +
>  uint32_t
>  weston_compositor_get_time(void);
>  
> -- 
> 1.7.8.6
>