From 7355b5f70d2bdf68c356873f2311025a81c7972a Mon Sep 17 00:00:00 2001 From: Michael M Date: Thu, 9 Nov 2017 12:14:21 -0800 Subject: [PATCH 1/4] ControllerInterface: invoke callbacks in AddDevice/RemoveDevice Some backends already cause this to happen, so make it consistent across systems. --- .../ControllerInterface.cpp | 55 ++++++++++++------- .../ControllerInterface/ControllerInterface.h | 3 +- .../ControllerInterface/Device.cpp | 9 ++- .../InputCommon/ControllerInterface/Device.h | 1 + .../ControllerInterface/OSX/OSX.mm | 5 -- .../ControllerInterface/evdev/evdev.cpp | 4 -- 6 files changed, 47 insertions(+), 30 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index 1a72fa172c..4f35c4a989 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -2,9 +2,11 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include "InputCommon/ControllerInterface/ControllerInterface.h" + #include -#include "InputCommon/ControllerInterface/ControllerInterface.h" +#include "Common/Logging/Log.h" #ifdef CIFACE_USE_XINPUT #include "InputCommon/ControllerInterface/XInput/XInput.h" @@ -165,30 +167,45 @@ void ControllerInterface::Shutdown() 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 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++; + std::lock_guard lk(m_devices_mutex); + // Try to find an ID for this device + int id = 0; + while (true) + { + 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++; + } + device->SetId(id); + + NOTICE_LOG(SERIALINTERFACE, "Added device: %s", device->GetQualifiedName().c_str()); + m_devices.emplace_back(std::move(device)); } - device->SetId(id); - m_devices.emplace_back(std::move(device)); + InvokeHotplugCallbacks(); } void ControllerInterface::RemoveDevice(std::function callback) { - std::lock_guard lk(m_devices_mutex); - m_devices.erase(std::remove_if(m_devices.begin(), m_devices.end(), - [&callback](const auto& dev) { return callback(dev.get()); }), - m_devices.end()); + { + std::lock_guard lk(m_devices_mutex); + auto it = std::remove_if(m_devices.begin(), m_devices.end(), [&callback](const auto& dev) { + if (callback(dev.get())) + { + NOTICE_LOG(SERIALINTERFACE, "Removed device: %s", dev->GetQualifiedName().c_str()); + return true; + } + return false; + }); + m_devices.erase(it, m_devices.end()); + } + InvokeHotplugCallbacks(); } // diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h index 23afab7d67..eb94a8de67 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h @@ -54,9 +54,10 @@ public: void UpdateInput(); void RegisterHotplugCallback(std::function callback); - void InvokeHotplugCallbacks() const; private: + void InvokeHotplugCallbacks() const; + std::vector> m_hotplug_callbacks; bool m_is_init; void* m_hwnd; diff --git a/Source/Core/InputCommon/ControllerInterface/Device.cpp b/Source/Core/InputCommon/ControllerInterface/Device.cpp index f83dae268e..ee28115adc 100644 --- a/Source/Core/InputCommon/ControllerInterface/Device.cpp +++ b/Source/Core/InputCommon/ControllerInterface/Device.cpp @@ -2,12 +2,14 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include "InputCommon/ControllerInterface/Device.h" + #include #include #include #include -#include "InputCommon/ControllerInterface/Device.h" +#include "Common/StringUtil.h" namespace ciface { @@ -39,6 +41,11 @@ void Device::AddOutput(Device::Output* const o) m_outputs.push_back(o); } +std::string Device::GetQualifiedName() const +{ + return StringFromFormat("%s/%i/%s", this->GetSource().c_str(), GetId(), this->GetName().c_str()); +} + Device::Input* Device::FindInput(const std::string& name) const { for (Input* input : m_inputs) diff --git a/Source/Core/InputCommon/ControllerInterface/Device.h b/Source/Core/InputCommon/ControllerInterface/Device.h index 7b601cc775..3bf9621e45 100644 --- a/Source/Core/InputCommon/ControllerInterface/Device.h +++ b/Source/Core/InputCommon/ControllerInterface/Device.h @@ -79,6 +79,7 @@ public: void SetId(int id) { m_id = id; } virtual std::string GetName() const = 0; virtual std::string GetSource() const = 0; + std::string GetQualifiedName() const; virtual void UpdateInput() {} virtual bool IsValid() const { return true; } const std::vector& Inputs() const { return m_inputs; } diff --git a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm index b13701209e..a923c1ea26 100644 --- a/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm +++ b/Source/Core/InputCommon/ControllerInterface/OSX/OSX.mm @@ -156,8 +156,6 @@ static void DeviceRemovalCallback(void* inContext, IOReturn inResult, void* inSe return false; }); - g_controller_interface.InvokeHotplugCallbacks(); - NOTICE_LOG(SERIALINTERFACE, "Removed device: %s", GetDeviceRefName(inIOHIDDeviceRef).c_str()); } static void DeviceMatchingCallback(void* inContext, IOReturn inResult, void* inSender, @@ -174,9 +172,6 @@ static void DeviceMatchingCallback(void* inContext, IOReturn inResult, void* inS { g_controller_interface.AddDevice(std::make_shared(inIOHIDDeviceRef, name)); } - - NOTICE_LOG(SERIALINTERFACE, "Added device: %s", name.c_str()); - g_controller_interface.InvokeHotplugCallbacks(); } void Init(void* window) diff --git a/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp b/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp index 6d9b7c1091..0223637b13 100644 --- a/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp +++ b/Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp @@ -92,9 +92,7 @@ static void HotplugThreadFunc() g_controller_interface.RemoveDevice([&name](const auto& device) { return device->GetSource() == "evdev" && device->GetName() == name && !device->IsValid(); }); - NOTICE_LOG(SERIALINTERFACE, "Removed device: %s", name.c_str()); s_devnode_name_map.erase(devnode); - g_controller_interface.InvokeHotplugCallbacks(); } // Only react to "device added" events for evdev devices that we can access. else if (strcmp(action, "add") == 0 && access(devnode, W_OK) == 0) @@ -107,8 +105,6 @@ static void HotplugThreadFunc() { g_controller_interface.AddDevice(std::move(device)); s_devnode_name_map.insert(std::pair(devnode, name)); - NOTICE_LOG(SERIALINTERFACE, "Added new device: %s", name.c_str()); - g_controller_interface.InvokeHotplugCallbacks(); } } udev_device_unref(dev); From 1ed7532af850691eea8d7eee502f59ed20a37694 Mon Sep 17 00:00:00 2001 From: Michael M Date: Sat, 4 Nov 2017 07:36:30 -0700 Subject: [PATCH 2/4] ControllerInterface: HotplugCallbacks -> DevicesChangedCallbacks --- Source/Core/Core/HW/GCKeyboard.cpp | 2 +- Source/Core/Core/HW/GCPad.cpp | 2 +- Source/Core/Core/HW/Wiimote.cpp | 2 +- Source/Core/Core/HotkeyManager.cpp | 2 +- .../ControllerInterface.cpp | 20 +++++++++---------- .../ControllerInterface/ControllerInterface.h | 7 +++---- 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/Source/Core/Core/HW/GCKeyboard.cpp b/Source/Core/Core/HW/GCKeyboard.cpp index 300ed0608c..b5ec80bf53 100644 --- a/Source/Core/Core/HW/GCKeyboard.cpp +++ b/Source/Core/Core/HW/GCKeyboard.cpp @@ -37,7 +37,7 @@ void Initialize() s_config.CreateController(i); } - g_controller_interface.RegisterHotplugCallback(LoadConfig); + g_controller_interface.RegisterDevicesChangedCallback(LoadConfig); // Load the saved controller config s_config.LoadConfig(true); diff --git a/Source/Core/Core/HW/GCPad.cpp b/Source/Core/Core/HW/GCPad.cpp index fcf9f524c5..9cefbb4db1 100644 --- a/Source/Core/Core/HW/GCPad.cpp +++ b/Source/Core/Core/HW/GCPad.cpp @@ -34,7 +34,7 @@ void Initialize() s_config.CreateController(i); } - g_controller_interface.RegisterHotplugCallback(LoadConfig); + g_controller_interface.RegisterDevicesChangedCallback(LoadConfig); // Load the saved controller config s_config.LoadConfig(true); diff --git a/Source/Core/Core/HW/Wiimote.cpp b/Source/Core/Core/HW/Wiimote.cpp index 0e0d3f9f78..178ee75540 100644 --- a/Source/Core/Core/HW/Wiimote.cpp +++ b/Source/Core/Core/HW/Wiimote.cpp @@ -80,7 +80,7 @@ void Initialize(InitializeMode init_mode) s_config.CreateController(i); } - g_controller_interface.RegisterHotplugCallback(LoadConfig); + g_controller_interface.RegisterDevicesChangedCallback(LoadConfig); LoadConfig(); diff --git a/Source/Core/Core/HotkeyManager.cpp b/Source/Core/Core/HotkeyManager.cpp index ed586e1c7c..b4574de51a 100644 --- a/Source/Core/Core/HotkeyManager.cpp +++ b/Source/Core/Core/HotkeyManager.cpp @@ -213,7 +213,7 @@ void Initialize() if (s_config.ControllersNeedToBeCreated()) s_config.CreateController(); - g_controller_interface.RegisterHotplugCallback(LoadConfig); + g_controller_interface.RegisterDevicesChangedCallback(LoadConfig); // load the saved controller config s_config.LoadConfig(true); diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index 4f35c4a989..1552cd7dc2 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -188,7 +188,7 @@ void ControllerInterface::AddDevice(std::shared_ptr device NOTICE_LOG(SERIALINTERFACE, "Added device: %s", device->GetQualifiedName().c_str()); m_devices.emplace_back(std::move(device)); } - InvokeHotplugCallbacks(); + InvokeDevicesChangedCallbacks(); } void ControllerInterface::RemoveDevice(std::function callback) @@ -205,7 +205,7 @@ void ControllerInterface::RemoveDevice(std::function callback) +void ControllerInterface::RegisterDevicesChangedCallback(std::function callback) { - m_hotplug_callbacks.emplace_back(std::move(callback)); + m_devices_changed_callbacks.emplace_back(std::move(callback)); } // -// InvokeHotplugCallbacks +// InvokeDevicesChangedCallbacks // // Invoke all callbacks that were registered // -void ControllerInterface::InvokeHotplugCallbacks() const +void ControllerInterface::InvokeDevicesChangedCallbacks() const { - for (const auto& callback : m_hotplug_callbacks) + for (const auto& callback : m_devices_changed_callbacks) callback(); } diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h index eb94a8de67..e70c135b3e 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h @@ -53,12 +53,11 @@ public: bool IsInit() const { return m_is_init; } void UpdateInput(); - void RegisterHotplugCallback(std::function callback); + void RegisterDevicesChangedCallback(std::function callback); + void InvokeDevicesChangedCallbacks() const; private: - void InvokeHotplugCallbacks() const; - - std::vector> m_hotplug_callbacks; + std::vector> m_devices_changed_callbacks; bool m_is_init; void* m_hwnd; }; From fd7cbd633edbac9e38592b9253a507410d7d6553 Mon Sep 17 00:00:00 2001 From: Michael M Date: Fri, 10 Nov 2017 12:29:25 -0800 Subject: [PATCH 3/4] ControllerInterface: add mutex around callbacks vector --- .../ControllerInterface/ControllerInterface.cpp | 4 +++- .../InputCommon/ControllerInterface/ControllerInterface.h | 8 +++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index 1552cd7dc2..9e3c7ba7af 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -4,7 +4,7 @@ #include "InputCommon/ControllerInterface/ControllerInterface.h" -#include +#include #include "Common/Logging/Log.h" @@ -232,6 +232,7 @@ void ControllerInterface::UpdateInput() // void ControllerInterface::RegisterDevicesChangedCallback(std::function callback) { + std::lock_guard lk(m_callbacks_mutex); m_devices_changed_callbacks.emplace_back(std::move(callback)); } @@ -242,6 +243,7 @@ void ControllerInterface::RegisterDevicesChangedCallback(std::function c // void ControllerInterface::InvokeDevicesChangedCallbacks() const { + std::lock_guard lk(m_callbacks_mutex); for (const auto& callback : m_devices_changed_callbacks) callback(); } diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h index e70c135b3e..b485cda31a 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h @@ -4,14 +4,11 @@ #pragma once -#include #include -#include -#include -#include +#include +#include #include -#include "Common/CommonTypes.h" #include "InputCommon/ControllerInterface/Device.h" // enable disable sources @@ -58,6 +55,7 @@ public: private: std::vector> m_devices_changed_callbacks; + mutable std::mutex m_callbacks_mutex; bool m_is_init; void* m_hwnd; }; From 8e6677be90d67a652eb4bcf7d0b76f548e254172 Mon Sep 17 00:00:00 2001 From: Michael M Date: Sat, 4 Nov 2017 07:37:03 -0700 Subject: [PATCH 4/4] ControllerInterface: don't call InvokeDevicesChangedCallbacks more than once when refreshing --- .../ControllerInterface/ControllerInterface.cpp | 14 ++++++++++++-- .../ControllerInterface/ControllerInterface.h | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index 9e3c7ba7af..328258a8b2 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -47,6 +47,7 @@ void ControllerInterface::Initialize(void* const hwnd) return; m_hwnd = hwnd; + m_is_populating_devices = true; #ifdef CIFACE_USE_DINPUT // nothing needed @@ -88,6 +89,8 @@ void ControllerInterface::RefreshDevices() m_devices.clear(); } + m_is_populating_devices = true; + #ifdef CIFACE_USE_DINPUT ciface::DInput::PopulateDevices(reinterpret_cast(m_hwnd)); #endif @@ -113,6 +116,9 @@ void ControllerInterface::RefreshDevices() #ifdef CIFACE_USE_PIPES ciface::Pipes::PopulateDevices(); #endif + + m_is_populating_devices = false; + InvokeDevicesChangedCallbacks(); } // @@ -188,7 +194,9 @@ void ControllerInterface::AddDevice(std::shared_ptr device NOTICE_LOG(SERIALINTERFACE, "Added device: %s", device->GetQualifiedName().c_str()); m_devices.emplace_back(std::move(device)); } - InvokeDevicesChangedCallbacks(); + + if (!m_is_populating_devices) + InvokeDevicesChangedCallbacks(); } void ControllerInterface::RemoveDevice(std::function callback) @@ -205,7 +213,9 @@ void ControllerInterface::RemoveDevice(std::function #include #include #include @@ -57,6 +58,7 @@ private: std::vector> m_devices_changed_callbacks; mutable std::mutex m_callbacks_mutex; bool m_is_init; + std::atomic m_is_populating_devices{false}; void* m_hwnd; };