Continuing migration to index and GVec based usage of objects

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 {

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<Light>, len: u32, light: &Light) -> Option<usize> {
         let target = light as *const Light;
-        for i in 0..len as usize {
+        (0..len as usize).find(|&i| unsafe { base.add(i) }.as_raw() == target)
-            if unsafe { base.add(i) }.as_raw() == target {
-                return Some(i);
-            }
-        }
-        None
     }
 
     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 {

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);

src/core/medium.rs

@@ -291,6 +291,8 @@ pub trait CreateMedium {
     }
 }
 
+impl CreateMedium for Medium {}
+
 fn create_homogeneous(
     parameters: &ParameterDictionary,
     loc: &FileLoc,

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);

src/core/scene/builder.rs

@@ -152,11 +152,20 @@ impl BasicSceneBuilder {
     }
 
     pub fn new(scene: Arc<BasicScene>) -> 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 {
+        let parameters = ParameterDictionary::from_array(
-            name: name.to_string(),
+            params,
-            loc,
+            &self.graphics_state.medium_attributes,
-            parameters,
+            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,
+        name: &str,
-        _params: ParsedParameterVector,
+        params: ParsedParameterVector,
-        _loc: FileLoc,
+        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> {
+    fn named_material(&mut self, name: &str, loc: FileLoc) -> Result<(), ParserError> {
-        todo!()
+        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(

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);
-                // log::error!("{}: named material '{}' not found", loc, name);
+                MaterialIdx::NONE
-                // crate::core::material::default_diffuse_material(arena)
             }
         },
+        // 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);
-                // log::error!("{}: material index {} out of bounds", loc, idx);
+                MaterialIdx::NONE
-                // crate::core::material::default_diffuse_material(arena)
             }
         }
-        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<Material> = Vec::new();
+        // Named materials must be *created* first: an anonymous material may be a
-        let mut named_materials: HashMap<String, MaterialIdx> = HashMap::new();
+        // 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<String, Material> = 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<Material> = Vec::with_capacity(state.materials.len());
         for entity in &state.materials {
             let result: Result<Material> = (|| {
                 let normal_map = self.get_normal_map(&state, &entity.parameters)?;
@@ -574,13 +583,19 @@ impl BasicScene {
             materials.push(mat);
         }
 
+        let mut named_materials: HashMap<String, MaterialIdx> = 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<String, Arc<Medium>> {
+    pub fn create_media(&self, arena: &Arena) -> HashMap<String, Arc<Medium>> {
         let mut state = self.media_state.lock();
         if !state.jobs.is_empty() {
             let jobs: Vec<(String, AsyncJob<Medium>)> = 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<String, MaterialIdx>,
         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<String, Arc<Medium>>,
         arena: &Arena,
     ) -> Vec<Primitive> {
         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))
+                    .and_then(|m| m.get(&(entity_idx, sub_idx)).copied())
-                    .copied()
                     .unwrap_or(LightIdx::NONE);
 
                 let prim =
@@ -973,7 +1018,6 @@ impl BasicScene {
         textures: &NamedTextures,
         named_materials: &HashMap<String, MaterialIdx>,
         materials: &[Material],
-        area_map: &AreaLightMap,
         media: &HashMap<String, Arc<Medium>>,
         arena: &Arena,
     ) -> Vec<Primitive> {
@@ -1215,10 +1259,10 @@ impl BasicScene {
         entities: &[ShapeSceneEntity],
         loaded: Vec<ShapeWithContext>,
         textures: &NamedTextures,
-        named_materials: &HashMap<String, Material>,
+        named_materials: &HashMap<String, MaterialIdx>,
         materials: &[Material],
         media: &HashMap<String, Arc<Medium>>,
-        shape_lights: &HashMap<usize, Vec<Light>>,
+        shape_lights: &AreaLightMap,
     ) -> Vec<Primitive> {
         // 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<Ptr<Shape>>,
         textures: &NamedTextures,
-        named_materials: &HashMap<String, Material>,
+        named_materials: &HashMap<String, MaterialIdx>,
         materials: &[Material],
         media: &HashMap<String, Arc<Medium>>,
     ) -> Vec<Primitive> {

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<String, AsyncJob<Medium>>,
     pub map: HashMap<String, Arc<Medium>>,
 }

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;

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);

src/utils/parameters.rs

@@ -281,7 +281,10 @@ impl ParameterDictionary {
         params: &[ParsedParameter],
         color_space: Option<Arc<RGBColorSpace>>,
     ) -> Result<Self> {
-        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 {

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 {

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);