[PATCH v2] xwm: Update input region on resize
Derek Foreman
derekf at osg.samsung.com
Mon Mar 19 19:13:04 UTC 2018
On 2018-03-16 06:42 PM, Scott Moreau wrote:
> Hi Pekka,
>
> On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen <ppaalanen at gmail.com
> <mailto:ppaalanen at gmail.com>> wrote:
>
> On Tue, 13 Mar 2018 21:22:04 -0600
> Scott Moreau <oreaus at gmail.com <mailto: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.
>
>
> 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?
>
> 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
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
More information about the wayland-devel
mailing list