From 797f88bf31d655a0448f0cfc4fca87b92f4e796e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= <hrydgard@gmail.com>
Date: Wed, 23 Aug 2023 19:03:33 +0200
Subject: [PATCH] Fix ordering problem with screenshots that was causing
 crashes.

Fixes #17781
---
 Common/UI/Screen.cpp | 13 ++++++++-----
 Core/Screenshot.cpp  |  5 +++--
 SDL/CocoaBarItems.mm |  6 +++---
 UI/NativeApp.cpp     | 11 +++++------
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/Common/UI/Screen.cpp b/Common/UI/Screen.cpp
index 036473794b..08ce0a31eb 100644
--- a/Common/UI/Screen.cpp
+++ b/Common/UI/Screen.cpp
@@ -178,12 +178,13 @@ void ScreenManager::render() {
 					iter++;
 				}
 				stack_.back().screen->render();
-				if (postRenderCb_) {
-					postRenderCb_(getUIContext(), postRenderUserdata_);
-				}
 				if (overlayScreen_) {
 					overlayScreen_->render();
 				}
+				if (postRenderCb_) {
+					// Really can't render anything after this! Will crash the screenshot mechanism if we do.
+					postRenderCb_(getUIContext(), postRenderUserdata_);
+				}
 				first->screen->postRender();
 				break;
 			}
@@ -191,11 +192,13 @@ void ScreenManager::render() {
 			_assert_(stack_.back().screen);
 			stack_.back().screen->preRender();
 			stack_.back().screen->render();
-			if (postRenderCb_)
-				postRenderCb_(getUIContext(), postRenderUserdata_);
 			if (overlayScreen_) {
 				overlayScreen_->render();
 			}
+			if (postRenderCb_) {
+				// Really can't render anything after this! Will crash the screenshot mechanism if we do.
+				postRenderCb_(getUIContext(), postRenderUserdata_);
+			}
 			stack_.back().screen->postRender();
 			break;
 		}
diff --git a/Core/Screenshot.cpp b/Core/Screenshot.cpp
index 492ffab35c..6bb52cab08 100644
--- a/Core/Screenshot.cpp
+++ b/Core/Screenshot.cpp
@@ -359,8 +359,8 @@ bool TakeGameScreenshot(const Path &filename, ScreenshotFormat fmt, ScreenshotTy
 		return false;
 	}
 
-	u8 *flipbuffer = nullptr;
 	if (success) {
+		u8 *flipbuffer = nullptr;
 		const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, w, h);
 		success = buffer != nullptr;
 		if (success) {
@@ -371,12 +371,13 @@ bool TakeGameScreenshot(const Path &filename, ScreenshotFormat fmt, ScreenshotTy
 
 			success = Save888RGBScreenshot(filename, fmt, buffer, w, h);
 		}
+		delete[] flipbuffer;
 	}
-	delete [] flipbuffer;
 
 	if (!success) {
 		ERROR_LOG(IO, "Failed to write screenshot.");
 	}
+
 	return success;
 }
 
diff --git a/SDL/CocoaBarItems.mm b/SDL/CocoaBarItems.mm
index e1bc11a398..95a6eca0e4 100644
--- a/SDL/CocoaBarItems.mm
+++ b/SDL/CocoaBarItems.mm
@@ -24,12 +24,12 @@
 #include "Common/Data/Text/I18n.h"
 #include "Common/StringUtils.h"
 
-void TakeScreenshot();
-
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+extern bool g_TakeScreenshot;
+
 #define MENU_ITEM(variableName, localizedTitleName, SEL, ConfigurationValueName, Tag) \
 NSMenuItem *variableName = [[NSMenuItem alloc] initWithTitle:localizedTitleName action:SEL keyEquivalent:@""]; \
 variableName.target = self; \
@@ -488,7 +488,7 @@ void OSXOpenURL(const char *url) {
 }
 
 -(void)takeScreenshot {
-    TakeScreenshot();
+    g_takeScreenshot = true;
 }
 
 -(void)resetSymbolTable {
diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp
index 9d5dfb802c..1eaeb1ebee 100644
--- a/UI/NativeApp.cpp
+++ b/UI/NativeApp.cpp
@@ -821,7 +821,7 @@ void NativeInit(int argc, const char *argv[], const char *savegame_dir, const ch
 	restarting = false;
 }
 
-void RenderOverlays(UIContext *dc, void *userdata);
+void CallbackPostRender(UIContext *dc, void *userdata);
 bool CreateGlobalPipelines();
 
 bool NativeInitGraphics(GraphicsContext *graphicsContext) {
@@ -860,7 +860,7 @@ bool NativeInitGraphics(GraphicsContext *graphicsContext) {
 
 	g_screenManager->setUIContext(uiContext);
 	g_screenManager->setDrawContext(g_draw);
-	g_screenManager->setPostRenderCallback(&RenderOverlays, nullptr);
+	g_screenManager->setPostRenderCallback(&CallbackPostRender, nullptr);
 	g_screenManager->deviceRestored();
 
 #ifdef _WIN32
@@ -993,9 +993,7 @@ void NativeShutdownGraphics() {
 	INFO_LOG(SYSTEM, "NativeShutdownGraphics done");
 }
 
-void TakeScreenshot() {
-	g_TakeScreenshot = false;
-
+static void TakeScreenshot() {
 	Path path = GetSysDirectory(DIRECTORY_SCREENSHOT);
 	if (!File::Exists(path)) {
 		File::CreateDir(path);
@@ -1027,9 +1025,10 @@ void TakeScreenshot() {
 	}
 }
 
-void RenderOverlays(UIContext *dc, void *userdata) {
+void CallbackPostRender(UIContext *dc, void *userdata) {
 	if (g_TakeScreenshot) {
 		TakeScreenshot();
+		g_TakeScreenshot = false;
 	}
 }