fix: random direction generation on unit sphere, not inside it

Walther · Oct 17, 2023 · 45a6867 · 45a6867
1 parent 6231979
commit 45a6867
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 22 deletions.
diff --git a/clovers/benches/random.rs b/clovers/benches/random.rs
@@ -8,7 +8,7 @@ pub fn criterion_benchmark(c: &mut Criterion) {
     let mut rng = SmallRng::from_entropy();
 
     c.bench_function("random in unit sphere", |b| {
-        b.iter(|| random_in_unit_sphere(black_box(&mut rng)))
+        b.iter(|| random_unit_vector(black_box(&mut rng)))
     });
 
     c.bench_function("random in unit disk", |b| {
@@ -25,7 +25,7 @@ pub fn criterion_benchmark(c: &mut Criterion) {
 
     let normal = Vec3::new(1.0, 0.0, 0.0);
     c.bench_function("random in hemisphere", |b| {
-        b.iter(|| random_in_hemisphere(normal, black_box(&mut rng)))
+        b.iter(|| random_on_hemisphere(normal, black_box(&mut rng)))
     });
 }
 

diff --git a/clovers/src/materials/gltf.rs b/clovers/src/materials/gltf.rs
@@ -10,7 +10,7 @@ use rand::rngs::SmallRng;
 use crate::{
     hitable::HitRecord,
     pdf::{ZeroPDF, PDF},
-    random::random_in_unit_sphere,
+    random::random_unit_vector,
     ray::Ray,
     Float, Vec2, Vec3, Vec4, PI,
 };
@@ -97,7 +97,7 @@ impl<'scene> MaterialTrait for GLTFMaterial<'scene> {
         if metalness > 0.0 {
             // TODO: borrowed from metal, should this be different?
             let reflected: Vec3 = reflect(ray.direction.normalize(), normal);
-            let direction = reflected + roughness * random_in_unit_sphere(rng);
+            let direction = reflected + roughness * random_unit_vector(rng);
 
             Some(ScatterRecord {
                 specular_ray: Some(Ray {

diff --git a/clovers/src/materials/metal.rs b/clovers/src/materials/metal.rs
@@ -4,7 +4,7 @@ use super::{reflect, MaterialTrait, MaterialType, ScatterRecord};
 use crate::{
     hitable::HitRecord,
     pdf::{ZeroPDF, PDF},
-    random::random_in_unit_sphere,
+    random::random_unit_vector,
     ray::Ray,
     textures::{Texture, TextureTrait},
     Float, Vec3,
@@ -34,7 +34,7 @@ impl MaterialTrait for Metal {
         Some(ScatterRecord {
             specular_ray: Some(Ray {
                 origin: hit_record.position,
-                direction: reflected + self.fuzz * random_in_unit_sphere(rng),
+                direction: reflected + self.fuzz * random_unit_vector(rng),
                 time: ray.time,
                 wavelength: ray.wavelength,
             }),

diff --git a/clovers/src/objects/moving_sphere.rs b/clovers/src/objects/moving_sphere.rs
@@ -5,7 +5,7 @@ use crate::{
     colors::Wavelength,
     hitable::{HitRecord, HitableTrait},
     materials::{Material, MaterialInit},
-    random::random_in_unit_sphere,
+    random::random_unit_vector,
     ray::Ray,
     Float, Vec3, PI,
 };
@@ -174,6 +174,6 @@ impl<'scene> HitableTrait for MovingSphere<'scene> {
     }
 
     fn random(&self, _origin: Vec3, rng: &mut SmallRng) -> Vec3 {
-        random_in_unit_sphere(rng)
+        random_unit_vector(rng)
     }
 }
diff --git a/clovers/src/pdf.rs b/clovers/src/pdf.rs
@@ -6,7 +6,7 @@ use crate::{
     colors::Wavelength,
     hitable::{Hitable, HitableTrait},
     onb::ONB,
-    random::{random_cosine_direction, random_in_unit_sphere},
+    random::{random_cosine_direction, random_unit_vector},
     Box, Float, Vec3, PI,
 };
 use enum_dispatch::enum_dispatch;
@@ -171,7 +171,7 @@ impl PDFTrait for SpherePDF {
 
     #[must_use]
     fn generate(&self, rng: &mut SmallRng) -> Vec3 {
-        random_in_unit_sphere(rng)
+        random_unit_vector(rng)
     }
 }
 
@@ -200,7 +200,7 @@ impl PDFTrait for ZeroPDF {
 
     #[must_use]
     fn generate(&self, rng: &mut SmallRng) -> Vec3 {
-        random_in_unit_sphere(rng)
+        random_unit_vector(rng)
     }
 }
 

diff --git a/clovers/src/random.rs b/clovers/src/random.rs
@@ -3,16 +3,9 @@
 use crate::{Float, Vec3, PI};
 use rand::rngs::SmallRng;
 use rand::Rng;
-use rand_distr::{Distribution, UnitBall, UnitDisc, UnitSphere};
+use rand_distr::{Distribution, UnitDisc, UnitSphere};
 
-/// Internal helper. Originally used for lambertian reflection with flaws
-#[must_use]
-#[inline]
-pub fn random_in_unit_sphere(rng: &mut SmallRng) -> Vec3 {
-    UnitBall.sample(rng).into()
-}
-
-/// Internal helper. Use this for the more correct "True Lambertian" reflection
+/// Internal helper.
 #[must_use]
 pub fn random_unit_vector(rng: &mut SmallRng) -> Vec3 {
     UnitSphere.sample(rng).into()
@@ -42,8 +35,8 @@ pub fn random_cosine_direction(rng: &mut SmallRng) -> Vec3 {
 
 /// Internal helper.
 #[must_use]
-pub fn random_in_hemisphere(normal: Vec3, rng: &mut SmallRng) -> Vec3 {
-    let in_unit_sphere: Vec3 = random_in_unit_sphere(rng);
+pub fn random_on_hemisphere(normal: Vec3, rng: &mut SmallRng) -> Vec3 {
+    let in_unit_sphere: Vec3 = random_unit_vector(rng);
     if in_unit_sphere.dot(&normal) > 0.0 {
         // In the same hemisphere as the normal
         in_unit_sphere