Port mesh loading to pugixml. Untested.

code/AssetLib/Irr/IRRLoader.cpp

@@ -66,8 +66,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <csignal>
 #include <iostream>
 #include <memory>
-#include <queue>
-#include <stack>
 
 using namespace Assimp;
 
@@ -1254,6 +1252,7 @@ void IRRImporter::InternReadFile(const std::string &pFile, aiScene *pScene, IOSy
     // Parse the XML
     // First node is the xml header. Awkwardly skip to sibling's children
     // I don't like recursion
+    // TODO clean up
     std::vector<pugi::xml_node> nextNodes;
     for (auto &node : rootElement.children().begin()->next_sibling().children()) {
         nextNodes.push_back(node); // Find second node, <irr_scene>, and push it's children to queue

code/AssetLib/Irr/IRRMeshLoader.cpp

@@ -133,6 +133,7 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
     meshes.reserve(5);
 
     // temporary data - current mesh buffer
+    // TODO move all these to inside loop
     aiMaterial *curMat = nullptr;
     aiMesh *curMesh = nullptr;
     unsigned int curMatFlags = 0;
@@ -142,23 +143,28 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
     std::vector<aiVector3D> curUVs, curUV2s;
 
     // some temporary variables
-    int textMeaning = 0;
+    // textMeaning is a 15 year old variable, that could've been an enum
-    int vertexFormat = 0; // 0 = normal; 1 = 2 tcoords, 2 = tangents
+    // int textMeaning = 0; // 0=none? 1=vertices 2=indices
+    // int vertexFormat = 0; // 0 = normal; 1 = 2 tcoords, 2 = tangents
     bool useColors = false;
 
+    /*
+    ** irrmesh files have a top level <mesh> owning multiple <buffer> nodes.
+    ** Each <buffer> contains <material>, <vertices>, and <indices>
+    ** <material> tags here directly owns the material data specs
+    ** <vertices> are a vertex per line, contains position, UV1 coords, maybe UV2, normal, tangent, bitangent
+    ** <boundingbox> is ignored, I think assimp recalculates those?
+    */
+
     // Parse the XML file
-    for (pugi::xml_node child : root.children()) {
+    // FIXME get <mesh> not root
-        if (child.type() == pugi::node_element) {
+    for (pugi::xml_node bufferNode : root.children()) {
-            if (!ASSIMP_stricmp(child.name(), "buffer") && (curMat || curMesh)) {
+        if (ASSIMP_stricmp(bufferNode.name(), "buffer")) {
-                // end of previous buffer. A material and a mesh should be there
+            // Might be a useless warning
-                if (!curMat || !curMesh) {
+            ASSIMP_LOG_WARN("IRRMESH: Ignoring non buffer node <", bufferNode.name(), "> in mesh declaration");
-                    ASSIMP_LOG_ERROR("IRRMESH: A buffer must contain a mesh and a material");
+            continue;
-                    releaseMaterial(&curMat);
-                    releaseMesh(&curMesh);
-                } else {
-                    materials.push_back(curMat);
-                    meshes.push_back(curMesh);
         }
+
         curMat = nullptr;
         curMesh = nullptr;
 
@@ -169,42 +175,47 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
         curUVs.clear();
         curTangents.clear();
         curBitangents.clear();
-            }
 
-            if (!ASSIMP_stricmp(child.name(), "material")) {
+        // TODO ensure all three nodes are present and populated
-                if (curMat) {
+        // before allocating everything
+
+        // Get first material node
+        pugi::xml_node materialNode = bufferNode.child("material");
+        if (materialNode) {
+            curMat = ParseMaterial(materialNode, curMatFlags);
+            // Warn if there's more materials
+            if (materialNode.next_sibling("material")) {
                 ASSIMP_LOG_WARN("IRRMESH: Only one material description per buffer, please");
-                    releaseMaterial(&curMat);
             }
-                // curMat = ParseMaterial(curMatFlags);
+        } else {
-            }
+            ASSIMP_LOG_ERROR("IRRMESH: Buffer must contain one material");
-            /* no else here! */ if (!ASSIMP_stricmp(child.name(), "vertices")) {
-                pugi::xml_attribute attr = child.attribute("vertexCount");
-                int num = attr.as_int();
-                // int num = reader->getAttributeValueAsInt("vertexCount");
-
-                if (!num) {
-                    // This is possible ... remove the mesh from the list and skip further reading
-                    ASSIMP_LOG_WARN("IRRMESH: Found mesh with zero vertices");
-
-                    releaseMaterial(&curMat);
-                    releaseMesh(&curMesh);
-                    textMeaning = 0;
             continue;
         }
 
-                curVertices.reserve(num);
+        // Get first vertices node
-                curNormals.reserve(num);
+        pugi::xml_node verticesNode = bufferNode.child("vertices");
-                curColors.reserve(num);
+        if (verticesNode) {
-                curUVs.reserve(num);
+            pugi::xml_attribute vertexCountAttrib = verticesNode.attribute("vertexCount");
+            int vertexCount = vertexCountAttrib.as_int();
+            if (vertexCount == 0) {
+                // This is possible ... remove the mesh from the list and skip further reading
+                ASSIMP_LOG_WARN("IRRMESH: Found mesh with zero vertices");
+                releaseMaterial(&curMat);
+                // releaseMesh(&curMesh);
+                continue; // Bail out early
+            };
 
+            curVertices.reserve(vertexCount);
+            curNormals.reserve(vertexCount);
+            curColors.reserve(vertexCount);
+            curUVs.reserve(vertexCount);
+
+            VertexFormat vertexFormat;
             // Determine the file format
-                // const char *t = reader->getAttributeValueSafe("type");
+            pugi::xml_attribute typeAttrib = verticesNode.attribute("type");
-                pugi::xml_attribute t = child.attribute("type");
+            if (!ASSIMP_stricmp("2tcoords", typeAttrib.value())) {
-                if (!ASSIMP_stricmp("2tcoords", t.name())) {
+                curUV2s.reserve(vertexCount);
-                    curUV2s.reserve(num);
+                vertexFormat = VertexFormat::t2coord;
-                    vertexFormat = 1;
-
                 if (curMatFlags & AI_IRRMESH_EXTRA_2ND_TEXTURE) {
                     // *********************************************************
                     // We have a second texture! So use this UV channel
@@ -223,29 +234,38 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
                         mat->AddProperty(&idx, 1, AI_MATKEY_UVWSRC_DIFFUSE(1));
                     }
                 }
-                } else if (!ASSIMP_stricmp("tangents", t.name())) {
+            } else if (!ASSIMP_stricmp("tangents", typeAttrib.value())) {
-                    curTangents.reserve(num);
+                curTangents.reserve(vertexCount);
-                    curBitangents.reserve(num);
+                curBitangents.reserve(vertexCount);
-                    vertexFormat = 2;
+                vertexFormat = VertexFormat::tangent;
-                } else if (ASSIMP_stricmp("standard", t.name())) {
+            } else if (!ASSIMP_stricmp("standard", typeAttrib.value())) {
+                vertexFormat = VertexFormat::standard;
+            } else {
+                // Unsupported format, discard whole buffer/mesh
+                // Assuming we have a correct material, then release it
+                // We don't have a correct mesh for sure here
                 releaseMaterial(&curMat);
-                    ASSIMP_LOG_WARN("IRRMESH: Unknown vertex format");
+                ASSIMP_LOG_ERROR("IRRMESH: Unknown vertex format");
-                } else
+                continue; // Skip rest of buffer
-                    vertexFormat = 0;
+            };
-                textMeaning = 1;
+
-            } else if (!ASSIMP_stricmp(child.name(), "indices")) {
+            // We know what format buffer is, collect numbers
-                if (curVertices.empty() && curMat) {
+            ParseBufferVertices(verticesNode.text().get(), vertexFormat,
-                    releaseMaterial(&curMat);
+                    curVertices, curNormals,
-                    throw DeadlyImportError("IRRMESH: indices must come after vertices");
+                    curTangents, curBitangents,
+                    curUVs, curUV2s, curColors, useColors);
         }
 
-                textMeaning = 2;
+        // Get indices
-
+        // At this point we have some vertices and a valid material
+        // Collect indices and create aiMesh at the same time
+        pugi::xml_node indicesNode = bufferNode.child("indices");
+        if (indicesNode) {
             // start a new mesh
             curMesh = new aiMesh();
 
             // allocate storage for all faces
-                pugi::xml_attribute attr = child.attribute("indexCount");
+            pugi::xml_attribute attr = indicesNode.attribute("indexCount");
             curMesh->mNumVertices = attr.as_int();
             if (!curMesh->mNumVertices) {
                 // This is possible ... remove the mesh from the list and skip further reading
@@ -256,9 +276,7 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
 
                 // material - away
                 releaseMaterial(&curMat);
-
+                continue; // Go to next buffer
-                    textMeaning = 0;
-                    continue;
             }
 
             if (curMesh->mNumVertices % 3) {
@@ -293,107 +311,6 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
             if (curUV2s.size() == curVertices.size()) {
                 curMesh->mTextureCoords[1] = new aiVector3D[curMesh->mNumVertices];
             }
-            }
-            // break;
-
-            // case EXN_TEXT: {
-            const char *sz = child.child_value();
-            if (textMeaning == 1) {
-                textMeaning = 0;
-
-                // read vertices
-                do {
-                    SkipSpacesAndLineEnd(&sz);
-                    aiVector3D temp;
-                    aiColor4D c;
-
-                    // Read the vertex position
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.x);
-                    SkipSpaces(&sz);
-
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.y);
-                    SkipSpaces(&sz);
-
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.z);
-                    SkipSpaces(&sz);
-                    curVertices.push_back(temp);
-
-                    // Read the vertex normals
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.x);
-                    SkipSpaces(&sz);
-
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.y);
-                    SkipSpaces(&sz);
-
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.z);
-                    SkipSpaces(&sz);
-                    curNormals.push_back(temp);
-
-                    // read the vertex colors
-                    uint32_t clr = strtoul16(sz, &sz);
-                    ColorFromARGBPacked(clr, c);
-
-                    if (!curColors.empty() && c != *(curColors.end() - 1))
-                        useColors = true;
-
-                    curColors.push_back(c);
-                    SkipSpaces(&sz);
-
-                    // read the first UV coordinate set
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.x);
-                    SkipSpaces(&sz);
-
-                    sz = fast_atoreal_move<float>(sz, (float &)temp.y);
-                    SkipSpaces(&sz);
-                    temp.z = 0.f;
-                    temp.y = 1.f - temp.y; // DX to OGL
-                    curUVs.push_back(temp);
-
-                    // read the (optional) second UV coordinate set
-                    if (vertexFormat == 1) {
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.x);
-                        SkipSpaces(&sz);
-
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.y);
-                        temp.y = 1.f - temp.y; // DX to OGL
-                        curUV2s.push_back(temp);
-                    }
-                    // read optional tangent and bitangent vectors
-                    else if (vertexFormat == 2) {
-                        // tangents
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.x);
-                        SkipSpaces(&sz);
-
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.z);
-                        SkipSpaces(&sz);
-
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.y);
-                        SkipSpaces(&sz);
-                        temp.y *= -1.0f;
-                        curTangents.push_back(temp);
-
-                        // bitangents
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.x);
-                        SkipSpaces(&sz);
-
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.z);
-                        SkipSpaces(&sz);
-
-                        sz = fast_atoreal_move<float>(sz, (float &)temp.y);
-                        SkipSpaces(&sz);
-                        temp.y *= -1.0f;
-                        curBitangents.push_back(temp);
-                    }
-                }
-
-                /* IMPORTANT: We assume that each vertex is specified in one
-                line. So we can skip the rest of the line - unknown vertex
-                elements are ignored.
-                */
-
-                while (SkipLine(&sz));
-            } else if (textMeaning == 2) {
-                textMeaning = 0;
 
             // read indices
             aiFace *curFace = curMesh->mFaces;
@@ -409,24 +326,33 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
 
             unsigned int curIdx = 0;
             unsigned int total = 0;
+
+            // NOTE this might explode for UTF-16 and wchars
+            const char *sz = indicesNode.text().get();
+            // For each index loop over aiMesh faces
             while (SkipSpacesAndLineEnd(&sz)) {
                 if (curFace >= faceEnd) {
                     ASSIMP_LOG_ERROR("IRRMESH: Too many indices");
                     break;
                 }
+                // if new face
                 if (!curIdx) {
                     curFace->mNumIndices = 3;
                     curFace->mIndices = new unsigned int[3];
                 }
 
+                // Read index base 10
+                // function advances the pointer
                 unsigned int idx = strtoul10(sz, &sz);
                 if (idx >= curVertices.size()) {
                     ASSIMP_LOG_ERROR("IRRMESH: Index out of range");
                     idx = 0;
                 }
 
+                // make up our own indices?
                 curFace->mIndices[curIdx] = total++;
 
+                // Copy over data to aiMesh
                 *pcV++ = curVertices[idx];
                 if (pcN) *pcN++ = curNormals[idx];
                 if (pcT) *pcT++ = curTangents[idx];
@@ -435,14 +361,16 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
                 if (pcT0) *pcT0++ = curUVs[idx];
                 if (pcT1) *pcT1++ = curUV2s[idx];
 
+                // start new face
                 if (++curIdx == 3) {
                     ++curFace;
                     curIdx = 0;
                 }
             }
-
+            // We should be at the end of mFaces
             if (curFace != faceEnd)
                 ASSIMP_LOG_ERROR("IRRMESH: Not enough indices");
+        }
 
         // Finish processing the mesh - do some small material workarounds
         if (curMatFlags & AI_IRRMESH_MAT_trans_vertex_alpha && !useColors) {
@@ -451,12 +379,9 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
             aiMaterial *mat = (aiMaterial *)curMat;
             mat->AddProperty(&curColors[0].a, 1, AI_MATKEY_OPACITY);
         }
-            }
+        // textMeaning = 2;
-        }
-    }
 
-    // End of the last buffer. A material and a mesh should be there
+        // end of previous buffer. A material and a mesh should be there
-    if (curMat || curMesh) {
         if (!curMat || !curMesh) {
             ASSIMP_LOG_ERROR("IRRMESH: A buffer must contain a mesh and a material");
             releaseMaterial(&curMat);
@@ -467,7 +392,8 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
         }
     }
 
-    if (materials.empty()) {
+    // If one is empty then so is the other
+    if (materials.empty() || meshes.empty()) {
         throw DeadlyImportError("IRRMESH: Unable to read a mesh from this file");
     }
 
@@ -492,11 +418,105 @@ void IRRMeshImporter::InternReadFile(const std::string &pFile,
 
     for (unsigned int i = 0; i < pScene->mNumMeshes; ++i) {
         pScene->mRootNode->mMeshes[i] = i;
-    }
+    };
 }
 
-void IRRMeshImporter::ParseMaterialBuffer(pugi::xml_node& bufferNode) {
+void IRRMeshImporter::ParseBufferVertices(const char *sz, VertexFormat vertexFormat,
+        std::vector<aiVector3D> &vertices, std::vector<aiVector3D> &normals,
+        std::vector<aiVector3D> &tangents, std::vector<aiVector3D> &bitangents,
+        std::vector<aiVector3D> &UVs, std::vector<aiVector3D> &UV2s,
+        std::vector<aiColor4D> &colors, bool &useColors) {
+    // read vertices
+    do {
+        SkipSpacesAndLineEnd(&sz);
+        aiVector3D temp;
+        aiColor4D c;
 
+        // Read the vertex position
+        sz = fast_atoreal_move<float>(sz, (float &)temp.x);
+        SkipSpaces(&sz);
+
+        sz = fast_atoreal_move<float>(sz, (float &)temp.y);
+        SkipSpaces(&sz);
+
+        sz = fast_atoreal_move<float>(sz, (float &)temp.z);
+        SkipSpaces(&sz);
+        vertices.push_back(temp);
+
+        // Read the vertex normals
+        sz = fast_atoreal_move<float>(sz, (float &)temp.x);
+        SkipSpaces(&sz);
+
+        sz = fast_atoreal_move<float>(sz, (float &)temp.y);
+        SkipSpaces(&sz);
+
+        sz = fast_atoreal_move<float>(sz, (float &)temp.z);
+        SkipSpaces(&sz);
+        normals.push_back(temp);
+
+        // read the vertex colors
+        uint32_t clr = strtoul16(sz, &sz);
+        ColorFromARGBPacked(clr, c);
+
+        // If we're pushing more than one distinct color
+        if (!colors.empty() && c != *(colors.end() - 1))
+            useColors = true;
+
+        colors.push_back(c);
+        SkipSpaces(&sz);
+
+        // read the first UV coordinate set
+        sz = fast_atoreal_move<float>(sz, (float &)temp.x);
+        SkipSpaces(&sz);
+
+        sz = fast_atoreal_move<float>(sz, (float &)temp.y);
+        SkipSpaces(&sz);
+        temp.z = 0.f;
+        temp.y = 1.f - temp.y; // DX to OGL
+        UVs.push_back(temp);
+
+        // NOTE these correspond to specific S3DVertex* structs in irr sourcecode
+        // So by definition, all buffers have either UV2 or tangents or neither
+        // read the (optional) second UV coordinate set
+        if (vertexFormat == VertexFormat::t2coord) {
+            sz = fast_atoreal_move<float>(sz, (float &)temp.x);
+            SkipSpaces(&sz);
+
+            sz = fast_atoreal_move<float>(sz, (float &)temp.y);
+            temp.y = 1.f - temp.y; // DX to OGL
+            UV2s.push_back(temp);
+        }
+        // read optional tangent and bitangent vectors
+        else if (vertexFormat == VertexFormat::tangent) {
+            // tangents
+            sz = fast_atoreal_move<float>(sz, (float &)temp.x);
+            SkipSpaces(&sz);
+
+            sz = fast_atoreal_move<float>(sz, (float &)temp.z);
+            SkipSpaces(&sz);
+
+            sz = fast_atoreal_move<float>(sz, (float &)temp.y);
+            SkipSpaces(&sz);
+            temp.y *= -1.0f;
+            tangents.push_back(temp);
+
+            // bitangents
+            sz = fast_atoreal_move<float>(sz, (float &)temp.x);
+            SkipSpaces(&sz);
+
+            sz = fast_atoreal_move<float>(sz, (float &)temp.z);
+            SkipSpaces(&sz);
+
+            sz = fast_atoreal_move<float>(sz, (float &)temp.y);
+            SkipSpaces(&sz);
+            temp.y *= -1.0f;
+            bitangents.push_back(temp);
+        }
+    } while (SkipLine(&sz));
+    /* IMPORTANT: We assume that each vertex is specified in one
+    line. So we can skip the rest of the line - unknown vertex
+    elements are ignored.
+    */
 }
 
 #endif // !! ASSIMP_BUILD_NO_IRRMESH_IMPORTER

code/AssetLib/Irr/IRRMeshLoader.h

@@ -85,8 +85,19 @@ protected:
      */
     void InternReadFile(const std::string &pFile, aiScene *pScene,
             IOSystem *pIOHandler) override;
- private:
+
-    void ParseMaterialBuffer(pugi::xml_node& bufferNode);
+private:
+    enum class VertexFormat {
+        standard = 0, // "standard" - also noted as 'normal' format elsewhere
+        t2coord = 1, // "2tcoord" - standard + 2 UV maps
+        tangent = 2, // "tangents" - standard + tangents and bitangents
+    };
+
+    void ParseBufferVertices(const char *sz, VertexFormat vertexFormat,
+            std::vector<aiVector3D> &vertices, std::vector<aiVector3D> &normals,
+            std::vector<aiVector3D> &tangents, std::vector<aiVector3D> &bitangents,
+            std::vector<aiVector3D> &UVs, std::vector<aiVector3D> &UV2s,
+            std::vector<aiColor4D> &colors, bool &useColors);
 };
 
 } // end of namespace Assimp