Merge pull request #434 from David-T-Campos/fix/share-sequences-across-edit-snapshots

project: share sequences across edit snapshots

crates/engine/src/clip_ops.rs

@@ -949,6 +949,7 @@ fn consolidate_duplicates(s: &mut Session, _: &Value) -> Result<Value> {
         for it in pr.items.values_mut() {
             match &mut it.kind {
                 ItemKind::Sequence(q) => {
+                    let q = std::sync::Arc::make_mut(q);
                     for t in q.all_tracks_mut() {
                         for i in &mut t.items {
                             if let Some(k) = map.get(&i.item) {

crates/engine/src/sequence_extras.rs

@@ -428,7 +428,7 @@ fn simplify(s: &mut Session, p: &Value) -> Result<Value> {
         let report = simplify_sequence(pr, &mut q, &o);
         q.check().map_err(EngineError::Other)?;
         let bin = pr.root.parent_of(seq_id).filter(|b| b.0 != 0);
-        let id = pr.add_item(&name, src.label, ItemKind::Sequence(Box::new(q)), bin);
+        let id = pr.add_item(&name, src.label, ItemKind::Sequence(std::sync::Arc::new(q)), bin);
         st.active_sequence = Some(id);
         if !st.open_sequences.contains(&id) {
             st.open_sequences.push(id);

crates/engine/src/source_monitor.rs

@@ -94,7 +94,7 @@ fn marker_list(p: &mut Project, item: ItemId) -> Result<&mut Vec<Marker>> {
     let it = p.item_mut(item).ok_or_else(|| EngineError::Other("Source item is unavailable".into()))?;
     match &mut it.kind {
         filmcraft_project::ItemKind::Media(m) => Ok(&mut m.markers),
-        filmcraft_project::ItemKind::Sequence(q) => Ok(&mut q.markers),
+        filmcraft_project::ItemKind::Sequence(q) => Ok(&mut std::sync::Arc::make_mut(q).markers),
         _ => Err(EngineError::Other("Source marker editing requires media or a sequence".into())),
     }
 }

crates/format/src/lib.rs

@@ -224,6 +224,7 @@ fn validate_loaded(mut loaded: Loaded) -> Result<Loaded, FormatError> {
                 m.info.audio_streams.truncate(max);
             }
             filmcraft_project::ItemKind::Sequence(sequence) => {
+                let sequence = std::sync::Arc::make_mut(sequence);
                 for clip in sequence.audio_tracks.iter_mut().flat_map(|t| t.items.iter_mut()) {
                     if clip.audio_stream >= max {
                         log::warn!("sequence `{name}`: clip `{}` uses audio stream {}, reset to 0", clip.name, clip.audio_stream);

crates/format/tests/fixtures.rs

@@ -131,7 +131,7 @@ pub fn big_project(n: usize) -> Project {
     let seq_id = ItemId(21);
     let mut next = 1_000_000u64;
     let seq = match &mut p.items.get_mut(&seq_id).unwrap().kind {
-        ItemKind::Sequence(s) => s,
+        ItemKind::Sequence(s) => std::sync::Arc::make_mut(s),
         _ => unreachable!(),
     };
     let v0 = seq.video_tracks[0].items[0].clone();

crates/interchange/src/common.rs

@@ -387,7 +387,7 @@ impl Builder {
     /// Add an empty sequence item (filled later with [`Builder::put_sequence`]).
     pub fn reserve_sequence(&mut self, name: &str, settings: SequenceSettings, bin: Option<BinId>) -> ItemId {
         let seq = empty_sequence(settings);
-        self.p.add_item(name, Label::Forest, ItemKind::Sequence(Box::new(seq)), bin)
+        self.p.add_item(name, Label::Forest, ItemKind::Sequence(std::sync::Arc::new(seq)), bin)
     }
 
     pub fn put_sequence(&mut self, id: ItemId, mut seq: Sequence) {

crates/interchange/src/lib.rs

@@ -438,6 +438,7 @@ pub fn merge_into(target: &mut Project, fragment: Imported, bin: Option<BinId>)
         it.created = it.id.0;
         match &mut it.kind {
             ItemKind::Sequence(seq) => {
+                let seq = std::sync::Arc::make_mut(seq);
                 for m in &mut seq.markers {
                     m.id = MarkerId(target.alloc_id());
                 }
@@ -510,6 +511,7 @@ pub fn rebase_source_timecode(project: &mut Project, item: ItemId, start_tc_fram
     }
     for it in project.items.values_mut() {
         if let ItemKind::Sequence(seq) = &mut it.kind {
+            let seq = std::sync::Arc::make_mut(seq);
             for t in seq.all_tracks_mut() {
                 for ti in t.items.iter_mut().filter(|ti| ti.item == item) {
                     ti.source_in -= off;

crates/project/src/lib.rs

@@ -262,7 +262,9 @@ impl MediaClip {
 #[allow(clippy::large_enum_variant)]
 pub enum ItemKind {
     Media(MediaClip),
-    Sequence(Box<Sequence>),
+    /// Shared so project snapshots used by undo and background readers do not deep-copy every
+    /// timeline. Mutators detach with `Arc::make_mut`.
+    Sequence(std::sync::Arc<Sequence>),
     /// A subclip: a media item restricted to a range.
     Subclip {
         parent: ItemId,
@@ -327,7 +329,7 @@ impl ProjectItem {
     }
     pub fn as_sequence_mut(&mut self) -> Option<&mut Sequence> {
         match &mut self.kind {
-            ItemKind::Sequence(s) => Some(s),
+            ItemKind::Sequence(s) => Some(std::sync::Arc::make_mut(s)),
             _ => None,
         }
     }
@@ -1502,7 +1504,7 @@ impl Project {
             let id = TrackId(self.alloc_id());
             seq.audio_tracks.push(Track::new(id, TrackKind::Audio, format!("Audio {}", i + 1)));
         }
-        self.add_item(name, Label::Forest, ItemKind::Sequence(Box::new(seq)), bin)
+        self.add_item(name, Label::Forest, ItemKind::Sequence(std::sync::Arc::new(seq)), bin)
     }
 
     /// Re-base an item's media time: whatever was at media time `t` is at `t - delta` afterwards
@@ -1524,6 +1526,7 @@ impl Project {
         }
         for it in self.items.values_mut() {
             if let ItemKind::Sequence(seq) = &mut it.kind {
+                let seq = std::sync::Arc::make_mut(seq);
                 for t in seq.all_tracks_mut() {
                     for ti in t.items.iter_mut().filter(|ti| users.contains(&ti.item)) {
                         ti.source_in -= delta;
@@ -1984,6 +1987,30 @@ mod tests {
         assert_eq!(p.root.parent_of(i), None);
     }
 
+    #[test]
+    fn project_clone_shares_sequences_until_mutated() {
+        fn sequence_arc(p: &Project, id: ItemId) -> &std::sync::Arc<Sequence> {
+            match &p.item(id).unwrap().kind {
+                ItemKind::Sequence(q) => q,
+                _ => unreachable!(),
+            }
+        }
+
+        let (mut p, _, a) = demo_project();
+        let b = p.new_sequence("Sequence 02", SequenceSettings::default(), 1, 1, None);
+        let before = p.clone();
+
+        assert!(std::sync::Arc::ptr_eq(sequence_arc(&p, a), sequence_arc(&before, a)));
+        assert!(std::sync::Arc::ptr_eq(sequence_arc(&p, b), sequence_arc(&before, b)));
+
+        p.sequence_mut(a).unwrap().mark_in = Some(Tick(1));
+
+        assert!(!std::sync::Arc::ptr_eq(sequence_arc(&p, a), sequence_arc(&before, a)));
+        assert!(std::sync::Arc::ptr_eq(sequence_arc(&p, b), sequence_arc(&before, b)));
+        assert_eq!(before.sequence(a).unwrap().mark_in, None);
+        assert_eq!(Project::from_json(&p.to_json()).unwrap(), p);
+    }
+
     #[test]
     fn nest_cycle_finds_sequences_that_contain_themselves() {
         let (mut p, clip, a) = demo_project();