Merge pull request #7829 from jordan-woyak/detect-input-improve

ControllerInterface/DolphinQt: Improve input detection.

Source/Core/InputCommon/ControlReference/ControlReference.cpp

@@ -2,19 +2,16 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
-#include "Common/Thread.h"
+#include "InputCommon/ControlReference/ControlReference.h"
+
 // For InputGateOn()
 // This is a bad layering violation, but it's the cleanest
 // place I could find to put it.
 #include "Core/ConfigManager.h"
 #include "Core/Host.h"
 
-#include "InputCommon/ControlReference/ControlReference.h"
-
 using namespace ciface::ExpressionParser;
 
-constexpr ControlState INPUT_DETECT_THRESHOLD = 0.55;
-
 bool ControlReference::InputGateOn()
 {
   return SConfig::GetInstance().m_BackgroundInput || Host_RendererHasFocus() ||
@@ -109,86 +106,3 @@ ControlState OutputReference::State(const ControlState state)
     m_parsed_expression->SetValue(state * range);
   return 0.0;
 }
-
-//
-// InputReference :: Detect
-//
-// Wait for input on all binded devices
-// supports not detecting inputs that were held down at the time of Detect start,
-// which is useful for those crazy flightsticks that have certain buttons that are always held down
-// or some crazy axes or something
-// upon input, return pointer to detected Control
-// else return nullptr
-//
-ciface::Core::Device::Control* InputReference::Detect(const unsigned int ms,
-                                                      ciface::Core::Device* const device)
-{
-  unsigned int time = 0;
-  std::vector<bool> states(device->Inputs().size());
-
-  if (device->Inputs().empty())
-    return nullptr;
-
-  // get starting state of all inputs,
-  // so we can ignore those that were activated at time of Detect start
-  std::vector<ciface::Core::Device::Input*>::const_iterator i = device->Inputs().begin(),
-                                                            e = device->Inputs().end();
-  for (std::vector<bool>::iterator state = states.begin(); i != e; ++i)
-    *state++ = ((*i)->GetState() > (1 - INPUT_DETECT_THRESHOLD));
-
-  while (time < ms)
-  {
-    device->UpdateInput();
-    i = device->Inputs().begin();
-    for (std::vector<bool>::iterator state = states.begin(); i != e; ++i, ++state)
-    {
-      // detected an input
-      if ((*i)->IsDetectable() && (*i)->GetState() > INPUT_DETECT_THRESHOLD)
-      {
-        // input was released at some point during Detect call
-        // return the detected input
-        if (false == *state)
-          return *i;
-      }
-      else if ((*i)->GetState() < (1 - INPUT_DETECT_THRESHOLD))
-      {
-        *state = false;
-      }
-    }
-    Common::SleepCurrentThread(10);
-    time += 10;
-  }
-
-  // no input was detected
-  return nullptr;
-}
-
-//
-// OutputReference :: Detect
-//
-// Totally different from the inputReference detect / I have them combined so it was simpler to make
-// the GUI.
-// The GUI doesn't know the difference between an input and an output / it's odd but I was lazy and
-// it was easy
-//
-// set all binded outputs to <range> power for x milliseconds return false
-//
-ciface::Core::Device::Control* OutputReference::Detect(const unsigned int ms,
-                                                       ciface::Core::Device* const device)
-{
-  // ignore device
-
-  // don't hang if we don't even have any controls mapped
-  if (BoundCount() > 0)
-  {
-    State(1);
-    unsigned int slept = 0;
-
-    // this loop is to make stuff like flashing keyboard LEDs work
-    while (ms > (slept += 10))
-      Common::SleepCurrentThread(10);
-
-    State(0);
-  }
-  return nullptr;
-}

Source/Core/InputCommon/ControlReference/ControlReference.h

@@ -26,8 +26,6 @@ public:
 
   virtual ~ControlReference();
   virtual ControlState State(const ControlState state = 0) = 0;
-  virtual ciface::Core::Device::Control* Detect(const unsigned int ms,
-                                                ciface::Core::Device* const device) = 0;
   virtual bool IsInput() const = 0;
 
   int BoundCount() const;
@@ -57,8 +55,6 @@ public:
   InputReference();
   bool IsInput() const override;
   ControlState State(const ControlState state) override;
-  ciface::Core::Device::Control* Detect(const unsigned int ms,
-                                        ciface::Core::Device* const device) override;
 };
 
 //
@@ -72,6 +68,4 @@ public:
   OutputReference();
   bool IsInput() const override;
   ControlState State(const ControlState state) override;
-  ciface::Core::Device::Control* Detect(const unsigned int ms,
-                                        ciface::Core::Device* const device) override;
 };

Source/Core/InputCommon/ControlReference/ExpressionParser.cpp

@@ -217,7 +217,19 @@ public:
   std::shared_ptr<Device> m_device;
 
   explicit ControlExpression(ControlQualifier qualifier_) : qualifier(qualifier_) {}
-  ControlState GetValue() const override { return control ? control->ToInput()->GetState() : 0.0; }
+  ControlState GetValue() const override
+  {
+    if (!control)
+      return 0.0;
+
+    // Note: Inputs may return negative values in situations where opposing directions are
+    // activated. We clamp off the negative values here.
+
+    // FYI: Clamping values greater than 1.0 is purposely not done to support unbounded values in
+    // the future. (e.g. raw accelerometer/gyro data)
+
+    return std::max(0.0, control->ToInput()->GetState());
+  }
   void SetValue(ControlState value) override
   {
     if (control)

Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp

@@ -259,7 +259,7 @@ std::string Joystick::Hat::GetName() const
 
 ControlState Joystick::Axis::GetState() const
 {
-  return std::max(0.0, ControlState(m_axis - m_base) / m_range);
+  return ControlState(m_axis - m_base) / m_range;
 }
 
 ControlState Joystick::Button::GetState() const

Source/Core/InputCommon/ControllerInterface/Device.cpp

@@ -4,22 +4,22 @@
 
 #include "InputCommon/ControllerInterface/Device.h"
 
+#include <cmath>
 #include <memory>
 #include <sstream>
 #include <string>
 #include <tuple>
 
 #include "Common/StringUtil.h"
+#include "Common/Thread.h"
 
 namespace ciface
 {
 namespace Core
 {
-//
-// Device :: ~Device
-//
-// Destructor, delete all inputs/outputs on device destruction
-//
+// Compared to an input's current state (ideally 1.0) minus abs(initial_state) (ideally 0.0).
+constexpr ControlState INPUT_DETECT_THRESHOLD = 0.55;
+
 Device::~Device()
 {
   // delete inputs
@@ -68,6 +68,11 @@ Device::Output* Device::FindOutput(const std::string& name) const
   return nullptr;
 }
 
+ControlState Device::FullAnalogSurface::GetState() const
+{
+  return (1 + std::max(0.0, m_high.GetState()) - std::max(0.0, m_low.GetState())) / 2;
+}
+
 //
 // DeviceQualifier :: ToString
 //
@@ -214,5 +219,83 @@ bool DeviceContainer::HasConnectedDevice(const DeviceQualifier& qualifier) const
   const auto device = FindDevice(qualifier);
   return device != nullptr && device->IsValid();
 }
+
+// Wait for input on a particular device.
+// Inputs are 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.
+// Upon input, return the detected Device and Input, else return nullptrs
+std::pair<std::shared_ptr<Device>, Device::Input*>
+DeviceContainer::DetectInput(u32 wait_ms, std::vector<std::string> device_strings)
+{
+  struct InputState
+  {
+    ciface::Core::Device::Input& input;
+    ControlState initial_state;
+  };
+
+  struct DeviceState
+  {
+    std::shared_ptr<Device> device;
+
+    std::vector<InputState> input_states;
+  };
+
+  // Acquire devices and initial input states.
+  std::vector<DeviceState> device_states;
+  for (auto& device_string : device_strings)
+  {
+    DeviceQualifier dq;
+    dq.FromString(device_string);
+    auto device = FindDevice(dq);
+
+    if (!device)
+      continue;
+
+    std::vector<InputState> input_states;
+
+    for (auto* input : device->Inputs())
+    {
+      // Don't detect things like absolute cursor position.
+      if (!input->IsDetectable())
+        continue;
+
+      // Undesirable axes will have negative values here when trying to map a
+      // "FullAnalogSurface".
+      input_states.push_back({*input, input->GetState()});
+    }
+
+    if (!input_states.empty())
+      device_states.emplace_back(DeviceState{std::move(device), std::move(input_states)});
+  }
+
+  if (device_states.empty())
+    return {};
+
+  u32 time = 0;
+  while (time < wait_ms)
+  {
+    Common::SleepCurrentThread(10);
+    time += 10;
+
+    for (auto& device_state : device_states)
+    {
+      device_state.device->UpdateInput();
+      for (auto& input_state : device_state.input_states)
+      {
+        // 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)
+          return {device_state.device, &input_state.input};
+      }
+    }
+  }
+
+  // No input was detected. :'(
+  return {};
 }
-}
+
+}  // namespace Core
+}  // namespace ciface

Source/Core/InputCommon/ControllerInterface/Device.h

@@ -55,9 +55,22 @@ public:
   class Input : public Control
   {
   public:
-    // things like absolute axes/ absolute mouse position will override this
+    // Things like absolute axes/ absolute mouse position should override this to prevent
+    // undesirable behavior in our mapping logic.
     virtual bool IsDetectable() { return true; }
+
+    // Implementations should return a value from 0.0 to 1.0 across their normal range.
+    // One input should be provided for each "direction". (e.g. 2 for each axis)
+    // If possible, negative values may be returned in situations where an opposing input is
+    // activated. (e.g. When an underlying axis, X, is currently negative, "Axis X-", will return a
+    // positive value and "Axis X+" may return a negative value.)
+    // Doing so is solely to allow our input detection logic to better detect false positives.
+    // This is necessary when making use of "FullAnalogSurface" as multiple inputs will be seen
+    // increasing from 0.0 to 1.0 as a user tries to map just one. The negative values provide a
+    // view of the underlying axis. (Negative values are clamped off before they reach
+    // expression-parser or controller-emu)
     virtual ControlState GetState() const = 0;
+
     Input* ToInput() override { return this; }
   };
 
@@ -96,11 +109,7 @@ protected:
   {
   public:
     FullAnalogSurface(Input* low, Input* high) : m_low(*low), m_high(*high) {}
-    ControlState GetState() const override
-    {
-      return (1 + m_high.GetState() - m_low.GetState()) / 2;
-    }
-
+    ControlState GetState() const override;
     std::string GetName() const override { return m_low.GetName() + *m_high.GetName().rbegin(); }
 
   private:
@@ -163,6 +172,9 @@ public:
 
   bool HasConnectedDevice(const DeviceQualifier& qualifier) const;
 
+  std::pair<std::shared_ptr<Device>, Device::Input*>
+  DetectInput(u32 wait_ms, std::vector<std::string> device_strings);
+
 protected:
   mutable std::mutex m_devices_mutex;
   std::vector<std::shared_ptr<Device>> m_devices;

Source/Core/InputCommon/ControllerInterface/XInput/XInput.cpp

@@ -228,7 +228,7 @@ ControlState Device::Trigger::GetState() const
 
 ControlState Device::Axis::GetState() const
 {
-  return std::max(0.0, ControlState(m_axis) / m_range);
+  return ControlState(m_axis) / m_range;
 }
 
 void Device::Motor::SetState(ControlState state)
@@ -236,5 +236,5 @@ void Device::Motor::SetState(ControlState state)
   m_motor = (WORD)(state * m_range);
   m_parent->UpdateMotors();
 }
-}
-}
+}  // namespace XInput
+}  // namespace ciface

Source/Core/InputCommon/ControllerInterface/evdev/evdev.cpp

@@ -343,7 +343,7 @@ ControlState evdevDevice::Axis::GetState() const
   int value = 0;
   libevdev_fetch_event_value(m_dev, EV_ABS, m_code, &value);
 
-  return std::max(0.0, ControlState(value - m_base) / m_range);
+  return ControlState(value - m_base) / m_range;
 }
 
 evdevDevice::Effect::Effect(int fd) : m_fd(fd)