Merge pull request #18779 from hrydgard/more-fixes

More fixes

Common/File/PathBrowser.cpp

@@ -111,12 +111,12 @@ bool LoadRemoteFileList(const Path &url, const std::string &userAgent, bool *can
 }
 
 PathBrowser::~PathBrowser() {
-	std::unique_lock<std::mutex> guard(pendingLock_);
-	pendingCancel_ = true;
-	pendingStop_ = true;
-	pendingCond_.notify_all();
-	guard.unlock();
-
+	{
+		std::unique_lock<std::mutex> guard(pendingLock_);
+		pendingCancel_ = true;
+		pendingStop_ = true;
+		pendingCond_.notify_all();
+	}
 	if (pendingThread_.joinable()) {
 		pendingThread_.join();
 	}

Common/File/VFS/VFS.cpp

@@ -63,7 +63,7 @@ uint8_t *VFS::ReadFile(const char *filename, size_t *size) {
 	if (!fileSystemFound) {
 		ERROR_LOG(IO, "Missing filesystem for '%s'", filename);
 	}  // Otherwise, the file was just missing. No need to log.
-	return 0;
+	return nullptr;
 }
 
 bool VFS::GetFileListing(const char *path, std::vector<File::FileInfo> *listing, const char *filter) {

Common/File/VFS/ZipFileReader.cpp

@@ -107,7 +107,7 @@ bool ZipFileReader::GetFileListing(const char *orig_path, std::vector<File::File
 
 	listing->clear();
 
-	INFO_LOG(SYSTEM, "Listing %s", orig_path);
+	// INFO_LOG(SYSTEM, "Zip: Listing '%s'", orig_path);
 
 	listing->reserve(directories.size() + files.size());
 	for (auto diter = directories.begin(); diter != directories.end(); ++diter) {

Common/GPU/Vulkan/VulkanRenderManager.cpp

@@ -385,13 +385,7 @@ void VulkanRenderManager::StopThreads() {
 		pushCondVar_.notify_one();
 		// Once the render thread encounters the above exit task, it'll exit.
 		renderThread_.join();
-	}
-
-	// Compiler and present thread still relies on this.
-	runCompileThread_ = false;
-
-	if (presentWaitThread_.joinable()) {
-		presentWaitThread_.join();
+		INFO_LOG(G3D, "Vulkan submission thread joined. Frame=%d", vulkan_->GetCurFrame());
 	}
 
 	for (int i = 0; i < vulkan_->GetInflightFrames(); i++) {
@@ -400,12 +394,18 @@ void VulkanRenderManager::StopThreads() {
 		frameData.profile.timestampDescriptions.clear();
 	}
 
-	INFO_LOG(G3D, "Vulkan submission thread joined. Frame=%d", vulkan_->GetCurFrame());
-
-	_assert_(compileThread_.joinable());
-	compileCond_.notify_all();
+	{
+		std::unique_lock<std::mutex> lock(compileMutex_);
+		runCompileThread_ = false;  // Compiler and present thread both look at this bool.
+		_assert_(compileThread_.joinable());
+		compileCond_.notify_one();
+	}
 	compileThread_.join();
 
+	if (presentWaitThread_.joinable()) {
+		presentWaitThread_.join();
+	}
+
 	INFO_LOG(G3D, "Vulkan compiler thread joined. Now wait for any straggling compile tasks.");
 	CreateMultiPipelinesTask::WaitForAll();
 
@@ -462,7 +462,7 @@ void VulkanRenderManager::CompileThreadFunc() {
 		std::vector<CompileQueueEntry> toCompile;
 		{
 			std::unique_lock<std::mutex> lock(compileMutex_);
-			if (compileQueue_.empty() && runCompileThread_) {
+			while (compileQueue_.empty() && runCompileThread_) {
 				compileCond_.wait(lock);
 			}
 			toCompile = std::move(compileQueue_);
@@ -786,7 +786,7 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
 			VKRRenderPassStoreAction::STORE, VKRRenderPassStoreAction::DONT_CARE, VKRRenderPassStoreAction::DONT_CARE,
 		};
 		VKRRenderPass *compatibleRenderPass = queueRunner_.GetRenderPass(key);
-		std::lock_guard<std::mutex> lock(compileMutex_);
+		std::unique_lock<std::mutex> lock(compileMutex_);
 		bool needsCompile = false;
 		for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) {
 			if (!(variantBitmask & (1 << i)))
@@ -809,7 +809,7 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
 			}
 
 			pipeline->pipeline[i] = Promise<VkPipeline>::CreateEmpty();
-			compileQueue_.push_back(CompileQueueEntry(pipeline, compatibleRenderPass->Get(vulkan_, rpType, sampleCount), rpType, sampleCount));
+			compileQueue_.emplace_back(pipeline, compatibleRenderPass->Get(vulkan_, rpType, sampleCount), rpType, sampleCount);
 			needsCompile = true;
 		}
 		if (needsCompile)
@@ -1551,10 +1551,12 @@ void VulkanRenderManager::FlushSync() {
 			VLOG("PUSH: Frame[%d]", curFrame);
 			VKRRenderThreadTask *task = new VKRRenderThreadTask(VKRRunType::SYNC);
 			task->frame = curFrame;
-			std::unique_lock<std::mutex> lock(pushMutex_);
-			renderThreadQueue_.push(task);
-			renderThreadQueue_.back()->steps = std::move(steps_);
-			pushCondVar_.notify_one();
+			{
+				std::unique_lock<std::mutex> lock(pushMutex_);
+				renderThreadQueue_.push(task);
+				renderThreadQueue_.back()->steps = std::move(steps_);
+				pushCondVar_.notify_one();
+			}
 			steps_.clear();
 		}
 

Common/Render/Text/draw_text_sdl.cpp

@@ -285,7 +285,6 @@ void TextDrawerSDL::MeasureString(const char *str, size_t len, float *w, float *
 	if (iter != sizeCache_.end()) {
 		entry = iter->second.get();
 	} else {
-		printf("re-measuring %s\n", str);
 		TTF_Font *font = fontMap_.find(fontHash_)->second;
 		int ptSize = TTF_FontHeight(font) / 1.35;
 

Common/Thread/ThreadManager.cpp

@@ -59,8 +59,8 @@ ThreadManager::~ThreadManager() {
 
 void ThreadManager::Teardown() {
 	for (TaskThreadContext *&threadCtx : global_->threads_) {
-		threadCtx->cancelled = true;
 		std::unique_lock<std::mutex> lock(threadCtx->mutex);
+		threadCtx->cancelled = true;
 		threadCtx->cond.notify_one();
 	}
 

Core/FrameTiming.cpp

@@ -81,6 +81,8 @@ void FrameTiming::PostSubmit() {
 }
 
 Draw::PresentMode ComputePresentMode(Draw::DrawContext *draw, int *interval) {
+	_assert_(draw);
+
 	Draw::PresentMode mode = Draw::PresentMode::FIFO;
 
 	if (draw->GetDeviceCaps().presentModesSupported & (Draw::PresentMode::IMMEDIATE | Draw::PresentMode::MAILBOX)) {

Core/HLE/scePsmf.cpp

@@ -312,6 +312,7 @@ class PsmfStream {
 
 		psmf->EPMap.clear();
 		for (u32 i = 0; i < psmf->EPMapEntriesNum; i++) {
+			// TODO: Should look into validating these offsets. Got a crash report here.
 			const u8 *const entryAddr = data + psmf->EPMapOffset + EP_MAP_STRIDE * i;
 			PsmfEntry entry;
 			entry.EPIndex = entryAddr[0];

Core/Reporting.cpp

@@ -20,6 +20,7 @@
 #include <deque>
 #include <thread>
 #include <mutex>
+#include <atomic>
 #include <condition_variable>
 #include <set>
 #include <cstdlib>
@@ -113,11 +114,11 @@ namespace Reporting
 	static std::condition_variable crcCond;
 	static Path crcFilename;
 	static std::map<Path, u32> crcResults;
-	static volatile bool crcPending = false;
-	static volatile bool crcCancel = false;
+	static std::atomic<bool> crcPending{};
+	static std::atomic<bool> crcCancel{};
 	static std::thread crcThread;
 
-	static u32 CalculateCRC(BlockDevice *blockDevice, volatile bool *cancel) {
+	static u32 CalculateCRC(BlockDevice *blockDevice, std::atomic<bool> *cancel) {
 		auto ga = GetI18NCategory(I18NCat::GAME);
 
 		u32 crc = crc32(0, Z_NULL, 0);
@@ -162,7 +163,6 @@ namespace Reporting
 		crcResults[crcFilename] = crc;
 		crcPending = false;
 		crcCond.notify_one();
-
 		return 0;
 	}
 

GPU/GPUCommonHW.cpp

@@ -397,8 +397,10 @@ GPUCommonHW::~GPUCommonHW() {
 	framebufferManager_->DestroyAllFBOs();
 	delete framebufferManager_;
 	delete textureCache_;
-	shaderManager_->ClearShaders();
-	delete shaderManager_;
+	if (shaderManager_) {
+		shaderManager_->ClearShaders();
+		delete shaderManager_;
+	}
 }
 
 // Called once per frame. Might also get called during the pause screen

UI/BackgroundAudio.cpp

@@ -380,9 +380,9 @@ inline int16_t ConvertU8ToI16(uint8_t value) {
 }
 
 Sample *Sample::Load(const std::string &path) {
-	size_t bytes;
+	size_t bytes = 0;
 	uint8_t *data = g_VFS.ReadFile(path.c_str(), &bytes);
-	if (!data) {
+	if (!data || bytes > 100000000) {
 		WARN_LOG(AUDIO, "Failed to load sample '%s'", path.c_str());
 		return nullptr;
 	}

UI/DevScreens.cpp

@@ -832,7 +832,7 @@ void SystemInfoScreen::CreateTabs() {
 			vulkanExtensions->Add(new TextView(extension, new LayoutParams(FILL_PARENT, WRAP_CONTENT)))->SetFocusable(true);
 		}
 
-		vulkanExtensions->Add(new ItemHeader("Instance"));
+		vulkanExtensions->Add(new ItemHeader(si->T("Instance")));
 		// Also get instance extensions
 		extensions = draw->GetExtensionList(false, false);
 		std::sort(extensions.begin(), extensions.end());

UI/GameInfoCache.cpp

@@ -355,13 +355,20 @@ void GameInfo::FinishPendingTextureLoads(Draw::DrawContext *draw) {
 }
 
 void GameInfo::SetupTexture(Draw::DrawContext *thin3d, GameInfoTex &tex) {
+	if (tex.timeLoaded) {
+		// Failed before, skip.
+		return;
+	}
+	if (tex.data.empty()) {
+		tex.timeLoaded = time_now_d();
+		return;
+	}
 	using namespace Draw;
 	// 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, GetTitle().c_str());
-	if (tex.texture) {
-		tex.timeLoaded = time_now_d();
-	} else {
+	tex.timeLoaded = time_now_d();
+	if (!tex.texture) {
 		ERROR_LOG(G3D, "Failed creating texture (%s) from %d-byte file", GetTitle().c_str(), (int)tex.data.size());
 	}
 }
@@ -801,10 +808,12 @@ void GameInfoCache::Shutdown() {
 void GameInfoCache::Clear() {
 	CancelAll();
 
+	std::lock_guard<std::mutex> lock(mapLock_);
 	info_.clear();
 }
 
 void GameInfoCache::CancelAll() {
+	std::lock_guard<std::mutex> lock(mapLock_);
 	for (auto info : info_) {
 		// GetFileLoader will create one if there isn't one already.
 		// Avoid that by checking.
@@ -818,6 +827,7 @@ void GameInfoCache::CancelAll() {
 }
 
 void GameInfoCache::FlushBGs() {
+	std::lock_guard<std::mutex> lock(mapLock_);
 	for (auto iter = info_.begin(); iter != info_.end(); iter++) {
 		std::lock_guard<std::mutex> lock(iter->second->lock);
 		iter->second->pic0.Clear();
@@ -831,19 +841,28 @@ void GameInfoCache::FlushBGs() {
 }
 
 void GameInfoCache::PurgeType(IdentifiedFileType fileType) {
-	for (auto iter = info_.begin(); iter != info_.end();) {
-		auto &info = iter->second;
-
-		// TODO: Find a better way to wait here.
-		while (info->pendingFlags != (GameInfoFlags)0) {
-			sleep_ms(1);
-		}
-		if (info->fileType == fileType) {
-			iter = info_.erase(iter);
-		} else {
-			iter++;
+	bool retry = false;
+	// Trickery to avoid sleeping with the lock held.
+	do {
+		{
+			std::lock_guard<std::mutex> lock(mapLock_);
+			for (auto iter = info_.begin(); iter != info_.end();) {
+				auto &info = iter->second;
+
+				// TODO: Find a better way to wait here.
+				while (info->pendingFlags != (GameInfoFlags)0) {
+					retry = true;
+					break;
+				}
+				if (info->fileType == fileType) {
+					iter = info_.erase(iter);
+				} else {
+					iter++;
+				}
+			}
 		}
-	}
+		sleep_ms(1);
+	} while (retry);
 }
 
 // Call on the main thread ONLY - that is from stuff called from NativeFrame.

UI/GameInfoCache.h

@@ -69,11 +69,15 @@ struct GameInfoTex {
 	Draw::Texture *texture = nullptr;
 	// The time at which the Icon and the BG were loaded.
 	// Can be useful to fade them in smoothly once they appear.
+	// Also, timeLoaded != 0 && texture == nullptr means that the load failed.
 	double timeLoaded = 0.0;
 	std::atomic<bool> dataLoaded{};
 
 	// Can ONLY be called from the main thread!
 	void Clear();
+	bool Failed() const {
+		return timeLoaded != 0.0 && !texture;
+	}
 };
 
 class GameInfo {

UI/GameSettingsScreen.cpp

@@ -1396,7 +1396,7 @@ UI::EventReturn GameSettingsScreen::OnChangeBackground(UI::EventParams &e) {
 				FILE *f = File::OpenCFile(path, "rb");
 				uint8_t buffer[8];
 				ImageFileType type = ImageFileType::UNKNOWN;
-				if (8 == fread(buffer, 1, ARRAY_SIZE(buffer), f)) {
+				if (f != nullptr && 8 == fread(buffer, 1, ARRAY_SIZE(buffer), f)) {
 					type = DetectImageFileType(buffer, ARRAY_SIZE(buffer));
 				}
 

UI/GameSettingsScreen.h

@@ -175,8 +175,11 @@ class HostnameSelectScreen : public PopupScreen {
 		}, this);
 	}
 	~HostnameSelectScreen() {
-		resolverState_ = ResolverState::QUIT;
-		resolverCond_.notify_one();
+		{
+			std::unique_lock<std::mutex> guard(resolverLock_);
+			resolverState_ = ResolverState::QUIT;
+			resolverCond_.notify_one();
+		}
 		resolver_.join();
 	}
 

UI/NativeApp.cpp

@@ -924,6 +924,7 @@ void NativeShutdownGraphics() {
 	}
 	g_iconCache.ClearTextures();
 
+	// TODO: This is not really necessary with Vulkan on Android - could keep shaders etc in memory
 	if (gpu)
 		gpu->DeviceLost();
 

UI/SavedataScreen.cpp

@@ -87,7 +87,7 @@ class SavedataPopupScreen : public PopupScreen {
 		UIContext &dc = *screenManager()->getUIContext();
 		const Style &textStyle = dc.theme->popupStyle;
 
-		std::shared_ptr<GameInfo> ginfo = g_gameInfoCache->GetInfo(screenManager()->getDrawContext(), savePath_, GameInfoFlags::PARAM_SFO | GameInfoFlags::SIZE);
+		std::shared_ptr<GameInfo> ginfo = g_gameInfoCache->GetInfo(screenManager()->getDrawContext(), savePath_, GameInfoFlags::PARAM_SFO | GameInfoFlags::ICON | GameInfoFlags::SIZE);
 		if (!ginfo->Ready(GameInfoFlags::PARAM_SFO))
 			return;
 
@@ -294,7 +294,7 @@ void SavedataButton::UpdateText(const std::shared_ptr<GameInfo> &ginfo) {
 }
 
 void SavedataButton::Draw(UIContext &dc) {
-	std::shared_ptr<GameInfo> ginfo = g_gameInfoCache->GetInfo(dc.GetDrawContext(), savePath_, GameInfoFlags::PARAM_SFO | GameInfoFlags::SIZE);
+	std::shared_ptr<GameInfo> ginfo = g_gameInfoCache->GetInfo(dc.GetDrawContext(), savePath_, GameInfoFlags::ICON | GameInfoFlags::PARAM_SFO | GameInfoFlags::SIZE);
 	Draw::Texture *texture = 0;
 	u32 color = 0, shadowColor = 0;
 	using namespace UI;

android/jni/OpenSLContext.cpp

@@ -10,7 +10,6 @@
 #include <SLES/OpenSLES_Android.h>
 
 #include "Common/Log.h"
-#include "Common/StringUtils.h"
 #include "OpenSLContext.h"
 #include "Core/HLE/sceUsbMic.h"
 
@@ -58,7 +57,9 @@ void OpenSLContext::BqPlayerCallback(SLAndroidSimpleBufferQueueItf bq) {
 		memset(buffer[curBuffer] + renderedFrames * 2, 0, byteCount);
 	}
 	SLresult result = (*bqPlayerBufferQueue)->Enqueue(bqPlayerBufferQueue, buffer[curBuffer], sizeInBytes);
-	CheckResult(result, StringFromFormat("Failed to enqueue: %d %d", renderedFrames, sizeInBytes).c_str());
+
+	// TODO: get rid of this snprintf too
+	CheckResult(result, "Failed to enqueue");
 	// Comment from sample code:
 	// the most likely other result is SL_RESULT_BUFFER_INSUFFICIENT,
 	// which for this code example would indicate a programming error