Xinput2: use raw events and queries #11762
Conversation
|
Would be interesting to know if this adds a proper fix for https://bugs.dolphin-emu.org/issues/12913. We added a workaround for this issue with d40dbe4. (Unfortunately, I don't have a Linux machine on hand that I can easily test on.) |
|
I tested the current changes, and they do not fix the Qt 6.3+ bug on Linux when the workaround is removed. That's too bad. |
|
I tested this branch with the For comparison, I tested master in Mutter with the workaround (clicks are recognized) and without (clicks are not recognized), so without this branch's changes, the workaround is necessary in my environment. I tested with: I run Openbox with |
|
This should be rebased. |
Because we care how many bits it has, not its arithmetic range. No functional change on all supported platforms.
We need XInput 2.1 to get raw events on the root window even while another client has a grab. We currently use raw events for relative mouse input, and upcoming commits will use raw events for buttons and keys.
In X, the ButtonPress events generated when a mouse button is pressed have a special property: if they don't activate an existing passive grab, the X server automatically activates the "implicit passive grab" on behalf of the client the event is delivered to. This ensures the ButtonRelease event is delivered to the same client even if the pointer moves between windows, but it also causes all events from that pointer to be delivered exclusively to that client. As a consequence of the implicit passive grab, for each window, only one client can listen for ButtonPress events; any further listeners would never receive the event. XInput 1 made the implicit grab optional and explicit by allowing clients to listen for DeviceButtonPress events without DeviceButtonPressGrab events. XInput 2 does not have a separate grab event class, but multiple clients can listen for XI_ButtonPress on the same window. When a button is pressed, the X server first tries to deliver an XI_ButtonPress event; if no clients want it, then the server tries to deliver a DeviceButtonPress event; if no clients want it, then the server tries to deliver a ButtonPress event. Once an event has been delivered, event processing stops and earlier protocol levels are not considered. The reason for this rule is not obviously documented, but it is probably because of the implicit passive grab; a client receiving a ButtonPress event assumes it is the only client receiving that event, and later protocols maintain that property for backward compatibility. Before this commit, Dolphin listened for XI_ButtonPress events on the root window. This interferes with window managers that expect to receive ButtonPress events on the root window, such as awesome and Openbox. In Openbox, applications are often launched from a menu activated by clicking on the root window, and desktops are switched by scroll wheel input on the root window. This makes normal use of other applications difficult when Dolphin is open (though Openbox keyboard shortcuts still work). Conversely, Dolphin only receives XI_ButtonPress events for clicks on the root window or window decorations (title bars), not on Dolphin's windows' content or the render window. In window managers that use a "virtual root window" covering the actual root window, such as Mutter running in X, Dolphin and the window manager do not conflict, but clicks delivered to other applications using XInput2 (for testing, try xinput --test-xi2) are not seen by Dolphin, which is relevant when background input is enabled. This commit changes Dolphin to listen for XI_RawButtonPress (and the raw versions of other events); Dolphin was already listening to XI_RawMotion for raw mouse movement. Raw events are always and exclusively delivered to the root window and are delivered to every client listening for them, so Dolphin will not interfere with (or be interfered with by) other applications listening for events. As part of being raw, button numbers and keycodes in raw events have not had mapping applied. If a left-handed user swapped the left and right buttons on their mouse, raw events do not reflect that. It is possible to query the mappings for each device and apply them manually, but that would require a fair amount of code, including listening for mapping changes. Instead, Dolphin now uses the events only to set a "changed" flag, then queries the current button and key state after processing all events. Dolphin was already querying the pointer to get its absolute position and querying the keyboard to filter the key bitmap it created from events; now Dolphin also uses the button state from the pointer query and uses the keyboard query directly. Queries have a performance cost because they are synchronous requests to the X server (Dolphin waits for the result). Commit 2b640a4 made the pointer query conditional on receiving a motion event to "cut down on round trips", but commit bbb12a7 added an unconditional keyboard query, and there have apparently been no performance complaints. This commit queries the pointer slightly more often (on button events in addition to motion), but only queries the keyboard after key events, so the total rate of queries should be substantially reduced. Fixes: https://bugs.dolphin-emu.org/issues/10668
A comment removed by this commit gives two reasons for listening to slave devices, both of which no longer apply: - "Only slaves emit raw motion events": perhaps this was true when the comment was written, but now master devices provide raw motion events along with the other raw events. - "Selecting slave keyboards avoids dealing with key focus": we get raw key events regardless of the focus. Listening to both master and slave devices results in duplicate raw events. For button and key events, that's a tiny waste of time setting the update flag a second time, but for raw mouse events the raw motion will be processed twice. That makes this commit a user-facing change.
jbosboom
force-pushed
the
xinput2-raw-event-query
branch
from
2000f0b
to
620955d
Compare
|
@AdmiralCurtiss Rebased. |
|
I'm not particularly familiar with XInput2, but this code seems generally sane and apparently fixes a long-standing bug, so if no one has any complaints I'll merge this in the next few days. |
This series reworks XInput2 to fix https://bugs.dolphin-emu.org/issues/10668 (and two duplicates). The first commit in this series is the (only) commit in #11758. GitHub won't let me make a pull request based off another pull request (it redirects me to my fork). I thought I was helping by fixing one bug per PR, but actually I just created noise. Sorry about that!
Bug 10668 is a complaint that while Dolphin is open, Openbox does not respond to clicks on the root window, and Dolphin only sees clicks on the root window or window decorations, not inside Dolphin's own windows. The root cause of this conflict is special X server behavior that causes button press events to grant the receiving client an implicit passive grab on the pointer. Because only one client can get the grab, the X server has special rules about button press event delivery across the core protocol and both XInput versions. This series switches to using raw events, which are always delivered to all clients listening for them. As an Openbox user, I can confirm this resolves the conflict between Dolphin and Openbox. I also tested using Mutter running under X, where it continues to work as it did before.
unsigned intwithu32in the definition of a flag field. No functional change. Could be part of 323d08f, which cares that it's specifically 32 bits, but split here for ease of review.This is my first nontrivial pull request, so I would appreciate general feedback or style nits. I also have these specific questions for reviewers: