Make controller button code thread-safe (#405)

* Refactor spinlock to meet Lockable requirements
* Input: Refactor button code and make it thread-safe

src/input/api/Controller.cpp

@@ -15,10 +15,7 @@ const ControllerState& ControllerBase::update_state()
 	ControllerState result = raw_state();
 
 	// ignore default buttons
-	for (auto&& el : m_default_state.buttons)
-	{
-		result.buttons[el.first] = result.buttons[el.first] && !el.second;
-	}
+	result.buttons.UnsetButtons(m_default_state.buttons);
 	// apply deadzone and range and ignore default axis values
 	apply_axis_setting(result.axis, m_default_state.axis, m_settings.axis);
 	apply_axis_setting(result.rotation, m_default_state.rotation, m_settings.rotation);
@@ -26,22 +23,22 @@ const ControllerState& ControllerBase::update_state()
 
 #define APPLY_AXIS_BUTTON(_axis_, _flag_) \
 	if (result._axis_.x < -ControllerState::kAxisThreshold) \
-		result.buttons[(_flag_) + (kAxisXN - kAxisXP)]=true; \
+		result.buttons.SetButtonState((_flag_) + (kAxisXN - kAxisXP), true); \
 	else if (result._axis_.x > ControllerState::kAxisThreshold) \
-		result.buttons[(_flag_)]=true; \
+		result.buttons.SetButtonState((_flag_), true); \
 	if (result._axis_.y < -ControllerState::kAxisThreshold) \
-		result.buttons[(_flag_) + 1 + (kAxisXN - kAxisXP)]=true; \
+		result.buttons.SetButtonState((_flag_) + 1 + (kAxisXN - kAxisXP), true); \
 	else if (result._axis_.y > ControllerState::kAxisThreshold) \
-		result.buttons[(_flag_) + 1]=true;
+		result.buttons.SetButtonState((_flag_) + 1, true);
 
 	if (result.axis.x < -ControllerState::kAxisThreshold) 
-		result.buttons[(kAxisXP) + (kAxisXN - kAxisXP)]=true;
+		result.buttons.SetButtonState((kAxisXP) + (kAxisXN - kAxisXP), true);
 	else if (result.axis.x > ControllerState::kAxisThreshold) 
-		result.buttons[(kAxisXP)]=true;
+		result.buttons.SetButtonState((kAxisXP), true);
 	if (result.axis.y < -ControllerState::kAxisThreshold) 
-		result.buttons[(kAxisXP) + 1 + (kAxisXN - kAxisXP)]=true;
+		result.buttons.SetButtonState((kAxisXP) + 1 + (kAxisXN - kAxisXP), true);
 	else if (result.axis.y > ControllerState::kAxisThreshold) 
-		result.buttons[(kAxisXP) + 1]=true;
+		result.buttons.SetButtonState((kAxisXP) + 1, true);
 	APPLY_AXIS_BUTTON(rotation, kRotationXP);
 	APPLY_AXIS_BUTTON(trigger, kTriggerXP);
 
@@ -129,8 +126,7 @@ bool ControllerBase::operator==(const ControllerBase& c) const
 
 float ControllerBase::get_axis_value(uint64 button) const
 {
-	auto buttonState=m_last_state.buttons.find(button);
-	if (buttonState!=m_last_state.buttons.end() && buttonState->second)
+	if (m_last_state.buttons.GetButtonState(button))
 	{
 		if (button <= kButtonNoneAxisMAX || !has_axis())
 			return 1.0f;

src/input/api/ControllerState.h

@@ -1,6 +1,115 @@
 #pragma once
 
 #include <glm/vec2.hpp>
+#include "util/helpers/fspinlock.h"
+
+// helper class for storing and managing button press states in a thread-safe manner
+struct ControllerButtonState
+{
+	ControllerButtonState() = default;
+	ControllerButtonState(const ControllerButtonState& other)
+	{
+		this->m_pressedButtons = other.m_pressedButtons;
+	}
+
+	ControllerButtonState(ControllerButtonState&& other)
+	{
+		this->m_pressedButtons = std::move(other.m_pressedButtons);
+	}
+
+	void SetButtonState(uint32 buttonId, bool isPressed)
+	{
+		std::lock_guard _l(this->m_spinlock);
+		if (isPressed)
+		{
+			if (std::find(m_pressedButtons.cbegin(), m_pressedButtons.cend(), buttonId) != m_pressedButtons.end())
+				return;
+			m_pressedButtons.emplace_back(buttonId);
+		}
+		else
+		{
+			std::erase(m_pressedButtons, buttonId);
+		}
+	}
+
+	// set multiple buttons at once within a single lock interval
+	void SetPressedButtons(std::span<uint32> buttonList)
+	{
+		std::lock_guard _l(this->m_spinlock);
+		for (auto& buttonId : buttonList)
+		{
+			if (std::find(m_pressedButtons.cbegin(), m_pressedButtons.cend(), buttonId) == m_pressedButtons.end())
+				m_pressedButtons.emplace_back(buttonId);
+		}
+	}
+
+	// returns true if pressed
+	bool GetButtonState(uint32 buttonId) const
+	{
+		std::lock_guard _l(this->m_spinlock);
+		bool r = std::find(m_pressedButtons.cbegin(), m_pressedButtons.cend(), buttonId) != m_pressedButtons.cend();
+		return r;
+	}
+
+	// remove pressed state for all pressed buttons in buttonsToUnset
+	void UnsetButtons(const ControllerButtonState& buttonsToUnset)
+	{
+		std::scoped_lock _l(this->m_spinlock, buttonsToUnset.m_spinlock);
+		for (auto it = m_pressedButtons.begin(); it != m_pressedButtons.end();)
+		{
+			if (std::find(buttonsToUnset.m_pressedButtons.cbegin(), buttonsToUnset.m_pressedButtons.cend(), *it) == buttonsToUnset.m_pressedButtons.cend())
+			{
+				++it;
+				continue;
+			}
+			it = m_pressedButtons.erase(it);
+		}
+	}
+
+	// returns true if no buttons are pressed
+	bool IsIdle() const
+	{
+		std::lock_guard _l(this->m_spinlock);
+		const bool r = m_pressedButtons.empty();
+		return r;
+	}
+
+	std::vector<uint32> GetButtonList() const
+	{
+		std::lock_guard _l(this->m_spinlock);
+		std::vector<uint32> copy = m_pressedButtons;
+		return copy;
+	}
+
+	bool operator==(const ControllerButtonState& other) const
+	{
+		std::scoped_lock _l(this->m_spinlock, other.m_spinlock);
+		auto& otherButtons = other.m_pressedButtons;
+		if (m_pressedButtons.size() != otherButtons.size())
+		{
+			return false;
+		}
+		for (auto& buttonId : m_pressedButtons)
+		{
+			if (std::find(otherButtons.cbegin(), otherButtons.cend(), buttonId) == otherButtons.cend())
+			{
+				return false;
+			}
+		}
+		return true;
+	}
+
+	ControllerButtonState& operator=(ControllerButtonState&& other)
+	{
+		cemu_assert_debug(!other.m_spinlock.is_locked());
+		this->m_pressedButtons = std::move(other.m_pressedButtons);
+		return *this;
+	}
+
+private:
+	std::vector<uint32> m_pressedButtons; // since only very few buttons are pressed at a time, using a vector with linear scan is more efficient than a set/map
+	mutable FSpinlock m_spinlock;
+};
 
 struct ControllerState
 {
@@ -17,7 +126,7 @@ struct ControllerState
 	glm::vec2 rotation{ };
 	glm::vec2 trigger{ };
 
-	std::unordered_map<uint32, bool> buttons{};
+	ControllerButtonState buttons{};
 
 	uint64 last_state = 0;
 

src/input/api/DSU/DSUController.cpp

@@ -137,7 +137,7 @@ ControllerState DSUController::raw_state()
 	{
 		if (HAS_BIT(state.data.state1, i))
 		{
-			result.buttons[bitindex]=true;
+			result.buttons.SetButtonState(bitindex, true);
 		}
 	}
 
@@ -145,12 +145,12 @@ ControllerState DSUController::raw_state()
 	{
 		if (HAS_BIT(state.data.state2, i))
 		{
-			result.buttons[bitindex]=true;
+			result.buttons.SetButtonState(bitindex, true);
 		}
 	}
 
 	if (state.data.touch)
-		result.buttons[kButton16]=true;
+		result.buttons.SetButtonState(kButton16, true);
 
 	result.axis.x = (float)state.data.lx / std::numeric_limits<uint8>::max();
 	result.axis.x = (result.axis.x * 2.0f) - 1.0f;

src/input/api/DirectInput/DirectInputController.cpp

@@ -245,7 +245,6 @@ ControllerState DirectInputController::raw_state()
 	ControllerState result{};
 	if (!is_connected())
 		return result;
-
 	HRESULT hr = m_device->Poll();
 	if (FAILED(hr))
 	{
@@ -277,9 +276,7 @@ ControllerState DirectInputController::raw_state()
 	for (size_t i = 0; i < std::size(state.rgbButtons); ++i)
 	{
 		if (HAS_BIT(state.rgbButtons[i], 7))
-		{
-			result.buttons[i]=true;
-		}
+			result.buttons.SetButtonState(i, true);
 	}
 	
 	// axis
@@ -316,19 +313,19 @@ ControllerState DirectInputController::raw_state()
 	{
 		switch (pov)
 		{
-		case 0: result.buttons[kButtonUp]=true;
+		case 0: result.buttons.SetButtonState(kButtonUp, true);
 			break;
-		case 4500: result.buttons[kButtonUp]=true; // up + right
-		case 9000: result.buttons[kButtonRight]=true;
+		case 4500: result.buttons.SetButtonState(kButtonUp, true); // up + right
+		case 9000: result.buttons.SetButtonState(kButtonRight, true);
 			break;
-		case 13500: result.buttons[kButtonRight] = true; // right + down
-		case 18000: result.buttons[kButtonDown] = true;
+		case 13500: result.buttons.SetButtonState(kButtonRight, true); // right + down
+		case 18000: result.buttons.SetButtonState(kButtonDown, true);
 			break;
-		case 22500: result.buttons[kButtonDown] = true; // down + left
-		case 27000: result.buttons[kButtonLeft] = true;
+		case 22500: result.buttons.SetButtonState(kButtonDown, true); // down + left
+		case 27000: result.buttons.SetButtonState(kButtonLeft, true);
 			break;
-		case 31500: result.buttons[kButtonLeft] = true; // left + up
-		result.buttons[kButtonUp] = true; // left + up
+		case 31500: result.buttons.SetButtonState(kButtonLeft, true); // left + up
+					result.buttons.SetButtonState(kButtonUp, true); // left + up
 			break;
 		}
 	}

src/input/api/Keyboard/KeyboardController.cpp

@@ -1,5 +1,6 @@
-#include "input/api/Keyboard/KeyboardController.h"
+#include <boost/container/small_vector.hpp>
 
+#include "input/api/Keyboard/KeyboardController.h"
 #include "gui/guiWrapper.h"
 
 KeyboardController::KeyboardController()
@@ -51,7 +52,9 @@ ControllerState KeyboardController::raw_state()
 	ControllerState result{};
 	if (g_window_info.app_active)
 	{
-		g_window_info.get_keystates(result.buttons);
+		boost::container::small_vector<uint32, 16> pressedKeys;
+		g_window_info.iter_keystates([&pressedKeys](const std::pair<const uint32, bool>& keyState) { if (keyState.second) pressedKeys.emplace_back(keyState.first); });
+		result.buttons.SetPressedButtons(pressedKeys);
 	}
 	return result;
 }

src/input/api/SDL/SDLController.cpp

@@ -146,9 +146,7 @@ ControllerState SDLController::raw_state()
 	for (int i = 0; i < SDL_CONTROLLER_BUTTON_MAX; ++i)
 	{
 		if (m_buttons[i] && SDL_GameControllerGetButton(m_controller, (SDL_GameControllerButton)i))
-		{
-			result.buttons[i]=true;
-		}
+			result.buttons.SetButtonState(i, true);
 	}
 
 	if (m_axis[SDL_CONTROLLER_AXIS_LEFTX])

src/input/api/Wiimote/NativeWiimoteController.cpp

@@ -207,16 +207,16 @@ ControllerState NativeWiimoteController::raw_state()
 
 	const auto state = m_provider->get_state(m_index);
 	for (int i = 0; i < std::numeric_limits<uint16>::digits; i++)
-		result.buttons[i] = state.buttons & (1<<i);
+		result.buttons.SetButtonState(i, (state.buttons & (1 << i)) != 0);
 
 	if (std::holds_alternative<NunchuckData>(state.m_extension))
 	{
 		const auto nunchuck = std::get<NunchuckData>(state.m_extension);
 		if (nunchuck.c)
-			result.buttons[kWiimoteButton_C]=true;
+			result.buttons.SetButtonState(kWiimoteButton_C, true);
 
 		if (nunchuck.z)
-			result.buttons[kWiimoteButton_Z]=true;
+			result.buttons.SetButtonState(kWiimoteButton_Z, true);
 
 		result.axis = nunchuck.axis;
 	}
@@ -225,8 +225,11 @@ ControllerState NativeWiimoteController::raw_state()
 		const auto classic = std::get<ClassicData>(state.m_extension);
 		uint64 buttons = (uint64)classic.buttons << kHighestWiimote;
 		for (int i = 0; i < std::numeric_limits<uint64>::digits; i++)
-			result.buttons[i] = result.buttons[i] || (buttons & (1 << i));
-
+		{
+			// OR with base buttons
+			if((buttons & (1 << i)))
+				result.buttons.SetButtonState(i, true);
+		}
 		result.axis = classic.left_axis;
 		result.rotation = classic.right_axis;
 		result.trigger = classic.trigger;

src/input/api/XInput/XInputController.cpp

@@ -121,7 +121,7 @@ ControllerState XInputController::raw_state()
 
 	// Buttons
 	for(int i=0;i<std::numeric_limits<WORD>::digits;i++)
-		result.buttons[i] = state.Gamepad.wButtons & (1<<i);
+		result.buttons.SetButtonState(i, (state.Gamepad.wButtons & (1 << i)) != 0);
 
 	if (state.Gamepad.sThumbLX > 0)
 		result.axis.x = (float)state.Gamepad.sThumbLX / std::numeric_limits<sint16>::max();

src/input/emulated/EmulatedController.cpp

@@ -279,13 +279,9 @@ bool EmulatedController::is_mapping_down(uint64 mapping) const
 	const auto it = m_mappings.find(mapping);
 	if (it != m_mappings.cend())
 	{
-		if (const auto controller = it->second.controller.lock()) {
-			auto& buttons=controller->get_state().buttons;
-			auto buttonState=buttons.find(it->second.button);
-			return buttonState!=buttons.end() && buttonState->second;
-		}
+		if (const auto controller = it->second.controller.lock())
+			return controller->get_state().buttons.GetButtonState(it->second.button);
 	}
-
 	return false;
 }