From 235c097682af32ff3fb3e549227f7b9c1d77915c Mon Sep 17 00:00:00 2001 From: Solokiller Date: Thu, 22 Jul 2021 15:50:29 +0200 Subject: [PATCH] Rework asset loading so the main asset file is opened only once Resolves #162 Ensure asset files are only opened if they aren't opened in any other programs Resolves #163 --- .../shared/studiomodel/StudioModelIO.cpp | 68 +++++++++++-------- .../shared/studiomodel/StudioModelIO.hpp | 6 +- src/tools/hlam/ui/MainWindow.cpp | 6 +- src/tools/hlam/ui/assets/Assets.cpp | 15 +++- src/tools/hlam/ui/assets/Assets.hpp | 5 +- .../assets/studiomodel/StudioModelAsset.cpp | 12 ++-- .../assets/studiomodel/StudioModelAsset.hpp | 4 +- src/tools/hlam/utility/IOUtils.cpp | 50 ++++++++++++++ src/tools/hlam/utility/IOUtils.hpp | 5 ++ 9 files changed, 123 insertions(+), 48 deletions(-) diff --git a/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.cpp b/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.cpp index 1d39972d..90866c22 100644 --- a/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.cpp +++ b/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.cpp @@ -8,45 +8,49 @@ namespace studiomdl { -bool IsStudioModel(const std::string& fileName) +bool IsStudioModel(FILE* file) { - bool isStudioModel = false; + if (!file) + { + return false; + } - if (FILE* file = utf8_fopen(fileName.c_str(), "rb"); file) + std::int32_t id; + + if (fread(&id, sizeof(id), 1, file) != 1) { - std::int32_t id; + return false; + } - if (fread(&id, sizeof(id), 1, file) == 1) - { - if (strncmp(reinterpret_cast(&id), STUDIOMDL_HDR_ID, 4) == 0) - { - std::int32_t version; - - if (fread(&version, sizeof(version), 1, file) == 1) - { - if (version == STUDIO_VERSION) - { - isStudioModel = true; - } - } - } - } + if (strncmp(reinterpret_cast(&id), STUDIOMDL_HDR_ID, 4) != 0) + { + return false; + } - fclose(file); + std::int32_t version; + + if (fread(&version, sizeof(version), 1, file) != 1) + { + return false; + } + + if (version != STUDIO_VERSION) + { + return false; } - return isStudioModel; + return true; } namespace { template -studio_ptr LoadStudioHeader(const std::filesystem::path& fileName, const bool bAllowSeqGroup, const bool externalTextures) +studio_ptr LoadStudioHeader(const std::filesystem::path& fileName, FILE* existingFile, const bool bAllowSeqGroup, const bool externalTextures) { const std::string utf8FileName{fileName.u8string()}; // load the model - FILE* file = utf8_fopen(utf8FileName.c_str(), "rb"); + FILE* file = existingFile ? existingFile : utf8_exclusive_read_fopen(utf8FileName.c_str(), true); if (!file) { @@ -64,13 +68,13 @@ studio_ptr LoadStudioHeader(const std::filesystem::path& fileName, const bool loweredFileName.replace_filename(stem); loweredFileName.replace_extension(fileName.extension()); - file = utf8_fopen(loweredFileName.u8string().c_str(), "rb"); + file = utf8_exclusive_read_fopen(loweredFileName.u8string().c_str(), true); } } if (!file) { - throw assets::AssetFileNotFound(std::string{"File \""} + utf8FileName + "\" not found"); + throw assets::AssetFileNotFound(std::string{"File \""} + utf8FileName + "\" does not exist or is currently opened by another program"); } } @@ -83,7 +87,11 @@ studio_ptr LoadStudioHeader(const std::filesystem::path& fileName, const bool auto header = reinterpret_cast(buffer.get()); const size_t readCount = fread(header, size, 1, file); - fclose(file); + + if (!existingFile) + { + fclose(file); + } if (readCount != 1) { @@ -120,7 +128,7 @@ studio_ptr LoadStudioHeader(const std::filesystem::path& fileName, const bool } } -std::unique_ptr LoadStudioModel(const std::filesystem::path& fileName) +std::unique_ptr LoadStudioModel(const std::filesystem::path& fileName, FILE* mainFile) { std::filesystem::path baseFileName{fileName}; @@ -129,7 +137,7 @@ std::unique_ptr LoadStudioModel(const std::filesystem::path& fileNa const auto isDol = fileName.extension() == ".dol"; //Load the model - auto mainHeader = LoadStudioHeader(fileName, false, false); + auto mainHeader = LoadStudioHeader(fileName, mainFile, false, false); if (mainHeader->name[0] == '\0') { @@ -155,7 +163,7 @@ std::unique_ptr LoadStudioModel(const std::filesystem::path& fileNa texturename += extension; - textureHeader = LoadStudioHeader(texturename, true, true); + textureHeader = LoadStudioHeader(texturename, nullptr, true, true); } std::vector> sequenceHeaders; @@ -177,7 +185,7 @@ std::unique_ptr LoadStudioModel(const std::filesystem::path& fileNa std::setfill('0') << std::setw(2) << i << std::setw(0) << suffix; - sequenceHeaders.emplace_back(LoadStudioHeader(seqgroupname.str(), true, false)); + sequenceHeaders.emplace_back(LoadStudioHeader(seqgroupname.str(), nullptr, true, false)); } } diff --git a/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.hpp b/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.hpp index dd0f87a7..94c3f616 100644 --- a/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.hpp +++ b/src/tools/hlam/engine/shared/studiomodel/StudioModelIO.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -18,17 +19,18 @@ class StudioModelIsNotMainHeader : public assets::AssetException using assets::AssetException::AssetException; }; -bool IsStudioModel(const std::string& fileName); +bool IsStudioModel(FILE* file); /** * @brief Loads a studio model * @param fileName Name of the model to load. This is the entire path, including the extension +* @param mainFile Handle to the main file * @exception assets::AssetNotFound If a file could not be found * @exception assets::AssetInvalidFormat If a file has an invalid format * @exception assets::AssetVersionDiffers If a file has the wrong studio version * @exception StudioModelIsNotMainHeader If the filename specifies a studio model file that is not the main file */ -std::unique_ptr LoadStudioModel(const std::filesystem::path& fileName); +std::unique_ptr LoadStudioModel(const std::filesystem::path& fileName, FILE* mainFile); /** * Saves a studio model. diff --git a/src/tools/hlam/ui/MainWindow.cpp b/src/tools/hlam/ui/MainWindow.cpp index 84ee9299..5ea6c753 100644 --- a/src/tools/hlam/ui/MainWindow.cpp +++ b/src/tools/hlam/ui/MainWindow.cpp @@ -186,7 +186,7 @@ bool MainWindow::TryLoadAsset(QString fileName) if (!QFile::exists(fileName)) { - QMessageBox::critical(this, "Error loading asset", QString{"Asset does not exist"}); + QMessageBox::critical(this, "Error loading asset", QString{"Asset \"%1\" does not exist"}.arg(fileName)); return false; } @@ -223,12 +223,12 @@ bool MainWindow::TryLoadAsset(QString fileName) } else { - QMessageBox::critical(this, "Error loading asset", QString{"Error loading asset:\nNull asset returned"}); + QMessageBox::critical(this, "Error loading asset", QString{"Error loading asset \"%1\":\nNull asset returned"}.arg(fileName)); } } catch (const ::assets::AssetException& e) { - QMessageBox::critical(this, "Error loading asset", QString{"Error loading asset:\n%1"}.arg(e.what())); + QMessageBox::critical(this, "Error loading asset", QString{"Error loading asset \"%1\":\n%2"}.arg(fileName).arg(e.what())); } return false; diff --git a/src/tools/hlam/ui/assets/Assets.cpp b/src/tools/hlam/ui/assets/Assets.cpp index 012d9387..c7cfa5b9 100644 --- a/src/tools/hlam/ui/assets/Assets.cpp +++ b/src/tools/hlam/ui/assets/Assets.cpp @@ -3,6 +3,7 @@ #include "assets/AssetIO.hpp" #include "ui/assets/Assets.hpp" +#include "utility/IOUtils.hpp" namespace ui::assets { @@ -35,12 +36,22 @@ void AssetProviderRegistry::AddProvider(std::unique_ptr&& provide std::unique_ptr AssetProviderRegistry::Load(EditorContext* editorContext, const QString& fileName) const { + std::unique_ptr file{utf8_exclusive_read_fopen(fileName.toStdString().c_str(), true), &::fclose}; + + if (!file) + { + throw ::assets::AssetException("Could not open asset: file does not exist or is currently opened by another program"); + } + for (const auto& provider : _providers) { - if (provider->CanLoad(fileName)) + if (provider->CanLoad(fileName, file.get())) { - return provider->Load(editorContext, fileName); + rewind(file.get()); + return provider->Load(editorContext, fileName, file.get()); } + + rewind(file.get()); } throw ::assets::AssetException("File type not supported"); diff --git a/src/tools/hlam/ui/assets/Assets.hpp b/src/tools/hlam/ui/assets/Assets.hpp index 4255b918..5398af1d 100644 --- a/src/tools/hlam/ui/assets/Assets.hpp +++ b/src/tools/hlam/ui/assets/Assets.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -107,10 +108,10 @@ class AssetProvider : public QObject */ virtual QMenu* CreateToolMenu(EditorContext* editorContext) = 0; - virtual bool CanLoad(const QString& fileName) const = 0; + virtual bool CanLoad(const QString& fileName, FILE* file) const = 0; //TODO: pass a filesystem object to resolve additional file locations with - virtual std::unique_ptr Load(EditorContext* editorContext, const QString& fileName) const = 0; + virtual std::unique_ptr Load(EditorContext* editorContext, const QString& fileName, FILE* file) const = 0; }; /** diff --git a/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.cpp b/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.cpp index 87db5164..a1fe809f 100644 --- a/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.cpp +++ b/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.cpp @@ -227,7 +227,7 @@ void StudioModelAsset::TryRefresh() try { const auto filePath = std::filesystem::u8path(GetFileName().toStdString()); - auto studioModel = studiomdl::LoadStudioModel(filePath); + auto studioModel = studiomdl::LoadStudioModel(filePath, nullptr); _editableStudioModel = std::make_unique(studiomdl::ConvertToEditable(*studioModel)); @@ -615,17 +615,15 @@ QMenu* StudioModelAssetProvider::CreateToolMenu(EditorContext* editorContext) return menu; } -bool StudioModelAssetProvider::CanLoad(const QString& fileName) const +bool StudioModelAssetProvider::CanLoad(const QString& fileName, FILE* file) const { - //TODO: race condition: the file could be modified between this call and a call to LoadStudioModel - //Ideally the file is opened exactly once - return studiomdl::IsStudioModel(fileName.toStdString()); + return studiomdl::IsStudioModel(file); } -std::unique_ptr StudioModelAssetProvider::Load(EditorContext* editorContext, const QString& fileName) const +std::unique_ptr StudioModelAssetProvider::Load(EditorContext* editorContext, const QString& fileName, FILE* file) const { const auto filePath = std::filesystem::u8path(fileName.toStdString()); - auto studioModel = studiomdl::LoadStudioModel(filePath); + auto studioModel = studiomdl::LoadStudioModel(filePath, file); auto editableStudioModel = studiomdl::ConvertToEditable(*studioModel); diff --git a/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.hpp b/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.hpp index 39ef0f3d..170667ef 100644 --- a/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.hpp +++ b/src/tools/hlam/ui/assets/studiomodel/StudioModelAsset.hpp @@ -58,9 +58,9 @@ class StudioModelAssetProvider final : public AssetProvider QMenu* CreateToolMenu(EditorContext* editorContext) override; - bool CanLoad(const QString& fileName) const override; + bool CanLoad(const QString& fileName, FILE* file) const override; - std::unique_ptr Load(EditorContext* editorContext, const QString& fileName) const override; + std::unique_ptr Load(EditorContext* editorContext, const QString& fileName, FILE* file) const override; settings::StudioModelSettings* GetStudioModelSettings() const { return _studioModelSettings.get(); } diff --git a/src/tools/hlam/utility/IOUtils.cpp b/src/tools/hlam/utility/IOUtils.cpp index df70cfc4..ad0b785d 100644 --- a/src/tools/hlam/utility/IOUtils.cpp +++ b/src/tools/hlam/utility/IOUtils.cpp @@ -5,6 +5,13 @@ #include "utility/IOUtils.hpp" +#ifdef WIN32 +#define WIN32_MEAN_AND_LEAN +#include +#include +#include +#endif + FILE* utf8_fopen(const char* filename, const char* mode) { #ifdef WIN32 @@ -18,3 +25,46 @@ FILE* utf8_fopen(const char* filename, const char* mode) return fopen(filename, mode); #endif } + +FILE* utf8_exclusive_read_fopen(const char* filename, bool asBinary) +{ +#ifdef WIN32 + std::wstring_convert> convert; + + auto wideFilename = convert.from_bytes(filename); + + const HANDLE fh = CreateFileW(wideFilename.c_str(), GENERIC_READ, 0 /* no sharing! exclusive */, NULL, OPEN_EXISTING, 0, nullptr); + + if (fh == nullptr || fh == INVALID_HANDLE_VALUE) + { + return nullptr; + } + + int osFlags = _O_RDONLY; + + if (!asBinary) + { + osFlags |= _O_TEXT; + } + + const int fileHandle = _open_osfhandle(reinterpret_cast(fh), osFlags); + + if (fileHandle == -1) + { + CloseHandle(fh); + return nullptr; + } + + FILE* file = _fdopen(fileHandle, asBinary ? "r" : "rb"); + + if (!file) + { + _close(fileHandle); + return nullptr; + } + + return file; +#else + return utf8_fopen(filename, "r"); +#endif +} diff --git a/src/tools/hlam/utility/IOUtils.hpp b/src/tools/hlam/utility/IOUtils.hpp index 346b3fd9..add67c1e 100644 --- a/src/tools/hlam/utility/IOUtils.hpp +++ b/src/tools/hlam/utility/IOUtils.hpp @@ -6,3 +6,8 @@ * @brief Wrapper around fopen to open filenames containing UTF8 characters on Windows */ FILE* utf8_fopen(const char* filename, const char* mode); + +/** +* @brief Opens the file for reading, but only if the file is not already opened elsewhere (Only on Windows, other platforms use @see utf8_fopen) +*/ +FILE* utf8_exclusive_read_fopen(const char* filename, bool asBinary);