Merge pull request #666 from marcoarnulfo/feat/remove-sync-point

Clip > Remove Sync Point takes the sync point off the selected clips

crates/engine/src/cmd/clip.rs

@@ -102,6 +102,7 @@ pub fn specs() -> Vec<CommandSpec> {
             }
             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<Value> {
     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<Value> {
+    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<ClipId> = 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<Value> {
     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]);
+    }
 }

crates/ui-egui/src/menus.rs

@@ -202,6 +202,11 @@ pub fn tree() -> Vec<MenuNode> {
         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");

crates/ui-egui/tests/command_reachability.rs

@@ -102,3 +102,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);
+}