From 36e53b75fa24b9286a57c1e1eaaf25a9427da26b Mon Sep 17 00:00:00 2001
From: Denis Biryukov <denis.birukov@gmail.com>
Date: Fri, 27 Jan 2017 14:29:18 +0300
Subject: [PATCH] Issue#1084 - Fix collada export. Don't duplicate
 TEXCOORD/NORMALS/COLORS in <vertices> and <polylist>

---
 code/ColladaExporter.cpp | 44 ++++++++++++++++++++++++----------------
 code/ColladaParser.cpp   |  3 +++
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/code/ColladaExporter.cpp b/code/ColladaExporter.cpp
index 6c576a12d..7f9540aa1 100644
--- a/code/ColladaExporter.cpp
+++ b/code/ColladaExporter.cpp
@@ -652,7 +652,7 @@ void ColladaExporter::WriteMaterials()
       if( materialCountWithThisName == 0 ) {
         materials[a].name = name.C_Str();
       } else {
-        materials[a].name = std::string(name.C_Str()) + to_string(materialCountWithThisName);  
+        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 ) {
@@ -809,8 +809,8 @@ void ColladaExporter::WriteGeometry( size_t pIndex)
     const std::string idstr = GetMeshId( pIndex);
     const std::string idstrEscaped = XMLEscape(idstr);
 
-  if( mesh->mNumFaces == 0 || mesh->mNumVertices == 0 )
-    return;
+    if ( mesh->mNumFaces == 0 || mesh->mNumVertices == 0 )
+        return;
 
     // opening tag
     mOutput << startstr << "<geometry id=\"" << idstrEscaped << "\" name=\"" << idstrEscaped << "_name\" >" << endstr;
@@ -843,22 +843,10 @@ 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 << "<vertices id=\"" << idstrEscaped << "-vertices" << "\">" << endstr;
     PushTag();
     mOutput << startstr << "<input semantic=\"POSITION\" source=\"#" << idstrEscaped << "-positions\" />" << endstr;
-    if( mesh->HasNormals() )
-        mOutput << startstr << "<input semantic=\"NORMAL\" source=\"#" << idstrEscaped << "-normals\" />" << endstr;
-    for( size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a )
-    {
-        if( mesh->HasTextureCoords(static_cast<unsigned int>(a)) )
-            mOutput << startstr << "<input semantic=\"TEXCOORD\" source=\"#" << idstrEscaped << "-tex" << a << "\" " /*<< "set=\"" << a << "\"" */ << " />" << endstr;
-    }
-    for( size_t a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; ++a )
-    {
-        if( mesh->HasVertexColors(static_cast<unsigned int>(a) ) )
-            mOutput << startstr << "<input semantic=\"COLOR\" source=\"#" << idstrEscaped << "-color" << a << "\" " /*<< set=\"" << a << "\"" */ << " />" << endstr;
-    }
-
     PopTag();
     mOutput << startstr << "</vertices>" << endstr;
 
@@ -877,6 +865,19 @@ void ColladaExporter::WriteGeometry( size_t pIndex)
         mOutput << startstr << "<lines count=\"" << countLines << "\" material=\"defaultMaterial\">" << endstr;
         PushTag();
         mOutput << startstr << "<input offset=\"0\" semantic=\"VERTEX\" source=\"#" << idstrEscaped << "-vertices\" />" << endstr;
+        if( mesh->HasNormals() )
+            mOutput << startstr << "<input semantic=\"NORMAL\" source=\"#" << idstrEscaped << "-normals\" />" << endstr;
+        for( size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a )
+        {
+            if( mesh->HasTextureCoords(static_cast<unsigned int>(a)) )
+                mOutput << startstr << "<input semantic=\"TEXCOORD\" source=\"#" << idstrEscaped << "-tex" << a << "\" " << "set=\"" << a << "\""  << " />" << endstr;
+        }
+        for( size_t a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; ++a )
+        {
+            if( mesh->HasVertexColors(static_cast<unsigned int>(a) ) )
+                mOutput << startstr << "<input semantic=\"COLOR\" source=\"#" << idstrEscaped << "-color" << a << "\" " << "set=\"" << a << "\""  << " />" << endstr;
+        }
+
         mOutput << startstr << "<p>";
         for( size_t a = 0; a < mesh->mNumFaces; ++a )
         {
@@ -898,10 +899,17 @@ void ColladaExporter::WriteGeometry( size_t pIndex)
         mOutput << startstr << "<polylist count=\"" << countPoly << "\" material=\"defaultMaterial\">" << endstr;
         PushTag();
         mOutput << startstr << "<input offset=\"0\" semantic=\"VERTEX\" source=\"#" << idstrEscaped << "-vertices\" />" << endstr;
+        if( mesh->HasNormals() )
+            mOutput << startstr << "<input offset=\"0\" semantic=\"NORMAL\" source=\"#" << idstrEscaped << "-normals\" />" << endstr;
         for( size_t a = 0; a < AI_MAX_NUMBER_OF_TEXTURECOORDS; ++a )
         {
-            if( mesh->HasTextureCoords(static_cast<unsigned int>(a) ) )
-                mOutput << startstr << "<input offset=\"0\" semantic=\"TEXCOORD\" source=\"#" << idstrEscaped << "-tex" << a << "\" set=\"" << a << "\" />" << endstr;
+            if( mesh->HasTextureCoords(static_cast<unsigned int>(a)) )
+                mOutput << startstr << "<input offset=\"0\" semantic=\"TEXCOORD\" source=\"#" << idstrEscaped << "-tex" << a << "\" " << "set=\"" << a << "\""  << " />" << endstr;
+        }
+        for( size_t a = 0; a < AI_MAX_NUMBER_OF_COLOR_SETS; ++a )
+        {
+            if( mesh->HasVertexColors(static_cast<unsigned int>(a) ) )
+                mOutput << startstr << "<input offset=\"0\" semantic=\"COLOR\" source=\"#" << idstrEscaped << "-color" << a << "\" " << "set=\"" << a << "\""  << " />" << endstr;
         }
 
         mOutput << startstr << "<vcount>";
diff --git a/code/ColladaParser.cpp b/code/ColladaParser.cpp
index c61ca251c..bb553b486 100644
--- a/code/ColladaParser.cpp
+++ b/code/ColladaParser.cpp
@@ -2451,6 +2451,9 @@ size_t ColladaParser::ReadPrimitives( Mesh* pMesh, std::vector<InputChannel>& pP
     return numPrimitives;
 }
 
+///@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){
     // calculate the base offset of the vertex whose attributes we ant to copy
     size_t baseOffset = currentPrimitive * numOffsets * numPoints + currentVertex * numOffsets;