From d139e19bd287ba340b923136c266a77367b83990 Mon Sep 17 00:00:00 2001 From: HampusM Date: Sat, 18 May 2024 16:57:03 +0200 Subject: refactor(engine): fix clippy lints --- engine/src/camera/fly.rs | 6 +- engine/src/file_format/wavefront/mtl.rs | 115 +++++----- engine/src/file_format/wavefront/obj.rs | 373 ++++++++++++++++++-------------- engine/src/lighting.rs | 1 + engine/src/math.rs | 4 +- engine/src/opengl/mod.rs | 2 +- engine/src/renderer/mod.rs | 6 +- engine/src/util.rs | 2 + 8 files changed, 272 insertions(+), 237 deletions(-) diff --git a/engine/src/camera/fly.rs b/engine/src/camera/fly.rs index 7fa435b..3f5a0c2 100644 --- a/engine/src/camera/fly.rs +++ b/engine/src/camera/fly.rs @@ -73,11 +73,7 @@ fn update( cursor_state.current_yaw += x_offset; cursor_state.current_pitch += y_offset; - if cursor_state.current_pitch > 89.0 { - cursor_state.current_pitch = 89.0; - } else if cursor_state.current_pitch < -89.0 { - cursor_state.current_pitch = -89.0; - } + cursor_state.current_pitch = cursor_state.current_pitch.clamp(-89.0, 89.0); // TODO: This casting to a f32 from a f64 is horrible. fix it #[allow(clippy::cast_possible_truncation)] diff --git a/engine/src/file_format/wavefront/mtl.rs b/engine/src/file_format/wavefront/mtl.rs index 9c6c136..ef6e894 100644 --- a/engine/src/file_format/wavefront/mtl.rs +++ b/engine/src/file_format/wavefront/mtl.rs @@ -133,64 +133,31 @@ fn statements_to_materials( match statement.keyword { Keyword::Ka => { - if statement.arguments.len() > 3 { - return Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no, - }); - } - - let red = statement.get_float_arg(0, line_no)?; - let green = statement.get_float_arg(1, line_no)?; - let blue = statement.get_float_arg(2, line_no)?; + let color = get_color_from_statement(&statement, line_no)?; #[cfg(feature = "debug")] tracing::debug!("Adding ambient color"); - curr_material.material_builder = curr_material - .material_builder - .ambient(Color { red, green, blue }); + curr_material.material_builder = + curr_material.material_builder.ambient(color); } Keyword::Kd => { - if statement.arguments.len() > 3 { - return Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no, - }); - } - - let red = statement.get_float_arg(0, line_no)?; - let green = statement.get_float_arg(1, line_no)?; - let blue = statement.get_float_arg(2, line_no)?; + let color = get_color_from_statement(&statement, line_no)?; #[cfg(feature = "debug")] tracing::debug!("Adding diffuse color"); - curr_material.material_builder = curr_material - .material_builder - .diffuse(Color { red, green, blue }); + curr_material.material_builder = + curr_material.material_builder.diffuse(color); } Keyword::Ks => { - if statement.arguments.len() > 3 { - return Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no, - }); - } - - let red = statement.get_float_arg(0, line_no)?; - let green = statement.get_float_arg(1, line_no)?; - let blue = statement.get_float_arg(2, line_no)?; + let color = get_color_from_statement(&statement, line_no)?; #[cfg(feature = "debug")] tracing::debug!("Adding specular color"); - curr_material.material_builder = curr_material - .material_builder - .specular(Color { red, green, blue }); + curr_material.material_builder = + curr_material.material_builder.specular(color); } Keyword::MapKa => { if statement.arguments.len() > 1 { @@ -216,17 +183,7 @@ fn statements_to_materials( .ambient_map(texture_id); } Keyword::MapKd => { - if statement.arguments.len() > 1 { - return Err(Error::UnsupportedArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no, - }); - } - - let texture_file_path = statement.get_text_arg(0, line_no)?; - - let texture = Texture::open(Path::new(texture_file_path))?; + let texture = get_map_from_texture(&statement, line_no)?; #[cfg(feature = "debug")] tracing::debug!("Adding diffuse map"); @@ -239,17 +196,7 @@ fn statements_to_materials( .diffuse_map(texture_id); } Keyword::MapKs => { - if statement.arguments.len() > 1 { - return Err(Error::UnsupportedArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no, - }); - } - - let texture_file_path = statement.get_text_arg(0, line_no)?; - - let texture = Texture::open(Path::new(texture_file_path))?; + let texture = get_map_from_texture(&statement, line_no)?; #[cfg(feature = "debug")] tracing::debug!("Adding specular map"); @@ -261,7 +208,7 @@ fn statements_to_materials( .texture(texture) .specular_map(texture_id); } - _ => {} + Keyword::Newmtl => {} } } @@ -277,6 +224,44 @@ fn statements_to_materials( Ok(materials) } +fn get_color_from_statement( + statement: &Statement, + line_no: usize, +) -> Result, Error> +{ + if statement.arguments.len() > 3 { + return Err(Error::InvalidArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no, + }); + } + + let red = statement.get_float_arg(0, line_no)?; + let green = statement.get_float_arg(1, line_no)?; + let blue = statement.get_float_arg(2, line_no)?; + + Ok(Color { red, green, blue }) +} + +fn get_map_from_texture( + statement: &Statement, + line_no: usize, +) -> Result +{ + if statement.arguments.len() > 1 { + return Err(Error::UnsupportedArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no, + }); + } + + let texture_file_path = statement.get_text_arg(0, line_no)?; + + Ok(Texture::open(Path::new(texture_file_path))?) +} + keyword! { #[derive(Debug, PartialEq, Eq, Clone, Copy)] enum Keyword { diff --git a/engine/src/file_format/wavefront/obj.rs b/engine/src/file_format/wavefront/obj.rs index 97e0110..88e6580 100644 --- a/engine/src/file_format/wavefront/obj.rs +++ b/engine/src/file_format/wavefront/obj.rs @@ -9,6 +9,7 @@ use crate::file_format::wavefront::common::{ keyword, parse_statement_line, ParsingError, + Statement, Triplet, }; use crate::mesh::Mesh; @@ -41,161 +42,12 @@ pub fn parse(obj_content: &str) -> Result }) .collect::, _>>()?; - let vertex_positions = statements - .iter() - .filter_map(|(line_no, statement)| { - if statement.keyword != Keyword::V { - return None; - } - - if statement.arguments.len() == 4 { - return Some(Err(Error::UnsupportedArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - if statement.arguments.len() > 4 { - return Some(Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - let x = try_option!(statement.get_float_arg(0, *line_no)); - let y = try_option!(statement.get_float_arg(1, *line_no)); - let z = try_option!(statement.get_float_arg(2, *line_no)); - - Some(Ok(Vec3 { x, y, z })) - }) - .collect::, Error>>()?; - - let texture_positions = statements - .iter() - .filter_map(|(line_no, statement)| { - if statement.keyword != Keyword::Vt { - return None; - } - - if statement.arguments.len() == 3 { - return Some(Err(Error::UnsupportedArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - if statement.arguments.len() > 3 { - return Some(Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - let u = try_option!(statement.get_float_arg(0, *line_no)); - let v = try_option!(statement.get_float_arg(1, *line_no)); - - Some(Ok(Vec2 { x: u, y: v })) - }) - .collect::, Error>>()?; - - let vertex_normals = statements - .iter() - .filter_map(|(line_no, statement)| { - if statement.keyword != Keyword::Vn { - return None; - } - - if statement.arguments.len() > 3 { - return Some(Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - let i = try_option!(statement.get_float_arg(0, *line_no)); - let j = try_option!(statement.get_float_arg(1, *line_no)); - let k = try_option!(statement.get_float_arg(2, *line_no)); - - Some(Ok(Vec3 { x: i, y: j, z: k })) - }) - .collect::, Error>>()?; - - let material_specifiers = statements - .iter() - .filter_map(|(line_no, statement)| { - if statement.keyword != Keyword::Usemtl { - return None; - } - - if statement.arguments.len() > 1 { - return Some(Err(Error::InvalidArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - let material_name = try_option!(statement.get_text_arg(0, *line_no)); - - Some(Ok(MaterialSpecifier { - material_name: material_name.to_string(), - line_no: *line_no, - })) - }) - .collect::, Error>>()?; - - let faces = statements - .iter() - .filter_map(|(line_no, statement)| { - if statement.keyword != Keyword::F { - return None; - } - - if statement.arguments.len() > 3 { - return Some(Err(Error::UnsupportedArgumentCount { - keyword: statement.keyword.to_string(), - arg_count: statement.arguments.len(), - line_no: *line_no, - })); - } - - let vertex_a = try_option!(statement.get_triplet_arg(0, *line_no)).into(); - let vertex_b = try_option!(statement.get_triplet_arg(1, *line_no)).into(); - let vertex_c = try_option!(statement.get_triplet_arg(2, *line_no)).into(); - - let material_name = - find_material_specifier_for_line_no(&material_specifiers, *line_no) - .map(|material_specifier| material_specifier.material_name.clone()); - - Some(Ok(Face { - vertices: [vertex_a, vertex_b, vertex_c], - material_name, - })) - }) - .collect::, Error>>()?; - - let mtl_libs = statements - .iter() - .filter_map(|(line_no, statement)| { - if statement.keyword != Keyword::Mtllib { - return None; - } - - let mtl_lib_paths = try_option!(statement - .arguments - .iter() - .enumerate() - .map(|(index, value)| Ok(PathBuf::from(value.to_text(index, *line_no)?))) - .collect::, ParsingError>>()); - - Some(Ok(mtl_lib_paths)) - }) - .collect::, Error>>()?; + let vertex_positions = get_vertex_positions_from_statements(&statements)?; + let texture_positions = get_texture_positions_from_statements(&statements)?; + let vertex_normals = get_vertex_normals_from_statements(&statements)?; + let material_specs = get_material_specs_from_statements(&statements)?; + let faces = get_faces_from_statements(&statements, &material_specs)?; + let mtl_libs = get_mtl_libs_from_statements(&statements)?; Ok(Obj { vertex_positions, @@ -206,7 +58,7 @@ pub fn parse(obj_content: &str) -> Result }) } -/// The output from parsing the content of a Wavefront `.obj` using [`parse`]. +/// The data of a Wavefront object file. #[derive(Debug)] #[non_exhaustive] pub struct Obj @@ -220,13 +72,20 @@ pub struct Obj impl Obj { + /// Creates a [`Mesh`] from this Wavefront object file data. + /// + /// # Errors + /// Returns `Err` if: + /// - A face's vertex position cannot be found + /// - A face's texture position cannot be found + /// - A face's vertex normal cannot be found + /// - A face index does not fit in a [`u32`] pub fn to_mesh(&self) -> Result { let vertices = self .faces .iter() - .map(|face| face.vertices.clone()) - .flatten() + .flat_map(|face| face.vertices.clone()) .map(|face_vertex| face_vertex.to_vertex(self)) .collect::, Error>>()?; @@ -235,8 +94,7 @@ impl Obj Some( self.faces .iter() - .map(|face| face.vertices.clone()) - .flatten() + .flat_map(|face| face.vertices.clone()) .enumerate() .map(|(index, _)| { u32::try_from(index).map_err(|_| Error::FaceIndexTooBig(index)) @@ -295,6 +153,12 @@ pub struct FaceVertex impl FaceVertex { /// Tries to convert this face vertex into a [`Vertex`]. + /// + /// # Errors + /// Returns `Err` if: + /// - The face's vertex position cannot be found in the given [`Obj`] + /// - The face's texture position cannot be found in the given [`Obj`] + /// - The face's vertex normal cannot be found in the given [`Obj`] pub fn to_vertex(&self, obj: &Obj) -> Result { let mut vertex_builder = VertexBuilder::default(); @@ -313,7 +177,7 @@ impl FaceVertex texture_pos_index: face_vertex_texture, })?; - vertex_builder = vertex_builder.texture_coords(texture_pos.clone()); + vertex_builder = vertex_builder.texture_coords(*texture_pos); } if let Some(face_vertex_normal) = self.normal { @@ -479,6 +343,193 @@ where } } +fn get_vertex_positions_from_statements( + statements: &[(usize, Statement)], +) -> Result>, Error> +{ + statements + .iter() + .filter_map(|(line_no, statement)| { + if statement.keyword != Keyword::V { + return None; + } + + if statement.arguments.len() == 4 { + return Some(Err(Error::UnsupportedArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + if statement.arguments.len() > 4 { + return Some(Err(Error::InvalidArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + let x = try_option!(statement.get_float_arg(0, *line_no)); + let y = try_option!(statement.get_float_arg(1, *line_no)); + let z = try_option!(statement.get_float_arg(2, *line_no)); + + Some(Ok(Vec3 { x, y, z })) + }) + .collect::, Error>>() +} + +fn get_texture_positions_from_statements( + statements: &[(usize, Statement)], +) -> Result>, Error> +{ + statements + .iter() + .filter_map(|(line_no, statement)| { + if statement.keyword != Keyword::Vt { + return None; + } + + if statement.arguments.len() == 3 { + return Some(Err(Error::UnsupportedArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + if statement.arguments.len() > 3 { + return Some(Err(Error::InvalidArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + let u = try_option!(statement.get_float_arg(0, *line_no)); + let v = try_option!(statement.get_float_arg(1, *line_no)); + + Some(Ok(Vec2 { x: u, y: v })) + }) + .collect::, Error>>() +} + +fn get_vertex_normals_from_statements( + statements: &[(usize, Statement)], +) -> Result>, Error> +{ + statements + .iter() + .filter_map(|(line_no, statement)| { + if statement.keyword != Keyword::Vn { + return None; + } + + if statement.arguments.len() > 3 { + return Some(Err(Error::InvalidArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + let i = try_option!(statement.get_float_arg(0, *line_no)); + let j = try_option!(statement.get_float_arg(1, *line_no)); + let k = try_option!(statement.get_float_arg(2, *line_no)); + + Some(Ok(Vec3 { x: i, y: j, z: k })) + }) + .collect::, Error>>() +} + +fn get_material_specs_from_statements( + statements: &[(usize, Statement)], +) -> Result, Error> +{ + statements + .iter() + .filter_map(|(line_no, statement)| { + if statement.keyword != Keyword::Usemtl { + return None; + } + + if statement.arguments.len() > 1 { + return Some(Err(Error::InvalidArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + let material_name = try_option!(statement.get_text_arg(0, *line_no)); + + Some(Ok(MaterialSpecifier { + material_name: material_name.to_string(), + line_no: *line_no, + })) + }) + .collect::, Error>>() +} + +fn get_faces_from_statements( + statements: &[(usize, Statement)], + material_specifiers: &[MaterialSpecifier], +) -> Result, Error> +{ + statements + .iter() + .filter_map(|(line_no, statement)| { + if statement.keyword != Keyword::F { + return None; + } + + if statement.arguments.len() > 3 { + return Some(Err(Error::UnsupportedArgumentCount { + keyword: statement.keyword.to_string(), + arg_count: statement.arguments.len(), + line_no: *line_no, + })); + } + + let vertex_a = try_option!(statement.get_triplet_arg(0, *line_no)).into(); + let vertex_b = try_option!(statement.get_triplet_arg(1, *line_no)).into(); + let vertex_c = try_option!(statement.get_triplet_arg(2, *line_no)).into(); + + let material_name = + find_material_specifier_for_line_no(material_specifiers, *line_no) + .map(|material_specifier| material_specifier.material_name.clone()); + + Some(Ok(Face { + vertices: [vertex_a, vertex_b, vertex_c], + material_name, + })) + }) + .collect::, Error>>() +} + +fn get_mtl_libs_from_statements( + statements: &[(usize, Statement)], +) -> Result>, Error> +{ + statements + .iter() + .filter_map(|(line_no, statement)| { + if statement.keyword != Keyword::Mtllib { + return None; + } + + let mtl_lib_paths = try_option!(statement + .arguments + .iter() + .enumerate() + .map(|(index, value)| Ok(PathBuf::from(value.to_text(index, *line_no)?))) + .collect::, ParsingError>>()); + + Some(Ok(mtl_lib_paths)) + }) + .collect::, Error>>() +} + #[cfg(test)] mod tests { diff --git a/engine/src/lighting.rs b/engine/src/lighting.rs index 6944ee5..d08276e 100644 --- a/engine/src/lighting.rs +++ b/engine/src/lighting.rs @@ -36,6 +36,7 @@ pub struct GlobalLight impl GlobalLight { + #[must_use] pub fn builder() -> GlobalLightBuilder { GlobalLightBuilder::default() diff --git a/engine/src/math.rs b/engine/src/math.rs index bda3b59..b86e760 100644 --- a/engine/src/math.rs +++ b/engine/src/math.rs @@ -10,8 +10,8 @@ pub fn calc_triangle_surface_normal( edge_c: &Vec3, ) -> Vec3 { - let v1 = edge_b.clone() - egde_a.clone(); - let v2 = edge_c.clone() - egde_a.clone(); + let v1 = edge_b - egde_a; + let v2 = edge_c - egde_a; v1.cross(&v2) } diff --git a/engine/src/opengl/mod.rs b/engine/src/opengl/mod.rs index edf6372..15f40cb 100644 --- a/engine/src/opengl/mod.rs +++ b/engine/src/opengl/mod.rs @@ -13,7 +13,7 @@ mod util; #[cfg(feature = "debug")] pub mod debug; -pub fn set_viewport(position: &Vec2, size: Dimens) +pub fn set_viewport(position: Vec2, size: Dimens) { unsafe { #[allow(clippy::cast_possible_wrap)] diff --git a/engine/src/renderer/mod.rs b/engine/src/renderer/mod.rs index f9082fc..e32f308 100644 --- a/engine/src/renderer/mod.rs +++ b/engine/src/renderer/mod.rs @@ -81,10 +81,10 @@ fn initialize(window: Single) let window_size = window.size().expect("Failed to get window size"); - set_viewport(&Vec2 { x: 0, y: 0 }, window_size); + set_viewport(Vec2 { x: 0, y: 0 }, window_size); window.set_framebuffer_size_callback(|new_window_size| { - set_viewport(&Vec2::ZERO, new_window_size); + set_viewport(Vec2::ZERO, new_window_size); }); enable(Capability::DepthTest); @@ -177,7 +177,7 @@ struct GlObjects textures: HashMap, } -fn set_viewport(position: &Vec2, size: Dimens) +fn set_viewport(position: Vec2, size: Dimens) { crate::opengl::set_viewport(position, size); } diff --git a/engine/src/util.rs b/engine/src/util.rs index a7d5794..409b98f 100644 --- a/engine/src/util.rs +++ b/engine/src/util.rs @@ -60,6 +60,7 @@ macro_rules! builder { impl $builder_name { $( + #[must_use] $visibility fn $field(mut self, $field: $field_type) -> Self { self.$field = $field; @@ -67,6 +68,7 @@ macro_rules! builder { } )* + #[must_use] $visibility fn build(self) -> $name { $name { $( -- cgit v1.2.3-18-g5258