diff --git a/Cargo.toml b/Cargo.toml index 49be0e3..f02ff59 100644 --- a/Cargo.toml +++ b/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 diff --git a/shared/Cargo.toml b/shared/Cargo.toml index 6a429ac..1d911a7 100644 --- a/shared/Cargo.toml +++ b/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 } diff --git a/shared/src/core/color.rs b/shared/src/core/color.rs index 7af72d0..24d1ba0 100644 --- a/shared/src/core/color.rs +++ b/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), } } } diff --git a/shared/src/core/image.rs b/shared/src/core/image.rs index 2998d60..bfcb76e 100644 --- a/shared/src/core/image.rs +++ b/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), } } } diff --git a/shared/src/utils/error.rs b/shared/src/utils/error.rs new file mode 100644 index 0000000..8a62081 --- /dev/null +++ b/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 = core::result::Result; diff --git a/shared/src/utils/mod.rs b/shared/src/utils/mod.rs index 7d76a4f..c6b8fe1 100644 --- a/shared/src/utils/mod.rs +++ b/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; diff --git a/shared/src/utils/options.rs b/shared/src/utils/options.rs index 334bd6f..f3eb9ce 100644 --- a/shared/src/utils/options.rs +++ b/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, pub gpu_device: Option, - pub mse_reference_image: Option<&'static str>, - pub mse_reference_output: Option<&'static str>, + pub mse_reference_image: Option, + pub mse_reference_output: Option, pub debug_start: Option<(Point2i, i32)>, pub quick_render: bool, pub upgrade: bool, diff --git a/shared/src/utils/transform.rs b/shared/src/utils/transform.rs index c6e2d78..977b6dc 100644 --- a/shared/src/utils/transform.rs +++ b/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!( - "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() - ); + return Err(Error::DegenerateLookAt); } 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 - .inverse() - .context("Failed to inverse viewing matrix")?; + let camera_from_world = world_from_camera.inverse().ok_or(Error::SingularMatrix)?; Ok(TransformGeneric::new(camera_from_world, world_from_camera)) } diff --git a/src/core/scene/builder.rs b/src/core/scene/builder.rs index 3f161c9..3170096 100644 --- a/src/core/scene/builder.rs +++ b/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::Arena; -use anyhow::Context; +use crate::utils::parser::{AtLoc, ParserError, ParserTarget}; +use shared::Float; 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::() } +/// 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, medium_names: HashSet, + /// `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, current_film: Option, current_integrator: Option, @@ -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(&mut self, f: F) where F: Fn(&Transform) -> Transform, @@ -254,12 +275,6 @@ impl BasicSceneBuilder { } } -impl From 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)) - .with_context(|| format!("at {}", loc))?; + let t = transform::look_at((ex, ey, ez), (lx, ly, lz), (ux, uy, uz)).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> { - todo!() + fn option(&mut self, name: &str, value: &str, loc: FileLoc) -> Result<(), ParserError> { + 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::() { + Ok(v) => opts.displacement_edge_scale = v, + Err(_) => return bad("a floating-point value"), + }, + "seed" => match value.parse::() { + 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(); diff --git a/src/textures/image.rs b/src/textures/image.rs index cd69b3d..63eda11 100644 --- a/src/textures/image.rs +++ b/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(¶meters.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, diff --git a/src/utils/parser.rs b/src/utils/parser.rs index 57a3679..0b68cab 100644 --- a/src/utils/parser.rs +++ b/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 { + fn at(self, loc: &FileLoc) -> Result; +} + +impl AtLoc for Result { + fn at(self, loc: &FileLoc) -> Result { + // `{:#}` 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),