Merge pull request #6180 from ligfx/sdlhotplug

Add hotplug support to SDL2 controller backend

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_link_libraries(inputcommon PRIVATE ${SDL_TARGET})
-    target_compile_definitions(inputcommon PRIVATE -DHAVE_SDL=1)
+    target_compile_definitions(inputcommon PRIVATE "CIFACE_USE_SDL=1")
   else()
     message(STATUS "SDL NOT found, disabling SDL input")
   endif()

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
+  ciface::SDL::DeInit();
-  SDL_Quit();
 #endif
 #ifdef CIFACE_USE_ANDROID
 // nothing needed

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

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 <algorithm>
 #include <map>
 #include <sstream>
+#include <thread>
 
+#include <SDL_events.h>
+
+#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,43 +40,146 @@ 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<Joystick>(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<const Joystick*>(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_VERSION_ATLEAST(2, 0, 0)
-  if (SDL_Init(SDL_INIT_JOYSTICK | SDL_INIT_HAPTIC) >= 0)
+  if (SDL_Init(SDL_INIT_JOYSTICK) != 0)
-  {
+    ERROR_LOG(SERIALINTERFACE, "SDL failed to initialize");
-    // Correctly initialized
+  return;
-  }
+#else
-  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<Uint32>(-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)
-  {
+    OpenAndAddDevice(i);
-    SDL_Joystick* dev = SDL_JoystickOpen(i);
+#else
-    if (dev)
+  if (!s_hotplug_thread.joinable())
-    {
+    return;
-      auto js = std::make_shared<Joystick>(dev, i);
+
-      // only add if it has some inputs/outputs
+  SDL_Event populate_event{s_populate_event_type};
-      if (js->Inputs().size() || js->Outputs().size())
+  SDL_PushEvent(&populate_event);
-        g_controller_interface.AddDevice(std::move(js));
+
-    }
+  s_populated_event.Wait();
-  }
+#endif
 }
 
 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 +394,7 @@ void Joystick::UpdateInput()
 
 std::string Joystick::GetName() const
 {
-  return StripSpaces(GetJoystickName(m_sdl_index));
+  return m_name;
 }
 
 std::string Joystick::GetSource() const
@@ -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;

Source/Core/InputCommon/ControllerInterface/SDL/SDL.h

@@ -4,8 +4,6 @@
 
 #pragma once
 
-#include <list>
-
 #include <SDL.h>
 
 #if SDL_VERSION_ATLEAST(1, 3, 0)
@@ -16,11 +14,14 @@
 #include <SDL_haptic.h>
 #endif
 
+#include "InputCommon/ControllerInterface/Device.h"
+
 namespace ciface
 {
 namespace SDL
 {
 void Init();
+void DeInit();
 void PopulateDevices();
 
 class Joystick : public Core::Device
@@ -148,10 +149,11 @@ public:
 
   std::string GetName() const override;
   std::string GetSource() const override;
+  SDL_Joystick* GetSDLJoystick() const;
 
 private:
   SDL_Joystick* const m_joystick;
-  const int m_sdl_index;
+  std::string m_name;
 
 #ifdef USE_SDL_HAPTIC
   SDL_Haptic* m_haptic;