From 0296fd2d348f2109f17973914406deb9125c1763 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 14 Apr 2017 21:37:29 -0700 Subject: [PATCH] Fixed a bunch of problems that came up when running debug build. The worst offender was an obvious and nasty bug in the bitstack implementation. Hooray for debug asserts! --- src/bitstack.rs | 25 +++++++++++++++---------- src/renderer.rs | 12 ++++++++++-- src/shading/surface_closure.rs | 4 +++- src/surface/triangle_mesh.rs | 2 +- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/bitstack.rs b/src/bitstack.rs index 0d4f95e..b85d167 100644 --- a/src/bitstack.rs +++ b/src/bitstack.rs @@ -18,8 +18,10 @@ impl BitStack128 { /// Push a bit onto the top of the stack. pub fn push(&mut self, value: bool) { - debug_assert!((self.data.1 >> (size_of::() - 1)) == 0); // Verify no stack overflow - self.data.1 = (self.data.1 << 1) | (self.data.0 >> (size_of::() - 1)); + // Verify no stack overflow + debug_assert!((self.data.1 >> ((size_of::() * 8) - 1)) == 0); + + self.data.1 = (self.data.1 << 1) | (self.data.0 >> ((size_of::() * 8) - 1)); self.data.0 <<= 1; self.data.0 |= value as u64; } @@ -35,12 +37,12 @@ impl BitStack128 { /// bits are non-zero this will produce incorrect results. pub fn push_n(&mut self, value: u8, count: u8) { // Verify no bitstack overflow - debug_assert!((self.data.1 >> (size_of::() - count as usize)) == 0); + debug_assert!((self.data.1 >> ((size_of::() * 8) - count as usize)) == 0); // Verify no bits outside of the n-bit range debug_assert!(value & (!((1 << count) - 1)) == 0); self.data.1 = (self.data.1 << count as usize) | - (self.data.0 >> (size_of::() - count as usize)); + (self.data.0 >> ((size_of::() * 8) - count as usize)); self.data.0 <<= count as u64; self.data.0 |= value as u64; } @@ -48,7 +50,7 @@ impl BitStack128 { /// Pop the top bit off the stack. pub fn pop(&mut self) -> bool { let b = (self.data.0 & 1) != 0; - self.data.0 = (self.data.0 >> 1) | (self.data.1 << (size_of::() - 1)); + self.data.0 = (self.data.0 >> 1) | (self.data.1 << ((size_of::() * 8) - 1)); self.data.1 >>= 1; return b; } @@ -57,10 +59,10 @@ impl BitStack128 { /// an integer, with the top bit in the least significant digit, /// and the rest following in order from there. pub fn pop_n(&mut self, n: usize) -> u64 { - debug_assert!(n < size_of::()); // Can't pop more than we have - debug_assert!(n < size_of::()); // Can't pop more than the return type can hold + debug_assert!(n < (size_of::() * 8)); // Can't pop more than we have + debug_assert!(n < (size_of::() * 8)); // Can't pop more than the return type can hold let b = self.data.0 & ((1 << n) - 1); - self.data.0 = (self.data.0 >> n) | (self.data.1 << (size_of::() - n)); + self.data.0 = (self.data.0 >> n) | (self.data.1 << ((size_of::() * 8) - n)); self.data.1 >>= n; return b; } @@ -74,8 +76,11 @@ impl BitStack128 { /// are returned as an integer, with the top bit in the least /// significant digit, and the rest following in order from there. pub fn peek_n(&self, n: usize) -> u64 { - debug_assert!(n < size_of::()); // Can't return more than we have - debug_assert!(n < size_of::()); // Can't return more than the return type can hold + // Can't return more than we have + debug_assert!(n < (size_of::() * 8)); + // Can't return more than the return type can hold + debug_assert!(n < (size_of::() * 8)); + self.data.0 & ((1 << n) - 1) } } diff --git a/src/renderer.rs b/src/renderer.rs index 938a1aa..3b373ff 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -202,8 +202,16 @@ impl<'a> Renderer<'a> { let x = bx as usize * bucket_w; let y = by as usize * bucket_h; - let w = min(bucket_w, img_width - x); - let h = min(bucket_h, img_height - y); + let w = if img_width >= x { + min(bucket_w, img_width - x) + } else { + bucket_w + }; + let h = if img_height >= y { + min(bucket_h, img_height - y) + } else { + bucket_h + }; if x < img_width && y < img_height && w > 0 && h > 0 { job_queue.push(BucketJob { x: x as u32, diff --git a/src/shading/surface_closure.rs b/src/shading/surface_closure.rs index 3109acb..851f0ca 100644 --- a/src/shading/surface_closure.rs +++ b/src/shading/surface_closure.rs @@ -342,7 +342,7 @@ pub struct GTRClosure { col: XYZ, roughness: f32, tail_shape: f32, - fresnel: f32, + fresnel: f32, // [0.0, 1.0] determines how much fresnel reflection comes into play normalization_factor: f32, } @@ -372,6 +372,8 @@ impl GTRClosure { // Makes sure values are in a valid range fn validate(&mut self) { + debug_assert!(self.fresnel >= 0.0 && self.fresnel <= 1.0); + // Clamp values to valid ranges self.roughness = clamp(self.roughness, 0.0, 0.9999); self.tail_shape = (0.0001f32).max(self.tail_shape); diff --git a/src/surface/triangle_mesh.rs b/src/surface/triangle_mesh.rs index eae0995..73a27f5 100644 --- a/src/surface/triangle_mesh.rs +++ b/src/surface/triangle_mesh.rs @@ -115,7 +115,7 @@ impl<'a> Surface for TriangleMesh<'a> { 0.8), 0.1, 2.0, - 1.2)), + 1.0)), }; r.max_t = t; }