From c827fdd2b576d34af43a9cb3b80ecd9616493cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Thu, 7 Jul 2016 15:40:14 +0200 Subject: [PATCH] WiimoteReal: Don't block on refresh This changes Refresh() to use the existing scanning thread to scan for devices, instead of running the scan on the UI thread and blocking it. Also makes the UI thread not block when Continuous Scanning is disabled and removes duplicated code. Should fix issue 8992. Under the hood: * The scanning thread is now always active, even when continuous scanning is disabled. * The initialize code which waits for Wiimotes to be connected also uses the scanning thread instead of scanning on yet another thread. * The scanning thread now always checks for disconnected devices, to avoid Dolphin thinking a Wiimote is still connected when it isn't. So we now check if we need new Wiimotes or a Balance Board at scan time. --- Source/Core/Core/Core.cpp | 4 +- Source/Core/Core/HW/Wiimote.cpp | 4 +- Source/Core/Core/HW/Wiimote.h | 10 +- .../Core/Core/HW/WiimoteReal/WiimoteReal.cpp | 156 ++++++------------ Source/Core/Core/HW/WiimoteReal/WiimoteReal.h | 23 ++- Source/Core/DolphinWX/ControllerConfigDiag.h | 2 +- Source/Core/DolphinWX/Frame.cpp | 6 +- 7 files changed, 79 insertions(+), 126 deletions(-) diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index f01b17beae..d4aae1f7ea 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -533,7 +533,9 @@ void EmuThread() if (core_parameter.bWii) { if (init_controllers) - Wiimote::Initialize(s_window_handle, !s_state_filename.empty()); + Wiimote::Initialize(s_window_handle, !s_state_filename.empty() ? + Wiimote::InitializeMode::DO_WAIT_FOR_WIIMOTES : + Wiimote::InitializeMode::DO_NOT_WAIT_FOR_WIIMOTES); else Wiimote::LoadConfig(); diff --git a/Source/Core/Core/HW/Wiimote.cpp b/Source/Core/Core/HW/Wiimote.cpp index 7b57eb4cbd..d10620c4fb 100644 --- a/Source/Core/Core/HW/Wiimote.cpp +++ b/Source/Core/Core/HW/Wiimote.cpp @@ -28,7 +28,7 @@ void Shutdown() g_controller_interface.Shutdown(); } -void Initialize(void* const hwnd, bool wait) +void Initialize(void* const hwnd, InitializeMode init_mode) { if (s_config.ControllersNeedToBeCreated()) { @@ -40,7 +40,7 @@ void Initialize(void* const hwnd, bool wait) s_config.LoadConfig(false); - WiimoteReal::Initialize(wait); + WiimoteReal::Initialize(init_mode); // Reload Wiimotes with our settings if (Movie::IsMovieActive()) diff --git a/Source/Core/Core/HW/Wiimote.h b/Source/Core/Core/HW/Wiimote.h index 96503f9905..1c77042342 100644 --- a/Source/Core/Core/HW/Wiimote.h +++ b/Source/Core/Core/HW/Wiimote.h @@ -35,8 +35,14 @@ extern unsigned int g_wiimote_sources[MAX_BBMOTES]; namespace Wiimote { +enum class InitializeMode +{ + DO_WAIT_FOR_WIIMOTES, + DO_NOT_WAIT_FOR_WIIMOTES, +}; + void Shutdown(); -void Initialize(void* const hwnd, bool wait = false); +void Initialize(void* const hwnd, InitializeMode init_mode); void ResetAllWiimotes(); void LoadConfig(); void Resume(); @@ -54,7 +60,7 @@ void Update(int _number, bool _connected); namespace WiimoteReal { -void Initialize(bool wait = false); +void Initialize(::Wiimote::InitializeMode init_mode); void Stop(); void Shutdown(); void Resume(); diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp index f8343e5b43..ee81729042 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp +++ b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp @@ -430,34 +430,29 @@ static unsigned int CalculateWantedBB() return wanted_bb; } -void WiimoteScanner::WantWiimotes(bool do_want) +void WiimoteScanner::StartThread() { - m_want_wiimotes.store(do_want); + if (m_scan_thread_running.IsSet()) + return; + m_scan_thread_running.Set(); + m_scan_thread = std::thread(&WiimoteScanner::ThreadFunc, this); } -void WiimoteScanner::WantBB(bool do_want) +void WiimoteScanner::StopThread() { - m_want_bb.store(do_want); -} - -void WiimoteScanner::StartScanning() -{ - if (!m_run_thread.load()) - { - m_run_thread.store(true); - m_scan_thread = std::thread(&WiimoteScanner::ThreadFunc, this); - } -} - -void WiimoteScanner::StopScanning() -{ - m_run_thread.store(false); - if (m_scan_thread.joinable()) + if (m_scan_thread_running.TestAndClear()) { + SetScanMode(WiimoteScanMode::DO_NOT_SCAN); m_scan_thread.join(); } } +void WiimoteScanner::SetScanMode(WiimoteScanMode scan_mode) +{ + m_scan_mode.store(scan_mode); + m_scan_mode_changed_event.Set(); +} + static void CheckForDisconnectedWiimotes() { std::lock_guard lk(g_refresh_lock); @@ -471,41 +466,33 @@ void WiimoteScanner::ThreadFunc() { Common::SetCurrentThreadName("Wiimote Scanning Thread"); - NOTICE_LOG(WIIMOTE, "Wiimote scanning has started."); + NOTICE_LOG(WIIMOTE, "Wiimote scanning thread has started."); - while (m_run_thread.load()) + while (m_scan_thread_running.IsSet()) { - std::vector found_wiimotes; - Wiimote* found_board = nullptr; + m_scan_mode_changed_event.WaitFor(std::chrono::milliseconds(500)); - // NOTICE_LOG(WIIMOTE, "In loop"); - - if (m_want_wiimotes.load() || m_want_bb.load()) - { - FindWiimotes(found_wiimotes, found_board); - } - else - { - // Does stuff needed to detect disconnects on Windows - Update(); - } - - // NOTICE_LOG(WIIMOTE, "After update"); - - // TODO: this is a fairly lame place for this + Update(); // Does stuff needed to detect disconnects on Windows CheckForDisconnectedWiimotes(); - if (m_want_wiimotes.load()) + if (m_scan_mode.load() == WiimoteScanMode::DO_NOT_SCAN) + continue; + + if (CalculateWantedWiimotes() != 0 || CalculateWantedBB() != 0) + { + std::vector found_wiimotes; + Wiimote* found_board = nullptr; + FindWiimotes(found_wiimotes, found_board); HandleFoundWiimotes(found_wiimotes); + if (found_board) + TryToConnectBalanceBoard(found_board); + } - if (m_want_bb.load() && found_board) - TryToConnectBalanceBoard(found_board); - - // std::this_thread::yield(); - Common::SleepCurrentThread(500); + if (m_scan_mode.load() == WiimoteScanMode::SCAN_ONCE) + m_scan_mode.store(WiimoteScanMode::DO_NOT_SCAN); } - NOTICE_LOG(WIIMOTE, "Wiimote scanning has stopped."); + NOTICE_LOG(WIIMOTE, "Wiimote scanning thread has stopped."); } bool Wiimote::Connect(int index) @@ -621,33 +608,27 @@ void LoadSettings() } // config dialog calls this when some settings change -void Initialize(bool wait) +void Initialize(::Wiimote::InitializeMode init_mode) { + if (!g_real_wiimotes_initialized) + g_wiimote_scanner.StartThread(); + if (SConfig::GetInstance().m_WiimoteContinuousScanning) - g_wiimote_scanner.StartScanning(); + g_wiimote_scanner.SetScanMode(WiimoteScanMode::CONTINUOUSLY_SCAN); else - g_wiimote_scanner.StopScanning(); + g_wiimote_scanner.SetScanMode(WiimoteScanMode::DO_NOT_SCAN); std::lock_guard lk(g_refresh_lock); - g_wiimote_scanner.WantWiimotes(0 != CalculateWantedWiimotes()); - g_wiimote_scanner.WantBB(0 != CalculateWantedBB()); - // wait for connection because it should exist before state load - if (wait) + if (init_mode == ::Wiimote::InitializeMode::DO_WAIT_FOR_WIIMOTES) { int timeout = 100; - std::vector found_wiimotes; - Wiimote* found_board = nullptr; - g_wiimote_scanner.FindWiimotes(found_wiimotes, found_board); - if (SConfig::GetInstance().m_WiimoteContinuousScanning) + g_wiimote_scanner.SetScanMode(WiimoteScanMode::SCAN_ONCE); + while (CalculateWantedWiimotes() > CalculateConnectedWiimotes() && timeout) { - while (CalculateWantedWiimotes() && CalculateConnectedWiimotes() < found_wiimotes.size() && - timeout) - { - Common::SleepCurrentThread(100); - timeout--; - } + Common::SleepCurrentThread(100); + timeout--; } } @@ -670,17 +651,12 @@ void Stop() // called when the Dolphin app exits void Shutdown() { - g_wiimote_scanner.StopScanning(); + g_wiimote_scanner.StopThread(); std::lock_guard lk(g_refresh_lock); - if (!g_real_wiimotes_initialized) - return; - NOTICE_LOG(WIIMOTE, "WiimoteReal::Shutdown"); - g_real_wiimotes_initialized = false; - for (unsigned int i = 0; i < MAX_BBMOTES; ++i) HandleWiimoteDisconnect(i); } @@ -704,8 +680,6 @@ void ChangeWiimoteSource(unsigned int index, int source) { std::lock_guard lk(g_refresh_lock); g_wiimote_sources[index] = source; - g_wiimote_scanner.WantWiimotes(0 != CalculateWantedWiimotes()); - g_wiimote_scanner.WantBB(0 != CalculateWantedBB()); // kill real connection (or swap to different slot) DoneWithWiimote(index); } @@ -744,8 +718,6 @@ void TryToConnectWiimote(Wiimote* wm) } } - g_wiimote_scanner.WantWiimotes(0 != CalculateWantedWiimotes()); - lk.unlock(); delete wm; @@ -760,8 +732,6 @@ void TryToConnectBalanceBoard(Wiimote* wm) wm = nullptr; } - g_wiimote_scanner.WantBB(0 != CalculateWantedBB()); - lk.unlock(); delete wm; @@ -790,10 +760,7 @@ void HandleWiimoteDisconnect(int index) { std::lock_guard lk(g_refresh_lock); - std::swap(wm, g_wiimotes[index]); - g_wiimote_scanner.WantWiimotes(0 != CalculateWantedWiimotes()); - g_wiimote_scanner.WantBB(0 != CalculateWantedBB()); } if (wm) @@ -811,39 +778,8 @@ void HandleFoundWiimotes(const std::vector& wiimotes) // This is called from the GUI thread void Refresh() { - g_wiimote_scanner.StopScanning(); - - { - std::unique_lock lk(g_refresh_lock); - std::vector found_wiimotes; - Wiimote* found_board = nullptr; - - if (0 != CalculateWantedWiimotes() || 0 != CalculateWantedBB()) - { - // Don't hang Dolphin when searching - lk.unlock(); - g_wiimote_scanner.FindWiimotes(found_wiimotes, found_board); - lk.lock(); - } - - CheckForDisconnectedWiimotes(); - - // Brief rumble for already connected Wiimotes. - // Don't do this for Balance Board as it doesn't have rumble anyway. - for (int i = 0; i < MAX_WIIMOTES; ++i) - { - if (g_wiimotes[i]) - { - g_wiimotes[i]->Prepare(); - } - } - - HandleFoundWiimotes(found_wiimotes); - if (found_board) - TryToConnectBalanceBoard(found_board); - } - - Initialize(); + if (!SConfig::GetInstance().m_WiimoteContinuousScanning) + g_wiimote_scanner.SetScanMode(WiimoteScanMode::SCAN_ONCE); } void InterruptChannel(int _WiimoteNumber, u16 _channelID, const void* _pData, u32 _Size) diff --git a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h index 4ac81de4c7..392745a5f8 100644 --- a/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h +++ b/Source/Core/Core/HW/WiimoteReal/WiimoteReal.h @@ -12,7 +12,9 @@ #include #include "Common/Common.h" +#include "Common/Event.h" #include "Common/FifoQueue.h" +#include "Common/Flag.h" #include "Common/NonCopyable.h" #include "Core/HW/Wiimote.h" #include "Core/HW/WiimoteReal/WiimoteRealBase.h" @@ -112,6 +114,13 @@ private: Common::FifoQueue m_write_reports; }; +enum class WiimoteScanMode +{ + DO_NOT_SCAN, + CONTINUOUSLY_SCAN, + SCAN_ONCE +}; + class WiimoteScanner { public: @@ -120,11 +129,9 @@ public: bool IsReady() const; - void WantWiimotes(bool do_want); - void WantBB(bool do_want); - - void StartScanning(); - void StopScanning(); + void StartThread(); + void StopThread(); + void SetScanMode(WiimoteScanMode scan_mode); void FindWiimotes(std::vector&, Wiimote*&); @@ -136,9 +143,9 @@ private: std::thread m_scan_thread; - std::atomic m_run_thread{false}; - std::atomic m_want_wiimotes{false}; - std::atomic m_want_bb{false}; + Common::Event m_scan_mode_changed_event; + Common::Flag m_scan_thread_running; + std::atomic m_scan_mode{WiimoteScanMode::DO_NOT_SCAN}; #if defined(_WIN32) void CheckDeviceType(std::basic_string& devicepath, WinWriteMethod& write_method, diff --git a/Source/Core/DolphinWX/ControllerConfigDiag.h b/Source/Core/DolphinWX/ControllerConfigDiag.h index 98c7a1af47..e91b405330 100644 --- a/Source/Core/DolphinWX/ControllerConfigDiag.h +++ b/Source/Core/DolphinWX/ControllerConfigDiag.h @@ -59,7 +59,7 @@ private: void OnContinuousScanning(wxCommandEvent& event) { SConfig::GetInstance().m_WiimoteContinuousScanning = event.IsChecked(); - WiimoteReal::Initialize(); + WiimoteReal::Initialize(Wiimote::InitializeMode::DO_NOT_WAIT_FOR_WIIMOTES); event.Skip(); } diff --git a/Source/Core/DolphinWX/Frame.cpp b/Source/Core/DolphinWX/Frame.cpp index c785fe0dd2..608a6fe507 100644 --- a/Source/Core/DolphinWX/Frame.cpp +++ b/Source/Core/DolphinWX/Frame.cpp @@ -348,12 +348,14 @@ bool CFrame::InitControllers() Window win = X11Utils::XWindowFromHandle(GetHandle()); Pad::Initialize(reinterpret_cast(win)); Keyboard::Initialize(reinterpret_cast(win)); - Wiimote::Initialize(reinterpret_cast(win)); + Wiimote::Initialize(reinterpret_cast(win), + Wiimote::InitializeMode::DO_NOT_WAIT_FOR_WIIMOTES); HotkeyManagerEmu::Initialize(reinterpret_cast(win)); #else Pad::Initialize(reinterpret_cast(GetHandle())); Keyboard::Initialize(reinterpret_cast(GetHandle())); - Wiimote::Initialize(reinterpret_cast(GetHandle())); + Wiimote::Initialize(reinterpret_cast(GetHandle()), + Wiimote::InitializeMode::DO_NOT_WAIT_FOR_WIIMOTES); HotkeyManagerEmu::Initialize(reinterpret_cast(GetHandle())); #endif return true;