ControllerInterface: Implement ChangeWindow on DInput without recreating the devices

Also polished DInput code in general to try and mitigate issue 11702.
Added a lot of logging and comments.

Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp

@@ -135,6 +135,26 @@ void ControllerInterface::RefreshDevices(RefreshReason reason)
   // This wasn't thread safe in multiple device sources.
   std::lock_guard lk_population(m_devices_population_mutex);
 
+#if defined(CIFACE_USE_WIN32) && !defined(CIFACE_USE_XLIB) && !defined(CIFACE_USE_OSX)
+  // If only the window changed, avoid removing and re-adding all devices.
+  // Instead only refresh devices that require the window handle.
+  if (reason == RefreshReason::WindowChangeOnly)
+  {
+    m_populating_devices_counter.fetch_add(1);
+
+    {
+      std::lock_guard lk(m_devices_mutex);
+      // No need to do anything else in this case.
+      // Only (Win32) DInput needs the window handle to be updated.
+      ciface::Win32::ChangeWindow(m_wsi.render_window);
+    }
+
+    if (m_populating_devices_counter.fetch_sub(1) == 1)
+      InvokeDevicesChangedCallbacks();
+    return;
+  }
+#endif
+
   m_populating_devices_counter.fetch_add(1);
 
   // We lock m_devices_mutex here to make everything simpler.

Source/Core/InputCommon/ControllerInterface/CoreDevice.h

@@ -125,6 +125,12 @@ public:
   // Currently handled on a per-backend basis but this could change.
   virtual bool IsValid() const { return true; }
 
+  // Returns true whether this device is "virtual/emulated", not linked
+  // to any actual physical device. Mostly used by keyboard and mouse devices,
+  // and to avoid uselessly recreating the device unless really necessary.
+  // Doesn't necessarily need to be set to true if the device is virtual.
+  virtual bool IsVirtualDevice() const { return false; }
+
   // (e.g. Xbox 360 controllers have controller number LEDs which should match the ID we use.)
   virtual std::optional<int> GetPreferredId() const;
 

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,
-  g_controller_interface.RemoveDevice(
+                                           IID_IDirectInput8, (LPVOID*)&s_idi8, nullptr)))
-      [](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)))
   {
-    ERROR_LOG_FMT(PAD, "DirectInput8Create failed.");
+    ERROR_LOG_FMT(CONTROLLERINTERFACE, "DirectInput8Create failed.");
     return;
   }
 
-  InitKeyboardMouse(idi8, hwnd);
+  // Remove old (invalid) devices. No need to ever remove the KeyboardMouse device.
-  InitJoystick(idi8, hwnd);
+  // 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(); });
 
-  idi8->Release();
+  InitKeyboardMouse(s_idi8, hwnd);
+  InitJoystick(s_idi8, hwnd);
+}
+
+void ChangeWindow(HWND hwnd)
+{
+  if (s_idi8)  // Has init? Ignore if called before the first PopulateDevices()
+  {
+    // 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);
+
+    SetKeyboardMouseWindow(hwnd);
+    InitJoystick(s_idi8, hwnd);
+  }
+}
+
+void DeInit()
+{
+  if (s_idi8)
+  {
+    s_idi8->Release();
+    s_idi8 = nullptr;
+  }
 }
 }  // namespace ciface::DInput

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

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.
+// Prevent duplicate keyboard/mouse devices. Modified by more threads.
-static bool s_keyboard_mouse_exists = false;
+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,16 +83,23 @@ 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;
@@ -105,9 +118,8 @@ KeyboardMouse::~KeyboardMouse()
 }
 
 KeyboardMouse::KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device,
-                             const LPDIRECTINPUTDEVICE8 mo_device, HWND hwnd)
+                             const LPDIRECTINPUTDEVICE8 mo_device)
-    : m_kb_device(kb_device), m_mo_device(mo_device), m_hwnd(hwnd), m_last_update(GetTickCount()),
+    : m_kb_device(kb_device), m_mo_device(mo_device), m_last_update(GetTickCount()), m_state_in()
-      m_state_in()
 {
   s_keyboard_mouse_exists = true;
 
@@ -160,11 +172,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);
@@ -236,6 +248,11 @@ int KeyboardMouse::GetSortPriority() const
   return 5;
 }
 
+bool KeyboardMouse::IsVirtualDevice() const
+{
+  return true;
+}
+
 // names
 std::string KeyboardMouse::Key::GetName() const
 {

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
 {
@@ -96,13 +97,13 @@ private:
 public:
   void UpdateInput() override;
 
-  KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device,
+  KeyboardMouse(const LPDIRECTINPUTDEVICE8 kb_device, const LPDIRECTINPUTDEVICE8 mo_device);
-                HWND hwnd);
   ~KeyboardMouse();
 
   std::string GetName() const override;
   std::string GetSource() const override;
   int GetSortPriority() const override;
+  bool IsVirtualDevice() const override;
 
 private:
   void UpdateCursorInput();
@@ -110,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;
 };

Source/Core/InputCommon/ControllerInterface/Win32/Win32.cpp

@@ -11,6 +11,7 @@
 #include <thread>
 
 #include "Common/Event.h"
+#include "Common/Flag.h"
 #include "Common/Logging/Log.h"
 #include "Common/ScopeGuard.h"
 #include "Common/Thread.h"
@@ -19,20 +20,30 @@
 
 constexpr UINT WM_DOLPHIN_STOP = WM_USER;
 
-static Common::Event s_done_populating;
+static Common::Event s_received_device_change_event;
-static std::atomic<HWND> s_hwnd;
+// Dolphin's render window
+static HWND s_hwnd;
+// Windows messaging window (hidden)
 static HWND s_message_window;
 static std::thread s_thread;
+static Common::Flag s_first_populate_devices_asked;
 
 static LRESULT CALLBACK WindowProc(HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam)
 {
   if (message == WM_INPUT_DEVICE_CHANGE)
   {
+    // Windows automatically sends this message before we ask for it and before we are "ready" to
+    // listen for it.
+    if (s_first_populate_devices_asked.IsSet())
+    {
+      s_received_device_change_event.Set();
+      // 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([] {
         ciface::DInput::PopulateDevices(s_hwnd);
         ciface::XInput::PopulateDevices();
       });
-    s_done_populating.Set();
+    }
   }
 
   return DefWindowProc(hwnd, message, wparam, lparam);
@@ -125,10 +136,14 @@ void ciface::Win32::PopulateDevices(void* hwnd)
   if (s_thread.joinable())
   {
     s_hwnd = static_cast<HWND>(hwnd);
-    s_done_populating.Reset();
+    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);
-    if (!s_done_populating.WaitFor(std::chrono::seconds(10)))
+    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();
   }
   else
   {
@@ -137,6 +152,15 @@ void ciface::Win32::PopulateDevices(void* hwnd)
   }
 }
 
+void ciface::Win32::ChangeWindow(void* hwnd)
+{
+  if (s_thread.joinable())  // "Has init?"
+  {
+    s_hwnd = static_cast<HWND>(hwnd);
+    ciface::DInput::ChangeWindow(s_hwnd);
+  }
+}
+
 void ciface::Win32::DeInit()
 {
   NOTICE_LOG_FMT(CONTROLLERINTERFACE, "win32 DeInit");
@@ -145,7 +169,11 @@ 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();
   }
+  s_hwnd = nullptr;
 
   XInput::DeInit();
 }

Source/Core/InputCommon/ControllerInterface/Win32/Win32.h

@@ -8,5 +8,6 @@ namespace ciface::Win32
 {
 void Init(void* hwnd);
 void PopulateDevices(void* hwnd);
+void ChangeWindow(void* hwnd);
 void DeInit();
 }  // namespace ciface::Win32