From a877d5f6dc29e3559f1b64752906917f2e193d79 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 28 Sep 2018 14:05:53 +1000 Subject: [PATCH 1/4] Remove unused Host_ShowVideoConfig --- Source/Android/jni/MainAndroid.cpp | 4 ---- Source/Core/Core/Host.h | 1 - Source/Core/DolphinNoGUI/MainNoGUI.cpp | 4 ---- Source/Core/DolphinQt/Host.cpp | 3 --- Source/Core/VideoCommon/VideoBackendBase.cpp | 8 -------- Source/Core/VideoCommon/VideoBackendBase.h | 1 - Source/DSPTool/StubHost.cpp | 3 --- Source/UnitTests/StubHost.cpp | 3 --- 8 files changed, 27 deletions(-) diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index a160786063..abd79fa6a0 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -127,10 +127,6 @@ bool Host_RendererIsFullscreen() return false; } -void Host_ShowVideoConfig(void*, const std::string&) -{ -} - void Host_YieldToUI() { } diff --git a/Source/Core/Core/Host.h b/Source/Core/Core/Host.h index ae14bb686f..72e9c3d8a0 100644 --- a/Source/Core/Core/Host.h +++ b/Source/Core/Core/Host.h @@ -42,7 +42,6 @@ void Host_RequestRenderWindowSize(int width, int height); void Host_UpdateDisasmDialog(); void Host_UpdateMainFrame(); void Host_UpdateTitle(const std::string& title); -void Host_ShowVideoConfig(void* parent, const std::string& backend_name); void Host_YieldToUI(); void Host_UpdateProgressDialog(const char* caption, int position, int total); diff --git a/Source/Core/DolphinNoGUI/MainNoGUI.cpp b/Source/Core/DolphinNoGUI/MainNoGUI.cpp index 23a502e8f6..5097b1b8f9 100644 --- a/Source/Core/DolphinNoGUI/MainNoGUI.cpp +++ b/Source/Core/DolphinNoGUI/MainNoGUI.cpp @@ -130,10 +130,6 @@ bool Host_RendererIsFullscreen() return rendererIsFullscreen; } -void Host_ShowVideoConfig(void*, const std::string&) -{ -} - void Host_YieldToUI() { } diff --git a/Source/Core/DolphinQt/Host.cpp b/Source/Core/DolphinQt/Host.cpp index 8b448cc288..975091eac5 100644 --- a/Source/Core/DolphinQt/Host.cpp +++ b/Source/Core/DolphinQt/Host.cpp @@ -150,9 +150,6 @@ bool Host_UINeedsControllerState() { return Settings::Instance().IsControllerStateNeeded(); } -void Host_ShowVideoConfig(void* parent, const std::string& backend_name) -{ -} void Host_RefreshDSPDebuggerWindow() { } diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp index 77ffb294ff..327064a4f2 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.cpp +++ b/Source/Core/VideoCommon/VideoBackendBase.cpp @@ -60,14 +60,6 @@ __declspec(dllexport) DWORD NvOptimusEnablement = 1; } #endif -void VideoBackendBase::ShowConfig(void* parent_handle) -{ - if (!m_initialized) - InitBackendInfo(); - - Host_ShowVideoConfig(parent_handle, GetDisplayName()); -} - void VideoBackendBase::Video_ExitLoop() { Fifo::ExitGpuLoop(); diff --git a/Source/Core/VideoCommon/VideoBackendBase.h b/Source/Core/VideoCommon/VideoBackendBase.h index 576cd1f3e6..35a35583c3 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.h +++ b/Source/Core/VideoCommon/VideoBackendBase.h @@ -40,7 +40,6 @@ public: virtual std::string GetName() const = 0; virtual std::string GetDisplayName() const { return GetName(); } - void ShowConfig(void* parent_handle); virtual void InitBackendInfo() = 0; void Video_ExitLoop(); diff --git a/Source/DSPTool/StubHost.cpp b/Source/DSPTool/StubHost.cpp index 73ef4fa19b..ce7243e265 100644 --- a/Source/DSPTool/StubHost.cpp +++ b/Source/DSPTool/StubHost.cpp @@ -46,9 +46,6 @@ bool Host_RendererIsFullscreen() { return false; } -void Host_ShowVideoConfig(void*, const std::string&) -{ -} void Host_YieldToUI() { } diff --git a/Source/UnitTests/StubHost.cpp b/Source/UnitTests/StubHost.cpp index b80f175a2f..a2ba02c20b 100644 --- a/Source/UnitTests/StubHost.cpp +++ b/Source/UnitTests/StubHost.cpp @@ -48,9 +48,6 @@ bool Host_RendererIsFullscreen() { return false; } -void Host_ShowVideoConfig(void*, const std::string&) -{ -} void Host_YieldToUI() { } From 93923e2b290febbcdccad225c81e57454d0f0b75 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 28 Sep 2018 14:22:18 +1000 Subject: [PATCH 2/4] Don't fill backend info when core is running The current approach results in the UI thread creating a graphics device whilst the core is running, leading to races on function pointers, and potentially crashing. --- .../DolphinQt/Config/Graphics/GraphicsWindow.cpp | 16 +--------------- Source/Core/VideoCommon/VideoBackendBase.cpp | 16 ++++++++++++++++ Source/Core/VideoCommon/VideoBackendBase.h | 4 ++++ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp b/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp index 2edac668d1..456b132847 100644 --- a/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp +++ b/Source/Core/DolphinQt/Config/Graphics/GraphicsWindow.cpp @@ -38,9 +38,6 @@ void GraphicsWindow::Initialize() m_lazy_initialized = true; - g_Config.Refresh(); - g_video_backend->InitBackendInfo(); - CreateMainLayout(); setWindowTitle(tr("Graphics")); @@ -109,18 +106,7 @@ void GraphicsWindow::CreateMainLayout() void GraphicsWindow::OnBackendChanged(const QString& backend_name) { SConfig::GetInstance().m_strVideoBackend = backend_name.toStdString(); - - for (const auto& backend : g_available_video_backends) - { - if (backend->GetName() == backend_name.toStdString()) - { - g_Config.Refresh(); - - g_video_backend = backend.get(); - g_video_backend->InitBackendInfo(); - break; - } - } + VideoBackendBase::PopulateBackendInfo(); setWindowTitle( tr("%1 Graphics Configuration").arg(tr(g_video_backend->GetDisplayName().c_str()))); diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp index 327064a4f2..0e413b2d3c 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.cpp +++ b/Source/Core/VideoCommon/VideoBackendBase.cpp @@ -14,6 +14,8 @@ #include "Common/CommonTypes.h" #include "Common/Event.h" #include "Common/Logging/Log.h" +#include "Core/ConfigManager.h" +#include "Core/Core.h" #include "Core/Host.h" // TODO: ugly @@ -223,6 +225,20 @@ void VideoBackendBase::ActivateBackend(const std::string& name) g_video_backend = iter->get(); } +void VideoBackendBase::PopulateBackendInfo() +{ + // If the core is running, the backend info will have been populated already. + // If we did it here, the UI thread can race with the with the GPU thread. + if (Core::IsRunning()) + return; + + // We refresh the config after initializing the backend info, as system-specific settings + // such as anti-aliasing, or the selected adapter may be invalid, and should be checked. + ActivateBackend(SConfig::GetInstance().m_strVideoBackend); + g_video_backend->InitBackendInfo(); + g_Config.Refresh(); +} + // Run from the CPU thread void VideoBackendBase::DoState(PointerWrap& p) { diff --git a/Source/Core/VideoCommon/VideoBackendBase.h b/Source/Core/VideoCommon/VideoBackendBase.h index 35a35583c3..5f43b904d4 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.h +++ b/Source/Core/VideoCommon/VideoBackendBase.h @@ -54,6 +54,10 @@ public: static void ClearList(); static void ActivateBackend(const std::string& name); + // Fills the backend_info fields with the capabilities of the selected backend/device. + // Called by the UI thread when the graphics config is opened. + static void PopulateBackendInfo(); + // the implementation needs not do synchronization logic, because calls to it are surrounded by // PauseAndLock now void DoState(PointerWrap& p); From eb33d7af6420ceb78b36c84cb82ac5a34faf58c1 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 28 Sep 2018 17:53:55 +1000 Subject: [PATCH 3/4] Core: Call InitBackendInfo before loading config --- Source/Core/Core/Core.cpp | 7 +++++++ Source/Core/VideoBackends/D3D/main.cpp | 1 - Source/Core/VideoBackends/Null/NullBackend.cpp | 1 - Source/Core/VideoBackends/OGL/main.cpp | 1 - Source/Core/VideoBackends/Software/SWmain.cpp | 1 - Source/Core/VideoCommon/VideoBackendBase.cpp | 1 - 6 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index 7ec7f6c634..7ebb070d2b 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -74,6 +74,7 @@ #include "VideoCommon/OnScreenDisplay.h" #include "VideoCommon/RenderBase.h" #include "VideoCommon/VideoBackendBase.h" +#include "VideoCommon/VideoConfig.h" namespace Core { @@ -434,6 +435,12 @@ static void EmuThread(std::unique_ptr boot) HLE::Clear(); }}; + // Backend info has to be initialized before we can initialize the backend. + // This is because when we load the config, we validate it against the current backend info. + // We also should have the correct adapter selected for creating the device in Initialize(). + g_video_backend->InitBackendInfo(); + g_Config.Refresh(); + if (!g_video_backend->Initialize(s_window_handle)) { PanicAlert("Failed to initialize video backend!"); diff --git a/Source/Core/VideoBackends/D3D/main.cpp b/Source/Core/VideoBackends/D3D/main.cpp index f2299945cc..69b6f47804 100644 --- a/Source/Core/VideoBackends/D3D/main.cpp +++ b/Source/Core/VideoBackends/D3D/main.cpp @@ -132,7 +132,6 @@ bool VideoBackend::Initialize(void* window_handle) if (window_handle == nullptr) return false; - InitBackendInfo(); InitializeShared(); if (FAILED(D3D::Create(reinterpret_cast(window_handle)))) diff --git a/Source/Core/VideoBackends/Null/NullBackend.cpp b/Source/Core/VideoBackends/Null/NullBackend.cpp index fa32ce0cf5..f3743773c5 100644 --- a/Source/Core/VideoBackends/Null/NullBackend.cpp +++ b/Source/Core/VideoBackends/Null/NullBackend.cpp @@ -57,7 +57,6 @@ void VideoBackend::InitBackendInfo() bool VideoBackend::Initialize(void* window_handle) { InitializeShared(); - InitBackendInfo(); g_renderer = std::make_unique(); g_vertex_manager = std::make_unique(); diff --git a/Source/Core/VideoBackends/OGL/main.cpp b/Source/Core/VideoBackends/OGL/main.cpp index 52d643543f..d2e37f07a9 100644 --- a/Source/Core/VideoBackends/OGL/main.cpp +++ b/Source/Core/VideoBackends/OGL/main.cpp @@ -159,7 +159,6 @@ bool VideoBackend::FillBackendInfo() bool VideoBackend::Initialize(void* window_handle) { - InitBackendInfo(); InitializeShared(); GLUtil::InitInterface(); diff --git a/Source/Core/VideoBackends/Software/SWmain.cpp b/Source/Core/VideoBackends/Software/SWmain.cpp index 546cf0a45c..6ca183b57c 100644 --- a/Source/Core/VideoBackends/Software/SWmain.cpp +++ b/Source/Core/VideoBackends/Software/SWmain.cpp @@ -80,7 +80,6 @@ void VideoSoftware::InitBackendInfo() bool VideoSoftware::Initialize(void* window_handle) { - InitBackendInfo(); InitializeShared(); SWOGLWindow::Init(window_handle); diff --git a/Source/Core/VideoCommon/VideoBackendBase.cpp b/Source/Core/VideoCommon/VideoBackendBase.cpp index 0e413b2d3c..725857a85d 100644 --- a/Source/Core/VideoCommon/VideoBackendBase.cpp +++ b/Source/Core/VideoCommon/VideoBackendBase.cpp @@ -302,7 +302,6 @@ void VideoBackendBase::InitializeShared() GeometryShaderManager::Init(); PixelShaderManager::Init(); - g_Config.Refresh(); UpdateActiveConfig(); } From 349765ba77df11a9d5eeac424ac87e0047c7f0b3 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Fri, 28 Sep 2018 22:01:23 +1000 Subject: [PATCH 4/4] GraphicsWindow: Ensure adapter selection isn't enabled while running --- Source/Core/DolphinQt/Config/Graphics/GeneralWidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/DolphinQt/Config/Graphics/GeneralWidget.cpp b/Source/Core/DolphinQt/Config/Graphics/GeneralWidget.cpp index cb4c4a29d0..b8ba245256 100644 --- a/Source/Core/DolphinQt/Config/Graphics/GeneralWidget.cpp +++ b/Source/Core/DolphinQt/Config/Graphics/GeneralWidget.cpp @@ -313,7 +313,7 @@ void GeneralWidget::OnBackendChanged(const QString& backend_name) const bool supports_adapters = !adapters.empty(); m_adapter_combo->setCurrentIndex(g_Config.iAdapter); - m_adapter_combo->setEnabled(supports_adapters); + m_adapter_combo->setEnabled(supports_adapters && !Core::IsRunning()); m_adapter_combo->setToolTip(supports_adapters ? QStringLiteral("") :