diff --git a/crates/engine/src/cmd/atn_cmds/steps_stack.rs b/crates/engine/src/cmd/atn_cmds/steps_stack.rs index 1b72fe5454..a351ab1122 100644 --- a/crates/engine/src/cmd/atn_cmds/steps_stack.rs +++ b/crates/engine/src/cmd/atn_cmds/steps_stack.rs @@ -82,6 +82,9 @@ pub(super) fn mode_to(id: &str, p: &Value) -> Option { 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 { 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. diff --git a/crates/engine/src/cmd/video_cmds.rs b/crates/engine/src/cmd/video_cmds.rs index d5540c1f79..3b1a631c81 100644 --- a/crates/engine/src/cmd/video_cmds.rs +++ b/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) -> 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 { 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 { fn insert_blank(s: &mut Session, _p: &Value) -> Result { 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 { fn duplicate_frame(s: &mut Session, _p: &Value) -> Result { 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); diff --git a/crates/engine/src/cmd/video_cmds/tests.rs b/crates/engine/src/cmd/video_cmds/tests.rs index f2acb0bd0f..d7feacb06a 100644 --- a/crates/engine/src/cmd/video_cmds/tests.rs +++ b/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(); diff --git a/crates/engine/tests/it/atn_stack_modes.rs b/crates/engine/tests/it/atn_stack_modes.rs index fa49b78976..bb6dd7626a 100644 --- a/crates/engine/tests/it/atn_stack_modes.rs +++ b/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()); + } } } diff --git a/crates/engine/tests/it/atn_video_frames.rs b/crates/engine/tests/it/atn_video_frames.rs index 9da5ef64a9..43c9261ae1 100644 --- a/crates/engine/tests/it/atn_video_frames.rs +++ b/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 {