Merge pull request #9702 from Filoppi/controller_interface_fixes

Controller Interface refactor

5 files changed, 134 insertions, 50 deletions

diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp
index d508de613a..f5e105dc45 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.cpp
@@ -16,6 +16,8 @@
 
 namespace ciface::DInput
 {
+static IDirectInput8* s_idi8 = nullptr;
+
 BOOL CALLBACK DIEnumDeviceObjectsCallback(LPCDIDEVICEOBJECTINSTANCE lpddoi, LPVOID pvRef)
 {
   ((std::list<DIDEVICEOBJECTINSTANCE>*)pvRef)->push_back(*lpddoi);
@@ -42,29 +44,57 @@ std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device)
   }
   else
   {
-    ERROR_LOG_FMT(PAD, "GetProperty(DIPROP_PRODUCTNAME) failed.");
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "GetProperty(DIPROP_PRODUCTNAME) failed.");
   }
 
   return result;
 }
 
+// Assumes hwnd had not changed from the previous call
 void PopulateDevices(HWND hwnd)
 {
-  // Remove unplugged devices.
+  if (!s_idi8 && FAILED(DirectInput8Create(GetModuleHandle(nullptr), DIRECTINPUT_VERSION,
+                                           IID_IDirectInput8, (LPVOID*)&s_idi8, nullptr)))
+  {
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "DirectInput8Create failed.");
+    return;
+  }
+
+  // Remove old (invalid) devices. No need to ever remove the KeyboardMouse device.
+  // Note that if we have 2+ DInput controllers, not fully repopulating devices
+  // will mean that a device with index "2" could persist while there is no device with index "0".
+  // This is slightly inconsistent as when we refresh all devices, they will instead reset, and
+  // that happens a lot (for uncontrolled reasons, like starting/stopping the emulation).
   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)))
+  InitKeyboardMouse(s_idi8, hwnd);
+  InitJoystick(s_idi8, hwnd);
+}
+
+void ChangeWindow(HWND hwnd)
+{
+  if (s_idi8)  // Has init? Ignore if called before the first PopulateDevices()
   {
-    ERROR_LOG_FMT(PAD, "DirectInput8Create failed.");
-    return;
-  }
+    // The KeyboardMouse device is marked as virtual device, so we avoid removing it.
+    // We need to force all the DInput joysticks to be destroyed now, or recreation would fail.
+    g_controller_interface.RemoveDevice(
+        [](const auto* dev) {
+          return dev->GetSource() == DINPUT_SOURCE_NAME && !dev->IsVirtualDevice();
+        },
+        true);
 
-  InitKeyboardMouse(idi8, hwnd);
-  InitJoystick(idi8, hwnd);
+    SetKeyboardMouseWindow(hwnd);
+    InitJoystick(s_idi8, hwnd);
+  }
+}
 
-  idi8->Release();
+void DeInit()
+{
+  if (s_idi8)
+  {
+    s_idi8->Release();
+    s_idi8 = nullptr;
+  }
 }
 }  // namespace ciface::DInput
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h
index ed159089e0..8be064a1f1 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInput.h
@@ -20,4 +20,6 @@ BOOL CALLBACK DIEnumDevicesCallback(LPCDIDEVICEINSTANCE lpddi, LPVOID pvRef);
 std::string GetDeviceName(const LPDIRECTINPUTDEVICE8 device);
 
 void PopulateDevices(HWND hwnd);
+void ChangeWindow(HWND hwnd);
+void DeInit();
 }  // namespace ciface::DInput
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
index 67fd33186b..3d51d9c043 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
@@ -4,6 +4,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <mutex>
 #include <set>
 #include <sstream>
 #include <type_traits>
@@ -29,6 +30,7 @@ struct GUIDComparator
 };
 
 static std::set<GUID, GUIDComparator> s_guids_in_use;
+static std::mutex s_guids_mutex;
 
 void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
 {
@@ -46,12 +48,16 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
     }
 
     // Skip devices we are already using.
-    if (s_guids_in_use.count(joystick.guidInstance))
     {
-      continue;
+      std::lock_guard lk(s_guids_mutex);
+      if (s_guids_in_use.count(joystick.guidInstance))
+      {
+        continue;
+      }
     }
 
     LPDIRECTINPUTDEVICE8 js_device;
+    // Don't print any warnings on failure
     if (SUCCEEDED(idi8->CreateDevice(joystick.guidInstance, &js_device, nullptr)))
     {
       if (SUCCEEDED(js_device->SetDataFormat(&c_dfDIJoystick)))
@@ -60,37 +66,40 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
                                                   DISCL_BACKGROUND | DISCL_EXCLUSIVE)))
         {
           WARN_LOG_FMT(
-              PAD,
+              CONTROLLERINTERFACE,
               "DInput: Failed to acquire device exclusively. Force feedback will be unavailable.");
           // Fall back to non-exclusive mode, with no rumble
           if (FAILED(
                   js_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)))
           {
-            // PanicAlert("SetCooperativeLevel failed!");
             js_device->Release();
             continue;
           }
         }
 
-        s_guids_in_use.insert(joystick.guidInstance);
         auto js = std::make_shared<Joystick>(js_device);
-
-        // only add if it has some inputs/outputs
+        // only add if it has some inputs/outputs.
+        // Don't even add it to our static list in case we first created it without a window handle,
+        // failing to get exclusive mode, and then later managed to obtain it, which mean it
+        // could now have some outputs if it didn't before.
         if (js->Inputs().size() || js->Outputs().size())
-          g_controller_interface.AddDevice(std::move(js));
+        {
+          if (g_controller_interface.AddDevice(std::move(js)))
+          {
+            std::lock_guard lk(s_guids_mutex);
+            s_guids_in_use.insert(joystick.guidInstance);
+          }
+        }
       }
       else
       {
-        // PanicAlert("SetDataFormat failed!");
         js_device->Release();
       }
     }
   }
 }
 
-Joystick::Joystick(/*const LPCDIDEVICEINSTANCE lpddi, */ const LPDIRECTINPUTDEVICE8 device)
-    : m_device(device)
-//, m_name(TStringToString(lpddi->tszInstanceName))
+Joystick::Joystick(const LPDIRECTINPUTDEVICE8 device) : m_device(device)
 {
   // seems this needs to be done before GetCapabilities
   // polled or buffered data
@@ -183,11 +192,12 @@ Joystick::~Joystick()
   info.dwSize = sizeof(info);
   if (SUCCEEDED(m_device->GetDeviceInfo(&info)))
   {
+    std::lock_guard lk(s_guids_mutex);
     s_guids_in_use.erase(info.guidInstance);
   }
   else
   {
-    ERROR_LOG_FMT(PAD, "DInputJoystick: GetDeviceInfo failed.");
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "DInputJoystick: GetDeviceInfo failed.");
   }
 
   DeInitForceFeedback();
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp
index ebe2e84286..aaa723849e 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.cpp
@@ -6,7 +6,8 @@
 
 #include <algorithm>
 
-#include <fmt/format.h>
+#include "Common/Logging/Log.h"
+#include "Core/Core.h"
 
 #include "InputCommon/ControllerInterface/ControllerInterface.h"
 #include "InputCommon/ControllerInterface/DInput/DInput.h"
@@ -54,15 +55,18 @@ static const struct
 #include "InputCommon/ControllerInterface/DInput/NamedKeys.h"  // NOLINT
 };
 
-// Prevent duplicate keyboard/mouse devices.
-static bool s_keyboard_mouse_exists = false;
+// Prevent duplicate keyboard/mouse devices. Modified by more threads.
+static bool s_keyboard_mouse_exists;
+static HWND s_hwnd;
 
 void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd)
 {
   if (s_keyboard_mouse_exists)
     return;
 
-  // mouse and keyboard are a combined device, to allow shift+click and stuff
+  s_hwnd = hwnd;
+
+  // 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
   // other devices
   // so there can be a separated Keyboard and mouse, as well as combined KeyboardMouse
@@ -70,6 +74,8 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd)
   LPDIRECTINPUTDEVICE8 kb_device = nullptr;
   LPDIRECTINPUTDEVICE8 mo_device = nullptr;
 
+  // These are "virtual" system devices, so they are always there even if we have no physical
+  // mouse and keyboard plugged into the computer
   if (SUCCEEDED(idi8->CreateDevice(GUID_SysKeyboard, &kb_device, nullptr)) &&
       SUCCEEDED(kb_device->SetDataFormat(&c_dfDIKeyboard)) &&
       SUCCEEDED(kb_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)) &&
@@ -77,20 +83,32 @@ void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd)
       SUCCEEDED(mo_device->SetDataFormat(&c_dfDIMouse2)) &&
       SUCCEEDED(mo_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)))
   {
-    g_controller_interface.AddDevice(std::make_shared<KeyboardMouse>(kb_device, mo_device, hwnd));
+    g_controller_interface.AddDevice(std::make_shared<KeyboardMouse>(kb_device, mo_device));
     return;
   }
 
+  ERROR_LOG_FMT(CONTROLLERINTERFACE, "KeyboardMouse device failed to be created");
+
   if (kb_device)
     kb_device->Release();
   if (mo_device)
     mo_device->Release();
 }
 
+void SetKeyboardMouseWindow(HWND hwnd)
+{
+  s_hwnd = hwnd;
+}
+
 KeyboardMouse::~KeyboardMouse()
 {
   s_keyboard_mouse_exists = false;
 
+  // Independently of the order in which we do these, if we put a breakpoint on Unacquire() (or in
+  // any place in the call stack before this), when refreshing devices from the UI, on the second
+  // attempt, it will get stuck in an infinite (while) loop inside dinput8.dll. Given that it can't
+  // be otherwise be reproduced (not even with sleeps), we can just ignore the problem.
+
   // kb
   m_kb_device->Unacquire();
   m_kb_device->Release();
@@ -100,14 +118,15 @@ KeyboardMouse::~KeyboardMouse()
 }
 
 KeyboardMouse::KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device,
-                             const LPDIRECTINPUTDEVICE8 mo_device, HWND hwnd)
-    : m_kb_device(kb_device), m_mo_device(mo_device), m_hwnd(hwnd), m_last_update(GetTickCount()),
-      m_state_in()
+                             const LPDIRECTINPUTDEVICE8 mo_device)
+    : m_kb_device(kb_device), m_mo_device(mo_device), m_last_update(GetTickCount()), m_state_in()
 {
   s_keyboard_mouse_exists = true;
 
-  m_kb_device->Acquire();
-  m_mo_device->Acquire();
+  if (FAILED(m_kb_device->Acquire()))
+    WARN_LOG_FMT(CONTROLLERINTERFACE, "Keyboard device failed to acquire. We'll retry later");
+  if (FAILED(m_mo_device->Acquire()))
+    WARN_LOG_FMT(CONTROLLERINTERFACE, "Mouse device failed to acquire. We'll retry later");
 
   // KEYBOARD
   // add keys
@@ -155,11 +174,11 @@ void KeyboardMouse::UpdateCursorInput()
 
   // Get the cursor position relative to the upper left corner of the current window
   // (separate or render to main)
-  ScreenToClient(m_hwnd, &point);
+  ScreenToClient(s_hwnd, &point);
 
-  // Get the size of the current window. (In my case Rect.top and Rect.left was zero.)
+  // Get the size of the current window (in my case Rect.top and Rect.left was zero).
   RECT rect;
-  GetClientRect(m_hwnd, &rect);
+  GetClientRect(s_hwnd, &rect);
 
   // Width and height are the size of the rendering window. They could be 0
   const auto win_width = std::max(rect.right - rect.left, 1l);
@@ -174,6 +193,8 @@ void KeyboardMouse::UpdateCursorInput()
 
 void KeyboardMouse::UpdateInput()
 {
+  UpdateCursorInput();
+
   DIMOUSESTATE2 tmp_mouse;
 
   // if mouse position hasn't been updated in a short while, skip a dev state
@@ -190,16 +211,14 @@ void KeyboardMouse::UpdateInput()
 
   m_last_update = cur_time;
 
-  HRESULT kb_hr = m_kb_device->GetDeviceState(sizeof(m_state_in.keyboard), &m_state_in.keyboard);
   HRESULT mo_hr = m_mo_device->GetDeviceState(sizeof(tmp_mouse), &tmp_mouse);
-
-  if (DIERR_INPUTLOST == kb_hr || DIERR_NOTACQUIRED == kb_hr)
-    m_kb_device->Acquire();
-
   if (DIERR_INPUTLOST == mo_hr || DIERR_NOTACQUIRED == mo_hr)
-    m_mo_device->Acquire();
-
-  if (SUCCEEDED(mo_hr))
+  {
+    INFO_LOG_FMT(CONTROLLERINTERFACE, "Mouse device failed to get state");
+    if (FAILED(m_mo_device->Acquire()))
+      INFO_LOG_FMT(CONTROLLERINTERFACE, "Mouse device failed to re-acquire, we'll retry later");
+  }
+  else if (SUCCEEDED(mo_hr))
   {
     m_state_in.relative_mouse.Move({tmp_mouse.lX, tmp_mouse.lY, tmp_mouse.lZ});
     m_state_in.relative_mouse.Update();
@@ -210,8 +229,16 @@ void KeyboardMouse::UpdateInput()
 
     // copy over the buttons
     std::copy_n(tmp_mouse.rgbButtons, std::size(tmp_mouse.rgbButtons), m_state_in.mouse.rgbButtons);
+  }
 
-    UpdateCursorInput();
+  HRESULT kb_hr = m_kb_device->GetDeviceState(sizeof(m_state_in.keyboard), &m_state_in.keyboard);
+  if (kb_hr == DIERR_INPUTLOST || kb_hr == DIERR_NOTACQUIRED)
+  {
+    INFO_LOG_FMT(CONTROLLERINTERFACE, "Keyboard device failed to get state");
+    if (SUCCEEDED(m_kb_device->Acquire()))
+      m_kb_device->GetDeviceState(sizeof(m_state_in.keyboard), &m_state_in.keyboard);
+    else
+      INFO_LOG_FMT(CONTROLLERINTERFACE, "Keyboard device failed to re-acquire, we'll retry later");
   }
 }
 
@@ -225,6 +252,17 @@ std::string KeyboardMouse::GetSource() const
   return DINPUT_SOURCE_NAME;
 }
 
+// Give this device a higher priority to make sure it shows first
+int KeyboardMouse::GetSortPriority() const
+{
+  return 5;
+}
+
+bool KeyboardMouse::IsVirtualDevice() const
+{
+  return true;
+}
+
 // names
 std::string KeyboardMouse::Key::GetName() const
 {
diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h
index ceda59d02c..b0fd39c00d 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h
+++ b/Source/Core/InputCommon/ControllerInterface/DInput/DInputKeyboardMouse.h
@@ -16,6 +16,7 @@ namespace ciface::DInput
 void InitKeyboardMouse(IDirectInput8* const idi8, HWND hwnd);
 
 using RelativeMouseState = RelativeInputState<Common::TVec3<LONG>>;
+void SetKeyboardMouseWindow(HWND hwnd);
 
 class KeyboardMouse : public Core::Device
 {
@@ -34,6 +35,7 @@ private:
     RelativeMouseState relative_mouse;
   };
 
+  // Keyboard key
   class Key : public Input
   {
   public:
@@ -46,6 +48,7 @@ private:
     const u8 m_index;
   };
 
+  // Mouse button
   class Button : public Input
   {
   public:
@@ -58,6 +61,7 @@ private:
     const u8 m_index;
   };
 
+  // Mouse movement offset axis. Includes mouse wheel
   class Axis : public Input
   {
   public:
@@ -72,6 +76,7 @@ private:
     const u8 m_index;
   };
 
+  // Mouse from window center
   class Cursor : public Input
   {
   public:
@@ -92,12 +97,13 @@ private:
 public:
   void UpdateInput() override;
 
-  KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device,
-                HWND hwnd);
+  KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device);
   ~KeyboardMouse();
 
   std::string GetName() const override;
   std::string GetSource() const override;
+  int GetSortPriority() const override;
+  bool IsVirtualDevice() const override;
 
 private:
   void UpdateCursorInput();
@@ -105,8 +111,6 @@ private:
   const LPDIRECTINPUTDEVICE8 m_kb_device;
   const LPDIRECTINPUTDEVICE8 m_mo_device;
 
-  const HWND m_hwnd;
-
   DWORD m_last_update;
   State m_state_in;
 };