Mouse is not grabbed on Wayland

[vanfanel]

Description

-Moving the mouse cursor in RetroArch/RGUI in fullscreen mode with a 4:3 video configuration on a 16:9 display, without any core, the cursor is "sticky" on the four borders of the screen ("sticky" cursor on the borders is typically caused by the cursor confinement not working at all, so mouse coorditantes are not limited to the screen area, but the cursor is).

-Using cores that rely on mouse grabbing, like DosBOX-CORE, on Wayland, results on cursor being confinated in an sub-area of the screen until moving the mouse around the invisible limits allow full movement.

All this seems to be because of missing mouse grab feature on the Wayland backend.

Expected behavior

-Cursor should not be "sticky" in the boders in RetroArch/RGUI.
-Cursor should not be initially trapped in an invisible sub-area in the mouse-enabled cores.

Actual behavior

What I described already.

Steps to reproduce the bug

1. Load RetroAch with the RGUI menu driver and the Wayland display driver, with fullscreen video and 4:3 AR on a 16:9 display.
2. Try to move the cursor around, near the corners if the screen. You will see it's sticky when it goes "into" the borders of the RGUI menu area.
3. Try to activate mouse grabbing (F11) or enable auto-mouse grabbing on the input options, to see that the option just doesn't do anything on Wayland.

OR:

1. Load RetroAch and the Dosbox-core core with the Wayland display driver, with fullscreen video and 4:3 AR on a 16:9 display.
2. Try to move the cursor around in any game using mouse in DOS (Ishar, Lemmings, etc)
3. Try to activate mouse grabbing (F11) or enable auto-mouse grabbing on the input options, to see that the option just doesn't do anything on Wayland.

Bisect Results

Has always happened. It's a missing feature of the Wayland backend.

Version/Commit

• RetroArch: Latest GIT code, commit 4101d81 as of today.

Environment information

• OS: GNU/Linux aarch64 & x86_64
• Compiler: gcc 11.3.0

[vanfanel]

More information: the issue has been discussed here:

labwc/labwc#809 (comment)

..And here:

realnc/dosbox-core#48 (comment)

The place to implement mouse grab in the Wayland driver would be:
https://github.com/libretro/RetroArch/blob/master/input/drivers/wayland_input.c#L410

It seems that functions zwp_pointer_constraints_v1_confine_pointer or zwp_pointer_constraints_v1_lock_pointer should be used to implement mouse grab in the Wayland driver.

However, they are undefined reference if I try to call them. I can't see a single mention of those functions in the RetroArch codebase (these are dynamically generated functions, they shoud be somewhere in https://github.com/libretro/RetroArch/tree/master/deps/wayland-protocols, if I understand it well)

[gouchi]

@Sunderland93 Might have a hint ? ;)

[LibretroAdmin]

@ColinKinloch Would you have any interest in looking into this?

[ColinKinloch]

#15071 Adds the functions for the relevant wayland protocols.
Looks like you need to add a listener for zwp_relative_pointer_v1:event:relative_motion and lock the cursor with zwp_pointer_constraints_v1::lock_pointer as @vanfanel mentioned.

[LibretroAdmin]

This has been merged now. @vanfanel Can you let us know what the current status is now?

[vanfanel]

@LibretroAdmin Hi! This is on its way to be solved I guess, but it's still not.
What happens is this: Now, with commit #15071 the needed functions are accesible from https://github.com/libretro/RetroArch/blob/master/input/drivers/wayland_input.c#L410

...but as you can see, they still must be called in order to solve this.

@ColinKinloch Do you have an idea on what should be done with these functions now? An example or something?

[ColinKinloch]

wlroots has a pretty simple example it seems:

• Get the protocol handles from the registry here.
• Add a listener for relative pointer motion here and here.
• Lock pointer and release it by calling zwp_locked_pointer_v1_destroy

[vanfanel]

@ColinKinloch Those links have the information I needed to start, thanks a lot!

However, I am having serious trouble adapting relative_pointer_handle_relative_motion to RetroArch in input/common/wayland_common.c

To begin with: Where shoud relative_pointer_handle_relative_motion be implemented? In gfx/drivers_context/wayland_ctx.c maybe? That's where the wayland EGL stuff is accesible...

[ColinKinloch]

I think you want to implement zwp_relative_pointer_v1_listener to set wl->mouse.delta_x alongside the other listeners in input/common/wayland_common.c, then update input_wl_poll to not modify wl->mouse.delta_x.
It looks like input_wl_state is already set up to report relative mouse motion to retroarch.

[vanfanel]

@ColinKinloch Oh, there's still something I don't understand. If I add, let's say, wl_pointer_handle_relative_motion along with the other listeners here:

RetroArch/input/common/wayland_common.c

Lines 931 to 937 in 92dfc80

	const struct wl_pointer_listener pointer_listener = {
	wl_pointer_handle_enter,
	wl_pointer_handle_leave,
	wl_pointer_handle_motion,
	wl_pointer_handle_button,
	wl_pointer_handle_axis,
	};

...and proto-implement it like this:

struct zwp_relative_pointer_v1_listener wl_pointer_handle_relative_motion = {
        .relative_motion = handle_relative_motion,
};

...I get this compilation error:

input/common/wayland_common.c:985:4: error: incompatible types when initializing type 'void (*)(void *, struct wl_pointer *, uint32_t,  uint32_t,  uint32_t,  uint32_t)' {aka 'void (*)(void *, struct wl_pointer *, unsigned int,  unsigned int,  unsigned int,  unsigned int)'} using type 'struct zwp_relative_pointer_v1_listener'
  985 |    wl_pointer_handle_relative_motion,
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So, any idea on how should this be done? Trying my best here, really, sorry if I sound a bit dumb.

[ColinKinloch]

No problem at all.
Your function signature for handle_relative_motion should look like this:

static void handle_relative_motion(void *data,
   struct zwp_relative_pointer_v1 *zwp_relative_pointer_v1,
   uint32_t utime_hi, uint32_t utime_lo,
   wl_fixed_t dx, wl_fixed_t dy,
   wl_fixed_t dx_unaccel, wl_fixed_t dy_unaccel)
{
    gfx_ctx_wayland_data_t *wl = (gfx_ctx_wayland_data_t*)data;
}

and I think the listener should be const and maybe call it relative_pointer_listener like this:

const struct zwp_relative_pointer_v1_listener relative_pointer_listener = {
        .relative_motion = handle_relative_motion,
};

[vanfanel]

@ColinKinloch Thanks for your patience.
After using your exact handle_relative_motion implementation, and adding relative_pointer_listener along with the other listeners in const struct wl_pointer_listener pointer_listener
...I still get the same error:

CC input/common/wayland_common.c
input/common/wayland_common.c:967:4: error: incompatible types when initializing type 'void (*)(void *, struct wl_pointer *, uint32_t,  uint32_t,  uint32_t,  uint32_t)' {aka 'void (*)(void *, struct wl_pointer *, unsigned int,  unsigned int,  unsigned int,  unsigned int)'} using type 'const struct zwp_relative_pointer_v1_listener'
  967 |    relative_pointer_listener,
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~

[ColinKinloch]

What's the code surrounding input/common/wayland_common.c line 967 referenced in that error?
The fact the second parameter of the function signature suggests that you're adding the handler to the wl_pointer_listener listener. zwp_relative_pointer_v1_listener is it's own thing.

You get a relative pointer handle from a pointer handle like this.

[vanfanel]

@ColinKinloch The sorrounding code is this:

const struct wl_pointer_listener pointer_listener = {
   wl_pointer_handle_enter,
   wl_pointer_handle_leave,
   wl_pointer_handle_motion,
   //MAC
   relative_pointer_listener,
   //// 
   wl_pointer_handle_button,
   wl_pointer_handle_axis,
};

So, I should not add relative_pointer_listener to wl_pointer_listener pointer_listener?
Then, adding relative_pointer_listener to zwp_relative_pointer_v1_listener is enough?

Mind you, I don't have much idea on what am I doing here because I don't understand Wayland's architecture very well. Maybe you can simply send a PR for this? You seem to have a very good understanding of all this stuff.

[vanfanel]

@ColinKinloch In case you still want me to do it:

1. What would be the contents of static void handle_relative_motion()?
   I currently have this only:
   gfx_ctx_wayland_data_t *wl = (gfx_ctx_wayland_data_t*)data;
   ...But It don't know what do do there, other than recover the wayland context data pointer.

2. Where should I be recovering relative pointer handle from a pointer handle like there?
   My guess is that I should do it in wl_pointer_handle_button()?

[ColinKinloch]

If you're enjoying implementing it I'm happy to give pointers.
wayland.app is a pretty handy reference for wayland protocols.

1. In handle_relative_motion you want to set wl->mouse.delta_x to dx using wl_fixed_to_int to convert it to an int
2. I'm not sure where to create them. It could be that you create one zwp_relative_pointer_v1 for every wl_pointer as they are created here. Alternatively it could be they should be created / destroyed when the pointer is locked / unlocked.

Do you know how retroarch triggers pointer lock? Does it send an event to the input or gfx driver?
F11 or scroll sock apparently.

[vanfanel]

@ColinKinloch There's this callback function that gets called when pointer is grabbed:

RetroArch/input/drivers/wayland_input.c

Line 410 in 17c1962

static void input_wl_grab_mouse(void *data, bool state)

I guess that's where the zwp_relative_pointer_v1 should be created for the wl_pointer? Does that make any sense?

[ColinKinloch]

I'd say that is where you should call zwp_pointer_constraints_v1_lock_pointer and zwp_pointer_constraints_v1_unlock_pointer depending on the value of state.
You could also create and destroy the relative pointer object in that function. However if retroarch uses wl->mouse.delta_x for anything else I don't see a reason not to create the relative pointer object just after the pointer is created here:

RetroArch/input/common/wayland_common.c

Lines 459 to 463 in 17c1962

	if ((caps & WL_SEAT_CAPABILITY_POINTER) && !wl->wl_pointer)
	{
	wl->wl_pointer = wl_seat_get_pointer(seat);
	wl_pointer_add_listener(wl->wl_pointer, &pointer_listener, wl);
	}

(I don't know if retroarch uses wl->mouse.delta_x for anything else)

[i30817]

It's possible this may fix another similar bug to the dosbox one in the first post in scummvm.

[vanfanel]

@ColinKinloch Ok, I am creating the relative pointer listener object like this:

wl_pointer_add_listener(wl->wl_pointer, &relative_pointer_listener, wl);

...and I get this compiler warning:

warning: passing argument 2 of 'wl_pointer_add_listener' from incompatible pointer type [-Wincompatible-pointer-types]
  477 |       wl_pointer_add_listener(wl->wl_pointer, &relative_pointer_listener, wl);
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                               |
      |                                               const struct zwp_relative_pointer_v1_listener *
In file included from /usr/local/include/wayland-client.h:40,
                 from input/common/wayland_common.h:22,
                 from input/common/wayland_common.c:36:
/usr/local/include/wayland-client-protocol.h:4716:38: note: expected 'const struct wl_pointer_listener *' but argument is of type 'const struct zwp_relative_pointer_v1_listener *'
 4716 |    const struct wl_pointer_listener *listener, void *data)
      |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~

That's because wl_pointer_add_listener expects a const struct wl_pointer_listener, but the relative_pointer_listener object is a zwp_relative_pointer_v1_listener (what you rightfully called "it's own thing").

Should I cast the zwp_relative_pointer_v1_listener to a const struct wl_pointer_listener, simply ignore the warning, anything else?

[ColinKinloch]

The headers for the protocol are generated in gfx/common/wayland/relative-pointer-unstable-v1.h.
The functions you want to use are:

struct zwp_relative_pointer_v1 * zwp_relative_pointer_manager_v1_get_relative_pointer(struct zwp_relative_pointer_manager_v1 *zwp_relative_pointer_manager_v1, struct wl_pointer *pointer);
int zwp_relative_pointer_v1_add_listener(struct zwp_relative_pointer_v1 *zwp_relative_pointer_v1, const struct zwp_relative_pointer_v1_listener *listener, void *data);

zwp_relative_pointer_manager_v1_get_relative_pointer to get the wayland relative pointer for a wayland pointer.
zwp_relative_pointer_v1_add_listener to set the listener object for that wayland relative pointer.

When a pointer is locked all the callbacks in wl_pointer_listener except wl_pointer_handle_motion are called as usual:

RetroArch/input/common/wayland_common.c

Lines 931 to 937 in 17c1962

	const struct wl_pointer_listener pointer_listener = {
	wl_pointer_handle_enter,
	wl_pointer_handle_leave,
	wl_pointer_handle_motion,
	wl_pointer_handle_button,
	wl_pointer_handle_axis,
	};

Since when the pointer is locked it doesn't have a meaningful position from the compositors point of view it's relative movements are reported via the relative_motion callback on the zwp_relative_pointer_v1_listener object you pass to zwp_relative_pointer_v1_add_listener.

[ColinKinloch]

To clarify wl_seat_get_pointer and wl_pointer_add_listener should be left as is.

RetroArch/input/common/wayland_common.c

Lines 459 to 463 in 17c1962

	if ((caps & WL_SEAT_CAPABILITY_POINTER) && !wl->wl_pointer)
	{
	wl->wl_pointer = wl_seat_get_pointer(seat);
	wl_pointer_add_listener(wl->wl_pointer, &pointer_listener, wl);
	}

followed by a with a call to zwp_relative_pointer_manager_v1_get_relative_pointer and zwp_relative_pointer_v1_add_listener.

[vanfanel]

@ColinKinloch Ok, everythig is done as you told me.
RetroArch builds fine, no warnings anymore!

However, I still see the same cursor problems.
I have activated the automatic mouse grab in RetroArch settings.

Any ideas on what could be missing?

[ColinKinloch]

If you log the values of dx and dy are they changing as you'd expect?

[vanfanel]

@ColinKinloch Ah, wait, I forgot to call zwp_pointer_constraints_v1_lock_pointer() in input_wl_grab_mouse()... sorry!

What should I pass as struct wl_region *region to zwp_pointer_constraints_v1_lock_pointer()?
And what should I pass as uint32_t lifetime?

[ColinKinloch]

SDL calls it with a NULL region and ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_PERSISTENT lifetime, which is probably sensible.

[vanfanel]

@ColinKinloch Ok, so I am calling zwp_pointer_constraints_v1_lock_pointer() in input_wl_grab_mouse() like this:

  if (state == true)
      zwp_pointer_constraints_v1_lock_pointer(wl->pointer_constraints, wl->surface,
       wl->wl_pointer, NULL, ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_PERSISTENT);

...but it segfaults.
Looking at the call with GDB, I discovered that all parameters are NULL at that point:

#1  0x00000055557acc18 in zwp_pointer_constraints_v1_lock_pointer (zwp_pointer_constraints_v1=0x0, 
    surface=0x0, pointer=0x0, region=0x0, lifetime=2)

...So, what could be missing? Maybe I should guard the zwp_pointer_constraints_v1_lock_pointer call o avoid it being called when any of these parameters are NULL? Or maybe I am missing something else?

[ColinKinloch]

Does it segfault on start or when you trigger mouse grab?
Try changing the if statement to:

if (state && wl->pointer_constraints)
      zwp_pointer_constraints_v1_lock_pointer(wl->pointer_constraints, wl->surface,
       wl->wl_pointer, NULL, ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_PERSISTENT);
else
  RARCH_LOG(" -- mouse grab %p %p %p \n", wl->pointer_constraints, wl->surface,
       wl->wl_pointer);

[vanfanel]

@ColinKinloch That made it work, yes.
Now, only X axis seems to be totally corrected: cursor is still stuck vertically upon dosbox-core start with Ishar1.
So, I went and did this:

static void handle_relative_motion(void *data,
   struct zwp_relative_pointer_v1 *zwp_relative_pointer_v1,
   uint32_t utime_hi, uint32_t utime_lo,
   wl_fixed_t dx, wl_fixed_t dy,
   wl_fixed_t dx_unaccel, wl_fixed_t dy_unaccel)
{
    gfx_ctx_wayland_data_t *wl = (gfx_ctx_wayland_data_t*)data;

    wl->input.mouse.delta_x = wl_fixed_to_int(dx);
    wl->input.mouse.delta_y = wl_fixed_to_int(dy);
}

..But Y axis is still wrong.
Is there anything else I could be missing with regard to the Y axis?
Do you think I should modify input_wl_poll too?

[ColinKinloch]

Assuming handle_relative_motion is called even when the pointer is not locked you can get rid of these lines:

RetroArch/input/drivers/wayland_input.c

Lines 75 to 76 in 17c1962

	wl->mouse.delta_x = wl->mouse.x - wl->mouse.last_x;
	wl->mouse.delta_y = wl->mouse.y - wl->mouse.last_y;

or perhaps change them to something like:

if (wl->relative_pointer)
{
   wl->mouse.delta_x = wl->mouse.x - wl->mouse.last_x; 
   wl->mouse.delta_y = wl->mouse.y - wl->mouse.last_y; 
}

where wl->relative_pointer is the value returned by zwp_relative_pointer_manager_v1_get_relative_pointer

[vanfanel]

@ColinKinloch PR sent!

#15103

[vanfanel]

@ColinKinloch Ooops, there's still some problem ee00814

The cursor auto-moves sometimes.. Any idea on what could be wrong?

[i30817]

Can be closed is fixed.

[vanfanel]

Yep! Closing now!