From d11af753f2a959f18a734af591819fc1510b6b5e Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Thu, 2 Jan 2020 12:35:00 +0000 Subject: [PATCH] Fix Codacity warnings, test Exporter metadata Pass std::string around instead as need to create one anyway. Use CamelCase version to avoid caseSensiTivity issues as will usually want the camelcase edition anyway. Not UTF-8 but the standard XML tags are ASCII anyway --- code/Collada/ColladaHelper.cpp | 41 ++++++++++++++++++++ code/Collada/ColladaHelper.h | 8 +++- code/Collada/ColladaParser.cpp | 46 ++++++---------------- code/Collada/ColladaParser.h | 5 +-- test/models/Collada/lights.dae | 1 + test/unit/utColladaExportLight.cpp | 59 ++++++++++++++++++++++++++--- test/unit/utColladaImportExport.cpp | 2 +- 7 files changed, 116 insertions(+), 46 deletions(-) diff --git a/code/Collada/ColladaHelper.cpp b/code/Collada/ColladaHelper.cpp index f19bb1047..aeabb49c4 100644 --- a/code/Collada/ColladaHelper.cpp +++ b/code/Collada/ColladaHelper.cpp @@ -44,6 +44,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "ColladaHelper.h" #include +#include namespace Assimp { namespace Collada { @@ -60,5 +61,45 @@ const MetaKeyPairVector &GetColladaAssimpMetaKeys() { return result; } +const MetaKeyPairVector MakeColladaAssimpMetaKeysCamelCase() { + MetaKeyPairVector result = MakeColladaAssimpMetaKeys(); + for (auto &val : result) + { + ToCamelCase(val.first); + } + return result; +}; + +const MetaKeyPairVector &GetColladaAssimpMetaKeysCamelCase() +{ + static const MetaKeyPairVector result = MakeColladaAssimpMetaKeysCamelCase(); + return result; +} + +// ------------------------------------------------------------------------------------------------ +// Convert underscore_separated to CamelCase: "authoring_tool" becomes "AuthoringTool" +void ToCamelCase(std::string &text) +{ + if (text.empty()) + return; + // Capitalise first character + text[0] = Assimp::ToUpper(text[0]); + for (auto it = text.begin(); it != text.end(); /*iterated below*/) + { + if ((*it) == '_') + { + it = text.erase(it); + if (it != text.end()) + (*it) = ToUpper(*it); + } + else + { + // Make lower case + (*it) = ToLower(*it); + ++it; + } + } +} + } // namespace Collada } // namespace Assimp diff --git a/code/Collada/ColladaHelper.h b/code/Collada/ColladaHelper.h index f9968f162..31285312c 100644 --- a/code/Collada/ColladaHelper.h +++ b/code/Collada/ColladaHelper.h @@ -105,10 +105,16 @@ enum MorphMethod }; /** Common metadata keys as */ -typedef std::pair MetaKeyPair; +typedef std::pair MetaKeyPair; typedef std::vector MetaKeyPairVector; +// Collada as lower_case (native) const MetaKeyPairVector &GetColladaAssimpMetaKeys(); +// Collada as CamelCase (used by Assimp for consistency) +const MetaKeyPairVector &GetColladaAssimpMetaKeysCamelCase(); + +/** Convert underscore_separated to CamelCase "authoring_tool" becomes "AuthoringTool" */ +void ToCamelCase(std::string &text); /** Contains all data for one of the different transformation types */ struct Transform diff --git a/code/Collada/ColladaParser.cpp b/code/Collada/ColladaParser.cpp index df5a95c5b..cced255f5 100644 --- a/code/Collada/ColladaParser.cpp +++ b/code/Collada/ColladaParser.cpp @@ -411,7 +411,7 @@ void ColladaParser::ReadAssetInfo() } else { - ReadMetaDataItem(mAssetMetaData, GetColladaAssimpMetaKeys()); + ReadMetaDataItem(mAssetMetaData); } } else if (mReader->getNodeType() == irr::io::EXN_ELEMENT_END) @@ -435,7 +435,7 @@ void ColladaParser::ReadContributorInfo() { if (mReader->getNodeType() == irr::io::EXN_ELEMENT) { - ReadMetaDataItem(mAssetMetaData, GetColladaAssimpMetaKeys()); + ReadMetaDataItem(mAssetMetaData); } else if (mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { @@ -446,25 +446,21 @@ void ColladaParser::ReadContributorInfo() } } -static bool FindCommonKey(const char *collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) { - found_index = 9999999u; - if ( nullptr == collada_key ) { - return false; - } - +static bool FindCommonKey(const std::string &collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) { for (size_t i = 0; i < key_renaming.size(); ++i) { - if (strncmp(key_renaming[i].first, collada_key, strlen(collada_key)) == 0) { + if (key_renaming[i].first == collada_key) { found_index = i; return true; } } - + found_index = std::numeric_limits::max(); return false; } // ------------------------------------------------------------------------------------------------ // Reads a single string metadata item -void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPairVector &key_renaming) { +void ColladaParser::ReadMetaDataItem(StringMetaData &metadata) { + const Collada::MetaKeyPairVector &key_renaming = GetColladaAssimpMetaKeysCamelCase(); // Metadata such as created, keywords, subject etc const char *key_char = mReader->getNodeName(); if (key_char != nullptr) { @@ -474,12 +470,13 @@ void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPair aiString aistr; aistr.Set(value_char); + std::string camel_key_str(key_str); + ToCamelCase(camel_key_str); + size_t found_index; - if (FindCommonKey(key_str.c_str(), key_renaming, found_index)) { + if (FindCommonKey(camel_key_str, key_renaming, found_index)) { metadata.emplace(key_renaming[found_index].second, aistr); } else { - std::string camel_key_str(key_str); - ToCamelCase(camel_key_str); metadata.emplace(camel_key_str, aistr); } } @@ -489,27 +486,6 @@ void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPair SkipElement(); } -// ------------------------------------------------------------------------------------------------ -// Convert underscore_seperated to CamelCase: "authoring_tool" becomes "AuthoringTool" -void ColladaParser::ToCamelCase(std::string &text) -{ - if (text.empty()) - return; - // Capitalise first character - text[0] = ToUpper(text[0]); - for (auto it = text.begin(); it != text.end(); /*iterated below*/) - { - if ((*it) == '_') - { - it = text.erase(it); - if (it != text.end()) - (*it) = ToUpper(*it); - } - else - ++it; - } -} - // ------------------------------------------------------------------------------------------------ // Reads the animation clips void ColladaParser::ReadAnimationClipLibrary() diff --git a/code/Collada/ColladaParser.h b/code/Collada/ColladaParser.h index 57d644aef..f421172c7 100644 --- a/code/Collada/ColladaParser.h +++ b/code/Collada/ColladaParser.h @@ -95,10 +95,7 @@ namespace Assimp void ReadContributorInfo(); /** Reads generic metadata into provided map and renames keys for Assimp */ - void ReadMetaDataItem(StringMetaData &metadata, const Collada::MetaKeyPairVector &key_renaming); - - /** Convert underscore_seperated to CamelCase "authoring_tool" becomes "AuthoringTool" */ - static void ToCamelCase(std::string &text); + void ReadMetaDataItem(StringMetaData &metadata); /** Reads the animation library */ void ReadAnimationLibrary(); diff --git a/test/models/Collada/lights.dae b/test/models/Collada/lights.dae index 9e040aac6..03507116c 100644 --- a/test/models/Collada/lights.dae +++ b/test/models/Collada/lights.dae @@ -4,6 +4,7 @@ Blender User Blender 2.74.0 commit date:2015-03-31, commit time:13:39, hash:000dfc0 + BSD 2015-05-17T21:55:44 2015-05-17T21:55:44 diff --git a/test/unit/utColladaExportLight.cpp b/test/unit/utColladaExportLight.cpp index 502cecf3a..9b3e8a0e7 100644 --- a/test/unit/utColladaExportLight.cpp +++ b/test/unit/utColladaExportLight.cpp @@ -43,6 +43,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "UnitTestPCH.h" #include +#include #include #include #include @@ -75,7 +76,7 @@ TEST_F(ColladaExportLight, testExportLight) const char* file = "lightsExp.dae"; const aiScene* pTest = im->ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/lights.dae", aiProcess_ValidateDataStructure); - ASSERT_TRUE(pTest!=NULL); + ASSERT_NE(pTest, nullptr); ASSERT_TRUE(pTest->HasLights()); const unsigned int origNumLights( pTest->mNumLights ); @@ -86,15 +87,63 @@ TEST_F(ColladaExportLight, testExportLight) origLights[ i ] = *(pTest->mLights[ i ]); } - EXPECT_EQ(AI_SUCCESS,ex->Export(pTest,"collada",file)); + // Common metadata + // Confirm was loaded by the Collada importer + aiString origImporter; + EXPECT_TRUE(pTest->mMetaData->Get(AI_METADATA_SOURCE_FORMAT, origImporter)) << "No importer format metadata"; + EXPECT_STREQ("Collada Importer", origImporter.C_Str()); + + aiString origGenerator; + EXPECT_TRUE(pTest->mMetaData->Get(AI_METADATA_SOURCE_GENERATOR, origGenerator)) << "No generator metadata"; + EXPECT_EQ(strncmp(origGenerator.C_Str(), "Blender", 7), 0) << "AI_METADATA_SOURCE_GENERATOR was: " << origGenerator.C_Str(); + + aiString origCopyright; + EXPECT_TRUE(pTest->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, origCopyright)) << "No copyright metadata"; + EXPECT_STREQ("BSD", origCopyright.C_Str()); + + aiString origCreated; + EXPECT_TRUE(pTest->mMetaData->Get("Created", origCreated)) << "No created metadata"; + EXPECT_STREQ("2015-05-17T21:55:44", origCreated.C_Str()); + + aiString origModified; + EXPECT_TRUE(pTest->mMetaData->Get("Modified", origModified)) << "No modified metadata"; + EXPECT_STREQ("2015-05-17T21:55:44", origModified.C_Str()); + + EXPECT_EQ(AI_SUCCESS, ex->Export(pTest, "collada", file)); + + // Drop the pointer as about to become invalid + pTest = nullptr; const aiScene* imported = im->ReadFile(file, aiProcess_ValidateDataStructure); - ASSERT_TRUE(imported!=NULL); + ASSERT_TRUE(imported != NULL); + // Check common metadata survived roundtrip + aiString readImporter; + EXPECT_TRUE(imported->mMetaData->Get(AI_METADATA_SOURCE_FORMAT, readImporter)) << "No importer format metadata after export"; + EXPECT_STREQ(origImporter.C_Str(), readImporter.C_Str()) << "Assimp Importer Format changed"; + + aiString readGenerator; + EXPECT_TRUE(imported->mMetaData->Get(AI_METADATA_SOURCE_GENERATOR, readGenerator)) << "No generator metadata"; + EXPECT_STREQ(origGenerator.C_Str(), readGenerator.C_Str()) << "Generator changed"; + + aiString readCopyright; + EXPECT_TRUE(imported->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, readCopyright)) << "No copyright metadata"; + EXPECT_STREQ(origCopyright.C_Str(), readCopyright.C_Str()) << "Copyright changed"; + + aiString readCreated; + EXPECT_TRUE(imported->mMetaData->Get("Created", readCreated)) << "No created metadata"; + EXPECT_STREQ(origCreated.C_Str(), readCreated.C_Str()) << "Created date changed"; + + aiString readModified; + EXPECT_TRUE(imported->mMetaData->Get("Modified", readModified)) << "No modified metadata"; + EXPECT_STRNE(origModified.C_Str(), readModified.C_Str()) << "Modified date did not change"; + EXPECT_GT(readModified.length, 18) << "Modified date too short"; + + // Lights EXPECT_TRUE(imported->HasLights()); - EXPECT_EQ( origNumLights,imported->mNumLights ); - for(size_t i=0; i< origNumLights; i++) { + EXPECT_EQ(origNumLights, imported->mNumLights); + for(size_t i=0; i < origNumLights; i++) { const aiLight *orig = &origLights[ i ]; const aiLight *read = imported->mLights[i]; EXPECT_EQ( 0,strncmp(origNames[ i ].c_str(),read->mName.C_Str(), origNames[ i ].size() ) ); diff --git a/test/unit/utColladaImportExport.cpp b/test/unit/utColladaImportExport.cpp index 1350d3607..749e57010 100644 --- a/test/unit/utColladaImportExport.cpp +++ b/test/unit/utColladaImportExport.cpp @@ -77,7 +77,7 @@ public: EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_GENERATOR, value)) << "No generator metadata"; EXPECT_EQ(strncmp(value.C_Str(), "Maya 8.0", 8), 0) << "AI_METADATA_SOURCE_GENERATOR was: " << value.C_Str(); - EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, value)); + EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, value)) << "No copyright metadata"; EXPECT_EQ(strncmp(value.C_Str(), "Copyright 2006", 14), 0) << "AI_METADATA_SOURCE_COPYRIGHT was: " << value.C_Str(); return true;