Merge #6570

6570: Only show assertion popup for assertions on main thread, destroy window before showing popup if graphics initialized  r=Jupeyy a=Robyt3



## Checklist

- [X] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [X] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>

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();
+	if(m_pProcessor != nullptr)
-	SDL_DestroyWindow(m_pWindow);
+		m_pProcessor->ErroneousCleanup();
-	SDL_ShowSimpleMessageBox(AsError ? SDL_MESSAGEBOX_ERROR : SDL_MESSAGEBOX_WARNING, pTitle, pMsg, nullptr);
+	// TODO: Remove this workaround when https://github.com/libsdl-org/SDL/issues/3750 is
-	return true;
+	// 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;

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);
 };
 

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");
@@ -5002,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);
 }

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();

src/engine/client/graphics_threaded.h

@@ -773,6 +773,9 @@ public:
 	virtual TGLBackendReadPresentedImageData &GetReadPresentedImageDataFuncUnsafe() = 0;
 
 	virtual bool GetWarning(std::vector<std::string> &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(); }

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)
 	{