From a902480cb053478af3f8df25aab8a36395685378 Mon Sep 17 00:00:00 2001 From: Jeffrey Bosboom Date: Sat, 15 Apr 2023 02:13:00 -0700 Subject: [PATCH 1/4] XInput2: Make button state a u32 Because we care how many bits it has, not its arithmetic range. No functional change on all supported platforms. --- .../Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp | 3 +-- Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h | 7 ++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp index 07d8e4bfa5..db7052996c 100644 --- a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp @@ -441,8 +441,7 @@ ControlState KeyboardMouse::Key::GetState() const return (m_keyboard[m_keycode / 8] & (1 << (m_keycode % 8))) != 0; } -KeyboardMouse::Button::Button(unsigned int index, unsigned int* buttons) - : m_buttons(buttons), m_index(index) +KeyboardMouse::Button::Button(unsigned int index, u32* buttons) : m_buttons(buttons), m_index(index) { name = fmt::format("Click {}", m_index + 1); } diff --git a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h index ff2c787b1d..52c78ab752 100644 --- a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h +++ b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h @@ -13,6 +13,7 @@ extern "C" { #include } +#include "Common/CommonTypes.h" #include "Common/Matrix.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" @@ -26,7 +27,7 @@ private: struct State { std::array keyboard; - unsigned int buttons; + u32 buttons; Common::Vec2 cursor; Common::Vec3 axis; Common::Vec3 relative_mouse; @@ -52,11 +53,11 @@ private: { public: std::string GetName() const override { return name; } - Button(unsigned int index, unsigned int* buttons); + Button(unsigned int index, u32* buttons); ControlState GetState() const override; private: - const unsigned int* m_buttons; + const u32* m_buttons; const unsigned int m_index; std::string name; }; From 46540ea42babe23a6579d231dd3ea180b1fa39ad Mon Sep 17 00:00:00 2001 From: Jeffrey Bosboom Date: Sat, 15 Apr 2023 02:13:01 -0700 Subject: [PATCH 2/4] XInput2: Request XInput 2.1 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. --- .../ControllerInterface/Xlib/XInput2.cpp | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp index db7052996c..4f93509f15 100644 --- a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp @@ -5,12 +5,14 @@ #include +#include #include #include #include #include +#include "Common/Logging/Log.h" #include "Common/StringUtil.h" #include "Core/Host.h" @@ -54,6 +56,14 @@ // more cleanly separate each scroll wheel click, but risks dropping some inputs #define SCROLL_AXIS_DECAY 1.1f +namespace +{ +// We need XInput 2.1 to get raw events on the root window even while another +// client has a grab. If we request 2.2 or later, the server will not generate +// emulated button presses from touch events, so we want exactly 2.1. +constexpr int XINPUT_MAJOR = 2, XINPUT_MINOR = 1; +} // namespace + namespace ciface::XInput2 { // This function will add zero or more KeyboardMouse objects to devices. @@ -67,13 +77,18 @@ void PopulateDevices(void* const hwnd) // verify that the XInput extension is available if (!XQueryExtension(dpy, "XInputExtension", &xi_opcode, &event, &error)) + { + WARN_LOG_FMT(CONTROLLERINTERFACE, "XInput extension not available (XQueryExtension)"); return; + } - // verify that the XInput extension is at at least version 2.0 - int major = 2, minor = 0; - - if (XIQueryVersion(dpy, &major, &minor) != Success) + int major = XINPUT_MAJOR, minor = XINPUT_MINOR; + if (XIQueryVersion(dpy, &major, &minor) != Success || major < XINPUT_MAJOR || + (major == XINPUT_MAJOR && minor < XINPUT_MINOR)) + { + WARN_LOG_FMT(CONTROLLERINTERFACE, "XInput extension not available (XIQueryVersion)"); return; + } // register all master devices with Dolphin @@ -160,6 +175,9 @@ KeyboardMouse::KeyboardMouse(Window window, int opcode, int pointer, int keyboar // "context." m_display = XOpenDisplay(nullptr); + int major = XINPUT_MAJOR, minor = XINPUT_MINOR; + XIQueryVersion(m_display, &major, &minor); + // should always be 1 int unused; XIDeviceInfo* const pointer_device = XIQueryDevice(m_display, pointer_deviceid, &unused); From b2a98c41eef80a28519d87abeb28c8869af1e403 Mon Sep 17 00:00:00 2001 From: Jeffrey Bosboom Date: Sat, 15 Apr 2023 02:13:02 -0700 Subject: [PATCH 3/4] XInput2: Use raw events and queries 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 2b640a4f made the pointer query conditional on receiving a motion event to "cut down on round trips", but commit bbb12a75 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 --- .../ControllerInterface/Xlib/XInput2.cpp | 90 ++++++++----------- 1 file changed, 37 insertions(+), 53 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp index 4f93509f15..4c47da4fc5 100644 --- a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp @@ -190,8 +190,8 @@ KeyboardMouse::KeyboardMouse(Window window, int opcode, int pointer, int keyboar { unsigned char mask_buf[(XI_LASTEVENT + 7) / 8] = {}; - XISetMask(mask_buf, XI_ButtonPress); - XISetMask(mask_buf, XI_ButtonRelease); + XISetMask(mask_buf, XI_RawButtonPress); + XISetMask(mask_buf, XI_RawButtonRelease); XISetMask(mask_buf, XI_RawMotion); XIEventMask mask; @@ -203,9 +203,8 @@ KeyboardMouse::KeyboardMouse(Window window, int opcode, int pointer, int keyboar { unsigned char mask_buf[(XI_LASTEVENT + 7) / 8] = {}; - XISetMask(mask_buf, XI_KeyPress); - XISetMask(mask_buf, XI_KeyRelease); - XISetMask(mask_buf, XI_FocusOut); + XISetMask(mask_buf, XI_RawKeyPress); + XISetMask(mask_buf, XI_RawKeyRelease); XIEventMask mask; mask.mask = mask_buf; @@ -272,6 +271,25 @@ void KeyboardMouse::UpdateCursor(bool should_center_mouse) const auto win_width = std::max(win_attribs.width, 1); const auto win_height = std::max(win_attribs.height, 1); + { + XIButtonState button_state; + XIModifierState mods; + XIGroupState group; + + // Get the absolute position of the mouse pointer and the button state. + XIQueryPointer(m_display, pointer_deviceid, m_window, &root, &child, &root_x, &root_y, &win_x, + &win_y, &button_state, &mods, &group); + + // X buttons are 1-indexed, so to get 32 button bits we need a larger type + // for the shift. + u64 buttons_zero_indexed = 0; + std::memcpy(&buttons_zero_indexed, button_state.mask, + std::min(button_state.mask_len, sizeof(m_state.buttons))); + m_state.buttons = buttons_zero_indexed >> 1; + + free(button_state.mask); + } + if (should_center_mouse) { win_x = win_width / 2; @@ -281,19 +299,6 @@ void KeyboardMouse::UpdateCursor(bool should_center_mouse) g_controller_interface.SetMouseCenteringRequested(false); } - else - { - // unused-- we're not interested in button presses here, as those are - // updated using events - XIButtonState button_state; - XIModifierState mods; - XIGroupState group; - - XIQueryPointer(m_display, pointer_deviceid, m_window, &root, &child, &root_x, &root_y, &win_x, - &win_y, &button_state, &mods, &group); - - free(button_state.mask); - } const auto window_scale = g_controller_interface.GetWindowInputScale(); @@ -309,10 +314,10 @@ void KeyboardMouse::UpdateInput() // for the axis controls float delta_x = 0.0f, delta_y = 0.0f, delta_z = 0.0f; double delta_delta; - bool mouse_moved = false; + bool update_mouse = false, update_keyboard = false; - // Iterate through the event queue - update the axis controls, mouse - // button controls, and keyboard controls. + // Iterate through the event queue, processing raw pointer motion events and + // noting whether the button or key state has changed. XEvent event; while (XPending(m_display)) { @@ -325,28 +330,21 @@ void KeyboardMouse::UpdateInput() if (!XGetEventData(m_display, &event.xcookie)) continue; - // only one of these will get used - XIDeviceEvent* dev_event = (XIDeviceEvent*)event.xcookie.data; - XIRawEvent* raw_event = (XIRawEvent*)event.xcookie.data; - switch (event.xcookie.evtype) { - case XI_ButtonPress: - m_state.buttons |= 1 << (dev_event->detail - 1); + case XI_RawButtonPress: + case XI_RawButtonRelease: + update_mouse = true; break; - case XI_ButtonRelease: - m_state.buttons &= ~(1 << (dev_event->detail - 1)); - break; - case XI_KeyPress: - m_state.keyboard[dev_event->detail / 8] |= 1 << (dev_event->detail % 8); - break; - case XI_KeyRelease: - m_state.keyboard[dev_event->detail / 8] &= ~(1 << (dev_event->detail % 8)); + case XI_RawKeyPress: + case XI_RawKeyRelease: + update_keyboard = true; break; case XI_RawMotion: { - mouse_moved = true; + update_mouse = true; + XIRawEvent* raw_event = (XIRawEvent*)event.xcookie.data; float values[4] = {}; size_t value_idx = 0; @@ -377,10 +375,6 @@ void KeyboardMouse::UpdateInput() break; } - case XI_FocusOut: - // Clear keyboard state on FocusOut as we will not be receiving KeyRelease events. - m_state.keyboard.fill(0); - break; } XFreeEventData(m_display, &event.xcookie); @@ -400,23 +394,13 @@ void KeyboardMouse::UpdateInput() m_state.axis.z += delta_z; m_state.axis.z /= SCROLL_AXIS_DECAY; - // Get the absolute position of the mouse pointer const bool should_center_mouse = g_controller_interface.IsMouseCenteringRequested() && Host_RendererHasFocus(); - if (mouse_moved || should_center_mouse) + if (update_mouse || should_center_mouse) UpdateCursor(should_center_mouse); - // KeyRelease and FocusOut events are sometimes not received. - // Cycling Alt-Tab and landing on the same window results in a stuck "Alt" key. - // Unpressed keys are released here. - // Because we called XISetClientPointer in the constructor, XQueryKeymap - // will return the state of the associated keyboard, even if it isn't the - // first master keyboard. (XInput2 doesn't provide a function to query - // keyboard state.) - std::array keyboard; - XQueryKeymap(m_display, keyboard.data()); - for (size_t i = 0; i != keyboard.size(); ++i) - m_state.keyboard[i] &= keyboard[i]; + if (update_keyboard) + XQueryKeymap(m_display, m_state.keyboard.data()); } std::string KeyboardMouse::GetName() const From 620955d397d4c40aca06a0811294ccb90a6434f0 Mon Sep 17 00:00:00 2001 From: Jeffrey Bosboom Date: Sat, 15 Apr 2023 02:13:03 -0700 Subject: [PATCH 4/4] XInput2: Listen to master devices only 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. --- .../ControllerInterface/Xlib/XInput2.cpp | 40 ++----------------- .../ControllerInterface/Xlib/XInput2.h | 1 - 2 files changed, 4 insertions(+), 37 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp index 4c47da4fc5..d90139fe03 100644 --- a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.cpp @@ -130,39 +130,6 @@ void PopulateDevices(void* const hwnd) XIFreeDeviceInfo(all_masters); } -// Apply the event mask to the device and all its slaves. Only used in the -// constructor. Remember, each KeyboardMouse has its own copy of the event -// stream, which is how multiple event masks can "coexist." -void KeyboardMouse::SelectEventsForDevice(XIEventMask* mask, int deviceid) -{ - // Set the event mask for the master device. - mask->deviceid = deviceid; - XISelectEvents(m_display, DefaultRootWindow(m_display), mask, 1); - - // Query all the master device's slaves and set the same event mask for - // those too. There are two reasons we want to do this. For mouse devices, - // we want the raw motion events, and only slaves (i.e. physical hardware - // devices) emit those. For keyboard devices, selecting slaves avoids - // dealing with key focus. - - int num_slaves; - XIDeviceInfo* const all_slaves = XIQueryDevice(m_display, XIAllDevices, &num_slaves); - - for (int i = 0; i < num_slaves; i++) - { - XIDeviceInfo* const slave = &all_slaves[i]; - if ((slave->use != XISlavePointer && slave->use != XISlaveKeyboard) || - slave->attachment != deviceid) - { - continue; - } - mask->deviceid = slave->deviceid; - XISelectEvents(m_display, DefaultRootWindow(m_display), mask, 1); - } - - XIFreeDeviceInfo(all_slaves); -} - KeyboardMouse::KeyboardMouse(Window window, int opcode, int pointer, int keyboard, double scroll_increment_) : m_window(window), xi_opcode(opcode), pointer_deviceid(pointer), keyboard_deviceid(keyboard), @@ -198,7 +165,8 @@ KeyboardMouse::KeyboardMouse(Window window, int opcode, int pointer, int keyboar mask.mask = mask_buf; mask.mask_len = sizeof(mask_buf); - SelectEventsForDevice(&mask, pointer_deviceid); + mask.deviceid = pointer_deviceid; + XISelectEvents(m_display, DefaultRootWindow(m_display), &mask, 1); } { @@ -209,8 +177,8 @@ KeyboardMouse::KeyboardMouse(Window window, int opcode, int pointer, int keyboar XIEventMask mask; mask.mask = mask_buf; mask.mask_len = sizeof(mask_buf); - - SelectEventsForDevice(&mask, keyboard_deviceid); + mask.deviceid = keyboard_deviceid; + XISelectEvents(m_display, DefaultRootWindow(m_display), &mask, 1); } // Keyboard Keys diff --git a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h index 52c78ab752..6a4f8436f1 100644 --- a/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h +++ b/Source/Core/InputCommon/ControllerInterface/Xlib/XInput2.h @@ -108,7 +108,6 @@ private: }; private: - void SelectEventsForDevice(XIEventMask* mask, int deviceid); void UpdateCursor(bool should_center_mouse); public: