mpv icon indicating copy to clipboard operation
mpv copied to clipboard

wayland: add support for tabel input

Open jp7677 opened this issue 7 months ago • 24 comments

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 avatar Apr 22 '25 06:04 jp7677

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

llyyr avatar Apr 22 '25 06:04 llyyr

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).

jp7677 avatar Apr 22 '25 06:04 jp7677

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.

kasper93 avatar Apr 22 '25 10:04 kasper93

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.

llyyr avatar Apr 22 '25 13:04 llyyr

Doesn't work on my end.

kasper93 avatar Apr 22 '25 13:04 kasper93

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.

llyyr avatar Apr 22 '25 13:04 llyyr

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.

mahkoh avatar Apr 25 '25 09:04 mahkoh

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 avatar Apr 25 '25 17:04 jp7677

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.

mahkoh avatar Apr 25 '25 17:04 mahkoh

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.

jp7677 avatar Apr 27 '25 16:04 jp7677

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

llyyr avatar Apr 27 '25 16:04 llyyr

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

kasper93 avatar Apr 27 '25 16:04 kasper93

Thanks for the comments, I’ve rework the code accordingly. What do you prefer w.r.t to resolving the discussions here? Should I close the discussions once it has been reworked to my best knowledge or do you prefer to leave resolving to the original poster?

jp7677 avatar May 13 '25 19:05 jp7677

What do you prefer w.r.t to resolving the discussions here? Should I close the discussions once it has been reworked to my best knowledge or do you prefer to leave resolving to the original poster?

Feel free to resolve mine. I don't have the privilege to resolve my own reviews.

na-na-hi avatar May 14 '25 01:05 na-na-hi

For me it works worse than emulated input. One issue is when you hover stylus over a OSC button, click on it, and keep stylus in the same location, any subsequent clicks on the same button doesn't work. No such issue on mpv master.

(not related to tablet issue, offtopic): and it still doesn't fix the issue which I'm most interested in, where touch input doesn't work in sway completely if you move mouse pointer in sway, because it resets touch pos to mouse pointer pos and it's not recognized as a click by the time it's processed, osc is pulling pos.

kasper93 avatar May 17 '25 19:05 kasper93

For me it works worse than emulated input. One issue is when you hover stylus over a OSC button, click on it, and keep stylus in the same location, any subsequent clicks on the same button doesn't work. No such issue on mpv master.

I've tried to reproduce this on both Gnome/mutter and labwc, but repeated clicks do work fine on both. Do you see that issue in another compositor? I don't have a sway setup, so don't know about their tablet support.

PS: Thanks for trying this PR!

jp7677 avatar May 17 '25 21:05 jp7677

I've tried to reproduce this on both Gnome/mutter and labwc, but repeated clicks do work fine on both. Do you see that issue in another compositor? I don't have a sway setup, so don't know about their tablet support.

I tested the same on Gnome. I can also reproduce it here. Basically what I try to do is to click play/pause button, repellently. Without moving away stylus, so the cursor is still there. And what happens is that it's not registering taps. Oftentimes it finally register, but not sure what is the exact trigger, probably I move tip more or something like that.

kasper93 avatar May 17 '25 21:05 kasper93

I've tried to reproduce this on both Gnome/mutter and labwc, but repeated clicks do work fine on both. Do you see that issue in another compositor? I don't have a sway setup, so don't know about their tablet support.

I tested the same on Gnome. I can also reproduce it here. Basically what I try to do is to click play/pause button, repellently. Without moving away stylus, so the cursor is still there. And what happens is that it's not registering taps. Oftentimes it finally register, but not sure what is the exact trigger, probably I move tip more or something like that.

Thanks. Strange, I just installed swaywm on my Fedora box and also on sway I can play/pause continuously just fine. Looking at the PR, the only thing that could explain this (without thinking of weird stuff), is your stylus going out of proximity. Could you try removing the proximity check at https://github.com/jp7677/mpv/blob/wayland-tablet-protocol/input/input.c#L1107 and https://github.com/jp7677/mpv/blob/wayland-tablet-protocol/input/input.c#L1118 ? Gnome marks the stylus out of proximity when start dragging with the stylus, may be something like this gets triggered?

jp7677 avatar May 17 '25 21:05 jp7677

stylus is always in proximity and mp_input_tablet_tool_{up/down} is called on every tap. Still the command doesn't trigger and video doesn't pause. Though when in paused state it always unpause. Either way it seems that feed_key is done correctly.

I think this is the exactly same issue I described before regarding tablet mode. In more generic terms, mouse emulation is not playing nice with OSC, because of timing dependency. What happens is that scripts observe mouse button events and trigger action on that, but by the time it is processed the "emulated" state might not be consistent. I haven't debug it, but it looks like that's the case.

See mouse_hit_coords in osc.lua. It does hit test on current mouse pos, which may no longer be emulated one. Or basically up/down events are not in sync with emulated ones.

I think we should resolve this first, because basically this PR by adding emulation inside mpv, making experience worse than the emulation provided by the compositors themselves.

We probably should recognize in input.c when mouse emulation is happening and make sure it's not interrupted somehow.

I can debug it, if I find some time, but not much of it lately.

kasper93 avatar May 17 '25 23:05 kasper93

stylus is always in proximity and mp_input_tablet_tool_{up/down} is called on every tap. Still the command doesn't trigger and video doesn't pause. Though when in paused state it always unpause. Either way it seems that feed_key is done correctly.

Thanks for confirming that at least I didn't messed up ;)

I think this is the exactly same issue I described before regarding tablet mode.

I guess you wanted to say "touch mode"?

See mouse_hit_coords in osc.lua. It does hit test on current mouse pos, which may no longer be emulated one. Or basically up/down events are not in sync with emulated ones.

So this is a timing issue and depends on machine speed/load/tablet/touch sample rate etc?

I think we should resolve this first, because basically this PR by adding emulation inside mpv, making experience worse than the emulation provided by the compositors themselves.

If compositors provide emulation, which isn't the case for Gnome/mutter, but I get your point.

I can debug it, if I find some time, but not much of it lately.

Thanks.

jp7677 avatar May 18 '25 09:05 jp7677

Sorry, linting should be good now.

Edit: that one is not on me ;)

check xml................................................................Failed
- hook id: check-xml
- exit code: 1

TOOLS/mpv-osd-symbols.sfdir/font.props: Failed to xml parse (TOOLS/mpv-osd-symbols.sfdir/font.props:1:0: syntax error)

Also don’t think this one is caused by this PR:

231/236 mpv:libmpv / libmpv-test-multilang.mkv                    FAIL              2.99s   exit status 1

jp7677 avatar May 28 '25 16:05 jp7677

In the meantime I've added two commits which let me program the pad buttons with a LUA script like this one (please don't judge the quality of the LUA script, this was the first time for me touching LUA, I also didn't looked much further once it worked ;) ):

tablet-input.lua

local utils = require "mp.utils"

mp.observe_property("tablet-pos", "native", function(name, value)
        if (value["pad-btns"]["1"] == "pressed") then
                mp.command("seek -30")
        end

	if (value["pad-btns"]["2"] == "pressed") then
                mp.command("keypress MBTN_RIGHT")
        end

	if (value["pad-btns"]["3"] == "pressed") then
                mp.command("seek 45")
        end

	if (value["pad-btns"]["4"] == "pressed") then
                mp.set_property("fullscreen", mp.get_property("fullscreen") == "no" and "yes" or "no")
        end
end)

Being able to do so is actually really cool!

Since this PR is quite heavy (I guess review per commit is still somewhat doable), please let me know if I should drop the pad button support for now and leave that for a follow-up PR.

jp7677 avatar Jun 06 '25 06:06 jp7677

Is there anything left I can do to bring this PR forward and into mergable state, with the exception of hunting down the mpv internal issue kasper93 has mentioned which is slightly out of my league (and because it unfortunately doesn't reproduces on my setup)?

jp7677 avatar Jun 15 '25 06:06 jp7677

Is there anything left I can do to bring this PR forward and into mergable state

Somebody to actually review it. I'll try to look at it next week sometime.

Dudemanguy avatar Jun 21 '25 20:06 Dudemanguy

(not related to tablet issue, offtopic): and it still doesn't fix the issue which I'm most interested in, where touch input doesn't work in sway completely if you move mouse pointer in sway, because it resets touch pos to mouse pointer pos and it's not recognized as a click by the time it's processed, osc is pulling pos.

Fixed by https://github.com/mpv-player/mpv/pull/16468.

For me it works worse than emulated input. One issue is when you hover stylus over a OSC button, click on it, and keep stylus in the same location, any subsequent clicks on the same button doesn't work. No such issue on mpv master.

Does the osc button become grey when you press stylus on it, like when pressing mouse button? If so, then it shares the same issue as the touch one and can be fixed in the same way.

na-na-hi avatar Jun 21 '25 22:06 na-na-hi

Does the osc button become grey when you press stylus on it, like when pressing mouse button? If so, then it shares the same issue as the touch one and can be fixed in the same way.

No, it is not that. There is no mouse down (gray button) when input is not recognized.

kasper93 avatar Jun 21 '25 23:06 kasper93

stylus is always in proximity and mp_input_tablet_tool_{up/down} is called on every tap.

Are all other events working correctly?

na-na-hi avatar Jun 28 '25 15:06 na-na-hi

@Dudemanguy thanks a lot for your review! Is there anything left I need to address? GitHub says that there are still changes requested, but I can’t see any open discussions anymore. I got two of your earlier points only by mail, hence the double check, but may be GitHub is just slightly confused today…

jp7677 avatar Jul 12 '25 17:07 jp7677

That's just how github works. I have to give it another review to change it.

Dudemanguy avatar Jul 12 '25 17:07 Dudemanguy