From cb1df4056cafc4ba774b464e36d9bf87b8134afa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 20 Mar 2025 17:21:20 +0100 Subject: [PATCH 1/5] Remove almost-empty files ThreadPools.cpp/h --- CMakeLists.txt | 2 -- Common/Thread/ThreadManager.cpp | 2 ++ Core/Core.vcxproj | 2 -- Core/Core.vcxproj.filters | 6 ------ Core/HLE/sceKernelMemory.cpp | 2 +- Core/ThreadPools.cpp | 3 --- Core/ThreadPools.h | 5 ----- GPU/Debugger/Record.cpp | 2 +- UI/NativeApp.cpp | 2 +- UWP/CoreUWP/CoreUWP.vcxproj | 4 +--- UWP/CoreUWP/CoreUWP.vcxproj.filters | 4 +--- android/jni/Android.mk | 1 - libretro/Makefile.common | 1 - 13 files changed, 7 insertions(+), 29 deletions(-) delete mode 100644 Core/ThreadPools.cpp delete mode 100644 Core/ThreadPools.h diff --git a/CMakeLists.txt b/CMakeLists.txt index bcd937741a..0a134fcf70 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2411,8 +2411,6 @@ add_library(${CoreLibName} ${CoreLinkType} Core/Screenshot.h Core/System.cpp Core/System.h - Core/ThreadPools.cpp - Core/ThreadPools.h Core/Util/AtracTrack.cpp Core/Util/AtracTrack.h Core/Util/AudioFormat.cpp diff --git a/Common/Thread/ThreadManager.cpp b/Common/Thread/ThreadManager.cpp index 547f13f75f..6153329115 100644 --- a/Common/Thread/ThreadManager.cpp +++ b/Common/Thread/ThreadManager.cpp @@ -24,6 +24,8 @@ const int MAX_CORES_TO_USE = 16; const int MIN_IO_BLOCKING_THREADS = 4; static constexpr size_t TASK_PRIORITY_COUNT = (size_t)TaskPriority::COUNT; +ThreadManager g_threadManager; + struct GlobalThreadContext { std::mutex mutex; std::deque compute_queue[TASK_PRIORITY_COUNT]; diff --git a/Core/Core.vcxproj b/Core/Core.vcxproj index c281c4ba59..fd23e717c5 100644 --- a/Core/Core.vcxproj +++ b/Core/Core.vcxproj @@ -1084,7 +1084,6 @@ - @@ -1470,7 +1469,6 @@ - diff --git a/Core/Core.vcxproj.filters b/Core/Core.vcxproj.filters index eb4f5f6c57..e3576f61a2 100644 --- a/Core/Core.vcxproj.filters +++ b/Core/Core.vcxproj.filters @@ -952,9 +952,6 @@ Core - - Core - Debugger\WebSocket @@ -2034,9 +2031,6 @@ Core - - Core - Debugger\WebSocket diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index 1b49e0c11c..55269af1a5 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -22,6 +22,7 @@ #include #include "Common/Thread/ParallelLoop.h" +#include "Common/Thread/ThreadManager.h" #include "Core/CoreTiming.h" #include "Core/Debugger/MemBlockInfo.h" #include "Core/HLE/HLE.h" @@ -30,7 +31,6 @@ #include "Core/MemMapHelpers.h" #include "Core/Reporting.h" #include "Core/System.h" -#include "Core/ThreadPools.h" #include "Common/Serialize/Serializer.h" #include "Common/Serialize/SerializeFuncs.h" #include "Common/Serialize/SerializeMap.h" diff --git a/Core/ThreadPools.cpp b/Core/ThreadPools.cpp deleted file mode 100644 index 103856020a..0000000000 --- a/Core/ThreadPools.cpp +++ /dev/null @@ -1,3 +0,0 @@ -#include "ThreadPools.h" - -ThreadManager g_threadManager; diff --git a/Core/ThreadPools.h b/Core/ThreadPools.h deleted file mode 100644 index efde2b9f47..0000000000 --- a/Core/ThreadPools.h +++ /dev/null @@ -1,5 +0,0 @@ -#pragma once - -#include "Common/Thread/ThreadManager.h" - -extern ThreadManager g_threadManager; diff --git a/GPU/Debugger/Record.cpp b/GPU/Debugger/Record.cpp index 67d03452c6..d36b0883b2 100644 --- a/GPU/Debugger/Record.cpp +++ b/GPU/Debugger/Record.cpp @@ -26,6 +26,7 @@ #include "Common/CommonTypes.h" #include "Common/File/FileUtil.h" +#include "Common/Thread/ThreadManager.h" #include "Common/Thread/ParallelLoop.h" #include "Common/Log.h" #include "Common/StringUtils.h" @@ -36,7 +37,6 @@ #include "Core/HLE/sceDisplay.h" #include "Core/MemMap.h" #include "Core/System.h" -#include "Core/ThreadPools.h" #include "GPU/Common/GPUDebugInterface.h" #include "GPU/GPUCommon.h" #include "GPU/GPUState.h" diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 65d12514bd..959de71620 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -84,6 +84,7 @@ #include "Common/OSVersion.h" #include "Common/GPU/ShaderTranslation.h" #include "Common/VR/PPSSPPVR.h" +#include "Common/Thread/ThreadManager.h" #include "Core/ControlMapper.h" #include "Core/Config.h" @@ -109,7 +110,6 @@ #include "Core/Util/AudioFormat.h" #include "Core/WebServer.h" #include "Core/TiltEventProcessor.h" -#include "Core/ThreadPools.h" #include "GPU/GPUCommon.h" #include "GPU/Common/PresentationCommon.h" diff --git a/UWP/CoreUWP/CoreUWP.vcxproj b/UWP/CoreUWP/CoreUWP.vcxproj index c88ae1961d..4dd08eb4c7 100644 --- a/UWP/CoreUWP/CoreUWP.vcxproj +++ b/UWP/CoreUWP/CoreUWP.vcxproj @@ -331,7 +331,6 @@ - @@ -631,7 +630,6 @@ - @@ -1042,4 +1040,4 @@ - \ No newline at end of file + diff --git a/UWP/CoreUWP/CoreUWP.vcxproj.filters b/UWP/CoreUWP/CoreUWP.vcxproj.filters index 8426b68001..8ccae0e868 100644 --- a/UWP/CoreUWP/CoreUWP.vcxproj.filters +++ b/UWP/CoreUWP/CoreUWP.vcxproj.filters @@ -1191,7 +1191,6 @@ Ext\libzip - @@ -1912,7 +1911,6 @@ Ext\jpge - @@ -1966,4 +1964,4 @@ Ext\gason - \ No newline at end of file + diff --git a/android/jni/Android.mk b/android/jni/Android.mk index 405e2345eb..c18a9aa3e7 100644 --- a/android/jni/Android.mk +++ b/android/jni/Android.mk @@ -619,7 +619,6 @@ EXEC_AND_LIB_FILES := \ $(SRC)/Core/Screenshot.cpp \ $(SRC)/Core/System.cpp \ $(SRC)/Core/TiltEventProcessor.cpp \ - $(SRC)/Core/ThreadPools.cpp \ $(SRC)/Core/WebServer.cpp \ $(SRC)/Core/Debugger/Breakpoints.cpp \ $(SRC)/Core/Debugger/DisassemblyManager.cpp \ diff --git a/libretro/Makefile.common b/libretro/Makefile.common index 4c4d836116..977dc8202e 100644 --- a/libretro/Makefile.common +++ b/libretro/Makefile.common @@ -825,7 +825,6 @@ SOURCES_CXX += \ $(COREDIR)/SaveState.cpp \ $(COREDIR)/Screenshot.cpp \ $(COREDIR)/System.cpp \ - $(COREDIR)/ThreadPools.cpp \ $(COREDIR)/Util/AtracTrack.cpp \ $(COREDIR)/Util/BlockAllocator.cpp \ $(COREDIR)/Util/MemStick.cpp \ From ae8f8c4abd12e6fe553fb8867c94b178956737a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 20 Mar 2025 17:41:44 +0100 Subject: [PATCH 2/5] Refactor TakeGameScreenshot a bit. --- Core/HLE/sceKernelModule.cpp | 2 +- Core/SaveState.cpp | 2 +- Core/Screenshot.cpp | 44 +++++++++++++--------------------- Core/Screenshot.h | 2 +- GPU/Common/GPUDebugInterface.h | 8 +++---- GPU/Software/SoftGpu.cpp | 8 +++---- UI/ReportScreen.cpp | 2 +- 7 files changed, 28 insertions(+), 40 deletions(-) diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index b27fd27da5..551e8e2143 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -483,7 +483,7 @@ void PSPModule::GetLongInfo(char *ptr, int bufSize) const { StringWriter w(ptr, bufSize); w.F("%s: Version %d.%d. %d segments", nm.name, nm.version[1], nm.version[0], nm.nsegment).endl(); w.F("Memory block: %08x (%08x/%d bytes)", memoryBlockAddr, memoryBlockSize, memoryBlockSize).endl(); - for (int i = 0; i < nm.nsegment; i++) { + for (int i = 0; i < (int)nm.nsegment; i++) { w.F(" %08x (%08x bytes)\n", nm.segmentaddr[i], nm.segmentsize[i]); } w.F("Text: %08x (%08x bytes)\n", nm.text_addr, nm.text_size); diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 4232ad56f3..50e07d2273 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -1094,7 +1094,7 @@ double g_lastSaveTime = -1.0; case SAVESTATE_SAVE_SCREENSHOT: { int maxResMultiplier = 2; - tempResult = TakeGameScreenshot(nullptr, op.filename, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, nullptr, nullptr, maxResMultiplier); + tempResult = TakeGameScreenshot(nullptr, op.filename, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, maxResMultiplier); callbackResult = tempResult ? Status::SUCCESS : Status::FAILURE; if (!tempResult) { WARN_LOG(Log::SaveState, "Failed to take a screenshot for the savestate! (%s) The savestate will lack an icon.", op.filename.c_str()); diff --git a/Core/Screenshot.cpp b/Core/Screenshot.cpp index 72838eb7ec..aaa17e41ac 100644 --- a/Core/Screenshot.cpp +++ b/Core/Screenshot.cpp @@ -328,9 +328,8 @@ static GPUDebugBuffer ApplyRotation(const GPUDebugBuffer &buf, DisplayRotation r return rotated; } -bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int *width, int *height, int maxRes) { +bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes) { GPUDebugBuffer buf; - bool success = false; u32 w = (u32)-1; u32 h = (u32)-1; @@ -339,45 +338,34 @@ bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, Screensho ERROR_LOG(Log::System, "Can't take screenshots when GPU not running"); return false; } - success = gpuDebug->GetCurrentFramebuffer(buf, type == SCREENSHOT_RENDER ? GPU_DBG_FRAMEBUF_RENDER : GPU_DBG_FRAMEBUF_DISPLAY, maxRes); + if (!gpuDebug->GetCurrentFramebuffer(buf, type == SCREENSHOT_RENDER ? GPU_DBG_FRAMEBUF_RENDER : GPU_DBG_FRAMEBUF_DISPLAY, maxRes)) { + return false; + } w = maxRes > 0 ? 480 * maxRes : PSP_CoreParameter().renderWidth; h = maxRes > 0 ? 272 * maxRes : PSP_CoreParameter().renderHeight; } else if (g_display.rotation != DisplayRotation::ROTATE_0) { _dbg_assert_(draw); GPUDebugBuffer temp; - success = ::GetOutputFramebuffer(draw, temp); - if (success) { - buf = ApplyRotation(temp, g_display.rotation); + if (!::GetOutputFramebuffer(draw, temp)) { + return false; } + buf = ApplyRotation(temp, g_display.rotation); } else { _dbg_assert_(draw); - success = ::GetOutputFramebuffer(draw, buf); + if (!GetOutputFramebuffer(draw, buf)) { + return false; + } } - if (!success) { + u8 *flipbuffer = nullptr; + const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, w, h); + if (!buffer) { return false; } - if (success) { - u8 *flipbuffer = nullptr; - const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, w, h); - success = buffer != nullptr; - if (success) { - if (width) - *width = w; - if (height) - *height = h; - - success = Save888RGBScreenshot(filename, fmt, buffer, w, h); - } - delete[] flipbuffer; - } - - if (!success) { - ERROR_LOG(Log::IO, "Failed to write screenshot."); - } - - return success; + bool success = Save888RGBScreenshot(filename, fmt, buffer, w, h); + delete[] flipbuffer; + return true; } bool Save888RGBScreenshot(const Path &filename, ScreenshotFormat fmt, const u8 *bufferRGB888, int w, int h) { diff --git a/Core/Screenshot.h b/Core/Screenshot.h index 69794bb6de..6e15c0bba2 100644 --- a/Core/Screenshot.h +++ b/Core/Screenshot.h @@ -41,7 +41,7 @@ enum ScreenshotType { const u8 *ConvertBufferToScreenshot(const GPUDebugBuffer &buf, bool alpha, u8 *&temp, u32 &w, u32 &h); // Can only be used while in game. -bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int *width = nullptr, int *height = nullptr, int maxRes = -1); +bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes = -1); 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); diff --git a/GPU/Common/GPUDebugInterface.h b/GPU/Common/GPUDebugInterface.h index 378ac361f4..2b3e437cfa 100644 --- a/GPU/Common/GPUDebugInterface.h +++ b/GPU/Common/GPUDebugInterface.h @@ -153,13 +153,13 @@ struct GPUDebugBuffer { void ZeroBytes(); - u8 *GetData() { - return data_; - } - u32 GetRawPixel(int x, int y) const; void SetRawPixel(int x, int y, u32 c); + u8 *GetDataWrite() { + return data_; + } + const u8 *GetData() const { return data_; } diff --git a/GPU/Software/SoftGpu.cpp b/GPU/Software/SoftGpu.cpp index fc6c1bb036..64f8d2f905 100644 --- a/GPU/Software/SoftGpu.cpp +++ b/GPU/Software/SoftGpu.cpp @@ -1384,7 +1384,7 @@ bool SoftGPU::GetCurrentFramebuffer(GPUDebugBuffer &buffer, GPUDebugFramebufferT buffer.Allocate(size.x, size.y, fmt); const int depth = fmt == GE_FORMAT_8888 ? 4 : 2; - u8 *dst = buffer.GetData(); + u8 *dst = buffer.GetDataWrite(); const int byteWidth = size.x * depth; for (int16_t y = 0; y < size.y; ++y) { memcpy(dst, src, byteWidth); @@ -1404,7 +1404,7 @@ bool SoftGPU::GetCurrentDepthbuffer(GPUDebugBuffer &buffer) { const int depth = 2; const u8 *src = depthbuf.data; - u8 *dst = buffer.GetData(); + u8 *dst = buffer.GetDataWrite(); for (int16_t y = 0; y < size.y; ++y) { memcpy(dst, src, size.x * depth); dst += size.x * depth; @@ -1430,7 +1430,7 @@ bool SoftGPU::GetCurrentStencilbuffer(GPUDebugBuffer &buffer) { DrawingCoords size = GetTargetSize(gstate.FrameBufStride()); buffer.Allocate(size.x, size.y, GPU_DBG_FORMAT_8BIT); - u8 *row = buffer.GetData(); + u8 *row = buffer.GetDataWrite(); for (int16_t y = 0; y < size.y; ++y) { for (int16_t x = 0; x < size.x; ++x) { row[x] = GetPixelStencil(gstate.FrameBufFormat(), gstate.FrameBufStride(), x, y); @@ -1451,7 +1451,7 @@ bool SoftGPU::GetCurrentClut(GPUDebugBuffer &buffer) const u32 pixels = 1024 / bpp; buffer.Allocate(pixels, 1, (GEBufferFormat)gstate.getClutPaletteFormat()); - memcpy(buffer.GetData(), clut, 1024); + memcpy(buffer.GetDataWrite(), clut, 1024); return true; } diff --git a/UI/ReportScreen.cpp b/UI/ReportScreen.cpp index 238c057174..425dc0ce2e 100644 --- a/UI/ReportScreen.cpp +++ b/UI/ReportScreen.cpp @@ -182,7 +182,7 @@ ScreenRenderFlags ReportScreen::render(ScreenRenderMode mode) { File::CreateDir(path); } screenshotFilename_ = path / ".reporting.jpg"; - if (TakeGameScreenshot(screenManager()->getDrawContext(), screenshotFilename_, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, nullptr, nullptr, 4)) { + if (TakeGameScreenshot(screenManager()->getDrawContext(), screenshotFilename_, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, 4)) { // Redo the views already, now with a screenshot included. RecreateViews(); } else { From 14ee85139da4a40623f1a59fb472fcf440e2a5d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 20 Mar 2025 19:41:10 +0100 Subject: [PATCH 3/5] Remove unused ability to call callbacks after taking a savestate screenshot --- Core/SaveState.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 50e07d2273..465dca2cdd 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -466,9 +466,9 @@ double g_lastSaveTime = -1.0; Enqueue(Operation(SAVESTATE_REWIND, Path(), -1, callback, cbUserData)); } - void SaveScreenshot(const Path &filename, Callback callback, void *cbUserData) { + static void SaveScreenshot(const Path &filename) { screenshotFailures = 0; - Enqueue(Operation(SAVESTATE_SAVE_SCREENSHOT, filename, -1, callback, cbUserData)); + Enqueue(Operation(SAVESTATE_SAVE_SCREENSHOT, filename, -1, nullptr, nullptr)); } bool CanRewind() @@ -690,7 +690,7 @@ double g_lastSaveTime = -1.0; DeleteIfExists(shotUndo); RenameIfExists(shot, shotUndo); } - SaveScreenshot(shot, Callback(), 0); + SaveScreenshot(shot); Save(fn.WithExtraExtension(".tmp"), slot, renameCallback, cbUserData); } else { if (callback) { @@ -1100,7 +1100,7 @@ double g_lastSaveTime = -1.0; WARN_LOG(Log::SaveState, "Failed to take a screenshot for the savestate! (%s) The savestate will lack an icon.", op.filename.c_str()); if (coreState != CORE_STEPPING_CPU && screenshotFailures++ < SCREENSHOT_FAILURE_RETRIES) { // Requeue for next frame (if we were stepping, no point, will just spam errors quickly). - SaveScreenshot(op.filename, op.callback, op.cbUserData); + SaveScreenshot(op.filename); } } else { screenshotFailures = 0; From b421c0791f64470ab141684b7e86d619260c8a7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 20 Mar 2025 19:58:24 +0100 Subject: [PATCH 4/5] Perform screenshot processing (including image encode) on background tasks --- Common/Thread/Promise.h | 16 ++++++++++++ Core/SaveState.cpp | 35 +++++++++++++++++-------- Core/Screenshot.cpp | 33 ++++++++++++++++------- Core/Screenshot.h | 15 ++++++++++- GPU/Common/GPUDebugInterface.h | 2 +- GPU/Software/SoftGpu.cpp | 11 ++++---- UI/NativeApp.cpp | 48 +++++++++++++++++----------------- UI/ReportScreen.cpp | 19 +++++++------- 8 files changed, 117 insertions(+), 62 deletions(-) diff --git a/Common/Thread/Promise.h b/Common/Thread/Promise.h index d4e942aeb8..4b57a3a386 100644 --- a/Common/Thread/Promise.h +++ b/Common/Thread/Promise.h @@ -7,6 +7,22 @@ #include "Common/Thread/Channel.h" #include "Common/Thread/ThreadManager.h" +// Nobody needs to wait for this (except threadpool shutdown). +template +class IndependentTask : public Task { +public: + IndependentTask(TaskType type, TaskPriority prio, T func) : func_(std::move(func)), type_(type), prio_(prio) {} + TaskType Type() const override { return type_; } + TaskPriority Priority() const override { return prio_; } + void Run() override { + func_(); + } +private: + T func_; + TaskType type_; + TaskPriority prio_; +}; + template class PromiseTask : public Task { public: diff --git a/Core/SaveState.cpp b/Core/SaveState.cpp index 465dca2cdd..648ad33d88 100644 --- a/Core/SaveState.cpp +++ b/Core/SaveState.cpp @@ -471,8 +471,7 @@ double g_lastSaveTime = -1.0; Enqueue(Operation(SAVESTATE_SAVE_SCREENSHOT, filename, -1, nullptr, nullptr)); } - bool CanRewind() - { + bool CanRewind() { return !rewindStates.Empty(); } @@ -966,11 +965,9 @@ double g_lastSaveTime = -1.0; bool readbackImage = false; - for (size_t i = 0, n = operations.size(); i < n; ++i) { - Operation &op = operations[i]; + for (const auto &op : operations) { CChunkFileReader::Error result; Status callbackResult; - bool tempResult; std::string callbackMessage; std::string title; @@ -1056,7 +1053,8 @@ double g_lastSaveTime = -1.0; break; case SAVESTATE_VERIFY: - tempResult = CChunkFileReader::Verify(state) == CChunkFileReader::ERROR_NONE; + { + int tempResult = CChunkFileReader::Verify(state) == CChunkFileReader::ERROR_NONE; callbackResult = tempResult ? Status::SUCCESS : Status::FAILURE; if (tempResult) { INFO_LOG(Log::SaveState, "Verified save state system"); @@ -1064,6 +1062,7 @@ double g_lastSaveTime = -1.0; ERROR_LOG(Log::SaveState, "Save state system verification failed"); } break; + } case SAVESTATE_REWIND: INFO_LOG(Log::SaveState, "Rewinding to recent savestate snapshot"); @@ -1093,18 +1092,32 @@ double g_lastSaveTime = -1.0; case SAVESTATE_SAVE_SCREENSHOT: { + _dbg_assert_(!op.callback); + int maxResMultiplier = 2; - tempResult = TakeGameScreenshot(nullptr, op.filename, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, maxResMultiplier); - callbackResult = tempResult ? Status::SUCCESS : Status::FAILURE; - if (!tempResult) { + ScreenshotResult tempResult = TakeGameScreenshot(nullptr, op.filename, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, maxResMultiplier, [](bool success) { + if (success) { + screenshotFailures = 0; + } + }); + + switch (tempResult) { + case ScreenshotResult::ScreenshotNotPossible: + // Try again soon, for a short while. + callbackResult = Status::FAILURE; WARN_LOG(Log::SaveState, "Failed to take a screenshot for the savestate! (%s) The savestate will lack an icon.", op.filename.c_str()); if (coreState != CORE_STEPPING_CPU && screenshotFailures++ < SCREENSHOT_FAILURE_RETRIES) { // Requeue for next frame (if we were stepping, no point, will just spam errors quickly). SaveScreenshot(op.filename); } - } else { - screenshotFailures = 0; + break; + case ScreenshotResult::DelayedResult: + case ScreenshotResult::Success: + // We might not know if the file write succeeded yet though. + callbackResult = Status::SUCCESS; + break; } + readbackImage = true; break; } diff --git a/Core/Screenshot.cpp b/Core/Screenshot.cpp index aaa17e41ac..781007450c 100644 --- a/Core/Screenshot.cpp +++ b/Core/Screenshot.cpp @@ -26,7 +26,10 @@ #include "Common/File/FileUtil.h" #include "Common/File/Path.h" #include "Common/Log.h" +#include "Common/System/System.h" #include "Common/System/Display.h" +#include "Common/System/NativeApp.h" +#include "Common/Thread/Promise.h" #include "Core/Screenshot.h" #include "Core/System.h" #include "GPU/Common/GPUDebugInterface.h" @@ -328,7 +331,7 @@ static GPUDebugBuffer ApplyRotation(const GPUDebugBuffer &buf, DisplayRotation r return rotated; } -bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes) { +ScreenshotResult TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes, std::function callback) { GPUDebugBuffer buf; u32 w = (u32)-1; u32 h = (u32)-1; @@ -336,10 +339,10 @@ bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, Screensho if (type == SCREENSHOT_DISPLAY || type == SCREENSHOT_RENDER) { if (!gpuDebug) { ERROR_LOG(Log::System, "Can't take screenshots when GPU not running"); - return false; + return ScreenshotResult::ScreenshotNotPossible; } if (!gpuDebug->GetCurrentFramebuffer(buf, type == SCREENSHOT_RENDER ? GPU_DBG_FRAMEBUF_RENDER : GPU_DBG_FRAMEBUF_DISPLAY, maxRes)) { - return false; + return ScreenshotResult::ScreenshotNotPossible; } w = maxRes > 0 ? 480 * maxRes : PSP_CoreParameter().renderWidth; h = maxRes > 0 ? 272 * maxRes : PSP_CoreParameter().renderHeight; @@ -347,25 +350,35 @@ bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, Screensho _dbg_assert_(draw); GPUDebugBuffer temp; if (!::GetOutputFramebuffer(draw, temp)) { - return false; + return ScreenshotResult::ScreenshotNotPossible; } buf = ApplyRotation(temp, g_display.rotation); } else { _dbg_assert_(draw); if (!GetOutputFramebuffer(draw, buf)) { - return false; + return ScreenshotResult::ScreenshotNotPossible; } } + if (callback) { + g_threadManager.EnqueueTask(new IndependentTask(TaskType::CPU_COMPUTE, TaskPriority::LOW, + [buf = std::move(buf), callback = std::move(callback), filename, fmt, w, h]() { + u8 *flipbuffer = nullptr; + u32 width = w, height = h; + const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, width, height); + bool success = Save888RGBScreenshot(filename, fmt, buffer, width, height); + delete[] flipbuffer; + System_RunOnMainThread([success, callback = std::move(callback)]() { + callback(success); + }); + })); + return ScreenshotResult::DelayedResult; + } u8 *flipbuffer = nullptr; const u8 *buffer = ConvertBufferToScreenshot(buf, false, flipbuffer, w, h); - if (!buffer) { - return false; - } - bool success = Save888RGBScreenshot(filename, fmt, buffer, w, h); delete[] flipbuffer; - return true; + return success ? ScreenshotResult::Success : ScreenshotResult::FailedToWriteFile; } bool Save888RGBScreenshot(const Path &filename, ScreenshotFormat fmt, const u8 *bufferRGB888, int w, int h) { diff --git a/Core/Screenshot.h b/Core/Screenshot.h index 6e15c0bba2..40d8cd6b7b 100644 --- a/Core/Screenshot.h +++ b/Core/Screenshot.h @@ -19,6 +19,8 @@ #include "Common/File/Path.h" +#include + struct GPUDebugBuffer; namespace Draw { class DrawContext; @@ -29,6 +31,8 @@ enum class ScreenshotFormat { JPG, }; +// 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, @@ -38,10 +42,19 @@ enum ScreenshotType { SCREENSHOT_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, +}; + const u8 *ConvertBufferToScreenshot(const GPUDebugBuffer &buf, bool alpha, u8 *&temp, u32 &w, u32 &h); // Can only be used while in game. -bool TakeGameScreenshot(Draw::DrawContext *draw, const Path &filename, ScreenshotFormat fmt, ScreenshotType type, int maxRes = -1); +// 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 callback = nullptr); 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); diff --git a/GPU/Common/GPUDebugInterface.h b/GPU/Common/GPUDebugInterface.h index 2b3e437cfa..1a87c4270b 100644 --- a/GPU/Common/GPUDebugInterface.h +++ b/GPU/Common/GPUDebugInterface.h @@ -156,7 +156,7 @@ struct GPUDebugBuffer { u32 GetRawPixel(int x, int y) const; void SetRawPixel(int x, int y, u32 c); - u8 *GetDataWrite() { + u8 *GetData() { return data_; } diff --git a/GPU/Software/SoftGpu.cpp b/GPU/Software/SoftGpu.cpp index 64f8d2f905..f2e36bc00c 100644 --- a/GPU/Software/SoftGpu.cpp +++ b/GPU/Software/SoftGpu.cpp @@ -1384,7 +1384,7 @@ bool SoftGPU::GetCurrentFramebuffer(GPUDebugBuffer &buffer, GPUDebugFramebufferT buffer.Allocate(size.x, size.y, fmt); const int depth = fmt == GE_FORMAT_8888 ? 4 : 2; - u8 *dst = buffer.GetDataWrite(); + u8 *dst = buffer.GetData(); const int byteWidth = size.x * depth; for (int16_t y = 0; y < size.y; ++y) { memcpy(dst, src, byteWidth); @@ -1404,7 +1404,7 @@ bool SoftGPU::GetCurrentDepthbuffer(GPUDebugBuffer &buffer) { const int depth = 2; const u8 *src = depthbuf.data; - u8 *dst = buffer.GetDataWrite(); + u8 *dst = buffer.GetData(); for (int16_t y = 0; y < size.y; ++y) { memcpy(dst, src, size.x * depth); dst += size.x * depth; @@ -1430,7 +1430,7 @@ bool SoftGPU::GetCurrentStencilbuffer(GPUDebugBuffer &buffer) { DrawingCoords size = GetTargetSize(gstate.FrameBufStride()); buffer.Allocate(size.x, size.y, GPU_DBG_FORMAT_8BIT); - u8 *row = buffer.GetDataWrite(); + u8 *row = buffer.GetData(); for (int16_t y = 0; y < size.y; ++y) { for (int16_t x = 0; x < size.x; ++x) { row[x] = GetPixelStencil(gstate.FrameBufFormat(), gstate.FrameBufStride(), x, y); @@ -1445,13 +1445,12 @@ bool SoftGPU::GetCurrentTexture(GPUDebugBuffer &buffer, int level, bool *isFrame return Rasterizer::GetCurrentTexture(buffer, level); } -bool SoftGPU::GetCurrentClut(GPUDebugBuffer &buffer) -{ +bool SoftGPU::GetCurrentClut(GPUDebugBuffer &buffer) { const u32 bpp = gstate.getClutPaletteFormat() == GE_CMODE_32BIT_ABGR8888 ? 4 : 2; const u32 pixels = 1024 / bpp; buffer.Allocate(pixels, 1, (GEBufferFormat)gstate.getClutPaletteFormat()); - memcpy(buffer.GetDataWrite(), clut, 1024); + memcpy(buffer.GetData(), clut, 1024); return true; } diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 959de71620..5fbf569d11 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -949,12 +949,14 @@ static void TakeScreenshot(Draw::DrawContext *draw) { } // First, find a free filename. - int i = 0; - std::string gameId = g_paramSFO.GetDiscID(); + const std::string gameId = g_paramSFO.GetDiscID(); Path filename; - while (i < 10000){ + int i = 0; + // TODO: This is really potentially way too slow if there are a lot of images! + + for (int i = 0; i < 10000; i++) { if (g_Config.bScreenshotsAsPNG) filename = path / StringFromFormat("%s_%05d.png", gameId.c_str(), i); else @@ -965,29 +967,27 @@ static void TakeScreenshot(Draw::DrawContext *draw) { i++; } - ScreenshotType type = SCREENSHOT_OUTPUT; - if (g_Config.iScreenshotMode == (int)ScreenshotMode::GameImage) { - type = SCREENSHOT_DISPLAY; - } + const ScreenshotType type = g_Config.iScreenshotMode == (int)ScreenshotMode::GameImage ? SCREENSHOT_DISPLAY : SCREENSHOT_OUTPUT; - bool success = TakeGameScreenshot(draw, filename, g_Config.bScreenshotsAsPNG ? ScreenshotFormat::PNG : ScreenshotFormat::JPG, type); - 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", [](bool clicked, void *data) -> void { - Path *path = reinterpret_cast(data); - if (clicked) { - System_ShowFileInFolder(*path); - } else { - delete path; - } - }, new Path(filename)); + 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", [](bool clicked, void *data) -> void { + Path *path = reinterpret_cast(data); + if (clicked) { + System_ShowFileInFolder(*path); + } else { + delete path; + } + }, new Path(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."); } - } 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) { diff --git a/UI/ReportScreen.cpp b/UI/ReportScreen.cpp index 425dc0ce2e..7b5df8181f 100644 --- a/UI/ReportScreen.cpp +++ b/UI/ReportScreen.cpp @@ -182,21 +182,22 @@ ScreenRenderFlags ReportScreen::render(ScreenRenderMode mode) { File::CreateDir(path); } screenshotFilename_ = path / ".reporting.jpg"; - if (TakeGameScreenshot(screenManager()->getDrawContext(), screenshotFilename_, ScreenshotFormat::JPG, SCREENSHOT_DISPLAY, 4)) { - // Redo the views already, now with a screenshot included. - RecreateViews(); - } else { - // Good news (?), the views are good as-is without a screenshot. - screenshotFilename_.clear(); - } + ScreenshotResult ignored = TakeGameScreenshot(screenManager()->getDrawContext(), screenshotFilename_, ScreenshotFormat::JPG, SCREENSHOT_RENDER, 4, [this](bool success) { + if (success) { + // Redo the views already, now with a screenshot included. + RecreateViews(); + } else { + // Good news (?), the views are good as-is without a screenshot. + screenshotFilename_.clear(); + } + }); tookScreenshot_ = true; } } // We take the screenshot first, then we start rendering. // We are the only screen visible so this avoid starting and then trying to resume a backbuffer render pass. - ScreenRenderFlags flags = UIScreen::render(mode); - + const ScreenRenderFlags flags = UIScreen::render(mode); return flags; } From 6a71cbee791313bef0f11dc433652c32b4909811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 20 Mar 2025 20:34:18 +0100 Subject: [PATCH 5/5] Speed up screenshot filename generation (checking for existing files more efficiently) --- UI/NativeApp.cpp | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 5fbf569d11..8c0227e758 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -949,22 +949,37 @@ static void TakeScreenshot(Draw::DrawContext *draw) { } // 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 files; + const std::string prefix = gameId + "_"; + File::GetFilesInDir(path, &files, nullptr, 0, prefix); + std::set existingNames; + for (auto &file : files) { + existingNames.insert(file.name); + } + Path filename; int i = 0; - // TODO: This is really potentially way too slow if there are a lot of images! - - for (int i = 0; i < 10000; i++) { - if (g_Config.bScreenshotsAsPNG) - filename = path / StringFromFormat("%s_%05d.png", gameId.c_str(), i); - else - filename = path / StringFromFormat("%s_%05d.jpg", gameId.c_str(), i); - File::FileInfo info; - if (!File::Exists(filename)) + 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; - i++; + } + } + + 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;