From 46a4edaee5bf9cf8a238b4257238edabfddaa66f Mon Sep 17 00:00:00 2001 From: Wito Wiala Date: Mon, 31 Aug 2026 15:35:55 +0100 Subject: [PATCH] Continuing migration to index and GVec based usage of objects --- shared/src/core/handle.rs | 26 ++++++++++ shared/src/lights/sampler.rs | 14 +++--- src/core/interaction.rs | 4 +- src/core/medium.rs | 2 + src/core/render.rs | 2 +- src/core/scene/builder.rs | 83 +++++++++++++++++++++++++------ src/core/scene/scene.rs | 96 ++++++++++++++++++++++++++---------- src/core/scene/state.rs | 4 +- src/integrators/base.rs | 2 +- src/integrators/path.rs | 4 +- src/utils/parameters.rs | 5 +- src/wavefront/aggregate.rs | 2 +- src/wavefront/integrator.rs | 8 +-- 13 files changed, 189 insertions(+), 63 deletions(-) diff --git a/shared/src/core/handle.rs b/shared/src/core/handle.rs index 199cabc..bbe53fb 100644 --- a/shared/src/core/handle.rs +++ b/shared/src/core/handle.rs @@ -1,4 +1,5 @@ use crate::core::light::Light; +use crate::core::material::Material; #[repr(C)] #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -16,8 +17,18 @@ impl Default for LightIdx { impl LightIdx { #[inline] pub fn get(self, lights: &[Light]) -> &Light { + debug_assert!(!self.is_none(), "LightIdx::get on NONE handle"); &lights[self.0 as usize] } + + #[inline] + pub fn try_get(self, lights: &[Light]) -> Option<&Light> { + if self.is_none() { + None + } else { + lights.get(self.0 as usize) + } + } } #[repr(C)] @@ -27,6 +38,21 @@ pub struct MaterialIdx(pub u32); impl MaterialIdx { pub const NONE: Self = MaterialIdx(u32::MAX); pub fn is_none(self) -> bool { self.0 == u32::MAX } + + #[inline] + pub fn get(self, materials: &[Material]) -> &Material { + debug_assert!(!self.is_none(), "MaterialIdx::get on NONE handle"); + &materials[self.0 as usize] + } + + #[inline] + pub fn try_get(self, materials: &[Material]) -> Option<&Material> { + if self.is_none() { + None + } else { + materials.get(self.0 as usize) + } + } } impl Default for MaterialIdx { diff --git a/shared/src/lights/sampler.rs b/shared/src/lights/sampler.rs index 16af7b4..aaf1146 100644 --- a/shared/src/lights/sampler.rs +++ b/shared/src/lights/sampler.rs @@ -1,7 +1,7 @@ +use crate::core::LightIdx; use crate::core::geometry::primitives::OctahedralVector; use crate::core::geometry::{Bounds3f, DirectionCone, Normal3f, Point3f, Vector3f, VectorLike}; use crate::core::light::{Light, LightBounds, LightSampleContext}; -use crate::core::LightIdx; use crate::spectra::{SampledSpectrum, SampledWavelengths}; use crate::utils::math::{clamp, lerp, sample_discrete}; use crate::utils::math::{safe_sqrt, square}; @@ -378,12 +378,7 @@ impl BVHLightSampler { #[inline(always)] fn light_index_in(&self, base: Ptr, len: u32, light: &Light) -> Option { let target = light as *const Light; - for i in 0..len as usize { - if unsafe { base.add(i) }.as_raw() == target { - return Some(i); - } - } - None + (0..len as usize).find(|&i| unsafe { base.add(i) }.as_raw() == target) } fn evaluate_cost(&self, b: &LightBounds, bounds: &Bounds3f, dim: usize) -> Float { @@ -414,7 +409,10 @@ impl LightSamplerTrait for BVHLightSampler { // in the scene lights array by construction) let ind = (u * inf_size).min(inf_size - 1.) as u32; let pmf = p_inf / inf_size; - return Some(SampledLight { light: LightIdx(ind), p: pmf }); + return Some(SampledLight { + light: LightIdx(ind), + p: pmf, + }); } if self.nodes_len == 0 { diff --git a/src/core/interaction.rs b/src/core/interaction.rs index d0621ef..8d1bb8d 100644 --- a/src/core/interaction.rs +++ b/src/core/interaction.rs @@ -44,7 +44,7 @@ impl InteractionGetter for SurfaceInteraction { if self.material.is_none() { return None; } - let mut active_mat: &Material = &materials[self.material.0 as usize]; + let mut active_mat: &Material = self.material.get(materials); let material = { let tex_eval = UniversalTextureEvaluator; while let Material::Mix(mix) = active_mat { @@ -82,7 +82,7 @@ impl InteractionGetter for SurfaceInteraction { if self.material.is_none() { return None; } - let mut active_mat: &Material = &materials[self.material.0 as usize]; + let mut active_mat: &Material = self.material.get(materials); let tex_eval = UniversalTextureEvaluator; while let Material::Mix(mix) = active_mat { let ctx = MaterialEvalContext::from(self); diff --git a/src/core/medium.rs b/src/core/medium.rs index 9d2d6b4..036a96d 100644 --- a/src/core/medium.rs +++ b/src/core/medium.rs @@ -291,6 +291,8 @@ pub trait CreateMedium { } } +impl CreateMedium for Medium {} + fn create_homogeneous( parameters: &ParameterDictionary, loc: &FileLoc, diff --git a/src/core/render.rs b/src/core/render.rs index c6f10b1..167f867 100644 --- a/src/core/render.rs +++ b/src/core/render.rs @@ -14,7 +14,7 @@ use crate::wavefront::integrator::CpuWavefrontRenderer; use shared::Float; pub fn render_scene(scene: &BasicScene, arena: &Arena) -> Result<()> { - let media = scene.create_media(); + let media = scene.create_media(arena); let textures = scene.create_textures(arena); let (named_materials, materials, _default_mtl) = scene.create_materials(&textures, arena)?; let (lights, al_map) = scene.create_lights(&textures, &media, arena); diff --git a/src/core/scene/builder.rs b/src/core/scene/builder.rs index e4638ff..3f161c9 100644 --- a/src/core/scene/builder.rs +++ b/src/core/scene/builder.rs @@ -152,11 +152,20 @@ impl BasicSceneBuilder { } pub fn new(scene: Arc) -> Self { + // pbrt seeds material index 0 with a default "diffuse" so that a shape + // declared before any Material statement still has one (scene.cpp:102). + let default_material_index = scene.add_material(SceneEntity { + name: "diffuse".into(), + loc: FileLoc::default(), + parameters: ParameterDictionary::new(Vec::new(), None).unwrap(), + }); + Self { scene, current_block: BlockState::OptionsBlock, graphics_state: GraphicsState { active_transform_bits: Self::ALL_TRANSFORM_BITS, + current_material_index: Some(default_material_index), ..Default::default() }, pushed_graphics_states: Vec::new(), @@ -512,23 +521,31 @@ impl ParserTarget for BasicSceneBuilder { params: ParsedParameterVector, loc: FileLoc, ) -> Result<(), ParserError> { - self.verify_world("MakeNamedMaterial", &loc)?; let curr_name = normalize_utf8(name); - if !self.named_material_names.insert(curr_name.to_string()) { + if !self.medium_names.insert(curr_name.clone()) { return Err(ParserError::Generic( - format!("Named material '{}' redefined.", name), + format!("Named medium '{}' redefined.", name), loc, )); } - let parameters = self.make_params(params, &loc)?; - let entity = SceneEntity { - name: name.to_string(), - loc, - parameters, + + let parameters = ParameterDictionary::from_array( + params, + &self.graphics_state.medium_attributes, + self.graphics_state.color_space.clone(), + )?; + let render_from_object = self.render_from_object(); + let entity = MediumSceneEntity { + base: SceneEntity { + name: name.to_string(), + loc, + parameters, + }, + render_from_object, }; - self.scene.add_named_material(&curr_name, entity); + self.scene.add_medium(&curr_name, entity); Ok(()) } @@ -741,10 +758,15 @@ impl ParserTarget for BasicSceneBuilder { loc: FileLoc, ) -> Result<(), ParserError> { self.verify_world("material", &loc)?; + let dict = ParameterDictionary::from_array( + params, + &self.graphics_state.material_attributes, + self.graphics_state.color_space.clone(), + )?; let entity = SceneEntity { name: name.to_string(), loc, - parameters: ParameterDictionary::new(params.clone(), None).unwrap(), + parameters: dict, }; let idx = self.scene.add_material(entity); self.graphics_state.current_material_index = Some(idx); @@ -754,15 +776,44 @@ impl ParserTarget for BasicSceneBuilder { fn make_named_material( &mut self, - _name: &str, - _params: ParsedParameterVector, - _loc: FileLoc, + name: &str, + params: ParsedParameterVector, + loc: FileLoc, ) -> Result<(), ParserError> { - todo!() + self.verify_world("MakeNamedMaterial", &loc)?; + let curr_name = normalize_utf8(name); + + if !self.named_material_names.insert(curr_name.clone()) { + return Err(ParserError::Generic( + format!("Named material '{}' redefined.", name), + loc, + )); + } + + let parameters = ParameterDictionary::from_array( + params, + &self.graphics_state.material_attributes, + self.graphics_state.color_space.clone(), + )?; + + // pbrt stores an empty entity name here: the material type comes from the + // "type" parameter (scene.cpp:719). + self.scene.add_named_material( + &curr_name, + SceneEntity { + name: String::new(), + loc, + parameters, + }, + ); + Ok(()) } - fn named_material(&mut self, _name: &str, _loc: FileLoc) -> Result<(), ParserError> { - todo!() + fn named_material(&mut self, name: &str, loc: FileLoc) -> Result<(), ParserError> { + self.verify_world("NamedMaterial", &loc)?; + self.graphics_state.current_material_name = normalize_utf8(name); + self.graphics_state.current_material_index = None; + Ok(()) } fn light_source( diff --git a/src/core/scene/scene.rs b/src/core/scene/scene.rs index c703a2d..c549278 100644 --- a/src/core/scene/scene.rs +++ b/src/core/scene/scene.rs @@ -6,6 +6,7 @@ use crate::core::film::FilmFactory; use crate::core::filter::FilterFactory; use crate::core::image::{HostImage, ImageIO}; use crate::core::material::MaterialFactory; +use crate::core::medium::CreateMedium; use crate::core::primitive::{CreateGeometricPrimitive, CreateSimplePrimitive}; use crate::core::sampler::SamplerFactory; use crate::core::shape::{ShapeFactory, ShapeWithContext}; @@ -87,21 +88,21 @@ fn resolve_material( MaterialRef::Name(name) => match named_materials.get(name) { Some(m) => *m, None => { - MaterialIdx::default() - // log::error!("{}: named material '{}' not found", loc, name); - // crate::core::material::default_diffuse_material(arena) + log::error!("{}: named material '{}' not found", loc, name); + MaterialIdx::NONE } }, + // Anonymous materials are placed at the front of `materials`, so the + // index handed out by `add_material` is the handle directly. MaterialRef::Index(idx) => { if *idx < materials.len() { MaterialIdx(*idx as u32) } else { - MaterialIdx::default() - // log::error!("{}: material index {} out of bounds", loc, idx); - // crate::core::material::default_diffuse_material(arena) + log::error!("{}: material index {} out of bounds", loc, idx); + MaterialIdx::NONE } } - MaterialRef::None => MaterialIdx::default(), + MaterialRef::None => MaterialIdx::NONE, } } @@ -258,6 +259,11 @@ impl BasicScene { state.named_materials.push((name.to_string(), material)); } + pub fn add_medium(&self, name: &str, medium: MediumSceneEntity) { + let mut state = self.media_state.lock(); + state.entities.push((name.to_string(), medium)); + } + pub fn add_material(&self, material: SceneEntity) -> usize { let mut state = self.material_state.lock(); self.start_loading_normal_maps(&mut state, &material.parameters); @@ -498,13 +504,16 @@ impl BasicScene { } } - let mut materials: Vec = Vec::new(); - let mut named_materials: HashMap = HashMap::new(); + // Named materials must be *created* first: an anonymous material may be a + // mix that resolves its sub-materials by name. They are *placed* after the + // anonymous ones though, so that `MaterialRef::Index(i)` — an index into + // `state.materials` handed out by `add_material` — is `MaterialIdx(i)`. + let mut named_created: Vec<(String, Material)> = Vec::new(); // Value map for resolving named sub-materials (e.g. mix) during creation. let mut named_values: HashMap = HashMap::new(); for (name, entity) in &state.named_materials { - if named_materials.contains_key(name) { + if named_values.contains_key(name) { log::error!( "{}: trying to redefine named material '{}'.", entity.loc, @@ -531,10 +540,8 @@ impl BasicScene { arena, ) { Ok(mat) => { - let idx = MaterialIdx(materials.len() as u32); - materials.push(mat); named_values.insert(name.clone(), mat); - named_materials.insert(name.clone(), idx); + named_created.push((name.clone(), mat)); } Err(e) => { log::error!( @@ -547,7 +554,9 @@ impl BasicScene { } } - // Indexed (anonymous) materials, appended after named ones. + // Indexed (anonymous) materials occupy [0, state.materials.len()) so that + // a `MaterialRef::Index` needs no offset. + let mut materials: Vec = Vec::with_capacity(state.materials.len()); for entity in &state.materials { let result: Result = (|| { let normal_map = self.get_normal_map(&state, &entity.parameters)?; @@ -574,13 +583,19 @@ impl BasicScene { materials.push(mat); } + let mut named_materials: HashMap = HashMap::new(); + for (name, mat) in named_created { + named_materials.insert(name, MaterialIdx(materials.len() as u32)); + materials.push(mat); + } + let default_mtl = MaterialIdx(materials.len() as u32); materials.push(crate::core::material::default_diffuse_material(arena)); Ok((named_materials, materials, default_mtl)) } - pub fn create_media(&self) -> HashMap> { + pub fn create_media(&self, arena: &Arena) -> HashMap> { let mut state = self.media_state.lock(); if !state.jobs.is_empty() { let jobs: Vec<(String, AsyncJob)> = state.jobs.drain().collect(); @@ -588,6 +603,29 @@ impl BasicScene { state.map.insert(name, Arc::new(job.wait())); } } + + let entities: Vec<(String, MediumSceneEntity)> = state.entities.drain(..).collect(); + for (name, entity) in entities { + if entity.render_from_object.is_animated() { + log::warn!( + "{}: animated media aren't supported, using start transform.", + entity.base.loc + ); + } + match Medium::create( + &entity.base.name, + &entity.base.parameters, + entity.render_from_object.start_transform, + &entity.base.loc, + arena, + ) { + Ok(m) => { + state.map.insert(name, Arc::new(*m)); + } + Err(e) => log::error!("{}: failed to create medium: {}", entity.base.loc, e), + } + } + state.map.clone() } @@ -711,7 +749,7 @@ impl BasicScene { textures, named_materials, materials, - area_map, + Some(area_map), media, arena, ); @@ -723,7 +761,6 @@ impl BasicScene { textures, named_materials, materials, - area_map, media, arena, ); @@ -740,7 +777,7 @@ impl BasicScene { textures, named_materials, materials, - area_map, + None, media, arena, ); @@ -749,7 +786,6 @@ impl BasicScene { textures, named_materials, materials, - area_map, media, arena, ); @@ -896,13 +932,23 @@ impl BasicScene { textures: &NamedTextures, named_materials: &HashMap, materials: &[Material], - area_map: &AreaLightMap, + // `None` for object-instance definitions: the map is keyed by the + // top-level shape index, so it must not be consulted for the + // independently-numbered shapes inside an instance definition. + area_map: Option<&AreaLightMap>, media: &HashMap>, arena: &Arena, ) -> Vec { let mut primitives = Vec::new(); for (entity_idx, entity) in shapes.iter().enumerate() { + if area_map.is_none() && entity.light_index.is_some() { + log::error!( + "{}: area lights are not supported with object instancing.", + entity.base.loc + ); + } + let created_shapes = match Shape::create( &entity.base.name, *entity.render_from_object, @@ -946,8 +992,7 @@ impl BasicScene { for (sub_idx, shape) in created_shapes.into_iter().enumerate() { // look up the pre-created light index instead of creating one let light_idx = area_map - .get(&(entity_idx, sub_idx)) - .copied() + .and_then(|m| m.get(&(entity_idx, sub_idx)).copied()) .unwrap_or(LightIdx::NONE); let prim = @@ -973,7 +1018,6 @@ impl BasicScene { textures: &NamedTextures, named_materials: &HashMap, materials: &[Material], - area_map: &AreaLightMap, media: &HashMap>, arena: &Arena, ) -> Vec { @@ -1215,10 +1259,10 @@ impl BasicScene { entities: &[ShapeSceneEntity], loaded: Vec, textures: &NamedTextures, - named_materials: &HashMap, + named_materials: &HashMap, materials: &[Material], media: &HashMap>, - shape_lights: &HashMap>, + shape_lights: &AreaLightMap, ) -> Vec { // TODO: GPU wavefront path — upload shapes into device-visible arena, // build SOA primitive arrays for kernel dispatch @@ -1242,7 +1286,7 @@ impl BasicScene { entities: &[AnimatedShapeSceneEntity], loaded: Vec>, textures: &NamedTextures, - named_materials: &HashMap, + named_materials: &HashMap, materials: &[Material], media: &HashMap>, ) -> Vec { diff --git a/src/core/scene/state.rs b/src/core/scene/state.rs index 5350615..f185802 100644 --- a/src/core/scene/state.rs +++ b/src/core/scene/state.rs @@ -1,4 +1,4 @@ -use super::{LightSceneEntity, SceneEntity, TextureSceneEntity}; +use super::{LightSceneEntity, MediumSceneEntity, SceneEntity, TextureSceneEntity}; use crate::core::image::HostImage; use crate::core::texture::{FloatTexture, SpectrumTexture}; use crate::utils::parallel::AsyncJob; @@ -34,6 +34,8 @@ pub struct LightState { #[derive(Debug, Default)] pub struct MediaState { + /// Media declared by `MakeNamedMedium`, not yet instantiated. + pub entities: Vec<(String, MediumSceneEntity)>, pub jobs: HashMap>, pub map: HashMap>, } diff --git a/src/integrators/base.rs b/src/integrators/base.rs index babc6e0..bd5e4c3 100644 --- a/src/integrators/base.rs +++ b/src/integrators/base.rs @@ -73,7 +73,7 @@ impl IntegratorBase { use_mis: bool, ) { for &idx in &self.infinite_lights { - let light = &self.lights[idx.0 as usize]; + let light = idx.get(&self.lights); let le = light.le(ray, lambda); if le.is_black() { continue; diff --git a/src/integrators/path.rs b/src/integrators/path.rs index 32d5e71..0d4745a 100644 --- a/src/integrators/path.rs +++ b/src/integrators/path.rs @@ -108,7 +108,7 @@ impl PathIntegrator { return SampledSpectrum::zero(); }; - let light = &self.base.lights[sampled.light.0 as usize]; + let light = sampled.light.get(&self.base.lights); let Some(ls) = light.sample_li(&ctx, sampler.get2d(), lambda, true) else { return SampledSpectrum::zero(); @@ -240,7 +240,7 @@ impl RayIntegratorTrait for PathIntegrator { state.l += state.beta * le; } else if self.config.use_mis && !isect.area_light.is_none() { let idx = isect.area_light; - let light = &self.base.lights[idx.0 as usize]; + let light = idx.get(&self.base.lights); let p_l = self.sampler.pmf_with_context(&state.prev_ctx, idx) * light.pdf_li(&state.prev_ctx, ray.d, true); let w_b = power_heuristic(1, state.prev_pdf, 1, p_l); diff --git a/src/utils/parameters.rs b/src/utils/parameters.rs index 98e8d9d..0fbaffe 100644 --- a/src/utils/parameters.rs +++ b/src/utils/parameters.rs @@ -281,7 +281,10 @@ impl ParameterDictionary { params: &[ParsedParameter], color_space: Option>, ) -> Result { - let n_owned_params = params.len(); + // paramdict.cpp:150 — the owned params are `p0`; the inherited attribute + // vector is appended (reversed) after `p0` has itself been reversed. + let n_owned_params = p0.len(); + p0.reverse(); p0.extend(params.iter().rev().cloned()); let dict = Self { diff --git a/src/wavefront/aggregate.rs b/src/wavefront/aggregate.rs index 7689a23..c1c80b9 100644 --- a/src/wavefront/aggregate.rs +++ b/src/wavefront/aggregate.rs @@ -101,7 +101,7 @@ impl WavefrontAggregate for CpuAggregate { } // Material eval queue dispatch - let material = &self.materials[intr.material.0 as usize]; + let material = intr.material.get(&self.materials); let eval_q = if material.can_evaluate_textures(&BasicTextureEvaluator) { basic_eval_mtl_q } else { diff --git a/src/wavefront/integrator.rs b/src/wavefront/integrator.rs index 3f6fba6..d3b9d09 100644 --- a/src/wavefront/integrator.rs +++ b/src/wavefront/integrator.rs @@ -364,7 +364,7 @@ impl CpuWavefrontRenderer { let mut l_contrib = SampledSpectrum::new(0.0); for idx in infinite_lights { - let light = &self.lights[idx.0 as usize]; + let light = idx.get(&self.lights); let ray = Ray::new(w.ray_o, w.ray_d, None, Ptr::null()); let le = light.le(&ray, &w.lambda); if le.is_black() { @@ -402,7 +402,7 @@ impl CpuWavefrontRenderer { if w.area_light.is_none() { return; } - let light = &self.lights[w.area_light.0 as usize]; + let light = w.area_light.get(&self.lights); let le = light.l(w.p, w.n, w.uv, w.wo, &w.lambda); if le.is_black() { @@ -475,7 +475,7 @@ impl CpuWavefrontRenderer { if w.material.is_none() { return; } - let material = &self.materials[w.material.0 as usize]; + let material = w.material.get(&self.materials); let pi = w.pixel_index as usize; let rs = pixel_sample_state.samples.get(pi); @@ -604,7 +604,7 @@ impl CpuWavefrontRenderer { DIAG_SAMPLE_LIGHT_NONE.fetch_add(1, Ordering::Relaxed); return; }; - let light = &self.lights[sampled_light.light.0 as usize]; + let light = sampled_light.light.get(&self.lights); let Some(ls) = light.sample_li(&light_ctx, rs.direct.u, &lambda, true) else { DIAG_SAMPLE_LI_NONE.fetch_add(1, Ordering::Relaxed);