Changed SurfaceLight API to return the sample point on the light.

More specifically: prior to this, SurfaceLights returned the
shadow ray direction vector to use.  That was fine, but it
kept the responsibility of generating proper offsets (to account
for floating point error) inside the lights.

Now the SurfaceLights return the world-space point on the light
to sample, along with its surface normal and error magnitude.
This allows the robust shadow ray generation code to be in one
place inside the renderer code.

src/light/mod.rs

@@ -5,7 +5,7 @@ mod sphere_light;
 use std::fmt::Debug;
 
 use color::SpectralSample;
-use math::{Vector, Point, Matrix4x4};
+use math::{Vector, Normal, Point, Matrix4x4};
 use surface::Surface;
 
 pub use self::distant_disk_light::DistantDiskLight;
@@ -24,8 +24,12 @@ pub trait SurfaceLight: Surface {
     /// - `wavelength`: The wavelength of light to sample at.
     /// - `time`: The time to sample at.
     ///
-    /// Returns: The light arriving at the point arr, the vector to use for
+    /// Returns:
-    /// shadow testing, and the pdf of the sample.
+    /// - The light arriving at the point arr.
+    /// - A tuple with the sample point on the light, the surface normal at
+    ///   that point, and the point's error magnitude.  These are used
+    ///   elsewhere to create a robust shadow ray.
+    /// - The pdf of the sample.
     fn sample_from_point(
         &self,
         space: &Matrix4x4,
@@ -34,7 +38,7 @@ pub trait SurfaceLight: Surface {
         v: f32,
         wavelength: f32,
         time: f32,
-    ) -> (SpectralSample, Vector, f32);
+    ) -> (SpectralSample, (Point, Normal, f32), f32);
 
 
     /// Returns whether the light has a delta distribution.

src/light/rectangle_light.rs

@@ -4,7 +4,7 @@ use bbox::BBox;
 use boundable::Boundable;
 use color::{XYZ, SpectralSample, Color};
 use lerp::lerp_slice;
-use math::{Vector, Point, Matrix4x4};
+use math::{Vector, Normal, Point, Matrix4x4};
 use ray::{Ray, AccelRay};
 use sampling::{spherical_triangle_solid_angle, uniform_sample_spherical_triangle};
 use shading::SurfaceShader;
@@ -109,7 +109,7 @@ impl<'a> SurfaceLight for RectangleLight<'a> {
         v: f32,
         wavelength: f32,
         time: f32,
-    ) -> (SpectralSample, Vector, f32) {
+    ) -> (SpectralSample, (Point, Normal, f32), f32) {
         // Calculate time interpolated values
         let dim = lerp_slice(self.dimensions, time);
         let col = lerp_slice(self.colors, time);
@@ -159,13 +159,18 @@ impl<'a> SurfaceLight for RectangleLight<'a> {
             sample_point_local.set_z(0.0);
         }
         let sample_point = sample_point_local * space_inv;
-        let shadow_vec = sample_point - arr;
+        let normal = Normal::new(0.0, 0.0, 1.0) * space_inv;
+        let point_err = 0.0001; // TODO: this is a hack, do properly.
 
         // Calculate pdf and light energy
         let pdf = 1.0 / (area_1 + area_2); // PDF of the ray direction being sampled
         let spectral_sample = (col * surface_area_inv as f32 * 0.5).to_spectral_sample(wavelength);
 
-        (spectral_sample, shadow_vec, pdf as f32)
+        (
+            spectral_sample,
+            (sample_point, normal, point_err),
+            pdf as f32,
+        )
     }
 
     fn is_delta(&self) -> bool {

src/light/sphere_light.rs

@@ -6,7 +6,7 @@ use bbox::BBox;
 use boundable::Boundable;
 use color::{XYZ, SpectralSample, Color};
 use lerp::lerp_slice;
-use math::{Vector, Point, Matrix4x4, dot, coordinate_system_from_vector};
+use math::{Vector, Normal, Point, Matrix4x4, dot, coordinate_system_from_vector};
 use ray::{Ray, AccelRay};
 use sampling::{uniform_sample_cone, uniform_sample_cone_pdf, uniform_sample_sphere};
 use shading::surface_closure::{SurfaceClosureUnion, EmitClosure};
@@ -17,8 +17,7 @@ use super::SurfaceLight;
 
 // TODO: use proper error bounds for sample generation to avoid self-shadowing
 // instead of these fudge factors.
-const SAMPLE_RADIUS_EXPAND_FACTOR: f32 = 1.001;
+const SAMPLE_POINT_FUDGE: f32 = 0.001;
-const SAMPLE_RADIUS_SHRINK_FACTOR: f32 = 0.99;
 
 // TODO: handle case where radius = 0.0.
 
@@ -92,11 +91,14 @@ impl<'a> SurfaceLight for SphereLight<'a> {
         v: f32,
         wavelength: f32,
         time: f32,
-    ) -> (SpectralSample, Vector, f32) {
+    ) -> (SpectralSample, (Point, Normal, f32), f32) {
         // TODO: track fp error due to transforms
         let arr = arr * *space;
         let pos = Point::new(0.0, 0.0, 0.0);
 
+        // Precalculate local->world space transform matrix
+        let inv_space = space.inverse();
+
         // Calculate time interpolated values
         let radius: f64 = lerp_slice(self.radii, time) as f64;
         let col = lerp_slice(self.colors, time);
@@ -110,6 +112,14 @@ impl<'a> SurfaceLight for SphereLight<'a> {
         let (z, x, y) = coordinate_system_from_vector(z);
         let (x, y, z) = (x.normalized(), y.normalized(), z.normalized());
 
+        // Pre-calculate sample point error magnitude.
+        // TODO: do this properly.  This is a total hack.
+        let sample_point_err = {
+            let v = Vector::new(radius as f32, radius as f32, radius as f32);
+            let v2 = v * inv_space;
+            v2.length() * SAMPLE_POINT_FUDGE
+        };
+
         // If we're outside the sphere, sample the surface based on
         // the angle it subtends from the point being lit.
         if d > radius {
@@ -145,26 +155,40 @@ impl<'a> SurfaceLight for SphereLight<'a> {
             );
 
             // Calculate the final values and return everything.
-            let shadow_vec = {
+            let (sample_point, normal) = {
                 let sample_vec = (x * sample.x()) + (y * sample.y()) + (z * sample.z());
-                let sample_point = (arr + sample_vec).into_vector().normalized() * radius as f32;
+                let normal = (arr + sample_vec).into_vector().normalized();
-                let adjusted_sample_point = sample_point * SAMPLE_RADIUS_EXPAND_FACTOR;
+                let point = normal * radius as f32;
-                (adjusted_sample_point.into_point() - arr) * space.inverse()
+                (
+                    point.into_point() * inv_space,
+                    normal.into_normal() * inv_space,
+                )
             };
             let pdf = uniform_sample_cone_pdf(cos_theta_max);
             let spectral_sample = (col * surface_area_inv as f32).to_spectral_sample(wavelength);
-            return (spectral_sample, shadow_vec, pdf as f32);
+            return (
+                spectral_sample,
+                (sample_point, normal, sample_point_err),
+                pdf as f32,
+            );
         } else {
             // If we're inside the sphere, there's light from every direction.
-            let shadow_vec = {
+            let (sample_point, normal) = {
                 let sample_vec = uniform_sample_sphere(u, v);
-                let sample_point = (arr + sample_vec).into_vector().normalized() * radius as f32;
+                let normal = (arr + sample_vec).into_vector().normalized();
-                let adjusted_sample_point = sample_point * SAMPLE_RADIUS_SHRINK_FACTOR;
+                let point = normal * radius as f32;
-                (adjusted_sample_point.into_point() - arr) * space.inverse()
+                (
+                    point.into_point() * inv_space,
+                    normal.into_normal() * inv_space,
+                )
             };
             let pdf = 1.0 / (4.0 * PI_64);
             let spectral_sample = (col * surface_area_inv as f32).to_spectral_sample(wavelength);
-            return (spectral_sample, shadow_vec, pdf as f32);
+            return (
+                spectral_sample,
+                (sample_point, normal, sample_point_err),
+                pdf as f32,
+            );
         }
     }
 

src/renderer.rs

@@ -21,7 +21,7 @@ use image::Image;
 use math::{fast_logit, upper_power_of_two};
 use mis::power_heuristic;
 use ray::Ray;
-use scene::Scene;
+use scene::{Scene, SceneLightSample};
 use surface;
 use timer::Timer;
 use tracer::Tracer;
@@ -463,7 +463,7 @@ impl LightPath {
                         return false;
                     }
 
-                    // Roll the previous closure pdf into the attentuation
+                    // Roll the previous closure pdf into the attenauation
                     self.light_attenuation /= self.closure_sample_pdf;
 
                     // Prepare light ray
@@ -474,72 +474,104 @@ impl LightPath {
                         self.next_lds_samp(),
                     );
                     xform_stack.clear();
-                    let found_light = if let Some((light_color,
+                    let light_info = scene.sample_lights(
-                                                   shadow_vec,
-                                                   light_pdf,
-                                                   light_sel_pdf,
-                                                   is_infinite)) =
-                        scene.sample_lights(
                         xform_stack,
                         light_n,
                         light_uvw,
                         self.wavelength,
                         self.time,
                         isect,
-                        )
-                    {
-                        // Check if pdf is zero, to avoid NaN's.
-                        if light_pdf > 0.0 {
-                            let material = closure.as_surface_closure();
-                            let attenuation =
-                                material.evaluate(ray.dir, shadow_vec, idata.nor, idata.nor_g);
-
-                            if attenuation.e.h_max() > 0.0 {
-                                // Calculate MIS
-                                let closure_pdf = material.sample_pdf(
-                                    ray.dir,
-                                    shadow_vec,
-                                    idata.nor,
-                                    idata.nor_g,
                     );
-                                let light_mis_pdf = power_heuristic(light_pdf, closure_pdf);
+                    let found_light = if light_info.is_none() || light_info.pdf() <= 0.0 ||
+                        light_info.selection_pdf() <= 0.0
+                    {
+                        false
+                    } else {
+                        let light_pdf = light_info.pdf();
+                        let light_sel_pdf = light_info.selection_pdf();
+                        let material = closure.as_surface_closure();
 
-                                // Calculate and store the light that will be contributed
+                        // Calculate the shadow ray and surface closure stuff
-                                // to the film plane if the light is not in shadow.
+                        let (attenuation, closure_pdf, shadow_ray) = match light_info {
-                                self.pending_color_addition = light_color.e * attenuation.e *
+                            SceneLightSample::None => unreachable!(),
-                                    self.light_attenuation /
-                                    (light_mis_pdf * light_sel_pdf);
 
+                            // Distant light
+                            SceneLightSample::Distant { direction, .. } => {
+                                let attenuation =
+                                    material.evaluate(ray.dir, direction, idata.nor, idata.nor_g);
+                                let closure_pdf =
+                                    material.sample_pdf(ray.dir, direction, idata.nor, idata.nor_g);
+                                let mut shadow_ray = {
                                     // Calculate the shadow ray for testing if the light is
                                     // in shadow or not.
                                     let offset_pos = robust_ray_origin(
                                         idata.pos,
                                         idata.pos_err,
                                         idata.nor_g.normalized(),
-                                    shadow_vec,
+                                        direction,
                                     );
-                                *ray = Ray::new(
+                                    Ray::new(
                                         offset_pos,
-                                    shadow_vec,
+                                        direction,
                                         self.time,
                                         self.wavelength,
                                         true,
-                                );
+                                    )
-
+                                };
-                                // For distant lights
+                                shadow_ray.max_t = std::f32::INFINITY;
-                                if is_infinite {
+                                (attenuation, closure_pdf, shadow_ray)
-                                    ray.max_t = std::f32::INFINITY;
                             }
 
+                            // Surface light
+                            SceneLightSample::Surface { sample_geo, .. } => {
+                                let dir = sample_geo.0 - idata.pos;
+                                let attenuation =
+                                    material.evaluate(ray.dir, dir, idata.nor, idata.nor_g);
+                                let closure_pdf =
+                                    material.sample_pdf(ray.dir, dir, idata.nor, idata.nor_g);
+                                let shadow_ray = {
+                                    // Calculate the shadow ray for testing if the light is
+                                    // in shadow or not.
+                                    let offset_pos = robust_ray_origin(
+                                        idata.pos,
+                                        idata.pos_err,
+                                        idata.nor_g.normalized(),
+                                        dir,
+                                    );
+                                    let offset_end = robust_ray_origin(
+                                        sample_geo.0,
+                                        sample_geo.2,
+                                        sample_geo.1.normalized(),
+                                        -dir,
+                                    );
+                                    Ray::new(
+                                        offset_pos,
+                                        offset_end - offset_pos,
+                                        self.time,
+                                        self.wavelength,
+                                        true,
+                                    )
+                                };
+                                (attenuation, closure_pdf, shadow_ray)
+                            }
+                        };
+
+                        // If there's any possible contribution, set up for a
+                        // light ray.
+                        if attenuation.e.h_max() <= 0.0 {
+                            false
+                        } else {
+                            // Calculate and store the light that will be contributed
+                            // to the film plane if the light is not in shadow.
+                            let light_mis_pdf = power_heuristic(light_pdf, closure_pdf);
+                            self.pending_color_addition = light_info.color().e * attenuation.e *
+                                self.light_attenuation /
+                                (light_mis_pdf * light_sel_pdf);
+
+                            *ray = shadow_ray;
+
                             true
-                            } else {
-                                false
                         }
-                        } else {
-                            false
-                        }
-                    } else {
-                        false
                     };
 
                     // Prepare bounce ray

src/scene/assembly.rs

@@ -9,7 +9,7 @@ use boundable::Boundable;
 use color::SpectralSample;
 use lerp::lerp_slice;
 use light::SurfaceLight;
-use math::{Matrix4x4, Vector};
+use math::{Matrix4x4, Normal, Point};
 use surface::{Surface, SurfaceIntersection};
 use shading::SurfaceShader;
 use transform_stack::TransformStack;
@@ -39,7 +39,7 @@ pub struct Assembly<'a> {
 }
 
 impl<'a> Assembly<'a> {
-    // Returns (light_color, shadow_vector, pdf, selection_pdf)
+    // Returns (light_color, (sample_point, normal, point_err), pdf, selection_pdf)
     pub fn sample_lights(
         &self,
         xform_stack: &mut TransformStack,
@@ -48,7 +48,7 @@ impl<'a> Assembly<'a> {
         wavelength: f32,
         time: f32,
         intr: &SurfaceIntersection,
-    ) -> Option<(SpectralSample, Vector, f32, f32)> {
+    ) -> Option<(SpectralSample, (Point, Normal, f32), f32, f32)> {
         if let SurfaceIntersection::Hit {
             intersection_data: idata,
             closure,
@@ -95,7 +95,7 @@ impl<'a> Assembly<'a> {
                                 };
 
                                 // Sample the light
-                                let (color, shadow_vec, pdf) = light.sample_from_point(
+                                let (color, sample_geo, pdf) = light.sample_from_point(
                                     &xform,
                                     idata.pos,
                                     uvw.0,
@@ -103,7 +103,7 @@ impl<'a> Assembly<'a> {
                                     wavelength,
                                     time,
                                 );
-                                return Some((color, shadow_vec, pdf, sel_pdf));
+                                return Some((color, sample_geo, pdf, sel_pdf));
                             }
 
                             _ => unimplemented!(),

src/scene/mod.rs

@@ -3,5 +3,5 @@ mod scene;
 mod world;
 
 pub use self::assembly::{Assembly, AssemblyBuilder, Object, InstanceType};
-pub use self::scene::Scene;
+pub use self::scene::{Scene, SceneLightSample};
 pub use self::world::World;

src/scene/scene.rs

@@ -2,7 +2,7 @@ use accel::LightAccel;
 use algorithm::weighted_choice;
 use camera::Camera;
 use color::SpectralSample;
-use math::Vector;
+use math::{Vector, Normal, Point};
 use surface::SurfaceIntersection;
 use transform_stack::TransformStack;
 
@@ -27,7 +27,7 @@ impl<'a> Scene<'a> {
         wavelength: f32,
         time: f32,
         intr: &SurfaceIntersection,
-    ) -> Option<(SpectralSample, Vector, f32, f32, bool)> {
+    ) -> SceneLightSample {
         // TODO: this just selects between world lights and local lights
         // with a 50/50 chance.  We should do something more sophisticated
         // than this, accounting for the estimated impact of the lights
@@ -52,7 +52,7 @@ impl<'a> Scene<'a> {
 
         // Decide either world or local lights, and select and sample a light.
         if tot_energy <= 0.0 {
-            return None;
+            return SceneLightSample::None;
         } else {
             let wl_prob = wl_energy / tot_energy;
 
@@ -62,12 +62,17 @@ impl<'a> Scene<'a> {
                 let (i, p) = weighted_choice(self.world.lights, n, |l| l.approximate_energy());
                 let (ss, sv, pdf) =
                     self.world.lights[i].sample_from_point(uvw.0, uvw.1, wavelength, time);
-                return Some((ss, sv, pdf, p * wl_prob, true));
+                return SceneLightSample::Distant {
+                    color: ss,
+                    direction: sv,
+                    pdf: pdf,
+                    selection_pdf: p * wl_prob,
+                };
             } else {
                 // Local lights
                 let n = (n - wl_prob) / (1.0 - wl_prob);
 
-                if let Some((ss, sv, pdf, spdf)) =
+                if let Some((ss, sgeo, pdf, spdf)) =
                     self.root.sample_lights(
                         xform_stack,
                         n,
@@ -77,11 +82,68 @@ impl<'a> Scene<'a> {
                         intr,
                     )
                 {
-                    return Some((ss, sv, pdf, spdf * (1.0 - wl_prob), false));
+                    return SceneLightSample::Surface {
+                        color: ss,
+                        sample_geo: sgeo,
+                        pdf: pdf,
+                        selection_pdf: spdf * (1.0 - wl_prob),
+                    };
                 } else {
-                    return None;
+                    return SceneLightSample::None;
                 }
             }
         }
     }
 }
+
+
+#[derive(Debug, Copy, Clone)]
+pub enum SceneLightSample {
+    None,
+    Distant {
+        color: SpectralSample,
+        direction: Vector,
+        pdf: f32,
+        selection_pdf: f32,
+    },
+    Surface {
+        color: SpectralSample,
+        sample_geo: (Point, Normal, f32),
+        pdf: f32,
+        selection_pdf: f32,
+    },
+}
+
+impl SceneLightSample {
+    pub fn is_none(&self) -> bool {
+        if let SceneLightSample::None = *self {
+            true
+        } else {
+            false
+        }
+    }
+
+    pub fn color(&self) -> SpectralSample {
+        match *self {
+            SceneLightSample::None => panic!(),
+            SceneLightSample::Distant { color, .. } => color,
+            SceneLightSample::Surface { color, .. } => color,
+        }
+    }
+
+    pub fn pdf(&self) -> f32 {
+        match *self {
+            SceneLightSample::None => panic!(),
+            SceneLightSample::Distant { pdf, .. } => pdf,
+            SceneLightSample::Surface { pdf, .. } => pdf,
+        }
+    }
+
+    pub fn selection_pdf(&self) -> f32 {
+        match *self {
+            SceneLightSample::None => panic!(),
+            SceneLightSample::Distant { selection_pdf, .. } => selection_pdf,
+            SceneLightSample::Surface { selection_pdf, .. } => selection_pdf,
+        }
+    }
+}