From c11e69105344c6f2c643981d3c728c435f56c0bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 11 Jan 2012 21:21:11 +0100 Subject: [PATCH] Cleaning overengineered MeshBuilder. * Removed VertexPointer typedef, as size_t is somewhere 64bit and OpenGL doesn't have corresponding data type. Using unsigned int everywhere instead, to avoid confusion. * Removed Face structure altogether, using vertex indices directly. * Greatly reduced size of unit tests, also thanks to C++11 std::initializer_list feature. --- src/MeshBuilder.h | 134 +++++++++++------------------ src/Primitives/Icosphere.h | 2 +- src/Test/MeshBuilderTest.cpp | 160 +++++++---------------------------- 3 files changed, 81 insertions(+), 215 deletions(-) diff --git a/src/MeshBuilder.h b/src/MeshBuilder.h index ec1d98c71..a8cd91a3c 100644 --- a/src/MeshBuilder.h +++ b/src/MeshBuilder.h @@ -42,31 +42,6 @@ vertices etc.). */ template class MeshBuilder { public: - typedef size_t VertexPointer; /**< @brief Type for indexing vertices */ - - /** @brief Triangle face */ - struct Face { - /** @brief Implicit constructor */ - Face() {} - - /** @brief Constructor */ - Face(VertexPointer first, VertexPointer second, VertexPointer third) { - vertices[0] = first; - vertices[1] = second; - vertices[2] = third; - } - - /** @brief Vertex data */ - VertexPointer vertices[3]; - - /** @brief Comparison operator */ - bool operator==(const Face& other) const { - return other.vertices[0] == vertices[0] && - other.vertices[1] == vertices[1] && - other.vertices[2] == vertices[2]; - } - }; - /** * @brief Destructor * @@ -83,22 +58,20 @@ template class MeshBuilder { */ void clear() { _vertices.clear(); - _faces.clear(); + _indices.clear(); } /** @brief Vertex count */ - inline size_t vertexCount() const { return _vertices.size(); } + inline unsigned int vertexCount() const { return _vertices.size(); } /** @brief Array with vertices */ inline const std::vector& vertices() const { return _vertices; } - inline std::vector& vertices() { return _vertices; } /**< @copydoc vertices() const */ - /** @brief Face count */ - inline size_t faceCount() const { return _faces.size(); } + /** @brief Index count */ + inline unsigned int indexCount() const { return _indices.size(); } - /** @brief Array with faces */ - inline const std::vector& faces() const { return _faces; } - inline std::vector& faces() { return _faces; } /**< @copydoc faces() const */ + /** @brief Array with indices */ + inline const std::vector& indices() const { return _indices; } /** * @brief Set mesh data @@ -110,36 +83,33 @@ template class MeshBuilder { * Replaces mesh builder data with given data. Type of indices is * detected from given pointer. */ - template void setData(const Vertex* vertexData, const IndexType* indices, GLsizei vertexCount, GLsizei indexCount) { + template void setData(const Vertex* vertexData, const IndexType* indices, unsigned int vertexCount, unsigned int indexCount) { clear(); - /* Map vertex indices to vertex pointers */ + /* Vertex array */ std::vector vertices; vertices.reserve(vertexCount); /* Using TypeTraits::IndexType to ensure we have allowed type for indexing */ for(typename TypeTraits::IndexType i = 0; i != vertexCount; ++i) addVertex(Vertex(vertexData[i])); - /* Faces array */ - _faces.reserve(indexCount/3); - for(size_t i = 0; i < static_cast(indexCount); i += 3) - addFace(indices[i], indices[i+1], indices[i+2]); + /* Index array */ + _indices.reserve(indexCount); + for(unsigned int i = 0; i != indexCount; ++i) + _indices.push_back(indices[i]); } /** @brief Add vertex */ - inline VertexPointer addVertex(const Vertex& v) { + inline unsigned int addVertex(const Vertex& v) { _vertices.push_back(v); return _vertices.size()-1; } /** @brief Add face */ - inline void addFace(const Face& face) { - _faces.push_back(face); - } - - /** @brief Add face */ - inline void addFace(VertexPointer first, VertexPointer second, VertexPointer third) { - _faces.push_back(Face(first, second, third)); + inline void addFace(unsigned int first, unsigned int second, unsigned int third) { + _indices.push_back(first); + _indices.push_back(second); + _indices.push_back(third); } /** @@ -152,15 +122,15 @@ template class MeshBuilder { * Cleaning the mesh is up to user. */ template void subdivide(Interpolator interpolator) { - size_t faceCount = _faces.size(); - _faces.reserve(_faces.size()*4); + size_t indexCount = _indices.size(); + _indices.reserve(_indices.size()*4); /* Subdivide each face to four new */ - for(size_t i = 0; i != faceCount; ++i) { + for(size_t i = 0; i != indexCount; i += 3) { /* Interpolate each side */ - VertexPointer newVertices[3]; + unsigned int newVertices[3]; for(int j = 0; j != 3; ++j) - newVertices[j] = addVertex(interpolator(_vertices[_faces[i].vertices[j]], _vertices[_faces[i].vertices[(j+1)%3]])); + newVertices[j] = addVertex(interpolator(_vertices[_indices[i+j]], _vertices[_indices[i+(j+1)%3]])); /* * Add three new faces (0, 1, 3) and update original (2) @@ -175,11 +145,11 @@ template class MeshBuilder { * / \ / \ * orig 1 ----- new 1 ---- orig 2 */ - addFace(_faces[i].vertices[0], newVertices[0], newVertices[2]); - addFace(newVertices[0], _faces[i].vertices[1], newVertices[1]); - addFace(newVertices[2], newVertices[1], _faces[i].vertices[2]); + addFace(_indices[i], newVertices[0], newVertices[2]); + addFace(newVertices[0], _indices[i+1], newVertices[1]); + addFace(newVertices[2], newVertices[1], _indices[i+2]); for(size_t j = 0; j != 3; ++j) - _faces[i].vertices[j] = newVertices[j]; + _indices[i+j] = newVertices[j]; } } @@ -194,7 +164,7 @@ template class MeshBuilder { * only first three fields of otherwise 4D vertex are important). */ template void cleanMesh(typename Vertex::Type epsilon = EPSILON) { - if(_faces.empty()) return; + if(_indices.empty()) return; /* Get mesh bounds */ Vertex min, max; @@ -229,27 +199,24 @@ template class MeshBuilder { table.reserve(_vertices.size()); /* Go through all faces' vertices */ - for(auto it = _faces.begin(); it != _faces.end(); ++it) { - for(size_t i = 0; i != 3; ++i) { - - /* Index of a vertex in vertexSize-dimensional table */ - size_t index[vertexSize]; - for(size_t ii = 0; ii != vertexSize; ++ii) - index[ii] = (_vertices[it->vertices[i]][ii]+moved[ii]-min[ii])/epsilon; - - /* Try inserting the vertex into table, if it already - exists, change vertex pointer of the face to already - existing vertex */ - HashedVertex v(it->vertices[i], table.size()); - auto result = table.insert(std::pair, HashedVertex>(index, v)); - it->vertices[i] = result.first->second.newVertexPointer; - } + for(auto it = _indices.begin(); it != _indices.end(); ++it) { + /* Index of a vertex in vertexSize-dimensional table */ + size_t index[vertexSize]; + for(size_t ii = 0; ii != vertexSize; ++ii) + index[ii] = (_vertices[*it][ii]+moved[ii]-min[ii])/epsilon; + + /* Try inserting the vertex into table, if it already + exists, change vertex pointer of the face to already + existing vertex */ + HashedVertex v(*it, table.size()); + auto result = table.insert(std::pair, HashedVertex>(index, v)); + *it = result.first->second.newIndex; } /* Shrink vertices array */ std::vector vertices(table.size()); for(auto it = table.cbegin(); it != table.cend(); ++it) - vertices[it->second.newVertexPointer] = _vertices[it->second.oldVertexPointer]; + vertices[it->second.newIndex] = _vertices[it->second.oldIndex]; std::swap(vertices, _vertices); /* Move vertex coordinates by epsilon/2 in next direction */ @@ -278,7 +245,7 @@ template class MeshBuilder { mesh->setPrimitive(Mesh::Triangles); mesh->setVertexCount(_vertices.size()); vertexBuffer->setData(sizeof(Vertex)*_vertices.size(), _vertices.data(), vertexBufferUsage); - SizeBasedCall(_vertices.size())(mesh, _faces, indexBufferUsage); + SizeBasedCall(_vertices.size())(mesh, _indices, indexBufferUsage); } /** @@ -297,11 +264,9 @@ template class MeshBuilder { } private: - std::vector _faces; + std::vector _indices; std::vector _vertices; - typedef size_t FacePointer; - template class IndexHash { public: inline size_t operator()(const Math::Vector& data) const { @@ -313,23 +278,20 @@ template class MeshBuilder { }; struct HashedVertex { - VertexPointer oldVertexPointer, newVertexPointer; + unsigned int oldIndex, newIndex; - HashedVertex(VertexPointer oldVertexPointer, VertexPointer newVertexPointer): oldVertexPointer(oldVertexPointer), newVertexPointer(newVertexPointer) {} + HashedVertex(unsigned int oldIndex, unsigned int newIndex): oldIndex(oldIndex), newIndex(newIndex) {} }; struct IndexBuilder { - template static void run(IndexedMesh* mesh, const std::vector& faces, Buffer::Usage indexBufferUsage) { + template static void run(IndexedMesh* mesh, const std::vector& _indices, Buffer::Usage indexBufferUsage) { /* Compress face array to index array. Using TypeTraits::IndexType to ensure we have allowed type for indexing */ std::vector::IndexType> indices; - indices.reserve(faces.size()*3); - for(auto it = faces.cbegin(); it != faces.cend(); ++it) { - indices.push_back(it->vertices[0]); - indices.push_back(it->vertices[1]); - indices.push_back(it->vertices[2]); - } + indices.reserve(_indices.size()); + for(auto it = _indices.cbegin(); it != _indices.cend(); ++it) + indices.push_back(*it); /* Update mesh parameters and fill index buffer */ mesh->setIndexCount(indices.size()); diff --git a/src/Primitives/Icosphere.h b/src/Primitives/Icosphere.h index caffa23c8..0bef205b6 100644 --- a/src/Primitives/Icosphere.h +++ b/src/Primitives/Icosphere.h @@ -49,7 +49,7 @@ template class Icosphere: public AbstractPrimitivevertexCount(); } - inline size_t indexCount() const { return builder()->faceCount()*3; } + inline size_t indexCount() const { return builder()->indexCount(); } inline void build(IndexedMesh* mesh, Buffer* vertexBuffer) { /* mesh is prepared by the builder, no need to call prepareMesh */ diff --git a/src/Test/MeshBuilderTest.cpp b/src/Test/MeshBuilderTest.cpp index 28866fe81..a42400d69 100644 --- a/src/Test/MeshBuilderTest.cpp +++ b/src/Test/MeshBuilderTest.cpp @@ -16,7 +16,6 @@ #include "MeshBuilderTest.h" #include -#include #include "MeshBuilder.h" @@ -29,152 +28,57 @@ namespace Magnum { namespace Test { void MeshBuilderTest::setData() { MeshBuilder builder; - int vertexData[] = { 1, 2, 3, 4 }; - GLubyte indices[] = { 0, 1, 2, 1, 2, 3 }; - builder.setData(reinterpret_cast(vertexData), indices, 4, 6); + Vector1 vertexData[] = { 1, 2, 3, 4 }; + GLubyte indexData[] = { 0, 1, 2, 1, 2, 3 }; + builder.setData(vertexData, indexData, 4, 6); - vector::Face> faces = builder.faces(); - QVERIFY(faces.size() == 2); - - vector::Face>::const_iterator it = faces.begin(); - QVERIFY(builder.vertices()[it->vertices[0]] == 1); - QVERIFY(builder.vertices()[it->vertices[1]] == 2); - QVERIFY(builder.vertices()[it->vertices[2]] == 3); - - it++; - QVERIFY(builder.vertices()[it->vertices[0]] == 2); - QVERIFY(builder.vertices()[it->vertices[1]] == 3); - QVERIFY(builder.vertices()[it->vertices[2]] == 4); - - vector vertices = builder.vertices(); - QVERIFY(vertices.size() == 4); - - int i = 0; - for(auto it = vertices.begin(); it != vertices.end(); ++it) - QVERIFY(*it == ++i); + QVERIFY((builder.vertices() == vector{1, 2, 3, 4})); + QVERIFY((builder.indices() == vector{0, 1, 2, 1, 2, 3})); } void MeshBuilderTest::addFace() { - Vector1 v1(1); - Vector1 v2(2); - Vector1 v3(3); - Vector1 v4(4); - MeshBuilder::Face f1(0, 1, 2); - MeshBuilder::Face f2(1, 2, 3); - MeshBuilder builder; - builder.addVertex(v1); - builder.addVertex(v2); - builder.addVertex(v3); - builder.addVertex(v4); - builder.addFace(f1); - builder.addFace(f2); - - vector vertices; - vertices.push_back(v1); - vertices.push_back(v2); - vertices.push_back(v3); - vertices.push_back(v4); - QVERIFY(builder.vertices() == vertices); - - vector::Face> faces; - faces.push_back(f1); - faces.push_back(f2); - QVERIFY(builder.faces() == faces); + builder.addVertex(1); + builder.addVertex(2); + builder.addVertex(3); + builder.addVertex(4); + builder.addFace(0, 1, 2); + builder.addFace(1, 2, 3); + + QVERIFY((builder.vertices() == vector{1, 2, 3, 4})); + QVERIFY((builder.indices() == vector{0, 1, 2, 1, 2, 3})); } void MeshBuilderTest::cleanMesh() { - Vector1 v1(1); - Vector1 v2(2); - Vector1 v3(1); - Vector1 v4(4); - MeshBuilder::Face f1(0, 1, 2); - MeshBuilder::Face f2(1, 2, 3); - MeshBuilder builder; - builder.addVertex(v1); - builder.addVertex(v2); - builder.addVertex(v3); - builder.addVertex(v4); - builder.addFace(f1); - builder.addFace(f2); + builder.addVertex(1); + builder.addVertex(2); + builder.addVertex(1); + builder.addVertex(4); + builder.addFace(0, 1, 2); + builder.addFace(1, 2, 3); builder.cleanMesh(1); /* Verify cleanup */ - vector vertices; - vertices.push_back(v1); - vertices.push_back(v2); - vertices.push_back(v4); - QVERIFY(builder.vertices() == vertices); - - vector::Face> faces; - faces.push_back(f1); - faces.push_back(f2); - QVERIFY(builder.faces()[0].vertices[0] == 0); - QVERIFY(builder.faces()[0].vertices[1] == 1); - QVERIFY(builder.faces()[0].vertices[2] == 0); - QVERIFY(builder.faces()[1].vertices[0] == 1); - QVERIFY(builder.faces()[1].vertices[1] == 0); - QVERIFY(builder.faces()[1].vertices[2] == 2); - QVERIFY(f1.vertices[2] == f2.vertices[1]); + QVERIFY((builder.vertices() == vector{1, 2, 4})); + QVERIFY((builder.indices() == vector{0, 1, 0, 1, 0, 2})); } void MeshBuilderTest::subdivide() { - Vector1 v1(0); - Vector1 v2(2); - Vector1 v3(6); - Vector1 v4(8); - MeshBuilder::Face f1(0, 1, 2); - MeshBuilder::Face f2(1, 2, 3); - MeshBuilder builder; - builder.addVertex(v1); - builder.addVertex(v2); - builder.addVertex(v3); - builder.addVertex(v4); - builder.addFace(f1); - builder.addFace(f2); + builder.addVertex(0); + builder.addVertex(2); + builder.addVertex(6); + builder.addVertex(8); + builder.addFace(0, 1, 2); + builder.addFace(1, 2, 3); builder.subdivide(interpolator); - QVERIFY(builder.faces().size() == 8); - - QVERIFY(builder.vertices().size() == 10); - QVERIFY(builder.vertices()[0] == Vector1(0)); - QVERIFY(builder.vertices()[1] == Vector1(2)); - QVERIFY(builder.vertices()[2] == Vector1(6)); - QVERIFY(builder.vertices()[3] == Vector1(8)); - QVERIFY(builder.vertices()[4] == Vector1(1)); - QVERIFY(builder.vertices()[5] == Vector1(4)); - QVERIFY(builder.vertices()[6] == Vector1(3)); - QVERIFY(builder.vertices()[7] == Vector1(4)); - QVERIFY(builder.vertices()[8] == Vector1(7)); - QVERIFY(builder.vertices()[9] == Vector1(5)); - - QVERIFY(builder.faces()[0].vertices[0] == 4); - QVERIFY(builder.faces()[0].vertices[1] == 5); - QVERIFY(builder.faces()[0].vertices[2] == 6); - QVERIFY(builder.faces()[1].vertices[0] == 7); - QVERIFY(builder.faces()[1].vertices[1] == 8); - QVERIFY(builder.faces()[1].vertices[2] == 9); - QVERIFY(builder.faces()[2].vertices[0] == 0); - QVERIFY(builder.faces()[2].vertices[1] == 4); - QVERIFY(builder.faces()[2].vertices[2] == 6); - QVERIFY(builder.faces()[3].vertices[0] == 4); - QVERIFY(builder.faces()[3].vertices[1] == 1); - QVERIFY(builder.faces()[3].vertices[2] == 5); - QVERIFY(builder.faces()[4].vertices[0] == 6); - QVERIFY(builder.faces()[4].vertices[1] == 5); - QVERIFY(builder.faces()[4].vertices[2] == 2); - QVERIFY(builder.faces()[5].vertices[0] == 1); - QVERIFY(builder.faces()[5].vertices[1] == 7); - QVERIFY(builder.faces()[5].vertices[2] == 9); - QVERIFY(builder.faces()[6].vertices[0] == 7); - QVERIFY(builder.faces()[6].vertices[1] == 2); - QVERIFY(builder.faces()[6].vertices[2] == 8); - QVERIFY(builder.faces()[7].vertices[0] == 9); - QVERIFY(builder.faces()[7].vertices[1] == 8); - QVERIFY(builder.faces()[7].vertices[2] == 3); + QVERIFY(builder.indices().size() == 24); + + QVERIFY((builder.vertices() == vector{0, 2, 6, 8, 1, 4, 3, 4, 7, 5})); + QVERIFY((builder.indices() == vector{4, 5, 6, 7, 8, 9, 0, 4, 6, 4, 1, 5, 6, 5, 2, 1, 7, 9, 7, 2, 8, 9, 8, 3})); builder.cleanMesh(1);