Improve StorageFileLoader enough to actually kind of work

Common/ThreadPools.cpp

@@ -16,4 +16,3 @@ void GlobalThreadPool::Inititialize() {
 		initialized = true;
 	}
 }
-

Core/FileSystems/ISOFileSystem.cpp

@@ -187,7 +187,7 @@ ISOFileSystem::ISOFileSystem(IHandleAllocator *_hAlloc, BlockDevice *_blockDevic
 	treeroot->valid = false;
 
 	if (memcmp(desc.cd001, "CD001", 5)) {
-		ERROR_LOG(FILESYS, "ISO looks bogus? Giving up...");
+		ERROR_LOG(FILESYS, "ISO looks bogus, expected CD001 signature not present? Giving up...");
 		return;
 	}
 

Core/Loaders.cpp

@@ -35,29 +35,26 @@
 #include "Core/ELF/PBPReader.h"
 #include "Core/ELF/ParamSFO.h"
 
-// Gross, gross hack! But necessary for UWP, fitting it in neatly would be a major refactor
-FileLoader *g_OverriddenLoader;
-IdentifiedFileType g_OverriddenFiletype = FILETYPE_UNKNOWN;
+static std::map<std::string, std::unique_ptr<FileLoaderFactory>> factories;
 
-void OverrideNextLoader(FileLoader *fileLoader, IdentifiedFileType fileType) {
-	g_OverriddenLoader = fileLoader;
-	g_OverriddenFiletype = fileType;
+void RegisterFileLoaderFactory(std::string prefix, std::unique_ptr<FileLoaderFactory> factory) {
+	factories[prefix] = std::move(factory);
 }
 
 FileLoader *ConstructFileLoader(const std::string &filename) {
 	if (filename.find("http://") == 0 || filename.find("https://") == 0)
 		return new CachingFileLoader(new DiskCachingFileLoader(new RetryingFileLoader(new HTTPFileLoader(filename))));
-	if (filename == "override://") {
-		return g_OverriddenLoader;
+
+	for (auto &iter : factories) {
+		if (startsWith(iter.first, filename)) {
+			return iter.second->ConstructFileLoader(filename);
+		}
 	}
 	return new LocalFileLoader(filename);
 }
 
 // TODO : improve, look in the file more
 IdentifiedFileType Identify_File(FileLoader *fileLoader) {
-	if (g_OverriddenFiletype != FILETYPE_UNKNOWN)
-		return g_OverriddenFiletype;
-
 	if (fileLoader == nullptr) {
 		ERROR_LOG(LOADER, "Invalid fileLoader");
 		return IdentifiedFileType::ERROR_IDENTIFYING;

Core/Loaders.h

@@ -18,6 +18,7 @@
 #pragma	once
 
 #include <string>
+#include <memory>
 
 #include "Common/CommonTypes.h"
 
@@ -102,8 +103,11 @@ std::string ResolvePBPFile(const std::string &filename);
 
 IdentifiedFileType Identify_File(FileLoader *fileLoader);
 
-void RegisterFileLoaderFactory(std::string name, std::function<FileLoader*(std::string)> factoryFunc);
-void OverrideNextLoader(FileLoader *fileLoader, IdentifiedFileType fileType);
+class FileLoaderFactory {
+public:
+	virtual FileLoader *ConstructFileLoader(const std::string &filename) = 0;
+};
+void RegisterFileLoaderFactory(std::string name, std::unique_ptr<FileLoaderFactory> factory);
 
 // Can modify the string filename, as it calls IdentifyFile above.
 bool LoadFile(FileLoader **fileLoaderPtr, std::string *error_string);

UI/GameInfoCache.cpp

@@ -222,6 +222,8 @@ bool GameInfo::LoadFromPath(const std::string &gamePath) {
 	if (filePath_ != gamePath) {
 		delete fileLoader;
 		fileLoader = ConstructFileLoader(gamePath);
+		if (!fileLoader)
+			return false;
 		filePath_ = gamePath;
 
 		// This is a fallback title, while we're loading / if unable to load.
@@ -413,7 +415,6 @@ public:
 					pbp.GetSubFileAsString(PBP_ICON0_PNG, &info_->iconTextureData);
 				} else {
 					// Read standard icon
-					DEBUG_LOG(LOADER, "Loading unknown.png because a PBP was missing an icon");
 					ReadVFSToString("unknown.png", &info_->iconTextureData, &info_->lock);
 				}
 				info_->iconDataLoaded = true;

UWP/PPSSPP_UWPMain.cpp

@@ -18,11 +18,13 @@
 #include "net/http_client.h"
 #include "net/resolve.h"
 #include "base/display.h"
+#include "thread/threadutil.h"
 #include "util/text/utf8.h"
 #include "Common/DirectXHelper.h"
 #include "NKCodeFromWindowsSystem.h"
 #include "XAudioSoundStream.h"
 #include "UWPHost.h"
+#include "UWPUtil.h"
 #include "StorageFileLoader.h"
 
 using namespace UWP;
@@ -153,6 +155,12 @@ bool PPSSPP_UWPMain::Render() {
 	ctx_->GetDrawContext()->HandleEvent(Draw::Event::PRESENTED, 0, 0, nullptr, nullptr);
 	NativeUpdate();
 
+	static bool hasSetThreadName = false;
+	if (!hasSetThreadName) {
+		setCurrentThreadName("UWPRenderThread");
+		hasSetThreadName = true;
+	}
+
 	time_update();
 	auto context = m_deviceResources->GetD3DDeviceContext();
 
@@ -258,7 +266,8 @@ void PPSSPP_UWPMain::OnSuspend() {
 }
 
 void PPSSPP_UWPMain::LoadStorageFile(StorageFile ^file) {
-	OverrideNextLoader(new StorageFileLoader(file), FILETYPE_PSP_ISO);
+	std::unique_ptr<FileLoaderFactory> factory(new StorageFileLoaderFactory(file, IdentifiedFileType::PSP_ISO));
+	RegisterFileLoaderFactory("override://", std::move(factory));
 	NativeMessageReceived("boot", "override://");
 }
 
@@ -319,13 +328,20 @@ void System_SendMessage(const char *command, const char *parameter) {
 	} else if (!strcmp(command, "browse_file")) {
 		auto picker = ref new Windows::Storage::Pickers::FileOpenPicker();
 		picker->ViewMode = Pickers::PickerViewMode::List;
+
+		// These are single files that can be loaded directly using StorageFileLoader.
 		picker->FileTypeFilter->Append(".cso");
 		picker->FileTypeFilter->Append(".iso");
-		picker->FileTypeFilter->Append(".bin");
+
+		// Can't load these this way currently, they require mounting the underlying folder.
+		// picker->FileTypeFilter->Append(".bin");
+		// picker->FileTypeFilter->Append(".elf");
 		picker->SuggestedStartLocation = Pickers::PickerLocationId::DocumentsLibrary;
 
 		create_task(picker->PickSingleFileAsync()).then([](StorageFile ^file){
+			if (file) {
 				g_main->LoadStorageFile(file);
+			}
 			/*
 			std::thread([file] {
 				create_task(file->OpenReadAsync()).then([](IRandomAccessStreamWithContentType^ imgStream) {
@@ -341,8 +357,7 @@ void System_SendMessage(const char *command, const char *parameter) {
 }
 
 void LaunchBrowser(const char *url) {
-	Platform::String ^pstr = ref new Platform::String(ConvertUTF8ToWString(url).c_str());
-	auto uri = ref new Windows::Foundation::Uri(pstr);
+	auto uri = ref new Windows::Foundation::Uri(ToPlatformString(url));
 
 	create_task(Windows::System::Launcher::LaunchUriAsync(uri)).then([](bool b) {});
 }

UWP/StorageFileLoader.cpp

@@ -1,7 +1,10 @@
 #include "pch.h"
 #include "ppltasks.h"
+#include "base/logging.h"
+#include "file/file_util.h"
+#include "thread/threadutil.h"
 #include "StorageFileLoader.h"
-
+#include "UWPUtil.h"
 
 using namespace Concurrency;
 using namespace Windows::Storage;
@@ -9,54 +12,93 @@ using namespace Windows::Storage::Streams;
 
 StorageFileLoader::StorageFileLoader(Windows::Storage::StorageFile ^file) {
 	file_ = file;
-	auto opentask = create_task(file->OpenReadAsync());
-	opentask.then([this](IRandomAccessStreamWithContentType ^stream) {
-		stream_ = stream;
-		active_ = true;
-		thread_ = std::thread([this]() { this->threadfunc(); });
-	});
-	auto attrtask = create_task(file->GetBasicPropertiesAsync());
-	attrtask.then([this](Windows::Storage::FileProperties::BasicProperties ^props) {
-		size_ = props->Size;
-	});
+	path_ = FromPlatformString(file_->Path);
+	thread_.reset(new std::thread([this]() { this->threadfunc(); }));
 }
 
 StorageFileLoader::~StorageFileLoader() {
 	active_ = false;
-	thread_.join();
+	operationRequested_ = false;
+	cond_.notify_all();
+	thread_->join();
 }
 
 void StorageFileLoader::threadfunc() {
+	setCurrentThreadName("StorageFileLoader");
+
+	initMutex.lock();
+
+	auto opentask = create_task(file_->OpenReadAsync()).then([this](IRandomAccessStreamWithContentType ^stream) {
+		stream_ = stream;
+		active_ = true;
+	});
+
+	try {
+		opentask.wait();
+	}
+	catch (const std::exception& e) {
+		operationFailed_ = true;
+		// TODO: What do we do?
+		const char *what = e.what();
+		ILOG("%s", what);
+	}
+
+	auto sizetask = create_task(file_->GetBasicPropertiesAsync()).then([this](Windows::Storage::FileProperties::BasicProperties ^props) {
+		size_ = props->Size;
+	});
+	try {
+		sizetask.wait();
+	}
+	catch (const std::exception& e) {
+		const char *what = e.what();
+		ILOG("%s", what);
+	}
+
+	initMutex.unlock();
+
 	std::unique_lock<std::mutex> lock(mutex_);
 	while (active_) {
+		if (!operationRequested_) {
 			cond_.wait(lock);
-		std::unique_lock<std::mutex> lock(mutexResponse_);
-		while (operations_.size()) {
-			Operation op = operations_.front();
-			operations_.pop();
-
-			switch (op.type) {
-			case OpType::READ: {
-				op.buffer = ref new Streams::Buffer(op.size);
-				auto task = create_task(stream_->ReadAsync(op.buffer, op.size, Streams::InputStreamOptions::None));
+		}
+		if (operationRequested_) {
+			switch (operation_.type) {
+			case OpType::READ_AT: {
+				Streams::Buffer ^buf = ref new Streams::Buffer(operation_.size);
+				operationFailed_ = false;
+				stream_->Seek(operation_.offset);
+				auto task = create_task(stream_->ReadAsync(buf, operation_.size, Streams::InputStreamOptions::None));
+				Streams::IBuffer ^output = nullptr;
+				try {
 					task.wait();
+					output = task.get();
+				} catch (const std::exception& e) {
+					operationFailed_ = true;
+					const char *what = e.what();
+					ILOG("%s", what);
+				}
+				operationRequested_ = false;
+				std::unique_lock<std::mutex> lock(mutexResponse_);
+				response_.buffer = output;
+				responseAvailable_ = true;
+				condResponse_.notify_one();
 				break;
-				responses_.push(op);
 			}
 			default:
+				ELOG("Unknown operation");
+				operationRequested_ = false;
 				break;
 			}
 		}
-		// OK, done with all operations.
-		condResponse_.notify_one();
 	}
 }
 
 bool StorageFileLoader::Exists() {
-	return true;
+	return file_ != nullptr;
 }
+
 bool StorageFileLoader::ExistsFast() {
-	return true;
+	return file_ != nullptr;
 }
 
 bool StorageFileLoader::IsDirectory() {
@@ -64,20 +106,24 @@ bool StorageFileLoader::IsDirectory() {
 }
 
 s64 StorageFileLoader::FileSize() {
+	EnsureOpen();
 	if (size_ == -1)
 		__debugbreak();  // crude race condition detection
 	return size_;
 }
 
 std::string StorageFileLoader::Path() const {
-	return "";
+	return path_;
 }
 
 std::string StorageFileLoader::Extension() {
-	return "";
+	return "." + getFileExtension(path_);
 }
 
 void StorageFileLoader::EnsureOpen() {
+	// UGLY!
+	while (!thread_)
+		Sleep(100);
 	while (size_ == -1)
 		Sleep(100);
 }
@@ -89,40 +135,39 @@ void StorageFileLoader::Seek(s64 absolutePos) {
 }
 
 size_t StorageFileLoader::Read(size_t bytes, size_t count, void *data, Flags flags) {
-	EnsureOpen();
-	{
-		std::unique_lock<std::mutex> lock(mutex_);
-		operations_.push(Operation{ OpType::READ, seekPos_, (int64_t)(bytes * count) });
-		cond_.notify_one();
-	}
-	// OK, now wait for response...
-	{
-		std::unique_lock<std::mutex> responseLock(mutexResponse_);
-		condResponse_.wait(responseLock);
-		Operation resp = responses_.front();
-		responses_.pop();
-		DataReader ^rd = DataReader::FromBuffer(resp.buffer);
-		Platform::Array<uint8_t> ^bytearray = ref new Platform::Array<uint8_t>(resp.buffer->Length);
-		rd->ReadBytes(bytearray);
-		memcpy(data, bytearray->Data, bytes * count);
-		return 0;
-	}
+	size_t bytesRead = ReadAt(seekPos_, bytes, count, data, flags);
+	seekPos_ += bytesRead;
+	return bytesRead;
 }
 
 size_t StorageFileLoader::ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags) {
 	EnsureOpen();
+	if (operationRequested_)
+		Crash();
 	{
 		std::unique_lock<std::mutex> lock(mutex_);
-		operations_.push(Operation{ OpType::READ, absolutePos, (int64_t)(bytes * count) });
+		operation_.type = OpType::READ_AT;
+		operation_.offset = absolutePos;
+		operation_.size = (int64_t)(bytes * count);
+		operationRequested_ = true;
 		cond_.notify_one();
 	}
+
 	// OK, now wait for response...
 	{
 		std::unique_lock<std::mutex> responseLock(mutexResponse_);
 		condResponse_.wait(responseLock);
-		Operation resp = responses_.front();
-		responses_.pop();
-		// memcpy(data,  bytes * count, )
-		return 0;
+		DataReader ^rd = DataReader::FromBuffer(response_.buffer);
+		size_t len = response_.buffer->Length;
+		Platform::Array<uint8_t> ^bytearray = ref new Platform::Array<uint8_t>((unsigned int)len);
+		rd->ReadBytes(bytearray);
+		memcpy(data, bytearray->Data, len);
+		responseAvailable_ = false;
+		response_.buffer = nullptr;
+		return len / bytes;
 	}
 }
+
+FileLoader *StorageFileLoaderFactory::ConstructFileLoader(const std::string &filename) {
+	return file_ ? new StorageFileLoader(file_) : nullptr;
+}

UWP/StorageFileLoader.h

@@ -6,13 +6,14 @@
 #include <mutex>
 #include <condition_variable>
 #include <queue>
+#include <string>
 
 #include "Common/CommonTypes.h"
 #include "Core/Loaders.h"
 
 // This thing is a terrible abomination that wraps asynchronous file access behind a synchronous interface,
 // completely defeating MS' design goals for StorageFile. But hey, you gotta do what you gotta do.
-
+// This opens a stream attached to the passed-in file. Multiple of these can be created against one StorageFile.
 class StorageFileLoader : public FileLoader {
 public:
 	StorageFileLoader(Windows::Storage::StorageFile ^file);
@@ -26,37 +27,64 @@ public:
 	std::string Path() const override;
 	std::string Extension() override;
 	void Seek(s64 absolutePos) override;
-	size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE);
-	size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE);
+
+	size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override;
+	size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override;
 
 private:
 	void threadfunc();
 	void EnsureOpen();
 
 	enum class OpType {
-		READ,
+		NONE,
+		READ_AT,
 	};
 
 	struct Operation {
 		OpType type;
 		int64_t offset;
 		int64_t size;
-		Windows::Storage::Streams::Buffer ^buffer;
+	};
+
+	struct Response {
+		Windows::Storage::Streams::IBuffer ^buffer;
 	};
 
 	bool active_ = false;
 	int64_t size_ = -1;
-	std::thread thread_;
+	std::unique_ptr<std::thread> thread_;
+
 	Windows::Storage::StorageFile ^file_;
 	Windows::Storage::Streams::IRandomAccessStreamWithContentType ^stream_;
+	std::string path_;
+
+	bool operationRequested_ = false;
+	Operation operation_{ OpType::NONE, 0, 0 };
 	std::condition_variable cond_;
 	std::mutex mutex_;
 
+	bool operationFailed_ = false;
+
+	bool responseAvailable_ = false;
+	Response response_;
 	std::condition_variable condResponse_;
 	std::mutex mutexResponse_;
 
-	std::queue<Operation> operations_;
 	int64_t seekPos_ = 0;
-
-	std::queue<Operation> responses_;
+};
+
+class StorageFileLoaderFactory : public FileLoaderFactory {
+public:
+	StorageFileLoaderFactory(Windows::Storage::StorageFile ^file, IdentifiedFileType fileType) : file_(file), fileType_(fileType) { }
+	FileLoader *ConstructFileLoader(const std::string &filename) override;
+
+private:
+	Windows::Storage::StorageFile ^file_;
+	IdentifiedFileType fileType_;
+};
+
+// Similar to StorageFileLoader but for directory browsing.
+class StorageDirectoryWrapper {
+private:
+	std::thread thread_;
 };

UWP/UWP.vcxproj

@@ -237,6 +237,7 @@
     <ClInclude Include="pch.h" />
     <ClInclude Include="StorageFileLoader.h" />
     <ClInclude Include="UWPHost.h" />
+    <ClInclude Include="UWPUtil.h" />
     <ClInclude Include="XAudioSoundStream.h" />
   </ItemGroup>
   <ItemGroup>

UWP/UWP.vcxproj.filters

@@ -60,6 +60,7 @@
     <ClInclude Include="StorageFileLoader.h" />
     <ClInclude Include="UWPHost.h" />
     <ClInclude Include="..\Windows\XinputDevice.h" />
+    <ClInclude Include="UWPUtil.h" />
   </ItemGroup>
   <ItemGroup>
     <Image Include="Assets\StoreLogo.png">

UWP/UWPUtil.h

@@ -0,0 +1,12 @@
+#pragma once
+
+#include "util/text/utf8.h"
+
+inline Platform::String ^ToPlatformString(std::string str) {
+	return ref new Platform::String(ConvertUTF8ToWString(str).c_str());
+}
+
+inline std::string FromPlatformString(Platform::String ^str) {
+	std::wstring wstr(str->Data());
+	return ConvertWStringToUTF8(wstr);
+}

UWP/XAudioSoundStream.cpp

@@ -1,7 +1,9 @@
 #include "pch.h"
 #include <XAudio2.h>
 
+#include "thread/threadutil.h"
 #include "XAudioSoundStream.h"
+
 #include <process.h>
 
 #define BUFSIZE 0x80000U
@@ -93,6 +95,7 @@ bool XAudioBackend::RunSound() {
 
   thread_ = (HANDLE)_beginthreadex( 0, 0, []( void* param ) 
   {
+		setCurrentThreadName("XAudio2");
     XAudioBackend *backend = (XAudioBackend *)param;
     backend->PollLoop();
     return 0U;