Merge pull request #3205 from RichardTea/bug-3201-collada_root_meshes

Export Collada Meshes on root aiNode

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<std::string> &idSet, const std::string &idStr) {
+    return (idSet.find(idStr) == idSet.end());
+}
+
+inline std::string MakeUniqueId(const std::unordered_set<std::string> &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 << "<scene>" << endstr;
     PushTag();
-    mOutput << startstr << "<instance_visual_scene url=\"#" + GetNodeUniqueId(mScene->mRootNode) + "\" />" << endstr;
+    mOutput << startstr << "<instance_visual_scene url=\"#" + mSceneId + "\" />" << endstr;
     PopTag();
     mOutput << startstr << "</scene>" << 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) {
+    // Assimp root nodes can have meshes, Collada Scenes cannot
-        add_root_node = true;
+    if (mScene->mRootNode->mNumChildren == 0 || mScene->mRootNode->mMeshes != 0) {
+        mAdd_root_node = true;
     }
 
-    if (add_root_node) {
+    if (mAdd_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;
-
         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);
+    // Determine if we are using the aiScene root or our own
-    const std::string sceneName = GetNodeName(mScene->mRootNode);
+    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 << "<library_visual_scenes>" << endstr;
     PushTag();
-    mOutput << startstr << "<visual_scene id=\"" + sceneId + "\" name=\"" + sceneName + "\">" << endstr;
+    mOutput << startstr << "<visual_scene id=\"" + mSceneId + "\" name=\"" + sceneName + "\">" << endstr;
     PushTag();
 
-    // start recursive write at the root node
+    if (mAdd_root_node) {
-    for (size_t a = 0; a < mScene->mRootNode->mNumChildren; ++a)
+        // Export the root node
-        WriteNode(mScene->mRootNode->mChildren[a]);
+        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 << "</visual_scene>" << endstr;
@@ -1493,12 +1504,9 @@ void ColladaExporter::WriteNode(const aiNode *pNode) {
     const std::string node_name = GetNodeName(pNode);
     mOutput << startstr << "<node ";
     if (is_skeleton_root) {
-        mOutput << "id=\"" << node_id << "\" " << (is_joint ? "sid=\"" + node_id + "\" " : ""); // For now, only support one skeleton in a scene.
+        mFoundSkeletonRootNodeID = node_id; // For now, only support one skeleton in a scene.
-        mFoundSkeletonRootNodeID = node_id;
-    } else {
-        mOutput << "id=\"" << node_id << "\" " << (is_joint ? "sid=\"" + node_id + "\" " : "");
     }
-
+    mOutput << "id=\"" << node_id << "\" " << (is_joint ? "sid=\"" + node_id + "\" " : "");
     mOutput << "name=\"" << node_name
             << "\" type=\"" << node_type
             << "\">" << endstr;
@@ -1612,23 +1620,6 @@ void ColladaExporter::WriteNode(const aiNode *pNode) {
     mOutput << startstr << "</node>" << endstr;
 }
 
-inline bool IsUniqueId(const std::unordered_set<std::string> &idSet, const std::string &idStr) {
-    return (idSet.find(idStr) == idSet.end());
-}
-
-inline std::string MakeUniqueId(const std::unordered_set<std::string> &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)

code/AssetLib/Collada/ColladaExporter.h

@@ -196,13 +196,14 @@ public:
     const std::string mFile;
 
     /// The scene to be written
-    const aiScene *mScene;
+    const aiScene *const mScene;
-    bool mSceneOwned;
+    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 {

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);
 

test/unit/utColladaImportExport.cpp

@@ -42,6 +42,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "UnitTestPCH.h"
 
 #include <assimp/ColladaMetaData.h>
+#include <assimp/SceneCombiner.h>
 #include <assimp/commonMetaData.h>
 #include <assimp/postprocess.h>
 #include <assimp/scene.h>
@@ -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;