From 66d599b50dacbc346604a15be3f77c428c6259ab Mon Sep 17 00:00:00 2001 From: Bill Thiede Date: Thu, 11 Oct 2018 19:53:07 -0700 Subject: [PATCH] Fix all clippy warnings or block them. --- rtiow/src/aabb.rs | 4 +-- rtiow/src/bin/noise_explorer.rs | 53 ++++++-------------------------- rtiow/src/bvh.rs | 39 ++++++++++++----------- rtiow/src/camera.rs | 2 ++ rtiow/src/cuboid.rs | 2 ++ rtiow/src/kdtree.rs | 12 ++++---- rtiow/src/lode.rs | 2 ++ rtiow/src/perlin.rs | 5 +++ rtiow/src/rect.rs | 2 ++ rtiow/src/scenes/final_scene.rs | 2 +- rtiow/src/scenes/perlin_debug.rs | 2 +- rtiow/src/scenes/test.rs | 2 +- rtiow/src/texture.rs | 13 +++++--- 13 files changed, 60 insertions(+), 80 deletions(-) diff --git a/rtiow/src/aabb.rs b/rtiow/src/aabb.rs index 599da88..2b98667 100644 --- a/rtiow/src/aabb.rs +++ b/rtiow/src/aabb.rs @@ -52,7 +52,7 @@ impl AABB { if yd >= xd && yd >= zd { return 1; } - return 2; + 2 } pub fn mid_point(&self) -> Vec3 { @@ -153,7 +153,7 @@ impl AABB { tmax = tmax.min(t1.max(t2)); } - return tmax > tmin.max(0.0); + tmax > tmin.max(0.0) } } diff --git a/rtiow/src/bin/noise_explorer.rs b/rtiow/src/bin/noise_explorer.rs index 8722acc..7e1c4b8 100644 --- a/rtiow/src/bin/noise_explorer.rs +++ b/rtiow/src/bin/noise_explorer.rs @@ -16,8 +16,6 @@ extern crate structopt; extern crate rtiow; use actix_web::Path as WebPath; -use std::fs; -use std::path::Path; use std::path::PathBuf; use std::time::SystemTime; @@ -135,27 +133,6 @@ struct NoiseParamsMarble { } impl NoiseParams { - fn compact_string(&self) -> String { - let mut s = format!("w{}-h{}", self.width, self.height); - - match self.noise { - NoiseType::Scale(scale) => s.push_str(&format!("-s{:.2}", scale)), - - NoiseType::Turbulence(turbulence) => s.push_str(&format!("-t{}", turbulence)), - NoiseType::Marble { - period, - power, - size, - } => s.push_str(&format!( - "-m{}-{}-{}", - period.to_string().replace(" ", "-"), - power, - size - )), - } - s - } - fn url(&self) -> String { format!( "/noise/{}/{}/{}", @@ -168,15 +145,11 @@ impl NoiseParams { fn big_url(&self) -> String { format!("/noise/1024/1024/{}", self.noise.to_url()) } - - fn image_name(&self) -> String { - format!("noise-{}.png", self.compact_string()) - } } fn render_noise(noise_params: &NoiseParams) -> image::GrayImage { const SEED: [u8; 16] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]; - let ref mut rng: XorShiftRng = SeedableRng::from_seed(SEED); + let rng: &mut XorShiftRng = &mut SeedableRng::from_seed(SEED); let mut img = image::GrayImage::new(noise_params.width, noise_params.height); let tex = Noise::new(rng); @@ -210,18 +183,6 @@ fn render_noise(noise_params: &NoiseParams) -> image::GrayImage { img } -fn render_noise_to_disk

(output_dir: P, noise_params: &NoiseParams) -> Result<(), std::io::Error> -where - P: AsRef, -{ - let img = render_noise(noise_params); - let ref output_dir: &Path = output_dir.as_ref(); - fs::create_dir_all(output_dir)?; - let path = output_dir.join(noise_params.image_name()); - info!("Saving {}", path.display()); - img.save(path) -} - struct Specimen { title: String, params: Vec, @@ -366,7 +327,7 @@ fn index(_req: &HttpRequest) -> Result { .body(render_html(specimens).unwrap())) } -fn noise(np: NoiseParams) -> Result { +fn noise(np: &NoiseParams) -> Result { let img = render_noise(&np); let mut buf = Vec::new(); let img = image::DynamicImage::ImageLuma8(img); @@ -376,24 +337,28 @@ fn noise(np: NoiseParams) -> Result { Ok(HttpResponse::Ok().content_type("image/png").body(buf)) } +// Making parameter breaks WebPath::From magic. +#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] fn noise_scale(np: WebPath) -> Result { - noise(NoiseParams { + noise(&NoiseParams { width: np.width, height: np.height, noise: NoiseType::Scale(np.scale), }) } +#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] fn noise_turbulence(np: WebPath) -> Result { - noise(NoiseParams { + noise(&NoiseParams { width: np.width, height: np.height, noise: NoiseType::Turbulence(np.turbulence), }) } +#[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] fn noise_marble(np: WebPath) -> Result { - noise(NoiseParams { + noise(&NoiseParams { width: np.width, height: np.height, noise: NoiseType::Marble { diff --git a/rtiow/src/bvh.rs b/rtiow/src/bvh.rs index de2ab47..1eff02c 100644 --- a/rtiow/src/bvh.rs +++ b/rtiow/src/bvh.rs @@ -41,28 +41,32 @@ impl fmt::Display for BVHNode { } } +// Lint is wrong when dealing with Box +#[cfg_attr(feature = "cargo-clippy", allow(borrowed_box))] fn box_x_compare(ah: &Box, bh: &Box) -> std::cmp::Ordering { match (ah.bounding_box(0., 0.), bh.bounding_box(0., 0.)) { (Some(box_left), Some(box_right)) => { - return box_left.min().x.partial_cmp(&box_right.min().x).unwrap(); + box_left.min().x.partial_cmp(&box_right.min().x).unwrap() } _ => panic!("hit missing bounding box"), } } +#[cfg_attr(feature = "cargo-clippy", allow(borrowed_box))] fn box_y_compare(ah: &Box, bh: &Box) -> std::cmp::Ordering { match (ah.bounding_box(0., 0.), bh.bounding_box(0., 0.)) { (Some(box_left), Some(box_right)) => { - return box_left.min().y.partial_cmp(&box_right.min().y).unwrap(); + box_left.min().y.partial_cmp(&box_right.min().y).unwrap() } _ => panic!("hit missing bounding box"), } } +#[cfg_attr(feature = "cargo-clippy", allow(borrowed_box))] fn box_z_compare(ah: &Box, bh: &Box) -> std::cmp::Ordering { match (ah.bounding_box(0., 0.), bh.bounding_box(0., 0.)) { (Some(box_left), Some(box_right)) => { - return box_left.min().z.partial_cmp(&box_right.min().z).unwrap(); + box_left.min().z.partial_cmp(&box_right.min().z).unwrap() } _ => panic!("hit missing bounding box"), } @@ -78,10 +82,7 @@ fn vec_first_into(v: Vec) -> T { v.len() )); } - for i in v.into_iter() { - return i; - } - panic!("Unreachable"); + v.into_iter().next().unwrap() } fn vec_split_into(v: Vec, offset: usize) -> (Vec, Vec) { @@ -100,7 +101,7 @@ fn vec_split_into(v: Vec, offset: usize) -> (Vec, Vec) { impl BVHNode { fn new(l: Vec>, t_min: f32, t_max: f32) -> BVHNode { if l.len() == 1 { - return BVHNode::Leaf(vec_first_into(l)); + BVHNode::Leaf(vec_first_into(l)) } else { let mut l: Vec> = l.into_iter().collect(); let mut rng = rand::thread_rng(); @@ -108,7 +109,7 @@ impl BVHNode { 0 => l.sort_by(box_x_compare), 1 => l.sort_by(box_y_compare), 2 => l.sort_by(box_z_compare), - val @ _ => panic!("unknown axis {}", val), + val => panic!("unknown axis {}", val), } let half = l.len() / 2; @@ -116,7 +117,7 @@ impl BVHNode { let left = Box::new(BVHNode::new(left_half, t_min, t_max)); let right = Box::new(BVHNode::new(right_half, t_min, t_max)); let bbox = BVHNode::surrounding_box(left.as_ref(), right.as_ref(), t_min, t_max); - return BVHNode::Branch { left, right, bbox }; + BVHNode::Branch { left, right, bbox } } } @@ -144,7 +145,6 @@ impl Hit for BVHNode { fn hit(&self, r: Ray, t_min: f32, t_max: f32) -> Option { match self { BVHNode::Leaf(ref hit) => hit.hit(r, t_min, t_max), - // TODO(wathiede): call bbox.hit() before recursing down both paths of tree. BVHNode::Branch { ref left, ref right, @@ -154,17 +154,16 @@ impl Hit for BVHNode { if bbox.hit(r, t_min, t_max) { match (left.hit(r, t_min, t_max), right.hit(r, t_min, t_max)) { (Some(hit_left), Some(hit_right)) => if hit_left.t < hit_right.t { - return Some(hit_left); + Some(hit_left) } else { - return Some(hit_right); + Some(hit_right) }, - (Some(hit_left), None) => return Some(hit_left), - (None, Some(hit_right)) => return Some(hit_right), - (None, None) => return None, + (Some(hit_left), None) => Some(hit_left), + (None, Some(hit_right)) => Some(hit_right), + (None, None) => None, }; - } else { - return None; } + None } None => None, }, @@ -203,7 +202,7 @@ impl BVH { fn print_tree(f: &mut fmt::Formatter, depth: usize, bvhn: &BVHNode) -> fmt::Result { let vol = bvhn.volume(); - write!(f, "{:.*}{}{}\n", 2, vol, " ".repeat(depth * 2), bvhn)?; + writeln!(f, "{:.*}{}{}", 2, vol, " ".repeat(depth * 2), bvhn)?; if let BVHNode::Branch { left, right, .. } = bvhn { print_tree(f, depth + 1, left)?; print_tree(f, depth + 1, right)?; @@ -213,7 +212,7 @@ fn print_tree(f: &mut fmt::Formatter, depth: usize, bvhn: &BVHNode) -> fmt::Resu impl fmt::Display for BVH { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Root\n")?; + writeln!(f, "Root")?; print_tree(f, 1, &self.root) } } diff --git a/rtiow/src/camera.rs b/rtiow/src/camera.rs index 6e8e281..c2a22ab 100644 --- a/rtiow/src/camera.rs +++ b/rtiow/src/camera.rs @@ -35,6 +35,8 @@ pub struct Camera { } impl Camera { + // Parameters match the example C++ code. + #[cfg_attr(feature = "cargo-clippy", allow(too_many_arguments))] // vfov is top to bottom in degrees pub fn new( lookfrom: Vec3, diff --git a/rtiow/src/cuboid.rs b/rtiow/src/cuboid.rs index 9e07fff..2be78c1 100644 --- a/rtiow/src/cuboid.rs +++ b/rtiow/src/cuboid.rs @@ -19,6 +19,8 @@ pub struct Cuboid { } impl Cuboid { + // This clippy doesn't work right with Arc. + #[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] pub fn new(p_min: Vec3, p_max: Vec3, material: Arc) -> Cuboid { Cuboid { p_min, diff --git a/rtiow/src/kdtree.rs b/rtiow/src/kdtree.rs index f6be57d..73b9fd3 100644 --- a/rtiow/src/kdtree.rs +++ b/rtiow/src/kdtree.rs @@ -43,7 +43,7 @@ fn vec_split_into(v: Vec, offset: usize) -> (Vec, Vec) { impl KDTree { pub fn new(l: Vec>, t_min: f32, t_max: f32) -> KDTree { - if l.len() == 0 { + if l.is_empty() { panic!("Attempt to build k-d tree with no Hit objects"); } @@ -119,15 +119,15 @@ fn print_tree(f: &mut fmt::Formatter, depth: usize, kdt: &KDTree) -> fmt::Result let vol = kdt.volume(); write!(f, "{:8.2}{}", vol, " ".repeat(depth * 2))?; match kdt { - KDTree::Leaf(ref hit) => write!( + KDTree::Leaf(ref hit) => writeln!( f, - "Leaf: {}\n", + "Leaf: {}", hit.bounding_box(0., 0.) .map_or("NO BBOX".to_owned(), |bb| bb.to_string()) )?, - KDTree::Branch { bbox, .. } => write!( + KDTree::Branch { bbox, .. } => writeln!( f, - "Branch: {}\n", + "Branch: {}", // TODO(wathiede): removing this .clone() results in a complaint about moving out // of a borrow. bbox.clone() @@ -143,7 +143,7 @@ fn print_tree(f: &mut fmt::Formatter, depth: usize, kdt: &KDTree) -> fmt::Result impl fmt::Display for KDTree { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "kd-tree\n")?; + writeln!(f, "kd-tree")?; print_tree(f, 1, &self) } } diff --git a/rtiow/src/lode.rs b/rtiow/src/lode.rs index e931be7..06ee3e5 100644 --- a/rtiow/src/lode.rs +++ b/rtiow/src/lode.rs @@ -17,6 +17,8 @@ impl Noise { R: rand::Rng, { let mut noise = vec![vec![vec![0.; NOISE_SIZE]; NOISE_SIZE]; NOISE_SIZE]; + // Squelch warning about only being used to index, as this way reads more consistently. + #[cfg_attr(feature = "cargo-clippy", allow(needless_range_loop))] for x in 0..NOISE_SIZE { for y in 0..NOISE_SIZE { for z in 0..NOISE_SIZE { diff --git a/rtiow/src/perlin.rs b/rtiow/src/perlin.rs index 69aa5ac..7961ec6 100644 --- a/rtiow/src/perlin.rs +++ b/rtiow/src/perlin.rs @@ -1,3 +1,5 @@ +// There are many math functions in this file, so we allow single letter variable names. +#![cfg_attr(feature = "cargo-clippy", allow(many_single_char_names))] use rand::Rng; use vec3::dot; @@ -131,6 +133,9 @@ impl Perlin { let k = p.z.floor() as usize; let mut c: [[[Vec3; 2]; 2]; 2] = Default::default(); + + // Squelch warning about di only being used to index, as this way reads more consistently. + #[cfg_attr(feature = "cargo-clippy", allow(needless_range_loop))] for di in 0..2 { for dj in 0..2 { for dk in 0..2 { diff --git a/rtiow/src/rect.rs b/rtiow/src/rect.rs index 746cfe1..3476ff7 100644 --- a/rtiow/src/rect.rs +++ b/rtiow/src/rect.rs @@ -1,3 +1,5 @@ +// There are many math functions in this file, so we allow single letter variable names. +#![cfg_attr(feature = "cargo-clippy", allow(many_single_char_names))] use aabb::AABB; use hitable::Hit; use hitable::HitRecord; diff --git a/rtiow/src/scenes/final_scene.rs b/rtiow/src/scenes/final_scene.rs index 31338f1..659240e 100644 --- a/rtiow/src/scenes/final_scene.rs +++ b/rtiow/src/scenes/final_scene.rs @@ -48,7 +48,7 @@ pub fn new(opt: &Opt) -> Scene { let nb = 20; - let ref mut rng = rand::thread_rng(); + let rng = &mut rand::thread_rng(); let mut boxlist: Vec> = Vec::with_capacity(10000); let mut list: Vec> = Vec::new(); let white: Arc = Arc::new(Lambertian::new(ConstantTexture::new([0.73, 0.73, 0.73]))); diff --git a/rtiow/src/scenes/perlin_debug.rs b/rtiow/src/scenes/perlin_debug.rs index ca4b0aa..448cf2a 100644 --- a/rtiow/src/scenes/perlin_debug.rs +++ b/rtiow/src/scenes/perlin_debug.rs @@ -33,7 +33,7 @@ pub fn new(opt: &Opt) -> Scene { time_max, ); // TODO(wathiede): Use XOR rng for predictability? - let ref mut rng = rand::thread_rng(); + let rng = &mut rand::thread_rng(); let pertext: Arc = Arc::new(NoiseTexture::with_scale(rng, 10.)); let objects: Vec> = vec![ diff --git a/rtiow/src/scenes/test.rs b/rtiow/src/scenes/test.rs index df9dc24..534f2e9 100644 --- a/rtiow/src/scenes/test.rs +++ b/rtiow/src/scenes/test.rs @@ -36,7 +36,7 @@ pub fn new(opt: &Opt) -> Scene { time_min, time_max, ); - let ref mut rng = rand::thread_rng(); + let rng = &mut rand::thread_rng(); let _ground_color = if opt.use_accel { Box::new(ConstantTexture::new(Vec3::new(1.0, 0.4, 0.4))) } else { diff --git a/rtiow/src/texture.rs b/rtiow/src/texture.rs index 0a09ef5..b7f9af0 100644 --- a/rtiow/src/texture.rs +++ b/rtiow/src/texture.rs @@ -114,8 +114,8 @@ impl ImageTexture { let (w, h) = img.dimensions(); ImageTexture { img, - width: w as f32, - height: h as f32, + width: f32::from(w.min(64000) as u16), + height: f32::from(h.min(64000) as u16), } } } @@ -137,9 +137,12 @@ impl Texture for ImageTexture { u, v, x, y, self.width, self.height )); } - let p = self.img.get_pixel(x, y); - let rgb = Vec3::new(p[0] as f32 / 255., p[1] as f32 / 255., p[2] as f32 / 255.); - rgb + let pixel = self.img.get_pixel(x, y); + Vec3::new( + f32::from(pixel[0]) / 255., + f32::from(pixel[1]) / 255., + f32::from(pixel[2]) / 255., + ) } }