ControllerInterface: Set DInput FF effect parameters sanely. This fixes a crash with periodic effects and my GCPad adapter (probably a divide by zero behind the scenes).

3 files changed, 63 insertions, 47 deletions

diff --git a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp b/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
index 6de1f97d7e..a01ae6c75a 100644
--- a/Source/Core/InputCommon/ControllerInterface/DInput/DInputJoystick.cpp
+++ b/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!");
-          // fall back to non-exclusive mode, with no rumble
+          WARN_LOG(
+              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,16 +139,21 @@ 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));
-  // set hats to center
-  memset(m_state_in.rgdwPOV, 0xFF, sizeof(m_state_in.rgdwPOV));
+  // Zero inputs:
+  m_state_in = {};
+  // Set hats to center:
+  std::fill(std::begin(m_state_in.rgdwPOV), std::end(m_state_in.rgdwPOV), 0xFF);
 }
 
 Joystick::~Joystick()
diff --git a/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.cpp b/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.cpp
index 3f8df41419..26112015af 100644
--- a/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.cpp
+++ b/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.cpp
@@ -4,7 +4,7 @@
 
 #include "InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h"
 
-#include <algorithm>
+#include <array>
 #include <string>
 
 #include "Common/Thread.h"
@@ -13,6 +13,14 @@ namespace ciface
 {
 namespace ForceFeedback
 {
+// 100Hz which homebrew docs very roughly imply is within WiiMote normal
+// range, used for periodic haptic effects though often ignored by devices
+constexpr int RUMBLE_PERIOD = DI_SECONDS / 100;
+// 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>;
@@ -74,41 +82,41 @@ void ForceFeedbackDevice::SignalUpdateThread()
   m_update_event.Set();
 }
 
-bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, int cAxes)
+bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, int axis_count)
 {
-  if (cAxes == 0)
+  if (axis_count == 0)
     return false;
 
-  // TODO: check for DIDC_FORCEFEEDBACK in devcaps?
-
-  // Temporary for creating the effect:
-  DWORD rgdwAxes[2] = {DIJOFS_X, DIJOFS_Y};
-  LONG rglDirection[2] = {-200, 0};
+  // 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};
 
   DIEFFECT eff{};
-  eff.dwSize = sizeof(DIEFFECT);
+  eff.dwSize = sizeof(eff);
   eff.dwFlags = DIEFF_CARTESIAN | DIEFF_OBJECTOFFSETS;
-  // Infinite seems to work just fine:
-  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.rgdwAxes = rgdwAxes;
-  eff.rglDirection = rglDirection;
+  eff.cAxes = DWORD(rgdwAxes.size());
+  eff.rgdwAxes = rgdwAxes.data();
+  eff.rglDirection = rglDirection.data();
+  eff.dwStartDelay = 0;
 
-  // Initialize parameters.
+  // Initialize parameters with zero force (their current state).
   DICONSTANTFORCE diCF{};
-  diCF.lMagnitude = DI_FFNOMINALMAX;
+  diCF.lMagnitude = 0;
   DIRAMPFORCE diRF{};
+  diRF.lStart = diRF.lEnd = 0;
   DIPERIODIC diPE{};
-
-  // This doesn't seem needed:
-  // DIENVELOPE env;
-  // eff.lpEnvelope = &env;
-  // ZeroMemory(&env, sizeof(env));
-  // env.dwSize = sizeof(env);
+  diPE.dwMagnitude = 0;
+  // Is it sensible to have a zero-offset?
+  diPE.lOffset = 0;
+  diPE.dwPhase = 0;
+  diPE.dwPeriod = RUMBLE_PERIOD;
 
   for (auto& f : force_type_names)
   {
@@ -133,11 +141,11 @@ bool ForceFeedbackDevice::InitForceFeedback(const LPDIRECTINPUTDEVICE8 device, i
     if (SUCCEEDED(device->CreateEffect(f.guid, &eff, &pEffect, nullptr)))
     {
       if (f.guid == GUID_ConstantForce)
-        AddOutput(new ForceConstant(this, f.name, pEffect));
+        AddOutput(new ForceConstant(this, f.name, pEffect, diCF));
       else if (f.guid == GUID_RampForce)
-        AddOutput(new ForceRamp(this, f.name, pEffect));
+        AddOutput(new ForceRamp(this, f.name, pEffect, diRF));
       else
-        AddOutput(new ForcePeriodic(this, f.name, pEffect));
+        AddOutput(new ForcePeriodic(this, f.name, pEffect, diPE));
     }
   }
 
@@ -163,12 +171,11 @@ template <typename P>
 void ForceFeedbackDevice::TypedForce<P>::PlayEffect()
 {
   DIEFFECT eff{};
-  eff.dwSize = sizeof(DIEFFECT);
-  eff.dwFlags = DIEFF_CARTESIAN | DIEFF_OBJECTOFFSETS;
-
+  eff.dwSize = sizeof(eff);
   eff.cbTypeSpecificParams = sizeof(m_params);
   eff.lpvTypeSpecificParams = &m_params;
-  m_effect->SetParameters(&eff, DIEP_TYPESPECIFICPARAMS | DIEP_START);
+
+  m_effect->SetParameters(&eff, DIEP_START | DIEP_TYPESPECIFICPARAMS);
 }
 
 template <typename P>
@@ -192,6 +199,8 @@ bool ForceFeedbackDevice::ForceRamp::UpdateParameters(int magnitude)
 {
   const auto old_magnitude = m_params.lStart;
 
+  // Having the same "start" and "end" here is a bit odd..
+  // But ramp forces don't really make sense for our rumble effects anyways..
   m_params.lStart = m_params.lEnd = magnitude;
 
   return old_magnitude != m_params.lStart;
@@ -203,16 +212,14 @@ bool ForceFeedbackDevice::ForcePeriodic::UpdateParameters(int magnitude)
   const auto old_magnitude = m_params.dwMagnitude;
 
   m_params.dwMagnitude = magnitude;
-  // Zero is working fine for me:
-  // params.dwPeriod = 0;//DWORD(0.05 * DI_SECONDS);
 
   return old_magnitude != m_params.dwMagnitude;
 }
 
 template <typename P>
-ForceFeedbackDevice::TypedForce<P>::TypedForce(ForceFeedbackDevice* parent, std::string name,
-                                               LPDIRECTINPUTEFFECT effect)
-    : Force(parent, std::move(name), effect), m_params{}
+ForceFeedbackDevice::TypedForce<P>::TypedForce(ForceFeedbackDevice* parent, const char* name,
+                                               LPDIRECTINPUTEFFECT effect, const P& params)
+    : Force(parent, name, effect), m_params(params)
 {
 }
 
@@ -233,9 +240,9 @@ std::string ForceFeedbackDevice::Force::GetName() const
   return m_name;
 }
 
-ForceFeedbackDevice::Force::Force(ForceFeedbackDevice* parent, const std::string name,
+ForceFeedbackDevice::Force::Force(ForceFeedbackDevice* parent, const char* name,
                                   LPDIRECTINPUTEFFECT effect)
-    : m_effect(effect), m_parent(*parent), m_name(std::move(name)), m_desired_magnitude()
+    : m_effect(effect), m_parent(*parent), m_name(name), m_desired_magnitude()
 {
 }
 
diff --git a/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h b/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h
index ca1ad6647a..8f5814106e 100644
--- a/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h
+++ b/Source/Core/InputCommon/ControllerInterface/ForceFeedback/ForceFeedbackDevice.h
@@ -26,7 +26,7 @@ namespace ForceFeedback
 class ForceFeedbackDevice : public Core::Device
 {
 public:
-  bool InitForceFeedback(const LPDIRECTINPUTDEVICE8, int cAxes);
+  bool InitForceFeedback(const LPDIRECTINPUTDEVICE8, int axis_count);
   void DeInitForceFeedback();
 
 private:
@@ -35,7 +35,7 @@ private:
   class Force : public Output
   {
   public:
-    Force(ForceFeedbackDevice* parent, const std::string name, LPDIRECTINPUTEFFECT effect);
+    Force(ForceFeedbackDevice* parent, const char* name, LPDIRECTINPUTEFFECT effect);
 
     void UpdateOutput();
     void Release();
@@ -50,7 +50,7 @@ private:
     virtual void UpdateEffect(int magnitude) = 0;
 
     ForceFeedbackDevice& m_parent;
-    const std::string m_name;
+    const char* const m_name;
     std::atomic<int> m_desired_magnitude;
   };
 
@@ -58,7 +58,8 @@ private:
   class TypedForce : public Force
   {
   public:
-    TypedForce(ForceFeedbackDevice* parent, const std::string name, LPDIRECTINPUTEFFECT effect);
+    TypedForce(ForceFeedbackDevice* parent, const char* name, LPDIRECTINPUTEFFECT effect,
+               const P& params);
 
   private:
     void UpdateEffect(int magnitude) override;