Re: [PATCH v2] xwm: Update input region on resize

[Derek Foreman]

On 2018-03-28 12:55 AM, Ray, Ian (GE Healthcare) wrote:
> On 27/03/2018, 21.50, "wayland-devel on behalf of Derek Foreman" 
>  der...@osg.samsung.com> 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.)

I can see why it landed, it looks trivially correct to me too. :/

> [*] 
> https://lists.freedesktop.org/archives/wayland-devel/2017-December/036402.html

Thanks for this - I couldn't find the thread because the commit landed
with a revised name.  I wasn't sure how to produce the bug it was
intended to fix.

I've reverted the patch for now.  Hopefully it can be revisited after
release with input regions handled on window resize (as the patch in
this thread intends to do).

Thanks,
Derek

> 
>> Thanks,
>> Derek
>>
>> On 2018-03-19 03:20 PM, Scott Moreau wrote:
>>>
>>>
>>> On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman >> > wrote:
>>>
>>> On 2018-03-16 06:42 PM, Scott Moreau wrote:
>>>
>>> Hi Pekka,
>>>
>>> On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen
>>> 
>>> >> wrote:
>>>
>>> On Tue, 13 Mar 2018 21:22:04 -0600
>>> Scott Moreau 
>>> >> 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 

Re: [PATCH v2] xwm: Update input region on resize

[Ray, Ian (GE Healthcare)]

On 27/03/2018, 21.50, "wayland-devel on behalf of Derek Foreman" 
 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  > > wrote:
> > 
> > On 2018-03-16 06:42 PM, Scott Moreau wrote:
> > 
> > Hi Pekka,
> > 
> > On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen
> > 
> > >> wrote:
> > 
> > On Tue, 13 Mar 2018 21:22:04 -0600
> > Scott Moreau 
> > >> 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(...)
> > 
> 

Re: [PATCH v2] xwm: Update input region on resize

[Derek Foreman]

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).

Thanks,
Derek

On 2018-03-19 03:20 PM, Scott Moreau wrote:
> 
> 
> On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman  > wrote:
> 
> On 2018-03-16 06:42 PM, Scott Moreau wrote:
> 
> Hi Pekka,
> 
> On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen
> 
> >> wrote:
> 
>     On Tue, 13 Mar 2018 21:22:04 -0600
>     Scott Moreau 
> >> 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
>

Re: [PATCH v2] xwm: Update input region on resize

[Scott Moreau]

On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman 
wrote:

> On 2018-03-16 06:42 PM, Scott Moreau wrote:
>
>> Hi Pekka,
>>
>> On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen > > wrote:
>>
>> On Tue, 13 Mar 2018 21:22:04 -0600
>> Scott Moreau > 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,
>>  >  }
>>  

Re: [PATCH v2] xwm: Update input region on resize

[Derek Foreman]

On 2018-03-16 06:42 PM, Scott Moreau wrote:

Hi Pekka,

On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen > wrote:


On Tue, 13 Mar 2018 21:22:04 -0600
Scott Moreau > 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, , , ,
);
 > +
 > +     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. */
 

Re: [PATCH v2] xwm: Update input region on resize

[Scott Moreau]

Hi Pekka,

On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen  wrote:

> On Tue, 13 Mar 2018 21:22:04 -0600
> Scott Moreau  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.
>
>
> 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, , , , );
> > +
> > + 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, );
> > +}
> > +
> > +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);
> > 

Re: [PATCH v2] xwm: Update input region on resize

[Pekka Paalanen]

On Tue, 13 Mar 2018 21:22:04 -0600
Scott Moreau  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.

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.


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?

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. 

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?

That leaves the XWM originated resizes, which boils down to...
send_configure(), or actually weston_wm_window_configure()?

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?

> 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, , , , );
> +
> + 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, );
> +}
> +
> +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?

>  }
>  
>  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)
>>format_rgba,
>width, height);
>  
> - 

[PATCH v2] xwm: Update input region on resize

[Scott Moreau]

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(-)

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, , , , );
+
+   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, );
+}
+
+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);
 }
 
 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)
 >format_rgba,
 width, height);
 
-   weston_wm_window_get_input_rect(window, , , , );
-   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,
-);
-
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);
 }
 
 static void
-- 
2.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel