Hey Carlos,
----- Original Message -----
> The checks in xwayland's XYToWindow handler pretty much assumes that the
> sprite is managed by the wl_pointer, which is not entirely right, given
> 1) The Virtual Core Pointer may be controlled from other interfaces, and
> 2) there may be other SpriteRecs than the VCP's.
>
> This makes XYToWindow calls return a sprite trace with just the root
> window if any of those two assumptions are broken, eg. on touch events.
>
> So turn the check upside down, first assume that the default XYToWindow
> proc behavior is right, and later cut down the spriteTrace if the
> current device happens to be the pointer. We work our way to the
> device's lastSlave here so as not to break assumption #1 above.
>
> Also, simplify the actual nested call to XYToWindow, the method pointer
> in ScreenPtr was reinstaurated before calling, and moved back to
> xwl_screen domain afterwards. This seems kind of pointless, as we have
> the function pointer anyway.
>
> Signed-off-by: Carlos Garnacho <carl...@gnome.org>
> ---
> hw/xwayland/xwayland-input.c | 64
> ++++++++++++++++++++++++++------------------
> 1 file changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 32cfb35..9b385dc 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -945,43 +945,55 @@ DDXRingBell(int volume, int pitch, int duration)
> {
> }
>
> -static WindowPtr
> -xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite, int x, int y)
> +static DeviceIntPtr
> +sprite_get_last_active_device(SpritePtr sprite, struct xwl_seat **xwl_seat)
> {
> - struct xwl_seat *xwl_seat = NULL;
> DeviceIntPtr device;
> - WindowPtr ret;
>
> for (device = inputInfo.devices; device; device = device->next) {
> - if (device->deviceProc == xwl_pointer_proc &&
> - device->spriteInfo->sprite == sprite) {
> - xwl_seat = device->public.devicePrivate;
> + /* Ignore non-wayland devices */
> + if (device->deviceProc != xwl_pointer_proc &&
> + device->deviceProc != xwl_touch_proc &&
> + device->deviceProc != xwl_keyboard_proc)
> + continue;
> +
> + if (device->spriteInfo->sprite == sprite)
> break;
> - }
> }
>
> - if (xwl_seat == NULL) {
> - sprite->spriteTraceGood = 1;
> - return sprite->spriteTrace[0];
> - }
> + if (!device || IsFloating(device))
> + return NULL;
>
> - screen->XYToWindow = xwl_seat->xwl_screen->XYToWindow;
> - ret = screen->XYToWindow(screen, sprite, x, y);
> - xwl_seat->xwl_screen->XYToWindow = screen->XYToWindow;
> - screen->XYToWindow = xwl_xy_to_window;
That wrapping is on purpose, see Daniel's review on my initial patch here:
https://lists.x.org/archives/xorg-devel/2016-June/050238.html
> + *xwl_seat = device->public.devicePrivate;
>
> - /* If the pointer has left the Wayland surface but the DIX still
> - * finds the pointer within the previous X11 window, it means that
> - * the pointer has crossed to another native Wayland window, in this
> - * case, pretend we entered the root window so that a LeaveNotify
> - * event is emitted.
> + /* We do want the last active slave, we only check on slave xwayland
> devices
> + * so we can find out the xwl_seat, but those don't actually own their
> + * sprite, so the match doesn't mean a lot.
> */
> - if (xwl_seat->focus_window == NULL && xwl_seat->last_xwindow == ret) {
> - sprite->spriteTraceGood = 1;
> - return sprite->spriteTrace[0];
> - }
> + return device->master->lastSlave;
I wonder, would it make sense to use the GetMaster() API here or else check if
master is actually non-null?
(note, I tried to detach the xwayland-pointer (with xinput float) from its
master, it doesn't crash because we actually get no more events from Xwayland
for this device after that)
> +}
>
> - xwl_seat->last_xwindow = ret;
> +static WindowPtr
> +xwl_xy_to_window(ScreenPtr screen, SpritePtr sprite, int x, int y)
> +{
> + struct xwl_seat *xwl_seat = NULL;
> + struct xwl_screen *xwl_screen;
> + DeviceIntPtr device;
> + WindowPtr ret;
> +
> + xwl_screen = xwl_screen_get(screen);
> + ret = xwl_screen->XYToWindow(screen, sprite, x, y);
> +
> + device = sprite_get_last_active_device(sprite, &xwl_seat);
Do you plan to reuse that sprite_get_last_active_device() elsewhere?
If not, the whole logic below (device && xwl_seat && xwl_seat->pointer ==
device) could be moved to the function that would return either the right
xwl_seat or NULL, as we don't actually use the value of device here, apart from
this test.
> + if (device && xwl_seat && xwl_seat->pointer == device) {
> + if (xwl_seat->focus_window == NULL && xwl_seat->last_xwindow == ret)
> {
> + sprite->spriteTraceGood = 1;
> + return sprite->spriteTrace[0];
> + }
> +
> + xwl_seat->last_xwindow = ret;
> + }
>
> return ret;
> }
> --
> 2.10.0
Cheers,
Olivier
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
