diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 000000000..2c4b8cb1a --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,30 @@ +version: 2 +updates: + # Maintain Rust / Cargo dependencies + - package-ecosystem: "cargo" + directory: "/" + schedule: + interval: "daily" + time: "06:00" + timezone: "America/New_York" + # Consolidate updates into a single PR rather than one PR per crate + groups: + cargo-dependencies: + patterns: + - "*" + commit-message: + prefix: "chore(deps)" + + # Maintain GitHub Actions workflows + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" + time: "06:00" + timezone: "America/New_York" + groups: + actions: + patterns: + - "*" + commit-message: + prefix: "ci(deps)" diff --git a/Cargo.lock b/Cargo.lock index 933e32951..412fb6672 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1163,7 +1163,7 @@ dependencies = [ [[package]] name = "effectcraft" -version = "0.11.1" +version = "0.12.0" dependencies = [ "cpal", "effectcraft-engine", @@ -1188,7 +1188,7 @@ dependencies = [ [[package]] name = "effectcraft-automation" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-engine", "effectcraft-host", @@ -1200,14 +1200,14 @@ dependencies = [ [[package]] name = "effectcraft-av1enc" -version = "0.11.1" +version = "0.12.0" dependencies = [ "filmcraft-av1", ] [[package]] name = "effectcraft-cli" -version = "0.11.1" +version = "0.12.0" dependencies = [ "cpu-time", "effectcraft-automation", @@ -1223,7 +1223,7 @@ dependencies = [ [[package]] name = "effectcraft-color" -version = "0.11.1" +version = "0.12.0" dependencies = [ "proptest", "serde", @@ -1231,7 +1231,7 @@ dependencies = [ [[package]] name = "effectcraft-effects" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1248,7 +1248,7 @@ dependencies = [ [[package]] name = "effectcraft-engine" -version = "0.11.1" +version = "0.12.0" dependencies = [ "craft-docpath", "effectcraft-color", @@ -1282,7 +1282,7 @@ dependencies = [ [[package]] name = "effectcraft-export" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-av1enc", "effectcraft-color", @@ -1312,7 +1312,7 @@ dependencies = [ [[package]] name = "effectcraft-expr" -version = "0.11.1" +version = "0.12.0" dependencies = [ "boa_engine", "effectcraft-color", @@ -1329,7 +1329,7 @@ dependencies = [ [[package]] name = "effectcraft-geom" -version = "0.11.1" +version = "0.12.0" dependencies = [ "proptest", "serde", @@ -1337,7 +1337,7 @@ dependencies = [ [[package]] name = "effectcraft-gpu" -version = "0.11.1" +version = "0.12.0" dependencies = [ "bytemuck", "effectcraft-color", @@ -1359,14 +1359,14 @@ dependencies = [ [[package]] name = "effectcraft-hevcenc" -version = "0.11.1" +version = "0.12.0" dependencies = [ "filmcraft-hevc", ] [[package]] name = "effectcraft-host" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-engine", "effectcraft-export", @@ -1391,7 +1391,7 @@ dependencies = [ [[package]] name = "effectcraft-interchange" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-keyframe", @@ -1410,7 +1410,7 @@ dependencies = [ [[package]] name = "effectcraft-keyframe" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1422,7 +1422,7 @@ dependencies = [ [[package]] name = "effectcraft-lottie" -version = "0.11.1" +version = "0.12.0" dependencies = [ "craft-docpath", "effectcraft-color", @@ -1440,7 +1440,7 @@ dependencies = [ [[package]] name = "effectcraft-media" -version = "0.11.1" +version = "0.12.0" dependencies = [ "craft-docpath", "effectcraft-model", @@ -1469,7 +1469,7 @@ dependencies = [ [[package]] name = "effectcraft-model" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-geom", "image", @@ -1481,14 +1481,14 @@ dependencies = [ [[package]] name = "effectcraft-opusenc" -version = "0.11.1" +version = "0.12.0" dependencies = [ "filmcraft-opus", ] [[package]] name = "effectcraft-path" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-geom", "effectcraft-keyframe", @@ -1500,7 +1500,7 @@ dependencies = [ [[package]] name = "effectcraft-pdf" -version = "0.11.1" +version = "0.12.0" dependencies = [ "craft-pdf", "effectcraft-raster", @@ -1516,7 +1516,7 @@ dependencies = [ [[package]] name = "effectcraft-plugin" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-effects", "effectcraft-keyframe", @@ -1528,7 +1528,7 @@ dependencies = [ [[package]] name = "effectcraft-project" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1541,14 +1541,14 @@ dependencies = [ [[package]] name = "effectcraft-psd" -version = "0.11.1" +version = "0.12.0" dependencies = [ "miniz_oxide 0.8.9", ] [[package]] name = "effectcraft-raster" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-geom", @@ -1557,7 +1557,7 @@ dependencies = [ [[package]] name = "effectcraft-render" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-color", "effectcraft-effects", @@ -1579,7 +1579,7 @@ dependencies = [ [[package]] name = "effectcraft-script" -version = "0.11.1" +version = "0.12.0" dependencies = [ "boa_engine", "effectcraft-engine", @@ -1596,7 +1596,7 @@ dependencies = [ [[package]] name = "effectcraft-segment" -version = "0.11.1" +version = "0.12.0" dependencies = [ "miniz_oxide 0.8.9", "ndarray", @@ -1605,7 +1605,7 @@ dependencies = [ [[package]] name = "effectcraft-svg" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-geom", "effectcraft-path", @@ -1617,7 +1617,7 @@ dependencies = [ [[package]] name = "effectcraft-text" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-keyframe", "harfrust", @@ -1629,7 +1629,7 @@ dependencies = [ [[package]] name = "effectcraft-time" -version = "0.11.1" +version = "0.12.0" dependencies = [ "proptest", "serde", @@ -1637,7 +1637,7 @@ dependencies = [ [[package]] name = "effectcraft-track" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-raster", "effectcraft-segment", @@ -1649,7 +1649,7 @@ dependencies = [ [[package]] name = "effectcraft-ui-egui" -version = "0.11.1" +version = "0.12.0" dependencies = [ "craft-ui", "effectcraft-color", @@ -1681,11 +1681,11 @@ dependencies = [ [[package]] name = "effectcraft-vp9enc" -version = "0.11.1" +version = "0.12.0" [[package]] name = "effectcraft-web" -version = "0.11.1" +version = "0.12.0" dependencies = [ "effectcraft-engine", "effectcraft-export", @@ -6625,7 +6625,7 @@ checksum = "e450f9b2ed1dff33c94c12589a87338689467b9c4f5d8a5710bd09a847d2c8a7" [[package]] name = "xtask" -version = "0.11.1" +version = "0.12.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 25fb0399e..7af58f009 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/*"] [workspace.package] -version = "0.11.1" +version = "0.12.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" 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 43e514048..c7a012c76 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 @@ -106,6 +108,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; @@ -189,6 +193,7 @@ const FLAGS: &[&str] = &[ "--small", "--adv3d", "--autosave", + "--progress", ]; struct Args { @@ -772,8 +777,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 30a0271b6..1111db403 100644 --- a/apps/effectcraft-cli/tests/cli.rs +++ b/apps/effectcraft-cli/tests/cli.rs @@ -593,3 +593,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 2f99bd5e0..a1fb6e4a5 100644 --- a/apps/effectcraft/src/main.rs +++ b/apps/effectcraft/src/main.rs @@ -23,8 +23,8 @@ mod control_server; mod launch_guard; #[cfg(target_os = "macos")] mod native_menu; -// 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 window; @@ -122,6 +122,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 @@ -301,11 +317,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)); } @@ -597,4 +613,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 577eb0b64..7a75ef2a7 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/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/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 422728ea3..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); } diff --git a/crates/engine/src/commands/file.rs b/crates/engine/src/commands/file.rs index 760c1f223..8c4c384ea 100644 --- a/crates/engine/src/commands/file.rs +++ b/crates/engine/src/commands/file.rs @@ -404,6 +404,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. 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/mod.rs b/crates/engine/src/commands/mod.rs index 685d4dad8..23388594b 100644 --- a/crates/engine/src/commands/mod.rs +++ b/crates/engine/src/commands/mod.rs @@ -442,6 +442,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/lib.rs b/crates/engine/src/lib.rs index a5b016b74..cdfc7468e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -345,6 +345,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 { @@ -864,11 +869,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); @@ -987,15 +988,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) { @@ -1232,6 +1242,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/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/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_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_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 58819f55c..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. 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 +/// `-