On 27/03/2018, 21.50, "wayland-devel on behalf of Derek Foreman" <[email protected] on behalf of [email protected]> wrote: > > Hey everyone, > > I've added Ian Ray to CC as the author of commit 332d1892bbb (xwm: do > not include shadow in input region) > > I think at this point reverting that patch for release is our most > sensible move. We don't seem to have much forward progress on the > horrible bug it's introduced. (Resizing an xwayland client doesn't > update input region) > > If there are no fixes forthcoming I'd like to revert commit 332d1892bbb > before RC1 (which is still scheduled for release on Monday, April 2).
Agreed. (FWIW, my submission* was marked RFC, and as such was not as thoroughly tested as it should have been.) [*] https://lists.freedesktop.org/archives/wayland-devel/2017-December/036402.html > Thanks, > Derek > > On 2018-03-19 03:20 PM, Scott Moreau wrote: > > > > > > On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman <[email protected] > > <mailto:[email protected]>> wrote: > > > > On 2018-03-16 06:42 PM, Scott Moreau wrote: > > > > Hi Pekka, > > > > On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen > > <[email protected] <mailto:[email protected]> > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > On Tue, 13 Mar 2018 21:22:04 -0600 > > Scott Moreau <[email protected] <mailto:[email protected]> > > <mailto:[email protected] <mailto:[email protected]>>> 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. > > > > > > 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? > > > > > > I think a revert would be a good place to start. The patch also has > > whitespace that does not match surrounding code. > > > > 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. > > > > Thanks, > > Scott > > > > > > > > > > > > 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. > > > > > > 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 > > > > > > > } > > > > > > 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 w edo 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 > > > > > > > > _______________________________________________ > > wayland-devel mailing list > > [email protected] > > <mailto:[email protected]> > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > > <https://lists.freedesktop.org/mailman/listinfo/wayland-devel> > > > > > > > > > > > > _______________________________________________ > > wayland-devel mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > > > > _______________________________________________ > wayland-devel mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
