From fa5277ecdb363f22f31ad4d0b981e12dde4eed0e Mon Sep 17 00:00:00 2001 From: german77 Date: Sat, 2 Apr 2022 00:02:31 -0600 Subject: [PATCH 1/3] core: hid: Reduce the amount of dataraces --- src/core/hid/emulated_console.cpp | 17 +- src/core/hid/emulated_console.h | 1 + src/core/hid/emulated_controller.cpp | 389 +++++++++++++++------------ src/core/hid/emulated_controller.h | 3 +- src/core/hid/emulated_devices.cpp | 33 ++- src/core/hid/emulated_devices.h | 1 + 6 files changed, 257 insertions(+), 187 deletions(-) diff --git a/src/core/hid/emulated_console.cpp b/src/core/hid/emulated_console.cpp index eef0ff493..639f61809 100644 --- a/src/core/hid/emulated_console.cpp +++ b/src/core/hid/emulated_console.cpp @@ -132,7 +132,7 @@ void EmulatedConsole::SetMotionParam(Common::ParamPackage param) { } void EmulatedConsole::SetMotion(const Common::Input::CallbackStatus& callback) { - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; auto& raw_status = console.motion_values.raw_status; auto& emulated = console.motion_values.emulated; @@ -151,6 +151,7 @@ void EmulatedConsole::SetMotion(const Common::Input::CallbackStatus& callback) { emulated.UpdateOrientation(raw_status.delta_timestamp); if (is_configuring) { + lock.unlock(); TriggerOnChange(ConsoleTriggerType::Motion); return; } @@ -166,6 +167,7 @@ void EmulatedConsole::SetMotion(const Common::Input::CallbackStatus& callback) { // Find what is this value motion.verticalization_error = 0.0f; + lock.unlock(); TriggerOnChange(ConsoleTriggerType::Motion); } @@ -173,11 +175,12 @@ void EmulatedConsole::SetTouch(const Common::Input::CallbackStatus& callback, st if (index >= console.touch_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; console.touch_values[index] = TransformToTouch(callback); if (is_configuring) { + lock.unlock(); TriggerOnChange(ConsoleTriggerType::Touch); return; } @@ -189,26 +192,32 @@ void EmulatedConsole::SetTouch(const Common::Input::CallbackStatus& callback, st .pressed = console.touch_values[index].pressed.value, }; + lock.unlock(); TriggerOnChange(ConsoleTriggerType::Touch); } ConsoleMotionValues EmulatedConsole::GetMotionValues() const { + std::lock_guard lock{mutex}; return console.motion_values; } TouchValues EmulatedConsole::GetTouchValues() const { + std::lock_guard lock{mutex}; return console.touch_values; } ConsoleMotion EmulatedConsole::GetMotion() const { + std::lock_guard lock{mutex}; return console.motion_state; } TouchFingerState EmulatedConsole::GetTouch() const { + std::lock_guard lock{mutex}; return console.touch_state; } void EmulatedConsole::TriggerOnChange(ConsoleTriggerType type) { + std::lock_guard lock{callback_mutex}; for (const auto& poller_pair : callback_list) { const ConsoleUpdateCallback& poller = poller_pair.second; if (poller.on_change) { @@ -218,13 +227,13 @@ void EmulatedConsole::TriggerOnChange(ConsoleTriggerType type) { } int EmulatedConsole::SetCallback(ConsoleUpdateCallback update_callback) { - std::lock_guard lock{mutex}; + std::lock_guard lock{callback_mutex}; callback_list.insert_or_assign(last_callback_key, update_callback); return last_callback_key++; } void EmulatedConsole::DeleteCallback(int key) { - std::lock_guard lock{mutex}; + std::lock_guard lock{callback_mutex}; const auto& iterator = callback_list.find(key); if (iterator == callback_list.end()) { LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); diff --git a/src/core/hid/emulated_console.h b/src/core/hid/emulated_console.h index 5eb170823..53677bdc5 100644 --- a/src/core/hid/emulated_console.h +++ b/src/core/hid/emulated_console.h @@ -183,6 +183,7 @@ private: TouchDevices touch_devices; mutable std::mutex mutex; + mutable std::mutex callback_mutex; std::unordered_map callback_list; int last_callback_key = 0; diff --git a/src/core/hid/emulated_controller.cpp b/src/core/hid/emulated_controller.cpp index 7e05666d6..4f3676ec3 100644 --- a/src/core/hid/emulated_controller.cpp +++ b/src/core/hid/emulated_controller.cpp @@ -353,14 +353,17 @@ void EmulatedController::DisableConfiguration() { } void EmulatedController::EnableSystemButtons() { + std::lock_guard lock{mutex}; system_buttons_enabled = true; } void EmulatedController::DisableSystemButtons() { + std::lock_guard lock{mutex}; system_buttons_enabled = false; } void EmulatedController::ResetSystemButtons() { + std::lock_guard lock{mutex}; controller.home_button_state.home.Assign(false); controller.capture_button_state.capture.Assign(false); } @@ -494,139 +497,141 @@ void EmulatedController::SetButton(const Common::Input::CallbackStatus& callback if (index >= controller.button_values.size()) { return; } - { - std::lock_guard lock{mutex}; - bool value_changed = false; - const auto new_status = TransformToButton(callback); - auto& current_status = controller.button_values[index]; + std::unique_lock lock{mutex}; + bool value_changed = false; + const auto new_status = TransformToButton(callback); + auto& current_status = controller.button_values[index]; - // Only read button values that have the same uuid or are pressed once - if (current_status.uuid != uuid) { - if (!new_status.value) { - return; - } - } - - current_status.toggle = new_status.toggle; - current_status.uuid = uuid; - - // Update button status with current - if (!current_status.toggle) { - current_status.locked = false; - if (current_status.value != new_status.value) { - current_status.value = new_status.value; - value_changed = true; - } - } else { - // Toggle button and lock status - if (new_status.value && !current_status.locked) { - current_status.locked = true; - current_status.value = !current_status.value; - value_changed = true; - } - - // Unlock button ready for next press - if (!new_status.value && current_status.locked) { - current_status.locked = false; - } - } - - if (!value_changed) { + // Only read button values that have the same uuid or are pressed once + if (current_status.uuid != uuid) { + if (!new_status.value) { return; } - - if (is_configuring) { - controller.npad_button_state.raw = NpadButton::None; - controller.debug_pad_button_state.raw = 0; - TriggerOnChange(ControllerTriggerType::Button, false); - return; - } - - switch (index) { - case Settings::NativeButton::A: - controller.npad_button_state.a.Assign(current_status.value); - controller.debug_pad_button_state.a.Assign(current_status.value); - break; - case Settings::NativeButton::B: - controller.npad_button_state.b.Assign(current_status.value); - controller.debug_pad_button_state.b.Assign(current_status.value); - break; - case Settings::NativeButton::X: - controller.npad_button_state.x.Assign(current_status.value); - controller.debug_pad_button_state.x.Assign(current_status.value); - break; - case Settings::NativeButton::Y: - controller.npad_button_state.y.Assign(current_status.value); - controller.debug_pad_button_state.y.Assign(current_status.value); - break; - case Settings::NativeButton::LStick: - controller.npad_button_state.stick_l.Assign(current_status.value); - break; - case Settings::NativeButton::RStick: - controller.npad_button_state.stick_r.Assign(current_status.value); - break; - case Settings::NativeButton::L: - controller.npad_button_state.l.Assign(current_status.value); - controller.debug_pad_button_state.l.Assign(current_status.value); - break; - case Settings::NativeButton::R: - controller.npad_button_state.r.Assign(current_status.value); - controller.debug_pad_button_state.r.Assign(current_status.value); - break; - case Settings::NativeButton::ZL: - controller.npad_button_state.zl.Assign(current_status.value); - controller.debug_pad_button_state.zl.Assign(current_status.value); - break; - case Settings::NativeButton::ZR: - controller.npad_button_state.zr.Assign(current_status.value); - controller.debug_pad_button_state.zr.Assign(current_status.value); - break; - case Settings::NativeButton::Plus: - controller.npad_button_state.plus.Assign(current_status.value); - controller.debug_pad_button_state.plus.Assign(current_status.value); - break; - case Settings::NativeButton::Minus: - controller.npad_button_state.minus.Assign(current_status.value); - controller.debug_pad_button_state.minus.Assign(current_status.value); - break; - case Settings::NativeButton::DLeft: - controller.npad_button_state.left.Assign(current_status.value); - controller.debug_pad_button_state.d_left.Assign(current_status.value); - break; - case Settings::NativeButton::DUp: - controller.npad_button_state.up.Assign(current_status.value); - controller.debug_pad_button_state.d_up.Assign(current_status.value); - break; - case Settings::NativeButton::DRight: - controller.npad_button_state.right.Assign(current_status.value); - controller.debug_pad_button_state.d_right.Assign(current_status.value); - break; - case Settings::NativeButton::DDown: - controller.npad_button_state.down.Assign(current_status.value); - controller.debug_pad_button_state.d_down.Assign(current_status.value); - break; - case Settings::NativeButton::SL: - controller.npad_button_state.left_sl.Assign(current_status.value); - controller.npad_button_state.right_sl.Assign(current_status.value); - break; - case Settings::NativeButton::SR: - controller.npad_button_state.left_sr.Assign(current_status.value); - controller.npad_button_state.right_sr.Assign(current_status.value); - break; - case Settings::NativeButton::Home: - if (!system_buttons_enabled) { - break; - } - controller.home_button_state.home.Assign(current_status.value); - break; - case Settings::NativeButton::Screenshot: - if (!system_buttons_enabled) { - break; - } - controller.capture_button_state.capture.Assign(current_status.value); - break; - } } + + current_status.toggle = new_status.toggle; + current_status.uuid = uuid; + + // Update button status with current + if (!current_status.toggle) { + current_status.locked = false; + if (current_status.value != new_status.value) { + current_status.value = new_status.value; + value_changed = true; + } + } else { + // Toggle button and lock status + if (new_status.value && !current_status.locked) { + current_status.locked = true; + current_status.value = !current_status.value; + value_changed = true; + } + + // Unlock button ready for next press + if (!new_status.value && current_status.locked) { + current_status.locked = false; + } + } + + if (!value_changed) { + return; + } + + if (is_configuring) { + controller.npad_button_state.raw = NpadButton::None; + controller.debug_pad_button_state.raw = 0; + lock.unlock(); + TriggerOnChange(ControllerTriggerType::Button, false); + return; + } + + switch (index) { + case Settings::NativeButton::A: + controller.npad_button_state.a.Assign(current_status.value); + controller.debug_pad_button_state.a.Assign(current_status.value); + break; + case Settings::NativeButton::B: + controller.npad_button_state.b.Assign(current_status.value); + controller.debug_pad_button_state.b.Assign(current_status.value); + break; + case Settings::NativeButton::X: + controller.npad_button_state.x.Assign(current_status.value); + controller.debug_pad_button_state.x.Assign(current_status.value); + break; + case Settings::NativeButton::Y: + controller.npad_button_state.y.Assign(current_status.value); + controller.debug_pad_button_state.y.Assign(current_status.value); + break; + case Settings::NativeButton::LStick: + controller.npad_button_state.stick_l.Assign(current_status.value); + break; + case Settings::NativeButton::RStick: + controller.npad_button_state.stick_r.Assign(current_status.value); + break; + case Settings::NativeButton::L: + controller.npad_button_state.l.Assign(current_status.value); + controller.debug_pad_button_state.l.Assign(current_status.value); + break; + case Settings::NativeButton::R: + controller.npad_button_state.r.Assign(current_status.value); + controller.debug_pad_button_state.r.Assign(current_status.value); + break; + case Settings::NativeButton::ZL: + controller.npad_button_state.zl.Assign(current_status.value); + controller.debug_pad_button_state.zl.Assign(current_status.value); + break; + case Settings::NativeButton::ZR: + controller.npad_button_state.zr.Assign(current_status.value); + controller.debug_pad_button_state.zr.Assign(current_status.value); + break; + case Settings::NativeButton::Plus: + controller.npad_button_state.plus.Assign(current_status.value); + controller.debug_pad_button_state.plus.Assign(current_status.value); + break; + case Settings::NativeButton::Minus: + controller.npad_button_state.minus.Assign(current_status.value); + controller.debug_pad_button_state.minus.Assign(current_status.value); + break; + case Settings::NativeButton::DLeft: + controller.npad_button_state.left.Assign(current_status.value); + controller.debug_pad_button_state.d_left.Assign(current_status.value); + break; + case Settings::NativeButton::DUp: + controller.npad_button_state.up.Assign(current_status.value); + controller.debug_pad_button_state.d_up.Assign(current_status.value); + break; + case Settings::NativeButton::DRight: + controller.npad_button_state.right.Assign(current_status.value); + controller.debug_pad_button_state.d_right.Assign(current_status.value); + break; + case Settings::NativeButton::DDown: + controller.npad_button_state.down.Assign(current_status.value); + controller.debug_pad_button_state.d_down.Assign(current_status.value); + break; + case Settings::NativeButton::SL: + controller.npad_button_state.left_sl.Assign(current_status.value); + controller.npad_button_state.right_sl.Assign(current_status.value); + break; + case Settings::NativeButton::SR: + controller.npad_button_state.left_sr.Assign(current_status.value); + controller.npad_button_state.right_sr.Assign(current_status.value); + break; + case Settings::NativeButton::Home: + if (!system_buttons_enabled) { + break; + } + controller.home_button_state.home.Assign(current_status.value); + break; + case Settings::NativeButton::Screenshot: + if (!system_buttons_enabled) { + break; + } + controller.capture_button_state.capture.Assign(current_status.value); + break; + } + + lock.unlock(); + if (!is_connected) { if (npad_id_type == NpadIdType::Player1 && npad_type != NpadStyleIndex::Handheld) { Connect(); @@ -643,7 +648,7 @@ void EmulatedController::SetStick(const Common::Input::CallbackStatus& callback, if (index >= controller.stick_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; const auto stick_value = TransformToStick(callback); // Only read stick values that have the same uuid or are over the threshold to avoid flapping @@ -659,6 +664,7 @@ void EmulatedController::SetStick(const Common::Input::CallbackStatus& callback, if (is_configuring) { controller.analog_stick_state.left = {}; controller.analog_stick_state.right = {}; + lock.unlock(); TriggerOnChange(ControllerTriggerType::Stick, false); return; } @@ -685,6 +691,7 @@ void EmulatedController::SetStick(const Common::Input::CallbackStatus& callback, break; } + lock.unlock(); TriggerOnChange(ControllerTriggerType::Stick, true); } @@ -693,7 +700,7 @@ void EmulatedController::SetTrigger(const Common::Input::CallbackStatus& callbac if (index >= controller.trigger_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; const auto trigger_value = TransformToTrigger(callback); // Only read trigger values that have the same uuid or are pressed once @@ -709,6 +716,7 @@ void EmulatedController::SetTrigger(const Common::Input::CallbackStatus& callbac if (is_configuring) { controller.gc_trigger_state.left = 0; controller.gc_trigger_state.right = 0; + lock.unlock(); TriggerOnChange(ControllerTriggerType::Trigger, false); return; } @@ -727,6 +735,7 @@ void EmulatedController::SetTrigger(const Common::Input::CallbackStatus& callbac break; } + lock.unlock(); TriggerOnChange(ControllerTriggerType::Trigger, true); } @@ -735,7 +744,7 @@ void EmulatedController::SetMotion(const Common::Input::CallbackStatus& callback if (index >= controller.motion_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; auto& raw_status = controller.motion_values[index].raw_status; auto& emulated = controller.motion_values[index].emulated; @@ -756,6 +765,7 @@ void EmulatedController::SetMotion(const Common::Input::CallbackStatus& callback force_update_motion = raw_status.force_update; if (is_configuring) { + lock.unlock(); TriggerOnChange(ControllerTriggerType::Motion, false); return; } @@ -767,6 +777,7 @@ void EmulatedController::SetMotion(const Common::Input::CallbackStatus& callback motion.orientation = emulated.GetOrientation(); motion.is_at_rest = !emulated.IsMoving(motion_sensitivity); + lock.unlock(); TriggerOnChange(ControllerTriggerType::Motion, true); } @@ -775,10 +786,11 @@ void EmulatedController::SetBattery(const Common::Input::CallbackStatus& callbac if (index >= controller.battery_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; controller.battery_values[index] = TransformToBattery(callback); if (is_configuring) { + lock.unlock(); TriggerOnChange(ControllerTriggerType::Battery, false); return; } @@ -835,6 +847,8 @@ void EmulatedController::SetBattery(const Common::Input::CallbackStatus& callbac }; break; } + + lock.unlock(); TriggerOnChange(ControllerTriggerType::Battery, true); } @@ -932,6 +946,7 @@ void EmulatedController::SetSupportedNpadStyleTag(NpadStyleTag supported_styles) } bool EmulatedController::IsControllerFullkey(bool use_temporary_value) const { + std::lock_guard lock{mutex}; const auto type = is_configuring && use_temporary_value ? tmp_npad_type : npad_type; switch (type) { case NpadStyleIndex::ProController: @@ -947,6 +962,7 @@ bool EmulatedController::IsControllerFullkey(bool use_temporary_value) const { } bool EmulatedController::IsControllerSupported(bool use_temporary_value) const { + std::lock_guard lock{mutex}; const auto type = is_configuring && use_temporary_value ? tmp_npad_type : npad_type; switch (type) { case NpadStyleIndex::ProController: @@ -982,40 +998,44 @@ void EmulatedController::Connect(bool use_temporary_value) { LOG_ERROR(Service_HID, "Controller type {} is not supported", type); return; } - { - std::lock_guard lock{mutex}; - if (is_configuring) { - tmp_is_connected = true; - TriggerOnChange(ControllerTriggerType::Connected, false); - return; - } - if (is_connected) { - return; - } - is_connected = true; + std::unique_lock lock{mutex}; + if (is_configuring) { + tmp_is_connected = true; + lock.unlock(); + TriggerOnChange(ControllerTriggerType::Connected, false); + return; } + + if (is_connected) { + return; + } + is_connected = true; + + lock.unlock(); TriggerOnChange(ControllerTriggerType::Connected, true); } void EmulatedController::Disconnect() { - { - std::lock_guard lock{mutex}; - if (is_configuring) { - tmp_is_connected = false; - TriggerOnChange(ControllerTriggerType::Disconnected, false); - return; - } - - if (!is_connected) { - return; - } - is_connected = false; + std::unique_lock lock{mutex}; + if (is_configuring) { + tmp_is_connected = false; + lock.unlock(); + TriggerOnChange(ControllerTriggerType::Disconnected, false); + return; } + + if (!is_connected) { + return; + } + is_connected = false; + + lock.unlock(); TriggerOnChange(ControllerTriggerType::Disconnected, true); } bool EmulatedController::IsConnected(bool get_temporary_value) const { + std::lock_guard lock{mutex}; if (get_temporary_value && is_configuring) { return tmp_is_connected; } @@ -1029,10 +1049,12 @@ bool EmulatedController::IsVibrationEnabled() const { } NpadIdType EmulatedController::GetNpadIdType() const { + std::lock_guard lock{mutex}; return npad_id_type; } NpadStyleIndex EmulatedController::GetNpadStyleIndex(bool get_temporary_value) const { + std::lock_guard lock{mutex}; if (get_temporary_value && is_configuring) { return tmp_npad_type; } @@ -1040,27 +1062,28 @@ NpadStyleIndex EmulatedController::GetNpadStyleIndex(bool get_temporary_value) c } void EmulatedController::SetNpadStyleIndex(NpadStyleIndex npad_type_) { - { - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; - if (is_configuring) { - if (tmp_npad_type == npad_type_) { - return; - } - tmp_npad_type = npad_type_; - TriggerOnChange(ControllerTriggerType::Type, false); + if (is_configuring) { + if (tmp_npad_type == npad_type_) { return; } - - if (npad_type == npad_type_) { - return; - } - if (is_connected) { - LOG_WARNING(Service_HID, "Controller {} type changed while it's connected", - NpadIdTypeToIndex(npad_id_type)); - } - npad_type = npad_type_; + tmp_npad_type = npad_type_; + lock.unlock(); + TriggerOnChange(ControllerTriggerType::Type, false); + return; } + + if (npad_type == npad_type_) { + return; + } + if (is_connected) { + LOG_WARNING(Service_HID, "Controller {} type changed while it's connected", + NpadIdTypeToIndex(npad_id_type)); + } + npad_type = npad_type_; + + lock.unlock(); TriggerOnChange(ControllerTriggerType::Type, true); } @@ -1088,30 +1111,37 @@ LedPattern EmulatedController::GetLedPattern() const { } ButtonValues EmulatedController::GetButtonsValues() const { + std::lock_guard lock{mutex}; return controller.button_values; } SticksValues EmulatedController::GetSticksValues() const { + std::lock_guard lock{mutex}; return controller.stick_values; } TriggerValues EmulatedController::GetTriggersValues() const { + std::lock_guard lock{mutex}; return controller.trigger_values; } ControllerMotionValues EmulatedController::GetMotionValues() const { + std::lock_guard lock{mutex}; return controller.motion_values; } ColorValues EmulatedController::GetColorsValues() const { + std::lock_guard lock{mutex}; return controller.color_values; } BatteryValues EmulatedController::GetBatteryValues() const { + std::lock_guard lock{mutex}; return controller.battery_values; } HomeButtonState EmulatedController::GetHomeButtons() const { + std::lock_guard lock{mutex}; if (is_configuring) { return {}; } @@ -1119,6 +1149,7 @@ HomeButtonState EmulatedController::GetHomeButtons() const { } CaptureButtonState EmulatedController::GetCaptureButtons() const { + std::lock_guard lock{mutex}; if (is_configuring) { return {}; } @@ -1126,6 +1157,7 @@ CaptureButtonState EmulatedController::GetCaptureButtons() const { } NpadButtonState EmulatedController::GetNpadButtons() const { + std::lock_guard lock{mutex}; if (is_configuring) { return {}; } @@ -1133,6 +1165,7 @@ NpadButtonState EmulatedController::GetNpadButtons() const { } DebugPadButton EmulatedController::GetDebugPadButtons() const { + std::lock_guard lock{mutex}; if (is_configuring) { return {}; } @@ -1140,6 +1173,7 @@ DebugPadButton EmulatedController::GetDebugPadButtons() const { } AnalogSticks EmulatedController::GetSticks() const { + std::lock_guard lock{mutex}; if (is_configuring) { return {}; } @@ -1154,6 +1188,7 @@ AnalogSticks EmulatedController::GetSticks() const { } NpadGcTriggerState EmulatedController::GetTriggers() const { + std::lock_guard lock{mutex}; if (is_configuring) { return {}; } @@ -1161,6 +1196,7 @@ NpadGcTriggerState EmulatedController::GetTriggers() const { } MotionState EmulatedController::GetMotions() const { + std::lock_guard lock{mutex}; if (force_update_motion) { for (auto& device : motion_devices) { if (!device) { @@ -1173,14 +1209,17 @@ MotionState EmulatedController::GetMotions() const { } ControllerColors EmulatedController::GetColors() const { + std::lock_guard lock{mutex}; return controller.colors_state; } BatteryLevelState EmulatedController::GetBattery() const { + std::lock_guard lock{mutex}; return controller.battery_state; } void EmulatedController::TriggerOnChange(ControllerTriggerType type, bool is_npad_service_update) { + std::lock_guard lock{callback_mutex}; for (const auto& poller_pair : callback_list) { const ControllerUpdateCallback& poller = poller_pair.second; if (!is_npad_service_update && poller.is_npad_service) { @@ -1193,13 +1232,13 @@ void EmulatedController::TriggerOnChange(ControllerTriggerType type, bool is_npa } int EmulatedController::SetCallback(ControllerUpdateCallback update_callback) { - std::lock_guard lock{mutex}; + std::lock_guard lock{callback_mutex}; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); return last_callback_key++; } void EmulatedController::DeleteCallback(int key) { - std::lock_guard lock{mutex}; + std::lock_guard lock{callback_mutex}; const auto& iterator = callback_list.find(key); if (iterator == callback_list.end()) { LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); diff --git a/src/core/hid/emulated_controller.h b/src/core/hid/emulated_controller.h index aa52f9572..1e224685d 100644 --- a/src/core/hid/emulated_controller.h +++ b/src/core/hid/emulated_controller.h @@ -400,7 +400,7 @@ private: */ void TriggerOnChange(ControllerTriggerType type, bool is_service_update); - NpadIdType npad_id_type; + const NpadIdType npad_id_type; NpadStyleIndex npad_type{NpadStyleIndex::None}; NpadStyleTag supported_style_tag{NpadStyleSet::All}; bool is_connected{false}; @@ -434,6 +434,7 @@ private: StickDevices tas_stick_devices; mutable std::mutex mutex; + mutable std::mutex callback_mutex; std::unordered_map callback_list; int last_callback_key = 0; diff --git a/src/core/hid/emulated_devices.cpp b/src/core/hid/emulated_devices.cpp index 708480f2d..f899f8ac0 100644 --- a/src/core/hid/emulated_devices.cpp +++ b/src/core/hid/emulated_devices.cpp @@ -169,7 +169,7 @@ void EmulatedDevices::SetKeyboardButton(const Common::Input::CallbackStatus& cal if (index >= device_status.keyboard_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; bool value_changed = false; const auto new_status = TransformToButton(callback); auto& current_status = device_status.keyboard_values[index]; @@ -201,6 +201,7 @@ void EmulatedDevices::SetKeyboardButton(const Common::Input::CallbackStatus& cal } if (is_configuring) { + lock.unlock(); TriggerOnChange(DeviceTriggerType::Keyboard); return; } @@ -208,6 +209,7 @@ void EmulatedDevices::SetKeyboardButton(const Common::Input::CallbackStatus& cal // Index should be converted from NativeKeyboard to KeyboardKeyIndex UpdateKey(index, current_status.value); + lock.unlock(); TriggerOnChange(DeviceTriggerType::Keyboard); } @@ -227,7 +229,7 @@ void EmulatedDevices::SetKeyboardModifier(const Common::Input::CallbackStatus& c if (index >= device_status.keyboard_moddifier_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; bool value_changed = false; const auto new_status = TransformToButton(callback); auto& current_status = device_status.keyboard_moddifier_values[index]; @@ -259,6 +261,7 @@ void EmulatedDevices::SetKeyboardModifier(const Common::Input::CallbackStatus& c } if (is_configuring) { + lock.unlock(); TriggerOnChange(DeviceTriggerType::KeyboardModdifier); return; } @@ -289,6 +292,7 @@ void EmulatedDevices::SetKeyboardModifier(const Common::Input::CallbackStatus& c break; } + lock.unlock(); TriggerOnChange(DeviceTriggerType::KeyboardModdifier); } @@ -297,7 +301,7 @@ void EmulatedDevices::SetMouseButton(const Common::Input::CallbackStatus& callba if (index >= device_status.mouse_button_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; bool value_changed = false; const auto new_status = TransformToButton(callback); auto& current_status = device_status.mouse_button_values[index]; @@ -329,6 +333,7 @@ void EmulatedDevices::SetMouseButton(const Common::Input::CallbackStatus& callba } if (is_configuring) { + lock.unlock(); TriggerOnChange(DeviceTriggerType::Mouse); return; } @@ -351,6 +356,7 @@ void EmulatedDevices::SetMouseButton(const Common::Input::CallbackStatus& callba break; } + lock.unlock(); TriggerOnChange(DeviceTriggerType::Mouse); } @@ -359,13 +365,14 @@ void EmulatedDevices::SetMouseAnalog(const Common::Input::CallbackStatus& callba if (index >= device_status.mouse_analog_values.size()) { return; } - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; const auto analog_value = TransformToAnalog(callback); device_status.mouse_analog_values[index] = analog_value; if (is_configuring) { device_status.mouse_position_state = {}; + lock.unlock(); TriggerOnChange(DeviceTriggerType::Mouse); return; } @@ -379,17 +386,19 @@ void EmulatedDevices::SetMouseAnalog(const Common::Input::CallbackStatus& callba break; } + lock.unlock(); TriggerOnChange(DeviceTriggerType::Mouse); } void EmulatedDevices::SetMouseStick(const Common::Input::CallbackStatus& callback) { - std::lock_guard lock{mutex}; + std::unique_lock lock{mutex}; const auto touch_value = TransformToTouch(callback); device_status.mouse_stick_value = touch_value; if (is_configuring) { device_status.mouse_position_state = {}; + lock.unlock(); TriggerOnChange(DeviceTriggerType::Mouse); return; } @@ -397,42 +406,52 @@ void EmulatedDevices::SetMouseStick(const Common::Input::CallbackStatus& callbac device_status.mouse_position_state.x = touch_value.x.value; device_status.mouse_position_state.y = touch_value.y.value; + lock.unlock(); TriggerOnChange(DeviceTriggerType::Mouse); } KeyboardValues EmulatedDevices::GetKeyboardValues() const { + std::lock_guard lock{mutex}; return device_status.keyboard_values; } KeyboardModifierValues EmulatedDevices::GetKeyboardModdifierValues() const { + std::lock_guard lock{mutex}; return device_status.keyboard_moddifier_values; } MouseButtonValues EmulatedDevices::GetMouseButtonsValues() const { + std::lock_guard lock{mutex}; return device_status.mouse_button_values; } KeyboardKey EmulatedDevices::GetKeyboard() const { + std::lock_guard lock{mutex}; return device_status.keyboard_state; } KeyboardModifier EmulatedDevices::GetKeyboardModifier() const { + std::lock_guard lock{mutex}; return device_status.keyboard_moddifier_state; } MouseButton EmulatedDevices::GetMouseButtons() const { + std::lock_guard lock{mutex}; return device_status.mouse_button_state; } MousePosition EmulatedDevices::GetMousePosition() const { + std::lock_guard lock{mutex}; return device_status.mouse_position_state; } AnalogStickState EmulatedDevices::GetMouseWheel() const { + std::lock_guard lock{mutex}; return device_status.mouse_wheel_state; } void EmulatedDevices::TriggerOnChange(DeviceTriggerType type) { + std::lock_guard lock{callback_mutex}; for (const auto& poller_pair : callback_list) { const InterfaceUpdateCallback& poller = poller_pair.second; if (poller.on_change) { @@ -442,13 +461,13 @@ void EmulatedDevices::TriggerOnChange(DeviceTriggerType type) { } int EmulatedDevices::SetCallback(InterfaceUpdateCallback update_callback) { - std::lock_guard lock{mutex}; + std::lock_guard lock{callback_mutex}; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); return last_callback_key++; } void EmulatedDevices::DeleteCallback(int key) { - std::lock_guard lock{mutex}; + std::lock_guard lock{callback_mutex}; const auto& iterator = callback_list.find(key); if (iterator == callback_list.end()) { LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); diff --git a/src/core/hid/emulated_devices.h b/src/core/hid/emulated_devices.h index 790d3b411..73e9f0293 100644 --- a/src/core/hid/emulated_devices.h +++ b/src/core/hid/emulated_devices.h @@ -200,6 +200,7 @@ private: MouseStickDevice mouse_stick_device; mutable std::mutex mutex; + mutable std::mutex callback_mutex; std::unordered_map callback_list; int last_callback_key = 0; From 9c85cb354a6c00f82278e6c39d4b474c49dd4c5a Mon Sep 17 00:00:00 2001 From: Narr the Reg Date: Thu, 7 Apr 2022 13:52:51 -0500 Subject: [PATCH 2/3] core: hid: Replace lock_guard with scoped_lock --- src/core/hid/emulated_console.cpp | 14 ++++---- src/core/hid/emulated_controller.cpp | 52 ++++++++++++++-------------- src/core/hid/emulated_devices.cpp | 22 ++++++------ 3 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/core/hid/emulated_console.cpp b/src/core/hid/emulated_console.cpp index 639f61809..de565048b 100644 --- a/src/core/hid/emulated_console.cpp +++ b/src/core/hid/emulated_console.cpp @@ -197,27 +197,27 @@ void EmulatedConsole::SetTouch(const Common::Input::CallbackStatus& callback, st } ConsoleMotionValues EmulatedConsole::GetMotionValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return console.motion_values; } TouchValues EmulatedConsole::GetTouchValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return console.touch_values; } ConsoleMotion EmulatedConsole::GetMotion() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return console.motion_state; } TouchFingerState EmulatedConsole::GetTouch() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return console.touch_state; } void EmulatedConsole::TriggerOnChange(ConsoleTriggerType type) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; for (const auto& poller_pair : callback_list) { const ConsoleUpdateCallback& poller = poller_pair.second; if (poller.on_change) { @@ -227,13 +227,13 @@ void EmulatedConsole::TriggerOnChange(ConsoleTriggerType type) { } int EmulatedConsole::SetCallback(ConsoleUpdateCallback update_callback) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; callback_list.insert_or_assign(last_callback_key, update_callback); return last_callback_key++; } void EmulatedConsole::DeleteCallback(int key) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; const auto& iterator = callback_list.find(key); if (iterator == callback_list.end()) { LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); diff --git a/src/core/hid/emulated_controller.cpp b/src/core/hid/emulated_controller.cpp index 4f3676ec3..d3b13dbbd 100644 --- a/src/core/hid/emulated_controller.cpp +++ b/src/core/hid/emulated_controller.cpp @@ -353,17 +353,17 @@ void EmulatedController::DisableConfiguration() { } void EmulatedController::EnableSystemButtons() { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; system_buttons_enabled = true; } void EmulatedController::DisableSystemButtons() { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; system_buttons_enabled = false; } void EmulatedController::ResetSystemButtons() { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; controller.home_button_state.home.Assign(false); controller.capture_button_state.capture.Assign(false); } @@ -946,7 +946,7 @@ void EmulatedController::SetSupportedNpadStyleTag(NpadStyleTag supported_styles) } bool EmulatedController::IsControllerFullkey(bool use_temporary_value) const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; const auto type = is_configuring && use_temporary_value ? tmp_npad_type : npad_type; switch (type) { case NpadStyleIndex::ProController: @@ -962,7 +962,7 @@ bool EmulatedController::IsControllerFullkey(bool use_temporary_value) const { } bool EmulatedController::IsControllerSupported(bool use_temporary_value) const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; const auto type = is_configuring && use_temporary_value ? tmp_npad_type : npad_type; switch (type) { case NpadStyleIndex::ProController: @@ -1035,7 +1035,7 @@ void EmulatedController::Disconnect() { } bool EmulatedController::IsConnected(bool get_temporary_value) const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (get_temporary_value && is_configuring) { return tmp_is_connected; } @@ -1049,12 +1049,12 @@ bool EmulatedController::IsVibrationEnabled() const { } NpadIdType EmulatedController::GetNpadIdType() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return npad_id_type; } NpadStyleIndex EmulatedController::GetNpadStyleIndex(bool get_temporary_value) const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (get_temporary_value && is_configuring) { return tmp_npad_type; } @@ -1111,37 +1111,37 @@ LedPattern EmulatedController::GetLedPattern() const { } ButtonValues EmulatedController::GetButtonsValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.button_values; } SticksValues EmulatedController::GetSticksValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.stick_values; } TriggerValues EmulatedController::GetTriggersValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.trigger_values; } ControllerMotionValues EmulatedController::GetMotionValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.motion_values; } ColorValues EmulatedController::GetColorsValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.color_values; } BatteryValues EmulatedController::GetBatteryValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.battery_values; } HomeButtonState EmulatedController::GetHomeButtons() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (is_configuring) { return {}; } @@ -1149,7 +1149,7 @@ HomeButtonState EmulatedController::GetHomeButtons() const { } CaptureButtonState EmulatedController::GetCaptureButtons() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (is_configuring) { return {}; } @@ -1157,7 +1157,7 @@ CaptureButtonState EmulatedController::GetCaptureButtons() const { } NpadButtonState EmulatedController::GetNpadButtons() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (is_configuring) { return {}; } @@ -1165,7 +1165,7 @@ NpadButtonState EmulatedController::GetNpadButtons() const { } DebugPadButton EmulatedController::GetDebugPadButtons() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (is_configuring) { return {}; } @@ -1173,7 +1173,7 @@ DebugPadButton EmulatedController::GetDebugPadButtons() const { } AnalogSticks EmulatedController::GetSticks() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (is_configuring) { return {}; } @@ -1188,7 +1188,7 @@ AnalogSticks EmulatedController::GetSticks() const { } NpadGcTriggerState EmulatedController::GetTriggers() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (is_configuring) { return {}; } @@ -1196,7 +1196,7 @@ NpadGcTriggerState EmulatedController::GetTriggers() const { } MotionState EmulatedController::GetMotions() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; if (force_update_motion) { for (auto& device : motion_devices) { if (!device) { @@ -1209,17 +1209,17 @@ MotionState EmulatedController::GetMotions() const { } ControllerColors EmulatedController::GetColors() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.colors_state; } BatteryLevelState EmulatedController::GetBattery() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return controller.battery_state; } void EmulatedController::TriggerOnChange(ControllerTriggerType type, bool is_npad_service_update) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; for (const auto& poller_pair : callback_list) { const ControllerUpdateCallback& poller = poller_pair.second; if (!is_npad_service_update && poller.is_npad_service) { @@ -1232,13 +1232,13 @@ void EmulatedController::TriggerOnChange(ControllerTriggerType type, bool is_npa } int EmulatedController::SetCallback(ControllerUpdateCallback update_callback) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); return last_callback_key++; } void EmulatedController::DeleteCallback(int key) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; const auto& iterator = callback_list.find(key); if (iterator == callback_list.end()) { LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); diff --git a/src/core/hid/emulated_devices.cpp b/src/core/hid/emulated_devices.cpp index f899f8ac0..cc0dcd931 100644 --- a/src/core/hid/emulated_devices.cpp +++ b/src/core/hid/emulated_devices.cpp @@ -411,47 +411,47 @@ void EmulatedDevices::SetMouseStick(const Common::Input::CallbackStatus& callbac } KeyboardValues EmulatedDevices::GetKeyboardValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.keyboard_values; } KeyboardModifierValues EmulatedDevices::GetKeyboardModdifierValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.keyboard_moddifier_values; } MouseButtonValues EmulatedDevices::GetMouseButtonsValues() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.mouse_button_values; } KeyboardKey EmulatedDevices::GetKeyboard() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.keyboard_state; } KeyboardModifier EmulatedDevices::GetKeyboardModifier() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.keyboard_moddifier_state; } MouseButton EmulatedDevices::GetMouseButtons() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.mouse_button_state; } MousePosition EmulatedDevices::GetMousePosition() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.mouse_position_state; } AnalogStickState EmulatedDevices::GetMouseWheel() const { - std::lock_guard lock{mutex}; + std::scoped_lock lock{mutex}; return device_status.mouse_wheel_state; } void EmulatedDevices::TriggerOnChange(DeviceTriggerType type) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; for (const auto& poller_pair : callback_list) { const InterfaceUpdateCallback& poller = poller_pair.second; if (poller.on_change) { @@ -461,13 +461,13 @@ void EmulatedDevices::TriggerOnChange(DeviceTriggerType type) { } int EmulatedDevices::SetCallback(InterfaceUpdateCallback update_callback) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; callback_list.insert_or_assign(last_callback_key, std::move(update_callback)); return last_callback_key++; } void EmulatedDevices::DeleteCallback(int key) { - std::lock_guard lock{callback_mutex}; + std::scoped_lock lock{callback_mutex}; const auto& iterator = callback_list.find(key); if (iterator == callback_list.end()) { LOG_ERROR(Input, "Tried to delete non-existent callback {}", key); From bbaa08d7f05816960204cbf0b1569972f0928199 Mon Sep 17 00:00:00 2001 From: Narr the Reg Date: Thu, 7 Apr 2022 17:08:01 -0500 Subject: [PATCH 3/3] core: hid: Fix double lock on softlock and forced updates --- src/core/hid/emulated_controller.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/core/hid/emulated_controller.cpp b/src/core/hid/emulated_controller.cpp index d3b13dbbd..c3f21066c 100644 --- a/src/core/hid/emulated_controller.cpp +++ b/src/core/hid/emulated_controller.cpp @@ -1173,17 +1173,22 @@ DebugPadButton EmulatedController::GetDebugPadButtons() const { } AnalogSticks EmulatedController::GetSticks() const { - std::scoped_lock lock{mutex}; + std::unique_lock lock{mutex}; + if (is_configuring) { return {}; } + // Some drivers like stick from buttons need constant refreshing for (auto& device : stick_devices) { if (!device) { continue; } + lock.unlock(); device->SoftUpdate(); + lock.lock(); } + return controller.analog_stick_state; } @@ -1196,15 +1201,20 @@ NpadGcTriggerState EmulatedController::GetTriggers() const { } MotionState EmulatedController::GetMotions() const { - std::scoped_lock lock{mutex}; + std::unique_lock lock{mutex}; + + // Some drivers like mouse motion need constant refreshing if (force_update_motion) { for (auto& device : motion_devices) { if (!device) { continue; } + lock.unlock(); device->ForceUpdate(); + lock.lock(); } } + return controller.motion_state; }