From 506baa21e68dfe928cecc7f53f4027d94ecd1fb3 Mon Sep 17 00:00:00 2001 From: Marco Feuerstein Date: Tue, 4 Jul 2023 09:25:45 +0200 Subject: [PATCH 1/3] Unify extension check for importers. This enables proper checking for all kinds of extensions (including the ones with multiple dots) for all importers and internal usage. --- code/Common/BaseImporter.cpp | 39 +++++++++++++++++++---------------- code/Common/Importer.cpp | 20 +++--------------- include/assimp/BaseImporter.h | 10 +++++++++ 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/code/Common/BaseImporter.cpp b/code/Common/BaseImporter.cpp index 87b385268..a37297b9a 100644 --- a/code/Common/BaseImporter.cpp +++ b/code/Common/BaseImporter.cpp @@ -229,27 +229,30 @@ void BaseImporter::GetExtensionList(std::set &extensions) { const char *ext0, const char *ext1, const char *ext2) { - std::string::size_type pos = pFile.find_last_of('.'); - - // no file extension - can't read - if (pos == std::string::npos) { - return false; + std::set extensions; + for (const char* ext : {ext0, ext1, ext2}) { + if (ext == nullptr) continue; + extensions.emplace(ext); } + return HasExtension(pFile, extensions); +} - const char *ext_real = &pFile[pos + 1]; - if (!ASSIMP_stricmp(ext_real, ext0)) { - return true; +// ------------------------------------------------------------------------------------------------ +// Check for file extension +/*static*/ bool BaseImporter::HasExtension(const std::string &pFile, const std::set &extensions) { + // CAUTION: Do not just search for the extension! + // GetExtension() returns the part after the *last* dot, but some extensions + // have dots inside them, e.g. ogre.mesh.xml. Compare the entire end of the + // string. + for (std::set::const_iterator it = extensions.cbegin(); it != extensions.cend(); ++it) { + // Yay for C++<20 not having std::string::ends_with() + const std::string extension = "." + *it; + if (extension.length() > pFile.length()) continue; + // Possible optimization: Fetch the lowercase filename! + if (0 == ASSIMP_stricmp(pFile.c_str() + pFile.length() - extension.length(), extension.c_str())) { + return true; + } } - - // check for other, optional, file extensions - if (ext1 && !ASSIMP_stricmp(ext_real, ext1)) { - return true; - } - - if (ext2 && !ASSIMP_stricmp(ext_real, ext2)) { - return true; - } - return false; } diff --git a/code/Common/Importer.cpp b/code/Common/Importer.cpp index b66059397..bdf64ac8f 100644 --- a/code/Common/Importer.cpp +++ b/code/Common/Importer.cpp @@ -637,24 +637,10 @@ const aiScene* Importer::ReadFile( const char* _pFile, unsigned int pFlags) { std::set extensions; pimpl->mImporter[a]->GetExtensionList(extensions); - // CAUTION: Do not just search for the extension! - // GetExtension() returns the part after the *last* dot, but some extensions have dots - // inside them, e.g. ogre.mesh.xml. Compare the entire end of the string. - for (std::set::const_iterator it = extensions.cbegin(); it != extensions.cend(); ++it) { - - // Yay for C++<20 not having std::string::ends_with() - std::string extension = "." + *it; - if (extension.length() <= pFile.length()) { - // Possible optimization: Fetch the lowercase filename! - if (0 == ASSIMP_stricmp(pFile.c_str() + pFile.length() - extension.length(), extension.c_str())) { - ImporterAndIndex candidate = { pimpl->mImporter[a], a }; - possibleImporters.push_back(candidate); - break; - } - } - + if (BaseImporter::HasExtension(pFile, extensions)) { + ImporterAndIndex candidate = { pimpl->mImporter[a], a }; + possibleImporters.push_back(candidate); } - } // If just one importer supports this extension, pick it and close the case. diff --git a/include/assimp/BaseImporter.h b/include/assimp/BaseImporter.h index 06b87f1e1..bf425a849 100644 --- a/include/assimp/BaseImporter.h +++ b/include/assimp/BaseImporter.h @@ -275,6 +275,16 @@ public: // static utilities const char *ext1 = nullptr, const char *ext2 = nullptr); + // ------------------------------------------------------------------- + /** @brief Check whether a file has one of the passed file extensions + * @param pFile Input file + * @param extensions Extensions to check for. Lowercase characters only, no dot! + * @note Case-insensitive + */ + static bool HasExtension( + const std::string &pFile, + const std::set &extensions); + // ------------------------------------------------------------------- /** @brief Extract file extension from a string * @param pFile Input file From bdde96867705d9cbfc0b95f5864d6bc6357c52fd Mon Sep 17 00:00:00 2001 From: Marco Feuerstein Date: Tue, 4 Jul 2023 12:29:17 +0200 Subject: [PATCH 2/3] Address reviewer comment. --- code/Common/BaseImporter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/Common/BaseImporter.cpp b/code/Common/BaseImporter.cpp index a37297b9a..d03b2adfc 100644 --- a/code/Common/BaseImporter.cpp +++ b/code/Common/BaseImporter.cpp @@ -244,7 +244,7 @@ void BaseImporter::GetExtensionList(std::set &extensions) { // GetExtension() returns the part after the *last* dot, but some extensions // have dots inside them, e.g. ogre.mesh.xml. Compare the entire end of the // string. - for (std::set::const_iterator it = extensions.cbegin(); it != extensions.cend(); ++it) { + for (auto it = extensions.cbegin(); it != extensions.cend(); ++it) { // Yay for C++<20 not having std::string::ends_with() const std::string extension = "." + *it; if (extension.length() > pFile.length()) continue; From 87cac888e474880efdacba4cddd28f83c582fd97 Mon Sep 17 00:00:00 2001 From: Marco Feuerstein Date: Tue, 4 Jul 2023 13:18:22 +0200 Subject: [PATCH 3/3] More simplifications. --- code/Common/BaseImporter.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/code/Common/BaseImporter.cpp b/code/Common/BaseImporter.cpp index d03b2adfc..39c93e4be 100644 --- a/code/Common/BaseImporter.cpp +++ b/code/Common/BaseImporter.cpp @@ -244,12 +244,12 @@ void BaseImporter::GetExtensionList(std::set &extensions) { // GetExtension() returns the part after the *last* dot, but some extensions // have dots inside them, e.g. ogre.mesh.xml. Compare the entire end of the // string. - for (auto it = extensions.cbegin(); it != extensions.cend(); ++it) { + for (const std::string& ext : extensions) { // Yay for C++<20 not having std::string::ends_with() - const std::string extension = "." + *it; - if (extension.length() > pFile.length()) continue; + const std::string dotExt = "." + ext; + if (dotExt.length() > pFile.length()) continue; // Possible optimization: Fetch the lowercase filename! - if (0 == ASSIMP_stricmp(pFile.c_str() + pFile.length() - extension.length(), extension.c_str())) { + if (0 == ASSIMP_stricmp(pFile.c_str() + pFile.length() - dotExt.length(), dotExt.c_str())) { return true; } }