From bcc5342f7a7893b587b966064cbf1dda13e290b9 Mon Sep 17 00:00:00 2001 From: Orual Date: Mon, 10 Aug 2026 10:09:30 -0400 Subject: [PATCH] PM-84: fix Arc sharing, replace additive sink with direct consumption --- crates/polymodel-ldraw-core/src/model.rs | 45 ----------------- crates/polymodel-ldraw-core/src/traversal.rs | 48 ------------------- .../polymodel-renderer-protocol/src/scene.rs | 2 +- crates/polymodel-renderer-worker/src/lib.rs | 46 ++++++++++-------- crates/polymodel-renderer-worker/src/tests.rs | 8 ++-- .../polymodel-renderer-worker/src/worker.rs | 6 +-- .../tests/ldraw_preview.rs | 14 ++++-- 7 files changed, 43 insertions(+), 126 deletions(-) diff --git a/crates/polymodel-ldraw-core/src/model.rs b/crates/polymodel-ldraw-core/src/model.rs index d397c8c..eaa7e6f 100644 --- a/crates/polymodel-ldraw-core/src/model.rs +++ b/crates/polymodel-ldraw-core/src/model.rs @@ -863,51 +863,6 @@ impl LdMeshSemantic { } } -/// Snapshot of semantic state at an emitted traversal event. -#[derive(Clone, Debug)] -pub struct TraversalContext { - pub transform: Transform, - pub colour: ColourCode, - pub bfc: BfcFrame, - pub texmap: Option, - pub source: CacheKey, - pub source_path: NormalizedPath, -} - -/// Typed semantic notifications emitted by the additive traversal event path. -#[derive(Clone, Debug)] -pub enum SemanticEvent { - ModelDefined { - model: ModelId, - path: NormalizedPath, - key: CacheKey, - }, - GeometryEmitted { - model: ModelId, - geometry: GeometryRecord, - }, - OccurrenceInstantiated { - model: ModelId, - target: ModelId, - transform: Transform, - colour: ColourCode, - }, - TexmapApplied { - association: TexmapAssociation, - }, - StepBoundary, - ColourDefined { - colour: ColourCode, - }, -} - -/// Consumer interface for semantic traversal events. -pub trait SemanticSink { - type Error; - - fn emit(&mut self, event: SemanticEvent, context: TraversalContext) -> Result<(), Self::Error>; -} - #[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 7d64b70..0ee1797 100644 --- a/crates/polymodel-ldraw-core/src/traversal.rs +++ b/crates/polymodel-ldraw-core/src/traversal.rs @@ -123,54 +123,6 @@ fn target_index( Ok(rooted.first().copied()) } -struct ParseSemanticSink; - -impl crate::SemanticSink for ParseSemanticSink { - type Error = ParseError; - - fn emit( - &mut self, - _event: crate::SemanticEvent, - _context: crate::TraversalContext, - ) -> Result<(), Self::Error> { - Ok(()) - } -} - -pub(crate) fn emit_model_events>( - model: &ModelData<'_>, - transform: Transform, - colour: crate::ColourCode, - sink: &mut S, -) -> Result<(), ParseError> { - let context = || crate::TraversalContext { - transform, - colour, - bfc: model.bfc, - texmap: None, - source: model.key.clone(), - source_path: model.path.clone(), - }; - sink.emit( - crate::SemanticEvent::ModelDefined { - model: crate::ModelId(model.file_id), - path: model.path.clone(), - key: model.key.clone(), - }, - context(), - )?; - for geometry in &model.geometry { - sink.emit( - crate::SemanticEvent::GeometryEmitted { - model: crate::ModelId(model.file_id), - geometry: geometry.clone(), - }, - context(), - )?; - } - Ok(()) -} - pub(crate) fn traverse( models: &[ModelData<'_>], options: &ParseOptions<'_>, diff --git a/crates/polymodel-renderer-protocol/src/scene.rs b/crates/polymodel-renderer-protocol/src/scene.rs index 5cee715..1122ad4 100644 --- a/crates/polymodel-renderer-protocol/src/scene.rs +++ b/crates/polymodel-renderer-protocol/src/scene.rs @@ -160,7 +160,7 @@ pub struct SceneOccurrence { pub node: NodeId, /// Optional per-occurrence UV overlay; the referenced geometry remains shared. #[serde(default)] - pub uvs: Option>, + pub uvs: Option>>, pub definition_id: SmolStr, pub canonical_path: SmolStr, pub source_class: SmolStr, diff --git a/crates/polymodel-renderer-worker/src/lib.rs b/crates/polymodel-renderer-worker/src/lib.rs index 654a7a8..aed38a6 100644 --- a/crates/polymodel-renderer-worker/src/lib.rs +++ b/crates/polymodel-renderer-worker/src/lib.rs @@ -16,7 +16,6 @@ use smol_str::SmolStr; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use std::rc::Rc; use std::sync::Arc; #[cfg(not(target_arch = "wasm32"))] use std::sync::Mutex; @@ -525,7 +524,7 @@ fn adapt_ldraw_assembly( } let mut geometries = Vec::with_capacity(models.len()); let mut provenance = Vec::with_capacity(models.len()); - let mut uv_cache = HashMap::<(GeometryId, u64, u64), Rc>>::new(); + let mut uv_cache = HashMap::<(GeometryId, u64, u64), Arc>>::new(); let mut uv_cache_hits = 0_u64; let mut uv_cache_misses = 0_u64; let mut texture_ids = HashMap::<(RootId, String, String), TextureId>::new(); @@ -826,7 +825,7 @@ fn adapt_ldraw_assembly( uvs.clone() } else { uv_cache_misses += 1; - let uvs = Rc::new(occurrence_texmap_uvs( + let uvs = Arc::new(occurrence_texmap_uvs( texmap, transform, &geometries[source_index].points, @@ -850,7 +849,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.as_ref().map(|uvs| uvs.as_ref().clone()), + uvs: occurrence_uvs, definition_id: metadata.definition_id.clone(), canonical_path: metadata.canonical_path.clone(), source_class: metadata.source_class.clone(), @@ -1018,7 +1017,11 @@ fn adapt_ldraw_assembly( } if std::env::var_os("POLYMODEL_LDRAW_TIMING").is_some() { let total = uv_cache_hits + uv_cache_misses; - let hit_rate = if total == 0 { 1.0 } else { uv_cache_hits as f64 / total as f64 }; + let hit_rate = if total == 0 { + 1.0 + } else { + uv_cache_hits as f64 / total as f64 + }; eprintln!( "[ldraw-metrics] phase=uv-cache hits={} misses={} hit_rate={hit_rate:.6} entries={}", uv_cache_hits, @@ -3296,9 +3299,9 @@ impl PickBvh { #[derive(Debug, Clone)] pub struct ScenePicker { - topologies: Rc>, - edge_sources: Rc>, - occurrences: Rc>, + topologies: Arc>, + edge_sources: Arc>, + occurrences: Arc>, logical_parts: Vec>, logical_bounds: Vec, ranges: Vec>, @@ -3385,11 +3388,11 @@ impl ScenePicker { }) } - pub fn edge_sources(&self) -> &Rc> { + pub fn edge_sources(&self) -> &Arc> { &self.edge_sources } - pub fn planned_occurrences(&self) -> &Rc> { + pub fn planned_occurrences(&self) -> &Arc> { &self.occurrences } @@ -3507,7 +3510,7 @@ pub struct SceneBatchInstance { pub struct SceneInstanceBatch { pub node: NodeId, /// UVs shared by occurrences with identical geometry/transform/TEXMAP keys. - pub uvs: Option>>, + pub uvs: Option>>, pub material: MaterialId, pub bfc_cull: bool, pub bfc_ccw: bool, @@ -3525,10 +3528,10 @@ pub struct SceneRenderPlan { /// Mapping from planned batch index to actual upload-object index. pub planned_to_upload: Vec>, pub occurrence_ranges: Vec>, - pub topologies: Rc>, - pub edge_sources: Rc>, + pub topologies: Arc>, + pub edge_sources: Arc>, /// Canonical occurrence records shared by face upload, edge output, and picking. - pub planned_occurrences: Rc>, + pub planned_occurrences: Arc>, logical_parts: Vec>, logical_bounds: Vec, logical_part_bounds: std::collections::HashMap, @@ -3605,7 +3608,7 @@ pub fn scene_render_plan(scene: &Scene) -> Result, _>>()?; - let edge_sources = Rc::new(edge_sources); + let edge_sources = Arc::new(edge_sources); let topology_complete = started.elapsed(); let local_bounds = topologies .iter() @@ -3656,9 +3659,9 @@ pub fn scene_render_plan(scene: &Scene) -> Result>>; +pub type NativeSceneModel = + Vec>>; pub fn build_scene_model( context: &three_d::Context, @@ -5213,7 +5217,7 @@ fn build_scene_model_from_batches( let mut albedo_cache = HashMap::::new(); let mut gloss_cache = HashMap::::new(); let mut transparency_cache = HashMap::::new(); - let mut material_cache = HashMap::<(MaterialId, bool), Rc>::new(); + let mut material_cache = HashMap::<(MaterialId, bool), Arc>::new(); let model = batches .iter() .map(|batch| { @@ -5303,7 +5307,7 @@ fn build_scene_model_from_batches( PhysicalMaterial::new_opaque(context, &cpu_material) }; material.render_states = scene_material_render_states(translucent, material_key.1); - let material = Rc::new(material); + let material = Arc::new(material); material_cache.insert(material_key, material.clone()); material }; diff --git a/crates/polymodel-renderer-worker/src/tests.rs b/crates/polymodel-renderer-worker/src/tests.rs index 6c357b5..c55aa39 100644 --- a/crates/polymodel-renderer-worker/src/tests.rs +++ b/crates/polymodel-renderer-worker/src/tests.rs @@ -716,8 +716,8 @@ 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()); + scene.occurrences[0].uvs = Some(Arc::new(uv_a.clone())); + scene.occurrences[1].uvs = Some(Arc::new(uv_b.clone())); let plan = scene_render_plan(&scene).expect("render plan"); assert_eq!(scene.geometries.len(), 1); @@ -746,7 +746,7 @@ fn textured_occurrences_share_geometry_and_split_uv_batches() { #[test] fn scene_render_plan_rejects_misaligned_occurrence_uvs() { let mut scene = picker_scene(); - scene.occurrences[0].uvs = Some(vec![[0.0, 0.0]]); + scene.occurrences[0].uvs = Some(Arc::new(vec![[0.0, 0.0]])); let error = scene_render_plan(&scene).expect_err("misaligned occurrence UVs"); assert!(matches!( error, @@ -761,7 +761,7 @@ fn scene_render_plan_rejects_misaligned_occurrence_uvs() { #[test] fn scene_render_plan_rejects_non_finite_occurrence_uvs() { let mut scene = picker_scene(); - scene.occurrences[0].uvs = Some(vec![[f32::NAN, 0.0]; 3]); + scene.occurrences[0].uvs = Some(Arc::new(vec![[f32::NAN, 0.0]; 3])); let error = scene_render_plan(&scene).expect_err("non-finite occurrence UVs"); assert!(matches!( error, diff --git a/crates/polymodel-renderer-worker/src/worker.rs b/crates/polymodel-renderer-worker/src/worker.rs index bc34cfa..29aa8db 100644 --- a/crates/polymodel-renderer-worker/src/worker.rs +++ b/crates/polymodel-renderer-worker/src/worker.rs @@ -64,7 +64,7 @@ struct RendererState { ldraw_static_model: Option, ldraw_explicit_model: Option>, ldraw_conditional_model: Option>, - ldraw_scene: Option>, + ldraw_scene: Option>, ldraw_plan: Option, ldraw_style: polymodel_renderer_worker::RenderStyle, ldraw_last_view: Option<[f64; 3]>, @@ -1880,7 +1880,7 @@ async fn render_compound_preview_request( .bounds .ok_or(CompoundLoadError::Bundle("LDraw scene has no bounds"))?; let target = create_preview_target(width, height)?; - let scene = Rc::new(scene); + let scene = Arc::new(scene); let style = polymodel_renderer_worker::RenderStyle::Faces; let (static_model, (_explicit, _conditional), _picker, _plan) = polymodel_renderer_worker::build_scene_model_parts_for_style_with_resources( @@ -2213,7 +2213,7 @@ fn finish_compound_source( return; } }; - let scene_ref = std::rc::Rc::new(scene); + let scene_ref = Arc::new(scene); let style = polymodel_renderer_worker::RenderStyle::Faces; let models = match polymodel_renderer_worker::build_scene_model_parts_for_style_with_resources( &context, diff --git a/crates/polymodel-renderer-worker/tests/ldraw_preview.rs b/crates/polymodel-renderer-worker/tests/ldraw_preview.rs index d60fb34..283a4eb 100644 --- a/crates/polymodel-renderer-worker/tests/ldraw_preview.rs +++ b/crates/polymodel-renderer-worker/tests/ldraw_preview.rs @@ -633,10 +633,16 @@ fn pinned_scene() -> polymodel_renderer_protocol::scene::Scene { "flattened face batches must resolve CURRENT/EDGE pseudo-colours" ); if std::env::var_os("POLYMODEL_LDRAW_TIMING").is_some() { - let source_bytes = materializations.iter().map(|item| item.bytes.len()).sum::(); - let upload_bytes = renderable.iter().map(|(_, topology)| { - topology.positions.len() * (3 * 4 + 3 * 4 + 4) + topology.indices.len() * 4 - }).sum::(); + let source_bytes = materializations + .iter() + .map(|item| item.bytes.len()) + .sum::(); + let upload_bytes = renderable + .iter() + .map(|(_, topology)| { + topology.positions.len() * (3 * 4 + 3 * 4 + 4) + topology.indices.len() * 4 + }) + .sum::(); eprintln!( "[ldraw-metrics] phase=scene-stats scene_bytes={} materializations={} model_definitions={} geometries={} occurrences={} points={} triangles={} lines={} conditional_lines={} upload_bytes={} lookup=hashmap complexity=O(1)", source_bytes, -- knotmirror 0.0.1