[PATCH weston 6/6] libweston-desktop/xwayland: add is_mapped handling for XWAYLAND

[Quentin Glidic]

On 24/11/2016 12:40, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> The xwayland window type XWAYLAND is not handled by the shell at all,
> instead libweston-desktop maps such surfaces itself. However, it forgot
> to set weston_surface::is_mapped and weston_view::is_mapped.
>
> weston_surface::is_mapped affects the behaviour of weston_view_unmap()
> and weston_surface_attach().
>
> weston_view::is_mapped affects the behaviour of weston_view_unmap() and
> weston_view_destroy().
>
> When manually mapping a window of type XWAYLAND, mark both the view and
> surface as mapped. This follows the expections in libweston, even though
> the meaning of is_mapped is not clearly defined for either surface or
> view.
>
> Also, when the XWAYLAND window is manually unmapped, unmap the
> weston_surface. This updates weston_surface::is_mapped to reflect the
> state. However, as a side-effect it will also unmap all sibling views,
> should any exist.
>
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
>  libweston-desktop/xwayland.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/libweston-desktop/xwayland.c b/libweston-desktop/xwayland.c
> index 7b8f6db..6fb566a 100644
> --- a/libweston-desktop/xwayland.c
> +++ b/libweston-desktop/xwayland.c
> @@ -71,6 +71,7 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
>  					     struct weston_desktop_surface *parent,
>  					     int32_t x, int32_t y)
>  {
> +	struct weston_surface *surface_base;
>  	bool to_add = (parent == NULL && state != XWAYLAND);
>
>  	assert(state != NONE);
> @@ -81,6 +82,8 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
>  		return;
>  	}
>
> +	surface_base = weston_desktop_surface_get_surface(surface->surface);

The convention in libweston-desktop is to use "wsurface" for 
weston_surface (and "dsurface" for weston_desktop_surface in 
libweston-desktop shells).
And why didn’t you assign the variable directly?


> +
>  	if (surface->state != state) {
>  		if (surface->state == XWAYLAND) {
>  			assert(!surface->added);
> @@ -88,6 +91,7 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
>  			weston_desktop_surface_unlink_view(surface->view);
>  			weston_view_destroy(surface->view);
>  			surface->view = NULL;
> +			weston_surface_unmap(surface_base);

If we define better what “mapped” means, maybe it would make sense to 
unmap a surface with no view?


Anyway, it is a good thing to do:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>

Cheers,


>  		}
>
>  		if (to_add) {
> @@ -109,6 +113,8 @@ weston_desktop_xwayland_surface_change_state(struct weston_desktop_xwayland_surf
>  			weston_layer_entry_insert(&surface->xwayland->layer.view_list,
>  						  &surface->view->layer_link);
>  			weston_view_set_position(surface->view, x, y);
> +			surface->view->is_mapped = true;
> +			surface_base->is_mapped = true;
>  		}
>
>  		surface->state = state;
>


-- 

Quentin “Sardem FF7” Glidic