Skip to content
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

XInput2: Accept input from keyboards other than the first master #11758

Merged
merged 1 commit into from May 19, 2023

Conversation

jbosboom
Copy link
Contributor

This is a fix for a bug I found while putting together a series that reworks XInput2 to fix https://bugs.dolphin-emu.org/issues/10668 (and two duplicates). Because this fix is necessary regardless of whether you take that series, I've sent it as its own pull request for ease of review and merging.


XInput2 was created to support multiple pointer/keyboard pairs (often called MPX for multi-pointer X). Dolphin's XInput2 implementation has always supported MPX by creating a KeyboardMouse object per master pointer. Since commit bbb12a7, Dolphin's keyboard state is filtered by the output of XQueryKeymap. As a core X function, XQueryKeymap queries "the" keyboard, which by default is the first master keyboard. As a result, Dolphin will ignore keys pressed on other master keyboards unless the first master is simultaneously pressing the same keys.

XInput2 doesn't provide a function to query the keyboard state. There is no XIQueryKeymap and the current state is not a member of the XIKeyClassInfo returned by XIQueryDevice. Instead, XInput2 allows a master pointer to be nominated as "the" pointer on a per-client basis, with "the" keyboard automatically becoming the associated master keyboard. The "documentation" 1 says passing None for the window is only for debugging purposes, but it is documented in the XISetClientPointer man page and seems to be the only way to query keyboards beyond the first.

With this commit, Dolphin correctly reads keys from keyboards other than the first master keyboard. To test, use the xinput command-line utility to create a master pointer and reattach a keyboard to the associated master keyboard.

XInput2 was created to support multiple pointer/keyboard pairs (often
called MPX for multi-pointer X).  Dolphin's XInput2 implementation has
always supported MPX by creating a KeyboardMouse object per master
pointer.  Since commit bbb12a7, Dolphin's keyboard state is filtered by
the output of XQueryKeymap.  As a core X function, XQueryKeymap queries
"the" keyboard, which by default is the first master keyboard.  As a
result, Dolphin will ignore keys pressed on other master keyboards
unless the first master is simultaneously pressing the same keys.

XInput2 doesn't provide a function to query the keyboard state.  There
is no XIQueryKeymap and the current state is not a member of the
XIKeyClassInfo returned by XIQueryDevice.  Instead, XInput2 allows a
master pointer to be nominated as "the" pointer on a per-client basis,
with "the" keyboard automatically becoming the associated master
keyboard.  The "documentation" [1] says passing None for the window is
only for debugging purposes, but it is documented in the
XISetClientPointer man page and seems to be the only way to query
keyboards beyond the first.

With this commit, Dolphin correctly reads keys from keyboards other than
the first master keyboard.  To test, use the xinput command-line utility
to create a master pointer and reattach a keyboard to the associated
master keyboard.

[1]: https://who-t.blogspot.com/2009/07/xi2-recipes-part-6.html
     (the XInput2 developer's blog)
@jbosboom
Copy link
Contributor Author

Not related to this commit, but related to multiple masters: in the controller configuration dialog, the devices are named "XInput2/0/Virtual core pointer" and "XInput2/0/foo pointer" (or whatever you called the second master). Should the "/0/" be a "/1/" for the second master? What's that number for, in general?

@dolphin-ci
Copy link

dolphin-ci bot commented Apr 14, 2023

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • milotic-texture on uberogl-lin-radeon: diff

automated-fifoci-reporter

@JosJuice
Copy link
Member

Not related to this commit, but related to multiple masters: in the controller configuration dialog, the devices are named "XInput2/0/Virtual core pointer" and "XInput2/0/foo pointer" (or whatever you called the second master). Should the "/0/" be a "/1/" for the second master? What's that number for, in general?

I believe the main purpose is to disambiguate in situations where two devices otherwise would have had identical names. So the current behavior is fine.

@AdmiralCurtiss AdmiralCurtiss merged commit 279fcaf into dolphin-emu:master May 19, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants