controllers/npad: Remove the old vibration filter

Previously we used a vibration filter that filters out amplitudes close to each other. It turns out there are cases where this results into vibrations that are too inaccurate. Remove this and move the 100Hz vibration filter (Only allowing a maximum of 100 vibrations per second) from sdl_impl to npad when enable_accurate_vibrations is set to false.

4 files changed, 64 insertions, 65 deletions

diff --git a/src/core/hle/service/hid/controllers/npad.cpp b/src/core/hle/service/hid/controllers/npad.cpp
index ecc33bc08..cfafabbd8 100644
--- a/src/core/hle/service/hid/controllers/npad.cpp
+++ b/src/core/hle/service/hid/controllers/npad.cpp
@@ -703,6 +703,23 @@ bool Controller_NPad::VibrateControllerAtIndex(std::size_t npad_index, std::size
         return false;
     }
 
+    if (!Settings::values.enable_accurate_vibrations.GetValue()) {
+        using std::chrono::duration_cast;
+        using std::chrono::milliseconds;
+        using std::chrono::steady_clock;
+
+        const auto now = steady_clock::now();
+
+        // Filter out non-zero vibrations that are within 10ms of each other.
+        if ((vibration_value.amp_low != 0.0f || vibration_value.amp_high != 0.0f) &&
+            duration_cast<milliseconds>(now - last_vibration_timepoints[npad_index][device_index]) <
+                milliseconds(10)) {
+            return false;
+        }
+
+        last_vibration_timepoints[npad_index][device_index] = now;
+    }
+
     return vibrations[npad_index][device_index]->SetRumblePlay(
         std::min(vibration_value.amp_low * player.vibration_strength / 100.0f, 1.0f),
         vibration_value.freq_low,
@@ -710,66 +727,59 @@ bool Controller_NPad::VibrateControllerAtIndex(std::size_t npad_index, std::size
         vibration_value.freq_high);
 }
 
-void Controller_NPad::VibrateControllers(const std::vector<DeviceHandle>& vibration_device_handles,
-                                         const std::vector<VibrationValue>& vibration_values) {
-    LOG_TRACE(Service_HID, "called");
-
+void Controller_NPad::VibrateController(const DeviceHandle& vibration_device_handle,
+                                        const VibrationValue& vibration_value) {
     if (!Settings::values.vibration_enabled.GetValue()) {
         return;
     }
 
-    ASSERT_MSG(vibration_device_handles.size() == vibration_values.size(),
-               "The amount of device handles does not match with the amount of vibration values,"
-               "this is undefined behavior!");
+    const auto npad_index = NPadIdToIndex(vibration_device_handle.npad_id);
+    const auto device_index = static_cast<std::size_t>(vibration_device_handle.device_index);
 
-    for (std::size_t i = 0; i < vibration_device_handles.size(); ++i) {
-        const auto npad_index = NPadIdToIndex(vibration_device_handles[i].npad_id);
-        const auto device_index =
-            static_cast<std::size_t>(vibration_device_handles[i].device_index);
+    if (!vibration_devices_mounted[npad_index][device_index] ||
+        !connected_controllers[npad_index].is_connected) {
+        return;
+    }
 
-        if (!vibration_devices_mounted[npad_index][device_index] ||
-            !connected_controllers[npad_index].is_connected) {
-            continue;
-        }
+    if (vibration_device_handle.device_index == DeviceIndex::None) {
+        UNREACHABLE_MSG("DeviceIndex should never be None!");
+        return;
+    }
 
-        if (vibration_device_handles[i].device_index == DeviceIndex::None) {
-            UNREACHABLE_MSG("DeviceIndex should never be None!");
-            continue;
-        }
+    // Some games try to send mismatched parameters in the device handle, block these.
+    if ((connected_controllers[npad_index].type == NPadControllerType::JoyLeft &&
+         (vibration_device_handle.npad_type == NpadType::JoyconRight ||
+          vibration_device_handle.device_index == DeviceIndex::Right)) ||
+        (connected_controllers[npad_index].type == NPadControllerType::JoyRight &&
+         (vibration_device_handle.npad_type == NpadType::JoyconLeft ||
+          vibration_device_handle.device_index == DeviceIndex::Left))) {
+        return;
+    }
 
-        // Some games try to send mismatched parameters in the device handle, block these.
-        if ((connected_controllers[npad_index].type == NPadControllerType::JoyLeft &&
-             (vibration_device_handles[i].npad_type == NpadType::JoyconRight ||
-              vibration_device_handles[i].device_index == DeviceIndex::Right)) ||
-            (connected_controllers[npad_index].type == NPadControllerType::JoyRight &&
-             (vibration_device_handles[i].npad_type == NpadType::JoyconLeft ||
-              vibration_device_handles[i].device_index == DeviceIndex::Left))) {
-            continue;
-        }
+    // Filter out vibrations with equivalent values to reduce unnecessary state changes.
+    if (vibration_value.amp_low == latest_vibration_values[npad_index][device_index].amp_low &&
+        vibration_value.amp_high == latest_vibration_values[npad_index][device_index].amp_high) {
+        return;
+    }
 
-        // Filter out vibrations with equivalent values to reduce unnecessary state changes.
-        if (vibration_values[i].amp_low ==
-                latest_vibration_values[npad_index][device_index].amp_low &&
-            vibration_values[i].amp_high ==
-                latest_vibration_values[npad_index][device_index].amp_high) {
-            continue;
-        }
+    if (VibrateControllerAtIndex(npad_index, device_index, vibration_value)) {
+        latest_vibration_values[npad_index][device_index] = vibration_value;
+    }
+}
 
-        // Filter out non-zero vibrations that are within 0.015625 absolute amplitude of each other.
-        if (!Settings::values.enable_accurate_vibrations.GetValue() &&
-            (vibration_values[i].amp_low != 0.0f || vibration_values[i].amp_high != 0.0f) &&
-            (latest_vibration_values[npad_index][device_index].amp_low != 0.0f ||
-             latest_vibration_values[npad_index][device_index].amp_high != 0.0f) &&
-            (abs(vibration_values[i].amp_low -
-                 latest_vibration_values[npad_index][device_index].amp_low) < 0.015625f &&
-             abs(vibration_values[i].amp_high -
-                 latest_vibration_values[npad_index][device_index].amp_high) < 0.015625f)) {
-            continue;
-        }
+void Controller_NPad::VibrateControllers(const std::vector<DeviceHandle>& vibration_device_handles,
+                                         const std::vector<VibrationValue>& vibration_values) {
+    if (!Settings::values.vibration_enabled.GetValue()) {
+        return;
+    }
 
-        if (VibrateControllerAtIndex(npad_index, device_index, vibration_values[i])) {
-            latest_vibration_values[npad_index][device_index] = vibration_values[i];
-        }
+    ASSERT_OR_EXECUTE_MSG(
+        vibration_device_handles.size() == vibration_values.size(), { return; },
+        "The amount of device handles does not match with the amount of vibration values,"
+        "this is undefined behavior!");
+
+    for (std::size_t i = 0; i < vibration_device_handles.size(); ++i) {
+        VibrateController(vibration_device_handles[i], vibration_values[i]);
     }
 }
 
diff --git a/src/core/hle/service/hid/controllers/npad.h b/src/core/hle/service/hid/controllers/npad.h
index 30e3cb02f..f5122124c 100644
--- a/src/core/hle/service/hid/controllers/npad.h
+++ b/src/core/hle/service/hid/controllers/npad.h
@@ -151,6 +151,9 @@ public:
     bool VibrateControllerAtIndex(std::size_t npad_index, std::size_t device_index,
                                   const VibrationValue& vibration_value = {});
 
+    void VibrateController(const DeviceHandle& vibration_device_handle,
+                           const VibrationValue& vibration_value);
+
     void VibrateControllers(const std::vector<DeviceHandle>& vibration_device_handles,
                             const std::vector<VibrationValue>& vibration_values);
 
@@ -421,6 +424,7 @@ private:
     NpadHandheldActivationMode handheld_activation_mode{NpadHandheldActivationMode::Dual};
     // Each controller should have their own styleset changed event
     std::array<Kernel::EventPair, 10> styleset_changed_events;
+    std::array<std::array<std::chrono::steady_clock::time_point, 2>, 10> last_vibration_timepoints;
     std::array<std::array<VibrationValue, 2>, 10> latest_vibration_values{};
     std::array<std::array<bool, 2>, 10> vibration_devices_mounted{};
     std::array<ControllerHolder, 10> connected_controllers{};
diff --git a/src/core/hle/service/hid/hid.cpp b/src/core/hle/service/hid/hid.cpp
index ecaa847b2..2e9682bed 100644
--- a/src/core/hle/service/hid/hid.cpp
+++ b/src/core/hle/service/hid/hid.cpp
@@ -1029,7 +1029,7 @@ void Hid::SendVibrationValue(Kernel::HLERequestContext& ctx) {
     const auto parameters{rp.PopRaw<Parameters>()};
 
     applet_resource->GetController<Controller_NPad>(HidController::NPad)
-        .VibrateControllers({parameters.vibration_device_handle}, {parameters.vibration_value});
+        .VibrateController(parameters.vibration_device_handle, parameters.vibration_value);
 
     LOG_DEBUG(Service_HID,
               "called, npad_type={}, npad_id={}, device_index={}, applet_resource_user_id={}",
diff --git a/src/input_common/sdl/sdl_impl.cpp b/src/input_common/sdl/sdl_impl.cpp
index a2a83cdc9..a9f7e5103 100644
--- a/src/input_common/sdl/sdl_impl.cpp
+++ b/src/input_common/sdl/sdl_impl.cpp
@@ -81,18 +81,6 @@ public:
     }
 
     bool RumblePlay(u16 amp_low, u16 amp_high) {
-        using std::chrono::duration_cast;
-        using std::chrono::milliseconds;
-        using std::chrono::steady_clock;
-
-        // Block non-zero vibrations less than 10ms apart from each other.
-        if ((amp_low != 0 || amp_high != 0) &&
-            duration_cast<milliseconds>(steady_clock::now() - last_vibration) < milliseconds(10)) {
-            return false;
-        }
-
-        last_vibration = steady_clock::now();
-
         if (sdl_controller) {
             return SDL_GameControllerRumble(sdl_controller.get(), amp_low, amp_high, 0) == 0;
         } else if (sdl_joystick) {
@@ -171,9 +159,6 @@ private:
     std::unique_ptr<SDL_GameController, decltype(&SDL_GameControllerClose)> sdl_controller;
     mutable std::mutex mutex;
 
-    // This is the timepoint of the last vibration and is used to ensure vibrations are 10ms apart.
-    std::chrono::steady_clock::time_point last_vibration;
-
     // Motion is initialized without PID values as motion input is not aviable for SDL2
     MotionInput motion{0.0f, 0.0f, 0.0f};
 };