On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman <[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]>> wrote: >> >> On Tue, 13 Mar 2018 21:22:04 -0600 >> Scott Moreau <[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 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 >> >> >> >> _______________________________________________ >> 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
