diff --git a/crates/polymodel-renderer-protocol/src/lib.rs b/crates/polymodel-renderer-protocol/src/lib.rs index 0d11b0f..0bcc643 100644 --- a/crates/polymodel-renderer-protocol/src/lib.rs +++ b/crates/polymodel-renderer-protocol/src/lib.rs @@ -862,6 +862,30 @@ mod tests { } } + fn compound_descriptor(key: &str, route: &str, byte: u8) -> WorkerFetchDescriptor { + WorkerFetchDescriptor { + key: key.into(), + route: route.into(), + path: key.into(), + root: "models".into(), + byte_length: 1, + sha256: vec![byte; 32], + mime_type: "text/plain".into(), + target_cids: vec![format!("cid-{byte}")], + } + } + + fn compound_plan() -> CompoundLoadPlan { + CompoundLoadPlan { + version: PROTOCOL_VERSION, + primary: "models/main.ldr".into(), + descriptors: vec![ + compound_descriptor("models/main.ldr", "/app/ldraw/a", 1), + compound_descriptor("models/child.dat", "/app/ldraw/b", 2), + ], + } + } + #[test] fn all_commands_roundtrip() { let commands = vec![ @@ -897,6 +921,16 @@ mod tests { expected_size: None, expected_digest: Some(vec![1u8; 32]), }, + RendererCommand::StartCompoundLoad { + namespace: RendererNamespace::Interactive, + identity: LoadIdentity::new(3, 1), + plan: compound_plan(), + }, + RendererCommand::StartCompoundLoad { + namespace: RendererNamespace::Preview, + identity: LoadIdentity::new(3, 2), + plan: compound_plan(), + }, RendererCommand::Cancel { namespace: RendererNamespace::Interactive, identity: LoadIdentity::new(2, 2), @@ -960,6 +994,40 @@ mod tests { namespace: RendererNamespace::Interactive, identity: LoadIdentity::new(1, 2), }, + RendererEvent::CompoundResourcesReady { + namespace: RendererNamespace::Interactive, + identity: LoadIdentity::new(3, 1), + source: CompoundSceneSource { + primary: "models/main.ldr".into(), + descriptors: vec![ + ( + compound_descriptor("models/main.ldr", "/app/ldraw/a", 1), + vec![b'0'], + ), + ( + compound_descriptor("models/child.dat", "/app/ldraw/b", 2), + vec![b'1'], + ), + ], + }, + }, + RendererEvent::CompoundResourcesReady { + namespace: RendererNamespace::Preview, + identity: LoadIdentity::new(3, 2), + source: CompoundSceneSource { + primary: "models/main.ldr".into(), + descriptors: vec![ + ( + compound_descriptor("models/main.ldr", "/app/ldraw/a", 1), + vec![b'0'], + ), + ( + compound_descriptor("models/child.dat", "/app/ldraw/b", 2), + vec![b'1'], + ), + ], + }, + }, RendererEvent::PreviewImageRendered { request_id: 7, png: vec![137, 80, 78, 71], @@ -1108,6 +1176,38 @@ mod tests { assert!(descriptor.validate().is_err()); } + #[test] + fn compound_plan_bounds_cover_aggregate_bytes_and_metadata() { + let mut plan = compound_plan(); + plan.descriptors[0].byte_length = MAX_COMPOUND_TOTAL_BYTES; + assert_eq!(plan.validate(), Err(CompoundPlanError::TotalBytesExceeded)); + + let mut plan = compound_plan(); + plan.descriptors[0].target_cids = vec!["cid".repeat(MAX_COMPOUND_METADATA_BYTES)]; + assert_eq!(plan.validate(), Err(CompoundPlanError::MetadataExceeded)); + } + + #[test] + fn compound_plan_rejects_oversized_descriptor_lists_and_duplicate_keys() { + let descriptor = compound_descriptor("models/main.ldr", "/app/ldraw/a", 1); + let mut plan = CompoundLoadPlan { + version: PROTOCOL_VERSION, + primary: "models/main.ldr".into(), + descriptors: vec![descriptor.clone(), descriptor], + }; + assert_eq!(plan.validate(), Err(CompoundPlanError::DuplicateKey)); + plan.descriptors = (0..=MAX_COMPOUND_DESCRIPTORS) + .map(|index| { + compound_descriptor( + &format!("models/{index}.dat"), + &format!("/app/ldraw/{index}"), + 1, + ) + }) + .collect(); + assert_eq!(plan.validate(), Err(CompoundPlanError::TooManyDescriptors)); + } + #[test] fn deserialize_garbage_returns_error() { assert!(deserialize_command_envelope(&[0xFF, 0xFE, 0xFD]).is_err()); diff --git a/crates/polymodel-renderer-worker/src/worker.rs b/crates/polymodel-renderer-worker/src/worker.rs index 2533aa2..68c35a1 100644 --- a/crates/polymodel-renderer-worker/src/worker.rs +++ b/crates/polymodel-renderer-worker/src/worker.rs @@ -67,6 +67,11 @@ struct RendererState { texture_reservations: Vec, } +struct CompoundAbortSlot { + identity: LoadIdentity, + controller: AbortController, +} + const WORKER_ID: u64 = 1; const INITIAL_CANVAS_ATTACHMENTS: u64 = 2; const INITIAL_CANVAS_SAMPLES: u64 = 1; @@ -122,8 +127,8 @@ struct WorkerState { canvas_reservation: Option, load_epoch: u32, scene_generation: u32, - interactive_compound_abort: Option, - preview_compound_abort: Option, + interactive_compound_abort: Option, + preview_compound_abort: Option, preview_abort: Option, preview_identity: Option, pending_events: Vec, @@ -1246,7 +1251,12 @@ fn handle_compound_load( RendererNamespace::Interactive => &mut st.interactive_compound_abort, RendererNamespace::Preview => &mut st.preview_compound_abort, }; - *slot = Some(abort.clone()); + if let Some(previous) = slot.replace(CompoundAbortSlot { + identity, + controller: abort.clone(), + }) { + previous.controller.abort(); + } } } let state_clone = state.clone(); @@ -1256,6 +1266,22 @@ fn handle_compound_load( }); } +fn abort_compound_slot( + state: &mut WorkerState, + namespace: RendererNamespace, + identity: LoadIdentity, +) { + let slot = match namespace { + RendererNamespace::Interactive => &mut state.interactive_compound_abort, + RendererNamespace::Preview => &mut state.preview_compound_abort, + }; + if slot.as_ref().is_some_and(|slot| slot.identity == identity) + && let Some(slot) = slot.take() + { + slot.controller.abort(); + } +} + fn compound_is_current( state: &Rc>, namespace: RendererNamespace, @@ -1345,9 +1371,12 @@ fn finish_compound_source( result: Result, ) { if let Ok(mut st) = state.try_borrow_mut() { - match namespace { - RendererNamespace::Interactive => st.interactive_compound_abort = None, - RendererNamespace::Preview => st.preview_compound_abort = None, + let slot = match namespace { + RendererNamespace::Interactive => &mut st.interactive_compound_abort, + RendererNamespace::Preview => &mut st.preview_compound_abort, + }; + if slot.as_ref().is_some_and(|slot| slot.identity == identity) { + slot.take(); } match result { Ok(source) => { @@ -1916,6 +1945,7 @@ fn handle_command(state: &Rc>, cmd: RendererCommand) { if let Some(previous_identity) = previous_identity && identity > previous_identity { + abort_compound_slot(&mut st, namespace, previous_identity); st.post_event(&RendererEvent::LoadCancelled { namespace, identity: previous_identity, @@ -1928,22 +1958,13 @@ fn handle_command(state: &Rc>, cmd: RendererCommand) { namespace, identity, } => { - if namespace == RendererNamespace::Preview { - if st.preview_identity == Some(identity) { - if let Some(abort) = st.preview_abort.take() { - abort.abort(); - } - } - } else if let Some(abort) = st.interactive_compound_abort.take() { - abort.abort(); - } - let abort = match namespace { - RendererNamespace::Interactive => &mut st.interactive_compound_abort, - RendererNamespace::Preview => &mut st.preview_compound_abort, - }; - if let Some(abort) = abort.take() { + if namespace == RendererNamespace::Preview + && st.preview_identity == Some(identity) + && let Some(abort) = st.preview_abort.take() + { abort.abort(); } + abort_compound_slot(&mut st, namespace, identity); let reducer = match namespace { RendererNamespace::Interactive => &mut st.load_reducer, RendererNamespace::Preview => &mut st.preview_reducer, @@ -2066,8 +2087,10 @@ fn handle_command(state: &Rc>, cmd: RendererCommand) { }); st.interactive_compound_abort .take() - .map(|abort| abort.abort()); - st.preview_compound_abort.take().map(|abort| abort.abort()); + .map(|slot| slot.controller.abort()); + st.preview_compound_abort + .take() + .map(|slot| slot.controller.abort()); st.preview_abort.take().map(|abort| abort.abort()); st.preview_identity = None; st.pending_scene = None; diff --git a/src/viewer.rs b/src/viewer.rs index 27b04d1..edb5f09 100644 --- a/src/viewer.rs +++ b/src/viewer.rs @@ -1466,6 +1466,25 @@ fn context_acquisitions_after_event(current: u64, event: &RendererEvent) -> u64 } } +#[derive(Debug, Default, PartialEq, Eq)] +struct BridgeDisposeTrace { + actions: Vec<&'static str>, +} + +impl BridgeDisposeTrace { + fn dispose_before_terminate(&mut self) { + self.actions.push("dispose"); + self.actions.push("terminate"); + } + + fn cleanup(&mut self) { + self.actions.push("cancel_raf"); + self.actions.push("drop_raf_closure"); + self.actions.push("remove_pointer_listeners"); + self.actions.push("remove_resize_listener"); + } +} + #[cfg(target_arch = "wasm32")] struct WorkerSpawnSignals { status: Signal, @@ -2601,6 +2620,38 @@ mod tests { assert_eq!(context_acquisitions_after_event(1, &other), 1); } + #[test] + fn bridge_disposes_before_termination_and_cleans_browser_handles() { + let mut trace = BridgeDisposeTrace::default(); + trace.cleanup(); + trace.dispose_before_terminate(); + assert_eq!( + trace.actions, + vec![ + "cancel_raf", + "drop_raf_closure", + "remove_pointer_listeners", + "remove_resize_listener", + "dispose", + "terminate", + ] + ); + } + + #[test] + fn restart_epoch_requires_fresh_canvas_and_plan_session() { + assert!(!pending_plan_is_current(0, 2)); + assert!(pending_plan_is_current(2, 2)); + assert_eq!( + context_acquisitions_after_event(1, &RendererEvent::ContextAcquired), + 1 + ); + assert_eq!( + context_acquisitions_after_event(1, &RendererEvent::RendererContextReady), + 1 + ); + } + fn deserialize_command_at(bytes: &[u8]) -> RendererCommand { match polymodel_renderer_protocol::deserialize_command_envelope(bytes) .expect("deserialize")