[PATCH v2] xwm: Update input region on resize
Scott Moreau
oreaus at gmail.com
Mon Mar 19 20:20:03 UTC 2018
On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman <derekf at osg.samsung.com>
wrote:
> 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?
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
>> wayland-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180319/1e2d3339/attachment-0001.html>
More information about the wayland-devel
mailing list