From f498a395e4a52b6a73d2394a2320337d6d684a60 Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Mon, 9 Dec 2019 14:05:41 +0000 Subject: [PATCH 1/8] Add common metadata to Collada Also add AI_METADATA_SOURCE_COPYRIGHT common metadata --- code/CMakeLists.txt | 2 + code/Collada/ColladaExporter.cpp | 5 +- code/Collada/ColladaHelper.cpp | 65 ++++++++++++++++ code/Collada/ColladaHelper.h | 5 ++ code/Collada/ColladaParser.cpp | 47 ++++++++---- code/Collada/ColladaParser.h | 4 +- include/assimp/commonMetaData.h | 4 + test/unit/utColladaImportExport.cpp | 111 ++++++++++++++++------------ 8 files changed, 176 insertions(+), 67 deletions(-) create mode 100644 code/Collada/ColladaHelper.cpp diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index 85aa620d9..8bad9365d 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -66,6 +66,7 @@ SET( PUBLIC_HEADERS ${HEADER_PATH}/color4.h ${HEADER_PATH}/color4.inl ${CMAKE_CURRENT_BINARY_DIR}/../include/assimp/config.h + ${HEADER_PATH}/commonMetaData.h ${HEADER_PATH}/defs.h ${HEADER_PATH}/Defines.h ${HEADER_PATH}/cfileio.h @@ -348,6 +349,7 @@ ADD_ASSIMP_IMPORTER( BVH ) ADD_ASSIMP_IMPORTER( COLLADA + Collada/ColladaHelper.cpp Collada/ColladaHelper.h Collada/ColladaLoader.cpp Collada/ColladaLoader.h diff --git a/code/Collada/ColladaExporter.cpp b/code/Collada/ColladaExporter.cpp index a93dfa59a..3e63990b8 100644 --- a/code/Collada/ColladaExporter.cpp +++ b/code/Collada/ColladaExporter.cpp @@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "ColladaExporter.h" #include +#include #include #include #include @@ -277,7 +278,7 @@ void ColladaExporter::WriteHeader() { mOutput << startstr << "" << XMLEscape(value.C_Str()) << "" << endstr; } - if (nullptr == meta || !meta->Get("AuthoringTool", value)) { + if (nullptr == meta || !meta->Get(AI_METADATA_SOURCE_GENERATOR, value)) { mOutput << startstr << "" << "Assimp Exporter" << "" << endstr; } else { mOutput << startstr << "" << XMLEscape(value.C_Str()) << "" << endstr; @@ -287,7 +288,7 @@ void ColladaExporter::WriteHeader() { if (meta->Get("Comments", value)) { mOutput << startstr << "" << XMLEscape(value.C_Str()) << "" << endstr; } - if (meta->Get("Copyright", value)) { + if (meta->Get(AI_METADATA_SOURCE_COPYRIGHT, value)) { mOutput << startstr << "" << XMLEscape(value.C_Str()) << "" << endstr; } if (meta->Get("SourceData", value)) { diff --git a/code/Collada/ColladaHelper.cpp b/code/Collada/ColladaHelper.cpp new file mode 100644 index 000000000..3a22d4db3 --- /dev/null +++ b/code/Collada/ColladaHelper.cpp @@ -0,0 +1,65 @@ +/** Helper structures for the Collada loader */ + +/* +Open Asset Import Library (assimp) +---------------------------------------------------------------------- + +Copyright (c) 2006-2019, assimp team + + +All rights reserved. + +Redistribution and use of this software in source and binary forms, +with or without modification, are permitted provided that the +following conditions are met: + +* Redistributions of source code must retain the above +copyright notice, this list of conditions and the +following disclaimer. + +* Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the +following disclaimer in the documentation and/or other +materials provided with the distribution. + +* Neither the name of the assimp team, nor the names of its +contributors may be used to endorse or promote products +derived from this software without specific prior +written permission of the assimp team. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +---------------------------------------------------------------------- +*/ + +#include "ColladaHelper.h" + +#include + +namespace Assimp { +namespace Collada { + +const MetaKeyPairVector MakeColladaAssimpMetaKeys() { + MetaKeyPairVector result; + result.emplace_back("authoring_tool", AI_METADATA_SOURCE_GENERATOR); + result.emplace_back("copyright", AI_METADATA_SOURCE_COPYRIGHT); + return result; +}; + +const MetaKeyPairVector &GetColladaAssimpMetaKeys() { + static const MetaKeyPairVector result = MakeColladaAssimpMetaKeys(); + return result; +} + +} // namespace Collada +} // namespace Assimp diff --git a/code/Collada/ColladaHelper.h b/code/Collada/ColladaHelper.h index b7d47da15..f9968f162 100644 --- a/code/Collada/ColladaHelper.h +++ b/code/Collada/ColladaHelper.h @@ -104,6 +104,11 @@ enum MorphMethod Relative }; +/** Common metadata keys as */ +typedef std::pair MetaKeyPair; +typedef std::vector MetaKeyPairVector; + +const MetaKeyPairVector &GetColladaAssimpMetaKeys(); /** 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 e2e6626c3..1b5b3b4d6 100644 --- a/code/Collada/ColladaParser.cpp +++ b/code/Collada/ColladaParser.cpp @@ -50,6 +50,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include "ColladaParser.h" +#include #include #include #include @@ -276,6 +277,9 @@ void ColladaParser::ReadContents() if (attrib != -1) { const char* version = mReader->getAttributeValue(attrib); + // Store declared format version string + mAssetMetaData.emplace(AI_METADATA_SOURCE_FORMAT_VERSION, version); + if (!::strncmp(version, "1.5", 3)) { mFormat = FV_1_5_n; ASSIMP_LOG_DEBUG("Collada schema version is 1.5.n"); @@ -399,7 +403,7 @@ void ColladaParser::ReadAssetInfo() } else { - ReadMetaDataItem(mAssetMetaData); + ReadMetaDataItem(mAssetMetaData, GetColladaAssimpMetaKeys()); } } else if (mReader->getNodeType() == irr::io::EXN_ELEMENT_END) @@ -423,7 +427,7 @@ void ColladaParser::ReadContributorInfo() { if (mReader->getNodeType() == irr::io::EXN_ELEMENT) { - ReadMetaDataItem(mAssetMetaData); + ReadMetaDataItem(mAssetMetaData, GetColladaAssimpMetaKeys()); } else if (mReader->getNodeType() == irr::io::EXN_ELEMENT_END) { @@ -434,23 +438,36 @@ void ColladaParser::ReadContributorInfo() } } +bool FindCommonKey(const char *collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) { + for (size_t i = 0; i < key_renaming.size(); ++i) { + if (strcmp(key_renaming[i].first, collada_key) == 0) { + found_index = i; + return true; + } + } + return false; +} + // ------------------------------------------------------------------------------------------------ // Reads a single string metadata item -void ColladaParser::ReadMetaDataItem(StringMetaData &metadata) -{ - // Metadata such as created, keywords, subject etc - const char* key_char = mReader->getNodeName(); - if (key_char != nullptr) - { +void ColladaParser::ReadMetaDataItem(StringMetaData &metadata, const MetaKeyPairVector &key_renaming) { + // Metadata such as created, keywords, subject etc + const char *key_char = mReader->getNodeName(); + if (key_char != nullptr) { const std::string key_str(key_char); - const char* value_char = TestTextContent(); - if (value_char != nullptr) - { - std::string camel_key_str = key_str; - ToCamelCase(camel_key_str); + const char *value_char = TestTextContent(); + if (value_char != nullptr) { aiString aistr; - aistr.Set(value_char); - metadata.emplace(camel_key_str, aistr); + aistr.Set(value_char); + + size_t found_index; + if (FindCommonKey(key_str.c_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); + } } TestClosing(key_str.c_str()); } diff --git a/code/Collada/ColladaParser.h b/code/Collada/ColladaParser.h index 4e8c8fccf..57d644aef 100644 --- a/code/Collada/ColladaParser.h +++ b/code/Collada/ColladaParser.h @@ -94,8 +94,8 @@ namespace Assimp /** Reads contributor information such as author and legal blah */ void ReadContributorInfo(); - /** Reads generic metadata into provided map */ - void ReadMetaDataItem(StringMetaData &metadata); + /** 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); diff --git a/include/assimp/commonMetaData.h b/include/assimp/commonMetaData.h index 1e176725e..cca21ed2a 100644 --- a/include/assimp/commonMetaData.h +++ b/include/assimp/commonMetaData.h @@ -60,4 +60,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. /// Not all formats add this metadata. #define AI_METADATA_SOURCE_GENERATOR "SourceAsset_Generator" +/// Scene metadata holding the source asset copyright statement, if available. +/// Not all formats add this metadata. +#define AI_METADATA_SOURCE_COPYRIGHT "SourceAsset_Copyright" + #endif diff --git a/test/unit/utColladaImportExport.cpp b/test/unit/utColladaImportExport.cpp index 8bce3a3dd..1350d3607 100644 --- a/test/unit/utColladaImportExport.cpp +++ b/test/unit/utColladaImportExport.cpp @@ -40,76 +40,91 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. --------------------------------------------------------------------------- */ -#include "UnitTestPCH.h" #include "AbstractImportExportBase.h" +#include "UnitTestPCH.h" -#include -#include +#include #include +#include +#include using namespace Assimp; class utColladaImportExport : public AbstractImportExportBase { public: - virtual bool importerTest() { - Assimp::Importer importer; - const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.dae", aiProcess_ValidateDataStructure); - if (scene == nullptr) - return false; + virtual bool importerTest() { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.dae", aiProcess_ValidateDataStructure); + if (scene == nullptr) + return false; - // Expected number of items - EXPECT_EQ(scene->mNumMeshes, 1u); - EXPECT_EQ(scene->mNumMaterials, 1u); - EXPECT_EQ(scene->mNumAnimations, 0u); - EXPECT_EQ(scene->mNumTextures, 0u); - EXPECT_EQ(scene->mNumLights, 1u); - EXPECT_EQ(scene->mNumCameras, 1u); + // Expected number of items + EXPECT_EQ(scene->mNumMeshes, 1u); + EXPECT_EQ(scene->mNumMaterials, 1u); + EXPECT_EQ(scene->mNumAnimations, 0u); + EXPECT_EQ(scene->mNumTextures, 0u); + EXPECT_EQ(scene->mNumLights, 1u); + EXPECT_EQ(scene->mNumCameras, 1u); - return true; - } + // Expected common metadata + aiString value; + EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_FORMAT, value)) << "No importer format metadata"; + EXPECT_STREQ("Collada Importer", value.C_Str()); + + EXPECT_TRUE(scene->mMetaData->Get(AI_METADATA_SOURCE_FORMAT_VERSION, value)) << "No format version metadata"; + EXPECT_STREQ("1.4.1", value.C_Str()); + + 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_EQ(strncmp(value.C_Str(), "Copyright 2006", 14), 0) << "AI_METADATA_SOURCE_COPYRIGHT was: " << value.C_Str(); + + return true; + } }; TEST_F(utColladaImportExport, importBlenFromFileTest) { - EXPECT_TRUE(importerTest()); + EXPECT_TRUE(importerTest()); } class utColladaZaeImportExport : public AbstractImportExportBase { public: - virtual bool importerTest() { - { - Assimp::Importer importer; - const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.zae", aiProcess_ValidateDataStructure); - if (scene == nullptr) - return false; + virtual bool importerTest() { + { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.zae", aiProcess_ValidateDataStructure); + if (scene == nullptr) + return false; - // Expected number of items - EXPECT_EQ(scene->mNumMeshes, 1u); - EXPECT_EQ(scene->mNumMaterials, 1u); - EXPECT_EQ(scene->mNumAnimations, 0u); - EXPECT_EQ(scene->mNumTextures, 1u); - EXPECT_EQ(scene->mNumLights, 1u); - EXPECT_EQ(scene->mNumCameras, 1u); - } + // Expected number of items + EXPECT_EQ(scene->mNumMeshes, 1u); + EXPECT_EQ(scene->mNumMaterials, 1u); + EXPECT_EQ(scene->mNumAnimations, 0u); + EXPECT_EQ(scene->mNumTextures, 1u); + EXPECT_EQ(scene->mNumLights, 1u); + EXPECT_EQ(scene->mNumCameras, 1u); + } - { - Assimp::Importer importer; - const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck_nomanifest.zae", aiProcess_ValidateDataStructure); - if (scene == nullptr) - return false; + { + Assimp::Importer importer; + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck_nomanifest.zae", aiProcess_ValidateDataStructure); + if (scene == nullptr) + return false; - // Expected number of items - EXPECT_EQ(scene->mNumMeshes, 1u); - EXPECT_EQ(scene->mNumMaterials, 1u); - EXPECT_EQ(scene->mNumAnimations, 0u); - EXPECT_EQ(scene->mNumTextures, 1u); - EXPECT_EQ(scene->mNumLights, 1u); - EXPECT_EQ(scene->mNumCameras, 1u); - } + // Expected number of items + EXPECT_EQ(scene->mNumMeshes, 1u); + EXPECT_EQ(scene->mNumMaterials, 1u); + EXPECT_EQ(scene->mNumAnimations, 0u); + EXPECT_EQ(scene->mNumTextures, 1u); + EXPECT_EQ(scene->mNumLights, 1u); + EXPECT_EQ(scene->mNumCameras, 1u); + } - return true; - } + return true; + } }; TEST_F(utColladaZaeImportExport, importBlenFromFileTest) { - EXPECT_TRUE(importerTest()); + EXPECT_TRUE(importerTest()); } From 986b67801dd138c43de3e39593b92302405cde4b Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Mon, 9 Dec 2019 14:30:12 +0000 Subject: [PATCH 2/8] Add Copyright common metadata to glTF importer/exporter Technically this only exists in glTF v2 but may as well include in both --- code/glTF/glTFAssetWriter.inl | 3 +++ code/glTF/glTFExporter.cpp | 7 +++++++ code/glTF/glTFImporter.cpp | 7 ++++++- code/glTF2/glTF2AssetWriter.inl | 2 ++ code/glTF2/glTF2Exporter.cpp | 7 +++++++ code/glTF2/glTF2Importer.cpp | 7 ++++++- 6 files changed, 31 insertions(+), 2 deletions(-) diff --git a/code/glTF/glTFAssetWriter.inl b/code/glTF/glTFAssetWriter.inl index a3c99a9a8..ed1f4c6bc 100644 --- a/code/glTF/glTFAssetWriter.inl +++ b/code/glTF/glTFAssetWriter.inl @@ -627,6 +627,9 @@ namespace glTF { asset.SetObject(); asset.AddMember("version", Value(mAsset.asset.version, mAl).Move(), mAl); asset.AddMember("generator", Value(mAsset.asset.generator, mAl).Move(), mAl); + if (!mAsset.asset.copyright.empty()) + asset.AddMember("copyright", Value(mAsset.asset.copyright, mAl).Move(), mAl); + mDoc.AddMember("asset", asset, mAl); } diff --git a/code/glTF/glTFExporter.cpp b/code/glTF/glTFExporter.cpp index 3faa1a84b..cf8ef974a 100644 --- a/code/glTF/glTFExporter.cpp +++ b/code/glTF/glTFExporter.cpp @@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "glTF/glTFAssetWriter.h" #include "PostProcessing/SplitLargeMeshes.h" +#include #include #include #include @@ -873,6 +874,12 @@ void glTFExporter::ExportMetadata() aiGetVersionMajor(), aiGetVersionMinor(), aiGetVersionRevision()); asset.generator = buffer; + + // Copyright + aiString copyright_str; + if (mScene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, copyright_str)) { + asset.copyright = copyright_str.C_Str(); + } } inline void ExtractAnimationData(Asset& mAsset, std::string& animId, Ref& animRef, Ref& buffer, const aiNodeAnim* nodeChannel, float ticksPerSecond) diff --git a/code/glTF/glTFImporter.cpp b/code/glTF/glTFImporter.cpp index 59e7d9b92..9e743bf88 100644 --- a/code/glTF/glTFImporter.cpp +++ b/code/glTF/glTFImporter.cpp @@ -703,7 +703,8 @@ void glTFImporter::ImportCommonMetadata(glTF::Asset& a) ai_assert(mScene->mMetaData == nullptr); const bool hasVersion = !a.asset.version.empty(); const bool hasGenerator = !a.asset.generator.empty(); - if (hasVersion || hasGenerator) + const bool hasCopyright = !a.asset.copyright.empty(); + if (hasVersion || hasGenerator || hasCopyright) { mScene->mMetaData = new aiMetadata; if (hasVersion) @@ -714,6 +715,10 @@ void glTFImporter::ImportCommonMetadata(glTF::Asset& a) { mScene->mMetaData->Add(AI_METADATA_SOURCE_GENERATOR, aiString(a.asset.generator)); } + if (hasCopyright) + { + mScene->mMetaData->Add(AI_METADATA_SOURCE_COPYRIGHT, aiString(a.asset.copyright)); + } } } diff --git a/code/glTF2/glTF2AssetWriter.inl b/code/glTF2/glTF2AssetWriter.inl index 8ba4aa10b..ae69f3908 100644 --- a/code/glTF2/glTF2AssetWriter.inl +++ b/code/glTF2/glTF2AssetWriter.inl @@ -720,6 +720,8 @@ namespace glTF2 { asset.SetObject(); asset.AddMember("version", Value(mAsset.asset.version, mAl).Move(), mAl); asset.AddMember("generator", Value(mAsset.asset.generator, mAl).Move(), mAl); + if (!mAsset.asset.copyright.empty()) + asset.AddMember("copyright", Value(mAsset.asset.copyright, mAl).Move(), mAl); mDoc.AddMember("asset", asset, mAl); } diff --git a/code/glTF2/glTF2Exporter.cpp b/code/glTF2/glTF2Exporter.cpp index f85803b52..82f3907ef 100644 --- a/code/glTF2/glTF2Exporter.cpp +++ b/code/glTF2/glTF2Exporter.cpp @@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "glTF2/glTF2AssetWriter.h" #include "PostProcessing/SplitLargeMeshes.h" +#include #include #include #include @@ -996,6 +997,12 @@ void glTF2Exporter::ExportMetadata() aiGetVersionMajor(), aiGetVersionMinor(), aiGetVersionRevision()); asset.generator = buffer; + + // Copyright + aiString copyright_str; + if (mScene->mMetaData->Get(AI_METADATA_SOURCE_COPYRIGHT, copyright_str)) { + asset.copyright = copyright_str.C_Str(); + } } inline Ref GetSamplerInputRef(Asset& asset, std::string& animId, Ref& buffer, std::vector& times) diff --git a/code/glTF2/glTF2Importer.cpp b/code/glTF2/glTF2Importer.cpp index b14d68c27..4e3f111da 100644 --- a/code/glTF2/glTF2Importer.cpp +++ b/code/glTF2/glTF2Importer.cpp @@ -1306,7 +1306,8 @@ void glTF2Importer::ImportCommonMetadata(glTF2::Asset& a) { ai_assert(mScene->mMetaData == nullptr); const bool hasVersion = !a.asset.version.empty(); const bool hasGenerator = !a.asset.generator.empty(); - if (hasVersion || hasGenerator) + const bool hasCopyright = !a.asset.copyright.empty(); + if (hasVersion || hasGenerator || hasCopyright) { mScene->mMetaData = new aiMetadata; if (hasVersion) @@ -1317,6 +1318,10 @@ void glTF2Importer::ImportCommonMetadata(glTF2::Asset& a) { { mScene->mMetaData->Add(AI_METADATA_SOURCE_GENERATOR, aiString(a.asset.generator)); } + if (hasCopyright) + { + mScene->mMetaData->Add(AI_METADATA_SOURCE_COPYRIGHT, aiString(a.asset.copyright)); + } } } From 71d7ff63f5673df467ae86a9d55ffcefe51bbb28 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 13 Dec 2019 08:25:45 +0100 Subject: [PATCH 3/8] Update ColladaHelper.cpp Add spaces instead of tabs. --- code/Collada/ColladaHelper.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/code/Collada/ColladaHelper.cpp b/code/Collada/ColladaHelper.cpp index 3a22d4db3..f19bb1047 100644 --- a/code/Collada/ColladaHelper.cpp +++ b/code/Collada/ColladaHelper.cpp @@ -6,7 +6,6 @@ Open Asset Import Library (assimp) Copyright (c) 2006-2019, assimp team - All rights reserved. Redistribution and use of this software in source and binary forms, @@ -50,15 +49,15 @@ namespace Assimp { namespace Collada { const MetaKeyPairVector MakeColladaAssimpMetaKeys() { - MetaKeyPairVector result; - result.emplace_back("authoring_tool", AI_METADATA_SOURCE_GENERATOR); + MetaKeyPairVector result; + result.emplace_back("authoring_tool", AI_METADATA_SOURCE_GENERATOR); result.emplace_back("copyright", AI_METADATA_SOURCE_COPYRIGHT); - return result; + return result; }; const MetaKeyPairVector &GetColladaAssimpMetaKeys() { - static const MetaKeyPairVector result = MakeColladaAssimpMetaKeys(); - return result; + static const MetaKeyPairVector result = MakeColladaAssimpMetaKeys(); + return result; } } // namespace Collada From 53bf442bebb54e57332a91aae749a70c18c48f08 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 13 Dec 2019 08:30:40 +0100 Subject: [PATCH 4/8] Update ColladaParser.cpp Fix review findings. --- code/Collada/ColladaParser.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/code/Collada/ColladaParser.cpp b/code/Collada/ColladaParser.cpp index 1b5b3b4d6..1577c794a 100644 --- a/code/Collada/ColladaParser.cpp +++ b/code/Collada/ColladaParser.cpp @@ -438,13 +438,19 @@ void ColladaParser::ReadContributorInfo() } } -bool FindCommonKey(const char *collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) { +static bool FindCommonKey(const char *collada_key, const MetaKeyPairVector &key_renaming, size_t &found_index) { + found_index = 9999999u; + if ( nullptr == collada_key ) { + return false; + } + for (size_t i = 0; i < key_renaming.size(); ++i) { - if (strcmp(key_renaming[i].first, collada_key) == 0) { + if (strncmp(key_renaming[i].first, collada_key, strlen(collada_key)) == 0) { found_index = i; return true; } } + return false; } From 0ff04b97695b03c0824750b0c69c44057685e93a Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Fri, 13 Dec 2019 11:20:50 +0100 Subject: [PATCH 5/8] Update ColladaParser.cpp Fix the build and fix 2 possible nullptr dereferences. --- code/Collada/ColladaParser.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/code/Collada/ColladaParser.cpp b/code/Collada/ColladaParser.cpp index 1577c794a..df5a95c5b 100644 --- a/code/Collada/ColladaParser.cpp +++ b/code/Collada/ColladaParser.cpp @@ -250,6 +250,9 @@ void ColladaParser::UriDecodePath(aiString& ss) bool ColladaParser::ReadBoolFromTextContent() { const char* cur = GetTextContent(); + if ( nullptr == cur) { + return false; + } return (!ASSIMP_strincmp(cur, "true", 4) || '0' != *cur); } @@ -258,6 +261,9 @@ bool ColladaParser::ReadBoolFromTextContent() ai_real ColladaParser::ReadFloatFromTextContent() { const char* cur = GetTextContent(); + if ( nullptr == cur ) { + return 0.0; + } return fast_atof(cur); } @@ -278,7 +284,9 @@ void ColladaParser::ReadContents() const char* version = mReader->getAttributeValue(attrib); // Store declared format version string - mAssetMetaData.emplace(AI_METADATA_SOURCE_FORMAT_VERSION, version); + aiString v; + v.Set(version); + mAssetMetaData.emplace(AI_METADATA_SOURCE_FORMAT_VERSION, v ); if (!::strncmp(version, "1.5", 3)) { mFormat = FV_1_5_n; 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 6/8] 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; From ab50b5e18165b6529d68d52f2460e124e23caa95 Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Thu, 2 Jan 2020 13:02:40 +0000 Subject: [PATCH 7/8] Fix typo, fix GCC build --- code/Collada/ColladaHelper.cpp | 5 +++-- test/unit/utColladaExportLight.cpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/code/Collada/ColladaHelper.cpp b/code/Collada/ColladaHelper.cpp index aeabb49c4..8fa42b4d9 100644 --- a/code/Collada/ColladaHelper.cpp +++ b/code/Collada/ColladaHelper.cpp @@ -94,9 +94,10 @@ void ToCamelCase(std::string &text) } else { - // Make lower case - (*it) = ToLower(*it); + // Make next one lower case ++it; + if (it != text.end()) + (*it) = ToLower(*it); } } } diff --git a/test/unit/utColladaExportLight.cpp b/test/unit/utColladaExportLight.cpp index 9b3e8a0e7..c2aa17b2b 100644 --- a/test/unit/utColladaExportLight.cpp +++ b/test/unit/utColladaExportLight.cpp @@ -138,7 +138,7 @@ TEST_F(ColladaExportLight, testExportLight) 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"; + EXPECT_GT(readModified.length, ai_uint32(18)) << "Modified date too short"; // Lights EXPECT_TRUE(imported->HasLights()); From 4e50b05b856e1cd20812079a73670f0a5338783a Mon Sep 17 00:00:00 2001 From: RichardTea <31507749+RichardTea@users.noreply.github.com> Date: Thu, 2 Jan 2020 13:24:50 +0000 Subject: [PATCH 8/8] Fix off-by-one error --- code/Collada/ColladaHelper.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/code/Collada/ColladaHelper.cpp b/code/Collada/ColladaHelper.cpp index 8fa42b4d9..510ac657e 100644 --- a/code/Collada/ColladaHelper.cpp +++ b/code/Collada/ColladaHelper.cpp @@ -83,8 +83,10 @@ 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*/) + auto it = text.begin(); + (*it) = ToUpper(*it); + ++it; + for (/*started above*/ ; it != text.end(); /*iterated below*/) { if ((*it) == '_') { @@ -94,10 +96,9 @@ void ToCamelCase(std::string &text) } else { - // Make next one lower case - ++it; - if (it != text.end()) - (*it) = ToLower(*it); + // Make lower case + (*it) = ToLower(*it); + ++it; } } }