Rework the internal screenshot API, fix bug causing screenshotting to get stuck

Core/Config.cpp

@@ -1891,8 +1891,8 @@ void PlayTimeTracker::Stop(std::string_view gameId) {
 		return;
 	}
 
-	// Shouldn't happen, ignore this case.
-	WARN_LOG(Log::System, "GameTimeTracker::Stop called without corresponding GameTimeTracker::Start");
+	// Can happen if boot gets cancelled. Not worth warn-logging.
+	DEBUG_LOG(Log::System, "GameTimeTracker::Stop called without corresponding GameTimeTracker::Start");
 }
 
 void PlayTimeTracker::Reset(std::string_view gameId) {

Core/SaveState.cpp

@@ -116,9 +116,6 @@ struct Operation {
 
 static std::vector<Operation> g_pendingOperations;
 
-// If this isn't empty, a screenshot operation is pending. It's protected by mutex.
-Path g_screenshotPath;
-
 int g_screenshotFailures;
 
 	CChunkFileReader::Error SaveToRam(std::vector<u8> &data) {
@@ -383,8 +380,26 @@ int g_screenshotFailures;
 
 	static void ScheduleSaveScreenshot(const Path &path) {
 		std::lock_guard<std::mutex> guard(mutex);
-		g_screenshotPath = path;
 		g_screenshotFailures = 0;
+
+		// Savestate thumbnails don't need to be bigger.
+		constexpr int maxResMultiplier = 2;
+		ScheduleScreenshot(path, ScreenshotFormat::JPG, ScreenshotType::Display, maxResMultiplier, [path](ScreenshotResult result) {
+			switch (result) {
+			case ScreenshotResult::ScreenshotNotPossible:
+				// Try again soon, for a short while.
+				WARN_LOG(Log::SaveState, "Failed to take a screenshot for the savestate! (%s) The savestate will lack an icon.", path.c_str());
+				if (coreState != CORE_STEPPING_CPU && g_screenshotFailures++ < SCREENSHOT_FAILURE_RETRIES) {
+					// Requeue for next frame (if we were stepping, no point, will just spam errors quickly).
+					ScheduleSaveScreenshot(path);
+				}
+				break;
+			case ScreenshotResult::FailedToWriteFile:
+			case ScreenshotResult::Success:
+				g_screenshotFailures = 0;
+				break;
+			}
+		});
 	}
 
 	void LoadSlot(std::string_view gamePrefix, int slot, Callback callback) {
@@ -973,43 +988,4 @@ int g_screenshotFailures;
 		}
 	}
 
-bool ProcessScreenshot(bool skipBufferEffects) {
-	Path screenshotPath;
-	{
-		std::lock_guard<std::mutex> guard(mutex);
-		if (!g_screenshotPath.empty()) {
-			screenshotPath = g_screenshotPath;
-			g_screenshotPath.clear();
-		} else {
-			return false;
-		}
-	}
-
-	// Savestate thumbnails don't need to be bigger.
-	constexpr int maxResMultiplier = 2;
-	ScreenshotResult tempResult = TakeGameScreenshot(nullptr, screenshotPath, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, maxResMultiplier, [](bool success) {
-		if (success) {
-			g_screenshotFailures = 0;
-		}
-	});
-
-	switch (tempResult) {
-	case ScreenshotResult::ScreenshotNotPossible:
-		// Try again soon, for a short while.
-		WARN_LOG(Log::SaveState, "Failed to take a screenshot for the savestate! (%s) The savestate will lack an icon.", g_screenshotPath.c_str());
-		if (coreState != CORE_STEPPING_CPU && g_screenshotFailures++ < SCREENSHOT_FAILURE_RETRIES) {
-			// Requeue for next frame (if we were stepping, no point, will just spam errors quickly).
-			ScheduleSaveScreenshot(g_screenshotPath);
-		}
-		break;
-	case ScreenshotResult::FailedToWriteFile:
-		break;
-	case ScreenshotResult::DelayedResult:
-		return true;
-	case ScreenshotResult::Success:
-		return true;
-	}
-	return false; // Didn't take a screenshot right now.
-}
-
 }  // namespace SaveState

Core/SaveState.h

@@ -112,10 +112,6 @@ namespace SaveState {
 	// Check if there's any save stating needing to be done.  Normally called once per frame.
 	void Process();
 
-	// Separate function to just process screenshots, as they need to be done at a specific time in a frame.
-	// Returns true if a screenshot was taken.
-	bool ProcessScreenshot(bool skipBufferEffects);
-
 	// Notify save state code that new save data has been written.
 	void NotifySaveData();
 

Core/Screenshot.cpp

@@ -29,16 +29,30 @@
 #include "Common/Log.h"
 #include "Common/System/System.h"
 #include "Common/System/Display.h"
+#include "Common/System/OSD.h"
+#include "Common/System/Request.h"
 #include "Common/System/NativeApp.h"
 #include "Common/Thread/Promise.h"
+#include "Common/StringUtils.h"
+#include "Common/Data/Text/I18n.h"
 #include "Core/Screenshot.h"
+#include "Core/ELF/ParamSFO.h"
 #include "Core/System.h"
+#include "Core/Core.h"
 #include "GPU/Common/GPUDebugInterface.h"
 #include "GPU/Common/FramebufferManagerCommon.h"
 #include "GPU/GPUCommon.h"
 
-// This is used to make non-ASCII paths work for filename.
-// Technically only needed on Windows.
+struct PendingScreenshot {
+	ScreenshotType type;
+	ScreenshotFormat format;
+	Path path;
+	std::function<void(ScreenshotResult result)> callback;
+	int maxRes;
+};
+
+PendingScreenshot g_pendingScreenshot;
+
 class JPEGFileStream : public jpge::output_stream {
 public:
 	JPEGFileStream(const Path &filename) {
@@ -49,22 +63,20 @@ public:
 			fclose(fp_);
 		}
 	}
-
-	bool put_buf(const void *buf, int len) override
-	{
-		if (fp_) {
-			if (fwrite(buf, len, 1, fp_) != 1) {
-				fclose(fp_);
-				fp_ = nullptr;
-			}
+	bool put_buf(const void *buf, int len) override {
+		if (!fp_) {
+			return false;
 		}
-		return Valid();
+		if (fwrite(buf, len, 1, fp_) != 1) {
+			fclose(fp_);
+			fp_ = nullptr;
+			return false;
+		}
+		return true;
 	}
-
 	bool Valid() {
 		return fp_ != nullptr;
 	}
-
 private:
 	FILE *fp_;
 };
@@ -88,7 +100,7 @@ static bool WriteScreenshotToJPEG(const Path &filename, int width, int height, i
 				return false;
 			}
 		}
-		if (!dst_image.process_scanline(NULL)) {
+		if (!dst_image.process_scanline(nullptr)) {
 			return false;
 		}
 	}
@@ -312,88 +324,124 @@ static GPUDebugBuffer ApplyRotation(const GPUDebugBuffer &buf, DisplayRotation r
 	return rotated;
 }
 
-ScreenshotResult TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes, std::function<void(bool success)> callback) {
-	GPUDebugBuffer buf;
-	u32 w = (u32)-1;
-	u32 h = (u32)-1;
+static void SaveScreenshotAsync(GPUDebugBuffer &&buf, int w, int h, int maxRes);
 
-	if (type == SCREENSHOT_DISPLAY || type == SCREENSHOT_RENDER) {
-		if (!gpuDebug) {
-			ERROR_LOG(Log::System, "Can't take screenshots when GPU not running");
-			return ScreenshotResult::ScreenshotNotPossible;
-		}
-		if (!gpuDebug->GetCurrentFramebuffer(buf, type == SCREENSHOT_RENDER ? GPU_DBG_FRAMEBUF_RENDER : GPU_DBG_FRAMEBUF_DISPLAY, maxRes)) {
-			return ScreenshotResult::ScreenshotNotPossible;
-		}
-		if (buf.IsBackBuffer()) {
-			w = buf.GetStride();
-			h = buf.GetHeight();
-		} else {
-			w = maxRes > 0 ? 480 * maxRes : buf.GetStride();
-			h = maxRes > 0 ? 272 * maxRes : buf.GetHeight();
-		}
-	} else if (g_display.rotation != DisplayRotation::ROTATE_0) {
+void ScheduleScreenshot(const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes, std::function<void(ScreenshotResult)> &&callback) {
+	g_pendingScreenshot = {};
+	g_pendingScreenshot.path = filename;
+	g_pendingScreenshot.format = fmt;
+	g_pendingScreenshot.type = type;
+	g_pendingScreenshot.callback = std::move(callback);
+	g_pendingScreenshot.maxRes = maxRes;
+}
+
+bool ScreenshotNotifyEndOfFrame(Draw::DrawContext *draw) {
+	if (g_pendingScreenshot.path.empty() || g_pendingScreenshot.type != ScreenshotType::Output) {
+		return false;
+	}
+
+	GPUDebugBuffer buf;
+	if (g_display.rotation != DisplayRotation::ROTATE_0) {
 		_dbg_assert_(draw);
 		GPUDebugBuffer temp;
 		if (!::GetOutputFramebuffer(draw, temp)) {
-			return ScreenshotResult::ScreenshotNotPossible;
+			g_pendingScreenshot.callback(ScreenshotResult::ScreenshotNotPossible);
+			g_pendingScreenshot = {};
+			return false;
 		}
 		buf = ApplyRotation(temp, g_display.rotation);
 	} else {
 		_dbg_assert_(draw);
 		if (!GetOutputFramebuffer(draw, buf)) {
-			return ScreenshotResult::ScreenshotNotPossible;
+			g_pendingScreenshot.callback(ScreenshotResult::ScreenshotNotPossible);
+			g_pendingScreenshot = {};
+			return false;
 		}
 	}
 
-	if (callback) {
-		g_threadManager.EnqueueTask(new IndependentTask(TaskType::IO_BLOCKING, TaskPriority::LOW,
-			[buf = std::move(buf), callback = std::move(callback), filename, fmt, w, h, maxRes]() {
-			u8 *flipbuffer = nullptr;
-			u32 width = w, height = h;
-			const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, width, height);
+	_dbg_assert_(buf.IsBackBuffer());
+	const int w = buf.GetStride();
+	const int h = buf.GetHeight();
+	SaveScreenshotAsync(std::move(buf), w, h, g_pendingScreenshot.maxRes);
+	return true;
+}
 
-			bool success;
-			if ((int)width <= 480 * maxRes) {
-				success = Save888RGBScreenshot(filename, fmt, buffer, width, height);
-				delete[] flipbuffer;
-			} else {
-				u8 *shrinkBuffer = new u8[width * height * 3];
-				memcpy(shrinkBuffer, buffer, width * height * 3);
-				delete[] flipbuffer;
+bool ScreenshotNotifyPostGameRender(Draw::DrawContext *draw) {
+	if (g_pendingScreenshot.path.empty() || g_pendingScreenshot.type == ScreenshotType::Output) {
+		return false;
+	}
 
-				// TODO: Speed this thing up.
-				while ((int)width > 480 * maxRes) {
-					u8 *halfSize = new u8[(width / 2) * (height / 2) * 3];
-					for (u32 y = 0; y < height / 2; y++) {
-						for (u32 x = 0; x < width / 2; x++) {
-							for (int c = 0; c < 3; c++) {
-								halfSize[(y * (width / 2) + x) * 3 + c] = (shrinkBuffer[((y * 2) * width + (x * 2)) * 3 + c] +
-										shrinkBuffer[((y * 2) * width + (x * 2 + 1)) * 3 + c] +
-										shrinkBuffer[(((y * 2) + 1) * width + (x * 2)) * 3 + c] +
-										shrinkBuffer[(((y * 2) + 1) * width + (x * 2 + 1)) * 3 + c]) / 4;
-							}
+	GPUDebugBuffer buf;
+	if (!gpuDebug) {
+		ERROR_LOG(Log::System, "Can't take screenshots when GPU not running");
+		g_pendingScreenshot.callback(ScreenshotResult::ScreenshotNotPossible);
+		g_pendingScreenshot = {};
+		return false;
+	}
+
+	const int maxRes = g_pendingScreenshot.maxRes;
+
+	if (!gpuDebug->GetCurrentFramebuffer(buf, g_pendingScreenshot.type == ScreenshotType::Render ? GPU_DBG_FRAMEBUF_RENDER : GPU_DBG_FRAMEBUF_DISPLAY, maxRes)) {
+		g_pendingScreenshot.callback(ScreenshotResult::ScreenshotNotPossible);
+		g_pendingScreenshot = {};
+		return false;
+	}
+
+	int w;
+	int h;
+	if (buf.IsBackBuffer()) {
+		w = buf.GetStride();
+		h = buf.GetHeight();
+	} else {
+		w = maxRes > 0 ? 480 * maxRes : buf.GetStride();
+		h = maxRes > 0 ? 272 * maxRes : buf.GetHeight();
+	}
+
+	SaveScreenshotAsync(std::move(buf), w, h, g_pendingScreenshot.maxRes);
+	return true;
+}
+
+static void SaveScreenshotAsync(GPUDebugBuffer &&buf, int w, int h, int maxRes) {
+	g_threadManager.EnqueueTask(new IndependentTask(TaskType::IO_BLOCKING, TaskPriority::LOW,
+		[filename = std::move(g_pendingScreenshot.path), buf = std::move(buf), callback = std::move(g_pendingScreenshot.callback), fmt = g_pendingScreenshot.format, w, h, maxRes]() {
+		u8 *flipbuffer = nullptr;
+		u32 width = w, height = h;
+		const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, width, height);
+
+		ScreenshotResult result;
+		if (maxRes <= 0 || (int)width <= 480 * maxRes) {
+			result = Save888RGBScreenshot(filename, fmt, buffer, width, height) ? ScreenshotResult::Success : ScreenshotResult::FailedToWriteFile;
+			delete[] flipbuffer;
+		} else {
+			u8 *shrinkBuffer = new u8[width * height * 3];
+			memcpy(shrinkBuffer, buffer, width * height * 3);
+			delete[] flipbuffer;
+
+			// TODO: Speed this thing up.
+			while ((int)width > 480 * maxRes) {
+				u8 *halfSize = new u8[(width / 2) * (height / 2) * 3];
+				for (u32 y = 0; y < height / 2; y++) {
+					for (u32 x = 0; x < width / 2; x++) {
+						for (int c = 0; c < 3; c++) {
+							halfSize[(y * (width / 2) + x) * 3 + c] = (shrinkBuffer[((y * 2) * width + (x * 2)) * 3 + c] +
+									shrinkBuffer[((y * 2) * width + (x * 2 + 1)) * 3 + c] +
+									shrinkBuffer[(((y * 2) + 1) * width + (x * 2)) * 3 + c] +
+									shrinkBuffer[(((y * 2) + 1) * width + (x * 2 + 1)) * 3 + c]) / 4;
 						}
 					}
-					std::swap(shrinkBuffer, halfSize);
-					delete[] halfSize;
-					width /= 2;
-					height /= 2;
 				}
-				success = Save888RGBScreenshot(filename, fmt, shrinkBuffer, width, height);
+				std::swap(shrinkBuffer, halfSize);
+				delete[] halfSize;
+				width /= 2;
+				height /= 2;
 			}
+			result = Save888RGBScreenshot(filename, fmt, shrinkBuffer, width, height) ? ScreenshotResult::Success : ScreenshotResult::FailedToWriteFile;
+		}
 
-			System_RunOnMainThread([success, callback = std::move(callback)]() {
-				callback(success);
-			});
-		}));
-		return ScreenshotResult::DelayedResult;
-	}
-	u8 *flipbuffer = nullptr;
-	const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, w, h);
-	bool success = Save888RGBScreenshot(filename, fmt, buffer, w, h);
-	delete[] flipbuffer;
-	return success ? ScreenshotResult::Success : ScreenshotResult::FailedToWriteFile;
+		System_RunOnMainThread([result, callback = std::move(callback)]() {
+			callback(result);
+		});
+	}));
 }
 
 bool Save888RGBScreenshot(const Path &filename, ScreenshotFormat fmt, const u8 *bufferRGB888, int w, int h) {
@@ -438,3 +486,71 @@ bool Save8888RGBAScreenshot(std::vector<uint8_t> &bufferPNG, const u8 *bufferRGB
 	}
 	return success;
 }
+
+void TakeUserScreenshotImpl() {
+	Path path = GetSysDirectory(DIRECTORY_SCREENSHOT);
+	// Make sure the screenshot directory exists.
+	File::CreateDir(path);
+
+	// First, find a free filename.
+	//
+	// NOTE: On Android, the old approach of checking filenames one by one doesn't scale.
+	// So let's just grab the full file listing, and then find a name that's not in it.
+	//
+	// TODO: Also, we could do this on a thread too. Not sure if worth it.
+
+	const std::string gameId = g_paramSFO.GetDiscID();
+
+	std::vector<File::FileInfo> files;
+	const std::string prefix = gameId + "_";
+	File::GetFilesInDir(path, &files, nullptr, 0, prefix);
+	std::set<std::string> existingNames;
+	for (auto &file : files) {
+		existingNames.insert(file.name);
+	}
+
+	Path filename;
+	int i = 0;
+	for (int i = 0; i < 20000; i++) {
+		const std::string pngName = prefix + StringFromFormat("%05d.png", i);
+		const std::string jpgName = prefix + StringFromFormat("%05d.jpg", i);
+		if (existingNames.find(pngName) == existingNames.end() && existingNames.find(jpgName) == existingNames.end()) {
+			filename = path / (g_Config.bScreenshotsAsPNG ? pngName : jpgName);
+			break;
+		}
+	}
+
+	if (filename.empty()) {
+		// Overwrite this one over and over.
+		filename = path / (prefix + (g_Config.bScreenshotsAsPNG ? "20000.png" : "20000.jpg"));
+	}
+
+	ScreenshotType type = g_Config.iScreenshotMode == (int)ScreenshotMode::GameImage ? ScreenshotType::Display : ScreenshotType::Output;
+
+	if (GetUIState() != UISTATE_INGAME) {
+		// We're out in the UI, no game. The only type of screenshot available is output.
+		type = ScreenshotType::Output;
+	}
+
+	ScheduleScreenshot(filename, g_Config.bScreenshotsAsPNG ? ScreenshotFormat::PNG : ScreenshotFormat::JPG, type, -1, [filename](ScreenshotResult result) {
+		if (result == ScreenshotResult::Success) {
+			g_OSD.Show(OSDType::MESSAGE_FILE_LINK, filename.ToVisualString(), 0.0f, "screenshot_link");
+			if (System_GetPropertyBool(SYSPROP_CAN_SHOW_FILE)) {
+				g_OSD.SetClickCallback("screenshot_link", [filename]() -> void {
+					System_ShowFileInFolder(filename);
+				});
+			}
+		} else if (result == ScreenshotResult::FailedToWriteFile) {
+			auto err = GetI18NCategory(I18NCat::ERRORS);
+			g_OSD.Show(OSDType::MESSAGE_ERROR, err->T("Could not save screenshot file"));
+			WARN_LOG(Log::System, "Failed to take screenshot.");
+		}
+		// TODO: What to do about ScreenshotNotPossible?
+	});
+}
+
+void TakeUserScreenshot() {
+	System_RunOnMainThread([]() {
+		TakeUserScreenshotImpl();
+	});
+}

Core/Screenshot.h

@@ -33,19 +33,17 @@ enum class ScreenshotFormat {
 
 // NOTE: The first two may need rotation, depending on the backend and screen orientation.
 // This is handled internally in TakeGameScreenshot().
-enum ScreenshotType {
-	// What's being show on screen (e.g. including FPS, etc.)
-	SCREENSHOT_OUTPUT,
-	// What the game rendered (e.g. at render resolution) to the display.
-	SCREENSHOT_DISPLAY,
-	// What the game is in-progress rendering now.
-	SCREENSHOT_RENDER,
+enum class ScreenshotType {
+	// What's being shown on the screen of the host device (e.g. including FPS, etc.)
+	Output,
+	// What the game has rendered and is presenting (e.g. at render resolution) to the display.
+	Display,
+	// What the game is in-progress rendering now. Should only be used by the debugger.
+	Render,
 };
 
 enum class ScreenshotResult {
 	ScreenshotNotPossible,
-	DelayedResult,  // This specifies that the actual result is one of the two below and will arrive in the callback.
-	// These result can be delayed and arrive in the callback, if one is specified.
 	FailedToWriteFile,
 	Success,
 };
@@ -54,9 +52,16 @@ const u8 *ConvertBufferToScreenshot(const GPUDebugBuffer &buf, bool alpha, u8 *&
 
 // Can only be used while in game.
 // If the callback is passed in, the saving action happens on a background thread.
-ScreenshotResult TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes = -1, std::function<void(bool success)> callback = nullptr);
+void ScheduleScreenshot(const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes, std::function<void(ScreenshotResult)> &&callback);
+
+bool ScreenshotNotifyPostGameRender(Draw::DrawContext *draw);
+bool ScreenshotNotifyEndOfFrame(Draw::DrawContext *draw);
 
 bool Save888RGBScreenshot(const Path &filename, ScreenshotFormat fmt, const u8 *bufferRGB888, int w, int h);
 bool Save8888RGBAScreenshot(const Path &filename, const u8 *bufferRGBA8888, int w, int h);
 // Overallocate bufferPNG for better encoding speed.
 bool Save8888RGBAScreenshot(std::vector<uint8_t> &bufferPNG, const u8 *bufferRGBA8888, int w, int h);
+
+// Handles generating filename etc.
+// Can be called from any thread, but will run the screenshot code on the main thread.
+void TakeUserScreenshot();

GPU/Common/FramebufferManagerCommon.cpp

@@ -3137,6 +3137,7 @@ bool GetOutputFramebuffer(Draw::DrawContext *draw, GPUDebugBuffer &buffer) {
 	bool flipped = g_Config.iGPUBackend == (int)GPUBackend::OPENGL;
 
 	buffer.Allocate(w, h, fmt == Draw::DataFormat::R8G8B8A8_UNORM ? GPU_DBG_FORMAT_8888 : GPU_DBG_FORMAT_8888_BGRA, flipped);
+	buffer.SetIsBackbuffer(true);
 	return draw->CopyFramebufferToMemory(nullptr, Draw::Aspect::COLOR_BIT, 0, 0, w, h, fmt, buffer.GetData(), w, Draw::ReadbackMode::BLOCK, "GetOutputFramebuffer");
 }
 

Qt/mainwindow.cpp

@@ -19,6 +19,7 @@
 #include "Core/HLE/sceUmd.h"
 #include "Core/SaveState.h"
 #include "Core/System.h"
+#include "Core/Screenshot.h"
 #include "GPU/GPUCommon.h"
 #include "UI/GamepadEmu.h"
 
@@ -519,6 +520,10 @@ void MainWindow::loadLanguage(const QString& language, bool translate)
 	}
 }
 
+void MainWindow::takeScreen() {
+	TakeUserScreenshot();
+}
+
 void MainWindow::createMenus()
 {
 	// File

Qt/mainwindow.h

@@ -21,8 +21,6 @@
 #include "Core/System.h"
 #include "Qt/QtMain.h"
 
-extern bool g_TakeScreenshot;
-
 class MenuAction;
 class MenuTree;
 
@@ -116,7 +114,7 @@ private slots:
 	void ssymAct();
 	void resetTableAct();
 	void dumpNextAct();
-	void takeScreen() { g_TakeScreenshot = true; }
+	void takeScreen();
 	void consoleAct();
 
 	// Game settings

UI/EmuScreen.cpp

@@ -75,6 +75,7 @@ using namespace std::placeholders;
 #include "Core/Debugger/SymbolMap.h"
 #include "Core/RetroAchievements.h"
 #include "Core/SaveState.h"
+#include "Core/Screenshot.h"
 #include "UI/ImDebugger/ImDebugger.h"
 #include "Core/HLE/__sceAudio.h"
 // #include "Core/HLE/proAdhoc.h"
@@ -863,8 +864,7 @@ void EmuScreen::OnVKey(VirtKey virtualKeyCode, bool down) {
 		}
 		break;
 	case VIRTKEY_SCREENSHOT:
-		if (down)
-			g_TakeScreenshot = true;
+		TakeUserScreenshot();
 		break;
 	case VIRTKEY_RAPID_FIRE:
 		__CtrlSetRapidFire(down, g_Config.iRapidFireInterval);
@@ -1792,7 +1792,7 @@ ScreenRenderFlags EmuScreen::RunEmulation(bool skipBufferEffects) {
 			gpu->EndHostFrame();
 
 			// The right time to run this
-			if (SaveState::ProcessScreenshot(skipBufferEffects) && skipBufferEffects) {
+			if (ScreenshotNotifyPostGameRender(draw) && skipBufferEffects) {
 				// Need to restore the backbuffer render pass, it probably got destroyed.
 				draw->BindFramebufferAsRenderTarget(nullptr, {RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR}, "BackBuffer");
 			}

UI/EmuScreen.h

@@ -36,7 +36,7 @@ struct AxisInput;
 class AsyncImageFileView;
 class ChatMenu;
 
-class EmuScreen : public UIScreen, public ControlListener {
+class EmuScreen : public UIScreen, protected ControlListener {
 public:
 	EmuScreen(const Path &filename);
 	~EmuScreen();

UI/NativeApp.cpp

@@ -968,69 +968,8 @@ void NativeShutdownGraphics() {
 	INFO_LOG(Log::System, "NativeShutdownGraphics end");
 }
 
-static void TakeScreenshot(Draw::DrawContext *draw) {
-	Path path = GetSysDirectory(DIRECTORY_SCREENSHOT);
-	if (!File::Exists(path)) {
-		File::CreateDir(path);
-	}
-
-	// First, find a free filename.
-	//
-	// NOTE: On Android, the old approach of checking filenames one by one doesn't scale.
-	// So let's just grab the full file listing, and then find a name that's not in it.
-	//
-	// TODO: Also, we could do this on a thread too. Not sure if worth it.
-
-	const std::string gameId = g_paramSFO.GetDiscID();
-
-	// TODO: Make something like IterateFileInDir instead.
-	std::vector<File::FileInfo> files;
-	const std::string prefix = gameId + "_";
-	File::GetFilesInDir(path, &files, nullptr, 0, prefix);
-	std::set<std::string> existingNames;
-	for (auto &file : files) {
-		existingNames.insert(file.name);
-	}
-
-	Path filename;
-	int i = 0;
-	for (int i = 0; i < 20000; i++) {
-		const std::string pngName = prefix + StringFromFormat("%05d.png", i);
-		const std::string jpgName = prefix + StringFromFormat("%05d.jpg", i);
-		if (existingNames.find(pngName) == existingNames.end() && existingNames.find(jpgName) == existingNames.end()) {
-			filename = path / (g_Config.bScreenshotsAsPNG ? pngName : jpgName);
-			break;
-		}
-	}
-
-	if (filename.empty()) {
-		// Overwrite this one over and over.
-		filename = path / (prefix + (g_Config.bScreenshotsAsPNG ? "20000.png" : "20000.jpg"));
-	}
-
-	const ScreenshotType type = g_Config.iScreenshotMode == (int)ScreenshotMode::GameImage ? SCREENSHOT_DISPLAY : SCREENSHOT_OUTPUT;
-
-	const ScreenshotResult result = TakeGameScreenshot(draw, filename, g_Config.bScreenshotsAsPNG ? ScreenshotFormat::PNG : ScreenshotFormat::JPG, type, -1, [filename](bool success) {
-		if (success) {
-			g_OSD.Show(OSDType::MESSAGE_FILE_LINK, filename.ToVisualString(), 0.0f, "screenshot_link");
-			if (System_GetPropertyBool(SYSPROP_CAN_SHOW_FILE)) {
-				g_OSD.SetClickCallback("screenshot_link", [filename]() -> void {
-					System_ShowFileInFolder(filename);
-				});
-			}
-		} else {
-			auto err = GetI18NCategory(I18NCat::ERRORS);
-			g_OSD.Show(OSDType::MESSAGE_ERROR, err->T("Could not save screenshot file"));
-			WARN_LOG(Log::System, "Failed to take screenshot.");
-		}
-	});
-}
-
 void CallbackPostRender(UIContext *dc, void *userdata) {
-	if (g_TakeScreenshot) {
-		TakeScreenshot(dc->GetDrawContext());
-		g_TakeScreenshot = false;
-	}
+	ScreenshotNotifyEndOfFrame(dc->GetDrawContext());
 }
 
 static void SendMouseDeltaAxis();

UI/ReportScreen.cpp

@@ -169,14 +169,14 @@ ReportScreen::ReportScreen(const Path &gamePath)
 ScreenRenderFlags ReportScreen::PreRender(ScreenRenderMode mode) {
 	if ((mode & ScreenRenderMode::TOP) && !tookScreenshot_ && !g_Config.bSkipBufferEffects) {
 		// We do this in PreRender because we need it to be before the main render pass.
-		// We could do it mid frame, but then we have to reapply viewport/scissor.
+		// We could do it mid-frame, but then we have to reapply viewport/scissor.
 		Path path = GetSysDirectory(DIRECTORY_SCREENSHOT);
 		if (!File::Exists(path)) {
 			File::CreateDir(path);
 		}
 		screenshotFilename_ = path / ".reporting.jpg";
-		ScreenshotResult ignored = TakeGameScreenshot(screenManager()->getDrawContext(), screenshotFilename_, ScreenshotFormat::JPG, SCREENSHOT_RENDER, 4, [this](bool success) {
-			if (success) {
+		ScheduleScreenshot(screenshotFilename_, ScreenshotFormat::JPG, ScreenshotType::Display, 4, [this](ScreenshotResult result) {
+			if (result == ScreenshotResult::Success) {
 				// Redo the views already, now with a screenshot included.
 				RecreateViews();
 			} else {

Windows/MainWindowMenu.cpp

@@ -30,6 +30,7 @@
 #include "Core/ConfigValues.h"
 #include "Core/FileSystems/MetaFileSystem.h"
 #include "Core/KeyMap.h"
+#include "Core/Screenshot.h"
 #include "Windows/MainWindowMenu.h"
 #include "Windows/MainWindow.h"
 #include "Windows/W32Util/DialogManager.h"
@@ -49,8 +50,6 @@
 #include "ext/rcheevos/include/rc_client_raintegration.h"
 #endif
 
-extern bool g_TakeScreenshot;
-
 namespace MainWindow {
 	extern bool noFocusPause;
 	std::vector<HMENU> g_topLevelMenus;
@@ -955,7 +954,7 @@ namespace MainWindow {
 			break;
 
 		case ID_DEBUG_TAKESCREENSHOT:
-			g_TakeScreenshot = true;
+			TakeUserScreenshot();
 			break;
 
 		case ID_DEBUG_RESTARTGRAPHICS: