Skip to content

wayland: add support for tabel input #16276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jp7677
Copy link

@jp7677 jp7677 commented Apr 22, 2025

This is a draft PR about adding support for tablet/stylus input for Wayland. The motivation behind this is that this PR lets me control mpv on a Gnome Wayland session with a tablet/stylus as I'm usually using a tablet as a full mouse replacement. Gnome Desktop unfortunately doesn't offer a mouse fallback for tablet input when a client doesn't support the tablet protocol, so a stylus can't interact with the mpv window there at all.

This is still early draft, but functional (tested on Gnome and labwc/wlroots) except for drag&drop like operations. It still needs polishing, some refactoring to avoid code duplication (cursor visibility) and splitting commits and better commit messages, but I wanted to create this PR in this state for general feedback. The main question is: is there interest in tablet/stylus support and does this PR has a chance of getting merged? The tablet protocol is not that simple and a lot of boilerplate code is needed for just a few basic handlers. I hope not, but I would understand if this is too much for an edge case.

Thanks for creating and maintaining mpv and looking forward to some general early feedback!

@jp7677 jp7677 force-pushed the wayland-tablet-protocol branch 2 times, most recently from 36504fa to 92498d3 Compare April 22, 2025 06:27
@llyyr
Copy link
Contributor

llyyr commented Apr 22, 2025

Gnome Desktop unfortunately doesn't offer a mouse fallback for tablet input when a client doesn't support the tablet protocol, so a stylus can't interact with the mpv window there at all.

Is this something GNOME won't implement or something that simply has never been reported? sway falls back to pointer events if a client doesn't support touch or tablet protocols, it would be much simpler to add this fallback in Mutter than go around fixing every single client

@jp7677
Copy link
Author

jp7677 commented Apr 22, 2025

Is this something GNOME won't implement or something that simply has never been reported? sway falls back to pointer events if a client doesn't support touch or tablet protocols, it would be much simpler to add this fallback in Mutter than go around fixing every single client

Looking at https://gitlab.gnome.org/GNOME/mutter/-/issues/2226 from three years ago, I think it is unfortunately the former, at least at that time.

PS: I agree by the way that mouse emulation in the compositor would be better, Labwc did the same as Sway for tablet input and that feels much more robust to me (that is, the Labwc implementation, I never looked that detailed at Sway and can't comment on how well it works there).

@kasper93
Copy link
Member

kasper93 commented Apr 22, 2025

sway falls back to pointer events if a client doesn't support touch or tablet protocols,

Note that it is completely broken in mpv, at least for osc.lua. Sway reset pointer position to "real" mouse pos when all fingers leave the touch screen. By the time osc.lua processes the touch, the pointer is in another place. Making all buttons non-
clickable in practice.

And there is no good way to "fix" or workaround that. Because we have no idea if the move was real or it was emulated switch to mouse.

So proper native touch support might be the only way forward.

Copy link

github-actions bot commented Apr 22, 2025

@llyyr
Copy link
Contributor

llyyr commented Apr 22, 2025

So proper native touch support might be the only way forward.

We already have native touch support, this is about tablet support. And sway translating tablet events to pointer events works perfectly in mpv. I can't say for touch events as I don't have a touch device, and would need to rip out touch support from mpv to even test it.

@kasper93
Copy link
Member

Doesn't work on my end.

@llyyr
Copy link
Contributor

llyyr commented Apr 22, 2025

Doesn't work on my end.

What doesn't work? Touch or tablet? If touch doesn't work that's a mpv bug, or a bug with sway/wlroots sending us touch events (not one with translating touch to pointer events).

edit: I checked and it's a sway bug with touch input, works on river.

@mahkoh
Copy link
Contributor

mahkoh commented Apr 25, 2025

sway falls back to pointer events if a client doesn't support touch or tablet protocols

That cannot work. Let's say an application has no tablet support and uses libdecor to draw window decorations, which (let's say) also doesn't support tablets. The user can now drag the window by its decorations in sway because sway sends emulated pointer events. But then the application adds support for tablet events and now the user can no longer interact with the window decorations because sway no longer sends emulated events and libdecor still doesn't support tablet events.

@jp7677 jp7677 force-pushed the wayland-tablet-protocol branch 3 times, most recently from f331ef9 to b3db60b Compare April 25, 2025 17:09
@jp7677
Copy link
Author

jp7677 commented Apr 25, 2025

I think this is pretty much ready from my side, moving the window by dragging the window with the stylus works now and with that I think the stylus support is solid. That said, I think I got the wiring correctly, but a keen eye is very welcome, especially for shutting things down. Let me know what you think and if it's worth it.

@mahkoh I contributed to the tablet support for labwc, which also uses mouse emulation for clients that do not support the tablet protocol. If you have a case with a reproduction scenario where it doesn't or shouldn't work, could you open an issue there since I would be curious to play with it.

@jp7677 jp7677 marked this pull request as ready for review April 25, 2025 17:21
@mahkoh
Copy link
Contributor

mahkoh commented Apr 25, 2025

If you have a case with a reproduction scenario where it doesn't or shouldn't work

What I described above should be easy enough to reproduce in any application that uses both libdecor and supports tablet input. You might have to enable CSD in your compositor for libdecor to do anything.

@jp7677 jp7677 force-pushed the wayland-tablet-protocol branch from b3db60b to 0661c5e Compare April 25, 2025 18:53
}

struct vo_wayland_tablet_seat *tablet_seat, *tablet_seat_tmp;
wl_list_for_each_safe(tablet_seat, tablet_seat_tmp, &seat->wl->wp_tablet_seat_list, link) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tablet_seat should just be a member of struct vo_wayland_seat instead of a list in struct vo_wayland_state because each tablet_seat only belongs to one seat. This way a single zwp_tablet_seat_v2_destroy is enough.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be good now. Indeed much better.

@@ -2658,6 +2877,24 @@ static void remove_seat(struct vo_wayland_seat *seat)
if (seat->xkb_state)
xkb_state_unref(seat->xkb_state);

struct vo_wayland_tablet_tool *tablet_tool, *tablet_tool_tmp;
wl_list_for_each_safe(tablet_tool, tablet_tool_tmp, &seat->wl->wp_tablet_tool_list, link) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tablet_tool_list should be a member of struct vo_wayland_seat because they belong to the respective seats.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good now.


tablet_tool->seat->pointer_button_serial = serial;
wl->last_button_seat = tablet_tool->seat;
mp_input_put_key(tablet_tool->wl->vo->input_ctx, MP_MBTN_LEFT | MP_KEY_STATE_DOWN | tablet_tool->seat->mpmod);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mouse emulation should be handled in the input system in a similar style as touch inputs so that the mouse emulation behavior can be controled by the user and makes it possible for client API users to distinguish between mouse/touch/tablet inputs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that means there should be a mp_input_tablet_tool_up/mp_input_tablet_tool_down/mp_input_tablet_tool_button?

Copy link
Contributor

@na-na-hi na-na-hi Apr 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It can be done in a similar way as the existing touch support, and reports the tablet tool status to the client API with a property. I would also add proximity events so that the property only needs to report the tools that are in range.

Copy link
Author

@jp7677 jp7677 Apr 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a WIP commit (to be squashed later) for moving tip down/up (not yet button) to input, https://github.com/mpv-player/mpv/pull/16276/commits . This is the direction that you have in mind?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks fine for mouse emulation for now. Documentation and client API property need some work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think I've found the relevant place for the documentation (options.rst/interface-changes.rst). What exactly do you mean with "client API property"?
(Sorry, I've never used mpv as a library and I didn't find any clues when searching for touch in client.h/client.c).

Comment on lines 735 to 733
if (button) {
mp_input_put_key(tablet_tool->wl->vo->input_ctx, button | state | tablet_tool->seat->mpmod);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Button should only take effect when the tool is logically making contact, not while hovering. Save the button here and apply on down event.

Copy link
Author

@jp7677 jp7677 Apr 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already what is happening here. See https://wayland.freedesktop.org/libinput/doc/latest/tablet-support.html#tool-tip-events-vs-tool-button-events

The tablet tool has three events that roughly matches mouse buttons:
Down - the tip of the stylus/pen touches the tablet surfaces
Up - the tip of the stylus/pen no longer touches the surface
Button - a side button is pressed or released.

So here the events for down/up matches left mouse button press/release and the tool button event matches other mouse buttons press and release.

Copy link
Contributor

@na-na-hi na-na-hi Apr 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So here the events for down/up matches left mouse button press/release and the tool button event matches other mouse buttons press and release.

This is incorrect from the documentation you linked:

Tablet tools may send button events; these are exclusively for extra buttons unrelated to the tip. A button event is independent of the tip and can while the tip is down or up.

If I click a button on the tool while the tool isn't in contact it will still send a button event. However, for mouse emulation purposes, the button click shouldn't do anything until the tool contacts the surface.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, the button click shouldn't do anything until the tool contacts the surface.

That’s not what I see in practice, the side buttons on my stylus are used nearly everywhere to simulate right and middle mouse button, so e.g. context menu or clipboard paste, whereas tip down/up acts as a left mouse click. The tool needs to be in proximity, but the side buttons do act immediately, regardless of the tip state (like independent mouse buttons).
This might indeed be handled differently by specialized drawing applications but I haven’t seen this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows it behaves like what I mentioned. Holding the pen button only changes the cursor shape and doesn't result in a click until the tip touches the screen. This allows drawing applications to use pressure information for tools that become active when the button is pressed.

It seems that tablet mouse emulation on X11 behaves differently and activates mouse click while hovering. IMO it's a worse behavior because clicking a button while hovering makes the pen shake slightly which reduces click accuracy, but might not be an issue if most Linux desktop users prefer this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah right sorry, I was indeed talking about Linux. My tablet experiences on Windows are limited to a firmware update years ago ;)

IMO it's a worse behavior because clicking a button while hovering makes the pen shake slightly which reduces click accuracy

Yes, I see that point. I guess I just got used to it and usually for standard UI applications / the desktop it doesn't matter that much imho compared to e.g. real drawing applications.

but might not be an issue if most Linux desktop users prefer this.

Thanks!

@jp7677 jp7677 marked this pull request as draft April 26, 2025 08:47
@jp7677 jp7677 force-pushed the wayland-tablet-protocol branch 4 times, most recently from 0f9c657 to efdd136 Compare April 26, 2025 17:25
struct vo_wayland_seat *seat = data;
struct vo_wayland_state *wl = seat->wl;

struct vo_wayland_tablet_tool *tablet_tool = talloc_zero(wl, struct vo_wayland_tablet_tool);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct vo_wayland_tablet_tool *tablet_tool = talloc_zero(wl, struct vo_wayland_tablet_tool);
struct vo_wayland_tablet_tool *tablet_tool = talloc_zero(seat, struct vo_wayland_tablet_tool);

zwp_tablet_tool_v2_destroy(tablet_tool->tablet_tool);
}

if (seat->xkb_state)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (seat->xkb_state)
if (seat->tablet_seat)

@@ -1066,6 +1072,34 @@ int mp_input_get_touch_pos(struct input_ctx *ictx, int count, int *x, int *y, in
return num_touch_points;
}

void mp_input_set_tablet_tool_in_proximity(struct input_ctx *ictx, bool in_proximity) {
ictx->tablet_tool_in_proximity = in_proximity;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add locking.

@jp7677 jp7677 force-pushed the wayland-tablet-protocol branch 2 times, most recently from 53ceec3 to 051710c Compare April 27, 2025 06:46
@jp7677
Copy link
Author

jp7677 commented Apr 27, 2025

CI for the latest pipeline on master fails with the same errors like the latest pipeline for this PR, so I don’t think this is on me.

@llyyr
Copy link
Contributor

llyyr commented Apr 27, 2025

CI for the latest pipeline on master fails with the same errors like the latest pipeline for this PR, so I don’t think this is on me.

Flaky windows CI is new mpv contributor hazing ritual don't worry

@kasper93
Copy link
Member

kasper93 commented Apr 27, 2025

CI for the latest pipeline on master fails with the same errors like the latest pipeline for this PR, so I don’t think this is on me.

Flaky windows CI is new mpv contributor hazing ritual don't worry

I don't understand, why is this Windows related?

EDIT:

Build with GCC 15 is fixed in #16293

jp7677 added 6 commits April 28, 2025 20:01
The tablet protocol is supported in the current winimum
wayland-protocols version, so we can always build this.
Note that this is still the unstable version, once the
minimum required wayland-protocol version is 1.35, the
stable version of protocol can be included.
Tablets require their own tablet manager and tablet
seats since multiple tablets/pens can be connected to
a single seat. So this is unfortunately a lot of
wiring.
For now, only the tablet tool events are set up, a.o.
motion and buttons. No interest (yet) in tablet or
tablet pad events. That said, the latter might become
useful in the future, e.g. for controlling the player
with buttons on the pad.
Pretty much copied from pointer cursors, including
the support for the cursor-shape protocol.
@jp7677 jp7677 force-pushed the wayland-tablet-protocol branch from 051710c to 4c4176d Compare April 28, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants