From c08a78f7c66bb1cddecbcd9b68cdfbdf9584c301 Mon Sep 17 00:00:00 2001 From: "Chris Weermann (TGE)" Date: Wed, 2 Oct 2019 01:32:53 +0200 Subject: [PATCH 1/3] ColladaExporter: improve name/id handling --- code/Collada/ColladaExporter.cpp | 150 ++++++++++++++++++------------- 1 file changed, 88 insertions(+), 62 deletions(-) diff --git a/code/Collada/ColladaExporter.cpp b/code/Collada/ColladaExporter.cpp index 6fd31ed3b..5aee96c23 100644 --- a/code/Collada/ColladaExporter.cpp +++ b/code/Collada/ColladaExporter.cpp @@ -92,6 +92,34 @@ void ExportSceneCollada(const char* pFile, IOSystem* pIOSystem, const aiScene* p } // end of namespace Assimp +// ------------------------------------------------------------------------------------------------ +const std::string XMLIDEncode(const std::string& name) +{ + const char XML_ID_CHARS[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-."; + const unsigned int XML_ID_CHARS_COUNT = sizeof(XML_ID_CHARS) / sizeof(char); + + if (name.length() == 0) + return name; + + std::stringstream idEncoded; + + // xsd:ID must start with letter or underscore + if (!((name[0] >= 'A' && name[0] <= 'z') || name[0] == '_')) + idEncoded << '_'; + + for (std::string::const_iterator it = name.begin(); it != name.end(); ++it) + { + // xsd:ID can only contain letters, digits, underscores, hyphens and periods + if (strchr(XML_ID_CHARS, *it) != nullptr) + idEncoded << *it; + else + // Select placeholder character based on invalid character to prevent name collisions + idEncoded << XML_ID_CHARS[(*it) % XML_ID_CHARS_COUNT]; + } + + return idEncoded.str(); +} + // ------------------------------------------------------------------------------------------------ // Constructor for a specific scene to export ColladaExporter::ColladaExporter( const aiScene* pScene, IOSystem* pIOSystem, const std::string& path, const std::string& file) @@ -146,7 +174,7 @@ void ColladaExporter::WriteFile() { // useless Collada fu at the end, just in case we haven't had enough indirections, yet. mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "mRootNode->mName.C_Str()) + "\" />" << endstr; + mOutput << startstr << "mRootNode->mName.C_Str()) + "\" />" << endstr; PopTag(); mOutput << startstr << "" << endstr; PopTag(); @@ -356,9 +384,10 @@ void ColladaExporter::WriteCamerasLibrary() { void ColladaExporter::WriteCamera(size_t pIndex){ const aiCamera *cam = mScene->mCameras[pIndex]; - const std::string idstrEscaped = XMLEscape(cam->mName.C_Str()); + const std::string cameraName = XMLEscape(cam->mName.C_Str()); + const std::string cameraId = XMLIDEncode(cam->mName.C_Str()); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; PushTag(); @@ -412,10 +441,11 @@ void ColladaExporter::WriteLightsLibrary() { void ColladaExporter::WriteLight(size_t pIndex){ const aiLight *light = mScene->mLights[pIndex]; - const std::string idstrEscaped = XMLEscape(light->mName.C_Str()); + const std::string lightName = XMLEscape(light->mName.C_Str()); + const std::string lightId = XMLIDEncode(light->mName.C_Str()); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; PushTag(); @@ -586,7 +616,7 @@ static bool isalnum_C(char c) { void ColladaExporter::WriteImageEntry( const Surface& pSurface, const std::string& pNameAdd) { if( !pSurface.texture.empty() ) { - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << ""; @@ -619,7 +649,7 @@ void ColladaExporter::WriteTextureColorEntry( const Surface& pSurface, const std } else { - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; } PopTag(); mOutput << startstr << "" << endstr; @@ -633,21 +663,21 @@ void ColladaExporter::WriteTextureParamEntry( const Surface& pSurface, const std // if surface is a texture, write out the sampler and the surface parameters necessary to reference the texture if( !pSurface.texture.empty() ) { - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << XMLEscape(pMatName) << "-" << pTypeName << "-image" << endstr; + mOutput << startstr << "" << XMLIDEncode(pMatName) << "-" << pTypeName << "-image" << endstr; PopTag(); mOutput << startstr << "" << endstr; PopTag(); mOutput << startstr << "" << endstr; - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << XMLEscape(pMatName) << "-" << pTypeName << "-surface" << endstr; + mOutput << startstr << "" << XMLIDEncode(pMatName) << "-" << pTypeName << "-surface" << endstr; PopTag(); mOutput << startstr << "" << endstr; PopTag(); @@ -699,11 +729,6 @@ void ColladaExporter::WriteMaterials() materials[a].name = std::string(name.C_Str()) + to_string(materialCountWithThisName); } } - for( std::string::iterator it = materials[a].name.begin(); it != materials[a].name.end(); ++it ) { - if( !isalnum_C( *it ) ) { - *it = '_'; - } - } aiShadingMode shading = aiShadingMode_Flat; materials[a].shading_model = "phong"; @@ -768,7 +793,7 @@ void ColladaExporter::WriteMaterials() { const Material& mat = *it; // this is so ridiculous it must be right - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; PushTag(); @@ -819,9 +844,9 @@ void ColladaExporter::WriteMaterials() for( std::vector::const_iterator it = materials.begin(); it != materials.end(); ++it ) { const Material& mat = *it; - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PopTag(); mOutput << startstr << "" << endstr; } @@ -851,7 +876,7 @@ void ColladaExporter::WriteController( size_t pIndex) { const aiMesh* mesh = mScene->mMeshes[pIndex]; const std::string idstr = GetMeshId( pIndex); - const std::string idstrEscaped = XMLEscape(idstr); + const std::string idstrEscaped = XMLIDEncode(idstr); if ( mesh->mNumFaces == 0 || mesh->mNumVertices == 0 ) return; @@ -886,7 +911,7 @@ void ColladaExporter::WriteController( size_t pIndex) mOutput << startstr << "mNumBones << "\">"; for( size_t i = 0; i < mesh->mNumBones; ++i ) - mOutput << XMLEscape(mesh->mBones[i]->mName.C_Str()) << " "; + mOutput << XMLIDEncode(mesh->mBones[i]->mName.C_Str()) << " "; mOutput << "" << endstr; @@ -1022,13 +1047,14 @@ void ColladaExporter::WriteGeometry( size_t pIndex) { const aiMesh* mesh = mScene->mMeshes[pIndex]; const std::string idstr = GetMeshId( pIndex); - const std::string idstrEscaped = XMLEscape(idstr); + const std::string geometryName = XMLEscape(idstr); + const std::string geometryId = XMLIDEncode(idstr); if ( mesh->mNumFaces == 0 || mesh->mNumVertices == 0 ) return; // opening tag - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; @@ -1059,9 +1085,9 @@ void ColladaExporter::WriteGeometry( size_t pIndex) // assemble vertex structure // Only write input for POSITION since we will write other as shared inputs in polygon definition - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PopTag(); mOutput << startstr << "" << endstr; @@ -1079,18 +1105,18 @@ void ColladaExporter::WriteGeometry( size_t pIndex) { mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; if( mesh->HasNormals() ) - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; for( size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a ) { if( mesh->HasTextureCoords(static_cast(a)) ) - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; } for( size_t a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; ++a ) { if( mesh->HasVertexColors(static_cast(a) ) ) - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; } mOutput << startstr << "

"; @@ -1113,18 +1139,18 @@ void ColladaExporter::WriteGeometry( size_t pIndex) { mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; if( mesh->HasNormals() ) - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; for( size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a ) { if( mesh->HasTextureCoords(static_cast(a)) ) - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; } for( size_t a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; ++a ) { if( mesh->HasVertexColors(static_cast(a) ) ) - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; } mOutput << startstr << ""; @@ -1173,13 +1199,13 @@ void ColladaExporter::WriteFloatArray( const std::string& pIdString, FloatDataTy return; } - std::string arrayId = pIdString + "-array"; + std::string arrayId = XMLIDEncode(pIdString) + "-array"; - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); // source array - mOutput << startstr << " "; + mOutput << startstr << " "; PushTag(); if( pType == FloatType_TexCoord2 ) @@ -1265,11 +1291,12 @@ void ColladaExporter::WriteFloatArray( const std::string& pIdString, FloatDataTy // Writes the scene library void ColladaExporter::WriteSceneLibrary() { - const std::string scene_name_escaped = XMLEscape(mScene->mRootNode->mName.C_Str()); + const std::string sceneName = XMLEscape(mScene->mRootNode->mName.C_Str()); + const std::string sceneId = XMLIDEncode(mScene->mRootNode->mName.C_Str()); mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); // start recursive write at the root node @@ -1300,7 +1327,7 @@ void ColladaExporter::WriteAnimationLibrary(size_t pIndex) idstr = idstr + ending; } - const std::string idstrEscaped = XMLEscape(idstr); + const std::string idstrEscaped = XMLIDEncode(idstr); mOutput << startstr << "" << endstr; PushTag(); @@ -1372,13 +1399,13 @@ void ColladaExporter::WriteAnimationLibrary(size_t pIndex) } const std::string node_idstr = nodeAnim->mNodeName.data + std::string("_matrix-interpolation"); - std::string arrayId = node_idstr + "-array"; + std::string arrayId = XMLIDEncode(node_idstr) + "-array"; - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); // source array - mOutput << startstr << " "; + mOutput << startstr << " "; for( size_t a = 0; a < names.size(); ++a ) { mOutput << names[a] << " "; } @@ -1387,7 +1414,7 @@ void ColladaExporter::WriteAnimationLibrary(size_t pIndex) mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); mOutput << startstr << "" << endstr; @@ -1409,12 +1436,12 @@ void ColladaExporter::WriteAnimationLibrary(size_t pIndex) { // samplers const std::string node_idstr = nodeAnim->mNodeName.data + std::string("_matrix-sampler"); - mOutput << startstr << "" << endstr; + mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "mNodeName.data + std::string("_matrix-input") ) << "\"/>" << endstr; - mOutput << startstr << "mNodeName.data + std::string("_matrix-output") ) << "\"/>" << endstr; - mOutput << startstr << "mNodeName.data + std::string("_matrix-interpolation") ) << "\"/>" << endstr; + mOutput << startstr << "mNodeName.data + std::string("_matrix-input") ) << "\"/>" << endstr; + mOutput << startstr << "mNodeName.data + std::string("_matrix-output") ) << "\"/>" << endstr; + mOutput << startstr << "mNodeName.data + std::string("_matrix-interpolation") ) << "\"/>" << endstr; PopTag(); mOutput << startstr << "" << endstr; @@ -1426,7 +1453,7 @@ void ColladaExporter::WriteAnimationLibrary(size_t pIndex) { // channels - mOutput << startstr << "mNodeName.data + std::string("_matrix-sampler") ) << "\" target=\"" << XMLEscape(nodeAnim->mNodeName.data) << "/matrix\"/>" << endstr; + mOutput << startstr << "mNodeName.data + std::string("_matrix-sampler") ) << "\" target=\"" << XMLIDEncode(nodeAnim->mNodeName.data) << "/matrix\"/>" << endstr; } } @@ -1437,8 +1464,6 @@ void ColladaExporter::WriteAnimationLibrary(size_t pIndex) // ------------------------------------------------------------------------------------------------ void ColladaExporter::WriteAnimationsLibrary() { - const std::string scene_name_escaped = XMLEscape(mScene->mRootNode->mName.C_Str()); - if ( mScene->mNumAnimations > 0 ) { mOutput << startstr << "" << endstr; PushTag(); @@ -1546,16 +1571,17 @@ void ColladaExporter::WriteNode( const aiScene* pScene, aiNode* pNode) } } - const std::string node_name_escaped = XMLEscape(pNode->mName.data); + const std::string node_id = XMLIDEncode(pNode->mName.data); + const std::string node_name = XMLEscape(pNode->mName.data); mOutput << startstr << "" << endstr; PushTag(); @@ -1594,14 +1620,14 @@ void ColladaExporter::WriteNode( const aiScene* pScene, aiNode* pNode) //check if it is a camera node for(size_t i=0; imNumCameras; i++){ if(mScene->mCameras[i]->mName == pNode->mName){ - mOutput << startstr <<"" << endstr; + mOutput << startstr <<"" << endstr; break; } } //check if it is a light node for(size_t i=0; imNumLights; i++){ if(mScene->mLights[i]->mName == pNode->mName){ - mOutput << startstr <<"" << endstr; + mOutput << startstr <<"" << endstr; break; } } @@ -1617,13 +1643,13 @@ void ColladaExporter::WriteNode( const aiScene* pScene, aiNode* pNode) if( mesh->mNumBones == 0 ) { - mOutput << startstr << "mMeshes[a])) << "\">" << endstr; + mOutput << startstr << "mMeshes[a])) << "\">" << endstr; PushTag(); } else { mOutput << startstr - << "mMeshes[a])) << "-skin\">" + << "mMeshes[a])) << "-skin\">" << endstr; PushTag(); @@ -1631,7 +1657,7 @@ void ColladaExporter::WriteNode( const aiScene* pScene, aiNode* pNode) // use the first bone to find skeleton root const aiNode * skeletonRootBoneNode = findSkeletonRootNode( pScene, mesh ); if ( skeletonRootBoneNode ) { - mFoundSkeletonRootNodeID = XMLEscape( skeletonRootBoneNode->mName.C_Str() ); + mFoundSkeletonRootNodeID = XMLIDEncode( skeletonRootBoneNode->mName.C_Str() ); } mOutput << startstr << "#" << mFoundSkeletonRootNodeID << "" << endstr; } @@ -1639,7 +1665,7 @@ void ColladaExporter::WriteNode( const aiScene* pScene, aiNode* pNode) PushTag(); mOutput << startstr << "" << endstr; PushTag(); - mOutput << startstr << "mMaterialIndex].name) << "\">" << endstr; + mOutput << startstr << "mMaterialIndex].name) << "\">" << endstr; PushTag(); for( size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a ) { From 38153748ab5ce6672ccc237738c25a6fbd107ca0 Mon Sep 17 00:00:00 2001 From: "Chris Weermann (TGE)" Date: Wed, 2 Oct 2019 19:18:48 +0200 Subject: [PATCH 2/3] ColladaExporter: use actual mesh names when available --- code/Collada/ColladaExporter.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/code/Collada/ColladaExporter.cpp b/code/Collada/ColladaExporter.cpp index 5aee96c23..c366fa72e 100644 --- a/code/Collada/ColladaExporter.cpp +++ b/code/Collada/ColladaExporter.cpp @@ -93,6 +93,7 @@ void ExportSceneCollada(const char* pFile, IOSystem* pIOSystem, const aiScene* p } // end of namespace Assimp // ------------------------------------------------------------------------------------------------ +// Encodes a string into a valid XML ID using the xsd:ID schema qualifications. const std::string XMLIDEncode(const std::string& name) { const char XML_ID_CHARS[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-."; @@ -875,7 +876,7 @@ void ColladaExporter::WriteControllerLibrary() void ColladaExporter::WriteController( size_t pIndex) { const aiMesh* mesh = mScene->mMeshes[pIndex]; - const std::string idstr = GetMeshId( pIndex); + const std::string idstr = mesh->mName.length == 0 ? GetMeshId(pIndex) : mesh->mName.C_Str(); const std::string idstrEscaped = XMLIDEncode(idstr); if ( mesh->mNumFaces == 0 || mesh->mNumVertices == 0 ) @@ -1046,7 +1047,7 @@ void ColladaExporter::WriteGeometryLibrary() void ColladaExporter::WriteGeometry( size_t pIndex) { const aiMesh* mesh = mScene->mMeshes[pIndex]; - const std::string idstr = GetMeshId( pIndex); + const std::string idstr = mesh->mName.length == 0 ? GetMeshId(pIndex) : mesh->mName.C_Str(); const std::string geometryName = XMLEscape(idstr); const std::string geometryId = XMLIDEncode(idstr); @@ -1641,15 +1642,17 @@ 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(); + if( mesh->mNumBones == 0 ) { - mOutput << startstr << "mMeshes[a])) << "\">" << endstr; + mOutput << startstr << "" << endstr; PushTag(); } else { mOutput << startstr - << "mMeshes[a])) << "-skin\">" + << "" << endstr; PushTag(); From c350d4f487308ba02ccf559bfab4521b9ff690b0 Mon Sep 17 00:00:00 2001 From: Kim Kulling Date: Sat, 19 Oct 2019 12:08:57 +0200 Subject: [PATCH 3/3] Update ColladaExporter.cpp Small review findings. --- code/Collada/ColladaExporter.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/code/Collada/ColladaExporter.cpp b/code/Collada/ColladaExporter.cpp index c366fa72e..a93dfa59a 100644 --- a/code/Collada/ColladaExporter.cpp +++ b/code/Collada/ColladaExporter.cpp @@ -94,28 +94,29 @@ void ExportSceneCollada(const char* pFile, IOSystem* pIOSystem, const aiScene* p // ------------------------------------------------------------------------------------------------ // Encodes a string into a valid XML ID using the xsd:ID schema qualifications. -const std::string XMLIDEncode(const std::string& name) -{ +static const std::string XMLIDEncode(const std::string& name) { const char XML_ID_CHARS[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-."; const unsigned int XML_ID_CHARS_COUNT = sizeof(XML_ID_CHARS) / sizeof(char); - if (name.length() == 0) + if (name.length() == 0) { return name; + } std::stringstream idEncoded; // xsd:ID must start with letter or underscore - if (!((name[0] >= 'A' && name[0] <= 'z') || name[0] == '_')) + if (!((name[0] >= 'A' && name[0] <= 'z') || name[0] == '_')) { idEncoded << '_'; + } - for (std::string::const_iterator it = name.begin(); it != name.end(); ++it) - { + for (std::string::const_iterator it = name.begin(); it != name.end(); ++it) { // xsd:ID can only contain letters, digits, underscores, hyphens and periods - if (strchr(XML_ID_CHARS, *it) != nullptr) + if (strchr(XML_ID_CHARS, *it) != nullptr) { idEncoded << *it; - else - // Select placeholder character based on invalid character to prevent name collisions + } else { + // Select placeholder character based on invalid character to prevent name collisions idEncoded << XML_ID_CHARS[(*it) % XML_ID_CHARS_COUNT]; + } } return idEncoded.str();