IOS: Diff device lists in USBHost instead of USBScanner

Instead of having USBScanner create "hooks" as it scans for devices,
let's have USBScanner present a list of devices to USBHost and have
USBHost diff the new device list with its old device list to create the
hook calls instead. This gets rid of some complex edge cases that the
next commit otherwise would have to deal with, in particular regarding
toggling determinism and adding new USBHosts to a USBScanner.

Source/Core/Core/IOS/USB/Host.cpp

@@ -9,6 +9,7 @@
 #include <mutex>
 #include <optional>
 #include <string>
+#include <utility>
 
 #include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
@@ -37,19 +38,12 @@ std::optional<IPCReply> USBHost::Open(const OpenRequest& request)
     // Force a device scan to complete, because some games (including Your Shape) only care
     // about the initial device list (in the first GETDEVICECHANGE reply).
     m_usb_scanner.WaitForFirstScan();
+    OnDevicesChangedInternal(m_usb_scanner.GetDevices());
     m_has_initialised = true;
   }
   return IPCReply(IPC_SUCCESS);
 }
 
-void USBHost::UpdateWantDeterminism(const bool new_want_determinism)
-{
-  if (new_want_determinism)
-    m_usb_scanner.Stop();
-  else if (IsOpened())
-    m_usb_scanner.Start();
-}
-
 void USBHost::DoState(PointerWrap& p)
 {
   Device::DoState(p);
@@ -57,7 +51,9 @@ void USBHost::DoState(PointerWrap& p)
   {
     // After a state has loaded, there may be insertion hooks for devices that were
     // already plugged in, and which need to be triggered.
-    m_usb_scanner.UpdateDevices(true);
+    std::lock_guard lk(m_devices_mutex);
+    m_devices.clear();
+    OnDevicesChanged(m_usb_scanner.GetDevices());
   }
 }
 
@@ -86,28 +82,54 @@ bool USBHost::ShouldAddDevice(const USB::Device& device) const
 void USBHost::Update()
 {
   if (Core::WantsDeterminism())
-    m_usb_scanner.UpdateDevices();
+    OnDevicesChangedInternal(m_usb_scanner.GetDevices());
 }
 
-void USBHost::DispatchHooks(const DeviceChangeHooks& hooks)
+void USBHost::OnDevicesChanged(const USBScanner::DeviceMap& new_devices)
+{
+  if (!Core::WantsDeterminism())
+    OnDevicesChangedInternal(new_devices);
+}
+
+void USBHost::OnDevicesChangedInternal(const USBScanner::DeviceMap& new_devices)
 {
   std::lock_guard lk(m_devices_mutex);
 
-  for (const auto& [device, event] : hooks)
+  bool changes = false;
+
+  for (auto it = m_devices.begin(); it != m_devices.end();)
   {
-    INFO_LOG_FMT(IOS_USB, "{} - {} device: {:04x}:{:04x}", GetDeviceName(),
-                 event == ChangeEvent::Inserted ? "New" : "Removed", device->GetVid(),
-                 device->GetPid());
+    const auto& [id, device] = *it;
+    if (!new_devices.contains(id))
+    {
+      INFO_LOG_FMT(IOS_USB, "{} - Removed device: {:04x}:{:04x}", GetDeviceName(), device->GetVid(),
+                   device->GetPid());
 
-    if (event == ChangeEvent::Inserted)
-      m_devices.emplace(device->GetId(), device);
-    else if (event == ChangeEvent::Removed)
-      m_devices.erase(device->GetId());
-
-    OnDeviceChange(event, device);
+      changes = true;
+      auto device_copy = std::move(device);
+      it = m_devices.erase(it);
+      OnDeviceChange(ChangeEvent::Removed, std::move(device_copy));
+    }
+    else
+    {
+      ++it;
+    }
   }
 
-  if (!hooks.empty())
+  for (const auto& [id, device] : new_devices)
+  {
+    if (!m_devices.contains(id))
+    {
+      INFO_LOG_FMT(IOS_USB, "{} - New device: {:04x}:{:04x}", GetDeviceName(), device->GetVid(),
+                   device->GetPid());
+
+      changes = true;
+      m_devices.emplace(id, device);
+      OnDeviceChange(ChangeEvent::Inserted, device);
+    }
+  }
+
+  if (changes)
     OnDeviceChangeEnd();
 }
 

Source/Core/Core/IOS/USB/Host.h

@@ -29,16 +29,19 @@ public:
 
   std::optional<IPCReply> Open(const OpenRequest& request) override;
 
-  void UpdateWantDeterminism(bool new_want_determinism) override;
   void DoState(PointerWrap& p) override;
 
   virtual bool ShouldAddDevice(const USB::Device& device) const;
 
-  void DispatchHooks(const USBScanner::DeviceChangeHooks& hooks);
+  void OnDevicesChanged(const USBScanner::DeviceMap& new_devices);
 
 protected:
-  using ChangeEvent = USBScanner::ChangeEvent;
-  using DeviceChangeHooks = USBScanner::DeviceChangeHooks;
+  enum class ChangeEvent
+  {
+    Inserted,
+    Removed,
+  };
+  using DeviceChangeHooks = std::map<std::shared_ptr<USB::Device>, ChangeEvent>;
 
   std::shared_ptr<USB::Device> GetDeviceById(u64 device_id) const;
   virtual void OnDeviceChange(ChangeEvent event, std::shared_ptr<USB::Device> changed_device);
@@ -54,6 +57,7 @@ protected:
 
 private:
   void Update() override;
+  void OnDevicesChangedInternal(const USBScanner::DeviceMap& new_devices);
 
   bool m_has_initialised = false;
 };

Source/Core/Core/IOS/USB/USBScanner.cpp

@@ -3,8 +3,10 @@
 
 #include "Core/IOS/USB/USBScanner.h"
 
+#include <algorithm>
 #include <memory>
 #include <mutex>
+#include <ranges>
 #include <set>
 #include <thread>
 #include <utility>
@@ -47,11 +49,6 @@ void USBScanner::WaitForFirstScan()
 
 void USBScanner::Start()
 {
-  if (Core::WantsDeterminism())
-  {
-    UpdateDevices();
-    return;
-  }
   if (m_thread_running.TestAndSet())
   {
     m_thread = std::thread([this] {
@@ -70,40 +67,34 @@ void USBScanner::Stop()
 {
   if (m_thread_running.TestAndClear())
     m_thread.join();
+}
 
-  // Clear all devices and dispatch removal hooks.
-  DeviceChangeHooks hooks;
-  DetectRemovedDevices(std::set<u64>(), hooks);
-  m_host->DispatchHooks(hooks);
+USBScanner::DeviceMap USBScanner::GetDevices() const
+{
+  std::lock_guard lk(m_devices_mutex);
+  return m_devices;
 }
 
 // This is called from the scan thread. Returns false if we failed to update the device list.
-bool USBScanner::UpdateDevices(const bool always_add_hooks)
+bool USBScanner::UpdateDevices()
 {
-  DeviceChangeHooks hooks;
-  std::set<u64> plugged_devices;
-  // If we failed to get a new, up-to-date list of devices, we cannot detect device removals.
-  if (!AddNewDevices(plugged_devices, hooks, always_add_hooks))
+  DeviceMap new_devices;
+  if (!AddNewDevices(&new_devices))
     return false;
-  DetectRemovedDevices(plugged_devices, hooks);
-  m_host->DispatchHooks(hooks);
-  return true;
-}
 
-bool USBScanner::AddDevice(std::unique_ptr<USB::Device> device)
-{
   std::lock_guard lk(m_devices_mutex);
-  if (m_devices.contains(device->GetId()))
-    return false;
+  if (!std::ranges::equal(std::views::keys(m_devices), std::views::keys(new_devices)))
+  {
+    m_devices = std::move(new_devices);
+    m_host->OnDevicesChanged(m_devices);
+  }
 
-  m_devices[device->GetId()] = std::move(device);
   return true;
 }
 
-bool USBScanner::AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks,
-                               const bool always_add_hooks)
+bool USBScanner::AddNewDevices(DeviceMap* new_devices) const
 {
-  AddEmulatedDevices(new_devices, hooks, always_add_hooks);
+  AddEmulatedDevices(new_devices);
 #ifdef __LIBUSB__
   if (!Core::WantsDeterminism())
   {
@@ -121,7 +112,7 @@ bool USBScanner::AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& ho
 
         auto usb_device =
             std::make_unique<USB::LibusbDevice>(m_host->GetEmulationKernel(), device, descriptor);
-        CheckAndAddDevice(std::move(usb_device), new_devices, hooks, always_add_hooks);
+        AddDevice(std::move(usb_device), new_devices);
         return true;
       });
       if (ret != LIBUSB_SUCCESS)
@@ -132,60 +123,24 @@ bool USBScanner::AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& ho
   return true;
 }
 
-void USBScanner::DetectRemovedDevices(const std::set<u64>& plugged_devices,
-                                      DeviceChangeHooks& hooks)
-{
-  std::lock_guard lk(m_devices_mutex);
-  for (auto it = m_devices.begin(); it != m_devices.end();)
-  {
-    if (!plugged_devices.contains(it->second->GetId()))
-    {
-      hooks.emplace(it->second, ChangeEvent::Removed);
-      it = m_devices.erase(it);
-    }
-    else
-    {
-      ++it;
-    }
-  }
-}
-
-void USBScanner::AddEmulatedDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks,
-                                    bool always_add_hooks)
+void USBScanner::AddEmulatedDevices(DeviceMap* new_devices) const
 {
   if (Config::Get(Config::MAIN_EMULATE_SKYLANDER_PORTAL) && !NetPlay::IsNetPlayRunning())
   {
     auto skylanderportal = std::make_unique<USB::SkylanderUSB>(m_host->GetEmulationKernel());
-    CheckAndAddDevice(std::move(skylanderportal), new_devices, hooks, always_add_hooks);
+    AddDevice(std::move(skylanderportal), new_devices);
   }
   if (Config::Get(Config::MAIN_EMULATE_INFINITY_BASE) && !NetPlay::IsNetPlayRunning())
   {
     auto infinity_base = std::make_unique<USB::InfinityUSB>(m_host->GetEmulationKernel());
-    CheckAndAddDevice(std::move(infinity_base), new_devices, hooks, always_add_hooks);
+    AddDevice(std::move(infinity_base), new_devices);
   }
 }
 
-void USBScanner::CheckAndAddDevice(std::unique_ptr<USB::Device> device, std::set<u64>& new_devices,
-                                   DeviceChangeHooks& hooks, bool always_add_hooks)
+void USBScanner::AddDevice(std::unique_ptr<USB::Device> device, DeviceMap* new_devices) const
 {
   if (m_host->ShouldAddDevice(*device))
-  {
-    const u64 deviceid = device->GetId();
-    new_devices.insert(deviceid);
-    if (AddDevice(std::move(device)) || always_add_hooks)
-    {
-      hooks.emplace(GetDeviceById(deviceid), ChangeEvent::Inserted);
-    }
-  }
-}
-
-std::shared_ptr<USB::Device> USBScanner::GetDeviceById(const u64 device_id) const
-{
-  std::lock_guard lk(m_devices_mutex);
-  const auto it = m_devices.find(device_id);
-  if (it == m_devices.end())
-    return nullptr;
-  return it->second;
+    (*new_devices)[device->GetId()] = std::move(device);
 }
 
 }  // namespace IOS::HLE

Source/Core/Core/IOS/USB/USBScanner.h

@@ -24,33 +24,24 @@ class USBHost;
 class USBScanner final
 {
 public:
+  using DeviceMap = std::map<u64, std::shared_ptr<USB::Device>>;
+
   explicit USBScanner(USBHost* host);
   ~USBScanner();
 
   void Start();
   void Stop();
   void WaitForFirstScan();
-  bool UpdateDevices(bool always_add_hooks = false);
 
-  enum class ChangeEvent
-  {
-    Inserted,
-    Removed,
-  };
-  using DeviceChangeHooks = std::map<std::shared_ptr<USB::Device>, ChangeEvent>;
+  DeviceMap GetDevices() const;
 
 private:
-  bool AddDevice(std::unique_ptr<USB::Device> device);
-  bool AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks, bool always_add_hooks);
-  void DetectRemovedDevices(const std::set<u64>& plugged_devices, DeviceChangeHooks& hooks);
-  void AddEmulatedDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks,
-                          bool always_add_hooks);
-  void CheckAndAddDevice(std::unique_ptr<USB::Device> device, std::set<u64>& new_devices,
-                         DeviceChangeHooks& hooks, bool always_add_hooks);
+  bool UpdateDevices();
+  bool AddNewDevices(DeviceMap* new_devices) const;
+  void AddEmulatedDevices(DeviceMap* new_devices) const;
+  void AddDevice(std::unique_ptr<USB::Device> device, DeviceMap* new_devices) const;
 
-  std::shared_ptr<USB::Device> GetDeviceById(u64 device_id) const;
-
-  std::map<u64, std::shared_ptr<USB::Device>> m_devices;
+  DeviceMap m_devices;
   mutable std::mutex m_devices_mutex;
 
   USBHost* m_host = nullptr;