Correcting some mistakes I had regarding error handling, enabling optimization on builds, added option handling to BasicSceneBuilder

Cargo.toml

@@ -76,3 +76,17 @@ wrong_self_convention = "allow"
 
 [profile.release]
 debug = true
+
+# Renders run through `cargo test`, whose profile inherits from `dev`. Cargo's
+# default there is opt-level = 0, which costs ~15x on ray throughput.
+[profile.dev]
+opt-level = 1
+
+# Applies to dependencies only -- Cargo excludes workspace members from "*".
+[profile.dev.package."*"]
+opt-level = 3
+
+# `shared` is a workspace member, so it needs naming explicitly. It holds the
+# geometry/BSDF/sampling math, so it wants full optimisation.
+[profile.dev.package.shared]
+opt-level = 3

shared/Cargo.toml

@@ -4,9 +4,8 @@ version = "0.1.0"
 edition = "2024"
 
 [dependencies]
-anyhow = "1.0.100"
 bitflags = "2.10.0"
-half = "2.7.1"
+half = { version = "2.7.1", default-features = false }
 bytemuck = { version = "1.24.0", features = ["derive"] }
 enum_dispatch = "0.3.13"
 ash = { version = "0.38", optional = true }

shared/src/core/color.rs

@@ -8,7 +8,7 @@ use core::fmt;
 use core::ops::{
     Add, AddAssign, Div, DivAssign, Index, IndexMut, Mul, MulAssign, Neg, Sub, SubAssign,
 };
-use anyhow::{Result, bail};
+use crate::utils::error::{Error, Result};
 use enum_dispatch::enum_dispatch;
 use num_traits::Float as NumFloat;
 
@@ -686,7 +686,7 @@ impl ColorEncoding {
         match name {
             "sRGB" | "srgb" => Ok(ColorEncoding::SRGB(SRGBEncoding)),
             "linear" => Ok(ColorEncoding::Linear(LinearEncoding)),
-            _ => bail!("Unknown color encoding: {}", name),
+            _ => Err(Error::UnknownColorEncoding),
         }
     }
 }

shared/src/core/image.rs

@@ -2,7 +2,7 @@ use crate::core::color::{ColorEncoding, ColorEncodingTrait, LINEAR};
 use crate::core::geometry::{Bounds2f, Point2f, Point2fi, Point2i};
 use crate::utils::math::{f16_to_f32_software, lerp, square};
 use crate::{gvec_with_capacity, Float, GVec, Ptr};
-use anyhow::{bail, Result};
+use crate::utils::error::{Error, Result};
 use core::hash;
 use core::ops::{Deref, DerefMut};
 use num_traits::Float as NumFloat;
@@ -23,7 +23,7 @@ impl WrapMode {
             "black" => Ok(WrapMode::Black),
             "repeat" => Ok(WrapMode::Repeat),
             "octahedralsphere" => Ok(WrapMode::OctahedralSphere),
-            _ => bail!("{:?}: wrap mode unknown", name),
+            _ => Err(Error::UnknownWrapMode),
         }
     }
 }
@@ -503,7 +503,7 @@ impl FilterFunction {
             "trilinear" => Ok(FilterFunction::Trilinear),
             "bilinear" => Ok(FilterFunction::Bilinear),
             "point" => Ok(FilterFunction::Point),
-            _ => bail!("Filter function unknown"),
+            _ => Err(Error::UnknownFilterFunction),
         }
     }
 }

shared/src/utils/error.rs

@@ -0,0 +1,39 @@
+//! Errors for the shared crate.
+//!
+//! `shared` is `#![no_std]` and is compiled for SPIR-V and CUDA, so it must not
+//! depend on `anyhow` -- whose default features enable `std`. These variants are
+//! `Copy` and allocation-free; the CPU-side caller holds the offending string and
+//! the `FileLoc`, so it supplies those when reporting.
+
+use core::fmt;
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Error {
+    UnknownWrapMode,
+    UnknownFilterFunction,
+    UnknownColorEncoding,
+    /// `look_at` received an up vector parallel to the viewing direction.
+    DegenerateLookAt,
+    /// A transform matrix could not be inverted (pbrt's `InverseOrDie`).
+    SingularMatrix,
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(match self {
+            Self::UnknownWrapMode => "unknown wrap mode",
+            Self::UnknownFilterFunction => "unknown filter function",
+            Self::UnknownColorEncoding => "unknown color encoding",
+            Self::DegenerateLookAt => {
+                "LookAt: \"up\" vector and viewing direction are parallel"
+            }
+            Self::SingularMatrix => "matrix is not invertible",
+        })
+    }
+}
+
+// Stable in core since 1.81, and the same trait `std::error::Error` re-exports --
+// so `?` and `anyhow::Context` keep working unchanged on the CPU side.
+impl core::error::Error for Error {}
+
+pub type Result<T> = core::result::Result<T, Error>;

shared/src/utils/mod.rs

@@ -2,6 +2,7 @@ pub mod alloc;
 pub mod atomic;
 pub mod complex;
 pub mod containers;
+pub mod error;
 pub mod hash;
 pub mod interval;
 pub mod math;

shared/src/utils/options.rs

@@ -1,3 +1,4 @@
+use alloc::string::String;
 use crate::Float;
 use crate::core::geometry::{Bounds2f, Bounds2i, Point2f, Point2i};
 use core::ops::Deref;
@@ -17,6 +18,7 @@ pub struct BasicPBRTOptions {
     pub disable_wavelength_jitter: bool,
     pub disable_texture_filtering: bool,
     pub force_diffuse: bool,
+    pub record_pixel_statistics: bool,
     pub use_gpu: bool,
     pub wavefront: bool,
     pub interactive: bool,
@@ -33,6 +35,7 @@ impl Default for BasicPBRTOptions {
             disable_wavelength_jitter: false,
             disable_texture_filtering: false,
             force_diffuse: false,
+            record_pixel_statistics: false,
             use_gpu: false,
             wavefront: false,
             interactive: false,
@@ -42,7 +45,7 @@ impl Default for BasicPBRTOptions {
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone)]
 pub struct PBRTOptions {
     pub basic: BasicPBRTOptions,
 
@@ -52,8 +55,8 @@ pub struct PBRTOptions {
     pub image_file: &'static str,
     pub pixel_samples: Option<i32>,
     pub gpu_device: Option<u32>,
-    pub mse_reference_image: Option<&'static str>,
+    pub mse_reference_image: Option<String>,
-    pub mse_reference_output: Option<&'static str>,
+    pub mse_reference_output: Option<String>,
     pub debug_start: Option<(Point2i, i32)>,
     pub quick_render: bool,
     pub upgrade: bool,

shared/src/utils/transform.rs

@@ -14,7 +14,7 @@ use crate::core::interaction::{
 };
 use crate::utils::gpu_array_from_fn;
 use crate::{gamma, Float};
-use anyhow::{bail, Context, Result};
+use crate::utils::error::{Error, Result};
 
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
@@ -2125,15 +2125,7 @@ pub fn look_at(
     // Initialize first three columns of viewing matrix
     let dir = (look - pos).normalize();
     if Vector3f::from(up).normalize().cross(dir).norm() == 0. {
-        bail!(
+        return Err(Error::DegenerateLookAt);
-            "LookAt: \"up\" vector ({}, {}, {}) and viewing direction ({}, {}, {}) passed to LookAt are pointing in the same direction.",
-            up.x(),
-            up.y(),
-            up.z(),
-            dir.x(),
-            dir.y(),
-            dir.z()
-        );
     }
     let right = Vector3f::from(up).normalize().cross(dir).normalize();
     let new_up = dir.cross(right);
@@ -2150,8 +2142,6 @@ pub fn look_at(
     world_from_camera[2][2] = dir.z();
     world_from_camera[3][2] = 0.;
 
-    let camera_from_world = world_from_camera
+    let camera_from_world = world_from_camera.inverse().ok_or(Error::SingularMatrix)?;
-        .inverse()
-        .context("Failed to inverse viewing matrix")?;
     Ok(TransformGeneric::new(camera_from_world, world_from_camera))
 }

src/core/scene/builder.rs

@@ -1,18 +1,17 @@
-use super::entities::*;
 use super::BasicScene;
+use super::entities::*;
+use crate::Arena;
 use crate::spectra::get_colorspace_device;
 use crate::utils::error::FileLoc;
 use crate::utils::parameters::{ParameterDictionary, ParsedParameterVector};
-use crate::utils::parser::{ParserError, ParserTarget};
+use crate::utils::parser::{AtLoc, ParserError, ParserTarget};
-use crate::Arena;
+use shared::Float;
-use anyhow::Context;
 use shared::core::camera::CameraTransform;
 use shared::core::geometry::Vector3f;
 use shared::spectra::RGBColorSpace;
-use shared::utils::options::RenderingCoordinateSystem;
+use shared::utils::options::{PBRTOptions, RenderingCoordinateSystem};
 use shared::utils::transform;
 use shared::utils::transform::{AnimatedTransform, Transform};
-use shared::Float;
 use std::collections::{HashMap, HashSet};
 use std::ops::{Index, IndexMut};
 use std::sync::Arc;
@@ -24,6 +23,16 @@ fn normalize_utf8(input: &str) -> String {
     input.nfc().collect::<String>()
 }
 
+/// pbrt's `normalizeArg` (util/args.h:22): downcase and drop `-`/`_` so option
+/// names can be written a little loosely.
+fn normalize_arg(input: &str) -> String {
+    input
+        .chars()
+        .filter(|c| *c != '-' && *c != '_')
+        .flat_map(|c| c.to_lowercase())
+        .collect()
+}
+
 #[derive(Debug, Default, Clone, Copy)]
 struct TransformSet {
     t: [Transform; MAX_TRANSFORMS],
@@ -125,6 +134,11 @@ pub struct BasicSceneBuilder {
     named_material_names: HashSet<String>,
     medium_names: HashSet<String>,
 
+    /// `Option` directives accumulate here during parsing. The global options are
+    /// a `OnceLock` written after the parse, so they cannot be mutated in place
+    /// the way pbrt mutates its global `Options`.
+    pending_options: PBRTOptions,
+
     current_camera: Option<CameraSceneEntity>,
     current_film: Option<SceneEntity>,
     current_integrator: Option<SceneEntity>,
@@ -177,6 +191,7 @@ impl BasicSceneBuilder {
             spectrum_texture_names: HashSet::new(),
             named_material_names: HashSet::new(),
             medium_names: HashSet::new(),
+            pending_options: PBRTOptions::default(),
             current_camera: Some(CameraSceneEntity {
                 base: SceneEntity {
                     name: "perspective".into(),
@@ -211,6 +226,12 @@ impl BasicSceneBuilder {
         }
     }
 
+    /// Options gathered from the scene's `Option` directives. Callers merge these
+    /// with any command-line options and pass the result to `init_pbrt`.
+    pub fn options(&self) -> &PBRTOptions {
+        &self.pending_options
+    }
+
     fn for_active_transforms<F>(&mut self, f: F)
     where
         F: Fn(&Transform) -> Transform,
@@ -254,12 +275,6 @@ impl BasicSceneBuilder {
     }
 }
 
-impl From<anyhow::Error> for ParserError {
-    fn from(e: anyhow::Error) -> Self {
-        ParserError::Generic(e.to_string(), FileLoc::default())
-    }
-}
-
 impl ParserTarget for BasicSceneBuilder {
     fn reverse_orientation(&mut self, loc: FileLoc) -> Result<(), ParserError> {
         self.verify_world("ReverseOrientation", &loc)?;
@@ -329,8 +344,7 @@ impl ParserTarget for BasicSceneBuilder {
         uz: Float,
         loc: FileLoc,
     ) -> Result<(), ParserError> {
-        let t = transform::look_at((ex, ey, ez), (lx, ly, lz), (ux, uy, uz))
+        let t = transform::look_at((ex, ey, ez), (lx, ly, lz), (ux, uy, uz)).at(&loc)?;
-            .with_context(|| format!("at {}", loc))?;
         self.for_active_transforms(|cur| cur * &t);
         Ok(())
     }
@@ -447,8 +461,65 @@ impl ParserTarget for BasicSceneBuilder {
         Ok(())
     }
 
-    fn option(&mut self, _name: &str, _value: &str, _loc: FileLoc) -> Result<(), ParserError> {
+    fn option(&mut self, name: &str, value: &str, loc: FileLoc) -> Result<(), ParserError> {
-        todo!()
+        let bad = |what: &str| {
+            Err(ParserError::Generic(
+                format!("{value:?}: expected {what} for option {name:?}"),
+                loc.clone(),
+            ))
+        };
+        let as_bool = |b: &mut bool| match value {
+            "true" => {
+                *b = true;
+                Ok(())
+            }
+            "false" => {
+                *b = false;
+                Ok(())
+            }
+            _ => bad("\"true\" or \"false\""),
+        };
+
+        let opts = &mut self.pending_options;
+        match normalize_arg(name).as_str() {
+            "disablepixeljitter" => as_bool(&mut opts.basic.disable_pixel_jitter)?,
+            "disabletexturefiltering" => as_bool(&mut opts.basic.disable_texture_filtering)?,
+            "disablewavelengthjitter" => as_bool(&mut opts.basic.disable_wavelength_jitter)?,
+            "forcediffuse" => as_bool(&mut opts.basic.force_diffuse)?,
+            "pixelstats" => as_bool(&mut opts.basic.record_pixel_statistics)?,
+            "wavefront" => as_bool(&mut opts.basic.wavefront)?,
+            "displacementedgescale" => match value.parse::<Float>() {
+                Ok(v) => opts.displacement_edge_scale = v,
+                Err(_) => return bad("a floating-point value"),
+            },
+            "seed" => match value.parse::<i32>() {
+                Ok(v) => opts.basic.seed = v,
+                Err(_) => return bad("an integer"),
+            },
+            // The tokenizer has already dequoted these, so unlike pbrt we do not
+            // re-check for surrounding quotes.
+            "msereferenceimage" => {
+                opts.mse_reference_image = Some(value.to_string())
+            }
+            "msereferenceout" => {
+                opts.mse_reference_output = Some(value.to_string())
+            }
+            "rendercoordsys" => {
+                opts.basic.rendering_space = match value {
+                    "camera" => RenderingCoordinateSystem::Camera,
+                    "cameraworld" => RenderingCoordinateSystem::CameraWorld,
+                    "world" => RenderingCoordinateSystem::World,
+                    _ => return bad("\"camera\", \"cameraworld\" or \"world\""),
+                }
+            }
+            _ => {
+                return Err(ParserError::Generic(
+                    format!("{name:?}: unknown option"),
+                    loc,
+                ));
+            }
+        }
+        Ok(())
     }
 
     fn pixel_filter(
@@ -534,7 +605,8 @@ impl ParserTarget for BasicSceneBuilder {
             params,
             &self.graphics_state.medium_attributes,
             self.graphics_state.color_space.clone(),
-        )?;
+        )
+        .at(&loc)?;
         let render_from_object = self.render_from_object();
         let entity = MediumSceneEntity {
             base: SceneEntity {
@@ -702,7 +774,8 @@ impl ParserTarget for BasicSceneBuilder {
             params.clone(),
             &self.graphics_state.texture_attributes,
             self.graphics_state.color_space.clone(),
-        )?;
+        )
+        .at(&loc)?;
 
         if type_name != "float" && type_name != "spectrum" {
             return Err(ParserError::Generic(
@@ -762,7 +835,8 @@ impl ParserTarget for BasicSceneBuilder {
             params,
             &self.graphics_state.material_attributes,
             self.graphics_state.color_space.clone(),
-        )?;
+        )
+        .at(&loc)?;
         let entity = SceneEntity {
             name: name.to_string(),
             loc,
@@ -794,7 +868,8 @@ impl ParserTarget for BasicSceneBuilder {
             params,
             &self.graphics_state.material_attributes,
             self.graphics_state.color_space.clone(),
-        )?;
+        )
+        .at(&loc)?;
 
         // pbrt stores an empty entity name here: the material type comes from the
         // "type" parameter (scene.cpp:719).
@@ -827,7 +902,8 @@ impl ParserTarget for BasicSceneBuilder {
             params.clone(),
             &self.graphics_state.medium_attributes,
             self.graphics_state.color_space.clone(),
-        )?;
+        )
+        .at(&loc)?;
 
         let render_from_light = self.render_from_object();
 
@@ -874,7 +950,8 @@ impl ParserTarget for BasicSceneBuilder {
             params.clone(),
             &self.graphics_state.shape_attributes,
             self.graphics_state.color_space.clone(),
-        )?;
+        )
+        .at(&loc)?;
 
         let render_from_object = self.render_from_object_at(0);
         let object_from_render = render_from_object.inverse();

src/textures/image.rs

@@ -6,7 +6,7 @@ use crate::core::texture::{
 use crate::utils::mipmap::{MIPMap, MIPMapFilterOptions};
 use crate::utils::{resolve_filename, FileLoc, TextureParameterDictionary};
 use crate::Arena;
-use anyhow::Result;
+use anyhow::{Context, Result};
 use shared::core::color::RGB;
 use shared::core::color::{ColorEncoding, SRGBEncoding};
 use shared::core::geometry::Vector2f;
@@ -271,10 +271,10 @@ impl CreateFloatTexture for FloatImageTexture {
         let mut filter_options = MIPMapFilterOptions::default();
         filter_options.max_anisotropy = max_aniso;
 
-        let ff = FilterFunction::parse(&filter)?;
+        let ff = FilterFunction::parse(&filter).with_context(|| format!("{:?}", filter))?;
         filter_options.filter = ff;
         let wrap_string = parameters.get_one_string("wrap", "repeat")?;
-        let wrap_mode = WrapMode::parse(&wrap_string)?;
+        let wrap_mode = WrapMode::parse(&wrap_string).with_context(|| format!("{:?}", wrap_string))?;
         let scale = parameters.get_one_float("scale", 1.)?;
         let invert = parameters.get_one_bool("invert", false)?;
         let filename = resolve_filename(&parameters.get_one_string("filename", "")?);
@@ -287,7 +287,8 @@ impl CreateFloatTexture for FloatImageTexture {
             "linear"
         };
         let encoding_str = parameters.get_one_string("encoding", default_encoding)?;
-        let encoding = ColorEncoding::from_name(&encoding_str)?;
+        let encoding =
+            ColorEncoding::from_name(&encoding_str).with_context(|| format!("{:?}", encoding_str))?;
 
         let tex = FloatImageTexture::new(
             mapping,

src/utils/parser.rs

@@ -1,3 +1,4 @@
+use thiserror::Error;
 use anyhow::Result;
 use flate2::read::GzDecoder;
 use memmap2::Mmap;
@@ -247,17 +248,39 @@ impl Token {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
 pub enum ParserError {
+    #[error("{0}")]
     Io(String),
+    #[error("unexpected end of file")]
     UnexpectedEof,
+    #[error("invalid UTF-8: {0}")]
     InvalidUtf8(String),
+    #[error("{1}: {0}")]
     Generic(String, FileLoc),
+    #[error("{1}: expected an integer: {0}")]
     ParseIntError(String, FileLoc),
+    #[error("{1}: expected a float: {0}")]
     ParseFloatError(String, FileLoc),
+    #[error("{1}: numeric overflow: {0}")]
     NumericOverflow(String, FileLoc),
 }
 
+/// Attach a source location to any foreign error, converting it into a
+/// `ParserError`. This is the only sanctioned direction: typed errors are
+/// produced *at* the boundary that knows the `FileLoc`, never recovered from an
+/// erased `anyhow::Error` after the fact.
+pub trait AtLoc<T> {
+    fn at(self, loc: &FileLoc) -> Result<T, ParserError>;
+}
+
+impl<T, E: std::fmt::Display> AtLoc<T> for Result<T, E> {
+    fn at(self, loc: &FileLoc) -> Result<T, ParserError> {
+        // `{:#}` renders anyhow's full context chain; harmless for plain errors.
+        self.map_err(|e| ParserError::Generic(format!("{e:#}"), loc.clone()))
+    }
+}
+
 pub enum TokenizerBuffer {
     Ram(String),
     Mapped(Mmap),