From 0eb47e1071b2e2e5fb18beddc5c4f16ddbfee609 Mon Sep 17 00:00:00 2001 From: Michael M Date: Thu, 9 Nov 2017 13:52:33 -0800 Subject: [PATCH 1/3] CMake: make SDL a private dep of InputCommon --- Source/Core/InputCommon/CMakeLists.txt | 4 ++-- .../InputCommon/ControllerInterface/ControllerInterface.h | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Source/Core/InputCommon/CMakeLists.txt b/Source/Core/InputCommon/CMakeLists.txt index b79d9fef19..99cf2f9679 100644 --- a/Source/Core/InputCommon/CMakeLists.txt +++ b/Source/Core/InputCommon/CMakeLists.txt @@ -114,8 +114,8 @@ if(ENABLE_SDL) endif() if(SDL_TARGET AND TARGET ${SDL_TARGET}) target_sources(inputcommon PRIVATE ControllerInterface/SDL/SDL.cpp) - target_link_libraries(inputcommon PUBLIC ${SDL_TARGET}) - target_compile_definitions(inputcommon PRIVATE -DHAVE_SDL=1) + target_link_libraries(inputcommon PRIVATE ${SDL_TARGET}) + target_compile_definitions(inputcommon PRIVATE "CIFACE_USE_SDL=1") else() message(STATUS "SDL NOT found, disabling SDL input") endif() diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h index 06fd7c544d..c576b20fb3 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h @@ -23,9 +23,6 @@ #if defined(__APPLE__) #define CIFACE_USE_OSX #endif -#if defined(HAVE_SDL) && HAVE_SDL -#define CIFACE_USE_SDL -#endif #if defined(HAVE_LIBEVDEV) && defined(HAVE_LIBUDEV) #define CIFACE_USE_EVDEV #endif From 7062967b5b50cf57bc7f3d11741ff0dd893c4ee0 Mon Sep 17 00:00:00 2001 From: Michael M Date: Thu, 9 Nov 2017 14:04:31 -0800 Subject: [PATCH 2/3] SDLJoystick: store name on creation Otherwise, Dolphin will crash when the joystick is removed. --- Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp | 4 ++-- Source/Core/InputCommon/ControllerInterface/SDL/SDL.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp index 7da8e66fc6..e17c307d1d 100644 --- a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp +++ b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp @@ -69,7 +69,7 @@ void PopulateDevices() } Joystick::Joystick(SDL_Joystick* const joystick, const int sdl_index) - : m_joystick(joystick), m_sdl_index(sdl_index) + : m_joystick(joystick), m_name(StripSpaces(GetJoystickName(sdl_index))) { // really bad HACKS: // to not use SDL for an XInput device @@ -284,7 +284,7 @@ void Joystick::UpdateInput() std::string Joystick::GetName() const { - return StripSpaces(GetJoystickName(m_sdl_index)); + return m_name; } std::string Joystick::GetSource() const diff --git a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h index c41bb57614..1dd0fb685c 100644 --- a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h +++ b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h @@ -151,7 +151,7 @@ public: private: SDL_Joystick* const m_joystick; - const int m_sdl_index; + std::string m_name; #ifdef USE_SDL_HAPTIC SDL_Haptic* m_haptic; From 932ca644aa92c01d60827206562938bfd877ecb3 Mon Sep 17 00:00:00 2001 From: Michael M Date: Thu, 9 Nov 2017 14:06:58 -0800 Subject: [PATCH 3/3] Add hotplug support to SDL2 controller backend --- .../ControllerInterface.cpp | 3 +- .../ControllerInterface/SDL/SDL.cpp | 161 +++++++++++++++--- .../InputCommon/ControllerInterface/SDL/SDL.h | 6 +- 3 files changed, 143 insertions(+), 27 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index 328258a8b2..0e6857c137 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -158,8 +158,7 @@ void ControllerInterface::Shutdown() ciface::Quartz::DeInit(); #endif #ifdef CIFACE_USE_SDL - // TODO: there seems to be some sort of memory leak with SDL, quit isn't freeing everything up - SDL_Quit(); + ciface::SDL::DeInit(); #endif #ifdef CIFACE_USE_ANDROID // nothing needed diff --git a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp index e17c307d1d..43ad1cf8c0 100644 --- a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp +++ b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp @@ -2,13 +2,20 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#include "InputCommon/ControllerInterface/SDL/SDL.h" + #include #include #include +#include +#include + +#include "Common/Event.h" +#include "Common/Logging/Log.h" +#include "Common/ScopeGuard.h" #include "Common/StringUtil.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" -#include "InputCommon/ControllerInterface/SDL/SDL.h" #ifdef _WIN32 #pragma comment(lib, "SDL2.lib") @@ -33,39 +40,142 @@ static std::string GetJoystickName(int index) #endif } +static void OpenAndAddDevice(int index) +{ + SDL_Joystick* dev = SDL_JoystickOpen(index); + if (dev) + { + auto js = std::make_shared(dev, index); + // only add if it has some inputs/outputs + if (!js->Inputs().empty() || !js->Outputs().empty()) + g_controller_interface.AddDevice(std::move(js)); + } +} + +#if SDL_VERSION_ATLEAST(2, 0, 0) +static Common::Event s_init_event; +static Uint32 s_stop_event_type; +static Uint32 s_populate_event_type; +static Common::Event s_populated_event; +static std::thread s_hotplug_thread; + +static bool HandleEventAndContinue(const SDL_Event& e) +{ + if (e.type == SDL_JOYDEVICEADDED) + { + OpenAndAddDevice(e.jdevice.which); + g_controller_interface.InvokeDevicesChangedCallbacks(); + } + else if (e.type == SDL_JOYDEVICEREMOVED) + { + g_controller_interface.RemoveDevice([&e](const auto* device) { + const Joystick* joystick = dynamic_cast(device); + return joystick && SDL_JoystickInstanceID(joystick->GetSDLJoystick()) == e.jdevice.which; + }); + g_controller_interface.InvokeDevicesChangedCallbacks(); + } + else if (e.type == s_populate_event_type) + { + for (int i = 0; i < SDL_NumJoysticks(); ++i) + OpenAndAddDevice(i); + s_populated_event.Set(); + } + else if (e.type == s_stop_event_type) + { + return false; + } + + return true; +} +#endif + void Init() { -#ifdef USE_SDL_HAPTIC - if (SDL_Init(SDL_INIT_JOYSTICK | SDL_INIT_HAPTIC) >= 0) - { - // Correctly initialized - } - else +#if !SDL_VERSION_ATLEAST(2, 0, 0) + if (SDL_Init(SDL_INIT_JOYSTICK) != 0) + ERROR_LOG(SERIALINTERFACE, "SDL failed to initialize"); + return; +#else + s_hotplug_thread = std::thread([] { + Common::ScopeGuard quit_guard([] { + // TODO: there seems to be some sort of memory leak with SDL, quit isn't freeing everything up + SDL_Quit(); + }); + + { + Common::ScopeGuard init_guard([] { s_init_event.Set(); }); + + if (SDL_Init(SDL_INIT_JOYSTICK | SDL_INIT_HAPTIC) != 0) + { + ERROR_LOG(SERIALINTERFACE, "SDL failed to initialize"); + return; + } + + Uint32 custom_events_start = SDL_RegisterEvents(2); + if (custom_events_start == static_cast(-1)) + { + ERROR_LOG(SERIALINTERFACE, "SDL failed to register custom events"); + return; + } + s_stop_event_type = custom_events_start; + s_populate_event_type = custom_events_start + 1; + + // Drain all of the events and add the initial joysticks before returning. Otherwise, the + // individual joystick events as well as the custom populate event will be handled _after_ + // ControllerInterface::Init/RefreshDevices has cleared its list of devices, resulting in + // duplicate devices. + SDL_Event e; + while (SDL_PollEvent(&e) != 0) + { + if (!HandleEventAndContinue(e)) + return; + } + } + + SDL_Event e; + while (SDL_WaitEvent(&e) != 0) + { + if (!HandleEventAndContinue(e)) + return; + } + }); + + s_init_event.Wait(); #endif - if (SDL_Init(SDL_INIT_JOYSTICK) < 0) - { - // Failed to initialize +} + +void DeInit() +{ +#if !SDL_VERSION_ATLEAST(2, 0, 0) + SDL_Quit(); +#else + if (!s_hotplug_thread.joinable()) return; - } + + SDL_Event stop_event{s_stop_event_type}; + SDL_PushEvent(&stop_event); + + s_hotplug_thread.join(); +#endif } void PopulateDevices() { - if (!(SDL_WasInit(SDL_INIT_EVERYTHING) & SDL_INIT_JOYSTICK)) +#if !SDL_VERSION_ATLEAST(2, 0, 0) + if (!SDL_WasInit(SDL_INIT_JOYSTICK)) return; - // joysticks for (int i = 0; i < SDL_NumJoysticks(); ++i) - { - SDL_Joystick* dev = SDL_JoystickOpen(i); - if (dev) - { - auto js = std::make_shared(dev, i); - // only add if it has some inputs/outputs - if (js->Inputs().size() || js->Outputs().size()) - g_controller_interface.AddDevice(std::move(js)); - } - } + OpenAndAddDevice(i); +#else + if (!s_hotplug_thread.joinable()) + return; + + SDL_Event populate_event{s_populate_event_type}; + SDL_PushEvent(&populate_event); + + s_populated_event.Wait(); +#endif } Joystick::Joystick(SDL_Joystick* const joystick, const int sdl_index) @@ -292,6 +402,11 @@ std::string Joystick::GetSource() const return "SDL"; } +SDL_Joystick* Joystick::GetSDLJoystick() const +{ + return m_joystick; +} + std::string Joystick::Button::GetName() const { std::ostringstream ss; diff --git a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h index 1dd0fb685c..311902e84a 100644 --- a/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h +++ b/Source/Core/InputCommon/ControllerInterface/SDL/SDL.h @@ -4,8 +4,6 @@ #pragma once -#include - #include #if SDL_VERSION_ATLEAST(1, 3, 0) @@ -16,11 +14,14 @@ #include #endif +#include "InputCommon/ControllerInterface/Device.h" + namespace ciface { namespace SDL { void Init(); +void DeInit(); void PopulateDevices(); class Joystick : public Core::Device @@ -148,6 +149,7 @@ public: std::string GetName() const override; std::string GetSource() const override; + SDL_Joystick* GetSDLJoystick() const; private: SDL_Joystick* const m_joystick;