From f2ae345f2022e2da42935f5a7c035f53f4107bd2 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 07:43:12 +0200 Subject: [PATCH] Clip > Remove Sync Point takes the sync point off the selected clips --- crates/engine/src/cmd/clip.rs | 94 ++++++++++++++++++++ crates/ui-egui/src/menus.rs | 5 ++ crates/ui-egui/tests/command_reachability.rs | 18 ++++ 3 files changed, 117 insertions(+) diff --git a/crates/engine/src/cmd/clip.rs b/crates/engine/src/cmd/clip.rs index 9d68dc25..aff6d8cc 100644 --- a/crates/engine/src/cmd/clip.rs +++ b/crates/engine/src/cmd/clip.rs @@ -102,6 +102,7 @@ pub fn specs() -> Vec { } Ok(json!({})) }), + cmd!("clip.remove_sync_point", "Remove Sync Point", ["Clip"], None, "{clips?}", has_sync_point, remove_sync_point), cmd!("clip.quantize_to_grid", "Quantize to Grid", ["Clip"], Some("Cmd+0"), "{clips?}", has_selection, quantize_to_grid), cmd!("clip.color", "Clip Color", [], None, "{clips?, color: [r,g,b] | null}", has_selection, |e, p| { let ids = clip_ids_param(e, p); @@ -348,6 +349,31 @@ fn nudge_gain(e: &mut Engine, p: &Value, db: f64) -> Result { gain(e, &json!({"clips": clips, "delta_db": db})) } +/// Sync point 0 is "none" (edit_window only draws one when > 0). +fn has_sync_point(e: &Engine) -> std::result::Result<(), String> { + let s = e.session(); + if clip_ids_param(e, &json!({})).iter().any(|id| s.find_clip(*id).is_some_and(|(_, c)| c.sync_point != 0)) { + Ok(()) + } else { + Err("no selected clip has a sync point".into()) + } +} + +fn remove_sync_point(e: &mut Engine, p: &Value) -> Result { + let s = e.session(); + // Only clips that have a point are touched, so a call that changes nothing leaves no undo step. + let ids: Vec = clip_ids_param(e, p).into_iter().filter(|id| s.find_clip(*id).is_some_and(|(_, c)| c.sync_point != 0)).collect(); + if !ids.is_empty() { + let s = e.session_mut(); + for id in &ids { + if let Some(c) = s.find_clip_mut(*id) { + c.sync_point = 0 + } + } + } + Ok(json!({"clips": ids.len()})) +} + fn quantize_to_grid(e: &mut Engine, p: &Value) -> Result { let ids = clip_ids_param(e, p); let s = e.session(); @@ -452,4 +478,72 @@ mod tests { e.execute("clip.gain_nudge_up", &json!({})).unwrap(); assert_eq!(gain(&e), 36.0); } + + /// Three 10 000-sample clips side by side on one track; sync points are set by each test. + fn three_clips() -> (crate::Engine, [ClipId; 3]) { + use soundcraft_model::{ChannelFormat, Clip, Session, TrackKind}; + let mut s = Session::default(); + let t = s.add_track(TrackKind::Audio, ChannelFormat::Mono, Some("Dialog")); + let buf = soundcraft_audio_io::AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.1; 10_000]] }; + let src = crate::io::add_source(&mut s, "Line", buf, None, soundcraft_audio_io::FileFormat::Wav); + let ids = [0, 1, 2].map(|k| { + let cid = s.new_clip_id(); + crate::edit::place_clip(&mut s, t, Clip::audio(cid, "Line", src, 0, k * 12_000, 10_000)); + cid + }); + (crate::Engine::new(s), ids) + } + + fn sync_of(e: &crate::Engine, id: ClipId) -> i64 { + e.session().find_clip(id).unwrap().1.sync_point + } + + #[test] + fn remove_sync_point_clears_the_point_identify_set() { + let (mut e, [a, ..]) = three_clips(); + e.execute("edit.select", &json!({"clips": [a.0]})).unwrap(); + e.execute("clip.identify_sync_point", &json!({"at": 4_000})).unwrap(); + assert_eq!(sync_of(&e, a), 4_000); + e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(sync_of(&e, a), 0); + } + + #[test] + fn remove_sync_point_clears_every_selected_clip_and_only_those() { + let (mut e, [a, b, c]) = three_clips(); + for (id, at) in [(a, 1_000), (b, 2_000), (c, 3_000)] { + e.session_mut().find_clip_mut(id).unwrap().sync_point = at; + } + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + let r = e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(r["clips"], 2); + assert_eq!([sync_of(&e, a), sync_of(&e, b), sync_of(&e, c)], [0, 0, 3_000]); + } + + #[test] + fn remove_sync_point_is_disabled_unless_a_selected_clip_has_one() { + let (mut e, [a, b, c]) = three_clips(); + e.session_mut().find_clip_mut(c).unwrap().sync_point = 3_000; + let enabled = |e: &crate::Engine| crate::find_command("clip.remove_sync_point").unwrap().info(e).enabled; + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + assert!(!enabled(&e), "neither selected clip has a sync point (the unselected one does)"); + e.execute("edit.select", &json!({"clips": [b.0, c.0]})).unwrap(); + assert!(enabled(&e)); + } + + #[test] + fn remove_sync_point_is_one_undo_step_that_puts_the_points_back() { + let (mut e, [a, b, _]) = three_clips(); + for (id, at) in [(a, 1_000), (b, 2_000)] { + e.session_mut().find_clip_mut(id).unwrap().sync_point = at; + } + e.execute("edit.select", &json!({"clips": [a.0, b.0]})).unwrap(); + let steps = e.undo_history().len(); + e.execute("clip.remove_sync_point", &json!({})).unwrap(); + assert_eq!(e.undo_history().len(), steps + 1); + assert_eq!(e.undo_label(), Some("Remove Sync Point")); + assert_eq!([sync_of(&e, a), sync_of(&e, b)], [0, 0]); + assert!(e.undo()); + assert_eq!([sync_of(&e, a), sync_of(&e, b)], [1_000, 2_000]); + } } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9eef0b22..fbe274f1 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -202,6 +202,11 @@ pub fn tree() -> Vec { insert_path(&mut roots, line); } insert_path(&mut roots, "Window > Reset Panel Layout"); + // Not in the observed catalog either; Pro Tools lists it right under Identify Sync Point. + if let Some(clip) = roots.iter_mut().find(|n| n.label == "Clip") { + let at = clip.children.iter().position(|n| n.label == "Identify Sync Point").map_or(clip.children.len(), |i| i + 1); + clip.children.insert(at, MenuNode { label: "Remove Sync Point".into(), path: "Clip > Remove Sync Point".into(), children: Vec::new() }); + } // Not in the incumbent catalog, but the only UI path to an empty MIDI clip (#282). // It creates a clip rather than acting on the selected ones, so it leads the Clip menu. insert_path(&mut roots, "Clip > New MIDI Clip"); diff --git a/crates/ui-egui/tests/command_reachability.rs b/crates/ui-egui/tests/command_reachability.rs index 5328947b..ce7c6346 100644 --- a/crates/ui-egui/tests/command_reachability.rs +++ b/crates/ui-egui/tests/command_reachability.rs @@ -103,3 +103,21 @@ fn new_midi_clip_is_in_the_clip_menu() { let clip = tree.iter().find(|n| n.label == "Clip").unwrap(); assert_eq!(clip.children[0].label, "New MIDI Clip", "creates rather than edits, so it comes first"); } + +#[test] +fn remove_sync_point_follows_identify_and_is_greyed_out_without_a_point() { + let tree = menus::tree(); + let clip = tree.iter().find(|n| n.label == "Clip").unwrap(); + let labels: Vec<&str> = clip.children.iter().map(|n| n.label.as_str()).collect(); + let at = labels.iter().position(|l| *l == "Identify Sync Point").unwrap(); + assert_eq!(labels.get(at + 1), Some(&"Remove Sync Point")); + let path = &clip.children[at + 1].path; + let id = menus::command_for_path(path, &menus::ui_aliases()); + assert_eq!(id.as_deref(), Some("clip.remove_sync_point")); + + let mut app = soundcraft_ui_egui::SoundApp::new(soundcraft_engine::demo::demo_engine(), None, soundcraft_ui_egui::Services::default()); + app.engine.execute("edit.select", &serde_json::json!({"tracks": ["Kick"], "start": {"seconds": 12.0}})).unwrap(); + assert!(!menus::item_state(&app, path, id.as_deref()).0, "the selected Kick clip has no sync point yet"); + app.engine.execute("clip.identify_sync_point", &serde_json::json!({})).unwrap(); + assert!(menus::item_state(&app, path, id.as_deref()).0); +}