From a505744d801db611e35ac9e9ae300c4386f0314f Mon Sep 17 00:00:00 2001 From: Daniel Oom Date: Fri, 16 Aug 2024 23:11:02 +0200 Subject: [PATCH] Misc clippy fixes --- kdtree-cli/src/main.rs | 25 ++++++++-------- kdtree-tester-cli/src/ray_tester.rs | 2 +- kdtree-tester-cli/src/reducer.rs | 44 +++++++++++------------------ kdtree/src/format.rs | 16 +++++------ kdtree/src/lib.rs | 2 +- pathtracer-gui/src/main.rs | 3 +- pathtracer-gui/src/stage.rs | 26 ++++++++--------- pathtracer-gui/src/worker.rs | 17 +++++------ scene/src/lib.rs | 11 ++++---- tracing/src/pathtracer.rs | 6 ++-- 10 files changed, 69 insertions(+), 83 deletions(-) diff --git a/kdtree-cli/src/main.rs b/kdtree-cli/src/main.rs index 38294e38..78487d6b 100644 --- a/kdtree-cli/src/main.rs +++ b/kdtree-cli/src/main.rs @@ -49,7 +49,7 @@ fn node_cost( cost_intersect: f32, empty_factor: f32, scene_surface_area: f32, - boundary: Aabb, + boundary: &Aabb, node: &KdNode, ) -> f32 { match node { @@ -64,7 +64,7 @@ fn node_cost( cost_intersect, empty_factor, scene_surface_area, - left_aabb, + &left_aabb, left, ); let right_cost = node_cost( @@ -72,7 +72,7 @@ fn node_cost( cost_intersect, empty_factor, scene_surface_area, - right_aabb, + &right_aabb, right, ); let node_cost = cost_traverse + split_cost + left_cost + right_cost; @@ -99,7 +99,7 @@ fn tree_cost( cost_intersect, empty_factor, bounding_box.surface_area(), - bounding_box, + &bounding_box, node, ) } @@ -114,7 +114,7 @@ struct Statistics { impl Statistics { fn compute(mut vec: Vec) -> Self { - vec.sort(); + vec.sort_unstable(); let median = if vec.len() == 1 { vec[0] as f32 } else if vec.len() % 2 == 0 { @@ -169,12 +169,11 @@ fn main() { .iter() .flat_map(|chunk| { chunk.faces.iter().map(|face| { - if face.points.len() != 3 { - panic!( - "Only tringular faces supported but found {} vertices.", - face.points.len() - ); - } + assert!( + face.points.len() == 3, + "Only tringular faces supported but found {} vertices.", + face.points.len() + ); Triangle { v0: obj.index_vertex(&face.points[0]).into(), v1: obj.index_vertex(&face.points[1]).into(), @@ -211,8 +210,8 @@ fn main() { let stats = statistics(&geometries, &kdtree); eprintln!("Done..."); eprintln!("Tree statistics:"); - eprintln!(" Build time: {:.3}", duration); - eprintln!(" SAH cost: {:.3}", cost); + eprintln!(" Build time: {duration:.3}"); + eprintln!(" SAH cost: {cost:.3}"); eprintln!(" Geometries: {}", stats.geometries); eprintln!(" Node count: {}", stats.node_count); eprintln!(" Leaf count: {}", stats.leaf_count); diff --git a/kdtree-tester-cli/src/ray_tester.rs b/kdtree-tester-cli/src/ray_tester.rs index d62c66ee..da4185ea 100644 --- a/kdtree-tester-cli/src/ray_tester.rs +++ b/kdtree-tester-cli/src/ray_tester.rs @@ -57,7 +57,7 @@ pub(crate) fn kdtree_ray_tester( println!("Found {} fails", fails.len()); if let Some(path) = output { - println!("Writing failed rays to {:?}...", path); + println!("Writing failed rays to {path:?}..."); let mut logger = BufWriter::new(File::create(path).unwrap()); fails.iter().enumerate().for_each(|(i, fail)| { logger.write_all(&fail.as_bytes(i as u16)).unwrap(); diff --git a/kdtree-tester-cli/src/reducer.rs b/kdtree-tester-cli/src/reducer.rs index 5202d4b6..a338a0aa 100644 --- a/kdtree-tester-cli/src/reducer.rs +++ b/kdtree-tester-cli/src/reducer.rs @@ -61,13 +61,10 @@ fn reduce_tree( geometries[2..].shuffle(&mut SmallRng::seed_from_u64(seed)); let mut try_index: usize = 2; let mut try_count = geometries.len() - try_index; - eprintln!( - "Kept {} with {} geometries left to check.", - try_index, try_count - ); + eprintln!("Kept {try_index} with {try_count} geometries left to check."); while try_index < geometries.len() { try_count = try_count.clamp(1, geometries.len() - try_index); - eprint!(" Trying to remove {: <5}", try_count); + eprint!(" Trying to remove {try_count: <5}"); let time_before = Instant::now(); let reduced = try_removing( &intersection.ray, @@ -76,26 +73,20 @@ fn reduce_tree( try_index, try_count, ); - let duration = Instant::now().duration_since(time_before).as_micros() as f64 / 1000.0; + let duration = Instant::now().duration_since(time_before).as_secs_f64(); if let Some(reduced) = reduced { geometries = reduced; try_count = geometries.len() - try_index; - eprintln!(" Time: {: <8.3} ms. Success!", duration); - eprintln!( - "Kept {} with {} geometries left to check.", - try_index, try_count, - ); + eprintln!(" Time: {duration: <8.3} ms. Success!"); + eprintln!("Kept {try_index} with {try_count} geometries left to check."); } else if try_count > 1 { try_count /= 2; - eprintln!(" Time: {: <8.3} ms. Fail!", duration); + eprintln!(" Time: {duration: <8.3} ms. Fail!"); } else { try_index += 1; try_count = geometries.len() - try_index; - eprintln!(" Time: {: <8.3} ms. Fail! Keeping 1 geometry.", duration); - eprintln!( - "Kept {} with {} geometries left to check.", - try_index, try_count - ); + eprintln!(" Time: {duration: <8.3} ms. Fail! Keeping 1 geometry."); + eprintln!("Kept {try_index} with {try_count} geometries left to check."); } } let tree = build_test_tree(&geometries); @@ -125,14 +116,14 @@ pub(crate) fn kdtree_reduce(input: PathBuf, output: PathBuf, fail: Option(write: &mut W, path: String, node: &KdNode) -> Result<(), io::Error> +pub fn write_node_dot(write: &mut W, path: &str, node: &KdNode) -> Result<(), io::Error> where W: io::Write, { match node { KdNode::Leaf(indices) => { - let formatted = format!("{:?}", indices); + let formatted = format!("{indices:?}"); let wrapped = textwrap::fill(formatted.as_str(), 60); - writeln!(write, " {} [label={:?}];", path, wrapped)?; + writeln!(write, " {path} [label={wrapped:?}];")?; } KdNode::Node { plane, left, right } => { writeln!( @@ -145,12 +145,12 @@ where " {} [label=\"{:?} {}\"];", path, plane.axis, plane.distance )?; - let left_path = path.clone() + "l"; - let right_path = path.clone() + "r"; + let left_path = path.to_string() + "l"; + let right_path = path.to_string() + "r"; writeln!(write, " {} -> {};", &path, left_path)?; writeln!(write, " {} -> {};", &path, right_path)?; - write_node_dot(write, left_path, left)?; - write_node_dot(write, right_path, right)?; + write_node_dot(write, &left_path, left)?; + write_node_dot(write, &right_path, right)?; } } Ok(()) @@ -163,7 +163,7 @@ where writeln!(write, "digraph {{")?; writeln!(write, " rankdir=\"LR\";")?; writeln!(write, " node [shape=\"box\"];")?; - write_node_dot(write, "t".to_string(), tree)?; + write_node_dot(write, "t", tree)?; writeln!(write, "}}")?; Ok(()) } diff --git a/kdtree/src/lib.rs b/kdtree/src/lib.rs index 297ae518..6e018f37 100644 --- a/kdtree/src/lib.rs +++ b/kdtree/src/lib.rs @@ -94,7 +94,7 @@ impl KdNode { KdNode::Leaf(indices) => { match intersect_closest_geometry( geometries, - indices.iter().cloned(), + indices.iter().copied(), ray, t1..=t2, ) { diff --git a/pathtracer-gui/src/main.rs b/pathtracer-gui/src/main.rs index 89fa36c7..c1ae0336 100644 --- a/pathtracer-gui/src/main.rs +++ b/pathtracer-gui/src/main.rs @@ -1,5 +1,6 @@ use clap::Parser; use kdtree::{build::build_kdtree, sah::SahCost}; +use miniquad::conf::Conf; use scene::Scene; use stage::Stage; use tracing::pathtracer::Pathtracer; @@ -43,7 +44,7 @@ fn main() { kdtree, }; - miniquad::start(Default::default(), move || { + miniquad::start(Conf::default(), move || { let camera = pathtracer.scene.cameras[0].clone(); Box::new(Stage::new(pathtracer, camera)) }); diff --git a/pathtracer-gui/src/stage.rs b/pathtracer-gui/src/stage.rs index 7df43227..e08e49ec 100644 --- a/pathtracer-gui/src/stage.rs +++ b/pathtracer-gui/src/stage.rs @@ -3,8 +3,8 @@ use std::time::Instant; use glam::{Mat3, UVec2, Vec3}; use miniquad::{ Bindings, BufferLayout, BufferSource, BufferType, BufferUsage, EventHandler, GlContext, - KeyCode, Pipeline, PipelineParams, RenderingBackend, ShaderSource, TextureId, VertexAttribute, - VertexFormat, + KeyCode, PassAction, Pipeline, PipelineParams, RenderingBackend, ShaderSource, TextureId, + VertexAttribute, VertexFormat, }; use scene::camera::{Camera, Pinhole}; use tracing::pathtracer::Pathtracer; @@ -159,13 +159,16 @@ impl Stage { } fn update_texture(&mut self) { - while let Some(result) = self.worker.as_ref().and_then(|worker| worker.try_receive()) { + while let Some(result) = self.worker.as_ref().and_then(Worker::try_receive) { eprintln!( "Received {:?} @ {} rendered in {:?}.", result.buffer.size, result.iterations, result.duration, ); let texture_size = self.ctx.texture_size(self.texture).into(); - if result.buffer.size != texture_size { + if result.buffer.size == texture_size { + self.ctx + .texture_update(self.texture, &result.buffer.to_rgb8(result.iterations)); + } else { self.ctx.delete_texture(self.texture); let width = result.buffer.size.x; let height = result.buffer.size.y; @@ -179,9 +182,6 @@ impl Stage { }, ); self.bindings.images = vec![self.texture]; - } else { - self.ctx - .texture_update(self.texture, &result.buffer.to_rgb8(result.iterations)) } } } @@ -230,7 +230,7 @@ impl EventHandler for Stage { } fn draw(&mut self) { - self.ctx.begin_default_pass(Default::default()); + self.ctx.begin_default_pass(PassAction::default()); self.ctx.apply_pipeline(&self.pipeline); self.ctx.apply_bindings(&self.bindings); @@ -248,9 +248,9 @@ impl EventHandler for Stage { } mod shader { - use miniquad::*; + use miniquad::{ShaderMeta, UniformBlockLayout}; - pub const VERTEX: &str = r#"#version 100 + pub const VERTEX: &str = r"#version 100 attribute vec2 in_pos; attribute vec2 in_uv; @@ -259,16 +259,16 @@ mod shader { void main() { gl_Position = vec4(in_pos, 0, 1); texcoord = in_uv; - }"#; + }"; - pub const FRAGMENT: &str = r#"#version 100 + pub const FRAGMENT: &str = r"#version 100 varying lowp vec2 texcoord; uniform sampler2D tex; void main() { gl_FragColor = texture2D(tex, texcoord); - }"#; + }"; pub fn meta() -> ShaderMeta { ShaderMeta { diff --git a/pathtracer-gui/src/worker.rs b/pathtracer-gui/src/worker.rs index 174964df..7860eea3 100644 --- a/pathtracer-gui/src/worker.rs +++ b/pathtracer-gui/src/worker.rs @@ -1,9 +1,6 @@ use std::{ ops::Add, - sync::{ - mpsc::{self, Receiver, Sender}, - Arc, - }, + sync::mpsc::{self, Receiver, Sender}, thread::JoinHandle, time, }; @@ -64,10 +61,10 @@ fn render_subdivided(pathtracer: &Pathtracer, pinhole: &Pinhole, sub_size: UVec2 } fn worker_loop( - pathtracer: Arc, + pathtracer: &Pathtracer, start_pinhole: Pinhole, - rx: Receiver, - tx: Sender, + rx: &Receiver, + tx: &Sender, ) { let mut pinhole = start_pinhole; let mut iteration = 0; @@ -90,12 +87,12 @@ fn worker_loop( UVec2::new(64, 64 * pinhole.size.y / pinhole.size.x), ); let (duration, buffer) = - measure(|| render_subdivided(&pathtracer, &pinhole_small, pinhole_small.size / 3)); + measure(|| render_subdivided(pathtracer, &pinhole_small, pinhole_small.size / 3)); let _ = tx.send(RenderResult::new(1, duration, buffer)); iteration = 1; } else { let (duration, buffer) = - measure(|| render_subdivided(&pathtracer, &pinhole, pinhole.size / 4)); + measure(|| render_subdivided(pathtracer, &pinhole, pinhole.size / 4)); combined_buffer += buffer; let _ = tx.send(RenderResult::new( iteration, @@ -119,7 +116,7 @@ impl Worker { let (pinhole_tx, pinhole_rx) = mpsc::channel::(); let thread = std::thread::Builder::new() .name("Pathtracer Worker".to_string()) - .spawn(move || worker_loop(Arc::new(pathtracer), pinhole, pinhole_rx, result_tx)) + .spawn(move || worker_loop(&pathtracer, pinhole, &pinhole_rx, &result_tx)) .unwrap(); Self { thread, diff --git a/scene/src/lib.rs b/scene/src/lib.rs index f423e6ab..b330dd02 100644 --- a/scene/src/lib.rs +++ b/scene/src/lib.rs @@ -114,12 +114,11 @@ impl Scene { .iter() .flat_map(|chunk| { chunk.faces.iter().map(|face| { - if face.points.len() != 3 { - panic!( - "Only tringular faces supported but found {} vertices.", - face.points.len() - ); - } + assert!( + face.points.len() == 3, + "Only tringular faces supported but found {} vertices.", + face.points.len() + ); let geometry = Geometry::from(Triangle { v0: obj.index_vertex(&face.points[0]).into(), v1: obj.index_vertex(&face.points[1]).into(), diff --git a/tracing/src/pathtracer.rs b/tracing/src/pathtracer.rs index d2d0c670..c40c9673 100644 --- a/tracing/src/pathtracer.rs +++ b/tracing/src/pathtracer.rs @@ -76,7 +76,7 @@ impl Pathtracer { let radiance = light.emitted(point); let brdf = material_brdf( self.scene.get_material(intersection_index), - &OutgoingRay { wi, n, wo, uv }, + &OutgoingRay { wi, n, uv, wo }, ); brdf * radiance * wo.dot(n).abs() }) @@ -136,7 +136,7 @@ impl Pathtracer { ) } - fn sample_ray_for_pixel(&self, pinhole: &Pinhole, rng: &mut SmallRng, pixel: UVec2) -> Ray { + fn sample_ray_for_pixel(pinhole: &Pinhole, rng: &mut SmallRng, pixel: UVec2) -> Ray { debug_assert!(pixel.x < pinhole.size.x && pixel.y < pinhole.size.y); let pixel_center = pixel.as_vec2() + uniform_sample_unit_square(rng); pinhole.ray(pixel_center / pinhole.size.as_vec2()) @@ -150,7 +150,7 @@ impl Pathtracer { rng: &mut SmallRng, ) -> Vec3 { let ray_logger = ray_logger.with_pixel(pixel.x as u16, pixel.y as u16); - let ray = self.sample_ray_for_pixel(pinhole, rng, pixel); + let ray = Self::sample_ray_for_pixel(pinhole, rng, pixel); self.trace_ray_defaults(ray_logger, rng, &ray) }