From c83db64a77bf9c37f0502cbe2350eab7a36e9833 Mon Sep 17 00:00:00 2001
From: Joseph Walton-Rivers <joseph@walton-rivers.uk>
Date: Sun, 24 Apr 2022 15:28:12 +0100
Subject: [PATCH] clean up code for procedural models

---
 fggl/data/model.cpp         | 43 ++++++++++-----------------
 fggl/data/procedural.cpp    | 59 +++++++++++--------------------------
 include/fggl/data/model.hpp | 32 ++++++++++++++------
 include/fggl/math/types.hpp |  1 +
 4 files changed, 58 insertions(+), 77 deletions(-)

diff --git a/fggl/data/model.cpp b/fggl/data/model.cpp
index fcb96eb..b57aa33 100644
--- a/fggl/data/model.cpp
+++ b/fggl/data/model.cpp
@@ -2,10 +2,11 @@
 
 #include <map>
 #include <stdexcept>
+#include <algorithm>
 
 using namespace fggl::data;
 
-Mesh::Mesh() : m_verts(0), m_index(0) {
+Mesh::Mesh() : m_verts(0), m_index(0), restartVertex(Mesh::INVALID_IDX) {
 }
 
 void Mesh::pushIndex(unsigned int idx) {
@@ -13,36 +14,36 @@ void Mesh::pushIndex(unsigned int idx) {
 	m_index.push_back(idx);
 }
 
-unsigned int Mesh::pushVertex(Vertex vert) {
-	unsigned int idx = m_verts.size();
+Mesh::IndexType Mesh::pushVertex(Vertex vert) {
+	auto idx = m_verts.size();
 	m_verts.push_back( vert );
 	return idx;
 }
 
-int Mesh::indexOf(Vertex vert) {
-	int i = 0;
-	for ( auto& vert_v : m_verts ) {
-		if ( vert.posititon == vert_v.posititon ) {
-			return i;
-		}
-		++i;
+Mesh::IndexType Mesh::indexOf(Vertex term) {
+	auto itr = std::find(m_verts.begin(), m_verts.end(), term);
+	if ( itr == m_verts.end() ){
+		return INVALID_IDX;
 	}
-	return -1;
+	return itr - m_verts.begin();
+
 }
 
 void Mesh::removeDups() {
 	// lookups to make detecting duplicates easier
 	std::map<Vertex, unsigned int> mapping;
-	unsigned int nextID = 0;
 
 	// new data
 	std::vector< Vertex > newVerts;
 	std::vector< unsigned int > newIndexes;
 	newIndexes.reserve( m_index.size() ); // newIndex will be same size as oldIndex
 
+	unsigned int nextID = 0;
 	for ( auto& idx : m_index ) {
 		auto& vertex = m_verts[idx];
 		auto newID = nextID;
+
+		// if the vertex is known, use the existing ID, else allocate a new ID
 		try {
 			newID = mapping.at( vertex );
 		} catch ( std::out_of_range& e ){
@@ -53,20 +54,8 @@ void Mesh::removeDups() {
 		newIndexes.push_back( newID );
 	}
 
-	// FIXME must be faster way to do this...
-	m_verts.clear();
-	m_index.clear();
-
-	for ( auto& vert : newVerts ) {
-		m_verts.push_back( vert );
-	}
-
-	for ( auto& idx : newIndexes ) {
-		m_index.push_back( idx );
-	}
-
-	// ensure we're not wasting space
-	m_verts.shrink_to_fit();
-	m_index.shrink_to_fit();
+	// assign the replacement lists
+	m_verts = newVerts;
+	m_index = newIndexes;
 }
 
diff --git a/fggl/data/procedural.cpp b/fggl/data/procedural.cpp
index eff2903..9917560 100644
--- a/fggl/data/procedural.cpp
+++ b/fggl/data/procedural.cpp
@@ -40,15 +40,15 @@ static void computeNormalsDirect( fggl::data::Mesh& mesh, const int* colIdx, int
 	}
 }
 
-static void computeNormals( fggl::data::Mesh& mesh,
-								   const std::vector<int> idxList, // source index
-								   const std::vector<unsigned int>& idxMapping // source-to-mesh lookup
-								   ) {
+static void compute_normals(fggl::data::Mesh& mesh,
+							const std::vector<int>& idxList, // source index
+							const std::vector<unsigned int>& idxMapping // source-to-mesh lookup
+							) {
 
 	// clear the normals, so the summation below works correctly
 	for (unsigned int vertexIndex : idxMapping) {
 		auto& vertex = mesh.vertex( vertexIndex );
-		vertex.normal = glm::vec3(0.0F);
+		vertex.normal = ILLEGAL_NORMAL;
 	}
 
 	// we need to calculate the contribution for each vertex
@@ -58,9 +58,11 @@ static void computeNormals( fggl::data::Mesh& mesh,
 		auto& v2 = mesh.vertex( idxMapping[idxList[i + 1]] );
 		auto& v3 = mesh.vertex( idxMapping[idxList[i + 2]] );
 
+		// calculate the normal and area (formula for area the math textbook)
 		float area = glm::length(glm::cross(v3.posititon - v2.posititon, v1.posititon - v3.posititon)) / 2;
 		auto faceNormal = calcSurfaceNormal(v1.posititon, v2.posititon, v3.posititon);
 
+		// weight the normal according to the area of the surface (bigger area = more impact)
 		v1.normal += area * faceNormal;
 		v2.normal += area * faceNormal;
 		v3.normal += area * faceNormal;
@@ -73,10 +75,6 @@ static void computeNormals( fggl::data::Mesh& mesh,
 	}
 }
 
-constexpr float HALF_PI = M_PI / 2.0F;
-constexpr fggl::math::vec3 ILLEGAL_NORMAL{0.0F, 0.0F, 0.F};
-constexpr fggl::math::vec3 DEFAULT_COLOUR{1.0F, 1.0F, 1.0F};
-
 static void populateMesh(fggl::data::Mesh& mesh, const fggl::math::mat4 transform,
 						 const int nIdx, const fggl::math::vec3* pos, const int* idx ) {
 
@@ -85,12 +83,7 @@ static void populateMesh(fggl::data::Mesh& mesh, const fggl::math::mat4 transfor
 	// generate mesh
 	for (int i=0; i<nIdx; i++) {
 		glm::vec3 rawPos = transform * glm::vec4( pos[ idx[i] ], 1.0 );
-
-		Vertex vert{};
-		vert.posititon = rawPos;
-		vert.normal = glm::vec3(0.0F, 0.0F, 0.0F);
-		vert.colour = fggl::math::vec3(1.0F, 1.0F, 1.0F);
-		colIdx[ i ] = mesh.pushVertex( vert );
+		colIdx[ i ] = mesh.pushVertex( Vertex::from_pos(rawPos) );
 		mesh.pushIndex( colIdx[i] );
 	}
 
@@ -110,12 +103,8 @@ static void populateMesh(fggl::data::Mesh& mesh,
 
 	// clion this thinks this loop is infinite, my assumption is it's gone bananas
 	for (std::size_t i = 0; i < posList.size(); ++i) {
-		Vertex vert{
-			posList[i],
-			ILLEGAL_NORMAL,
-			DEFAULT_COLOUR
-		};
-		colIdx[i] = mesh.pushVertex(vert);
+		glm::vec3 position = transform * fggl::math::vec4(posList[i], 1.0F);
+		colIdx[i] = mesh.pushVertex( Vertex::from_pos(position) );
 	}
 
 	// use the remapped indexes for the mesh
@@ -123,7 +112,7 @@ static void populateMesh(fggl::data::Mesh& mesh,
 		mesh.pushIndex( colIdx[idx] );
 	}
 
-	computeNormals(mesh, idxList, colIdx);
+	compute_normals(mesh, idxList, colIdx);
 }
 
 namespace fggl::data {
@@ -152,12 +141,12 @@ namespace fggl::data {
 
 		std::vector<math::vec3> positions;
 
-		float verticalAngularStride = M_PI / stacks;
-		float horizontalAngularStride = (M_PI * 2) / slices;
+		float verticalAngularStride = math::PI / stacks;
+		float horizontalAngularStride = math::TAU / slices;
 
 		// calculate vertex positions
 		for (int vertical = 0; vertical < (stacks + 1); vertical++) {
-			float theta = (M_PI / 2.0f) - verticalAngularStride * vertical;
+			float theta = (math::HALF_PI) - verticalAngularStride * vertical;
 			for ( int horz = 0; horz < (slices + 1); horz++ ) {
 				float phi = horizontalAngularStride * horz;
 				math::vec3 position {
@@ -190,11 +179,7 @@ fggl::data::Mesh fggl::data::make_triangle() {
     // add points
     fggl::data::Mesh mesh;
     for (auto po : pos) {
-        Vertex vert{};
-        vert.posititon = po;
-	vert.normal = glm::vec3(0.0F, 0.0F, 0.0F);
-        vert.colour = fggl::math::vec3(1.0F, 1.0F, 1.0F);
-        mesh.push(vert);
+        mesh.push( Vertex::from_pos(po) );
     }
 
     // mesh
@@ -220,11 +205,7 @@ fggl::data::Mesh fggl::data::make_quad_xy() {
 	fggl::data::Mesh mesh;
 	std::array<int,4> colIdx;
 	for (int i = 0; i < 4; ++i){
-		Vertex vert{};
-		vert.posititon = pos[i];
-		vert.normal = glm::vec3(0.0F, 0.0F, 0.0F);
-		vert.colour = fggl::math::vec3(1.0F, 1.0F, 1.0F);
-		colIdx[ i ] = mesh.pushVertex(vert);
+		colIdx[ i ] = mesh.pushVertex( Vertex::from_pos(pos[i]));
 	}
 
 	for( auto i : idx ) {
@@ -249,11 +230,7 @@ fggl::data::Mesh fggl::data::make_quad_xz() {
 	fggl::data::Mesh mesh;
 	int colIdx[4];
 	for (int i = 0; i < 4; ++i){
-		Vertex vert{};
-		vert.posititon = pos[i];
-		vert.normal = glm::vec3(0.0F, 0.0F, 0.0F);
-		vert.colour = fggl::math::vec3(1.0F, 1.0F, 1.0F);
-		colIdx[ i ] = mesh.pushVertex(vert);
+		colIdx[ i ] = mesh.pushVertex( Vertex::from_pos(pos[i]) );
 	}
 
 	for( auto i : idx ) {
@@ -487,7 +464,7 @@ Mesh fggl::data::make_cube() {
 
 	// triagulate the quads
 	quadsToTris(cube, vertIndex, cubeIndexes, nQuads);
-	computeNormals(cube);
+	compute_normals(cube);
 
 	return cube;
 }*/
diff --git a/include/fggl/data/model.hpp b/include/fggl/data/model.hpp
index ca37309..2a21df4 100644
--- a/include/fggl/data/model.hpp
+++ b/include/fggl/data/model.hpp
@@ -4,15 +4,27 @@
 #include <tuple>
 #include <vector>
 #include <string>
+#include <climits>
 
 #include <fggl/math/types.hpp>
 
 namespace fggl::data {
 
+	constexpr math::vec3 ILLEGAL_NORMAL{0.0F, 0.0F, 0.F};
+	constexpr math::vec3 DEFAULT_COLOUR{1.0F, 1.0F, 1.0F};
+
 	struct Vertex {
 		math::vec3 posititon;
 		math::vec3 normal;
 		math::vec3 colour;
+
+		inline static Vertex from_pos(math::vec3 pos) {
+			return {
+				pos,
+				ILLEGAL_NORMAL,
+				DEFAULT_COLOUR
+			};
+		}
 	};
 
 	struct Vertex2D {
@@ -54,6 +66,9 @@ namespace fggl::data {
 	}
 
 	class Mesh {
+			using IndexType = unsigned int;
+			constexpr static const IndexType INVALID_IDX = IndexType(UINT_MAX);
+
 		public:
 			Mesh();
 			~Mesh() = default;
@@ -66,7 +81,7 @@ namespace fggl::data {
 			 */
 			inline void push(Vertex vert) {
 				auto idx = indexOf(vert);
-				if (idx == -1) {
+				if (idx == INVALID_IDX) {
 					idx = pushVertex(vert);
 					pushIndex(idx);
 				} else {
@@ -79,14 +94,14 @@ namespace fggl::data {
 			 *
 			 * Don't push non-existant indexes, it'll make the rendering backend sad.
 			 */
-			void pushIndex(unsigned int idx);
+			void pushIndex(IndexType idx);
 
 			/**
 			 * Force push a vertex duplicate.
 			 *
 			 * If the vetex already exists, it will be duplicated.
 			 */
-			unsigned int pushVertex(Vertex vert);
+			IndexType pushVertex(Vertex vert);
 
 			/**
 			 * remove Duplicate verticies
@@ -101,7 +116,7 @@ namespace fggl::data {
 			 *
 			 * Returns the index of a vert, else returns -1.
 			 */
-			int indexOf(Vertex vert);
+			IndexType indexOf(Vertex vert);
 
 			inline std::vector<Vertex> &vertexList() {
 				return m_verts;
@@ -111,7 +126,7 @@ namespace fggl::data {
 				return m_verts.size();
 			}
 
-			inline std::vector<uint32_t> &indexList() {
+			inline std::vector<IndexType> &indexList() {
 				return m_index;
 			}
 
@@ -119,15 +134,14 @@ namespace fggl::data {
 				return m_index.size();
 			}
 
-			inline Vertex &vertex(uint32_t idx) {
+			inline Vertex &vertex(IndexType idx) {
 				return m_verts[idx];
 			}
 
-			unsigned int restartVertex;
-
+			IndexType restartVertex;
 		private:
 			std::vector<Vertex> m_verts;
-			std::vector<unsigned int> m_index;
+			std::vector<IndexType> m_index;
 	};
 
 	struct StaticMesh {
diff --git a/include/fggl/math/types.hpp b/include/fggl/math/types.hpp
index 8d10352..70f2a22 100644
--- a/include/fggl/math/types.hpp
+++ b/include/fggl/math/types.hpp
@@ -20,6 +20,7 @@ namespace fggl::math {
 
 	constexpr float PI = M_PI;
 	constexpr float HALF_PI = M_PI_2;
+	constexpr float TAU = PI * 2;
 
 	// math types (aliased for ease of use)
 	using vec4 = glm::vec4;
-- 
GitLab