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(),
+    const auto& [id, device] = *it;
-                 event == ChangeEvent::Inserted ? "New" : "Removed", device->GetVid(),
+    if (!new_devices.contains(id))
-                 device->GetPid());
+    {
+      INFO_LOG_FMT(IOS_USB, "{} - Removed device: {:04x}:{:04x}", GetDeviceName(), device->GetVid(),
+                   device->GetPid());
 
-    if (event == ChangeEvent::Inserted)
+      changes = true;
-      m_devices.emplace(device->GetId(), device);
+      auto device_copy = std::move(device);
-    else if (event == ChangeEvent::Removed)
+      it = m_devices.erase(it);
-      m_devices.erase(device->GetId());
+      OnDeviceChange(ChangeEvent::Removed, std::move(device_copy));
-
+    }
-    OnDeviceChange(event, device);
+    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;
+  enum class ChangeEvent
-  using DeviceChangeHooks = USBScanner::DeviceChangeHooks;
+  {
+    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.
+USBScanner::DeviceMap USBScanner::GetDevices() const
-  DeviceChangeHooks hooks;
+{
-  DetectRemovedDevices(std::set<u64>(), hooks);
+  std::lock_guard lk(m_devices_mutex);
-  m_host->DispatchHooks(hooks);
+  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;
+  DeviceMap new_devices;
-  std::set<u64> plugged_devices;
+  if (!AddNewDevices(&new_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))
     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()))
+  if (!std::ranges::equal(std::views::keys(m_devices), std::views::keys(new_devices)))
-    return false;
+  {
+    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,
+bool USBScanner::AddNewDevices(DeviceMap* new_devices) const
-                               const bool always_add_hooks)
 {
-  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,
+void USBScanner::AddEmulatedDevices(DeviceMap* new_devices) const
-                                      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)
 {
   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,
+void USBScanner::AddDevice(std::unique_ptr<USB::Device> device, DeviceMap* new_devices) const
-                                   DeviceChangeHooks& hooks, bool always_add_hooks)
 {
   if (m_host->ShouldAddDevice(*device))
-  {
+    (*new_devices)[device->GetId()] = std::move(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;
 }
 
 }  // 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
+  DeviceMap GetDevices() const;
-  {
-    Inserted,
-    Removed,
-  };
-  using DeviceChangeHooks = std::map<std::shared_ptr<USB::Device>, ChangeEvent>;
 
 private:
-  bool AddDevice(std::unique_ptr<USB::Device> device);
+  bool UpdateDevices();
-  bool AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks, bool always_add_hooks);
+  bool AddNewDevices(DeviceMap* new_devices) const;
-  void DetectRemovedDevices(const std::set<u64>& plugged_devices, DeviceChangeHooks& hooks);
+  void AddEmulatedDevices(DeviceMap* new_devices) const;
-  void AddEmulatedDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks,
+  void AddDevice(std::unique_ptr<USB::Device> device, DeviceMap* new_devices) const;
-                          bool always_add_hooks);
-  void CheckAndAddDevice(std::unique_ptr<USB::Device> device, std::set<u64>& new_devices,
-                         DeviceChangeHooks& hooks, bool always_add_hooks);
 
-  std::shared_ptr<USB::Device> GetDeviceById(u64 device_id) const;
+  DeviceMap m_devices;
-
-  std::map<u64, std::shared_ptr<USB::Device>> m_devices;
   mutable std::mutex m_devices_mutex;
 
   USBHost* m_host = nullptr;