diff --git a/Cargo.lock b/Cargo.lock index 703e51d4c..ca811a8fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -968,6 +968,11 @@ dependencies = [ "libc", ] +[[package]] +name = "craft-docpath" +version = "0.1.0" +source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63" + [[package]] name = "craft-log" version = "0.1.0" @@ -1170,7 +1175,7 @@ dependencies = [ [[package]] name = "effectcraft" -version = "0.10.0" +version = "0.12.0" dependencies = [ "cpal", "craft-log", @@ -1198,7 +1203,7 @@ dependencies = [ [[package]] name = "effectcraft-aep-native" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-effects", @@ -1210,7 +1215,7 @@ dependencies = [ [[package]] name = "effectcraft-automation" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-engine", "effectcraft-host", @@ -1222,14 +1227,14 @@ dependencies = [ [[package]] name = "effectcraft-av1enc" -version = "0.10.0" +version = "0.12.0" dependencies = [ "filmcraft-av1", ] [[package]] name = "effectcraft-cli" -version = "0.10.0" +version = "0.12.0" dependencies = [ "cpu-time", "craft-log", @@ -1248,7 +1253,7 @@ dependencies = [ [[package]] name = "effectcraft-color" -version = "0.10.0" +version = "0.12.0" dependencies = [ "proptest", "serde", @@ -1256,7 +1261,7 @@ dependencies = [ [[package]] name = "effectcraft-effects" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1273,8 +1278,9 @@ dependencies = [ [[package]] name = "effectcraft-engine" -version = "0.10.0" +version = "0.12.0" dependencies = [ + "craft-docpath", "effectcraft-aep-native", "effectcraft-color", "effectcraft-effects", @@ -1307,7 +1313,7 @@ dependencies = [ [[package]] name = "effectcraft-export" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-av1enc", "effectcraft-color", @@ -1338,7 +1344,7 @@ dependencies = [ [[package]] name = "effectcraft-expr" -version = "0.10.0" +version = "0.12.0" dependencies = [ "boa_engine", "effectcraft-color", @@ -1355,7 +1361,7 @@ dependencies = [ [[package]] name = "effectcraft-geom" -version = "0.10.0" +version = "0.12.0" dependencies = [ "proptest", "serde", @@ -1363,7 +1369,7 @@ dependencies = [ [[package]] name = "effectcraft-gpu" -version = "0.10.0" +version = "0.12.0" dependencies = [ "bytemuck", "effectcraft-color", @@ -1385,14 +1391,14 @@ dependencies = [ [[package]] name = "effectcraft-hevcenc" -version = "0.10.0" +version = "0.12.0" dependencies = [ "filmcraft-hevc", ] [[package]] name = "effectcraft-host" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-engine", "effectcraft-export", @@ -1417,7 +1423,7 @@ dependencies = [ [[package]] name = "effectcraft-interchange" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-keyframe", @@ -1436,7 +1442,7 @@ dependencies = [ [[package]] name = "effectcraft-keyframe" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1448,8 +1454,9 @@ dependencies = [ [[package]] name = "effectcraft-lottie" -version = "0.10.0" +version = "0.12.0" dependencies = [ + "craft-docpath", "effectcraft-color", "effectcraft-effects", "effectcraft-keyframe", @@ -1465,8 +1472,9 @@ dependencies = [ [[package]] name = "effectcraft-media" -version = "0.10.0" +version = "0.12.0" dependencies = [ + "craft-docpath", "effectcraft-model", "effectcraft-pdf", "effectcraft-project", @@ -1493,7 +1501,7 @@ dependencies = [ [[package]] name = "effectcraft-model" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-geom", "image", @@ -1505,14 +1513,14 @@ dependencies = [ [[package]] name = "effectcraft-opusenc" -version = "0.10.0" +version = "0.12.0" dependencies = [ "filmcraft-opus", ] [[package]] name = "effectcraft-path" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-geom", "effectcraft-keyframe", @@ -1524,7 +1532,7 @@ dependencies = [ [[package]] name = "effectcraft-pdf" -version = "0.10.0" +version = "0.12.0" dependencies = [ "craft-pdf", "effectcraft-raster", @@ -1540,7 +1548,7 @@ dependencies = [ [[package]] name = "effectcraft-plugin" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-effects", "effectcraft-keyframe", @@ -1552,7 +1560,7 @@ dependencies = [ [[package]] name = "effectcraft-project" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1565,14 +1573,14 @@ dependencies = [ [[package]] name = "effectcraft-psd" -version = "0.10.0" +version = "0.12.0" dependencies = [ "miniz_oxide 0.8.9", ] [[package]] name = "effectcraft-raster" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1581,7 +1589,7 @@ dependencies = [ [[package]] name = "effectcraft-render" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-effects", @@ -1603,7 +1611,7 @@ dependencies = [ [[package]] name = "effectcraft-script" -version = "0.10.0" +version = "0.12.0" dependencies = [ "boa_engine", "effectcraft-engine", @@ -1620,7 +1628,7 @@ dependencies = [ [[package]] name = "effectcraft-segment" -version = "0.10.0" +version = "0.12.0" dependencies = [ "miniz_oxide 0.8.9", "ndarray", @@ -1629,7 +1637,7 @@ dependencies = [ [[package]] name = "effectcraft-svg" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-geom", "effectcraft-path", @@ -1641,7 +1649,7 @@ dependencies = [ [[package]] name = "effectcraft-sysmem" -version = "0.10.0" +version = "0.12.0" dependencies = [ "libc", "windows-sys 0.61.2", @@ -1649,7 +1657,7 @@ dependencies = [ [[package]] name = "effectcraft-text" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-keyframe", "harfrust", @@ -1661,7 +1669,7 @@ dependencies = [ [[package]] name = "effectcraft-time" -version = "0.10.0" +version = "0.12.0" dependencies = [ "proptest", "serde", @@ -1669,7 +1677,7 @@ dependencies = [ [[package]] name = "effectcraft-track" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-raster", "effectcraft-segment", @@ -1681,7 +1689,7 @@ dependencies = [ [[package]] name = "effectcraft-ui-egui" -version = "0.10.0" +version = "0.12.0" dependencies = [ "craft-ui", "effectcraft-color", @@ -1713,11 +1721,11 @@ dependencies = [ [[package]] name = "effectcraft-vp9enc" -version = "0.10.0" +version = "0.12.0" [[package]] name = "effectcraft-web" -version = "0.10.0" +version = "0.12.0" dependencies = [ "effectcraft-engine", "effectcraft-export", @@ -6704,7 +6712,7 @@ checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7" [[package]] name = "xtask" -version = "0.10.0" +version = "0.12.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 6ccc994e5..e4316b78a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/*"] [workspace.package] -version = "0.10.0" +version = "0.12.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" @@ -12,6 +12,7 @@ repository = "https://github.com/storytold/effectcraft" [workspace.dependencies] craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "ca50d63b773c99f96e75c77ce5a1d21e7c9cda0d", version = "=0.1.0", default-features = false, features = ["read"] } +craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # internal effectcraft-time = { path = "crates/time" } effectcraft-geom = { path = "crates/geom" } diff --git a/ROADMAP.md b/ROADMAP.md index e21039c02..1887d7b82 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -117,6 +117,10 @@ Come tell us what matters most to you on [Discord](https://discord.gg/artcraft). ## Progress log +- 2026-10-16: Convert To Bezier Path turns a shape layer's Rectangle, Ellipse or Polystar + path into an editable Path 1 with the same geometry, as After Effects does; it used to + apply to mask RotoBezier only, so a parametric shape path could not be converted at all + and its vertices could not be edited or animated (#932). - 2026-10-12: Roto Brush span edits report strokes outside a trimmed layer without panicking; strokes and undo history are preserved so restoring the timing allows the span edit again. - 2026-10-11: Ukrainian panels caught up with the Chinese catalogs. The 95 rows the panel diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index 01aa3e186..ea52c99a7 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -16,6 +16,8 @@ //! [--profile main|main10] [--level auto|4.1] [--rate-control bitrate|quality] [--video-quality 1-100] //! [--keyint FRAMES] [--webm-codec vp9|av1] [--audio-bitrate KBPS] [--opus-app audio|voice] //! (a relative --out is relative to the working directory; --queue outputs follow the project's own settings) +//! [--progress] report progress on stderr when it is not a terminal, up to four lines a second: +//! `progress ` //! effectcraft-cli render --template T.ectemplate [--values V.json] --out FILE //! effectcraft-cli render-frame --template T.ectemplate [--values V.json] [--time S] [--out F.png] //! effectcraft-cli render F.ecproj --queue render the project's Render Queue @@ -107,6 +109,8 @@ const USAGE: &str = "usage: effectcraft-cli ) render/render-frame --template T.ectemplate [--values V.json] render one template instance bench [--comp C] [--time S] [--scale K] [--n N] [--play N] [--gpu [--adv3d]] per-layer/effect render timings; --play N renders N consecutive frames with/without the layer cache; @@ -191,6 +195,7 @@ const FLAGS: &[&str] = &[ "--small", "--adv3d", "--autosave", + "--progress", ]; struct Args { @@ -782,8 +787,21 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { } s.execute("renderQueue.render", json!({"wait": false})).map_err(err)?; let tty = std::io::IsTerminal::is_terminal(&std::io::stderr()); + // `--progress`: one line per update for a program reading stderr (a terminal gets the live line). + let lines = args.flag("--progress") && !tty; + let (mut last_line, mut last_done) = (std::time::Instant::now(), u64::MAX); while s.is_rendering() { std::thread::sleep(std::time::Duration::from_millis(100)); + if lines + && last_line.elapsed() >= std::time::Duration::from_millis(250) + && let Some(p) = s.render_progress() + && p.done != last_done + { + last_line = std::time::Instant::now(); + last_done = p.done; + let left = p.remaining.map_or_else(|| "-".to_string(), |r| format!("{r:.1}")); + note!("progress {} {} {:.1} {left}", p.done, p.total, p.item_elapsed); + } if tty && let Some(p) = s.render_progress() { let left = p.remaining.map(|r| format!(", ~{r:.1}s left")).unwrap_or_default(); let _ = write!( diff --git a/apps/effectcraft-cli/src/template.rs b/apps/effectcraft-cli/src/template.rs index f84303dd0..844c978b2 100644 --- a/apps/effectcraft-cli/src/template.rs +++ b/apps/effectcraft-cli/src/template.rs @@ -4,7 +4,7 @@ use std::io::Read; use effectcraft_engine::{ EngineError, Session, - project::{ItemId, LayerId}, + project::{ItemId, LayerId, essential::EgKind}, }; use serde_json::{Map, Value, json}; @@ -92,7 +92,7 @@ fn preflight(s: &Session, path: &str, values: &Map) -> Result<(), project.fix_next_id(); let mut check = Session::new(); check.project = project.into(); - Ok(instantiate(&mut check, source, values)) + Ok(instantiate(&mut check, source, values, false)) }) .map_err(|e| Failure::Error(e.to_string()))? } @@ -105,10 +105,27 @@ pub(super) fn prepare(s: &mut Session, args: &Args) -> Result<(), Failure> { } let imported = execute(s, "essential.importTemplate", json!({"path": path, "addToComp": false}))?; let source = imported.get("comp").and_then(Value::as_u64).ok_or_else(|| Failure::Error("template import did not return a composition".into()))?; - instantiate(s, source, &values) + instantiate(s, source, &values, true) } -fn instantiate(s: &mut Session, source: u64, values: &Map) -> Result<(), Failure> { +/// The file a Media Replacement control is given: a path, or `{path|file|media: path}`. +fn media_path(value: &Value) -> Option<&str> { + match value { + Value::String(path) => Some(path.as_str()), + Value::Object(m) => ["path", "file", "media"].iter().find_map(|k| m.get(*k)).and_then(Value::as_str), + _ => None, + } +} + +/// Whether the template's control of that name (looked up as `essential.set` does) is a Media Replacement. +fn is_media_control(s: &Session, source: u64, name: &str) -> bool { + let Some(eg) = s.project.comp(ItemId(source)).and_then(|c| c.essential.as_ref()) else { return false }; + let all = eg.flat(); + let control = all.iter().find(|c| c.name == name).or_else(|| all.iter().find(|c| c.name.eq_ignore_ascii_case(name))); + control.is_some_and(|c| matches!(c.kind, EgKind::Media { .. })) +} + +fn instantiate(s: &mut Session, source: u64, values: &Map, import_media: bool) -> Result<(), Failure> { // Duplicate and empty the source with commands: this preserves exact rational frame rate, // Tick duration, background, work area and renderer settings without round-tripping floats. let duplicate = execute(s, "project.duplicate", json!({"items": [source]}))?; @@ -145,6 +162,25 @@ fn instantiate(s: &mut Session, source: u64, values: &Map) -> Res // Let the wrapper carry the source composition's authored motion blur. execute(s, "layer.setSwitch", json!({"comp": parent, "layers": [instance], "switch": "motionBlur", "value": true}))?; for (control, value) in values { + if is_media_control(s, source, control) { + let path = media_path(value).ok_or_else(|| Failure::Error(format!("control `{control}` is a media control: give it a file path")))?; + // The preflight session cannot import files; the real run reports a bad path. + if !import_media { + continue; + } + let imported = execute(s, "file.import", json!({"paths": [path]}))?; + if let Some(err) = imported.get("errors").and_then(Value::as_array).and_then(|e| e.first()) { + return Err(Failure::Error(format!("control `{control}`: cannot import {path}: {err}"))); + } + let item = imported + .get("items") + .and_then(Value::as_array) + .and_then(|items| items.first()) + .and_then(Value::as_u64) + .ok_or_else(|| Failure::Error(format!("control `{control}`: {path} did not import as footage")))?; + execute(s, "essential.set", json!({"comp": parent, "layer": instance, "control": control, "item": item}))?; + continue; + } execute(s, "essential.set", json!({"comp": parent, "layer": instance, "control": control, "value": value}))?; } Ok(()) diff --git a/apps/effectcraft-cli/tests/cli.rs b/apps/effectcraft-cli/tests/cli.rs index cc31e8726..70a58aaad 100644 --- a/apps/effectcraft-cli/tests/cli.rs +++ b/apps/effectcraft-cli/tests/cli.rs @@ -596,3 +596,77 @@ solid.property("ADBE Transform Group").property("ADBE Opacity").addToMotionGraph assert_eq!(reply["error"], "command `renderQueue.add` is not available right now: select or open a composition", "{reply}"); let _ = std::fs::remove_dir_all(&root); } + +/// `WGPU_ADAPTER_NAME` that matches nothing is a clean `--gpu` error listing the adapters +/// found (wgpu's own helper panics). Read once per process, hence a subprocess. +#[test] +fn gpu_adapter_name_mismatch_is_a_clean_error() { + let out = bin() + .args(["render-frame", "--gpu", "--max-side", "64", "--out", tmp("nomatch.png").to_str().unwrap(), "--json"]) + .env("WGPU_ADAPTER_NAME", "NoSuchAdapter-xyz") + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert_eq!(out.status.code(), Some(1), "stdout: {stdout}\nstderr: {}", String::from_utf8_lossy(&out.stderr)); + let v: Value = serde_json::from_str(stdout.trim()).unwrap(); + let e = v["error"].as_str().unwrap(); + assert!(e.contains("WGPU_ADAPTER_NAME=\"NoSuchAdapter-xyz\""), "{e}"); + assert!(e.contains("available: [") || e.contains("no adapters on the enabled backends"), "{e}"); +} + +/// An exported-but-empty `WGPU_ADAPTER_NAME` behaves like unset: it never produces the +/// adapter-name error (whether a GPU exists on this machine is not asserted). +#[test] +fn gpu_adapter_name_empty_is_unset() { + let out = bin() + .args(["render-frame", "--gpu", "--max-side", "64", "--out", tmp("emptyname.png").to_str().unwrap(), "--json"]) + .env("WGPU_ADAPTER_NAME", " ") + .output() + .unwrap(); + let all = format!("{}{}", String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr)); + assert!(!all.contains("WGPU_ADAPTER_NAME"), "{all}"); +} + +/// Renders two seconds of the demo at quarter resolution (long enough for several progress +/// updates) and returns stderr, which `output()` reads through a pipe: not a terminal. +fn render_demo_stderr(name: &str, extra: &[&str]) -> String { + let out = tmp(name); + let mut args = vec!["render", "--demo", "--format", "gif", "--start", "0", "--end", "2", "--resolution", "quarter", "--out", out.to_str().unwrap()]; + args.extend_from_slice(extra); + let o = bin().args(&args).output().unwrap(); + let stderr = String::from_utf8_lossy(&o.stderr).to_string(); + assert!(o.status.success(), "{args:?}: {stderr}"); + let _ = std::fs::remove_file(out); + stderr +} + +/// `render --progress` with a piped stderr reports the render's frames as +/// `progress ` lines, one per +/// change (#1778). +#[test] +fn render_progress_reports_frames_on_a_piped_stderr() { + let stderr = render_demo_stderr("progress.gif", &["--progress"]); + let mut last_done = None; + let mut totals = vec![]; + for line in stderr.lines().filter(|l| l.starts_with("progress")) { + let fields: Vec<&str> = line.split(' ').collect(); + assert_eq!(fields.len(), 5, "{line}"); + let done: u64 = fields[1].parse().unwrap_or_else(|e| panic!("{line}: frames done: {e}")); + let total: u64 = fields[2].parse().unwrap_or_else(|e| panic!("{line}: frames total: {e}")); + let elapsed: f64 = fields[3].parse().unwrap_or_else(|e| panic!("{line}: seconds elapsed: {e}")); + assert!(fields[4] == "-" || fields[4].parse::().is_ok_and(|left| left >= 0.0), "{line}: seconds left"); + assert!(done <= total && total > 0 && elapsed >= 0.0, "{line}"); + assert!(last_done.is_none_or(|last| done > last), "one line per change, and frames only go up: {stderr}"); + last_done = Some(done); + totals.push(total); + } + assert!(!totals.is_empty(), "no progress line on a piped stderr: {stderr}"); + assert!(totals.iter().all(|&t| t == totals[0]), "one render, one total: {stderr}"); +} + +/// Without `--progress`, the same render writes no progress lines to a piped stderr (#1778). +#[test] +fn render_without_progress_writes_no_progress_lines() { + let stderr = render_demo_stderr("no-progress.gif", &[]); + assert!(!stderr.lines().any(|l| l.starts_with("progress")), "{stderr}"); +} diff --git a/apps/effectcraft-cli/tests/qa_template.rs b/apps/effectcraft-cli/tests/qa_template.rs index 2765b163e..8b877755b 100644 --- a/apps/effectcraft-cli/tests/qa_template.rs +++ b/apps/effectcraft-cli/tests/qa_template.rs @@ -689,3 +689,81 @@ fn template_frame_output_cannot_alias_its_values_through_a_hard_link() { assert_eq!(reply["error"], "--out hard-linked-values.png would overwrite --values values.json", "{reply}"); let _ = std::fs::remove_dir_all(&dir); } + +/// A template whose `Clip` control is a Media Replacement showing `source.png` (blue), with +/// `other.png` (red) beside it to replace it with, exported as `lt.ectemplate` in a fresh `dir`. +fn media_template(case: &str) -> PathBuf { + let temp_root = std::env::temp_dir().canonicalize().unwrap(); + let dir = temp_root.join(format!("ec-qa-template-{case}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("build.jsx"), BUILD).unwrap(); + ok(&dir, &["script", "build.jsx", "--save-as", "t.ecproj"]); + image::RgbaImage::from_pixel(2, 2, image::Rgba([30, 60, 90, 255])).save(dir.join("source.png")).unwrap(); + image::RgbaImage::from_pixel(2, 2, image::Rgba([200, 20, 40, 255])).save(dir.join("other.png")).unwrap(); + let imported = ok(&dir, &["exec", "file.import", r#"{"paths":["source.png"]}"#, "t.ecproj", "--save"]); + let item = imported["result"]["items"][0].as_u64().unwrap(); + let params = serde_json::json!({"comp": "Lower Third", "item": item, "position": [8, 8], "time": 0}).to_string(); + let added = ok(&dir, &["exec", "layer.addItem", ¶ms, "t.ecproj", "--save"]); + let layer = added["result"]["layer"].as_u64().unwrap(); + let params = serde_json::json!({"comp": "Lower Third", "layer": layer, "name": "Clip"}).to_string(); + ok(&dir, &["exec", "essential.addMedia", ¶ms, "t.ecproj", "--save"]); + ok(&dir, &["exec", "essential.exportTemplate", r#"{"comp":"Lower Third","path":"lt.ectemplate"}"#, "t.ecproj"]); + dir +} + +/// Renders the template's frame with `values` and checks that the control shows `other.png`. +fn assert_renders_the_replacement(dir: &PathBuf, values: &str) { + std::fs::write(dir.join("values.json"), values).unwrap(); + ok(dir, &["render-frame", "--template", "lt.ectemplate", "--values", "values.json", "--time", "1", "--out", "out.png"]); + let px = image::open(dir.join("out.png")).unwrap().to_rgba8().get_pixel(8, 8).0; + assert!( + (px[0] as i32 - 200).abs() <= 2 && (px[1] as i32 - 20).abs() <= 2 && (px[2] as i32 - 40).abs() <= 2, + "{values}: replacement media rendered: {px:?}" + ); +} + +#[test] +fn template_values_fill_a_media_replacement_control_with_a_file_path() { + let dir = media_template("media-control"); + assert_renders_the_replacement(&dir, r#"{"Clip":"other.png"}"#); + + // A control that is not given a path is rejected with a clear message. + std::fs::write(dir.join("bad.json"), r#"{"Clip":5}"#).unwrap(); + let (code, reply) = cli(&dir, &["render-frame", "--template", "lt.ectemplate", "--values", "bad.json", "--time", "1", "--out", "bad.png"]); + assert_eq!(code, 1, "{reply}"); + assert!(reply["error"].as_str().unwrap().contains("media control"), "{reply}"); + let _ = std::fs::remove_dir_all(&dir); +} + +/// The object forms `{"path"|"file"|"media": …}` take a file path too, and the control is found +/// by name the way `essential.set` finds it: exactly, then ignoring case (#1787). +#[test] +fn template_media_control_takes_a_path_object_and_a_name_in_any_case() { + let dir = media_template("media-forms"); + for values in [ + r#"{"Clip":{"path":"other.png"}}"#, + r#"{"Clip":{"file":"other.png"}}"#, + r#"{"Clip":{"media":"other.png"}}"#, + r#"{"clip":"other.png"}"#, + r#"{"CLIP":{"path":"other.png"}}"#, + ] { + let _ = std::fs::remove_file(dir.join("out.png")); + assert_renders_the_replacement(&dir, values); + } + let _ = std::fs::remove_dir_all(&dir); +} + +/// A media path that cannot be imported fails the render with an error naming the control and +/// the path, and writes no frame (#1787). +#[test] +fn template_media_control_reports_a_path_that_cannot_be_imported() { + let dir = media_template("media-missing"); + std::fs::write(dir.join("missing.json"), r#"{"Clip":"missing.png"}"#).unwrap(); + let (code, reply) = cli(&dir, &["render-frame", "--template", "lt.ectemplate", "--values", "missing.json", "--time", "1", "--out", "missing-out.png"]); + assert_eq!(code, 1, "{reply}"); + let error = reply["error"].as_str().unwrap(); + assert!(error.contains("control `Clip`: cannot import missing.png"), "{reply}"); + assert!(!dir.join("missing-out.png").exists(), "{reply}"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/apps/effectcraft-web/js/host.js b/apps/effectcraft-web/js/host.js index 3db6ad35c..5c438550e 100644 --- a/apps/effectcraft-web/js/host.js +++ b/apps/effectcraft-web/js/host.js @@ -57,6 +57,26 @@ function req(r) { return new Promise((res, rej) => { r.onsuccess = () => res(r.result); r.onerror = () => rej(r.error); }); } +// Run `op` on `store` in a readwrite transaction of its own; resolves once the transaction has +// committed. A request's success doesn't mean the write is stored: the transaction can still +// abort afterwards (quota at commit, the browser closing), rolling it back, and a write +// reported as saved then is never retried (#1242). A failed request aborts the transaction +// with its error; an explicit abort leaves no error, hence the AbortError. +function write(db, store, op) { + return new Promise((res, rej) => { + let tx = null; + try { + tx = db.transaction(store, "readwrite"); + tx.oncomplete = () => res(); + tx.onabort = () => rej(tx.error ?? new DOMException("the storage transaction was aborted", "AbortError")); + op(tx.objectStore(store)); + } catch (e) { + try { tx?.abort(); } catch {} + rej(e); + } + }); +} + async function idb() { if (!self.indexedDB) throw new Error("no IndexedDB"); const open = indexedDB.open(DB, 1); @@ -70,10 +90,10 @@ async function idb() { return all.map((r) => ({ key: r.key, data: new Uint8Array(r.data), modified: r.modified })); }, async put(key, data, modified) { - await req(tx("readwrite").put({ key, data: data.slice().buffer, modified })); + await write(db, STORE, (s) => s.put({ key, data: data.slice().buffer, modified })); }, async del(key) { - await req(tx("readwrite").delete(key)); + await write(db, STORE, (s) => s.delete(key)); }, }; } @@ -315,7 +335,15 @@ function spawn(base) { /// path, bytes)` receives "reply" (json), "file" (path, bytes: a rendered file to download), /// "store" (path, bytes: a file to keep in browser storage) and "error" (json = message). export function workerRun(id, json, files, base, onMessage) { - const w = idle.pop() ?? spawn(base); + let w; + try { + w = idle.pop() ?? spawn(base); + } catch (e) { + // `new Worker` throws at once when the page's origin is opaque (a sandboxed iframe): the job + // fails like one whose worker didn't start. + setTimeout(() => onMessage("error", String((e && e.message) || e), null, null)); + return; + } running.set(id, w); w.ready.then(() => { w.worker.onmessage = (e) => { @@ -380,7 +408,17 @@ const frameWorkers = []; /// GPU effects on it. export function frameWorkerStart(base, onMessage, gpu) { const index = frameWorkers.length; - const worker = new Worker(new URL("worker.js", base), { type: "module", name: `effectcraft-frames-${index}` }); + let worker; + try { + worker = new Worker(new URL("worker.js", base), { type: "module", name: `effectcraft-frames-${index}` }); + } catch (e) { + // The constructor throws at once when the page's origin is opaque (a sandboxed iframe). The + // slot stays, and the failure is reported like a worker that died, after the caller has + // recorded the index: frames then render on the page's thread (as with `?noworkers`). + frameWorkers.push(null); + setTimeout(() => onMessage(index, undefined, undefined, String((e && e.message) || e || "frame worker failed"))); + return index; + } worker.onmessage = (e) => { const m = e.data; if (m && m.type === "frameReply") onMessage(index, m.json, m.bytes ? new Uint8Array(m.bytes) : null, undefined); @@ -550,7 +588,7 @@ async function openFolder(handle, remember) { if (remember) { try { const db = await handlesDb(); - await req(db.transaction("folders", "readwrite").objectStore("folders").put({ name, handle })); + await write(db, "folders", (s) => s.put({ name, handle })); } catch (e) { console.info("media browser: cannot remember the folder", e); } } return { name, entries }; @@ -602,7 +640,7 @@ export async function browseForget(name) { folders.delete(name); try { const db = await handlesDb(); - await req(db.transaction("folders", "readwrite").objectStore("folders").delete(name)); + await write(db, "folders", (s) => s.delete(name)); } catch {} } diff --git a/apps/effectcraft-web/tests/storage.mjs b/apps/effectcraft-web/tests/storage.mjs new file mode 100644 index 000000000..0855c1674 --- /dev/null +++ b/apps/effectcraft-web/tests/storage.mjs @@ -0,0 +1,180 @@ +// The IndexedDB storage backend of the web app (js/host.js) under Node with a fake IndexedDB, no +// browser or Wasm build needed: +// +// node apps/effectcraft-web/tests/storage.mjs +// +// A write is acknowledged only once its transaction has committed, not when its request +// succeeded: a transaction that aborts afterwards rolls the write back, and the write must be +// reported as failed so the flush keeps it for a retry (#1242). Exits non-zero on failure. +import assert from "node:assert/strict"; +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const here = dirname(fileURLToPath(import.meta.url)); +const tick = () => new Promise((r) => setTimeout(r, 0)); +globalThis.self = globalThis; + +// A small IndexedDB: requests succeed (or fail) one task after they're made, and a readwrite +// transaction commits once its requests are done, unless it was aborted (which rolls back its +// writes). `fault` makes the next request do something else. +let fault = null; // "abortAfterSuccess" | "requestError" | "throw" +const databases = new Map(); + +class Request { + constructor(tx) { + this.transaction = tx; + this.result = undefined; + this.error = null; + } +} + +class Store { + constructor(tx, rows) { + this.tx = tx; + this.rows = rows; + } + request(apply) { + const tx = this.tx; + if (tx.finished) throw new DOMException("the transaction has finished", "TransactionInactiveError"); + const f = fault; + fault = null; + if (f === "throw") throw new DOMException("the value can't be cloned", "DataCloneError"); + const r = new Request(tx); + tx.pending++; + setTimeout(() => { + tx.pending--; + if (tx.finished) return; + if (f === "requestError") { + r.error = new DOMException("the key already exists", "ConstraintError"); + const e = { target: r, defaultPrevented: false, preventDefault() { this.defaultPrevented = true; } }; + r.onerror?.(e); + tx.onerror?.(e); + if (!e.defaultPrevented) tx.fail(r.error); + return; + } + r.result = apply(tx.staged); + r.onsuccess?.({ target: r }); + if (f === "abortAfterSuccess") tx.abort(); + tx.maybeCommit(); + }, 0); + return r; + } + put(v) { + return this.request((rows) => { rows.set(v[this.keyPath], structuredClone(v)); return v[this.keyPath]; }); + } + delete(k) { + return this.request((rows) => { rows.delete(k); }); + } + get(k) { + return this.request((rows) => rows.get(k)); + } + getAll() { + return this.request((rows) => [...rows.values()]); + } +} + +class Transaction { + constructor(db, name) { + this.db = db; + this.name = name; + this.staged = new Map(db.stores.get(name)); + this.pending = 0; + this.finished = false; + this.error = null; + setTimeout(() => this.maybeCommit(), 0); + } + objectStore(name) { + const s = new Store(this, this.staged); + s.keyPath = this.db.keyPaths.get(name); + return s; + } + maybeCommit() { + if (this.finished || this.pending > 0) return; + this.finished = true; + this.db.stores.set(this.name, this.staged); + setTimeout(() => this.oncomplete?.(), 0); + } + fail(error) { + this.error = error; + this.abort(); + } + abort() { + if (this.finished) throw new DOMException("the transaction has finished", "InvalidStateError"); + this.finished = true; + setTimeout(() => this.onabort?.(), 0); + } +} + +class Database { + constructor() { + this.stores = new Map(); + this.keyPaths = new Map(); + } + createObjectStore(name, { keyPath }) { + this.stores.set(name, new Map()); + this.keyPaths.set(name, keyPath); + } + transaction(name) { + if (!this.stores.has(name)) throw new DOMException(`no store ${name}`, "NotFoundError"); + return new Transaction(this, name); + } +} + +globalThis.indexedDB = { + open(name) { + const r = new Request(null); + setTimeout(() => { + let db = databases.get(name); + if (!db) { + db = new Database(); + databases.set(name, db); + r.result = db; + r.onupgradeneeded?.(); + } + r.result = db; + r.onsuccess?.({ target: r }); + }, 0); + return r; + }, +}; + +const host = await import(pathToFileURL(join(here, "../js/host.js"))); +assert.equal(await host.storeOpen("indexeddb"), "indexeddb"); +const stored = (key) => databases.get("effectcraft").stores.get("entries").get(key); +const bytes = (s) => new TextEncoder().encode(s); +const text = (row) => row && new TextDecoder().decode(new Uint8Array(row.data)); + +// A healthy write resolves once committed, and the store has it then. +await host.storePut("project.json", bytes("v1"), 1); +assert.equal(text(stored("project.json")), "v1", "a resolved put is committed"); + +// The put's request succeeds, then the transaction aborts: the put fails, the old value stays. +fault = "abortAfterSuccess"; +await assert.rejects(host.storePut("project.json", bytes("v2"), 2), { name: "AbortError" }, "an aborted put is reported"); +assert.equal(text(stored("project.json")), "v1", "the aborted put was rolled back"); + +// The same for a delete: it fails and the entry stays. +fault = "abortAfterSuccess"; +await assert.rejects(host.storeDelete("project.json"), { name: "AbortError" }, "an aborted delete is reported"); +assert.equal(text(stored("project.json")), "v1", "the aborted delete was rolled back"); + +// A failed request aborts its transaction with the request's error. +fault = "requestError"; +await assert.rejects(host.storePut("project.json", bytes("v3"), 3), { name: "ConstraintError" }); +assert.equal(text(stored("project.json")), "v1"); + +// A request that throws at once (a value that can't be stored) fails the write. +fault = "throw"; +await assert.rejects(host.storePut("project.json", bytes("v4"), 4), { name: "DataCloneError" }); +assert.equal(text(stored("project.json")), "v1"); + +// The retry after a failure commits the latest value, and a later delete removes it. +await host.storePut("project.json", bytes("v5"), 5); +assert.equal(text(stored("project.json")), "v5"); +await host.storeDelete("project.json"); +assert.equal(stored("project.json"), undefined, "a resolved delete is committed"); +const all = await host.storeLoadAll(); +assert.deepEqual(all, [], "nothing else was written"); + +await tick(); +console.log("storage: ok"); diff --git a/apps/effectcraft-web/tests/workers.mjs b/apps/effectcraft-web/tests/workers.mjs index 15a77f1d5..58901885f 100644 --- a/apps/effectcraft-web/tests/workers.mjs +++ b/apps/effectcraft-web/tests/workers.mjs @@ -7,6 +7,8 @@ // (#218), and not when it is unchanged. // - A worker whose start fails fails its job and is terminated, instead of the job waiting for // "ready" forever (#216): host.js on the worker's error, worker.js re-raising the failed start. +// - A `new Worker` that throws (a sandboxed iframe's opaque origin, #1586) fails the job, and a +// frame worker is reported dead, instead of the exception escaping into startup. // Exits non-zero on failure. import assert from "node:assert/strict"; import { copyFileSync, mkdtempSync, writeFileSync } from "node:fs"; @@ -67,6 +69,35 @@ assert.equal(failed.kind, "error"); assert.ok(workers[1].terminated, "the failed worker is terminated"); assert.equal(host.workerStats().running, 0); +// A `new Worker` that throws (a sandboxed iframe's opaque origin): the job fails, and a frame +// worker is reported dead, instead of the exception escaping into startup. +failStart = false; +const RealWorker = globalThis.Worker; +globalThis.Worker = class { + constructor() { + throw new Error("SecurityError: cannot be accessed from origin 'null'"); + } +}; +// (Take the idle workers out of play first, so that a new one has to be built.) +for (let n = host.workerStats().idle, id = 100; n > 0; n--, id++) { + host.workerRun(id, "{}", [], "http://localhost/", () => {}); + host.workerCancel(id); +} +assert.equal(host.workerStats().idle, 0); +const threw = await run(6, []); +assert.equal(threw.kind, "error"); +assert.match(threw.json, /SecurityError/); +assert.equal(host.workerStats().running, 0); +let frameCall = null; +const frameDone = new Promise((resolve) => { + frameCall = host.frameWorkerStart("http://localhost/", (index, json, bytes, error) => resolve({ index, error }), false); +}); +const dead = await frameDone; +assert.equal(dead.index, frameCall, "the dead worker keeps its index"); +assert.match(dead.error, /SecurityError/); +host.frameWorkerPost(frameCall, { type: "frame" }); // no worker behind it: ignored +globalThis.Worker = RealWorker; + // #216, worker side: a rejected start is re-raised where the page's `onerror` sees it. const dir = mkdtempSync(join(tmpdir(), "ec-worker-")); copyFileSync(join(here, "../web/worker.js"), join(dir, "worker.js")); diff --git a/apps/effectcraft-web/web/index.html b/apps/effectcraft-web/web/index.html index 70ab93ff5..85049007f 100644 --- a/apps/effectcraft-web/web/index.html +++ b/apps/effectcraft-web/web/index.html @@ -71,7 +71,13 @@ ...ec.info(), crossOriginIsolated: self.crossOriginIsolated, webgpu: !!navigator.gpu, - serviceWorker: !!(navigator.serviceWorker && navigator.serviceWorker.controller), + serviceWorker: (() => { + try { + return !!(navigator.serviceWorker && navigator.serviceWorker.controller); + } catch { + return false; + } + })(), load: window.effectcraftLoad, }), // Browser storage (OPFS / IndexedDB): save without downloading, list, remove, wait for writes. @@ -83,8 +89,14 @@ workerFrameCheck: (params) => ec.workerFrameCheck(params ?? {}).then(JSON.parse), }; // Offline start and install (PWA): the service worker caches the app's files. - if ("serviceWorker" in navigator && !new URLSearchParams(location.search).has("nosw")) { - navigator.serviceWorker.register("sw.js").catch((e) => console.warn("service worker:", e)); + if (!new URLSearchParams(location.search).has("nosw")) { + try { + if ("serviceWorker" in navigator && navigator.serviceWorker) { + navigator.serviceWorker.register("sw.js").catch((e) => console.warn("service worker:", e)); + } + } catch (e) { + console.warn("service worker:", e); + } } try { // Compile once: the page and its render workers instantiate the same module. diff --git a/apps/effectcraft/src/main.rs b/apps/effectcraft/src/main.rs index e0cc1c67a..43375947b 100644 --- a/apps/effectcraft/src/main.rs +++ b/apps/effectcraft/src/main.rs @@ -24,8 +24,8 @@ mod launch_guard; #[cfg(target_os = "macos")] mod native_menu; mod self_logging; // ours -// Built everywhere so it stays checked; only the macOS app uses it. -#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +// Built everywhere so it stays checked; the macOS and Linux apps use it. +#[cfg_attr(not(any(target_os = "macos", target_os = "linux")), allow(dead_code))] mod sheet_dialogs; mod wayland_screen; // ours mod window; @@ -124,6 +124,22 @@ fn control_port(name: &str, value: Option<&str>) -> Result { } } +/// The commands' file dialogs open without blocking the frame where the host's backend would: +/// on macOS as sheets instead of nested modal loops (#511), on Linux through the desktop +/// portal from a worker thread — a blocking portal dialog leaves the main thread waiting on its +/// answer without pumping events, so the window stops answering the compositor and the desktop +/// offers to force-quit it while the Open dialog is up (#1275). Windows' blocking dialog pumps +/// its own messages, so it keeps [`effectcraft_ui_egui::Hooks::pick_files`] and friends. +fn install_file_dialogs(app: &mut EffectcraftApp, ctx: &egui::Context) { + #[cfg(any(target_os = "macos", target_os = "linux"))] + { + let ctx = ctx.clone(); + app.hooks.file_dialog_async = Some(Box::new(move |dialog, done| sheet_dialogs::show(dialog, done, ctx.clone()))); + } + #[cfg(not(any(target_os = "macos", target_os = "linux")))] + let _ = (app, ctx); +} + fn main() -> eframe::Result { // First, so the panic hook and every start-up warning are recorded. Records go to stderr // and the session log; Help ▸ Enable Logging writes through this logger too, and warnings @@ -314,11 +330,11 @@ fn main() -> eframe::Result { let file = path.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(); d.set_file_name(file).save_file().map(|p| p.to_string_lossy().to_string()) })); + // The commands' file dialogs don't block the frame where the platform's backend + // would (#511, #1275); see [`install_file_dialogs`]. + install_file_dialogs(&mut app, &cc.egui_ctx); #[cfg(target_os = "macos")] { - // The commands' file dialogs open as sheets instead of nested modal loops (#511). - let ctx = cc.egui_ctx.clone(); - app.hooks.file_dialog_async = Some(Box::new(move |dialog, done| sheet_dialogs::show(dialog, done, ctx.clone()))); app.hooks.app_action = Some(Box::new(native_menu::app_action)); app.hooks.clipboard_text = Some(Box::new(native_menu::clipboard_text)); } @@ -610,4 +626,17 @@ mod tests { assert_eq!(parse_env(&[], Err(not_utf8)).unwrap_err(), format!("{need}, not `9\u{fffd}`")); } } + + /// #1275: a blocking desktop-portal dialog on Linux leaves the window not answering the + /// compositor while it is open, so the commands' file dialogs go through the non-blocking + /// host hook there (as they do on macOS, #511). Windows' dialog pumps its own messages. + #[test] + fn commands_file_dialogs_do_not_block_the_frame() { + let mut app = effectcraft_ui_egui::EffectcraftApp::new(effectcraft_engine::Session::default()); + super::install_file_dialogs(&mut app, &egui::Context::default()); + #[cfg(any(target_os = "macos", target_os = "linux"))] + assert!(app.hooks.file_dialog_async.is_some(), "the commands' file dialogs must not block the frame"); + #[cfg(not(any(target_os = "macos", target_os = "linux")))] + assert!(app.hooks.file_dialog_async.is_none(), "the blocking dialog pumps its own messages here"); + } } diff --git a/apps/effectcraft/src/sheet_dialogs.rs b/apps/effectcraft/src/sheet_dialogs.rs index 733bb378f..b06f8bf2a 100644 --- a/apps/effectcraft/src/sheet_dialogs.rs +++ b/apps/effectcraft/src/sheet_dialogs.rs @@ -1,10 +1,14 @@ -//! File dialogs that don't block the frame (macOS), for `Hooks::file_dialog_async`. +//! File dialogs that don't block the frame (macOS and Linux), for `Hooks::file_dialog_async`. //! //! A blocking `rfd::FileDialog` runs a nested modal event loop inside eframe's frame, that is //! inside winit's event handler. On macOS a file dragged over the window while that loop ran //! re-entered winit's handler, which panics across the Objective-C boundary and aborts the app -//! (#511). `rfd::AsyncFileDialog` shows the dialog as a sheet on the window and returns at once; -//! the answer arrives when the sheet closes, outside the frame, and the app runs the command then. +//! (#511). On Linux the blocking dialog is worse than slow: rfd uses the XDG desktop portal, so +//! the main thread sits waiting on the portal's answer and never pumps winit's events — the +//! window stops answering the compositor and the desktop offers to force-quit the app while the +//! Open dialog is up (#1275). `rfd::AsyncFileDialog` shows the dialog as a sheet on the window +//! (macOS) or through the portal from a worker thread (Linux) and returns at once; the answer +//! arrives when the dialog closes, outside the frame, and the app runs the command then. use effectcraft_ui_egui::{DialogDone, FileDialog}; use rfd::{AsyncFileDialog, FileHandle}; diff --git a/contributors/people.toml b/contributors/people.toml index 2fa1927ce..625fcdb73 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -66,7 +66,7 @@ sync_github_name = true display_name = "Jason Brashear" [people.nanox333] -display_name = "nanox333" +display_name = "nnaannoo" [people.Nael-Nathanael] sync_github_name = true @@ -87,3 +87,11 @@ sync_github_name = false real_name = "Marco Arnulfo" display_name = "Marco Arnulfo" sync_github_name = false + +[people.krinry] +display_name = "krinry" +sync_github_name = true + +[people.Silvertree2010] +display_name = "Netter" +sync_github_name = false diff --git a/crates/craft-log/src/collapse.rs b/crates/craft-log/src/collapse.rs index dc7632bf7..943210c9e 100644 --- a/crates/craft-log/src/collapse.rs +++ b/crates/craft-log/src/collapse.rs @@ -5,6 +5,11 @@ use std::sync::{Mutex, PoisonError}; /// Lines kept in memory for crash files and failure reports. pub const RING_LINES: usize = 200; + +/// Tests that flood the ring, or need a line of theirs to stay in it, hold this so they never +/// run at the same time (the ring is one per process, and tests run in parallel). +#[cfg(test)] +pub(crate) static RING_TEST_LOCK: Mutex<()> = Mutex::new(()); /// A line repeated this many times gets a "so far" note, so a flood still shows it is going on. pub const REPEAT_NOTE_EVERY: u64 = 10_000; @@ -100,6 +105,7 @@ mod tests { #[test] fn the_ring_keeps_the_newest_lines() { + let _turn = RING_TEST_LOCK.lock().unwrap_or_else(PoisonError::into_inner); for i in 0..(RING_LINES + 5) { remember(&format!("ring-test {i}\n")); } diff --git a/crates/craft-log/src/crash.rs b/crates/craft-log/src/crash.rs index 2e2700532..6e29602d0 100644 --- a/crates/craft-log/src/crash.rs +++ b/crates/craft-log/src/crash.rs @@ -129,6 +129,8 @@ mod tests { #[test] fn a_second_crash_in_the_same_instant_gets_its_own_file() { + // the ring test pushes 205 lines in parallel: it would push ours out + let _turn = collapse::RING_TEST_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); let dir = temp_dir("write"); collapse::remember("crash-test: a line before the panic"); let a = write(&dir, &APP, "first", "t", "bt").unwrap(); diff --git a/crates/effects/src/color3.rs b/crates/effects/src/color3.rs index c85ab8d09..4850710ab 100644 --- a/crates/effects/src/color3.rs +++ b/crates/effects/src/color3.rs @@ -301,7 +301,8 @@ impl OffsetCurve { } let n = ext.len(); // Tangents: the mean of the neighbouring slopes, flat at local extrema and next to flat - // segments so the curve never overshoots its points. + // segments, and at most three times either slope (Fritsch-Carlson) so the curve never + // overshoots its points, even beside a short steep segment. let slope = |k: usize| (ext[k + 1].1 - ext[k].1) / (ext[k + 1].0 - ext[k].0).max(1e-6); let m: Vec = (0..n) .map(|k| { @@ -309,7 +310,12 @@ impl OffsetCurve { return 0.0; } let (a, b) = (slope(k - 1), slope(k)); - if a * b <= 0.0 { 0.0 } else { (a + b) * 0.5 } + if a * b <= 0.0 { + 0.0 + } else { + let lim = 3.0 * a.abs().min(b.abs()); + ((a + b) * 0.5).clamp(-lim, lim) + } }) .collect(); let eval = |x: f32| -> f32 { @@ -989,6 +995,28 @@ mod tests { } } + #[test] + fn hue_saturation_curve_stays_between_its_points() { + // #1656: a point dragged to the bottom right next to another one. The steep short + // segment used to pull the long one beside it far outside the box (|offset| > 0.5). + // The third pair sits across the hue wrap (0.98 and 0.005 are neighbours). + let cases = [("0.3,0.4 0.31,0 0.7,0.6", true), ("0.1,0.3 0.3,0.5 0.31,1 0.7,0.5", false), ("0.5,0.6 0.98,0.4 0.005,0", true)]; + for (pts, periodic) in cases { + let c = OffsetCurve::parse(pts, periodic).unwrap(); + for i in 0..=1000 { + let x = i as f32 / 1000.0; + let v = c.at(x); + assert!((-0.5..=0.5).contains(&v), "{pts} at {x}: {v}"); + } + for p in parse_points(pts) { + assert!((c.at(p.0) - (p.1 - 0.5)).abs() < 0.02, "{pts}: passes through {p:?}"); + } + if periodic { + assert!((c.at(0.0) - c.at(1.0)).abs() < 1e-4, "{pts}: no seam at the wrap"); + } + } + } + fn close_px(a: [f32; 4], b: [f32; 4], eps: f32) -> bool { a.iter().zip(b).all(|(x, y)| (x - y).abs() <= eps) } diff --git a/crates/effects/src/distort.rs b/crates/effects/src/distort.rs index a68de1094..534da8061 100644 --- a/crates/effects/src/distort.rs +++ b/crates/effects/src/distort.rs @@ -164,7 +164,10 @@ fn wave_warp(ctx: &EffectCtx, mut b: Buf) -> Buf { let kind = ctx.params.e("waveType"); let pin = ctx.params.e("pinning"); let seed = ctx.seed ^ 0x3a7e; - let (dx, dy) = (dir.sin(), -dir.cos()); + // Cardinal directions must not acquire a tiny perpendicular slope: at a square-wave + // boundary cos(90°)'s rounding residue can flip a whole column on CPU but not GPU. + let axis = |v: f64| if v.abs() < 4.0 * f64::EPSILON { 0.0 } else { v }; + let (dx, dy) = (axis(dir.sin()), -axis(dir.cos())); if !ctx.adjustment { b.pad(h.abs().ceil() as u32 + 1); } @@ -556,8 +559,21 @@ mod tests { let (a, b) = (o.img.get(pad, y + pad), im.get(0, y)); assert!((0..4).all(|i| (a[i] - b[i]).abs() < 0.01), "row {y}: {a:?} vs {b:?}"); } - let free = run("ec.distort.wavewarp", &[("direction", num(0.0)), ("height", num(5.0))], im); + let free = run("ec.distort.wavewarp", &[("direction", num(0.0)), ("height", num(5.0))], im.clone()); assert!(free.img.data.iter().zip(&o.img.data).any(|(a, b)| differs(*a, *b))); + // #1181: direction 90° → vertical displacement. Top Edge (4) holds the first row, + // Bottom Edge (6) the last, and the rest of the layer still moves. + for (pin, edge_row) in [(Value::Enum(4), 0i64), (Value::Enum(6), 29i64)] { + let set = [("direction", num(90.0)), ("height", num(5.0)), ("pinning", pin)]; + let o = run("ec.distort.wavewarp", &set, im.clone()); + let pad = ((o.img.width - im.width) / 2) as i64; + for x in 0..40 { + let (a, b) = (o.img.get(x + pad, edge_row + pad), im.get(x, edge_row)); + assert!((0..4).all(|i| (a[i] - b[i]).abs() < 0.01), "pin {edge_row} col {x}: {a:?} vs {b:?}"); + } + let mid = 15; + assert!((0..40).any(|x| differs(o.img.get(x + pad, mid + pad), im.get(x, mid))), "pin {edge_row}: the middle of the layer must still distort"); + } } #[test] diff --git a/crates/effects/src/keying2.rs b/crates/effects/src/keying2.rs index f594bfbb3..a03fe804d 100644 --- a/crates/effects/src/keying2.rs +++ b/crates/effects/src/keying2.rs @@ -363,7 +363,9 @@ fn wire_removal(ctx: &EffectCtx, mut b: Buf) -> Buf { let (qx, qy) = (x as f64 + 0.5 - ax, y as f64 + 0.5 - ay); let d = qx * nx + qy * ny; let ad = d.abs(); - if ad > half + soft_w { + // The wire is the segment A to B, not an endless line. + let along = qx * tx + qy * ty; + if !(0.0..=len).contains(&along) || ad > half + soft_w { continue; } // Positions just outside the wire on both sides. @@ -580,6 +582,24 @@ mod tests { } } + #[test] + fn wire_removal_stays_between_a_and_b() { + let mut img = Image::filled(20, 20, [0.5, 0.5, 0.5, 1.0]); + for y in 0..20 { + img.set(10, y, [0.0, 0.0, 0.0, 1.0]); + } + let out = run_fx( + "ec.key.ccsimplewireremoval", + &[("pointA", Value::Vec2([10.5, 5.0])), ("pointB", Value::Vec2([10.5, 15.0])), ("removalStyle", Value::Enum(0)), ("thickness", num(3.0))], + img.clone(), + 0.0, + EffectEnv::default(), + ); + assert!((out.img.get(10, 10)[0] - 0.5).abs() < 0.05, "inside the segment"); + assert_eq!(out.img.get(10, 2), img.get(10, 2), "before A"); + assert_eq!(out.img.get(10, 18), img.get(10, 18), "after B"); + } + #[test] fn wire_removal_frame_offset_uses_neighbour_frame() { use crate::{EffectHost, LayerPixels}; diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index a11c66035..d35e08827 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -10,6 +10,7 @@ rust-version.workspace = true workspace = true [dependencies] +craft-docpath = { workspace = true } effectcraft-time = { workspace = true } effectcraft-geom = { workspace = true } effectcraft-color = { workspace = true } diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index e1c56ef26..37127ac56 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -47,6 +47,11 @@ pub struct AutoSaveState { #[derive(Clone)] pub struct PendingWrite { pub path: PathBuf, + pub(crate) slot: u32, + pub(crate) revision: u64, + /// Originating project identity: the path changes on Save As, the epoch on Open/New. + pub(crate) project_path: Option, + pub(crate) project_epoch: u64, handle: std::sync::Arc>>>>, } @@ -57,9 +62,16 @@ impl std::fmt::Debug for PendingWrite { } impl PendingWrite { - pub fn spawn(path: PathBuf, work: impl FnOnce() -> std::io::Result<()> + Send + 'static) -> std::io::Result { + pub fn spawn( + path: PathBuf, + slot: u32, + revision: u64, + project_path: Option, + project_epoch: u64, + work: impl FnOnce() -> std::io::Result<()> + Send + 'static, + ) -> std::io::Result { let h = std::thread::Builder::new().name("autosave".into()).spawn(work)?; - Ok(PendingWrite { path, handle: std::sync::Arc::new(std::sync::Mutex::new(Some(h))) }) + Ok(PendingWrite { path, slot, revision, project_path, project_epoch, handle: std::sync::Arc::new(std::sync::Mutex::new(Some(h))) }) } pub fn is_finished(&self) -> bool { self.handle.lock().map(|h| h.as_ref().is_none_or(|h| h.is_finished())).unwrap_or(true) diff --git a/crates/engine/src/commands/align_data.rs b/crates/engine/src/commands/align_data.rs index 1f18de5f2..097bddc22 100644 --- a/crates/engine/src/commands/align_data.rs +++ b/crates/engine/src/commands/align_data.rs @@ -182,7 +182,7 @@ fn run(s: &mut Session, p: &Value) -> Result { let video = match p.get("videoStart").filter(|v| v.as_str() != Some("")) { Some(v) => parse_stamp(v, fps).ok_or_else(|| bad(c, "videoStart: ISO 8601 date-time, hh:mm:ss, timecode or seconds"))?, None => { - let (created, _, _) = crate::media_browser::file_dates(&footage.path); + let (created, _, _) = if footage.missing { (None, None, None) } else { crate::media_browser::file_dates(&footage.path) }; Stamp { secs: created.ok_or_else(|| bad(c, "the footage has no recording time; pass `videoStart`"))? as f64, dated: true } } }; diff --git a/crates/engine/src/commands/anim.rs b/crates/engine/src/commands/anim.rs index 3ea27b0df..325845115 100644 --- a/crates/engine/src/commands/anim.rs +++ b/crates/engine/src/commands/anim.rs @@ -1,8 +1,8 @@ //! Keyframe clipboard, selection, nudging, direct key/ease edits (Graph Editor handles) and key //! inspection for the Keyframe Velocity / Keyframe Interpolation dialogs. -use effectcraft_keyframe::{Ease, Interp, Keyframe, key_at, retime_roving, side_ease}; -use effectcraft_project::{LayerId, Property, Uid}; +use effectcraft_keyframe::{Ease, Interp, Keyframe, ShapePath, Value as KValue, key_at, retime_roving, side_ease}; +use effectcraft_project::{ItemId, LayerId, Property, Uid}; use effectcraft_time::Tick; use serde_json::{Value, json}; @@ -61,6 +61,13 @@ fn paste(s: &mut Session, p: &Value) -> Result { if clip.is_empty() { return Err(bad("keys.paste", "no keyframes have been copied")); } + // A copied Mask Path / shape Path key pasted onto Position becomes a motion path (#1425). + if let [c] = clip.as_slice() + && let Some(KValue::Path(path)) = c.keys.first().map(|k| &k.value) + && let Some(target) = path_target(s, p)? + { + return paste_path(s, p, target, path); + } let cid = comp_id(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; let t = time_p(s, p, Some(comp)); @@ -134,6 +141,86 @@ fn paste(s: &mut Session, p: &Value) -> Result { }) } +/// The spatial property (Position, Anchor Point…) a copied path pastes onto as a motion path: +/// the `prop` / `path` given, else a selected one of the target layers. `None` when there's none. +pub(super) fn path_target(s: &Session, p: &Value) -> Result> { + let (cid, targets) = layers_p(s, p)?; + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let motion = |pr: &Property| pr.spatial && !pr.static_only && matches!(pr.value, KValue::Vec2(_) | KValue::Vec3(_)); + let explicit = targets.first().and_then(|lid| { + let l = comp.layer(*lid)?; + let pr = match p.get("prop").and_then(Value::as_u64) { + Some(u) => l.props.find(u), + None => l.props.prop(str_p(p, "path")?), + }?; + Some((*lid, pr)) + }); + if p.get("prop").is_some() || p.get("path").is_some() { + return Ok(explicit.filter(|(_, pr)| motion(pr)).map(|(lid, pr)| (cid, lid, pr.uid))); + } + let selected = s + .state + .selected_props + .iter() + .filter(|(lid, _)| targets.contains(lid)) + .find(|(lid, uid)| comp.layer(*lid).and_then(|l| l.props.find(*uid)).is_some_and(motion)); + Ok(selected.map(|(lid, uid)| (cid, *lid, *uid))) +} + +/// Paste a mask or shape path onto a spatial property as After Effects does: a keyframe per +/// vertex with the path's tangents, from the CTI over 2 s (a closed path ends back on its first +/// vertex), the inner keys roving so the layer moves at one speed. Like After Effects, the +/// vertices are pasted as they are (layer-space numbers, not converted to the target's space). +pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, LayerId, Uid), path: &ShapePath) -> Result { + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let t = time_p(s, p, Some(comp)); + let target = comp.layer(lid).ok_or(EngineError::NoComp)?; + let mut order: Vec = (0..path.vertices.len()).collect(); + if path.closed && !order.is_empty() { + order.push(0); + } + if order.is_empty() { + return Err(bad("keys.paste", "the copied path has no points")); + } + let span = Tick::from_seconds_f64(2.0).0 / i64::try_from(order.len().saturating_sub(1).max(1)).unwrap_or(i64::MAX); + let at = |xy: Option<&[f64; 2]>| { + let [x, y] = xy.copied().unwrap_or_default(); + [x, y, 0.0] + }; + let last = order.len() - 1; + let keys: Vec<(Tick, KValue, [f64; 3], [f64; 3], bool)> = order + .iter() + .enumerate() + .map(|(i, &v)| { + let [x, y, z] = at(path.vertices.get(v)); + let value = match target.props.find(uid).map(|pr| &pr.value) { + Some(KValue::Vec2(_)) => KValue::Vec2([x, y]), + _ => KValue::Vec3([x, y, z]), + }; + let time = target.layer_time(t + Tick(span.saturating_mul(i64::try_from(i).unwrap_or(i64::MAX)))); + (time, value, at(path.in_tangents.get(v)), at(path.out_tangents.get(v)), i != 0 && i != last) + }) + .collect(); + s.edit("Paste Keyframes", merge_p(p), |proj, st| { + let l = super::layer_mut(proj, cid, lid)?; + let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.paste", "no property"))?; + let mut sel = vec![]; + for (time, value, spatial_in, spatial_out, roving) in keys { + let k = Keyframe { spatial_in, spatial_out, spatial_auto: false, roving, ..Keyframe::new(time, value) }; + match pr.keys.binary_search_by(|x| x.time.cmp(&time)) { + Ok(i) => pr.keys[i] = k, + Err(i) => pr.keys.insert(i, k), + } + sel.push(time); + } + // Re-timing keeps the keys' order, so the pasted keys are found again by index. + let pasted: Vec = pr.keys.iter().enumerate().filter(|(_, k)| sel.contains(&k.time)).map(|(i, _)| i).collect(); + retime_roving(&mut pr.keys, pr.spatial); + st.selected_keys = pasted.iter().filter_map(|&i| pr.keys.get(i)).map(|k| KeyRef { layer: lid, prop: uid, time: k.time }).collect(); + Ok(json!(st.selected_keys.len())) + }) +} + /// Select every keyframe of the selected properties, or of the given / selected layers. fn select_all(s: &mut Session, p: &Value) -> Result { let (cid, mut layers) = layers_p(s, p)?; diff --git a/crates/engine/src/commands/animation.rs b/crates/engine/src/commands/animation.rs index a556d9a42..1bdf3fed7 100644 --- a/crates/engine/src/commands/animation.rs +++ b/crates/engine/src/commands/animation.rs @@ -10,7 +10,7 @@ use effectcraft_project::{GroupKind, Layer, LayerId, LayerSource, Node, Project, use effectcraft_time::Tick; use serde_json::{Value, json}; -use super::{CommandSpec, bad, frontend, has_comp, has_keys, has_layers, layer_mut, layers_p, match_path_of, selected_leaf_props, str_p}; +use super::{CommandSpec, bad, frontend, has_comp, has_keys, has_layers, layer_mut, layers_p, match_path_of, resolve_effect_path, selected_leaf_props, str_p}; use crate::{EngineError, KeyRef, Result, Session, cmd}; /// Preset file format version tag (`*.ecpreset`, JSON). @@ -147,7 +147,8 @@ fn apply_preset(s: &mut Session, p: &Value) -> Result { } } for (path, src) in &props { - match l.props.prop_mut(path) { + let resolved = resolve_effect_path(l, path); + match l.props.prop_mut(&resolved) { Some(pr) if std::mem::discriminant(&pr.value) == std::mem::discriminant(&src.value) => { pr.value = src.value.clone(); pr.keys = src diff --git a/crates/engine/src/commands/edit.rs b/crates/engine/src/commands/edit.rs index bbdc43fe3..019d56d1d 100644 --- a/crates/engine/src/commands/edit.rs +++ b/crates/engine/src/commands/edit.rs @@ -115,6 +115,11 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { if !items.is_empty() { return super::prop_groups::duplicate_contents(s, &items); } + // Text animators or their selectors selected → duplicate them inside their text layers. + let items = super::prop_groups::selected_animator_items(s); + if !items.is_empty() { + return super::prop_groups::duplicate_contents(s, &items); + } } let (cid, ids) = layers_p(s, p)?; let new = s.edit("Duplicate", None, |proj, st| { @@ -328,7 +333,8 @@ fn paste_properties(s: &mut Session, p: &Value) -> Result { } let lt = l.layer_time(t); for (path, v) in &values { - if let Some(pr) = l.props.prop_mut(path) + let path = super::resolve_effect_path(l, path); + if let Some(pr) = l.props.prop_mut(&path) && let Some(v) = pr.value.coerce_json(&v.to_json()) { pr.set_value_at(lt, v); @@ -458,7 +464,8 @@ fn paste_links(s: &mut Session, clip: LinkClip) -> Result { for lid in &ids { let l = super::layer_mut(proj, cid, *lid)?; for (path, e) in &pairs { - if let Some(pr) = l.props.prop_mut(path) { + let path = super::resolve_effect_path(l, path); + if let Some(pr) = l.props.prop_mut(&path) { pr.expr = Some(e.clone()); n += 1; } @@ -496,6 +503,13 @@ fn paste(s: &mut Session, p: &Value) -> Result { if let Some(clip) = s.state.link_clipboard.clone() { return paste_links(s, clip); } + // A copied Mask Path / shape Path pasted onto Position becomes a motion path (#1425). + if let [(_, effectcraft_keyframe::Value::Path(path))] = s.state.value_clipboard.as_slice() + && let Some(target) = super::anim::path_target(s, p)? + { + let path = path.clone(); + return super::anim::paste_path(s, p, target, &path); + } if !s.state.value_clipboard.is_empty() { return paste_properties(s, p); } @@ -808,8 +822,30 @@ fn original_path(s: &Session) -> Option { } } +/// Whether Edit Original may hand `path` to the system's default app: a regular file (after +/// following links) whose name, and the name it links to, have a media extension the footage +/// importer takes. Anything else (a folder or app bundle, a script, a shortcut) is refused. +pub(crate) fn openable_original(path: &str) -> std::result::Result<(), String> { + let media = |p: &std::path::Path| { + let ext = p.extension().and_then(|e| e.to_str()).map(str::to_ascii_lowercase).unwrap_or_default(); + crate::media_browser::kind_of(&p.to_string_lossy()).is_some_and(|k| k != "project") || matches!(ext.as_str(), "pdf" | "ai" | "eps") + }; + let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| path.to_string()); + let real = std::fs::canonicalize(path).map_err(|e| format!("cannot open “{name}”: {e}"))?; + if !real.is_file() { + return Err(format!("“{name}” is not a file")); + } + if !media(std::path::Path::new(path)) || !media(&real) { + return Err(format!("Edit Original opens media files only; “{name}” is not one")); + } + Ok(()) +} + fn edit_original(s: &mut Session, _: &Value) -> Result { - let path = original_path(s).ok_or_else(|| super::bad("edit.editOriginal", "select a footage item or layer"))?; + let stored = original_path(s).ok_or_else(|| super::bad("edit.editOriginal", "select a footage item or layer"))?; + let path = s.stored_path(&stored).map_err(|why| super::bad("edit.editOriginal", crate::doc_path::refusal(&stored, why)))?; + let path = path.to_string_lossy(); + openable_original(&path).map_err(|e| super::bad("edit.editOriginal", e))?; let url = super::layer_menu::file_url(&path); s.events.push(crate::Event::OpenUrl(url.clone())); Ok(json!({"url": url})) @@ -952,20 +988,67 @@ fn always_ok(_: &Session) -> std::result::Result<(), String> { mod tests { use serde_json::json; + fn tmp(name: &str) -> std::path::PathBuf { + let d = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/test-out").join(name); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d + } + /// Edit Original opens the footage as a well-formed `file://` URL, whatever its name contains. #[test] fn edit_original_opens_an_encoded_file_url() { + let clip = tmp("edit-original-url").join("take #1.mov"); + std::fs::write(&clip, b"").unwrap(); + let clip = clip.to_string_lossy().into_owned(); + let url = super::super::layer_menu::file_url(&clip); + assert!(url.ends_with("/take%20%231.mov"), "{url}"); let mut s = crate::Session::default(); let id = s .edit("add", None, |p, _| { - let f = effectcraft_project::Footage { path: "/clips/take #1.mov".into(), ..Default::default() }; + let f = effectcraft_project::Footage { path: clip.clone(), ..Default::default() }; Ok(p.add_item("take", effectcraft_color::Label::Lavender, None, effectcraft_project::ItemKind::Footage(f))) }) .unwrap(); s.state.project_selection = vec![id]; s.drain_events(); let r = s.execute("edit.editOriginal", json!({})).unwrap(); - assert_eq!(r["url"], "file:///clips/take%20%231.mov"); - assert!(s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(u) if u == "file:///clips/take%20%231.mov"))); + assert_eq!(r["url"], url.as_str()); + assert!(s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(u) if *u == url))); + } + + /// Only regular media files reach the system opener. + #[test] + fn edit_original_opens_media_files_only() { + let d = tmp("edit-original-kinds"); + for f in ["clip.mov", "Still.PNG", "run.bat", "app.desktop", "clip.mov.lnk"] { + std::fs::write(d.join(f), b"").unwrap(); + } + for f in ["Tool.app", "folder.mov"] { + std::fs::create_dir_all(d.join(f)).unwrap(); + } + let ok = |f: &str| super::openable_original(&d.join(f).to_string_lossy()).is_ok(); + assert!(ok("clip.mov") && ok("Still.PNG")); + for f in ["run.bat", "app.desktop", "clip.mov.lnk", "Tool.app", "folder.mov", "missing.mov", ""] { + assert!(!ok(f), "{f}"); + } + #[cfg(unix)] + { + std::os::unix::fs::symlink(d.join("app.desktop"), d.join("link.png")).unwrap(); + assert!(!ok("link.png"), "a media name linking to another kind of file"); + } + // The command refuses without opening anything. + let mut s = crate::Session::default(); + let path = d.join("run.bat").to_string_lossy().into_owned(); + let id = s + .edit("add", None, |p, _| { + let f = effectcraft_project::Footage { path, ..Default::default() }; + Ok(p.add_item("bat", effectcraft_color::Label::Lavender, None, effectcraft_project::ItemKind::Footage(f))) + }) + .unwrap(); + s.state.project_selection = vec![id]; + s.drain_events(); + assert!(s.execute("edit.editOriginal", json!({})).is_err()); + assert!(!s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(_)))); } } diff --git a/crates/engine/src/commands/essential.rs b/crates/engine/src/commands/essential.rs index 7ffa02d8f..cb6702660 100644 --- a/crates/engine/src/commands/essential.rs +++ b/crates/engine/src/commands/essential.rs @@ -891,6 +891,22 @@ pub fn read_template(bytes: &[u8]) -> std::result::Result<(Value, Project, Vec<( Ok((manifest, project, entries)) } +/// Where a template's `media//` entry is written: `//`. +/// `None` unless `` is all digits and `` is a plain file name (not empty, `.` or +/// `..`, no separators, and on Windows no drive, stream or device name), so every entry lands +/// in an item folder directly inside `media_dir`. +fn template_media_path(media_dir: &std::path::Path, entry: &str) -> Option { + let (item, file) = entry.strip_prefix("media/")?.split_once('/')?; + if item.is_empty() || !item.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + if !super::scripts::is_bare_name(file) || (cfg!(windows) && super::scripts::windows_unsafe_name(file)) { + return None; + } + let out = media_dir.join(item).join(file); + (out.parent().and_then(std::path::Path::parent) == Some(media_dir) && out.file_name() == Some(std::ffi::OsStr::new(file))).then_some(out) +} + fn import_template(s: &mut Session, p: &Value) -> Result { const C: &str = "essential.importTemplate"; let path = str_p(p, "path").ok_or_else(|| bad(C, "missing `path` (.ectemplate)"))?.to_string(); @@ -905,8 +921,7 @@ fn import_template(s: &mut Session, p: &Value) -> Result { let media_dir = dir.join(format!("{stem} Media")); let mut written = std::collections::HashMap::new(); for (k, v) in entries.iter().filter(|(k, _)| k.starts_with("media/")) { - let rel = k.trim_start_matches("media/"); - let out = media_dir.join(rel); + let Some(out) = template_media_path(&media_dir, k) else { continue }; if let Some(parent) = out.parent() { let _ = std::fs::create_dir_all(parent); } diff --git a/crates/engine/src/commands/file.rs b/crates/engine/src/commands/file.rs index add3d327e..0ff09b446 100644 --- a/crates/engine/src/commands/file.rs +++ b/crates/engine/src/commands/file.rs @@ -18,7 +18,10 @@ fn new_project(s: &mut Session, _: &Value) -> Result { if s.prefs.project.use_template && !tpl.is_empty() { let bytes = s.services.read_file(&tpl).map_err(|e| EngineError::Other(format!("cannot read the new project template {tpl}: {e}")))?; let text = String::from_utf8(bytes).map_err(|_| EngineError::Other("the new project template is not a project file".into()))?; - s.replace_project(Project::from_json(&text)?, None); + let mut proj = Project::from_json(&text)?; + crate::doc_path::resolve_footage(&mut proj, Some(&tpl)); + s.replace_project(proj, None); + s.stored_paths.add(&s.project, Some(&tpl)); s.update_sentinel(); return Ok(json!({"template": tpl})); } @@ -49,9 +52,13 @@ pub(crate) fn open(s: &mut Session, p: &Value) -> Result { let text = if crate::xml_project::is_xml(&text) { crate::xml_project::from_xml(&text).map_err(EngineError::Other)? } else { text }; let (mut proj, repaired) = Project::from_json_repaired(&text).map_err(|e| EngineError::Other(format!("{path} is not a project EffectCraft can open: {e}")))?; - resolve_relative_footage(&mut proj, path, s.services.as_ref()); - let followed = crate::media_anchor::follow(&mut proj, &effectcraft_project::saved_anchor(&text), path, s.services.as_ref()); // ours + let followed = crate::media_anchor::follow(&mut proj, &effectcraft_project::saved_anchor(&text), path, s.services.as_ref()); // ours: before resolve_footage, which rewrites the stored paths the anchor is keyed by + // Footage saved with a relative path (imported by the CLI or an agent as `clip.mp4`, or + // written by hand) is found next to the project file, as After Effects looks for footage + // relative to the project (#652). + let refused = crate::doc_path::resolve_footage(&mut proj, Some(path)); s.replace_project(proj, Some(path.to_string())); + s.stored_paths.add(&s.project, Some(path)); s.restore_view(&effectcraft_project::saved_view(&text)); crate::media_anchor::report(s, followed); // ours s.note_project_path(path); @@ -62,6 +69,10 @@ pub(crate) fn open(s: &mut Session, p: &Value) -> Result { let more = if more > 0 { format!(" and {more} more") } else { String::new() }; s.toast(format!("This project had values outside EffectCraft's limits, corrected when it opened: {shown}{more}.")); } + if let Some((_, first, why)) = refused.first() { + let n = refused.len(); + s.toast(format!("{n} footage item{} marked missing: {}.", if n == 1 { "" } else { "s" }, crate::doc_path::refusal(first, *why))); + } // Written by a newer EffectCraft: what this version doesn't know would be lost on saving. let saved_by = effectcraft_project::saved_by(&text); if let Some(v) = saved_by.as_deref().filter(|v| effectcraft_project::is_newer_version(v, effectcraft_project::APP_VERSION)) { @@ -78,28 +89,6 @@ pub(crate) fn open(s: &mut Session, p: &Value) -> Result { Ok(json!({"path": path, "savedBy": saved_by, "repaired": repaired})) } -/// Footage saved with a relative path (imported by the CLI or an agent as `clip.mp4`, or written -/// by hand) is found next to the project file, as After Effects looks for footage relative to -/// the project. It used to be looked up from the working directory: the CLI started in the -/// project's folder found it, the desktop app (started from the Finder, Explorer or a launcher, -/// in another folder) showed and rendered nothing (#652). A path that exists from the working -/// directory is kept. -fn resolve_relative_footage(p: &mut Project, project_path: &str, services: &dyn crate::Services) { - use std::path::Path; - let Some(dir) = Path::new(project_path).parent().filter(|d| !d.as_os_str().is_empty()) else { return }; - let beside = |f: &str| dir.join(f).to_string_lossy().to_string(); - for item in p.items.values_mut() { - let ItemKind::Footage(f) = &mut item.kind else { continue }; - if f.path.is_empty() || !Path::new(&f.path).is_relative() || services.exists(&f.path) || !services.exists(&beside(&f.path)) { - continue; - } - f.path = beside(&f.path); - for frame in f.sequence.iter_mut().filter(|q| Path::new(q.as_str()).is_relative()) { - *frame = beside(frame); - } - } -} - /// The project file's text for `path`: `.ecproj` JSON, or XML for `.ecprojx` / `.xml` (a /// project opened from an XML copy saves as XML again). Fails, writing nothing, when the project /// wouldn't read back. @@ -434,6 +423,13 @@ fn prepare_import(s: &mut Session, p: &Value) -> Result { } } paths = rest; + // Composition imports already made their sources, so they bypass add_footage's + // probed-footage refresh. Refresh them before any background probe job starts (#1070). + for id in &items { + if let Some(ItemKind::Footage(f)) = s.project.item(ItemId(*id)).map(|it| &it.kind) { + s.forget_decoded(f); + } + } } // Numbered stills as image sequences (the Import dialog's " Sequence" and Force // Alphabetical Order), at `frameRate` or Settings ▸ Import ▸ Sequence Footage. @@ -478,6 +474,7 @@ fn add_footage(s: &mut Session, pending: PendingImport, probed: Vec Result { // Replacing a missing file relinks the other missing files that moved with it (as After Effects // does when you locate one). let moved = match s.project.item(item).map(|it| &it.kind) { - Some(ItemKind::Footage(previous)) if previous.missing || !s.services.exists(&previous.path) => { + Some(ItemKind::Footage(previous)) if previous.missing || !s.stored_paths.exists(s.services.as_ref(), &previous.path) => { super::relink_moved::moved_with(s, item, &previous.path, &path) } _ => vec![], }; let relinked: Vec = moved.iter().map(|(id, _)| id.0).collect(); + for nf in std::iter::once(&f).chain(moved.iter().map(|(_, nf)| nf)) { + s.stored_paths.allow(nf); + } s.edit("Replace Footage", None, |proj, _| { let it = proj.item_mut(item).ok_or_else(|| bad("file.replaceFootage", "no such item"))?; it.name = name; @@ -948,7 +951,7 @@ fn reveal_in_finder(s: &mut Session, p: &Value) -> Result { Some(ItemKind::Footage(f)) if !f.path.is_empty() => f.path.clone(), _ => return Err(bad("file.revealInFinder", "the item is not a file")), }; - let url = super::layer_menu::folder_url(&path); + let url = super::layer_menu::reveal_url(s, &path).map_err(|e| bad("file.revealInFinder", e))?; s.events.push(crate::Event::OpenUrl(url.clone())); Ok(json!({"url": url})) } diff --git a/crates/engine/src/commands/frame_export.rs b/crates/engine/src/commands/frame_export.rs index c939d67e1..baad9b6ef 100644 --- a/crates/engine/src/commands/frame_export.rs +++ b/crates/engine/src/commands/frame_export.rs @@ -140,9 +140,10 @@ fn srgb_to_linear(c: f32) -> f32 { if c <= 0.04045 { c / 12.92 } else { ((c + 0.055) / 1.055).powf(2.4) } } -/// A unique EXR layer name (no dots: they separate the layer from the channel). +/// A unique EXR layer name (no dots: they separate the layer from the channel). Printable ASCII only: +/// the `exr` crate panics on characters it cannot store in a channel name (#1702). fn exr_name(name: &str, used: &mut Vec) -> String { - let base: String = name.chars().map(|c| if c == '.' || c.is_control() { '_' } else { c }).collect(); + let base: String = name.chars().map(|c| if c == '.' || !(c == ' ' || c.is_ascii_graphic()) { '_' } else { c }).collect(); let base = if base.trim().is_empty() { "Layer".to_string() } else { base }; let mut n = base.clone(); let mut k = 2; diff --git a/crates/engine/src/commands/import_project.rs b/crates/engine/src/commands/import_project.rs index f0474ee82..af37771a8 100644 --- a/crates/engine/src/commands/import_project.rs +++ b/crates/engine/src/commands/import_project.rs @@ -27,40 +27,24 @@ fn read_project(s: &Session, path: &str) -> Result<(Project, Option)> { Ok((project, effectcraft_project::saved_by(&text))) } -/// Footage paths relative to the imported project's folder become absolute when the file is -/// there; returns the footage paths that are missing. -fn resolve_footage(s: &Session, project: &mut Project, dir: &std::path::Path) -> Vec { +/// Footage paths stored in the imported project are resolved against its file +/// ([`crate::doc_path::resolve_footage`]): relative ones become paths beside it, and items whose +/// path is not followed are marked missing. Returns the footage paths that are missing, as stored. +fn resolve_footage(s: &Session, project: &mut Project, path: &str) -> Vec { + let stored: std::collections::HashMap = + project.items.values().filter_map(|i| if let ItemKind::Footage(f) = &i.kind { Some((i.id, f.path.clone())) } else { None }).collect(); + let refused = crate::doc_path::resolve_footage(project, Some(path)); let services = s.services.clone(); - let resolve = |raw: &mut String| -> bool { - if raw.is_empty() || services.exists(raw) { - return true; - } - if std::path::Path::new(raw.as_str()).is_absolute() { - return false; - } - let joined = dir.join(raw.as_str()).to_string_lossy().to_string(); - if services.exists(&joined) { - *raw = joined; - true - } else { - false - } - }; - let mut missing = vec![]; - for it in project.items.values_mut() { - if let ItemKind::Footage(f) = &mut it.kind { - if f.sequence.is_empty() { - if !resolve(&mut f.path) { - missing.push(f.path.clone()); - } - } else { - let mut all = true; - for frame in &mut f.sequence { - all &= resolve(frame); - } - if !all { - missing.push(f.path.clone()); - } + let exists = |p: &String| p.is_empty() || services.exists(p); + let mut missing: Vec = refused.iter().map(|(_, p, _)| p.clone()).collect(); + for it in project.items.values() { + if let ItemKind::Footage(f) = &it.kind { + if refused.iter().any(|(id, _, _)| *id == it.id) { + continue; + } + let present = if f.sequence.is_empty() { exists(&f.path) } else { f.sequence.iter().all(exists) }; + if !present { + missing.push(stored.get(&it.id).cloned().unwrap_or_else(|| f.path.clone())); } } } @@ -79,8 +63,8 @@ fn import_project(s: &mut Session, p: &Value) -> Result { let (mut sub, saved_by) = read_project(s, &path)?; crate::upgrade_effects(&mut sub); let file = std::path::Path::new(&path); - let dir = file.parent().map(|d| d.to_path_buf()).unwrap_or_default(); - let missing = resolve_footage(s, &mut sub, &dir); + let missing = resolve_footage(s, &mut sub, &path); + s.stored_paths.add(&sub, Some(&path)); // The imported project's own render queue stays behind (as for templates). let base = s.project.next_id; if sub.next_id.checked_add(base).is_none() { diff --git a/crates/engine/src/commands/layer_menu.rs b/crates/engine/src/commands/layer_menu.rs index 83a1510ac..9cac80412 100644 --- a/crates/engine/src/commands/layer_menu.rs +++ b/crates/engine/src/commands/layer_menu.rs @@ -677,6 +677,54 @@ pub(crate) fn folder_url(path: &str) -> String { file_url(&dir) } +/// Extensions of folders that macOS opens as an app, plug-in or other bundle rather than showing +/// their contents. +const BUNDLE_EXTENSIONS: &[&str] = &[ + "app", + "appex", + "bundle", + "component", + "dext", + "driver", + "framework", + "kext", + "mdimporter", + "mpkg", + "osax", + "pkg", + "plugin", + "prefpane", + "qlgenerator", + "saver", + "service", + "systemextension", + "vst", + "vst3", + "wdgt", + "workflow", + "xpc", +]; + +/// Whether the system file manager would open folder `dir` as something other than a folder: a +/// bundle (`Tool.app`), or a Windows folder with a `{CLSID}` extension. +fn opens_as_bundle(dir: &std::path::Path) -> bool { + let ext = dir.extension().and_then(|e| e.to_str()).unwrap_or_default().to_ascii_lowercase(); + ext.starts_with('{') || BUNDLE_EXTENSIONS.contains(&ext.as_str()) +} + +/// The folder of the footage file `stored` (a path stored in the project) as a `file://` URL for +/// Reveal in Finder. A path that is not followed ([`crate::doc_path`]), or a folder that is not a +/// plain folder after following links, is refused. +pub(crate) fn reveal_url(s: &Session, stored: &str) -> std::result::Result { + let path = s.stored_path(stored).map_err(|why| crate::doc_path::refusal(stored, why))?; + let dir = path.parent().filter(|d| !d.as_os_str().is_empty()).ok_or_else(|| format!("{stored} is not in a folder"))?; + let real = std::fs::canonicalize(dir).map_err(|e| format!("cannot show the folder of {stored}: {e}"))?; + if !real.is_dir() || opens_as_bundle(dir) || opens_as_bundle(&real) { + return Err(format!("the folder of {stored} is not a plain folder")); + } + Ok(file_url(&dir.to_string_lossy())) +} + /// `path` as a `file://` URL: backslashes become `/`, a Windows drive path gets the third slash /// (`file:///C:/…`), and every byte that isn't a plain path character is percent-encoded (spaces, /// `#`, `%`, `?`, non-ASCII as UTF-8), so the URL names exactly this file. @@ -700,7 +748,7 @@ fn reveal_in_finder(s: &mut Session, p: &Value) -> Result { Some(effectcraft_project::ItemKind::Footage(f)) if !f.path.is_empty() => f.path.clone(), _ => return Err(bad("layer.revealInFinder", "the layer's source is not a file")), }; - let url = folder_url(&path); + let url = reveal_url(s, &path).map_err(|e| bad("layer.revealInFinder", e))?; s.events.push(crate::Event::OpenUrl(url.clone())); Ok(json!({"url": url})) } @@ -892,4 +940,27 @@ mod tests { fn folder_url_names_the_containing_folder() { assert_eq!(folder_url("/media/My Clips/a.mov"), "file:///media/My%20Clips"); } + + /// Reveal in Finder shows plain folders only: a bundle the system would open as an app, or a + /// link to one, is refused. + #[test] + fn reveal_refuses_folders_that_open_as_apps() { + let d = std::env::temp_dir().join(format!("ec-reveal-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + for f in ["Clips", "Tool.app", "Shell.{20D04FE0-3AEA-1069-A2D8-08002B30309D}"] { + std::fs::create_dir_all(d.join(f)).unwrap(); + } + let s = crate::Session::default(); + let reveal = |folder: &str| super::reveal_url(&s, &d.join(folder).join("a.mov").to_string_lossy()); + assert_eq!(reveal("Clips"), Ok(super::file_url(&d.join("Clips").to_string_lossy()))); + assert!(reveal("Tool.app").is_err()); + assert!(reveal("Shell.{20D04FE0-3AEA-1069-A2D8-08002B30309D}").is_err()); + assert!(reveal("Gone").is_err()); + #[cfg(unix)] + { + std::os::unix::fs::symlink(d.join("Tool.app"), d.join("Footage")).unwrap(); + assert!(reveal("Footage").is_err(), "a link to a bundle"); + } + let _ = std::fs::remove_dir_all(&d); + } } diff --git a/crates/engine/src/commands/mod.rs b/crates/engine/src/commands/mod.rs index 43340fb07..a5bae24b8 100644 --- a/crates/engine/src/commands/mod.rs +++ b/crates/engine/src/commands/mod.rs @@ -444,6 +444,24 @@ pub fn match_path_of(g: &effectcraft_project::PropGroup, uid: effectcraft_projec None } +/// Turn `effects/[#n]/` (as [`match_path_of`] writes it) into a uid path on +/// `l`. Path parsing splits on `.`, which would cut a match name like `ec.control.slider` apart. +/// Any other path, or an effect that is not on the layer, comes back unchanged. +pub(crate) fn resolve_effect_path(l: &Layer, path: &str) -> String { + let Some((seg, tail)) = path.strip_prefix("effects/").and_then(|r| r.split_once('/')) else { return path.to_string() }; + let (match_id, nth) = match seg.rsplit_once('#') { + Some((m, n)) if !m.is_empty() => match n.parse::() { + Ok(n) if n >= 1 => (m, n), + _ => (seg, 1), + }, + _ => (seg, 1), + }; + match l.effects().and_then(|fx| fx.groups().filter(|g| g.match_id == match_id).nth(nth - 1)) { + Some(g) => format!("effects/@{}/{tail}", g.uid), + None => path.to_string(), + } +} + /// Selected properties (leaves) of the active comp as (layer, uid); selected groups expand to /// their properties. pub(crate) fn selected_leaf_props(s: &Session) -> Vec<(LayerId, effectcraft_project::Uid)> { diff --git a/crates/engine/src/commands/paths.rs b/crates/engine/src/commands/paths.rs index c74c3bb5e..96767d967 100644 --- a/crates/engine/src/commands/paths.rs +++ b/crates/engine/src/commands/paths.rs @@ -108,13 +108,89 @@ fn roto_bezier(s: &mut Session, p: &Value) -> Result { Ok(json!({"rotoBezier": on})) } +/// The parametric shape paths that Convert To Bezier Path replaces with an editable path. +const PARAMETRIC: [&str; 3] = ["rect", "ellipse", "star"]; + +/// The path a parametric shape group draws, in the group's own space, read at `lt`. This is the +/// geometry the renderer builds from the same values (`render::shapes::collect`), so the converted +/// path is the shape that was on screen rather than an approximation of it; the defaults match +/// that code for a property a group doesn't carry (a polygon has no inner radius or roundness). +fn parametric_path(g: &PropGroup, lt: effectcraft_time::Tick) -> Option { + let f = |id: &str, d: f64| g.prop(id).map_or(d, |pr| pr.value_at(lt).as_f64()); + let v2 = |id: &str, d: [f64; 2]| g.prop(id).map_or(d, |pr| pr.value_at(lt).as_vec2()); + let path = match g.match_id.as_str() { + "rect" => effectcraft_path::rect(v2("size", [100.0; 2]), v2("position", [0.0; 2]), f("roundness", 0.0)), + "ellipse" => effectcraft_path::ellipse(v2("size", [100.0; 2]), v2("position", [0.0; 2])), + "star" => effectcraft_path::polystar( + g.prop("type").map_or(0, |pr| pr.value_at(lt).as_enum()) == 0, + f("points", 5.0), + v2("position", [0.0; 2]), + f("rotation", 0.0), + f("innerRadius", 50.0), + f("outerRadius", 100.0), + f("innerRoundness", 0.0), + f("outerRoundness", 0.0), + ), + _ => return None, + }; + // A closed parametric shape is one subpath; anything else would not be this shape. + effectcraft_path::from_kurbo(&path).into_iter().next() +} + fn convert_to_bezier(s: &mut Session, p: &Value) -> Result { - edit_mask_groups(s, p, "Convert To Bezier Path", |g| { - if let GroupKind::Mask { roto_bezier, .. } = &mut g.kind { - *roto_bezier = false; + // On a shape layer this replaces a Rectangle, Ellipse or Polystar path with an editable + // Path 1 of the same geometry, as After Effects does (#932). The parametric properties are + // read at the current time and the path that comes out is static: a size or roundness + // animation cannot survive the conversion, in After Effects either. + let (cid, items) = selected_shape_items(s, p)?; + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let shapes: Vec<(LayerId, Uid)> = items + .into_iter() + .filter(|(l, u)| { + comp.layer(*l).and_then(|l| l.props.sub("contents")).and_then(|c| c.find_group(*u)).is_some_and(|g| PARAMETRIC.contains(&g.match_id.as_str())) + }) + .collect(); + if shapes.is_empty() { + return edit_mask_groups(s, p, "Convert To Bezier Path", |g| { + if let GroupKind::Mask { roto_bezier, .. } = &mut g.kind { + *roto_bezier = false; + } + Ok(()) + }); + } + let t = s.time(); + let n = shapes.len(); + let made = s.edit("Convert To Bezier Path", None, |proj, st| { + let mut next = proj.next_id; + let mut made = Vec::new(); + for (lid, uid) in &shapes { + let l = layer_mut(proj, cid, *lid)?; + let lt = l.layer_time(t); + let contents = l.props.sub_mut("contents").ok_or_else(|| bad("path.convertToBezier", "not a shape layer"))?; + let Some(old) = contents.find_group(*uid) else { continue }; + let Some(path) = parametric_path(old, lt) else { continue }; + // Path Direction is the shape's own, not a default: reversing is a separate command. + let direction = old.prop("direction").map_or(0, |pr| pr.value_at(lt).as_enum()); + let enabled = old.enabled; + let mut g = build::shape_path(&mut Ids(&mut next), path); + if let Some(pr) = g.prop_mut("direction") { + pr.value = KV::Enum(direction); + } + g.enabled = enabled; + let new_uid = g.uid; + let parent = contents.parent_of_mut(*uid).ok_or_else(|| bad("path.convertToBezier", "no such shape item"))?; + let Some(at) = parent.children.iter().position(|c| c.uid() == *uid) else { continue }; + parent.children[at] = Node::Group(g); + made.push((*lid, new_uid)); } - Ok(()) - }) + proj.next_id = next; + // The new paths are what is selected, so the vertices can be edited straight away. + if !made.is_empty() { + st.selected_props = made.clone(); + } + Ok(made) + })?; + Ok(json!({"paths": made.len(), "of": n})) } // ---------------------------------------------------------------- mask options diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index 29da9c1e0..3af877d34 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -44,7 +44,8 @@ fn with_prop( ) -> Result { let t = s.time_of(cid); let linear = s.prefs.general.default_spatial_linear; - s.edit(label, merge, |proj, _| { + s.edit(label, merge, |proj, state| { + let mut next = proj.next_id; let l = layer_mut(proj, cid, lid)?; let lt = l.layer_time(t); let pr = l.props.find_mut(uid).ok_or_else(|| bad("prop", "property vanished"))?; @@ -59,10 +60,61 @@ fn with_prop( k.spatial_out = [0.0; 3]; } } + if pr.match_id == "type" { + let removed = sync_polystar_type(l, uid, lt, &mut next)?; + if !removed.is_empty() { + state.selected_props.retain(|(layer, prop)| *layer != lid || !removed.contains(prop)); + state.selected_keys.retain(|key| key.layer != lid || !removed.contains(&key.prop)); + } + } + proj.next_id = next; Ok(r) }) } +/// A static Polygon has no inner controls; switching to Star creates them in schema order +/// with AE's switch defaults (#1685). Animated/expression-driven types need the controls at +/// every time, including when the current value is Polygon. +fn sync_polystar_type(layer: &mut Layer, uid: Uid, time: Tick, next: &mut Uid) -> Result> { + if !matches!(layer.source, effectcraft_project::LayerSource::Shape) { + return Ok(vec![]); + } + let Some(group) = + layer.props.parent_of_mut(uid).filter(|g| g.match_id == "star" && matches!(g.kind, GroupKind::Indexed) && g.get("type").is_some_and(|p| p.uid == uid)) + else { + return Ok(vec![]); + }; + let Some(kind) = group.get("type") else { return Ok(vec![]) }; + let needs_inner = kind.is_animated() || kind.has_expression() || kind.value_at(time) == effectcraft_keyframe::Value::Enum(0); + if !needs_inner { + let mut removed = vec![]; + group.children.retain(|node| { + if let effectcraft_project::Node::Prop(p) = node + && matches!(p.match_id.as_str(), "innerRadius" | "innerRoundness") + { + removed.push(p.uid); + return false; + } + true + }); + return Ok(removed); + } + for (inner, name, outer, value, ui) in [ + ("innerRadius", "Inner Radius", "outerRadius", 50.0, effectcraft_project::ParamUi::Pixels), + ("innerRoundness", "Inner Roundness", "outerRoundness", 0.0, effectcraft_project::ParamUi::Percent), + ] { + if group.get(inner).is_none() { + let uid = *next; + *next = next.checked_add(1).ok_or_else(|| bad("prop", "property ids exhausted"))?; + let prop = Property::new(uid, inner, name, effectcraft_keyframe::Value::Scalar(value)).with_ui(ui); + let index = + group.children.iter().position(|n| matches!(n, effectcraft_project::Node::Prop(p) if p.match_id == outer)).unwrap_or(group.children.len()); + group.children.insert(index, effectcraft_project::Node::Prop(prop)); + } + } + Ok(vec![]) +} + /// Explicit command key times default to layer time; readers and the CTI use comp time. #[derive(Clone, Copy)] pub(crate) enum KeyTimeBase { @@ -241,7 +293,9 @@ pub(crate) fn proportional_share(index: usize, count: usize) -> f64 { } /// `prop.set`; with `selected`, also on the other selected layers: the same value, or with -/// `offset` (a scrub) the same change from each layer's own value. With `offset` and +/// `offset` (a scrub) the same change from each layer's own value. A typed value with `dims` +/// (the components edited, e.g. `[2]` for Z) sets only those components on the other layers, +/// which keep the rest of their own value (#1434). With `offset` and /// `proportional` (Proportional Scrubbing) each layer takes [`proportional_share`] of the change /// by its place in the selection order, the scrubbed layer too. One undo step (with `merge`, /// one that a continuing scrub keeps folding into). @@ -261,6 +315,23 @@ fn set_selected(s: &mut Session, p: &Value) -> Result { .then(|| s.state.selected_layers.iter().copied().filter(|o| *o == lid || twins.iter().any(|(t, _)| t == o)).collect()); let share = |l: LayerId| order.as_ref().map_or(1.0, |o| o.iter().position(|x| *x == l).map_or(1.0, |i| proportional_share(i, o.len()))); let scalar = p.get("value").is_some_and(Value::is_number); + // Typed into some components only: the others stay each layer's own. + let dims: Option> = match (&delta, p.get("dims").and_then(Value::as_array), &new) { + (None, Some(a), Some(_)) if !scalar => Some(a.iter().filter_map(Value::as_u64).filter_map(|d| usize::try_from(d).ok()).collect()), + _ => None, + }; + let typed = |s: &Session, q: &mut Value, olid: LayerId, ouid: Uid| { + if let (Some(dims), Some(n)) = (&dims, &new) + && let Some(mut c) = current_components(s, cid, olid, ouid) + { + for &d in dims { + if let (Some(slot), Some(v)) = (c.get_mut(d), n.get(d)) { + *slot = *v; + } + } + q["value"] = json!(c); + } + }; // `p` aimed at (layer, prop) with its value moved by `k` × the change. let shifted = |s: &Session, q: &mut Value, olid: LayerId, ouid: Uid, k: f64| { if let Some(d) = &delta @@ -291,6 +362,7 @@ fn set_selected(s: &mut Session, p: &Value) -> Result { } let mut q = twin_params(p, olid, ouid); shifted(s, &mut q, olid, ouid, k); + typed(s, &mut q, olid, ouid); // (a layer that can't take it, e.g. a clamped or static property, keeps its value) let _ = set(s, &q); } @@ -967,6 +1039,7 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { let sel = s.state.selected_keys.clone(); s.edit("Delete Keyframes", None, |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; + let mut removed = 0usize; for k in &sel { if let Some(pr) = comp.layer_mut(k.layer).and_then(|l| l.props.find_mut(k.prop)) && let Some(i) = key_at(&pr.keys, k.time) @@ -975,10 +1048,13 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { pr.value = pr.keys[0].value.clone(); } pr.keys.remove(i); + removed += 1; } } st.selected_keys.clear(); - Ok(json!(sel.len())) + // The keys actually removed: a selected key that is gone (or listed twice) isn't counted, + // so a caller can tell that a delete changed nothing. + Ok(json!(removed)) }) } @@ -1229,7 +1305,7 @@ pub fn specs() -> Vec { "Set Property Value", [], None, - "{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?, selected? (also the other selected layers), offset? (them by the same change), proportional? (with offset: Proportional Scrubbing, each layer by its share in selection order, the first none, the last all)}", + "{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?, selected? (also the other selected layers), offset? (them by the same change), proportional? (with offset: Proportional Scrubbing, each layer by its share in selection order, the first none, the last all), dims? (with selected, a typed value: the components edited, e.g. [2] for Z; the other layers keep the rest of their own value)}", has_layers, set_selected ), diff --git a/crates/engine/src/commands/prop_groups.rs b/crates/engine/src/commands/prop_groups.rs index e4d80b16d..6f7d74aef 100644 --- a/crates/engine/src/commands/prop_groups.rs +++ b/crates/engine/src/commands/prop_groups.rs @@ -100,6 +100,22 @@ pub(crate) fn selected_contents(s: &Session) -> Vec<(LayerId, Uid)> { out } +/// Text animators and their selectors selected on the active comp's text layers, top to bottom. +/// A selector inside a selected animator isn't listed, since it is copied with it. +pub(crate) fn selected_animator_items(s: &Session) -> Vec<(LayerId, Uid)> { + let mut out = vec![]; + let Some(c) = s.active_comp().filter(|_| !s.state.selected_props.is_empty()) else { return out }; + for l in c.layers.iter().filter(|l| matches!(l.source, LayerSource::Text)) { + if let Some(animators) = l.props.group("text/animators") { + let selected = |u: Uid| s.state.selected_props.contains(&(l.id, u)); + let mut uids = vec![]; + collect_selected(animators, &selected, &mut uids, 0); + out.extend(uids.into_iter().map(|u| (l.id, u))); + } + } + out +} + fn collect_selected(g: &PropGroup, selected: &dyn Fn(Uid) -> bool, out: &mut Vec, depth: usize) { if depth > super::shape_stroke::MAX_DEPTH { return; @@ -127,8 +143,9 @@ pub(crate) fn cut_contents(s: &mut Session, items: &[(LayerId, Uid)]) -> Result< } /// Edit ▸ Paste with shape items on the clipboard: copies go into each selected shape layer's -/// Contents, above its topmost selected item (in that item's group), else on top. Names stay -/// unique in their group ("Rectangle 2"…), and the copies are selected. +/// Contents, above its topmost selected item (in that item's group), else on top (paints and path +/// operations alone: at the bottom). Names stay unique in their group ("Rectangle 2"…), and the +/// copies are selected. pub(crate) fn paste_contents(s: &mut Session, p: &Value) -> Result { let c = "edit.paste"; let clip = s.state.contents_clipboard.clone(); @@ -140,6 +157,9 @@ pub(crate) fn paste_contents(s: &mut Session, p: &Value) -> Result { return Err(bad(c, "select a shape layer to paste shape items into")); } let sel = selected_contents(s); + let acts_on_above = clip.iter().all(|g| !matches!(g.match_id.as_str(), "group" | "rect" | "ellipse" | "star" | "path")); + // Merge Paths clears the draws above it, so at the bottom it would erase the paints (#1681). + let has_merge = clip.iter().any(|g| g.match_id == "merge"); let label = match clip.as_slice() { [g] => format!("Paste {}", g.name), _ => "Paste".to_string(), @@ -154,11 +174,19 @@ pub(crate) fn paste_contents(s: &mut Session, p: &Value) -> Result { let parent = l.props.parent_of(*u)?; Some((parent.uid, parent.children.iter().position(|n| n.uid() == *u)?)) }); + let unselected = spot.is_none(); let (parent, at) = match spot { Some((pu, i)) => (l.props.find_group_mut(pu), i), None => (l.props.sub_mut("contents"), 0), }; let parent = parent.ok_or_else(|| bad(c, "the layer has no contents"))?; + // Trim Paths, Wiggle Paths, fills… act on the items above them (as Add places them), + // so with nothing selected they go to the bottom: on top they'd affect nothing (#1681). + let at = match unselected { + true if has_merge => merge_spot(parent), + true if acts_on_above => parent.children.len(), + _ => at, + }; for (k, g) in clip.iter().enumerate() { let mut g = g.clone(); g.reassign_uids(&mut next); @@ -174,8 +202,21 @@ pub(crate) fn paste_contents(s: &mut Session, p: &Value) -> Result { Ok(json!({"contents": pasted.iter().map(|(_, u)| *u).collect::>()})) } -/// Edit ▸ Duplicate with shape items selected: each is copied in place, above the original and -/// named as After Effects does ("Rectangle 1" → "Rectangle 2"), and the copies are selected. +/// Where Merge Paths goes in `contents` when nothing is selected: before the first paint, so the +/// paints still draw the merged path (the renderer drops the draws above a Merge Paths), else after +/// the last path item. +fn merge_spot(contents: &PropGroup) -> usize { + let is = |n: &Node, ids: &[&str]| matches!(n, Node::Group(g) if ids.contains(&g.match_id.as_str())); + let paints = ["fill", "stroke", "gfill", "gstroke"]; + match contents.children.iter().position(|n| is(n, &paints)) { + Some(i) => i, + None => contents.children.iter().rposition(|n| is(n, &["rect", "ellipse", "star", "path"])).map_or(0, |i| i + 1), + } +} + +/// Edit ▸ Duplicate with shape items, text animators or selectors selected: each is copied in +/// place, above the original and named as After Effects does ("Rectangle 1" → "Rectangle 2"), +/// and the copies are selected. pub(crate) fn duplicate_contents(s: &mut Session, items: &[(LayerId, Uid)]) -> Result { let c = "edit.duplicate"; let cid = s.active_comp_id().ok_or(EngineError::NoComp)?; @@ -183,7 +224,7 @@ pub(crate) fn duplicate_contents(s: &mut Session, items: &[(LayerId, Uid)]) -> R let mut next = proj.next_id; let mut out = vec![]; for (lid, uid) in items { - let parent = layer_mut(proj, cid, *lid)?.props.parent_of_mut(*uid).ok_or_else(|| bad(c, "the shape item is gone"))?; + let parent = layer_mut(proj, cid, *lid)?.props.parent_of_mut(*uid).ok_or_else(|| bad(c, "the group is gone"))?; let Some((i, Node::Group(g))) = parent.children.iter().enumerate().find(|(_, n)| n.uid() == *uid) else { continue }; let mut g = g.clone(); g.reassign_uids(&mut next); diff --git a/crates/engine/src/commands/relink.rs b/crates/engine/src/commands/relink.rs index 1c095ddac..2d7c2ea13 100644 --- a/crates/engine/src/commands/relink.rs +++ b/crates/engine/src/commands/relink.rs @@ -81,12 +81,13 @@ fn run(s: &mut Session, p: &Value) -> Result { } let importer = s.importer.clone().ok_or_else(|| bad(COMMAND, "media import is not available in this build"))?; let services = s.services.clone(); + let stored = s.stored_paths.clone(); let missing: Vec<(ItemId, Footage)> = s .project .items .values() .filter_map(|item| match &item.kind { - ItemKind::Footage(f) if !f.path.is_empty() && !services.exists(&f.path) => Some((item.id, f.clone())), + ItemKind::Footage(f) if !f.path.is_empty() && !stored.exists(services.as_ref(), &f.path) => Some((item.id, f.clone())), _ => None, }) .collect(); @@ -149,7 +150,8 @@ fn run(s: &mut Session, p: &Value) -> Result { // A background search must not overwrite edits made while it was running. for (id, old, new) in &updates { if s.project.item(*id).is_none_or(|i| !matches!(&i.kind, ItemKind::Footage(f) if f == old)) - || services.exists(&old.path) || !services.exists(&new.path) + || stored.exists(services.as_ref(), &old.path) + || !services.exists(&new.path) || (new.kind == FootageKind::Sequence && new.sequence.iter().any(|p| !services.exists(p))) { return Err(bad(COMMAND, "footage changed during search; run the search again")); @@ -157,6 +159,9 @@ fn run(s: &mut Session, p: &Value) -> Result { } let matched = updates.len(); if !dry_run && !updates.is_empty() { + for (_, _, new) in &updates { + s.stored_paths.allow(new); + } s.edit("Relink Missing Footage", None, |project, _| { for (id, _, new) in updates { if let Some(item) = project.item_mut(id) { diff --git a/crates/engine/src/commands/relink_moved.rs b/crates/engine/src/commands/relink_moved.rs index ef64a4235..ecc531bc2 100644 --- a/crates/engine/src/commands/relink_moved.rs +++ b/crates/engine/src/commands/relink_moved.rs @@ -91,8 +91,8 @@ fn moved_footage(footage: &Footage, f: impl Fn(&str) -> Option) -> Optio Some(nf) } -fn present(s: &dyn Services, f: &Footage) -> bool { - !f.path.is_empty() && s.exists(&f.path) +fn present(s: &dyn Services, stored: &crate::doc_path::StoredPaths, f: &Footage) -> bool { + stored.exists(s, &f.path) } /// Footage items whose file is gone (flagged by the footage check, or not there now). @@ -101,7 +101,7 @@ fn missing_items(s: &Session) -> Vec<(ItemId, Footage)> { .items .values() .filter_map(|i| match &i.kind { - ItemKind::Footage(f) if f.kind != FootageKind::Data && !f.path.is_empty() && (f.missing || !present(s.services.as_ref(), f)) => { + ItemKind::Footage(f) if f.kind != FootageKind::Data && !f.path.is_empty() && (f.missing || !present(s.services.as_ref(), &s.stored_paths, f)) => { Some((i.id, f.clone())) } _ => None, @@ -146,6 +146,9 @@ fn apply(s: &mut Session, label: &str, found: Vec<(ItemId, Footage)>) -> Result< return Ok(json!({"relinked": [], "stillMissing": missing_items(s).len()})); } let ids: Vec = found.iter().map(|(id, _)| id.0).collect(); + for (_, nf) in &found { + s.stored_paths.allow(nf); + } s.edit(label, None, |proj: &mut Project, _| { for (id, nf) in found { if let Some(it) = proj.item_mut(id) { @@ -175,7 +178,7 @@ pub(crate) fn moved_with(s: &Session, item: ItemId, old_path: &str, new_path: &s if id == item { continue; } - let same_place = moved_footage(&f, |q| remap(q, &old_root, &new_root)).filter(|nf| present(services.as_ref(), nf)); + let same_place = moved_footage(&f, |q| remap(q, &old_root, &new_root)).filter(|nf| present(services.as_ref(), &s.stored_paths, nf)); let same_folder = || { let dir = new_dir.as_deref()?; let dir_parts = parts(dir); @@ -183,7 +186,7 @@ pub(crate) fn moved_with(s: &Session, item: ItemId, old_path: &str, new_path: &s let old_dir: Vec<&str> = parts(&f.path).get(..old_dir_len)?.to_vec(); moved_footage(&f, |q| remap(q, &old_dir, dir).or_else(|| Some(join(dir, &[file_name(q)])))).filter(|nf| { // Only frames in that one folder (a name match, not a guess deeper down). - present(services.as_ref(), nf) && parts(&nf.path).len() == dir_parts.len() + 1 + present(services.as_ref(), &s.stored_paths, nf) && parts(&nf.path).len() == dir_parts.len() + 1 }) }; if let Some(nf) = same_place.or_else(same_folder) { diff --git a/crates/engine/src/commands/scripts.rs b/crates/engine/src/commands/scripts.rs index 3de5e6fe8..24049459c 100644 --- a/crates/engine/src/commands/scripts.rs +++ b/crates/engine/src/commands/scripts.rs @@ -132,7 +132,7 @@ fn is_script_name(name: &str) -> bool { } /// One path component on every platform: not empty, `.` or `..`, and without `/`, `\` or NUL. -fn is_bare_name(name: &str) -> bool { +pub(crate) fn is_bare_name(name: &str) -> bool { !name.is_empty() && name != "." && name != ".." && !name.contains(['/', '\\', '\0']) } @@ -140,7 +140,7 @@ fn is_bare_name(name: &str) -> bool { /// stream), ending in `.` or a space (Windows drops them), or a device name, which names a device /// in any folder and with any extension (`con.jsx`): `CON`, `PRN`, `AUX`, `NUL`, `CONIN$`, /// `CONOUT$`, `COM0`–`COM9` and `LPT0`–`LPT9` (also with `¹²³`). -fn windows_unsafe_name(name: &str) -> bool { +pub(crate) fn windows_unsafe_name(name: &str) -> bool { let stem = name.split('.').next().unwrap_or_default().trim_end_matches(' ').to_ascii_uppercase(); let port = |p: &str| { let mut n = stem.strip_prefix(p).unwrap_or_default().chars(); diff --git a/crates/engine/src/doc_path.rs b/crates/engine/src/doc_path.rs new file mode 100644 index 000000000..c7416a8ce --- /dev/null +++ b/crates/engine/src/doc_path.rs @@ -0,0 +1,158 @@ +//! Footage paths stored in a document (the project file, an imported project, the New Project +//! template) reach the file system only through [`resolve`]: a relative path is taken from the +//! document's folder, and only local files, or files on the network share the document itself is +//! on, are followed (see `craft_docpath`). +//! +//! Loading a document resolves its footage paths once ([`resolve_footage`]): followed paths are +//! replaced by the resolved path, and an item with a refused path is marked missing, so the media +//! layer never reads it. The session then remembers which paths came from which document +//! ([`StoredPaths`]); the footage check, the changed-on-disk scan, Reload Footage, Relink, Edit +//! Original and Reveal in Finder follow a path only through it. Paths the user imports or relinks +//! in this session are followed as given. + +use std::collections::HashMap; +use std::fmt; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +pub use craft_docpath::Refused; +use effectcraft_project::{Footage, ItemId, ItemKind, Project}; + +/// Why a stored path is not followed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NotFollowed { + /// The path is not a local file, or is not on the project's own share. + Refused(Refused), + /// The path is relative and the project has no folder to take it from. + NoProjectFolder, +} + +impl fmt::Display for NotFollowed { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Refused(r) => r.fmt(f), + Self::NoProjectFolder => f.write_str("the path is relative and the project has not been saved"), + } + } +} + +impl std::error::Error for NotFollowed {} + +/// The file to open for `stored`, a footage path or `file:` URL stored in the project saved at +/// `project` (`None` for a project without a file), or why it is not followed. A relative +/// `project` is taken from the working directory, where it was opened from. +pub fn resolve(stored: &str, project: Option<&str>) -> Result { + let project = project.filter(|p| !p.is_empty()).map(|p| { + let p = Path::new(p); + std::path::absolute(p).unwrap_or_else(|_| p.to_path_buf()) + }); + let path = craft_docpath::resolve(stored, project.as_deref()).map_err(NotFollowed::Refused)?; + // A path that is still relative would be taken from the working directory, which the + // project does not choose. + if path.is_relative() { + return Err(NotFollowed::NoProjectFolder); + } + Ok(path) +} + +/// The paths of `f` (its file and every frame of an image sequence) resolved against the document +/// saved at `document`, or why one of them is not followed. +fn footage_paths(f: &Footage, document: Option<&str>) -> Result<(String, Vec), (String, NotFollowed)> { + let one = |q: &String| -> Result { + if q.is_empty() { + return Ok(String::new()); + } + resolve(q, document).map(|p| p.to_string_lossy().into_owned()).map_err(|why| (q.clone(), why)) + }; + Ok((one(&f.path)?, f.sequence.iter().map(one).collect::>()?)) +} + +fn paths_of(f: &Footage) -> impl Iterator { + std::iter::once(&f.path).chain(&f.sequence).filter(|q| !q.is_empty()) +} + +/// Footage paths that came from a document, with the path of that document (`None`: one without +/// a file). Any other path was chosen in this session and is followed as given. +#[derive(Clone, Debug, Default)] +pub struct StoredPaths(Arc>>); + +impl StoredPaths { + /// Record the footage paths of `p`, loaded from the document at `document`. A path already + /// recorded keeps the document it came from first. + pub(crate) fn add(&mut self, p: &Project, document: Option<&str>) { + let map = Arc::make_mut(&mut self.0); + for item in p.items.values() { + if let ItemKind::Footage(f) = &item.kind { + for q in paths_of(f) { + map.entry(q.clone()).or_insert_with(|| document.map(str::to_string)); + } + } + } + } + + /// The user chose the files of `f` in this session (import, relink, Replace Footage): follow + /// them as given from now on. + pub(crate) fn allow(&mut self, f: &Footage) { + if paths_of(f).any(|q| self.0.contains_key(q)) { + let map = Arc::make_mut(&mut self.0); + for q in paths_of(f) { + map.remove(q); + } + } + } + + /// The file to open for footage path `path`. + pub fn follow(&self, path: &str) -> Result { + match self.0.get(path) { + Some(document) => resolve(path, document.as_deref()), + None => Ok(PathBuf::from(path)), + } + } + + /// Whether the file at footage path `path` exists. A path that is not followed is not looked + /// for. + pub(crate) fn exists(&self, services: &dyn crate::Services, path: &str) -> bool { + !path.is_empty() && self.follow(path).is_ok_and(|p| p.to_str().is_some_and(|p| services.exists(p))) + } + + /// Whether every file of `f` is followed, or the first that is not and why. + pub(crate) fn check(&self, f: &Footage) -> Result<(), (String, NotFollowed)> { + paths_of(f).try_for_each(|q| self.follow(q).map(|_| ()).map_err(|why| (q.clone(), why))) + } +} + +/// Resolve the footage paths stored in `p`, the project saved at `project`: followed paths are +/// replaced by the file they resolve to, and an item with a path that is not followed is marked +/// missing. Returns those items with the refused path and why. +pub(crate) fn resolve_footage(p: &mut Project, project: Option<&str>) -> Vec<(ItemId, String, NotFollowed)> { + let mut refused = vec![]; + for item in p.items.values_mut() { + let ItemKind::Footage(f) = &mut item.kind else { continue }; + match footage_paths(f, project) { + Ok((path, sequence)) => { + f.path = path; + f.sequence = sequence; + } + Err((path, why)) => { + f.missing = true; + refused.push((item.id, path, why)); + } + } + } + refused +} + +/// The message for footage whose stored path is not followed. +pub(crate) fn refusal(stored: &str, why: NotFollowed) -> String { + format!("the footage file {stored} is not opened: {why}") +} + +impl crate::Session { + /// The file to open for footage path `stored` in this session's project. + pub fn stored_path(&self, stored: &str) -> Result { + self.stored_paths.follow(stored) + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/engine/src/doc_path/tests.rs b/crates/engine/src/doc_path/tests.rs new file mode 100644 index 000000000..b8dcbc190 --- /dev/null +++ b/crates/engine/src/doc_path/tests.rs @@ -0,0 +1,153 @@ +use std::sync::{Arc, Mutex}; + +use effectcraft_project::{Footage, ItemId, ItemKind}; +use serde_json::json; + +use crate::{Event, Services, Session}; + +/// File access that records every path it is asked about. +#[derive(Default)] +struct Recording(Mutex>); + +impl Recording { + fn note(&self, path: &str) { + self.0.lock().unwrap().push(path.to_string()); + } +} + +impl Services for Recording { + fn read_file(&self, path: &str) -> std::io::Result> { + self.note(path); + std::fs::read(path) + } + fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { + std::fs::write(path, data) + } + fn exists(&self, path: &str) -> bool { + self.note(path); + std::path::Path::new(path).exists() + } + fn stamp(&self, path: &str) -> Option<(u64, u128)> { + self.note(path); + let meta = std::fs::metadata(path).ok()?; + Some((meta.len(), meta.modified().ok()?.duration_since(std::time::UNIX_EPOCH).ok()?.as_nanos())) + } +} + +/// Probes every file as a still. +struct Stills; + +impl crate::Importer for Stills { + fn probe(&self, path: &str) -> Result { + if !std::path::Path::new(path).is_file() { + return Err(format!("{path}: no such file")); + } + Ok(Footage { path: path.into(), kind: effectcraft_project::FootageKind::Still, width: 64, height: 32, has_video: true, ..Default::default() }) + } +} + +fn footage_id(s: &Session, name: &str) -> ItemId { + s.project.items.values().find(|i| i.name == name).unwrap().id +} + +fn footage<'a>(s: &'a Session, name: &str) -> &'a Footage { + match &s.project.item(footage_id(s, name)).unwrap().kind { + ItemKind::Footage(f) => f, + _ => panic!("{name} is not footage"), + } +} + +/// Footage paths to other computers stored in a project are never looked at: not on open, by +/// the footage check, by the changed-on-disk scan, by Edit Original or by Reveal in Finder. A +/// relative path is taken from the project's folder, and a file the user imports in this session +/// is used as given until the project is saved and opened again. +#[test] +fn footage_on_other_computers_is_never_looked_at() { + let root = std::env::temp_dir().join(format!("ec-doc-path-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let dir = root.join("job"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::create_dir_all(root.join("media")).unwrap(); + std::fs::write(dir.join("clip.png"), b"x").unwrap(); + std::fs::write(root.join("media").join("far.png"), b"x").unwrap(); + let mut stored = vec![("unc", r"\\192.0.2.1\share\x.png".to_string()), ("url", "file://192.0.2.1/s/x.png".to_string()), ("beside", "clip.png".to_string())]; + // Linux reads `//tmp/…/far.png` as the local file: a sink that skipped the check would find it. + if cfg!(unix) { + stored.push(("slashes", format!("/{}", root.join("media").join("far.png").display()))); + } + let mut s = Session::default(); + for (name, path) in &stored { + let f = Footage { path: path.clone(), width: 64, height: 48, has_video: true, ..Default::default() }; + Arc::make_mut(&mut s.project).add_item(name, crate::color::Label::None, None, ItemKind::Footage(f)); + } + let project = dir.join("p.ecproj"); + s.execute("file.saveAs", json!({"path": project.to_string_lossy()})).unwrap(); + + let rec = Arc::new(Recording::default()); + let mut o = Session { services: rec.clone(), ..Default::default() }; + o.execute("file.open", json!({"path": project.to_string_lossy()})).unwrap(); + for (name, _) in &stored { + assert_eq!(footage(&o, name).missing, *name != "beside", "{name}"); + } + assert_eq!(std::path::Path::new(&footage(&o, "beside").path), dir.join("clip.png")); + let r = o.execute("footage.check", json!({"wait": true})).unwrap(); + assert_eq!(r["missing"].as_u64(), Some(stored.len() as u64 - 1), "{r}"); + o.execute("footage.reloadChanged", json!({})).unwrap(); + o.drain_events(); + for (name, _) in &stored { + o.state.project_selection = vec![footage_id(&o, name)]; + let opened = o.execute("edit.editOriginal", json!({})); + let revealed = o.execute("file.revealInFinder", json!({})); + assert_eq!((opened.is_ok(), revealed.is_ok()), (*name == "beside", *name == "beside"), "{name}: {opened:?} {revealed:?}"); + } + let urls: Vec = o.drain_events().into_iter().filter_map(|e| if let Event::OpenUrl(u) = e { Some(u) } else { None }).collect(); + assert_eq!(urls.len(), 2, "{urls:?}"); + + let seen = rec.0.lock().unwrap().clone(); + assert!(seen.iter().any(|p| std::path::Path::new(p) == dir.join("clip.png")), "{seen:?}"); + // Only files in the temporary folder (the project's folder, where moved footage is looked for). + let local = root.to_string_lossy().into_owned(); + for p in &seen { + assert!(p.starts_with(&local), "looked at {p}"); + } + + // Chosen in this session, the same file is followed as given (as a file on a NAS would be)… + #[cfg(unix)] + { + let far = stored.iter().find(|(n, _)| *n == "slashes").unwrap().1.clone(); + o.importer = Some(Arc::new(Stills)); + o.execute("file.import", json!({"paths": [far.clone()]})).unwrap(); + let imported = |s: &Session| -> Vec { + s.project.items.values().filter(|i| matches!(&i.kind, ItemKind::Footage(f) if f.path == far)).map(|i| i.id).collect() + }; + let id = *imported(&o).iter().find(|id| matches!(&o.project.item(**id).unwrap().kind, ItemKind::Footage(f) if !f.missing)).unwrap(); + o.state.project_selection = vec![id]; + o.execute("edit.editOriginal", json!({})).unwrap(); + o.execute("file.revealInFinder", json!({})).unwrap(); + assert!(o.execute("footage.check", json!({"wait": true, "items": [id.0]})).unwrap()["missing"] == 0); + // …and is a stored path again once the project is saved and opened. + o.execute("file.save", json!({})).unwrap(); + let mut q = Session::default(); + q.execute("file.open", json!({"path": project.to_string_lossy()})).unwrap(); + let ids = imported(&q); + assert_eq!(ids.len(), 2); + for id in ids { + assert!(matches!(&q.project.item(id).unwrap().kind, ItemKind::Footage(f) if f.missing)); + q.state.project_selection = vec![id]; + assert!(q.execute("edit.editOriginal", json!({})).is_err()); + assert!(q.execute("file.revealInFinder", json!({})).is_err()); + } + } + let _ = std::fs::remove_dir_all(&root); +} + +#[test] +fn stored_paths_resolve_against_the_project() { + let doc = std::env::temp_dir().join("art").join("intro.ecproj"); + let doc = doc.to_str().unwrap(); + assert_eq!(super::resolve("clip.mov", Some(doc)), Ok(std::env::temp_dir().join("art").join("clip.mov"))); + assert_eq!(super::resolve("clip.mov", None), Err(super::NotFollowed::NoProjectFolder)); + assert_eq!(super::resolve(r"\\192.0.2.1\share\x.mov", Some(doc)), Err(super::NotFollowed::Refused(super::Refused::Network))); + #[cfg(windows)] + assert!(super::resolve(r"\\server\share\x.mov", Some(r"\\SERVER\share\job\intro.ecproj")).is_ok()); +} diff --git a/crates/engine/src/footage_check.rs b/crates/engine/src/footage_check.rs index adfb17683..6bb803f0d 100644 --- a/crates/engine/src/footage_check.rs +++ b/crates/engine/src/footage_check.rs @@ -31,10 +31,6 @@ enum Found { Moved(Box), } -fn exists(s: &dyn crate::Services, path: &str) -> bool { - !path.is_empty() && s.exists(path) -} - /// Start the check (all footage, or `items`). `wait` runs it to completion first (always on /// wasm32); the result is then `{checked, missing, probed}`. pub fn start(s: &mut Session, items: Option>, wait: bool) -> crate::Result { @@ -54,6 +50,7 @@ pub fn start(s: &mut Session, items: Option>, wait: bool) -> crate:: let services = s.services.clone(); let importer = s.importer.clone(); let rate = s.sequence_rate(); + let stored = s.stored_paths.clone(); let project_dir = s.path.as_deref().and_then(crate::commands::relink_moved::project_dir); let n = jobs.len(); s.spawn_task("footageCheck", format!("Checking footage ({n} items)"), wait, move |ctl| { @@ -64,13 +61,17 @@ pub fn start(s: &mut Session, items: Option>, wait: bool) -> crate:: return Err(crate::render_queue::CANCELLED.into()); } let f = &j.footage; - let present = if f.sequence.is_empty() { - exists(services.as_ref(), &f.path) - } else { - // A sequence is there while any of its frames is (missing frames render as - // placeholders, as in After Effects). - exists(services.as_ref(), &f.path) || f.sequence.iter().any(|p| exists(services.as_ref(), p)) - }; + // A path that is not followed is never looked for (the item stays missing). + let followed = stored.check(f).is_ok(); + let exists = |p: &str| stored.exists(services.as_ref(), p); + let present = followed + && if f.sequence.is_empty() { + exists(&f.path) + } else { + // A sequence is there while any of its frames is (missing frames render as + // placeholders, as in After Effects). + exists(&f.path) || f.sequence.iter().any(|p| exists(p)) + }; // Moved with the project: the same place under the project's folder. if !present && let Some(dir) = &project_dir @@ -233,9 +234,10 @@ mod tests { let mut paths: Vec = o.project.items.values().filter_map(|i| if let ItemKind::Footage(f) = &i.kind { Some(f.path.clone()) } else { None }).collect(); paths.sort(); - // Windows joins the stored "media/..." onto the project's folder and leaves a path with mixed - // separators; `Path` equality compares components, so both spellings are the same file. - let mut expected = ["/nonexistent/ec/abs.png", clip.to_str().unwrap(), "media/ec-relative-gone.png"]; + // `Path` equality compares components, so `/` and `\` spell the same file on Windows. + let abs = crate::doc_path::resolve("/nonexistent/ec/abs.png", path.to_str()).unwrap(); + let gone = dir.join("media").join("ec-relative-gone.png"); + let mut expected = [abs.to_str().unwrap(), clip.to_str().unwrap(), gone.to_str().unwrap()]; expected.sort(); assert_eq!(paths.len(), expected.len(), "{paths:?}"); for (got, want) in paths.iter().zip(expected) { diff --git a/crates/engine/src/footage_reload.rs b/crates/engine/src/footage_reload.rs index ca51069a4..fa12b27aa 100644 --- a/crates/engine/src/footage_reload.rs +++ b/crates/engine/src/footage_reload.rs @@ -32,12 +32,13 @@ pub type Scan = Arc)>>>>; /// The files footage items read: each item's file and every frame of an image sequence. Image /// sequences count only with Automatically Reload Footage set to All Footage (After Effects' /// default, Non-Sequence Footage, leaves them alone: thousands of frames on a share add up). +/// Items with a stored path that is not followed ([`crate::doc_path`]) are not watched. fn watched(s: &Session) -> BTreeSet { let sequences = s.prefs.import.auto_reload_footage == "all"; let mut paths = BTreeSet::new(); for it in s.project.items.values() { if let ItemKind::Footage(f) = &it.kind { - if !f.sequence.is_empty() && !sequences { + if (!f.sequence.is_empty() && !sequences) || s.stored_paths.check(f).is_err() { continue; } if !f.path.is_empty() { @@ -136,8 +137,10 @@ fn reload(s: &mut Session, ids: &[ItemId], undoable: bool) -> crate::Result<(usi let mut failed = BTreeSet::new(); for i in ids { if let Some(ItemKind::Footage(f)) = s.project.item(*i).map(|x| &x.kind) { - // Probed the way it imports by default, so a sequence is found as one. - let nf = match s.probe_footage(&f.path) { + // Probed the way it imports by default, so a sequence is found as one. A path that + // is not followed is not read. + let probed = s.stored_paths.check(f).map_err(|(p, why)| crate::doc_path::refusal(&p, why)).and_then(|_| s.probe_footage(&f.path)); + let nf = match probed { Ok(p) => refreshed(s, f, p), Err(_) if undoable => Footage { missing: true, ..f.clone() }, Err(_) => { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 11a31b763..d9a6c015d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -15,6 +15,7 @@ pub mod camera_track; pub mod commands; pub mod config; pub mod demo; +pub mod doc_path; pub mod footage_check; pub mod footage_reload; pub mod guard; @@ -360,6 +361,11 @@ fn one_view() -> u8 { 1 } +/// `t` as a current time of `comp`: a frame within its duration. +fn comp_frame_at(comp: &Comp, t: Tick) -> Tick { + comp.frame_rate.snap_nearest(t.min(comp.duration - comp.frame_duration()).max(Tick::ZERO)) +} + /// What Copy with Property Links / Copy Expression Only put on the clipboard. #[derive(Clone, Debug, PartialEq)] pub enum LinkClip { @@ -524,6 +530,8 @@ pub struct Session { /// event loop (the desktop app) turn this on; headless sessions run `footage.check` when /// they want it. pub check_footage_on_open: bool, + /// Footage paths that came from a document, with that document ([`doc_path`]). + pub stored_paths: doc_path::StoredPaths, /// Each footage file's size and modification time when last seen (Settings ▸ Import ▸ /// Reload Footage Changed on Disk, [`footage_reload`]). pub footage_stamps: std::collections::HashMap, @@ -624,6 +632,7 @@ impl Default for Session { next_task_id: 1, learn: None, check_footage_on_open: false, + stored_paths: Default::default(), footage_stamps: Default::default(), footage_scan: None, script_calls: ScriptCalls::default(), @@ -876,11 +885,7 @@ impl Session { pub fn set_time(&mut self, t: Tick) { if let Some(c) = self.state.active_comp { - let comp = self.project.comp(c); - let t = match comp { - Some(comp) => comp.frame_rate.snap_nearest(t.clamp(Tick::ZERO, comp.duration - comp.frame_duration())), - None => t, - }; + let t = self.project.comp(c).map_or(t, |comp| comp_frame_at(comp, t)); self.state.times.insert(c, t); if self.prefs.general.sync_time_related_items { self.sync_related_times(c, t); @@ -999,15 +1004,24 @@ impl Session { self.events.push(Event::OpenComp(id)); } - /// The open compositions, as a project file records them ([`effectcraft_project::SavedView`]). + /// The open compositions and their current times, as a project file records them + /// ([`effectcraft_project::SavedView`]). pub fn saved_view(&self) -> effectcraft_project::SavedView { - effectcraft_project::SavedView { active_comp: self.state.active_comp, open_comps: self.state.open_comps.clone() } + let comp_times = self.state.times.iter().filter(|(id, _)| self.project.comp(**id).is_some()).map(|(id, t)| (*id, *t)).collect(); + effectcraft_project::SavedView { active_comp: self.state.active_comp, open_comps: self.state.open_comps.clone(), comp_times } } - /// Reopen the compositions a project file was saved with (#748). Ids that no longer name a - /// comp are skipped; with none left the project stays on the comp [`Session::replace_project`] - /// opened (its first), as files from before the view was saved do. + /// Reopen the compositions a project file was saved with (#748), each at its saved current + /// time (#1714). Ids that no longer name a comp are skipped; with none left the project stays + /// on the comp [`Session::replace_project`] opened (its first), as files from before the view + /// was saved do. pub fn restore_view(&mut self, view: &effectcraft_project::SavedView) { + for (id, t) in &view.comp_times { + if let Some(comp) = self.project.comp(*id) { + let t = comp_frame_at(comp, *t); + self.state.times.insert(*id, t); + } + } let mut open: Vec = Vec::new(); for id in view.open_comps.iter().copied().filter(|id| self.project.comp(*id).is_some()) { if !open.contains(&id) { @@ -1144,6 +1158,7 @@ impl Session { job.wait(); } self.render_job = None; + self.stored_paths = Default::default(); self.stop_track(); self.track_job = None; self.stop_mask_track(); @@ -1245,6 +1260,8 @@ mod tests_codec_options; #[cfg(test)] mod tests_color_view; #[cfg(test)] +mod tests_delete_keys; +#[cfg(test)] mod tests_disk_cache; #[cfg(test)] mod tests_effects; diff --git a/crates/engine/src/media_anchor.rs b/crates/engine/src/media_anchor.rs index c1fbb3f65..61c2dc813 100644 --- a/crates/engine/src/media_anchor.rs +++ b/crates/engine/src/media_anchor.rs @@ -306,7 +306,12 @@ mod tests { let paths = all_paths(&s); let moved = root.join("Moved").to_string_lossy().into_owned(); assert_eq!(paths.iter().filter(|p| p.starts_with(&moved)).count(), 1 + 1 + 3 + 1, "movie, sequence (path and 3 frames), proxy: {paths:?}"); - assert!(paths.contains(&"/elsewhere/table.csv".to_string()), "Data footage is left alone"); + // Upstream's resolve_footage writes the stored path the platform's way (on Windows a + // rooted path gets its drive and backslashes), so compare by its tail. + assert!( + paths.iter().any(|p| !p.starts_with(&moved) && p.replace('\\', "/").ends_with("/elsewhere/table.csv")), + "Data footage is left alone: {paths:?}" + ); assert!(!s.is_dirty(), "following the folder is not an edit"); } diff --git a/crates/engine/src/media_browser.rs b/crates/engine/src/media_browser.rs index b394ad937..d22875f17 100644 --- a/crates/engine/src/media_browser.rs +++ b/crates/engine/src/media_browser.rs @@ -306,7 +306,7 @@ fn kind_name(k: FootageKind) -> &'static str { /// Metadata of footage (the fields the Metadata panel lists). pub fn footage_metadata(f: &Footage) -> Value { - let (created, modified, size) = file_dates(f.sequence.first().map(String::as_str).unwrap_or(&f.path)); + let (created, modified, size) = if f.missing { (None, None, None) } else { file_dates(f.sequence.first().map(String::as_str).unwrap_or(&f.path)) }; json!({ "path": f.path, "kind": kind_name(f.kind), diff --git a/crates/engine/src/offload.rs b/crates/engine/src/offload.rs index bd1fec8a3..ad2c0cde0 100644 --- a/crates/engine/src/offload.rs +++ b/crates/engine/src/offload.rs @@ -310,13 +310,15 @@ pub fn next_id() -> u64 { NEXT_ID.fetch_add(1, Ordering::Relaxed) } -/// Footage files a project reads (paths, sequence frames). +/// Footage files a project reads (paths, sequence frames; missing items read none). pub fn footage_files(p: &Project) -> Vec { let mut v: Vec = p .items .values() .filter_map(|i| match &i.kind { - ItemKind::Footage(f) if !f.path.is_empty() => Some(std::iter::once(f.path.clone()).chain(f.sequence.iter().cloned()).collect::>()), + ItemKind::Footage(f) if !f.path.is_empty() && !f.missing => { + Some(std::iter::once(f.path.clone()).chain(f.sequence.iter().cloned()).collect::>()) + } _ => None, }) .flatten() diff --git a/crates/engine/src/prefs.rs b/crates/engine/src/prefs.rs index 3abf336b0..a2ce579ea 100644 --- a/crates/engine/src/prefs.rs +++ b/crates/engine/src/prefs.rs @@ -428,6 +428,8 @@ pub struct Prefs { pub recent_presets: Vec, /// Character panel: recently used font families, newest first. pub recent_fonts: Vec, + /// Effects & Presets: the starred effects' ids, in the order starred. + pub effects_favorites: Vec, /// Preview panel: every shortcut's options. pub preview: crate::preview::PreviewSettings, /// View ▸ Simulate Output ▸ My Custom RGB. @@ -468,6 +470,7 @@ impl Default for Prefs { recent_footage: vec![], recent_presets: vec![], recent_fonts: vec![], + effects_favorites: vec![], preview: crate::preview::PreviewSettings::default(), custom_rgb: crate::viewer::CustomRgb::default(), comp_presets: vec![], @@ -597,6 +600,9 @@ impl Prefs { pub fn normalize(&mut self) { self.version = PREFS_VERSION; self.comp_presets.retain(CompPreset::is_valid); + let mut seen = std::collections::HashSet::new(); + self.effects_favorites.retain(|id| !id.is_empty() && seen.insert(id.clone())); + self.effects_favorites.truncate(MAX_EFFECTS_FAVORITES); if !is_choice(WINDOW_GRAPHICS, &self.startup.window_graphics) { self.startup.window_graphics = Startup::default().window_graphics; } @@ -715,6 +721,7 @@ impl Prefs { recent_footage: std::mem::take(&mut self.recent_footage), recent_presets: std::mem::take(&mut self.recent_presets), recent_fonts: std::mem::take(&mut self.recent_fonts), + effects_favorites: std::mem::take(&mut self.effects_favorites), preview: std::mem::take(&mut self.preview), comp_presets: std::mem::take(&mut self.comp_presets), ..d @@ -916,6 +923,23 @@ impl Prefs { push_mru(&mut self.recent_fonts, family, 30); } + /// Is effect `id` starred in Effects & Presets? + pub fn is_effect_favorite(&self, id: &str) -> bool { + self.effects_favorites.iter().any(|f| f == id) + } + + /// Star effect `id` in Effects & Presets, or un-star it; true when it is starred now. + pub fn toggle_effect_favorite(&mut self, id: &str) -> bool { + if self.is_effect_favorite(id) { + self.effects_favorites.retain(|f| f != id); + false + } else { + self.effects_favorites.push(id.to_string()); + self.effects_favorites.truncate(MAX_EFFECTS_FAVORITES); + self.is_effect_favorite(id) + } + } + /// The recent fonts the Character panel shows (at most Number of Recent Fonts to Display). pub fn recent_fonts_shown(&self) -> &[String] { let n = (self.type_.recent_fonts as usize).min(self.recent_fonts.len()); @@ -933,6 +957,9 @@ impl Prefs { } } +/// Most effects Effects & Presets keeps starred (a hand-edited settings file can hold more). +const MAX_EFFECTS_FAVORITES: usize = 1000; + fn push_mru(list: &mut Vec, v: &str, max: usize) { list.retain(|x| x != v); list.insert(0, v.to_string()); diff --git a/crates/engine/src/session_settings.rs b/crates/engine/src/session_settings.rs index 55c8e4154..0b457926c 100644 --- a/crates/engine/src/session_settings.rs +++ b/crates/engine/src/session_settings.rs @@ -116,6 +116,11 @@ impl Session { self.layer_cache.set_disk(None); return; }; + // Tests: the Roto/Object Matte segment store below is one per process, so a test that + // opens a disk cache (often the platform folder, through a settings store) would take + // over the store of a roto test running beside it. Serialise them with the roto tests. + #[cfg(test)] + crate::tests_roto::hold_roto_cache_test_lock(); let dc = match &self.disk_cache { Some(c) if c.folder() == folder.as_path() => { c.set_max_bytes(max); @@ -225,36 +230,50 @@ impl Session { let prefs = self.autosave_prefs(); let fail = |e: std::io::Error| EngineError::Other(format!("auto-save failed: {e}")); let plan = autosave::plan_in(self.file_ops(), &prefs, self.path.as_deref(), root.as_deref(), self.autosave.last_slot).map_err(fail)?; + let path = plan.path.to_string_lossy().into_owned(); if self.autosave.background && !cfg!(target_arch = "wasm32") { let project = self.project.clone(); let config = self.config.clone(); let job = plan.clone(); - let pending = autosave::PendingWrite::spawn(plan.path.clone(), move || { - let json = project.to_json_compact(); - match &config { - Some(c) => job.write(c.files(), json.as_bytes()), - None => job.write(&crate::config::StdFiles, json.as_bytes()), - } - }) - .map_err(fail)?; + let pending = + autosave::PendingWrite::spawn(plan.path.clone(), plan.slot, self.revision, self.path.clone(), self.script_ui.project_epoch, move || { + let json = project.to_json_compact(); + match &config { + Some(c) => job.write(c.files(), json.as_bytes()), + None => job.write(&crate::config::StdFiles, json.as_bytes()), + } + }) + .map_err(fail)?; self.autosave.pending = Some(pending); } else { let json = self.project.to_json_compact(); plan.write(self.file_ops(), json.as_bytes()).map_err(fail)?; + self.commit_autosave(path.clone(), plan.slot, self.revision); } - let (path, slot) = (plan.path, plan.slot); - let p = path.to_string_lossy().to_string(); + Ok(path) + } + + /// Advance recovery bookkeeping only for a successfully written snapshot. + fn commit_autosave(&mut self, path: String, slot: u32, revision: u64) { self.autosave.last_slot = Some(slot); - self.autosave.saved_revision = Some(self.revision); - self.autosave.last_path = Some(p.clone()); + self.autosave.saved_revision = Some(revision); + self.autosave.last_path = Some(path); self.update_sentinel(); - Ok(p) } /// Wait for a background auto-save ([`autosave::AutoSaveState::background`]) to land. pub fn autosave_wait(&mut self) -> Result<()> { match self.autosave.pending.take() { - Some(w) => w.join().map_err(|e| EngineError::Other(format!("auto-save failed: {e}"))), + Some(w) => { + w.join().map_err(|e| EngineError::Other(format!("auto-save failed: {e}")))?; + // Open/New can replace the project before note_project_path collects this + // write. Save As changes only its path. Never publish the old snapshot as the + // new project's recovery record, even when that record's final reset fails. + if w.project_path == self.path && w.project_epoch == self.script_ui.project_epoch { + self.commit_autosave(w.path.to_string_lossy().into_owned(), w.slot, w.revision); + } + Ok(()) + } None => Ok(()), } } @@ -278,7 +297,7 @@ impl Session { return None; } self.autosave.last = Some(now); - if !self.is_dirty() || self.autosave.saved_revision == Some(self.revision) || self.render_job.is_some() { + if !self.is_dirty() || self.autosave.pending.is_some() || self.autosave.saved_revision == Some(self.revision) || self.render_job.is_some() { return None; } if self.path.is_none() && self.prefs.auto_save.location != "custom" && self.default_autosave_root().is_none() && self.autosave_folder_override.is_none() diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 72133c5d6..b3f0602b3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -686,3 +686,50 @@ fn reopening_a_project_restores_its_active_comp() { o.insert("openComps".into(), open); } } + +/// Layer times are saved as exact ticks and read back unchanged, and each comp reopens at the +/// current time it was saved at, not at 0, where a layer starting later shows nothing (#1714). +#[test] +fn reopening_a_project_restores_layer_times_and_current_time() { + use effectcraft_project::ItemId; + use effectcraft_time::{FrameRate, Tick}; + let dir = std::env::temp_dir().join(format!("ec-test-1714-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("times.ecproj").to_string_lossy().to_string(); + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 108, "height": 192, "duration": 60})).unwrap(); + s.execute("layer.newSolid", json!({"name": "crown"})).unwrap(); + let cid = s.active_comp_id().unwrap(); + let comp = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap(); + comp.frame_rate = FrameRate::FPS_59_94; + // The layer from the issue: 34f, 315f and 3033f at 59.94 fps. + let l = comp.layers.first_mut().unwrap(); + (l.start_time, l.in_point, l.out_point) = (Tick(144_086_342_400), Tick(1_334_917_584_000), Tick(12_853_349_308_800)); + s.execute("time.set", json!({"time": 6.0})).unwrap(); + let t = s.time(); + // A second comp keeps its own time. + let b = ItemId(s.execute("comp.new", json!({"name": "B"})).unwrap()["comp"].as_u64().unwrap()); + s.execute("time.set", json!({"time": 2.0})).unwrap(); + let tb = s.time(); + s.execute("comp.open", json!({"comp": "C"})).unwrap(); + s.execute("file.saveAs", json!({"path": path})).unwrap(); + + let mut s2 = Session::default(); + s2.execute("file.open", json!({"path": path})).unwrap(); + let l = s2.project.comp(cid).unwrap().layers.first().unwrap(); + assert_eq!((l.start_time, l.in_point, l.out_point), (Tick(144_086_342_400), Tick(1_334_917_584_000), Tick(12_853_349_308_800))); + assert_eq!((s2.time(), s2.state.times.get(&b).copied()), (t, Some(tb))); + assert!(l.in_point <= s2.time() && s2.time() < l.out_point, "the layer shows at the reopened time"); + assert!(!s2.is_dirty()); + + // Hand-edited times past the end or before the start land on the last or first frame. + let mut file: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + file["compTimes"] = json!({cid.0.to_string(): i64::MAX, b.0.to_string(): -5}); + std::fs::write(&path, file.to_string()).unwrap(); + let mut s3 = Session::default(); + s3.execute("file.open", json!({"path": path})).unwrap(); + let c = s3.project.comp(cid).unwrap(); + assert_eq!(s3.time(), c.frame_rate.snap_nearest(c.duration - c.frame_duration())); + assert_eq!(s3.time(), c.frame_rate.snap_nearest(c.duration - c.frame_duration())); + assert_eq!(s3.state.times.get(&b).copied(), Some(Tick::ZERO)); +} diff --git a/crates/engine/src/tests_delete_keys.rs b/crates/engine/src/tests_delete_keys.rs new file mode 100644 index 000000000..b1b9af316 --- /dev/null +++ b/crates/engine/src/tests_delete_keys.rs @@ -0,0 +1,45 @@ +//! `keys.delete` reports the keys it removed, not the keys it was asked to remove. + +use effectcraft_project::LayerId; +use serde_json::json; + +use crate::{KeyRef, Session}; + +fn setup() -> (Session, u64, u64) { + let mut s = Session::default(); + s.execute_checked("comp.new", json!({"width": 64, "height": 64, "frameRate": 30, "duration": 4})).unwrap(); + let layer = s.execute_checked("layer.newSolid", json!({"name": "Box", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for t in 0..3 { + s.execute_checked("prop.addKey", json!({"layer": layer, "path": "transform/opacity", "time": t, "value": t * 25})).unwrap(); + } + let prop = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("transform/opacity").unwrap().uid; + (s, layer, prop) +} + +fn keys_of(s: &Session, layer: u64, prop: u64) -> Vec { + let keys = &s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys; + keys.iter().map(|k| KeyRef { layer: LayerId(layer), prop, time: k.time }).collect() +} + +#[test] +fn delete_keys_counts_only_the_keys_it_removed() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + assert_eq!(keys.len(), 3); + // Two real keys, one of them listed twice, and a key of a property that does not exist. + let missing = KeyRef { layer: LayerId(layer), prop: u64::MAX, time: keys[1].time }; + s.state.selected_keys = vec![keys[0], keys[2], keys[0], missing]; + let removed = s.execute_checked("keys.delete", json!({})).unwrap(); + assert_eq!(removed, json!(2)); + assert_eq!(keys_of(&s, layer, prop), vec![keys[1]]); +} + +#[test] +fn delete_keys_reports_zero_when_no_selected_key_exists() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + s.state.selected_keys = vec![KeyRef { layer: LayerId(layer), prop: u64::MAX, time: keys[0].time }]; + let removed = s.execute_checked("keys.delete", json!({})).unwrap(); + assert_eq!(removed, json!(0)); + assert_eq!(keys_of(&s, layer, prop), keys); +} diff --git a/crates/engine/src/tests_effects.rs b/crates/engine/src/tests_effects.rs index 6f87b4aea..18fcc6f84 100644 --- a/crates/engine/src/tests_effects.rs +++ b/crates/engine/src/tests_effects.rs @@ -741,3 +741,56 @@ fn effect_colour_labels_set_clear_undo_and_round_trip() { let l = old.comp(s.active_comp_id().unwrap()).unwrap().layer(LayerId(a)).unwrap().clone(); assert_eq!(l.effects().unwrap().groups().next().unwrap().label, None); } + +/// #1746: a preset saved from an effect property (`effects/ec.control.slider/slider`, `#2` for the second +/// instance) applies back to the same layer instead of landing in `skipped`. +#[test] +fn preset_round_trips_effect_properties_by_match_name() { + let (mut s, a, _) = comp_with_two_solids(); + apply(&mut s, a, "Slider Control"); + apply(&mut s, a, "Slider Control"); + let slider = |s: &Session, n: usize| match layer(s, a).effects().unwrap().groups().nth(n).unwrap().get("slider").unwrap().value { + KV::Scalar(v) => v, + ref other => panic!("not a scalar: {other:?}"), + }; + for (n, value) in [(1, 30.0), (2, 70.0)] { + s.execute("prop.set", json!({"layer": a, "path": format!("effects/#{n}/slider"), "value": value})).unwrap(); + } + for n in [1, 2] { + let path = std::env::temp_dir().join(format!("ec-1746-{}-{n}.ecpreset", std::process::id())).to_string_lossy().to_string(); + s.execute("prop.select", json!({"layer": a, "path": format!("effects/#{n}/slider")})).unwrap(); + s.execute("anim.savePreset", json!({"path": path})).unwrap(); + let saved = std::fs::read_to_string(&path).unwrap(); + let want = if n == 1 { "effects/ec.control.slider/slider" } else { "effects/ec.control.slider#2/slider" }; + assert!(saved.contains(want), "{saved}"); + s.execute("prop.set", json!({"layer": a, "path": format!("effects/#{n}/slider"), "value": 0.0})).unwrap(); + let r = s.execute("anim.applyPreset", json!({"path": path, "layers": [a]})).unwrap(); + let _ = std::fs::remove_file(&path); + assert_eq!(r["applied"], 1, "{r}"); + assert_eq!(r["skipped"], json!([]), "{r}"); + assert_eq!(slider(&s, n - 1), if n == 1 { 30.0 } else { 70.0 }); + } +} + +/// #1746: Edit ▸ Copy of an effect property writes the same `effects/ec.blur.gaussian/blurriness` path, +/// and Paste (the value, or Copy Expression Only) sets that property of the other layer's effect. +#[test] +fn copied_effect_property_pastes_onto_the_same_effect() { + let (mut s, a, b) = comp_with_two_solids(); + apply(&mut s, a, "Gaussian Blur"); + apply(&mut s, b, "Gaussian Blur"); + s.execute("prop.set", json!({"layer": a, "path": "effects/#1/blurriness", "value": 12.5})).unwrap(); + s.execute("prop.select", json!({"layer": a, "path": "effects/#1/blurriness"})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("layer.select", json!({"layers": [b]})).unwrap(); + let r = s.execute("edit.paste", json!({})).unwrap(); + assert_eq!(r["values"], 1, "{r}"); + assert_eq!(layer(&s, b).props.prop("effects/#1/blurriness").unwrap().value, KV::Scalar(12.5)); + s.execute("prop.setExpression", json!({"layer": a, "path": "effects/#1/blurriness", "expression": "time * 10"})).unwrap(); + s.execute("prop.select", json!({"layer": a, "path": "effects/#1/blurriness"})).unwrap(); + s.execute("edit.copyExpressionOnly", json!({})).unwrap(); + s.execute("layer.select", json!({"layers": [b]})).unwrap(); + let r = s.execute("edit.paste", json!({})).unwrap(); + assert_eq!(r["expressions"], 1, "{r}"); + assert_eq!(layer(&s, b).props.prop("effects/#1/blurriness").unwrap().expr.as_ref().unwrap().text, "time * 10"); +} diff --git a/crates/engine/src/tests_essential.rs b/crates/engine/src/tests_essential.rs index 0732020ae..191416b9d 100644 --- a/crates/engine/src/tests_essential.rs +++ b/crates/engine/src/tests_essential.rs @@ -274,6 +274,55 @@ fn template_export_import_round_trip_with_overrides() { assert!(t.execute("essential.importTemplate", json!({"path": dir.join("bad.ectemplate").to_string_lossy()})).is_err()); } +#[test] +fn template_import_writes_media_only_into_its_media_folder() { + let root = out_dir("ectemplate-names"); + let dir = root.join("a").join("b"); + std::fs::create_dir_all(&dir).unwrap(); + let (mut s, card, _solid, _ids) = card(); + let src = dir.join("Src.ectemplate"); + s.execute("essential.exportTemplate", json!({"comp": card.0, "path": src.to_string_lossy()})).unwrap(); + let mut entries = effectcraft_lottie::zip::read_stored(&std::fs::read(&src).unwrap()); + std::fs::remove_file(&src).unwrap(); + let abs = root.join("abs").join("x").to_string_lossy().replace('\\', "/"); + for name in [ + "../x", + "media/../../x", + "media/1/../../../x", + &format!("media/{abs}"), + &format!("media/6{abs}"), + "media/C:\\x", + "media/7/C:\\x", + "media/x/clip.png", + "media/8/sub/clip.png", + "media/2/..", + "media/3/", + "media/4/clip.png", + "media/5/clip.png", + ] { + entries.push((name.to_string(), b"data".to_vec())); + } + let path = dir.join("T.ectemplate"); + std::fs::write(&path, effectcraft_lottie::zip::store(&entries)).unwrap(); + s.execute("essential.importTemplate", json!({"path": path.to_string_lossy()})).unwrap(); + + let media = dir.join("T Media"); + fn files(d: &std::path::Path, out: &mut Vec) { + for e in std::fs::read_dir(d).unwrap().map(|e| e.unwrap().path()) { + if e.is_dir() { files(&e, out) } else { out.push(e) } + } + } + let mut found = vec![]; + files(&root, &mut found); + let in_item = |f: &std::path::Path| f.parent().and_then(std::path::Path::parent) == Some(media.as_path()); + for f in &found { + assert!(f == &path || in_item(f), "written outside an item folder of the media folder: {f:?}"); + } + let mut names: Vec = found.iter().filter(|f| in_item(f)).map(|f| f.strip_prefix(&media).unwrap().to_string_lossy().replace('\\', "/")).collect(); + names.sort(); + assert_eq!(names, ["4/clip.png", "5/clip.png"]); +} + #[test] fn media_replacement_swaps_footage_per_instance() { use std::sync::Arc; diff --git a/crates/engine/src/tests_frame_export.rs b/crates/engine/src/tests_frame_export.rs index 87b0864ac..a45c5d444 100644 --- a/crates/engine/src/tests_frame_export.rs +++ b/crates/engine/src/tests_frame_export.rs @@ -77,6 +77,19 @@ fn proexr_writes_layer_prefixed_channels_and_the_composite() { assert!(at("R", 16, 16) < 0.5, "multiplied by 50% blue (linear)"); } +#[test] +fn proexr_names_a_non_ascii_layer_with_underscores_instead_of_panicking() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Frame", "width": 8, "height": 8, "frameRate": 24, "duration": 1})).unwrap(); + s.execute("layer.newSolid", json!({"name": "你好", "color": "#ff0000"})).unwrap(); + let dir = out_dir("frame-exr-non-ascii"); + let path = dir.join("frame.exr").to_string_lossy().to_string(); + let r = s.execute("comp.saveFrameAsExr", json!({"path": path})).unwrap(); + let names: Vec<&str> = r["channels"].as_array().unwrap().iter().map(|c| c.as_str().unwrap()).collect(); + assert!(names.contains(&"__.R") && names.contains(&"__.A"), "{names:?}"); + assert!(std::path::Path::new(&path).exists()); +} + /// Save a 32×32 comp holding one opaque solid of `color` (an sRGB-encoded triple) as ProEXR and /// return the centre pixel of the composite and of the solid's own layer: `[R, G, B]` each. fn proexr_centre(color: [f64; 3], bit_depth: u64) -> ([f32; 3], [f32; 3]) { diff --git a/crates/engine/src/tests_import_project.rs b/crates/engine/src/tests_import_project.rs index 6aa52b411..cbc161c9c 100644 --- a/crates/engine/src/tests_import_project.rs +++ b/crates/engine/src/tests_import_project.rs @@ -113,7 +113,7 @@ fn import_project_merges_with_fresh_ids_and_one_undo() { // A relative footage path resolves next to the imported project; a missing one is reported. let data = t.project.items.values().find(|i| i.name == "data.json").unwrap(); let ItemKind::Footage(f) = &data.kind else { panic!("footage") }; - assert_eq!(std::path::Path::new(&f.path), dir.join("data.json")); + assert_eq!(std::fs::canonicalize(&f.path).unwrap(), std::fs::canonicalize(dir.join("data.json")).unwrap()); assert_eq!(r["missingMedia"], json!(["/offline/gone.mov"])); // One undo step removes all of it; redo brings it back. diff --git a/crates/engine/src/tests_menu_cmds.rs b/crates/engine/src/tests_menu_cmds.rs index 4e01e380f..6da07dd40 100644 --- a/crates/engine/src/tests_menu_cmds.rs +++ b/crates/engine/src/tests_menu_cmds.rs @@ -1445,6 +1445,60 @@ fn keys_from_several_layers_paste_in_order_and_paste_reversed_respects_stretch() assert_eq!(vals, vec![10.0, 0.0], "reversed"); } +/// #1425: a mask path (its value, or a keyframe of it) pasted onto Position becomes a motion +/// path, as in After Effects: a key per vertex with the path's tangents, over 2 s from the CTI, +/// the inner keys roving; a closed path returns to its start. AE pastes the layer-space numbers +/// as they are, so the source layer's and the target parent's transforms don't move them. +#[test] +fn a_copied_path_pastes_onto_position_as_a_motion_path() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + // A moved and scaled source: converting to comp space would give 2v + (200, 100). + s.execute("prop.set", json!({"layer": a, "path": "transform/position", "value": [400, 200]})).unwrap(); + s.execute("prop.set", json!({"layer": a, "path": "transform/scale", "value": [200, 200]})).unwrap(); + let rect = json!([[0, 0], [40, 0], [40, 40], [0, 40]]); + s.execute("mask.new", json!({"layer": a, "vertices": rect.clone(), "outTangents": [[10, 0]], "closed": true})).unwrap(); + // B's parent would shift a converted Position by (0, −50). + let parent = solid(&mut s, "#0000ff"); + s.execute("prop.set", json!({"layer": parent, "path": "transform/position", "value": [100, 100]})).unwrap(); + let b = solid(&mut s, "#00ff00"); + s.execute("layer.setParent", json!({"layers": [b], "parent": parent})).unwrap(); + let motion = |s: &Session, id: u64| { + let l = layer(s, id); + let pr = l.props.prop("transform/position").unwrap().clone(); + pr.keys.iter().map(|k| (l.comp_time(k.time).seconds(), k.value.as_vec3(), k.spatial_out, k.roving)).collect::>() + }; + let check = |keys: Vec<(f64, [f64; 3], [f64; 3], bool)>, t0: f64| { + let want = [[0.0, 0.0], [40.0, 0.0], [40.0, 40.0], [0.0, 40.0], [0.0, 0.0]]; + assert_eq!(keys.len(), 5, "a key per vertex and the closing one: {keys:?}"); + for (i, ((t, v, _, roving), w)) in keys.iter().zip(want).enumerate() { + assert!((v[0] - w[0]).abs() < 1e-6 && (v[1] - w[1]).abs() < 1e-6, "key {i}: {v:?}"); + assert_eq!(*roving, i != 0 && i != 4, "key {i} roving"); + assert!(*t > t0 - 1e-6 && *t < t0 + 2.0 + 1e-6, "key {i} at {t}"); + } + assert!((keys[0].0 - t0).abs() < 1e-6 && (keys[4].0 - t0 - 2.0).abs() < 1e-6, "2 s from the CTI"); + assert!(keys.windows(2).all(|w| w[0].0 < w[1].0)); + assert_eq!(keys[0].2[..2], [10.0, 0.0], "the path's tangent"); + }; + // A Mask Path keyframe, pasted at 1 s. + s.execute("prop.toggleAnimation", json!({"layer": a, "path": "masks/#1/path"})).unwrap(); + s.execute("prop.select", json!({"layer": a, "path": "masks/#1/path"})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("time.set", json!({"time": 1.0})).unwrap(); + s.execute("prop.select", json!({"layer": b, "path": "transform/position"})).unwrap(); + s.execute("edit.paste", json!({})).unwrap(); + check(motion(&s, b), 1.0); + // The same path as an unanimated mask, pasted at 5 s onto an unparented layer. + s.execute("mask.new", json!({"layer": a, "vertices": rect, "outTangents": [[10, 0]], "closed": true})).unwrap(); + s.execute("prop.select", json!({"layer": a, "path": "masks/#2/path"})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + let c = solid(&mut s, "#ffffff"); + s.execute("time.set", json!({"time": 5.0})).unwrap(); + s.execute("prop.select", json!({"layer": c, "path": "transform/position"})).unwrap(); + s.execute("edit.paste", json!({})).unwrap(); + check(motion(&s, c), 5.0); +} + #[test] fn j_and_k_stop_at_keys_markers_and_the_work_area() { let mut s = comp(); @@ -1541,6 +1595,40 @@ fn shape_contents_copy_cut_and_paste_between_shape_layers() { assert!(s.execute("edit.paste", json!({})).is_err()); } +/// Pasting Trim Paths into another shape layer with nothing selected puts it below the shapes, +/// where it acts on them (it acts on the items above it); on top it would trim nothing (#1681, #1629). +#[test] +fn pasted_path_operator_goes_below_the_shapes_it_acts_on() { + let mut s = comp(); + let a = s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(); + let trim = s.execute("layer.addShapeItem", json!({"layer": a, "kind": "trim"})).unwrap()["uid"].as_u64().unwrap(); + let b = s.execute("layer.newShape", json!({"kind": "star", "name": "B"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.select", json!({"layer": a, "prop": trim})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("layer.select", json!({"layers": [b]})).unwrap(); + s.execute("edit.paste", json!({})).unwrap(); + assert_eq!(contents_names(&s, b), ["Polystar 1", "Trim Paths 1"]); +} + +/// Pasting Merge Paths onto a painted layer with nothing selected keeps the paint: the renderer +/// drops the draws above a Merge Paths, so it goes between the path and the paints (#1681). +#[test] +fn pasted_merge_paths_keeps_the_targets_paint() { + let mut s = comp(); + let a = s.execute("layer.newShape", json!({"kind": "none", "name": "A"})).unwrap()["layer"].as_u64().unwrap(); + let merge = s.execute("layer.addShapeItem", json!({"layer": a, "kind": "merge"})).unwrap()["uid"].as_u64().unwrap(); + let b = s.execute("layer.newShape", json!({"kind": "none", "name": "B"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.addShapeItem", json!({"layer": b, "kind": "rect"})).unwrap(); + s.execute("layer.addShapeItem", json!({"layer": b, "kind": "fill"})).unwrap(); + let before = opaque_centroid(&s).2; + s.execute("prop.select", json!({"layer": a, "prop": merge})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("layer.select", json!({"layers": [b]})).unwrap(); + s.execute("edit.paste", json!({})).unwrap(); + assert_eq!(contents_names(&s, b), ["Rectangle Path 1", "Merge Paths 1", "Fill 1"]); + assert_eq!(opaque_centroid(&s).2, before); +} + /// Edit ▸ Duplicate with shape items selected duplicates them in their layer, above the /// original and named "Rectangle 2", not the layer (#227). #[test] diff --git a/crates/engine/src/tests_settings.rs b/crates/engine/src/tests_settings.rs index 65e39b467..a4f4d89eb 100644 --- a/crates/engine/src/tests_settings.rs +++ b/crates/engine/src/tests_settings.rs @@ -212,6 +212,34 @@ fn prefs_commands_get_set_reset_and_persist() { ); } +/// Effects & Presets' starred effects live in the settings (#1692): they come back in the next +/// session, un-starring sticks too, Reset All keeps them, and a hand-edited list loses its +/// repeats and empty ids on load. +#[test] +fn effects_favorites_persist_across_sessions() { + let store = Arc::new(MemoryConfig::default()); + let mut s = Session { config: Some(store.clone()), ..Default::default() }; + assert!(s.prefs.toggle_effect_favorite("ADBE Gaussian Blur 2")); + assert!(s.prefs.toggle_effect_favorite("ADBE Glo2")); + s.save_prefs(); + let reopen = |store: &Arc| { + let mut s = Session { config: Some(store.clone()), ..Default::default() }; + s.load_settings(); + s + }; + let mut s2 = reopen(&store); + assert_eq!(s2.prefs.effects_favorites, ["ADBE Gaussian Blur 2", "ADBE Glo2"]); + assert!(!s2.prefs.toggle_effect_favorite("ADBE Glo2"), "un-starred"); + s2.save_prefs(); + let mut s3 = reopen(&store); + assert_eq!(s3.prefs.effects_favorites, ["ADBE Gaussian Blur 2"]); + s3.execute("prefs.reset", json!({})).unwrap(); + assert!(s3.prefs.is_effect_favorite("ADBE Gaussian Blur 2"), "Reset All keeps the stars"); + let edited = store.read(PREFS_FILE).unwrap().replace("\"effectsFavorites\": [", "\"effectsFavorites\": [\"\", \"ADBE Glo2\", \"ADBE Glo2\", "); + store.write(PREFS_FILE, &edited).unwrap(); + assert_eq!(reopen(&store).prefs.effects_favorites, ["ADBE Glo2", "ADBE Gaussian Blur 2"]); +} + /// Composition Settings presets the user saves live in the settings: they persist, a save with /// an existing name replaces it, they survive Reset All, and invalid ones (from a hand-edited /// settings file) are dropped on load. @@ -608,6 +636,216 @@ fn atomic_write_does_not_follow_a_link_at_the_old_temp_name() { let _ = std::fs::remove_dir_all(d); } +struct AutosaveTestStore { + config: DirConfig, + fail_next: std::sync::atomic::AtomicBool, + gate: std::sync::Mutex, std::sync::mpsc::Receiver<()>)>>, + release_on_prefs: std::sync::Mutex>>, + fail_reset_for: std::sync::Mutex>, + failed_resets: std::sync::atomic::AtomicUsize, +} + +impl ConfigStore for AutosaveTestStore { + fn read(&self, name: &str) -> Option { + self.config.read(name) + } + fn write(&self, name: &str, data: &str) -> std::io::Result<()> { + // note_project_path saves preferences after the project/path switch, before joining + // the pending autosave. Release that writer at this exact transition, without sleeps. + if name == PREFS_FILE + && let Some(release) = self.release_on_prefs.lock().unwrap().take() + { + release.send(()).unwrap(); + } + if name == autosave::SENTINEL + && let Some(path) = self.fail_reset_for.lock().unwrap().as_ref() + { + let record: autosave::Sentinel = serde_json::from_str(data).unwrap(); + if record.project.as_ref() == Some(path) && record.autosave.is_none() { + self.failed_resets.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + return Err(std::io::Error::other("injected final recovery reset failure")); + } + } + self.config.write(name, data) + } + fn remove(&self, name: &str) -> std::io::Result<()> { + self.config.remove(name) + } + fn dir(&self) -> Option { + self.config.dir() + } + fn files(&self) -> &dyn crate::config::FileOps { + self + } +} + +impl crate::config::FileOps for AutosaveTestStore { + fn list(&self, dir: &std::path::Path) -> Vec<(String, std::time::SystemTime)> { + crate::config::StdFiles.list(dir) + } + fn write(&self, path: &std::path::Path, data: &[u8]) -> std::io::Result<()> { + if let Some((started, release)) = self.gate.lock().unwrap().take() { + started.send(()).unwrap(); + release.recv().unwrap(); + } + if self.fail_next.swap(false, std::sync::atomic::Ordering::SeqCst) { + return Err(std::io::Error::other("injected autosave write failure")); + } + crate::config::StdFiles.write(path, data) + } + fn remove(&self, path: &std::path::Path) -> std::io::Result<()> { + crate::config::StdFiles.remove(path) + } + fn is_file(&self, path: &std::path::Path) -> bool { + crate::config::StdFiles.is_file(path) + } +} + +fn background_autosave_session(dir: &std::path::Path) -> (Session, Arc) { + let store = Arc::new(AutosaveTestStore { + config: DirConfig::new(dir.join("config")), + fail_next: Default::default(), + gate: Default::default(), + release_on_prefs: Default::default(), + fail_reset_for: Default::default(), + failed_resets: Default::default(), + }); + let mut s = session_with_comp(); + s.config = Some(store.clone()); + s.prefs.auto_save.enabled = true; + s.prefs.auto_save.interval_minutes = 1; + s.prefs.auto_save.location = "custom".into(); + s.prefs.auto_save.folder = dir.join("saves").to_string_lossy().into_owned(); + s.autosave.background = true; + s.begin_recovery(); + (s, store) +} + +#[test] +fn background_autosave_failure_retries_the_same_revision_and_retains_recovery() { + let dir = tmp("autosave-retry"); + let (mut s, store) = background_autosave_session(&dir); + let first = s.autosave_now().unwrap(); + s.autosave_wait().unwrap(); + let committed = s.autosave.saved_revision; + s.execute("layer.newNull", json!({"name": "work to recover"})).unwrap(); + let revision = s.revision; + store.fail_next.store(true, std::sync::atomic::Ordering::SeqCst); + assert!(s.autosave_tick(1000.0).is_none()); + let failed_path = s.autosave_tick(1061.0).unwrap().unwrap(); + assert!(s.autosave_wait().unwrap_err().to_string().contains("injected autosave write failure")); + assert_eq!(s.autosave.saved_revision, committed); + assert_eq!(s.autosave.last_slot, Some(1)); + assert_eq!(s.autosave.last_path.as_deref(), Some(first.as_str())); + let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap(); + assert_eq!(sentinel.autosave.as_deref(), Some(first.as_str())); + assert!(!std::path::Path::new(&failed_path).is_file()); + let retried = s.autosave_tick(1122.0).expect("retry without another edit").unwrap(); + assert_eq!(retried, failed_path, "failed writes do not rotate the slot"); + s.autosave_wait().unwrap(); + assert_eq!(s.revision, revision); + assert_eq!(s.autosave.saved_revision, Some(revision)); + assert_eq!(s.autosave.last_slot, Some(2)); + assert!(std::fs::read_to_string(&retried).unwrap().contains("work to recover")); + let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap(); + assert_eq!(sentinel.autosave.as_deref(), Some(retried.as_str())); + assert!(s.autosave_tick(1183.0).is_none(), "committed revision needs no duplicate write"); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn background_autosave_commits_only_the_snapshot_that_finished() { + let dir = tmp("autosave-pending"); + let (mut s, store) = background_autosave_session(&dir); + let (started, started_rx) = std::sync::mpsc::channel(); + let (release, release_rx) = std::sync::mpsc::channel(); + *store.gate.lock().unwrap() = Some((started, release_rx)); + let revision = s.revision; + let path = s.autosave_now().unwrap(); + started_rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap(); + assert!(s.autosave_tick(1000.0).is_none()); + let saved = s.autosave.saved_revision; + let last_path = s.autosave.last_path.clone(); + let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap(); + s.execute("layer.newNull", json!({"name": "edited during save"})).unwrap(); + assert!(s.autosave_tick(1061.0).is_none(), "a pending write must not block the next tick or launch a duplicate"); + release.send(()).unwrap(); + s.autosave_wait().unwrap(); + assert_eq!(saved, None, "an in-flight save is not committed"); + assert_eq!(last_path, None); + assert_eq!(sentinel.autosave, None); + assert_eq!(s.autosave.saved_revision, Some(revision), "completion must not mark newer edits as saved"); + assert_eq!(s.autosave.last_path.as_deref(), Some(path.as_str())); + assert!(!std::fs::read_to_string(&path).unwrap().contains("edited during save")); + let newer = s.autosave_tick(1122.0).unwrap().unwrap(); + s.autosave_wait().unwrap(); + assert_eq!(s.autosave.saved_revision, Some(s.revision)); + assert!(std::fs::read_to_string(newer).unwrap().contains("edited during save")); + std::fs::remove_dir_all(dir).unwrap(); +} + +fn autosave_project_transition_with_failed_reset(command: &str, reopen: bool) { + let dir = tmp("autosave-project-transition"); + let a = dir.join("A.ecproj").to_string_lossy().into_owned(); + let b = if reopen { a.clone() } else { dir.join("B.ecproj").to_string_lossy().into_owned() }; + let (mut s, store) = background_autosave_session(&dir); + s.execute("file.saveAs", json!({"path": a})).unwrap(); + if !reopen { + let mut other = session_with_comp(); + other.execute("layer.newNull", json!({"name": "project B"})).unwrap(); + other.execute("file.saveAs", json!({"path": b})).unwrap(); + } + let b_bytes = std::fs::read(&b).unwrap(); + s.execute("layer.newNull", json!({"name": "unsaved project A"})).unwrap(); + s.update_sentinel(); + let (started, started_rx) = std::sync::mpsc::channel(); + let (release, release_rx) = std::sync::mpsc::channel(); + *store.gate.lock().unwrap() = Some((started, release_rx)); + let snapshot = s.autosave_now().unwrap(); + started_rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap(); + assert!(!s.autosave.pending.as_ref().unwrap().is_finished()); + let previous_record = store.read(autosave::SENTINEL).unwrap(); + *store.fail_reset_for.lock().unwrap() = Some(b.clone()); + *store.release_on_prefs.lock().unwrap() = Some(release); + s.execute(command, json!({"path": b})).unwrap(); + assert_eq!(s.path.as_deref(), Some(b.as_str())); + assert_eq!(store.failed_resets.load(std::sync::atomic::Ordering::SeqCst), 1, "the final recovery reset really failed"); + assert_eq!( + store.read(autosave::SENTINEL).as_deref(), + Some(previous_record.as_str()), + "completion must not publish A's snapshot under the switched project" + ); + assert_eq!(s.autosave.last_path, None); + assert_eq!(s.autosave.saved_revision, None); + if command == "file.open" { + assert_eq!(std::fs::read(&b).unwrap(), b_bytes, "opening B must not alter its file bytes"); + } + let prefs = s.prefs.clone(); + drop(s); // Simulate an unclean exit, leaving the failed-reset sentinel in place. + let mut restart = Session { config: Some(store), prefs, ..Default::default() }; + let recovery = restart.begin_recovery().expect("previous project remains recoverable"); + assert_eq!(recovery.project.as_deref(), Some(a.as_str())); + assert_eq!(recovery.autosave.as_deref(), Some(snapshot.as_str())); + assert!(recovery.dirty); + assert!(std::fs::read_to_string(snapshot).unwrap().contains("unsaved project A")); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn opening_another_project_during_autosave_keeps_recovery_consistent_if_the_reset_fails() { + autosave_project_transition_with_failed_reset("file.open", false); +} + +#[test] +fn reopening_the_same_path_during_autosave_keeps_recovery_consistent_if_the_reset_fails() { + autosave_project_transition_with_failed_reset("file.open", true); +} + +#[test] +fn save_as_during_autosave_keeps_recovery_consistent_if_the_reset_fails() { + autosave_project_transition_with_failed_reset("file.saveAs", false); +} + #[test] fn atomic_write_survives_a_crash_mid_write() { let d = tmp("atomic"); diff --git a/crates/engine/src/tests_stubs.rs b/crates/engine/src/tests_stubs.rs index edf773552..84321ee7e 100644 --- a/crates/engine/src/tests_stubs.rs +++ b/crates/engine/src/tests_stubs.rs @@ -476,3 +476,110 @@ fn implemented_stubs_are_enabled_and_workspaces_are_frontend() { assert!(!s.is_enabled(id), "{id}"); } } + +/// The first path item in a shape layer's Contents, at any depth: (uid, match_id). A new shape +/// layer nests it in a group ("Rectangle 1" ▸ "Rectangle Path 1"), as After Effects does. +fn first_shape(s: &Session, l: u64) -> (u64, String) { + fn walk(g: &effectcraft_project::PropGroup) -> Option<(u64, String)> { + if ["rect", "ellipse", "star", "path"].contains(&g.match_id.as_str()) { + return Some((g.uid, g.match_id.clone())); + } + g.groups().find_map(walk) + } + walk(layer(s, l).props.sub("contents").unwrap()).unwrap() +} + +fn shape_path_of(s: &Session, l: u64, uid: u64) -> effectcraft_keyframe::ShapePath { + let ly = layer(s, l); + let g = ly.props.sub("contents").unwrap().find_group(uid).unwrap(); + let _ = &g; + g.get("path").unwrap().value.as_path().unwrap().clone() +} + +/// Convert To Bezier Path on a shape layer's parametric path gives an editable Path 1 with the +/// same geometry, as After Effects does (#932). The command only turned RotoBezier off for masks, +/// so a Rectangle, Ellipse or Polystar path could not be converted at all. +#[test] +fn convert_to_bezier_turns_a_parametric_shape_into_an_editable_path() { + let convert = |args: serde_json::Value| { + let mut s = comp(); + let l = s.execute("layer.newShape", args).unwrap()["layer"].as_u64().unwrap(); + let (uid, was) = first_shape(&s, l); + assert_ne!(was, "path", "it starts out parametric"); + s.state.selected_props = vec![(LayerId(l), uid)]; + let out = s.execute("path.convertToBezier", json!({})).unwrap(); + assert_eq!(out["paths"].as_u64(), Some(1)); + let (new_uid, now) = first_shape(&s, l); + assert_eq!(now, "path", "it is a bezier path now"); + // The new path is selected, so its vertices can be edited straight away. + assert_eq!(s.state.selected_props, vec![(LayerId(l), new_uid)]); + let path = shape_path_of(&s, l, new_uid); + assert!(path.closed, "a closed shape stays closed"); + (s, l, was, path) + }; + let radius = |v: [f64; 2]| v[0].hypot(v[1]); + + // A 120 x 80 rectangle is its four corners, joined by straight edges. + let (mut s, l, was, rect) = convert(json!({"kind": "rect", "size": [120, 80]})); + assert_eq!(rect.vertices, vec![[-60.0, -40.0], [60.0, -40.0], [60.0, 40.0], [-60.0, 40.0]]); + assert!(rect.in_tangents.iter().chain(&rect.out_tangents).all(|t| *t == [0.0, 0.0]), "straight edges: {rect:?}"); + // Undo brings the parametric shape back. + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(first_shape(&s, l).1, was, "undo restores the parametric shape"); + + // An ellipse is four cubic arcs: a vertex on each axis, with tangents of 0.5523 x the radius + // (the usual circle approximation). Straight lines here would be a diamond. + let (_, _, _, ell) = convert(json!({"kind": "ellipse", "size": [120, 80]})); + assert_eq!(ell.vertices, vec![[0.0, -40.0], [60.0, 0.0], [0.0, 40.0], [-60.0, 0.0]]); + let kappa = 4.0 / 3.0 * (std::f64::consts::SQRT_2 - 1.0); + for (i, want) in [60.0 * kappa, 40.0 * kappa, 60.0 * kappa, 40.0 * kappa].into_iter().enumerate() { + let t = ell.in_tangents[i]; + assert!((radius(t) - want).abs() < 1e-6, "vertex {i}: tangent {t:?} should be {want}"); + } + + // A five-point star alternates the outer and inner radius, so ten vertices. + let (_, _, _, star) = convert(json!({"kind": "star", "size": [120, 120]})); + assert_eq!(star.vertices.len(), 10); + for (i, v) in star.vertices.iter().enumerate() { + let want = if i % 2 == 0 { 60.0 } else { 30.0 }; + assert!((radius(*v) - want).abs() < 1e-6, "vertex {i} at {v:?} should be {want} from the centre"); + } + + // A polygon has one vertex per point, all at the outer radius. + let (_, _, _, poly) = convert(json!({"kind": "polygon", "size": [120, 120]})); + assert!(poly.vertices.len() >= 3); + assert!(poly.vertices.iter().all(|v| (radius(*v) - 60.0).abs() < 1e-6), "{:?}", poly.vertices); + assert_ne!(poly.vertices.len(), star.vertices.len(), "a polygon is not a star"); +} + +/// The shape's own values are read, not defaults: Roundness rounds the corners and Position moves +/// the path, both at the time the conversion is made. +#[test] +fn convert_to_bezier_reads_the_shape_as_it_is_set() { + let mut s = comp(); + let l = s.execute("layer.newShape", json!({"kind": "rect", "size": [120, 80]})).unwrap()["layer"].as_u64().unwrap(); + let (uid, _) = first_shape(&s, l); + // Properties are addressed by their own uid. + let pr_uid = |s: &Session, id: &str| { + let ly = layer(s, l); + ly.props.sub("contents").unwrap().find_group(uid).unwrap().get(id).unwrap().uid + }; + let (round, pos) = (pr_uid(&s, "roundness"), pr_uid(&s, "position")); + s.execute("prop.set", json!({"layer": l, "path": format!("@{round}"), "value": 20.0})).unwrap(); + s.execute("prop.set", json!({"layer": l, "path": format!("@{pos}"), "value": [10.0, 5.0]})).unwrap(); + + s.state.selected_props = vec![(LayerId(l), uid)]; + s.execute("path.convertToBezier", json!({})).unwrap(); + let (new_uid, now) = first_shape(&s, l); + assert_eq!(now, "path"); + let path = shape_path_of(&s, l, new_uid); + + // Rounded corners need more than four vertices, and curves between them. + assert!(path.vertices.len() > 4, "roundness rounds the corners: {} vertices", path.vertices.len()); + assert!(path.in_tangents.iter().chain(&path.out_tangents).any(|t| *t != [0.0, 0.0]), "with curves"); + // Position moves the path: the bounds are centred on it, not on the origin. + let xs: Vec = path.vertices.iter().map(|v| v[0]).collect(); + let ys: Vec = path.vertices.iter().map(|v| v[1]).collect(); + let mid = |v: &[f64]| (v.iter().copied().fold(f64::MAX, f64::min) + v.iter().copied().fold(f64::MIN, f64::max)) / 2.0; + assert!((mid(&xs) - 10.0).abs() < 1e-6 && (mid(&ys) - 5.0).abs() < 1e-6, "centred on the position: {:?} {:?}", mid(&xs), mid(&ys)); +} diff --git a/crates/engine/src/tests_text.rs b/crates/engine/src/tests_text.rs index a57f87a70..84f88fd33 100644 --- a/crates/engine/src/tests_text.rs +++ b/crates/engine/src/tests_text.rs @@ -2,7 +2,7 @@ //! undo/redo and serde round-trips). use effectcraft_keyframe::Value as KV; -use effectcraft_project::{Node, Project, PropGroup}; +use effectcraft_project::{LayerId, Node, Project, PropGroup}; use serde_json::json; use crate::Session; @@ -244,3 +244,27 @@ fn menus_have_text_animation_entries() { assert!(s.is_enabled("layer.enablePerChar3D")); assert!(!s.is_enabled("layer.addTextSelector"), "needs an animator"); } + +/// Duplicate with a Range Selector selected under a text animator copies the selector, not the +/// whole text layer (#1665); a selected animator is copied the same way. +#[test] +fn duplicate_copies_selected_text_selector_not_the_layer() { + let (mut s, t) = session(); + s.execute("layer.addTextAnimator", json!({"layer": t, "properties": ["tracking"]})).unwrap(); + let anims = |s: &Session| s.active_comp().unwrap().layer(LayerId(t)).unwrap().props.group("text/animators").unwrap().clone(); + let selectors = |s: &Session| -> Vec { anims(s).groups().next().unwrap().sub("selectors").unwrap().groups().map(|g| g.uid).collect() }; + let sel = selectors(&s)[0]; + + s.execute("prop.select", json!({"layer": t, "prop": sel, "selectKeys": false})).unwrap(); + s.execute("edit.duplicate", json!({})).unwrap(); + assert_eq!(s.active_comp().unwrap().layers.len(), 1, "no second text layer"); + assert_eq!(selectors(&s).len(), 2, "the selector is copied"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(selectors(&s), vec![sel]); + + let anim = anims(&s).groups().next().unwrap().uid; + s.execute("prop.select", json!({"layer": t, "prop": anim, "selectKeys": false})).unwrap(); + s.execute("edit.duplicate", json!({})).unwrap(); + assert_eq!(s.active_comp().unwrap().layers.len(), 1); + assert_eq!(anims(&s).groups().count(), 2, "the animator is copied"); +} diff --git a/crates/engine/src/tests_text_edit.rs b/crates/engine/src/tests_text_edit.rs index 3e41688d2..243a9c676 100644 --- a/crates/engine/src/tests_text_edit.rs +++ b/crates/engine/src/tests_text_edit.rs @@ -317,6 +317,66 @@ fn fonts_filter_variable_families() { assert!(menu.iter().any(|r| r.family == "Inter" && !r.variable)); } +/// A variable font of `name` and `fvar` only: one `wght` axis 100..900 (default 300, style +/// `Light`) and the named `instances` (style, `wght`), each with a PostScript name +/// `-