From a0ecca1a84afb61809aa848405f119a9a7fff965 Mon Sep 17 00:00:00 2001 From: Filoppi Date: Sat, 15 May 2021 12:20:20 +0300 Subject: [PATCH] ControllerInterface: Implement ChangeWindow on DInput without recreating the devices Also polished DInput code in general to try and mitigate issue 11702. Added a lot of logging and comments. --- .../ControllerInterface.cpp | 20 +++++++ .../ControllerInterface/CoreDevice.h | 6 +++ .../ControllerInterface/DInput/DInput.cpp | 54 ++++++++++++++----- .../ControllerInterface/DInput/DInput.h | 2 + .../DInput/DInputKeyboardMouse.cpp | 39 ++++++++++---- .../DInput/DInputKeyboardMouse.h | 7 ++- .../ControllerInterface/Win32/Win32.cpp | 48 +++++++++++++---- .../ControllerInterface/Win32/Win32.h | 1 + 8 files changed, 140 insertions(+), 37 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index dd22ffe730..7c17a61448 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -135,6 +135,26 @@ void ControllerInterface::RefreshDevices(RefreshReason reason) // This wasn't thread safe in multiple device sources. std::lock_guard lk_population(m_devices_population_mutex); +#if defined(CIFACE_USE_WIN32) && !defined(CIFACE_USE_XLIB) && !defined(CIFACE_USE_OSX) + // If only the window changed, avoid removing and re-adding all devices. + // Instead only refresh devices that require the window handle. + if (reason == RefreshReason::WindowChangeOnly) + { + m_populating_devices_counter.fetch_add(1); + + { + std::lock_guard lk(m_devices_mutex); + // No need to do anything else in this case. + // Only (Win32) DInput needs the window handle to be updated. + ciface::Win32::ChangeWindow(m_wsi.render_window); + } + + if (m_populating_devices_counter.fetch_sub(1) == 1) + InvokeDevicesChangedCallbacks(); + return; + } +#endif + m_populating_devices_counter.fetch_add(1); // We lock m_devices_mutex here to make everything simpler. diff --git a/Source/Core/InputCommon/ControllerInterface/CoreDevice.h b/Source/Core/InputCommon/ControllerInterface/CoreDevice.h index e094d2477d..d3312568b2 100644 --- a/Source/Core/InputCommon/ControllerInterface/CoreDevice.h +++ b/Source/Core/InputCommon/ControllerInterface/CoreDevice.h @@ -125,6 +125,12 @@ public: // Currently handled on a per-backend basis but this could change. virtual bool IsValid() const { return true; } + // Returns true whether this device is "virtual/emulated", not linked + // to any actual physical device. Mostly used by keyboard and mouse devices, + // and to avoid uselessly recreating the device unless really necessary. + // Doesn't necessarily need to be set to true if the device is virtual. + virtual bool IsVirtualDevice() const { return false; } + // (e.g. Xbox 360 controllers have controller number LEDs which should match the ID we use.) virtual std::optional GetPreferredId() const; diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp index d508de613a..f5e105dc45 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp @@ -16,6 +16,8 @@ namespace ciface::DInput { +static IDirectInput8* s_idi8 = nullptr; + BOOL CALLBACK DIEnumDeviceObjectsCallback(LPCDIDEVICEOBJECTINSTANCE lpddoi, LPVOID pvRef) { ((std::list*)pvRef)->push_back(*lpddoi); @@ -42,29 +44,57 @@ std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device) } else { - ERROR_LOG_FMT(PAD, "GetProperty(DIPROP_PRODUCTNAME) failed."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "GetProperty(DIPROP_PRODUCTNAME) failed."); } return result; } +// Assumes hwnd had not changed from the previous call void PopulateDevices(HWND hwnd) { - // Remove unplugged devices. - g_controller_interface.RemoveDevice( - [](const auto* dev) { return dev->GetSource() == DINPUT_SOURCE_NAME && !dev->IsValid(); }); - - IDirectInput8* idi8; - if (FAILED(DirectInput8Create(GetModuleHandle(nullptr), DIRECTINPUT_VERSION, IID_IDirectInput8, - (LPVOID*)&idi8, nullptr))) + if (!s_idi8 && FAILED(DirectInput8Create(GetModuleHandle(nullptr), DIRECTINPUT_VERSION, + IID_IDirectInput8, (LPVOID*)&s_idi8, nullptr))) { - ERROR_LOG_FMT(PAD, "DirectInput8Create failed."); + ERROR_LOG_FMT(CONTROLLERINTERFACE, "DirectInput8Create failed."); return; } - InitKeyboardMouse(idi8, hwnd); - InitJoystick(idi8, hwnd); + // Remove old (invalid) devices. No need to ever remove the KeyboardMouse device. + // Note that if we have 2+ DInput controllers, not fully repopulating devices + // will mean that a device with index "2" could persist while there is no device with index "0". + // This is slightly inconsistent as when we refresh all devices, they will instead reset, and + // that happens a lot (for uncontrolled reasons, like starting/stopping the emulation). + g_controller_interface.RemoveDevice( + [](const auto* dev) { return dev->GetSource() == DINPUT_SOURCE_NAME && !dev->IsValid(); }); - idi8->Release(); + InitKeyboardMouse(s_idi8, hwnd); + InitJoystick(s_idi8, hwnd); +} + +void ChangeWindow(HWND hwnd) +{ + if (s_idi8) // Has init? Ignore if called before the first PopulateDevices() + { + // The KeyboardMouse device is marked as virtual device, so we avoid removing it. + // We need to force all the DInput joysticks to be destroyed now, or recreation would fail. + g_controller_interface.RemoveDevice( + [](const auto* dev) { + return dev->GetSource() == DINPUT_SOURCE_NAME && !dev->IsVirtualDevice(); + }, + true); + + SetKeyboardMouseWindow(hwnd); + InitJoystick(s_idi8, hwnd); + } +} + +void DeInit() +{ + if (s_idi8) + { + s_idi8->Release(); + s_idi8 = nullptr; + } } } // namespace ciface::DInput diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h index ed159089e0..8be064a1f1 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h @@ -20,4 +20,6 @@ BOOL CALLBACK DIEnumDevicesCallback(LPCDIDEVICEINSTANCE lpddi, LPVOID pvRef); std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device); void PopulateDevices(HWND hwnd); +void ChangeWindow(HWND hwnd); +void DeInit(); } // namespace ciface::DInput diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp index 0da09ad6e0..e952da6660 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp @@ -6,7 +6,8 @@ #include -#include +#include "Common/Logging/Log.h" +#include "Core/Core.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" #include "InputCommon/ControllerInterface/DInput/DInput.h" @@ -54,15 +55,18 @@ static const struct #include "InputCommon/ControllerInterface/DInput/NamedKeys.h" // NOLINT }; -// Prevent duplicate keyboard/mouse devices. -static bool s_keyboard_mouse_exists = false; +// Prevent duplicate keyboard/mouse devices. Modified by more threads. +static bool s_keyboard_mouse_exists; +static HWND s_hwnd; void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd) { if (s_keyboard_mouse_exists) return; - // mouse and keyboard are a combined device, to allow shift+click and stuff + s_hwnd = hwnd; + + // Mouse and keyboard are a combined device, to allow shift+click and stuff // if that's dumb, I will make a VirtualDevice class that just uses ranges of inputs/outputs from // other devices // so there can be a separated Keyboard and mouse, as well as combined KeyboardMouse @@ -70,6 +74,8 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd) LPDIRECTINPUTDEVICE8 kb_device = nullptr; LPDIRECTINPUTDEVICE8 mo_device = nullptr; + // These are "virtual" system devices, so they are always there even if we have no physical + // mouse and keyboard plugged into the computer if (SUCCEEDED(idi8->CreateDevice(GUID_SysKeyboard, &kb_device, nullptr)) && SUCCEEDED(kb_device->SetDataFormat(&c_dfDIKeyboard)) && SUCCEEDED(kb_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)) && @@ -77,16 +83,23 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd) SUCCEEDED(mo_device->SetDataFormat(&c_dfDIMouse2)) && SUCCEEDED(mo_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE))) { - g_controller_interface.AddDevice(std::make_shared(kb_device, mo_device, hwnd)); + g_controller_interface.AddDevice(std::make_shared(kb_device, mo_device)); return; } + ERROR_LOG_FMT(CONTROLLERINTERFACE, "KeyboardMouse device failed to be created"); + if (kb_device) kb_device->Release(); if (mo_device) mo_device->Release(); } +void SetKeyboardMouseWindow(HWND hwnd) +{ + s_hwnd = hwnd; +} + KeyboardMouse::~KeyboardMouse() { s_keyboard_mouse_exists = false; @@ -105,9 +118,8 @@ KeyboardMouse::~KeyboardMouse() } KeyboardMouse::KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, - const LPDIRECTINPUTDEVICE8 mo_device, HWND hwnd) - : m_kb_device(kb_device), m_mo_device(mo_device), m_hwnd(hwnd), m_last_update(GetTickCount()), - m_state_in() + const LPDIRECTINPUTDEVICE8 mo_device) + : m_kb_device(kb_device), m_mo_device(mo_device), m_last_update(GetTickCount()), m_state_in() { s_keyboard_mouse_exists = true; @@ -160,11 +172,11 @@ void KeyboardMouse::UpdateCursorInput() // Get the cursor position relative to the upper left corner of the current window // (separate or render to main) - ScreenToClient(m_hwnd, &point); + ScreenToClient(s_hwnd, &point); - // Get the size of the current window. (In my case Rect.top and Rect.left was zero.) + // Get the size of the current window (in my case Rect.top and Rect.left was zero). RECT rect; - GetClientRect(m_hwnd, &rect); + GetClientRect(s_hwnd, &rect); // Width and height are the size of the rendering window. They could be 0 const auto win_width = std::max(rect.right - rect.left, 1l); @@ -236,6 +248,11 @@ int KeyboardMouse::GetSortPriority() const return 5; } +bool KeyboardMouse::IsVirtualDevice() const +{ + return true; +} + // names std::string KeyboardMouse::Key::GetName() const { diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h index 28c6407471..b0fd39c00d 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h @@ -16,6 +16,7 @@ namespace ciface::DInput void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd); using RelativeMouseState = RelativeInputState>; +void SetKeyboardMouseWindow(HWND hwnd); class KeyboardMouse : public Core::Device { @@ -96,13 +97,13 @@ private: public: void UpdateInput() override; - KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device, - HWND hwnd); + KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device); ~KeyboardMouse(); std::string GetName() const override; std::string GetSource() const override; int GetSortPriority() const override; + bool IsVirtualDevice() const override; private: void UpdateCursorInput(); @@ -110,8 +111,6 @@ private: const LPDIRECTINPUTDEVICE8 m_kb_device; const LPDIRECTINPUTDEVICE8 m_mo_device; - const HWND m_hwnd; - DWORD m_last_update; State m_state_in; }; diff --git a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp index b7cf19a13d..066c50a693 100644 --- a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp @@ -11,6 +11,7 @@ #include #include "Common/Event.h" +#include "Common/Flag.h" #include "Common/Logging/Log.h" #include "Common/ScopeGuard.h" #include "Common/Thread.h" @@ -19,20 +20,30 @@ constexpr UINT WM_DOLPHIN_STOP = WM_USER; -static Common::Event s_done_populating; -static std::atomic s_hwnd; +static Common::Event s_received_device_change_event; +// Dolphin's render window +static HWND s_hwnd; +// Windows messaging window (hidden) static HWND s_message_window; static std::thread s_thread; +static Common::Flag s_first_populate_devices_asked; static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) { if (message == WM_INPUT_DEVICE_CHANGE) { - g_controller_interface.PlatformPopulateDevices([] { - ciface::DInput::PopulateDevices(s_hwnd); - ciface::XInput::PopulateDevices(); - }); - s_done_populating.Set(); + // Windows automatically sends this message before we ask for it and before we are "ready" to + // listen for it. + if (s_first_populate_devices_asked.IsSet()) + { + s_received_device_change_event.Set(); + // TODO: we could easily use the message passed alongside this event, which tells + // whether a device was added or removed, to avoid removing old, still connected, devices + g_controller_interface.PlatformPopulateDevices([] { + ciface::DInput::PopulateDevices(s_hwnd); + ciface::XInput::PopulateDevices(); + }); + } } return DefWindowProc(hwnd, message, wparam, lparam); @@ -125,10 +136,14 @@ void ciface::Win32::PopulateDevices(void* hwnd) if (s_thread.joinable()) { s_hwnd = static_cast(hwnd); - s_done_populating.Reset(); + s_first_populate_devices_asked.Set(); + s_received_device_change_event.Reset(); + // Do this forced devices refresh in the messaging thread so it won't cause any race conditions PostMessage(s_message_window, WM_INPUT_DEVICE_CHANGE, 0, 0); - if (!s_done_populating.WaitFor(std::chrono::seconds(10))) - ERROR_LOG_FMT(CONTROLLERINTERFACE, "win32 timed out when trying to populate devices"); + std::thread([] { + if (!s_received_device_change_event.WaitFor(std::chrono::seconds(5))) + ERROR_LOG_FMT(CONTROLLERINTERFACE, "win32 timed out when trying to populate devices"); + }).detach(); } else { @@ -137,6 +152,15 @@ void ciface::Win32::PopulateDevices(void* hwnd) } } +void ciface::Win32::ChangeWindow(void* hwnd) +{ + if (s_thread.joinable()) // "Has init?" + { + s_hwnd = static_cast(hwnd); + ciface::DInput::ChangeWindow(s_hwnd); + } +} + void ciface::Win32::DeInit() { NOTICE_LOG_FMT(CONTROLLERINTERFACE, "win32 DeInit"); @@ -145,7 +169,11 @@ void ciface::Win32::DeInit() PostMessage(s_message_window, WM_DOLPHIN_STOP, 0, 0); s_thread.join(); s_message_window = nullptr; + s_received_device_change_event.Reset(); + s_first_populate_devices_asked.Clear(); + DInput::DeInit(); } + s_hwnd = nullptr; XInput::DeInit(); } diff --git a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h index acf72d76a0..8d390022a4 100644 --- a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h +++ b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.h @@ -8,5 +8,6 @@ namespace ciface::Win32 { void Init(void* hwnd); void PopulateDevices(void* hwnd); +void ChangeWindow(void* hwnd); void DeInit(); } // namespace ciface::Win32