diff --git a/crates/polymodel-ldraw-core/src/lib.rs b/crates/polymodel-ldraw-core/src/lib.rs index b0a359c..5caec9a 100644 --- a/crates/polymodel-ldraw-core/src/lib.rs +++ b/crates/polymodel-ldraw-core/src/lib.rs @@ -400,13 +400,20 @@ mod tests { } #[test] - fn repeated_instances_charge_aggregate_geometry_against_limits() { - let mut parse_options = options(); - parse_options.limits.triangles = 2; + fn repeated_instances_charge_only_rendered_geometry_against_limits() { let bytes = b"0 FILE root.ldr\n1 16 0 0 0 1 0 0 0 1 0 0 0 0 child.dat\n1 16 2 0 0 1 0 0 0 1 0 0 0 0 child.dat\n0 NOFILE\n0 FILE child.dat\n3 16 0 0 0 1 0 0 0 1 0\n0 NOFILE\n"; + + let mut passing_options = options(); + passing_options.limits.triangles = 2; + LdrawParser + .parse_bytes(bytes, passing_options, &[]) + .expect("two child occurrences render two triangles"); + + let mut failing_options = options(); + failing_options.limits.triangles = 1; let error = LdrawParser - .parse_bytes(bytes, parse_options, &[]) - .expect_err("repeated instances must charge each occurrence"); + .parse_bytes(bytes, failing_options, &[]) + .expect_err("two rendered occurrences must exceed a one-triangle limit"); assert!(matches!( error, LdrawError::Diagnostic(Diagnostic { diff --git a/crates/polymodel-ldraw-core/src/model.rs b/crates/polymodel-ldraw-core/src/model.rs index 86269bd..a304931 100644 --- a/crates/polymodel-ldraw-core/src/model.rs +++ b/crates/polymodel-ldraw-core/src/model.rs @@ -846,27 +846,6 @@ pub struct LdMeshSemantic { pub step_offsets: Vec, } -impl LdMeshSemantic { - pub fn validate_finite(&self) -> Result<(), &'static str> { - if self - .positions - .iter() - .flatten() - .any(|value| !value.is_finite()) - { - return Err("semantic positions must be finite"); - } - if self - .normals - .as_ref() - .is_some_and(|normals| normals.iter().flatten().any(|value| !value.is_finite())) - { - return Err("semantic normals must be finite"); - } - Ok(()) - } -} - #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ModelSummary { pub model: ModelId, diff --git a/crates/polymodel-ldraw-core/src/traversal.rs b/crates/polymodel-ldraw-core/src/traversal.rs index b31ef9a..24794d6 100644 --- a/crates/polymodel-ldraw-core/src/traversal.rs +++ b/crates/polymodel-ldraw-core/src/traversal.rs @@ -306,21 +306,6 @@ pub(crate) fn traverse( if !summarized.insert(definition_id) { continue; } - let definition_triangles = model - .quads - .checked_mul(2) - .and_then(|quads| model.triangles.checked_add(quads)) - .ok_or(ParseError::Overflow("definition triangles"))?; - counters - .add(LimitKind::Triangles, definition_triangles, &options.limits) - .map_err(|name| limit_error(name, None))?; - let definition_lines = model - .lines - .checked_add(model.conditional_lines) - .ok_or(ParseError::Overflow("definition lines"))?; - counters - .add(LimitKind::Lines, definition_lines, &options.limits) - .map_err(|name| limit_error(name, None))?; metrics.definition_visits += 1; let model_id = if let Some(model_id) = ids.get(&definition_id) { model_id.clone() diff --git a/crates/polymodel-renderer-protocol/src/scene.rs b/crates/polymodel-renderer-protocol/src/scene.rs index b768475..909acd8 100644 --- a/crates/polymodel-renderer-protocol/src/scene.rs +++ b/crates/polymodel-renderer-protocol/src/scene.rs @@ -164,8 +164,8 @@ pub struct SceneOccurrence { pub definition_id: SmolStr, pub canonical_path: SmolStr, pub source_class: SmolStr, - pub source: Arc, - pub target: Arc, + pub source: SmolStr, + pub target: SmolStr, pub source_span: Option<[u32; 4]>, pub direct_colour: u32, pub effective_colour: u32, diff --git a/crates/polymodel-renderer-worker/src/lib.rs b/crates/polymodel-renderer-worker/src/lib.rs index 49ba856..bb9b7a1 100644 --- a/crates/polymodel-renderer-worker/src/lib.rs +++ b/crates/polymodel-renderer-worker/src/lib.rs @@ -182,6 +182,8 @@ pub enum RendererError { ParseMessage(String), #[error("I/O failure: {0}")] Io(String), + #[error("renderer generation exhausted; restart the worker")] + GenerationExhausted, #[error("external runtime failure: {0}")] External(String), } @@ -307,13 +309,6 @@ fn adapt_ldraw_parse_result( Ok(scene) } -#[derive(Clone)] -struct DefinitionMetadata { - definition_id: SmolStr, - canonical_path: SmolStr, - source_class: SmolStr, -} - #[derive(Clone, Hash, PartialEq, Eq)] struct PrimitiveBatchKey { colour: u32, @@ -360,7 +355,6 @@ struct AdapterSink<'a> { geometries: Vec, provenance: Vec, nodes_by_model: HashMap>, - metadata_by_model: HashMap, batch_indices: HashMap<(ProvenanceId, u64), Vec>, resources: Option<&'a [ModelResource]>, current_model: Option, @@ -374,7 +368,6 @@ impl<'a> AdapterSink<'a> { geometries: Vec::new(), provenance: Vec::new(), nodes_by_model: HashMap::new(), - metadata_by_model: HashMap::new(), batch_indices: HashMap::new(), resources, current_model: None, @@ -592,14 +585,6 @@ impl SemanticSink for AdapterSink<'_> { source: name.clone(), span: None, }); - self.metadata_by_model.insert( - root.clone(), - DefinitionMetadata { - definition_id: smol_str::format_smolstr!("{}", root.content_hash), - canonical_path: root.canonical_path.as_str().into(), - source_class: smol_str::format_smolstr!("{:?}", root.resolved_root), - }, - ); self.current_model = Some(root.clone()); self.current_provenance = Some(provenance); Ok(()) @@ -636,19 +621,6 @@ fn adapt_ldraw_assembly_with_sink( let diagnostics_enabled = std::env::var_os("POLYMODEL_LDRAW_ADAPTER_DIAGNOSTICS").is_some() || std::env::var_os("POLYMODEL_LDRAW_SCENE_DIAGNOSTICS").is_some(); let mut node_by_key = diagnostics_enabled.then(HashMap::new); - let metadata_by_model = event_models - .iter() - .map(|model| { - ( - model.model, - DefinitionMetadata { - definition_id: smol_str::format_smolstr!("{}", model.cache_key.content_hash), - canonical_path: model.cache_key.canonical_path.as_str().into(), - source_class: smol_str::format_smolstr!("{:?}", model.cache_key.resolved_root), - }, - ) - }) - .collect::>(); let nodes_by_model = event_models .iter() .map(|model| { @@ -664,7 +636,34 @@ fn adapt_ldraw_assembly_with_sink( .collect::>(); let path_by_model = event_models .iter() - .map(|model| (model.model, Arc::::from(model.path.as_str()))) + .map(|model| (model.model, SmolStr::from(model.path.as_str()))) + .collect::>(); + let definition_id_by_model = event_models + .iter() + .map(|model| { + ( + model.model, + SmolStr::from(model.cache_key.content_hash.to_string()), + ) + }) + .collect::>(); + let canonical_path_by_model = event_models + .iter() + .map(|model| { + ( + model.model, + SmolStr::from(model.cache_key.canonical_path.as_str()), + ) + }) + .collect::>(); + let source_class_by_model = event_models + .iter() + .map(|model| { + ( + model.model, + SmolStr::from(format!("{:?}", model.cache_key.resolved_root)), + ) + }) .collect::>(); let source_by_path = event_models .iter() @@ -884,9 +883,6 @@ fn adapt_ldraw_assembly_with_sink( let root_batches = nodes_by_model .get(&event_models[0].model) .ok_or_else(|| RendererError::ParseAdaptation("root model has no geometry batches"))?; - let root_metadata = metadata_by_model - .get(&event_models[0].model) - .ok_or_else(|| RendererError::ParseAdaptation("root model has no definition metadata"))?; let mut occurrences = Vec::new(); let mut stats = SceneStats::default(); for (node, primitive) in root_batches { @@ -912,9 +908,9 @@ fn adapt_ldraw_assembly_with_sink( 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(), + definition_id: definition_id_by_model[&event_models[0].model].clone(), + canonical_path: canonical_path_by_model[&event_models[0].model].clone(), + source_class: source_class_by_model[&event_models[0].model].clone(), source: path_by_model[&event_models[0].model].clone(), target: path_by_model[&event_models[0].model].clone(), source_span: None, @@ -948,9 +944,6 @@ fn adapt_ldraw_assembly_with_sink( let batches = nodes_by_model.get(&occurrence.model).ok_or_else(|| { RendererError::ParseAdaptation("occurrence model was not materialized") })?; - let metadata = metadata_by_model.get(&occurrence.model).ok_or_else(|| { - RendererError::ParseAdaptation("occurrence model has no definition metadata") - })?; let logical_part = inherited_logical_part.or_else(|| { meaningful_part(target_key.canonical_path.as_str()).then(|| { ( @@ -1028,9 +1021,9 @@ fn adapt_ldraw_assembly_with_sink( 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(), + definition_id: definition_id_by_model[&occurrence.model].clone(), + canonical_path: canonical_path_by_model[&occurrence.model].clone(), + source_class: source_class_by_model[&occurrence.model].clone(), source: source_by_path .get(&occurrence.source_model) .cloned() @@ -3612,7 +3605,7 @@ impl ScenePicker { .collect(); Ok(SelectionIdentity { canonical_occurrence_id: scene_occurrence.canonical_occurrence_id.clone(), - canonical_path: scene_occurrence.canonical_path.clone(), + canonical_path: scene_occurrence.canonical_path.as_str().into(), logical_part_occurrence_id: scene_occurrence .logical_part_occurrence_id .clone() diff --git a/crates/polymodel-renderer-worker/src/tests.rs b/crates/polymodel-renderer-worker/src/tests.rs index 25c3ada..0a9f2e1 100644 --- a/crates/polymodel-renderer-worker/src/tests.rs +++ b/crates/polymodel-renderer-worker/src/tests.rs @@ -2089,8 +2089,8 @@ fn seeded_ldconfig_red_is_not_the_grey_material_fallback() { assert_eq!(scene.provenance[0].source, "seeded-ldconfig-red"); assert_eq!(scene.occurrences.len(), 1); assert_eq!(scene.geometries[0].provenance, ProvenanceId(1)); - assert_eq!(scene.occurrences[0].source, "seeded-ldconfig-red".into()); - assert_eq!(scene.occurrences[0].target, "seeded-ldconfig-red".into()); + assert_eq!(scene.occurrences[0].source, "seeded-ldconfig-red"); + assert_eq!(scene.occurrences[0].target, "seeded-ldconfig-red"); let material = scene.materials.first().expect("seeded code-4 material"); assert_eq!(material.colour, 4); assert_eq!(material.colour_rgb, Some([201, 26, 9])); diff --git a/crates/polymodel-renderer-worker/src/worker.rs b/crates/polymodel-renderer-worker/src/worker.rs index 39bc7df..4c4498d 100644 --- a/crates/polymodel-renderer-worker/src/worker.rs +++ b/crates/polymodel-renderer-worker/src/worker.rs @@ -248,10 +248,10 @@ fn parse_compound_source( const MAX_UNKNOWN_FETCH_BYTES: u64 = 128 * 1024 * 1024; -fn next_generation(current: u32) -> Result { +fn next_generation(current: u32) -> Result { current .checked_add(1) - .ok_or("renderer generation exhausted; restart the worker") + .ok_or(RendererError::GenerationExhausted) } struct WorkerState { @@ -639,7 +639,7 @@ mod tests { assert_eq!(next_generation(u32::MAX - 1), Ok(u32::MAX)); assert_eq!( next_generation(u32::MAX), - Err("renderer generation exhausted; restart the worker") + Err(RendererError::GenerationExhausted) ); } @@ -1456,10 +1456,11 @@ fn handle_load_mesh( }; st.scene_generation = match next_generation(st.scene_generation) { Ok(generation) => generation, - Err(detail) => { + Err(error) => { + let detail = error.to_string(); let error = LoadError { code: LoadErrorCode::Install, - detail: detail.to_owned(), + detail: detail.clone(), }; st.load_reducer.reduce(LoadEvent::InstallFailed { namespace,