From cd3c3ebadb8d288741d71d58b13fdfb269e279dd Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 1 Mar 2024 18:42:21 +0100 Subject: [PATCH 1/5] Use VFS::Path::Normalized for ResourceManager cache key --- components/resource/resourcemanager.hpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/resource/resourcemanager.hpp b/components/resource/resourcemanager.hpp index b2427c308a..e2626665c8 100644 --- a/components/resource/resourcemanager.hpp +++ b/components/resource/resourcemanager.hpp @@ -3,6 +3,8 @@ #include +#include + #include "objectcache.hpp" namespace VFS @@ -70,11 +72,11 @@ namespace Resource double mExpiryDelay; }; - class ResourceManager : public GenericResourceManager + class ResourceManager : public GenericResourceManager { public: explicit ResourceManager(const VFS::Manager* vfs, double expiryDelay) - : GenericResourceManager(vfs, expiryDelay) + : GenericResourceManager(vfs, expiryDelay) { } }; From 79b73e45a12b322f611c7004ef8eb4591c12e904 Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 1 Mar 2024 18:57:43 +0100 Subject: [PATCH 2/5] Replace std::filesystem::path by std::string and std::string_view in nif code It's used only for error reporting. --- apps/niftest/niftest.cpp | 4 ++-- components/nif/exception.hpp | 13 ++++++++----- components/nif/niffile.hpp | 12 ++++++------ components/nifbullet/bulletnifloader.cpp | 2 +- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/apps/niftest/niftest.cpp b/apps/niftest/niftest.cpp index fe60238cd5..b37d85d739 100644 --- a/apps/niftest/niftest.cpp +++ b/apps/niftest/niftest.cpp @@ -65,10 +65,10 @@ void readNIF( std::cout << " from '" << Files::pathToUnicodeString(isBSA(source) ? source.filename() : source) << "'"; std::cout << std::endl; } - std::filesystem::path fullPath = !source.empty() ? source / path : path; + const std::filesystem::path fullPath = !source.empty() ? source / path : path; try { - Nif::NIFFile file(fullPath); + Nif::NIFFile file(Files::pathToUnicodeString(fullPath)); Nif::Reader reader(file, nullptr); if (vfs != nullptr) reader.parse(vfs->get(pathStr)); diff --git a/components/nif/exception.hpp b/components/nif/exception.hpp index 15f0e76d70..b123d6dc4f 100644 --- a/components/nif/exception.hpp +++ b/components/nif/exception.hpp @@ -1,18 +1,21 @@ #ifndef OPENMW_COMPONENTS_NIF_EXCEPTION_HPP #define OPENMW_COMPONENTS_NIF_EXCEPTION_HPP -#include #include #include -#include - namespace Nif { struct Exception : std::runtime_error { - explicit Exception(const std::string& message, const std::filesystem::path& path) - : std::runtime_error("NIFFile Error: " + message + " when reading " + Files::pathToUnicodeString(path)) + explicit Exception(std::string_view message, std::string_view path) + : std::runtime_error([&] { + std::string result = "NIFFile Error: "; + result += message; + result += " when reading "; + result += path; + return result; + }()) { } }; diff --git a/components/nif/niffile.hpp b/components/nif/niffile.hpp index 993e9b7eea..6bff30a225 100644 --- a/components/nif/niffile.hpp +++ b/components/nif/niffile.hpp @@ -4,7 +4,7 @@ #define OPENMW_COMPONENTS_NIF_NIFFILE_HPP #include -#include +#include #include #include @@ -45,7 +45,7 @@ namespace Nif std::uint32_t mBethVersion = 0; /// File name, used for error messages and opening the file - std::filesystem::path mPath; + std::string mPath; std::string mHash; /// Record list @@ -56,7 +56,7 @@ namespace Nif bool mUseSkinning = false; - explicit NIFFile(const std::filesystem::path& path) + explicit NIFFile(std::string_view path) : mPath(path) { } @@ -77,7 +77,7 @@ namespace Nif std::size_t numRoots() const { return mFile->mRoots.size(); } /// Get the name of the file - const std::filesystem::path& getFilename() const { return mFile->mPath; } + const std::string& getFilename() const { return mFile->mPath; } const std::string& getHash() const { return mFile->mHash; } @@ -104,7 +104,7 @@ namespace Nif std::uint32_t& mBethVersion; /// File name, used for error messages and opening the file - std::filesystem::path& mFilename; + std::string_view mFilename; std::string& mHash; /// Record list @@ -144,7 +144,7 @@ namespace Nif void setUseSkinning(bool skinning); /// Get the name of the file - std::filesystem::path getFilename() const { return mFilename; } + std::string_view getFilename() const { return mFilename; } /// Get the version of the NIF format used std::uint32_t getVersion() const { return mVersion; } diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 0737d0a165..a57e8b3c06 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -50,7 +50,7 @@ namespace NifBullet if (node) roots.emplace_back(node); } - mShape->mFileName = Files::pathToUnicodeString(nif.getFilename()); + mShape->mFileName = nif.getFilename(); if (roots.empty()) { warn("Found no root nodes in NIF file " + mShape->mFileName); From a98ce7f76a6a0d78857e7a5476bc48f0c8f969fa Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 1 Mar 2024 19:02:41 +0100 Subject: [PATCH 3/5] Replace std::filesystem::path by std::string_view in Files::getHash argument --- apps/openmw_test_suite/files/hash.cpp | 9 ++++++--- components/files/hash.cpp | 10 ++++++---- components/files/hash.hpp | 4 ++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/apps/openmw_test_suite/files/hash.cpp b/apps/openmw_test_suite/files/hash.cpp index 6ad19713dc..32c8380422 100644 --- a/apps/openmw_test_suite/files/hash.cpp +++ b/apps/openmw_test_suite/files/hash.cpp @@ -10,6 +10,8 @@ #include #include +#include + #include "../testing_util.hpp" namespace @@ -35,7 +37,8 @@ namespace std::fill_n(std::back_inserter(content), 1, 'a'); std::istringstream stream(content); stream.exceptions(std::ios::failbit | std::ios::badbit); - EXPECT_THAT(getHash(fileName, stream), ElementsAre(9607679276477937801ull, 16624257681780017498ull)); + EXPECT_THAT(getHash(Files::pathToUnicodeString(fileName), stream), + ElementsAre(9607679276477937801ull, 16624257681780017498ull)); } TEST_P(FilesGetHash, shouldReturnHashForStringStream) @@ -44,7 +47,7 @@ namespace std::string content; std::fill_n(std::back_inserter(content), GetParam().mSize, 'a'); std::istringstream stream(content); - EXPECT_EQ(getHash(fileName, stream), GetParam().mHash); + EXPECT_EQ(getHash(Files::pathToUnicodeString(fileName), stream), GetParam().mHash); } TEST_P(FilesGetHash, shouldReturnHashForConstrainedFileStream) @@ -57,7 +60,7 @@ namespace std::fstream(file, std::ios_base::out | std::ios_base::binary) .write(content.data(), static_cast(content.size())); const auto stream = Files::openConstrainedFileStream(file, 0, content.size()); - EXPECT_EQ(getHash(file, *stream), GetParam().mHash); + EXPECT_EQ(getHash(Files::pathToUnicodeString(file), *stream), GetParam().mHash); } INSTANTIATE_TEST_SUITE_P(Params, FilesGetHash, diff --git a/components/files/hash.cpp b/components/files/hash.cpp index afb59b2e9e..1f1839ed0c 100644 --- a/components/files/hash.cpp +++ b/components/files/hash.cpp @@ -1,5 +1,4 @@ #include "hash.hpp" -#include "conversion.hpp" #include @@ -10,7 +9,7 @@ namespace Files { - std::array getHash(const std::filesystem::path& fileName, std::istream& stream) + std::array getHash(std::string_view fileName, std::istream& stream) { std::array hash{ 0, 0 }; try @@ -35,8 +34,11 @@ namespace Files } catch (const std::exception& e) { - throw std::runtime_error( - "Error while reading \"" + Files::pathToUnicodeString(fileName) + "\" to get hash: " + e.what()); + std::string message = "Error while reading \""; + message += fileName; + message += "\" to get hash: "; + message += e.what(); + throw std::runtime_error(message); } return hash; } diff --git a/components/files/hash.hpp b/components/files/hash.hpp index 0e6ce29ab5..48c373b971 100644 --- a/components/files/hash.hpp +++ b/components/files/hash.hpp @@ -3,12 +3,12 @@ #include #include -#include #include +#include namespace Files { - std::array getHash(const std::filesystem::path& fileName, std::istream& stream); + std::array getHash(std::string_view fileName, std::istream& stream); } #endif From 3ea3eeb6136fc7a98343cbbd87cb8de5da5bc9e9 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 9 Mar 2024 01:20:56 +0100 Subject: [PATCH 4/5] Use string_view for canOptimize --- components/resource/scenemanager.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/resource/scenemanager.cpp b/components/resource/scenemanager.cpp index 787f2e8441..26d719a106 100644 --- a/components/resource/scenemanager.cpp +++ b/components/resource/scenemanager.cpp @@ -778,12 +778,12 @@ namespace Resource } }; - bool canOptimize(const std::string& filename) + static bool canOptimize(std::string_view filename) { - size_t slashpos = filename.find_last_of("\\/"); - if (slashpos != std::string::npos && slashpos + 1 < filename.size()) + const std::string_view::size_type slashpos = filename.find_last_of('/'); + if (slashpos != std::string_view::npos && slashpos + 1 < filename.size()) { - std::string basename = filename.substr(slashpos + 1); + const std::string_view basename = filename.substr(slashpos + 1); // xmesh.nif can not be optimized because there are keyframes added in post if (!basename.empty() && basename[0] == 'x') return false; @@ -796,7 +796,7 @@ namespace Resource // For spell VFX, DummyXX nodes must remain intact. Not adding those to reservedNames to avoid being overly // cautious - instead, decide on filename - if (filename.find("vfx_pattern") != std::string::npos) + if (filename.find("vfx_pattern") != std::string_view::npos) return false; return true; } From 859d76592108b083fa3284582e7ec7958bb561b9 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 9 Mar 2024 01:45:21 +0100 Subject: [PATCH 5/5] Use normalized path for NifFileManager::get --- components/resource/bulletshapemanager.cpp | 2 +- components/resource/niffilemanager.cpp | 4 ++-- components/resource/niffilemanager.hpp | 2 +- components/resource/scenemanager.cpp | 19 ++++++++++--------- components/vfs/manager.cpp | 5 +++++ components/vfs/manager.hpp | 2 ++ 6 files changed, 21 insertions(+), 13 deletions(-) diff --git a/components/resource/bulletshapemanager.cpp b/components/resource/bulletshapemanager.cpp index f817d6b89a..b37e81111d 100644 --- a/components/resource/bulletshapemanager.cpp +++ b/components/resource/bulletshapemanager.cpp @@ -110,7 +110,7 @@ namespace Resource osg::ref_ptr BulletShapeManager::getShape(const std::string& name) { - const std::string normalized = VFS::Path::normalizeFilename(name); + const VFS::Path::Normalized normalized(name); osg::ref_ptr shape; osg::ref_ptr obj = mCache->getRefFromObjectCache(normalized); diff --git a/components/resource/niffilemanager.cpp b/components/resource/niffilemanager.cpp index 352d367f9b..0cc48d4247 100644 --- a/components/resource/niffilemanager.cpp +++ b/components/resource/niffilemanager.cpp @@ -41,14 +41,14 @@ namespace Resource NifFileManager::~NifFileManager() = default; - Nif::NIFFilePtr NifFileManager::get(const std::string& name) + Nif::NIFFilePtr NifFileManager::get(VFS::Path::NormalizedView name) { osg::ref_ptr obj = mCache->getRefFromObjectCache(name); if (obj) return static_cast(obj.get())->mNifFile; else { - auto file = std::make_shared(name); + auto file = std::make_shared(name.value()); Nif::Reader reader(*file, mEncoder); reader.parse(mVFS->get(name)); obj = new NifFileHolder(file); diff --git a/components/resource/niffilemanager.hpp b/components/resource/niffilemanager.hpp index dab4b70748..a5395fef7e 100644 --- a/components/resource/niffilemanager.hpp +++ b/components/resource/niffilemanager.hpp @@ -26,7 +26,7 @@ namespace Resource /// Retrieve a NIF file from the cache, or load it from the VFS if not cached yet. /// @note For performance reasons the NifFileManager does not handle case folding, needs /// to be done in advance by other managers accessing the NifFileManager. - Nif::NIFFilePtr get(const std::string& name); + Nif::NIFFilePtr get(VFS::Path::NormalizedView name); void reportStats(unsigned int frameNumber, osg::Stats* stats) const override; }; diff --git a/components/resource/scenemanager.cpp b/components/resource/scenemanager.cpp index 26d719a106..9ed72d5f05 100644 --- a/components/resource/scenemanager.cpp +++ b/components/resource/scenemanager.cpp @@ -545,9 +545,9 @@ namespace Resource namespace { osg::ref_ptr loadNonNif( - const std::string& normalizedFilename, std::istream& model, Resource::ImageManager* imageManager) + VFS::Path::NormalizedView normalizedFilename, std::istream& model, Resource::ImageManager* imageManager) { - auto ext = Misc::getFileExtension(normalizedFilename); + const std::string_view ext = Misc::getFileExtension(normalizedFilename.value()); osgDB::ReaderWriter* reader = osgDB::Registry::instance()->getReaderWriterForExtension(std::string(ext)); if (!reader) { @@ -566,7 +566,7 @@ namespace Resource if (ext == "dae") options->setOptionString("daeUseSequencedTextureUnits"); - const std::array fileHash = Files::getHash(normalizedFilename, model); + const std::array fileHash = Files::getHash(normalizedFilename.value(), model); osgDB::ReaderWriter::ReadResult result = reader->readNode(model, options); if (!result.success()) @@ -721,10 +721,10 @@ namespace Resource } } - osg::ref_ptr load(const std::string& normalizedFilename, const VFS::Manager* vfs, + osg::ref_ptr load(VFS::Path::NormalizedView normalizedFilename, const VFS::Manager* vfs, Resource::ImageManager* imageManager, Resource::NifFileManager* nifFileManager) { - auto ext = Misc::getFileExtension(normalizedFilename); + const std::string_view ext = Misc::getFileExtension(normalizedFilename.value()); if (ext == "nif") return NifOsg::Loader::load(*nifFileManager->get(normalizedFilename), imageManager); else if (ext == "spt") @@ -843,11 +843,12 @@ namespace Resource { try { + VFS::Path::Normalized path("meshes/marker_error.****"); for (const auto meshType : { "nif", "osg", "osgt", "osgb", "osgx", "osg2", "dae" }) { - const std::string normalized = "meshes/marker_error." + std::string(meshType); - if (mVFS->exists(normalized)) - return load(normalized, mVFS, mImageManager, mNifFileManager); + path.changeExtension(meshType); + if (mVFS->exists(path)) + return load(path, mVFS, mImageManager, mNifFileManager); } } catch (const std::exception& e) @@ -869,7 +870,7 @@ namespace Resource osg::ref_ptr SceneManager::getTemplate(std::string_view name, bool compile) { - std::string normalized = VFS::Path::normalizeFilename(name); + const VFS::Path::Normalized normalized(name); osg::ref_ptr obj = mCache->getRefFromObjectCache(normalized); if (obj) diff --git a/components/vfs/manager.cpp b/components/vfs/manager.cpp index ef5dd495c9..936dd64679 100644 --- a/components/vfs/manager.cpp +++ b/components/vfs/manager.cpp @@ -43,6 +43,11 @@ namespace VFS return getNormalized(name); } + Files::IStreamPtr Manager::get(Path::NormalizedView name) const + { + return getNormalized(name.value()); + } + Files::IStreamPtr Manager::getNormalized(std::string_view normalizedName) const { assert(Path::isNormalized(normalizedName)); diff --git a/components/vfs/manager.hpp b/components/vfs/manager.hpp index 955538627f..59211602de 100644 --- a/components/vfs/manager.hpp +++ b/components/vfs/manager.hpp @@ -50,6 +50,8 @@ namespace VFS /// @note May be called from any thread once the index has been built. Files::IStreamPtr get(const Path::Normalized& name) const; + Files::IStreamPtr get(Path::NormalizedView name) const; + /// Retrieve a file by name (name is already normalized). /// @note Throws an exception if the file can not be found. /// @note May be called from any thread once the index has been built.