Merge pull request #7640 from jordan-woyak/input-fixes

ControllerInterface: Output/Rumble fixes

Source/Core/Core/Core.cpp

@@ -227,12 +227,10 @@ static void ResetRumble()
 #if defined(__LIBUSB__)
   GCAdapter::ResetRumble();
 #endif
-#if defined(CIFACE_USE_XINPUT) || defined(CIFACE_USE_DINPUT)
   if (!Pad::IsInitialized())
     return;
   for (int i = 0; i < 4; ++i)
     Pad::ResetRumble(i);
-#endif
 }
 
 // Called from GUI thread

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

@@ -12,6 +12,9 @@
 
 namespace MappingCommon
 {
+constexpr int INPUT_DETECT_TIME = 3000;
+constexpr int OUTPUT_DETECT_TIME = 2000;
+
 QString GetExpressionForControl(const QString& control_name,
                                 const ciface::Core::DeviceQualifier& control_device,
                                 const ciface::Core::DeviceQualifier& default_device, Quote quote)
@@ -41,7 +44,9 @@ QString GetExpressionForControl(const QString& control_name,
 QString DetectExpression(ControlReference* reference, ciface::Core::Device* device,
                          const ciface::Core::DeviceQualifier& default_device, Quote quote)
 {
-  ciface::Core::Device::Control* const ctrl = reference->Detect(5000, device);
+  const int ms = reference->IsInput() ? INPUT_DETECT_TIME : OUTPUT_DETECT_TIME;
+
+  ciface::Core::Device::Control* const ctrl = reference->Detect(ms, device);
 
   if (ctrl)
   {

Source/Core/InputCommon/ControlReference/ExpressionParser.cpp

@@ -340,11 +340,7 @@ public:
   }
 
   ControlState GetValue() const override { return GetActiveChild()->GetValue(); }
-  void SetValue(ControlState value) override
+  void SetValue(ControlState value) override { GetActiveChild()->SetValue(value); }
-  {
-    m_lhs->SetValue(GetActiveChild() == m_lhs ? value : 0.0);
-    m_rhs->SetValue(GetActiveChild() == m_rhs ? value : 0.0);
-  }
 
   int CountNumControls() const override { return GetActiveChild()->CountNumControls(); }
   operator std::string() const override
@@ -549,5 +545,5 @@ std::pair<ParseStatus, std::unique_ptr<Expression>> ParseExpression(const std::s
                                                             std::move(complex_result.expr));
   return std::make_pair(complex_result.status, std::move(combined_expr));
 }
-}
+}  // namespace ExpressionParser
-}
+}  // namespace ciface

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

@@ -6,6 +6,7 @@
 #include <map>
 #include <sstream>
 
+#include "Common/Logging/Log.h"
 #include "InputCommon/ControllerInterface/ControllerInterface.h"
 #include "InputCommon/ControllerInterface/DInput/DInput.h"
 #include "InputCommon/ControllerInterface/DInput/DInputJoystick.h"
@@ -40,8 +41,10 @@ void InitJoystick(IDirectInput8* const idi8, HWND hwnd)
         if (FAILED(js_device->SetCooperativeLevel(GetAncestor(hwnd, GA_ROOT),
                                                   DISCL_BACKGROUND | DISCL_EXCLUSIVE)))
         {
-          // PanicAlert("SetCooperativeLevel(DISCL_EXCLUSIVE) failed!");
+          WARN_LOG(
-          // fall back to non-exclusive mode, with no rumble
+              PAD,
+              "DInput: Failed to acquire device exclusively. Force feedback will be unavailable.");
+          // Fall back to non-exclusive mode, with no rumble
           if (FAILED(
                   js_device->SetCooperativeLevel(nullptr, DISCL_BACKGROUND | DISCL_NONEXCLUSIVE)))
           {
@@ -136,20 +139,27 @@ Joystick::Joystick(/*const LPCDIDEVICEINSTANCE lpddi, */ const LPDIRECTINPUTDEVI
     }
   }
 
-  // force feedback
+  // Force feedback:
   std::list<DIDEVICEOBJECTINSTANCE> objects;
   if (SUCCEEDED(m_device->EnumObjects(DIEnumDeviceObjectsCallback, (LPVOID)&objects, DIDFT_AXIS)))
   {
-    InitForceFeedback(m_device, (int)objects.size());
+    const int num_ff_axes =
+        std::count_if(std::begin(objects), std::end(objects), [](DIDEVICEOBJECTINSTANCE& pdidoi) {
+          return pdidoi.dwFlags && DIDOI_FFACTUATOR;
+        });
+    InitForceFeedback(m_device, num_ff_axes);
   }
 
-  ZeroMemory(&m_state_in, sizeof(m_state_in));
+  // Zero inputs:
-  // set hats to center
+  m_state_in = {};
-  memset(m_state_in.rgdwPOV, 0xFF, sizeof(m_state_in.rgdwPOV));
+  // Set hats to center:
+  std::fill(std::begin(m_state_in.rgdwPOV), std::end(m_state_in.rgdwPOV), 0xFF);
 }
 
 Joystick::~Joystick()
 {
+  DeInitForceFeedback();
+
   m_device->Unacquire();
   m_device->Release();
 }
@@ -265,5 +275,5 @@ ControlState Joystick::Hat::GetState() const
 
   return (abs((int)(m_hat / 4500 - m_direction * 2 + 8) % 8 - 4) > 2);
 }
-}
+}  // namespace DInput
-}
+}  // namespace ciface

Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.cpp

@@ -3,23 +3,33 @@
 // Refer to the license.txt file included.
 
 #include "InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h"
-#include <algorithm>
+
+#include <array>
 #include <string>
+
 #include "Common/Thread.h"
 
 namespace ciface
 {
 namespace ForceFeedback
 {
-// template instantiation
+// 100Hz which homebrew docs very roughly imply is within WiiMote normal
-template class ForceFeedbackDevice::Force<DICONSTANTFORCE>;
+// range, used for periodic haptic effects though often ignored by devices
-template class ForceFeedbackDevice::Force<DIRAMPFORCE>;
+constexpr int RUMBLE_PERIOD = DI_SECONDS / 100;
-template class ForceFeedbackDevice::Force<DIPERIODIC>;
+// This needs to be at least as long as the longest rumble that might ever be played.
+// Too short and it's going to stop in the middle of a long effect.
+// "INFINITE" is invalid for ramp effects and probably not sensible.
+constexpr int RUMBLE_LENGTH_MAX = DI_SECONDS * 10;
+
+// Template instantiation:
+template class ForceFeedbackDevice::TypedForce<DICONSTANTFORCE>;
+template class ForceFeedbackDevice::TypedForce<DIRAMPFORCE>;
+template class ForceFeedbackDevice::TypedForce<DIPERIODIC>;
 
 struct ForceType
 {
   GUID guid;
-  const std::string name;
+  const char* name;
 };
 
 static const ForceType force_type_names[] = {
@@ -36,43 +46,79 @@ static const ForceType force_type_names[] = {
     //{GUID_Friction, "Friction"},
 };
 
-bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, int cAxes)
+void ForceFeedbackDevice::DeInitForceFeedback()
 {
-  if (cAxes == 0)
+  if (!m_run_thread.TestAndClear())
+    return;
+
+  SignalUpdateThread();
+  m_update_thread.join();
+}
+
+void ForceFeedbackDevice::ThreadFunc()
+{
+  Common::SetCurrentThreadName("ForceFeedback update thread");
+
+  while (m_run_thread.IsSet())
+  {
+    m_update_event.Wait();
+
+    for (auto output : Outputs())
+    {
+      auto& force = *static_cast<Force*>(output);
+      force.UpdateOutput();
+    }
+  }
+
+  for (auto output : Outputs())
+  {
+    auto& force = *static_cast<Force*>(output);
+    force.Release();
+  }
+}
+
+void ForceFeedbackDevice::SignalUpdateThread()
+{
+  m_update_event.Set();
+}
+
+bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, int axis_count)
+{
+  if (axis_count == 0)
     return false;
 
-  // TODO: check for DIDC_FORCEFEEDBACK in devcaps?
+  // We just use the X axis (for wheel left/right).
+  // Gamepads seem to not care which axis you use.
+  // These are temporary for creating the effect:
+  std::array<DWORD, 1> rgdwAxes = {DIJOFS_X};
+  std::array<LONG, 1> rglDirection = {-200};
 
-  // temporary
+  DIEFFECT eff{};
-  DWORD rgdwAxes[2] = {DIJOFS_X, DIJOFS_Y};
+  eff.dwSize = sizeof(eff);
-  LONG rglDirection[2] = {-200, 0};
-
-  DIEFFECT eff;
-  memset(&eff, 0, sizeof(eff));
-  eff.dwSize = sizeof(DIEFFECT);
   eff.dwFlags = DIEFF_CARTESIAN | DIEFF_OBJECTOFFSETS;
-  eff.dwDuration = INFINITE;  // (4 * DI_SECONDS)
+  eff.dwDuration = RUMBLE_LENGTH_MAX;
   eff.dwSamplePeriod = 0;
   eff.dwGain = DI_FFNOMINALMAX;
   eff.dwTriggerButton = DIEB_NOTRIGGER;
   eff.dwTriggerRepeatInterval = 0;
-  eff.cAxes = std::min<DWORD>(1, cAxes);
+  eff.cAxes = DWORD(rgdwAxes.size());
-  eff.rgdwAxes = rgdwAxes;
+  eff.rgdwAxes = rgdwAxes.data();
-  eff.rglDirection = rglDirection;
+  eff.rglDirection = rglDirection.data();
+  eff.dwStartDelay = 0;
 
-  // initialize parameters
+  // Initialize parameters with zero force (their current state).
-  DICONSTANTFORCE diCF = {-10000};
+  DICONSTANTFORCE diCF{};
-  diCF.lMagnitude = DI_FFNOMINALMAX;
+  diCF.lMagnitude = 0;
-  DIRAMPFORCE diRF = {0};
+  DIRAMPFORCE diRF{};
-  DIPERIODIC diPE = {0};
+  diRF.lStart = diRF.lEnd = 0;
+  DIPERIODIC diPE{};
+  diPE.dwMagnitude = 0;
+  // Is it sensible to have a zero-offset?
+  diPE.lOffset = 0;
+  diPE.dwPhase = 0;
+  diPE.dwPeriod = RUMBLE_PERIOD;
 
-  // doesn't seem needed
+  for (auto& f : force_type_names)
-  // DIENVELOPE env;
-  // eff.lpEnvelope = &env;
-  // ZeroMemory(&env, sizeof(env));
-  // env.dwSize = sizeof(env);
-
-  for (const ForceType& f : force_type_names)
   {
     if (f.guid == GUID_ConstantForce)
     {
@@ -86,7 +132,7 @@ bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, i
     }
     else
     {
-      // all other forces need periodic parameters
+      // All other forces need periodic parameters:
       eff.cbTypeSpecificParams = sizeof(DIPERIODIC);
       eff.lpvTypeSpecificParams = &diPE;
     }
@@ -95,15 +141,15 @@ bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, i
     if (SUCCEEDED(device->CreateEffect(f.guid, &eff, &pEffect, nullptr)))
     {
       if (f.guid == GUID_ConstantForce)
-        AddOutput(new ForceConstant(f.name, pEffect));
+        AddOutput(new ForceConstant(this, f.name, pEffect, diCF));
       else if (f.guid == GUID_RampForce)
-        AddOutput(new ForceRamp(f.name, pEffect));
+        AddOutput(new ForceRamp(this, f.name, pEffect, diRF));
       else
-        AddOutput(new ForcePeriodic(f.name, pEffect));
+        AddOutput(new ForcePeriodic(this, f.name, pEffect, diPE));
     }
   }
 
-  // disable autocentering
+  // Disable autocentering:
   if (Outputs().size())
   {
     DIPROPDWORD dipdw;
@@ -113,95 +159,113 @@ bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, i
     dipdw.diph.dwHow = DIPH_DEVICE;
     dipdw.dwData = DIPROPAUTOCENTER_OFF;
     device->SetProperty(DIPROP_AUTOCENTER, &dipdw.diph);
+
+    m_run_thread.Set();
+    m_update_thread = std::thread(&ForceFeedbackDevice::ThreadFunc, this);
   }
 
   return true;
 }
 
 template <typename P>
-ForceFeedbackDevice::Force<P>::~Force()
+void ForceFeedbackDevice::TypedForce<P>::PlayEffect()
 {
-  m_iface->Stop();
+  DIEFFECT eff{};
-  m_iface->Unload();
+  eff.dwSize = sizeof(eff);
-  m_iface->Release();
+  eff.cbTypeSpecificParams = sizeof(m_params);
+  eff.lpvTypeSpecificParams = &m_params;
+
+  m_effect->SetParameters(&eff, DIEP_START | DIEP_TYPESPECIFICPARAMS);
 }
 
 template <typename P>
-void ForceFeedbackDevice::Force<P>::Update()
+void ForceFeedbackDevice::TypedForce<P>::StopEffect()
 {
-  DIEFFECT eff = {};
+  m_effect->Stop();
-  eff.dwSize = sizeof(DIEFFECT);
-  eff.dwFlags = DIEFF_CARTESIAN | DIEFF_OBJECTOFFSETS;
-
-  eff.cbTypeSpecificParams = sizeof(P);
-  eff.lpvTypeSpecificParams = &params;
-
-  // set params and start effect
-  m_iface->SetParameters(&eff, DIEP_TYPESPECIFICPARAMS | DIEP_START);
-}
-
-template <typename P>
-void ForceFeedbackDevice::Force<P>::Stop()
-{
-  m_iface->Stop();
 }
 
 template <>
-void ForceFeedbackDevice::ForceConstant::SetState(const ControlState state)
+bool ForceFeedbackDevice::ForceConstant::UpdateParameters(int magnitude)
 {
-  const LONG new_val = LONG(10000 * state);
+  const auto old_magnitude = m_params.lMagnitude;
 
-  if (params.lMagnitude == new_val)
+  m_params.lMagnitude = magnitude;
-    return;
 
-  params.lMagnitude = new_val;
+  return old_magnitude != m_params.lMagnitude;
-  if (new_val)
-    Update();
-  else
-    Stop();
 }
 
 template <>
-void ForceFeedbackDevice::ForceRamp::SetState(const ControlState state)
+bool ForceFeedbackDevice::ForceRamp::UpdateParameters(int magnitude)
 {
-  const LONG new_val = LONG(10000 * state);
+  const auto old_magnitude = m_params.lStart;
 
-  if (params.lStart == new_val)
+  // Having the same "start" and "end" here is a bit odd..
-    return;
+  // But ramp forces don't really make sense for our rumble effects anyways..
+  m_params.lStart = m_params.lEnd = magnitude;
 
-  params.lStart = params.lEnd = new_val;
+  return old_magnitude != m_params.lStart;
-  if (new_val)
-    Update();
-  else
-    Stop();
 }
 
 template <>
-void ForceFeedbackDevice::ForcePeriodic::SetState(const ControlState state)
+bool ForceFeedbackDevice::ForcePeriodic::UpdateParameters(int magnitude)
 {
-  const DWORD new_val = DWORD(10000 * state);
+  const auto old_magnitude = m_params.dwMagnitude;
 
-  if (params.dwMagnitude == new_val)
+  m_params.dwMagnitude = magnitude;
-    return;
 
-  params.dwMagnitude = new_val;
+  return old_magnitude != m_params.dwMagnitude;
-  if (new_val)
-    Update();
-  else
-    Stop();
 }
 
 template <typename P>
-ForceFeedbackDevice::Force<P>::Force(const std::string& name, LPDIRECTINPUTEFFECT iface)
+ForceFeedbackDevice::TypedForce<P>::TypedForce(ForceFeedbackDevice* parent, const char* name,
-    : m_name(name), m_iface(iface)
+                                               LPDIRECTINPUTEFFECT effect, const P& params)
+    : Force(parent, name, effect), m_params(params)
 {
-  memset(&params, 0, sizeof(params));
 }
 
 template <typename P>
-std::string ForceFeedbackDevice::Force<P>::GetName() const
+void ForceFeedbackDevice::TypedForce<P>::UpdateEffect(int magnitude)
+{
+  if (UpdateParameters(magnitude))
+  {
+    if (magnitude)
+      PlayEffect();
+    else
+      StopEffect();
+  }
+}
+
+std::string ForceFeedbackDevice::Force::GetName() const
 {
   return m_name;
 }
+
+ForceFeedbackDevice::Force::Force(ForceFeedbackDevice* parent, const char* name,
+                                  LPDIRECTINPUTEFFECT effect)
+    : m_effect(effect), m_parent(*parent), m_name(name), m_desired_magnitude()
+{
 }
+
+void ForceFeedbackDevice::Force::SetState(ControlState state)
+{
+  const auto new_val = int(DI_FFNOMINALMAX * state);
+
+  if (m_desired_magnitude.exchange(new_val) != new_val)
+    m_parent.SignalUpdateThread();
 }
+
+void ForceFeedbackDevice::Force::UpdateOutput()
+{
+  UpdateEffect(m_desired_magnitude);
+}
+
+void ForceFeedbackDevice::Force::Release()
+{
+  // This isn't in the destructor because it should happen before the device is released.
+  m_effect->Stop();
+  m_effect->Unload();
+  m_effect->Release();
+}
+
+}  // namespace ForceFeedback
+}  // namespace ciface

Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h

@@ -4,9 +4,12 @@
 
 #pragma once
 
-#include <list>
+#include <atomic>
 #include <string>
+#include <thread>
 
+#include "Common/Event.h"
+#include "Common/Flag.h"
 #include "InputCommon/ControllerInterface/Device.h"
 
 #ifdef _WIN32
@@ -22,30 +25,64 @@ namespace ForceFeedback
 {
 class ForceFeedbackDevice : public Core::Device
 {
+public:
+  bool InitForceFeedback(const LPDIRECTINPUTDEVICE8, int axis_count);
+  void DeInitForceFeedback();
+
 private:
-  template <typename P>
+  void ThreadFunc();
+
   class Force : public Output
   {
   public:
-    Force(const std::string& name, LPDIRECTINPUTEFFECT iface);
+    Force(ForceFeedbackDevice* parent, const char* name, LPDIRECTINPUTEFFECT effect);
-    ~Force();
+
+    void UpdateOutput();
+    void Release();
 
-    std::string GetName() const override;
     void SetState(ControlState state) override;
-    void Update();
+    std::string GetName() const override;
-    void Stop();
+
+  protected:
+    const LPDIRECTINPUTEFFECT m_effect;
 
   private:
-    const std::string m_name;
+    virtual void UpdateEffect(int magnitude) = 0;
-    LPDIRECTINPUTEFFECT m_iface;
-    P params;
-  };
-  typedef Force<DICONSTANTFORCE> ForceConstant;
-  typedef Force<DIRAMPFORCE> ForceRamp;
-  typedef Force<DIPERIODIC> ForcePeriodic;
 
-public:
+    ForceFeedbackDevice& m_parent;
-  bool InitForceFeedback(const LPDIRECTINPUTDEVICE8, int cAxes);
+    const char* const m_name;
+    std::atomic<int> m_desired_magnitude;
+  };
+
+  template <typename P>
+  class TypedForce : public Force
+  {
+  public:
+    TypedForce(ForceFeedbackDevice* parent, const char* name, LPDIRECTINPUTEFFECT effect,
+               const P& params);
+
+  private:
+    void UpdateEffect(int magnitude) override;
+
+    // Returns true if parameters changed.
+    bool UpdateParameters(int magnitude);
+
+    void PlayEffect();
+    void StopEffect();
+
+    P m_params = {};
+  };
+
+  void SignalUpdateThread();
+
+  typedef TypedForce<DICONSTANTFORCE> ForceConstant;
+  typedef TypedForce<DIRAMPFORCE> ForceRamp;
+  typedef TypedForce<DIPERIODIC> ForcePeriodic;
+
+  std::thread m_update_thread;
+  Common::Event m_update_event;
+  Common::Flag m_run_thread;
 };
-}
+
-}
+}  // namespace ForceFeedback
+}  // namespace ciface

Source/Core/InputCommon/ControllerInterface/OSX/OSXJoystick.mm

@@ -99,6 +99,8 @@ Joystick::Joystick(IOHIDDeviceRef device, std::string name)
 
 Joystick::~Joystick()
 {
+  DeInitForceFeedback();
+
   if (m_ff_device)
     m_ff_device->Release();
 }