diff --git a/code/AssetLib/Collada/ColladaExporter.cpp b/code/AssetLib/Collada/ColladaExporter.cpp index 3f4f53a3d..567f7c8e7 100644 --- a/code/AssetLib/Collada/ColladaExporter.cpp +++ b/code/AssetLib/Collada/ColladaExporter.cpp @@ -117,22 +117,37 @@ static const std::string XMLIDEncode(const std::string &name) { return idEncoded.str(); } +// ------------------------------------------------------------------------------------------------ +// Helper functions to create unique ids +inline bool IsUniqueId(const std::unordered_set &idSet, const std::string &idStr) { + return (idSet.find(idStr) == idSet.end()); +} + +inline std::string MakeUniqueId(const std::unordered_set &idSet, const std::string &idPrefix, const std::string &postfix) { + std::string result(idPrefix + postfix); + if (!IsUniqueId(idSet, result)) { + // Select a number to append + size_t idnum = 1; + do { + result = idPrefix + '_' + to_string(idnum) + postfix; + ++idnum; + } while (!IsUniqueId(idSet, result)); + } + return result; +} + // ------------------------------------------------------------------------------------------------ // Constructor for a specific scene to export ColladaExporter::ColladaExporter(const aiScene *pScene, IOSystem *pIOSystem, const std::string &path, const std::string &file) : mIOSystem(pIOSystem), mPath(path), - mFile(file) { + mFile(file), + mScene(pScene), + endstr("\n") { // make sure that all formatting happens using the standard, C locale and not the user's current locale mOutput.imbue(std::locale("C")); mOutput.precision(ASSIMP_AI_REAL_TEXT_PRECISION); - mScene = pScene; - mSceneOwned = false; - - // set up strings - endstr = "\n"; - // start writing the file WriteFile(); } @@ -140,9 +155,6 @@ ColladaExporter::ColladaExporter(const aiScene *pScene, IOSystem *pIOSystem, con // ------------------------------------------------------------------------------------------------ // Destructor ColladaExporter::~ColladaExporter() { - if (mSceneOwned) { - delete mScene; - } } // ------------------------------------------------------------------------------------------------ @@ -171,10 +183,11 @@ void ColladaExporter::WriteFile() { // customized, Writes the animation library WriteAnimationsLibrary(); - // useless Collada fu at the end, just in case we haven't had enough indirections, yet. + // instantiate the scene(s) + // For Assimp there will only ever be one mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "mRootNode) + "\" />" << endstr; + mOutput << startstr << "" << endstr; PopTag(); mOutput << startstr << "" << endstr; PopTag(); @@ -209,13 +222,13 @@ void ColladaExporter::WriteHeader() { mScene->mRootNode->mTransformation.Decompose(scaling, rotation, position); rotation.Normalize(); - bool add_root_node = false; + mAdd_root_node = false; ai_real scale = 1.0; if (std::abs(scaling.x - scaling.y) <= epsilon && std::abs(scaling.x - scaling.z) <= epsilon && std::abs(scaling.y - scaling.z) <= epsilon) { scale = (ai_real)((((double)scaling.x) + ((double)scaling.y) + ((double)scaling.z)) / 3.0); } else { - add_root_node = true; + mAdd_root_node = true; } std::string up_axis = "Y_UP"; @@ -226,33 +239,19 @@ void ColladaExporter::WriteHeader() { } else if (rotation.Equal(z_rot, epsilon)) { up_axis = "Z_UP"; } else { - add_root_node = true; + mAdd_root_node = true; } if (!position.Equal(aiVector3D(0, 0, 0))) { - add_root_node = true; + mAdd_root_node = true; } - if (mScene->mRootNode->mNumChildren == 0) { - add_root_node = true; + // Assimp root nodes can have meshes, Collada Scenes cannot + if (mScene->mRootNode->mNumChildren == 0 || mScene->mRootNode->mMeshes != 0) { + mAdd_root_node = true; } - if (add_root_node) { - aiScene *scene; - SceneCombiner::CopyScene(&scene, mScene); - - aiNode *root = new aiNode("Scene"); - - root->mNumChildren = 1; - root->mChildren = new aiNode *[root->mNumChildren]; - - root->mChildren[0] = scene->mRootNode; - scene->mRootNode->mParent = root; - scene->mRootNode = root; - - mScene = scene; - mSceneOwned = true; - + if (mAdd_root_node) { up_axis = "Y_UP"; scale = 1.0; } @@ -1226,17 +1225,29 @@ void ColladaExporter::WriteFloatArray(const std::string &pIdString, FloatDataTyp // ------------------------------------------------------------------------------------------------ // Writes the scene library void ColladaExporter::WriteSceneLibrary() { - const std::string sceneId = GetNodeUniqueId(mScene->mRootNode); - const std::string sceneName = GetNodeName(mScene->mRootNode); + // Determine if we are using the aiScene root or our own + std::string sceneName("Scene"); + if (mAdd_root_node) { + mSceneId = MakeUniqueId(mUniqueIds, sceneName, std::string()); + mUniqueIds.insert(mSceneId); + } else { + mSceneId = GetNodeUniqueId(mScene->mRootNode); + sceneName = GetNodeName(mScene->mRootNode); + } mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); - // start recursive write at the root node - for (size_t a = 0; a < mScene->mRootNode->mNumChildren; ++a) - WriteNode(mScene->mRootNode->mChildren[a]); + if (mAdd_root_node) { + // Export the root node + WriteNode(mScene->mRootNode); + } else { + // Have already exported the root node + for (size_t a = 0; a < mScene->mRootNode->mNumChildren; ++a) + WriteNode(mScene->mRootNode->mChildren[a]); + } PopTag(); mOutput << startstr << "" << endstr; @@ -1493,12 +1504,9 @@ void ColladaExporter::WriteNode(const aiNode *pNode) { const std::string node_name = GetNodeName(pNode); mOutput << startstr << "" << endstr; @@ -1612,23 +1620,6 @@ void ColladaExporter::WriteNode(const aiNode *pNode) { mOutput << startstr << "" << endstr; } -inline bool IsUniqueId(const std::unordered_set &idSet, const std::string &idStr) { - return (idSet.find(idStr) == idSet.end()); -} - -inline std::string MakeUniqueId(const std::unordered_set &idSet, const std::string &idPrefix, const std::string &postfix) { - std::string result(idPrefix + postfix); - if (!IsUniqueId(idSet, result)) { - // Select a number to append - size_t idnum = 1; - do { - result = idPrefix + '_' + to_string(idnum) + postfix; - ++idnum; - } while (!IsUniqueId(idSet, result)); - } - return result; -} - void ColladaExporter::CreateNodeIds(const aiNode *node) { GetNodeUniqueId(node); for (size_t a = 0; a < node->mNumChildren; ++a) diff --git a/code/AssetLib/Collada/ColladaExporter.h b/code/AssetLib/Collada/ColladaExporter.h index bea65bafc..e9a3530ae 100644 --- a/code/AssetLib/Collada/ColladaExporter.h +++ b/code/AssetLib/Collada/ColladaExporter.h @@ -196,13 +196,14 @@ public: const std::string mFile; /// The scene to be written - const aiScene *mScene; - bool mSceneOwned; + const aiScene *const mScene; + std::string mSceneId; + bool mAdd_root_node = false; /// current line start string, contains the current indentation for simple stream insertion std::string startstr; /// current line end string for simple stream insertion - std::string endstr; + const std::string endstr; // pair of color and texture - texture precedences color struct Surface { diff --git a/code/AssetLib/Collada/ColladaParser.cpp b/code/AssetLib/Collada/ColladaParser.cpp index d83980929..04e6eb7d4 100644 --- a/code/AssetLib/Collada/ColladaParser.cpp +++ b/code/AssetLib/Collada/ColladaParser.cpp @@ -2479,7 +2479,7 @@ void ColladaParser::ReadSceneLibrary() { // read name if given. int indexName = TestAttribute("name"); - const char *attrName = "unnamed"; + const char *attrName = "Scene"; if (indexName > -1) attrName = mReader->getAttributeValue(indexName); diff --git a/test/unit/utColladaImportExport.cpp b/test/unit/utColladaImportExport.cpp index 876f60c54..451c8e235 100644 --- a/test/unit/utColladaImportExport.cpp +++ b/test/unit/utColladaImportExport.cpp @@ -42,6 +42,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "UnitTestPCH.h" #include +#include #include #include #include @@ -52,6 +53,20 @@ using namespace Assimp; class utColladaImportExport : public AbstractImportExportBase { public: + // Clones the scene in an exception-safe way + struct SceneCloner { + SceneCloner(const aiScene *scene) { + sceneCopy = nullptr; + SceneCombiner::CopyScene(&sceneCopy, scene); + } + + ~SceneCloner() { + delete sceneCopy; + sceneCopy = nullptr; + } + aiScene *sceneCopy; + }; + virtual bool importerTest() final { Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.dae", aiProcess_ValidateDataStructure); @@ -211,6 +226,60 @@ TEST_F(utColladaImportExport, importDaeFromFileTest) { EXPECT_TRUE(importerTest()); } +unsigned int GetMeshUseCount(const aiNode *rootNode) { + unsigned int result = rootNode->mNumMeshes; + for (unsigned int i = 0; i < rootNode->mNumChildren; ++i) { + result += GetMeshUseCount(rootNode->mChildren[i]); + } + return result; +} + +TEST_F(utColladaImportExport, exportRootNodeMeshTest) { + Assimp::Importer importer; + Assimp::Exporter exporter; + const char *outFile = "exportRootNodeMeshTest_out.dae"; + + const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.dae", aiProcess_ValidateDataStructure); + ASSERT_TRUE(scene != nullptr) << "Fatal: could not import duck.dae!"; + + ASSERT_EQ(0u, scene->mRootNode->mNumMeshes) << "Collada import should not give the root node a mesh"; + + { + SceneCloner clone(scene); + ASSERT_TRUE(clone.sceneCopy != nullptr) << "Fatal: could not copy scene!"; + // Do this by moving the meshes from the first child that has some + aiNode *rootNode = clone.sceneCopy->mRootNode; + ASSERT_TRUE(rootNode->mNumChildren > 0) << "Fatal: root has no children"; + aiNode *meshNode = rootNode->mChildren[0]; + ASSERT_EQ(1u, meshNode->mNumMeshes) << "Fatal: First child node has no duck mesh"; + + // Move the meshes to the parent + rootNode->mNumMeshes = meshNode->mNumMeshes; + rootNode->mMeshes = new unsigned int[rootNode->mNumMeshes]; + for (unsigned int i = 0; i < rootNode->mNumMeshes; ++i) { + rootNode->mMeshes[i] = meshNode->mMeshes[i]; + } + + // Remove the meshes from the original node + meshNode->mNumMeshes = 0; + delete[] meshNode->mMeshes; + meshNode->mMeshes = nullptr; + + ASSERT_EQ(AI_SUCCESS, exporter.Export(clone.sceneCopy, "collada", outFile)) << "Fatal: Could not export file"; + } + + // Reimport and look for meshes + scene = importer.ReadFile(outFile, aiProcess_ValidateDataStructure); + ASSERT_TRUE(scene != nullptr) << "Fatal: could not reimport!"; + + // A Collada root node is not allowed to have a mesh + ASSERT_EQ(0u, scene->mRootNode->mNumMeshes) << "Collada reimport should not give the root node a mesh"; + + // Walk nodes and counts used meshes + // Should be exactly one + EXPECT_EQ(1u, GetMeshUseCount(scene->mRootNode)) << "Nodes had unexpected number of meshes in use"; +} + TEST_F(utColladaImportExport, exporterUniqueIdsTest) { Assimp::Importer importer; Assimp::Exporter exporter;