Make recording an undoable action; fixes MIDI recording not marking doc modified
Recording (audio and MIDI) mutated the document directly in the AudioEvent handlers, outside the action system — so it was never undoable, and dirty- tracking leaned on an ad-hoc `media_modified` flag that the MIDI stop handler forgot to set (hence: recording a MIDI clip didn't trigger the save-on-close prompt). Recording is temporal (the clip streams into the document live over the take), so it can't be applied by one synchronous execute(). Instead, commit the finished take as an *already-applied* action: - ActionExecutor::push_applied(action) — registers an action whose effect is already present (clears redo, bumps the epoch so the doc reads as modified, pushes to the undo stack) WITHOUT re-running execute()/execute_backend(). Undo then removes the content via rollback/rollback_backend; redo re-adds it. - AddClipInstanceAction::already_applied(...) — constructs the action pre-seeded into its post-execute state (executed + the existing backend clip id) so the first undo can remove the live-recorded clip from both doc and backend, and redo re-adds it through the normal path. - Both recording stop handlers now finalize, then push_applied this action. Keeping the clip in the document (not a transient) matters for streaming-to- disk and keeps the doc the single source of truth. Recordings now bump the epoch like every other edit, so the media_modified flag is dropped for recordings (kept only as a defensive fallback if the action can't be built). Whole workspace compiles; 299 core tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Skyler Lehmkuhl
parent
8ed2320dca
commit
ba4395602d
|
|
@ -236,6 +236,24 @@ impl ActionExecutor {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Register an action whose effect has **already been applied** to the document (and backend)
|
||||
/// outside the executor — e.g. a recording, which streams its content into the document live
|
||||
/// over time and can't be applied by a single synchronous `execute()`.
|
||||
///
|
||||
/// Unlike `execute`, this does NOT run `execute()`/`execute_backend()` (the effect is already
|
||||
/// present). It clears the redo stack, bumps the epoch (so the document reads as modified), and
|
||||
/// pushes the action so it becomes undoable: undo runs `rollback`/`rollback_backend` to remove
|
||||
/// the content, redo runs `execute`/`execute_backend` to bring it back. The action must be
|
||||
/// constructed already in its post-execute state (see e.g. `AddClipInstanceAction::already_applied`).
|
||||
pub fn push_applied(&mut self, action: Box<dyn Action>) {
|
||||
self.redo_stack.clear();
|
||||
self.epoch = self.epoch.wrapping_add(1);
|
||||
self.undo_stack.push(action);
|
||||
if self.undo_stack.len() > self.max_undo_depth {
|
||||
self.undo_stack.remove(0);
|
||||
}
|
||||
}
|
||||
|
||||
/// Undo the last action
|
||||
///
|
||||
/// Returns Ok(true) if an action was undone, Ok(false) if undo stack is empty,
|
||||
|
|
|
|||
|
|
@ -47,6 +47,31 @@ impl AddClipInstanceAction {
|
|||
}
|
||||
}
|
||||
|
||||
/// Construct the action in its **already-applied** state: the clip instance is already in the
|
||||
/// document and its backend clip already exists (e.g. a finished recording). Pair with
|
||||
/// `ActionExecutor::push_applied` so the recording becomes undoable without re-adding anything.
|
||||
/// Undo will `rollback`/`rollback_backend` (removing the clip from doc + backend via the seeded
|
||||
/// ids); redo re-adds it via the normal `execute`/`execute_backend` path.
|
||||
pub fn already_applied(
|
||||
layer_id: Uuid,
|
||||
clip_instance: ClipInstance,
|
||||
backend_track_id: daw_backend::TrackId,
|
||||
backend_id: crate::action::BackendClipInstanceId,
|
||||
) -> Self {
|
||||
let (backend_midi_instance_id, backend_audio_instance_id) = match backend_id {
|
||||
crate::action::BackendClipInstanceId::Midi(id) => (Some(id), None),
|
||||
crate::action::BackendClipInstanceId::Audio(id) => (None, Some(id)),
|
||||
};
|
||||
Self {
|
||||
layer_id,
|
||||
clip_instance,
|
||||
executed: true, // already present in the document
|
||||
backend_track_id: Some(backend_track_id),
|
||||
backend_midi_instance_id,
|
||||
backend_audio_instance_id,
|
||||
}
|
||||
}
|
||||
|
||||
/// Get the ID of the clip instance that will be/was added
|
||||
pub fn clip_instance_id(&self) -> Uuid {
|
||||
self.clip_instance.id
|
||||
|
|
|
|||
|
|
@ -6547,10 +6547,8 @@ impl eframe::App for EditorApp {
|
|||
};
|
||||
|
||||
if !clip_id.is_nil() {
|
||||
// Finalize the clip (update pool_index and duration)
|
||||
// A finished recording (samples in the pool) needs capturing.
|
||||
// Finalize the clip (update pool_index and duration).
|
||||
self.autosave.pending_event = true;
|
||||
self.media_modified = true;
|
||||
if let Some(clip) = self.action_executor.document_mut().audio_clips.get_mut(&clip_id) {
|
||||
if clip.finalize_recording(pool_index, duration) {
|
||||
clip.name = format!("Recording {}", pool_index);
|
||||
|
|
@ -6563,11 +6561,31 @@ impl eframe::App for EditorApp {
|
|||
// Map the document instance_id → the existing backend clip so that
|
||||
// delete/move/trim actions can reference it correctly.
|
||||
// DO NOT call AddAudioClipSync — that would create a duplicate clip.
|
||||
self.clip_instance_to_backend_map.insert(
|
||||
instance_id,
|
||||
lightningbeam_core::action::BackendClipInstanceId::Audio(_backend_clip_id),
|
||||
);
|
||||
let backend_id = lightningbeam_core::action::BackendClipInstanceId::Audio(_backend_clip_id);
|
||||
self.clip_instance_to_backend_map.insert(instance_id, backend_id);
|
||||
eprintln!("[AUDIO] Mapped doc instance {} → backend clip {}", instance_id, _backend_clip_id);
|
||||
|
||||
// Register the finished recording as an already-applied action so
|
||||
// it bumps the epoch (marks the document modified for save-on-close
|
||||
// and autosave) and can be undone/redone like any other edit.
|
||||
let clip_instance = self.layer_to_track_map.get(&layer_id).copied().and_then(|track_id| {
|
||||
self.action_executor.document()
|
||||
.get_layer(&layer_id)
|
||||
.and_then(|l| if let AnyLayer::Audio(al) = l {
|
||||
al.clip_instances.iter().find(|ci| ci.id == instance_id).cloned()
|
||||
} else { None })
|
||||
.map(|ci| (track_id, ci))
|
||||
});
|
||||
if let Some((track_id, clip_instance)) = clip_instance {
|
||||
let action = lightningbeam_core::actions::AddClipInstanceAction::already_applied(
|
||||
layer_id, clip_instance, track_id, backend_id,
|
||||
);
|
||||
self.action_executor.push_applied(Box::new(action));
|
||||
} else {
|
||||
// Couldn't build the action; still mark modified so the recording
|
||||
// isn't silently lost on close.
|
||||
self.media_modified = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -6702,9 +6720,33 @@ impl eframe::App for EditorApp {
|
|||
}
|
||||
}
|
||||
|
||||
// TODO: Store clip_instance_to_backend_map entry for this MIDI clip.
|
||||
// The backend created the instance in create_midi_clip(), but doesn't
|
||||
// report the instance_id back. Needed for move/trim operations later.
|
||||
// Register the finished MIDI recording as an already-applied action so it
|
||||
// marks the document modified (save-on-close / autosave) and is undoable,
|
||||
// like the audio path. The backend instance id was mapped during
|
||||
// MidiRecordingProgress.
|
||||
if let Some(&layer_id) = self.track_to_layer_map.get(&track_id) {
|
||||
let doc_clip_id = self.action_executor.document()
|
||||
.audio_clip_by_midi_clip_id(clip_id).map(|(id, _)| id);
|
||||
if let Some(doc_clip_id) = doc_clip_id {
|
||||
let instance = self.action_executor.document()
|
||||
.get_layer(&layer_id)
|
||||
.and_then(|l| if let AnyLayer::Audio(al) = l {
|
||||
al.clip_instances.iter().find(|ci| ci.clip_id == doc_clip_id).cloned()
|
||||
} else { None });
|
||||
if let Some(instance) = instance {
|
||||
if let Some(&backend_id) = self.clip_instance_to_backend_map.get(&instance.id) {
|
||||
let action = lightningbeam_core::actions::AddClipInstanceAction::already_applied(
|
||||
layer_id, instance, track_id, backend_id,
|
||||
);
|
||||
self.action_executor.push_applied(Box::new(action));
|
||||
} else {
|
||||
// No backend mapping (e.g. snapshot lookup missed); still mark
|
||||
// modified so the recording isn't silently lost on close.
|
||||
self.media_modified = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Remove this MIDI layer from active recordings
|
||||
if let Some(&layer_id) = self.track_to_layer_map.get(&track_id) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue