Collada: Ensure <geometry> has unique id

Use the "id" for mesh names by default.
Set option AI_CONFIG_IMPORT_COLLADA_USE_COLLADA_NAMES to use the mesh "name" instead

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}/ColladaMetaData.h
   ${HEADER_PATH}/commonMetaData.h
   ${HEADER_PATH}/defs.h
   ${HEADER_PATH}/Defines.h

code/Collada/ColladaExporter.cpp

@@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "ColladaExporter.h"
 #include <assimp/Bitmap.h>
+#include <assimp/ColladaMetaData.h>
 #include <assimp/DefaultIOSystem.h>
 #include <assimp/MathFunctions.h>
 #include <assimp/SceneCombiner.h>
@@ -115,7 +116,7 @@ static const std::string XMLIDEncode(const std::string &name) {
         if (strchr(XML_ID_CHARS, *it) != nullptr) {
             idEncoded << *it;
         } else {
-            // Select placeholder character based on invalid character to prevent name collisions
+            // Select placeholder character based on invalid character to reduce ID collisions
             idEncoded << XML_ID_CHARS[(*it) % XML_ID_CHARS_COUNT];
         }
     }
@@ -854,8 +855,8 @@ void ColladaExporter::WriteControllerLibrary() {
 // Writes a skin controller of the given mesh
 void ColladaExporter::WriteController(size_t pIndex) {
     const aiMesh *mesh = mScene->mMeshes[pIndex];
-    const std::string idstr = mesh->mName.length == 0 ? GetMeshId(pIndex) : mesh->mName.C_Str();
+    const std::string idstr = GetMeshUniqueId(pIndex);
-    const std::string idstrEscaped = XMLIDEncode(idstr);
+    const std::string namestr = GetMeshName(pIndex);
 
     if (mesh->mNumFaces == 0 || mesh->mNumVertices == 0)
         return;
@@ -863,11 +864,11 @@ void ColladaExporter::WriteController(size_t pIndex) {
     if (mesh->mNumBones == 0)
         return;
 
-    mOutput << startstr << "<controller id=\"" << idstrEscaped << "-skin\" ";
+    mOutput << startstr << "<controller id=\"" << idstr << "-skin\" ";
     mOutput << "name=\"skinCluster" << pIndex << "\">" << endstr;
     PushTag();
 
-    mOutput << startstr << "<skin source=\"#" << idstrEscaped << "\">" << endstr;
+    mOutput << startstr << "<skin source=\"#" << idstr << "\">" << endstr;
     PushTag();
 
     // bind pose matrix
@@ -884,10 +885,10 @@ void ColladaExporter::WriteController(size_t pIndex) {
     PopTag();
     mOutput << startstr << "</bind_shape_matrix>" << endstr;
 
-    mOutput << startstr << "<source id=\"" << idstrEscaped << "-skin-joints\" name=\"" << idstrEscaped << "-skin-joints\">" << endstr;
+    mOutput << startstr << "<source id=\"" << idstr << "-skin-joints\" name=\"" << namestr << "-skin-joints\">" << endstr;
     PushTag();
 
-    mOutput << startstr << "<Name_array id=\"" << idstrEscaped << "-skin-joints-array\" count=\"" << mesh->mNumBones << "\">";
+    mOutput << startstr << "<Name_array id=\"" << idstr << "-skin-joints-array\" count=\"" << mesh->mNumBones << "\">";
 
     for (size_t i = 0; i < mesh->mNumBones; ++i)
         mOutput << XMLIDEncode(mesh->mBones[i]->mName.C_Str()) << " ";
@@ -897,7 +898,7 @@ void ColladaExporter::WriteController(size_t pIndex) {
     mOutput << startstr << "<technique_common>" << endstr;
     PushTag();
 
-    mOutput << startstr << "<accessor source=\"#" << idstrEscaped << "-skin-joints-array\" count=\"" << mesh->mNumBones << "\" stride=\"" << 1 << "\">" << endstr;
+    mOutput << startstr << "<accessor source=\"#" << idstr << "-skin-joints-array\" count=\"" << mesh->mNumBones << "\" stride=\"" << 1 << "\">" << endstr;
     PushTag();
 
     mOutput << startstr << "<param name=\"JOINT\" type=\"Name\"></param>" << endstr;
@@ -934,8 +935,8 @@ void ColladaExporter::WriteController(size_t pIndex) {
     mOutput << startstr << "<joints>" << endstr;
     PushTag();
 
-    mOutput << startstr << "<input semantic=\"JOINT\" source=\"#" << idstrEscaped << "-skin-joints\"></input>" << endstr;
+    mOutput << startstr << "<input semantic=\"JOINT\" source=\"#" << idstr << "-skin-joints\"></input>" << endstr;
-    mOutput << startstr << "<input semantic=\"INV_BIND_MATRIX\" source=\"#" << idstrEscaped << "-skin-bind_poses\"></input>" << endstr;
+    mOutput << startstr << "<input semantic=\"INV_BIND_MATRIX\" source=\"#" << idstr << "-skin-bind_poses\"></input>" << endstr;
 
     PopTag();
     mOutput << startstr << "</joints>" << endstr;
@@ -943,8 +944,8 @@ void ColladaExporter::WriteController(size_t pIndex) {
     mOutput << startstr << "<vertex_weights count=\"" << mesh->mNumVertices << "\">" << endstr;
     PushTag();
 
-    mOutput << startstr << "<input semantic=\"JOINT\" source=\"#" << idstrEscaped << "-skin-joints\" offset=\"0\"></input>" << endstr;
+    mOutput << startstr << "<input semantic=\"JOINT\" source=\"#" << idstr << "-skin-joints\" offset=\"0\"></input>" << endstr;
-    mOutput << startstr << "<input semantic=\"WEIGHT\" source=\"#" << idstrEscaped << "-skin-weights\" offset=\"1\"></input>" << endstr;
+    mOutput << startstr << "<input semantic=\"WEIGHT\" source=\"#" << idstr << "-skin-weights\" offset=\"1\"></input>" << endstr;
 
     mOutput << startstr << "<vcount>";
 
@@ -1019,9 +1020,8 @@ void ColladaExporter::WriteGeometryLibrary() {
 // Writes the given mesh
 void ColladaExporter::WriteGeometry(size_t pIndex) {
     const aiMesh *mesh = mScene->mMeshes[pIndex];
-    const std::string idstr = mesh->mName.length == 0 ? GetMeshId(pIndex) : mesh->mName.C_Str();
+    const std::string geometryName = GetMeshName(pIndex);
-    const std::string geometryName = XMLEscape(idstr);
+    const std::string geometryId = GetMeshUniqueId(pIndex);
-    const std::string geometryId = XMLIDEncode(idstr);
 
     if (mesh->mNumFaces == 0 || mesh->mNumVertices == 0)
         return;
@@ -1034,15 +1034,15 @@ void ColladaExporter::WriteGeometry(size_t pIndex) {
     PushTag();
 
     // Positions
-    WriteFloatArray(idstr + "-positions", FloatType_Vector, (ai_real *)mesh->mVertices, mesh->mNumVertices);
+    WriteFloatArray(geometryId + "-positions", FloatType_Vector, (ai_real *)mesh->mVertices, mesh->mNumVertices);
     // Normals, if any
     if (mesh->HasNormals())
-        WriteFloatArray(idstr + "-normals", FloatType_Vector, (ai_real *)mesh->mNormals, mesh->mNumVertices);
+        WriteFloatArray(geometryId + "-normals", FloatType_Vector, (ai_real *)mesh->mNormals, mesh->mNumVertices);
 
     // texture coords
     for (size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a) {
         if (mesh->HasTextureCoords(static_cast<unsigned int>(a))) {
-            WriteFloatArray(idstr + "-tex" + to_string(a), mesh->mNumUVComponents[a] == 3 ? FloatType_TexCoord3 : FloatType_TexCoord2,
+            WriteFloatArray(geometryId + "-tex" + to_string(a), mesh->mNumUVComponents[a] == 3 ? FloatType_TexCoord3 : FloatType_TexCoord2,
                     (ai_real *)mesh->mTextureCoords[a], mesh->mNumVertices);
         }
     }
@@ -1050,7 +1050,7 @@ void ColladaExporter::WriteGeometry(size_t pIndex) {
     // vertex colors
     for (size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a) {
         if (mesh->HasVertexColors(static_cast<unsigned int>(a)))
-            WriteFloatArray(idstr + "-color" + to_string(a), FloatType_Color, (ai_real *)mesh->mColors[a], mesh->mNumVertices);
+            WriteFloatArray(geometryId + "-color" + to_string(a), FloatType_Color, (ai_real *)mesh->mColors[a], mesh->mNumVertices);
     }
 
     // assemble vertex structure
@@ -1530,13 +1530,13 @@ void ColladaExporter::WriteNode(const aiScene *pScene, aiNode *pNode) {
     const std::string node_name = XMLEscape(pNode->mName.data);
     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.
+        mOutput << "id=\"" << node_id << "\" " << (is_joint ? "sid=\"" + 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
+    mOutput << "name=\"" << node_name
             << "\" type=\"" << node_type
             << "\">" << endstr;
     PushTag();
@@ -1595,14 +1595,14 @@ void ColladaExporter::WriteNode(const aiScene *pScene, aiNode *pNode) {
             if (mesh->mNumFaces == 0 || mesh->mNumVertices == 0)
                 continue;
 
-            const std::string meshName = mesh->mName.length == 0 ? GetMeshId(pNode->mMeshes[a]) : mesh->mName.C_Str();
+            const std::string meshId = GetMeshUniqueId(pNode->mMeshes[a]);
 
             if (mesh->mNumBones == 0) {
-                mOutput << startstr << "<instance_geometry url=\"#" << XMLIDEncode(meshName) << "\">" << endstr;
+                mOutput << startstr << "<instance_geometry url=\"#" << meshId << "\">" << endstr;
                 PushTag();
             } else {
                 mOutput << startstr
-                        << "<instance_controller url=\"#" << XMLIDEncode(meshName) << "-skin\">"
+                        << "<instance_controller url=\"#" << meshId << "-skin\">"
                         << endstr;
                 PushTag();
 
@@ -1649,5 +1649,59 @@ void ColladaExporter::WriteNode(const aiScene *pScene, aiNode *pNode) {
     mOutput << startstr << "</node>" << endstr;
 }
 
+/// Get or Create a unique mesh ID string for the given mesh index
+std::string Assimp::ColladaExporter::GetMeshUniqueId(size_t pIndex) {
+    auto meshId = mMeshIdMap.find(pIndex);
+    if (meshId != mMeshIdMap.cend())
+        return meshId->second;
+
+    // Not seen this mesh before, create and add
+    return AddMeshIndexToMaps(pIndex, true);
+}
+
+std::string Assimp::ColladaExporter::GetMeshName(size_t pIndex) {
+    auto meshName = mMeshNameMap.find(pIndex);
+    if (meshName != mMeshNameMap.cend())
+        return meshName->second;
+
+    // Not seen this mesh before, create and add
+    return AddMeshIndexToMaps(pIndex, false);
+}
+
+inline bool ValueIsUnique(const std::map<size_t, std::string> &map, const std::string &value) {
+    for (const auto &map_val : map) {
+        if (value == map_val.second)
+            return false;
+    }
+    return true;
+}
+
+// Add the mesh index to both Id and Name maps and return either Id or Name
+std::string Assimp::ColladaExporter::AddMeshIndexToMaps(size_t pIndex, bool meshId) {
+    const aiMesh *mesh = mScene->mMeshes[pIndex];
+    std::string idStr = mesh->mName.length == 0 ? std::string("meshId_") + to_string(pIndex) : XMLIDEncode(mesh->mName.C_Str());
+    // Ensure is unique. Relatively slow but will only happen once per mesh
+    if (!ValueIsUnique(mMeshIdMap, idStr)) {
+        // Select a number to append
+        size_t postfix = 1;
+        idStr.append("_");
+        while (!ValueIsUnique(mMeshIdMap, idStr + to_string(postfix))) {
+            ++postfix;
+        }
+        idStr = idStr + to_string(postfix);
+    }
+    // Add to map
+    mMeshIdMap.insert(std::make_pair(pIndex, idStr));
+
+    // Add name to map
+    const std::string nameStr = mesh->mName.length == 0 ? idStr : XMLEscape(mesh->mName.C_Str());
+    mMeshNameMap.insert(std::make_pair(pIndex, nameStr));
+
+    if (meshId)
+        return idStr;
+    else
+        return nameStr;
+}
+
 #endif
 #endif

code/Collada/ColladaExporter.h

@@ -145,10 +145,14 @@ protected:
         startstr.erase(startstr.length() - 2);
     }
 
-    /// Creates a mesh ID for the given mesh
+    /// Get or Create a unique mesh ID string for the given mesh index
-    std::string GetMeshId(size_t pIndex) const {
+    std::string GetMeshUniqueId(size_t pIndex);
-        return std::string("meshId") + to_string(pIndex);
+    std::string GetMeshName(size_t pIndex);
-    }
+
+private:
+    std::string AddMeshIndexToMaps(size_t pIndex, bool meshId);
+    mutable std::map<size_t, std::string> mMeshIdMap; // Cache of encoded unique IDs
+    mutable std::map<size_t, std::string> mMeshNameMap; // Cache of encoded mesh names
 
 public:
     /// Stringstream to write all output into

code/Collada/ColladaHelper.h

@@ -339,11 +339,13 @@ struct SubMesh {
 
 /** Contains data for a single mesh */
 struct Mesh {
-    Mesh() {
+    Mesh(const std::string &id) :
+            mId(id) {
         for (unsigned int i = 0; i < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++i)
             mNumUVComponents[i] = 2;
     }
 
+    const std::string mId;
     std::string mName;
 
     // just to check if there's some sophisticated addressing involved...

code/Collada/ColladaLoader.cpp

@@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "ColladaLoader.h"
 #include "ColladaParser.h"
 
+#include <assimp/ColladaMetaData.h>
 #include <assimp/Defines.h>
 #include <assimp/anim.h>
 #include <assimp/importerdesc.h>
@@ -265,6 +266,13 @@ aiNode *ColladaLoader::BuildHierarchy(const ColladaParser &pParser, const Collad
 
     // find a name for the new node. It's more complicated than you might think
     node->mName.Set(FindNameForNode(pNode));
+    // if we're not using the unique IDs, hold onto them for reference and export
+    if (useColladaName) {
+        if (!pNode->mID.empty())
+            node->mMetaData->Add(AI_METADATA_COLLADA_ID, aiString(pNode->mID));
+        if (!pNode->mSID.empty())
+            node->mMetaData->Add(AI_METADATA_COLLADA_SID, aiString(pNode->mSID));
+    }
 
     // calculate the transformation matrix for it
     node->mTransformation = pParser.CalculateResultTransform(pNode->mTransforms);
@@ -603,7 +611,11 @@ aiMesh *ColladaLoader::CreateMesh(const ColladaParser &pParser, const Collada::M
         const Collada::Controller *pSrcController, size_t pStartVertex, size_t pStartFace) {
     std::unique_ptr<aiMesh> dstMesh(new aiMesh);
 
+    if (useColladaName) {
         dstMesh->mName = pSrcMesh->mName;
+    } else {
+        dstMesh->mName = pSrcMesh->mId;
+    }
 
     // count the vertices addressed by its faces
     const size_t numVertices = std::accumulate(pSrcMesh->mFaceSize.begin() + pStartFace,
@@ -700,10 +712,10 @@ aiMesh *ColladaLoader::CreateMesh(const ColladaParser &pParser, const Collada::M
             for (unsigned int i = 0; i < targetData.mStrings.size(); ++i) {
                 const Collada::Mesh *targetMesh = pParser.ResolveLibraryReference(pParser.mMeshLibrary, targetData.mStrings.at(i));
 
-                aiMesh *aimesh = findMesh(targetMesh->mName);
+                aiMesh *aimesh = findMesh(useColladaName ? targetMesh->mName : targetMesh->mId);
                 if (!aimesh) {
                     if (targetMesh->mSubMeshes.size() > 1) {
-                        throw DeadlyImportError("Morhing target mesh must be a single");
+                        throw DeadlyImportError("Morphing target mesh must be a single");
                     }
                     aimesh = CreateMesh(pParser, targetMesh, targetMesh->mSubMeshes.at(0), NULL, 0, 0);
                     mTargetMeshes.push_back(aimesh);

code/Collada/ColladaParser.cpp

@@ -1716,9 +1716,10 @@ void ColladaParser::ReadGeometryLibrary() {
                 // TODO: (thom) support SIDs
                 // ai_assert( TestAttribute( "sid") == -1);
 
-                // create a mesh and store it in the library under its ID
+                // create a mesh and store it in the library under its (resolved) ID
-                Mesh *mesh = new Mesh;
+                // Skip and warn if ID is not unique
-                mMeshLibrary[id] = mesh;
+                if (mMeshLibrary.find(id) == mMeshLibrary.cend()) {
+                    std::unique_ptr<Mesh> mesh(new Mesh(id));
 
                     // read the mesh name if it exists
                     const int nameIndex = TestAttribute("name");
@@ -1727,7 +1728,13 @@ void ColladaParser::ReadGeometryLibrary() {
                     }
 
                     // read on from there
-                ReadGeometry(mesh);
+                    ReadGeometry(*mesh);
+                    // Read successfully, add to library
+                    mMeshLibrary.insert({ id, mesh.release() });
+                } else {
+                    ASSIMP_LOG_ERROR_F("Collada: Skipped duplicate geometry id: \"", id, "\"");
+                    SkipElement();
+                }
             } else {
                 // ignore the rest
                 SkipElement();
@@ -1743,7 +1750,7 @@ void ColladaParser::ReadGeometryLibrary() {
 
 // ------------------------------------------------------------------------------------------------
 // Reads a geometry from the geometry library.
-void ColladaParser::ReadGeometry(Collada::Mesh *pMesh) {
+void ColladaParser::ReadGeometry(Collada::Mesh &pMesh) {
     if (mReader->isEmptyElement())
         return;
 
@@ -1767,7 +1774,7 @@ void ColladaParser::ReadGeometry(Collada::Mesh *pMesh) {
 
 // ------------------------------------------------------------------------------------------------
 // Reads a mesh from the geometry library
-void ColladaParser::ReadMesh(Mesh *pMesh) {
+void ColladaParser::ReadMesh(Mesh &pMesh) {
     if (mReader->isEmptyElement())
         return;
 
@@ -1997,16 +2004,16 @@ void ColladaParser::ReadAccessor(const std::string &pID) {
 
 // ------------------------------------------------------------------------------------------------
 // Reads input declarations of per-vertex mesh data into the given mesh
-void ColladaParser::ReadVertexData(Mesh *pMesh) {
+void ColladaParser::ReadVertexData(Mesh &pMesh) {
     // extract the ID of the <vertices> element. Not that we care, but to catch strange referencing schemes we should warn about
     int attrID = GetAttribute("id");
-    pMesh->mVertexID = mReader->getAttributeValue(attrID);
+    pMesh.mVertexID = mReader->getAttributeValue(attrID);
 
     // a number of <input> elements
     while (mReader->read()) {
         if (mReader->getNodeType() == irr::io::EXN_ELEMENT) {
             if (IsElement("input")) {
-                ReadInputChannel(pMesh->mPerVertexData);
+                ReadInputChannel(pMesh.mPerVertexData);
             } else {
                 ThrowException(format() << "Unexpected sub element <" << mReader->getNodeName() << "> in tag <vertices>");
             }
@@ -2021,7 +2028,7 @@ void ColladaParser::ReadVertexData(Mesh *pMesh) {
 
 // ------------------------------------------------------------------------------------------------
 // Reads input declarations of per-index mesh data into the given mesh
-void ColladaParser::ReadIndexData(Mesh *pMesh) {
+void ColladaParser::ReadIndexData(Mesh &pMesh) {
     std::vector<size_t> vcount;
     std::vector<InputChannel> perIndexData;
 
@@ -2111,7 +2118,7 @@ void ColladaParser::ReadIndexData(Mesh *pMesh) {
 
     // only when we're done reading all <p> tags (and thus know the final vertex count) can we commit the submesh
     subgroup.mNumFaces = actualPrimitives;
-    pMesh->mSubMeshes.push_back(subgroup);
+    pMesh.mSubMeshes.push_back(subgroup);
 }
 
 // ------------------------------------------------------------------------------------------------
@@ -2158,7 +2165,7 @@ void ColladaParser::ReadInputChannel(std::vector<InputChannel> &poChannels) {
 
 // ------------------------------------------------------------------------------------------------
 // Reads a <p> primitive index list and assembles the mesh data into the given mesh
-size_t ColladaParser::ReadPrimitives(Mesh *pMesh, std::vector<InputChannel> &pPerIndexChannels,
+size_t ColladaParser::ReadPrimitives(Mesh &pMesh, std::vector<InputChannel> &pPerIndexChannels,
         size_t pNumPrimitives, const std::vector<size_t> &pVCount, PrimitiveType pPrimType) {
     // determine number of indices coming per vertex
     // find the offset index for all per-vertex channels
@@ -2220,7 +2227,7 @@ size_t ColladaParser::ReadPrimitives(Mesh *pMesh, std::vector<InputChannel> &pPe
         ThrowException("Expected different index count in <p> element.");
 
     // find the data for all sources
-    for (std::vector<InputChannel>::iterator it = pMesh->mPerVertexData.begin(); it != pMesh->mPerVertexData.end(); ++it) {
+    for (std::vector<InputChannel>::iterator it = pMesh.mPerVertexData.begin(); it != pMesh.mPerVertexData.end(); ++it) {
         InputChannel &input = *it;
         if (input.mResolved)
             continue;
@@ -2241,7 +2248,7 @@ size_t ColladaParser::ReadPrimitives(Mesh *pMesh, std::vector<InputChannel> &pPe
         // ignore vertex pointer, it doesn't refer to an accessor
         if (input.mType == IT_Vertex) {
             // warn if the vertex channel does not refer to the <vertices> element in the same mesh
-            if (input.mAccessor != pMesh->mVertexID)
+            if (input.mAccessor != pMesh.mVertexID)
                 ThrowException("Unsupported vertex referencing scheme.");
             continue;
         }
@@ -2268,8 +2275,8 @@ size_t ColladaParser::ReadPrimitives(Mesh *pMesh, std::vector<InputChannel> &pPe
         numPrimitives = numberOfVertices - 1;
     }
 
-    pMesh->mFaceSize.reserve(numPrimitives);
+    pMesh.mFaceSize.reserve(numPrimitives);
-    pMesh->mFacePosIndices.reserve(indices.size() / numOffsets);
+    pMesh.mFacePosIndices.reserve(indices.size() / numOffsets);
 
     size_t polylistStartVertex = 0;
     for (size_t currentPrimitive = 0; currentPrimitive < numPrimitives; currentPrimitive++) {
@@ -2314,7 +2321,7 @@ size_t ColladaParser::ReadPrimitives(Mesh *pMesh, std::vector<InputChannel> &pPe
         }
 
         // store the face size to later reconstruct the face from
-        pMesh->mFaceSize.push_back(numPoints);
+        pMesh.mFaceSize.push_back(numPoints);
     }
 
     // if I ever get my hands on that guy who invented this steaming pile of indirection...
@@ -2325,7 +2332,7 @@ size_t ColladaParser::ReadPrimitives(Mesh *pMesh, std::vector<InputChannel> &pPe
 ///@note This function willn't work correctly if both PerIndex and PerVertex channels have same channels.
 ///For example if TEXCOORD present in both <vertices> and <polylist> tags this function will create wrong uv coordinates.
 ///It's not clear from COLLADA documentation is this allowed or not. For now only exporter fixed to avoid such behavior
-void ColladaParser::CopyVertex(size_t currentVertex, size_t numOffsets, size_t numPoints, size_t perVertexOffset, Mesh *pMesh, std::vector<InputChannel> &pPerIndexChannels, size_t currentPrimitive, const std::vector<size_t> &indices) {
+void ColladaParser::CopyVertex(size_t currentVertex, size_t numOffsets, size_t numPoints, size_t perVertexOffset, Mesh &pMesh, std::vector<InputChannel> &pPerIndexChannels, size_t currentPrimitive, const std::vector<size_t> &indices) {
     // calculate the base offset of the vertex whose attributes we ant to copy
     size_t baseOffset = currentPrimitive * numOffsets * numPoints + currentVertex * numOffsets;
 
@@ -2333,17 +2340,17 @@ void ColladaParser::CopyVertex(size_t currentVertex, size_t numOffsets, size_t n
     ai_assert((baseOffset + numOffsets - 1) < indices.size());
 
     // extract per-vertex channels using the global per-vertex offset
-    for (std::vector<InputChannel>::iterator it = pMesh->mPerVertexData.begin(); it != pMesh->mPerVertexData.end(); ++it)
+    for (std::vector<InputChannel>::iterator it = pMesh.mPerVertexData.begin(); it != pMesh.mPerVertexData.end(); ++it)
         ExtractDataObjectFromChannel(*it, indices[baseOffset + perVertexOffset], pMesh);
     // and extract per-index channels using there specified offset
     for (std::vector<InputChannel>::iterator it = pPerIndexChannels.begin(); it != pPerIndexChannels.end(); ++it)
         ExtractDataObjectFromChannel(*it, indices[baseOffset + it->mOffset], pMesh);
 
     // store the vertex-data index for later assignment of bone vertex weights
-    pMesh->mFacePosIndices.push_back(indices[baseOffset + perVertexOffset]);
+    pMesh.mFacePosIndices.push_back(indices[baseOffset + perVertexOffset]);
 }
 
-void ColladaParser::ReadPrimTriStrips(size_t numOffsets, size_t perVertexOffset, Mesh *pMesh, std::vector<InputChannel> &pPerIndexChannels, size_t currentPrimitive, const std::vector<size_t> &indices) {
+void ColladaParser::ReadPrimTriStrips(size_t numOffsets, size_t perVertexOffset, Mesh &pMesh, std::vector<InputChannel> &pPerIndexChannels, size_t currentPrimitive, const std::vector<size_t> &indices) {
     if (currentPrimitive % 2 != 0) {
         //odd tristrip triangles need their indices mangled, to preserve winding direction
         CopyVertex(1, numOffsets, 1, perVertexOffset, pMesh, pPerIndexChannels, currentPrimitive, indices);
@@ -2358,7 +2365,7 @@ void ColladaParser::ReadPrimTriStrips(size_t numOffsets, size_t perVertexOffset,
 
 // ------------------------------------------------------------------------------------------------
 // Extracts a single object from an input channel and stores it in the appropriate mesh data array
-void ColladaParser::ExtractDataObjectFromChannel(const InputChannel &pInput, size_t pLocalIndex, Mesh *pMesh) {
+void ColladaParser::ExtractDataObjectFromChannel(const InputChannel &pInput, size_t pLocalIndex, Mesh &pMesh) {
     // ignore vertex referrer - we handle them that separate
     if (pInput.mType == IT_Vertex)
         return;
@@ -2380,40 +2387,40 @@ void ColladaParser::ExtractDataObjectFromChannel(const InputChannel &pInput, siz
     switch (pInput.mType) {
     case IT_Position: // ignore all position streams except 0 - there can be only one position
         if (pInput.mIndex == 0)
-            pMesh->mPositions.push_back(aiVector3D(obj[0], obj[1], obj[2]));
+            pMesh.mPositions.push_back(aiVector3D(obj[0], obj[1], obj[2]));
         else
             ASSIMP_LOG_ERROR("Collada: just one vertex position stream supported");
         break;
     case IT_Normal:
         // pad to current vertex count if necessary
-        if (pMesh->mNormals.size() < pMesh->mPositions.size() - 1)
+        if (pMesh.mNormals.size() < pMesh.mPositions.size() - 1)
-            pMesh->mNormals.insert(pMesh->mNormals.end(), pMesh->mPositions.size() - pMesh->mNormals.size() - 1, aiVector3D(0, 1, 0));
+            pMesh.mNormals.insert(pMesh.mNormals.end(), pMesh.mPositions.size() - pMesh.mNormals.size() - 1, aiVector3D(0, 1, 0));
 
         // ignore all normal streams except 0 - there can be only one normal
         if (pInput.mIndex == 0)
-            pMesh->mNormals.push_back(aiVector3D(obj[0], obj[1], obj[2]));
+            pMesh.mNormals.push_back(aiVector3D(obj[0], obj[1], obj[2]));
         else
             ASSIMP_LOG_ERROR("Collada: just one vertex normal stream supported");
         break;
     case IT_Tangent:
         // pad to current vertex count if necessary
-        if (pMesh->mTangents.size() < pMesh->mPositions.size() - 1)
+        if (pMesh.mTangents.size() < pMesh.mPositions.size() - 1)
-            pMesh->mTangents.insert(pMesh->mTangents.end(), pMesh->mPositions.size() - pMesh->mTangents.size() - 1, aiVector3D(1, 0, 0));
+            pMesh.mTangents.insert(pMesh.mTangents.end(), pMesh.mPositions.size() - pMesh.mTangents.size() - 1, aiVector3D(1, 0, 0));
 
         // ignore all tangent streams except 0 - there can be only one tangent
         if (pInput.mIndex == 0)
-            pMesh->mTangents.push_back(aiVector3D(obj[0], obj[1], obj[2]));
+            pMesh.mTangents.push_back(aiVector3D(obj[0], obj[1], obj[2]));
         else
             ASSIMP_LOG_ERROR("Collada: just one vertex tangent stream supported");
         break;
     case IT_Bitangent:
         // pad to current vertex count if necessary
-        if (pMesh->mBitangents.size() < pMesh->mPositions.size() - 1)
+        if (pMesh.mBitangents.size() < pMesh.mPositions.size() - 1)
-            pMesh->mBitangents.insert(pMesh->mBitangents.end(), pMesh->mPositions.size() - pMesh->mBitangents.size() - 1, aiVector3D(0, 0, 1));
+            pMesh.mBitangents.insert(pMesh.mBitangents.end(), pMesh.mPositions.size() - pMesh.mBitangents.size() - 1, aiVector3D(0, 0, 1));
 
         // ignore all bitangent streams except 0 - there can be only one bitangent
         if (pInput.mIndex == 0)
-            pMesh->mBitangents.push_back(aiVector3D(obj[0], obj[1], obj[2]));
+            pMesh.mBitangents.push_back(aiVector3D(obj[0], obj[1], obj[2]));
         else
             ASSIMP_LOG_ERROR("Collada: just one vertex bitangent stream supported");
         break;
@@ -2421,13 +2428,13 @@ void ColladaParser::ExtractDataObjectFromChannel(const InputChannel &pInput, siz
         // up to 4 texture coord sets are fine, ignore the others
         if (pInput.mIndex < AI_MAX_NUMBER_OF_TEXTURECOORDS) {
             // pad to current vertex count if necessary
-            if (pMesh->mTexCoords[pInput.mIndex].size() < pMesh->mPositions.size() - 1)
+            if (pMesh.mTexCoords[pInput.mIndex].size() < pMesh.mPositions.size() - 1)
-                pMesh->mTexCoords[pInput.mIndex].insert(pMesh->mTexCoords[pInput.mIndex].end(),
+                pMesh.mTexCoords[pInput.mIndex].insert(pMesh.mTexCoords[pInput.mIndex].end(),
-                        pMesh->mPositions.size() - pMesh->mTexCoords[pInput.mIndex].size() - 1, aiVector3D(0, 0, 0));
+                        pMesh.mPositions.size() - pMesh.mTexCoords[pInput.mIndex].size() - 1, aiVector3D(0, 0, 0));
 
-            pMesh->mTexCoords[pInput.mIndex].push_back(aiVector3D(obj[0], obj[1], obj[2]));
+            pMesh.mTexCoords[pInput.mIndex].push_back(aiVector3D(obj[0], obj[1], obj[2]));
             if (0 != acc.mSubOffset[2] || 0 != acc.mSubOffset[3]) /* hack ... consider cleaner solution */
-                pMesh->mNumUVComponents[pInput.mIndex] = 3;
+                pMesh.mNumUVComponents[pInput.mIndex] = 3;
         } else {
             ASSIMP_LOG_ERROR("Collada: too many texture coordinate sets. Skipping.");
         }
@@ -2436,15 +2443,15 @@ void ColladaParser::ExtractDataObjectFromChannel(const InputChannel &pInput, siz
         // up to 4 color sets are fine, ignore the others
         if (pInput.mIndex < AI_MAX_NUMBER_OF_COLOR_SETS) {
             // pad to current vertex count if necessary
-            if (pMesh->mColors[pInput.mIndex].size() < pMesh->mPositions.size() - 1)
+            if (pMesh.mColors[pInput.mIndex].size() < pMesh.mPositions.size() - 1)
-                pMesh->mColors[pInput.mIndex].insert(pMesh->mColors[pInput.mIndex].end(),
+                pMesh.mColors[pInput.mIndex].insert(pMesh.mColors[pInput.mIndex].end(),
-                        pMesh->mPositions.size() - pMesh->mColors[pInput.mIndex].size() - 1, aiColor4D(0, 0, 0, 1));
+                        pMesh.mPositions.size() - pMesh.mColors[pInput.mIndex].size() - 1, aiColor4D(0, 0, 0, 1));
 
             aiColor4D result(0, 0, 0, 1);
             for (size_t i = 0; i < pInput.mResolved->mSize; ++i) {
                 result[static_cast<unsigned int>(i)] = obj[pInput.mResolved->mSubOffset[i]];
             }
-            pMesh->mColors[pInput.mIndex].push_back(result);
+            pMesh.mColors[pInput.mIndex].push_back(result);
         } else {
             ASSIMP_LOG_ERROR("Collada: too many vertex color sets. Skipping.");
         }

code/Collada/ColladaParser.h

@@ -174,10 +174,10 @@ protected:
     void ReadGeometryLibrary();
 
     /** Reads a geometry from the geometry library. */
-    void ReadGeometry(Collada::Mesh *pMesh);
+    void ReadGeometry(Collada::Mesh &pMesh);
 
     /** Reads a mesh from the geometry library */
-    void ReadMesh(Collada::Mesh *pMesh);
+    void ReadMesh(Collada::Mesh &pMesh);
 
     /** Reads a source element - a combination of raw data and an accessor defining
          * things that should not be redefinable. Yes, that's another rant.
@@ -195,29 +195,29 @@ protected:
     void ReadAccessor(const std::string &pID);
 
     /** Reads input declarations of per-vertex mesh data into the given mesh */
-    void ReadVertexData(Collada::Mesh *pMesh);
+    void ReadVertexData(Collada::Mesh &pMesh);
 
     /** Reads input declarations of per-index mesh data into the given mesh */
-    void ReadIndexData(Collada::Mesh *pMesh);
+    void ReadIndexData(Collada::Mesh &pMesh);
 
     /** Reads a single input channel element and stores it in the given array, if valid */
     void ReadInputChannel(std::vector<Collada::InputChannel> &poChannels);
 
     /** Reads a <p> primitive index list and assembles the mesh data into the given mesh */
-    size_t ReadPrimitives(Collada::Mesh *pMesh, std::vector<Collada::InputChannel> &pPerIndexChannels,
+    size_t ReadPrimitives(Collada::Mesh &pMesh, std::vector<Collada::InputChannel> &pPerIndexChannels,
             size_t pNumPrimitives, const std::vector<size_t> &pVCount, Collada::PrimitiveType pPrimType);
 
     /** Copies the data for a single primitive into the mesh, based on the InputChannels */
     void CopyVertex(size_t currentVertex, size_t numOffsets, size_t numPoints, size_t perVertexOffset,
-            Collada::Mesh *pMesh, std::vector<Collada::InputChannel> &pPerIndexChannels,
+            Collada::Mesh &pMesh, std::vector<Collada::InputChannel> &pPerIndexChannels,
             size_t currentPrimitive, const std::vector<size_t> &indices);
 
     /** Reads one triangle of a tristrip into the mesh */
-    void ReadPrimTriStrips(size_t numOffsets, size_t perVertexOffset, Collada::Mesh *pMesh,
+    void ReadPrimTriStrips(size_t numOffsets, size_t perVertexOffset, Collada::Mesh &pMesh,
             std::vector<Collada::InputChannel> &pPerIndexChannels, size_t currentPrimitive, const std::vector<size_t> &indices);
 
     /** Extracts a single object from an input channel and stores it in the appropriate mesh data array */
-    void ExtractDataObjectFromChannel(const Collada::InputChannel &pInput, size_t pLocalIndex, Collada::Mesh *pMesh);
+    void ExtractDataObjectFromChannel(const Collada::InputChannel &pInput, size_t pLocalIndex, Collada::Mesh &pMesh);
 
     /** Reads the library of node hierarchies and scene parts */
     void ReadSceneLibrary();

include/assimp/ColladaMetaData.h

@@ -0,0 +1,53 @@
+/*
+Open Asset Import Library (assimp)
+----------------------------------------------------------------------
+
+Copyright (c) 2006-2020, 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.
+
+----------------------------------------------------------------------
+*/
+
+/** @file ColladaMetaData.h
+ * Declares common metadata constants used by Collada files
+ */
+#pragma once
+#ifndef AI_COLLADAMETADATA_H_INC
+#define AI_COLLADAMETADATA_H_INC
+
+#define AI_METADATA_COLLADA_ID "COLLADA_ID"
+#define AI_METADATA_COLLADA_SID "COLLADA_SID"
+
+#endif

include/assimp/config.h.in

@@ -1030,10 +1030,10 @@ enum aiComponent
 #define AI_CONFIG_IMPORT_COLLADA_IGNORE_UP_DIRECTION "IMPORT_COLLADA_IGNORE_UP_DIRECTION"
 
 // ---------------------------------------------------------------------------
-/** @brief Specifies whether the Collada loader should use Collada names as node names.
+/** @brief Specifies whether the Collada loader should use Collada names.
  *
- * If this property is set to true, the Collada names will be used as the
+ * If this property is set to true, the Collada names will be used as the node and
- * node name. The default is to use the id tag (resp. sid tag, if no id tag is present)
+ * mesh names. The default is to use the id tag (resp. sid tag, if no id tag is present)
  * instead.
  * Property type: Bool. Default value: false.
  */

test/unit/utColladaImportExport.cpp

@@ -44,13 +44,14 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/commonMetaData.h>
 #include <assimp/postprocess.h>
 #include <assimp/scene.h>
+#include <assimp/Exporter.hpp>
 #include <assimp/Importer.hpp>
 
 using namespace Assimp;
 
 class utColladaImportExport : public AbstractImportExportBase {
 public:
-    virtual bool importerTest() {
+    virtual bool importerTest() final {
         Assimp::Importer importer;
         const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.dae", aiProcess_ValidateDataStructure);
         if (scene == nullptr)
@@ -80,15 +81,61 @@ public:
 
         return true;
     }
+
+    void ImportAndCheckIds(const char *file, size_t meshCount) {
+        // Import the Collada using the 'default' where mesh names are the ids
+        Assimp::Importer importer;
+        const aiScene *scene = importer.ReadFile(file, aiProcess_ValidateDataStructure);
+        ASSERT_TRUE(scene != nullptr) << "Fatal: could not re-import " << file;
+        EXPECT_EQ(meshCount, scene->mNumMeshes) << "in " << file;
+
+        // Check the mesh ids are unique
+        std::map<std::string, size_t> meshNameMap;
+        for (size_t idx = 0; idx < scene->mNumMeshes; ++idx) {
+            std::string meshName(scene->mMeshes[idx]->mName.C_Str());
+            const auto result = meshNameMap.insert(std::make_pair(meshName, idx));
+            EXPECT_TRUE(result.second) << "Duplicate name: " << meshName << " index " << result.first->second;
+        }
+    }
 };
 
-TEST_F(utColladaImportExport, importBlenFromFileTest) {
+TEST_F(utColladaImportExport, importDaeFromFileTest) {
     EXPECT_TRUE(importerTest());
 }
 
+TEST_F(utColladaImportExport, exporterUniqueIdsTest) {
+    Assimp::Importer importer;
+    Assimp::Exporter exporter;
+    const char *outFileEmpty = "exportMeshIdTest_empty_out.dae";
+    const char *outFileNamed = "exportMeshIdTest_named_out.dae";
+
+    // Load a sample file containing multiple meshes
+    const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/teapots.DAE", aiProcess_ValidateDataStructure);
+
+    ASSERT_TRUE(scene != nullptr) << "Fatal: could not import teapots.DAE!";
+    ASSERT_EQ(3u, scene->mNumMeshes) << "Fatal: teapots.DAE initial load failed";
+
+    // Clear the mesh names
+    for (size_t idx = 0; idx < scene->mNumMeshes; ++idx) {
+        scene->mMeshes[idx]->mName.Clear();
+    }
+    ASSERT_EQ(AI_SUCCESS, exporter.Export(scene, "collada", outFileEmpty)) << "Fatal: Could not export un-named meshes file";
+
+    ImportAndCheckIds(outFileEmpty, 3);
+
+    // Force the meshes to have the same non-empty name
+    aiString testName("test_mesh");
+    for (size_t idx = 0; idx < scene->mNumMeshes; ++idx) {
+        scene->mMeshes[idx]->mName = testName;
+    }
+    ASSERT_EQ(AI_SUCCESS, exporter.Export(scene, "collada", outFileNamed)) << "Fatal: Could not export named meshes file";
+
+    ImportAndCheckIds(outFileNamed, 3);
+}
+
 class utColladaZaeImportExport : public AbstractImportExportBase {
 public:
-    virtual bool importerTest() {
+    virtual bool importerTest() final {
         {
             Assimp::Importer importer;
             const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/Collada/duck.zae", aiProcess_ValidateDataStructure);