diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 18392587..01cb9cd6 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -18,16 +18,6 @@ concurrency: cancel-in-progress: true jobs: - eval: - runs-on: ubuntu-latest - timeout-minutes: 15 - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: cachix/install-nix-action@13d8dd58da0234aa297dedd986986ccb8e7f3e24 # v31.11.1 - - name: nix flake check --no-build - run: | - if [ -f flake.nix ]; then - nix flake check --no-build --all-systems - else - echo "No flake.nix present in this repository; skipping check." - fi + nix: + # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. + uses: storytold/craft-common-actions/.github/workflows/nix.yml@main diff --git a/Cargo.lock b/Cargo.lock index b6ae1910..3b70881e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1455,6 +1455,7 @@ dependencies = [ "log", "rayon", "serde_json", + "symphonia", "thiserror 2.0.21", ] diff --git a/Cargo.toml b/Cargo.toml index 5bcb4bf1..2eba7988 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,6 +92,9 @@ filmcraft-av1 = { git = "https://github.com/storytold/filmcraft", rev = "2c5ba7a filmcraft-interchange = { git = "https://github.com/storytold/filmcraft", rev = "2c5ba7a72619935f0481f99d59409e0d1844b234" } filmcraft-project = { git = "https://github.com/storytold/filmcraft", rev = "2c5ba7a72619935f0481f99d59409e0d1844b234" } filmcraft-geom = { git = "https://github.com/storytold/filmcraft", rev = "2c5ba7a72619935f0481f99d59409e0d1844b234" } +# The symphonia filmcraft-codecs already builds, named only to turn on its AIFF reader: the pinned +# rev enables wav alone (FilmCraft main enables aiff too). Drop it when the pin moves past that. +symphonia = { version = "0.5.5", default-features = false, features = ["aiff"] } # gpu (the same wgpu eframe / egui-wgpu use) wgpu = { version = "30.0.1", features = ["fragile-send-sync-non-atomic-wasm"] } diff --git a/apps/effectcraft-cli/src/main.rs b/apps/effectcraft-cli/src/main.rs index fe523807..43e51404 100644 --- a/apps/effectcraft-cli/src/main.rs +++ b/apps/effectcraft-cli/src/main.rs @@ -263,8 +263,18 @@ fn value_arg(s: &str) -> Value { serde_json::from_str(s).unwrap_or_else(|_| json!(s)) } +/// The command line as text. `std::env::args` panics on an argument that is not Unicode (a +/// Latin-1 file name on Linux). Every argument is used as a string, and a lossy copy would read or +/// write another file than the one named, so such an argument is a usage error naming it. +fn utf8_args(raw: impl IntoIterator) -> Result, String> { + raw.into_iter().map(|a| a.into_string().map_err(|a| format!("argument `{}` is not valid Unicode", a.to_string_lossy()))).collect() +} + fn main() { - let raw: Vec = std::env::args().skip(1).collect(); + let raw = match utf8_args(std::env::args_os().skip(1)) { + Ok(raw) => raw, + Err(e) => fail_usage(&e), + }; if matches!(raw.first().map(String::as_str), Some("--version" | "-V" | "version")) { say!("effectcraft-cli {}", env!("CARGO_PKG_VERSION")); return; @@ -493,6 +503,9 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { }; let mut b = backend(args, true)?; let r = b.exec(&id, params)?; + if let Some(e) = failed_render(&id, &r) { + return Err(Failure::Error(e)); + } let saved = maybe_save(&mut b, args)?; emit(&with_saved(r, saved), json_out); } @@ -515,6 +528,9 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> { }; i += 1; let r = b.exec(id, params).map_err(|e| Failure::Error(format!("{id}: {e}")))?; + if let Some(e) = failed_render(id, &r) { + return Err(Failure::Error(format!("{id}: {e}"))); + } if !json_out { emit(&r, false); } @@ -637,6 +653,21 @@ fn positional(args: &Args, usage: &str) -> Result<[String; N], F Ok(std::array::from_fn(|i| args.pos[i].clone())) } +/// `renderQueue.render` reports a failed item in its result, not as an error. Under `run`/`exec` +/// that item fails the command (#1426), as it fails `render --queue`. +fn failed_render(id: &str, r: &Value) -> Option { + if id != "renderQueue.render" { + return None; + } + let failed: Vec = r["items"] + .as_array() + .into_iter() + .flatten() + .filter_map(|i| Some(format!("{}: {}", i["compName"].as_str().unwrap_or("?"), i["status"].get("Failed")?.as_str().unwrap_or("failed")))) + .collect(); + (!failed.is_empty()).then(|| failed.join("; ")) +} + fn with_saved(v: Value, saved: Option) -> Value { match saved { None => v, @@ -660,11 +691,10 @@ fn render(args: &Args, json_out: bool) -> Result<(), Failure> { let mut s = session(args)?; if args.opt("--template").is_some() { template::prepare(&mut s, args)?; - } else { - match &args.project { - Some(p) => s.execute("file.open", json!({"path": p})).map_err(|e| Failure::Error(e.to_string()))?, - None => s.execute("file.openDemoProject", json!({})).map_err(|e| Failure::Error(e.to_string()))?, - }; + } else if let Some(p) = &args.project { + s.execute("file.open", json!({"path": p})).map_err(|e| Failure::Error(e.to_string()))?; + } else if args.flag("--demo") || !args.flag("--empty") { + s.execute("file.openDemoProject", json!({})).map_err(|e| Failure::Error(e.to_string()))?; } let err = |e: effectcraft_engine::EngineError| Failure::Error(e.to_string()); if !args.flag("--queue") { @@ -1270,6 +1300,18 @@ mod tests { assert!(a.flag("--work-area") && a.flag("--json")); } + /// `std::env::args` panicked on an argument that is not Unicode; it is a usage error naming + /// it, and Unicode arguments pass through unchanged. + #[cfg(unix)] + #[test] + fn non_unicode_arguments_are_refused() { + use std::ffi::OsString; + use std::os::unix::ffi::OsStringExt; + let args = [OsString::from("render-frame"), OsString::from("--out"), OsString::from_vec(b"caf\xe9.png".to_vec())]; + assert_eq!(utf8_args(args).unwrap_err(), "argument `caf\u{fffd}.png` is not valid Unicode"); + assert_eq!(utf8_args([OsString::from("info"), OsString::from("caf\u{e9}.png")]).unwrap(), ["info", "caf\u{e9}.png"]); + } + /// Every `--option` the CLI reads is declared in VALUED or FLAGS, so none is refused. #[test] fn every_option_read_is_declared() { diff --git a/apps/effectcraft-cli/src/template.rs b/apps/effectcraft-cli/src/template.rs index f4da23cd..f84303dd 100644 --- a/apps/effectcraft-cli/src/template.rs +++ b/apps/effectcraft-cli/src/template.rs @@ -36,7 +36,20 @@ pub(super) fn validate(cmd: &str, args: &Args) -> Result<(), Failure> { { for input in ["--template", "--values"] { if let Some(path) = args.opt(input) - && std::fs::canonicalize(path).is_ok_and(|path| path == output) + && std::fs::canonicalize(path).is_ok_and(|path| { + if path == output { + return true; + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + // Distinct canonical paths can still name the same file through hard links. + if let (Ok(input), Ok(output)) = (std::fs::metadata(&path), std::fs::metadata(&output)) { + return input.dev() == output.dev() && input.ino() == output.ino(); + } + } + false + }) { return Err(Failure::Error(format!("--out {out} would overwrite {input} {path}"))); } diff --git a/apps/effectcraft-cli/tests/cli.rs b/apps/effectcraft-cli/tests/cli.rs index bd5d5002..30a0271b 100644 --- a/apps/effectcraft-cli/tests/cli.rs +++ b/apps/effectcraft-cli/tests/cli.rs @@ -167,6 +167,21 @@ fn unknown_options_are_usage_errors() { } } +/// An argument that is not Unicode (a Latin-1 file name on Linux) is a usage error (exit 2) +/// naming it, before anything runs: `std::env::args` panicked on one (exit 101). +#[cfg(unix)] +#[test] +fn non_unicode_arguments_are_usage_errors() { + use std::os::unix::ffi::OsStrExt; + let name = std::ffi::OsStr::from_bytes(b"caf\xe9.png"); + let out = bin().args(["render-frame", "--out"]).arg(name).output().unwrap(); + let err = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code(), Some(2), "{err}"); + assert!(err.contains("argument `caf\u{fffd}.png` is not valid Unicode"), "{err}"); + assert!(!err.contains("panicked"), "{err}"); + assert!(out.stdout.is_empty(), "nothing ran"); +} + /// A reader that closes stdout before the CLI writes (`| head`) is not a crash: the command /// still does its work and exits 0, without a panic (#167). #[test] @@ -469,6 +484,32 @@ fn mcp_autosave_is_opt_in_and_rejects_invalid_combinations() { std::fs::remove_dir_all(config).unwrap(); } +/// `run`/`exec renderQueue.render` (#1426): a failed item is a command error, exit 1 like +/// `render --queue`, not a success with the failure buried in the JSON. +#[test] +fn a_failed_render_under_run_and_exec_exits_1() { + let root = tmp("render-fails"); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + // The output's folder is a file, so the encoder cannot open the output. + let blocker = root.join("not-a-folder"); + std::fs::write(&blocker, "x").unwrap(); + let out = blocker.join("out.gif"); + let add = json!({"comp": "T", "format": "gif", "output": out.to_str().unwrap()}).to_string(); + let project = root.join("q.ecproj"); + let steps = ["comp.new", r#"{"name":"T","width":16,"height":16,"duration":0.1}"#, "layer.newSolid", "{}", "renderQueue.add", &add]; + ok_json(&[&["run", "--empty", "--save-as", project.to_str().unwrap()][..], &steps[..]].concat()); + + let (code, v) = run_json(&[&["run", "--empty"][..], &steps[..], &["renderQueue.render", r#"{"wait":true}"#][..]].concat()); + assert_eq!(code, 1, "{v}"); + let e = v["error"].as_str().unwrap_or_else(|| panic!("{v}")); + assert!(e.starts_with("renderQueue.render: T: "), "{e}"); + + let (code, v) = run_json(&["exec", "renderQueue.render", "--project", project.to_str().unwrap()]); + assert_eq!(code, 1, "{v}"); + assert!(v["error"].as_str().is_some_and(|e| e.starts_with("T: ")), "{v}"); +} + /// `render --bitrate` (#440) targets the bitrate in VP9 WebM. #[test] fn render_bitrate_applies_to_vp9_webm() { @@ -498,3 +539,57 @@ fn render_bitrate_applies_to_vp9_webm() { assert!(high > low, "100 kbps: {low} bytes, 8000 kbps: {high} bytes"); let _ = std::fs::remove_dir_all(&root); } + +/// Render uses the same empty/project/demo selection as the other CLI commands. +#[test] +fn render_empty_does_not_load_the_demo() { + let root = tmp("render-empty"); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + let project = root.join("control.ecproj"); + let template = root.join("control.ectemplate"); + let project_path = project.to_str().unwrap(); + let template_path = template.to_str().unwrap(); + let build = r#" +var comp = app.project.items.addComp("Empty control", 64, 36, 1, 1, 30); +var solid = comp.layers.addSolid([0, 1, 0], "Green", 64, 36, 1); +solid.property("ADBE Transform Group").property("ADBE Opacity").addToMotionGraphicsTemplateAs(comp, "Opacity"); +"#; + ok_json(&["script", "--eval", build, "--save-as", project_path]); + let params = json!({"comp": "Empty control", "path": template_path}).to_string(); + let exported = ok_json(&["exec", "essential.exportTemplate", ¶ms, "--project", project_path]); + assert_eq!(exported["controls"], 1, "{exported}"); + let empty = ok_json(&["info", "--empty"]); + assert!(empty["project"]["items"].as_array().unwrap().is_empty(), "{empty}"); + + let render = |options: &[&str], name: &str| { + let output = root.join(name); + assert!(!output.exists()); + let mut args = vec!["render", "--format", "gif", "--start", "0", "--end", "0.1", "--resolution", "quarter", "--out", output.to_str().unwrap()]; + args.extend_from_slice(options); + let reply = ok_json(&args); + assert_eq!(reply["rendered"].as_array().unwrap().len(), 1, "{reply}"); + assert_eq!(PathBuf::from(reply["rendered"][0]["output"].as_str().unwrap()), output); + let bytes = std::fs::read(&output).unwrap(); + assert!(bytes.len() >= 10 && (bytes.starts_with(b"GIF87a") || bytes.starts_with(b"GIF89a")), "{reply}"); + let dimensions = (u16::from_le_bytes([bytes[6], bytes[7]]), u16::from_le_bytes([bytes[8], bytes[9]])); + (reply["rendered"][0]["comp"].clone(), dimensions) + }; + // Defaults still render the demo, and an explicit demo wins over --empty. + let demo = render(&[], "default.gif"); + assert_eq!(render(&["--demo", "--empty"], "demo.gif"), demo); + // A supplied project wins over both flags, while template mode also accepts --empty. + let project_result = render(&["--project", project_path, "--demo", "--empty"], "project.gif"); + assert_eq!(project_result, (json!("Empty control"), (16, 9))); + assert_eq!(render(&["--template", template_path, "--empty"], "template.gif").1, (16, 9)); + + let output = root.join("empty.gif"); + assert!(!output.exists()); + let (code, reply) = + run_json(&["render", "--empty", "--format", "gif", "--start", "0", "--end", "0.1", "--resolution", "quarter", "--out", output.to_str().unwrap()]); + let output_exists = output.exists(); + assert!(!output_exists, "--empty must not render the demo into the requested output"); + assert_eq!(code, 1, "{reply}"); + 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); +} diff --git a/apps/effectcraft-cli/tests/qa_template.rs b/apps/effectcraft-cli/tests/qa_template.rs index ac67280e..2765b163 100644 --- a/apps/effectcraft-cli/tests/qa_template.rs +++ b/apps/effectcraft-cli/tests/qa_template.rs @@ -247,7 +247,7 @@ fn template_output_alias_fixture(case: &str) -> PathBuf { dir } -fn assert_template_output_preserves_inputs(dir: &PathBuf, out: &str) { +fn assert_template_output_preserves_inputs(dir: &PathBuf, out: &str) -> Value { let template_before = std::fs::read(dir.join("lt.ectemplate")).unwrap(); let values_before = std::fs::read(dir.join("values.json")).unwrap(); // Read the inputs before interpreting the subprocess status or error text. @@ -272,7 +272,8 @@ fn assert_template_output_preserves_inputs(dir: &PathBuf, out: &str) { ); assert!(template_preserved, "template rendering must preserve its template input when --out aliases that file"); assert!(values_preserved, "template rendering must preserve its values input when --out aliases that file"); - assert!(!result.status.success(), "an output alias must fail before rendering overwrites an input"); + assert_eq!(result.status.code(), Some(1), "an output alias must fail before rendering overwrites an input"); + serde_json::from_slice(&result.stdout).unwrap() } #[test] @@ -652,3 +653,39 @@ fn template_missing_output_does_not_replace_extracted_media() { assert!(media_preserved, "missing --out must fail before overwriting previously extracted media"); let _ = std::fs::remove_dir_all(&dir); } + +#[cfg(unix)] +#[test] +fn template_frame_output_cannot_alias_its_template_through_a_hard_link() { + use std::os::unix::fs::MetadataExt; + + let dir = template_output_alias_fixture("hard-linked-template"); + let input = dir.join("lt.ectemplate"); + let output = dir.join("hard-linked-template.png"); + std::fs::hard_link(&input, &output).unwrap(); + let input_metadata = std::fs::metadata(&input).unwrap(); + let output_metadata = std::fs::metadata(&output).unwrap(); + assert_eq!((input_metadata.dev(), input_metadata.ino()), (output_metadata.dev(), output_metadata.ino()), "actual hard-link file identity"); + assert_ne!(input.canonicalize().unwrap(), output.canonicalize().unwrap(), "the hard link has a distinct canonical path"); + let reply = assert_template_output_preserves_inputs(&dir, "hard-linked-template.png"); + assert_eq!(reply["error"], "--out hard-linked-template.png would overwrite --template lt.ectemplate", "{reply}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[cfg(unix)] +#[test] +fn template_frame_output_cannot_alias_its_values_through_a_hard_link() { + use std::os::unix::fs::MetadataExt; + + let dir = template_output_alias_fixture("hard-linked-values"); + let input = dir.join("values.json"); + let output = dir.join("hard-linked-values.png"); + std::fs::hard_link(&input, &output).unwrap(); + let input_metadata = std::fs::metadata(&input).unwrap(); + let output_metadata = std::fs::metadata(&output).unwrap(); + assert_eq!((input_metadata.dev(), input_metadata.ino()), (output_metadata.dev(), output_metadata.ino()), "actual hard-link file identity"); + assert_ne!(input.canonicalize().unwrap(), output.canonicalize().unwrap(), "the hard link has a distinct canonical path"); + let reply = assert_template_output_preserves_inputs(&dir, "hard-linked-values.png"); + assert_eq!(reply["error"], "--out hard-linked-values.png would overwrite --values values.json", "{reply}"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/apps/effectcraft-web/src/files.rs b/apps/effectcraft-web/src/files.rs index 76b79912..3f3f3a99 100644 --- a/apps/effectcraft-web/src/files.rs +++ b/apps/effectcraft-web/src/files.rs @@ -144,6 +144,10 @@ impl Services for WebServices { let dir = dir.trim_end_matches('/'); list().into_iter().map(|(p, _)| p).filter(|p| p.rsplit_once('/').is_some_and(|(d, _)| d == dir)).collect() } + /// The table's files (the default asks `std::fs`, which has no files on wasm32). + fn exists(&self, path: &str) -> bool { + get(path).is_some() + } fn is_dir(&self, _path: &str) -> bool { false } diff --git a/apps/effectcraft-web/tests/smoke.mjs b/apps/effectcraft-web/tests/smoke.mjs index ee272a67..3a6c1337 100644 --- a/apps/effectcraft-web/tests/smoke.mjs +++ b/apps/effectcraft-web/tests/smoke.mjs @@ -441,6 +441,18 @@ try { const comps = await js(`effectcraft.inspect().then(i => i.activeComp)`); if (!comps) throw new Error("reopened project has no active comp"); report.steps.reopen = { ...opened, activeComp: comps }; + + // Files the page holds (browser storage, no std::fs): a script opens one next to the project, + // and the footage check finds the imported media instead of marking it missing. + const hf = await js(`(async () => { + const p = await effectcraft.addFile(new File(["from the page"], "script-data.txt")); + await effectcraft.saveToBrowser("/host-files.ecproj"); + const run = await effectcraft.execute("script.run", {code: "var f = new File(" + JSON.stringify(p) + "); writeLn(f.open('r') ? f.read() : f.error);"}); + return {output: run.output, footage: await effectcraft.execute("footage.check", {wait: true})}; + })()`); + report.steps.hostFiles = hf; + check(hf.output === "from the page", `a script opens a file the page holds (${hf.output})`); + check(hf.footage.checked > 0 && hf.footage.missing === 0, `stored footage isn't missing (${JSON.stringify(hf.footage)})`); report.steps.final = { screenshot: await shot("02-after") }; // Persistence: the session, imported media, settings and a project saved to browser storage diff --git a/contributors/people.toml b/contributors/people.toml index 3e6a78c3..2fa1927c 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -75,10 +75,15 @@ sync_github_name = true real_name = "Robert Lee" sync_github_name = true -[people.moecr7] +[people.onlymoein] real_name = "Moein Saharkhiz" display_name = "onlymoein" [people.arnav-hiwarkar] display_name = "arnav-hiwarkar" sync_github_name = false + +[people.marcoarnulfo] +real_name = "Marco Arnulfo" +display_name = "Marco Arnulfo" +sync_github_name = false diff --git a/crates/effects/src/catalog.rs b/crates/effects/src/catalog.rs index 82678eb8..0be2b04f 100644 --- a/crates/effects/src/catalog.rs +++ b/crates/effects/src/catalog.rs @@ -98,6 +98,9 @@ pub fn gpu_supported(id: &str, ctx: &crate::EffectCtx) -> bool { match id { "ec.generate.cellpattern" if (6..=10).contains(&ctx.params.e("cellPattern")) => return false, "ec.distort.turbulentdisplace" if ctx.params.e("pinning") >= 8 => return false, + // #1195: CC Particle World's motion trails (the averaged shutter samples) render on the + // CPU kernel until the GPU particle pass supports them. + "ec.sim.ccparticleworld" if ctx.env.shutter.filter(|(a, _, _)| *a > 0.0).is_some() => return false, _ => {} } let Some((_, controls)) = CPU_ONLY_CONTROLS.iter().find(|(e, _)| *e == id) else { return true }; @@ -199,6 +202,14 @@ mod tests { assert!(ok("ec.color.lumetri", &[])); assert!(!ok("ec.color.lumetri", &[("hslSecondary/correction/saturation", Value::Scalar(50.0))])); assert!(!ok("ec.noise.addgrain", &[("preset", Value::Enum(2))])); + // #1195: CC Particle World's motion trails (shutter samples) render on the CPU. + { + let params = ctx_with("ec.sim.ccparticleworld", &[]); + let env = crate::EffectEnv { frame_rate: 30.0, shutter: Some((180.0, -90.0, 16)), ..Default::default() }; + let ctx = crate::EffectCtx { params: ¶ms, time: 0.0, layer_size: [64.0, 64.0], seed: 0, adjustment: false, env }; + assert!(!super::gpu_supported("ec.sim.ccparticleworld", &ctx)); + } + assert!(ok("ec.sim.ccparticleworld", &[])); assert!(ok("ec.generate.cellpattern", &[("cellPattern", Value::Enum(3))])); assert!(!ok("ec.generate.cellpattern", &[("cellPattern", Value::Enum(9))])); assert!(!ok("ec.distort.turbulentdisplace", &[("pinning", Value::Enum(9))])); diff --git a/crates/effects/src/color_fx.rs b/crates/effects/src/color_fx.rs index e431ff22..cf6698fe 100644 --- a/crates/effects/src/color_fx.rs +++ b/crates/effects/src/color_fx.rs @@ -326,20 +326,27 @@ pub fn mirror_pow(v: f32, p: f32) -> f32 { v.abs().powf(p).copysign(v) } +/// Black & White: a pixel's gray is its smallest channel plus its chroma (largest − smallest +/// channel) times the colour sliders' weight for its hue. A neutral pixel keeps its value, a pure +/// hue becomes its slider's percentage (Reds 40 % turns pure red into 40 % gray), and the gray +/// never moves further from the smallest channel than the pixel's own chroma times the weight. +/// It used to offset the luminance by the HSL saturation, which is near 1 for any tinted +/// near-black pixel: the faint colour noise of compressed footage's shadows came out as bright +/// and black blocks (#1463), and 32-bpc overbrights divided by zero. fn black_white(ctx: &EffectCtx, mut b: Buf) -> Buf { let w = [ctx.params.f("reds"), ctx.params.f("yellows"), ctx.params.f("greens"), ctx.params.f("cyans"), ctx.params.f("blues"), ctx.params.f("magentas")] .map(|v| v as f32 / 100.0); let tint_on = ctx.params.b("tint"); let tc = ctx.params.color("tintColor"); b.img.map_straight(|c| { - let (h, s, _) = rgb_to_hsl(c[0], c[1], c[2]); + let (h, _, _) = rgb_to_hsl(c[0], c[1], c[2]); // Interpolate the six hue weights around the colour wheel. let hp = h * 6.0; let i = hp.floor() as usize % 6; let f = hp - hp.floor(); let wt = w[i] + (w[(i + 1) % 6] - w[i]) * f; - let base = luminance(c[0], c[1], c[2]); - let gray = (base + (wt - 0.5) * s * 0.6).max(0.0); + let (max, min) = (c[0].max(c[1]).max(c[2]), c[0].min(c[1]).min(c[2])); + let gray = (min + (max - min) * wt).max(0.0); if tint_on { let (th, ts, _) = rgb_to_hsl(tc[0], tc[1], tc[2]); let (r, g, bl) = hsl_to_rgb(th, ts, gray.min(1.0)); @@ -1220,6 +1227,66 @@ mod tests { assert!(go(0.0, 100.0, 64) >= 30); } + /// #1463: Black & White's sliders set the gray of each pure hue and blend between + /// neighbouring hues; neutral pixels keep their value, overbright ones too, and an overbright + /// colour stays finite (it used to divide by zero). + #[test] + fn black_white_sliders_set_the_gray_of_each_hue() { + let bw = |vals: &[(&str, Value)], c: [f32; 3]| px("ec.color.blackwhite", vals, [c[0], c[1], c[2], 1.0]); + let gray = |v: f32| [v, v, v, 1.0]; + // The defaults: Reds 40, Yellows 60, Greens 40, Cyans 60, Blues 20, Magentas 80 (%). + let hues = [ + ([1.0, 0.0, 0.0], 0.4), + ([1.0, 1.0, 0.0], 0.6), + ([0.0, 1.0, 0.0], 0.4), + ([0.0, 1.0, 1.0], 0.6), + ([0.0, 0.0, 1.0], 0.2), + ([1.0, 0.0, 1.0], 0.8), + // Half as bright, half the gray; orange is halfway from Reds to Yellows. + ([0.5, 0.0, 0.0], 0.2), + ([1.0, 0.5, 0.0], 0.5), + // Neutral pixels keep their value. + ([0.3, 0.3, 0.3], 0.3), + ([4.0, 4.0, 4.0], 4.0), + // An overbright red: its smallest channel plus its chroma at Reds' 40 %. + ([1.5, 0.5, 0.5], 0.9), + ]; + for (c, want) in hues { + let o = bw(&[], c); + assert!(close(o, gray(want), 1e-5), "{c:?}: {o:?}, want {want}"); + } + assert!(close(bw(&[("reds", num(120.0))], [1.0, 0.0, 0.0]), gray(1.2), 1e-5)); + assert!(close(bw(&[("reds", num(-200.0))], [1.0, 0.0, 0.0]), gray(0.0), 1e-5)); + assert!(close(bw(&[("blues", num(300.0))], [0.1, 0.1, 0.3]), gray(0.7), 1e-5)); + } + + /// #1463: faint colour noise stays faint. The shadows of compressed footage carry a code or + /// two of colour noise, which Black & White turned into bright and black blocks (the HSL + /// saturation it scaled is near 1 for any tinted near-black pixel). At 8, 16 and 32 bpc the + /// gray of a dark ramp with ±2 steps of colour noise stays within 2 steps of the clean ramp's. + #[test] + fn black_white_keeps_faint_colour_noise_faint() { + // A step of 8 bpc, of 16 bpc (After Effects' 0..32768), and a small 32-bpc float one. + for step in [1.0 / 255.0, 1.0 / 32768.0, 1e-3_f32] { + let (w, h) = (64u32, 4u32); + let (mut clean, mut noisy) = (Image::new(w, h), Image::new(w, h)); + for y in 0..h { + for x in 0..w { + // A dark ramp up to a quarter gray, in whole steps... + let v = (x as f32 / w as f32 * 0.25 / step).round() * step; + // ...and up to ±2 steps of noise per channel, mixed differently in each pixel. + let n = |k: u32| (((x * 7 + y * 13 + k * 2) % 5) as f32 - 2.0) * step; + clean.set(x, y, [v, v, v, 1.0]); + noisy.set(x, y, [(v + n(0)).max(0.0), (v + n(1)).max(0.0), (v + n(2)).max(0.0), 1.0]); + } + } + let bw = |img| run_fx("ec.color.blackwhite", &[], img, 0.0, EffectEnv::default()).img; + let (a, b) = (bw(clean), bw(noisy)); + let worst = a.data.iter().zip(&b.data).map(|(p, q)| (p[0] - q[0]).abs()).fold(0.0, f32::max); + assert!(worst <= 2.0 * step + 1e-6, "step {step}: the noise moved the gray by {} steps", worst / step); + } + } + #[test] fn levels_clips_black_and_white_independently() { let lv = |vals: &[(&str, Value)], v: f32| px("ec.color.levels", vals, [v, v, v, 1.0])[0]; diff --git a/crates/effects/src/distort4.rs b/crates/effects/src/distort4.rs index d1d1b0c2..b724a583 100644 --- a/crates/effects/src/distort4.rs +++ b/crates/effects/src/distort4.rs @@ -589,7 +589,7 @@ pub fn specs() -> Vec { p("meshColor", "Mesh Color", col(0.5, 0.5, 0.5), ParamUi::Color), // The strokes (see `LiquifyStroke`); written by the `liquify.stroke` command. p("distortionMesh", "Distortion Mesh", Value::Str(String::new()), ParamUi::Hidden), - p("distortionMeshOffset", "Distortion Mesh Offset", Value::Vec2([0.0, 0.0]), ParamUi::Hidden), + p("distortionMeshOffset", "Distortion Mesh Offset", Value::Vec2([0.0, 0.0]), ParamUi::Point), p("distortionPercentage", "Distortion Percentage", num(100.0), slider(0.0, 200.0, 0.0, 200.0, 1)), p("distortionDisplayQuality", "Distortion Display Quality", Value::Enum(1), popup(&["Draft", "Smooth"])), ], @@ -672,6 +672,35 @@ mod tests { assert_eq!(zero.img.data, img.data); let again = run_fx("ec.distort.liquify", &[("distortionMesh", Value::Str(s.to_line()))], img.clone(), 0.0, EffectEnv::default()); assert_eq!(out.img.data, again.img.data); + // #1194: Distortion Mesh Offset is an exposed, editable control (AE's Distortion ▸ + // Offset) and it moves the mesh: the pattern rendered at x renders at x + offset. + let spec = crate::find("ec.distort.liquify").unwrap(); + assert!( + spec.params.iter().any(|ps| ps.id == "distortionMeshOffset" && ps.ui == effectcraft_project::ParamUi::Point), + "offset is shown in Effect Controls" + ); + let shifted = run_fx( + "ec.distort.liquify", + &[("distortionMesh", Value::Str(s.to_line())), ("distortionMeshOffset", Value::Vec2([20.0, 0.0]))], + img.clone(), + 0.0, + EffectEnv::default(), + ); + assert_ne!(shifted.img.data, out.img.data, "the offset changes the render"); + // The displaced region's centroid moves right by the offset (the mesh is sampled at + // layer minus offset), and the row far left of the shifted mesh is back to untouched. + let centroid = |o: &Image| -> f32 { + let (mut wsum, mut msum) = (0.0f32, 0.0f32); + for (i, (p, q)) in o.data.iter().zip(img.data.iter()).enumerate() { + let d: f32 = (0..4).map(|k| (p[k] - q[k]).abs()).sum(); + wsum += d; + msum += d * (i % 64) as f32; + } + if wsum > 1e-6 { msum / wsum } else { 0.0 } + }; + let (co, cs) = (centroid(&out.img), centroid(&shifted.img)); + assert!((cs - co - 20.0).abs() < 5.0, "centroids {co} -> {cs}"); + assert_eq!(shifted.img.get(5, 32), img.get(5, 32), "left of the shifted mesh: untouched"); // Reconstruction undoes the warp; freeze protects. let mut rec = s.clone(); rec.tool = 9; diff --git a/crates/effects/src/generate.rs b/crates/effects/src/generate.rs index b29ff915..fd5a5412 100644 --- a/crates/effects/src/generate.rs +++ b/crates/effects/src/generate.rs @@ -138,7 +138,10 @@ fn four_color(ctx: &EffectCtx, mut b: Buf) -> Buf { let n = (effectcraft_raster::hash_noise(x as u32, y as u32, seed) - 0.5) * jitter * (8.0 / 255.0); c = c.map(|v| v + n); } - *px = gen_blend(*px, [c[0] * op, c[1] * op, c[2] * op, op], mode); + // None keeps the layer's alpha (#1381), as the Gradient Ramp does: the gradient fills + // a shape or text, not its bounding box. + let a = if gen_mode(mode).is_none() { op * px[3] } else { op }; + *px = gen_blend(*px, [c[0] * a, c[1] * a, c[2] * a, a], mode); } }); b @@ -420,6 +423,25 @@ mod generator_tests { } } + #[test] + fn four_color_none_stays_inside_the_layer_alpha() { + // #1381: like the Gradient Ramp, the gradient fills a shape or text, not its bounding box. + let mut img = Image::filled(40, 24, [1.0, 0.0, 0.0, 1.0]); + for (i, p) in img.data.iter_mut().enumerate() { + let a = [0.0, 0.5, 1.0][i % 40 / 14]; + *p = p.map(|c| c * a); + } + for op in [100.0, 50.0] { + let o = run_fx("ec.generate.fourcolor", &[("opacity", num(op))], img.clone(), 0.0, EffectEnv::default()); + let k = op as f32 / 100.0; + for (src, out) in img.data.iter().zip(&o.img.data) { + assert!((src[3] * k - out[3]).abs() < 1e-6, "opacity {op}: alpha {} became {}", src[3], out[3]); + assert!(out[..3].iter().all(|&c| c <= out[3] + 1e-6), "opacity {op}: premultiplied colour above alpha"); + } + assert!(o.img.data.iter().any(|p| p[1] > 0.1), "opacity {op}: the gradient replaces the layer's colour"); + } + } + #[test] fn checkerboard_blending_none_shows_pattern_only_and_size_from_width() { let set = [("width", num(8.0)), ("height", num(2.0))]; diff --git a/crates/effects/src/migrate.rs b/crates/effects/src/migrate.rs index 0c0dad9d..0e037022 100644 --- a/crates/effects/src/migrate.rs +++ b/crates/effects/src/migrate.rs @@ -239,6 +239,8 @@ fn refresh(pr: &mut Property, ps: &ParamSpec, spec: &EffectSpec) { // A path typed as text, or set only by commands, that got a Choose… button (Apply Color // LUT's LUT, the OCIO files, #514). (ParamUi::Hidden | ParamUi::Text, ParamUi::File { .. }) => pr.ui = ps.ui.clone(), + // An internal point that became a control (Liquify's Distortion Mesh Offset, #1194). + (ParamUi::Hidden, ParamUi::Point) => pr.ui = ParamUi::Point, // Same kind of value: take the spec's hints (units, ranges). (a, b) if std::mem::discriminant(a) == std::mem::discriminant(b) || is_numeric_ui(a) && is_numeric_ui(b) => pr.ui = ps.ui.clone(), _ => {} @@ -514,6 +516,19 @@ mod tests { } } + /// #1194: Liquify's Distortion Mesh Offset was hidden; old projects show it, keeping its value. + #[test] + fn a_hidden_point_that_became_a_control_is_shown() { + let mut next = 1; + let (spec, mut g) = instance("ec.distort.liquify", &mut next); + let pr = g.get_mut("distortionMeshOffset").unwrap(); + pr.ui = ParamUi::Hidden; + pr.value = Value::Vec2([20.0, -5.0]); + assert!(upgrade_instance(spec, &mut g, &mut Ids(&mut next), [100.0, 100.0])); + let pr = g.get("distortionMeshOffset").unwrap(); + assert_eq!((&pr.ui, &pr.value), (&ParamUi::Point, &Value::Vec2([20.0, -5.0]))); + } + #[test] fn upgrade_moves_params_into_twirl_down_groups() { let spec = find(crate::warp_stab::ID).unwrap(); diff --git a/crates/effects/src/sim.rs b/crates/effects/src/sim.rs index 2c7b97d1..14146a9f 100644 --- a/crates/effects/src/sim.rs +++ b/crates/effects/src/sim.rs @@ -238,9 +238,22 @@ impl SpritePlan { } /// Rasterise (after [`SpritePlan::resolve`]) and meet the original layer `b`. - pub fn finish(&self, mut b: Buf) -> Buf { + pub fn finish(&self, b: Buf) -> Buf { let fx = splat(b.img.width, b.img.height, &self.sprites, self.acc); - b.img = match self.post { + self.finish_with(b, fx) + } + + /// [`finish`](Self::finish) with a prepared sprite image (the motion-blurred average of + /// several frames, #1195). + pub fn finish_with(&self, b: Buf, fx: Image) -> Buf { + self.post.apply(b, fx) + } +} + +impl Post { + /// Meet the sprite image `fx` with the original layer `b` (see [`SpritePlan::finish`]). + fn apply(self, mut b: Buf, fx: Image) -> Buf { + b.img = match self { Post::Combine(m) => combine(&b.img, &fx, m), Post::Replace => fx, Post::Lerp(t) => { @@ -1069,6 +1082,29 @@ fn pw_phys(ctx: &EffectCtx) -> Phys { } fn particle_world(ctx: &EffectCtx, b: Buf) -> Buf { + // #1195: with the comp's motion-blur shutter open, particles draw as motion trails: the + // sprite fields of samples across the shutter are averaged, as Transform averages its + // matrix samples. (The GPU compositor runs this case on the CPU: catalog::gpu_supported.) + if let Some((angle, phase, n)) = ctx.env.shutter.filter(|(a, _, _)| *a > 0.0) { + let n = (n.clamp(2, 64) as usize).max(2); + let fd = 1.0 / ctx.fps().max(0.001); + let mut acc = Image::new(b.img.width, b.img.height); + let mut post: Option = None; + for k in 0..n { + let t = ctx.time + (phase + angle * k as f64 / (n - 1) as f64) / 360.0 * fd; + let sub = EffectCtx { params: ctx.params, time: t, layer_size: ctx.layer_size, seed: ctx.seed, adjustment: ctx.adjustment, env: ctx.env }; + if let Some(plan) = particle_world_plan(&sub, &b) { + let fx = splat(b.img.width, b.img.height, &plan.sprites, plan.acc); + acc.data.iter_mut().zip(&fx.data).for_each(|(a, o)| (0..4).for_each(|c| a[c] += o[c])); + post.get_or_insert(plan.post); + } + } + acc.data.iter_mut().for_each(|p| p.iter_mut().for_each(|v| *v /= n as f32)); + return match post { + Some(p) => p.apply(b, acc), + None => b, + }; + } match particle_world_plan(ctx, &b) { Some(plan) => plan.finish(b), None => b, @@ -1715,6 +1751,31 @@ mod tests { ] } + /// #1195: with the comp's shutter open, the particles smear across it: the average of the + /// sampled sprite fields differs from the sharp frame, touches more pixels and keeps the + /// alpha energy. + #[test] + fn particle_world_motion_blur_trails() { + let v = pw_vals(21.0); + let blank = Image::new(40, 40); + let sharp = run("ec.sim.ccparticleworld", &v, blank.clone(), 2.0); + let blurred = crate::run_fx( + "ec.sim.ccparticleworld", + &v, + blank.clone(), + 2.0, + EffectEnv { frame_rate: 10.0, shutter: Some((360.0, -180.0, 8)), ..Default::default() }, + ) + .img; + assert!(sharp.data.iter().any(|p| p[3] > 0.0), "sharp frame has particles"); + assert_ne!(sharp.data, blurred.data, "the shutter smears the particles"); + let lit = |img: &Image| img.data.iter().filter(|p| p[3] > 0.005).count(); + let sum = |img: &Image| img.data.iter().map(|p| p[3]).sum::(); + assert!(lit(&blurred) > lit(&sharp), "trails light more pixels: {} vs {}", lit(&blurred), lit(&sharp)); + let (sa, ba) = (sum(&sharp), sum(&blurred)); + assert!((ba - sa).abs() < sa * 0.6 + 1.0, "alpha energy roughly kept: {ba} vs {sa}"); + } + #[test] fn particle_world_seek_consistent() { let v = pw_vals(11.0); diff --git a/crates/engine/src/commands/app_more.rs b/crates/engine/src/commands/app_more.rs index 020880e8..1bb94952 100644 --- a/crates/engine/src/commands/app_more.rs +++ b/crates/engine/src/commands/app_more.rs @@ -245,7 +245,8 @@ fn has_footage(s: &Session) -> std::result::Result<(), String> { /// `File.execute()` from scripts (and agents): open a file with its default application. /// Needs Allow Scripts to Write Files and Access Network; with Warn User When Executing Files on, -/// the frontend asks first (`confirmed: true` skips the question). +/// the frontend asks first. `confirmed: true` is the frontend's answer to that question: a +/// script can't give it for itself (#1246), so a script's request always asks. fn execute_file(s: &mut Session, p: &Value) -> Result { let path = str_p(p, "path").ok_or_else(|| bad("file.executeFile", "missing `path`"))?; if !s.prefs.scripting.allow_scripts_write_files { @@ -257,7 +258,8 @@ fn execute_file(s: &mut Session, p: &Value) -> Result { if !std::path::Path::new(path).exists() { return Err(bad("file.executeFile", format!("no file {path}"))); } - if s.prefs.scripting.warn_executing_files && !b_p(p, "confirmed").unwrap_or(false) { + let confirmed = b_p(p, "confirmed").unwrap_or(false) && !s.running_for_script(); + if s.prefs.scripting.warn_executing_files && !confirmed { s.events.push(Event::Frontend { command: "app.confirmExecute".into(), params: json!({"path": path}) }); return Ok(json!({"needsConfirmation": path})); } diff --git a/crates/engine/src/commands/file_more.rs b/crates/engine/src/commands/file_more.rs index e5de4de1..346f8392 100644 --- a/crates/engine/src/commands/file_more.rs +++ b/crates/engine/src/commands/file_more.rs @@ -356,13 +356,12 @@ fn find_missing(s: &mut Session, p: &Value) -> Result { Ok(json!({"effects": found})) } "fonts" => { - let families = crate::text_families(); let mut found = vec![]; for (cid, c) in s.project.comps() { for l in &c.layers { if let Some(pr) = l.props.prop("text/sourceText") && let Some(doc) = pr.value.as_text() - && !families.iter().any(|f| f.eq_ignore_ascii_case(&doc.font)) + && effectcraft_text::resolve(&doc.font, &doc.style).missing { found.push(json!({"comp": cid.0, "layer": l.id.0, "font": doc.font})); } diff --git a/crates/engine/src/commands/layer.rs b/crates/engine/src/commands/layer.rs index ca8aa865..b798493b 100644 --- a/crates/engine/src/commands/layer.rs +++ b/crates/engine/src/commands/layer.rs @@ -417,6 +417,8 @@ fn set_switch_others(s: &mut Session, cid: ItemId, ids: &[LayerId], name: &str, let applies = |l: &Layer| match name { "audio" => l.props.sub("audio").is_some(), "lock" | "locked" => true, + // Nulls have a Video switch too (it hides their outline in the viewer). + "video" | "eye" => (l.source.is_av() || matches!(l.source, LayerSource::Null)) && !mattes.contains(&l.id), _ => l.source.is_av() && !mattes.contains(&l.id), }; let mut alone = true; diff --git a/crates/engine/src/commands/layer_menu.rs b/crates/engine/src/commands/layer_menu.rs index 7737b8af..83a1510a 100644 --- a/crates/engine/src/commands/layer_menu.rs +++ b/crates/engine/src/commands/layer_menu.rs @@ -115,7 +115,7 @@ fn hide_other_video(s: &mut Session, p: &Value) -> Result { let (cid, ids) = layers_p(s, p)?; s.edit("Hide Other Video", None, |proj, _| { for l in proj.comp_mut(cid).ok_or(EngineError::NoComp)?.layers.iter_mut() { - if !ids.contains(&l.id) && l.source.is_av() { + if !ids.contains(&l.id) && (l.source.is_av() || matches!(l.source, LayerSource::Null)) { l.switches.video = false; } } diff --git a/crates/engine/src/commands/mask.rs b/crates/engine/src/commands/mask.rs index 359de14d..778dfb54 100644 --- a/crates/engine/src/commands/mask.rs +++ b/crates/engine/src/commands/mask.rs @@ -132,6 +132,9 @@ fn edit_path_counted(s: &mut Session, p: &Value, label: &str, f: impl FnOnce( let roto = super::paths::is_roto(g); let pr = g.get_mut("path").ok_or_else(|| bad(label, "mask has no path"))?; let KV::Path(mut sp) = pr.value_at(lt) else { return Err(bad(label, "not a path")) }; + // A path read from a file may have fewer tangents than vertices; the edits below index + // all three arrays by vertex (#1250). + sp.fit_tangents(); let (r, op) = f(&mut sp)?; if roto { super::paths::roto_smooth(&mut sp); @@ -287,6 +290,7 @@ fn edit_vertices_counted(s: &mut Session, p: &Value, label: &str, removes: bool, let roto = super::paths::is_roto(g); let Some(pr) = g.get_mut("path") else { continue }; let KV::Path(mut sp) = pr.value_at(lt) else { continue }; + sp.fit_tangents(); f(&mut sp, idx); if roto { super::paths::roto_smooth(&mut sp); diff --git a/crates/engine/src/commands/relink_moved.rs b/crates/engine/src/commands/relink_moved.rs index 4f70dd6e..ef64a423 100644 --- a/crates/engine/src/commands/relink_moved.rs +++ b/crates/engine/src/commands/relink_moved.rs @@ -126,17 +126,18 @@ pub(crate) fn find_moved(services: &dyn Services, project_dir: &str, f: &Footage }) } -/// The folder a project file is in. +/// The folder a project file is in, with the path's own separator and its leading `/` or UNC `\\`. pub(crate) fn project_dir(project_path: &str) -> Option { let p = parts(project_path); - if p.len() < 2 { - return None; - } - let mut dir = join(if project_path.starts_with('/') { "/" } else { "" }, p.get(..p.len() - 1)?); - if !project_path.starts_with('/') { - dir = dir.trim_start_matches(['/', '\\']).to_string(); - } - Some(dir) + let dir = p.get(..p.len().checked_sub(1)?).filter(|d| !d.is_empty())?; + let root = if project_path.starts_with("\\\\") { + "\\\\" + } else if project_path.starts_with('/') { + "/" + } else { + "" + }; + Some(format!("{root}{}", dir.join(&sep_of(project_path).to_string()))) } /// Relink `found` in one undo step and say how many. @@ -262,6 +263,10 @@ mod tests { ); assert_eq!(project_dir("/x/y/p.ecproj").as_deref(), Some("/x/y")); assert_eq!(project_dir("p.ecproj"), None); + // Windows paths keep their separator, and a UNC share its `\\`. + assert_eq!(project_dir("C:\\Work\\Job\\p.ecproj").as_deref(), Some("C:\\Work\\Job")); + assert_eq!(project_dir("\\\\server\\share\\Job\\p.ecproj").as_deref(), Some("\\\\server\\share\\Job")); + assert_eq!(moved_candidates("\\\\server\\share\\Job", "D:\\Old\\Job\\shots\\a.png")[2], "\\\\server\\share\\Job\\shots\\a.png"); // Hostile input doesn't panic. for (a, b) in [("", ""), ("/", "/"), ("x", ""), ("\\\\", "//"), ("/a", "/a/b/c")] { let _ = moved_root(a, b); @@ -302,11 +307,11 @@ mod tests { let b = add(&mut s, "/gone/proj/shots/b.mov"); let m = add(&mut s, "/gone/proj/audio/m.wav"); let other = add(&mut s, "/elsewhere/c.mov"); - let target = new.join("shots/a.mov").to_string_lossy().to_string(); + let target = new.join("shots").join("a.mov").to_string_lossy().to_string(); let r = s.execute("footage.relink", json!({"item": a.0, "path": target})).unwrap(); assert_eq!(r["relinked"], json!([a.0, b.0, m.0]), "{r}"); assert_eq!(r["stillMissing"], 1); - assert_eq!(path_of(&s, m), new.join("audio/m.wav").to_string_lossy()); + assert_eq!(path_of(&s, m), new.join("audio").join("m.wav").to_string_lossy()); assert_eq!(path_of(&s, other), "/elsewhere/c.mov"); // One undo step brings every path back. s.execute("edit.undo", json!({})).unwrap(); diff --git a/crates/engine/src/footage_check.rs b/crates/engine/src/footage_check.rs index de14b06c..adfb1768 100644 --- a/crates/engine/src/footage_check.rs +++ b/crates/engine/src/footage_check.rs @@ -235,7 +235,8 @@ mod tests { paths.sort(); // Windows joins the stored "media/..." onto the project's folder and leaves a path with mixed // separators; `Path` equality compares components, so both spellings are the same file. - let expected = ["/nonexistent/ec/abs.png", clip.to_str().unwrap(), "media/ec-relative-gone.png"]; + let mut expected = ["/nonexistent/ec/abs.png", clip.to_str().unwrap(), "media/ec-relative-gone.png"]; + expected.sort(); assert_eq!(paths.len(), expected.len(), "{paths:?}"); for (got, want) in paths.iter().zip(expected) { assert_eq!(std::path::Path::new(got), std::path::Path::new(want), "{paths:?}"); @@ -254,7 +255,7 @@ mod tests { std::fs::write(d.join("Job/shots/a.png"), b"x").unwrap(); // Saved on another machine: the footage sat next to the project in /Users/x/Job. let mut s = session_with(&["/Users/x/Job/shots/a.png", "/Users/x/elsewhere/b.png"]); - let path = d.join("Job/p.ecproj"); + let path = d.join("Job").join("p.ecproj"); s.execute("file.saveAs", json!({"path": path.to_string_lossy()})).unwrap(); let mut o = Session::default(); o.execute("file.open", json!({"path": path.to_string_lossy()})).unwrap(); @@ -269,7 +270,7 @@ mod tests { _ => None, }) .collect(); - assert!(paths.contains(&d.join("Job/shots/a.png").to_string_lossy().to_string()), "{paths:?}"); + assert!(paths.contains(&d.join("Job").join("shots").join("a.png").to_string_lossy().to_string()), "{paths:?}"); assert!(!o.is_dirty(), "finding the files is not an edit"); let _ = std::fs::remove_dir_all(d); } diff --git a/crates/engine/src/media_browser.rs b/crates/engine/src/media_browser.rs index a77859d5..b394ad93 100644 --- a/crates/engine/src/media_browser.rs +++ b/crates/engine/src/media_browser.rs @@ -316,7 +316,9 @@ pub fn footage_metadata(f: &Footage) -> Value { "pixelAspect": f.pixel_aspect, "frameRate": f.frame_rate.as_f64(), "duration": f.duration.seconds(), - "frames": if f.has_video && f.kind != FootageKind::Still { Some(f.frame_rate.frame_at(f.duration)) } else { None }, + // Rounded like the media pool's frame count: Matroska stores the duration in whole + // milliseconds, a hair short of the last frame at 30 fps (#1429). + "frames": if f.has_video && f.kind != FootageKind::Still { Some(f.frame_rate.frame_at(f.frame_rate.snap_nearest(f.duration))) } else { None }, "hasVideo": f.has_video, "hasAudio": f.has_audio, "alpha": format!("{:?}", f.alpha), diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 56754e7c..74b33315 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -362,8 +362,15 @@ impl Session { // Settings ▸ Export ▸ Default Output Folder, else the project's folder. let folder = self.prefs.export.default_output_folder.trim(); let base = (!folder.is_empty()).then(|| std::path::PathBuf::from(folder)); - let base = - base.or_else(|| self.path.as_deref().and_then(|s| std::path::Path::new(s).parent().map(|d| d.to_path_buf())).filter(|d| !d.as_os_str().is_empty())); + // A project saved under a bare filename has the empty path as its parent. It sits in the + // working directory, so resolve against that rather than falling through to the + // unsaved-project default below (#1427). + let base = base.or_else(|| { + self.path + .as_deref() + .and_then(|s| std::path::Path::new(s).parent()) + .map(|d| if d.as_os_str().is_empty() { std::path::PathBuf::from(".") } else { d.to_path_buf() }) + }); // Unsaved project: ~/Movies, else the home folder (as FilmCraft's exports do). The working directory // is `/` for an app started from the macOS Finder, where nothing can be written (#446). let home = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")).map(std::path::PathBuf::from); diff --git a/crates/engine/src/tests_m145.rs b/crates/engine/src/tests_m145.rs index e52e3ffb..fe0a1693 100644 --- a/crates/engine/src/tests_m145.rs +++ b/crates/engine/src/tests_m145.rs @@ -280,6 +280,28 @@ fn import_export_helpers() { assert_eq!(p.recent_fonts_shown(), ["A".to_string(), "C".to_string()]); } +#[test] +fn render_queue_resolves_a_bare_project_filename_against_the_working_directory() { + use effectcraft_project::render_queue::{OutputFormat, RenderQueueItem}; + let mut s = session(); + let cid = s.active_comp_id().unwrap(); + let mut item = RenderQueueItem::new(1, cid); + item.output.output = "clip.mp4".into(); + item.output.format = OutputFormat::H264; + + // `--project ./proj.ecproj`: the parent is `.`, so the output goes beside the project. + s.path = Some("./proj.ecproj".into()); + let dotted = s.resolve_output(&item).unwrap(); + + // `--project proj.ecproj`: the parent is the empty path, which must mean the same folder + // rather than the unsaved-project default (#1427). + s.path = Some("proj.ecproj".into()); + let bare = s.resolve_output(&item).unwrap(); + + assert_eq!(PathBuf::from(&bare), PathBuf::from(&dotted), "a bare filename resolved to {bare} instead of {dotted}"); + assert_eq!(PathBuf::from(&bare), PathBuf::from("./clip.mp4")); +} + #[test] fn render_queue_uses_default_folder_bit_depth_names_and_segments() { use effectcraft_project::render_queue::{OutputFormat, RenderQueueItem}; @@ -349,6 +371,34 @@ fn execute_file_needs_the_gate_and_asks_first() { assert!(s.execute("file.executeFile", p).unwrap().get("opened").is_some()); } +#[test] +fn a_script_cannot_confirm_executing_a_file_for_itself() { + // Warn User When Executing Files: only the user answers the question (#1246). + let mut s = session(); + let f = tmp("script-run.txt"); + std::fs::write(&f, "x").unwrap(); + let path = f.to_string_lossy().to_string(); + let opened = |s: &mut Session| s.drain_events().iter().any(|e| matches!(e, crate::Event::OpenUrl(_))); + // Without the gate a script can't run files at all, confirmed or not. + assert!(s.execute_for_script("file.executeFile", json!({"path": path, "confirmed": true})).is_err()); + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap(); + s.drain_events(); + // A script asserting `confirmed` still gets the question, directly or in a batch. + let r = s.execute_for_script("file.executeFile", json!({"path": path, "confirmed": true})).unwrap(); + assert!(r.get("needsConfirmation").is_some(), "{r}"); + assert!(!opened(&mut s)); + let steps = json!({"steps": [{"command": "file.executeFile", "params": {"path": path, "confirmed": true}}]}); + let r = s.execute_for_script("engine.batch", steps).unwrap(); + assert!(r.to_string().contains("needsConfirmation"), "{r}"); + assert!(!opened(&mut s)); + // The user's answer from the frontend's question still opens it. + assert!(s.execute("file.executeFile", json!({"path": path, "confirmed": true})).unwrap().get("opened").is_some()); + assert!(opened(&mut s)); + // With the warning off a script's request opens the file without a question, as before. + s.execute("prefs.set", json!({"key": "scripting.warnExecutingFiles", "value": false})).unwrap(); + assert!(s.execute_for_script("file.executeFile", json!({"path": path})).unwrap().get("opened").is_some()); +} + // ---------------------------------------------------------------- Edit ▸ History, recent lists #[test] diff --git a/crates/engine/src/tests_menu_cmds.rs b/crates/engine/src/tests_menu_cmds.rs index 5203872a..4e01e380 100644 --- a/crates/engine/src/tests_menu_cmds.rs +++ b/crates/engine/src/tests_menu_cmds.rs @@ -111,6 +111,38 @@ fn set_switch_others() { assert!(s.execute("layer.setSwitch", json!({"layers": [c], "switch": "nope", "others": true})).is_err()); } +/// A null's Video switch (the eye) works like any layer's, as in After Effects: it toggles, +/// Alt-click and Hide Other Video take nulls along, and its children still follow it while it is +/// hidden (parenting ignores the eye). +#[test] +fn null_video_switch() { + let mut s = comp(); + let a = solid(&mut s, "#ff0000"); + let n = s.execute("layer.newNull", json!({})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.setSwitch", json!({"layers": [n], "switch": "video"})).unwrap(); + assert!(!layer(&s, n).switches.video); + s.execute("edit.undo", json!({})).unwrap(); + assert!(layer(&s, n).switches.video); + // Alt-click on the solid's eye hides the null too; on the null's, the solid. + s.execute("layer.setSwitch", json!({"layers": [a], "switch": "video", "others": true})).unwrap(); + assert!(layer(&s, a).switches.video && !layer(&s, n).switches.video); + s.execute("layer.setSwitch", json!({"layers": [n], "switch": "video", "others": true})).unwrap(); + assert!(!layer(&s, a).switches.video && layer(&s, n).switches.video); + s.execute("layer.showAllVideo", json!({})).unwrap(); + s.execute("layer.hideOtherVideo", json!({"layers": [a]})).unwrap(); + assert!(layer(&s, a).switches.video && !layer(&s, n).switches.video); + // The hidden null still carries its child. + s.execute("layer.setParent", json!({"layers": [a], "parent": n})).unwrap(); + s.execute("prop.set", json!({"layer": n, "path": "transform/position", "value": [420, 180, 0]})).unwrap(); + let cid = s.active_comp_id().unwrap(); + let c = s.project.comp(cid).unwrap().clone(); + let ctx = effectcraft_render::EvalCtx::new(&s.project, cid, &c, Tick::ZERO); + let child = c.layer(effectcraft_project::LayerId(a)).unwrap(); + let (m, _) = ctx.layer_to_comp(child); + let centre = m.apply(effectcraft_geom::vec2(100.0, 50.0)); + assert!((centre.x - 420.0).abs() < 1e-6 && (centre.y - 180.0).abs() < 1e-6, "{centre:?}"); +} + #[test] fn layer_switches_menu() { let mut s = comp(); @@ -708,6 +740,21 @@ fn sequence_layers() { assert_eq!(layer(&s, c).props.prop("transform/opacity").unwrap().keys.len(), 2); } +/// File ▸ Dependencies ▸ Find Missing Fonts lists a text layer only when no face matches its +/// font by family, PostScript or full name. +#[test] +fn fonts_named_by_their_postscript_or_full_name_are_not_missing() { + let mut s = comp(); + // The first three name bundled faces by family, PostScript and full name; no face has the fourth. + let fonts = ["Noto Serif", "Inter-Bold", "Inter SemiBold", "No Such Font Family 123"]; + let ids: Vec = fonts.iter().map(|f| s.execute("layer.newText", json!({"text": "Aa", "font": f})).unwrap()["layer"].as_u64().unwrap()).collect(); + let r = s.execute("file.findMissing", json!({"what": "fonts"})).unwrap(); + let listed: Vec<(u64, &str)> = r["fonts"].as_array().unwrap().iter().map(|f| (f["layer"].as_u64().unwrap(), f["font"].as_str().unwrap())).collect(); + assert_eq!(listed, [(ids[3], fonts[3])], "{r}"); + let f = s.execute("text.fontFeatures", json!({"layer": ids[1]})).unwrap(); + assert_eq!((&f["family"], &f["style"], &f["missing"]), (&json!("Inter"), &json!("Bold"), &json!(false)), "{f}"); +} + #[test] fn file_menu_project_ops() { let mut s = comp(); @@ -1558,6 +1605,31 @@ fn interpret_footage_loop_extends_video_layers() { assert_eq!(s.project.item(effectcraft_project::ItemId(p)).unwrap().duration(), Some(Tick::from_seconds_f64(6.0))); } +/// #1068: the same for an interpreted PNG sequence — the loop count survives interpretation and +/// a freshly dragged layer plays every cycle. +#[test] +fn interpret_footage_loop_extends_sequence_layers() { + let mut s = comp(); + let p = s.execute("file.importPlaceholder", json!({"name": "Seq", "width": 320, "height": 240, "duration": 0.4})).unwrap()["item"].as_u64().unwrap(); + let id = effectcraft_project::ItemId(p); + // Make it a sequence like the importer's grouped PNGs. + s.edit("Seq", None, |proj, _| { + if let Some(ItemKind::Footage(f)) = proj.item_mut(id).map(|i| &mut i.kind) { + f.kind = effectcraft_project::FootageKind::Sequence; + f.sequence = (0..12).map(|n| format!("/tmp/seq_{n:04}.png")).collect(); + f.sync_sequence_duration(); + } + Ok(()) + }) + .unwrap(); + s.state.project_selection = vec![id]; + s.execute("file.interpretFootage", json!({"loop": 15.0})).unwrap(); + let l = s.execute("layer.addItem", json!({"item": p})).unwrap()["layer"].as_u64().unwrap(); + let layer = s.active_comp().unwrap().layer(effectcraft_project::LayerId(l)).unwrap(); + let dur = (layer.out_point - layer.in_point).seconds(); + assert!((dur - 6.0).abs() < 0.05, "15 loops of a 0.4 s sequence: {dur}"); +} + #[test] fn percentage_guides_follow_the_comp_size() { let mut s = comp(); diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index 39a4d40b..14eb7dd4 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -410,6 +410,77 @@ fn pen_tool_draws_edits_and_closes_a_mask() { assert_eq!(s.active_comp().unwrap().layers[0].masks().unwrap().children.len(), 1); } +/// Cut every mask path's tangent arrays in the saved project to one entry (static values and +/// keyframes alike), then open the result: the shape of a hand-edited or foreign file (#1250). +fn reopen_with_short_tangents(s: &mut Session) { + fn cut(v: &mut Value) { + match v { + Value::Object(o) => { + if o.contains_key("vertices") { + for k in ["in_tangents", "out_tangents"] { + if let Some(Value::Array(a)) = o.get_mut(k) { + a.truncate(1); + } + } + } + o.values_mut().for_each(cut); + } + Value::Array(a) => a.iter_mut().for_each(cut), + _ => {} + } + } + let mut j: Value = serde_json::from_str(&s.project.to_json()).unwrap(); + cut(&mut j); + let p = effectcraft_project::Project::from_json(&j.to_string()).unwrap(); + s.replace_project(p, None); +} + +#[test] +fn vertex_commands_fill_in_missing_tangents_of_a_loaded_path() { + let (mut s, l) = setup(); + let m = s.execute("mask.new", json!({"layer": l, "vertices": [[10, 10], [90, 10], [90, 90]]})).unwrap()["mask"].as_u64().unwrap(); + reopen_with_short_tangents(&mut s); + let path = |s: &Session| prop(s, l, "masks/#1/path").value.as_path().unwrap().clone(); + assert_eq!(path(&s).in_tangents.len(), 1, "the file really has short tangents"); + // Both used to index the tangent arrays by vertex and panic. + s.execute("mask.setVertex", json!({"layer": l, "mask": m, "index": 2, "in": [0, -20], "out": [0, 20]})).unwrap(); + s.execute("mask.addVertex", json!({"layer": l, "mask": m, "point": [10, 90]})).unwrap(); + let p = path(&s); + assert_eq!(p.vertices.len(), 4); + assert_eq!((p.in_tangents.len(), p.out_tangents.len()), (4, 4)); + assert_eq!((p.in_tangents[2], p.out_tangents[2]), ([0.0, -20.0], [0.0, 20.0])); + assert_eq!(p.in_tangents[1], [0.0, 0.0], "a missing tangent is a corner"); +} + +#[test] +fn deleting_vertices_of_a_loaded_path_with_short_tangents_does_not_panic() { + let (mut s, l) = setup(); + let m = s.execute("mask.new", json!({"layer": l, "vertices": [[10, 10], [90, 10], [90, 90]]})).unwrap()["mask"].as_u64().unwrap(); + reopen_with_short_tangents(&mut s); + s.execute("mask.selectVertices", json!({"vertices": [{"layer": l, "mask": m, "index": 2}]})).unwrap(); + s.execute("mask.deleteVertices", json!({})).unwrap(); + let p = prop(&s, l, "masks/#1/path").value.as_path().unwrap().clone(); + assert_eq!((p.vertices.len(), p.in_tangents.len(), p.out_tangents.len()), (2, 2, 2)); +} + +#[test] +fn vertex_commands_fill_in_missing_tangents_of_a_loaded_animated_path() { + let (mut s, l) = setup(); + let m = s.execute("mask.new", json!({"layer": l, "vertices": [[10, 10], [90, 10], [90, 90]]})).unwrap()["mask"].as_u64().unwrap(); + s.execute("prop.toggleAnimation", json!({"layer": l, "path": "masks/#1/path"})).unwrap(); + s.execute("time.set", json!({"time": 1.0})).unwrap(); + s.execute("mask.setVertex", json!({"layer": l, "mask": m, "index": 0, "point": [0, 0]})).unwrap(); + reopen_with_short_tangents(&mut s); + s.execute("time.set", json!({"time": 1.0})).unwrap(); + s.execute("mask.setVertex", json!({"layer": l, "mask": m, "index": 2, "in": [5, 5]})).unwrap(); + s.execute("mask.addVertex", json!({"layer": l, "mask": m, "index": 3, "point": [10, 90]})).unwrap(); + for k in &prop(&s, l, "masks/#1/path").keys { + let p = k.value.as_path().unwrap(); + assert_eq!(p.vertices.len(), 4, "the other key gains the vertex too"); + assert!(p.in_tangents.len() == 4 && p.out_tangents.len() == 4, "{p:?}"); + } +} + #[test] fn alt_click_stopwatch_toggles_a_self_reference_expression() { let (mut s, l) = setup(); diff --git a/crates/engine/src/viewer.rs b/crates/engine/src/viewer.rs index cac75024..997c2023 100644 --- a/crates/engine/src/viewer.rs +++ b/crates/engine/src/viewer.rs @@ -362,7 +362,7 @@ const MAX_COLLAPSED_DEPTH: usize = 8; fn visible_layers<'a>(ctx: &EvalCtx<'a>, exclude: &'a [LayerId]) -> impl Iterator { let comp = ctx.comp; let time = ctx.time; - let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av() && l.is_active_at(time)); + let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av()); comp.layers.iter().filter(move |l| l.is_active_at(time) && l.switches.video && (!any_solo || l.switches.solo) && !exclude.contains(&l.id)) } diff --git a/crates/export/src/pipeline.rs b/crates/export/src/pipeline.rs index a0093f4c..8dcee03e 100644 --- a/crates/export/src/pipeline.rs +++ b/crates/export/src/pipeline.rs @@ -48,22 +48,43 @@ pub(crate) fn gpu_inflight(var: Option<&str>) -> usize { /// one window of rendered images is held in memory. Chunks hold no lock or slot, so a failing /// or panicking frame cannot leak one, and nested parallelism (or a single thread) cannot /// deadlock. +#[cfg(test)] pub(crate) fn map_frames( ks: Vec, cap: Option, render: impl Fn(u64) -> I + Sync + Send, encode: impl Fn(u64, I) -> T + Sync + Send, +) -> Vec { + map_frames_while(ks, cap, || true, render, encode) +} + +/// [`map_frames`] whose `cap` holds only while `capped()`: once it turns false (the GPU stopped +/// for good, so every frame falls back to the CPU), the remaining frames render in parallel +/// like the uncapped CPU path instead of one chunk of `cap` at a time. +pub(crate) fn map_frames_while( + ks: Vec, + cap: Option, + capped: impl Fn() -> bool, + render: impl Fn(u64) -> I + Sync + Send, + encode: impl Fn(u64, I) -> T + Sync + Send, ) -> Vec { use rayon::prelude::*; - let Some(n) = cap.map(|n| n.max(1)) else { + let Some(n) = cap.map(|n| n.max(1)).filter(|_| capped()) else { return ks.into_par_iter().map(|k| encode(k, render(k))).collect(); }; let window = n.max(rayon::current_num_threads()); let mut out = Vec::with_capacity(ks.len()); - for win in ks.chunks(window) { + for (w, win) in ks.chunks(window).enumerate() { + if !capped() { + out.par_extend(ks[w * window..].par_iter().map(|&k| encode(k, render(k)))); + break; + } let mut imgs = Vec::with_capacity(win.len()); - for chunk in win.chunks(n) { - imgs.par_extend(chunk.par_iter().map(|&k| render(k))); + let mut i = 0; + while i < win.len() { + let end = if capped() { (i + n).min(win.len()) } else { win.len() }; + imgs.par_extend(win[i..end].par_iter().map(|&k| render(k))); + i = end; } out.par_extend(win.par_iter().copied().zip(imgs.into_par_iter()).map(|(k, img)| encode(k, img))); } @@ -187,7 +208,10 @@ impl<'a> Cx<'a> { } return out; } - map_frames(ks, self.gpu_cap(), |k| self.frame(comp, k), f) + // The cap is for the GPU's memory: once it has stopped for good, frames render on the + // CPU in parallel. + let gpu_live = || self.job.accel.is_none_or(|a| a.available()); + map_frames_while(ks, self.gpu_cap(), gpu_live, |k| self.frame(comp, k), f) } /// GPU frames rendered at once (#879), decided once per batch: with a GPU accelerator every @@ -453,6 +477,36 @@ mod tests { (out, max.load(Ordering::SeqCst)) } + #[test] + fn frames_render_in_parallel_once_the_gpu_stops() { + // Every frame after the GPU stopped falls back to the CPU: the cap no longer applies. + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + let pool = rayon::ThreadPoolBuilder::new().num_threads(4).build().unwrap(); + let (live, now, max) = (AtomicBool::new(true), AtomicUsize::new(0), AtomicUsize::new(0)); + let out = pool.install(|| { + map_frames_while( + (0..24).collect(), + Some(1), + || live.load(Ordering::SeqCst), + |k| { + if k == 1 { + live.store(false, Ordering::SeqCst); + } + let c = now.fetch_add(1, Ordering::SeqCst) + 1; + if k >= 2 { + max.fetch_max(c, Ordering::SeqCst); + } + std::thread::sleep(std::time::Duration::from_millis(20)); + now.fetch_sub(1, Ordering::SeqCst); + k * 10 + }, + |_, v| v, + ) + }); + assert_eq!(out, (0..24).map(|k| k * 10).collect::>()); + assert!(max.load(Ordering::SeqCst) > 1, "frames after the GPU stopped ran one at a time"); + } + #[test] fn capped_frames_keep_their_order() { let want: Vec = (0..40).map(|k| k * 10).collect(); diff --git a/crates/export/tests/roundtrip.rs b/crates/export/tests/roundtrip.rs index 94b83991..0fc74701 100644 --- a/crates/export/tests/roundtrip.rs +++ b/crates/export/tests/roundtrip.rs @@ -613,3 +613,28 @@ fn wav_and_aiff_audio_only() { } } } + +/// An AIFF EffectCraft renders imports back (#1428): plain 16-bit and AIFF-C `fl32`. +#[test] +fn rendered_aiff_imports_back() { + use effectcraft_export::render_queue::AudioFormat; + let d = out_dir("aiff-import"); + let wav = d.join("tone.wav"); + write_tone(&wav, 2.0); + let (p, cid) = project(Some(&wav)); + let pool = MediaPool::new(); + for (format, name) in [(AudioFormat::S16, "s16.aif"), (AudioFormat::F32, "f32.aif")] { + // 44.1 kHz, not the 48 kHz the samples are read back at: a wrong rate shows in the duration. + let om = OutputModule { audio_sample_rate: 44_100, audio_channels: 2, audio_format: format, ..OutputModule::for_format(OutputFormat::Aiff) }; + let path = d.join(name); + assert!(run(&p, cid, &pool, &om, &path).audio); + let f = effectcraft_media::probe(&path).unwrap_or_else(|e| panic!("{name}: {e}")); + assert!(f.has_audio && !f.has_video, "{name}"); + assert_eq!(f.duration, Tick::from_seconds_f64(1.2), "{name}: 52 920 frames at 44.1 kHz"); + // Tone on the left, silence on the right: the two channels are read as two. + let s = MediaPool::new().audio_samples(&f, Tick::from_seconds_f64(0.3), 24_000, 48_000); + let (l, r) = (rms(s.iter().step_by(2).copied()), rms(s.iter().skip(1).step_by(2).copied())); + assert!((l - 0.3536).abs() < 0.01, "{name}: left RMS {l}"); + assert!(r < 0.01, "{name}: right RMS {r}"); + } +} diff --git a/crates/expr/src/prelude.js b/crates/expr/src/prelude.js index 5f6dd015..a1dff678 100644 --- a/crates/expr/src/prelude.js +++ b/crates/expr/src/prelude.js @@ -261,7 +261,10 @@ function noise(x) { function timeToFrames(t, fps, isDuration) { if (t === undefined) t = time + thisComp.displayStartTime; if (fps === undefined) fps = 1 / __S.fd; - return Math.floor(t * fps + 1e-6); + var frames = t * fps; + // The 1e-6 keeps a whole number of frames that floating point overshoots (31/24 - 5/24) whole. + if (isDuration && frames !== 0) return frames < 0 ? Math.floor(frames + 1e-6) : Math.ceil(frames - 1e-6); + return Math.floor(frames + 1e-6); } function framesToTime(frames, fps) { diff --git a/crates/expr/src/tests.rs b/crates/expr/src/tests.rs index d71db390..8a4db485 100644 --- a/crates/expr/src/tests.rs +++ b/crates/expr/src/tests.rs @@ -1285,3 +1285,80 @@ fn layer_control_property_stands_for_its_layer() { assert_close!(f.num("effect('Map')(1).transform.scale[0]", 0.0), 100.0); assert_close!(f.num("effect('Map')(1).value.name == 'B' ? 1 : 0", 0.0), 1.0); } + +#[test] +fn time_to_frames_rounds_durations_away_from_zero() { + let f = fx(); + // Adobe's public expression reference distinguishes durations from absolute times. + // Expected integer counts are literal values, independent of the implementation. + for (expression, expected) in [ + ("timeToFrames(-0.01, 24, true)", -1.0), + ("timeToFrames(-0.05, 24, true)", -2.0), + ("timeToFrames(0, 24, true)", 0.0), + ("timeToFrames(-0.125, 24, true)", -3.0), + ("timeToFrames(0.125, 24, true)", 3.0), + ("timeToFrames(0.01, 24, true)", 1.0), + ("timeToFrames(0.05, 24, true)", 2.0), + ] { + assert_eq!(f.num(expression, 0.0), expected, "{expression}"); + } +} + +#[test] +fn time_to_frames_rounds_absolute_times_toward_negative_infinity() { + let f = fx(); + for (expression, expected) in [ + ("timeToFrames(-0.01, 24, false)", -1.0), + ("timeToFrames(-0.05, 24, false)", -2.0), + ("timeToFrames(0, 24, false)", 0.0), + ("timeToFrames(-0.125, 24, false)", -3.0), + ("timeToFrames(0.125, 24, false)", 3.0), + ("timeToFrames(0.01, 24, false)", 0.0), + ("timeToFrames(0.05, 24, false)", 1.0), + ] { + assert_eq!(f.num(expression, 0.0), expected, "{expression}"); + } +} + +#[test] +fn time_to_frames_preserves_default_arguments() { + let f = fx(); + // The fixture's composition is 30 fps with displayStartTime = 0. + for (expression, evaluation_time, expected) in [ + ("timeToFrames(0.01, 24)", 0.0, 0.0), + ("timeToFrames(-0.01, 24)", 0.0, -1.0), + ("timeToFrames(0.05)", 0.0, 1.0), + ("timeToFrames(-0.05)", 0.0, -2.0), + ("timeToFrames()", 0.0, 0.0), + ("timeToFrames()", 0.05, 1.0), + ("timeToFrames()", -0.05, -2.0), + ("timeToFrames(undefined, 24, true)", 0.05, 2.0), + ("timeToFrames(0.05, undefined, true)", 0.0, 2.0), + ] { + assert_eq!(f.num(expression, evaluation_time), expected, "{expression} at {evaluation_time}s"); + } +} + +#[test] +fn time_to_frames_preserves_coercion_and_small_durations() { + let f = fx(); + // Duration rounding must not erase tiny nonzero values; absolute times keep the existing tolerance. + for (expression, expected) in [ + ("timeToFrames('0.05', '24', false)", 1.0), + ("timeToFrames('0.05', '24', true)", 2.0), + ("timeToFrames('-0.05', '24', true)", -2.0), + ("timeToFrames(null, 24, true)", 0.0), + ("timeToFrames(0.05, null, true)", 0.0), + ("timeToFrames(0.05, 0, true)", 0.0), + ("timeToFrames(31 / 24 - 5 / 24, 24, true)", 26.0), + ("timeToFrames(5 / 24 - 31 / 24, 24, true)", -26.0), + ("timeToFrames(-1e-10, 24, false)", 0.0), + ("1 / timeToFrames(-0, 24, true) === Infinity ? 1 : 0", 1.0), + ("isNaN(timeToFrames(NaN, 24, true)) ? 1 : 0", 1.0), + ("timeToFrames(Infinity, 24, true) === Infinity ? 1 : 0", 1.0), + ("timeToFrames(-Infinity, 24, true) === -Infinity ? 1 : 0", 1.0), + ("var calls = 0; var t = {valueOf: function () { calls++; return 0.05; }}; timeToFrames(t, 24, true) + 100 * calls", 102.0), + ] { + assert_eq!(f.num(expression, 0.0), expected, "{expression}"); + } +} diff --git a/crates/gpu/src/context.rs b/crates/gpu/src/context.rs index 51221abc..3c372222 100644 --- a/crates/gpu/src/context.rs +++ b/crates/gpu/src/context.rs @@ -917,6 +917,12 @@ impl GpuContext { Some(g) } + /// Entries in the layer upload cache. + #[cfg(test)] + pub(crate) fn upload_entries(&self) -> usize { + self.uploads.lock().map(|u| u.map.len()).unwrap_or_default() + } + /// Upload a (layer cache) buffer, reusing the texture while the same `Arc` is alive: static /// layers upload once, not every frame. pub(crate) fn upload_buf(&self, buf: &Arc) -> Option { @@ -939,14 +945,14 @@ impl GpuContext { u.bytes -= old.bytes; } u.bytes += bytes; - // Drop textures of freed buffers, then the least recently used over budget. + // Drop textures of freed buffers at once (a layer whose buffer changes every frame + // makes a new one each time, and nothing can hit the old one again), then the least + // recently used over budget. let budget = self.upload_budget(); - if u.bytes > budget { - let dead: Vec = u.map.iter().filter(|(_, e)| e.buf.strong_count() == 0).map(|(k, _)| *k).collect(); - for k in dead { - if let Some(e) = u.map.remove(&k) { - u.bytes -= e.bytes; - } + let dead: Vec = u.map.iter().filter(|(_, e)| e.buf.strong_count() == 0).map(|(k, _)| *k).collect(); + for k in dead { + if let Some(e) = u.map.remove(&k) { + u.bytes -= e.bytes; } } if u.bytes > budget && !self.upload_full_warned.swap(true, std::sync::atomic::Ordering::Relaxed) { diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index 252145c1..1cff8be0 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -317,6 +317,10 @@ impl Accelerator for Gpu { self.ctx.name.clone() } + fn available(&self) -> bool { + self.ctx.check_health().is_ok() + } + fn comp_frame(&self, r: &Renderer, comp: ItemId, t: Tick) -> Option { let img = self.render(r, comp, t); if img.is_none() { diff --git a/crates/gpu/src/shaders/fx_color.wgsl b/crates/gpu/src/shaders/fx_color.wgsl index c7d6010e..6b8641dd 100644 --- a/crates/gpu/src/shaders/fx_color.wgsl +++ b/crates/gpu/src/shaders/fx_color.wgsl @@ -154,8 +154,9 @@ fn fxc_point(@builtin(global_invocation_id) gid: vec3) { let i = u32(max(floor(hp), 0.0)) % 6u; let f = hp - floor(hp); let wt = w[i] + (w[(i + 1u) % 6u] - w[i]) * f; - let base = luminance(c); - let gray = max(base + (wt - 0.5) * hsl.y * 0.6, 0.0); + let mx = max(max(c.x, c.y), c.z); + let mn = min(min(c.x, c.y), c.z); + let gray = max(mn + (mx - mn) * wt, 0.0); var o = vec3(gray); if (P.u[0].y != 0u) { o = hsl_to_rgb(P.f[2].x, P.f[2].y, min(gray, 1.0)); diff --git a/crates/gpu/src/shaders/fx_generate.wgsl b/crates/gpu/src/shaders/fx_generate.wgsl index 60fc2f9d..f46b2f0b 100644 --- a/crates/gpu/src/shaders/fx_generate.wgsl +++ b/crates/gpu/src/shaders/fx_generate.wgsl @@ -510,8 +510,10 @@ fn gen_fourcolor(@builtin(global_invocation_id) gid: vec3) { let n = (hash_noise(u32(p.x), u32(p.y), P.u[0].y) - 0.5) * jitter * (8.0 / 255.0); c += vec3(n); } - let op = P.f[8].z; - textureStore(out, p, gen_blend(textureLoad(src, p, 0), vec4(c * op, op), P.u[0].x)); + let o = textureLoad(src, p, 0); + // None keeps the layer's alpha, as the CPU path does (#1381). + let op = select(P.f[8].z, P.f[8].z * o.w, P.u[0].x == 255u); + textureStore(out, p, gen_blend(o, vec4(c * op, op), P.u[0].x)); } // ---------------------------------------------------------------- Noise / Add Grain diff --git a/crates/gpu/src/tests.rs b/crates/gpu/src/tests.rs index 696402d6..24f8c961 100644 --- a/crates/gpu/src/tests.rs +++ b/crates/gpu/src/tests.rs @@ -49,18 +49,29 @@ pub(crate) fn hold_gpu_lock() { /// The shared test device (the calling test holds [`hold_gpu_lock`]). pub(crate) fn gpu() -> Option<&'static Gpu> { - static G: OnceLock> = OnceLock::new(); + static G: OnceLock> = OnceLock::new(); hold_gpu_lock(); - G.get_or_init(|| { - let g = Gpu::headless(); - if g.is_none() { - eprintln!("effectcraft-gpu tests: no GPU adapter, skipping GPU comparisons"); - } else if let Some(g) = &g { - eprintln!("effectcraft-gpu tests: adapter {}", g.ctx.name); + let g = G.get_or_init(|| { + let g = Gpu::try_headless(); + match &g { + Ok(g) => eprintln!("effectcraft-gpu tests: adapter {}", g.ctx.name), + Err(e) => eprintln!("effectcraft-gpu tests: no GPU ({e}), skipping GPU comparisons"), } g - }) - .as_ref() + }); + require_gpu("effectcraft-gpu tests", g.as_ref()) +} + +/// `g`, or `None` (the GPU tests skip) when there is no GPU, unless `EFFECTCRAFT_REQUIRE_GPU=1` +/// (as in `tests_init`): then a GPU that didn't start fails the test, with why. +pub(crate) fn require_gpu(tests: &str, g: Result) -> Option { + match g { + Ok(g) => Some(g), + Err(e) if std::env::var("EFFECTCRAFT_REQUIRE_GPU").is_ok_and(|v| v == "1") => { + panic!("{tests}: EFFECTCRAFT_REQUIRE_GPU=1 but GPU initialization failed: {e}") + } + Err(_) => None, + } } /// The test device is on wgpu's OpenGL backend (Mesa's llvmpipe on the FreeBSD CI job). @@ -1044,3 +1055,19 @@ fn static_effect_layers_draw_from_the_cache() { assert!(frames[1] == frames[2]); check("static effect layer vs CPU", compare_at(&s, opts(), t(2)), 0.0); } + +/// The layer upload cache drops the textures of freed buffers when it inserts, not only once +/// it is over budget (an animated layer makes a new buffer every frame). +#[test] +fn upload_cache_drops_textures_of_freed_buffers() { + let Some(g) = gpu() else { return }; + let img = effectcraft_raster::Image::new(256, 256); + let before = g.ctx.upload_entries(); + for _ in 0..6 { + let buf = std::sync::Arc::new(effectcraft_effects::Buf { img: img.clone(), offset: [0.0; 2], scale: 1.0 }); + let up = g.ctx.upload_buf(&buf).expect("upload"); + drop((up, buf)); + } + let kept = g.ctx.upload_entries(); + assert!(kept <= before + 1, "the upload cache kept {kept} textures of freed buffers (had {before})"); +} diff --git a/crates/gpu/src/tests_adv3d.rs b/crates/gpu/src/tests_adv3d.rs index ee99c5c0..869c60aa 100644 --- a/crates/gpu/src/tests_adv3d.rs +++ b/crates/gpu/src/tests_adv3d.rs @@ -17,11 +17,13 @@ use crate::Gpu; /// A device of the test's own (the test holds [`crate::tests::hold_gpu_lock`]). fn gpu() -> Option { crate::tests::hold_gpu_lock(); - let g = Gpu::headless(); - if g.is_none() { - eprintln!("effectcraft-gpu adv3d tests: no GPU adapter, skipping"); + match Gpu::try_headless() { + Ok(g) => Some(g), + Err(e) => { + eprintln!("effectcraft-gpu adv3d tests: no GPU ({e}), skipping"); + crate::tests::require_gpu("effectcraft-gpu adv3d tests", Err(&e)) + } } - g } fn add(p: &mut Project, cid: ItemId, src: LayerSource, edit: impl FnOnce(&mut effectcraft_project::Layer)) { diff --git a/crates/gpu/src/tests_deferred.rs b/crates/gpu/src/tests_deferred.rs index e798bd8f..b6e43b55 100644 --- a/crates/gpu/src/tests_deferred.rs +++ b/crates/gpu/src/tests_deferred.rs @@ -17,19 +17,19 @@ use crate::tests::{Pattern, Scene, diff, n, set, tolerance, v3}; /// The device, held exclusively: deferred state is per device and a frame renders at a time /// (a frame worker renders one frame at a time too). pub(crate) fn deferred_gpu() -> Option<(&'static Gpu, std::sync::MutexGuard<'static, ()>)> { - static G: OnceLock> = OnceLock::new(); + static G: OnceLock> = OnceLock::new(); static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); // Also no other GPU test while it runs (see `hold_gpu_lock`). crate::tests::hold_gpu_lock(); - let g = G - .get_or_init(|| { - let g = pollster::block_on(Gpu::request_deferred()).map_err(|e| eprintln!("deferred gpu tests skipped: {e}")).ok(); - if let Some(g) = &g { - eprintln!("deferred gpu tests: adapter {}", g.ctx.name); - } - g - }) - .as_ref()?; + let g = G.get_or_init(|| { + let g = pollster::block_on(Gpu::request_deferred()); + match &g { + Ok(g) => eprintln!("deferred gpu tests: adapter {}", g.ctx.name), + Err(e) => eprintln!("deferred gpu tests: no GPU ({e}), skipping"), + } + g + }); + let g = crate::tests::require_gpu("deferred gpu tests", g.as_ref())?; Some((g, LOCK.lock().unwrap_or_else(|e| e.into_inner()))) } diff --git a/crates/gpu/src/tests_fx_color.rs b/crates/gpu/src/tests_fx_color.rs index 28e80278..a3fe3f65 100644 --- a/crates/gpu/src/tests_fx_color.rs +++ b/crates/gpu/src/tests_fx_color.rs @@ -29,6 +29,7 @@ fn color_balance_vibrance_tritone_mixer() { fn black_white_colorama_selective_color() { effect_case("ec.color.blackwhite", &[]); effect_case("ec.color.blackwhite", &[("reds", n(120.0)), ("blues", n(-50.0)), ("cyans", n(200.0)), ("tint", on())]); + effect_case("ec.color.blackwhite", &[("yellows", n(300.0)), ("greens", n(-200.0)), ("magentas", n(0.0))]); effect_case("ec.color.colorama", &[]); effect_case("ec.color.colorama", &[("inputPhase/mode", e(4)), ("inputPhase/phaseShift", n(75.0)), ("outputCycle/cycles", n(2.5)), ("blend", n(40.0))]); effect_case("ec.color.colorama", &[("inputPhase/mode", e(8)), ("outputCycle/cycles", n(1.3))]); diff --git a/crates/gpu/src/walk.rs b/crates/gpu/src/walk.rs index e9541229..b1209070 100644 --- a/crates/gpu/src/walk.rs +++ b/crates/gpu/src/walk.rs @@ -73,6 +73,9 @@ fn draw_comp<'a>(e: &mut Enc, r: &Renderer<'a>, ctx: &EvalCtx<'a>, canvas: &mut let levels = r.pipe().levels; let mut i = 0; while i < visible.len() { + // A device that ran out of memory (or was lost) on an earlier layer fails every later + // one: give the frame back to the CPU now instead of drawing the rest of the comp. + e.g.check_health().ok()?; if visible[i].is_3d() { let mut j = i; while j < visible.len() && visible[j].is_3d() { diff --git a/crates/host/tests/relink.rs b/crates/host/tests/relink.rs index 5bb31b4e..f66d8b80 100644 --- a/crates/host/tests/relink.rs +++ b/crates/host/tests/relink.rs @@ -38,14 +38,14 @@ fn replacing_a_missing_file_relinks_the_others_that_moved_with_it() { let before = footage_paths(&s); assert_eq!(before.len(), 2, "{before:?}"); // The job folder moves; both files go missing. - let new = root.join("Archive/Job"); + let new = root.join("Archive").join("Job"); std::fs::create_dir_all(root.join("Archive")).unwrap(); std::fs::rename(&old, &new).unwrap(); let r = s.execute("footage.check", json!({"wait": true})).unwrap(); assert_eq!(r["missing"], 2, "{r}"); // Locating one finds the other. let a = before.iter().find(|(_, p, _)| p.ends_with("a.png")).unwrap().0; - let r = s.execute("file.replaceFootage", json!({"item": a, "path": new.join("stills/a.png").to_string_lossy()})).unwrap(); + let r = s.execute("file.replaceFootage", json!({"item": a, "path": new.join("stills").join("a.png").to_string_lossy()})).unwrap(); assert_eq!(r["relinked"].as_array().map(Vec::len), Some(1), "{r}"); let after = footage_paths(&s); assert!(after.iter().all(|(_, p, missing)| p.starts_with(&*new.to_string_lossy()) && !missing), "{after:?}"); diff --git a/crates/host/tests/webm_metadata.rs b/crates/host/tests/webm_metadata.rs new file mode 100644 index 00000000..2e3fed9d --- /dev/null +++ b/crates/host/tests/webm_metadata.rs @@ -0,0 +1,28 @@ +//! A WebM rendered by EffectCraft and imported back reports its real frame count (#1429). Matroska +//! stores the duration in whole milliseconds, so at 30 fps a 1-frame file lasts 33 ms, just short +//! of one 33.33 ms frame. + +use serde_json::json; + +fn tmp(name: &str) -> String { + let d = std::env::temp_dir().join(format!("effectcraft-host-webm-meta-{}", std::process::id())); + std::fs::create_dir_all(&d).unwrap(); + d.join(name).to_string_lossy().to_string() +} + +#[test] +fn a_webm_at_30_fps_reports_every_frame() { + for n in [1u32, 4] { + let mut s = effectcraft_host::session(); + let out = tmp(&format!("frames-{n}.webm")); + s.execute("comp.new", json!({"name": "Main", "width": 64, "height": 36, "frameRate": 30, "duration": 1})).unwrap(); + s.execute("layer.newSolid", json!({"name": "BG", "color": "#142850"})).unwrap(); + let span = json!({"comp": "Main", "format": "webm", "output": out, "timeSpan": "custom", "start": 0, "duration": n as f64 / 30.0}); + s.execute("renderQueue.add", span).unwrap(); + s.execute("renderQueue.render", json!({"wait": true})).unwrap(); + let r = s.execute("file.import", json!({"paths": [out.clone()]})).unwrap(); + let m = s.execute("item.metadata", json!({"item": r["items"][0]})).unwrap(); + let _ = std::fs::remove_file(&out); + assert_eq!(m["item"]["frames"], json!(n), "{n} rendered frames: {m}"); + } +} diff --git a/crates/keyframe/src/value.rs b/crates/keyframe/src/value.rs index fa66711b..1dff4477 100644 --- a/crates/keyframe/src/value.rs +++ b/crates/keyframe/src/value.rs @@ -42,6 +42,16 @@ impl ShapePath { pub fn len(&self) -> usize { self.vertices.len() } + /// Give both tangent arrays one entry per vertex: missing tangents become zero (a corner), + /// extra ones are dropped. The three arrays are read independently from a project file, so a + /// loaded path can disagree with itself (#1250). Returns whether anything changed. + pub fn fit_tangents(&mut self) -> bool { + let n = self.vertices.len(); + let changed = self.in_tangents.len() != n || self.out_tangents.len() != n; + self.in_tangents.resize(n, [0.0; 2]); + self.out_tangents.resize(n, [0.0; 2]); + changed + } /// Number of segments (vertex to next vertex, the closing one included). pub fn segment_count(&self) -> usize { let n = self.vertices.len(); diff --git a/crates/lottie/src/shapes.rs b/crates/lottie/src/shapes.rs index d7e3cc08..1283a249 100644 --- a/crates/lottie/src/shapes.rs +++ b/crates/lottie/src/shapes.rs @@ -308,7 +308,7 @@ pub(crate) fn export_items(ex: &mut Ex, contents: &PropGroup, label: &str) -> Ve "op" => { o.put("a", "amount", &anim::num); o.raw("lj", json!(o.e("join") + 1)); - o.raw("ml", json!(o.f("miter"))); + o.put("ml", "miter", &anim::num); if (o.f("copies") - 1.0).abs() > 1e-9 || o.animated("copies") || o.f("copyOffset") != 0.0 { o.ex.warn(format!("{lbl}: Offset Paths copies are not part of Lottie (one copy exported)")); } @@ -543,9 +543,7 @@ pub(crate) fn import_items(im: &mut Im, ids: &mut Ids, items: &[Json]) -> Vec set(im, &mut g, "amount", j.get("a"), &anim::to_scalar), "tw" => { diff --git a/crates/lottie/src/tests.rs b/crates/lottie/src/tests.rs index a677def1..19aac0f9 100644 --- a/crates/lottie/src/tests.rs +++ b/crates/lottie/src/tests.rs @@ -358,6 +358,55 @@ fn path_operators_roundtrip() { compare_renders(&p, cid, &q, nc, 2.0 / 255.0); } +#[test] +fn offset_paths_miter_uses_animatable_lottie_value() { + let (mut p, cid) = setup(); + let l = shape_layer(&mut p, cid, |ids| { + let mut animated = build::shape_simple_op(ids, "offset").unwrap(); + animated.get_mut("miter").unwrap().keys = vec![Keyframe::new(s(0.0), Value::Scalar(5.0)), Keyframe::new(s(1.0), Value::Scalar(9.0))]; + let mut constant = build::shape_simple_op(ids, "offset").unwrap(); + constant.get_mut("miter").unwrap().value = Value::Scalar(6.0); + let mut stroke = build::shape_stroke(ids, [1.0, 1.0, 1.0, 1.0], 2.0); + stroke.get_mut("miter").unwrap().value = Value::Scalar(7.0); + vec![animated, constant, stroke] + }); + push(&mut p, cid, l); + + let (q, nc, warnings, json) = roundtrip(&p, cid, &ExportOptions::default()); + assert!(warnings.is_empty(), "{warnings:?}"); + let shapes = &json["layers"][0]["shapes"]; + assert_eq!(shapes[0]["ty"], "op"); + assert_eq!(shapes[0]["ml"]["a"], json!(1)); + assert_eq!(shapes[0]["ml"]["k"][0]["s"], json!([5.0])); + assert_eq!(shapes[0]["ml"]["k"][1]["s"], json!([9.0])); + assert_eq!(shapes[1]["ml"], json!({"a": 0, "k": 6.0})); + assert_eq!(shapes[2]["ty"], "st"); + assert_eq!(shapes[2]["ml"], json!(7.0)); + assert!(compare_comps(&p, cid, &q, nc) > 10); + let restored = &q.comp(nc).unwrap().layers[0]; + assert_eq!(restored.props.group("contents/offset").unwrap().get("miter").unwrap().keys.len(), 2); + + // Previously exported Offset Paths have a scalar ml; they remain importable. + let mut old = json; + old["layers"][0]["shapes"][1]["ml"] = json!(6.0); + let mut legacy = Project::default(); + let bytes = serde_json::to_vec(&old).unwrap(); + let imported = import(&mut legacy, &bytes, "Legacy", "", &mut |_| None).unwrap(); + let constant = &legacy.comp(imported.comp).unwrap().layers[0].props; + let second = constant + .sub("contents") + .unwrap() + .children + .iter() + .filter_map(|n| match n { + Node::Group(g) if g.match_id == "offset" => Some(g), + _ => None, + }) + .nth(1) + .unwrap(); + assert_eq!(second.get("miter").unwrap().value, Value::Scalar(6.0)); +} + // ---------------------------------------------------------------- layers #[test] diff --git a/crates/media/Cargo.toml b/crates/media/Cargo.toml index a3ff624b..bb47dccb 100644 --- a/crates/media/Cargo.toml +++ b/crates/media/Cargo.toml @@ -24,6 +24,8 @@ filmcraft-isobmff = { workspace = true } filmcraft-frame = { workspace = true } filmcraft-color = { workspace = true } filmcraft-time = { workspace = true } +# AIFF import (#1428): enables the aiff reader of filmcraft-codecs' symphonia. +symphonia = { workspace = true } exr = { workspace = true } gif = { workspace = true } image = { workspace = true, features = ["gif", "webp", "tiff", "bmp", "exr", "tga", "hdr"] } diff --git a/crates/pdf/src/cff.rs b/crates/pdf/src/cff.rs index 1c8c603e..275b75aa 100644 --- a/crates/pdf/src/cff.rs +++ b/crates/pdf/src/cff.rs @@ -394,11 +394,16 @@ impl Cff { let private = |pd: &[(u16, Vec)]| -> (Vec>, (f64, f64)) { let Some(pv) = get(pd, 18).filter(|v| v.len() == 2) else { return (vec![], (0.0, 0.0)) }; let (size, off) = (pv[0] as usize, pv[1] as usize); - let Some(bytes) = d.get(off..off + size) else { return (vec![], (0.0, 0.0)) }; + let Some(bytes) = off.checked_add(size).and_then(|end| d.get(off..end)) else { return (vec![], (0.0, 0.0)) }; let pr = dict(bytes); let nominal = get(&pr, 21).and_then(|v| v.first().copied()).unwrap_or(0.0); let default = get(&pr, 20).and_then(|v| v.first().copied()).unwrap_or(0.0); - let subrs = get(&pr, 19).and_then(|v| v.first().copied()).and_then(|s| index(d, off + s as usize)).map(|x| x.0).unwrap_or_default(); + let subrs = get(&pr, 19) + .and_then(|v| v.first().copied()) + .and_then(|s| off.checked_add(s as usize)) + .and_then(|p| index(d, p)) + .map(|x| x.0) + .unwrap_or_default(); (subrs, (default, nominal)) }; let mut lsubrs = vec![]; @@ -533,6 +538,10 @@ impl Cff { /// A glyph's outline in glyph space (`font_matrix` maps it to text space) and its advance. pub fn outline(&self, gid: u16) -> Option<(BezPath, f64)> { + self.outline_depth(gid, 0) + } + + fn outline_depth(&self, gid: u16, depth: usize) -> Option<(BezPath, f64)> { let r = self.charstrings.get(gid as usize)?.clone(); let fd = self.fdselect.get(gid as usize).copied().unwrap_or(0) as usize; let fd = fd.min(self.lsubrs.len() - 1); @@ -547,13 +556,17 @@ impl Cff { _ => self.widths[fd].0, }; let mut path = st.path; - // endchar seac: base and accent glyphs by standard code. - if let Some((adx, ady, b, a)) = st.seac { + // endchar seac: base and accent glyphs by standard code. The nesting is bounded like the + // Type 1 interpreter's: a font whose accented glyphs seac into each other (or into + // themselves) must not recurse until the stack overflows. + if let Some((adx, ady, b, a)) = st.seac + && depth < 2 + { let find = |code: u8| crate::encoding::Base::Standard.name(code).and_then(|n| self.gid_by_name(&n)); - if let Some((bp, _)) = find(b).and_then(|g| self.outline(g)) { + if let Some((bp, _)) = find(b).and_then(|g| self.outline_depth(g, depth + 1)) { path.extend(bp); } - if let Some((ap, _)) = find(a).and_then(|g| self.outline(g)) { + if let Some((ap, _)) = find(a).and_then(|g| self.outline_depth(g, depth + 1)) { path.extend(kurbo::Affine::translate((adx, ady)) * ap); } } @@ -1048,4 +1061,88 @@ mod tests { assert!((p.area().abs() - 250_000.0).abs() < 1e-6, "{}", p.area()); assert_eq!(p.bounding_box(), kurbo::Rect::new(50.0, 0.0, 550.0, 500.0)); } + + /// An `endchar` seac whose base and accent name its own glyph used to recurse until the + /// stack overflowed and aborted the app; the depth limit cuts it off like the Type 1 + /// interpreter's. + #[test] + fn seac_cycles_are_cut_off_not_a_stack_overflow() { + let enc = |v: i32| -> Vec { + if (-107..=107).contains(&v) { + vec![(v + 139) as u8] + } else { + let mut o = vec![28]; + o.extend_from_slice(&(v as i16).to_be_bytes()); + o + } + }; + // `w adx ady bchar achar endchar`: both components are 'A' (standard code 65), this glyph. + let mut cs = vec![]; + for v in [500, 0, 0, 65, 65] { + cs.extend(enc(v)); + } + cs.push(14); + let font = write_test_cff(&[(".notdef", vec![14]), ("A", cs)]); + let cff = Cff::parse(&font).unwrap(); + let (p, _) = cff.outline(cff.gid_by_name("A").unwrap()).unwrap(); + assert!(p.elements().is_empty()); + } + + /// A CFF whose top DICT points its Private DICT at offsets near `usize::MAX`: `size` and `off` + /// come from the file, and reading the Private DICT must not overflow `off + size`. + #[test] + fn huge_private_dict_offsets_do_not_overflow() { + fn idx(items: &[Vec]) -> Vec { + let mut out = (items.len() as u16).to_be_bytes().to_vec(); + if items.is_empty() { + return out; + } + out.push(4); + let mut off = 1u32; + out.extend_from_slice(&off.to_be_bytes()); + for it in items { + off += it.len() as u32; + out.extend_from_slice(&off.to_be_bytes()); + } + for it in items { + out.extend_from_slice(it); + } + out + } + let int = |v: i32| { + let mut o = vec![29]; + o.extend_from_slice(&v.to_be_bytes()); + o + }; + // The BCD real "1E300": a size and an offset that both saturate at `usize::MAX`. + let huge = vec![0x1E, 0x1B, 0x30, 0x0F]; + let header = [1u8, 0, 4, 4]; + let names = idx(&[b"Test".to_vec()]); + let strings = idx(&[]); + let gsubrs = idx(&[]); + let charset = vec![0u8]; + let cs = idx(&[vec![14]]); // one `.notdef` charstring: endchar + let top_len = 5 + 1 + 5 + 1 + huge.len() * 2 + 1; + let top_index_len = 2 + 1 + 8 + top_len; + let base = header.len() + names.len() + top_index_len + strings.len() + gsubrs.len(); + let cs_off = base + charset.len(); + let mut top = vec![]; + top.extend(int(0)); + top.push(15); // charset: ISOAdobe + top.extend(int(cs_off as i32)); + top.push(17); // CharStrings + top.extend(&huge); + top.extend(&huge); + top.push(18); // Private: [size, offset] + assert_eq!(top.len(), top_len); + let mut out = header.to_vec(); + out.extend(names); + out.extend(idx(&[top])); + out.extend(strings); + out.extend(gsubrs); + out.extend(charset); + out.extend(cs); + let cff = Cff::parse(&out).expect("the damaged Private DICT is skipped, the font parses"); + assert_eq!(cff.glyph_count(), 1); + } } diff --git a/crates/pdf/src/encoding.rs b/crates/pdf/src/encoding.rs index ebb63c32..a9f0c840 100644 --- a/crates/pdf/src/encoding.rs +++ b/crates/pdf/src/encoding.rs @@ -208,10 +208,10 @@ pub fn name_to_unicode(name: &str) -> Option { if let Some((_, u)) = NAMES.iter().find(|(n, _)| *n == base) { return char::from_u32(*u); } - if let Some(h) = base.strip_prefix("uni") - && h.len() >= 4 - { - return u32::from_str_radix(&h[..4], 16).ok().and_then(char::from_u32); + // `get(..4)` also insists on a character boundary: a name like `uni\u{20AC}\u{20AC}` used to + // panic slicing inside a multi-byte character. + if let Some(x) = base.strip_prefix("uni").and_then(|h| h.get(..4)) { + return u32::from_str_radix(x, 16).ok().and_then(char::from_u32); } if let Some(h) = base.strip_prefix('u') && (4..=6).contains(&h.len()) @@ -386,4 +386,12 @@ mod tests { assert_eq!(unicode_to_name('Ω'), "Omega"); assert_eq!(unicode_to_name('中'), "uni4E2D"); } + + /// A glyph name like `uni\u{20AC}\u{20AC}` (multi-byte UTF-8 after `uni`) must not slice + /// inside a character. + #[test] + fn multibyte_glyph_names_do_not_panic() { + assert_eq!(name_to_unicode("uni\u{20AC}\u{20AC}"), None); + assert_eq!(name_to_unicode("uni\u{20AC}"), None); + } } diff --git a/crates/pdf/src/image.rs b/crates/pdf/src/image.rs index 5e6c49a5..2c8adda6 100644 --- a/crates/pdf/src/image.rs +++ b/crates/pdf/src/image.rs @@ -82,7 +82,10 @@ pub(crate) fn inline_len(file: &File, d: &Dict, res: Option<&Dict>) -> Option Interp<'a> { && (p + 2 == data.len() || crate::object::is_white(data[p + 2])) }; let end = match crate::image::inline_len(self.file, &d, Some(res)) { - Some(len) => (start + len).min(data.len()), + Some(len) => start.saturating_add(len).min(data.len()), None => { let mut p = start; while p + 1 < data.len() && !is_ei(p) { diff --git a/crates/pdf/src/tests_import.rs b/crates/pdf/src/tests_import.rs index e53727ef..7cebcd43 100644 --- a/crates/pdf/src/tests_import.rs +++ b/crates/pdf/src/tests_import.rs @@ -263,6 +263,22 @@ fn hostile_structures_return_errors_or_draw_nothing() { assert!(!doc.skipped.is_empty()); } +#[test] +fn inline_image_with_absurd_dimensions_falls_back_to_ei_scan() { + // `/W 5000000000000000000` overflows the row math (`* bpc` alone exceeds usize::MAX): + // `inline_len` must return `None` (scan for `EI`) instead of overflowing; the oversized + // image itself is then refused by size. + let bytes = page_pdf("q 10 0 0 10 0 0 cm BI /W 5000000000000000000 /H 100 /CS /G /BPC 8 ID \0 EI Q", "", vec![]); + let doc = parse(&bytes).unwrap(); + let _ = render(&doc); + assert!(!doc.skipped.is_empty()); + // A length just under `usize::MAX` passes the row math; past byte 4096 of the content the + // caller's `start + len` would then overflow. + let content = format!("{}q 10 0 0 10 0 0 cm BI /W 2305843009213693440 /H 8 /CS /G /BPC 8 ID EI Q", " ".repeat(5000)); + let doc = parse(&page_pdf(&content, "", vec![])).unwrap(); + let _ = render(&doc); +} + #[test] fn truncated_and_corrupt_eps_never_panic() { let eps = b"%!PS-Adobe-3.0 EPSF-3.0\n%%BoundingBox: 0 0 100 100\n\ diff --git a/crates/pdf/src/type1.rs b/crates/pdf/src/type1.rs index f489ad2a..4b3e0575 100644 --- a/crates/pdf/src/type1.rs +++ b/crates/pdf/src/type1.rs @@ -143,7 +143,7 @@ impl Type1 { q += 1; } q += 1; - let bytes = priv_.get(q..q + n)?; + let bytes = priv_.get(q..q.checked_add(n)?)?; Some((cs(bytes), q + n)) }; if let Some(p) = find(&priv_, b"/Subrs", 0) { @@ -532,4 +532,17 @@ mod tests { assert!((p.area().abs() - 250_000.0).abs() < 1e-6); assert_eq!(p.bounding_box(), kurbo::Rect::new(50.0, 0.0, 550.0, 500.0)); } + + /// A `/Subrs` entry whose `RD` length is `usize::MAX`: the bytes are not in the file, and + /// `q + n` must not overflow. + #[test] + fn huge_rd_lengths_do_not_overflow() { + let mut out = b"%!PS-AdobeFont-1.0: Test 001\n/FontMatrix [0.001 0 0 0.001 0 0] readonly def\ncurrentfile eexec\n".to_vec(); + let mut priv_ = b"dup /Private 8 dict dup begin /lenIV -1 def\n/Subrs 1 array\ndup 0 18446744073709551615 RD\n".to_vec(); + priv_.extend_from_slice(b"end\n/CharStrings 1 dict dup begin\n/.notdef 4 RD \x8b\x8b\r\x0e ND\nend\nend\nmark currentfile closefile\n"); + out.extend(encrypt(&priv_, EEXEC_R, 4)); + out.extend_from_slice(b"\n0000000000000000\ncleartomark\n"); + let t = Type1::parse(&out).expect("the damaged Subrs entry is skipped"); + assert!(t.has_glyph(".notdef")); + } } diff --git a/crates/project/src/render_queue.rs b/crates/project/src/render_queue.rs index 5c13dffc..2d3ab741 100644 --- a/crates/project/src/render_queue.rs +++ b/crates/project/src/render_queue.rs @@ -769,10 +769,14 @@ impl Crop { if self.use_roi && let Some([x, y, rw, rh]) = self.roi { - let l = (x * scale).round() as i32; - let t = (y * scale).round() as i32; - let r = w as i32 - ((x + rw) * scale).round() as i32; - let b = h as i32 - ((y + rh) * scale).round() as i32; + // The quad arrives unvalidated (control channel or a hand-edited project): + // clamp like the sibling edge fields (plus/minus 20 000) and subtract + // without overflow. + let conv = |v: f64| v.round().clamp(-20_000.0, 20_000.0) as i32; + let l = conv(x * scale); + let t = conv(y * scale); + let r = (w as i32).saturating_sub(conv((x + rw) * scale)); + let b = (h as i32).saturating_sub(conv((y + rh) * scale)); return (t, l, b, r); } (self.top, self.left, self.bottom, self.right) @@ -1094,8 +1098,8 @@ impl OutputModule { /// scale `scale`: after Crop and Resize (before codec rounding). pub fn frame_size(&self, w: u32, h: u32, scale: f64) -> (u32, u32) { let (t, l, b, r) = self.crop.edges(w, h, scale); - let cw = (w as i32 - l - r).max(1) as u32; - let ch = (h as i32 - t - b).max(1) as u32; + let cw = (w as i32).saturating_sub(l).saturating_sub(r).max(1) as u32; + let ch = (h as i32).saturating_sub(t).saturating_sub(b).max(1) as u32; if !self.resize.enabled { return (cw, ch); } @@ -1643,6 +1647,22 @@ mod tests { assert!(s.frame_time(&c, 0) >= Tick::from_seconds_f64(1.0)); } + #[test] + fn hostile_roi_cannot_overflow_crop_math() { + // `-1e12` saturates to `i32::MIN` on conversion: `w as i32 - l` must saturate, + // not overflow, and still yield a usable (degenerate) frame size. + let c = Comp::new(1920, 1080, FrameRate::new(30, 1), Tick::from_seconds_f64(1.0)); + let rs = RenderSettings::default(); + let mut m = OutputModule::for_format(OutputFormat::PngSequence); + m.crop = Crop { enabled: true, use_roi: true, roi: Some([-1e12, -1e12, 200.0, 200.0]), ..Default::default() }; + let (w, h) = m.output_size(&c, &rs); + assert!((w, h) == (1, 1), "degenerate but valid: {w}x{h}"); + // Sane ROIs are unaffected (existing behavior). + m.crop = Crop { enabled: true, use_roi: true, roi: Some([100.0, 50.0, 800.0, 400.0]), ..Default::default() }; + let half = RenderSettings { resolution: 0.5, ..Default::default() }; + assert_eq!(m.output_size(&c, &half), (400, 200)); + } + #[test] fn uncompressed_quicktime_format() { let f = OutputFormat::from_name("uncompressed").unwrap(); diff --git a/crates/render/src/cache/dependencies.rs b/crates/render/src/cache/dependencies.rs index eaccd058..483b19da 100644 --- a/crates/render/src/cache/dependencies.rs +++ b/crates/render/src/cache/dependencies.rs @@ -78,6 +78,11 @@ impl Dependencies<'_> { hash_debug(self.h, layer); hash_values(self.h, ctx, layer, &layer.props); hash_debug(self.h, &ctx.world_matrix(layer)); + // A cached owner never runs this layer's effects, so the comp camera and lights its + // scene-reading effects see belong in the owner's key too (#1252). + if super::reads_comp_scene(layer) { + hash_debug(self.h, &crate::comp_scene(ctx, layer)); + } if super::time_dependent(layer, true) { layer.layer_time(ctx.time).0.hash(self.h); } diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 808356f1..cfefd02d 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -455,6 +455,12 @@ pub trait Accelerator: Send + Sync { fn auto_pick(&self) -> Option<&AutoPick> { None } + /// Whether the accelerator can still render frames. `false` once it has stopped for good + /// (its device was lost or retired): every frame it is asked for falls back to the CPU, so + /// callers that limit frames in flight for it can stop doing so. + fn available(&self) -> bool { + true + } /// Deferred readbacks: where a GPU readback cannot be waited for (WebGPU in a browser /// worker), a frame renders in *passes*. A readback the accelerator does not have yet is /// started, the step returns a cheap placeholder and the pass is marked missed @@ -2094,7 +2100,7 @@ impl<'a> Renderer<'a> { /// applied). pub fn visible_layers(&self, ctx: &EvalCtx<'a>) -> Vec<&'a Layer> { let (comp, t) = (ctx.comp, ctx.time); - let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av() && l.is_active_at(t)); + let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av()); comp.layers .iter() .rev() diff --git a/crates/render/src/shapes.rs b/crates/render/src/shapes.rs index aa46fa50..bd59ad83 100644 --- a/crates/render/src/shapes.rs +++ b/crates/render/src/shapes.rs @@ -542,10 +542,25 @@ fn draw_bounds(d: &Draw, paths: &[BezPath]) -> Option { Paint::Gradient { stroke: Some(style), .. } => Some(style), _ => None, }; - let grow = stroke.map_or(1.0, |style| style.width * style.miter.max(1.0) * 0.5 * d.xf.mean_scale().max(1.0) + 1.0); + let grow = stroke.map_or(1.0, |style| style.width * stroke_reach(style, paths) * 0.5 * d.xf.mean_scale().max(1.0) + 1.0); Some(b.inflate(grow, grow)) } +/// How far a stroke can reach past its path, in half widths: the miter limit only where a +/// miter join exists (a path of two or more segments, or a closed one), √2 at square cap +/// corners, else one. Padding every stroke by the miter limit made a 1080 px straight line +/// (a wipe matte) a 4326×6366 buffer, ~440 MB per layer per frame. +fn stroke_reach(style: &StrokeStyle, paths: &[BezPath]) -> f64 { + if style.is_variable() { + // Taper and Wave can widen the stroke: keep the widest padding. + return style.miter.max(std::f64::consts::SQRT_2); + } + let joins = + style.join == Join::Miter && paths.iter().any(|p| p.segments().nth(1).is_some() || p.elements().iter().any(|e| matches!(e, kurbo::PathEl::ClosePath))); + let cap = if style.cap == Cap::Square { std::f64::consts::SQRT_2 } else { 1.0 }; + if joins { style.miter.max(cap) } else { cap } +} + /// Local-space visual bounds of a draw, as After Effects' layer bounds (`sourceRectAtTime` with /// extents): the path bounds, plus half the stroke width for strokes. No anti-aliasing or miter /// padding, so snapping and Align put edges exactly together (#352). diff --git a/crates/render/src/shapes/tests.rs b/crates/render/src/shapes/tests.rs index 0a5d6893..3946fc9d 100644 --- a/crates/render/src/shapes/tests.rs +++ b/crates/render/src/shapes/tests.rs @@ -424,3 +424,40 @@ fn radial_gradient_highlight_moves_the_focal_point() { // The outer circle is unchanged: points just outside it are black in both. assert!(a.get(100, 182)[0] < 0.03 && b.get(100, 182)[0] < 0.03 && b.get(100, 17)[0] < 0.03); } + +/// The layer-space buffer `super::render` draws the comp's only shape layer into. +fn shape_buf(p: &Project, cid: ItemId) -> effectcraft_effects::Buf { + let comp = p.comp(cid).unwrap(); + let ctx = crate::eval::EvalCtx::new(p, cid, comp, Tick::ZERO); + let layer = &comp.layers[0]; + super::render(&ctx, layer, layer.props.sub("contents").unwrap(), 1.0) +} + +#[test] +fn straight_stroke_buffer_is_not_padded_by_the_miter_limit() { + // A 1080 px wide vertical wipe (a track matte): no joins, so the buffer is the line plus half + // the width, not 4× (Miter Limit 4) the width on every side (4326×6366, ~440 MB per frame). + let (p, cid) = shape_comp_sized(1080, 1920, |ids| vec![line_path(ids, [0.0, 1020.0], [0.0, -1020.0]), build::shape_stroke(ids, RED, 1080.0)]); + let b = shape_buf(&p, cid); + assert!(b.img.width <= 1090 && b.img.height <= 3130, "{}×{}", b.img.width, b.img.height); +} + +#[test] +fn miter_corner_still_fits_its_buffer() { + // A sharp V with a 20 px miter-joined stroke: the miter tip reaches well past half the width. + let (p, cid) = shape_comp(|ids| { + let sp = effectcraft_keyframe::ShapePath { + vertices: vec![[-40.0, -30.0], [0.0, 20.0], [40.0, -30.0]], + in_tangents: vec![[0.0; 2]; 3], + out_tangents: vec![[0.0; 2]; 3], + closed: false, + feather: Vec::new(), + }; + vec![build::shape_path(ids, sp), build::shape_stroke(ids, RED, 20.0)] + }); + // The miter tip of the V points down from (100, 70): its pixels are drawn, not clipped. + let img = frame(&p, cid); + assert!(alpha(&img, 100, 80) > 0.9, "{}", alpha(&img, 100, 80)); + let b = shape_buf(&p, cid); + assert!(b.img.height as f64 >= 50.0 + 20.0 * 4.0 * 0.5, "{}", b.img.height); +} diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 39e022b0..72dd2650 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -953,3 +953,60 @@ fn shape_content_bounds_are_the_visual_edges() { assert!(b.iter().zip([-half, -half, half, half]).all(|(a, e)| (a - e).abs() < 1e-6), "{stroke:?}: {b:?}"); } } + +/// Solo is not time-dependent (as in After Effects): while any layer is soloed, the others stay +/// excluded even when the playhead is outside the soloed layer's in-out range. +#[test] +fn solo_excludes_other_layers_outside_the_soloed_layers_range() { + let (mut p, cid, comp) = setup(); + let a = solid(&mut p, &comp, [1.0, 0.0, 0.0], 200, 100); + let mut b = solid(&mut p, &comp, [0.0, 0.0, 1.0], 200, 100); + b.in_point = Tick::from_seconds_f64(1.0); + b.switches.solo = true; + let c = p.comp_mut(cid).unwrap(); + c.layers.push(b); + c.layers.push(a); + let first = render_frame(&p, cid, Tick::from_seconds_f64(0.5), 1.0); + assert_eq!(first.get(100, 50), [0.0, 0.0, 0.0, 0.0], "neither A nor B before B's in point"); + let second = render_frame(&p, cid, Tick::from_seconds_f64(1.5), 1.0); + assert_eq!(second.get(100, 50), [0.0, 0.0, 1.0, 1.0], "only B inside its range"); +} + +/// #1252: a layer blended with a CC Particle World layer, under a camera panned by `pan` px. +fn scene_reader_dependency(pan: f64) -> (Project, ItemId) { + let (mut p, cid, comp) = setup(); + let mut source = solid(&mut p, &comp, [0.0, 0.0, 0.0], 200, 100); + let vals = [("birthRate", Value::Scalar(2.0)), ("particle/particleType", Value::Enum(2)), ("particle/birthSize", Value::Scalar(0.1))]; + add_effect(&mut p, &mut source, "ec.sim.ccparticleworld", [200.0, 100.0], &vals); + source.switches.video = false; + let mut owner = solid(&mut p, &comp, [0.0, 0.0, 0.0], 200, 100); + add_effect_200(&mut p, &mut owner, "ec.channel.blend", &[("blendWithLayer", Value::Layer(Some(source.id.0))), ("blendWithOriginal", Value::Scalar(0.0))]); + let mut cam = build::layer(&mut p, &comp, "Camera", LayerSource::Camera, (200, 100), None); + for path in ["transform/position", "transform/poi"] { + let pr = cam.props.prop_mut(path).unwrap(); + let Value::Vec3(v) = pr.value else { panic!("{path}: {:?}", pr.value) }; + pr.value = Value::Vec3([v[0] + pan, v[1], v[2]]); + } + p.comp_mut(cid).unwrap().layers = vec![cam, owner, source]; + (p, cid) +} + +/// #1252: the owner's cache key covers the comp camera and lights that a layer it selects reads, +/// though the owner itself runs no scene-reading effect. +#[test] +fn a_selected_layer_reading_the_comp_scene_invalidates_its_owner() { + let t = Tick::from_seconds_f64(1.0); + let (p, cid) = scene_reader_dependency(0.0); + let (q, _) = scene_reader_dependency(40.0); + let key = |p: &Project| { + let c = p.comp(cid).unwrap(); + crate::cache::layer_key(&crate::EvalCtx::new(p, cid, c, t), &c.layers[1], 1.0, false, false).unwrap() + }; + assert_ne!(key(&p), key(&q), "moving the camera changes the owner's key"); + let cache = crate::LayerCache::default(); + let here = render_cached(&p, cid, t, Some(&cache)); + let panned = render_cached(&q, cid, t, Some(&cache)); + assert_same(&panned, &render_cached(&q, cid, t, None), "cached render after the camera moved"); + let (a, b) = (particles_x(&here), particles_x(&panned)); + assert!(b < a - 20.0, "the blended particles follow the camera: {a} -> {b}"); +} diff --git a/crates/render/src/three_d/adv/scene.rs b/crates/render/src/three_d/adv/scene.rs index 5b5ae956..34517933 100644 --- a/crates/render/src/three_d/adv/scene.rs +++ b/crates/render/src/three_d/adv/scene.rs @@ -873,7 +873,7 @@ fn collapsed_layer<'a>(r: &Renderer<'a>, ctx: &EvalCtx<'a>, layer: &Layer, item: let saved = b.outer; b.outer = saved * ctx.world_matrix(layer); let t = nctx.time; - let any_solo = nctx.comp.layers.iter().any(|l| l.switches.solo && l.source.is_av() && l.is_active_at(t)); + let any_solo = nctx.comp.layers.iter().any(|l| l.switches.solo && l.source.is_av()); for l in nctx.comp.layers.iter().rev() { if !l.is_active_at(t) || l.is_light() diff --git a/crates/render/src/three_d/compose.rs b/crates/render/src/three_d/compose.rs index 0d70b32f..831ab3ec 100644 --- a/crates/render/src/three_d/compose.rs +++ b/crates/render/src/three_d/compose.rs @@ -497,7 +497,7 @@ fn collapsed_items<'a>( let op = ctx.opacity(layer) as f32 * r.opacity_mul(); let Some((sub, nctx)) = r.collapse_into(ctx, layer, item, op) else { return vec![] }; let t = nctx.time; - let any_solo = nctx.comp.layers.iter().any(|l| l.switches.solo && l.source.is_av() && l.is_active_at(t)); + let any_solo = nctx.comp.layers.iter().any(|l| l.switches.solo && l.source.is_av()); let nested: Vec<&'a Layer> = nctx .comp .layers diff --git a/crates/script/src/lib.rs b/crates/script/src/lib.rs index 067b85fd..19904c2a 100644 --- a/crates/script/src/lib.rs +++ b/crates/script/src/lib.rs @@ -106,5 +106,9 @@ mod tests_ui_more; // #760: `$.evalFile` and `BridgeTalk`. #[cfg(test)] mod tests_evalfile; +// `/browser/…` is the web editor's file table, an absolute path on wasm; on Windows it has no +// drive, so the script runtime resolves it under the current drive, where the host holds nothing. +#[cfg(all(test, not(windows)))] +mod tests_services; #[cfg(test)] mod tests_ui_mouse; diff --git a/crates/script/src/prelude.js b/crates/script/src/prelude.js index 29f2fc49..2eeca86b 100644 --- a/crates/script/src/prelude.js +++ b/crates/script/src/prelude.js @@ -1978,6 +1978,7 @@ var app = { savePrefAsBool: function () {}, savePrefAsFloat: function () {}, reload: function () {}, + saveToDisk: function () {}, }, newProject: function () { __call("file.newProject", {}); return new Project(); }, open: function (file) { diff --git a/crates/script/src/runtime.rs b/crates/script/src/runtime.rs index b94bd80f..192da574 100644 --- a/crates/script/src/runtime.rs +++ b/crates/script/src/runtime.rs @@ -157,11 +157,16 @@ pub(crate) fn exec(s: &mut Session, id: &str, params: J) -> Result { s.state.selected_layers.clear(); } let r = s.execute_for_script(id, params); - if let Some(c) = target - && prev != Some(c) - && s.state.active_comp == Some(c) - && id != "comp.open" - { + if let Some(c) = target { + leave_comp(s, id, c, prev); + } + r.map_err(|e| e.to_string()) +} + +/// After a script's command on comp `c`, make `prev` (the comp active before it) active again, +/// unless the command opened `c` on purpose; with no comp to go back to, `c` stays open. +fn leave_comp(s: &mut Session, id: &str, c: effectcraft_engine::project::ItemId, prev: Option) { + if prev != Some(c) && s.state.active_comp == Some(c) && id != "comp.open" { match prev { Some(p) if s.project.comp(p).is_some() => s.activate_comp(p), _ => { @@ -171,7 +176,6 @@ pub(crate) fn exec(s: &mut Session, id: &str, params: J) -> Result { } } } - r.map_err(|e| e.to_string()) } /// Run a keyframe command that acts on the selected keys (`keys.velocity`, `keys.interpolation`, @@ -195,11 +199,17 @@ fn exec_on_keys(s: &mut Session, id: &str, params: J, targets: &J) -> Result std::path::PathBuf { // Resolve symlinks where the file (or its folder) exists, so links can't escape the project. std::fs::canonicalize(&out) .or_else(|_| out.parent().map(std::fs::canonicalize).unwrap_or(Ok(out.clone())).map(|d| d.join(out.file_name().unwrap_or_default()))) + .map(without_verbatim_prefix) .unwrap_or(out) } +/// A canonical Windows drive path without its `\\?\` prefix, so it compares with a lexical one +/// (a path whose folder doesn't exist on disk stays lexical). +fn without_verbatim_prefix(p: std::path::PathBuf) -> std::path::PathBuf { + match p.to_str().and_then(|s| s.strip_prefix(r"\\?\")) { + Some(plain) if plain.as_bytes().get(1) == Some(&b':') => plain.into(), + _ => p, + } +} + pub(crate) const GATE: &str = "Preferences ▸ Scripting & Expressions ▸ Allow Scripts to Write Files and Access Network"; /// The file-access gate: reads under the project's folder are always allowed; other reads, all @@ -380,11 +400,14 @@ fn native_file(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult Ok(json!(check_access(s, &path, false)?.exists())), + // Through the session's services, like the reads: hosts without a file system (the + // browser's file table) answer for their own files. + "access" => Ok(json!(s.services.exists(&check_access(s, &path, false)?.to_string_lossy()))), "exists" | "isDir" => { // Unreadable locations report "missing" instead of failing. let Ok(p) = check_access(s, &path, false) else { return Ok(json!(false)) }; - Ok(json!(if op == "exists" { p.exists() } else { p.is_dir() })) + let p = p.to_string_lossy(); + Ok(json!(if op == "exists" { s.services.exists(&p) } else { s.services.is_dir(&p) })) } "read" | "readBinary" => { let p = check_access(s, &path, false)?; @@ -563,27 +586,32 @@ fn position(msg: &str, file: &str, code: &str) -> (Option, Option) { let mut it = l[i..].split([':', ')']); Some((it.next()?.parse().ok()?, it.next()?.parse().ok()?)) }; + // The innermost frame in the user's script, or the error's own position. + let frames: Vec<&str> = msg.lines().skip(1).collect(); + let frame = frames.iter().find_map(|l| pick(l)).or_else(|| msg.lines().next().and_then(pick)); // `ReferenceError: name is not defined`: boa places it at the start of the function that reads - // the name, so the first use of the name in the code is the better position (#1063). + // the name, so the name's first use from that frame's line on is the better position (#1063). + // With no frame to start from, the first use anywhere in the code is (#1161). let first = msg.lines().next().unwrap_or(""); let undefined = first.split("ReferenceError: ").nth(1).and_then(|r| r.split(" is not defined").next()); - if let Some(at) = undefined.and_then(|name| first_use(name, code)) { + let start_line = frame.map_or(1, |(line, _)| line); + if let Some(at) = undefined.and_then(|name| first_use(name, code, start_line)) { return (Some(at.0), Some(at.1)); } - let frames: Vec<&str> = msg.lines().skip(1).collect(); - if let Some((a, b)) = frames.iter().find_map(|l| pick(l)).or_else(|| msg.lines().next().and_then(pick)) { + if let Some((a, b)) = frame { return (Some(a), Some(b)); } (None, None) } -/// The first use of `name` as a whole identifier in `code`, as a 1-based line and column. -fn first_use(name: &str, code: &str) -> Option<(u32, u32)> { +/// The first use of `name` as a whole identifier in `code` on or after `start_line` (1-based), as a +/// 1-based line and column. +fn first_use(name: &str, code: &str, start_line: u32) -> Option<(u32, u32)> { if name.is_empty() { return None; } let ident = |c: u8| c.is_ascii_alphanumeric() || c == b'_' || c == b'$'; - for (n, line) in code.lines().enumerate() { + for (n, line) in code.lines().enumerate().skip(start_line.saturating_sub(1) as usize) { let b = line.as_bytes(); let mut from = 0; while let Some(i) = line.get(from..).and_then(|rest| rest.find(name)).map(|i| i + from) { @@ -829,4 +857,14 @@ mod nesting_tests { assert_eq!(out.result, serde_json::json!(2)); assert!(check_nesting(&("(".repeat(MAX_NESTING) + &")".repeat(MAX_NESTING))).is_ok()); } + + /// A canonical drive path and a lexical one compare once the `\\?\` prefix is gone; a UNC + /// share and a Unix path are left as they are. + #[test] + fn only_a_verbatim_drive_prefix_is_dropped() { + use std::path::PathBuf; + assert_eq!(without_verbatim_prefix(r"\\?\C:\proj\a.txt".into()), PathBuf::from(r"C:\proj\a.txt")); + assert_eq!(without_verbatim_prefix(r"\\?\UNC\server\share\a.txt".into()), PathBuf::from(r"\\?\UNC\server\share\a.txt")); + assert_eq!(without_verbatim_prefix("/proj/a.txt".into()), PathBuf::from("/proj/a.txt")); + } } diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 2c821e6a..109b0974 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -187,6 +187,18 @@ fn errors_inside_a_function_wrapper_report_the_failing_line() { assert_eq!(e.line, Some(3), "{e:?}"); } +/// The first use of an undefined name is the one inside the function that reads it: a mention +/// before it, in a comment, a string or another function, doesn't move the report (#1161). +#[test] +fn an_undefined_name_mentioned_earlier_reports_its_use_in_the_reading_function() { + let mut s = session(); + let code = "// nope is mentioned here\nvar text = 'nope';\nfunction other() { return nope; }\n(function () {\n var a = 1;\n nope.call();\n})();\n"; + let o = run_code(&mut s, code, "earlier.jsx"); + let e = o.error.expect("reference error"); + assert!(e.message.contains("nope"), "{e:?}"); + assert_eq!(e.line, Some(6), "{e:?}"); +} + #[test] fn match_names_and_lookups() { let mut s = session(); @@ -430,6 +442,32 @@ fn save_frame_to_png_writes_behind_the_file_gate() { let _ = std::fs::remove_dir_all(&dir); } +/// #1246: with Warn User When Executing Files on, a script that asserts `confirmed` itself still +/// gets the question; only the user's answer opens the file. +#[test] +fn a_script_cannot_confirm_executing_a_file_for_itself() { + let dir = std::env::temp_dir().join(format!("ec-script-exec-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let f = dir.join("run.txt"); + std::fs::write(&f, "x").unwrap(); + let q = serde_json::to_string(&f.to_string_lossy()).unwrap(); + let mut s = session(); + // The user allows file access; the warning stays on (the default). + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap(); + s.drain_events(); + let opened = |s: &mut Session| s.drain_events().iter().any(|e| matches!(e, effectcraft_engine::Event::OpenUrl(_))); + for code in [ + format!("JSON.stringify(app.run('file.executeFile', {{path: {q}, confirmed: true}}))"), + format!("JSON.stringify(app.run('engine.batch', {{steps: [{{command: 'file.executeFile', params: {{path: {q}, confirmed: true}}}}]}}))"), + format!("new File({q}).execute()"), + ] { + let o = ok(&mut s, &code); + assert!(!o.result.to_string().contains("opened") || o.result == json!(false), "{code}: {:?}", o.result); + assert!(!opened(&mut s), "{code} opened the file without asking"); + } + let _ = std::fs::remove_dir_all(&dir); +} + /// #590: a script can't give itself file and network access (or turn off the warning before /// executing files) by changing Settings ▸ Scripting & Expressions; the user still can. #[test] @@ -1258,6 +1296,36 @@ fn effect_parameter_interpolation_and_ease() { assert_eq!(o.result, json!([true, true, 16.67, 33.33, true])); } +/// #1531: the per-key methods act on a comp that isn't the active one too. Activating the keys' +/// comp brought back its own (empty) key selection over the keys named, so the interpolation and +/// the ease were silently dropped once another comp with a layer had been made first. +#[test] +fn key_interpolation_and_ease_apply_outside_the_active_comp() { + let mut s = session(); + let o = ok( + &mut s, + r#" + var a = app.project.items.addComp("A", 400, 400, 1, 10, 20); a.layers.addNull(); + var c = app.project.items.addComp("C", 400, 400, 1, 10, 20); + var n = c.layers.addNull(); n.threeDLayer = true; + var p = n.property("ADBE Transform Group").property("ADBE Rotate Y"); + p.setValueAtTime(1, 0); p.setValueAtTime(2, -178); + for (var j = 1; j <= 2; j++) { + p.setInterpolationTypeAtKey(j, KeyframeInterpolationType.BEZIER, KeyframeInterpolationType.BEZIER); + p.setTemporalEaseAtKey(j, [new KeyframeEase(0, 60)], [new KeyframeEase(0, 60)]); + } + [p.keyOutInterpolationType(1) == KeyframeInterpolationType.BEZIER, + p.keyInInterpolationType(2) == KeyframeInterpolationType.BEZIER, + p.keyOutTemporalEase(1)[0].influence, + Math.round(p.valueAtTime(1.25, false) * 100) / 100] + "#, + ); + let v = o.result; + assert_eq!((&v[0], &v[1], &v[2]), (&json!(true), &json!(true), &json!(60)), "{v}"); + // Eased, not the linear -44.5. + assert!(v[3].as_f64().is_some_and(|x| (x - -44.5).abs() > 1.0), "{v}"); +} + /// #808: sourcePointToComp and compPointToSource follow the layer's transform. #[test] fn source_point_conversions_follow_the_layer_transform() { @@ -1375,3 +1443,42 @@ fn scripted_levels_alpha_controls_resolve_from_the_effect() { ); assert_eq!(o.result, json!([0.5, 0.5])); } + +/// After Effects resolves the Transform properties and Source Text from the layer itself, without +/// naming their group: `layer.property("Position")` (#1348). +#[test] +fn layer_property_resolves_transform_and_source_text_shortcuts() { + let mut s = session(); + let o = ok( + &mut s, + r#" + var c = app.project.items.addComp("x", 100, 100, 1, 5, 30); + var t = c.layers.addText("hi"); + var s = c.layers.addSolid([1, 0, 0], "s", 100, 100, 1); + var listed = [t.property("Source Text"), s.property("Position"), s.property("Opacity"), s.property("Scale")].join(", "); + s.property("Position").setValue([30, 40]); + s.property("Opacity").setValue(25); + t.property("Source Text").setValue("bye"); + s.threeDLayer = true; + [listed, s.property("Position").value[0], s.property("Opacity").value, t.property("Source Text").value.text, + s.property("ADBE Position").matchName, s("Anchor Point").matchName, s.property("Rotation").matchName, + s.property("X Rotation").matchName, s.property("Orientation").matchName] + "#, + ); + assert_eq!( + o.result, + json!([ + "[object Property], [object Property], [object Property], [object Property]", + 30, + 25, + "bye", + "ADBE Position", + "ADBE Anchor Point", + "ADBE Rotate Z", + "ADBE Rotate X", + "ADBE Orientation" + ]) + ); + let (_, c) = comp_named(&s, "x"); + assert_eq!(layer(c, "s").props.prop("transform/opacity").unwrap().value, KV::Scalar(25.0)); +} diff --git a/crates/script/src/tests_callable.rs b/crates/script/src/tests_callable.rs index 13d6f787..e8452935 100644 --- a/crates/script/src/tests_callable.rs +++ b/crates/script/src/tests_callable.rs @@ -94,3 +94,10 @@ fn calling_without_a_name_is_a_script_error() { let o = ok(&mut s, "var c = app.project.items.addComp('N', 100, 100, 1, 1, 25); var l = c.layers.addNull(); l.name = 'Ctl'; l"); assert_eq!(o.result, json!({ "type": "[object AVLayer]", "name": "Ctl" })); } + +#[test] +fn app_preferences_save_to_disk_is_callable() { + let mut s = session(); + let o = ok(&mut s, "app.preferences.saveToDisk(); app.preferences.reload(); [typeof app.preferences.saveToDisk, app.preferences.getPrefAsLong('a', 'b')]"); + assert_eq!(o.result, json!(["function", 0])); +} diff --git a/crates/script/src/tests_services.rs b/crates/script/src/tests_services.rs new file mode 100644 index 00000000..c0ecfab4 --- /dev/null +++ b/crates/script/src/tests_services.rs @@ -0,0 +1,47 @@ +//! Scripts see the host's files: existence checks go through `Services`, like reads, so a host +//! that keeps files in memory (the web editor's browser storage) answers for its own files. + +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; + +use effectcraft_engine::{Services, Session}; + +use crate::run_code; + +#[derive(Default)] +struct Mem(Mutex>>); + +impl Services for Mem { + fn read_file(&self, path: &str) -> std::io::Result> { + self.0.lock().unwrap().get(path).cloned().ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, path.to_string())) + } + fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { + self.0.lock().unwrap().insert(path.into(), data.into()); + Ok(()) + } + fn exists(&self, path: &str) -> bool { + self.0.lock().unwrap().contains_key(path) + } + fn is_dir(&self, path: &str) -> bool { + path == "/browser/footage" + } +} + +#[test] +fn a_script_opens_a_file_only_the_host_holds() { + let mem = Arc::new(Mem::default()); + mem.write_file("/browser/data.txt", b"hello").unwrap(); + let mut s = Session { services: mem, ..Default::default() }; + crate::install(&mut s); + // A project in a folder that isn't on disk: scripts may read files next to it. + s.path = Some("/browser/project.ecproj".into()); + let out = run_code( + &mut s, + r#"var f = new File("/browser/data.txt"); var ok = f.open("r"); + writeLn(ok + " " + (ok ? f.read() : f.error)); + writeLn(new File("/browser/missing.txt").exists + " " + new Folder("/browser/footage").exists);"#, + "t.jsx", + ); + assert!(out.error.is_none(), "{:?}", out.error); + assert_eq!(out.output, vec!["true hello".to_string(), "false true".to_string()]); +} diff --git a/crates/ui-egui/src/color_picker.rs b/crates/ui-egui/src/color_picker.rs index 35d8e4eb..efaae6d7 100644 --- a/crates/ui-egui/src/color_picker.rs +++ b/crates/ui-egui/src/color_picker.rs @@ -45,7 +45,13 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) if ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { out.close = Some(false); } - if pressed_outside(ui.ctx(), &area.response) { + // A press in the pass that first draws the picker must not close it: an Area's rect is only + // at its requested position from its second pass on (the first pass reports it at the + // origin), so the click that just opened the picker would be read as a press outside it and + // the picker would flash and close in that one frame (#1476). + let pass_id = id.with("open-pass"); + let drawn_before = ui.data(|d| d.get_temp::(pass_id)).is_some_and(|p| p < ui.ctx().cumulative_pass_nr()); + if drawn_before && pressed_outside(ui.ctx(), &area.response) { out.close.get_or_insert(true); } if let Some(keep) = out.close { @@ -56,13 +62,17 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) ui.data_mut(|d| { d.insert_temp(id.with("open"), false); d.remove::<[f32; 3]>(orig_id); + d.remove::(pass_id); // Closed: the next Escape is the dialog's again. d.remove::(open_pass_id()); }); } else { // Still open after this pass: dialogs leave Escape to the picker (see `open_recently`). let pass = ui.ctx().cumulative_pass_nr(); - ui.data_mut(|d| d.insert_temp(open_pass_id(), pass)); + ui.data_mut(|d| { + d.insert_temp(pass_id, pass); + d.insert_temp(open_pass_id(), pass); + }); } out.changed } diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 4caae8dd..81343f9f 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -111,6 +111,13 @@ pub(crate) fn label<'a>(app: &EffectcraftApp, command: &str, source: &'a str) -> _ => {} } } + if language == "de" && matches!(command, "file.revealInFinder" | "layer.revealInFinder") { + match source { + "Reveal in Explorer" => return "Im Explorer anzeigen", + "Reveal in File Manager" => return "Im Dateimanager anzeigen", + _ => {} + } + } if language == "cs" && matches!(command, "file.revealInFinder" | "layer.revealInFinder") { match source { "Reveal in Explorer" => return "Zobrazit v Průzkumníku", @@ -2034,9 +2041,9 @@ const JAPANESE: &[(&str, &str, &str)] = &[ ("view.zoomOut", "Zoom Out", "ズームアウト"), ("", "Resolution", "解像度"), ("view.res.full", "Full", "フル画質"), - ("view.res.half", "Half", "1/2画質"), - ("view.res.third", "Third", "1/3画質"), - ("view.res.quarter", "Quarter", "1/4画質"), + ("view.res.half", "Half", "1/2 画質"), + ("view.res.third", "Third", "1/3 画質"), + ("view.res.quarter", "Quarter", "1/4 画質"), ("view.res.custom", "Custom...", "カスタム..."), ("view.displayColorManagement", "Use Display Color Management", "ディスプレイカラーマネジメントを使用"), ("", "Simulate Output", "出力をシミュレート"), @@ -5555,7 +5562,7 @@ const GERMAN: &[(&str, &str, &str)] = &[ ("layer.settings", "Layer Settings...", "Ebeneneinstellungen..."), ("layer.openLayer", "Open Layer", "Ebene öffnen"), ("layer.openSource", "Open Layer Source", "Ebenenquelle öffnen"), - ("layer.revealInFinder", "Reveal in Finder", "Im Finder einblenden"), + ("layer.revealInFinder", "Reveal in Finder", "Im Finder anzeigen"), ("", "Mask", "Maske"), ("layer.addMask", "New Mask", "Neue Maske"), ("layer.mask.shape", "Mask Shape...", "Maskenform..."), @@ -5565,7 +5572,7 @@ const GERMAN: &[(&str, &str, &str)] = &[ ("layer.mask.reset", "Reset Mask", "Maske zurücksetzen"), ("layer.mask.remove", "Remove Mask", "Maske entfernen"), ("layer.mask.removeAll", "Remove All Masks", "Alle Masken entfernen"), - ("track.mask", "Track Mask", "Maske tracken"), + ("track.mask", "Track Mask", "Maske verfolgen"), ("", "Mode", "Modus"), ("layer.mask.mode", "None", "Ohne"), ("layer.mask.mode", "Add", "Addieren"), @@ -5602,8 +5609,8 @@ const GERMAN: &[(&str, &str, &str)] = &[ ("layer.hideOtherVideo", "Hide Other Video", "Anderes Video ausblenden"), ("layer.showAllVideo", "Show All Video", "Gesamtes Video einblenden"), ("layer.unlockAll", "Unlock All Layers", "Alle Ebenen entsperren"), - ("layer.expressions", "Enable Expressions", "Expressions aktivieren"), - ("layer.expressions", "Disable Expressions", "Expressions deaktivieren"), + ("layer.expressions", "Enable Expressions", "Ausdrücke aktivieren"), + ("layer.expressions", "Disable Expressions", "Ausdrücke deaktivieren"), ("layer.setSwitch", "Shy", "Schüchtern"), ("layer.setSwitch", "Lock", "Sperren"), ("layer.setSwitch", "Audio", "Audio"), @@ -5631,7 +5638,7 @@ const GERMAN: &[(&str, &str, &str)] = &[ ("layer.autoOrient", "Auto-Orient...", "Automatische Ausrichtung..."), ("", "Time", "Zeit"), ("layer.enableTimeRemap", "Enable Time Remapping", "Zeitverzerrung aktivieren"), - ("layer.timeReverse", "Time-Reverse Layer", "Ebenenreihenfolge umkehren"), + ("layer.timeReverse", "Time-Reverse Layer", "Ebene zeitlich umkehren"), ("layer.timeStretch", "Time Stretch...", "Zeitdehnung..."), ("layer.freezeFrame", "Freeze Frame", "Frame fixieren"), ("layer.freezeOnLastFrame", "Freeze On Last Frame", "Bei letztem Frame fixieren"), @@ -5700,7 +5707,7 @@ const GERMAN: &[(&str, &str, &str)] = &[ ("layer.trackMatte", "Matte with Layer Below", "Spurmaske mit Ebene darunter"), ("", "Layer Styles", "Ebenenstile"), ("layer.style.options", "Layer Style Options...", "Ebenenstiloptionen..."), - ("layer.style.convertToEditable", "Convert to Editable Styles", "In editierbare Formate umwandeln"), + ("layer.style.convertToEditable", "Convert to Editable Styles", "In bearbeitbare Stile umwandeln"), ("layer.style.showAll", "Show All", "Alle anzeigen"), ("layer.style.removeAll", "Remove All", "Alle entfernen"), ("layer.style.dropShadow", "Drop Shadow", "Schlagschatten"), @@ -5715,7 +5722,7 @@ const GERMAN: &[(&str, &str, &str)] = &[ ("path.groupShapes", "Group Shapes", "Formen gruppieren"), ("path.ungroupShapes", "Ungroup Shapes", "Gruppierung von Formen aufheben"), ("", "Arrange", "Anordnen"), - ("layer.arrange", "Bring Layer to Front", "Ebene ganz nach vorn"), + ("layer.arrange", "Bring Layer to Front", "Ebene ganz nach vorne"), ("layer.arrange", "Bring Layer Forward", "Ebene nach vorne"), ("layer.arrange", "Send Layer Backward", "Ebene nach hinten"), ("layer.arrange", "Send Layer to Back", "Ebene ganz nach hinten"), @@ -6679,6 +6686,59 @@ mod tests { assert_eq!(entry(&app, &redo, "Redo Text Čeština".into()), "Znovu Text Čeština"); } + #[test] + fn german_reveal_labels_follow_the_platform_file_manager() { + // As `czech_…` and `ukrainian_…` do. Asserted through `label`, not `entry`: `entry` + // returns the English for German, Russian and French, which have catalogues but are + // not wired into the menus (see the undo/redo match in `entry` and `supported`). + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.session.execute("prefs.set", json!({"key": "general.language", "value": "de"})).unwrap(); + for command in ["file.revealInFinder", "layer.revealInFinder"] { + for (source, translated) in [ + ("Reveal in Finder", "Im Finder anzeigen"), + ("Reveal in Explorer", "Im Explorer anzeigen"), + ("Reveal in File Manager", "Im Dateimanager anzeigen"), + ] { + assert_eq!(label(&app, command, source), translated, "{command} / {source}"); + // Another command keeps a label that merely looks like one of these. + assert_eq!(label(&app, "file.openRecent", source), source); + } + assert_eq!(label(&app, command, "My file.mp4"), "My file.mp4", "a user's filename is never looked up"); + } + } + + #[test] + fn german_menu_labels_name_what_their_command_does() { + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.session.execute("prefs.set", json!({"key": "general.language", "value": "de"})).unwrap(); + // "Ebenenreihenfolge umkehren" reads as reversing the stacking order, not time, and + // "Formate" are formats, not layer styles (#1443). + assert_eq!(label(&app, "layer.timeReverse", "Time-Reverse Layer"), "Ebene zeitlich umkehren"); + assert_eq!(label(&app, "keys.timeReverse", "Time-Reverse Keyframes"), "Keyframes zeitlich umkehren"); + assert_eq!(label(&app, "layer.style.convertToEditable", "Convert to Editable Styles"), "In bearbeitbare Stile umwandeln"); + } + + #[test] + fn german_uses_one_word_per_term() { + let german: Vec<&str> = GERMAN.iter().map(|(_, _, de)| *de).collect(); + let with = |needle: &str| german.iter().filter(|de| de.contains(needle)).copied().collect::>(); + // "Expression" is "Ausdruck", not the English word. + assert!(with("Expression").is_empty(), "{:?}", with("Expression")); + // "Track" as a verb is "verfolgen", not "tracken". + assert!(with("tracken").is_empty(), "{:?}", with("tracken")); + // "Reveal " is "anzeigen". ("einblenden" stays right for "Show All", + // "Fade from Blur" and the like, so only the Reveal entries are checked.) + let reveal: Vec<&str> = + GERMAN.iter().filter(|(_, en, _)| en.starts_with("Reveal")).map(|(_, _, de)| *de).filter(|de| !de.to_lowercase().contains("anzeigen")).collect(); + assert!(reveal.is_empty(), "{reveal:?}"); + // One spelling of "to the front". + let vorn: Vec<&str> = german.iter().filter(|de| de.ends_with("nach vorn")).copied().collect(); + assert!(vorn.is_empty(), "{vorn:?}"); + // The terms those entries joined are still the majority ones. + assert!(with("verfolgen").len() >= 4, "{:?}", with("verfolgen")); + assert!(with("anzeigen").len() >= 12, "{}", with("anzeigen").len()); + } + #[test] fn czech_reveal_labels_follow_the_platform_file_manager() { let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); diff --git a/crates/ui-egui/src/i18n/effects.rs b/crates/ui-egui/src/i18n/effects.rs index 3ab11558..5c6d09ba 100644 --- a/crates/ui-egui/src/i18n/effects.rs +++ b/crates/ui-egui/src/i18n/effects.rs @@ -439,13 +439,56 @@ const TRADITIONAL_CHINESE: &[Row] = &[]; /// `CC` series, the `VR` set, the `OCIO` set, `Cryptomatte`, `EXtractoR`, `IDentifier`, /// `Key Light`, `Unmult`, `Twirl (Legacy)` and the two EffectCraft-only effects) repeat that /// English name here rather than being left out. -const JAPANESE_EFFECTS: &[Row] = &[]; +/// +/// The rows below were read from the installed After Effects 2026 (26.5) Japanese interface +/// (its generated Effect menu and its Effects & Presets panel). +const JAPANESE_EFFECTS: &[Row] = &[ + ("ec.blur.bilateral", "Bilateral Blur", "ブラー(バイラテラル)"), + ("ec.blur.cameralens", "Camera Lens Blur", "ブラー(カメラレンズ)"), + ("ec.blur.channel", "Channel Blur", "ブラー(チャンネル)"), + ("ec.blur.compound", "Compound Blur", "ブラー(合成)"), + ("ec.blur.directional", "Directional Blur", "ブラー(方向)"), + ("ec.blur.fastbox", "Fast Box Blur", "高速ボックスブラー"), + ("ec.blur.gaussian", "Gaussian Blur", "ブラー(ガウス)"), + ("ec.blur.radial", "Radial Blur", "ブラー(放射状)"), + ("ec.color.lumetri", "Lumetri Color", "Lumetri カラー"), + ("ec.key.colorkey", "Color Key", "カラーキー"), + ("ec.key.luma", "Luma Key", "ルミナンスキー"), + ("ec.key.spill", "Spill Suppressor", "スピルサプレッション"), + ("ec.obsolete.basic3d", "Basic 3D", "基本 3D"), + ("ec.obsolete.basictext", "Basic Text", "基本テキスト"), + ("ec.obsolete.gaussianlegacy", "Gaussian Blur (Legacy)", "ブラー(ガウス)(レガシー)"), + ("ec.obsolete.lightning", "Lightning", "稲妻"), + ("ec.obsolete.pathtext", "Path Text", "パステキスト"), + ("ec.time.pixelmotionblur", "Pixel Motion Blur", "ピクセルモーションブラー"), +]; /// Traditional Chinese (`zh-hant`) category labels, keyed by the registry's English category. const TRADITIONAL_CHINESE_CATEGORIES: &[(&str, &str)] = &[]; /// Japanese (`ja`) category labels, keyed by the registry's English category. -const JAPANESE_CATEGORIES: &[(&str, &str)] = &[]; +const JAPANESE_CATEGORIES: &[(&str, &str)] = &[ + ("3D Channel", "3D チャンネル"), + ("Audio", "オーディオ"), + ("Blur & Sharpen", "ブラー&シャープ"), + ("Channel", "チャンネル"), + ("Color Correction", "カラー補正"), + ("Distort", "ディストーション"), + ("Expression Controls", "式制御"), + ("Immersive Video", "イマーシブビデオ"), + ("Keying", "キーイング"), + ("Matte", "マット"), + ("Noise & Grain", "ノイズ&グレイン"), + ("Obsolete", "旧バージョン"), + ("Paint", "描画"), + ("Perspective", "遠近"), + ("Simulation", "シミュレーション"), + ("Stylize", "スタイライズ"), + ("Text", "テキスト"), + ("Time", "時間"), + ("Transition", "トランジション"), + ("Utility", "ユーティリティ"), +]; /// The 21 category folder names the generated Effect menu and Effects & Presets draw, in /// Simplified Chinese. Read from the installed After Effects 2026 (26.5.0.89) Simplified Chinese @@ -521,14 +564,17 @@ mod tests { /// The languages that have no rows yet, with the table a translator fills in for each. A new /// language is added here and to [`catalog`]; the test then guards its rows as they arrive. - const PENDING: &[(&str, &[Row])] = &[("zh-hant", TRADITIONAL_CHINESE), ("ja", JAPANESE_EFFECTS)]; + const PENDING: &[(&str, &[Row])] = &[("zh-hant", TRADITIONAL_CHINESE)]; + + /// The languages whose tables are being filled in: guarded like [`PENDING`] row by row. + const PARTIAL: &[(&str, &[Row])] = &[("ja", JAPANESE_EFFECTS)]; /// A pending language may lag Simplified Chinese but may not invent ids or disagree about /// which effect an id names: a translator fills these tables row by row. While they are /// empty the test is a no-op, which is why the slot ships before the data does. #[test] fn a_pending_language_only_uses_known_ids() { - for (language, rows) in PENDING { + for (language, rows) in PENDING.iter().chain(PARTIAL) { let mut ids: Vec<&str> = vec![]; for (id, english, translated) in *rows { let Some((_, hans_english, _)) = SIMPLIFIED_CHINESE.iter().find(|(hans_id, _, _)| hans_id == id) else { @@ -606,6 +652,9 @@ mod tests { assert_eq!(category("Blur & Sharpen"), "模糊和锐化"); assert_eq!(name("ec.unknown.effect", "Mystery"), "Mystery"); assert_eq!(category("Not A Category"), "Not A Category"); + set_current("ja"); + assert_eq!(name("ec.blur.gaussian", "Gaussian Blur"), "ブラー(ガウス)"); + assert_eq!(name("ec.unknown.effect", "Mystery"), "Mystery", "a row a partial table lacks draws the registry name"); set_current("en"); } diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index 2543081b..9831092d 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -555,7 +555,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("3D Reference Axes", "3D参照軸"), ("Affect all layers that use this solid", "この平面を使用するすべてのレイヤーに適用"), ("Choose grid and guide options", "グリッドとガイドのオプションを選択"), - ("Colorize", "カラー化"), + ("Colorize", "色付き"), ("Custom", "カスタム"), ("Custom...", "カスタム..."), ("Fit", "フィット"), @@ -563,7 +563,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Full", "フル画質"), ("Half", "1/2 画質"), ("Only this layer uses this solid.", "この平面を使用するのはこのレイヤーだけです。"), - ("Proportional Grid", "均等グリッド"), + ("Proportional Grid", "プロポーショナルグリッド"), ("Quarter", "1/4 画質"), ("Region of Interest", "関心領域"), ("Reset Exposure", "露出をリセット"), @@ -589,7 +589,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Import Image Sequence", "画像シーケンスを読み込み"), ("Import PDF / Illustrator File", "PDF / Illustratorファイルを読み込み"), ("Import Photoshop File", "Photoshopファイルを読み込み"), - ("Interpret Unlabeled Alpha As", "ラベルなしアルファの解釈"), + ("Interpret Unlabeled Alpha As", "ラベルのないアルファを変換"), ("Join the ArtCraft Discord", "ArtCraftのDiscordに参加"), ("Mercury GPU Acceleration ({})", "Mercury GPUアクセラレーション({})"), ("Mercury Software Only", "Mercuryソフトウェアのみ"), @@ -597,7 +597,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Off (Final Quality)", "オフ(最終品質)"), ("OpenGL (compatibility)", "OpenGL(互換)"), ("Opus Bitrate: {} kbps", "Opusビットレート:{} kbps"), - ("RGB Straight", "RGB(ストレート)"), + ("RGB Straight", "RGB ストレート"), ("Red", "赤"), ("Tone map", "トーンマップ"), ("Use In-Window Menu Bar on macOS", "macOSでウィンドウ内メニューバーを使用"), diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index b1eba4f0..26a7b66f 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1526,6 +1526,19 @@ pub(crate) fn panel_shown(app: &EffectcraftApp, p: PanelKind) -> bool { } } +/// The command a menu entry runs when chosen from a menu: Window ▸ closes a panel whose +/// entry is checked, as in After Effects (#1630). Shortcuts and agents calling `window.panel` +/// still only show the panel. +pub(crate) fn menu_choice(app: &EffectcraftApp, id: String, params: Value) -> (String, Value) { + if id == "window.panel" + && let Some(p) = params.get("panel").and_then(Value::as_str).and_then(PanelKind::from_name) + && panel_shown(app, p) + { + return ("window.closePanel".into(), params); + } + (id, params) +} + /// Check-mark state of frontend toggles (engine state is answered by the engine). pub(crate) fn entry_checked(app: &EffectcraftApp, e: &MenuEntry) -> Option { let v = &app.ui.viewer; @@ -1995,7 +2008,7 @@ pub fn menu_bar(app: &mut EffectcraftApp, ui: &mut egui::Ui) { }); }); crate::menu_keys::handle(&ctx, &nav); - if let Some((id, params)) = clicked + if let Some((id, params)) = clicked.map(|(id, params)| menu_choice(app, id, params)) && let Err(e) = invoke(app, &ctx, &id, if params.is_null() { json!({}) } else { params }) { app.ui.status = e; diff --git a/crates/ui-egui/src/native_menu.rs b/crates/ui-egui/src/native_menu.rs index 3e098449..ad30ded5 100644 --- a/crates/ui-egui/src/native_menu.rs +++ b/crates/ui-egui/src/native_menu.rs @@ -376,6 +376,7 @@ pub fn activate(app: &mut EffectcraftApp, ctx: &egui::Context, menu: &NativeMenu } } let params = if params.is_null() { json!({}) } else { params }; + let (command, params) = crate::menus::menu_choice(app, command, params); let r = crate::menus::invoke(app, ctx, &command, params); if let Err(e) = &r { app.ui.status = e.clone(); @@ -612,6 +613,23 @@ mod tests { assert!(spec2.items().iter().any(|i| i.command == "file.openRecent" && i.label == "Recent Test.ecproj")); } + /// #1630: a checked Window menu entry closes its panel; an unchecked one opens it. + #[test] + fn window_menu_entry_toggles_its_panel() { + use crate::dock::PanelKind; + let mut app = app(); + let ctx = egui::Context::default(); + let choose = |app: &mut EffectcraftApp| { + let spec = build(app); + let id = spec.items().into_iter().find(|i| i.command == "window.panel" && i.params["panel"] == "align").unwrap().id.clone(); + activate(app, &ctx, &spec, &id).unwrap(); + }; + choose(&mut app); + assert!(app.ui.dock.contains(PanelKind::Align)); + choose(&mut app); + assert!(!app.ui.dock.contains(PanelKind::Align)); + } + #[test] fn activation_dispatches_the_menu_command() { let mut app = app(); diff --git a/crates/ui-egui/src/panels/expr_editor.rs b/crates/ui-egui/src/panels/expr_editor.rs index f800b1a4..f342d563 100644 --- a/crates/ui-egui/src/panels/expr_editor.rs +++ b/crates/ui-egui/src/panels/expr_editor.rs @@ -2,6 +2,7 @@ //! Font Size, Syntax Highlighting, Line Numbers, Auto-complete (Tab accepts), Bracket Matching //! and Word Wrap. The Composition viewer uses the shared diagnostics in `expr_bar`. +use effectcraft_engine::commands::expr_tools::language_menu; use effectcraft_engine::prefs::Scripting; use egui::text::{LayoutJob, TextFormat}; use egui::{Color32, FontId, Rect, Stroke, pos2, vec2}; @@ -211,6 +212,37 @@ pub fn completions(text: &str, cursor: usize) -> (String, Vec<&'static str>) { (word, c) } +/// How `name` is called, from the Expression Language menu's signatures: `None` for a +/// property, `Some(true)` for a function that takes arguments, `Some(false)` for one that never +/// does. +fn call_shape(name: &str) -> Option { + let calls: Vec<&str> = + language_menu().into_iter().flat_map(|(_, items)| items).filter_map(|(_, text)| text.strip_prefix(name)?.strip_prefix('(')).collect(); + (!calls.is_empty()).then(|| calls.iter().any(|rest| !rest.starts_with(')'))) +} + +/// Accepts the first auto-complete candidate at char index `cursor`: inserts the rest of the +/// name, plus `()` for a function (#1393), and returns the new cursor: between the parentheses, +/// or after them when the function takes no arguments. +pub fn accept_completion(buf: &mut String, cursor: usize) -> Option { + let (word, cands) = completions(buf, cursor); + let first = cands.first()?; + let at = byte_of(buf, cursor); + let rest = &first[word.len()..]; + let end = cursor + first.chars().count() - word.chars().count(); + let already_called = buf.get(at..).is_some_and(|after| after.starts_with('(')); + match call_shape(first).filter(|_| !already_called) { + Some(takes_args) => { + buf.insert_str(at, &format!("{rest}()")); + Some(end + if takes_args { 1 } else { 2 }) + } + None => { + buf.insert_str(at, rest); + Some(end) + } + } +} + /// Colour runs for JavaScript: (byte start, byte end, colour). pub fn highlight(text: &str, base: Color32, sx: &Syntax) -> Vec<(usize, usize, Color32)> { let b = text.as_bytes(); @@ -320,15 +352,10 @@ pub fn editor(ui: &mut egui::Ui, id: egui::Id, buf: &mut String, rect: Rect, p: if p.auto_complete && child.memory(|m| m.has_focus(id)) && let Some(c) = cursor + && !completions(buf, c).1.is_empty() + && child.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Tab)) { - let (word, cands) = completions(buf, c); - if let Some(first) = cands.first() - && child.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Tab)) - { - let at = byte_of(buf, c); - buf.insert_str(at, &first[word.len()..]); - accepted = Some(c + first.chars().count() - word.chars().count()); - } + accepted = accept_completion(buf, c); } let out = egui::TextEdit::multiline(buf) .id(id) @@ -446,3 +473,37 @@ mod contrast_tests { assert_eq!(col("// note"), Some(sx.comment)); } } + +#[cfg(test)] +mod completion_tests { + use super::*; + + /// #1393: accepting a function inserts the call, with the cursor between the parentheses + /// (after them when the function takes no arguments); a property stays a bare name. + #[test] + fn accepting_a_function_completion_inserts_the_call() { + let mut buf = String::from("x = loopO"); + assert_eq!(accept_completion(&mut buf, 9), Some(12)); + assert_eq!(buf, "x = loopOut()"); + + let mut buf = String::from("wigg"); + assert_eq!(accept_completion(&mut buf, 4), Some(7)); + assert_eq!(buf, "wiggle()"); + + let mut buf = String::from("isClo"); + assert_eq!(accept_completion(&mut buf, 5), Some(10)); + assert_eq!(buf, "isClosed()"); + + let mut buf = String::from("tim"); + assert_eq!(accept_completion(&mut buf, 3), Some(4)); + assert_eq!(buf, "time"); + } + + /// Completing a name already followed by its call does not add a second pair. + #[test] + fn accepting_before_an_existing_call_keeps_one_pair_of_parentheses() { + let mut buf = String::from("loopO(\"cycle\")"); + assert_eq!(accept_completion(&mut buf, 5), Some(7)); + assert_eq!(buf, "loopOut(\"cycle\")"); + } +} diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index fc6d40e8..0b372c0a 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -109,6 +109,15 @@ fn shown_dims(l: &Layer, pr: &Property) -> usize { if pr.shown_dims > 0 && !l.is_3d() { (pr.shown_dims as usize).min(n) } else { n.min(4) } } +/// The speed graph's value of dimension `d` at layer time `lt` (one curve for a spatial property). +fn speed_at(pr: &Property, spatial: bool, d: usize, lt: Tick) -> f64 { + if spatial { + effectcraft_engine::keyframe::speed(&pr.keys, lt, true) + } else { + effectcraft_engine::keyframe::velocity(&pr.keys, lt, false).get(d).copied().unwrap_or(0.0).abs() + } +} + /// Build the curves for the visible time span. fn curves<'a>(app: &EffectcraftApp, comp: &'a Comp, ectx: &EvalCtx, tm: TMap, plot: Rect, speed: bool) -> Vec> { let sel = &app.session.state.selected_keys; @@ -126,16 +135,7 @@ fn curves<'a>(app: &EffectcraftApp, comp: &'a Comp, ectx: &EvalCtx, tm: TMap, pl let x = plot.min.x + plot.width() * i as f32 / (n - 1) as f32; let ct = tm.t(x); let tt = Tick::from_seconds_f64(ct); - let v = if speed { - let lt = l.layer_time(tt); - if spatial { - effectcraft_engine::keyframe::speed(&pr.keys, lt, true) - } else { - effectcraft_engine::keyframe::velocity(&pr.keys, lt, false).get(d).copied().unwrap_or(0.0).abs() - } - } else { - ectx.at(tt).value(l, pr).components().get(d).copied().unwrap_or(0.0) - }; + let v = if speed { speed_at(pr, spatial, d, l.layer_time(tt)) } else { ectx.at(tt).value(l, pr).components().get(d).copied().unwrap_or(0.0) }; pts.push((ct, v)); } let mut keys = vec![]; @@ -290,7 +290,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte ctx.data_mut(|d| d.insert_temp(was_speed_id, speed)); if ctx.data_mut(|d| d.remove_temp::(fit_pending_id())).unwrap_or(false) || type_changed { if cs.iter().any(|c| c.keys.iter().any(|k| k.sel)) { - fit_selection(app, &cs, tm, plot); + fit_selection(app, &cs, tm, plot, speed); } else { fit_all(app); } @@ -584,7 +584,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte fit_all(app); } if text_button(app, ui, p, &mut buttons, y, "Fit Selection", false, "fitSelection", "Fit selection to view") { - fit_selection(app, &cs, tm, plot); + fit_selection(app, &cs, tm, plot, speed); } buttons.x += 6.0; let (snap, reference, tbox) = (app.ui.timeline.graph_snap, app.ui.timeline.graph_reference, app.ui.timeline.graph_transform_box); @@ -646,7 +646,7 @@ pub(crate) fn track_open(ctx: &egui::Context, on: bool) { } /// Zoom time and value to the selected keys (and their handles). -fn fit_selection(app: &mut EffectcraftApp, cs: &[Curve], tm: TMap, plot: Rect) { +fn fit_selection(app: &mut EffectcraftApp, cs: &[Curve], tm: TMap, plot: Rect, speed: bool) { let mut t0 = f64::INFINITY; let mut t1 = f64::NEG_INFINITY; let mut v0 = f64::INFINITY; @@ -678,6 +678,26 @@ fn fit_selection(app: &mut EffectcraftApp, cs: &[Curve], tm: TMap, plot: Rect) { } } } + // In the speed graph eased keys sit at speed 0 and the curve peaks between them (#1617): + // frame the curve from the first to the last selected key. A lone key has its handles on + // the speed graph, so the check above misses it: it frames its neighbouring segments here. + if speed { + for c in cs { + let (Some(first), Some(last)) = (c.keys.iter().position(|k| k.sel), c.keys.iter().rposition(|k| k.sel)) else { continue }; + let (a, b) = if first == last { (first.saturating_sub(1), last + 1) } else { (first, last) }; + let (Some(ka), Some(kb)) = (c.keys.get(a), c.keys.get(b.min(c.keys.len().saturating_sub(1)))) else { continue }; + t0 = t0.min(ka.t); + t1 = t1.max(kb.t); + let (s0, s1) = (ka.time.seconds(), kb.time.seconds()); + const SAMPLES: usize = 128; + for i in 0..=SAMPLES { + let lt = Tick::from_seconds_f64(s0 + (s1 - s0) * i as f64 / SAMPLES as f64); + let v = speed_at(c.prop, !c.editable, c.dim, lt); + v0 = v0.min(v); + v1 = v1.max(v); + } + } + } let span = (t1 - t0).max(0.2); let pps = plot.width() as f64 * 0.8 / span; // Through the timeline's zoom, so the fit never zooms out past the whole comp. diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index b2d2ca43..28a7c014 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -664,6 +664,8 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.vertical(|ui| { ui.set_width(180.0); egui::ScrollArea::vertical().id_salt("settings-pages").max_height(body_h).auto_shrink([false, false]).show(ui, |ui| { + // The rows touch, so the pointer never falls between two of them. + ui.spacing_mut().item_spacing.y = 0.0; for p in &all { let (r, resp) = ui.allocate_exact_size(vec2(176.0, 24.0), egui::Sense::click()); let sel = page == p.id; diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index ad8b17d1..aff4ef2c 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -1996,8 +1996,11 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { gp.line_segment([pos2(graph_x0, r.max.y - 0.5), pos2(r.max.x, r.max.y - 0.5)], Stroke::new(1.0, t.app_bg)); // A/V features. let sw = &layer.switches; + // Every layer has the Video switch (the eye), nulls, cameras and lights included, + // as in After Effects: it hides a null's outline in the viewer and turns a camera or + // light off. let av_items: [(Icon, bool, &str, bool); 4] = [ - (Icon::Eye, sw.video, "video", layer.source.is_av()), + (Icon::Eye, sw.video, "video", true), (Icon::Speaker, sw.audio, "audio", layer.props.sub("audio").is_some()), (Icon::Solo, sw.solo, "solo", layer.source.is_av()), (Icon::Lock, sw.locked, "lock", true), @@ -2404,6 +2407,16 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { }, ); gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, t.tl_bg); + // An effect's colour label (26.5): the same bar the Effect Controls header wears, + // on the row's left edge. + if let Some(lab) = + group.filter(|g| matches!(g.kind, GroupKind::Effect { .. })).and_then(|g| g.label).filter(|l| *l != effectcraft_engine::color::Label::None) + { + let [lr, lg, lb] = app.session.prefs.label_rgb(lab); + let bar = Rect::from_min_size(left.min, vec2(4.0, left.height())); + lp.rect_filled(bar, 0.0, Color32::from_rgb(lr, lg, lb)); + app.auto.add(&format!("timeline.group.{uid}.label"), bar, &app.session.prefs.label_name(lab)); + } let indent = cw.name + 6.0 + 14.0 * row.depth as f32; if *has_children { let tw = Rect::from_center_size(pos2(indent, cy), vec2(13.0, 13.0)); @@ -3334,8 +3347,18 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { { let released = ctx.input(|i| i.pointer.any_released()); if let crate::panels::DragPayload::Effect(e) = payload.as_ref() { - p.rect_stroke(rows_rect, 0.0, Stroke::new(2.0, t.accent), StrokeKind::Inside); let target = hit_rows.iter().find(|(r, _)| ptr.y >= r.min.y && ptr.y < r.max.y).map(|(_, row)| row.layer); + // The layer the effect would land on (its row and its open properties) is the one + // that lights up, not the whole timeline. + if let Some(l) = target + && let Some(span) = hit_rows.iter().filter(|(_, row)| row.layer == l).map(|(r, _)| *r).reduce(|a, b| a.union(b)) + { + let lit = span.intersect(rows_rect); + let rp = p.with_clip_rect(rows_rect); + rp.rect_filled(lit, 0.0, t.accent.gamma_multiply(0.18)); + rp.rect_stroke(lit, 0.0, Stroke::new(2.0, t.accent), StrokeKind::Inside); + app.auto.add("timeline.dropTarget", lit, "Drop target layer"); + } if released && let Some(l) = target { actions.push(("effect.apply".into(), json!({"effect": e, "layers": [l.0]}))); } diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index f4bb5a88..51bab7fd 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -996,7 +996,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { // Layer Handles and Paths. let hs = app.session.prefs.general.path_point_size as f32 + 2.0; let label_handles = app.session.prefs.appearance.use_label_color_for_handles; - for l in comp.layers.iter().filter(|l| selected.contains(&l.id) && l.is_active_at(time)) { + // A layer whose Video switch is off has no box, handles or paths in the viewer (a hidden + // null disappears from the comp, as in After Effects), even while it is selected. + for l in comp.layers.iter().filter(|l| selected.contains(&l.id) && l.is_active_at(time) && l.switches.video) { let col = if label_handles { t.label(l.label) } else { t.accent }; // Motion path of animated position (keyframes and spatial tangents are draggable). ov::motion_path(app, &painter, &map, &ectx, l, col, &mut key_hits, &mut motion_frames); @@ -1281,6 +1283,13 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { context_actions.push(("comp.copyFrame".to_owned(), serde_json::json!({"comp": cid.0}))); ui.close(); } + // The comp this viewer shows, which is not always the active one. + let settings = ui.button(crate::i18n::tr("Composition Settings…")); + app.auto.add("viewer.context.settings", settings.rect, "Composition Settings…"); + if settings.clicked() { + context_actions.push(("comp.settings".to_owned(), serde_json::json!({"comp": cid.0}))); + ui.close(); + } }); for (id, params) in context_actions { if let Err(e) = crate::menus::invoke(app, &ctx, &id, params) { @@ -2221,6 +2230,12 @@ fn pen_press( }); return; } + // Shift keeps the new segment at a multiple of 45° from the vertex before it (#1544). + let last = p.sp.vertices.last().map(|v| p.m.apply(gv2(v[0], v[1]))); + let c = match last { + Some(q) if mods.shift => constrain_45([q.x, q.y], c), + _ => c, + }; let lp = inv.apply(gv2(c[0], c[1])); let lp = [lp.x, lp.y]; if let Ok(v) = app.session.execute("mask.addVertex", json!({"layer": lid.0, "mask": uid, "point": lp})) { @@ -2282,10 +2297,25 @@ fn pen_press( }); } +/// `to` moved onto the nearest line through `from` at a multiple of 45° (horizontal, vertical or +/// diagonal), as far along it as `to` lies: the Pen's Shift-click. +fn constrain_45(from: [f64; 2], to: [f64; 2]) -> [f64; 2] { + let (dx, dy) = (to[0] - from[0], to[1] - from[1]); + if !(dx.is_finite() && dy.is_finite()) || (dx == 0.0 && dy == 0.0) { + return to; + } + let step = std::f64::consts::FRAC_PI_4; + let a = (dy.atan2(dx) / step).round() * step; + let (ux, uy) = (a.cos(), a.sin()); + let along = dx * ux + dy * uy; + [from[0] + ux * along, from[1] + uy * along] +} + /// Visible, unlocked layers eligible for viewer hit tests, in front-to-back order. -/// Solo scope matches the renderer: only active AV layers activate it, even when hidden or locked. +/// Solo scope matches the renderer: any soloed AV layer activates it at every time (as in After +/// Effects), even when hidden, locked or outside its in-out range. pub(crate) fn selectable_layers(comp: &Comp, time: Tick) -> impl Iterator { - let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av() && l.is_active_at(time)); + let any_solo = comp.layers.iter().any(|l| l.switches.solo && l.source.is_av()); comp.layers .iter() .filter(move |l| l.is_active_at(time) && l.switches.video && !l.switches.locked && !l.is_camera() && !l.is_light() && (!any_solo || l.switches.solo)) @@ -2684,6 +2714,18 @@ mod tests { use super::*; use effectcraft_engine::geom::{Mat4, vec3}; + /// #1544: Shift-clicking with the Pen keeps the new segment horizontal, vertical or diagonal. + #[test] + fn pen_shift_click_keeps_the_segment_at_45_degrees() { + let near = |a: [f64; 2], b: [f64; 2]| (a[0] - b[0]).abs() < 1e-9 && (a[1] - b[1]).abs() < 1e-9; + let from = [100.0, 100.0]; + assert!(near(constrain_45(from, [104.0, 180.0]), [100.0, 180.0]), "straight down"); + assert!(near(constrain_45(from, [30.0, 97.0]), [30.0, 100.0]), "left"); + assert!(near(constrain_45(from, [150.0, 46.0]), [152.0, 48.0]), "up and right, on the diagonal"); + assert!(near(constrain_45(from, from), from), "on the vertex"); + assert!(constrain_45(from, [f64::NAN, 3.0])[0].is_nan(), "a non-finite point stays as it is"); + } + /// The Rotation tool counts past a full turn in both directions, as in After Effects: the /// pointer going round the anchor 1.5 turns clockwise gives 540°, then back 2 turns gives /// −180°, with no jump where atan2 wraps from 180° to −180°. diff --git a/crates/ui-egui/tests/ui_blank_context_menus.rs b/crates/ui-egui/tests/ui_blank_context_menus.rs index 9b06add9..20f7467c 100644 --- a/crates/ui-egui/tests/ui_blank_context_menus.rs +++ b/crates/ui-egui/tests/ui_blank_context_menus.rs @@ -87,6 +87,20 @@ fn timeline_blank_menu_opens_solid_settings() { assert_eq!(h.state().dialog, Some(Dialog::SolidSettings)); } +/// Right-clicking the composition in the viewer offers Composition Settings…, which opens on the +/// comp the viewer shows. +#[test] +fn viewer_menu_opens_composition_settings() { + let mut h = harness(); + let comp = h.state().session.active_comp_id().unwrap(); + let p = rect(&h, "viewer.comp").center(); + right_click(&mut h, p); + h.get_by_label("Composition Settings…").click(); + h.run_steps(3); + assert_eq!(h.state().dialog, Some(Dialog::CompSettings)); + assert_eq!(h.state().session.active_comp_id(), Some(comp)); +} + #[test] fn project_item_and_timeline_layer_keep_their_own_context_menus() { let mut h = harness(); diff --git a/crates/ui-egui/tests/ui_color_hex.rs b/crates/ui-egui/tests/ui_color_hex.rs index 90eb19b8..4f8a8ab1 100644 --- a/crates/ui-egui/tests/ui_color_hex.rs +++ b/crates/ui-egui/tests/ui_color_hex.rs @@ -114,6 +114,32 @@ fn is_open(h: &Harness<'_, [f32; 3]>) -> bool { h.query_by_role_and_label(Role::TextInput, "#").is_some() } +/// #1476: the click that opens the picker must not close it. The picker's area only reports its +/// real position from its second pass — on the pass it first draws, its rect is at the origin — +/// so a click on a swatch outside that stale rect was read as a press outside the popup and the +/// picker flashed and closed in the frame it opened. +#[test] +fn a_click_that_opens_the_picker_does_not_close_it_in_the_same_frame() { + let mut h = Harness::builder().with_size(vec2(560.0, 400.0)).build_ui_state( + |ui, c: &mut [f32; 3]| { + // Away from the top-left corner: the stale first-pass rect covers it. + ui.allocate_space(vec2(440.0, 120.0)); + ui.horizontal(|ui| { + ui.add_space(440.0); + widgets::srgb_color_button(ui, c); + }); + }, + [1.0, 0.0, 0.0], + ); + h.run_steps(2); + let button = h.get_by_role(Role::ColorWell).rect().center(); + h.input_mut().events.push(Event::PointerMoved(button)); + h.input_mut().events.push(Event::PointerButton { pos: button, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.input_mut().events.push(Event::PointerButton { pos: button, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(2); + assert!(is_open(&h), "the click that opens the picker must not close it"); +} + #[test] fn the_dialog_colour_button_stores_srgb() { let mut h = picker([1.0, 1.0, 1.0]); diff --git a/crates/ui-egui/tests/ui_effect_drop_highlight.rs b/crates/ui-egui/tests/ui_effect_drop_highlight.rs new file mode 100644 index 00000000..6c0ede47 --- /dev/null +++ b/crates/ui-egui/tests/ui_effect_drop_highlight.rs @@ -0,0 +1,40 @@ +//! Dragging an effect over the Timeline lights up the layer it would land on, not the whole panel. +use effectcraft_engine::Session; +use effectcraft_ui_egui::{EffectcraftApp, panels::DragPayload}; +use egui::{Event, Modifiers, PointerButton, Pos2, Rect, pos2, vec2}; +use egui_kittest::Harness; +use serde_json::json; + +fn rect(h: &Harness<'_, EffectcraftApp>, id: &str) -> Rect { + let e = h.state().auto.find(id).unwrap_or_else(|| panic!("missing {id}")); + Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3])) +} + +fn drag_to(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.input_mut().events.push(Event::PointerMoved(p)); + h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + egui::DragAndDrop::set_payload(&h.ctx, DragPayload::Effect("CC Bend It".into())); + h.run_steps(2); +} + +#[test] +fn effect_drag_highlights_the_layer_under_the_pointer() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 640, "height": 360, "duration": 4})).unwrap(); + let a = s.execute("layer.newNull", json!({})).unwrap()["layer"].as_u64().unwrap(); + let b = s.execute("layer.newNull", json!({})).unwrap()["layer"].as_u64().unwrap(); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(3); + assert!(h.state().auto.find("timeline.dropTarget").is_none(), "nothing is lit without a drag"); + let (row_a, row_b) = (rect(&h, &format!("timeline.layer.{a}.row")), rect(&h, &format!("timeline.layer.{b}.row"))); + let panel = rect(&h, "timeline.layerMarquee"); + + drag_to(&mut h, row_b.center()); + let lit = rect(&h, "timeline.dropTarget"); + assert!(lit.contains(row_b.center()) && !lit.contains(row_a.center()), "lit {lit:?} for row {row_b:?}"); + assert!(lit.height() < panel.height(), "only the layer is lit, not the whole timeline"); + + drag_to(&mut h, row_a.center()); + let lit = rect(&h, "timeline.dropTarget"); + assert!(lit.contains(row_a.center()) && !lit.contains(row_b.center()), "the highlight follows the pointer: {lit:?}"); +} diff --git a/crates/ui-egui/tests/ui_keyframes.rs b/crates/ui-egui/tests/ui_keyframes.rs index 6167fbf8..b47963d8 100644 --- a/crates/ui-egui/tests/ui_keyframes.rs +++ b/crates/ui-egui/tests/ui_keyframes.rs @@ -562,3 +562,39 @@ fn speed_graph_handle_drags_upward_past_the_top() { // The speed per pixel depends on the graph's fitted scale; only the proportion matters. assert!(short > 0.0 && long > 9.0 * short, "40 px up gave {short}, 400 px up gave {long}"); } + +/// #1617: with Easy Ease both keys sit at speed 0 in the speed graph, so fitting the selection to +/// its keys and handles framed -1..1 while the curve between them peaks far above. Opening the +/// Graph Editor on selected keys and Fit Selection both frame the curve's peak, and a lone +/// selected key frames its neighbouring segments. +#[test] +fn speed_graph_fit_selection_frames_the_curve_between_the_keys() { + let (mut h, id, uid) = harness(); + let s = &mut h.state_mut().session; + s.execute("prop.select", json!({"layer": id.0, "prop": uid})).unwrap(); + s.execute("keys.selectAll", json!({})).unwrap(); + s.execute("keys.easyEase", json!({})).unwrap(); + let keys = h.state().session.active_comp().unwrap().layer(id).unwrap().props.prop("transform/opacity").unwrap().keys.clone(); + let peak = (0..=100) + .map(|i| effectcraft_engine::keyframe::velocity(&keys, effectcraft_engine::time::Tick::from_seconds_f64(i as f64 / 100.0), false)[0].abs()) + .fold(0.0, f64::max); + assert!(peak > 100.0, "the eased curve peaks above its average speed: {peak}"); + h.state_mut().ui.timeline.graph_mode = "speed".into(); + h.state_mut().ui.timeline.graph_editor = true; + h.run_steps(4); + let frames_peak = |h: &Harness<'_, EffectcraftApp>, what: &str| { + let (lo, hi) = h.state().ui.timeline.graph_range.expect("a fitted range"); + assert!(lo <= 0.0 && hi > peak * 0.9, "{what}: range {lo}..{hi}, the curve peaks at {peak}"); + }; + frames_peak(&h, "on open"); + h.state_mut().ui.timeline.graph_range = Some((-1.0, 1.0)); + let fit = h.state().auto.find("timeline.graph.fitSelection").expect("the Fit Selection button").rect; + click_with(&mut h, pos2(fit[0] + fit[2] / 2.0, fit[1] + fit[3] / 2.0), Default::default()); + frames_peak(&h, "Fit Selection"); + let last = keys[1].time.seconds(); + h.state_mut().session.execute("keys.select", json!({"keys": [{"layer": id.0, "prop": uid, "time": last}]})).unwrap(); + assert_eq!(h.state().session.state.selected_keys.len(), 1); + h.state_mut().ui.timeline.graph_range = Some((-1.0, 1.0)); + click_with(&mut h, pos2(fit[0] + fit[2] / 2.0, fit[1] + fit[3] / 2.0), Default::default()); + frames_peak(&h, "lone key"); +} diff --git a/crates/ui-egui/tests/ui_menus.rs b/crates/ui-egui/tests/ui_menus.rs index e198a619..ca4c8cab 100644 --- a/crates/ui-egui/tests/ui_menus.rs +++ b/crates/ui-egui/tests/ui_menus.rs @@ -225,3 +225,22 @@ fn import_dialog_offers_mxf() { assert!(offered.iter().any(|e| e == "mxf"), "offered {offered:?}"); assert!(offered.iter().any(|e| e == "mov")); } + +/// #1059: opening the demo from the start screen and immediately pressing Spacebar previews +/// the demo — the start screen's keyboard focus must not outlive it. +#[test] +fn space_previews_after_opening_the_demo_from_home() { + let mut h = { + let s = Session::default(); + let mut app = EffectcraftApp::new(s); + app.ui.start_screen = true; + Harness::builder().with_size(vec2(1400.0, 900.0)).build_eframe(|_| app) + }; + h.run_steps(3); + let demo = rect(&h, "home.file.openDemoProject"); + click_at(&mut h, demo.center()); + assert!(h.state().session.active_comp().is_some(), "the demo comp is open"); + assert!(!h.state().ui.start_screen, "the start screen closed"); + key(&mut h, Key::Space); + assert!(h.state().playback.playing, "Space previews the demo (#1059)"); +} diff --git a/crates/ui-egui/tests/ui_panels.rs b/crates/ui-egui/tests/ui_panels.rs index 0c54445b..240f2de8 100644 --- a/crates/ui-egui/tests/ui_panels.rs +++ b/crates/ui-egui/tests/ui_panels.rs @@ -67,6 +67,26 @@ fn window_menu_opens_the_new_panels() { } } +/// #1630: choosing a checked panel in the Window menu closes it, as in After Effects (it only +/// brought the panel to the front, so the check mark never went away); choosing it again opens it. +#[test] +fn window_menu_closes_a_checked_panel() { + let mut h = harness(); + let shown = |h: &Harness<'_, EffectcraftApp>| { + h.state().ui.dock.contains(PanelKind::Align) || h.state().ui.floating.iter().any(|f| f.panels.contains(&PanelKind::Align)) + }; + open(&mut h, "align"); + assert!(shown(&h)); + click(&mut h, "menu.Window"); + let at = h.query_by_label("✔ Align").expect("Window ▸ Align, checked").rect().center(); + click_at(&mut h, at); + assert!(!shown(&h), "Align closed"); + click(&mut h, "menu.Window"); + let at = h.query_by_label(" Align").expect("Window ▸ Align, unchecked").rect().center(); + click_at(&mut h, at); + assert!(shown(&h), "Align open again"); +} + #[test] fn scopes_switch_kind() { let mut h = harness(); diff --git a/crates/ui-egui/tests/ui_timeline_effect_label.rs b/crates/ui-egui/tests/ui_timeline_effect_label.rs new file mode 100644 index 00000000..deeee9e5 --- /dev/null +++ b/crates/ui-egui/tests/ui_timeline_effect_label.rs @@ -0,0 +1,45 @@ +//! An effect's colour label (26.5) shows on its Timeline row as well as in Effect Controls. + +use effectcraft_engine::Session; +use effectcraft_engine::project::LayerId; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::PanelKind; +use egui::vec2; +use egui_kittest::Harness; +use serde_json::json; + +fn harness() -> (Harness<'static, EffectcraftApp>, u64, u64) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "frameRate": 30, "duration": 4})).unwrap(); + let layer = s.execute("layer.newSolid", json!({"name": "A", "color": "#406080"})).unwrap()["layer"].as_u64().unwrap(); + let fx = s.execute("effect.apply", json!({"layers": [layer], "effect": "ec.blur.gaussian"})).unwrap()["effects"][0].as_u64().unwrap(); + s.state.selected_layers = vec![LayerId(layer)]; + s.state.selected_props = vec![(LayerId(layer), fx)]; + let mut app = EffectcraftApp::new(s); + app.show_panel(PanelKind::Timeline); + app.ui.timeline.open_layers.insert(layer); + let effects = app.session.active_comp().and_then(|c| c.layer(LayerId(layer))).and_then(|l| l.props.sub("effects")).map(|g| g.uid); + app.ui.timeline.open_groups.extend(effects); + let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_eframe(|_| app); + h.run_steps(4); + (h, layer, fx) +} + +/// The label bar appears on the effect's row once the effect has a label, carries the label's +/// name for agents, and goes away when the label is cleared. +#[test] +fn the_effect_row_wears_its_colour_label() { + let (mut h, layer, fx) = harness(); + let id = format!("timeline.group.{fx}.label"); + assert!(h.state().auto.find(&format!("timeline.group.{fx}.fx")).is_some(), "the effect's row is on screen"); + assert!(h.state().auto.find(&id).is_none(), "an unlabelled effect has no bar"); + + h.state_mut().session.execute("effect.setLabel", json!({"layer": layer, "effect": fx, "label": "Blue"})).unwrap(); + h.run_steps(3); + let blue = h.state().session.prefs.label_name(effectcraft_engine::color::Label::Blue); + assert_eq!(h.state().auto.find(&id).map(|e| e.label.clone()), Some(blue)); + + h.state_mut().session.execute("effect.setLabel", json!({"layer": layer, "effect": fx, "label": 0})).unwrap(); + h.run_steps(3); + assert!(h.state().auto.find(&id).is_none(), "clearing the label removes the bar"); +} diff --git a/crates/ui-egui/tests/ui_viewer.rs b/crates/ui-egui/tests/ui_viewer.rs index e60226b1..47bb475c 100644 --- a/crates/ui-egui/tests/ui_viewer.rs +++ b/crates/ui-egui/tests/ui_viewer.rs @@ -121,8 +121,10 @@ fn viewer_click_does_not_pick_through_hidden_or_locked_solo_layers() { } } +/// Solo is not time-dependent (as in After Effects): outside the soloed layer's in-out range the +/// viewer shows nothing, so neither the soloed layer nor the others can be picked. #[test] -fn viewer_click_ignores_inactive_solo_layers() { +fn viewer_click_picks_nothing_while_the_solo_layer_is_inactive() { let mut h = selection_harness(); let comp = h.state().session.active_comp().unwrap(); let (front, back) = (comp.layers[0].id, comp.layers[1].id); @@ -133,7 +135,7 @@ fn viewer_click_ignores_inactive_solo_layers() { h.run_steps(3); let at = screen(&h, [320.0, 180.0]); click(&mut h, at); - assert_eq!(h.state().session.state.selected_layers, vec![back]); + assert!(h.state().session.state.selected_layers.is_empty(), "{back:?} stays excluded by solo"); } /// A selected layer under the pointer takes the press before the layers in front of it, as in @@ -761,6 +763,40 @@ fn mask_path(h: &Harness<'_, EffectcraftApp>, name: &str) -> effectcraft_engine: l.masks().unwrap().groups().next().unwrap().get("path").unwrap().value.as_path().unwrap().clone() } +/// #1544: a Shift-click with the Pen puts the next point straight below, beside or diagonal to the +/// one before it; without Shift it goes where it was clicked. +#[test] +fn pen_shift_click_draws_a_straight_segment() { + let mut h = harness(); + let plate = layer_id(&h, "Plate"); + h.state_mut().session.execute("layer.select", json!({"layers": [plate]})).unwrap(); + h.state_mut().session.execute("view.snapping", json!({"value": false})).unwrap(); + h.state_mut().ui.tool = Tool::Pen; + h.run_steps(2); + let first = screen(&h, [100.0, 60.0]); + click(&mut h, first); + let shift_click = |h: &mut Harness<'_, EffectcraftApp>, p: Pos2| { + let shift = egui::Modifiers::SHIFT; + h.input_mut().events.push(Event::ModifiersChanged(shift)); + h.input_mut().events.push(Event::PointerMoved(p)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: shift }); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: shift }); + h.input_mut().events.push(Event::ModifiersChanged(egui::Modifiers::NONE)); + h.run_steps(2); + }; + let down = screen(&h, [112.0, 250.0]); + shift_click(&mut h, down); + let free = screen(&h, [300.0, 290.0]); + click(&mut h, free); + let drawn = mask_path(&h, "Plate"); + let near = |a: [f64; 2], b: [f64; 2]| (a[0] - b[0]).abs() < 1.0 && (a[1] - b[1]).abs() < 1.0; + assert_eq!(drawn.vertices.len(), 3, "{drawn:?}"); + assert!(near(drawn.vertices[1], [100.0, 250.0]), "straight down: {drawn:?}"); + assert!(near(drawn.vertices[2], [300.0, 290.0]), "no Shift, no constraint: {drawn:?}"); +} + /// Drawing a mask with the Pen, moving a point and undoing take one action back per undo; a /// double-click on a point selects all of them in a free-transform box, and dragging inside it /// (or, once the box is gone, dragging a point) moves the whole mask (#290). @@ -2270,3 +2306,41 @@ fn mask_marquee_selects_vertices() { let sel = &h.state().session.state.selected_vertices; assert_eq!(sel.len(), 2, "shift-marquee adds: {sel:?}"); } + +/// A null's eye in the Timeline hides it in the Composition viewer, as in After Effects: its box +/// and handles go even while it is selected and a click on its box doesn't pick it; the eye back +/// on brings them back. +#[test] +fn null_eye_hides_it_in_the_viewer() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Nulls", "width": 640, "height": 360, "duration": 4})).unwrap(); + let n = s.execute("layer.newNull", json!({})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.select", json!({"layers": [n]})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(move |_| EffectcraftApp::new(s)); + h.run_steps(3); + let handle = format!("viewer.handle.{n}.0"); + let eye = format!("timeline.layer.{n}.video"); + assert!(h.state().auto.find(&handle).is_some(), "a shown null has handles"); + // The eye is there for a null and turns its video off. + let at = rect(&h, &eye).center(); + click(&mut h, at); + assert!(!h.state().session.active_comp().unwrap().layer(LayerId(n)).unwrap().switches.video); + h.run_steps(2); + assert!(h.state().auto.find(&handle).is_none(), "a hidden null has no handles"); + // A click on its box doesn't pick it. + h.state_mut().session.execute("edit.deselectAll", json!({})).unwrap(); + h.run_steps(40); // A new click, not a double-click. + let centre = screen(&h, [320.0, 180.0]); + click(&mut h, centre); + assert!(h.state().session.state.selected_layers.is_empty()); + // The eye back on: selectable again, with its handles. + h.run_steps(40); + let at = rect(&h, &eye).center(); + click(&mut h, at); + assert!(h.state().session.active_comp().unwrap().layer(LayerId(n)).unwrap().switches.video); + h.run_steps(40); + click(&mut h, centre); + assert_eq!(h.state().session.state.selected_layers, vec![LayerId(n)]); + h.run_steps(2); + assert!(h.state().auto.find(&handle).is_some(), "handles back with the eye on"); +} diff --git a/docs/effects.md b/docs/effects.md index 0bfc6a7b..b8214e43 100644 --- a/docs/effects.md +++ b/docs/effects.md @@ -149,7 +149,7 @@ Effects are addressed by id (`ec..`) or display name in commands | Corner Pin | `ec.distort.cornerpin` | 4 | GPU | 32 | Implemented | | Detail-preserving Upscale | `ec.distort.upscale` | 4 | | 32 | Implemented | | Displacement Map | `ec.distort.displacementmap` | 8 | GPU | 32 | Implemented | -| Liquify | `ec.distort.liquify` | 12 | GPU | 32 | Implemented | +| Liquify | `ec.distort.liquify` | 13 | GPU | 32 | Implemented | | Magnify | `ec.distort.magnify` | 10 | GPU | 32 | Implemented | | Mesh Warp | `ec.distort.meshwarp` | 3 | GPU | 32 | Implemented | | Mirror | `ec.distort.mirror` | 2 | GPU | 32 | Implemented | diff --git a/docs/gaps.md b/docs/gaps.md index c22a97fb..a20e779c 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -77,7 +77,7 @@ are grouped by area; the issue tracker has the rest. | 18 | **Relative paths and relinking** | Projects store absolute footage paths; no relink when a project moves | Moving or sharing a project breaks it | 6–12 h | G4 · [file-format-parity.md](file-format-parity.md) | | 19 | **Plug-in ecosystem** | No After Effects SDK hosting (#600, #327 OpenFX, #334, #376 plug-in manager); our WebAssembly API has no third-party plug-ins | Many pro projects depend on Trapcode, Element, Saber… | 40–80 h for API growth; SDK hosting needs an owner decision | G8 · [scripting-parity.md](scripting-parity.md) | | 20 | **Multi-monitor and video output** | #486: any panel can move into a native window of its own (panel menu ▸ Move Panel to New Window, `window.panelWindow`) and so onto another screen, restored with the workspace; still missing: dragging tabs between those windows, app shortcuts while one has the focus, named workspaces remembering them, Mercury Transmit to video hardware (we have a second preview window only) | Two-screen setups are standard for professionals | 10–20 h | G2 · [hardware-parity.md](hardware-parity.md) | -| 21 | **Small 26.x features** | Proportional Scrubbing (26.2), pasting Illustrator's own clipboard data as shapes (26.3; SVG markup on the clipboard pastes as a shape layer, #738), variable-font filter (26.3), the rebuilt Effect Controls (26.5). Percentage guides (View ▸ Add Percentage Guide, Convert Guides to Percentage / Pixels) and effect colour labels (Effect Controls header bar and right-click Label menu, `effect.setLabel`) are in (#1095), built from the release notes' one-line descriptions and never compared with After Effects: the Timeline shows no effect label, and the labels' exact UI there is unknown | Users of the current version expect them | 10–20 h left | — · [ui-parity.md](ui-parity.md) | +| 21 | **Small 26.x features** | Proportional Scrubbing (26.2), pasting Illustrator's own clipboard data as shapes (26.3; SVG markup on the clipboard pastes as a shape layer, #738), variable-font filter (26.3), the rebuilt Effect Controls (26.5). Percentage guides (View ▸ Add Percentage Guide, Convert Guides to Percentage / Pixels) and effect colour labels (Effect Controls header bar and right-click Label menu, `effect.setLabel`) are in (#1095), built from the release notes' one-line descriptions and never compared with After Effects: the Timeline's effect rows wear the label as a colour bar on the left edge (#1509), but After Effects' exact UI there is unknown | Users of the current version expect them | 10–20 h left | — · [ui-parity.md](ui-parity.md) | | 22 | **Encoder efficiency** | HEVC / AV1 have no B-frames, multi-reference or SAO / CDEF; Opus no FEC / DTX; single-file intermediates are 8-bit uncompressed QuickTime only (#372 closed by #778: no 16-bpc, AVI or lossless-compressed codec); #325 render over existing file | Larger files than After Effects + Media Encoder | 20–40 h | G6 · [file-format-parity.md](file-format-parity.md) | | 23 | **Essential Graphics interchange** | Templates are `.ectemplate`, not `.mogrt`; #338 and #611 closed | Premiere editors can't use our templates | 15–30 h | — · [file-format-parity.md](file-format-parity.md) | | 24 | **Timeline depth requests** | #601 expand precomps in place, #602 reorderable render stack, #604 guide-only effects, #425 keyframe confusion (#355 and #428 closed) | Power-user speed | 15–30 h | G2 · [ui-parity.md](ui-parity.md) | @@ -117,7 +117,8 @@ The most important missing piece: it turns every other estimate here into a meas - Regression evidence for [#66](https://github.com/storytold/effectcraft/issues/66): scripted pointer tests in `ui_viewer` and `ui_text_edit`, checked headlessly on Linux, cover viewer clicks, marquee selection and text picking when layers are hidden or excluded by solo. They also check - hidden/locked solo layers and solo layers outside their active time range. + hidden/locked solo layers, and that solo still excludes the other layers while the soloed + layer is outside its active time range (as in After Effects, so nothing is picked there). - Regression evidence for [#67](https://github.com/storytold/effectcraft/issues/67): scripted input tests in `ui_project_delete`, checked headlessly on Linux, cover Delete/Backspace on Project items, multiple selection and undo. They check that an unrelated Timeline layer stays, diff --git a/docs/testing.md b/docs/testing.md index fca9762e..9017e0c2 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -70,3 +70,9 @@ date, load average, scenario, settings, metrics); `--compare OLD.jsonl` prints t the latest matching runs there, `--compare OLD --with NEW` compares two files without rendering. The table notes the 1-minute load average: on a busy machine wall time is noise, so compare peak memory and instructions retired, and rerun when the load differs a lot. + +Results files allow blank lines; corrupt records or invalid comparison fields report the file +and line instead of being discarded. `--fail-over PCT` requires the same run keys on both sides, +successful latest runs, and at least one comparable resource metric per run. Failed latest runs, +missing coverage, empty results and wall-time-only measurements cannot pass the gate. Omit +`--fail-over` for an exploratory partial comparison; failed latest runs remain visible. diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 1a201232..5173e769 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -17,12 +17,12 @@ feel and platform differences, and the 26.x interface changes are not in yet. |---|---|---:|---| | Menu bar | After Effects' menus, order, submenus and labels; native on macOS; keyboard-operable in-window bar | 95% | 587 entries in `crates/engine/src/menus.rs`; Adobe-service entries omitted by design; `ui_menus` tests (#279) | | Panels and workspaces | Every After Effects panel; all standard workspaces; drag-to-dock, tabs, floating panels, maximise | 75% | `ui_tabs`, `ui_panels`; panels move into their own native windows on any screen (#486 open: no dragging tabs between those windows); #302, #301 and #320 (docking, stacked-panel headers, workspace persistence) are closed | -| Timeline | Switches, modes, parent & link, twirl-downs, reveal shortcuts (U, UU, P, S, R, T, A, E, M, F, Alt+Shift…), pick whips, graph editor, work area, markers | 75% | `ui_keyframes`, `ui_timeline_*`; expand precomps in place (#601); #456, #457, #569 and #355 (Graph Editor jitter and buttons, negative rotation, bulk keyframing) are closed; Proportional Scrubbing (26.2: Ctrl+Alt / Cmd+Opt scrub of hot text spreads the change over the selected layers by selection order, #770) matched for layers selected as a whole, not yet for individually selected properties | +| Timeline | Switches, modes, parent & link, twirl-downs, reveal shortcuts (U, UU, P, S, R, T, A, E, M, F, Alt+Shift…), pick whips, graph editor, work area, markers | 75% | `ui_keyframes`, `ui_timeline_*`; expand precomps in place (#601); #456, #457, #569 and #355 (Graph Editor jitter and buttons, negative rotation, bulk keyframing) are closed; Proportional Scrubbing (26.2: Ctrl+Alt / Cmd+Opt scrub of hot text spreads the change over the selected layers by selection order, #770) matched for layers selected as a whole, not yet for individually selected properties; every layer has the Video switch, nulls, cameras and lights included (a null with its eye off has no box or handles in the viewer and isn't picked there) | | Composition viewer | Selection, handles with direction-aware cursors, rulers, guides, grids, snapping, channels, exposure, region of interest, snapshots, 3D views, Spacebar hand | 70% | `ui_viewer`, `ui_viewers`; Cmd/Ctrl +/− doesn't zoom (#570); anchor point grab on Linux (#492); #352 (snapping) is closed; no 3D transform gizmos (#392, #347); percentage guides (26.5) exist as View ▸ Add Percentage Guide and Convert Guides to Percentage / Pixels (a guide stores a percent of the comp, so it follows comp-size changes; behaviour inferred from the release note, not compared with After Effects); Copy Frame to Clipboard (26.3) is in the Composition menu and the viewer's context menu | | Tools panel | All After Effects tools; long-press flyouts; Tool Creates Shape / Mask; Fill / Stroke options | 80% | #213, #227 fixed; #553 (pen and vertex tools on motion paths) is closed | | Nudging and modifiers | Arrow nudge (1 px, Shift 10 px), Alt/Option duplicates and slips, Shift constrains | 75% | #290 nudging fixed; #491 (Linux multi-layer edits) is closed | | Numeric entry | Scrubbable values, typed entry with Tab / Shift+Tab, angle revolutions, Escape cancels, non-finite rejected | 85% | `ui_angles`, `ui_inline_edit` (#93, #141, #361); rename arrows on Linux (#488) | -| Effect Controls | After Effects' twirl-downs, angle dial, point crosshair, eyedropper, curves/levels editors, effect menu on right-click | 70% | `ui_effect_controls`, `ui_fx_editors`; 26.5 rebuilt this panel with icon controls and effect colour labels: labels exist (a colour bar on the effect header, right-click Label menu, `effect.setLabel`; not in the Timeline), the rebuilt controls are not matched | +| Effect Controls | After Effects' twirl-downs, angle dial, point crosshair, eyedropper, curves/levels editors, effect menu on right-click | 70% | `ui_effect_controls`, `ui_fx_editors`; 26.5 rebuilt this panel with icon controls and effect colour labels: labels exist (a colour bar on the effect header and on the effect's Timeline row, right-click Label menu, `effect.setLabel`), the rebuilt controls are not matched | | Context menus | Layer, property, Project, Timeline empty area, work area, labels with colours | 80% | `ui_blank_context_menus`, `ui_project_rows`; #227, #290, #407 fixed; a comp's Project row menu has Composition Settings… (#1058); layer context menu has Layer Styles (#1611) | | Keyboard shortcuts | After Effects' defaults; Keyboard Shortcuts editor that rebinds and saves sets | 75% | Shortcut editor in Settings; #467 (adding shortcuts on Linux), #428 and #336 are closed; no shortcuts for scripts | | Drag and drop | Project → Timeline / viewer / New Comp / Render Queue; effects onto layers; files from the OS | 65% | `ui_drag_drop`; OS drops fail on Wayland (winit) and some Linux setups (#268); #552, #351 (multi-file drops) and #511 (crash dragging over the import dialog) are closed | diff --git a/docs/web.md b/docs/web.md index e73f7a89..23ee1da1 100644 --- a/docs/web.md +++ b/docs/web.md @@ -413,7 +413,8 @@ no npm packages): load, the GPU path (WebGPU → GPU compositor, viewer on the G in the viewer, `render.frame`, Render Queue GIF and PNG sequence (`.zip`) downloads, a background render in a worker while measuring the page's event-loop gaps (must stay under 400 ms) and its progress, the AudioContext starting on a (simulated) key press and a tone playing through it -with the meters moving, Save As and re-opening the saved project, persistence across a reload +with the meters moving, Save As and re-opening the saved project, a script opening a file from +browser storage and the footage check finding stored media, persistence across a reload (session snapshot, a stored project, imported media, a setting, Open Recent) and an offline reload through the service worker. It writes screenshots and `report.json`: diff --git a/packaging/linux/ai.storyteller.effectcraft.metainfo.xml.in b/packaging/linux/ai.storyteller.effectcraft.metainfo.xml.in index d8377c4c..50408021 100644 --- a/packaging/linux/ai.storyteller.effectcraft.metainfo.xml.in +++ b/packaging/linux/ai.storyteller.effectcraft.metainfo.xml.in @@ -37,8 +37,8 @@ - Editing The Great Wave off Kanagawa with type layers and adjustment layers - https://raw.githubusercontent.com/storytold/effectcraft/main/docs/images/effectcraft-demo.jpg + Demo composition with text layers, timeline and properties + https://raw.githubusercontent.com/storytold/effectcraft/05955c08b4fd025f1a09882df72e6dbff1a8148a/docs/images/effectcraft-hero.png diff --git a/packaging/linux/nfpm.yaml b/packaging/linux/nfpm.yaml index 76d41450..ec961215 100644 --- a/packaging/linux/nfpm.yaml +++ b/packaging/linux/nfpm.yaml @@ -44,10 +44,17 @@ overrides: - mesa-vulkan-drivers - xdg-desktop-portal rpm: + # Library sonames rather than package names, so the same requirements resolve on Fedora/RHEL + # and openSUSE (which has no libxkbcommon, vulkan-loader or libglvnd-egl package). The glibc + # floor names libm because the binaries' GLIBC_2.35 symbols (hypot, hypotf) are libm's: RHEL 9's + # glibc 2.34 provides libc.so.6(GLIBC_2.35) through backports, but not this, so it refuses. depends: - - libxkbcommon - - (vulkan-loader or libglvnd-egl) - - alsa-lib + - libc.so.6()(64bit) + - libm.so.6(GLIBC_2.35)(64bit) + - libgcc_s.so.1()(64bit) + - libxkbcommon.so.0()(64bit) + - (libvulkan.so.1()(64bit) or libEGL.so.1()(64bit)) + - libasound.so.2()(64bit) recommends: - libwayland-client - libX11 diff --git a/packaging/macos/package.sh b/packaging/macos/package.sh index 607290bb..e6427d02 100755 --- a/packaging/macos/package.sh +++ b/packaging/macos/package.sh @@ -138,15 +138,40 @@ resvg --skip-system-fonts -w 1320 "$HERE/dmg/background.svg" "$WORK/background@2 sips -s dpiWidth 144 -s dpiHeight 144 "$WORK/background@2x.png" >/dev/null tiffutil -cathidpicheck "$WORK/background.png" "$WORK/background@2x.png" -out "$STAGE/.background/background.tiff" cp "$HERE/dmg/dmg-layout.DS_Store" "$STAGE/.DS_Store" -rm -f "$DMG" "$WORK/raw.dmg" -# makehybrid + convert builds the image without attaching a device, unlike `create -srcfolder`, -# which is flaky on CI runners ("Resource busy") and hangs in sandboxed sessions. -# The volume name has no version: .DS_Store finds the background through an alias that includes it. -hdiutil makehybrid -hfs -hfs-volume-name "EffectCraft" -hfs-openfolder "$STAGE" -o "$WORK/raw.dmg" "$STAGE" -hdiutil convert "$WORK/raw.dmg" -format UDZO -imagekey zlib-level=9 -o "$DMG" -rm -f "$WORK/raw.dmg" +rm -f "$DMG" +# `hdiutil create -srcfolder` writes the files as they are. `makehybrid -hfs` gave every file a +# com.apple.FinderInfo, which `codesign --verify --strict` rejects as "Finder information, or +# similar detritus" on the app in the image and on a copy installed from it (as VectorCraft's +# storytold/vectorcraft#1085 and PdfCraft's storytold/pdfcraft#823). `create` attaches a device +# while it copies, which fails now and then on CI runners ("Resource busy"), so it gets a few +# tries. The volume name has no version: .DS_Store finds the background through an alias that +# includes it. +# Usage: hdiutil_retry ; tries five times with growing pauses. +hdiutil_retry() { + local what="$1" attempt + shift + for attempt in 1 2 3 4 5; do + if hdiutil "$@"; then + return 0 + fi + [ "$attempt" = 5 ] && { echo "hdiutil $what failed 5 times" >&2; return 1; } + echo "hdiutil $what failed (attempt $attempt of 5); retrying in $((attempt * 10))s" >&2 + sleep $((attempt * 10)) + done +} +hdiutil_retry create create -srcfolder "$STAGE" -volname "EffectCraft" -fs HFS+ -format UDZO -imagekey zlib-level=9 -ov "$DMG" sign "$DMG" codesign --verify --strict --verbose=2 "$DMG" +# The app inside the image must still pass --strict, so a makehybrid-style regression can't ship. +MOUNT="$WORK/dmg-check" +rm -rf "$MOUNT" +mkdir -p "$MOUNT" +hdiutil_retry attach attach -nobrowse -readonly -mountpoint "$MOUNT" "$DMG" >/dev/null +if ! codesign --verify --strict --deep --verbose=2 "$MOUNT/EffectCraft.app"; then + hdiutil_retry detach detach "$MOUNT" >/dev/null || true + exit 1 +fi +hdiutil_retry detach detach "$MOUNT" >/dev/null if [ "$NOTARIZE" = 1 ]; then notarize "$DMG" xcrun stapler staple "$DMG" diff --git a/xtask/src/bench.rs b/xtask/src/bench.rs index ef7a470a..fdfc3443 100644 --- a/xtask/src/bench.rs +++ b/xtask/src/bench.rs @@ -32,7 +32,8 @@ Run it under `nice -n 10` on a busy machine; the table notes the load average of --with NEW.jsonl with --compare: compare two result files, without rendering --fail-over PCT with --compare: exit 1 when a matched run got more than PCT % worse in CPU time, peak memory, max RSS or instructions retired (wall time is - left out: it follows the machine's load) + left out: it follows the machine's load). Requires matching successful + latest runs and comparable resource measurements for every key Scenarios: demo (the built-in demo comp), layers (1080p, 100 solid/text/shape layers with Gaussian Blur, Glow, Levels, Curves, Drop Shadow and keyframes), precomps (static and animated precomps @@ -744,7 +745,49 @@ fn append_line(path: &Path, v: &Value) -> Result<(), String> { fn read_jsonl(path: &Path) -> Result, String> { let text = std::fs::read_to_string(path).map_err(|e| format!("{}: {e}", path.display()))?; - Ok(text.lines().filter(|l| !l.trim().is_empty()).filter_map(|l| serde_json::from_str(l).ok()).collect()) + text.lines() + .enumerate() + .filter(|(_, line)| !line.trim().is_empty()) + .map(|(index, line)| { + let context = |error| format!("{}:{}: {error}", path.display(), index.saturating_add(1)); + let row: Value = serde_json::from_str(line).map_err(|error| context(format!("invalid JSON: {error}")))?; + validate_row(&row).map_err(context)?; + Ok(row) + }) + .collect() +} + +/// Validate comparison keys and measurements; unavailable platform metrics may be null/absent. +fn validate_row(row: &Value) -> Result<(), String> { + for (field, value) in [ + ("scenario", &row["scenario"]), + ("run", &row["run"]), + ("settings.resolution", &row["settings"]["resolution"]), + ("settings.format", &row["settings"]["format"]), + ] { + if value.as_str().is_none_or(|text| text.trim().is_empty()) { + return Err(format!("`{field}` must be a nonempty string")); + } + } + if row["settings"]["frames"].as_u64().is_none() { + return Err("`settings.frames` must be a nonnegative integer".into()); + } + if row["ok"].as_bool().is_none() { + return Err("`ok` must be a boolean".into()); + } + if !row["metrics"].is_object() { + return Err("`metrics` must be an object".into()); + } + if row["metrics"]["wall_s"].is_null() { + return Err("`metrics.wall_s` is required".into()); + } + for (field, _) in COMPARED { + let value = &row["metrics"][*field]; + if !value.is_null() && value.as_f64().is_none_or(|number| !number.is_finite() || number < 0.0) { + return Err(format!("`metrics.{field}` must be a finite nonnegative number or null")); + } + } + Ok(()) } // ---------------------------------------------------------------------------------------- output @@ -816,10 +859,10 @@ fn key_of(v: &Value) -> String { ) } -/// The last successful result per key (a results file accumulates runs; the latest one counts). +/// The latest result per key, including failures that supersede older successful runs. fn latest_per_key(rows: &[Value]) -> BTreeMap { let mut map = BTreeMap::new(); - for r in rows.iter().filter(|r| r["ok"] == true) { + for r in rows { map.insert(key_of(r), r.clone()); } map @@ -857,6 +900,31 @@ fn regressions(old: &BTreeMap, new: &BTreeMap, lim fn check_regressions(old: &BTreeMap, new: &BTreeMap, limit: Option) -> Result<(), String> { let Some(limit) = limit else { return Ok(()) }; + if old.is_empty() || new.is_empty() { + return Err("insufficient benchmark evidence: baseline and candidate must contain runs".into()); + } + let mut missing = vec![]; + for (key, baseline) in old { + match new.get(key) { + None => missing.push(format!("missing candidate run: {key}")), + Some(candidate) => { + for (label, row) in [("baseline", baseline), ("candidate", candidate)] { + if row["ok"] != true { + missing.push(format!("failed latest {label} run: {key}")); + } + } + if !GATED.iter().any(|field| pct(baseline["metrics"][*field].as_f64(), candidate["metrics"][*field].as_f64()).is_some()) { + missing.push(format!("no comparable resource measurements: {key}")); + } + } + } + } + for key in new.keys().filter(|key| !old.contains_key(*key)) { + missing.push(format!("missing baseline run: {key}")); + } + if !missing.is_empty() { + return Err(format!("insufficient benchmark evidence:\n {}", missing.join("\n "))); + } let bad = regressions(old, new, limit); if bad.is_empty() { println!("no run got more than {limit}% worse"); @@ -883,6 +951,10 @@ fn compare_table(old: &BTreeMap, new: &BTreeMap) - n["run"].as_str().unwrap_or(""), n["settings"]["frames"].as_u64().unwrap_or(0) )); + if o["ok"] != true || n["ok"] != true { + s.push_str(&format!(" (failed latest {} run)\n", if n["ok"] != true { "candidate" } else { "baseline" })); + continue; + } for (field, _) in COMPARED { s.push_str(&format!(" {:>8}", fmt_pct(pct(o["metrics"][field].as_f64(), n["metrics"][field].as_f64())))); } @@ -963,11 +1035,11 @@ mod tests { assert_eq!(fmt_pct(Some(25.0)), "+25.0%"); assert_eq!(fmt_pct(None), "-"); - // The latest successful run per key is the baseline; failed runs don't count. + // A successful run supersedes an earlier failure for the same key. let old = latest_per_key(&[ + row("layers", "full", 1.0, None, false), row("layers", "full", 20.0, Some(1000), true), row("layers", "full", 10.0, Some(2000), true), - row("layers", "full", 1.0, None, false), ]); assert_eq!(old.len(), 1); let new = latest_per_key(&[row("layers", "full", 8.0, Some(1500), true), row("demo", "half", 1.0, None, true)]); diff --git a/xtask/tests/bench.rs b/xtask/tests/bench.rs new file mode 100644 index 00000000..9f4a1b2f --- /dev/null +++ b/xtask/tests/bench.rs @@ -0,0 +1,134 @@ +//! Regression gates must fail when benchmark evidence is corrupt or incomplete. +use serde_json::{Value, json}; +use std::path::PathBuf; +use std::process::{Command, Output}; + +struct ResultsDir(PathBuf); +impl ResultsDir { + fn new() -> Self { + use std::sync::atomic::{AtomicU64, Ordering}; + static NEXT: AtomicU64 = AtomicU64::new(0); + let dir = std::env::temp_dir().join(format!("effectcraft-bench-{}-{}", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed))); + std::fs::create_dir_all(&dir).unwrap(); + Self(dir) + } + fn compare(&self, old: &str, new: &str, gate: bool) -> Output { + let baseline = self.0.join("baseline.jsonl"); + let candidate = self.0.join("candidate.jsonl"); + std::fs::write(&baseline, old).unwrap(); + std::fs::write(&candidate, new).unwrap(); + let mut command = Command::new(env!("CARGO_BIN_EXE_xtask")); + command.args(["bench", "--compare"]).arg(baseline).arg("--with").arg(candidate); + if gate { + command.args(["--fail-over", "5"]); + } + command.output().unwrap() + } +} +impl Drop for ResultsDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +fn row(scenario: &str, ok: bool) -> Value { + json!({"scenario": scenario, "run": "full", "ok": ok, + "settings": {"resolution": "full", "frames": 60, "format": "png"}, + "metrics": {"wall_s": 1.0, "cpu_s": 2.0, "peak_footprint_bytes": 1000, "max_rss_bytes": null, "instructions": null}}) +} +fn diagnostic(output: &Output) -> String { + format!("{}{}", String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr)) +} + +#[test] +fn corrupt_records_report_the_input_file_and_line() { + let files = ResultsDir::new(); + let valid = row("demo", true); + let mut missing = valid.clone(); + missing["settings"].as_object_mut().unwrap().remove("frames"); + let mut missing_wall = valid.clone(); + missing_wall["metrics"].as_object_mut().unwrap().remove("wall_s"); + let mut bad_metric = valid.clone(); + bad_metric["metrics"]["cpu_s"] = json!("not a measurement"); + let mut negative_metric = valid.clone(); + negative_metric["metrics"]["cpu_s"] = json!(-1); + for bad in [ + "{".into(), + "null".into(), + "[]".into(), + "{}".into(), + missing.to_string(), + missing_wall.to_string(), + bad_metric.to_string(), + negative_metric.to_string(), + ] { + let output = files.compare(&format!("{valid}\n"), &format!("{valid}\n\n \n{bad}\n"), true); + let message = diagnostic(&output); + assert!(!output.status.success(), "corrupt input passed: {bad}\n{message}"); + assert!(message.contains("candidate.jsonl:4:"), "{message}"); + } + let output = files.compare("{\n", &format!("{valid}\n"), false); + assert!(!output.status.success()); + assert!(diagnostic(&output).contains("baseline.jsonl:1:")); +} + +#[test] +fn gates_reject_failed_latest_runs_and_missing_coverage() { + let files = ResultsDir::new(); + let good = row("demo", true); + let failed = row("demo", false); + let other = row("layers", true); + for (old, new) in [ + (good.to_string(), String::new()), + (String::new(), String::new()), + (good.to_string(), failed.to_string()), + (good.to_string(), format!("{good}\n{failed}")), + (format!("{good}\n{failed}"), good.to_string()), + (good.to_string(), other.to_string()), + (format!("{good}\n{other}"), good.to_string()), + (good.to_string(), format!("{good}\n{other}")), + ] { + let output = files.compare(&old, &new, true); + assert!(!output.status.success(), "incomplete gate passed: {old} -> {new}\n{}", diagnostic(&output)); + } +} + +#[test] +fn gates_require_a_comparable_resource_metric_for_every_run() { + let files = ResultsDir::new(); + let good = row("demo", true); + let mut wall_only = good.clone(); + wall_only["metrics"]["cpu_s"] = Value::Null; + wall_only["metrics"]["peak_footprint_bytes"] = Value::Null; + let output = files.compare(&good.to_string(), &wall_only.to_string(), true); + assert!(!output.status.success(), "{}", diagnostic(&output)); + assert!(diagnostic(&output).contains("comparable")); + let mut zero = good.clone(); + zero["metrics"]["cpu_s"] = json!(0); + zero["metrics"]["peak_footprint_bytes"] = json!(0); + assert!(!files.compare(&zero.to_string(), &good.to_string(), true).status.success()); + let mut mixed = row("layers", true); + mixed["metrics"] = wall_only["metrics"].clone(); + assert!(!files.compare(&format!("{good}\n{mixed}"), &format!("{good}\n{mixed}"), true).status.success()); +} + +#[test] +fn valid_results_keep_comparison_and_exploratory_mode() { + let files = ResultsDir::new(); + let good = row("demo", true); + let failed = row("demo", false); + let output = files.compare(&format!("\n{failed}\n{good}\n"), &format!("{good}\n \n"), true); + assert!(output.status.success(), "{}", diagnostic(&output)); + assert!(diagnostic(&output).contains("+0.0%")); + let mut open = good.clone(); + open["run"] = json!("open"); + open["settings"]["resolution"] = json!("-"); + open["settings"]["frames"] = json!(0); + assert!(files.compare(&open.to_string(), &open.to_string(), true).status.success(), "project-open runs have zero rendered frames"); + let mut regression = good.clone(); + regression["metrics"]["cpu_s"] = json!(3); + assert!(!files.compare(&good.to_string(), ®ression.to_string(), true).status.success()); + let output = files.compare(&good.to_string(), &format!("{good}\n{failed}"), false); + assert!(output.status.success(), "exploratory comparison stays available"); + assert!(diagnostic(&output).contains("failed"), "latest failure is visible: {}", diagnostic(&output)); +}