[PATCH weston] compositor: update the transform when attaching a new buffer

[Pekka Paalanen]

On Tue, 10 Dec 2013 15:55:29 +0100
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> if a surface has not a buffer yet and a weston_view gets created for
> it, the surface's width and height will be 0 and the view's
> output_mask will be 0, because the surface's area is 0. Later commits
> on the surface with valid buffers will then trigger a surface
> repaint, which will do nothing because of the output_mask set to 0.
> So by calling weston_update_transform() on the views of the surface
> at the end of weston_surface_commit() we update the output_mask of
> the surface and of the views, so they will be repainted.
> ---
>  src/compositor.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 6beea9c..20d9efb 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2005,7 +2005,6 @@ weston_surface_commit(struct weston_surface
> *surface) surface->pending.buffer = NULL;
>  	surface->pending.sx = 0;
>  	surface->pending.sy = 0;
> -	surface->pending.newly_attached = 0;
>  
>  	/* wl_surface.damage */
>  	pixman_region32_union(&surface->damage, &surface->damage,
> @@ -2046,7 +2045,13 @@ weston_surface_commit(struct weston_surface
> *surface) 
>  	weston_surface_commit_subsurface_order(surface);
>  
> +	if (surface->pending.newly_attached) {
> +		wl_list_for_each(view, &surface->views, surface_link)
> +			weston_view_update_transform(view);
> +	}
>  	weston_surface_schedule_repaint(surface);
> +
> +	surface->pending.newly_attached = 0;
>  }
>  
>  static void

Hi,

Why is this fix needed in the first place, in what use case does this
bug manifest? Is this a regression? What introduced it?

Weren't we relying on the weston_surface->configure call that maps the
surface the first time to enforce a non-zero output_mask?

If you look at all (most?) of the functions in shell.c that actually
map surfaces, starting from an unmapped state, you see that they a) put
the view into a layer, and b) forcefully call
weston_view_update_transform(), which will then set up the output_mask.
Or at least it used to be like that, now the only case I can clearly
see in the code is lock_surface_configure(), but looks like map() with
at least shell_map_popup() works roughly the same.

There are likely also other details that have to be right in the right
order when a surface is mapped, but I forget. Every surface type gets
mapped slightly differently which is why we have not had a generic
weston_surface_map() function.

I don't think calling weston_view_update_transform() from the commit
handler *always* when there is a new attach is right. It should happen
only when mapping a surface. I believe there used to be the idea, that
e.g. input events are generated with the old surface state, until
output_repaint actually puts the new state on screen. Though, is that
idea relevant anymore?

(That actually shows a bug I think, a newly mapped surface may get input
events before it is painted on screen, does it not? No wait, we only do
input /from/ output_repaint? or do we? umm...)


Thanks,
pq