From aa7b0d2556e9667afe48a5788b04629dd6767b62 Mon Sep 17 00:00:00 2001 From: Shingo INADA Date: Thu, 9 Jan 2025 06:18:37 +0900 Subject: [PATCH 1/3] Revert "input: fix potential race condition with analog ramp up/down" This reverts commit 6bf38e50854490f9797d18a6ee60e63e401e0c15. --- core/input/gamepad_device.cpp | 27 +++++++++++++++++---------- core/input/gamepad_device.h | 4 ---- core/oslib/oslib.cpp | 1 + 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/core/input/gamepad_device.cpp b/core/input/gamepad_device.cpp index b142bc9c8..b228b9269 100644 --- a/core/input/gamepad_device.cpp +++ b/core/input/gamepad_device.cpp @@ -449,18 +449,24 @@ bool GamepadDevice::find_mapping(int system /* = settings.platform.system */) return false; } -int GamepadDevice::GetGamepadCount() { - Lock _(_gamepads_mutex); - return _gamepads.size(); +int GamepadDevice::GetGamepadCount() +{ + _gamepads_mutex.lock(); + int count = _gamepads.size(); + _gamepads_mutex.unlock(); + return count; } std::shared_ptr GamepadDevice::GetGamepad(int index) { - Lock _(_gamepads_mutex); + _gamepads_mutex.lock(); + std::shared_ptr dev; if (index >= 0 && index < (int)_gamepads.size()) - return _gamepads[index]; + dev = _gamepads[index]; else - return nullptr; + dev = NULL; + _gamepads_mutex.unlock(); + return dev; } void GamepadDevice::save_mapping(int system /* = settings.platform.system */) @@ -553,19 +559,21 @@ void GamepadDevice::Register(const std::shared_ptr& gamepad) setbuf(record_input, NULL); } #endif - Lock _(_gamepads_mutex); + _gamepads_mutex.lock(); _gamepads.push_back(gamepad); + _gamepads_mutex.unlock(); MapleConfigMap::UpdateVibration = updateVibration; } void GamepadDevice::Unregister(const std::shared_ptr& gamepad) { - Lock _(_gamepads_mutex); + _gamepads_mutex.lock(); for (auto it = _gamepads.begin(); it != _gamepads.end(); it++) if (*it == gamepad) { _gamepads.erase(it); break; } + _gamepads_mutex.unlock(); } void GamepadDevice::SaveMaplePorts() @@ -607,7 +615,6 @@ s16 (&GamepadDevice::getTargetArray(DigAnalog axis))[4] void GamepadDevice::rampAnalog() { - Lock _(rampMutex); if (lastAnalogUpdate == 0) // also used as a flag that no analog ramping is needed on this device (yet) return; @@ -650,7 +657,7 @@ void GamepadDevice::rampAnalog() void GamepadDevice::RampAnalog() { - Lock _(_gamepads_mutex); + std::lock_guard _(_gamepads_mutex); for (auto& gamepad : _gamepads) gamepad->rampAnalog(); } diff --git a/core/input/gamepad_device.h b/core/input/gamepad_device.h index 4cf560f68..50d5bbe0b 100644 --- a/core/input/gamepad_device.h +++ b/core/input/gamepad_device.h @@ -168,7 +168,6 @@ class GamepadDevice { if (port < 0) return; - Lock _(rampMutex); DigAnalog axis = key == DcNegDir ? NegDir : PosDir; if (pressed) digitalToAnalogState[port] |= axis; @@ -197,12 +196,9 @@ class GamepadDevice u64 lastAnalogUpdate = 0; u32 rampAnalogState[4] {}; static constexpr float AnalogRamp = 32767.f / 100.f; // 100 ms ramp time - std::mutex rampMutex; static std::vector> _gamepads; static std::mutex _gamepads_mutex; - - using Lock = std::lock_guard; }; #ifdef TEST_AUTOMATION diff --git a/core/oslib/oslib.cpp b/core/oslib/oslib.cpp index 7c0a2d8c5..a80647e41 100644 --- a/core/oslib/oslib.cpp +++ b/core/oslib/oslib.cpp @@ -379,6 +379,7 @@ void os_UpdateInputState() { FC_PROFILE_SCOPE; + // FIXME threading (android) this will be called on the render thread, events are on the main app thread GamepadDevice::RampAnalog(); #if defined(USE_SDL) input_sdl_handle(); From a5f13e03d2d9b97aaacb63856c12465167de7ac7 Mon Sep 17 00:00:00 2001 From: Shingo INADA Date: Thu, 9 Jan 2025 06:19:06 +0900 Subject: [PATCH 2/3] Revert "input: implement ramp up/down for analog axes mapped to buttons" This reverts commit 792aa38d34fd033d527ac11c0b406f4cfb8e40f3. --- core/input/gamepad_device.cpp | 76 ----------------------------------- core/input/gamepad_device.h | 22 +++++----- core/oslib/oslib.cpp | 9 ++--- 3 files changed, 13 insertions(+), 94 deletions(-) diff --git a/core/input/gamepad_device.cpp b/core/input/gamepad_device.cpp index b228b9269..895ffa4e1 100644 --- a/core/input/gamepad_device.cpp +++ b/core/input/gamepad_device.cpp @@ -586,82 +586,6 @@ void GamepadDevice::SaveMaplePorts() } } -s16 (&GamepadDevice::getTargetArray(DigAnalog axis))[4] -{ - switch (axis) - { - case DIGANA_LEFT: - case DIGANA_RIGHT: - return joyx; - case DIGANA_UP:; - case DIGANA_DOWN: - return joyy; - case DIGANA2_LEFT: - case DIGANA2_RIGHT: - return joyrx; - case DIGANA2_UP: - case DIGANA2_DOWN: - return joyry; - case DIGANA3_LEFT: - case DIGANA3_RIGHT: - return joy3x; - case DIGANA3_UP: - case DIGANA3_DOWN: - return joy3y; - default: - die("unknown axis"); - } -} - -void GamepadDevice::rampAnalog() -{ - if (lastAnalogUpdate == 0) - // also used as a flag that no analog ramping is needed on this device (yet) - return; - - const u64 now = getTimeMs(); - const int delta = std::round(static_cast(now - lastAnalogUpdate) * AnalogRamp); - lastAnalogUpdate = now; - for (unsigned port = 0; port < std::size(digitalToAnalogState); port++) - { - for (int axis = 0; axis < 12; axis += 2) // 3 sticks with 2 axes each - { - DigAnalog negDir = static_cast(1 << axis); - if ((rampAnalogState[port] & negDir) == 0) - // axis not active - continue; - DigAnalog posDir = static_cast(1 << (axis + 1)); - const int socd = digitalToAnalogState[port] & (negDir | posDir); - s16& axisValue = getTargetArray(negDir)[port]; - if (socd != 0 && socd != (negDir | posDir)) - { - // One axis is pressed => ramp up - if (socd == posDir) - axisValue = std::min(32767, axisValue + delta); - else - axisValue = std::max(-32768, axisValue - delta); - } - else - { - // No axis is pressed (or both) => ramp down - if (axisValue > 0) - axisValue = std::max(0, axisValue - delta); - else if (axisValue < 0) - axisValue = std::min(0, axisValue + delta); - else - rampAnalogState[port] &= ~negDir; - } - } - } -} - -void GamepadDevice::RampAnalog() -{ - std::lock_guard _(_gamepads_mutex); - for (auto& gamepad : _gamepads) - gamepad->rampAnalog(); -} - #ifdef TEST_AUTOMATION #include "cfg/option.h" static bool replay_inited; diff --git a/core/input/gamepad_device.h b/core/input/gamepad_device.h index 50d5bbe0b..97f91a72d 100644 --- a/core/input/gamepad_device.h +++ b/core/input/gamepad_device.h @@ -20,7 +20,6 @@ #pragma once #include "types.h" #include "mapping.h" -#include "stdclass.h" #include #include @@ -97,7 +96,6 @@ class GamepadDevice static int GetGamepadCount(); static std::shared_ptr GetGamepad(int index); static void SaveMaplePorts(); - static void RampAnalog(); template static std::shared_ptr GetGamepad() @@ -173,13 +171,15 @@ class GamepadDevice digitalToAnalogState[port] |= axis; else digitalToAnalogState[port] &= ~axis; - rampAnalogState[port] |= NegDir; - if (lastAnalogUpdate == 0) - lastAnalogUpdate = getTimeMs(); - } + const u32 socd = digitalToAnalogState[port] & (NegDir | PosDir); + if (socd == 0 || socd == (NegDir | PosDir)) + joystick = 0; + else if (socd == NegDir) + joystick = -32768; + else + joystick = 32767; - s16 (&getTargetArray(DigAnalog axis))[4]; - void rampAnalog(); + } std::string _api_name; int _maple_port; @@ -192,11 +192,7 @@ class GamepadDevice std::map lastAxisValue[4]; bool perGameMapping = false; bool instanceMapping = false; - - u64 lastAnalogUpdate = 0; - u32 rampAnalogState[4] {}; - static constexpr float AnalogRamp = 32767.f / 100.f; // 100 ms ramp time - + static std::vector> _gamepads; static std::mutex _gamepads_mutex; }; diff --git a/core/oslib/oslib.cpp b/core/oslib/oslib.cpp index a80647e41..6f155108e 100644 --- a/core/oslib/oslib.cpp +++ b/core/oslib/oslib.cpp @@ -40,7 +40,6 @@ #include #endif #include "profiler/fc_profiler.h" -#include "input/gamepad_device.h" namespace hostfs { @@ -379,12 +378,12 @@ void os_UpdateInputState() { FC_PROFILE_SCOPE; - // FIXME threading (android) this will be called on the render thread, events are on the main app thread - GamepadDevice::RampAnalog(); #if defined(USE_SDL) input_sdl_handle(); -#elif defined(USE_EVDEV) - input_evdev_handle(); +#else + #if defined(USE_EVDEV) + input_evdev_handle(); + #endif #endif } From 90c4cccdded3ceff625ffb3e767baa5a44d95bd2 Mon Sep 17 00:00:00 2001 From: Shingo INADA Date: Thu, 9 Jan 2025 06:46:13 +0900 Subject: [PATCH 3/3] restore using Lock --- core/input/gamepad_device.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/input/gamepad_device.h b/core/input/gamepad_device.h index 97f91a72d..5f28e44e8 100644 --- a/core/input/gamepad_device.h +++ b/core/input/gamepad_device.h @@ -195,6 +195,8 @@ class GamepadDevice static std::vector> _gamepads; static std::mutex _gamepads_mutex; + + using Lock = std::lock_guard; }; #ifdef TEST_AUTOMATION