diff --git a/daw-backend/src/audio/engine.rs b/daw-backend/src/audio/engine.rs index 2aed285..b129a5f 100644 --- a/daw-backend/src/audio/engine.rs +++ b/daw-backend/src/audio/engine.rs @@ -645,8 +645,10 @@ impl Engine { // every pass and — because this block also resizes the backend clip instance // below — shrank the instance back to nothing, so the notes just merged into it // were never scheduled and you overdubbed against silence. + // Pinned only once a pass has actually completed — until then the bar still grows. let duration = recording .cycle_loop_len + .filter(|_| recording.wrapped) .unwrap_or(current_time - recording.start_time); // In separate-takes mode the preview shows only the pass being played now — the // earlier passes are alternative takes, not layers, so drawing them all on top of @@ -1490,9 +1492,9 @@ impl Engine { None => {} } } - Command::StartRecording(track_id, start_time) => { + Command::StartRecording(track_id, start_time, force_takes) => { // Start recording on the specified track - self.handle_start_recording(track_id, start_time); + self.handle_start_recording(track_id, start_time, force_takes); } Command::StopRecording => { // Stop the current recording @@ -1510,9 +1512,9 @@ impl Engine { recording.resume(); } } - Command::StartMidiRecording(track_id, clip_id, start_time) => { + Command::StartMidiRecording(track_id, clip_id, start_time, force_takes) => { // Start MIDI recording on the specified track - self.handle_start_midi_recording(track_id, clip_id, start_time); + self.handle_start_midi_recording(track_id, clip_id, start_time, force_takes); } Command::StopMidiRecording => { eprintln!("[ENGINE] Received StopMidiRecording command"); @@ -3264,7 +3266,7 @@ impl Engine { } /// Handle starting a recording - fn handle_start_recording(&mut self, track_id: TrackId, start_time: Beats) { + fn handle_start_recording(&mut self, track_id: TrackId, start_time: Beats, force_takes: bool) { use crate::io::WavWriter; use std::env; @@ -3348,6 +3350,7 @@ impl Engine { loop_len_frames: (le - ls).max(0) as usize, lead_pad_frames: (self.playhead - ls).max(0) as usize, wrap_count: 0, + force_takes, } }) } else { @@ -3552,12 +3555,23 @@ impl Engine { } /// Handle starting MIDI recording - fn handle_start_midi_recording(&mut self, track_id: TrackId, clip_id: MidiClipId, start_time: Beats) { + fn handle_start_midi_recording(&mut self, track_id: TrackId, clip_id: MidiClipId, start_time: Beats, force_takes: bool) { // Check if track exists and is a MIDI track if let Some(crate::audio::track::TrackNode::Midi(_)) = self.project.get_track_mut(track_id) { // Create MIDI recording state let mut recording_state = MidiRecordingState::new(track_id, clip_id, start_time); + // Note the cycle region up front. `wrapped` stays false until a pass actually completes, + // so the clip bar still grows with the playhead through the first pass. + if self.loop_enabled { + if let Some((ls, le)) = self.loop_region { + if le > ls { + recording_state.cycle_loop_len = Some(le - ls); + recording_state.force_takes = force_takes; + } + } + } + // Inject any notes currently held on this track (pressed during count-in pre-roll) // so they start at t=0 of the recording rather than being lost if let Some(held) = self.midi_held_notes.get(&track_id) { @@ -3598,12 +3612,13 @@ impl Engine { // ---- Separate takes: one pool clip per cycle pass ---- // - // Only when the transport actually wrapped; a recording that stopped inside the first - // pass is an ordinary single recording and falls through to the merge path below, just - // as it does for audio. - if let (true, Some(loop_len)) = - (self.cycle_midi_separate_takes, recording.cycle_loop_len) - { + // Normally only when the transport actually wrapped: a recording that stopped inside the + // first pass is an ordinary single recording and falls through to the merge path below, + // just as it does for audio. `force_takes` overrides that, because the region already + // holds takes and this is another one however short it ran. + let takes_mode = self.cycle_midi_separate_takes + && (recording.wrapped || recording.force_takes); + if let (true, Some(loop_len)) = (takes_mode, recording.cycle_loop_len) { let loop_start = recording.start_time; // a cycle recording is anchored at the region let passes = recording.pass_count(); let buckets = recording.notes_by_pass(passes); @@ -3667,10 +3682,11 @@ impl Engine { let notes = recording.get_notes().to_vec(); let note_count = notes.len(); - // A cycle MIDI recording is anchored at the region start and every pass overdubs into - // the same clip (MERGE), so the clip is exactly one region long — not however long the - // user held the record button, which would run past the loop end. - let recording_duration = match recording.cycle_loop_len { + // A cycle MIDI recording that came round is anchored at the region start and every pass + // overdubs into the same clip (MERGE), so the clip is exactly one region long — not + // however long the user held the record button, which would run past the loop end. One + // that stopped inside the first pass is just an ordinary recording. + let recording_duration = match recording.cycle_loop_len.filter(|_| recording.wrapped) { Some(loop_len) => loop_len, None => end_time - recording.start_time, }; @@ -4206,8 +4222,11 @@ impl EngineController { } /// Start recording on a track - pub fn start_recording(&mut self, track_id: TrackId, start_time: Beats) { - let _ = self.command_tx.push(Command::StartRecording(track_id, start_time)); + /// `force_takes`: cut takes even if the transport never wraps, because the cycle region already + /// holds takes and this recording is another one. Whether that's so is document state, so only + /// the editor can answer it. + pub fn start_recording(&mut self, track_id: TrackId, start_time: Beats, force_takes: bool) { + let _ = self.command_tx.push(Command::StartRecording(track_id, start_time, force_takes)); } /// Stop the current recording @@ -4226,8 +4245,9 @@ impl EngineController { } /// Start MIDI recording on a track - pub fn start_midi_recording(&mut self, track_id: TrackId, clip_id: MidiClipId, start_time: Beats) { - let _ = self.command_tx.push(Command::StartMidiRecording(track_id, clip_id, start_time)); + /// `force_takes`: see [`EngineController::start_recording`]. + pub fn start_midi_recording(&mut self, track_id: TrackId, clip_id: MidiClipId, start_time: Beats, force_takes: bool) { + let _ = self.command_tx.push(Command::StartMidiRecording(track_id, clip_id, start_time, force_takes)); } /// Stop the current MIDI recording diff --git a/daw-backend/src/audio/recording.rs b/daw-backend/src/audio/recording.rs index 504f69a..652d603 100644 --- a/daw-backend/src/audio/recording.rs +++ b/daw-backend/src/audio/recording.rs @@ -24,9 +24,20 @@ pub struct CycleRecordInfo { /// punch-in (record while already rolling); take 1 gets this much silence prepended so it still /// spans the whole region. pub lead_pad_frames: usize, - /// How many times the transport wrapped during this recording. Zero means the user stopped - /// before completing a pass, which stays an ordinary single recording. + /// How many times the transport wrapped during this recording. Zero normally means the user + /// stopped before completing a pass, which stays an ordinary single recording — unless + /// `force_takes` says otherwise. pub wrap_count: usize, + /// Cut takes even if the transport never wrapped. + /// + /// Set when the region already holds takes: a further recording there is another take, however + /// short, and it gets padded out to the region like any partial pass. Without this a run that + /// stopped before the loop came round would land as a separate overlapping clip instead of + /// joining the take list. + /// + /// The editor decides this at record start, because whether takes already exist is document + /// state the engine can't see. + pub force_takes: bool, } /// Min/max waveform peaks for a finished buffer of interleaved samples. @@ -130,7 +141,7 @@ impl RecordingState { /// single recording, which keeps the existing path untouched). pub fn slice_takes(&self) -> Option>> { let cycle = self.cycle?; - if cycle.wrap_count == 0 || cycle.loop_len_frames == 0 { + if (cycle.wrap_count == 0 && !cycle.force_takes) || cycle.loop_len_frames == 0 { return None; } @@ -295,13 +306,19 @@ pub struct MidiRecordingState { active_notes: HashMap, /// Completed notes: (time_offset, note, velocity, duration) — all times in beats pub completed_notes: Vec<(Beats, u8, u8, Beats)>, - /// The cycle region's length in beats, when recording into a cycle. + /// The cycle region's length in beats, if one was armed at record start. /// /// A cycle MIDI recording is anchored at the region start (`start_time == loop_start`), which is /// what makes MERGE fall out for free: the transport always wraps back into the region, so every /// note's offset already lands inside `[0, loop_len)` and successive passes overdub onto each - /// other with no folding needed. Set only if the transport actually wrapped. + /// other with no folding needed. pub cycle_loop_len: Option, + /// Whether the transport actually came round. Distinct from `cycle_loop_len`, which only says a + /// region was armed: the clip only pins to the full region once a pass has completed, so until + /// then the bar still grows with the playhead. + pub wrapped: bool, + /// Cut takes even if the transport never wrapped — see [`CycleRecordInfo::force_takes`]. + pub force_takes: bool, /// Which cycle pass is currently being recorded (0-based). Bumped at each wrap. current_pass: usize, /// The pass each completed note belongs to, parallel to `completed_notes`. @@ -320,6 +337,8 @@ impl MidiRecordingState { active_notes: HashMap::new(), completed_notes: Vec::new(), cycle_loop_len: None, + wrapped: false, + force_takes: false, current_pass: 0, note_pass: Vec::new(), } @@ -450,9 +469,10 @@ impl MidiRecordingState { self.note_on(note, velocity, region_start); } - // The transport wrapped, so this is a cycle recording: the clip spans the whole region - // rather than however long the user happened to hold the record button. + // A pass has completed, so from here the clip spans the whole region rather than however long + // the user happens to hold the record button. self.cycle_loop_len = Some(region_end - region_start); + self.wrapped = true; } } @@ -462,6 +482,14 @@ mod cycle_tests { /// A recording state holding `audio_data`, armed for cycle recording. Mono, 100 Hz, so a frame /// is a sample and 5 frames is 50 ms (exactly the min-take threshold). + fn rec_forced(audio: Vec, loop_len_frames: usize, lead_pad_frames: usize, wraps: usize) -> RecordingState { + let mut r = rec(audio, loop_len_frames, lead_pad_frames, wraps); + if let Some(c) = r.cycle.as_mut() { + c.force_takes = true; + } + r + } + fn rec(audio: Vec, loop_len_frames: usize, lead_pad_frames: usize, wraps: usize) -> RecordingState { let mut r = RecordingState::new( 0, @@ -480,6 +508,7 @@ mod cycle_tests { loop_len_frames, lead_pad_frames, wrap_count: wraps, + force_takes: false, }); r } @@ -492,6 +521,18 @@ mod cycle_tests { assert!(r.slice_takes().is_none()); } + #[test] + fn force_takes_makes_a_partial_pass_a_take() { + // Recording over a region that already holds takes: this run is another take however short + // it ran, so it's cut and padded like any partial pass rather than landing as a separate + // overlapping clip. Two real frames of an 8-frame region -> one take, silence for the rest. + let takes = rec_forced(vec![1.0, 2.0], 8, 0, 0) + .slice_takes() + .expect("forced takes"); + assert_eq!(takes.len(), 1); + assert_eq!(takes[0], vec![1.0, 2.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]); + } + #[test] fn takes_are_cut_at_exact_loop_multiples() { // 12 frames of audio, 4-frame loop, started at the region start => 3 clean takes. diff --git a/daw-backend/src/command/types.rs b/daw-backend/src/command/types.rs index e475179..a87d58a 100644 --- a/daw-backend/src/command/types.rs +++ b/daw-backend/src/command/types.rs @@ -142,7 +142,9 @@ pub enum Command { // Recording commands /// Start recording on a track (track_id, start_time) - StartRecording(TrackId, Beats), + /// (track, start_time, force_takes — cut takes even if the transport never wraps, because the + /// region already holds takes and this is another one) + StartRecording(TrackId, Beats, bool), /// Stop the current recording StopRecording, /// Pause the current recording @@ -152,7 +154,8 @@ pub enum Command { // MIDI Recording commands /// Start MIDI recording on a track (track_id, clip_id, start_time) - StartMidiRecording(TrackId, MidiClipId, Beats), + /// (track, clip, start_time, force_takes — see [`Command::StartRecording`]) + StartMidiRecording(TrackId, MidiClipId, Beats, bool), /// Stop the current MIDI recording StopMidiRecording, diff --git a/lightningbeam-ui/lightningbeam-core/src/action.rs b/lightningbeam-ui/lightningbeam-core/src/action.rs index 4ac3547..2c96842 100644 --- a/lightningbeam-ui/lightningbeam-core/src/action.rs +++ b/lightningbeam-ui/lightningbeam-core/src/action.rs @@ -76,7 +76,7 @@ impl BackendContext<'_> { .get(layer_id) .ok_or_else(|| format!("Layer {} not mapped to backend track", layer_id))?; - let resolved = clip.resolve(instance.active_take); + let resolved = instance.resolve(clip); let content = clip.content_duration(); let internal_start = instance.trim_start; let internal_end = instance diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/append_takes.rs b/lightningbeam-ui/lightningbeam-core/src/actions/append_takes.rs index b32626e..24f9bf4 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/append_takes.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/append_takes.rs @@ -9,17 +9,16 @@ //! plays, which has to be repointed at the newly-active take. use crate::action::{Action, BackendClipInstanceId, BackendContext}; -use crate::clip::{AudioClipType, AudioTake}; +use crate::clip::AudioTake; use crate::document::Document; use crate::layer::AnyLayer; use uuid::Uuid; -/// Action that appends takes to a take-folder clip and selects the last of them. +/// Action that appends takes to an instance's take list and selects the last of them. pub struct AppendTakesAction { layer_id: Uuid, - /// The instance whose folder is being extended (and whose active take changes). + /// The instance whose take list is being extended (and whose active take changes). instance_id: Uuid, - clip_id: Uuid, /// The takes to add, in recording order. new_takes: Vec, @@ -30,11 +29,10 @@ pub struct AppendTakesAction { } impl AppendTakesAction { - pub fn new(layer_id: Uuid, instance_id: Uuid, clip_id: Uuid, new_takes: Vec) -> Self { + pub fn new(layer_id: Uuid, instance_id: Uuid, new_takes: Vec) -> Self { Self { layer_id, instance_id, - clip_id, new_takes, old_take_count: 0, old_active_take: None, @@ -71,32 +69,11 @@ impl AppendTakesAction { impl Action for AppendTakesAction { fn execute(&mut self, document: &mut Document) -> Result<(), String> { - let clip = document - .audio_clips - .get_mut(&self.clip_id) - .ok_or_else(|| format!("Audio clip {} not found", self.clip_id))?; - - let AudioClipType::TakeFolder { takes, .. } = &mut clip.clip_type else { - return Err("Can only append takes to a take folder".to_string()); - }; - - // Only record the pre-state on the first execute; a redo must not overwrite it with the - // post-state left behind by the previous run. - if !self.executed { - self.old_take_count = takes.len(); - } - takes.extend(self.new_takes.iter().cloned()); - // Renumber so the names stay in step with the take indices the badge shows. - for (i, take) in takes.iter_mut().enumerate() { - take.name = format!("Take {}", i + 1); - } - let new_active = takes.len() - 1; - let layer = document .get_layer_mut(&self.layer_id) .ok_or_else(|| format!("Layer {} not found", self.layer_id))?; let AnyLayer::Audio(audio_layer) = layer else { - return Err("Take folders only exist on audio layers".to_string()); + return Err("Takes only exist on audio layers".to_string()); }; let instance = audio_layer .clip_instances @@ -104,31 +81,38 @@ impl Action for AppendTakesAction { .find(|ci| ci.id == self.instance_id) .ok_or_else(|| format!("Clip instance {} not found", self.instance_id))?; + // Only record the pre-state on the first execute; a redo must not overwrite it with the + // post-state left behind by the previous run. if !self.executed { + self.old_take_count = instance.takes.len(); self.old_active_take = instance.active_take; } + + // Number the new takes on from what's already there. Existing names are left alone — the + // user may well have renamed them, and renumbering would clobber that. + let base = instance.takes.len(); + for (i, take) in self.new_takes.iter().enumerate() { + let mut take = take.clone(); + if take.name.is_empty() { + take.name = format!("Take {}", base + i + 1); + } + instance.takes.push(take); + } + // Land on the take just recorded, GarageBand-style. - instance.active_take = Some(new_active); + instance.active_take = Some(instance.takes.len() - 1); self.executed = true; Ok(()) } fn rollback(&mut self, document: &mut Document) -> Result<(), String> { - if let Some(clip) = document.audio_clips.get_mut(&self.clip_id) { - if let AudioClipType::TakeFolder { takes, .. } = &mut clip.clip_type { - takes.truncate(self.old_take_count); - for (i, take) in takes.iter_mut().enumerate() { - take.name = format!("Take {}", i + 1); - } - } - } - if let Some(AnyLayer::Audio(audio_layer)) = document.get_layer_mut(&self.layer_id) { if let Some(instance) = audio_layer .clip_instances .iter_mut() .find(|ci| ci.id == self.instance_id) { + instance.takes.truncate(self.old_take_count); instance.active_take = self.old_active_take; } } diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/loop_clip_instances.rs b/lightningbeam-ui/lightningbeam-core/src/actions/loop_clip_instances.rs index a68795a..4def017 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/loop_clip_instances.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/loop_clip_instances.rs @@ -141,7 +141,7 @@ impl LoopClipInstancesAction { let external_start = instance.timeline_start - left_duration; let get_backend_clip_id = |inst_id: &Uuid| -> Result { - match &clip.resolve(instance.active_take) { + match &instance.resolve(clip) { ResolvedContent::Midi { midi_clip_id } => Ok(*midi_clip_id), ResolvedContent::Audio { .. } => { let backend_id = backend.clip_instance_to_backend_map.get(inst_id) diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/manage_takes.rs b/lightningbeam-ui/lightningbeam-core/src/actions/manage_takes.rs new file mode 100644 index 0000000..4790a2b --- /dev/null +++ b/lightningbeam-ui/lightningbeam-core/src/actions/manage_takes.rs @@ -0,0 +1,419 @@ +//! Take management: delete and rename the takes on a clip instance. +//! +//! Takes live on the INSTANCE, so both of these are naturally scoped to the one the user clicked — +//! deleting a take from one half of a comped split leaves the other half's list alone. + +use crate::action::{Action, BackendClipInstanceId, BackendContext}; +use crate::clip::{AudioTake, ClipInstance}; +use crate::document::Document; +use crate::layer::AnyLayer; +use uuid::Uuid; + +/// The instance a take action targets, looked up mutably. +fn instance_mut<'a>( + document: &'a mut Document, + layer_id: &Uuid, + instance_id: &Uuid, +) -> Result<&'a mut ClipInstance, String> { + let layer = document + .get_layer_mut(layer_id) + .ok_or_else(|| format!("Layer {} not found", layer_id))?; + let AnyLayer::Audio(audio_layer) = layer else { + return Err("Takes only exist on audio layers".to_string()); + }; + audio_layer + .clip_instances + .iter_mut() + .find(|ci| ci.id == *instance_id) + .ok_or_else(|| format!("Clip instance {} not found", instance_id)) +} + +/// Swap an instance's backend clip to whatever take the document now says is active. +/// +/// The same remove + re-add as `SetActiveTakeAction` — there's no in-place pool-swap command. +fn resync( + backend: &mut BackendContext, + document: &Document, + layer_id: &Uuid, + instance_id: &Uuid, +) -> Result<(), String> { + let instance = document + .get_layer(layer_id) + .and_then(|l| match l { + AnyLayer::Audio(al) => al.clip_instances.iter().find(|ci| ci.id == *instance_id), + _ => None, + }) + .cloned() + .ok_or_else(|| format!("Clip instance {} not found", instance_id))?; + + let existing: Option = backend + .clip_instance_to_backend_map + .get(instance_id) + .copied(); + let track_id = backend.layer_to_track_map.get(layer_id).copied(); + if let (Some(backend_id), Some(track_id)) = (existing, track_id) { + backend.remove_clip_instance(track_id, backend_id, *instance_id); + } + + backend.add_clip_instance(document, layer_id, &instance)?; + Ok(()) +} + +/// Remove a take from an instance's take list. +/// +/// The take's recorded audio/MIDI stays in the backend pool — undo has to be able to put it back, +/// and other instances (the other half of a split, say) may still be playing it. +pub struct DeleteTakeAction { + layer_id: Uuid, + instance_id: Uuid, + take_index: usize, + + // Stored during execute for rollback. + removed: Option, + old_active_take: Option, +} + +impl DeleteTakeAction { + pub fn new(layer_id: Uuid, instance_id: Uuid, take_index: usize) -> Self { + Self { + layer_id, + instance_id, + take_index, + removed: None, + old_active_take: None, + } + } +} + +impl Action for DeleteTakeAction { + fn execute(&mut self, document: &mut Document) -> Result<(), String> { + let instance = instance_mut(document, &self.layer_id, &self.instance_id)?; + + if self.take_index >= instance.takes.len() { + return Err(format!("Take {} does not exist", self.take_index + 1)); + } + // An instance with no takes at all would fall back to the clip's own content, which for a + // cycle recording is a take we may just have deleted. Refuse rather than strand it. + if instance.takes.len() == 1 { + return Err("Can't delete the only take".to_string()); + } + + self.old_active_take = instance.active_take; + self.removed = Some(instance.takes.remove(self.take_index)); + + // Everything above the removed take shifts down one, so the selection has to move with it. + // Deleting the *active* take lands on the one that took its place (or the new last take, if + // it was at the end) — that keeps the clip sounding rather than silently picking take 1. + let active = instance.active_take.unwrap_or(0); + instance.active_take = Some(if active > self.take_index { + active - 1 + } else if active == self.take_index { + self.take_index.min(instance.takes.len() - 1) + } else { + active + }); + + Ok(()) + } + + fn rollback(&mut self, document: &mut Document) -> Result<(), String> { + let Some(take) = self.removed.take() else { + return Ok(()); + }; + let instance = instance_mut(document, &self.layer_id, &self.instance_id)?; + let at = self.take_index.min(instance.takes.len()); + instance.takes.insert(at, take); + instance.active_take = self.old_active_take; + Ok(()) + } + + fn description(&self) -> String { + format!("Delete take {}", self.take_index + 1) + } + + fn execute_backend( + &mut self, + backend: &mut BackendContext, + document: &Document, + ) -> Result<(), String> { + resync(backend, document, &self.layer_id, &self.instance_id) + } + + fn rollback_backend( + &mut self, + backend: &mut BackendContext, + document: &Document, + ) -> Result<(), String> { + resync(backend, document, &self.layer_id, &self.instance_id) + } +} + +/// Throw away every take except the one that's playing. +/// +/// The tidy-up once you've picked your keeper. Scoped to this instance, so on a comped split it +/// prunes the half you clicked and leaves the other half's alternatives intact. +pub struct DeleteUnusedTakesAction { + layer_id: Uuid, + instance_id: Uuid, + + // Stored during execute for rollback. + old_takes: Vec, + old_active_take: Option, +} + +impl DeleteUnusedTakesAction { + pub fn new(layer_id: Uuid, instance_id: Uuid) -> Self { + Self { + layer_id, + instance_id, + old_takes: Vec::new(), + old_active_take: None, + } + } +} + +impl Action for DeleteUnusedTakesAction { + fn execute(&mut self, document: &mut Document) -> Result<(), String> { + let instance = instance_mut(document, &self.layer_id, &self.instance_id)?; + if instance.takes.len() < 2 { + return Err("Nothing to delete".to_string()); + } + + let keep = instance.active_take_index(); + self.old_takes = instance.takes.clone(); + self.old_active_take = instance.active_take; + + let kept = instance.takes.remove(keep); + instance.takes.clear(); + instance.takes.push(kept); + instance.active_take = Some(0); + Ok(()) + } + + fn rollback(&mut self, document: &mut Document) -> Result<(), String> { + let instance = instance_mut(document, &self.layer_id, &self.instance_id)?; + instance.takes = std::mem::take(&mut self.old_takes); + instance.active_take = self.old_active_take; + Ok(()) + } + + fn description(&self) -> String { + "Delete unused takes".to_string() + } + + // The take that plays doesn't change, so the backend clip is already correct. +} + +/// Rename a take. Document-only — which take *plays* doesn't change, so the backend is untouched. +pub struct RenameTakeAction { + layer_id: Uuid, + instance_id: Uuid, + take_index: usize, + new_name: String, + old_name: String, +} + +impl RenameTakeAction { + pub fn new(layer_id: Uuid, instance_id: Uuid, take_index: usize, new_name: String) -> Self { + Self { + layer_id, + instance_id, + take_index, + new_name, + old_name: String::new(), + } + } +} + +impl Action for RenameTakeAction { + fn execute(&mut self, document: &mut Document) -> Result<(), String> { + let instance = instance_mut(document, &self.layer_id, &self.instance_id)?; + let take = instance + .takes + .get_mut(self.take_index) + .ok_or_else(|| format!("Take {} does not exist", self.take_index + 1))?; + self.old_name = std::mem::replace(&mut take.name, self.new_name.clone()); + Ok(()) + } + + fn rollback(&mut self, document: &mut Document) -> Result<(), String> { + let instance = instance_mut(document, &self.layer_id, &self.instance_id)?; + if let Some(take) = instance.takes.get_mut(self.take_index) { + take.name = self.old_name.clone(); + } + Ok(()) + } + + fn description(&self) -> String { + format!("Rename take to \"{}\"", self.new_name) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::clip::TakeContent; + use crate::layer::AudioLayer; + + /// A document with one audio layer holding one instance with 4 takes (pools 10..13). + fn doc_with_takes() -> (Document, Uuid, Uuid) { + let mut document = Document::new("Test"); + let clip = crate::clip::AudioClip::new_sampled("Cycle rec", 10, 2.0); + let clip_id = document.add_audio_clip(clip); + + let mut instance = ClipInstance::new(clip_id); + instance.takes = (10..14) + .map(|pool| AudioTake { + name: format!("Take {}", pool - 9), + content: TakeContent::Audio { audio_pool_index: pool }, + }) + .collect(); + let instance_id = instance.id; + + let mut layer = AudioLayer::new("Layer"); + layer.clip_instances.push(instance); + let layer_id = document.root.add_child(AnyLayer::Audio(layer)); + (document, layer_id, instance_id) + } + + fn takes_of(document: &Document, layer_id: &Uuid, instance_id: &Uuid) -> ClipInstance { + let AnyLayer::Audio(al) = document.get_layer(layer_id).unwrap() else { panic!() }; + al.clip_instances.iter().find(|ci| ci.id == *instance_id).unwrap().clone() + } + + #[test] + fn deleting_a_take_below_the_active_one_shifts_the_selection_down() { + // Everything above the removed take shifts down one, so a selection above it has to move + // with it — otherwise the instance silently starts playing a different take. + let (mut document, layer_id, instance_id) = doc_with_takes(); + { + let AnyLayer::Audio(al) = document.get_layer_mut(&layer_id).unwrap() else { panic!() }; + al.clip_instances[0].active_take = Some(3); // playing pool 13 + } + + DeleteTakeAction::new(layer_id, instance_id, 1) + .execute(&mut document) + .expect("delete"); + + let inst = takes_of(&document, &layer_id, &instance_id); + assert_eq!(inst.takes.len(), 3); + assert_eq!(inst.active_take, Some(2), "index shifted down with the take"); + assert_eq!( + inst.takes[inst.active_take_index()].content, + TakeContent::Audio { audio_pool_index: 13 }, + "still playing the same take it was", + ); + } + + #[test] + fn deleting_the_active_take_lands_on_its_replacement() { + // Deleting what you're listening to should hand you the take that took its place, not + // silently jump you back to take 1. + let (mut document, layer_id, instance_id) = doc_with_takes(); + { + let AnyLayer::Audio(al) = document.get_layer_mut(&layer_id).unwrap() else { panic!() }; + al.clip_instances[0].active_take = Some(1); // playing pool 11 + } + + DeleteTakeAction::new(layer_id, instance_id, 1) + .execute(&mut document) + .expect("delete"); + + let inst = takes_of(&document, &layer_id, &instance_id); + assert_eq!(inst.active_take, Some(1)); + assert_eq!( + inst.takes[1].content, + TakeContent::Audio { audio_pool_index: 12 }, + "the take that slid into the deleted one's place", + ); + } + + #[test] + fn deleting_the_last_take_in_the_list_steps_back() { + let (mut document, layer_id, instance_id) = doc_with_takes(); + { + let AnyLayer::Audio(al) = document.get_layer_mut(&layer_id).unwrap() else { panic!() }; + al.clip_instances[0].active_take = Some(3); + } + + DeleteTakeAction::new(layer_id, instance_id, 3) + .execute(&mut document) + .expect("delete"); + + let inst = takes_of(&document, &layer_id, &instance_id); + assert_eq!(inst.active_take, Some(2), "there is no take 4 to land on"); + } + + #[test] + fn the_only_take_cannot_be_deleted() { + let (mut document, layer_id, instance_id) = doc_with_takes(); + { + let AnyLayer::Audio(al) = document.get_layer_mut(&layer_id).unwrap() else { panic!() }; + al.clip_instances[0].takes.truncate(1); + } + assert!(DeleteTakeAction::new(layer_id, instance_id, 0) + .execute(&mut document) + .is_err()); + } + + #[test] + fn undoing_a_delete_puts_the_take_back_where_it_was() { + let (mut document, layer_id, instance_id) = doc_with_takes(); + { + let AnyLayer::Audio(al) = document.get_layer_mut(&layer_id).unwrap() else { panic!() }; + al.clip_instances[0].active_take = Some(2); + } + + let mut action = DeleteTakeAction::new(layer_id, instance_id, 1); + action.execute(&mut document).expect("delete"); + action.rollback(&mut document).expect("undo"); + + let inst = takes_of(&document, &layer_id, &instance_id); + assert_eq!(inst.takes.len(), 4); + assert_eq!( + inst.takes[1].content, + TakeContent::Audio { audio_pool_index: 11 }, + "restored at its original index", + ); + assert_eq!(inst.active_take, Some(2), "and the selection with it"); + } + + #[test] + fn deleting_unused_takes_keeps_the_one_thats_playing() { + let (mut document, layer_id, instance_id) = doc_with_takes(); + { + let AnyLayer::Audio(al) = document.get_layer_mut(&layer_id).unwrap() else { panic!() }; + al.clip_instances[0].active_take = Some(2); // pool 12 — the keeper + } + + let mut action = DeleteUnusedTakesAction::new(layer_id, instance_id); + action.execute(&mut document).expect("prune"); + + let inst = takes_of(&document, &layer_id, &instance_id); + assert_eq!(inst.takes.len(), 1); + assert_eq!(inst.active_take, Some(0)); + assert_eq!( + inst.takes[0].content, + TakeContent::Audio { audio_pool_index: 12 }, + "the take that was playing survives, and nothing else", + ); + + action.rollback(&mut document).expect("undo"); + let inst = takes_of(&document, &layer_id, &instance_id); + assert_eq!(inst.takes.len(), 4); + assert_eq!(inst.active_take, Some(2), "back to what was playing before"); + } + + #[test] + fn renaming_a_take_round_trips() { + let (mut document, layer_id, instance_id) = doc_with_takes(); + let mut action = + RenameTakeAction::new(layer_id, instance_id, 2, "The good one".to_string()); + + action.execute(&mut document).expect("rename"); + assert_eq!(takes_of(&document, &layer_id, &instance_id).takes[2].name, "The good one"); + + action.rollback(&mut document).expect("undo"); + assert_eq!(takes_of(&document, &layer_id, &instance_id).takes[2].name, "Take 3"); + } +} diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs b/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs index 148c1f6..a848cab 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/mod.rs @@ -16,6 +16,7 @@ pub mod paint_bucket; pub mod remove_effect; pub mod set_cycle_region; pub mod append_takes; +pub mod manage_takes; pub mod set_active_take; pub mod set_document_properties; pub mod set_instance_properties; @@ -55,6 +56,7 @@ pub mod resize_text_box; pub use add_clip_instance::AddClipInstanceAction; pub use set_cycle_region::SetCycleRegionAction; pub use append_takes::AppendTakesAction; +pub use manage_takes::{DeleteTakeAction, DeleteUnusedTakesAction, RenameTakeAction}; pub use set_active_take::SetActiveTakeAction; pub use add_effect::AddEffectAction; pub use add_layer::AddLayerAction; 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 0439e04..20aa7ef 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/move_clip_instances.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/move_clip_instances.rs @@ -247,7 +247,7 @@ impl Action for MoveClipInstancesAction { .ok_or_else(|| format!("Audio clip {} not found", instance.clip_id))?; // Handle move based on clip type - match &clip.resolve(instance.active_take) { + match &instance.resolve(clip) { ResolvedContent::Midi { midi_clip_id } => { // For MIDI: move_clip expects the pool clip ID controller.move_clip(*track_id, *midi_clip_id, *new_start); @@ -332,7 +332,7 @@ impl Action for MoveClipInstancesAction { .ok_or_else(|| format!("Audio clip {} not found", instance.clip_id))?; // Handle move based on clip type (restore old position) - match &clip.resolve(instance.active_take) { + match &instance.resolve(clip) { ResolvedContent::Midi { midi_clip_id } => { // For MIDI: move_clip expects the pool clip ID controller.move_clip(*track_id, *midi_clip_id, *old_start); diff --git a/lightningbeam-ui/lightningbeam-core/src/actions/split_clip_instance.rs b/lightningbeam-ui/lightningbeam-core/src/actions/split_clip_instance.rs index c234510..2f19b85 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/split_clip_instance.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/split_clip_instance.rs @@ -364,7 +364,7 @@ impl Action for SplitClipInstanceAction { .ok_or_else(|| "Audio clip not found".to_string())?; use crate::clip::ResolvedContent; - if matches!(clip.resolve(original_instance.active_take), ResolvedContent::Recording) { + if matches!(original_instance.resolve(clip), ResolvedContent::Recording) { return Err("Cannot split a clip that is currently recording".to_string()); } @@ -455,7 +455,7 @@ impl Action for SplitClipInstanceAction { // Restore based on clip type use crate::clip::ResolvedContent; - match &clip.resolve(instance.active_take) { + match &instance.resolve(clip) { ResolvedContent::Midi { .. } => { if let Some(crate::action::BackendClipInstanceId::Midi(orig_backend_id)) = backend.clip_instance_to_backend_map.get(&self.instance_id) 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 ef97fee..7e99d94 100644 --- a/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs +++ b/lightningbeam-ui/lightningbeam-core/src/actions/trim_clip_instances.rs @@ -487,7 +487,7 @@ impl Action for TrimClipInstancesAction { .unwrap_or(ContentTime(clip.content_duration().native())); // Handle trim based on clip type - match &clip.resolve(instance.active_take) { + match &instance.resolve(clip) { ResolvedContent::Midi { midi_clip_id } => { // For MIDI: trim_clip expects the pool clip ID controller.trim_clip(*track_id, *midi_clip_id, clip.trim_range(internal_start, internal_end)); @@ -584,7 +584,7 @@ impl Action for TrimClipInstancesAction { }; // Handle trim based on clip type - match &clip.resolve(instance.active_take) { + match &instance.resolve(clip) { ResolvedContent::Midi { midi_clip_id } => { // For MIDI: trim_clip expects the pool clip ID controller.trim_clip(*track_id, *midi_clip_id, clip.trim_range(internal_start, internal_end)); diff --git a/lightningbeam-ui/lightningbeam-core/src/clip.rs b/lightningbeam-ui/lightningbeam-core/src/clip.rs index 447fbfe..f39da7c 100644 --- a/lightningbeam-ui/lightningbeam-core/src/clip.rs +++ b/lightningbeam-ui/lightningbeam-core/src/clip.rs @@ -472,39 +472,20 @@ pub enum AudioClipType { /// Placeholder for a clip that is currently being recorded. /// The audio_pool_index will be assigned when recording stops. Recording, - /// A folder of alternate takes, produced by cycle recording. - /// - /// Each pass of the transport around the cycle region becomes one take. Every take spans the - /// **full** cycle region (partial passes are padded with silence at capture time), so all takes - /// are the same length and share this clip's `duration` — which in turn means switching takes - /// never changes the clip's geometry, and splitting a take-folder instance yields two halves - /// whose takes still line up. Which take actually sounds is per-*instance* - /// ([`ClipInstance::active_take`]), not per-clip, so a split can play take 1 on the left and - /// take 3 on the right. That's comping. - TakeFolder { - /// The takes, in the order they were recorded. Never empty in practice. - takes: Vec, - /// The cycle region's length in beats at the time of recording. - /// - /// Audio takes are segmented geometrically (by sample count), so they're only meaningful - /// against the tempo they were cut at. Keeping the recorded length lets a future - /// time-stretch/conform feature reconcile the takes if the tempo changes underneath them. - recorded_loop_beats: Beats, - }, } -/// One take in a [`AudioClipType::TakeFolder`]. +/// One take of a cycle recording — see [`ClipInstance::takes`]. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct AudioTake { - /// Display name, e.g. "Take 1". + /// Display name, e.g. "Take 1". User-editable. pub name: String, /// The recorded content this take points at. pub content: TakeContent, } -/// What a take actually holds. A folder's takes are all the same kind — one cycle-record session +/// What a take actually holds. An instance's takes are all the same kind — one cycle-record session /// captures either audio or MIDI, never a mix. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub enum TakeContent { /// Sampled audio: index into the audio pool. Audio { audio_pool_index: usize }, @@ -608,18 +589,8 @@ impl AudioClip { } /// Whether this clip's `duration` is measured in beats (MIDI) rather than seconds. - /// - /// A take folder inherits the domain of its takes, which are all the same kind — one - /// cycle-record session captures either audio or MIDI, never a mix. An empty folder can't - /// happen in practice; call it seconds so the fallback is the common case. fn is_midi_domain(&self) -> bool { - match &self.clip_type { - AudioClipType::Midi { .. } => true, - AudioClipType::Sampled { .. } | AudioClipType::Recording => false, - AudioClipType::TakeFolder { takes, .. } => { - matches!(takes.first().map(|t| &t.content), Some(TakeContent::Midi { .. })) - } - } + matches!(self.clip_type, AudioClipType::Midi { .. }) } /// Create a new sampled audio clip @@ -706,34 +677,11 @@ impl AudioClip { } } - /// The clip's takes, if it's a take folder. - pub fn takes(&self) -> Option<&[AudioTake]> { - match &self.clip_type { - AudioClipType::TakeFolder { takes, .. } => Some(takes), - _ => None, - } - } - - /// The take an instance's `active_take` actually selects. + /// The clip's own content, ignoring takes. /// - /// `None` means take 0, which is also what an out-of-range index falls back to — an index can - /// go stale (an old `.beam`, an undo that shrank the folder), and silently playing the first - /// take beats refusing to play anything. - fn take_for(&self, active_take: Option) -> Option<&AudioTake> { - let takes = self.takes()?; - takes - .get(active_take.unwrap_or(0)) - .or_else(|| takes.first()) - } - - /// What this clip plays *for a given instance*, with take folders collapsed to the instance's - /// active take. - /// - /// This is the sanctioned way to ask "what content do I hand the backend for this instance?". - /// Matching on `clip_type` directly will see a `TakeFolder` and have to handle it separately; - /// matching on this won't, because a folder is never a distinct case here — it's just an audio - /// or MIDI clip whose identity depends on which take is active. - pub fn resolve(&self, active_take: Option) -> ResolvedContent { + /// Callers that are handing content to the backend want [`ClipInstance::resolve`] instead — an + /// instance with takes overrides the clip's content with whichever take is active. + pub fn resolve(&self) -> ResolvedContent { match &self.clip_type { AudioClipType::Sampled { audio_pool_index } => ResolvedContent::Audio { audio_pool_index: *audio_pool_index, @@ -742,17 +690,6 @@ impl AudioClip { midi_clip_id: *midi_clip_id, }, AudioClipType::Recording => ResolvedContent::Recording, - AudioClipType::TakeFolder { .. } => match self.take_for(active_take).map(|t| &t.content) { - Some(TakeContent::Audio { audio_pool_index }) => ResolvedContent::Audio { - audio_pool_index: *audio_pool_index, - }, - Some(TakeContent::Midi { midi_clip_id }) => ResolvedContent::Midi { - midi_clip_id: *midi_clip_id, - }, - // An empty folder has nothing to play. Treat it like a recording placeholder: - // the backend gets nothing, rather than a bogus pool index. - None => ResolvedContent::Recording, - }, } } @@ -787,49 +724,19 @@ impl AudioClip { } } - /// Whether this clip owns the given audio pool index — either as a plain sampled clip, or as - /// *any* take of a take folder. Reverse lookups (backend resource → document clip) must use - /// this: a folder owns one pool file per take, not just the active one. + /// Whether this clip's own content is the given audio pool index. pub fn owns_audio_pool_index(&self, pool_index: usize) -> bool { - match &self.clip_type { - AudioClipType::Sampled { audio_pool_index } => *audio_pool_index == pool_index, - AudioClipType::TakeFolder { takes, .. } => takes.iter().any(|t| { - matches!(t.content, TakeContent::Audio { audio_pool_index } if audio_pool_index == pool_index) - }), - _ => false, - } + self.audio_pool_index() == Some(pool_index) } - /// Whether this clip owns the given backend MIDI clip ID. See [`Self::owns_audio_pool_index`]. + /// Whether this clip's own content is the given backend MIDI clip ID. pub fn owns_midi_clip_id(&self, id: u32) -> bool { - match &self.clip_type { - AudioClipType::Midi { midi_clip_id } => *midi_clip_id == id, - AudioClipType::TakeFolder { takes, .. } => takes.iter().any(|t| { - matches!(t.content, TakeContent::Midi { midi_clip_id } if midi_clip_id == id) - }), - _ => false, - } - } - - /// The audio pool index this *instance* should play. See [`Self::resolve`]. - pub fn resolved_audio_pool_index(&self, active_take: Option) -> Option { - match self.resolve(active_take) { - ResolvedContent::Audio { audio_pool_index } => Some(audio_pool_index), - _ => None, - } - } - - /// The backend MIDI clip ID this *instance* should play. See [`Self::resolve`]. - pub fn resolved_midi_clip_id(&self, active_take: Option) -> Option { - match self.resolve(active_take) { - ResolvedContent::Midi { midi_clip_id } => Some(midi_clip_id), - _ => None, - } + self.midi_clip_id() == Some(id) } } -/// What a clip instance actually plays, once take folders are resolved to their active take. -/// Produced by [`AudioClip::resolve`]. +/// What a clip instance actually plays, once takes are resolved to the active one. +/// Produced by [`ClipInstance::resolve`]. #[derive(Clone, Copy, Debug, PartialEq)] pub enum ResolvedContent { Audio { audio_pool_index: usize }, @@ -942,14 +849,34 @@ pub struct ClipInstance { #[serde(default, skip_serializing_if = "Option::is_none")] pub loop_before: Option, - /// Which take of a [`AudioClipType::TakeFolder`] clip this instance plays. + /// Alternate takes from cycle recording. Empty = an ordinary instance with no takes. /// - /// Per-instance rather than per-clip so two instances of the same folder — e.g. the two halves - /// of a split — can play different takes. That's how comping works. `None` means take 0; - /// meaningless (and ignored) on non-folder clips. + /// The takes live on the INSTANCE, not the clip, so managing them is per-instance: deleting or + /// renaming a take on one instance leaves every other instance alone. Splitting clones the list + /// along with the rest of the instance, so the two halves get independent take lists — and, + /// since they can each select a different take, comping still falls out for free. + /// + /// Every take spans the full cycle region (partial passes are padded with silence at capture + /// time), so they're all the same length as the clip's own content. That uniformity is what lets + /// a take switch leave the instance's geometry untouched. + /// + /// When non-empty, these OVERRIDE the clip's own content — see [`Self::resolve`]. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub takes: Vec, + + /// Which of [`Self::takes`] plays. `None` (or a stale index) means take 0. /// Default: None #[serde(default, skip_serializing_if = "Option::is_none")] pub active_take: Option, + + /// The cycle region's length in beats when these takes were recorded. + /// + /// Audio takes are cut geometrically (by sample count), so they're only meaningful against the + /// tempo they were recorded at. Keeping the recorded length lets a future time-stretch/conform + /// feature reconcile them if the tempo moves underneath, and lets a new recording tell whether + /// it belongs in this take list or a fresh one. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub recorded_loop_beats: Option, } /// High 64-bit sentinel used to identify UUIDs that encode a backend audio clip instance ID. @@ -1008,7 +935,9 @@ impl ClipInstance { playback_speed: 1.0, gain: 1.0, loop_before: None, + takes: Vec::new(), active_take: None, + recorded_loop_beats: None, } } @@ -1027,7 +956,9 @@ impl ClipInstance { playback_speed: 1.0, gain: 1.0, loop_before: None, + takes: Vec::new(), active_take: None, + recorded_loop_beats: None, } } @@ -1087,6 +1018,55 @@ impl ClipInstance { self } + /// The take this instance plays, if it has any. + /// + /// A `None`/stale `active_take` falls back to take 0 — an index can go stale (an undo that + /// shrank the list, an old `.beam`), and silently playing the first take beats playing nothing. + pub fn active_take(&self) -> Option<&AudioTake> { + self.takes + .get(self.active_take.unwrap_or(0)) + .or_else(|| self.takes.first()) + } + + /// The index [`Self::active_take`] actually resolves to, clamped into range. + pub fn active_take_index(&self) -> usize { + let i = self.active_take.unwrap_or(0); + if i < self.takes.len() { i } else { 0 } + } + + /// What this instance plays: its active take if it has takes, otherwise the clip's own content. + /// + /// This is the sanctioned way to ask "what content do I hand the backend for this instance?". + /// Takes are never a distinct *case* at the call site — an instance with takes is just an audio + /// or MIDI instance whose identity depends on which take is live. + pub fn resolve(&self, clip: &AudioClip) -> ResolvedContent { + match self.active_take().map(|t| &t.content) { + Some(TakeContent::Audio { audio_pool_index }) => ResolvedContent::Audio { + audio_pool_index: *audio_pool_index, + }, + Some(TakeContent::Midi { midi_clip_id }) => ResolvedContent::Midi { + midi_clip_id: *midi_clip_id, + }, + None => clip.resolve(), + } + } + + /// The audio pool index this instance plays. See [`Self::resolve`]. + pub fn resolved_audio_pool_index(&self, clip: &AudioClip) -> Option { + match self.resolve(clip) { + ResolvedContent::Audio { audio_pool_index } => Some(audio_pool_index), + _ => None, + } + } + + /// The backend MIDI clip ID this instance plays. See [`Self::resolve`]. + pub fn resolved_midi_clip_id(&self, clip: &AudioClip) -> Option { + match self.resolve(clip) { + ResolvedContent::Midi { midi_clip_id } => Some(midi_clip_id), + _ => None, + } + } + /// Content window (`trim_end - trim_start`) in the clip's own content domain. /// Used for internal looping calculations. pub fn content_window(&self, clip_content: ClipDuration) -> ClipDuration { @@ -1328,79 +1308,89 @@ mod tests { assert_eq!(instance.gain, 0.8); } - /// Build a take folder of `n` audio takes with the given pool indices. - fn take_folder(pool_indices: &[usize]) -> AudioClip { - let mut clip = AudioClip::new_sampled("Cycle rec", 0, 2.0); - clip.clip_type = AudioClipType::TakeFolder { - takes: pool_indices - .iter() - .enumerate() - .map(|(i, &audio_pool_index)| AudioTake { - name: format!("Take {}", i + 1), - content: TakeContent::Audio { audio_pool_index }, - }) - .collect(), - recorded_loop_beats: Beats(8.0), - }; - clip + /// A clip whose own content is pool 10, plus an instance carrying takes over the given pools. + fn with_takes(pool_indices: &[usize]) -> (AudioClip, ClipInstance) { + let clip = AudioClip::new_sampled("Cycle rec", pool_indices[0], 2.0); + let mut instance = ClipInstance::new(clip.id); + instance.takes = pool_indices + .iter() + .enumerate() + .map(|(i, &audio_pool_index)| AudioTake { + name: format!("Take {}", i + 1), + content: TakeContent::Audio { audio_pool_index }, + }) + .collect(); + instance.recorded_loop_beats = Some(Beats(8.0)); + (clip, instance) } #[test] fn active_take_selects_the_pool_file() { - let clip = take_folder(&[10, 11, 12]); - assert_eq!(clip.resolved_audio_pool_index(Some(0)), Some(10)); - assert_eq!(clip.resolved_audio_pool_index(Some(2)), Some(12)); + let (clip, mut instance) = with_takes(&[10, 11, 12]); + instance.active_take = Some(0); + assert_eq!(instance.resolved_audio_pool_index(&clip), Some(10)); + instance.active_take = Some(2); + assert_eq!(instance.resolved_audio_pool_index(&clip), Some(12)); // None means take 0. - assert_eq!(clip.resolved_audio_pool_index(None), Some(10)); + instance.active_take = None; + assert_eq!(instance.resolved_audio_pool_index(&clip), Some(10)); } #[test] fn out_of_range_take_falls_back_to_the_first() { - // An index can go stale (an old .beam, an undo that shrank the folder). Playing the first - // take beats playing nothing. - let clip = take_folder(&[10, 11]); - assert_eq!(clip.resolved_audio_pool_index(Some(99)), Some(10)); + // An index can go stale (an undo that shrank the list, an old .beam). Playing the first take + // beats playing nothing. + let (clip, mut instance) = with_takes(&[10, 11]); + instance.active_take = Some(99); + assert_eq!(instance.resolved_audio_pool_index(&clip), Some(10)); + assert_eq!(instance.active_take_index(), 0); } #[test] - fn take_folder_owns_every_takes_pool_file() { - // Reverse lookups (backend resource -> document clip) must find the folder via ANY take, - // not just the active one. - let clip = take_folder(&[10, 11, 12]); - assert!(clip.owns_audio_pool_index(10)); - assert!(clip.owns_audio_pool_index(12)); - assert!(!clip.owns_audio_pool_index(13)); + fn an_instance_without_takes_plays_the_clips_own_content() { + let clip = AudioClip::new_sampled("Plain", 42, 2.0); + let instance = ClipInstance::new(clip.id); + assert!(instance.takes.is_empty()); + assert_eq!(instance.resolved_audio_pool_index(&clip), Some(42)); } #[test] - fn midi_take_folder_measures_duration_in_beats() { - // A folder inherits its takes' domain: MIDI takes mean the duration is beats, not seconds. - let mut clip = AudioClip::new_sampled("Cycle rec", 0, 4.0); - clip.clip_type = AudioClipType::TakeFolder { - takes: vec![AudioTake { - name: "Take 1".into(), - content: TakeContent::Midi { midi_clip_id: 7 }, - }], - recorded_loop_beats: Beats(4.0), - }; + fn midi_takes_resolve_to_midi_content() { + let clip = AudioClip::new_midi("Cycle rec", 7, Beats(4.0)); + let mut instance = ClipInstance::new(clip.id); + instance.takes = vec![AudioTake { + name: "Take 1".into(), + content: TakeContent::Midi { midi_clip_id: 7 }, + }]; assert_eq!(clip.content_duration(), ClipDuration::Beats(Beats(4.0))); - assert_eq!(clip.resolved_midi_clip_id(Some(0)), Some(7)); - assert_eq!(clip.resolved_audio_pool_index(Some(0)), None); + assert_eq!(instance.resolved_midi_clip_id(&clip), Some(7)); + assert_eq!(instance.resolved_audio_pool_index(&clip), None); } #[test] - fn takes_are_per_instance_so_a_split_can_comp() { - // The whole point of putting active_take on the instance: two instances of the same folder - // (which is what a split produces) can play different takes. - let clip = take_folder(&[10, 11, 12]); - let mut left = ClipInstance::new(clip.id); - let mut right = left.clone(); + fn splitting_gives_each_half_an_independent_take_list() { + // Takes live on the INSTANCE, so a split (which clones the instance) hands each half its own + // list. Two consequences, both wanted: the halves can select different takes (comping), and + // deleting a take from one leaves the other alone. + let (clip, left_src) = with_takes(&[10, 11, 12]); + let mut left = left_src.clone(); + let mut right = left_src.clone(); right.id = Uuid::new_v4(); + left.active_take = Some(0); right.active_take = Some(2); + assert_eq!(left.resolved_audio_pool_index(&clip), Some(10)); + assert_eq!(right.resolved_audio_pool_index(&clip), Some(12)); - assert_eq!(clip.resolved_audio_pool_index(left.active_take), Some(10)); - assert_eq!(clip.resolved_audio_pool_index(right.active_take), Some(12)); + // Delete take 2 (pool 11) from the left half only. + left.takes.remove(1); + assert_eq!(left.takes.len(), 2); + assert_eq!(right.takes.len(), 3, "the other half keeps its own takes"); + assert_eq!( + right.resolved_audio_pool_index(&clip), + Some(12), + "and its selection still points where it did", + ); } #[test] diff --git a/lightningbeam-ui/lightningbeam-core/src/document.rs b/lightningbeam-ui/lightningbeam-core/src/document.rs index a7b8d67..72bde66 100644 --- a/lightningbeam-ui/lightningbeam-core/src/document.rs +++ b/lightningbeam-ui/lightningbeam-core/src/document.rs @@ -944,31 +944,27 @@ impl Document { /// would break the uniform-take invariant comping depends on. /// /// `exclude` is the in-progress recording's own instance, which is on the layer but isn't a - /// candidate. Returns (instance id, clip id). + /// candidate. Returns the instance id. pub fn take_folder_at( &self, layer_id: &Uuid, loop_start: Beats, loop_len: Beats, exclude: &Uuid, - ) -> Option<(Uuid, Uuid)> { + ) -> Option { let Some(AnyLayer::Audio(audio_layer)) = self.get_layer(layer_id) else { return None; }; const EPS: f64 = 1e-6; audio_layer.clip_instances.iter().find_map(|ci| { - if ci.id == *exclude || (ci.timeline_start - loop_start).beats_to_f64().abs() > EPS { + if ci.id == *exclude + || ci.takes.is_empty() + || (ci.timeline_start - loop_start).beats_to_f64().abs() > EPS + { return None; } - let clip = self.audio_clips.get(&ci.clip_id)?; - match clip.clip_type { - crate::clip::AudioClipType::TakeFolder { recorded_loop_beats, .. } - if (recorded_loop_beats - loop_len).beats_to_f64().abs() < EPS => - { - Some((ci.id, ci.clip_id)) - } - _ => None, - } + let recorded = ci.recorded_loop_beats?; + ((recorded - loop_len).beats_to_f64().abs() < EPS).then_some(ci.id) }) } diff --git a/lightningbeam-ui/lightningbeam-editor/src/main.rs b/lightningbeam-ui/lightningbeam-editor/src/main.rs index f9bfe5c..5a4aa5a 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/main.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/main.rs @@ -1112,6 +1112,11 @@ struct PendingTakeAppend { /// The throwaway clip + instance the recording itself was captured into. recording_instance_id: Uuid, recording_clip_id: Uuid, + /// The recording's backend clip, which has to be torn down or it keeps playing alongside the + /// take folder we're appending to. Carried explicitly rather than looked up in + /// `clip_instance_to_backend_map`, because on the audio path the recording instance isn't in + /// that map yet — it's only added after promotion, which the append path skips. + recording_backend_id: Option, loop_start: Beats, loop_len: Beats, takes: Vec, @@ -2069,11 +2074,12 @@ impl EditorApp { layer_id: uuid::Uuid, recording_instance_id: uuid::Uuid, recording_clip_id: uuid::Uuid, + recording_backend_id: Option, loop_start: Beats, loop_len: Beats, takes: Vec, ) -> bool { - let Some((target_instance_id, target_clip_id)) = self.action_executor.document().take_folder_at( + let Some(target_instance_id) = self.action_executor.document().take_folder_at( &layer_id, loop_start, loop_len, @@ -2082,9 +2088,12 @@ impl EditorApp { return false; }; - // Drop the recording's backend clip; the target instance's own clip gets repointed at the - // new active take by the action's backend sync. - let backend_id = self.clip_instance_to_backend_map.remove(&recording_instance_id); + // Tear down the recording's own backend clip. Without this it keeps playing on top of the + // take folder we're appending to — two takes sounding at once. The target instance's clip is + // separately repointed at the new active take by the action's backend sync below. + let backend_id = recording_backend_id + .or_else(|| self.clip_instance_to_backend_map.remove(&recording_instance_id)); + self.clip_instance_to_backend_map.remove(&recording_instance_id); let track_id = self.layer_to_track_map.get(&layer_id).copied(); if let (Some(backend_id), Some(track_id), Some(controller_arc)) = (backend_id, track_id, self.audio_controller.as_ref()) @@ -2112,7 +2121,6 @@ impl EditorApp { let action = lightningbeam_core::actions::AppendTakesAction::new( layer_id, target_instance_id, - target_clip_id, takes, ); @@ -6692,6 +6700,13 @@ impl eframe::App for EditorApp { layer_id, recording_instance_id: instance_id, recording_clip_id: clip_id, + // The engine's recording clip. It isn't in + // clip_instance_to_backend_map yet (that only happens on + // promotion, which we're skipping), so hand it over + // directly or it'll keep sounding alongside the folder. + recording_backend_id: Some( + lightningbeam_core::action::BackendClipInstanceId::Audio(backend_clip_id), + ), loop_start, loop_len: loop_len_beats, takes: new_takes, @@ -6701,22 +6716,18 @@ impl eframe::App for EditorApp { continue; } - // Promote the in-progress recording clip to a take folder. + // Finalize the recording clip, and hang the takes off the INSTANCE. { let doc = self.action_executor.document_mut(); if let Some(clip) = doc.audio_clips.get_mut(&clip_id) { - clip.clip_type = lightningbeam_core::clip::AudioClipType::TakeFolder { - takes: takes.iter().enumerate().map(|(i, &(pool_index, _))| { - lightningbeam_core::clip::AudioTake { - name: format!("Take {}", i + 1), - content: lightningbeam_core::clip::TakeContent::Audio { audio_pool_index: pool_index }, - } - }).collect(), - recorded_loop_beats: loop_len_beats, - }; - // Audio takes are seconds-domain, and every take is - // exactly one cycle region long. - clip.set_content_duration(ClipDuration::Seconds(loop_len_seconds)); + // The clip's own content is take 1; the instance's take + // list overrides it with whichever take is active. Every + // take is exactly one cycle region long, so the clip's + // duration is the region. + clip.finalize_recording( + takes[0].0, + loop_len_seconds.seconds_to_f64(), + ); clip.name = format!("Cycle recording ({} takes)", takes.len()); } @@ -6735,7 +6746,14 @@ impl eframe::App for EditorApp { inst.trim_start = daw_backend::ContentTime::ZERO; inst.trim_end = Some(daw_backend::ContentTime(loop_len_seconds.seconds_to_f64())); + inst.takes = takes.iter().enumerate().map(|(i, &(pool_index, _))| { + lightningbeam_core::clip::AudioTake { + name: format!("Take {}", i + 1), + content: lightningbeam_core::clip::TakeContent::Audio { audio_pool_index: pool_index }, + } + }).collect(); inst.active_take = Some(last_take); + inst.recorded_loop_beats = Some(loop_len_beats); } } } @@ -7029,6 +7047,9 @@ impl eframe::App for EditorApp { layer_id, recording_instance_id: rec_inst, recording_clip_id: doc_clip_id, + // The MIDI path maps its recording instance during + // MidiRecordingProgress, so the map has it. + recording_backend_id: None, loop_start, loop_len: loop_len_beats, takes: new_takes, @@ -7046,17 +7067,10 @@ impl eframe::App for EditorApp { { let doc = self.action_executor.document_mut(); if let Some(clip) = doc.audio_clips.get_mut(&doc_clip_id) { - clip.clip_type = lightningbeam_core::clip::AudioClipType::TakeFolder { - takes: clip_ids.iter().enumerate().map(|(i, &mid)| { - lightningbeam_core::clip::AudioTake { - name: format!("Take {}", i + 1), - content: lightningbeam_core::clip::TakeContent::Midi { midi_clip_id: mid }, - } - }).collect(), - recorded_loop_beats: loop_len_beats, - }; - // MIDI takes are beats-domain, and every take spans exactly - // one cycle region. + // The clip's own content stays take 1 (the clip the + // recording started on); the instance's take list + // overrides it with whichever take is active. MIDI takes + // are beats-domain and each spans one cycle region. clip.set_content_duration(ClipDuration::Beats(loop_len_beats)); clip.name = format!("Cycle recording ({} takes)", clip_ids.len()); } @@ -7071,7 +7085,14 @@ impl eframe::App for EditorApp { inst.timeline_duration = None; inst.trim_start = daw_backend::ContentTime::ZERO; inst.trim_end = Some(daw_backend::ContentTime(loop_len_beats.beats_to_f64())); + inst.takes = clip_ids.iter().enumerate().map(|(i, &mid)| { + lightningbeam_core::clip::AudioTake { + name: format!("Take {}", i + 1), + content: lightningbeam_core::clip::TakeContent::Midi { midi_clip_id: mid }, + } + }).collect(); inst.active_take = Some(last_take); + inst.recorded_loop_beats = Some(loop_len_beats); } } } @@ -7263,6 +7284,7 @@ impl eframe::App for EditorApp { req.layer_id, req.recording_instance_id, req.recording_clip_id, + req.recording_backend_id, req.loop_start, req.loop_len, req.takes, diff --git a/lightningbeam-ui/lightningbeam-editor/src/mobile/icons.rs b/lightningbeam-ui/lightningbeam-editor/src/mobile/icons.rs index f1b0e21..7132ce8 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/mobile/icons.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/mobile/icons.rs @@ -43,6 +43,7 @@ pub const CHEVRONS_UP: &str = "\u{e074}"; pub const PLAY: &str = "\u{e13c}"; pub const PAUSE: &str = "\u{e12e}"; pub const REPEAT: &str = "\u{e146}"; // cycle / loop region toggle +pub const TRASH: &str = "\u{e18d}"; // delete a take pub const SETTINGS: &str = "\u{e154}"; pub const SEARCH: &str = "\u{e151}"; pub const PLUS: &str = "\u{e13d}"; diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/asset_library.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/asset_library.rs index 5cfc40c..3506beb 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/asset_library.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/asset_library.rs @@ -7,17 +7,7 @@ //! - Image Assets (static images) use eframe::egui; -use lightningbeam_core::clip::{AudioClip, ResolvedContent, VectorClip}; - -/// Library label for an audio clip: a take folder advertises how many takes it holds, anything else -/// just names its kind. The library lists *clips*, not placements, so there's no active take here — -/// a folder is previewed by its first take. -fn take_label(clip: &AudioClip, kind: &str) -> String { - match clip.takes() { - Some(takes) => format!("{} ({} takes)", kind, takes.len()), - None => kind.to_string(), - } -} +use lightningbeam_core::clip::{ResolvedContent, VectorClip}; use lightningbeam_core::document::Document; use lightningbeam_core::layer::AnyLayer; use std::collections::{HashMap, HashSet}; @@ -928,9 +918,9 @@ impl AssetLibraryPane { continue; } - let (extra_info, drag_clip_type) = match &clip.resolve(None) { - ResolvedContent::Audio { .. } => (take_label(clip, "Sampled"), DragClipType::AudioSampled), - ResolvedContent::Midi { .. } => (take_label(clip, "MIDI"), DragClipType::AudioMidi), + let (extra_info, drag_clip_type) = match &clip.resolve() { + ResolvedContent::Audio { .. } => ("Sampled".to_string(), DragClipType::AudioSampled), + ResolvedContent::Midi { .. } => ("MIDI".to_string(), DragClipType::AudioMidi), ResolvedContent::Recording => { // Skip recording-in-progress clips (and empty take folders) from asset library continue; @@ -1128,12 +1118,12 @@ impl AssetLibraryPane { for (id, clip) in &document.audio_clips { if !linked_audio_ids.contains(id) && clip.folder_id == current_folder { - let (extra_info, drag_clip_type) = match &clip.resolve(None) { + let (extra_info, drag_clip_type) = match &clip.resolve() { ResolvedContent::Audio { .. } => { - (take_label(clip, "Sampled"), DragClipType::AudioSampled) + ("Sampled".to_string(), DragClipType::AudioSampled) } ResolvedContent::Midi { .. } => { - (take_label(clip, "MIDI"), DragClipType::AudioMidi) + ("MIDI".to_string(), DragClipType::AudioMidi) } ResolvedContent::Recording => { // Skip recording-in-progress clips (and empty take folders) @@ -1775,7 +1765,7 @@ impl AssetLibraryPane { let prefetched_waveform: Option> = if asset_category == AssetCategory::Audio && !self.thumbnail_cache.has(&asset_id) { if let Some(clip) = document.audio_clips.get(&asset_id) { - if let Some(audio_pool_index) = clip.resolved_audio_pool_index(None).as_ref() { + if let Some(audio_pool_index) = clip.audio_pool_index().as_ref() { shared.raw_audio_cache.get(audio_pool_index) .map(|raw| peaks_from_raw_audio(raw, THUMBNAIL_SIZE as usize)) } else { @@ -1800,7 +1790,7 @@ impl AssetLibraryPane { AssetCategory::Audio => { if let Some(clip) = document.audio_clips.get(&asset_id) { let bg_color = egui::Color32::from_rgba_unmultiplied(40, 40, 40, 200); - match &clip.resolve(None) { + match &clip.resolve() { ResolvedContent::Audio { .. } => { let wave_color = egui::Color32::from_rgb(100, 200, 100); if let Some(ref peaks) = prefetched_waveform { @@ -2354,7 +2344,7 @@ impl AssetLibraryPane { AssetCategory::Audio => { if let Some(clip) = document.audio_clips.get(&asset_id) { let bg_color = egui::Color32::from_rgba_unmultiplied(40, 40, 40, 200); - match &clip.resolve(None) { + match &clip.resolve() { ResolvedContent::Audio { audio_pool_index } => { let wave_color = egui::Color32::from_rgb(100, 200, 100); let waveform: Option> = shared.raw_audio_cache.get(audio_pool_index) @@ -2491,7 +2481,7 @@ impl AssetLibraryPane { AssetCategory::Audio => { if let Some(clip) = document.audio_clips.get(&asset_id) { let bg_color = egui::Color32::from_rgba_unmultiplied(40, 40, 40, 200); - match &clip.resolve(None) { + match &clip.resolve() { ResolvedContent::Audio { audio_pool_index } => { let wave_color = egui::Color32::from_rgb(100, 200, 100); let waveform: Option> = shared.raw_audio_cache.get(audio_pool_index) @@ -2812,7 +2802,7 @@ impl AssetLibraryPane { let prefetched_waveform: Option> = if asset_category == AssetCategory::Audio && !self.thumbnail_cache.has(&asset_id) { if let Some(clip) = document.audio_clips.get(&asset_id) { - if let Some(audio_pool_index) = clip.resolved_audio_pool_index(None).as_ref() { + if let Some(audio_pool_index) = clip.audio_pool_index().as_ref() { let waveform: Option> = shared.raw_audio_cache.get(audio_pool_index) .map(|raw| peaks_from_raw_audio(raw, THUMBNAIL_SIZE as usize)); if waveform.is_some() { @@ -2852,7 +2842,7 @@ impl AssetLibraryPane { // Check if it's sampled or MIDI if let Some(clip) = document.audio_clips.get(&asset_id) { let bg_color = egui::Color32::from_rgba_unmultiplied(40, 40, 40, 200); - match &clip.resolve(None) { + match &clip.resolve() { ResolvedContent::Audio { .. } => { let wave_color = egui::Color32::from_rgb(100, 200, 100); if let Some(ref peaks) = prefetched_waveform { @@ -3197,7 +3187,7 @@ impl PaneRenderer for AssetLibraryPane { println!("🎨 [ASSET_LIB] Checking for thumbnails to invalidate (pools: {:?})", shared.audio_pools_with_new_waveforms); let mut invalidated_any = false; for (asset_id, clip) in &document_arc.audio_clips { - if let Some(audio_pool_index) = clip.resolved_audio_pool_index(None).as_ref() { + if let Some(audio_pool_index) = clip.audio_pool_index().as_ref() { if shared.audio_pools_with_new_waveforms.contains(audio_pool_index) { println!("❌ [ASSET_LIB] Invalidating thumbnail for asset {} (pool {})", asset_id, audio_pool_index); self.thumbnail_cache.invalidate(asset_id); diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/infopanel.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/infopanel.rs index 0df4edf..e894887 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/infopanel.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/infopanel.rs @@ -1807,16 +1807,10 @@ impl InfopanelPane { ui.label("Name:"); ui.label(&clip.name); }); - let take_count = clip.takes().map(|t| t.len()).unwrap_or(0); - let take_folder_label; let type_name = match &clip.clip_type { lightningbeam_core::clip::AudioClipType::Sampled { .. } => "Audio (Sampled)", lightningbeam_core::clip::AudioClipType::Midi { .. } => "Audio (MIDI)", lightningbeam_core::clip::AudioClipType::Recording => "Audio (Recording)", - lightningbeam_core::clip::AudioClipType::TakeFolder { .. } => { - take_folder_label = format!("Audio ({} takes)", take_count); - &take_folder_label - } }; ui.horizontal(|ui| { ui.label("Type:"); diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs index 5586901..7a81053 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/piano_roll.rs @@ -467,7 +467,7 @@ impl PianoRollPane { if let Some(clip) = document.audio_clips.get(&instance.clip_id) { // Resolve through the instance's active take, so a MIDI take folder edits // whichever take it's actually playing. - if let Some(midi_clip_id) = clip.resolved_midi_clip_id(instance.active_take) { + if let Some(midi_clip_id) = instance.resolved_midi_clip_id(clip) { let duration = instance.effective_duration(clip.content_duration(), document.tempo_map()); // A MIDI clip's content time IS beats, which is what the piano roll's // x-axis uses. diff --git a/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs b/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs index 798180c..15d08b2 100644 --- a/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs +++ b/lightningbeam-ui/lightningbeam-editor/src/panes/timeline.rs @@ -298,6 +298,9 @@ pub struct TimelinePane { take_badge_hits: Vec<(egui::Rect, uuid::Uuid, uuid::Uuid, usize, usize)>, /// The take-folder instance whose take menu is open, if any. open_take_menu: Option<(uuid::Uuid, uuid::Uuid)>, + /// The (instance, take index) being renamed inline in the take menu. + renaming_take: Option<(uuid::Uuid, usize)>, + take_rename_buffer: String, /// Seconds between the cycle region's start and where the current recording actually began. /// Zero unless the user punched in mid-region. Used to line the live waveform preview up with /// the region on each pass. @@ -741,6 +744,8 @@ impl TimelinePane { keyframe_diamond_hits: Vec::new(), take_badge_hits: Vec::new(), open_take_menu: None, + renaming_take: None, + take_rename_buffer: String::new(), cycle_record_lead_secs: 0.0, duration: 10.0, // Default 10 seconds is_scrubbing: false, @@ -1148,6 +1153,28 @@ impl TimelinePane { // The backend records in the beats domain; start_time is the seconds playhead. let start_beats = shared.action_executor.document().tempo_map().seconds_to_beats(Seconds(start_time)); + // Does this layer already hold takes over the cycle region? If so this recording is another + // take, however short it runs — without this, stopping before the loop came round would land + // it as a separate overlapping clip instead of joining the take list. The engine can't work + // this out for itself: whether takes exist is document state. + let force_takes: std::collections::HashMap = { + let doc = shared.action_executor.document(); + let region = match (doc.cycle_enabled, doc.cycle_region) { + (true, Some((ls, le))) if le > ls => Some((ls, le - ls)), + _ => None, + }; + candidates + .iter() + .map(|&(layer_id, _, _)| { + let forced = region.is_some_and(|(loop_start, loop_len)| { + doc.take_folder_at(&layer_id, loop_start, loop_len, &uuid::Uuid::nil()) + .is_some() + }); + (layer_id, forced) + }) + .collect() + }; + // Step 4: Dispatch recording for each candidate for &(layer_id, ref cat, _) in &candidates { match cat { @@ -1171,7 +1198,7 @@ impl TimelinePane { } if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); - controller.start_recording(track_id, start_beats); + controller.start_recording(track_id, start_beats, force_takes.get(&layer_id).copied().unwrap_or(false)); println!("🎤 Started audio recording on track {:?} at {:.2}s", track_id, start_time); } shared.recording_layer_ids.push(layer_id); @@ -1184,7 +1211,7 @@ impl TimelinePane { if let Some(controller_arc) = shared.audio_controller { let mut controller = controller_arc.lock().unwrap(); let clip_id = controller.create_midi_clip(track_id, start_beats, Beats::ZERO); - controller.start_midi_recording(track_id, clip_id, start_beats); + controller.start_midi_recording(track_id, clip_id, start_beats, force_takes.get(&layer_id).copied().unwrap_or(false)); shared.recording_clips.insert(layer_id, clip_id); println!("🎹 Started MIDI recording on track {:?} at {:.2}s, clip_id={}", track_id, start_time, clip_id); @@ -1695,9 +1722,7 @@ impl TimelinePane { return; }; - // The instance's *stored* selection, which is what rollback must restore — not `active`, - // which is that value clamped for display. - let old_take = document + let Some(instance) = document .get_layer(&layer_id) .and_then(|l| match l { lightningbeam_core::layer::AnyLayer::Audio(al) => { @@ -1705,7 +1730,14 @@ impl TimelinePane { } _ => None, }) - .and_then(|ci| ci.active_take); + else { + self.open_take_menu = None; + return; + }; + // The instance's *stored* selection, which is what rollback must restore — not `active`, + // which is that value clamped for display. + let old_take = instance.active_take; + let take_names: Vec = instance.takes.iter().map(|t| t.name.clone()).collect(); let mut close = false; let area = egui::Area::new(ui.id().with(("take_menu", instance_id))) @@ -1715,22 +1747,53 @@ impl TimelinePane { egui::Frame::popup(ui.style()).show(ui, |ui| { for i in 0..count { let is_active = i == active; - if ui - .selectable_label(is_active, format!("Take {}", i + 1)) - .clicked() - { - if !is_active { - pending_actions.push(Box::new( - lightningbeam_core::actions::SetActiveTakeAction::new( - layer_id, - instance_id, - i, - old_take, - ), - )); + let renaming = self.renaming_take == Some((instance_id, i)); + + ui.horizontal(|ui| { + if renaming { + let edit = ui.add( + egui::TextEdit::singleline(&mut self.take_rename_buffer) + .desired_width(110.0), + ); + edit.request_focus(); + // Commit on Enter or on clicking away; Escape abandons. + let enter = ui.input(|i| i.key_pressed(egui::Key::Enter)); + let escape = ui.input(|i| i.key_pressed(egui::Key::Escape)); + if enter || edit.lost_focus() && !escape { + let name = self.take_rename_buffer.trim().to_string(); + if !name.is_empty() && name != take_names[i] { + pending_actions.push(Box::new( + lightningbeam_core::actions::RenameTakeAction::new( + layer_id, instance_id, i, name, + ), + )); + } + self.renaming_take = None; + } else if escape { + self.renaming_take = None; + } + return; } - close = true; - } + + let label = ui.selectable_label(is_active, &take_names[i]); + if label.clicked() { + if !is_active { + pending_actions.push(Box::new( + lightningbeam_core::actions::SetActiveTakeAction::new( + layer_id, instance_id, i, old_take, + ), + )); + } + close = true; + } + // Double-click a take to rename it in place. Deleting lives on the clip's + // right-click menu, not here — a trash icon per row is a small target + // sitting right next to the one you actually meant to click. + if label.double_clicked() { + self.renaming_take = Some((instance_id, i)); + self.take_rename_buffer = take_names[i].clone(); + } + }); } }); }); @@ -1743,6 +1806,7 @@ impl TimelinePane { } if close { self.open_take_menu = None; + self.renaming_take = None; } } @@ -3971,7 +4035,7 @@ impl TimelinePane { if let Some(clip) = document.get_audio_clip(&clip_instance.clip_id) { // Resolve through the instance's active take, so a take folder draws // whichever take it actually plays. - match &clip.resolve(clip_instance.active_take) { + match &clip_instance.resolve(clip) { // MIDI: Draw piano roll (with loop iterations) lightningbeam_core::clip::ResolvedContent::Midi { midi_clip_id } => { if let Some(events) = midi_event_cache.get(midi_clip_id) { @@ -4357,12 +4421,10 @@ impl TimelinePane { // only. Records a hit rect so the click that opens the take menu can be // dispatched after rendering (the usual two-phase pattern), rather than // mutating the document mid-paint. - if let Some(take_count) = document - .get_audio_clip(&clip_instance.clip_id) - .and_then(|c| c.takes().map(|t| t.len())) - .filter(|n| *n > 0) - { - let active = clip_instance.active_take.unwrap_or(0).min(take_count - 1); + // Only worth showing when there's actually a choice to make. + if clip_instance.takes.len() > 1 { + let take_count = clip_instance.takes.len(); + let active = clip_instance.active_take_index(); let label = format!("Take {}/{}", active + 1, take_count); let text_color = theme.text_color( &["#timeline", ".take-badge"], @@ -6596,6 +6658,22 @@ impl PaneRenderer for TimelinePane { enabled }; + // Take management for the clip that was right-clicked. Only offered when there's more + // than one take — with a single take there's nothing to choose between, and "delete the + // only take" would leave the instance with nothing to play. + let take_target: Option<(uuid::Uuid, uuid::Uuid, usize, String)> = ctx_clip_id.and_then(|instance_id| { + let context_layers = document.context_layers(shared.editing_clip_id.as_ref()); + for (layer, instances) in all_layer_clip_instances(&context_layers) { + if let Some(ci) = instances.iter().find(|ci| ci.id == instance_id) { + if ci.takes.len() > 1 { + let active = ci.active_take_index(); + return Some((layer.id(), instance_id, active, ci.takes[active].name.clone())); + } + } + } + None + }); + let area_id = ui.id().with("clip_context_menu"); let mut item_clicked = false; let area_response = egui::Area::new(area_id) @@ -6661,6 +6739,34 @@ impl PaneRenderer for TimelinePane { shared.pending_menu_actions.push(crate::menu::MenuAction::Delete); item_clicked = true; } + + // Take management, on the clip that was right-clicked. Takes live on the + // INSTANCE, so on a comped split this prunes the half you clicked and leaves + // the other half's alternatives alone. + if let Some((take_layer_id, take_instance_id, active_index, active_name)) = &take_target { + ui.separator(); + // Deletes the take that's PLAYING — the one the badge is showing — so + // it's named rather than just "Delete Take". + if menu_item(ui, &format!("Delete \"{}\"", active_name), true) { + shared.pending_actions.push(Box::new( + lightningbeam_core::actions::DeleteTakeAction::new( + *take_layer_id, + *take_instance_id, + *active_index, + ), + )); + item_clicked = true; + } + if menu_item(ui, "Delete Unused Takes", true) { + shared.pending_actions.push(Box::new( + lightningbeam_core::actions::DeleteUnusedTakesAction::new( + *take_layer_id, + *take_instance_id, + ), + )); + item_clicked = true; + } + } }); });