Merge pull request #9702 from Filoppi/controller_interface_fixes

Controller Interface refactor

Source/Core/Common/Logging/Log.h

@@ -54,7 +54,6 @@ enum LOG_TYPE
   OSHLE,
   OSREPORT,
   OSREPORT_HLE,
-  PAD,
   PIXELENGINE,
   PROCESSORINTERFACE,
   POWERPC,

Source/Core/Common/Logging/LogManager.cpp

@@ -157,7 +157,6 @@ LogManager::LogManager()
   m_log[OSHLE] = {"HLE", "OSHLE"};
   m_log[OSREPORT] = {"OSREPORT", "OSReport EXI"};
   m_log[OSREPORT_HLE] = {"OSREPORT_HLE", "OSReport HLE"};
-  m_log[PAD] = {"PAD", "Pad"};
   m_log[PIXELENGINE] = {"PE", "Pixel Engine"};
   m_log[PROCESSORINTERFACE] = {"PI", "Processor Interface"};
   m_log[POWERPC] = {"PowerPC", "PowerPC IBM CPU"};

Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp

@@ -96,8 +96,15 @@ static void TryToFillWiimoteSlot(u32 index)
     s_wiimote_pool.erase(s_wiimote_pool.begin());
 }
 
+void PopulateDevices()
+{
+  // There is a very small chance of deadlocks if we didn't do it in another thread
+  s_wiimote_scanner.PopulateDevices();
+}
+
 // Attempts to fill enabled real wiimote slots.
 // Push/pull wiimotes to/from ControllerInterface as needed.
+// Should be called PopulateDevices() to be in line with other implementations.
 void ProcessWiimotePool()
 {
   std::lock_guard lk(g_wiimotes_mutex);
@@ -545,7 +552,7 @@ void WiimoteScanner::StopThread()
 void WiimoteScanner::SetScanMode(WiimoteScanMode scan_mode)
 {
   m_scan_mode.store(scan_mode);
-  m_scan_mode_changed_event.Set();
+  m_scan_mode_changed_or_population_event.Set();
 }
 
 bool WiimoteScanner::IsReady() const
@@ -610,6 +617,12 @@ void WiimoteScanner::PoolThreadFunc()
   }
 }
 
+void WiimoteScanner::PopulateDevices()
+{
+  m_populate_devices.Set();
+  m_scan_mode_changed_or_population_event.Set();
+}
+
 void WiimoteScanner::ThreadFunc()
 {
   std::thread pool_thread(&WiimoteScanner::PoolThreadFunc, this);
@@ -634,7 +647,12 @@ void WiimoteScanner::ThreadFunc()
 
   while (m_scan_thread_running.IsSet())
   {
-    m_scan_mode_changed_event.WaitFor(std::chrono::milliseconds(500));
+    m_scan_mode_changed_or_population_event.WaitFor(std::chrono::milliseconds(500));
+
+    if (m_populate_devices.TestAndClear())
+    {
+      g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); });
+    }
 
     // Does stuff needed to detect disconnects on Windows
     for (const auto& backend : m_backends)
@@ -675,7 +693,7 @@ void WiimoteScanner::ThreadFunc()
           }
 
           AddWiimoteToPool(std::unique_ptr<Wiimote>(wiimote));
-          ProcessWiimotePool();
+          g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); });
         }
 
         if (found_board)
@@ -956,12 +974,12 @@ void HandleWiimoteSourceChange(unsigned int index)
     if (auto removed_wiimote = std::move(g_wiimotes[index]))
       AddWiimoteToPool(std::move(removed_wiimote));
   });
-  ProcessWiimotePool();
+  g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); });
 }
 
 void HandleWiimotesInControllerInterfaceSettingChange()
 {
-  ProcessWiimotePool();
+  g_controller_interface.PlatformPopulateDevices([] { ProcessWiimotePool(); });
 }
 
 }  // namespace WiimoteReal

Source/Core/Core/HW/WiimoteReal/WiimoteReal.h

@@ -171,6 +171,7 @@ class WiimoteScanner
   void StartThread();
   void StopThread();
   void SetScanMode(WiimoteScanMode scan_mode);
+  void PopulateDevices();
 
   bool IsReady() const;
 
@@ -183,7 +184,8 @@ class WiimoteScanner
 
   std::thread m_scan_thread;
   Common::Flag m_scan_thread_running;
-  Common::Event m_scan_mode_changed_event;
+  Common::Flag m_populate_devices;
+  Common::Event m_scan_mode_changed_or_population_event;
   std::atomic<WiimoteScanMode> m_scan_mode{WiimoteScanMode::DO_NOT_SCAN};
 };
 
@@ -204,6 +206,7 @@ void InitAdapterClass();
 #endif
 
 void HandleWiimotesInControllerInterfaceSettingChange();
+void PopulateDevices();
 void ProcessWiimotePool();
 
 }  // namespace WiimoteReal

Source/Core/DolphinQt/Config/Mapping/IOWindow.cpp

@@ -9,8 +9,6 @@
 
 #include <QComboBox>
 #include <QDialogButtonBox>
-#include <QGroupBox>
-#include <QHBoxLayout>
 #include <QHeaderView>
 #include <QItemDelegate>
 #include <QLabel>
@@ -31,6 +29,7 @@
 #include "DolphinQt/Config/Mapping/MappingWindow.h"
 #include "DolphinQt/QtUtils/BlockUserInputFilter.h"
 #include "DolphinQt/QtUtils/ModalMessageBox.h"
+#include "DolphinQt/Settings.h"
 
 #include "InputCommon/ControlReference/ControlReference.h"
 #include "InputCommon/ControlReference/ExpressionParser.h"
@@ -220,6 +219,8 @@ IOWindow::IOWindow(MappingWidget* parent, ControllerEmu::EmulatedController* con
   CreateMainLayout();
 
   connect(parent, &MappingWidget::Update, this, &IOWindow::Update);
+  connect(parent->GetParent(), &MappingWindow::ConfigChanged, this, &IOWindow::ConfigChanged);
+  connect(&Settings::Instance(), &Settings::ConfigChanged, this, &IOWindow::ConfigChanged);
 
   setWindowTitle(type == IOWindow::Type::Input ? tr("Configure Input") : tr("Configure Output"));
   setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
@@ -229,7 +230,7 @@ IOWindow::IOWindow(MappingWidget* parent, ControllerEmu::EmulatedController* con
   ConnectWidgets();
 }
 
-std::shared_ptr<ciface::Core::Device> IOWindow::GetSelectedDevice()
+std::shared_ptr<ciface::Core::Device> IOWindow::GetSelectedDevice() const
 {
   return m_selected_device;
 }
@@ -258,7 +259,7 @@ void IOWindow::CreateMainLayout()
   m_expression_text->setFont(QFontDatabase::systemFont(QFontDatabase::FixedFont));
   new ControlExpressionSyntaxHighlighter(m_expression_text->document());
 
-  m_operators_combo = new QComboBoxWithMouseWheelDisabled();
+  m_operators_combo = new QComboBoxWithMouseWheelDisabled(this);
   m_operators_combo->addItem(tr("Operators"));
   m_operators_combo->insertSeparator(1);
   if (m_type == Type::Input)
@@ -340,6 +341,7 @@ void IOWindow::CreateMainLayout()
     m_option_list->horizontalHeader()->setSectionResizeMode(1, QHeaderView::Fixed);
 
     m_option_list->setItemDelegate(new InputStateDelegate(this, 1, [&](int row) {
+      std::lock_guard lock(m_selected_device_mutex);
       // Clamp off negative values but allow greater than one in the text display.
       return std::max(GetSelectedDevice()->Inputs()[row]->GetState(), 0.0);
     }));
@@ -400,7 +402,7 @@ void IOWindow::CreateMainLayout()
 void IOWindow::ConfigChanged()
 {
   const QSignalBlocker blocker(this);
-  const auto lock = m_controller->GetStateLock();
+  const auto lock = ControllerEmu::EmulatedController::GetStateLock();
 
   // ensure m_parse_text is in the right state
   UpdateExpression(m_reference->GetExpression(), UpdateMode::Force);
@@ -410,10 +412,10 @@ void IOWindow::ConfigChanged()
   m_range_spinbox->setValue(m_reference->range * SLIDER_TICK_COUNT);
   m_range_slider->setValue(m_reference->range * SLIDER_TICK_COUNT);
 
-  m_devq = m_controller->GetDefaultDevice();
+  if (m_devq.ToString().empty())
+    m_devq = m_controller->GetDefaultDevice();
 
   UpdateDeviceList();
-  UpdateOptionList();
 }
 
 void IOWindow::Update()
@@ -425,6 +427,8 @@ void IOWindow::Update()
 void IOWindow::ConnectWidgets()
 {
   connect(m_select_button, &QPushButton::clicked, [this] { AppendSelectedOption(); });
+  connect(&Settings::Instance(), &Settings::ReleaseDevices, this, &IOWindow::ReleaseDevices);
+  connect(&Settings::Instance(), &Settings::DevicesChanged, this, &IOWindow::UpdateDeviceList);
 
   connect(m_detect_button, &QPushButton::clicked, this, &IOWindow::OnDetectButtonPressed);
   connect(m_test_button, &QPushButton::clicked, this, &IOWindow::OnTestButtonPressed);
@@ -479,16 +483,19 @@ void IOWindow::ConnectWidgets()
 
 void IOWindow::AppendSelectedOption()
 {
-  if (m_option_list->currentItem() == nullptr)
+  if (m_option_list->currentRow() < 0)
     return;
 
   m_expression_text->insertPlainText(MappingCommon::GetExpressionForControl(
-      m_option_list->currentItem()->text(), m_devq, m_controller->GetDefaultDevice()));
+      m_option_list->item(m_option_list->currentRow(), 0)->text(), m_devq,
+      m_controller->GetDefaultDevice()));
 }
 
-void IOWindow::OnDeviceChanged(const QString& device)
+void IOWindow::OnDeviceChanged()
 {
-  m_devq.FromString(device.toStdString());
+  const std::string device_name =
+      m_devices_combo->count() > 0 ? m_devices_combo->currentData().toString().toStdString() : "";
+  m_devq.FromString(device_name);
   UpdateOptionList();
 }
 
@@ -500,7 +507,7 @@ void IOWindow::OnDialogButtonPressed(QAbstractButton* button)
     return;
   }
 
-  const auto lock = m_controller->GetStateLock();
+  const auto lock = ControllerEmu::EmulatedController::GetStateLock();
 
   UpdateExpression(m_expression_text->toPlainText().toStdString());
   m_original_expression = m_reference->GetExpression();
@@ -525,6 +532,7 @@ void IOWindow::OnDetectButtonPressed()
 
   const auto list = m_option_list->findItems(expression, Qt::MatchFixedString);
 
+  // Try to select the first. If this fails, the last selected item would still appear as such
   if (!list.empty())
     m_option_list->setCurrentItem(list[0]);
 }
@@ -541,8 +549,15 @@ void IOWindow::OnRangeChanged(int value)
   m_range_slider->setValue(m_reference->range * SLIDER_TICK_COUNT);
 }
 
+void IOWindow::ReleaseDevices()
+{
+  std::lock_guard lock(m_selected_device_mutex);
+  m_selected_device = nullptr;
+}
+
 void IOWindow::UpdateOptionList()
 {
+  std::lock_guard lock(m_selected_device_mutex);
   m_selected_device = g_controller_interface.FindDevice(m_devq);
   m_option_list->setRowCount(0);
 
@@ -575,13 +590,62 @@ void IOWindow::UpdateOptionList()
 
 void IOWindow::UpdateDeviceList()
 {
+  const QSignalBlocker blocker(m_devices_combo);
+
+  const auto previous_device_name = m_devices_combo->currentData().toString().toStdString();
+
   m_devices_combo->clear();
 
+  // Default to the default device or to the first device if there isn't a default.
+  // Try to the keep the previous selected device, mark it as disconnected if it's gone, as it could
+  // reconnect soon after if this is a devices refresh and it would be annoying to lose the value.
+  const auto default_device_name = m_controller->GetDefaultDevice().ToString();
+  int default_device_index = -1;
+  int previous_device_index = -1;
   for (const auto& name : g_controller_interface.GetAllDeviceStrings())
-    m_devices_combo->addItem(QString::fromStdString(name));
+  {
+    QString qname = QString();
+    if (name == default_device_name)
+    {
+      default_device_index = m_devices_combo->count();
+      // Specify "default" even if we only have one device
+      qname.append(QLatin1Char{'['} + tr("default") + QStringLiteral("] "));
+    }
+    if (name == previous_device_name)
+    {
+      previous_device_index = m_devices_combo->count();
+    }
+    qname.append(QString::fromStdString(name));
+    m_devices_combo->addItem(qname, QString::fromStdString(name));
+  }
 
-  m_devices_combo->setCurrentText(
-      QString::fromStdString(m_controller->GetDefaultDevice().ToString()));
+  if (previous_device_index >= 0)
+  {
+    m_devices_combo->setCurrentIndex(previous_device_index);
+  }
+  else if (!previous_device_name.empty())
+  {
+    const QString qname = QString::fromStdString(previous_device_name);
+    QString adjusted_qname;
+    if (previous_device_name == default_device_name)
+    {
+      adjusted_qname.append(QLatin1Char{'['} + tr("default") + QStringLiteral("] "));
+    }
+    adjusted_qname.append(QLatin1Char{'['} + tr("disconnected") + QStringLiteral("] "))
+        .append(qname);
+    m_devices_combo->addItem(adjusted_qname, qname);
+    m_devices_combo->setCurrentIndex(m_devices_combo->count() - 1);
+  }
+  else if (default_device_index >= 0)
+  {
+    m_devices_combo->setCurrentIndex(default_device_index);
+  }
+  else if (m_devices_combo->count() > 0)
+  {
+    m_devices_combo->setCurrentIndex(0);
+  }
+  // The device object might have changed so we need to always refresh it
+  OnDeviceChanged();
 }
 
 void IOWindow::UpdateExpression(std::string new_expression, UpdateMode mode)

Source/Core/DolphinQt/Config/Mapping/IOWindow.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <memory>
+#include <mutex>
 #include <string>
 
 #include <QComboBox>
@@ -69,23 +70,24 @@ class IOWindow final : public QDialog
   explicit IOWindow(MappingWidget* parent, ControllerEmu::EmulatedController* m_controller,
                     ControlReference* ref, Type type);
 
-  std::shared_ptr<ciface::Core::Device> GetSelectedDevice();
-
 private:
+  std::shared_ptr<ciface::Core::Device> GetSelectedDevice() const;
+
   void CreateMainLayout();
   void ConnectWidgets();
   void ConfigChanged();
   void Update();
 
   void OnDialogButtonPressed(QAbstractButton* button);
-  void OnDeviceChanged(const QString& device);
+  void OnDeviceChanged();
   void OnDetectButtonPressed();
   void OnTestButtonPressed();
   void OnRangeChanged(int range);
 
   void AppendSelectedOption();
   void UpdateOptionList();
   void UpdateDeviceList();
+  void ReleaseDevices();
 
   enum class UpdateMode
   {
@@ -135,4 +137,5 @@ class IOWindow final : public QDialog
   ciface::Core::DeviceQualifier m_devq;
   Type m_type;
   std::shared_ptr<ciface::Core::Device> m_selected_device;
+  std::mutex m_selected_device_mutex;
 };

Source/Core/DolphinQt/Config/Mapping/MappingWindow.cpp

@@ -328,7 +328,7 @@ bool MappingWindow::IsMappingAllDevices() const
 
 void MappingWindow::RefreshDevices()
 {
-  Core::RunAsCPUThread([&] { g_controller_interface.RefreshDevices(); });
+  g_controller_interface.RefreshDevices();
 }
 
 void MappingWindow::OnGlobalDevicesChanged()