[PATCH weston v2 09/24] xwm: detect legacy fullscreen on MapRequest

[Quentin Glidic]

On 21/12/2016 15:40, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> The legacy fullscreen state needs to be detected at MapRequest time,
> because that is when the X11 client has alredy set up the initial window
> state.
> 
> Doing it at xserver_map_shell_surface() meant that it would be done as a
> response to Xwayland creating the wl_surface and XWM receiving the
> WL_SURFACE_ID ClientMessage, whichever came later. At that point the X11
> client might still be setting things up in theory, though in practice
> most of the X11 communication has already happened when
> xserver_map_shell_surface() gets called.
> 
> The real reason for this is to clean up xserver_map_shell_surface() from
> everything that would affect drawing the decorations. This patch is one
> part of that clean-up.
> 
> The weston_output_weak_ref logic is not put into compositor.h, because
> there are no other users for it at this time. We need to protect against
> the output going away.

Just to be clear: “protect” here means “not segfault”, just passing NULL 
if the output got away in the meantime, right?


> 
> A side-effect of this patch is that saved_width and saved_height will
> now get overwritten also for legacy fullscreen windows. Previously, they
> were left to zero as far as I could tell.
> 
> NOTE: This stops override-redirect legacy fullscreen windows from being
> detected as fullscreen. MapRequest processing does not happen for OR
> windows. These windows get detected as type XWAYLAND instead.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Nice way to do it:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>


> ---
>   xwayland/window-manager.c | 70 +++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 65 insertions(+), 5 deletions(-)
> 
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index 10bb390..02a44a7 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -36,6 +36,7 @@
>   #include <unistd.h>
>   #include <signal.h>
>   #include <limits.h>
> +#include <assert.h>
>   #include <X11/Xcursor/Xcursor.h>
>   #include <linux/input.h>
>   
> @@ -126,6 +127,11 @@ struct motif_wm_hints {
>   #define _NET_WM_MOVERESIZE_MOVE_KEYBOARD    10   /* move via keyboard */
>   #define _NET_WM_MOVERESIZE_CANCEL           11   /* cancel operation */
>   
> +struct weston_output_weak_ref {
> +	struct weston_output *output;
> +	struct wl_listener destroy_listener;
> +};
> +
>   struct weston_wm_window {
>   	struct weston_wm *wm;
>   	xcb_window_t id;
> @@ -152,6 +158,7 @@ struct weston_wm_window {
>   	bool pos_dirty;
>   	int map_request_x;
>   	int map_request_y;
> +	struct weston_output_weak_ref legacy_fullscreen_output;
>   	int saved_width, saved_height;
>   	int decorate;
>   	int override_redirect;
> @@ -174,6 +181,11 @@ weston_wm_set_net_active_window(struct weston_wm *wm, xcb_window_t window);
>   static void
>   weston_wm_window_schedule_repaint(struct weston_wm_window *window);
>   
> +static int
> +legacy_fullscreen(struct weston_wm *wm,
> +		  struct weston_wm_window *window,
> +		  struct weston_output **output_ret);
> +
>   static void
>   xserver_map_shell_surface(struct weston_wm_window *window,
>   			  struct weston_surface *surface);
> @@ -212,6 +224,47 @@ wm_log_continue(const char *fmt, ...)
>   #endif
>   }
>   
> +static void
> +weston_output_weak_ref_init(struct weston_output_weak_ref *ref)
> +{
> +	ref->output = NULL;
> +}
> +
> +static void
> +weston_output_weak_ref_clear(struct weston_output_weak_ref *ref)
> +{
> +	if (!ref->output)
> +		return;
> +
> +	wl_list_remove(&ref->destroy_listener.link);
> +	ref->output = NULL;
> +}
> +
> +static void
> +weston_output_weak_ref_handle_destroy(struct wl_listener *listener, void *data)
> +{
> +	struct weston_output_weak_ref *ref;
> +
> +	ref = wl_container_of(listener, ref, destroy_listener);
> +	assert(ref->output == data);
> +
> +	weston_output_weak_ref_clear(ref);
> +}
> +
> +static void
> +weston_output_weak_ref_set(struct weston_output_weak_ref *ref,
> +			   struct weston_output *output)
> +{
> +	weston_output_weak_ref_clear(ref);
> +
> +	if (!output)
> +		return;
> +
> +	ref->destroy_listener.notify = weston_output_weak_ref_handle_destroy;
> +	wl_signal_add(&output->destroy_signal, &ref->destroy_listener);
> +	ref->output = output;
> +}
> +
>   static bool __attribute__ ((warn_unused_result))
>   wm_lookup_window(struct weston_wm *wm, xcb_window_t hash,
>   		 struct weston_wm_window **window)
> @@ -955,6 +1008,7 @@ weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event)
>   	xcb_map_request_event_t *map_request =
>   		(xcb_map_request_event_t *) event;
>   	struct weston_wm_window *window;
> +	struct weston_output *output;
>   
>   	if (our_resource(wm, map_request->window)) {
>   		wm_log("XCB_MAP_REQUEST (window %d, ours)\n",
> @@ -982,6 +1036,12 @@ weston_wm_handle_map_request(struct weston_wm *wm, xcb_generic_event_t *event)
>   	weston_wm_window_set_net_wm_state(window);
>   	weston_wm_window_set_virtual_desktop(window, 0);
>   
> +	if (legacy_fullscreen(wm, window, &output)) {
> +		window->fullscreen = 1;
> +		weston_output_weak_ref_set(&window->legacy_fullscreen_output,
> +					   output);
> +	}
> +
>   	xcb_map_window(wm->conn, map_request->window);
>   	xcb_map_window(wm->conn, window->frame_id);
>   }
> @@ -1216,6 +1276,7 @@ weston_wm_window_create(struct weston_wm *wm,
>   	window->pos_dirty = false;
>   	window->map_request_x = INT_MIN; /* out of range for valid positions */
>   	window->map_request_y = INT_MIN; /* out of range for valid positions */
> +	weston_output_weak_ref_init(&window->legacy_fullscreen_output);
>   
>   	geometry_reply = xcb_get_geometry_reply(wm->conn, geometry_cookie, NULL);
>   	/* technically we should use XRender and check the visual format's
> @@ -1232,6 +1293,8 @@ weston_wm_window_destroy(struct weston_wm_window *window)
>   {
>   	struct weston_wm *wm = window->wm;
>   
> +	weston_output_weak_ref_clear(&window->legacy_fullscreen_output);
> +
>   	if (window->repaint_source)
>   		wl_event_source_remove(window->repaint_source);
>   	if (window->cairo_surface)
> @@ -2542,7 +2605,6 @@ xserver_map_shell_surface(struct weston_wm_window *window,
>   		wm->server->compositor->xwayland;
>   	const struct weston_desktop_xwayland_interface *xwayland_interface =
>   		wm->server->compositor->xwayland_interface;
> -	struct weston_output *output;
>   	struct weston_wm_window *parent;
>   
>   	weston_wm_window_read_properties(window);
> @@ -2586,11 +2648,9 @@ xserver_map_shell_surface(struct weston_wm_window *window,
>   	if (window->fullscreen) {
>   		window->saved_width = window->width;
>   		window->saved_height = window->height;
> -		xwayland_interface->set_fullscreen(window->shsurf, NULL);
> +		xwayland_interface->set_fullscreen(window->shsurf,
> +						   window->legacy_fullscreen_output.output);
>   		return;
> -	} else if (legacy_fullscreen(wm, window, &output)) {
> -		window->fullscreen = 1;
> -		xwayland_interface->set_fullscreen(window->shsurf, output);
>   	} else if (window->override_redirect) {
>   		xwayland_interface->set_xwayland(window->shsurf,
>   						 window->x, window->y);
> 


-- 

Quentin “Sardem FF7” Glidic