InputCommon: Improve input detection to produce buton combinations.

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

@@ -5,6 +5,7 @@
 #include "DolphinQt/Config/Mapping/MappingCommon.h"
 
 #include <tuple>
+#include <vector>
 
 #include <QApplication>
 #include <QPushButton>
@@ -14,14 +15,23 @@
 
 #include "DolphinQt/QtUtils/BlockUserInputFilter.h"
 #include "InputCommon/ControlReference/ControlReference.h"
-#include "InputCommon/ControllerInterface/Device.h"
 
 #include "Common/Thread.h"
 
 namespace MappingCommon
 {
-constexpr int INPUT_DETECT_TIME = 3000;
-constexpr int OUTPUT_TEST_TIME = 2000;
+constexpr auto INPUT_DETECT_INITIAL_TIME = std::chrono::seconds(3);
+constexpr auto INPUT_DETECT_CONFIRMATION_TIME = std::chrono::milliseconds(500);
+constexpr auto INPUT_DETECT_MAXIMUM_TIME = std::chrono::seconds(5);
+
+constexpr auto OUTPUT_TEST_TIME = std::chrono::seconds(2);
+
+// Pressing inputs at the same time will result in the & operator vs a hotkey expression.
+constexpr auto HOTKEY_VS_CONJUNCION_THRESHOLD = std::chrono::milliseconds(50);
+
+// Some devices (e.g. DS4) provide an analog and digital input for the trigger.
+// We prefer just the analog input for simultaneous digital+analog input detections.
+constexpr auto SPURIOUS_TRIGGER_COMBO_THRESHOLD = std::chrono::milliseconds(150);
 
 QString GetExpressionForControl(const QString& control_name,
                                 const ciface::Core::DeviceQualifier& control_device,
@@ -68,7 +78,11 @@ QString DetectExpression(QPushButton* button, ciface::Core::DeviceContainer& dev
   // Avoid that the button press itself is registered as an event
   Common::SleepCurrentThread(50);
 
-  const auto detections = device_container.DetectInput(INPUT_DETECT_TIME, device_strings);
+  auto detections =
+      device_container.DetectInput(device_strings, INPUT_DETECT_INITIAL_TIME,
+                                   INPUT_DETECT_CONFIRMATION_TIME, INPUT_DETECT_MAXIMUM_TIME);
+
+  RemoveSpuriousTriggerCombinations(&detections);
 
   const auto timer = new QTimer(button);
 
@@ -83,30 +97,7 @@ QString DetectExpression(QPushButton* button, ciface::Core::DeviceContainer& dev
 
   button->setText(old_text);
 
-  QString full_expression;
-
-  for (auto [device, input] : detections)
-  {
-    ciface::Core::DeviceQualifier device_qualifier;
-    device_qualifier.FromDevice(device.get());
-
-    if (!full_expression.isEmpty())
-      full_expression += QChar::fromLatin1('+');
-
-    // Return the parent-most name if there is one for better hotkey strings.
-    // Detection of L/R_Ctrl will be changed to just Ctrl.
-    // Users can manually map L_Ctrl if they so desire.
-    if (quote == Quote::On)
-      input = device->GetParentMostInput(input);
-
-    full_expression += MappingCommon::GetExpressionForControl(
-        QString::fromStdString(input->GetName()), device_qualifier, default_device, quote);
-  }
-
-  if (detections.size() > 1)
-    return QStringLiteral("@(%1)").arg(std::move(full_expression));
-
-  return full_expression;
+  return BuildExpression(detections, default_device, quote);
 }
 
 void TestOutput(QPushButton* button, OutputReference* reference)
@@ -118,10 +109,103 @@ void TestOutput(QPushButton* button, OutputReference* reference)
   QApplication::processEvents();
 
   reference->State(1.0);
-  Common::SleepCurrentThread(OUTPUT_TEST_TIME);
+  std::this_thread::sleep_for(OUTPUT_TEST_TIME);
   reference->State(0.0);
 
   button->setText(old_text);
 }
 
+void RemoveSpuriousTriggerCombinations(
+    std::vector<ciface::Core::DeviceContainer::InputDetection>* detections)
+{
+  const auto is_spurious = [&](auto& detection) {
+    return std::any_of(detections->begin(), detections->end(), [&](auto& d) {
+      // This is a suprious digital detection if a "smooth" (analog) detection is temporally near.
+      return &d != &detection && d.smoothness > 1 &&
+             abs(d.press_time - detection.press_time) < SPURIOUS_TRIGGER_COMBO_THRESHOLD;
+    });
+  };
+
+  detections->erase(std::remove_if(detections->begin(), detections->end(), is_spurious),
+                    detections->end());
+}
+
+QString
+BuildExpression(const std::vector<ciface::Core::DeviceContainer::InputDetection>& detections,
+                const ciface::Core::DeviceQualifier& default_device, Quote quote)
+{
+  std::vector<const ciface::Core::DeviceContainer::InputDetection*> pressed_inputs;
+
+  QStringList alternations;
+
+  const auto get_control_expression = [&](auto& detection) {
+    // Return the parent-most name if there is one for better hotkey strings.
+    // Detection of L/R_Ctrl will be changed to just Ctrl.
+    // Users can manually map L_Ctrl if they so desire.
+    const auto input = (quote == Quote::On) ?
+                           detection.device->GetParentMostInput(detection.input) :
+                           detection.input;
+
+    ciface::Core::DeviceQualifier device_qualifier;
+    device_qualifier.FromDevice(detection.device.get());
+
+    return MappingCommon::GetExpressionForControl(QString::fromStdString(input->GetName()),
+                                                  device_qualifier, default_device, quote);
+  };
+
+  bool new_alternation = false;
+
+  const auto handle_press = [&](auto& detection) {
+    pressed_inputs.emplace_back(&detection);
+    new_alternation = true;
+  };
+
+  const auto handle_release = [&]() {
+    if (!new_alternation)
+      return;
+
+    new_alternation = false;
+
+    QStringList alternation;
+    for (auto* input : pressed_inputs)
+      alternation.push_back(get_control_expression(*input));
+
+    const bool is_hotkey = pressed_inputs.size() >= 2 &&
+                           (pressed_inputs[1]->press_time - pressed_inputs[0]->press_time) >
+                               HOTKEY_VS_CONJUNCION_THRESHOLD;
+
+    if (is_hotkey)
+    {
+      alternations.push_back(QStringLiteral("@(%1)").arg(alternation.join(QLatin1Char('+'))));
+    }
+    else
+    {
+      alternation.sort();
+      alternations.push_back(alternation.join(QLatin1Char('&')));
+    }
+  };
+
+  for (auto& detection : detections)
+  {
+    // Remove since released inputs.
+    for (auto it = pressed_inputs.begin(); it != pressed_inputs.end();)
+    {
+      if (!((*it)->release_time > detection.press_time))
+      {
+        handle_release();
+        it = pressed_inputs.erase(it);
+      }
+      else
+        ++it;
+    }
+
+    handle_press(detection);
+  }
+
+  handle_release();
+
+  alternations.removeDuplicates();
+  return alternations.join(QLatin1Char('|'));
+}
+
 }  // namespace MappingCommon

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

@@ -7,16 +7,12 @@
 #include <string>
 #include <vector>
 
+#include "InputCommon/ControllerInterface/Device.h"
+
 class QString;
 class OutputReference;
 class QPushButton;
 
-namespace ciface::Core
-{
-class DeviceContainer;
-class DeviceQualifier;
-}  // namespace ciface::Core
-
 namespace MappingCommon
 {
 enum class Quote
@@ -37,4 +33,9 @@ QString DetectExpression(QPushButton* button, ciface::Core::DeviceContainer& dev
 
 void TestOutput(QPushButton* button, OutputReference* reference);
 
+void RemoveSpuriousTriggerCombinations(std::vector<ciface::Core::DeviceContainer::InputDetection>*);
+
+QString BuildExpression(const std::vector<ciface::Core::DeviceContainer::InputDetection>&,
+                        const ciface::Core::DeviceQualifier& default_device, Quote quote);
+
 }  // namespace MappingCommon

Source/Core/InputCommon/ControllerInterface/Device.cpp

@@ -13,6 +13,7 @@
 
 #include <fmt/format.h>
 
+#include "Common/MathUtil.h"
 #include "Common/Thread.h"
 
 namespace ciface::Core
@@ -301,19 +302,54 @@ bool DeviceContainer::HasConnectedDevice(const DeviceQualifier& qualifier) const
   return device != nullptr && device->IsValid();
 }
 
-// Wait for input on a particular device.
-// Inputs are considered if they are first seen in a neutral state.
+// Wait for inputs on supplied devices.
+// Inputs are only considered if they are first seen in a neutral state.
 // This is useful for crazy flightsticks that have certain buttons that are always held down
 // and also properly handles detection when using "FullAnalogSurface" inputs.
-// Detects multiple inputs if they are pressed before others are released.
-// Upon input, return the detected Device and Input pairs, else return an empty container
-std::vector<std::pair<std::shared_ptr<Device>, Device::Input*>>
-DeviceContainer::DetectInput(u32 wait_ms, const std::vector<std::string>& device_strings) const
+// Multiple detections are returned until the various timeouts have been reached.
+auto DeviceContainer::DetectInput(const std::vector<std::string>& device_strings,
+                                  std::chrono::milliseconds initial_wait,
+                                  std::chrono::milliseconds confirmation_wait,
+                                  std::chrono::milliseconds maximum_wait) const
+    -> std::vector<InputDetection>
 {
   struct InputState
   {
+    InputState(ciface::Core::Device::Input* input_) : input{input_} { stats.Push(0.0); }
+
     ciface::Core::Device::Input* input;
-    ControlState initial_state;
+    ControlState initial_state = input->GetState();
+    ControlState last_state = initial_state;
+    MathUtil::RunningVariance<ControlState> stats;
+
+    // Prevent multiiple detections until after release.
+    bool is_ready = true;
+
+    void Update()
+    {
+      const auto new_state = input->GetState();
+
+      if (!is_ready && new_state < (1 - INPUT_DETECT_THRESHOLD))
+      {
+        last_state = new_state;
+        is_ready = true;
+        stats.Clear();
+      }
+
+      const auto difference = new_state - last_state;
+      stats.Push(difference);
+      last_state = new_state;
+    }
+
+    bool IsPressed()
+    {
+      if (!is_ready)
+        return false;
+
+      // We want an input that was initially 0.0 and currently 1.0.
+      const auto detection_score = (last_state - std::abs(initial_state));
+      return detection_score > INPUT_DETECT_THRESHOLD;
+    }
   };
 
   struct DeviceState
@@ -338,13 +374,13 @@ DeviceContainer::DetectInput(u32 wait_ms, const std::vector<std::string>& device
 
     for (auto* input : device->Inputs())
     {
-      // Don't detect things like absolute cursor position.
+      // Don't detect things like absolute cursor positions, accelerometers, or gyroscopes.
       if (!input->IsDetectable())
         continue;
 
       // Undesirable axes will have negative values here when trying to map a
       // "FullAnalogSurface".
-      input_states.push_back({input, input->GetState()});
+      input_states.push_back(InputState{input});
     }
 
     if (!input_states.empty())
@@ -354,44 +390,59 @@ DeviceContainer::DetectInput(u32 wait_ms, const std::vector<std::string>& device
   if (device_states.empty())
     return {};
 
-  std::vector<std::pair<std::shared_ptr<Device>, Device::Input*>> detections;
+  std::vector<InputDetection> detections;
 
-  u32 time = 0;
-  while (time < wait_ms)
+  const auto start_time = Clock::now();
+  while (true)
   {
+    const auto now = Clock::now();
+    const auto elapsed_time = now - start_time;
+
+    if (elapsed_time >= maximum_wait || (detections.empty() && elapsed_time >= initial_wait) ||
+        (!detections.empty() && detections.back().release_time.has_value() &&
+         now >= *detections.back().release_time + confirmation_wait))
+    {
+      break;
+    }
+
     Common::SleepCurrentThread(10);
-    time += 10;
 
     for (auto& device_state : device_states)
     {
       for (std::size_t i = 0; i != device_state.input_states.size(); ++i)
       {
         auto& input_state = device_state.input_states[i];
+        input_state.Update();
 
-        // We want an input that was initially 0.0 and currently 1.0.
-        const auto detection_score =
-            (input_state.input->GetState() - std::abs(input_state.initial_state));
-
-        if (detection_score > INPUT_DETECT_THRESHOLD)
+        if (input_state.IsPressed())
         {
+          input_state.is_ready = false;
+
+          // Digital presses will evaluate as 1 here.
+          // Analog presses will evaluate greater than 1.
+          const auto smoothness =
+              1 / std::sqrt(input_state.stats.Variance() / input_state.stats.Mean());
+
+          InputDetection new_detection;
+          new_detection.device = device_state.device;
+          new_detection.input = input_state.input;
+          new_detection.press_time = Clock::now();
+          new_detection.smoothness = smoothness;
+
           // We found an input. Add it to our detections.
-          detections.emplace_back(device_state.device, input_state.input);
-
-          // And remove from input_states to prevent more detections.
-          device_state.input_states.erase(device_state.input_states.begin() + i--);
+          detections.emplace_back(std::move(new_detection));
         }
       }
     }
 
-    for (auto& detection : detections)
+    // Check for any releases of our detected inputs.
+    for (auto& d : detections)
     {
-      // If one of our detected inputs is released we are done.
-      if (detection.second->GetState() < (1 - INPUT_DETECT_THRESHOLD))
-        return detections;
+      if (!d.release_time.has_value() && d.input->GetState() < (1 - INPUT_DETECT_THRESHOLD))
+        d.release_time = Clock::now();
     }
   }
 
-  // No input was detected. :'(
-  return {};
+  return detections;
 }
 }  // namespace ciface::Core

Source/Core/InputCommon/ControllerInterface/Device.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <chrono>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -194,6 +195,17 @@ public:
 class DeviceContainer
 {
 public:
+  using Clock = std::chrono::steady_clock;
+
+  struct InputDetection
+  {
+    std::shared_ptr<Device> device;
+    Device::Input* input;
+    Clock::time_point press_time;
+    std::optional<Clock::time_point> release_time;
+    ControlState smoothness;
+  };
+
   Device::Input* FindInput(std::string_view name, const Device* def_dev) const;
   Device::Output* FindOutput(std::string_view name, const Device* def_dev) const;
 
@@ -203,8 +215,10 @@ public:
 
   bool HasConnectedDevice(const DeviceQualifier& qualifier) const;
 
-  std::vector<std::pair<std::shared_ptr<Device>, Device::Input*>>
-  DetectInput(u32 wait_ms, const std::vector<std::string>& device_strings) const;
+  std::vector<InputDetection> DetectInput(const std::vector<std::string>& device_strings,
+                                          std::chrono::milliseconds initial_wait,
+                                          std::chrono::milliseconds confirmation_wait,
+                                          std::chrono::milliseconds maximum_wait) const;
 
 protected:
   mutable std::recursive_mutex m_devices_mutex;