diff --git a/CMakeLists.txt b/CMakeLists.txt index 27a63a02ce..6063ede1c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2481,6 +2481,7 @@ if(UNITTEST) unittest/TestIRPassSimplify.cpp unittest/TestX64Emitter.cpp unittest/TestVertexJit.cpp + unittest/TestVFS.cpp unittest/TestRiscVEmitter.cpp unittest/TestSoftwareGPUJit.cpp unittest/TestThreadManager.cpp diff --git a/Common/File/VFS/VFS.h b/Common/File/VFS/VFS.h index 5d585e309e..a3dc73e2e3 100644 --- a/Common/File/VFS/VFS.h +++ b/Common/File/VFS/VFS.h @@ -37,6 +37,7 @@ class VFSInterface { public: virtual ~VFSInterface() {} virtual uint8_t *ReadFile(const char *path, size_t *size) = 0; + // If listing already contains files, it'll be cleared. virtual bool GetFileListing(const char *path, std::vector *listing, const char *filter = nullptr) = 0; }; diff --git a/Common/File/VFS/ZipFileReader.cpp b/Common/File/VFS/ZipFileReader.cpp index c789ff899f..6f784ee6de 100644 --- a/Common/File/VFS/ZipFileReader.cpp +++ b/Common/File/VFS/ZipFileReader.cpp @@ -38,10 +38,13 @@ ZipFileReader *ZipFileReader::Create(const Path &zipFile, const char *inZipPath, return nullptr; } - ZipFileReader *reader = new ZipFileReader(); - reader->zip_file_ = zip_file; - truncate_cpy(reader->inZipPath_, inZipPath); - return reader; + // The inZipPath is supposed to be a folder, and internally in this class, we suffix + // folder paths with '/', matching how the zip library works. + std::string path = inZipPath; + if (!path.empty() && path.back() != '/') { + path.push_back('/'); + } + return new ZipFileReader(zip_file, path); } ZipFileReader::~ZipFileReader() { @@ -50,16 +53,15 @@ ZipFileReader::~ZipFileReader() { } uint8_t *ZipFileReader::ReadFile(const char *path, size_t *size) { - char temp_path[2048]; - snprintf(temp_path, sizeof(temp_path), "%s%s", inZipPath_, path); + std::string temp_path = inZipPath_ + path; std::lock_guard guard(lock_); // Figure out the file size first. struct zip_stat zstat; - zip_stat(zip_file_, temp_path, ZIP_FL_NOCASE | ZIP_FL_UNCHANGED, &zstat); - zip_file *file = zip_fopen(zip_file_, temp_path, ZIP_FL_NOCASE | ZIP_FL_UNCHANGED); + zip_stat(zip_file_, temp_path.c_str(), ZIP_FL_NOCASE | ZIP_FL_UNCHANGED, &zstat); + zip_file *file = zip_fopen(zip_file_, temp_path.c_str(), ZIP_FL_NOCASE | ZIP_FL_UNCHANGED); if (!file) { - ERROR_LOG(IO, "Error opening %s from ZIP", temp_path); + ERROR_LOG(IO, "Error opening %s from ZIP", temp_path.c_str()); return 0; } uint8_t *contents = new uint8_t[zstat.size + 1]; @@ -72,8 +74,10 @@ uint8_t *ZipFileReader::ReadFile(const char *path, size_t *size) { } bool ZipFileReader::GetFileListing(const char *orig_path, std::vector *listing, const char *filter = 0) { - char path[2048]; - snprintf(path, sizeof(path), "%s%s", inZipPath_, orig_path); + std::string path = std::string(inZipPath_) + orig_path; + if (!path.empty() && path.back() != '/') { + path.push_back('/'); + } std::set filters; std::string tmp; @@ -95,17 +99,27 @@ bool ZipFileReader::GetFileListing(const char *orig_path, std::vector files; std::set directories; - GetZipListings(path, files, directories); + bool success = GetZipListings(path, files, directories); + if (!success) { + // This means that no file prefix matched the path. + return false; + } + + listing->clear(); + + INFO_LOG(SYSTEM, "Listing %s", orig_path); for (auto diter = directories.begin(); diter != directories.end(); ++diter) { File::FileInfo info; info.name = *diter; // Remove the "inzip" part of the fullname. - info.fullName = Path(std::string(path).substr(strlen(inZipPath_))) / *diter; + std::string relativePath = std::string(path).substr(inZipPath_.size()); + info.fullName = Path(relativePath + *diter); info.exists = true; info.isWritable = false; info.isDirectory = true; + INFO_LOG(SYSTEM, "Found file: %s (%s)", info.name.c_str(), info.fullName.c_str()); listing->push_back(info); } @@ -113,7 +127,8 @@ bool ZipFileReader::GetFileListing(const char *orig_path, std::vectorpush_back(info); } @@ -130,39 +146,44 @@ bool ZipFileReader::GetFileListing(const char *orig_path, std::vector &files, std::set &directories) { - size_t pathlen = strlen(path); - if (pathlen == 1 && path[0] == '/') { - // Root. We simply use a zero length string. - pathlen = 0; - } +// path here is from the root, so inZipPath needs to already be added. +bool ZipFileReader::GetZipListings(const std::string &path, std::set &files, std::set &directories) { + _dbg_assert_(path.empty() || path.back() == '/'); std::lock_guard guard(lock_); int numFiles = zip_get_num_files(zip_file_); + bool anyPrefixMatched = false; for (int i = 0; i < numFiles; i++) { const char* name = zip_get_name(zip_file_, i, 0); if (!name) continue; // shouldn't happen, I think - if (!memcmp(name, path, pathlen)) { - const char *slashPos = strchr(name + pathlen + 1, '/'); + if (startsWith(name, path)) { + if (strlen(name) == path.size()) { + // Don't want to return the same folder. + continue; + } + const char *slashPos = strchr(name + path.size(), '/'); if (slashPos != 0) { + anyPrefixMatched = true; // A directory. Let's pick off the only part we care about. - int offset = pathlen; + size_t offset = path.size(); std::string dirName = std::string(name + offset, slashPos - (name + offset)); + // We might get a lot of these if the tree is deep. The std::set deduplicates. directories.insert(dirName); } else { + anyPrefixMatched = true; // It's a file. - const char *fn = name + pathlen; + const char *fn = name + path.size(); files.insert(std::string(fn)); } } } + return anyPrefixMatched; } bool ZipFileReader::GetFileInfo(const char *path, File::FileInfo *info) { struct zip_stat zstat; - char temp_path[1024]; - snprintf(temp_path, sizeof(temp_path), "%s%s", inZipPath_, path); + std::string temp_path = inZipPath_ + path; // Clear some things to start. info->isDirectory = false; @@ -171,7 +192,7 @@ bool ZipFileReader::GetFileInfo(const char *path, File::FileInfo *info) { { std::lock_guard guard(lock_); - if (0 != zip_stat(zip_file_, temp_path, ZIP_FL_NOCASE | ZIP_FL_UNCHANGED, &zstat)) { + if (0 != zip_stat(zip_file_, temp_path.c_str(), ZIP_FL_NOCASE | ZIP_FL_UNCHANGED, &zstat)) { // ZIP files do not have real directories, so we'll end up here if we // try to stat one. For now that's fine. info->exists = false; diff --git a/Common/File/VFS/ZipFileReader.h b/Common/File/VFS/ZipFileReader.h index 44a3292ede..30c61ccc76 100644 --- a/Common/File/VFS/ZipFileReader.h +++ b/Common/File/VFS/ZipFileReader.h @@ -40,9 +40,11 @@ public: } private: - void GetZipListings(const char *path, std::set &files, std::set &directories); + ZipFileReader(zip *zip_file, const std::string &inZipPath) : zip_file_(zip_file), inZipPath_(inZipPath) {} + // Path has to be either an empty string, or a string ending with a /. + bool GetZipListings(const std::string &path, std::set &files, std::set &directories); zip *zip_file_ = nullptr; std::mutex lock_; - char inZipPath_[256]; + std::string inZipPath_; }; diff --git a/GPU/Common/TextureReplacer.cpp b/GPU/Common/TextureReplacer.cpp index 090f6a5e29..6b72e879f1 100644 --- a/GPU/Common/TextureReplacer.cpp +++ b/GPU/Common/TextureReplacer.cpp @@ -160,7 +160,7 @@ bool TextureReplacer::LoadIni() { } INFO_LOG(G3D, "Loading extra texture ini: %s", overrideFilename.c_str()); - if (!LoadIniValues(overrideIni, dir, true)) { + if (!LoadIniValues(overrideIni, nullptr, true)) { delete dir; return false; } @@ -261,26 +261,28 @@ bool TextureReplacer::LoadIniValues(IniFile &ini, VFSBackend *dir, bool isOverri // Scan the root of the texture folder/zip and preinitialize the hash map. std::vector filesInRoot; - dir->GetFileListing("/", &filesInRoot, nullptr); - for (auto file : filesInRoot) { - if (file.isDirectory) - continue; - if (file.name.empty() || file.name[0] == '.') - continue; - Path path(file.name); - std::string ext = path.GetFileExtension(); + if (dir) { + dir->GetFileListing("", &filesInRoot, nullptr); + for (auto file : filesInRoot) { + if (file.isDirectory) + continue; + if (file.name.empty() || file.name[0] == '.') + continue; + Path path(file.name); + std::string ext = path.GetFileExtension(); - std::string hash = file.name.substr(0, file.name.size() - ext.size()); - if (!((hash.size() >= 26 && hash.size() <= 27 && hash[24] == '_') || hash.size() == 24)) { - continue; - } - // OK, it's hash-like enough to try to parse it into the map. - if (equalsNoCase(ext, ".ktx2") || equalsNoCase(ext, ".png") || equalsNoCase(ext, ".dds")) { - ReplacementCacheKey key(0, 0); - int level = 0; // sscanf might fail to pluck the level, but that's ok, we default to 0. sscanf doesn't write to non-matched outputs. - if (sscanf(hash.c_str(), "%16llx%8x_%d", &key.cachekey, &key.hash, &level) >= 1) { - INFO_LOG(G3D, "hash-like file in root, adding: %s", file.name.c_str()); - filenameMap[key][level] = file.name; + std::string hash = file.name.substr(0, file.name.size() - ext.size()); + if (!((hash.size() >= 26 && hash.size() <= 27 && hash[24] == '_') || hash.size() == 24)) { + continue; + } + // OK, it's hash-like enough to try to parse it into the map. + if (equalsNoCase(ext, ".ktx2") || equalsNoCase(ext, ".png") || equalsNoCase(ext, ".dds") || equalsNoCase(ext, ".zim")) { + ReplacementCacheKey key(0, 0); + int level = 0; // sscanf might fail to pluck the level, but that's ok, we default to 0. sscanf doesn't write to non-matched outputs. + if (sscanf(hash.c_str(), "%16llx%8x_%d", &key.cachekey, &key.hash, &level) >= 1) { + // INFO_LOG(G3D, "hash-like file in root, adding: %s", file.name.c_str()); + filenameMap[key][level] = file.name; + } } } } diff --git a/android/jni/Android.mk b/android/jni/Android.mk index 38301fb436..64005a3f42 100644 --- a/android/jni/Android.mk +++ b/android/jni/Android.mk @@ -811,6 +811,7 @@ ifeq ($(UNITTEST),1) $(SRC)/unittest/TestSoftwareGPUJit.cpp \ $(SRC)/unittest/TestThreadManager.cpp \ $(SRC)/unittest/TestVertexJit.cpp \ + $(SRC)/unittest/TestVFS.cpp \ $(TESTARMEMITTER_FILE) \ $(SRC)/unittest/UnitTest.cpp diff --git a/source_assets/ziptest.zip b/source_assets/ziptest.zip new file mode 100644 index 0000000000..57e117dc0a Binary files /dev/null and b/source_assets/ziptest.zip differ diff --git a/unittest/TestVFS.cpp b/unittest/TestVFS.cpp new file mode 100644 index 0000000000..8eb57886d0 --- /dev/null +++ b/unittest/TestVFS.cpp @@ -0,0 +1,94 @@ +#include +#include + +#include "Common/Log.h" +#include "Common/File/VFS/ZipFileReader.h" + +#include "UnitTest.h" + +static bool CheckContainsDir(const std::vector &listing, const char *name) { + for (auto &file : listing) { + if (file.name == name && file.isDirectory) { + return true; + } + } + return false; +} + +static bool CheckContainsFile(const std::vector &listing, const char *name) { + for (auto &file : listing) { + if (file.name == name && !file.isDirectory) { + return true; + } + } + return false; +} + +// ziptest.zip file structure: +// +// ziptest/ +// data/ +// a/ +// in_a.txt +// b/ +// in_b.txt +// argh.txt +// big.txt +// lang/ +// en_us.txt +// sv_se.txt +// langregion.txt +// in_root.txt + +// TODO: Also test the filter. +bool TestZipFile() { + // First, check things relative to root, with an empty internal path. + Path zipPath = Path("../source_assets/ziptest.zip"); + if (!File::Exists(zipPath)) { + zipPath = Path("source_assets/ziptest.zip"); + } + + ZipFileReader *dir = ZipFileReader::Create(zipPath, "", true); + EXPECT_TRUE(dir != nullptr); + + std::vector listing; + EXPECT_TRUE(dir->GetFileListing("", &listing, nullptr)); + EXPECT_EQ_INT(listing.size(), 2); + EXPECT_TRUE(CheckContainsDir(listing, "ziptest")); + EXPECT_TRUE(CheckContainsFile(listing, "in_root.txt")); + EXPECT_FALSE(dir->GetFileListing("ziptestwrong", &listing, nullptr)); + + // Next, do a file listing in a directory, but keep the root. + EXPECT_TRUE(dir->GetFileListing("ziptest", &listing, nullptr)); + EXPECT_EQ_INT(listing.size(), 3); + EXPECT_TRUE(CheckContainsDir(listing, "data")); + EXPECT_TRUE(CheckContainsDir(listing, "lang")); + EXPECT_TRUE(CheckContainsFile(listing, "langregion.txt")); + delete dir; + + // Next, we'll destroy the reader and create a new one based in a subdirectory. + dir = ZipFileReader::Create(zipPath, "ziptest/data", true); + EXPECT_TRUE(dir != nullptr); + EXPECT_TRUE(dir->GetFileListing("", &listing, nullptr)); + EXPECT_EQ_INT(listing.size(), 4); + EXPECT_TRUE(CheckContainsDir(listing, "a")); + EXPECT_TRUE(CheckContainsDir(listing, "b")); + EXPECT_TRUE(CheckContainsFile(listing, "argh.txt")); + EXPECT_TRUE(CheckContainsFile(listing, "big.txt")); + + EXPECT_TRUE(dir->GetFileListing("a", &listing, nullptr)); + EXPECT_TRUE(CheckContainsFile(listing, "in_a.txt")); + EXPECT_EQ_INT(listing.size(), 1); + EXPECT_TRUE(dir->GetFileListing("b", &listing, nullptr)); + EXPECT_TRUE(CheckContainsFile(listing, "in_b.txt")); + EXPECT_EQ_INT(listing.size(), 1); + delete dir; + + return true; +} + +bool TestVFS() { + if (!TestZipFile()) + return false; + return true; +} diff --git a/unittest/UnitTest.cpp b/unittest/UnitTest.cpp index 605ba0bfb8..0c41d6302a 100644 --- a/unittest/UnitTest.cpp +++ b/unittest/UnitTest.cpp @@ -930,6 +930,7 @@ bool TestShaderGenerators(); bool TestSoftwareGPUJit(); bool TestIRPassSimplify(); bool TestThreadManager(); +bool TestVFS(); TestItem availableTests[] = { #if PPSSPP_ARCH(ARM64) || PPSSPP_ARCH(AMD64) || PPSSPP_ARCH(X86) @@ -968,6 +969,7 @@ TestItem availableTests[] = { TEST_ITEM(DepthMath), TEST_ITEM(InputMapping), TEST_ITEM(EscapeMenuString), + TEST_ITEM(VFS), }; int main(int argc, const char *argv[]) { diff --git a/unittest/UnitTests.vcxproj b/unittest/UnitTests.vcxproj index 473429fa92..687314e43d 100644 --- a/unittest/UnitTests.vcxproj +++ b/unittest/UnitTests.vcxproj @@ -397,6 +397,7 @@ + true diff --git a/unittest/UnitTests.vcxproj.filters b/unittest/UnitTests.vcxproj.filters index 14344efb6a..eb084afaf0 100644 --- a/unittest/UnitTests.vcxproj.filters +++ b/unittest/UnitTests.vcxproj.filters @@ -16,6 +16,7 @@ +