From 86610caf8abc676db5c5506ffe65f34c20bf89bc Mon Sep 17 00:00:00 2001 From: Orual Date: Mon, 10 Aug 2026 00:12:21 -0400 Subject: [PATCH] PM-84: eliminate per-occurrence geometry cloning, continue typed error migration --- .../polymodel-renderer-protocol/src/scene.rs | 46 ++-- .../src/bin/ldraw_preview.rs | 12 +- crates/polymodel-renderer-worker/src/lib.rs | 218 ++++++++++-------- crates/polymodel-renderer-worker/src/tests.rs | 29 ++- 4 files changed, 185 insertions(+), 120 deletions(-) diff --git a/crates/polymodel-renderer-protocol/src/scene.rs b/crates/polymodel-renderer-protocol/src/scene.rs index ba849aa..86819d9 100644 --- a/crates/polymodel-renderer-protocol/src/scene.rs +++ b/crates/polymodel-renderer-protocol/src/scene.rs @@ -157,6 +157,9 @@ pub struct SceneOccurrence { #[serde(default)] pub logical_part_path: Option, pub node: NodeId, + /// Optional per-occurrence UV overlay; the referenced geometry remains shared. + #[serde(default)] + pub uvs: Option>, pub definition_id: SmolStr, pub canonical_path: SmolStr, pub source_class: SmolStr, @@ -203,50 +206,62 @@ pub struct Scene { pub stats: SceneStats, } +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum SceneValidationError { + #[error("duplicate geometry id")] + DuplicateGeometryId, + #[error("occurrence references unknown geometry")] + UnknownGeometry, + #[error("{0} count overflow")] + CountOverflow(&'static str), +} + impl Scene { - pub fn recompute_stats(&mut self) -> Result<(), &'static str> { + pub fn recompute_stats(&mut self) -> Result<(), SceneValidationError> { use std::collections::HashMap; let mut geometries = HashMap::with_capacity(self.geometries.len()); for geometry in &self.geometries { if geometries.insert(geometry.id.0, geometry).is_some() { - return Err("duplicate geometry id"); + return Err(SceneValidationError::DuplicateGeometryId); } } let mut stats = SceneStats { occurrences: u64::try_from(self.occurrences.len()) - .map_err(|_| "occurrence count overflow")?, + .map_err(|_| SceneValidationError::CountOverflow("occurrence"))?, ..SceneStats::default() }; for occurrence in &self.occurrences { let geometry = geometries .get(&occurrence.node.0) - .ok_or("occurrence references unknown geometry")?; - let points = - u64::try_from(geometry.points.len()).map_err(|_| "point count overflow")?; - let triangles = - u64::try_from(geometry.triangles.len()).map_err(|_| "triangle count overflow")?; - let quads = u64::try_from(geometry.quads.len()).map_err(|_| "quad count overflow")?; - let lines = u64::try_from(geometry.lines.len()).map_err(|_| "line count overflow")?; + .ok_or(SceneValidationError::UnknownGeometry)?; + let points = u64::try_from(geometry.points.len()) + .map_err(|_| SceneValidationError::CountOverflow("point"))?; + let triangles = u64::try_from(geometry.triangles.len()) + .map_err(|_| SceneValidationError::CountOverflow("triangle"))?; + let quads = u64::try_from(geometry.quads.len()) + .map_err(|_| SceneValidationError::CountOverflow("quad"))?; + let lines = u64::try_from(geometry.lines.len()) + .map_err(|_| SceneValidationError::CountOverflow("line"))?; let conditional_lines = u64::try_from(geometry.conditional_lines.len()) - .map_err(|_| "conditional-line count overflow")?; + .map_err(|_| SceneValidationError::CountOverflow("conditional-line"))?; stats.points = stats .points .checked_add(points) - .ok_or("point count overflow")?; + .ok_or(SceneValidationError::CountOverflow("point"))?; stats.triangles = stats .triangles .checked_add(triangles) .and_then(|count| count.checked_add(quads.checked_mul(2)?)) - .ok_or("triangle count overflow")?; + .ok_or(SceneValidationError::CountOverflow("triangle"))?; stats.lines = stats .lines .checked_add(lines) - .ok_or("line count overflow")?; + .ok_or(SceneValidationError::CountOverflow("line"))?; stats.conditional_lines = stats .conditional_lines .checked_add(conditional_lines) - .ok_or("conditional-line count overflow")?; + .ok_or(SceneValidationError::CountOverflow("conditional-line"))?; } self.stats = stats; Ok(()) @@ -293,6 +308,7 @@ mod tests { logical_part_occurrence_id: None, logical_part_path: None, node: NodeId(1), + uvs: None, definition_id: "root".into(), canonical_path: "omr-10030-1.mpd".into(), source_class: "project".into(), diff --git a/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs b/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs index 28e3120..2d9e25d 100644 --- a/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs +++ b/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs @@ -10,8 +10,8 @@ use polymodel_renderer_protocol::scene::Scene; #[cfg(not(target_arch = "wasm32"))] use polymodel_renderer_worker::{ NativeCaptureMetadata, NativeLights, NativeSelectionSlots, NativeStage, NativeTerminal, - NativeTrace, PickRay, RenderStyle, ScenePicker, SceneRenderPlan, SceneVariantBinding, - SelectionFallback, SelectionInteraction, SelectionPointerAction, + NativeTrace, PickRay, RenderStyle, RendererError, ScenePicker, SceneRenderPlan, + SceneVariantBinding, SelectionFallback, SelectionInteraction, SelectionPointerAction, build_conditional_edge_models_for_view, build_scene_model_parts_for_style, build_selected_conditional_edge_models_for_view, build_selected_edge_models_for_view, camera_space_clip_planes, diagnostic_edge_model_slot, diagnostic_edge_models, @@ -972,8 +972,12 @@ fn run_visible_preview( let started = std::time::Instant::now(); let result = (|| { let (x, y) = (select_x, select_y); - let active_camera = camera.as_ref().ok_or("missing-camera")?; - let picker = scene_picker.as_ref().ok_or("missing-picker")?; + let active_camera = camera + .as_ref() + .ok_or_else(|| RendererError::invalid("missing-camera"))?; + let picker = scene_picker + .as_ref() + .ok_or_else(|| RendererError::invalid("missing-picker"))?; let viewport = active_camera.viewport(); let pixel = three_d::vec2( x as f32, diff --git a/crates/polymodel-renderer-worker/src/lib.rs b/crates/polymodel-renderer-worker/src/lib.rs index 36e27ec..5713e89 100644 --- a/crates/polymodel-renderer-worker/src/lib.rs +++ b/crates/polymodel-renderer-worker/src/lib.rs @@ -149,7 +149,7 @@ pub fn basis_determinant() -> f64 { 1.0 } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, PartialEq, Eq, thiserror::Error)] pub enum RendererError { #[error("invalid scene data: {0}")] InvalidScene(String), @@ -163,8 +163,14 @@ pub enum RendererError { ParseAdaptation(String), } +impl From for String { + fn from(error: RendererError) -> Self { + error.to_string() + } +} + impl RendererError { - fn invalid(message: impl Into) -> Self { + pub fn invalid(message: impl Into) -> Self { Self::InvalidScene(message.into()) } @@ -641,7 +647,7 @@ fn adapt_ldraw_assembly( [0.0, 0.0, 1.0, 0.0], [0.0, 0.0, 0.0, 1.0], ]; - let mut geometry_indices = geometries + let geometry_indices = geometries .iter() .enumerate() .map(|(index, geometry)| (geometry.id, index)) @@ -667,6 +673,7 @@ fn adapt_ldraw_assembly( logical_part_occurrence_id: None, logical_part_path: None, node: *node, + uvs: None, definition_id: root_metadata.definition_id.clone(), canonical_path: root_metadata.canonical_path.clone(), source_class: root_metadata.source_class.clone(), @@ -722,29 +729,16 @@ fn adapt_ldraw_assembly( let inherited_colour = occurrence.colour.map(u32::from).unwrap_or(16); let transform = world_transform(occurrence.transform)?; for (node, primitive) in batches { - let render_node = if let Some(texmap) = primitive.texmap.as_ref() { + let (render_node, occurrence_uvs) = if let Some(texmap) = primitive.texmap.as_ref() { let source_index = geometry_indices .get(&GeometryId(node.0)) .copied() .ok_or_else(|| RendererError::invalid("occurrence geometry is missing"))?; - let mut occurrence_geometry = geometries[source_index].clone(); - occurrence_geometry.id = GeometryId( - geometries - .last() - .map(|geometry| geometry.id.0) - .unwrap_or(0) - .checked_add(1) - .ok_or_else(|| RendererError::invalid("geometry id overflow"))?, - ); - occurrence_geometry.uvs = - occurrence_texmap_uvs(texmap, transform, &occurrence_geometry.points)?; - let render_node = NodeId(occurrence_geometry.id.0); - let occurrence_index = geometries.len(); - geometry_indices.insert(occurrence_geometry.id, occurrence_index); - geometries.push(occurrence_geometry); - render_node + let occurrence_uvs = + occurrence_texmap_uvs(texmap, transform, &geometries[source_index].points)?; + (*node, Some(occurrence_uvs)) } else { - *node + (*node, None) }; let effective_colour = match primitive.colour { 16 => inherited_colour, @@ -758,6 +752,7 @@ fn adapt_ldraw_assembly( logical_part_occurrence_id: logical_part.as_ref().map(|(id, _)| id.clone()), logical_part_path: logical_part.as_ref().map(|(_, path)| path.clone()), node: render_node, + uvs: occurrence_uvs, definition_id: metadata.definition_id.clone(), canonical_path: metadata.canonical_path.clone(), source_class: metadata.source_class.clone(), @@ -960,7 +955,7 @@ fn adapt_ldraw_assembly( }; scene .recompute_stats() - .map_err(|error| RendererError::invalid(error))?; + .map_err(|error| RendererError::invalid(error.to_string()))?; Ok(scene) } @@ -2219,7 +2214,7 @@ pub fn scene_oriented_mesh_topology( bfc_ccw: bool, bfc_inverted: bool, reflection: bool, -) -> Result { +) -> Result { scene_oriented_mesh_topology_for_view( geometry, bfc_cull, @@ -2237,7 +2232,7 @@ fn scene_oriented_mesh_topology_for_view( bfc_inverted: bool, reflection: bool, view: [f64; 3], -) -> Result { +) -> Result { scene_oriented_mesh_topology_for_view_kind( geometry, bfc_cull, @@ -2257,7 +2252,7 @@ fn scene_oriented_mesh_topology_for_view_kind( reflection: bool, view: [f64; 3], kind: SceneTopologyKind, -) -> Result { +) -> Result { let mut topology = scene_mesh_topology_for_view_kind(geometry, view, kind)?; // `bfc_inverted` is the authoritative effective winding bit produced by // traversal. It already includes accumulated transform reflection; applying @@ -2272,14 +2267,14 @@ fn scene_oriented_mesh_topology_for_view_kind( Ok(topology) } -pub fn scene_mesh_topology(geometry: &SceneGeometry) -> Result { +pub fn scene_mesh_topology(geometry: &SceneGeometry) -> Result { scene_mesh_topology_for_view(geometry, [0.0, 0.0, 1.0]) } pub fn scene_mesh_topology_for_view( geometry: &SceneGeometry, view: [f64; 3], -) -> Result { +) -> Result { scene_mesh_topology_for_view_kind(geometry, view, SceneTopologyKind::Complete) } @@ -2293,7 +2288,7 @@ fn scene_mesh_topology_for_view_kind( geometry: &SceneGeometry, view: [f64; 3], kind: SceneTopologyKind, -) -> Result { +) -> Result { let include_faces = matches!(kind, SceneTopologyKind::Faces | SceneTopologyKind::Complete); let include_explicit = matches!(kind, SceneTopologyKind::Complete); let include_conditional = matches!(kind, SceneTopologyKind::Complete); @@ -2305,10 +2300,10 @@ fn scene_mesh_topology_for_view_kind( if converted.iter().all(|coordinate| coordinate.is_finite()) { Ok(converted) } else { - Err(format!( + Err(RendererError::invalid(format!( "geometry {} contains a non-finite position", geometry.id.0 - )) + ))) } }) .collect::, _>>()?; @@ -2327,10 +2322,10 @@ fn scene_mesh_topology_for_view_kind( .flatten() .any(|value| !value.is_finite()) { - return Err(format!( + return Err(RendererError::invalid(format!( "geometry {} has invalid UV alignment", geometry.id.0 - )); + ))); } else { geometry.uvs.clone() } @@ -2350,24 +2345,24 @@ fn scene_mesh_topology_for_view_kind( indices.extend_from_slice(&[quad[0], quad[1], quad[2], quad[0], quad[2], quad[3]]); } } - let mut add_line = |line: [u32; 2]| -> Result<(), String> { + let mut add_line = |line: [u32; 2]| -> Result<(), RendererError> { let start = source_positions .get(line[0] as usize) .copied() .ok_or_else(|| { - format!( + RendererError::invalid(format!( "geometry {} line start is outside its position domain", geometry.id.0 - ) + )) })?; let end = source_positions .get(line[1] as usize) .copied() .ok_or_else(|| { - format!( + RendererError::invalid(format!( "geometry {} line end is outside its position domain", geometry.id.0 - ) + )) })?; let direction = [ f64::from(end[0] - start[0]), @@ -2422,8 +2417,8 @@ fn scene_mesh_topology_for_view_kind( (side[1] / side_length * radius) as f32, (side[2] / side_length * radius) as f32, ]; - let base = - u32::try_from(positions.len()).map_err(|_| "line vertex range overflow".to_owned())?; + let base = u32::try_from(positions.len()) + .map_err(|_| RendererError::invalid("line vertex range overflow"))?; positions.extend([ [ start[0] - offset[0], @@ -2452,11 +2447,11 @@ fn scene_mesh_topology_for_view_kind( .iter() .map(|index| { geometry.points.get(*index as usize).copied().ok_or_else(|| { - format!( + RendererError::invalid(format!( "geometry {} conditional-line index {index} is outside its local {}-position domain", geometry.id.0, geometry.points.len() - ) + )) }) }) .collect::, _>>()?; @@ -2468,21 +2463,21 @@ fn scene_mesh_topology_for_view_kind( } } if indices.len() % 3 != 0 { - return Err(format!( + return Err(RendererError::invalid(format!( "geometry {} has an incomplete triangle index stream", geometry.id.0 - )); + ))); } if let Some(index) = indices .iter() .copied() .find(|index| *index as usize >= positions.len()) { - return Err(format!( + return Err(RendererError::invalid(format!( "geometry {} index {index} is outside its local {}-position domain", geometry.id.0, positions.len() - )); + ))); } Ok(SceneMeshTopology { positions, @@ -2557,23 +2552,29 @@ struct SceneIndex { } impl SceneIndex { - fn new(scene: &Scene) -> Result { + fn new(scene: &Scene) -> Result { let mut geometries = std::collections::HashMap::with_capacity(scene.geometries.len()); for (index, geometry) in scene.geometries.iter().enumerate() { if geometry.id.0 == 0 { - return Err("geometry id zero is invalid".to_owned()); + return Err(RendererError::invalid("geometry id zero is invalid")); } if geometries.insert(geometry.id, index).is_some() { - return Err(format!("duplicate geometry id {}", geometry.id.0)); + return Err(RendererError::invalid(format!( + "duplicate geometry id {}", + geometry.id.0 + ))); } } let mut materials = std::collections::HashMap::with_capacity(scene.materials.len()); for (index, material) in scene.materials.iter().enumerate() { if material.id.0 == 0 { - return Err("material id zero is invalid".to_owned()); + return Err(RendererError::invalid("material id zero is invalid")); } if materials.insert(material.id, index).is_some() { - return Err(format!("duplicate material id {}", material.id.0)); + return Err(RendererError::invalid(format!( + "duplicate material id {}", + material.id.0 + ))); } } let index = Self { @@ -2592,39 +2593,49 @@ impl SceneIndex { .flatten() .any(|value| !value.is_finite()) { - return Err(format!( + return Err(RendererError::invalid(format!( "occurrence {} has a non-finite transform", occurrence.id.0 - )); + ))); } } Ok(index) } - fn geometry<'a>(&self, scene: &'a Scene, node: NodeId) -> Result<&'a SceneGeometry, String> { + fn geometry<'a>( + &self, + scene: &'a Scene, + node: NodeId, + ) -> Result<&'a SceneGeometry, RendererError> { let id = GeometryId(node.0); self.geometries .get(&id) .and_then(|index| scene.geometries.get(*index)) - .ok_or_else(|| format!("unknown geometry id {}", node.0)) + .ok_or_else(|| RendererError::invalid(format!("unknown geometry id {}", node.0))) } - fn geometry_index(&self, node: NodeId) -> Result { + fn geometry_index(&self, node: NodeId) -> Result { self.geometries .get(&GeometryId(node.0)) .copied() - .ok_or_else(|| format!("unknown geometry id {}", node.0)) + .ok_or_else(|| RendererError::invalid(format!("unknown geometry id {}", node.0))) } - fn material<'a>(&self, scene: &'a Scene, id: MaterialId) -> Result<&'a SceneMaterial, String> { + fn material<'a>( + &self, + scene: &'a Scene, + id: MaterialId, + ) -> Result<&'a SceneMaterial, RendererError> { self.materials .get(&id) .and_then(|index| scene.materials.get(*index)) - .ok_or_else(|| format!("unknown material id {}", id.0)) + .ok_or_else(|| RendererError::invalid(format!("unknown material id {}", id.0))) } } -pub fn occurrence_gpu_ranges(scene: &Scene) -> Result>, String> { +pub fn occurrence_gpu_ranges( + scene: &Scene, +) -> Result>, RendererError> { Ok(scene_render_plan(scene)?.occurrence_ranges) } @@ -2632,14 +2643,14 @@ fn finalized_upload_batches( index: &SceneIndex, batches: &[SceneInstanceBatch], topologies: &[SceneMeshTopology], -) -> Result<(Vec, Vec>), String> { +) -> Result<(Vec, Vec>), RendererError> { let mut upload_batches = Vec::new(); let mut planned_to_upload = vec![None; batches.len()]; for (planned_index, batch) in batches.iter().enumerate() { let geometry_index = index.geometry_index(batch.node)?; let topology = topologies .get(geometry_index) - .ok_or("render plan is missing geometry topology")?; + .ok_or_else(|| RendererError::invalid("render plan is missing geometry topology"))?; if topology.indices.is_empty() { continue; } @@ -2654,24 +2665,26 @@ fn occurrence_gpu_ranges_from_batches( index: &SceneIndex, batches: &[SceneInstanceBatch], topologies: &[SceneMeshTopology], -) -> Result>, String> { +) -> Result>, RendererError> { let mut ranges = vec![None; scene.occurrences.len()]; for (upload_batch_index, batch) in batches.iter().enumerate() { let geometry_index = index.geometry_index(batch.node)?; let topology = topologies .get(geometry_index) - .ok_or("render plan is missing geometry topology")?; - let vertex_count = - u32::try_from(topology.positions.len()).map_err(|_| "vertex range overflow")?; - let index_count = - u32::try_from(topology.indices.len()).map_err(|_| "index range overflow")?; + .ok_or_else(|| RendererError::invalid("render plan is missing geometry topology"))?; + let vertex_count = u32::try_from(topology.positions.len()) + .map_err(|_| RendererError::invalid("vertex range overflow"))?; + let index_count = u32::try_from(topology.indices.len()) + .map_err(|_| RendererError::invalid("index range overflow"))?; for (instance_index, instance) in batch.instances.iter().enumerate() { let range = ranges .get_mut(instance.occurrence_index) - .ok_or("batch references an unknown occurrence")?; + .ok_or_else(|| RendererError::invalid("batch references an unknown occurrence"))?; if range.is_some() { - return Err("occurrence appears in multiple native GPU batches".to_owned()); + return Err(RendererError::invalid( + "occurrence appears in multiple native GPU batches", + )); } *range = Some(OccurrenceGpuRange { batch: upload_batch_index, @@ -3005,9 +3018,11 @@ pub struct ScenePicker { } impl ScenePicker { - pub fn new(scene: &Scene, plan: SceneRenderPlan) -> Result { + pub fn new(scene: &Scene, plan: SceneRenderPlan) -> Result { if plan.planned_occurrences.len() != scene.occurrences.len() { - return Err("render plan pick occurrences are incomplete".to_owned()); + return Err(RendererError::invalid( + "render plan pick occurrences are incomplete", + )); } let bvh = PickBvh::new(&plan.logical_bounds); let logical_part_indices = plan @@ -3042,17 +3057,18 @@ impl ScenePicker { self.logical_part_bounds.get(occurrence_id).copied() } - pub fn selection(&self, raw_index: usize) -> Result { - let raw = self - .occurrences - .get(raw_index) - .ok_or("raw occurrence is outside the render-plan cache")?; + pub fn selection(&self, raw_index: usize) -> Result { + let raw = self.occurrences.get(raw_index).ok_or_else(|| { + RendererError::invalid("raw occurrence is outside the render-plan cache") + })?; let logical_part_occurrence_id = raw.logical_part_occurrence_id.clone(); let group_index = self .logical_part_indices .get(&logical_part_occurrence_id) .copied() - .ok_or("logical part is missing from the render-plan cache")?; + .ok_or_else(|| { + RendererError::invalid("logical part is missing from the render-plan cache") + })?; let occurrence_indices = self.logical_parts[group_index].clone(); let transforms = occurrence_indices .iter() @@ -3080,7 +3096,7 @@ impl ScenePicker { &self.occurrences } - pub fn pick(&self, ray: PickRay) -> Result, String> { + pub fn pick(&self, ray: PickRay) -> Result, RendererError> { let mut candidate_parts = Vec::new(); if let Some(bvh) = &self.bvh { bvh.candidates(ray, f64::INFINITY, &mut candidate_parts); @@ -3126,7 +3142,7 @@ impl ScenePicker { f64::from(point[2]), ], ) - .map_err(|error| error.to_string()) + .map_err(|error| RendererError::invalid(error.to_string())) }; ray_triangle_distance( ray, @@ -3153,7 +3169,7 @@ impl ScenePicker { } #[cfg(not(target_arch = "wasm32"))] -fn pick_scene(scene: &Scene, ray: PickRay) -> Result, String> { +fn pick_scene(scene: &Scene, ray: PickRay) -> Result, RendererError> { let plan = scene_render_plan(scene)?; ScenePicker::new(scene, plan)?.pick(ray) } @@ -3162,7 +3178,7 @@ fn pick_scene(scene: &Scene, ray: PickRay) -> Result, Strin fn logical_part_selection( picker: &ScenePicker, raw_index: usize, -) -> Result { +) -> Result { picker.selection(raw_index) } @@ -3204,6 +3220,8 @@ pub struct SceneBatchInstance { #[derive(Debug, Clone, PartialEq)] pub struct SceneInstanceBatch { pub node: NodeId, + /// UVs owned by a textured occurrence; base geometry remains shared by node. + pub uvs: Option>, pub material: MaterialId, pub bfc_cull: bool, pub bfc_ccw: bool, @@ -3231,7 +3249,7 @@ pub struct SceneRenderPlan { index: SceneIndex, } -fn planned_edge_source(geometry: &SceneGeometry) -> Result { +fn planned_edge_source(geometry: &SceneGeometry) -> Result { let explicit = geometry .lines .iter() @@ -3268,7 +3286,7 @@ fn planned_edge_source(geometry: &SceneGeometry) -> Result Result { +pub fn scene_render_plan(scene: &Scene) -> Result { #[cfg(not(target_arch = "wasm32"))] let diagnostics = std::env::var_os("POLYMODEL_LDRAW_PLAN_DIAGNOSTICS").is_some(); #[cfg(target_arch = "wasm32")] @@ -3302,10 +3320,10 @@ pub fn scene_render_plan(scene: &Scene) -> Result { ] })) .ok_or_else(|| { - format!( + RendererError::invalid(format!( "geometry {} has no finite render bounds", scene.geometries[geometry_index].id.0 - ) + )) }) }) .collect::, _>>()?; @@ -3371,7 +3389,7 @@ fn scene_instance_batches_for_mode( index: &SceneIndex, local_bounds: &[PickBounds], accumulator: &mut SceneBatchAccumulator<'_>, -) -> Result, String> { +) -> Result, RendererError> { #[derive(Clone, Copy, PartialEq, Eq, Hash)] struct BatchKey { node: NodeId, @@ -3380,6 +3398,7 @@ fn scene_instance_batches_for_mode( reverse_winding: bool, reflection: bool, _conditional_occurrence: Option, + textured_occurrence: Option, } let default_material = scene @@ -3394,7 +3413,7 @@ fn scene_instance_batches_for_mode( let local = local_bounds[geometry_index]; let bounds = local .transformed(occurrence.transform) - .ok_or("occurrence has no finite render bounds")?; + .ok_or_else(|| RendererError::invalid("occurrence has no finite render bounds"))?; let logical_part_occurrence_id = occurrence .logical_part_occurrence_id .clone() @@ -3446,8 +3465,9 @@ fn scene_instance_batches_for_mode( }); let material = occurrence.material.unwrap_or(default_material); let reverse_winding = occurrence.bfc_inverted ^ !occurrence.bfc_ccw; - let reflection = scene_transform_reflection(occurrence.transform) - .ok_or("occurrence has a degenerate or non-finite transform")?; + let reflection = scene_transform_reflection(occurrence.transform).ok_or_else(|| { + RendererError::invalid("occurrence has a degenerate or non-finite transform") + })?; let key = BatchKey { node: occurrence.node, material, @@ -3455,6 +3475,7 @@ fn scene_instance_batches_for_mode( reverse_winding, reflection, _conditional_occurrence: None, + textured_occurrence: occurrence.uvs.as_ref().map(|_| occurrence_index), }; let instance = SceneBatchInstance { occurrence_index, @@ -3466,6 +3487,7 @@ fn scene_instance_batches_for_mode( let index = batches.len(); batches.push(SceneInstanceBatch { node: occurrence.node, + uvs: occurrence.uvs.clone(), material, bfc_cull: occurrence.bfc_cull, bfc_ccw: occurrence.bfc_ccw, @@ -4955,9 +4977,9 @@ fn build_scene_model_from_batches( return Ok(None); } let geometry_index = index.geometry_index(batch.node)?; - let topology = topologies - .get(geometry_index) - .ok_or("render plan is missing geometry topology")?; + let topology = topologies.get(geometry_index).ok_or_else(|| { + RendererError::invalid("render plan is missing geometry topology") + })?; if topology.indices.is_empty() { return Ok(None); } @@ -4976,13 +4998,11 @@ fn build_scene_model_from_batches( .collect(), ), indices: Indices::U32(indices), - uvs: (!topology.uvs.is_empty()).then(|| { - topology - .uvs - .iter() - .map(|uv| three_d::vec2(uv[0], uv[1])) - .collect() - }), + uvs: batch + .uvs + .as_deref() + .or_else(|| (!topology.uvs.is_empty()).then_some(topology.uvs.as_slice())) + .map(|uvs| uvs.iter().map(|uv| three_d::vec2(uv[0], uv[1])).collect()), ..Default::default() }; cpu_mesh.compute_normals(); diff --git a/crates/polymodel-renderer-worker/src/tests.rs b/crates/polymodel-renderer-worker/src/tests.rs index 8b31ce2..08c325b 100644 --- a/crates/polymodel-renderer-worker/src/tests.rs +++ b/crates/polymodel-renderer-worker/src/tests.rs @@ -631,6 +631,7 @@ fn picker_scene() -> Scene { logical_part_occurrence_id: Some(smol_str::format_smolstr!("root/{id}")), logical_part_path: Some("parts/triangle.dat".into()), node: NodeId(1), + uvs: None, definition_id: "triangle.dat".into(), canonical_path: "parts/triangle.dat".into(), source_class: "part".into(), @@ -678,6 +679,22 @@ fn picker_scene() -> Scene { } } +#[test] +fn textured_occurrences_share_geometry_and_split_uv_batches() { + let mut scene = picker_scene(); + let uv_a = vec![[0.0, 0.0], [1.0, 0.0], [0.0, 1.0]]; + let uv_b = vec![[0.25, 0.25], [0.75, 0.25], [0.25, 0.75]]; + scene.occurrences[0].uvs = Some(uv_a.clone()); + scene.occurrences[1].uvs = Some(uv_b.clone()); + + let plan = scene_render_plan(&scene).expect("render plan"); + assert_eq!(scene.geometries.len(), 1); + assert_eq!(scene.occurrences[0].node, scene.occurrences[1].node); + assert_eq!(plan.batches.len(), 2); + assert_eq!(plan.batches[0].uvs, Some(uv_a)); + assert_eq!(plan.batches[1].uvs, Some(uv_b)); +} + #[test] fn selection_interaction_requires_shift_and_pointer_up() { let mut interaction = SelectionInteraction::default(); @@ -885,6 +902,7 @@ fn empty_face_batch_does_not_shift_later_gpu_ranges() { logical_part_occurrence_id: None, logical_part_path: None, node: NodeId(99), + uvs: None, definition_id: "edge-only".into(), canonical_path: "edge-only.ldr".into(), source_class: "CurrentMpd".into(), @@ -1040,6 +1058,7 @@ fn scene_consumers_resolve_sparse_opaque_ids_and_reject_invalid_references() { assert!( occurrence_gpu_ranges(&duplicate) .unwrap_err() + .to_string() .contains("duplicate geometry id") ); @@ -1048,6 +1067,7 @@ fn scene_consumers_resolve_sparse_opaque_ids_and_reject_invalid_references() { assert!( occurrence_gpu_ranges(&dangling_geometry) .unwrap_err() + .to_string() .contains("unknown geometry id 99") ); @@ -1056,6 +1076,7 @@ fn scene_consumers_resolve_sparse_opaque_ids_and_reject_invalid_references() { assert!( occurrence_gpu_ranges(&dangling_material) .unwrap_err() + .to_string() .contains("unknown material id 99") ); } @@ -1249,7 +1270,11 @@ fn native_topology_rejects_foreign_index_domains() { Vec::new(), )) .unwrap_err(); - assert!(error.contains("outside its local 3-position domain")); + assert!( + error + .to_string() + .contains("outside its local 3-position domain") + ); } #[cfg(not(target_arch = "wasm32"))] @@ -1331,7 +1356,7 @@ fn native_topology_rejects_malformed_conditional_line_indices() { ); geometry.conditional_lines = vec![[0, 1, 2, 3]]; let error = scene_mesh_topology(&geometry).unwrap_err(); - assert!(error.contains("conditional-line index 2")); + assert!(error.to_string().contains("conditional-line index 2")); } #[cfg(not(target_arch = "wasm32"))] -- knotmirror 0.0.1