[PATCH] xwayland-input: Don't assume all input is children of the focused window

[Jasper St. Pierre]

On Fri, Feb 20, 2015 at 1:47 AM, Olivier Fourdan <ofourdan at redhat.com>
wrote:

> Hi Jasper,
>
> I was working on the same issue (https://bugzilla.redhat.com/
> show_bug.cgi?id=1188289) but did not think about translating the
> coordinates!
>
> I applied your patch along with the patch for mutter (on mutter 3.14.3 as
> found in F21) not to map the cow but that does not work very well.
>
> It works, as long as the window is not moved, as soon as the toplevel
> window is moved, the translated coordinates seem correct but the window
> returned by miSpriteTrace() is not, e.g. I get the "mutter guard window"
> instead of the actual expected window.
>
> Oddly enough the coordinates of the toplevel xwindow (as seen with xprop)
> are not updated after the move, so that miSpriteTrace() picks the wrong
> window - Maybe I am missing some other patch in mutter, I'll check that.
>

Hm, that's strange. We should be configuring the toplevel window on every
move -- mutter still is an X11 WM. In fact, for the client to map the
combobox in the right place, the toplevel coordinates have to be correct.


> That being said, I don't think it's a good idea to make a disruptive
> change in Xwayland that requires code change in the Wayland compositors,
> what about Weston or the other Wayland compositors out there, we can't
> really expect all of them to change their code in a synchronously with
> Xwayland.
>

We don't require any changes in Weston, from what I can tell. The window
still has to be configured in the correct place for the O-R window to be
put in that location. If Weston doesn't configure the window, behavior will
be incorrect.


> We discussed that with ajax and halfline and the the idea would be rather
> to check if the cow exists and is mapped (e.g. as in mutter before your
> patch) then use the current code, otherwise, translate the coordinates as
> you did.
>

That doesn't make much sense to me. The issue we have is that the combobox
window isn't ever a child of the focused window, it's a separate O-R child
of the root. As such, we can't *ever* trace down from the focused window.


> i.e. something like that:
>
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -32,6 +32,9 @@
>  #include <xkbsrv.h>
>  #include <xserver-properties.h>
>  #include <inpututils.h>
> +#ifdef COMPOSITE
> +#include "compint.h"
> +#endif
>
>  static void
>  xwl_pointer_control(DeviceIntPtr device, PtrCtrl *ctrl)
> @@ -628,8 +631,25 @@ xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite,
> int x, int y)
>      }
>
>      if (xwl_seat->focus_window) {
> -        sprite->spriteTraceGood = 2;
> -        sprite->spriteTrace[1] = xwl_seat->focus_window->window;
> +#ifdef COMPOSITE
> +        CompScreenPtr cs;
> +
> +        /* The compositor may have mapped the COW.
> +         * If this is the case, translating to the root coordinates to
> search
> +         * for the window will give us the COW which we don't want, so
> use the
> +         * seat's focused window with relative coordinates instead.
> +         */
> +        cs = GetCompScreen(screen);
> +        if (cs->pOverlayWin && cs->pOverlayWin->mapped) {
> +           sprite->spriteTraceGood = 2;
> +           sprite->spriteTrace[1] = xwl_seat->focus_window->window;
> +           return miSpriteTrace(sprite, x, y);
> +        }
> +#endif
> +        /* convert to root coordinates */
> +        x += xwl_seat->focus_window->window->drawable.x;
> +        y += xwl_seat->focus_window->window->drawable.y;
> +        sprite->spriteTraceGood = 1;
>          return miSpriteTrace(sprite, x, y);
>      }
>      else {
>
>
> But we still need to make sure it works reliably with translated
> coordinates.
>
> Cheers,
> Olivier
>
>
> On 18/02/15 21:41, Jasper St. Pierre wrote:
>
>> Some toolkits implement comboboxes or menus or other things through
>> O-R windows, which aren't children of the focused window. In order to
>> properly get input on them during grab conditions, we need to trace the
>> whole input tree, not just the focused window down.
>>
>> Signed-off-by: Jasper St. Pierre <jstpierre at mecheye.net>
>> ---
>>   hw/xwayland/xwayland-input.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
>> index 5e20418..8b96bd2 100644
>> --- a/hw/xwayland/xwayland-input.c
>> +++ b/hw/xwayland/xwayland-input.c
>> @@ -654,8 +654,10 @@ xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite,
>> int x, int y)
>>       }
>>
>>       if (xwl_seat->focus_window) {
>> -        sprite->spriteTraceGood = 2;
>> -        sprite->spriteTrace[1] = xwl_seat->focus_window->window;
>> +        /* convert to root coordinates */
>> +        x += xwl_seat->focus_window->window->drawable.x;
>> +        y += xwl_seat->focus_window->window->drawable.y;
>> +        sprite->spriteTraceGood = 1;
>>           return miSpriteTrace(sprite, x, y);
>>       }
>>       else {
>>
>>
>


-- 
  Jasper
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150220/affd0150/attachment-0001.html>