diff --git a/crates/rustmotion-cli/src/commands/validation.rs b/crates/rustmotion-cli/src/commands/validation.rs index 9a7dafb..96ff6b0 100644 --- a/crates/rustmotion-cli/src/commands/validation.rs +++ b/crates/rustmotion-cli/src/commands/validation.rs @@ -188,6 +188,13 @@ pub fn load_with_vars( // renders. expand::expand_directives(&mut json_value, &label)?; + // Assets are relative to the scenario file, like `include` — and this must + // happen before `raw` is captured, so the existence check below and the + // renderer look at the same, already-resolved paths. + if let Some(dir) = source_path.as_ref().and_then(|p| p.parent()) { + rustmotion::assets::rebase_relative_paths(&mut json_value, dir); + } + let scenario: Scenario = serde_json::from_value(json_value.clone())?; let resolved = include::resolve_includes(scenario, &include_source)?; diff --git a/crates/rustmotion/src/assets.rs b/crates/rustmotion/src/assets.rs new file mode 100644 index 0000000..1086fc8 --- /dev/null +++ b/crates/rustmotion/src/assets.rs @@ -0,0 +1,186 @@ +//! Resolve relative asset paths against the scenario file that names them. +//! +//! `"src": "assets/logo.png"` used to resolve against the *process* working +//! directory, so the same scenario rendered from its own folder and failed from +//! anywhere else — including the studio, which runs from the repository root. +//! `include` had always resolved relative to the including file; two path-like +//! fields in one document following two different rules is the trap. +//! +//! The rewrite happens on the raw JSON, before deserialisation, so no component +//! needs to know about it: by the time an `image` or an `audio` track is +//! constructed its `src` is already absolute. + +use std::path::Path; + +use serde_json::Value; + +/// Keys whose string value names a file on disk. +/// +/// `src` covers `image`, `video`, `gif`, `avatar` (and each entry of an +/// `avatar_group`), `mockup`, `lottie` and `audio`; `track` is the audio-source +/// reference on `waveform`/`audio_spectrum` and in `style.audio-reactive`, +/// which must name the same string the audio track does or the analysis lookup +/// misses. +const PATH_KEYS: &[&str] = &["src", "track"]; + +fn is_remote(s: &str) -> bool { + s.starts_with("http://") || s.starts_with("https://") || s.starts_with("data:") +} + +/// Rewrite every relative asset path in `value` to an absolute one, resolved +/// against `base_dir`. +/// +/// Deliberately conservative: a path is rewritten **only** when the file exists +/// next to the scenario. Anything else is left exactly as written, so a path +/// that used to resolve against the working directory still does, and a genuine +/// typo still reaches the validator with the author's own spelling in the +/// message rather than a rewritten one they never typed. +pub fn rebase_relative_paths(value: &mut Value, base_dir: &Path) { + match value { + Value::Object(map) => { + for (key, child) in map.iter_mut() { + if PATH_KEYS.contains(&key.as_str()) { + if let Value::String(s) = child { + if let Some(abs) = rebased(s, base_dir) { + *s = abs; + continue; + } + } + } + rebase_relative_paths(child, base_dir); + } + } + Value::Array(items) => { + for item in items { + rebase_relative_paths(item, base_dir); + } + } + _ => {} + } +} + +/// `Some(absolute)` when `src` is relative and names an existing file under +/// `base_dir`; `None` when it must be left alone. +fn rebased(src: &str, base_dir: &Path) -> Option { + if src.is_empty() || is_remote(src) { + return None; + } + let path = Path::new(src); + if path.is_absolute() { + return None; + } + let candidate = base_dir.join(path); + if !candidate.is_file() { + return None; + } + // `canonicalize` resolves `..` and symlinks so two spellings of the same + // file share one cache key — the audio analysis and the GIF/image caches + // are keyed by this string. + let resolved = std::fs::canonicalize(&candidate).unwrap_or(candidate); + Some(resolved.to_str()?.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn scratch() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "rustmotion_assets_{}_{:?}", + std::process::id(), + std::thread::current().id() + )); + std::fs::create_dir_all(dir.join("assets")).expect("scratch dir"); + dir + } + + #[test] + fn a_relative_src_next_to_the_scenario_becomes_absolute() { + let dir = scratch(); + std::fs::write(dir.join("assets/logo.png"), b"x").expect("fixture"); + + let mut v = + json!({"scenes": [{"children": [{"type": "image", "src": "assets/logo.png"}]}]}); + rebase_relative_paths(&mut v, &dir); + + let got = v["scenes"][0]["children"][0]["src"].as_str().expect("src"); + assert!(Path::new(got).is_absolute(), "not rewritten: {got}"); + assert!(Path::new(got).is_file(), "rewritten to a non-file: {got}"); + std::fs::remove_dir_all(&dir).ok(); + } + + /// The whole point: the rewrite must not depend on where the process runs. + #[test] + fn the_result_does_not_depend_on_the_working_directory() { + let dir = scratch(); + std::fs::write(dir.join("assets/logo.png"), b"x").expect("fixture"); + + let mut a = json!({"src": "assets/logo.png"}); + let mut b = json!({"src": "assets/logo.png"}); + rebase_relative_paths(&mut a, &dir); + rebase_relative_paths(&mut b, &dir); + assert_eq!(a, b); + assert_ne!(a["src"], json!("assets/logo.png")); + std::fs::remove_dir_all(&dir).ok(); + } + + /// A path that does not exist beside the scenario keeps the author's own + /// spelling, so the validator's message names what they typed. + #[test] + fn a_missing_file_is_left_untouched() { + let dir = scratch(); + let mut v = json!({"src": "assets/absent.png"}); + rebase_relative_paths(&mut v, &dir); + assert_eq!(v["src"], json!("assets/absent.png")); + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn absolute_and_remote_sources_are_left_untouched() { + let dir = scratch(); + let mut v = json!({ + "a": {"src": "/etc/hosts"}, + "b": {"src": "https://example.com/x.png"}, + "c": {"src": "data:image/png;base64,AAAA"} + }); + let before = v.clone(); + rebase_relative_paths(&mut v, &dir); + assert_eq!(v, before); + std::fs::remove_dir_all(&dir).ok(); + } + + /// `track` must be rewritten the same way as `src`: the audio analysis is + /// cached under the track's `src`, and a `waveform` finds it by `track`. + /// Rewriting one and not the other would make every lookup miss. + #[test] + fn track_is_rebased_like_src_so_the_analysis_lookup_still_matches() { + let dir = scratch(); + std::fs::write(dir.join("assets/t.wav"), b"x").expect("fixture"); + + let mut v = json!({ + "audio": [{"src": "assets/t.wav"}], + "scenes": [{"children": [{"type": "waveform", "track": "assets/t.wav"}]}] + }); + rebase_relative_paths(&mut v, &dir); + + assert_eq!( + v["audio"][0]["src"], v["scenes"][0]["children"][0]["track"], + "src and track must resolve to the same string" + ); + std::fs::remove_dir_all(&dir).ok(); + } + + /// Keys that merely *contain* a path-like string are not touched — only the + /// documented asset fields are. + #[test] + fn unrelated_keys_are_not_rewritten() { + let dir = scratch(); + std::fs::write(dir.join("assets/logo.png"), b"x").expect("fixture"); + let mut v = json!({"content": "assets/logo.png", "title": "assets/logo.png"}); + let before = v.clone(); + rebase_relative_paths(&mut v, &dir); + assert_eq!(v, before); + std::fs::remove_dir_all(&dir).ok(); + } +} diff --git a/crates/rustmotion/src/include.rs b/crates/rustmotion/src/include.rs index 378d3e4..32507cf 100644 --- a/crates/rustmotion/src/include.rs +++ b/crates/rustmotion/src/include.rs @@ -148,6 +148,15 @@ fn fetch_and_resolve( directive.config.as_ref(), &directive.include, )?; + + // An included file's assets are relative to *that* file, not to the parent + // that pulled it in — otherwise moving an include would silently break + // every path inside it. + if let IncludeSource::File(ref p) = child_source { + if let Some(dir) = p.parent() { + crate::assets::rebase_relative_paths(&mut json_value, dir); + } + } // `components` (and any `for-each`/`use` inside this file's own scenes) // is scoped to this document: expanded here, per included file, using // ONLY this file's own `components` block — never the parent's, and diff --git a/crates/rustmotion/src/lib.rs b/crates/rustmotion/src/lib.rs index 395b89d..d7aba3c 100644 --- a/crates/rustmotion/src/lib.rs +++ b/crates/rustmotion/src/lib.rs @@ -18,6 +18,7 @@ pub mod engine { } // Local modules +pub mod assets; pub mod encode; pub mod include; pub mod loader; diff --git a/crates/rustmotion/src/loader.rs b/crates/rustmotion/src/loader.rs index b7aa5c5..724187d 100644 --- a/crates/rustmotion/src/loader.rs +++ b/crates/rustmotion/src/loader.rs @@ -24,6 +24,13 @@ pub fn load_scenario_with_vars( let label = input.display().to_string(); variables::apply_variables(&mut json_value, overrides, &label)?; expand::expand_directives(&mut json_value, &label)?; + // Asset paths are relative to the file that names them, like `include` — + // not to wherever the process happens to run. + if let Some(dir) = input.parent() { + { + crate::assets::rebase_relative_paths(&mut json_value, dir); + } + } let scenario: Scenario = serde_json::from_value(json_value).map_err(RustmotionError::from)?; include::resolve_includes(scenario, &include::IncludeSource::File(input.clone())) @@ -100,6 +107,10 @@ pub fn load_scenario_from_html_with_vars( let label = input.display().to_string(); variables::apply_variables(&mut value, overrides, &label)?; expand::expand_directives(&mut value, &label)?; + // Same rule as the JSON loader: assets are relative to the file naming them. + if let Some(dir) = input.parent() { + crate::assets::rebase_relative_paths(&mut value, dir); + } let scenario: Scenario = serde_json::from_value(value).map_err(RustmotionError::from)?; include::resolve_includes(scenario, &include::IncludeSource::File(input.clone())) } diff --git a/crates/rustmotion/src/tests.rs b/crates/rustmotion/src/tests.rs index ec80f89..f2a9c02 100644 --- a/crates/rustmotion/src/tests.rs +++ b/crates/rustmotion/src/tests.rs @@ -1984,6 +1984,47 @@ mod audio_tests { count } + /// The failure the whole rewrite exists for: a scenario naming an asset + /// beside itself must load identically whatever directory the process + /// runs from. Authoring from the scenario's folder worked; the studio, + /// which runs from the repository root, resolved nothing. + #[test] + fn a_relative_asset_resolves_against_the_scenario_not_the_cwd() { + let dir = std::env::temp_dir().join(format!("rustmotion_cwd_{}", nanos())); + std::fs::create_dir_all(dir.join("assets")).expect("scratch"); + std::fs::write( + dir.join("assets/t.wav"), + make_sine_wav(4410, 4410, 440.0, 44100), + ) + .expect("fixture"); + + let scenario_path = dir.join("scene.json"); + std::fs::write( + &scenario_path, + serde_json::json!({ + "video": {"width": 32, "height": 32, "fps": 30}, + "audio": [{"src": "assets/t.wav"}], + "scenes": [{"duration": 0.1, "children": []}] + }) + .to_string(), + ) + .expect("write scenario"); + + let loaded = crate::loader::load_scenario_with_vars(&scenario_path, None) + .expect("scenario must load"); + let src = &loaded.audio[0].src; + + assert!( + std::path::Path::new(src).is_absolute(), + "the asset path must not stay relative to the process: {src}" + ); + assert!( + std::path::Path::new(src).is_file(), + "and it must point at the file beside the scenario: {src}" + ); + std::fs::remove_dir_all(&dir).ok(); + } + /// A track placed at `start` must be *read* from `start` too. /// /// The mux places the file at `track.start` on the scenario timeline and