Models all tested and refactored so that the draw() call doesn't do a bunch of irrelevant shit.

14-refactor/src/main.cpp

@@ -213,16 +213,23 @@ Config setup() {
     },
   };
 
+  // REFACTOR: really this should be done somewhere else
+  for(auto& model : config.models) {
+    model.connect_shader(config.shader);
+  }
+
   return config;
 }
 
-void draw_popcorn(Config& config, size_t model_i, size_t model_pos, glm::mat4& projection, glm::mat4& view)
+void draw_models(Config& config, size_t model_i, size_t model_pos, glm::mat4& projection, glm::mat4& view)
 {
   glm::vec3 objectColor = {1.0, 0.94, 0.78};
   config.shader.applyMaterial(config.cubeMaterial);
 
+  float time = glfwGetTime();
   glm::mat4 model = glm::mat4(1.0f);
   model = glm::translate(model, popcorn_positions[model_pos]);
+  model = glm::rotate(model, glm::radians(time * 10.0f), glm::vec3(1.0f, 0.3f, 0.5f));
 
   config.shader.setMat4("model", model);
 
@@ -256,7 +263,7 @@ void render(GLFWwindow* window, Config& config) {
   config.shader.applyLighting(config.light);
 
   for(size_t i = 0; i < CUBE_COUNT; i++) {
-    draw_popcorn(config, 0, i, projection, view);
+    draw_models(config, 0, i, projection, view);
   }
 
   glfwSwapBuffers(window);

14-refactor/src/mesh.cpp

@@ -1,45 +1,27 @@
 #include "mesh.hpp"
 #include "dbc.hpp"
 
-void Mesh::Draw(Shader &shader) {
+void Mesh::draw(Shader &shader) {
-  unsigned int diffuseNr = 1;
-  unsigned int specularNr = 1;
-  unsigned int normalNr = 1;
-  unsigned int heightNr = 1;
-
-  for(unsigned int i = 0; i < textures.size(); i++) {
-    glActiveTexture(GL_TEXTURE0 + i);
-    unsigned int number = 0;
-    std::string& name = textures[i].type;
-
-    if(name == "texture_diffuse") {
-      number = diffuseNr++;
-    } else if(name == "texture_specular") {
-      number = specularNr++;
-    } else if(name == "texture_normal") {
-      number = normalNr++;
-    } else if(name == "texture_height") {
-      number = heightNr++;
-    } else {
-      dbc::sentinel($F("Invalid texture type name {}", name));
-    }
-
-    std::string target = std::format("{}{}", name, number);
-
-    // TODO: Get this uniform ID once and cache it?
-    glUniform1i(glGetUniformLocation(shader.ID, target.c_str()), i);
-    glBindTexture(GL_TEXTURE_2D, textures[i].id);
-  }
-
   glBindVertexArray(VAO);
-  glDrawElements(GL_TRIANGLES, static_cast<unsigned int>(indices.size()), GL_UNSIGNED_INT, 0);
+  glDrawElements(GL_TRIANGLES, indices.size(), GL_UNSIGNED_INT, 0);
   glBindVertexArray(0);
-
   glActiveTexture(GL_TEXTURE0);
 }
 
+void Mesh::connect_shader(const Shader& shader) {
+  for(unsigned int i = 0; i < textures.size(); i++) {
+    Texture& texture = textures[i];
+    glActiveTexture(GL_TEXTURE0 + i);
 
-void Mesh::setupMesh() {
+    // move this to setup_mesh
+    glUniform1i(texture.uniform_id, i);
+    glBindTexture(GL_TEXTURE_2D, texture.id);
+
+    texture.uniform_id = glGetUniformLocation(shader.ID, texture.target.c_str());
+  }
+}
+
+void Mesh::setup_mesh() {
   glGenVertexArrays(1, &VAO);
   glGenBuffers(1, &VBO);
   glGenBuffers(1, &EBO);
@@ -53,11 +35,11 @@ void Mesh::setupMesh() {
 
   // this matches the locations in the vertex shader
   glEnableVertexAttribArray(0);
-  glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, Position));
+  glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, position));
 
   glEnableVertexAttribArray(1);
-  glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, Normal));
+  glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, normal));
 
   glEnableVertexAttribArray(2);
-  glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, TexCoords));
+  glVertexAttribPointer(2, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex), (void*)offsetof(Vertex, tex_coords));
 }

14-refactor/src/mesh.hpp

@@ -10,33 +10,63 @@
 #include <vector>
 
 struct Vertex {
-  glm::vec3 Position;
+  glm::vec3 position;
-  glm::vec3 Normal;
+  glm::vec3 normal;
-  glm::vec2 TexCoords;
+  glm::vec2 tex_coords;
+};
+
+struct TextureCounts {
+  unsigned int diffuseNr = 1;
+  unsigned int specularNr = 1;
+  unsigned int normalNr = 1;
+  unsigned int heightNr = 1;
 };
 
 struct Texture {
   unsigned int id;
   std::string type;
   std::string path;
+  std::string target;
+  int uniform_id=-1;
+
+  Texture(unsigned int id, const std::string& type, const std::string& path, TextureCounts& count) :
+    id(id), type(type)
+  {
+    unsigned int number = 0;
+
+    if(type == "texture_diffuse") {
+      number = count.diffuseNr++;
+    } else if(type == "texture_specular") {
+      number = count.specularNr++;
+    } else if(type == "texture_normal") {
+      number = count.normalNr++;
+    } else if(type == "texture_height") {
+      number = count.heightNr++;
+    } else {
+      dbc::sentinel($F("Invalid texture type={} for file={}", type, path));
+    }
+
+    target = std::format("{}{}", type, number);
+  }
 };
 
 struct Mesh {
   std::vector<Vertex> vertices;
   std::vector<unsigned int> indices;
   std::vector<Texture> textures;
-  unsigned int VAO;
+  unsigned int VAO = 0;
-  unsigned int VBO;
+  unsigned int VBO = 0;
-  unsigned int EBO;
+  unsigned int EBO = 0;
 
-  Mesh(std::vector<Vertex> vertices, std::vector<unsigned int> indices, std::vector<Texture> textures) :
+  Mesh(std::vector<Vertex>& vertices, std::vector<unsigned int>& indices, std::vector<Texture>& textures) :
     vertices(vertices),
     indices(indices),
     textures(textures)
   {
-    setupMesh();
+    setup_mesh();
   }
 
-  void Draw(Shader &shader);
+  void draw(Shader &shader);
-  void setupMesh();
+  void setup_mesh();
+  void connect_shader(const Shader& shader);
 };

14-refactor/src/model.cpp

@@ -82,8 +82,8 @@ unsigned int texture_from_file(const std::string& directory, const std::string&
 }
 
 void Model::draw(Shader &shader) {
-  for(unsigned int i = 0; i < meshes.size(); i++) {
+  for(auto& mesh : meshes) {
-    meshes[i].Draw(shader);
+    mesh.draw(shader);
   }
 }
 
@@ -122,36 +122,36 @@ Mesh Model::process_mesh(const aiScene *scene, aiMesh *mesh) {
   std::vector<Texture> textures;
 
   for(unsigned int i = 0; i < mesh->mNumVertices; i++) {
-    Vertex vertex;
+    glm::vec3 position{
-    vertex.Position = glm::vec3(
       mesh->mVertices[i].x,
       mesh->mVertices[i].y,
-      mesh->mVertices[i].z);
+      mesh->mVertices[i].z};
+
+    glm::vec3 normal{};
+    glm::vec2 tex_coords{0.0f, 0.0f};
 
     if(mesh->HasNormals()) {
-      vertex.Normal = glm::vec3(
+      normal = glm::vec3(
         mesh->mNormals[i].x,
         mesh->mNormals[i].y,
         mesh->mNormals[i].z);
     }
 
     if(mesh->mTextureCoords[0]) {
-      vertex.TexCoords = glm::vec2(
+      tex_coords = glm::vec2(
           mesh->mTextureCoords[0][i].x,
           mesh->mTextureCoords[0][i].y
       );
-    } else {
-      vertex.TexCoords = glm::vec2(0.0f, 0.0f);
     }
 
-    vertices.push_back(vertex);
+    vertices.emplace_back(position, normal, tex_coords);
   }
 
   for(unsigned int i = 0; i < mesh->mNumFaces; i++) {
-    aiFace face = mesh->mFaces[i];
+    aiFace& face = mesh->mFaces[i];
 
     for(unsigned int j = 0; j < face.mNumIndices; j++) {
-      indices.push_back(face.mIndices[j]);
+      indices.emplace_back(face.mIndices[j]);
     }
   }
 
@@ -179,32 +179,39 @@ Mesh Model::process_mesh(const aiScene *scene, aiMesh *mesh) {
   return Mesh(vertices, indices, textures);
 }
 
-void Model::load_material_textures(const aiScene *scene, std::vector<Texture>& textures, aiMaterial *mat, aiTextureType type, std::string typeName)
+void Model::load_material_textures(const aiScene *scene, std::vector<Texture>& textures, aiMaterial *mat, aiTextureType type, std::string type_name)
 {
   for(unsigned int i = 0; i < mat->GetTextureCount(type); i++) {
     aiString str;
 
     mat->GetTexture(type, i, &str);
-    std::string tx_str{str.C_Str()};
+    std::string tx_path{str.C_Str()};
-    dbc::check(!tx_str.empty(), "Texture has empty path, should be impossible?");
+    dbc::check(!tx_path.empty(), "Texture has empty path, should be impossible?");
 
-    if(textures_loaded.contains(tx_str)) {
+    if(textures_loaded.contains(tx_path)) {
-      textures.push_back(textures_loaded.at(tx_str));
+      textures.push_back(textures_loaded.at(tx_path));
     } else {
       unsigned int tx_id = 0;
 
       // if it's a *# style path then it's internal
-      if(tx_str[0] == '*') {
+      if(tx_path[0] == '*') {
         // get the texture from the internal version
-        tx_id = texture_from_internal(scene, std::stoi(tx_str.substr(1)));
+        tx_id = texture_from_internal(scene, std::stoi(tx_path.substr(1)));
       } else {
         // else get it from a file
-        tx_id = texture_from_file(directory, tx_str);
+        tx_id = texture_from_file(directory, tx_path);
       }
 
       // dubious code here, but seems to be right
-      auto& result = textures.emplace_back(tx_id, typeName, tx_str);
+      auto& result = textures.emplace_back(tx_id, type_name, tx_path, texture_counts);
-      textures_loaded.try_emplace(tx_str, result);
+
+      textures_loaded.try_emplace(tx_path, result);
     }
   }
 }
+
+void Model::connect_shader(const Shader& shader) {
+  for(auto& mesh : meshes) {
+    mesh.connect_shader(shader);
+  }
+}

14-refactor/src/model.hpp

@@ -23,6 +23,7 @@ struct Model {
   std::vector<Mesh> meshes;
   std::string directory;
   std::string model_path;
+  TextureCounts texture_counts;
   bool gammaCorrection;
 
   Model(const std::string& directory, const std::string& model_path) :
@@ -38,4 +39,6 @@ struct Model {
   Mesh process_mesh(const aiScene *scene, aiMesh *mesh);
 
   void load_material_textures(const aiScene *scene, std::vector<Texture>& textures, aiMaterial *mat, aiTextureType type, std::string typeName);
+
+  void connect_shader(const Shader& shader);
 };

14-refactor/tests/model_tests.cpp

@@ -6,6 +6,9 @@
 
 using namespace fuc2;
 
+/*
+ * This also tests Mesh by using Model to load them.
+ */
 namespace model_tests {
   void test_load_textures() {
     Shader shader{"shaders/14-vert.glsl", "shaders/14-frag.glsl"};