From 9681c904fb38d7e6003f3c7d74e18ac37d2d14cf Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Wed, 12 Aug 2026 17:53:09 +0200 Subject: [PATCH] fix(loader): resolve relative asset paths against the scenario, not the CWD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "src": "assets/logo.png" resolved against the process working directory, so the same file rendered from its own folder and failed from anywhere else — the studio runs from the repository root, which is why a scenario authored beside its assets showed nothing there. `include` had always resolved relative to the including file. Two path-like fields in one document following two different rules is the trap, and it is the single cause behind a family of "component X does not render" reports: a hard error for image and gif, a warning for video, silence for an audio track feeding a waveform. Rewrite on the raw JSON before deserialisation, so no component changes: by the time an image or an audio track is constructed its src is already absolute. Applied in the JSON loader, the HTML loader, the CLI's validation pipeline, and per included file — an include's assets belong to the file that names them, not to the parent that pulled it in. 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. `track` is rewritten alongside `src`: the audio analysis is cached under the track's src and a waveform finds it by track, so rewriting one and not the other would make every lookup miss. --- .../rustmotion-cli/src/commands/validation.rs | 7 + crates/rustmotion/src/assets.rs | 186 ++++++++++++++++++ crates/rustmotion/src/include.rs | 9 + crates/rustmotion/src/lib.rs | 1 + crates/rustmotion/src/loader.rs | 11 ++ crates/rustmotion/src/tests.rs | 41 ++++ 6 files changed, 255 insertions(+) create mode 100644 crates/rustmotion/src/assets.rs diff --git a/crates/rustmotion-cli/src/commands/validation.rs b/crates/rustmotion-cli/src/commands/validation.rs index 9a7dafb5..96ff6b0f 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 00000000..1086fc89 --- /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 378d3e4d..32507cf8 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 395b89dc..d7aba3ca 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 b7aa5c56..724187d5 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 ec80f898..f2a9c025 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