diff --git a/crates/effects/src/ocio.rs b/crates/effects/src/ocio.rs index d00f6e9e..c8fcf848 100644 --- a/crates/effects/src/ocio.rs +++ b/crates/effects/src/ocio.rs @@ -372,7 +372,7 @@ pub fn parse_spimtx(text: &str) -> Option { } let mut m = [0.0; 16]; let mut offset = [0.0; 4]; - for (r, row) in v.chunks_exact(4).enumerate() { + for (r, row) in v.as_chunks::<4>().0.iter().enumerate() { m[r * 4..r * 4 + 3].copy_from_slice(&row[..3]); offset[r] = row[3] / 65535.0; } diff --git a/crates/engine/src/history.rs b/crates/engine/src/history.rs index 36630969..6d905a83 100644 --- a/crates/engine/src/history.rs +++ b/crates/engine/src/history.rs @@ -206,19 +206,26 @@ impl History { self.branches.push(Branch { parent: from.clone(), steps }); } - /// Apply the undo-levels limit: drop the oldest working-line states, the branches that grew - /// from them, and the oldest branches when branches hold more states than `levels`. + /// Apply the undo-levels limit: keep the nearest past and future states on the working + /// line, drop branches whose parents were removed, and bound branch states separately. pub fn trim(&mut self, levels: usize, current: &Arc) { let levels = levels.max(1); if self.undo.len() > levels { let extra = self.undo.len() - levels; self.undo.drain(..extra); } - let mut total: usize = self.branches.iter().map(|b| b.steps.len()).sum(); - while total > levels && !self.branches.is_empty() { - total -= self.branches.remove(0).steps.len(); + if self.redo.len() > levels { + // The next redo is last: discard the farthest future states first. + let extra = self.redo.len() - levels; + self.redo.drain(..extra); } + // Orphans from truncated line states must not consume the reachable branch budget. self.prune(current); + while self.branch_states() > levels && !self.branches.is_empty() { + self.branches.remove(0); + // Removing a parent branch also removes its descendants before counting again. + self.prune(current); + } } /// Drop branches whose parent state is no longer in the history (`current` = the current diff --git a/crates/engine/src/tests_history.rs b/crates/engine/src/tests_history.rs index c67e7c14..d53f6ef3 100644 --- a/crates/engine/src/tests_history.rs +++ b/crates/engine/src/tests_history.rs @@ -1,9 +1,13 @@ //! Branching history: undo then a new step keeps the undone line as a branch; the tree lists //! every state; jumping to any state (on any branch) restores it and keeps everything else. +use std::sync::Arc; + +use effectcraft_project::Project; use serde_json::{Value, json}; use crate::Session; +use crate::history::{Branch, History}; fn names(s: &Session) -> Vec { s.project.comps().next().map(|(_, c)| c.layers.iter().map(|l| l.name.clone()).collect()).unwrap_or_default() @@ -190,3 +194,46 @@ fn a_session_restored_with_unsaved_changes_stays_modified() { s.execute("edit.undo", json!({})).unwrap(); assert!(s.is_dirty(), "undo doesn't reach a saved state"); } + +#[test] +fn dropped_redo_parent_does_not_discard_reachable_branches() { + let state = || Arc::new(Project::default()); + let current = state(); + let futures: Vec<_> = (0..5).map(|_| state()).collect(); + let reachable = state(); + let mut history = History { + redo: futures.iter().rev().map(|p| ("future".into(), p.clone())).collect(), + branches: vec![ + Branch { parent: current.clone(), steps: vec![("reachable".into(), reachable.clone())] }, + Branch { parent: futures[4].clone(), steps: vec![("orphan1".into(), state()), ("orphan2".into(), state())] }, + ], + ..Default::default() + }; + history.trim(2, ¤t); + assert_eq!(history.redo.len(), 2); + assert_eq!(history.branches.len(), 1); + assert!(Arc::ptr_eq(&history.branches[0].steps[0].1, &reachable)); + assert_eq!(history.branch_states(), 1); + assert!(history.tree(¤t).iter().any(|node| node.label == "reachable")); +} + +#[test] +fn evicted_branch_parent_does_not_discard_other_reachable_branches() { + let state = || Arc::new(Project::default()); + let current = state(); + let parent = state(); + let reachable = state(); + let mut history = History { + branches: vec![ + Branch { parent: current.clone(), steps: vec![("parent".into(), parent.clone())] }, + Branch { parent: current.clone(), steps: vec![("reachable".into(), reachable.clone())] }, + Branch { parent, steps: vec![("orphan1".into(), state()), ("orphan2".into(), state())] }, + ], + ..Default::default() + }; + history.trim(2, ¤t); + assert_eq!(history.branches.len(), 1); + assert!(Arc::ptr_eq(&history.branches[0].steps[0].1, &reachable)); + assert_eq!(history.branch_states(), 1); + assert_eq!(history.tree(¤t).len(), 2); +}