From f73d0587d4c65af7c0c677855952866ad5a36029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 19 Jan 2024 11:25:46 +0100 Subject: [PATCH] Comments, make locking more consistent in GameInfoCache --- Common/File/Path.cpp | 3 +- Common/File/Path.h | 2 +- Common/Render/ManagedTexture.cpp | 6 +-- Common/Render/ManagedTexture.h | 3 +- Core/MIPS/MIPSDebugInterface.cpp | 4 +- Core/MemFault.cpp | 2 + UI/GameInfoCache.cpp | 66 ++++++++++++++++++++------------ 7 files changed, 53 insertions(+), 33 deletions(-) diff --git a/Common/File/Path.cpp b/Common/File/Path.cpp index 24bc8bdee0b1..c9302437eb71 100644 --- a/Common/File/Path.cpp +++ b/Common/File/Path.cpp @@ -227,14 +227,13 @@ std::string Path::GetDirectory() const { return path_; } -bool Path::FilePathContainsNoCase(const std::string &needle) const { +bool Path::FilePathContainsNoCase(std::string_view needle) const { std::string haystack; if (type_ == PathType::CONTENT_URI) { haystack = AndroidContentURI(path_).FilePath(); } else { haystack = path_; } - auto pred = [](char ch1, char ch2) { return std::toupper(ch1) == std::toupper(ch2); }; auto found = std::search(haystack.begin(), haystack.end(), needle.begin(), needle.end(), pred); return found != haystack.end(); diff --git a/Common/File/Path.h b/Common/File/Path.h index 0be3e0aabde9..e94caf14c314 100644 --- a/Common/File/Path.h +++ b/Common/File/Path.h @@ -118,7 +118,7 @@ class Path { return path_ != other.path_ || type_ != other.type_; } - bool FilePathContainsNoCase(const std::string &needle) const; + bool FilePathContainsNoCase(std::string_view needle) const; bool StartsWith(const Path &other) const; diff --git a/Common/Render/ManagedTexture.cpp b/Common/Render/ManagedTexture.cpp index ed76da4d90c5..a3d9f2344b53 100644 --- a/Common/Render/ManagedTexture.cpp +++ b/Common/Render/ManagedTexture.cpp @@ -39,7 +39,7 @@ class TextureLoadTask : public Task { return; } - if (!tempImage_->LoadTextureLevels(buffer, fileSize, type_)) { + if (!tempImage_->LoadTextureLevelsFromFileData(buffer, fileSize, type_)) { *state_ = ManagedTexture::LoadState::FAILED; waitable_->Notify(); return; @@ -85,7 +85,7 @@ static ImageFileType DetectImageFileType(const uint8_t *data, size_t size) { } } -bool TempImage::LoadTextureLevels(const uint8_t *data, size_t size, ImageFileType typeSuggestion) { +bool TempImage::LoadTextureLevelsFromFileData(const uint8_t *data, size_t size, ImageFileType typeSuggestion) { if (typeSuggestion == ImageFileType::DETECT) { typeSuggestion = DetectImageFileType(data, size); } @@ -166,7 +166,7 @@ Draw::Texture *CreateTextureFromTempImage(Draw::DrawContext *draw, const TempIma Draw::Texture *CreateTextureFromFileData(Draw::DrawContext *draw, const uint8_t *data, size_t dataSize, ImageFileType type, bool generateMips, const char *name) { TempImage image; - if (!image.LoadTextureLevels(data, dataSize, type)) { + if (!image.LoadTextureLevelsFromFileData(data, dataSize, type)) { return nullptr; } Draw::Texture *texture = CreateTextureFromTempImage(draw, image, generateMips, name); diff --git a/Common/Render/ManagedTexture.h b/Common/Render/ManagedTexture.h index f90ee457870f..e9de5f3c6cf3 100644 --- a/Common/Render/ManagedTexture.h +++ b/Common/Render/ManagedTexture.h @@ -32,7 +32,7 @@ struct TempImage { int height[16]{}; int numLevels = 0; - bool LoadTextureLevels(const uint8_t *data, size_t size, ImageFileType typeSuggestion = ImageFileType::DETECT); + bool LoadTextureLevelsFromFileData(const uint8_t *data, size_t size, ImageFileType typeSuggestion = ImageFileType::DETECT); void Free() { if (levels[0]) { free(levels[0]); @@ -79,3 +79,4 @@ class ManagedTexture { Draw::Texture *CreateTextureFromFileData(Draw::DrawContext *draw, const uint8_t *data, size_t dataSize, ImageFileType type, bool generateMips, const char *name); Draw::Texture *CreateTextureFromFile(Draw::DrawContext *draw, const char *filename, ImageFileType type, bool generateMips); +Draw::Texture *CreateTextureFromTempImage(Draw::DrawContext *draw, const TempImage &image, bool generateMips, const char *name); diff --git a/Core/MIPS/MIPSDebugInterface.cpp b/Core/MIPS/MIPSDebugInterface.cpp index e68ac4489675..b598e0fe676c 100644 --- a/Core/MIPS/MIPSDebugInterface.cpp +++ b/Core/MIPS/MIPSDebugInterface.cpp @@ -158,9 +158,9 @@ class MipsExpressionFunctions: public IExpressionFunctions if (referenceIndex == REF_INDEX_MODULE) return __KernelGetCurThreadModuleId(); if (referenceIndex == REF_INDEX_USEC) - return CoreTiming::GetGlobalTimeUs(); + return (uint32_t)CoreTiming::GetGlobalTimeUs(); // Loses information if (referenceIndex == REF_INDEX_USEC) - return CoreTiming::GetTicks(); + return (uint32_t)CoreTiming::GetTicks(); if ((referenceIndex & ~(REF_INDEX_FPU | REF_INDEX_FPU_INT)) < 32) return cpu->GetRegValue(1, referenceIndex & ~(REF_INDEX_FPU | REF_INDEX_FPU_INT)); if ((referenceIndex & ~(REF_INDEX_VFPU | REF_INDEX_VFPU_INT)) < 128) diff --git a/Core/MemFault.cpp b/Core/MemFault.cpp index 95a7088ae3e4..7b7896d1e154 100644 --- a/Core/MemFault.cpp +++ b/Core/MemFault.cpp @@ -119,6 +119,8 @@ bool HandleFault(uintptr_t hostAddress, void *ctx) { bool inJitSpace = MIPSComp::jit && MIPSComp::jit->CodeInRange(codePtr); if (!inJitSpace) { // This is a crash in non-jitted code. Not something we want to handle here, ignore. + // Actually, we could handle crashes from the IR interpreter here, although recovering the call stack + // might be tricky... inCrashHandler = false; return false; } diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp index 91c6ead1937c..b00549780094 100644 --- a/UI/GameInfoCache.cpp +++ b/UI/GameInfoCache.cpp @@ -360,6 +360,20 @@ static bool ReadFileToString(IFileSystem *fs, const char *filename, std::string return true; } +static bool ReadLocalFileToString(const Path &path, std::string *contents, std::mutex *mtx) { + std::string data; + if (!File::ReadFileToString(false, path, *contents)) { + return false; + } + if (mtx) { + std::lock_guard lock(*mtx); + *contents = std::move(data); + } else { + *contents = std::move(data); + } + return true; +} + static bool ReadVFSToString(const char *filename, std::string *contents, std::mutex *mtx) { size_t sz; uint8_t *data = g_VFS.ReadFile(filename, &sz); @@ -469,9 +483,9 @@ class GameInfoWorkItem : public Task { Path screenshot_png = GetSysDirectory(DIRECTORY_SCREENSHOT) / (info_->id + "_00000.png"); // Try using png/jpg screenshots first if (File::Exists(screenshot_png)) - File::ReadFileToString(false, screenshot_png, info_->icon.data); + ReadLocalFileToString(screenshot_png, &info_->icon.data, &info_->lock); else if (File::Exists(screenshot_jpg)) - File::ReadFileToString(false, screenshot_jpg, info_->icon.data); + ReadLocalFileToString(screenshot_jpg, &info_->icon.data, &info_->lock); else // Read standard icon ReadVFSToString("unknown.png", &info_->icon.data, &info_->lock); @@ -522,9 +536,9 @@ class GameInfoWorkItem : public Task { Path screenshot_png = GetSysDirectory(DIRECTORY_SCREENSHOT) / (info_->id + "_00000.png"); // Try using png/jpg screenshots first if (File::Exists(screenshot_png)) { - File::ReadFileToString(false, screenshot_png, info_->icon.data); + ReadLocalFileToString(screenshot_png, &info_->icon.data, &info_->lock); } else if (File::Exists(screenshot_jpg)) { - File::ReadFileToString(false, screenshot_jpg, info_->icon.data); + ReadLocalFileToString(screenshot_jpg, &info_->icon.data, &info_->lock); } else { // Read standard icon VERBOSE_LOG(LOADER, "Loading unknown.png because there was an ELF"); @@ -558,34 +572,36 @@ class GameInfoWorkItem : public Task { case IdentifiedFileType::PPSSPP_SAVESTATE: { - info_->SetTitle(SaveState::GetTitle(gamePath_)); - - std::lock_guard guard(info_->lock); + Path screenshotPath; + { + info_->SetTitle(SaveState::GetTitle(gamePath_)); + std::lock_guard guard(info_->lock); + screenshotPath = gamePath_.WithReplacedExtension(".ppst", ".jpg"); + } // Let's use the screenshot as an icon, too. - Path screenshotPath = gamePath_.WithReplacedExtension(".ppst", ".jpg"); - if (File::Exists(screenshotPath)) { - if (File::ReadFileToString(false, screenshotPath, info_->icon.data)) { - info_->icon.dataLoaded = true; - } else { - ERROR_LOG(G3D, "Error loading screenshot data: '%s'", screenshotPath.c_str()); - } + if (ReadLocalFileToString(screenshotPath, &info_->icon.data, &info_->lock)) { + info_->icon.dataLoaded = true; + } else { + ERROR_LOG(G3D, "Error loading screenshot data: '%s'", screenshotPath.c_str()); } break; } case IdentifiedFileType::PPSSPP_GE_DUMP: { - std::lock_guard guard(info_->lock); + Path screenshotPath; + + { + std::lock_guard guard(info_->lock); + screenshotPath = gamePath_.WithReplacedExtension(".ppdmp", ".png"); + } // Let's use the comparison screenshot as an icon, if it exists. - Path screenshotPath = gamePath_.WithReplacedExtension(".ppdmp", ".png"); - if (File::Exists(screenshotPath)) { - if (File::ReadFileToString(false, screenshotPath, info_->icon.data)) { - info_->icon.dataLoaded = true; - } else { - ERROR_LOG(G3D, "Error loading screenshot data: '%s'", screenshotPath.c_str()); - } + if (ReadLocalFileToString(screenshotPath, &info_->icon.data, &info_->lock)) { + info_->icon.dataLoaded = true; + } else { + ERROR_LOG(G3D, "Error loading screenshot data: '%s'", screenshotPath.c_str()); } break; } @@ -658,9 +674,9 @@ class GameInfoWorkItem : public Task { Path screenshot_png = GetSysDirectory(DIRECTORY_SCREENSHOT) / (info_->id + "_00000.png"); // Try using png/jpg screenshots first if (File::Exists(screenshot_png)) - info_->icon.dataLoaded = File::ReadFileToString(false, screenshot_png, info_->icon.data); + info_->icon.dataLoaded = ReadLocalFileToString(screenshot_png, &info_->icon.data, &info_->lock); else if (File::Exists(screenshot_jpg)) - info_->icon.dataLoaded = File::ReadFileToString(false, screenshot_jpg, info_->icon.data); + info_->icon.dataLoaded = ReadLocalFileToString(screenshot_jpg, &info_->icon.data, &info_->lock); else { DEBUG_LOG(LOADER, "Loading unknown.png because no icon was found"); info_->icon.dataLoaded = ReadVFSToString("unknown.png", &info_->icon.data, &info_->lock); @@ -842,6 +858,8 @@ void GameInfoCache::SetupTexture(std::shared_ptr &info, Draw::DrawCont using namespace Draw; if (tex.data.size()) { if (!tex.texture) { + // TODO: Use TempImage to semi-load the image in the worker task, then here we + // could just call CreateTextureFromTempImage. tex.texture = CreateTextureFromFileData(thin3d, (const uint8_t *)tex.data.data(), (int)tex.data.size(), ImageFileType::DETECT, false, info->GetTitle().c_str()); if (tex.texture) { tex.timeLoaded = time_now_d();