Merge pull request #7689 from jordan-woyak/sdl-improve

ControllerInterface: SDL cleanups/fixes

Source/Core/InputCommon/ControllerInterface/SDL/SDL.cpp

@@ -5,8 +5,6 @@
 #include "InputCommon/ControllerInterface/SDL/SDL.h"
 
 #include <algorithm>
-#include <map>
-#include <sstream>
 #include <thread>
 
 #include <SDL_events.h>
@@ -25,12 +23,6 @@ namespace ciface
 {
 namespace SDL
 {
-// 10ms = 100Hz which homebrew docs very roughly imply is within WiiMote normal
-// range, used for periodic haptic effects though often ignored by devices
-static const u16 RUMBLE_PERIOD = 10;
-static const u16 RUMBLE_LENGTH_MAX =
-    500;  // ms: enough to span multiple frames at low FPS, but still finite
-
 static std::string GetJoystickName(int index)
 {
 #if SDL_VERSION_ATLEAST(2, 0, 0)
@@ -42,7 +34,7 @@ static std::string GetJoystickName(int index)
 
 static void OpenAndAddDevice(int index)
 {
-  SDL_Joystick* dev = SDL_JoystickOpen(index);
+  SDL_Joystick* const dev = SDL_JoystickOpen(index);
   if (dev)
   {
     auto js = std::make_shared<Joystick>(dev, index);
@@ -64,7 +56,6 @@ static bool HandleEventAndContinue(const SDL_Event& e)
   if (e.type == SDL_JOYDEVICEADDED)
   {
     OpenAndAddDevice(e.jdevice.which);
-    g_controller_interface.InvokeDevicesChangedCallbacks();
   }
   else if (e.type == SDL_JOYDEVICEREMOVED)
   {
@@ -72,7 +63,6 @@ static bool HandleEventAndContinue(const SDL_Event& e)
       const Joystick* joystick = dynamic_cast<const Joystick*>(device);
       return joystick && SDL_JoystickInstanceID(joystick->GetSDLJoystick()) == e.jdevice.which;
     });
-    g_controller_interface.InvokeDevicesChangedCallbacks();
   }
   else if (e.type == s_populate_event_type)
   {
@@ -111,7 +101,7 @@ void Init()
         return;
       }
 
-      Uint32 custom_events_start = SDL_RegisterEvents(2);
+      const Uint32 custom_events_start = SDL_RegisterEvents(2);
       if (custom_events_start == static_cast<Uint32>(-1))
       {
         ERROR_LOG(SERIALINTERFACE, "SDL failed to register custom events");
@@ -229,34 +219,37 @@ Joystick::Joystick(SDL_Joystick* const joystick, const int sdl_index)
   }
 
 #ifdef USE_SDL_HAPTIC
-  // try to get supported ff effects
   m_haptic = SDL_HapticOpenFromJoystick(m_joystick);
-  if (m_haptic)
+  if (!m_haptic)
+    return;
+
+  const unsigned int supported_effects = SDL_HapticQuery(m_haptic);
+
+  // Disable autocenter:
+  if (supported_effects & SDL_HAPTIC_AUTOCENTER)
+    SDL_HapticSetAutocenter(m_haptic, 0);
+
+  // Constant
+  if (supported_effects & SDL_HAPTIC_CONSTANT)
+    AddOutput(new ConstantEffect(m_haptic));
+
+  // Ramp
+  if (supported_effects & SDL_HAPTIC_RAMP)
+    AddOutput(new RampEffect(m_haptic));
+
+  // Periodic
+  for (auto waveform :
+       {SDL_HAPTIC_SINE, SDL_HAPTIC_TRIANGLE, SDL_HAPTIC_SAWTOOTHUP, SDL_HAPTIC_SAWTOOTHDOWN})
   {
-    // SDL_HapticSetGain( m_haptic, 1000 );
+    if (supported_effects & waveform)
-    // SDL_HapticSetAutocenter( m_haptic, 0 );
+      AddOutput(new PeriodicEffect(m_haptic, waveform));
+  }
 
-    const unsigned int supported_effects = SDL_HapticQuery(m_haptic);
+  // LeftRight
-
+  if (supported_effects & SDL_HAPTIC_LEFTRIGHT)
-    // constant effect
+  {
-    if (supported_effects & SDL_HAPTIC_CONSTANT)
+    AddOutput(new LeftRightEffect(m_haptic, LeftRightEffect::Motor::Strong));
-      AddOutput(new ConstantEffect(m_haptic));
+    AddOutput(new LeftRightEffect(m_haptic, LeftRightEffect::Motor::Weak));
-
-    // ramp effect
-    if (supported_effects & SDL_HAPTIC_RAMP)
-      AddOutput(new RampEffect(m_haptic));
-
-    // sine effect
-    if (supported_effects & SDL_HAPTIC_SINE)
-      AddOutput(new SineEffect(m_haptic));
-
-    // triangle effect
-    if (supported_effects & SDL_HAPTIC_TRIANGLE)
-      AddOutput(new TriangleEffect(m_haptic));
-
-    // left-right effect
-    if (supported_effects & SDL_HAPTIC_LEFTRIGHT)
-      AddOutput(new LeftRightEffect(m_haptic));
   }
 #endif
 }
@@ -278,24 +271,82 @@ Joystick::~Joystick()
 }
 
 #ifdef USE_SDL_HAPTIC
-void Joystick::HapticEffect::Update()
+void Joystick::HapticEffect::UpdateEffect()
 {
-  if (m_id == -1 && m_effect.type > 0)
+  if (m_effect.type != DISABLED_EFFECT_TYPE)
   {
-    m_id = SDL_HapticNewEffect(m_haptic, &m_effect);
+    if (m_id < 0)
-    if (m_id > -1)
+    {
-      SDL_HapticRunEffect(m_haptic, m_id, 1);
+      // Upload and try to play the effect.
+      m_id = SDL_HapticNewEffect(m_haptic, &m_effect);
+
+      if (m_id >= 0)
+        SDL_HapticRunEffect(m_haptic, m_id, 1);
+    }
+    else
+    {
+      // Effect is already playing. Update parameters.
+      SDL_HapticUpdateEffect(m_haptic, m_id, &m_effect);
+    }
   }
-  else if (m_id > -1 && m_effect.type == 0)
+  else if (m_id >= 0)
   {
+    // Stop and remove the effect.
     SDL_HapticStopEffect(m_haptic, m_id);
     SDL_HapticDestroyEffect(m_haptic, m_id);
     m_id = -1;
   }
-  else if (m_id > -1)
+}
-  {
+
-    SDL_HapticUpdateEffect(m_haptic, m_id, &m_effect);
+Joystick::HapticEffect::HapticEffect(SDL_Haptic* haptic) : m_haptic(haptic)
-  }
+{
+  // FYI: type is set within UpdateParameters.
+  m_effect.type = DISABLED_EFFECT_TYPE;
+}
+
+Joystick::HapticEffect::~HapticEffect()
+{
+  m_effect.type = DISABLED_EFFECT_TYPE;
+  UpdateEffect();
+}
+
+void Joystick::HapticEffect::SetDirection(SDL_HapticDirection* dir)
+{
+  // Left direction (for wheels)
+  dir->type = SDL_HAPTIC_CARTESIAN;
+  dir->dir[0] = -1;
+}
+
+Joystick::ConstantEffect::ConstantEffect(SDL_Haptic* haptic) : HapticEffect(haptic)
+{
+  m_effect.constant = {};
+  SetDirection(&m_effect.constant.direction);
+  m_effect.constant.length = RUMBLE_LENGTH_MS;
+}
+
+Joystick::RampEffect::RampEffect(SDL_Haptic* haptic) : HapticEffect(haptic)
+{
+  m_effect.ramp = {};
+  SetDirection(&m_effect.ramp.direction);
+  m_effect.ramp.length = RUMBLE_LENGTH_MS;
+}
+
+Joystick::PeriodicEffect::PeriodicEffect(SDL_Haptic* haptic, u16 waveform)
+    : HapticEffect(haptic), m_waveform(waveform)
+{
+  m_effect.periodic = {};
+  SetDirection(&m_effect.periodic.direction);
+  m_effect.periodic.length = RUMBLE_LENGTH_MS;
+  m_effect.periodic.period = RUMBLE_PERIOD_MS;
+  m_effect.periodic.offset = 0;
+  m_effect.periodic.phase = 0;
+}
+
+Joystick::LeftRightEffect::LeftRightEffect(SDL_Haptic* haptic, Motor motor)
+    : HapticEffect(haptic), m_motor(motor)
+{
+  m_effect.leftright = {};
+  m_effect.leftright.length = RUMBLE_LENGTH_MS;
 }
 
 std::string Joystick::ConstantEffect::GetName() const
@@ -308,87 +359,91 @@ std::string Joystick::RampEffect::GetName() const
   return "Ramp";
 }
 
-std::string Joystick::SineEffect::GetName() const
+std::string Joystick::PeriodicEffect::GetName() const
 {
-  return "Sine";
+  switch (m_waveform)
-}
+  {
-
+  case SDL_HAPTIC_SINE:
-std::string Joystick::TriangleEffect::GetName() const
+    return "Sine";
-{
+  case SDL_HAPTIC_TRIANGLE:
-  return "Triangle";
+    return "Triangle";
+  case SDL_HAPTIC_SAWTOOTHUP:
+    return "Sawtooth Up";
+  case SDL_HAPTIC_SAWTOOTHDOWN:
+    return "Sawtooth Down";
+  default:
+    return "Unknown";
+  }
 }
 
 std::string Joystick::LeftRightEffect::GetName() const
 {
-  return "LeftRight";
+  return (Motor::Strong == m_motor) ? "Strong" : "Weak";
 }
 
 void Joystick::HapticEffect::SetState(ControlState state)
 {
-  memset(&m_effect, 0, sizeof(m_effect));
+  // Maximum force value for all SDL effects:
-  if (state)
+  constexpr s16 MAX_FORCE_VALUE = 0x7fff;
+
+  if (UpdateParameters(s16(state * MAX_FORCE_VALUE)))
   {
-    SetSDLHapticEffect(state);
+    UpdateEffect();
   }
-  else
-  {
-    // this module uses type==0 to indicate 'off'
-    m_effect.type = 0;
-  }
-  Update();
 }
 
-void Joystick::ConstantEffect::SetSDLHapticEffect(ControlState state)
+bool Joystick::ConstantEffect::UpdateParameters(s16 value)
 {
-  m_effect.type = SDL_HAPTIC_CONSTANT;
+  s16& level = m_effect.constant.level;
-  m_effect.constant.length = RUMBLE_LENGTH_MAX;
+  const s16 old_level = level;
-  m_effect.constant.level = (Sint16)(state * 0x7FFF);
+
+  level = value;
+
+  m_effect.type = level ? SDL_HAPTIC_CONSTANT : DISABLED_EFFECT_TYPE;
+  return level != old_level;
 }
 
-void Joystick::RampEffect::SetSDLHapticEffect(ControlState state)
+bool Joystick::RampEffect::UpdateParameters(s16 value)
 {
-  m_effect.type = SDL_HAPTIC_RAMP;
+  s16& level = m_effect.ramp.start;
-  m_effect.ramp.length = RUMBLE_LENGTH_MAX;
+  const s16 old_level = level;
-  m_effect.ramp.start = (Sint16)(state * 0x7FFF);
+
+  level = value;
+  // FYI: Setting end to same as start is odd,
+  // but so is using Ramp effects for rumble simulation.
+  m_effect.ramp.end = level;
+
+  m_effect.type = level ? SDL_HAPTIC_RAMP : DISABLED_EFFECT_TYPE;
+  return level != old_level;
 }
 
-void Joystick::SineEffect::SetSDLHapticEffect(ControlState state)
+bool Joystick::PeriodicEffect::UpdateParameters(s16 value)
 {
-  m_effect.type = SDL_HAPTIC_SINE;
+  s16& level = m_effect.periodic.magnitude;
-  m_effect.periodic.period = RUMBLE_PERIOD;
+  const s16 old_level = level;
-  m_effect.periodic.magnitude = (Sint16)(state * 0x7FFF);
+
-  m_effect.periodic.offset = 0;
+  level = value;
-  m_effect.periodic.phase = 18000;
+
-  m_effect.periodic.length = RUMBLE_LENGTH_MAX;
+  m_effect.type = level ? m_waveform : DISABLED_EFFECT_TYPE;
-  m_effect.periodic.delay = 0;
+  return level != old_level;
-  m_effect.periodic.attack_length = 0;
 }
 
-void Joystick::TriangleEffect::SetSDLHapticEffect(ControlState state)
+bool Joystick::LeftRightEffect::UpdateParameters(s16 value)
 {
-  m_effect.type = SDL_HAPTIC_TRIANGLE;
+  u16& level = (Motor::Strong == m_motor) ? m_effect.leftright.large_magnitude :
-  m_effect.periodic.period = RUMBLE_PERIOD;
+                                            m_effect.leftright.small_magnitude;
-  m_effect.periodic.magnitude = (Sint16)(state * 0x7FFF);
+  const u16 old_level = level;
-  m_effect.periodic.offset = 0;
-  m_effect.periodic.phase = 18000;
-  m_effect.periodic.length = RUMBLE_LENGTH_MAX;
-  m_effect.periodic.delay = 0;
-  m_effect.periodic.attack_length = 0;
-}
 
-void Joystick::LeftRightEffect::SetSDLHapticEffect(ControlState state)
+  level = value;
-{
+
-  m_effect.type = SDL_HAPTIC_LEFTRIGHT;
+  m_effect.type = level ? SDL_HAPTIC_LEFTRIGHT : DISABLED_EFFECT_TYPE;
-  m_effect.leftright.length = RUMBLE_LENGTH_MAX;
+  return level != old_level;
-  // max ranges tuned to 'feel' similar in magnitude to triangle/sine on xbox360 controller
-  m_effect.leftright.large_magnitude = (Uint16)(state * 0x4000);
-  m_effect.leftright.small_magnitude = (Uint16)(state * 0xFFFF);
 }
 #endif
 
 void Joystick::UpdateInput()
 {
-  // each joystick is doin this, o well
+  // TODO: Don't call this for every Joystick, only once per ControllerInterface::UpdateInput()
   SDL_JoystickUpdate();
 }
 
@@ -409,25 +464,17 @@ SDL_Joystick* Joystick::GetSDLJoystick() const
 
 std::string Joystick::Button::GetName() const
 {
-  std::ostringstream ss;
+  return "Button " + std::to_string(m_index);
-  ss << "Button " << (int)m_index;
-  return ss.str();
 }
 
 std::string Joystick::Axis::GetName() const
 {
-  std::ostringstream ss;
+  return "Axis " + std::to_string(m_index) + (m_range < 0 ? '-' : '+');
-  ss << "Axis " << (int)m_index << (m_range < 0 ? '-' : '+');
-  return ss.str();
 }
 
 std::string Joystick::Hat::GetName() const
 {
-  static char tmpstr[] = "Hat . .";
+  return "Hat " + std::to_string(m_index) + ' ' + "NESW"[m_direction];
-  // I don't think more than 10 hats are supported
-  tmpstr[4] = (char)('0' + m_index);
-  tmpstr[6] = "NESW"[m_direction];
-  return tmpstr;
 }
 
 ControlState Joystick::Button::GetState() const
@@ -437,12 +484,12 @@ ControlState Joystick::Button::GetState() const
 
 ControlState Joystick::Axis::GetState() const
 {
-  return std::max(0.0, ControlState(SDL_JoystickGetAxis(m_js, m_index)) / m_range);
+  return ControlState(SDL_JoystickGetAxis(m_js, m_index)) / m_range;
 }
 
 ControlState Joystick::Hat::GetState() const
 {
   return (SDL_JoystickGetHat(m_js, m_index) & (1 << m_direction)) > 0;
 }
-}
+}  // namespace SDL
-}
+}  // namespace ciface

Source/Core/InputCommon/ControllerInterface/SDL/SDL.h

@@ -71,73 +71,72 @@ private:
   class HapticEffect : public Output
   {
   public:
-    HapticEffect(SDL_Haptic* haptic) : m_haptic(haptic), m_id(-1) {}
+    HapticEffect(SDL_Haptic* haptic);
-    ~HapticEffect()
+    ~HapticEffect();
-    {
-      m_effect.type = 0;
-      Update();
-    }
 
   protected:
-    void Update();
+    virtual bool UpdateParameters(s16 value) = 0;
-    virtual void SetSDLHapticEffect(ControlState state) = 0;
+    static void SetDirection(SDL_HapticDirection* dir);
 
-    SDL_HapticEffect m_effect;
+    SDL_HapticEffect m_effect = {};
-    SDL_Haptic* m_haptic;
+
-    int m_id;
+    static constexpr u16 DISABLED_EFFECT_TYPE = 0;
 
   private:
     virtual void SetState(ControlState state) override final;
+    void UpdateEffect();
+    SDL_Haptic* const m_haptic;
+    int m_id = -1;
   };
 
   class ConstantEffect : public HapticEffect
   {
   public:
-    ConstantEffect(SDL_Haptic* haptic) : HapticEffect(haptic) {}
+    ConstantEffect(SDL_Haptic* haptic);
     std::string GetName() const override;
 
   private:
-    void SetSDLHapticEffect(ControlState state) override;
+    bool UpdateParameters(s16 value) override;
   };
 
   class RampEffect : public HapticEffect
   {
   public:
-    RampEffect(SDL_Haptic* haptic) : HapticEffect(haptic) {}
+    RampEffect(SDL_Haptic* haptic);
     std::string GetName() const override;
 
   private:
-    void SetSDLHapticEffect(ControlState state) override;
+    bool UpdateParameters(s16 value) override;
   };
 
-  class SineEffect : public HapticEffect
+  class PeriodicEffect : public HapticEffect
   {
   public:
-    SineEffect(SDL_Haptic* haptic) : HapticEffect(haptic) {}
+    PeriodicEffect(SDL_Haptic* haptic, u16 waveform);
     std::string GetName() const override;
 
   private:
-    void SetSDLHapticEffect(ControlState state) override;
+    bool UpdateParameters(s16 value) override;
-  };
 
-  class TriangleEffect : public HapticEffect
+    const u16 m_waveform;
-  {
-  public:
-    TriangleEffect(SDL_Haptic* haptic) : HapticEffect(haptic) {}
-    std::string GetName() const override;
-
-  private:
-    void SetSDLHapticEffect(ControlState state) override;
   };
 
   class LeftRightEffect : public HapticEffect
   {
   public:
-    LeftRightEffect(SDL_Haptic* haptic) : HapticEffect(haptic) {}
+    enum class Motor : u8
+    {
+      Weak,
+      Strong,
+    };
+
+    LeftRightEffect(SDL_Haptic* haptic, Motor motor);
     std::string GetName() const override;
 
   private:
-    void SetSDLHapticEffect(ControlState state) override;
+    bool UpdateParameters(s16 value) override;
+
+    const Motor m_motor;
   };
 #endif
 
@@ -159,5 +158,5 @@ private:
   SDL_Haptic* m_haptic;
 #endif
 };
-}
+}  // namespace SDL
-}
+}  // namespace ciface

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

@@ -256,8 +256,8 @@ evdevDevice::evdevDevice(const std::string& devnode) : m_devfile(devnode)
   // Rumble (i.e. Left/Right) (i.e. Strong/Weak) effect
   if (libevdev_has_event_code(m_dev, EV_FF, FF_RUMBLE))
   {
-    AddOutput(new RumbleEffect(m_fd, RumbleEffect::Motor::STRONG));
+    AddOutput(new RumbleEffect(m_fd, RumbleEffect::Motor::Strong));
-    AddOutput(new RumbleEffect(m_fd, RumbleEffect::Motor::WEAK));
+    AddOutput(new RumbleEffect(m_fd, RumbleEffect::Motor::Weak));
   }
 
   // TODO: Add leds as output devices
@@ -388,7 +388,7 @@ std::string evdevDevice::PeriodicEffect::GetName() const
 
 std::string evdevDevice::RumbleEffect::GetName() const
 {
-  return (Motor::STRONG == m_motor) ? "Strong" : "Weak";
+  return (Motor::Strong == m_motor) ? "Strong" : "Weak";
 }
 
 void evdevDevice::Effect::SetState(ControlState state)
@@ -482,7 +482,7 @@ bool evdevDevice::PeriodicEffect::UpdateParameters(ControlState state)
 
 bool evdevDevice::RumbleEffect::UpdateParameters(ControlState state)
 {
-  u16& value = (Motor::STRONG == m_motor) ? m_effect.u.rumble.strong_magnitude :
+  u16& value = (Motor::Strong == m_motor) ? m_effect.u.rumble.strong_magnitude :
                                             m_effect.u.rumble.weak_magnitude;
   const u16 old_value = value;
 

Source/Core/InputCommon/ControllerInterface/evdev/evdev.h

@@ -90,8 +90,8 @@ private:
   public:
     enum class Motor : u8
     {
-      WEAK,
+      Weak,
-      STRONG,
+      Strong,
     };
 
     RumbleEffect(int fd, Motor motor);