[PATCH v2] xwm: Update input region on resize

[Pekka Paalanen]

On Tue, 13 Mar 2018 21:22:04 -0600
Scott Moreau <oreaus at gmail.com> wrote:

> Commit 332d1892 introduced a bug because the window was
> shaped only when the frame was created, leaving the input
> region unchanged regardless if the window was resized.
> This patch updates the input region shape on resize,
> fixing the problem.
> 
> ---
> 
> Changed in v2:
> 
> - Bail in shape function if (window->override_redirect || !window->frame)
> - Call shape function from send_configure()
> 
>  xwayland/window-manager.c | 53 +++++++++++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 20 deletions(-)

Hi,

while trying to wrap my head around this, I started feeling dizzy. For
real. So I have to keep this short.

The first decision we should make is, do we want a quick patch for an
immediate issue at hand, or do we want to make things better in the long
run. Taking in this patch as is seems to be the former to me, and given
the phase of the release cycle can be justified.

The following mind flow is for a long term solution, and the comments
inlined with the code below are for the quick patch.


Taking a step back, the aim to keep the input shape up-to-date whenever
something happens.

If we have a frame window with decorations, then we call
frame_resize_inside() to adjust its size. Would it not be logical to
set the input shape in at least all those cases?

Except it looks like we can have decorated O-R windows, and those
should be exempt? Oh, I'm told decorated O-R windows don't make sense,
but I see some code in weston that would only apply in such case...
if (window->override_redirect) { ... if (window->frame) frame_resize_inside(...)

All windows that go through map_request handler will get the frame
window (window->frame_id) and the frame (window->frame) created. The
only windows that don't get this treatment are therefore windows that
are O-R at the time of mapping them. It's somewhat complicated by the
fact that XWM does not support dynamically changing O-R flag... or
maybe it makes it easier.

Now, we have configure_request and configure_notify handlers. O-R
windows will not hit configure_request handler, and the X server
processes XWM's configure commands immediately. This sounds like
configure_request handler would be a good place to update the input
shape. 

configure_notify handler gets called for O-R as well, and it happens
after the fact, so updating there would be a tiny bit late. Would you
agree?

That leaves the XWM originated resizes, which boils down to...
send_configure(), or actually weston_wm_window_configure()?

It looks like configure_request handler is open-coding all of
weston_wm_window_configure() but it also adds some bits specific to the
configure request.

Am I making sense?

> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
> index c307e19..cd72a59 100644
> --- a/xwayland/window-manager.c
> +++ b/xwayland/window-manager.c
> @@ -659,6 +659,33 @@ weston_wm_window_get_input_rect(struct weston_wm_window *window,
>  }
>  
>  static void
> +weston_wm_window_shape(struct weston_wm_window *window)
> +{
> +	struct weston_wm *wm = window->wm;
> +	xcb_rectangle_t rect;
> +	int x, y, width, height;
> +
> +	if (window->override_redirect || !window->frame)
> +		return;
> +
> +	weston_wm_window_get_input_rect(window, &x, &y, &width, &height);
> +
> +	rect.x = x;
> +	rect.y = y;
> +	rect.width = width;
> +	rect.height = height;
> +
> +	/* The window frame was created with position and size which include
> +	 * an offset for margins and shadow. Set the input region to ignore
> +	 * shadow. */
> +	xcb_shape_rectangles(wm->conn,
> +			     XCB_SHAPE_SO_SET,
> +			     XCB_SHAPE_SK_INPUT,
> +			     0, window->frame_id,
> +			     0, 0, 1, &rect);
> +}
> +
> +static void
>  weston_wm_window_send_configure_notify(struct weston_wm_window *window)
>  {
>  	xcb_configure_notify_event_t configure_notify;
> @@ -789,6 +816,8 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, xcb_generic_event_t *eve
>  			xwayland_api->set_xwayland(window->shsurf,
>  						   window->x, window->y);
>  	}
> +
> +	weston_wm_window_shape(window);

From Daniel I understood that there would have been a better place to
call this?

>  }
>  
>  static void
> @@ -983,7 +1012,6 @@ weston_wm_window_create_frame(struct weston_wm_window *window)
>  {
>  	struct weston_wm *wm = window->wm;
>  	uint32_t values[3];
> -	xcb_rectangle_t rect;
>  	int x, y, width, height;
>  	int buttons = FRAME_BUTTON_CLOSE;
>  
> @@ -1040,26 +1068,9 @@ weston_wm_window_create_frame(struct weston_wm_window *window)
>  							     &wm->format_rgba,
>  							     width, height);
>  
> -	weston_wm_window_get_input_rect(window, &x, &y, &width, &height);
> -	rect.x = x;
> -	rect.y = y;
> -	rect.width = width;
> -	rect.height = height;
> -
> -	/* The window frame was created with position and size which include
> -	 * an offset for margins and shadow. Set the input region to ignore
> -	 * shadow. */
> -	xcb_shape_rectangles(wm->conn,
> -			     XCB_SHAPE_SO_SET,
> -			     XCB_SHAPE_SK_INPUT,
> -			     0,
> -			     window->frame_id,
> -			     0,
> -			     0,
> -			     1,
> -			     &rect);
> -
>  	hash_table_insert(wm->window_hash, window->frame_id, window);
> +
> +	weston_wm_window_shape(window);
>  }
>  
>  /*
> @@ -2726,6 +2737,8 @@ send_configure(struct weston_surface *surface, int32_t width, int32_t height)
>  	window->configure_source =
>  		wl_event_loop_add_idle(wm->server->loop,
>  				       weston_wm_window_configure, window);
> +
> +	weston_wm_window_shape(window);

This means we do the shaping immediately, but the configure is postponed
to the idle handler. Shouldn't those go together?

OTOH, weston_wm_window_configure() is also called from
weston_mw_window_set_toplevel(). To me it seems it would be appropriate
to call weston_wm_window_shape() from weston_wm_window_configure(). Is it?

>  }
>  
>  static void


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180316/e06a0cbe/attachment.sig>