<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman <<a href="mailto:derekf@osg.samsung.com" target="_blank">derekf@osg.samsung.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2018-03-16 06:42 PM, Scott Moreau wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Hi Pekka, On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a> <mailto:<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>>> wrote: On Tue, 13 Mar 2018 21:22:04 -0600 Scott Moreau <<a href="mailto:oreaus@gmail.com" target="_blank">oreaus@gmail.com</a> <mailto:<a href="mailto:oreaus@gmail.com" target="_blank">oreaus@gmail.com</a>>> 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. I think this is what happens when trying to sync two display servers. 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. </blockquote> FWIW, I'd be open to landing something quick at this point. The bug it fixes seems hugely annoying. I resize an xterm, and I can't click in the new area. Alternately, I'm wondering if we should consider reverting the patch that introduced this bug? I guess it wasn't tested well enough, and this behaviour seems more painful than what it was intended to fix?</blockquote><div> </div><div>I think a revert would be a good place to start. The patch also has whitespace that does not match surrounding code. </div><div>To clarify the bug, start an xwayland window and resize it to a larger dimension. Mouse input will only work for the size of the window before resize. The rest of the window does not respond to input. </div><div>Thanks, </div><div>Scott </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"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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? Yes, maybe there can be a function that calls frame_resize_inside and the shape function to replace calls to frame_resize_inside. 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. Yes 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? I was thinking there might be some change that comes in configure notify that we don't know about until the event happens. That leaves the XWM originated resizes, which boils down to... send_configure(), or actually weston_wm_window_configure()? Yes 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? Yes, and thanks for taking the time to try and help unravel this. > 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? I must have misunderstood. </blockquote> </div></div> Since this thread's been quiet for a couple of days, I'll ask: where should this have gone? :) What needs to be resolved to move forward on this? Would rather not get to a weston final with this bug present. Thanks, Derek <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > } > > 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? Yes but what are you proposing to do to make them go together, call the shape function from weston_wm_window_configure() instead? 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 Thanks, Scott </div></div> _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org" target="_blank">wayland-devel@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/wayland-devel" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote> </blockquote></div> </div></div>