-
Notifications
You must be signed in to change notification settings - Fork 3k
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
base: master
Are you sure you want to change the base?
Conversation
36504fa
to
92498d3
Compare
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). |
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- 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. |
Download the artifacts for this pull request: Windows |
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. |
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. |
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. |
f331ef9
to
b3db60b
Compare
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. |
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. |
b3db60b
to
0661c5e
Compare
0f9c657
to
efdd136
Compare
53ceec3
to
051710c
Compare
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 |
Thanks for the comments, I’ve rework the code accordingly. |
player/command.c
Outdated
node_map_add_int64(&node, "x", xs); | ||
node_map_add_int64(&node, "y", ys); | ||
node_map_add_flag(&node, "tool-in-proximity", tool_in_proximity); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also include tool down/up and button states here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I couldn't find any examples for button states, I ended up with a structure like this:
{"x":0,"y":0,"tool-in-proximity":false,"tool-tip":"up","tool-stylus-btn1":"released","tool-stylus-btn2":"released","tool-stylus-btn3":"released"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playing a bit with the lua
side, is it correct that observing a sub property doesn't work?
This snippet doesn't yield a callback: mp.observe_property("tablet-pos/tool-tip", "string", on_tablet_tool_tip_change)
This one: mp.observe_property("tablet-pos", "native", on_tablet_change)
does work, but I think only when x
/y
have changed, not when only a button press has occurred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should call mp_notify
or mp_notify_property
at appropriate place to trigger client observers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've added a notify_tablet_update()
(similar to touch) to trigger an update to tablet-pos
when any of its properties have changed. Still looking for way to make e.g. mp.observe_property("tablet-pos/tool-tip",...
work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmh, running mp.observe_property("mouse-pos/x"
doesn't yield anything either, so I tend to conclude that it currently just doesn't works like this.
That said, something like this still works for overriding tip and buttons (when additionally disabling tablet input with --input-tablet-emulate-mouse=no
) which is actually really cool:
tablet_pos_tool_tip_old = ""
mp.observe_property("tablet-pos", "native", function(name, value)
tablet_pos_tool_tip = value["tool-tip"]
if (tablet_pos_tool_tip ~= tablet_pos_tool_tip_old and tablet_pos_tool_tip == "up") then
mp.msg.log("info", "Tablet tool tip tap ..")
end
tablet_pos_tool_tip_old = tablet_pos_tool_tip
end)
Feel free to resolve mine. I don't have the privilege to resolve my own reviews. |
a73fc44
to
bfe7eb2
Compare
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. |
I've tried to reproduce this on both Gnome/mutter and PS: Thanks for trying this PR! |
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 |
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 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. |
Thanks for confirming that at least I didn't messed up ;)
I guess you wanted to say "touch mode"?
So this is a timing issue and depends on machine speed/load/tablet/touch sample rate etc?
If compositors provide emulation, which isn't the case for Gnome/mutter, but I get your point.
Thanks. |
373b860
to
f062f69
Compare
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.
This ensures that tablet objects are properly destroyed on removal or shutdown, even if we are not really interested in tablet events.
Setup tablet tool events, we want to use motion, tip and button events later.
f062f69
to
54aad7b
Compare
This ensures that tablet pad objects are properly destroyed on removal or shutdown, even if we are not (yet) interested in tablet pad events.
This ensures that tablet pad objects that belong to a group, thus rings and strips, are all properly destroyed on removal or shutdown. Note that event listeners for ring a strip are not added since we are not (yet) interested in them and they are not needed for proper destroy.
Pretty much copied from pointer cursors, including the support for the cursor-shape protocol.
Just simple mouse emulation for now.
Upgrade tablet input to first class citizen similar like touch input, but more simple since use case for now is mouse emulation only. Keep state for coordinates and in-proximity. Introduce property for disabling tablet input. Introduce Client-API property `tablet-pos`.
54aad7b
to
f9c441c
Compare
Sorry, linting should be good now. Edit: that one is not on me ;)
Also don’t think this one is caused by this PR:
|
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!