From 817d96a73d6ad8f4d6032a68ea28df8a950fd6c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sun, 7 May 2023 19:21:43 +0200 Subject: [PATCH 1/2] Only show assertion popup for assertions on main thread The SDL function may only be called from the main thread, so we can't show a popup for failed assertions on other threads. --- src/engine/client/client.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index cb7737486..fee56a174 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -4589,7 +4589,10 @@ int main(int argc, const char **argv) pKernel->RegisterInterface(pClient, false); pClient->RegisterInterfaces(); - dbg_assert_set_handler([pClient](const char *pMsg) { + const std::thread::id MainThreadId = std::this_thread::get_id(); + dbg_assert_set_handler([MainThreadId, pClient](const char *pMsg) { + if(MainThreadId != std::this_thread::get_id()) + return; char aVersionStr[128]; if(os_version_str(aVersionStr, sizeof(aVersionStr))) str_copy(aVersionStr, "unknown"); From c841c7ad05d2d14605078a6d0809bcd9c7956cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sun, 7 May 2023 19:23:03 +0200 Subject: [PATCH 2/2] Destroy window before showing popup if graphics initialized Work around SDL bug that prevents message popup from being closed. --- src/engine/client/backend_sdl.cpp | 24 ++++++++++++++++++------ src/engine/client/backend_sdl.h | 8 ++------ src/engine/client/client.cpp | 3 ++- src/engine/client/graphics_threaded.cpp | 7 +++++++ src/engine/client/graphics_threaded.h | 4 ++++ src/engine/graphics.h | 3 +++ 6 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/engine/client/backend_sdl.cpp b/src/engine/client/backend_sdl.cpp index 4559e4aa8..ca859f1f8 100644 --- a/src/engine/client/backend_sdl.cpp +++ b/src/engine/client/backend_sdl.cpp @@ -161,7 +161,7 @@ void CGraphicsBackend_Threaded::ProcessError() else VerboseStr.append(ErrStr.m_Err + "\n"); } - const auto CreatedMsgBox = TryCreateMsgBox(true, "Graphics Assertion", VerboseStr.c_str()); + const bool CreatedMsgBox = ShowMessageBox(SDL_MESSAGEBOX_ERROR, "Graphics Assertion", VerboseStr.c_str()); // check if error msg can be shown, then assert dbg_assert(!CreatedMsgBox, VerboseStr.c_str()); } @@ -774,12 +774,19 @@ void CGraphicsBackend_SDL_GL::ClampDriverVersion(EBackendType BackendType) } } -bool CGraphicsBackend_SDL_GL::TryCreateMsgBox(bool AsError, const char *pTitle, const char *pMsg) +bool CGraphicsBackend_SDL_GL::ShowMessageBox(unsigned Type, const char *pTitle, const char *pMsg) { - m_pProcessor->ErroneousCleanup(); - SDL_DestroyWindow(m_pWindow); - SDL_ShowSimpleMessageBox(AsError ? SDL_MESSAGEBOX_ERROR : SDL_MESSAGEBOX_WARNING, pTitle, pMsg, nullptr); - return true; + if(m_pProcessor != nullptr) + m_pProcessor->ErroneousCleanup(); + // TODO: Remove this workaround when https://github.com/libsdl-org/SDL/issues/3750 is + // fixed and pass the window to SDL_ShowSimpleMessageBox to make the popup modal instead + // of destroying the window before opening the popup. + if(m_pWindow != nullptr) + { + SDL_DestroyWindow(m_pWindow); + m_pWindow = nullptr; + } + return SDL_ShowSimpleMessageBox(Type, pTitle, pMsg, nullptr) == 0; } bool CGraphicsBackend_SDL_GL::IsModernAPI(EBackendType BackendType) @@ -1203,6 +1210,7 @@ int CGraphicsBackend_SDL_GL::Init(const char *pName, int *pScreen, int *pWidth, if(m_GLContext == NULL) { SDL_DestroyWindow(m_pWindow); + m_pWindow = nullptr; dbg_msg("gfx", "unable to create graphic context: %s", SDL_GetError()); return EGraphicsBackendErrorCodes::GRAPHICS_BACKEND_ERROR_CODE_GL_CONTEXT_FAILED; } @@ -1211,6 +1219,7 @@ int CGraphicsBackend_SDL_GL::Init(const char *pName, int *pScreen, int *pWidth, { SDL_GL_DeleteContext(m_GLContext); SDL_DestroyWindow(m_pWindow); + m_pWindow = nullptr; return EGraphicsBackendErrorCodes::GRAPHICS_BACKEND_ERROR_CODE_UNKNOWN; } } @@ -1237,6 +1246,7 @@ int CGraphicsBackend_SDL_GL::Init(const char *pName, int *pScreen, int *pWidth, if(m_GLContext) SDL_GL_DeleteContext(m_GLContext); SDL_DestroyWindow(m_pWindow); + m_pWindow = nullptr; // try setting to glew supported version g_Config.m_GfxGLMajor = GlewMajor; @@ -1339,6 +1349,7 @@ int CGraphicsBackend_SDL_GL::Init(const char *pName, int *pScreen, int *pWidth, if(m_GLContext) SDL_GL_DeleteContext(m_GLContext); SDL_DestroyWindow(m_pWindow); + m_pWindow = nullptr; // try setting to version string's supported version if(InitError == -2) @@ -1403,6 +1414,7 @@ int CGraphicsBackend_SDL_GL::Shutdown() if(m_GLContext != nullptr) SDL_GL_DeleteContext(m_GLContext); SDL_DestroyWindow(m_pWindow); + m_pWindow = nullptr; SDL_QuitSubSystem(SDL_INIT_VIDEO); return 0; diff --git a/src/engine/client/backend_sdl.h b/src/engine/client/backend_sdl.h index c6c9c54e9..e16250c2b 100644 --- a/src/engine/client/backend_sdl.h +++ b/src/engine/client/backend_sdl.h @@ -92,9 +92,6 @@ protected: return m_Warning.m_WarningType != GFX_WARNING_TYPE_NONE; } - // returns true if the error msg was shown - virtual bool TryCreateMsgBox(bool AsError, const char *pTitle, const char *pMsg) = 0; - private: ICommandProcessor *m_pProcessor; std::mutex m_BufferSwapMutex; @@ -243,9 +240,6 @@ class CGraphicsBackend_SDL_GL : public CGraphicsBackend_Threaded static EBackendType DetectBackend(); static void ClampDriverVersion(EBackendType BackendType); -protected: - bool TryCreateMsgBox(bool AsError, const char *pTitle, const char *pMsg) override; - public: CGraphicsBackend_SDL_GL(TTranslateFunc &&TranslateFunc); int Init(const char *pName, int *pScreen, int *pWidth, int *pHeight, int *pRefreshRate, int *pFsaaSamples, int Flags, int *pDesktopWidth, int *pDesktopHeight, int *pCurrentWidth, int *pCurrentHeight, class IStorage *pStorage) override; @@ -313,6 +307,8 @@ public: TGLBackendReadPresentedImageData &GetReadPresentedImageDataFuncUnsafe() override; + bool ShowMessageBox(unsigned Type, const char *pTitle, const char *pMsg) override; + static bool IsModernAPI(EBackendType BackendType); }; diff --git a/src/engine/client/client.cpp b/src/engine/client/client.cpp index fee56a174..01ebcb901 100644 --- a/src/engine/client/client.cpp +++ b/src/engine/client/client.cpp @@ -5005,5 +5005,6 @@ static Uint32 GetSdlMessageBoxFlags(IClient::EMessageBoxType Type) void CClient::ShowMessageBox(const char *pTitle, const char *pMessage, EMessageBoxType Type) { - SDL_ShowSimpleMessageBox(GetSdlMessageBoxFlags(Type), pTitle, pMessage, nullptr); + if(m_pGraphics == nullptr || !m_pGraphics->ShowMessageBox(GetSdlMessageBoxFlags(Type), pTitle, pMessage)) + SDL_ShowSimpleMessageBox(GetSdlMessageBoxFlags(Type), pTitle, pMessage, nullptr); } diff --git a/src/engine/client/graphics_threaded.cpp b/src/engine/client/graphics_threaded.cpp index 901ef68b0..f1277c52a 100644 --- a/src/engine/client/graphics_threaded.cpp +++ b/src/engine/client/graphics_threaded.cpp @@ -3279,6 +3279,13 @@ SWarning *CGraphics_Threaded::GetCurWarning() } } +bool CGraphics_Threaded::ShowMessageBox(unsigned Type, const char *pTitle, const char *pMsg) +{ + if(m_pBackend == nullptr) + return false; + return m_pBackend->ShowMessageBox(Type, pTitle, pMsg); +} + const char *CGraphics_Threaded::GetVendorString() { return m_pBackend->GetVendorString(); diff --git a/src/engine/client/graphics_threaded.h b/src/engine/client/graphics_threaded.h index 3705df48c..03bc57ff9 100644 --- a/src/engine/client/graphics_threaded.h +++ b/src/engine/client/graphics_threaded.h @@ -773,6 +773,9 @@ public: virtual TGLBackendReadPresentedImageData &GetReadPresentedImageDataFuncUnsafe() = 0; virtual bool GetWarning(std::vector &WarningStrings) = 0; + + // returns true if the error msg was shown + virtual bool ShowMessageBox(unsigned Type, const char *pTitle, const char *pMsg) = 0; }; class CGraphics_Threaded : public IEngineGraphics @@ -1296,6 +1299,7 @@ public: void WaitForIdle() override; SWarning *GetCurWarning() override; + bool ShowMessageBox(unsigned Type, const char *pTitle, const char *pMsg) override; bool GetDriverVersion(EGraphicsDriverAgeType DriverAgeType, int &Major, int &Minor, int &Patch, const char *&pName, EBackendType BackendType) override { return m_pBackend->GetDriverVersion(DriverAgeType, Major, Minor, Patch, pName, BackendType); } bool IsConfigModernAPI() override { return m_pBackend->IsConfigModernAPI(); } diff --git a/src/engine/graphics.h b/src/engine/graphics.h index 45ed38e3a..44f26ae00 100644 --- a/src/engine/graphics.h +++ b/src/engine/graphics.h @@ -521,6 +521,9 @@ public: virtual SWarning *GetCurWarning() = 0; + // returns true if the error msg was shown + virtual bool ShowMessageBox(unsigned Type, const char *pTitle, const char *pMsg) = 0; + protected: inline CTextureHandle CreateTextureHandle(int Index) {