From eadbdd6bc389cd89c3e3e57a31dabaeadf161865 Mon Sep 17 00:00:00 2001 From: Jordan Woyak Date: Sat, 9 Mar 2019 09:57:37 -0600 Subject: [PATCH] ControllerInterface/Win32: Prevent devcies from losing their "id" on a hotplug event. --- .../ControllerInterface.cpp | 37 ++++++++++------ .../ControllerInterface/DInput/DInput.cpp | 21 ++++++--- .../DInput/DInputJoystick.cpp | 43 +++++++++++++++++-- .../DInput/DInputJoystick.h | 6 ++- .../DInput/DInputKeyboardMouse.cpp | 17 +++++--- .../DInput/DInputKeyboardMouse.h | 6 +-- .../ControllerInterface/Device.cpp | 5 +++ .../InputCommon/ControllerInterface/Device.h | 11 ++++- .../ControllerInterface/XInput/XInput.cpp | 5 +++ .../ControllerInterface/XInput/XInput.h | 9 ++-- 10 files changed, 121 insertions(+), 39 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index 3a4f374c9d..bb5ccb3f0e 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -93,6 +93,9 @@ void ControllerInterface::RefreshDevices() m_is_populating_devices = true; + // Make sure shared_ptr objects are released before repopulating. + InvokeDevicesChangedCallbacks(); + #ifdef CIFACE_USE_WIN32 ciface::Win32::PopulateDevices(m_wsi.render_surface); #endif @@ -179,21 +182,29 @@ void ControllerInterface::AddDevice(std::shared_ptr device { std::lock_guard lk(m_devices_mutex); - // Try to find an ID for this device - int id = 0; - while (true) + + const auto is_id_in_use = [&device, this](int id) { + return std::any_of(m_devices.begin(), m_devices.end(), [&device, &id](const auto& d) { + return d->GetSource() == device->GetSource() && d->GetName() == device->GetName() && + d->GetId() == id; + }); + }; + + const auto preferred_id = device->GetPreferredId(); + if (preferred_id.has_value() && !is_id_in_use(*preferred_id)) { - const auto it = - std::find_if(m_devices.begin(), m_devices.end(), [&device, &id](const auto& d) { - return d->GetSource() == device->GetSource() && d->GetName() == device->GetName() && - d->GetId() == id; - }); - if (it == m_devices.end()) // no device with the same name with this ID, so we can use it - break; - else - id++; + // Use the device's preferred ID if available. + device->SetId(*preferred_id); + } + else + { + // Find the first available ID to use. + int id = 0; + while (is_id_in_use(id)) + ++id; + + device->SetId(id); } - device->SetId(id); NOTICE_LOG(SERIALINTERFACE, "Added device: %s", device->GetQualifiedName().c_str()); m_devices.emplace_back(std::move(device)); diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp index e65da0eb78..cb484cc6ca 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp @@ -3,9 +3,11 @@ // Refer to the license.txt file included. #include "InputCommon/ControllerInterface/DInput/DInput.h" -#include "Common/StringUtil.h" -#include "InputCommon/ControllerInterface/ControllerInterface.h" +#include "Common/Logging/Log.h" +#include "Common/StringUtil.h" + +#include "InputCommon/ControllerInterface/ControllerInterface.h" #include "InputCommon/ControllerInterface/DInput/DInputJoystick.h" #include "InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h" @@ -40,25 +42,32 @@ std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device) { result = StripSpaces(UTF16ToUTF8(str.wsz)); } + else + { + ERROR_LOG(PAD, "GetProperty(DIPROP_PRODUCTNAME) failed."); + } return result; } void PopulateDevices(HWND hwnd) { - g_controller_interface.RemoveDevice([](const auto* dev) { return dev->GetSource() == "DInput"; }); + // 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))) { + ERROR_LOG(PAD, "DirectInput8Create failed."); return; } - InitKeyboardMouse(idi8, hwnd); + InitKeyboardMouse(idi8); InitJoystick(idi8, hwnd); idi8->Release(); } -} -} +} // namespace DInput +} // namespace ciface diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp index 3bb8dd64a3..e919952bec 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp @@ -4,8 +4,9 @@ #include #include -#include +#include #include +#include #include "Common/Logging/Log.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" @@ -17,7 +18,19 @@ namespace ciface { namespace DInput { -#define DATA_BUFFER_SIZE 32 +constexpr DWORD DATA_BUFFER_SIZE = 32; + +struct GUIDComparator +{ + bool operator()(const GUID& left, const GUID& right) const + { + static_assert(std::is_trivially_copyable_v); + + return memcmp(&left, &right, sizeof(left)) < 0; + } +}; + +static std::set s_guids_in_use; void InitJoystick(IDirectInput8* const idi8, HWND hwnd) { @@ -28,12 +41,18 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd) std::unordered_set xinput_guids = GetXInputGUIDS(); for (DIDEVICEINSTANCE& joystick : joysticks) { - // skip XInput Devices + // Skip XInput Devices if (xinput_guids.count(joystick.guidProduct.Data1)) { continue; } + // Skip devices we are already using. + if (s_guids_in_use.count(joystick.guidInstance)) + { + continue; + } + LPDIRECTINPUTDEVICE8 js_device; if (SUCCEEDED(idi8->CreateDevice(joystick.guidInstance, &js_device, nullptr))) { @@ -55,7 +74,9 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd) } } + s_guids_in_use.insert(joystick.guidInstance); auto js = std::make_shared(js_device); + // only add if it has some inputs/outputs if (js->Inputs().size() || js->Outputs().size()) g_controller_interface.AddDevice(std::move(js)); @@ -161,6 +182,17 @@ Joystick::Joystick(/*const LPCDIDEVICEINSTANCE lpddi, */ const LPDIRECTINPUTDEVI Joystick::~Joystick() { + DIDEVICEINSTANCE info = {}; + info.dwSize = sizeof(info); + if (SUCCEEDED(m_device->GetDeviceInfo(&info))) + { + s_guids_in_use.erase(info.guidInstance); + } + else + { + ERROR_LOG(PAD, "DInputJoystick: GetDeviceInfo failed."); + } + DeInitForceFeedback(); m_device->Unacquire(); @@ -177,7 +209,10 @@ std::string Joystick::GetSource() const return DINPUT_SOURCE_NAME; } -// update IO +bool Joystick::IsValid() const +{ + return SUCCEEDED(m_device->Acquire()); +} void Joystick::UpdateInput() { diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h index 82a6b61f0f..074b879051 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h @@ -68,6 +68,8 @@ public: std::string GetName() const override; std::string GetSource() const override; + bool IsValid() const final override; + private: const LPDIRECTINPUTDEVICE8 m_device; @@ -75,5 +77,5 @@ private: bool m_buffered; }; -} -} +} // namespace DInput +} // namespace ciface diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp index 384174a3f1..23746e61db 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp @@ -29,12 +29,13 @@ static const struct #include "InputCommon/ControllerInterface/DInput/NamedKeys.h" // NOLINT }; -// lil silly -static HWND m_hwnd; +// Prevent duplicate keyboard/mouse devices. +static bool s_keyboard_mouse_exists = false; -void InitKeyboardMouse(IDirectInput8* const idi8, HWND _hwnd) +void InitKeyboardMouse(IDirectInput8* const idi8) { - m_hwnd = _hwnd; + if (s_keyboard_mouse_exists) + return; // 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 @@ -63,6 +64,8 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND _hwnd) KeyboardMouse::~KeyboardMouse() { + s_keyboard_mouse_exists = false; + // kb m_kb_device->Unacquire(); m_kb_device->Release(); @@ -75,6 +78,8 @@ KeyboardMouse::KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device) : m_kb_device(kb_device), m_mo_device(mo_device) { + s_keyboard_mouse_exists = true; + m_kb_device->Acquire(); m_mo_device->Acquire(); @@ -237,5 +242,5 @@ ControlState KeyboardMouse::Cursor::GetState() const { return std::max(0.0, ControlState(m_axis) / (m_positive ? 1.0 : -1.0)); } -} -} +} // namespace DInput +} // namespace ciface diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h index c80a13e343..a8de55fe7a 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h @@ -13,7 +13,7 @@ namespace ciface { namespace DInput { -void InitKeyboardMouse(IDirectInput8* const idi8, HWND _hwnd); +void InitKeyboardMouse(IDirectInput8* const idi8); class KeyboardMouse : public Core::Device { @@ -98,5 +98,5 @@ private: DWORD m_last_update; State m_state_in; }; -} -} +} // namespace DInput +} // namespace ciface diff --git a/Source/Core/InputCommon/ControllerInterface/Device.cpp b/Source/Core/InputCommon/ControllerInterface/Device.cpp index 14f5cb23d9..4cd50fa08e 100644 --- a/Source/Core/InputCommon/ControllerInterface/Device.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Device.cpp @@ -31,6 +31,11 @@ Device::~Device() delete output; } +std::optional Device::GetPreferredId() const +{ + return {}; +} + void Device::AddInput(Device::Input* const i) { m_inputs.push_back(i); diff --git a/Source/Core/InputCommon/ControllerInterface/Device.h b/Source/Core/InputCommon/ControllerInterface/Device.h index 7d7623ba03..defbb6cd4a 100644 --- a/Source/Core/InputCommon/ControllerInterface/Device.h +++ b/Source/Core/InputCommon/ControllerInterface/Device.h @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -82,7 +83,7 @@ public: class Output : public Control { public: - virtual ~Output() {} + virtual ~Output() = default; virtual void SetState(ControlState state) = 0; Output* ToOutput() override { return this; } }; @@ -95,9 +96,17 @@ public: virtual std::string GetSource() const = 0; std::string GetQualifiedName() const; virtual void UpdateInput() {} + + // May be overridden to implement hotplug removal. + // Currently handled on a per-backend basis but this could change. virtual bool IsValid() const { return true; } + + // (e.g. Xbox 360 controllers have controller number LEDs which should match the ID we use.) + virtual std::optional GetPreferredId() const; + const std::vector& Inputs() const { return m_inputs; } const std::vector& Outputs() const { return m_outputs; } + Input* FindInput(const std::string& name) const; Output* FindOutput(const std::string& name) const; diff --git a/Source/Core/InputCommon/ControllerInterface/XInput/XInput.cpp b/Source/Core/InputCommon/ControllerInterface/XInput/XInput.cpp index f3e984d897..d6ecdb9d81 100644 --- a/Source/Core/InputCommon/ControllerInterface/XInput/XInput.cpp +++ b/Source/Core/InputCommon/ControllerInterface/XInput/XInput.cpp @@ -194,6 +194,11 @@ void Device::UpdateMotors() } } +std::optional Device::GetPreferredId() const +{ + return m_index; +} + // GET name/source/id std::string Device::Button::GetName() const diff --git a/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h b/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h index f2a81e3840..0b299e6324 100644 --- a/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h +++ b/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h @@ -92,8 +92,9 @@ public: Device(const XINPUT_CAPABILITIES& capabilities, u8 index); - std::string GetName() const override; - std::string GetSource() const override; + std::string GetName() const final override; + std::string GetSource() const final override; + std::optional GetPreferredId() const final override; void UpdateMotors(); @@ -104,5 +105,5 @@ private: const BYTE m_subtype; const u8 m_index; }; -} -} +} // namespace XInput +} // namespace ciface