Handle edited Stack Modes and cap video frame growth

crates/engine/src/cmd/atn_cmds/steps_stack.rs

@@ -82,6 +82,9 @@ pub(super) fn mode_to(id: &str, p: &Value) -> Option<Descriptor> {
         return Some(canonical);
     }
     let source = original(p)?;
+    if source.items.is_empty() {
+        return Some(canonical);
+    }
     let (_, recorded) = recorded_mode(&source)?;
     Some(if recorded == mode { source } else { canonical })
 }
@@ -99,7 +102,10 @@ pub(super) fn none_to(_: &str, p: &Value) -> Option<Descriptor> {
         return Some(desc("null"));
     }
     let source = original(p)?;
-    source.items.is_empty().then_some(source)
+    if source.items.is_empty() {
+        return Some(source);
+    }
+    recorded_mode(&source).map(|_| desc("null"))
 }
 
 /// Import otherwise presents an absent descriptor as an empty one. Retain the distinction for None.

crates/engine/src/cmd/video_cmds.rs

@@ -110,6 +110,15 @@ fn blank_of(fmt: photocraft_color::PixelFormat) -> Surface {
     Surface::new(fmt)
 }
 
+/// Both the altered and original footage stacks must have room for an added frame.
+fn check_frame_growth(frames: usize, original_frames: Option<usize>) -> Result<()> {
+    let limit = crate::timeline_cmds::MAX_DURATION;
+    if frames.max(original_frames.unwrap_or(0)) >= limit {
+        return Err(EngineError::Other(format!("a video layer is limited to {limit} frames")));
+    }
+    Ok(())
+}
+
 fn new_blank(s: &mut Session, _p: &Value) -> Result<Value> {
     let id = s.edit("New Blank Video Layer", |doc, active| {
         store(doc);
@@ -138,7 +147,8 @@ fn new_blank(s: &mut Session, _p: &Value) -> Result<Value> {
 fn insert_blank(s: &mut Session, _p: &Value) -> Result<Value> {
     let fmt = s.active().ok_or(EngineError::NoDocument)?.doc.pixel_format();
     edit_video(s, "Insert Blank Frame", move |v, cur| {
-        let at = (cur + 1).min(v.frames.len());
+        check_frame_growth(v.frames.len(), v.original_frames.as_ref().map(Vec::len))?;
+        let at = cur.saturating_add(1).min(v.frames.len());
         v.frames.insert(at, blank_of(fmt));
         if let Some(original) = &mut v.original_frames {
             // The stacks stay aligned, but never index past a shorter one.
@@ -150,7 +160,9 @@ fn insert_blank(s: &mut Session, _p: &Value) -> Result<Value> {
 
 fn duplicate_frame(s: &mut Session, _p: &Value) -> Result<Value> {
     edit_video(s, "Duplicate Frame", |v, cur| {
-        if let Some(f) = v.frames.get(cur).cloned() {
+        if let Some(f) = v.frames.get(cur) {
+            check_frame_growth(v.frames.len(), v.original_frames.as_ref().map(Vec::len))?;
+            let f = f.clone();
             if let Some(original) = &mut v.original_frames {
                 let orig = original.get(cur).cloned().unwrap_or_else(|| blank_of(f.format()));
                 original.insert((cur + 1).min(original.len()), orig);

crates/engine/src/cmd/video_cmds/tests.rs

@@ -29,6 +29,17 @@ fn new_blank_insert_duplicate_delete_frames() {
     assert!(s.active().unwrap().doc.timeline.as_ref().unwrap().duration >= frames(&s, id));
 }
 
+/// Check the shared growth limit without constructing a million-frame document.
+#[test]
+fn frame_growth_stops_at_the_limit_of_either_stack() {
+    let limit = crate::timeline_cmds::MAX_DURATION;
+    assert!(check_frame_growth(limit - 1, None).is_ok());
+    assert!(check_frame_growth(limit - 1, Some(limit - 1)).is_ok());
+    assert!(check_frame_growth(limit, None).is_err());
+    assert!(check_frame_growth(limit - 1, Some(limit)).is_err());
+    assert!(check_frame_growth(limit + 1, None).is_err());
+}
+
 #[test]
 fn rasterize_drops_the_stack() {
     let mut s = session();

crates/engine/tests/it/atn_stack_modes.rs

@@ -211,16 +211,28 @@ fn stack_mode_edits_and_unrepresented_native_parameters_are_safe() {
     let mut action = imported.actions[0].clone();
     action.steps[0].0 = "layer.smartObjects.stackMode.maximum".into();
     assert_eq!(saved(&action).descriptor, Some(descriptor("maxx", "Maximum")));
+
+    action.steps[0].0 = "layer.smartObjects.stackMode.none".into();
+    let cleared = saved(&action);
+    assert_eq!(cleared.event, AtnEvent::name(CLEAR));
+    assert_eq!(cleared.descriptor, Some(Descriptor::new("null")));
+
+    let mut action = load(item(CLEAR, Some(Descriptor::new("null")))).actions.remove(0);
+    action.steps[0].0 = "layer.smartObjects.stackMode.mean".into();
+    let applied = saved(&action);
+    assert_eq!(applied.event, AtnEvent::name(APPLY));
+    assert_eq!(applied.descriptor, Some(descriptor("avrg", "Mean")));
+
+    let unknown_class = photocraft_engine::paint::tile::b64_encode(&descriptor("nope", "Mean").to_bytes());
     for command in ["layer.smartObjects.stackMode.mean", "layer.smartObjects.stackMode.none"] {
         for params in [json!(null), json!([]), json!({"layer":12}), json!({"layer":"Named"}), json!({"target":"mask"}), json!({"unknown":true})] {
             let action = Action::new("Mode", vec![(command.into(), params)]);
             assert!(atn_cmds::export("Stacks", &[&action], &[], false).is_err());
         }
-    }
-    let unknown_class = photocraft_engine::paint::tile::b64_encode(&descriptor("nope", "Mean").to_bytes());
-    for origin in [json!(false), json!("!invalid-base64"), json!(""), json!(unknown_class)] {
-        let mut action = Action::new("Mode", vec![("layer.smartObjects.stackMode.mean".into(), json!({}))]);
-        action.steps[0].1[ORIGIN] = origin;
-        assert!(atn_cmds::export("Stacks", &[&action], &[], false).is_err());
+        for origin in [json!(false), json!("!invalid-base64"), json!(""), json!(unknown_class)] {
+            let mut action = Action::new("Mode", vec![(command.into(), json!({}))]);
+            action.steps[0].1[ORIGIN] = origin;
+            assert!(atn_cmds::export("Stacks", &[&action], &[], false).is_err());
+        }
     }
 }

crates/engine/tests/it/atn_video_frames.rs

@@ -36,8 +36,8 @@ fn saved(action: &Action) -> AtnItem {
 }
 
 /// Playback tests require a native mapping, rather than a successfully loaded kept step.
-fn load(s: &mut Session, event: &str) {
-    let bytes = file(item(event, Some(Descriptor::default())));
+fn load(s: &mut Session, source: AtnItem) {
+    let bytes = file(source);
     let report = s.execute("actions.load", json!({"data":photocraft_engine::paint::tile::b64_encode(&bytes)})).unwrap();
     assert_eq!(report["unsupported"], json!([]));
 }
@@ -115,7 +115,7 @@ fn descriptors_and_step_flags_round_trip_including_an_absent_descriptor() {
 #[test]
 /// One recorded command must change the correct stack position and remain one undoable edit.
 fn playback_edits_the_current_frame_and_original_stack_with_undo_redo_at_each_depth() {
-    for depth in [8, 16, 32] {
+    for (depth, has_descriptor) in [(8, true), (16, true), (32, true), (8, false)] {
         for (event, _) in EVENTS {
             let (mut s, id) = fixture(depth);
             let before = pixels(&s, id, false);
@@ -127,7 +127,7 @@ fn playback_edits_the_current_frame_and_original_stack_with_undo_redo_at_each_de
                 "deleteFrame" => vec![frames[0].clone(), frames[2].clone()],
                 _ => panic!("fixture event"),
             };
-            load(&mut s, event);
+            load(&mut s, item(event, has_descriptor.then(Descriptor::default)));
             let report = s.execute("actions.play", json!({"action":0})).unwrap();
             assert!(report.get("failed").is_none(), "{depth}/{event}: {report}");
             assert_eq!(report["ran"], 1);
@@ -164,7 +164,7 @@ fn deleting_the_last_frame_keeps_its_pixels() {
     *t = photocraft_doc::video::Timeline::new(1, 24.0);
     let before = pixels(&s, id, false);
     let original = pixels(&s, id, true);
-    load(&mut s, "deleteFrame");
+    load(&mut s, item("deleteFrame", Some(Descriptor::default())));
     let report = s.execute("actions.play", json!({"action":0})).unwrap();
     assert!(report.get("failed").is_none(), "{report}");
     assert_eq!(pixels(&s, id, false), before);
@@ -212,7 +212,7 @@ fn playback_without_a_video_target_fails_without_editing_a_document() {
                 s.execute("file.new", json!({"width":2,"height":2})).unwrap();
             }
             let before = s.active().map(|state| state.doc.clone());
-            load(&mut s, event);
+            load(&mut s, item(event, Some(Descriptor::default())));
             let report = s.execute("actions.play", json!({"action":0})).unwrap();
             assert!(report.get("failed").is_some(), "{event}: {report}");
             match before {