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

Olivier Fourdan ofourdan at redhat.com
Fri Feb 20 09:05:09 PST 2015


Hi

Sorry, I probably didn't express myself correctly here. What I've been 
trying to say is:

  - The patch does not work for me, it makes things worse than before, 
the X window field is not consistent depending on where the event 
occurs. So it works with some menus, but other mouse events won't work 
anymore.

  - That patch requires a change in Mutter (the cow part), I'd rather 
not require this and adapt the behavior of Xwayland depending if the cow 
is mapped or not (Weston was just an example, there are other Wayland 
compositor out there).

Cheers,
Olivier

On 20/02/15 17:40, Jasper St. Pierre wrote:
>
>
> On Fri, Feb 20, 2015 at 1:47 AM, Olivier Fourdan <ofourdan at redhat.com
> <mailto:ofourdan at redhat.com>> wrote:
>
>     Hi Jasper,
>
>     I was working on the same issue
>     (https://bugzilla.redhat.com/__show_bug.cgi?id=1188289
>     <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
>         <mailto: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



More information about the xorg-devel mailing list