From 125971d9f2cbfff81985610a750364a54c3d452d Mon Sep 17 00:00:00 2001 From: Filoppi Date: Sat, 27 Nov 2021 14:31:04 +0200 Subject: [PATCH] InputCommon: fix default input config default device not being loaded/found Fixes bug: https://bugs.dolphin-emu.org/issues/12744 Before https://github.com/dolphin-emu/dolphin/commit/e1e3db13baabefa89991388d37db0bb260c4f535 the ControllerInterface m_devices_mutex was "wrongfully" locked for the whole Initialize() call, which included the first device population refresh, this has the unwanted (accidental) consequence of often preventing the different pads (GC Pad, Wii Contollers, ...) input configs from loading until that mutex was released (the input config defaults loading was blocked in EmulatedController::LoadDefaults()), which meant that the devices population would often have the time to finish adding its first device, which would then be selected as default device (by design, the first device added to the CI is the default default device, usually the "Keyboard and Mouse" device). After the commit mentioned above removed the unnecessary m_devices_mutex calls, the default default device would fail to load (be found) causing the default input mappings, which are specifically written for the default default device on every platform, to not be bound to any physical device input, breaking input on new dolphin installations (until a user tried to customize the default device manually). Default devices are now always added synchronously to avoid the problem, and so they should in the future (I added comments and warnings to help with that) --- Source/Core/DolphinQt/MainWindow.cpp | 7 +++++++ .../ControllerInterface.cpp | 4 ++++ .../ControllerInterface/CoreDevice.cpp | 10 +++++++++- .../ControllerInterface/CoreDevice.h | 2 ++ .../DInput/DInputJoystick.h | 1 + .../DualShockUDPClient/DualShockUDPClient.cpp | 2 +- .../Wiimote/WiimoteController.cpp | 2 +- .../ControllerInterface/Win32/Win32.cpp | 20 +++++++++---------- .../ControllerInterface/XInput/XInput.h | 1 + 9 files changed, 35 insertions(+), 14 deletions(-) diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp index aacdea6a71..ca32c71dcd 100644 --- a/Source/Core/DolphinQt/MainWindow.cpp +++ b/Source/Core/DolphinQt/MainWindow.cpp @@ -319,6 +319,13 @@ void MainWindow::InitControllers() return; g_controller_interface.Initialize(GetWindowSystemInfo(windowHandle())); + if (!g_controller_interface.HasDefaultDevice()) + { + // Note that the CI default device could be still temporarily removed at any time + WARN_LOG(CONTROLLERINTERFACE, + "No default device has been added in time. EmulatedController(s) defaulting adds" + " input mappings made for a specific default device depending on the platform"); + } Pad::Initialize(); Pad::InitializeGBA(); Keyboard::Initialize(); diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index aaaa3971e6..1c4dc71da8 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -159,6 +159,10 @@ void ControllerInterface::RefreshDevices(RefreshReason reason) // with their own PlatformPopulateDevices(). // This means that devices might end up in different order, unless we override their priority. // It also means they might appear as "disconnected" in the Qt UI for a tiny bit of time. + // This helps the emulation and host thread to not stall when repopulating devices for any reason. + // Every platform that adds a device that is meant to be used as default device should try to not + // do it async, to not risk the emulated controllers default config loading not finding a default + // device. #ifdef CIFACE_USE_WIN32 ciface::Win32::PopulateDevices(m_wsi.render_window); diff --git a/Source/Core/InputCommon/ControllerInterface/CoreDevice.cpp b/Source/Core/InputCommon/ControllerInterface/CoreDevice.cpp index 3d224b3cf4..4e46179fc6 100644 --- a/Source/Core/InputCommon/ControllerInterface/CoreDevice.cpp +++ b/Source/Core/InputCommon/ControllerInterface/CoreDevice.cpp @@ -258,10 +258,18 @@ std::vector DeviceContainer::GetAllDeviceStrings() const return device_strings; } +bool DeviceContainer::HasDefaultDevice() const +{ + std::lock_guard lk(m_devices_mutex); + // Devices are already sorted by priority + return !m_devices.empty() && m_devices[0]->GetSortPriority() >= 0; +} + std::string DeviceContainer::GetDefaultDeviceString() const { std::lock_guard lk(m_devices_mutex); - if (m_devices.empty()) + // Devices are already sorted by priority + if (m_devices.empty() || m_devices[0]->GetSortPriority() < 0) return ""; DeviceQualifier device_qualifier; diff --git a/Source/Core/InputCommon/ControllerInterface/CoreDevice.h b/Source/Core/InputCommon/ControllerInterface/CoreDevice.h index 2840f43463..227a1febbe 100644 --- a/Source/Core/InputCommon/ControllerInterface/CoreDevice.h +++ b/Source/Core/InputCommon/ControllerInterface/CoreDevice.h @@ -136,6 +136,7 @@ public: // Use this to change the order in which devices are sorted in their list. // A higher priority means it will be one of the first ones (smaller index), making it more // likely to be index 0, which is automatically set as the default device when there isn't one. + // Every platform should have at least one device with priority >= 0. virtual int GetSortPriority() const { return 0; } const std::vector& Inputs() const { return m_inputs; } @@ -226,6 +227,7 @@ public: Device::Output* FindOutput(std::string_view name, const Device* def_dev) const; std::vector GetAllDeviceStrings() const; + bool HasDefaultDevice() const; std::string GetDefaultDeviceString() const; std::shared_ptr FindDevice(const DeviceQualifier& devq) const; diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h index a142aeba00..b80b84d416 100644 --- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h +++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.h @@ -64,6 +64,7 @@ public: std::string GetName() const override; std::string GetSource() const override; + int GetSortPriority() const override { return -2; } bool IsValid() const final override; diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp index 7342ef4d47..a16f9cfc23 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp @@ -136,7 +136,7 @@ public: std::string GetSource() const final override; std::optional GetPreferredId() const final override; // Always add these at the end, given their hotplug nature - int GetSortPriority() const override { return -2; } + int GetSortPriority() const override { return -4; } private: void ResetPadData(); diff --git a/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp b/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp index dbdf53fc92..2cf459ca11 100644 --- a/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Wiimote/WiimoteController.cpp @@ -323,7 +323,7 @@ std::string Device::GetSource() const // Always add these at the end, given their hotplug nature int Device::GetSortPriority() const { - return -1; + return -3; } void Device::RunTasks() diff --git a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp index f1066cfa3c..21ee1fa425 100644 --- a/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp @@ -7,9 +7,9 @@ #include #include +#include #include -#include "Common/Event.h" #include "Common/Flag.h" #include "Common/Logging/Log.h" #include "Common/ScopeGuard.h" @@ -19,12 +19,12 @@ constexpr UINT WM_DOLPHIN_STOP = WM_USER; -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 std::mutex s_populate_mutex; static Common::Flag s_first_populate_devices_asked; static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) @@ -35,7 +35,7 @@ static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam, LPARA // listen for it. if (s_first_populate_devices_asked.IsSet()) { - s_received_device_change_event.Set(); + std::lock_guard lk_population(s_populate_mutex); // 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([] { @@ -135,14 +135,12 @@ void ciface::Win32::PopulateDevices(void* hwnd) if (s_thread.joinable()) { s_hwnd = static_cast(hwnd); + // To avoid blocking this thread until the async population has finished, directly do it here + // (we need the DInput Keyboard and Mouse "default" device to always be added without any wait). + std::lock_guard lk_population(s_populate_mutex); 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); - 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(); + ciface::DInput::PopulateDevices(s_hwnd); + ciface::XInput::PopulateDevices(); } else { @@ -156,6 +154,7 @@ void ciface::Win32::ChangeWindow(void* hwnd) if (s_thread.joinable()) // "Has init?" { s_hwnd = static_cast(hwnd); + std::lock_guard lk_population(s_populate_mutex); ciface::DInput::ChangeWindow(s_hwnd); } } @@ -168,7 +167,6 @@ 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(); } diff --git a/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h b/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h index bebb20cb4c..d2e1898c5a 100644 --- a/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h +++ b/Source/Core/InputCommon/ControllerInterface/XInput/XInput.h @@ -31,6 +31,7 @@ public: std::string GetName() const override; std::string GetSource() const override; std::optional GetPreferredId() const override; + int GetSortPriority() const override { return -1; } void UpdateInput() override;