Merge branch 'master' into sfjohnston_mods

code/AssetLib/glTF2/glTF2Exporter.cpp

@@ -730,7 +730,7 @@ bool glTF2Exporter::GetMatSpecGloss(const aiMaterial &mat, glTF2::PbrSpecularGlo
 
 bool glTF2Exporter::GetMatSpecular(const aiMaterial &mat, glTF2::MaterialSpecular &specular) {
     // Specular requires either/or, default factors of zero disables specular, so do not export
-    if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) != AI_SUCCESS || mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) != AI_SUCCESS) {
+    if (GetMatColor(mat, specular.specularColorFactor, AI_MATKEY_COLOR_SPECULAR) != AI_SUCCESS && mat.Get(AI_MATKEY_SPECULAR_FACTOR, specular.specularFactor) != AI_SUCCESS) {
         return false;
     }
     // The spec states that the default is 1.0 and [1.0, 1.0, 1.0]. We if both are 0, which should disable specular. Otherwise, if one is 0, set to 1.0

code/Common/BaseImporter.cpp

@@ -59,6 +59,31 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <memory>
 #include <sstream>
 
+namespace {
+// Checks whether the passed string is a gcs version.
+bool IsGcsVersion(const std::string &s) {
+    if (s.empty()) return false;
+    return std::all_of(s.cbegin(), s.cend(), [](const char c) {
+        // gcs only permits numeric characters.
+        return std::isdigit(static_cast<int>(c));
+    });
+}
+
+// Removes a possible version hash from a filename, as found for example in
+// gcs uris (e.g. `gs://bucket/model.glb#1234`), see also
+// https://github.com/GoogleCloudPlatform/gsutil/blob/c80f329bc3c4011236c78ce8910988773b2606cb/gslib/storage_url.py#L39.
+std::string StripVersionHash(const std::string &filename) {
+    const std::string::size_type pos = filename.find_last_of('#');
+    // Only strip if the hash is behind a possible file extension and the part
+    // behind the hash is a version string.
+    if (pos != std::string::npos && pos > filename.find_last_of('.') &&
+        IsGcsVersion(filename.substr(pos + 1))) {
+        return filename.substr(0, pos);
+    }
+    return filename;
+}
+}  // namespace
+
 using namespace Assimp;
 
 // ------------------------------------------------------------------------------------------------
@@ -230,33 +255,38 @@ void BaseImporter::GetExtensionList(std::set<std::string> &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<std::string> 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<std::string> &extensions) {
+    const std::string file = StripVersionHash(pFile);
+    // 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 (const std::string& ext : extensions) {
+        // Yay for C++<20 not having std::string::ends_with()
+        const std::string dotExt = "." + ext;
+        if (dotExt.length() > file.length()) continue;
+        // Possible optimization: Fetch the lowercase filename!
+        if (0 == ASSIMP_stricmp(file.c_str() + file.length() - dotExt.length(), dotExt.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;
 }
 
 // ------------------------------------------------------------------------------------------------
 // Get file extension from path
-std::string BaseImporter::GetExtension(const std::string &file) {
+std::string BaseImporter::GetExtension(const std::string &pFile) {
+    const std::string file = StripVersionHash(pFile);
     std::string::size_type pos = file.find_last_of('.');
 
     // no file extension at all

code/Common/Importer.cpp

@@ -637,24 +637,10 @@ const aiScene* Importer::ReadFile( const char* _pFile, unsigned int pFlags) {
             std::set<std::string> 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<std::string>::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.