refactor: mesh load method

src/engine.cpp

@@ -142,9 +142,9 @@ void Engine::setup()
 	world_ = std::make_unique<World>();
 	Logger::info("World created");
 
-	// Create camera at position (0, 1, 3) with -20 degrees X rotation
+	// Create camera
 	camera_ = std::make_shared<Camera>();
-	camera_->set_position(glm::vec3(0.0f, 1.0f, 3.0f));
+	camera_->set_position(glm::vec3(0.0f, 1.0f, 10.0f));
 	camera_->set_rotation(glm::vec3(glm::radians(-20.0f), 0.0f, 0.0f));
 	camera_->set_projection(
 	    glm::perspective(glm::radians(45.0f),

src/mesh.cpp

@@ -14,6 +14,125 @@
 #include "texture.h"
 #include "vertex.h"
 
+// Helper structures and functions for OBJ loading
+namespace {
+
+struct LoadContext {
+	std::string						  base_dir;
+	std::unordered_map<std::string, std::shared_ptr<Texture>> texture_cache;
+	const std::vector<tinyobj::material_t>*			  materials;
+	const std::shared_ptr<Shader>&				  shader;
+};
+
+Vertex create_vertex(const tinyobj::attrib_t& attrib,
+		     const tinyobj::index_t&  idx)
+{
+	Vertex vertex;
+
+	// Position
+	vertex.position.x = attrib.vertices[3 * idx.vertex_index + 0];
+	vertex.position.y = attrib.vertices[3 * idx.vertex_index + 1];
+	vertex.position.z = attrib.vertices[3 * idx.vertex_index + 2];
+
+	// Color (default to white)
+	vertex.color = glm::vec3(1.0f, 1.0f, 1.0f);
+
+	// Texture coordinates
+	if (idx.texcoord_index >= 0) {
+		vertex.tex_coord.x =
+		    attrib.texcoords[2 * idx.texcoord_index + 0];
+		vertex.tex_coord.y = 1.0f
+		    - attrib.texcoords[2 * idx.texcoord_index + 1]; // Flip Y
+	}
+
+	return vertex;
+}
+
+void group_vertices_by_material(
+    const tinyobj::shape_t& shape, const tinyobj::attrib_t& attrib,
+    std::unordered_map<int, std::vector<Vertex>>&   out_vertices,
+    std::unordered_map<int, std::vector<uint16_t>>& out_indices)
+{
+	size_t index_offset = 0;
+	for (size_t f = 0; f < shape.mesh.num_face_vertices.size(); ++f) {
+		int fv		= shape.mesh.num_face_vertices[f];
+		int material_id = shape.mesh.material_ids[f];
+
+		for (size_t v = 0; v < static_cast<size_t>(fv); ++v) {
+			const tinyobj::index_t& idx =
+			    shape.mesh.indices[index_offset + v];
+
+			Vertex vertex = create_vertex(attrib, idx);
+			out_vertices[material_id].push_back(vertex);
+			out_indices[material_id].push_back(
+			    static_cast<uint16_t>(
+				out_vertices[material_id].size() - 1));
+		}
+
+		index_offset += fv;
+	}
+}
+
+std::shared_ptr<Texture> load_material_texture(int	    material_id,
+					       LoadContext& context)
+{
+	if (material_id < 0
+	    || material_id >= static_cast<int>(context.materials->size())) {
+		return nullptr;
+	}
+
+	const auto& mat		 = (*context.materials)[material_id];
+	std::string texture_name = mat.diffuse_texname;
+
+	// Try ambient texture if diffuse is not available
+	if (texture_name.empty()) {
+		texture_name = mat.ambient_texname;
+	}
+
+	if (texture_name.empty()) {
+		return nullptr;
+	}
+
+	// Check cache first
+	auto it = context.texture_cache.find(texture_name);
+	if (it != context.texture_cache.end()) {
+		return it->second;
+	}
+
+	// Load texture
+	std::string texture_path = context.base_dir + texture_name;
+	auto	    texture	 = Texture::load(texture_path.c_str());
+
+	if (texture) {
+		context.texture_cache[texture_name] = texture;
+		Logger::info(sstr("Loaded texture: ", texture_path));
+	} else {
+		Logger::warn(sstr("Failed to load texture: ", texture_path));
+	}
+
+	return texture;
+}
+
+void create_buffers_for_materials(
+    const std::unordered_map<int, std::vector<Vertex>>&	  material_vertices,
+    const std::unordered_map<int, std::vector<uint16_t>>& material_indices,
+    LoadContext& context, std::shared_ptr<Mesh>& mesh)
+{
+	for (const auto& [material_id, vertices] : material_vertices) {
+		const auto& indices = material_indices.at(material_id);
+
+		auto buffer  = std::make_shared<Buffer>(vertices, indices);
+		auto texture = load_material_texture(material_id, context);
+		auto material_shader =
+		    context.shader ? context.shader : state::default_shader;
+
+		Material mat(texture, material_shader);
+		mesh->add_buffer(buffer, mat);
+	}
+}
+
+} // namespace
+
 void Mesh::add_buffer(const std::shared_ptr<Buffer>& buffer,
 		      const Material&		     material)
 {
@@ -26,152 +145,44 @@ std::shared_ptr<Mesh> Mesh::load(const char*			filename,
 {
 	Logger::info(sstr("Loading mesh from: ", filename));
 
-	// Load the OBJ file
+	// Load OBJ file
 	tinyobj::attrib_t		 attrib;
 	std::vector<tinyobj::shape_t>	 shapes;
 	std::vector<tinyobj::material_t> materials;
-	std::string			 warn_err;
+	std::string			 err;
 
-	// Get the directory of the OBJ file for loading materials
 	std::filesystem::path obj_path(filename);
 	std::string	      base_dir = obj_path.parent_path().string();
 	if (!base_dir.empty()) {
 		base_dir += "/";
 	}
 
-	if (!tinyobj::LoadObj(&attrib, &shapes, &materials, &warn_err,
+	if (!tinyobj::LoadObj(&attrib, &shapes, &materials, &err, filename,
-			      filename, base_dir.c_str())) {
+			      base_dir.c_str())) {
 		Logger::error(
-		    sstr("Failed to load OBJ file: ", filename, " - ",
+		    sstr("Failed to load OBJ file: ", filename, " - ", err));
-			 warn_err));
 		return nullptr;
 	}
 
-	if (!warn_err.empty()) {
+	if (!err.empty()) {
-		Logger::warn(sstr("TinyObjLoader warning: ", warn_err));
+		Logger::warn(sstr("TinyObjLoader warning: ", err));
 	}
 
-	// Create the mesh
+	// Setup load context
-	auto mesh = std::make_shared<Mesh>();
+	LoadContext context{base_dir, {}, &materials, shader};
-
-	// Cache loaded textures to avoid loading the same texture multiple
-	// times
-	std::unordered_map<std::string, std::shared_ptr<Texture>> texture_cache;
 
 	// Process each shape
+	auto mesh = std::make_shared<Mesh>();
 	for (const auto& shape : shapes) {
 		Logger::info(sstr("Processing shape: ", shape.name));
 
-		// Group faces by material
 		std::unordered_map<int, std::vector<Vertex>> material_vertices;
 		std::unordered_map<int, std::vector<uint16_t>> material_indices;
 
-		// Process all faces in this shape
+		group_vertices_by_material(shape, attrib, material_vertices,
-		size_t index_offset = 0;
+					   material_indices);
-		for (size_t f = 0; f < shape.mesh.num_face_vertices.size();
+		create_buffers_for_materials(material_vertices,
-		     f++) {
+					     material_indices, context, mesh);
-			int fv		= shape.mesh.num_face_vertices[f];
-			int material_id = shape.mesh.material_ids[f];
-
-			// Process each vertex in the face
-			for (size_t v = 0; v < static_cast<size_t>(fv); v++) {
-				tinyobj::index_t idx =
-				    shape.mesh.indices[index_offset + v];
-
-				Vertex vertex;
-
-				// Position
-				vertex.position.x =
-				    attrib.vertices[3 * idx.vertex_index + 0];
-				vertex.position.y =
-				    attrib.vertices[3 * idx.vertex_index + 1];
-				vertex.position.z =
-				    attrib.vertices[3 * idx.vertex_index + 2];
-
-				// Color (default to white)
-				vertex.color = glm::vec3(1.0f, 1.0f, 1.0f);
-
-				// Texture coordinates
-				if (idx.texcoord_index >= 0) {
-					vertex.tex_coord.x =
-					    attrib.texcoords
-						[2 * idx.texcoord_index + 0];
-					vertex.tex_coord.y = 1.0f
-					    - attrib.texcoords
-						  [2 * idx.texcoord_index
-						   + 1]; // Flip Y
-				}
-
-				material_vertices[material_id].push_back(
-				    vertex);
-				material_indices[material_id].push_back(
-				    static_cast<uint16_t>(
-					material_vertices[material_id].size()
-					- 1));
-			}
-
-			index_offset += fv;
-		}
-
-		// Create buffers for each material
-		for (const auto& [material_id, vertices] : material_vertices) {
-			const auto& indices = material_indices[material_id];
-
-			// Create buffer
-			auto buffer =
-			    std::make_shared<Buffer>(vertices, indices);
-
-			// Create material
-			std::shared_ptr<Texture> texture = nullptr;
-
-			// Load texture if material has one
-			if (material_id >= 0
-			    && material_id
-				< static_cast<int>(materials.size())) {
-				const auto& mat = materials[material_id];
-				std::string texture_name = mat.diffuse_texname;
-
-				// Try ambient texture if diffuse is not
-				// available
-				if (texture_name.empty()) {
-					texture_name = mat.ambient_texname;
-				}
-
-				if (!texture_name.empty()) {
-					// Check cache first
-					auto it =
-					    texture_cache.find(texture_name);
-					if (it != texture_cache.end()) {
-						texture = it->second;
-					} else {
-						// Load texture
-						std::string texture_path =
-						    base_dir + texture_name;
-						texture = Texture::load(
-						    texture_path.c_str());
-						if (texture) {
-							texture_cache
-							    [texture_name] =
-								texture;
-							Logger::info(sstr(
-							    "Loaded texture: ",
-							    texture_path));
-						} else {
-							Logger::warn(sstr(
-							    "Failed to load texture: ",
-							    texture_path));
-						}
-					}
-				}
-			}
-
-			// Use provided shader or default shader
-			auto material_shader =
-			    shader ? shader : state::default_shader;
-			Material mat(texture, material_shader);
-
-			mesh->add_buffer(buffer, mat);
-		}
 	}
 
 	Logger::info(sstr("Mesh loaded successfully with ", mesh->num_buffers(),

src/mesh.h

@@ -14,9 +14,9 @@ public:
 	Mesh()	= default;
 	~Mesh() = default;
 
-	static std::shared_ptr<Mesh> load(
+	static std::shared_ptr<Mesh>
-	    const char*			     filename,
+	load(const char*		    filename,
-	    const std::shared_ptr<Shader>& shader = nullptr);
+	     const std::shared_ptr<Shader>& shader = nullptr);
 
 	void add_buffer(const std::shared_ptr<Buffer>& buffer,
 			const Material&		       material);