From f0d51899c1c26dc4e12c4d3592b60652a258c86f Mon Sep 17 00:00:00 2001 From: Megamouse Date: Sun, 8 Aug 2021 18:50:37 +0200 Subject: [PATCH] input: fix minor data race While usually not exposed to the user, there was a slight chance that user input was read in a dirty state. This became apparent during usage of the new pressure sensitivity button --- rpcs3/Emu/Cell/Modules/cellGem.cpp | 4 +- rpcs3/Emu/Cell/Modules/cellPad.cpp | 4 +- rpcs3/Emu/Io/Buzz.cpp | 3 +- rpcs3/Emu/Io/GHLtar.cpp | 4 +- rpcs3/Emu/Io/Turntable.cpp | 3 +- rpcs3/Input/pad_thread.cpp | 100 ++++++++++++++++------------- rpcs3/Input/pad_thread.h | 3 +- 7 files changed, 69 insertions(+), 52 deletions(-) diff --git a/rpcs3/Emu/Cell/Modules/cellGem.cpp b/rpcs3/Emu/Cell/Modules/cellGem.cpp index 00f39f0a04..ad19079818 100644 --- a/rpcs3/Emu/Cell/Modules/cellGem.cpp +++ b/rpcs3/Emu/Cell/Modules/cellGem.cpp @@ -193,7 +193,7 @@ static bool check_gem_num(const u32 gem_num) */ static bool ds3_input_to_pad(const u32 port_no, be_t& digital_buttons, be_t& analog_t) { - std::scoped_lock lock(pad::g_pad_mutex); + std::lock_guard lock(pad::g_pad_mutex); const auto handler = pad::get_current_handler(); auto& pad = handler->GetPads()[port_no]; @@ -277,7 +277,7 @@ static bool ds3_input_to_pad(const u32 port_no, be_t& digital_buttons, be_t */ static bool ds3_input_to_ext(const u32 port_no, const gem_config::gem_controller& controller, CellGemExtPortData& ext) { - std::scoped_lock lock(pad::g_pad_mutex); + std::lock_guard lock(pad::g_pad_mutex); const auto handler = pad::get_current_handler(); auto& pad = handler->GetPads()[port_no]; diff --git a/rpcs3/Emu/Cell/Modules/cellPad.cpp b/rpcs3/Emu/Cell/Modules/cellPad.cpp index 7a8e3b3263..ced267288a 100644 --- a/rpcs3/Emu/Cell/Modules/cellPad.cpp +++ b/rpcs3/Emu/Cell/Modules/cellPad.cpp @@ -889,7 +889,7 @@ error_code cellPadSetSensorMode(u32 port_no, u32 mode) if (port_no >= CELL_PAD_MAX_PORT_NUM) return CELL_OK; - const auto pad = pads[port_no]; + const auto& pad = pads[port_no]; // TODO: find out if this is checked here or later or at all if (!(pad->m_device_capability & CELL_PAD_CAPABILITY_SENSOR_MODE)) @@ -993,6 +993,8 @@ error_code cellPadLddUnregisterController(s32 handle) const auto& pads = handler->GetPads(); + // TODO: check if handle >= pads.size() + if (!pads[handle]->ldd) return CELL_PAD_ERROR_NO_DEVICE; diff --git a/rpcs3/Emu/Io/Buzz.cpp b/rpcs3/Emu/Io/Buzz.cpp index 499f9191e5..40bbcbd950 100644 --- a/rpcs3/Emu/Io/Buzz.cpp +++ b/rpcs3/Emu/Io/Buzz.cpp @@ -59,12 +59,13 @@ void usb_device_buzz::interrupt_transfer(u32 buf_size, u8* buf, u32 /*endpoint*/ buf[3] = 0x00; buf[4] = 0xf0; + std::lock_guard lock(pad::g_pad_mutex); const auto handler = pad::get_current_handler(); const auto& pads = handler->GetPads(); for (int index = 0; index <= (last_controller - first_controller); index++) { - const auto pad = pads[first_controller + index]; + const auto& pad = pads[first_controller + index]; if (!(pad->m_port_status & CELL_PAD_STATUS_CONNECTED)) continue; diff --git a/rpcs3/Emu/Io/GHLtar.cpp b/rpcs3/Emu/Io/GHLtar.cpp index e6d80e8134..7f921935e7 100644 --- a/rpcs3/Emu/Io/GHLtar.cpp +++ b/rpcs3/Emu/Io/GHLtar.cpp @@ -96,9 +96,9 @@ void usb_device_ghltar::interrupt_transfer(u32 buf_size, u8* buf, u32 /*endpoint // buf[7] through buf[18] are always 0x00 // buf[21]/[23]/[25] are also always 0x00 + std::lock_guard lock(pad::g_pad_mutex); const auto handler = pad::get_current_handler(); - const auto& pads = handler->GetPads(); - const auto pad = pads[6]; + const auto& pad = handler->GetPads()[6]; if (!(pad->m_port_status & CELL_PAD_STATUS_CONNECTED)) return; diff --git a/rpcs3/Emu/Io/Turntable.cpp b/rpcs3/Emu/Io/Turntable.cpp index b458285c85..e5fe6b7b22 100644 --- a/rpcs3/Emu/Io/Turntable.cpp +++ b/rpcs3/Emu/Io/Turntable.cpp @@ -117,8 +117,7 @@ void usb_device_turntable::interrupt_transfer(u32 buf_size, u8* buf, u32 /*endpo // All other bufs are always 0x00 const auto handler = pad::get_current_handler(); - const auto& pads = handler->GetPads(); - const auto pad = pads[6]; + const auto& pad = handler->GetPads()[6]; if (!(pad->m_port_status & CELL_PAD_STATUS_CONNECTED)) return; diff --git a/rpcs3/Input/pad_thread.cpp b/rpcs3/Input/pad_thread.cpp index 25445b2f21..0c6a8da1bb 100644 --- a/rpcs3/Input/pad_thread.cpp +++ b/rpcs3/Input/pad_thread.cpp @@ -160,18 +160,21 @@ void pad_thread::Init() input_log.error("Failed to bind device %s to handler %s", g_cfg_input.player[i]->device.to_string(), handler_type); nullpad->bindPadToDevice(m_pads[i], g_cfg_input.player[i]->device.to_string()); } + + m_pads_interface[i] = std::make_shared(CELL_PAD_STATUS_DISCONNECTED, pad_settings[i].device_capability, pad_settings[i].device_type); + *m_pads_interface[i] = *m_pads[i]; } } void pad_thread::SetRumble(const u32 pad, u8 largeMotor, bool smallMotor) { - if (pad > m_pads.size()) + if (pad > m_pads_interface.size()) return; - if (m_pads[pad]->m_vibrateMotors.size() >= 2) + if (m_pads_interface[pad]->m_vibrateMotors.size() >= 2) { - m_pads[pad]->m_vibrateMotors[0].m_value = largeMotor; - m_pads[pad]->m_vibrateMotors[1].m_value = smallMotor ? 255 : 0; + m_pads_interface[pad]->m_vibrateMotors[0].m_value = largeMotor; + m_pads_interface[pad]->m_vibrateMotors[1].m_value = smallMotor ? 255 : 0; } } @@ -206,69 +209,74 @@ void pad_thread::ThreadFunc() u32 connected_devices = 0; + // Copy public pad data - which might have been changed - to internal pads + { + std::lock_guard lock(pad::g_pad_mutex); + + for (usz i = 0; i < m_pads.size(); i++) + { + *m_pads[i] = *m_pads_interface[i]; + } + } + for (auto& cur_pad_handler : handlers) { cur_pad_handler.second->ThreadProc(); connected_devices += cur_pad_handler.second->connected_devices; } - m_info.now_connect = connected_devices + num_ldd_pad; - - // The following section is only reached when a dialog was closed and the pads are still intercepted. - // As long as any of the listed buttons is pressed, cellPadGetData will ignore all input (needed for Hotline Miami). - // ignore_input was added because if we keep the pads intercepted, then some games will enter the menu due to unexpected system interception (tested with Ninja Gaiden Sigma). - if (!(m_info.system_info & CELL_PAD_INFO_INTERCEPTED) && m_info.ignore_input) + // Copy new internal pad data back to public pads { + std::lock_guard lock(pad::g_pad_mutex); + + m_info.now_connect = connected_devices + num_ldd_pad; + + // The input_ignored section is only reached when a dialog was closed and the pads are still intercepted. + // As long as any of the listed buttons is pressed, cellPadGetData will ignore all input (needed for Hotline Miami). + // ignore_input was added because if we keep the pads intercepted, then some games will enter the menu due to unexpected system interception (tested with Ninja Gaiden Sigma). + const bool input_ignored = m_info.ignore_input && !(m_info.system_info & CELL_PAD_INFO_INTERCEPTED); bool any_button_pressed = false; - for (const auto& pad : m_pads) + for (usz i = 0; i < m_pads.size(); i++) { + const auto& pad = m_pads[i]; + + // I guess this is the best place to add pressure sensitivity without too much code duplication. if (pad->m_port_status & CELL_PAD_STATUS_CONNECTED) { - for (const auto& button : pad->m_buttons) + const bool adjust_pressure = pad->m_pressure_intensity_button_index >= 0 && pad->m_buttons[pad->m_pressure_intensity_button_index].m_pressed; + + for (auto& button : pad->m_buttons) { - if (button.m_pressed && ( - button.m_outKeyCode == CELL_PAD_CTRL_CROSS || - button.m_outKeyCode == CELL_PAD_CTRL_CIRCLE || - button.m_outKeyCode == CELL_PAD_CTRL_TRIANGLE || - button.m_outKeyCode == CELL_PAD_CTRL_SQUARE || - button.m_outKeyCode == CELL_PAD_CTRL_START || - button.m_outKeyCode == CELL_PAD_CTRL_SELECT)) + if (button.m_pressed) { - any_button_pressed = true; - break; + if (button.m_outKeyCode == CELL_PAD_CTRL_CROSS || + button.m_outKeyCode == CELL_PAD_CTRL_CIRCLE || + button.m_outKeyCode == CELL_PAD_CTRL_TRIANGLE || + button.m_outKeyCode == CELL_PAD_CTRL_SQUARE || + button.m_outKeyCode == CELL_PAD_CTRL_START || + button.m_outKeyCode == CELL_PAD_CTRL_SELECT) + { + any_button_pressed = true; + } + + if (adjust_pressure) + { + button.m_value = pad->m_pressure_intensity; + } } } - - if (any_button_pressed) - { - break; - } } + + *m_pads_interface[i] = *pad; } - if (!any_button_pressed) + if (input_ignored && !any_button_pressed) { m_info.ignore_input = false; } } - // I guess this is the best place to add pressure sensitivity without too much code duplication. - for (const auto& pad : m_pads) - { - if ((pad->m_port_status & CELL_PAD_STATUS_CONNECTED) && - pad->m_pressure_intensity_button_index >= 0 && pad->m_buttons[pad->m_pressure_intensity_button_index].m_pressed) - { - for (auto& button : pad->m_buttons) - { - if (button.m_pressed) - { - button.m_value = pad->m_pressure_intensity; - } - } - } - } - std::this_thread::sleep_for(1ms); } } @@ -295,6 +303,8 @@ void pad_thread::InitLddPad(u32 handle) 50 ); + *m_pads_interface[handle] = *m_pads[handle]; + num_ldd_pad++; } @@ -315,6 +325,10 @@ s32 pad_thread::AddLddPad() void pad_thread::UnregisterLddPad(u32 handle) { + ensure(handle < m_pads.size()); + m_pads[handle]->ldd = false; + m_pads_interface[handle]->ldd = false; + num_ldd_pad--; } diff --git a/rpcs3/Input/pad_thread.h b/rpcs3/Input/pad_thread.h index b78d8a97bc..ccb0914524 100644 --- a/rpcs3/Input/pad_thread.h +++ b/rpcs3/Input/pad_thread.h @@ -23,7 +23,7 @@ public: ~pad_thread(); PadInfo& GetInfo() { return m_info; } - auto& GetPads() { return m_pads; } + auto& GetPads() { return m_pads_interface; } void SetRumble(const u32 pad, u8 largeMotor, bool smallMotor); void Init(); void SetIntercepted(bool intercepted); @@ -44,6 +44,7 @@ protected: PadInfo m_info{ 0, 0, false }; std::array, CELL_PAD_MAX_PORT_NUM> m_pads; + std::array, CELL_PAD_MAX_PORT_NUM> m_pads_interface; std::shared_ptr thread;