<div dir="ltr">Hi Pekka,<br><div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Tue, 13 Mar 2018 21:22:04 -0600<br>
Scott Moreau <<a href="mailto:oreaus@gmail.com">oreaus@gmail.com</a>> wrote:<br>
<br>
> Commit 332d1892 introduced a bug because the window was<br>
> shaped only when the frame was created, leaving the input<br>
> region unchanged regardless if the window was resized.<br>
> This patch updates the input region shape on resize,<br>
> fixing the problem.<br>
><br>
> ---<br>
><br>
> Changed in v2:<br>
><br>
> - Bail in shape function if (window->override_redirect || !window->frame)<br>
> - Call shape function from send_configure()<br>
><br>
> xwayland/window-manager.c | 53 +++++++++++++++++++++++++++++-<wbr>-----------------<br>
> 1 file changed, 33 insertions(+), 20 deletions(-)<br>
<br>
</span>Hi,<br>
<br>
while trying to wrap my head around this, I started feeling dizzy. For<br>
real. So I have to keep this short.<br></blockquote><div><br></div><div>I think this is what happens when trying to sync two display servers.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The first decision we should make is, do we want a quick patch for an<br>
immediate issue at hand, or do we want to make things better in the long<br>
run. Taking in this patch as is seems to be the former to me, and given<br>
the phase of the release cycle can be justified.<br>
<br>
The following mind flow is for a long term solution, and the comments<br>
inlined with the code below are for the quick patch.<br>
<br>
<br>
Taking a step back, the aim to keep the input shape up-to-date whenever<br>
something happens.<br>
<br>
If we have a frame window with decorations, then we call<br>
frame_resize_inside() to adjust its size. Would it not be logical to<br>
set the input shape in at least all those cases?<br></blockquote><div><br></div><div>Yes, maybe there can be a function that calls frame_resize_inside and the shape function to replace calls to frame_resize_inside.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Except it looks like we can have decorated O-R windows, and those<br>
should be exempt? Oh, I'm told decorated O-R windows don't make sense,<br>
but I see some code in weston that would only apply in such case...<br>
if (window->override_redirect) { ... if (window->frame) frame_resize_inside(...)<br>
<br>
All windows that go through map_request handler will get the frame<br>
window (window->frame_id) and the frame (window->frame) created. The<br>
only windows that don't get this treatment are therefore windows that<br>
are O-R at the time of mapping them. It's somewhat complicated by the<br>
fact that XWM does not support dynamically changing O-R flag... or<br>
maybe it makes it easier.<br>
<br>
Now, we have configure_request and configure_notify handlers. O-R<br>
windows will not hit configure_request handler, and the X server<br>
processes XWM's configure commands immediately. This sounds like<br>
configure_request handler would be a good place to update the input<br>
shape.<br></blockquote><div><br></div><div>Yes <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
configure_notify handler gets called for O-R as well, and it happens<br>
after the fact, so updating there would be a tiny bit late. Would you<br>
agree?<br></blockquote><div>I was thinking there might be some change that comes in configure notify that we don't know about until the event happens. <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
That leaves the XWM originated resizes, which boils down to...<br>
send_configure(), or actually weston_wm_window_configure()?<br></blockquote><div>Yes <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
It looks like configure_request handler is open-coding all of<br>
weston_wm_window_configure() but it also adds some bits specific to the<br>
configure request.<br>
<br>
Am I making sense?<br></blockquote><div>Yes, and thanks for taking the time to try and help unravel this.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div class="h5"><br>
> diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c<br>
> index c307e19..cd72a59 100644<br>
> --- a/xwayland/window-manager.c<br>
> +++ b/xwayland/window-manager.c<br>
> @@ -659,6 +659,33 @@ weston_wm_window_get_input_<wbr>rect(struct weston_wm_window *window,<br>
> }<br>
><br>
> static void<br>
> +weston_wm_window_shape(struct weston_wm_window *window)<br>
> +{<br>
> + struct weston_wm *wm = window->wm;<br>
> + xcb_rectangle_t rect;<br>
> + int x, y, width, height;<br>
> +<br>
> + if (window->override_redirect || !window->frame)<br>
> + return;<br>
> +<br>
> + weston_wm_window_get_input_<wbr>rect(window, &x, &y, &width, &height);<br>
> +<br>
> + rect.x = x;<br>
> + rect.y = y;<br>
> + rect.width = width;<br>
> + rect.height = height;<br>
> +<br>
> + /* The window frame was created with position and size which include<br>
> + * an offset for margins and shadow. Set the input region to ignore<br>
> + * shadow. */<br>
> + xcb_shape_rectangles(wm->conn,<br>
> + XCB_SHAPE_SO_SET,<br>
> + XCB_SHAPE_SK_INPUT,<br>
> + 0, window->frame_id,<br>
> + 0, 0, 1, &rect);<br>
> +}<br>
> +<br>
> +static void<br>
> weston_wm_window_send_<wbr>configure_notify(struct weston_wm_window *window)<br>
> {<br>
> xcb_configure_notify_event_t configure_notify;<br>
> @@ -789,6 +816,8 @@ weston_wm_handle_configure_<wbr>notify(struct weston_wm *wm, xcb_generic_event_t *eve<br>
> xwayland_api->set_xwayland(<wbr>window->shsurf,<br>
> window->x, window->y);<br>
> }<br>
> +<br>
> + weston_wm_window_shape(window)<wbr>;<br>
<br>
</div></div>From Daniel I understood that there would have been a better place to<br>
call this?<br></blockquote><div>I must have misunderstood.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div class="h5"><br>
> }<br>
><br>
> static void<br>
> @@ -983,7 +1012,6 @@ weston_wm_window_create_frame(<wbr>struct weston_wm_window *window)<br>
> {<br>
> struct weston_wm *wm = window->wm;<br>
> uint32_t values[3];<br>
> - xcb_rectangle_t rect;<br>
> int x, y, width, height;<br>
> int buttons = FRAME_BUTTON_CLOSE;<br>
><br>
> @@ -1040,26 +1068,9 @@ weston_wm_window_create_frame(<wbr>struct weston_wm_window *window)<br>
> &wm->format_rgba,<br>
> width, height);<br>
><br>
> - weston_wm_window_get_input_<wbr>rect(window, &x, &y, &width, &height);<br>
> - rect.x = x;<br>
> - rect.y = y;<br>
> - rect.width = width;<br>
> - rect.height = height;<br>
> -<br>
> - /* The window frame was created with position and size which include<br>
> - * an offset for margins and shadow. Set the input region to ignore<br>
> - * shadow. */<br>
> - xcb_shape_rectangles(wm->conn,<br>
> - XCB_SHAPE_SO_SET,<br>
> - XCB_SHAPE_SK_INPUT,<br>
> - 0,<br>
> - window->frame_id,<br>
> - 0,<br>
> - 0,<br>
> - 1,<br>
> - &rect);<br>
> -<br>
> hash_table_insert(wm->window_<wbr>hash, window->frame_id, window);<br>
> +<br>
> + weston_wm_window_shape(window)<wbr>;<br>
> }<br>
><br>
> /*<br>
> @@ -2726,6 +2737,8 @@ send_configure(struct weston_surface *surface, int32_t width, int32_t height)<br>
> window->configure_source =<br>
> wl_event_loop_add_idle(wm-><wbr>server->loop,<br>
> weston_wm_window_configure, window);<br>
> +<br>
> + weston_wm_window_shape(window)<wbr>;<br>
<br>
</div></div>This means we do the shaping immediately, but the configure is postponed<br>
to the idle handler. Shouldn't those go together?<br></blockquote><div>Yes but what are you proposing to do to make them go together, call the shape function from weston_wm_window_configure() instead?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
OTOH, weston_wm_window_configure() is also called from<br>
weston_mw_window_set_toplevel(<wbr>). To me it seems it would be appropriate<br>
to call weston_wm_window_shape() from weston_wm_window_configure(). Is it?<br>
<br>
> }<br>
><br>
> static void<br>
<br>
<br>
Thanks,<br>
pq<br></blockquote><div><br></div><div>Thanks,<br><br></div><div>Scott <br></div></div><br></div></div></div>