From 64bf9bb431e66d528049c86494cded1ad62650b7 Mon Sep 17 00:00:00 2001 From: Skyler Lehmkuhl Date: Sat, 11 Jul 2026 14:33:14 -0400 Subject: [PATCH] Type the rest of the audio-controller boundary (no more bare-f64 wrapping) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Push Beats/Seconds through the remaining controller methods that took a bare f64 and let the audio thread wrap it in a newtype, so the caller's domain is now compiler-checked (the seam that hid the recording bug): - seek -> Seconds - set_trim_start/set_trim_end -> Seconds / Option (metatrack, always seconds) - add_midi_note, add_loaded_midi_clip, update_midi_clip_notes -> Beats - add_automation_point, remove_automation_point, automation_add_keyframe, automation_remove_keyframe -> Beats Command enums stay raw f64 transport; only the public signatures + call sites change. No behavior change — every caller already passed the right domain, this just makes it enforced. Two deliberate exceptions, documented in place: - trim_clip stays f64: the TrimClip handler interprets it as Seconds for a sampled-audio clip but Beats for a MIDI clip, so no single newtype fits; callers pass the clip's own trim value, which matches its content domain. - The piano-roll MIDI note model stays f64 internally (a beats-only subsystem with no seconds anywhere); it's converted to Beats at the update_midi_clip_notes boundary in UpdateMidiNotesAction, same as trim_start f64 -> Seconds at add_audio_clip. --- daw-backend/src/audio/engine.rs | 51 ++++++++++--------- daw-backend/src/tui/mod.rs | 4 +- .../src/actions/move_clip_instances.rs | 8 +-- .../src/actions/trim_clip_instances.rs | 8 +-- .../src/actions/update_midi_notes.rs | 11 +++- .../src/mobile/transport.rs | 5 +- .../src/panes/piano_roll.rs | 4 +- .../src/panes/timeline.rs | 24 ++++----- 8 files changed, 64 insertions(+), 51 deletions(-) diff --git a/daw-backend/src/audio/engine.rs b/daw-backend/src/audio/engine.rs index 7581025..bc3c224 100644 --- a/daw-backend/src/audio/engine.rs +++ b/daw-backend/src/audio/engine.rs @@ -3348,8 +3348,8 @@ impl EngineController { } /// Seek to a specific position in seconds - pub fn seek(&mut self, seconds: f64) { - let _ = self.command_tx.push(Command::Seek(seconds)); + pub fn seek(&mut self, seconds: Seconds) { + let _ = self.command_tx.push(Command::Seek(seconds.seconds_to_f64())); } /// Set track volume (0.0 = silence, 1.0 = unity gain) @@ -3386,6 +3386,10 @@ impl EngineController { /// Trim a clip's internal boundaries (changes which portion of source content is used) /// This also resets external_duration to match internal duration (disables looping) + /// Trim a clip's internal content bounds. The units are content-domain and depend on the + /// track type: SECONDS for a sampled-audio clip, BEATS for a MIDI clip (see the TrimClip + /// handler). Left as raw f64 because a single newtype can't express both; callers pass the + /// clip's own `trim_start`/`trim_end`, which already match its content domain. pub fn trim_clip(&mut self, track_id: TrackId, clip_id: ClipId, new_internal_start: f64, new_internal_end: f64) { let _ = self.command_tx.push(Command::TrimClip(track_id, clip_id, new_internal_start, new_internal_end)); } @@ -3450,13 +3454,13 @@ impl EngineController { } /// Set metatrack trim start in seconds - pub fn set_trim_start(&mut self, track_id: TrackId, trim_start: f64) { - let _ = self.command_tx.push(Command::SetTrimStart(track_id, trim_start)); + pub fn set_trim_start(&mut self, track_id: TrackId, trim_start: Seconds) { + let _ = self.command_tx.push(Command::SetTrimStart(track_id, trim_start.seconds_to_f64())); } /// Set metatrack trim end in seconds (None = no end trim) - pub fn set_trim_end(&mut self, track_id: TrackId, trim_end: Option) { - let _ = self.command_tx.push(Command::SetTrimEnd(track_id, trim_end)); + pub fn set_trim_end(&mut self, track_id: TrackId, trim_end: Option) { + let _ = self.command_tx.push(Command::SetTrimEnd(track_id, trim_end.map(|s| s.seconds_to_f64()))); } /// Create a new audio track @@ -3615,17 +3619,18 @@ impl EngineController { } /// Add a MIDI note to a clip - pub fn add_midi_note(&mut self, track_id: TrackId, clip_id: MidiClipId, time_offset: f64, note: u8, velocity: u8, duration: f64) { - let _ = self.command_tx.push(Command::AddMidiNote(track_id, clip_id, time_offset, note, velocity, duration)); + pub fn add_midi_note(&mut self, track_id: TrackId, clip_id: MidiClipId, time_offset: Beats, note: u8, velocity: u8, duration: Beats) { + let _ = self.command_tx.push(Command::AddMidiNote(track_id, clip_id, time_offset.beats_to_f64(), note, velocity, duration.beats_to_f64())); } - /// Add a pre-loaded MIDI clip to a track at the given timeline position - pub fn add_loaded_midi_clip(&mut self, track_id: TrackId, clip: MidiClip, start_time: f64) { - let _ = self.command_tx.push(Command::AddLoadedMidiClip(track_id, clip, start_time)); + /// Add a pre-loaded MIDI clip to a track at the given timeline position (beats) + pub fn add_loaded_midi_clip(&mut self, track_id: TrackId, clip: MidiClip, start_time: Beats) { + let _ = self.command_tx.push(Command::AddLoadedMidiClip(track_id, clip, start_time.beats_to_f64())); } - /// Update all notes in a MIDI clip - pub fn update_midi_clip_notes(&mut self, track_id: TrackId, clip_id: MidiClipId, notes: Vec<(f64, u8, u8, f64)>) { + /// Update all notes in a MIDI clip. Note tuples are (start [beats], note, velocity, duration [beats]). + pub fn update_midi_clip_notes(&mut self, track_id: TrackId, clip_id: MidiClipId, notes: Vec<(Beats, u8, u8, Beats)>) { + let notes = notes.into_iter().map(|(t, n, v, d)| (t.beats_to_f64(), n, v, d.beats_to_f64())).collect(); let _ = self.command_tx.push(Command::UpdateMidiClipNotes(track_id, clip_id, notes)); } @@ -3661,25 +3666,25 @@ impl EngineController { &mut self, track_id: TrackId, lane_id: crate::audio::AutomationLaneId, - time: f64, + time: Beats, value: f32, curve: crate::audio::CurveType, ) { let _ = self.command_tx.push(Command::AddAutomationPoint( - track_id, lane_id, time, value, curve, + track_id, lane_id, time.beats_to_f64(), value, curve, )); } - /// Remove an automation point at a specific time + /// Remove an automation point at a specific time (beats); tolerance is a beats delta pub fn remove_automation_point( &mut self, track_id: TrackId, lane_id: crate::audio::AutomationLaneId, - time: f64, - tolerance: f64, + time: Beats, + tolerance: Beats, ) { let _ = self.command_tx.push(Command::RemoveAutomationPoint( - track_id, lane_id, time, tolerance, + track_id, lane_id, time.beats_to_f64(), tolerance.beats_to_f64(), )); } @@ -3715,16 +3720,16 @@ impl EngineController { /// Add a keyframe to an AutomationInput node pub fn automation_add_keyframe(&mut self, track_id: TrackId, node_id: u32, - time: f64, value: f32, interpolation: String, + time: Beats, value: f32, interpolation: String, ease_out: (f32, f32), ease_in: (f32, f32)) { let _ = self.command_tx.push(Command::AutomationAddKeyframe( - track_id, node_id, time, value, interpolation, ease_out, ease_in)); + track_id, node_id, time.beats_to_f64(), value, interpolation, ease_out, ease_in)); } /// Remove a keyframe from an AutomationInput node - pub fn automation_remove_keyframe(&mut self, track_id: TrackId, node_id: u32, time: f64) { + pub fn automation_remove_keyframe(&mut self, track_id: TrackId, node_id: u32, time: Beats) { let _ = self.command_tx.push(Command::AutomationRemoveKeyframe( - track_id, node_id, time)); + track_id, node_id, time.beats_to_f64())); } /// Set the display name of an AutomationInput node diff --git a/daw-backend/src/tui/mod.rs b/daw-backend/src/tui/mod.rs index 85d19bc..0d44a6b 100644 --- a/daw-backend/src/tui/mod.rs +++ b/daw-backend/src/tui/mod.rs @@ -790,7 +790,7 @@ fn execute_command( return Err("Usage: seek ".to_string()); } let pos: f64 = parts[1].parse().map_err(|_| "Invalid position")?; - controller.seek(pos); + controller.seek(crate::Seconds(pos)); app.set_status(format!("Seeked to {:.2}s", pos)); } "track" => { @@ -882,7 +882,7 @@ fn execute_command( app.next_clip_id += 1; // Send to audio engine with the start_time (clip content is separate from timeline position) - controller.add_loaded_midi_clip(track_id, midi_clip, start_time); + controller.add_loaded_midi_clip(track_id, midi_clip, crate::Beats(start_time)); app.set_status(format!("Loaded {} ({} events, {:.2}s) to track {} at {:.2}s", file_path, event_count, duration, track_id, start_time)); diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/move_clip_instances.rs b/lightningbeam-ui/lightningbeam-core/src/actions/move_clip_instances.rs index 6664d3d..73f49b2 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/move_clip_instances.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/move_clip_instances.rs @@ -211,8 +211,8 @@ impl Action for MoveClipInstancesAction { // Check if this clip has a metatrack if let Some(&metatrack_id) = backend.layer_to_track_map.get(&instance.clip_id) { controller.set_offset(metatrack_id, document.tempo_map().beats_to_seconds(*new_start)); - controller.set_trim_start(metatrack_id, instance.trim_start); - controller.set_trim_end(metatrack_id, instance.trim_end); + controller.set_trim_start(metatrack_id, daw_backend::Seconds(instance.trim_start)); + controller.set_trim_end(metatrack_id, instance.trim_end.map(daw_backend::Seconds)); } } } @@ -295,8 +295,8 @@ impl Action for MoveClipInstancesAction { if let Some(instance) = vl.clip_instances.iter().find(|ci| ci.id == *instance_id) { if let Some(&metatrack_id) = backend.layer_to_track_map.get(&instance.clip_id) { controller.set_offset(metatrack_id, document.tempo_map().beats_to_seconds(*old_start)); - controller.set_trim_start(metatrack_id, instance.trim_start); - controller.set_trim_end(metatrack_id, instance.trim_end); + controller.set_trim_start(metatrack_id, daw_backend::Seconds(instance.trim_start)); + controller.set_trim_end(metatrack_id, instance.trim_end.map(daw_backend::Seconds)); } } } diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs b/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs index b3da86c..55fa95d 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs @@ -387,8 +387,8 @@ impl Action for TrimClipInstancesAction { if let Some(&metatrack_id) = backend.layer_to_track_map.get(&instance.clip_id) { // Instance already has new values after execute() controller.set_offset(metatrack_id, document.tempo_map().beats_to_seconds(instance.timeline_start)); - controller.set_trim_start(metatrack_id, instance.trim_start); - controller.set_trim_end(metatrack_id, instance.trim_end); + controller.set_trim_start(metatrack_id, daw_backend::Seconds(instance.trim_start)); + controller.set_trim_end(metatrack_id, instance.trim_end.map(daw_backend::Seconds)); } } } @@ -477,8 +477,8 @@ impl Action for TrimClipInstancesAction { if let Some(&metatrack_id) = backend.layer_to_track_map.get(&instance.clip_id) { // Instance already has old values after rollback() controller.set_offset(metatrack_id, document.tempo_map().beats_to_seconds(instance.timeline_start)); - controller.set_trim_start(metatrack_id, instance.trim_start); - controller.set_trim_end(metatrack_id, instance.trim_end); + controller.set_trim_start(metatrack_id, daw_backend::Seconds(instance.trim_start)); + controller.set_trim_end(metatrack_id, instance.trim_end.map(daw_backend::Seconds)); } } } diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/update_midi_notes.rs b/lightningbeam-ui/lightningbeam-core/src/actions/update_midi_notes.rs index 06ca4f2..a74074b 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/update_midi_notes.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/update_midi_notes.rs @@ -1,7 +1,13 @@ use crate::action::Action; use crate::document::Document; +use daw_backend::Beats; use uuid::Uuid; +/// Convert editor-side beats-domain note tuples to the typed backend form. +fn notes_to_beats(notes: &[(f64, u8, u8, f64)]) -> Vec<(Beats, u8, u8, Beats)> { + notes.iter().map(|&(t, n, v, d)| (Beats(t), n, v, Beats(d))).collect() +} + /// Action to update MIDI notes in a clip (supports undo/redo) /// /// Stores the before and after note states. MIDI note data lives in the backend, @@ -49,7 +55,8 @@ impl Action for UpdateMidiNotesAction { .get(&self.layer_id) .ok_or_else(|| format!("Layer {} not mapped to backend track", self.layer_id))?; - controller.update_midi_clip_notes(*track_id, self.midi_clip_id, self.new_notes.clone()); + // Note times/durations are beats (MIDI content domain); assert that at the typed boundary. + controller.update_midi_clip_notes(*track_id, self.midi_clip_id, notes_to_beats(&self.new_notes)); Ok(()) } @@ -68,7 +75,7 @@ impl Action for UpdateMidiNotesAction { .get(&self.layer_id) .ok_or_else(|| format!("Layer {} not mapped to backend track", self.layer_id))?; - controller.update_midi_clip_notes(*track_id, self.midi_clip_id, self.old_notes.clone()); + controller.update_midi_clip_notes(*track_id, self.midi_clip_id, notes_to_beats(&self.old_notes)); Ok(()) } diff --git a/lightningbeam-ui/lightningbeam-editor/src/mobile/transport.rs b/lightningbeam-ui/lightningbeam-editor/src/mobile/transport.rs index 8b4bc1b..101fe5c 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/mobile/transport.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/mobile/transport.rs @@ -2,6 +2,7 @@ //! project scrub. Wired to the audio controller exactly like `TimelinePane`'s header. use eframe::egui; +use daw_backend::Seconds; use super::{icons, Palette}; use crate::panes::SharedPaneState; @@ -32,7 +33,7 @@ pub fn render(ui: &mut egui::Ui, rect: egui::Rect, shared: &mut SharedPaneState, if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); if *shared.is_playing { - controller.seek(*shared.playback_time); + controller.seek(Seconds(*shared.playback_time)); controller.play(); } else { controller.pause(); @@ -101,7 +102,7 @@ pub fn render(ui: &mut egui::Ui, rect: egui::Rect, shared: &mut SharedPaneState, *shared.playback_time = new_time; if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(new_time); + controller.seek(Seconds(new_time)); } } } diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs index c1e24ad..34b2145 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs @@ -651,7 +651,7 @@ impl PianoRollPane { *shared.playback_time = nt; if let Some(ctrl) = shared.audio_controller.as_ref() { if let Ok(mut c) = ctrl.lock() { - c.seek(nt); + c.seek(Seconds(nt)); } } } @@ -1764,7 +1764,7 @@ impl PianoRollPane { let seek_time = snap_to_value(time.max(0.0), self.snap_value, tempo_map); *shared.playback_time = seek_time; if let Some(ctrl) = shared.audio_controller.as_ref() { - if let Ok(mut c) = ctrl.lock() { c.seek(seek_time); } + if let Ok(mut c) = ctrl.lock() { c.seek(Seconds(seek_time)); } } } } diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs index 8274f1d..a694da2 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs @@ -1110,7 +1110,7 @@ impl TimelinePane { if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(seek_to); + controller.seek(Seconds(seek_to)); controller.set_metronome_enabled(true); if !*shared.is_playing { controller.play(); @@ -5006,7 +5006,7 @@ impl TimelinePane { *playback_time = kf_time; if let Some(controller_arc) = audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(kf_time); + controller.seek(Seconds(kf_time)); } } } @@ -5034,7 +5034,7 @@ impl TimelinePane { // Seek immediately so it works while playing if let Some(controller_arc) = audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(new_time); + controller.seek(Seconds(new_time)); } } } @@ -5046,7 +5046,7 @@ impl TimelinePane { *playback_time = new_time; if let Some(controller_arc) = audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(new_time); + controller.seek(Seconds(new_time)); } } } @@ -5206,7 +5206,7 @@ impl PaneRenderer for TimelinePane { *shared.playback_time = 0.0; if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(0.0); + controller.seek(Seconds(0.0)); } } @@ -5215,7 +5215,7 @@ impl PaneRenderer for TimelinePane { *shared.playback_time = (*shared.playback_time - 0.1).max(0.0); if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(*shared.playback_time); + controller.seek(Seconds(*shared.playback_time)); } } @@ -5251,7 +5251,7 @@ impl PaneRenderer for TimelinePane { *shared.playback_time = (*shared.playback_time + 0.1).min(self.duration); if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(*shared.playback_time); + controller.seek(Seconds(*shared.playback_time)); } } @@ -5260,7 +5260,7 @@ impl PaneRenderer for TimelinePane { *shared.playback_time = self.duration; if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.seek(self.duration); + controller.seek(Seconds(self.duration)); } } @@ -5692,7 +5692,7 @@ impl PaneRenderer for TimelinePane { self.automation_cache.remove(&layer_id); } else if let Some(&track_id) = shared.layer_to_track_map.get(&layer_id) { // time is already in beats (all automation x-axes use beats) - controller.automation_add_keyframe(track_id, node_id, time, value, "linear".to_string(), (0.0, 0.0), (0.0, 0.0)); + controller.automation_add_keyframe(track_id, node_id, Beats(time), value, "linear".to_string(), (0.0, 0.0), (0.0, 0.0)); // Optimistic cache update (beats) if let Some(lanes) = self.automation_cache.get_mut(&layer_id) { if let Some(lane) = lanes.iter_mut().find(|l| l.node_id == node_id) { @@ -5740,8 +5740,8 @@ impl PaneRenderer for TimelinePane { self.automation_cache.remove(&layer_id); } else if let Some(&track_id) = shared.layer_to_track_map.get(&layer_id) { // old_time / new_time are already in beats - controller.automation_remove_keyframe(track_id, node_id, old_time); - controller.automation_add_keyframe(track_id, node_id, new_time, new_value, interpolation.clone(), ease_out, ease_in); + controller.automation_remove_keyframe(track_id, node_id, Beats(old_time)); + controller.automation_add_keyframe(track_id, node_id, Beats(new_time), new_value, interpolation.clone(), ease_out, ease_in); // Optimistic cache update (beats) if let Some(lanes) = self.automation_cache.get_mut(&layer_id) { if let Some(lane) = lanes.iter_mut().find(|l| l.node_id == node_id) { @@ -5769,7 +5769,7 @@ impl PaneRenderer for TimelinePane { } } else if let Some(&track_id) = shared.layer_to_track_map.get(&layer_id) { // time is already in beats - controller.automation_remove_keyframe(track_id, node_id, time); + controller.automation_remove_keyframe(track_id, node_id, Beats(time)); // Optimistic cache update (beats) if let Some(lanes) = self.automation_cache.get_mut(&layer_id) { if let Some(lane) = lanes.iter_mut().find(|l| l.node_id == node_id) {