[PATCH 1/2] compositor: call configure on surfaces with a null buffer too

[Pekka Paalanen]

On Tue, 19 Feb 2013 22:22:46 +0100
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> This way the shell can know when a surface has been unmapped by
> checking the value of pending.remove_content. The configure handlers
> must now make sure that the buffer exist though, by checking if
> the surface's buffer_ref.buffer is NULL.

Hi Giulio,

thanks for looking into this.

> ---
>  src/compositor.c |  2 +-
>  src/shell.c      | 33 +++++++++++++++++++++++++++++----
>  2 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 64d0830..ea720cd 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1404,7 +1404,7 @@ surface_commit(struct wl_client *client, struct wl_resource *resource)
>  	if (surface->pending.buffer || surface->pending.remove_contents)
>  		weston_surface_attach(surface, surface->pending.buffer);
>  
> -	if (surface->buffer_ref.buffer && surface->configure)
> +	if (surface->configure)
>  		surface->configure(surface, surface->pending.sx,
>  				   surface->pending.sy);

This causes configure() to be called every wl_surface.commit. Would it
not be better to call configure() only when needed? That is:
- pending.{sx,sy} are not zero, or
- a different sized buffer has been attached, a NULL buffer considered
  as having size 0x0.

I am unsure if we have other cases where configure() must be called,
but these would feel logical. I have a feeling I might be forgetting
something, not had a look on the configuring code for some time now.

Especially, we can avoid calling configure() if a client has not sent
wl_surface.attach() at all since the last commit.

I would also try to change the configure() signature to take the
logical buffer size as arguments. This way the implementations of
configure() would never need surface->buffer_ref.buffer (right?). I also
would not like any configure() implementations looking into any of the
surface->pending.* fields, since those are sort of private to the
commit logic.

Could this work?


Thanks,
pq

>  	surface->pending.sx = 0;
> diff --git a/src/shell.c b/src/shell.c
> index af802a5..e332630 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -2198,6 +2198,9 @@ background_configure(struct weston_surface *es, int32_t sx, int32_t sy)
>  {
>  	struct desktop_shell *shell = es->private;
>  
> +	if (!es->buffer_ref.buffer)
> +		return;
> +
>  	configure_static_surface(es, &shell->background_layer);
>  }
>  
> @@ -2231,6 +2234,9 @@ panel_configure(struct weston_surface *es, int32_t sx, int32_t sy)
>  {
>  	struct desktop_shell *shell = es->private;
>  
> +	if (!es->buffer_ref.buffer)
> +		return;
> +
>  	configure_static_surface(es, &shell->panel_layer);
>  }
>  
> @@ -2264,6 +2270,9 @@ lock_surface_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
>  {
>  	struct desktop_shell *shell = surface->private;
>  
> +	if (!surface->buffer_ref.buffer)
> +		return;
> +
>  	center_on_output(surface, get_default_output(shell->compositor));
>  
>  	if (!weston_surface_is_mapped(surface)) {
> @@ -3088,10 +3097,17 @@ shell_surface_configure(struct weston_surface *es, int32_t sx, int32_t sy)
>  {
>  	struct shell_surface *shsurf = get_shell_surface(es);
>  	struct desktop_shell *shell = shsurf->shell;
> -	int32_t width = weston_surface_buffer_width(es);
> -	int32_t height = weston_surface_buffer_height(es);
> +
> +	int32_t width;
> +	int32_t height;
>  	int type_changed = 0;
>  
> +	if (!es->buffer_ref.buffer)
> +		return;
> +
> +	width = weston_surface_buffer_width(es);
> +	height = weston_surface_buffer_height(es);
> +
>  	if (shsurf->next_type != SHELL_SURFACE_NONE &&
>  	    shsurf->type != shsurf->next_type) {
>  		set_surface_type(shsurf);
> @@ -3208,6 +3224,9 @@ screensaver_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
>  {
>  	struct desktop_shell *shell = surface->private;
>  
> +	if (!surface->buffer_ref.buffer)
> +		return;
> +
>  	/* XXX: starting weston-screensaver beforehand does not work */
>  	if (!shell->locked)
>  		return;
> @@ -3278,10 +3297,16 @@ static void
>  input_panel_configure(struct weston_surface *surface, int32_t sx, int32_t sy)
>  {
>  	struct weston_mode *mode;
> -	int32_t width = weston_surface_buffer_width(surface);
> -	int32_t height = weston_surface_buffer_height(surface);
> +	int32_t width;
> +	int32_t height;
>  	float x, y;
>  
> +	if (!surface->buffer_ref.buffer)
> +		return;
> +
> +	width = weston_surface_buffer_width(surface);
> +	height = weston_surface_buffer_height(surface);
> +
>  	if (!weston_surface_is_mapped(surface))
>  		return;
>