Merge pull request #188 from Walther/2023-10-20-refactoring

improvement: move priority to objects, materials to separate list

clovers-cli/src/json_scene.rs

@@ -18,6 +18,6 @@ pub(crate) fn initialize<'scene>(
     let scene_file: SceneFile = serde_json::from_str(&contents)?;
     info!("Initializing the scene");
     let scene: Scene = scenes::initialize(scene_file, opts.width, opts.height);
-    info!("Count of nodes in the BVH tree: {}", scene.objects.count());
+    info!("Count of nodes in the BVH tree: {}", scene.hitables.count());
     Ok(scene)
 }

clovers/src/bvhnode.rs

@@ -28,7 +28,7 @@ pub struct BVHNode<'scene> {
 impl<'scene> BVHNode<'scene> {
     /// Create a new `BVHNode` tree from a given list of [Object](crate::objects::Object)s
     #[must_use]
-    pub fn from_list(mut objects: Vec<Hitable>, time_0: Float, time_1: Float) -> BVHNode {
+    pub fn from_list(mut hitables: Vec<Hitable>, time_0: Float, time_1: Float) -> BVHNode {
         // Initialize two child nodes
         let left: Box<Hitable>;
         let right: Box<Hitable>;
@@ -38,7 +38,7 @@ impl<'scene> BVHNode<'scene> {
         // What is the axis with the largest span?
         // TODO: horribly inefficient, improve!
         let bounding: AABB =
-            vec_bounding_box(&objects, time_0, time_1).expect("No bounding box for objects");
+            vec_bounding_box(&hitables, time_0, time_1).expect("No bounding box for objects");
         let spans = [
             bounding.axis(0).size(),
             bounding.axis(1).size(),
@@ -50,12 +50,12 @@ impl<'scene> BVHNode<'scene> {
         let comparator = comparators[axis];
 
         // How many objects do we have?
-        let object_span = objects.len();
+        let object_span = hitables.len();
 
         if object_span == 1 {
             // If we only have one object, add one and an empty object.
             // TODO: can this hack be removed?
-            left = Box::new(objects[0].clone());
+            left = Box::new(hitables[0].clone());
             right = Box::new(Hitable::Empty(Empty {}));
             let bounding_box = left.bounding_box(time_0, time_1).unwrap().clone(); // TODO: remove unwrap
             return BVHNode {
@@ -66,14 +66,14 @@ impl<'scene> BVHNode<'scene> {
         } else if object_span == 2 {
             // If we are comparing two objects, perform the comparison
             // Insert the child nodes in order
-            match comparator(&objects[0], &objects[1]) {
+            match comparator(&hitables[0], &hitables[1]) {
                 Ordering::Less => {
-                    left = Box::new(objects[0].clone());
-                    right = Box::new(objects[1].clone());
+                    left = Box::new(hitables[0].clone());
+                    right = Box::new(hitables[1].clone());
                 }
                 Ordering::Greater => {
-                    left = Box::new(objects[1].clone());
-                    right = Box::new(objects[0].clone());
+                    left = Box::new(hitables[1].clone());
+                    right = Box::new(hitables[0].clone());
                 }
                 Ordering::Equal => {
                     // TODO: what should happen here?
@@ -82,29 +82,29 @@ impl<'scene> BVHNode<'scene> {
             }
         } else if object_span == 3 {
             // Three objects: create one bare object and one BVHNode with two objects
-            objects.sort_by(comparator);
-            left = Box::new(objects[0].clone());
+            hitables.sort_by(comparator);
+            left = Box::new(hitables[0].clone());
             right = Box::new(Hitable::BVHNode(BVHNode {
-                left: Box::new(objects[1].clone()),
-                right: Box::new(objects[2].clone()),
+                left: Box::new(hitables[1].clone()),
+                right: Box::new(hitables[2].clone()),
                 bounding_box: AABB::surrounding_box(
                     // TODO: no unwrap?
-                    objects[1].bounding_box(time_0, time_1).unwrap(),
-                    objects[2].bounding_box(time_0, time_1).unwrap(),
+                    hitables[1].bounding_box(time_0, time_1).unwrap(),
+                    hitables[2].bounding_box(time_0, time_1).unwrap(),
                 ),
             }));
         } else {
             // Otherwise, recurse
-            objects.sort_by(comparator);
+            hitables.sort_by(comparator);
 
             // Split the vector; divide and conquer
             let mid = object_span / 2;
-            let objects_right = objects.split_off(mid);
+            let hitables_right = hitables.split_off(mid);
             left = Box::new(Hitable::BVHNode(BVHNode::from_list(
-                objects, time_0, time_1,
+                hitables, time_0, time_1,
             )));
             right = Box::new(Hitable::BVHNode(BVHNode::from_list(
-                objects_right,
+                hitables_right,
                 time_0,
                 time_1,
             )));

clovers/src/colorize.rs

@@ -35,7 +35,10 @@ pub fn colorize(
 
     // Send the ray to the scene, and see if it hits anything.
     // distance_min is set to an epsilon to avoid "shadow acne" that can happen when set to zero
-    let Some(hit_record) = scene.objects.hit(ray, EPSILON_SHADOW_ACNE, Float::MAX, rng) else {
+    let Some(hit_record) = scene
+        .hitables
+        .hit(ray, EPSILON_SHADOW_ACNE, Float::MAX, rng)
+    else {
         // If the ray hits nothing, early return the background color.
         return bg;
     };
@@ -77,7 +80,7 @@ pub fn colorize(
             // Use a probability density function to figure out where to scatter a new ray
             // TODO: this weighed priority sampling should be adjusted or removed - doesn't feel ideal.
             let light_ptr = PDF::HitablePDF(HitablePDF::new(
-                &scene.priority_objects,
+                &scene.priority_hitables,
                 hit_record.position,
             ));
             let mixture_pdf = MixturePDF::new(light_ptr, scatter_record.pdf_ptr);

clovers/src/normals.rs

@@ -7,7 +7,10 @@ use rand::rngs::SmallRng;
 /// Rendering function for getting a normal map in tangent space. Sends a [Ray] to the [Scene], sees what it hits, gets the normal at that point, and returns a color based on the normal mapping colorization. Wikipedia: [Normal mapping](https://en.wikipedia.org/wiki/Normal_mapping).
 #[must_use]
 pub fn normal_map(ray: &Ray, scene: &Scene, rng: &mut SmallRng) -> LinSrgb {
-    let Some(hit_record) = scene.objects.hit(ray, EPSILON_SHADOW_ACNE, Float::MAX, rng) else {
+    let Some(hit_record) = scene
+        .hitables
+        .hit(ray, EPSILON_SHADOW_ACNE, Float::MAX, rng)
+    else {
         // If the ray hits nothing, early return black
         return LinSrgb::new(0.0, 0.0, 0.0);
     };

clovers/src/objects.rs

@@ -42,6 +42,9 @@ pub use triangle::*;
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 /// A list of objects. Allows multiple objects to be used e.g. in a Rotate or Translate object as the target.
 pub struct ObjectList {
+    /// Priority
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// The encased [Object] list
     pub objects: Vec<Object>,
 }

clovers/src/objects/boxy.rs

@@ -15,6 +15,9 @@ use rand::{rngs::SmallRng, Rng};
 #[derive(Debug, Clone)]
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 pub struct BoxyInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// First corner for the box
     pub corner_0: Vec3,
     /// Second, opposing corner for the box

clovers/src/objects/constant_medium.rs

@@ -19,6 +19,9 @@ use super::Object;
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 /// `ConstantMediumInit` structure describes the necessary data for constructing a [`ConstantMedium`]. Used with [serde] when importing [`SceneFiles`](crate::scenes::SceneFile).
 pub struct ConstantMediumInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// The boundary object for the constant medium. This determines the size and shape of the fog object.
     pub boundary: Box<Object>,
     #[cfg_attr(feature = "serde-derive", serde(default = "default_density"))]

clovers/src/objects/gltf.rs

@@ -24,6 +24,9 @@ use crate::{
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 pub struct GLTFInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// Path of the .gltf file
     pub path: String,
 }

clovers/src/objects/moving_sphere.rs

@@ -15,6 +15,9 @@ use rand::rngs::SmallRng;
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 /// `SphereInit` structure describes the necessary data for constructing a [`Sphere`](super::Sphere). Used with [serde] when importing [`SceneFile`](crate::scenes::SceneFile)s.
 pub struct MovingSphereInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// Center point of the sphere at time_0
     pub center_0: Vec3,
     /// Center point of the sphere at time_1

clovers/src/objects/quad.rs

@@ -16,6 +16,9 @@ use rand::Rng;
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 pub struct QuadInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// Corner point
     pub q: Vec3,
     /// Vector describing the u side

clovers/src/objects/rotate.rs

@@ -15,6 +15,9 @@ use super::Object;
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 /// `RotateInit` structure describes the necessary data for constructing a [`RotateY`]. Used with [serde] when importing [`SceneFile`](crate::scenes::SceneFile)s.
 pub struct RotateInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// The encased [Object] to rotate
     pub object: Box<Object>,
     /// Angle to rotate the object, in degrees

clovers/src/objects/sphere.rs

@@ -15,6 +15,9 @@ use rand::{rngs::SmallRng, Rng};
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 /// `SphereInit` structure describes the necessary data for constructing a [Sphere]. Used with [serde] when importing [`SceneFile`](crate::scenes::SceneFile)s.
 pub struct SphereInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// Center of the sphere.
     pub center: Vec3,
     /// Radius of the sphere.

clovers/src/objects/stl.rs

@@ -71,6 +71,9 @@ impl<'scene> HitableTrait for STL<'scene> {
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 pub struct STLInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// Path of the .stl file
     pub path: String,
     /// Material to use for the .stl object

clovers/src/objects/translate.rs

@@ -15,6 +15,9 @@ use super::Object;
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 /// `TranslateInit` structure describes the necessary data for constructing a [Translate] object. Used with [serde] when importing [`SceneFile`](crate::scenes::SceneFile)s.
 pub struct TranslateInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// The encased [Object] to translate i.e. move
     pub object: Box<Object>,
     /// The vector describing the movement of the object

clovers/src/objects/triangle.rs

@@ -17,6 +17,9 @@ use rand::Rng;
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "serde-derive", derive(serde::Serialize, serde::Deserialize))]
 pub struct TriangleInit {
+    /// Used for multiple importance sampling
+    #[cfg_attr(feature = "serde-derive", serde(default))]
+    pub priority: bool,
     /// Corner point
     pub q: Vec3,
     /// Vector describing the u side

clovers/src/scenes.rs

@@ -17,13 +17,13 @@ use tracing::info;
 /// A representation of the scene that is being rendered.
 pub struct Scene<'scene> {
     /// Bounding-volume hierarchy of [Hitable] objects in the scene. This could, as currently written, be any [Hitable] - in practice, we place the root of the [BVHNode] tree here.
-    pub objects: BVHNode<'scene>,
+    pub hitables: BVHNode<'scene>,
     /// The camera object used for rendering the scene.
     pub camera: Camera,
     /// The background color to use when the rays do not hit anything in the scene.
     pub background_color: Srgb, // TODO: make into Texture or something?
     /// A [BVHNode] tree of prioritized objects - e.g. glass items or lights - that affect the biased sampling of the scene. Wrapped into a [Hitable] for convenience reasons (see various PDF functions).
-    pub priority_objects: Hitable<'scene>,
+    pub priority_hitables: Hitable<'scene>,
 }
 
 impl<'scene> Scene<'scene> {
@@ -33,16 +33,16 @@ impl<'scene> Scene<'scene> {
         time_0: Float,
         time_1: Float,
         camera: Camera,
-        objects: Vec<Hitable<'scene>>,
-        priority_objects: Vec<Hitable<'scene>>,
+        hitables: Vec<Hitable<'scene>>,
+        priority_hitables: Vec<Hitable<'scene>>,
         background_color: Srgb,
     ) -> Scene<'scene> {
         Scene {
-            objects: BVHNode::from_list(objects, time_0, time_1),
+            hitables: BVHNode::from_list(hitables, time_0, time_1),
             camera,
             background_color,
-            priority_objects: Hitable::BVHNode(BVHNode::from_list(
-                priority_objects,
+            priority_hitables: Hitable::BVHNode(BVHNode::from_list(
+                priority_hitables,
                 time_0,
                 time_1,
             )),
@@ -62,7 +62,6 @@ pub struct SceneFile {
     objects: Vec<Object>,
     #[cfg_attr(feature = "serde-derive", serde(default))]
     materials: Vec<SharedMaterial>,
-    priority_objects: Vec<Object>,
 }
 
 /// Initializes a new [Scene] instance by parsing the contents of a [`SceneFile`] structure and then using those details to construct the [Scene].
@@ -90,26 +89,39 @@ pub fn initialize<'scene>(scene_file: SceneFile, width: u32, height: u32) -> Sce
 
     #[cfg(feature = "traces")]
     info!("Creating a flattened list from the objects");
-    let hitables: Vec<Hitable> = objects_to_hitables(scene_file.objects, materials);
-    let priority_objects: Vec<Hitable> =
-        objects_to_hitables(scene_file.priority_objects, materials);
+    let mut hitables: Vec<Hitable> = Vec::new();
+    let mut priority_hitables: Vec<Hitable> = Vec::new();
+
+    // TODO: this isn't the greatest ergonomics, but it gets the job done for now
+    for object in scene_file.objects {
+        if match &object {
+            Object::Boxy(i) => i.priority,
+            Object::ConstantMedium(i) => i.priority,
+            Object::MovingSphere(i) => i.priority,
+            Object::ObjectList(i) => i.priority,
+            Object::Quad(i) => i.priority,
+            Object::RotateY(i) => i.priority,
+            Object::Sphere(i) => i.priority,
+            Object::STL(i) => i.priority,
+            Object::GLTF(i) => i.priority,
+            Object::Translate(i) => i.priority,
+            Object::Triangle(i) => i.priority,
+        } {
+            let hitable = object_to_hitable(object, materials);
+            hitables.push(hitable.clone());
+            priority_hitables.push(hitable);
+        } else {
+            let hitable = object_to_hitable(object, materials);
+            hitables.push(hitable.clone());
+        }
+    }
 
     Scene::new(
         time_0,
         time_1,
         camera,
         hitables,
-        priority_objects,
+        priority_hitables,
         background_color,
     )
 }
-
-#[must_use]
-fn objects_to_hitables(objects: Vec<Object>, materials: &[SharedMaterial]) -> Vec<Hitable<'_>> {
-    let mut hitables = Vec::new();
-    for obj in objects {
-        hitables.push(object_to_hitable(obj, materials));
-    }
-
-    hitables
-}