diff --git a/crates/rustmotion-core/src/engine/renderer/assets.rs b/crates/rustmotion-core/src/engine/renderer/assets.rs index 0cf2ee0..8270908 100644 --- a/crates/rustmotion-core/src/engine/renderer/assets.rs +++ b/crates/rustmotion-core/src/engine/renderer/assets.rs @@ -67,10 +67,89 @@ pub fn icon_cache_dir() -> PathBuf { base.join("rustmotion").join("icons") } -fn icon_cache_file(cache_dir: &Path, icon: &str, color: &str, width: u32, height: u32) -> PathBuf { - let slug = icon.replace(':', "_"); - let hex_color = color.trim_start_matches('#').to_lowercase(); - cache_dir.join(format!("{slug}-{hex_color}-{width}x{height}.svg")) +fn icon_source_cache_file(cache_dir: &Path, icon: &str) -> PathBuf { + cache_dir.join(format!("{}.svg", icon.replace(':', "_"))) +} + +fn set_root_attribute(svg: &str, attribute: &str, value: &str) -> String { + let Some(tag_start) = svg.find("') else { + return svg.to_string(); + }; + let tag = &svg[tag_start..tag_start + tag_len]; + + let needle = format!(" {attribute}=\""); + let replaced = match tag.find(&needle) { + Some(at) => { + let value_start = at + needle.len(); + match tag[value_start..].find('"') { + Some(value_len) => format!( + "{}{}\"{}", + &tag[..value_start], + value, + &tag[value_start + value_len..] + ), + None => tag.to_string(), + } + } + None => format!(" Vec { + let coloured = + String::from_utf8_lossy(source).replace("currentColor", &format!("#{hex_color}")); + let sized = set_root_attribute(&coloured, "width", &width.to_string()); + set_root_attribute(&sized, "height", &height.to_string()).into_bytes() +} + +const ICON_FETCH_ATTEMPTS: u32 = 4; + +fn status_is_worth_retrying(error: &ureq::Error) -> bool { + matches!(error, ureq::Error::StatusCode(code) if *code == 429 || *code >= 500) +} + +fn fetch_icon_source(icon: &str, prefix: &str, name: &str) -> Result> { + let url = format!("https://api.iconify.design/{prefix}/{name}.svg"); + let mut backoff = Duration::from_millis(250); + let mut last_reason = String::new(); + + for attempt in 0..ICON_FETCH_ATTEMPTS { + match http_agent().get(&url).call() { + Ok(response) => { + return response + .into_body() + .read_to_vec() + .map_err(|e| RustmotionError::IconFetch { + icon: icon.to_string(), + reason: e.to_string(), + }) + } + Err(e) => { + last_reason = e.to_string(); + let is_last_attempt = attempt + 1 == ICON_FETCH_ATTEMPTS; + if !status_is_worth_retrying(&e) || is_last_attempt { + break; + } + std::thread::sleep(backoff); + backoff *= 2; + } + } + } + + Err(RustmotionError::IconFetch { + icon: icon.to_string(), + reason: last_reason, + }) } pub fn fetch_icon_svg(icon: &str, color: &str, width: u32, height: u32) -> Result> { @@ -93,37 +172,21 @@ pub fn fetch_icon_svg_in( let width = width.max(1); let height = height.max(1); - let cache_file = icon_cache_file(cache_dir, icon, color, width, height); - if let Ok(data) = std::fs::read(&cache_file) { - if !data.is_empty() { - return Ok(data); + let hex_color = hex_color.to_lowercase(); + let source_file = icon_source_cache_file(cache_dir, icon); + + let source = match std::fs::read(&source_file) { + Ok(data) if !data.is_empty() => data, + _ => { + let fetched = fetch_icon_source(icon, prefix, name)?; + if std::fs::create_dir_all(cache_dir).is_ok() { + let _ = std::fs::write(&source_file, &fetched); + } + fetched } - } - - let url = format!( - "https://api.iconify.design/{}/{}.svg?color=%23{}&width={}&height={}", - prefix, name, hex_color, width, height - ); - let response = http_agent() - .get(&url) - .call() - .map_err(|e| RustmotionError::IconFetch { - icon: icon.to_string(), - reason: e.to_string(), - })?; - let body = response - .into_body() - .read_to_vec() - .map_err(|e| RustmotionError::IconFetch { - icon: icon.to_string(), - reason: e.to_string(), - })?; - - if std::fs::create_dir_all(cache_dir).is_ok() { - let _ = std::fs::write(&cache_file, &body); - } + }; - Ok(body) + Ok(recolour_and_resize(&source, &hex_color, width, height)) } static VIDEO_FRAME_CACHE: OnceLock = OnceLock::new(); @@ -419,21 +482,34 @@ mod tests { let (w, h) = (48, 48); let svg_bytes = b"fake cached icon for the test suite".to_vec(); - let cache_file = icon_cache_file(&cache_dir, icon, color, w, h); + let cache_file = icon_source_cache_file(&cache_dir, icon); + std::fs::create_dir_all(&cache_dir).unwrap(); std::fs::write(&cache_file, &svg_bytes).unwrap(); let result = fetch_icon_svg_in(icon, color, w, h, &cache_dir).expect("cache hit"); - assert_eq!(result, svg_bytes); + let result = String::from_utf8(result).unwrap(); + assert!( + result.contains("fake cached icon for the test suite"), + "the cached source must be what is served — the network was never reached: {result}" + ); + assert!( + result.contains(r#"width="48""#), + "and it is resized on the way out, which is what lets one cached source serve \ + every colour and size: {result}" + ); } #[test] - fn disk_cache_is_keyed_by_icon_color_and_size() { + fn disk_cache_is_keyed_by_the_icon_alone() { let cache_dir = unique_temp_dir("cache-keying"); - let a = icon_cache_file(&cache_dir, "lucide:home", "#FFFFFF", 80, 80); - let b = icon_cache_file(&cache_dir, "lucide:home", "#000000", 80, 80); - let c = icon_cache_file(&cache_dir, "lucide:home", "#FFFFFF", 40, 40); - assert_ne!(a, b, "different colors must not share a cache file"); - assert_ne!(a, c, "different sizes must not share a cache file"); + assert_eq!( + icon_source_cache_file(&cache_dir, "lucide:home"), + icon_source_cache_file(&cache_dir, "lucide:home"), + ); + assert_ne!( + icon_source_cache_file(&cache_dir, "lucide:home"), + icon_source_cache_file(&cache_dir, "lucide:check"), + ); } #[test] @@ -457,7 +533,7 @@ mod tests { let first = fetch_icon_svg_in(icon, color, w, h, &cache_dir).expect("live fetch"); assert!(!first.is_empty()); - let cache_file = icon_cache_file(&cache_dir, icon, color, w, h); + let cache_file = icon_source_cache_file(&cache_dir, icon); assert!( cache_file.exists(), "a successful live fetch must be persisted to disk" @@ -647,3 +723,78 @@ mod tests { let _ = ffprobe_available(); } } + +#[cfg(test)] +mod icon_source_cache_tests { + use super::*; + + const LUCIDE_CHECK: &str = r#""#; + + #[test] + fn the_disk_entry_is_named_by_the_icon_alone() { + let dir = Path::new("/tmp/icons"); + assert_eq!( + icon_source_cache_file(dir, "lucide:check"), + dir.join("lucide_check.svg"), + "one entry per icon: naming it by colour and size is what filled a cache with 20 \ + copies of the same glyph and made a colour change hit the network" + ); + } + + #[test] + fn recolouring_substitutes_current_color_everywhere() { + let out = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "2a2f6b", 84, 84); + let out = String::from_utf8(out).unwrap(); + assert!(!out.contains("currentColor")); + assert!(out.contains(r##"stroke="#2a2f6b""##), "got {out}"); + } + + #[test] + fn resizing_rewrites_the_root_size_and_leaves_the_viewbox_alone() { + let out = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "ffffff", 84, 96); + let out = String::from_utf8(out).unwrap(); + assert!(out.contains(r#"width="84""#), "got {out}"); + assert!(out.contains(r#"height="96""#), "got {out}"); + assert!( + out.contains(r#"viewBox="0 0 24 24""#), + "the viewBox is the glyph's own coordinate space and must survive a resize: {out}" + ); + } + + #[test] + fn a_root_without_a_size_gains_one() { + let bare = r#""#; + let out = + String::from_utf8(recolour_and_resize(bare.as_bytes(), "000000", 32, 32)).unwrap(); + assert!( + out.contains(r#"width="32""#) && out.contains(r#"height="32""#), + "got {out}" + ); + } + + #[test] + fn two_colours_of_one_icon_share_a_single_source_entry() { + let dir = Path::new("/tmp/icons"); + assert_eq!( + icon_source_cache_file(dir, "lucide:sparkles"), + icon_source_cache_file(dir, "lucide:sparkles") + ); + let red = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "ff0000", 24, 24); + let blue = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "0000ff", 24, 24); + assert_ne!( + red, blue, + "the same source still produces different renders" + ); + } + + #[test] + fn only_rate_limits_and_server_faults_are_retried() { + assert!(status_is_worth_retrying(&ureq::Error::StatusCode(429))); + assert!(status_is_worth_retrying(&ureq::Error::StatusCode(503))); + assert!( + !status_is_worth_retrying(&ureq::Error::StatusCode(404)), + "a missing icon is not going to appear on the fourth try — retrying it would just \ + make a typo take four times as long to report" + ); + } +} diff --git a/crates/rustmotion-core/src/error.rs b/crates/rustmotion-core/src/error.rs index abcb740..6e52aed 100644 --- a/crates/rustmotion-core/src/error.rs +++ b/crates/rustmotion-core/src/error.rs @@ -49,6 +49,17 @@ pub enum RustmotionError { #[error("Failed to parse icon SVG '{icon}': {reason}")] IconParse { icon: String, reason: String }, + #[error( + "{count} icon(s) could not be loaded — checked the disk cache at {cache_dir} and the \ + network, both failed:\n - {details}\nA render must not silently omit an icon: fix \ + the identifier(s), or connect once so they are downloaded and cached for offline use." + )] + IconsUnresolved { + count: usize, + cache_dir: String, + details: String, + }, + #[error("Failed to create Skia image from {target}")] SkiaImageCreation { target: String }, diff --git a/crates/rustmotion/src/cli/commands/sheet.rs b/crates/rustmotion/src/cli/commands/sheet.rs index c674641..2fc7184 100644 --- a/crates/rustmotion/src/cli/commands/sheet.rs +++ b/crates/rustmotion/src/cli/commands/sheet.rs @@ -228,6 +228,10 @@ pub fn cmd_sheet( engine::renderer::load_custom_fonts(&scenario.fonts); } + for view in &scenario.views { + engine::prefetch_icons(&view.scenes)?; + } + let start_time = std::time::Instant::now(); let config = &scenario.video; let fps = config.fps; diff --git a/crates/rustmotion/src/cli/commands/still.rs b/crates/rustmotion/src/cli/commands/still.rs index ee5c52e..7c9da76 100644 --- a/crates/rustmotion/src/cli/commands/still.rs +++ b/crates/rustmotion/src/cli/commands/still.rs @@ -43,6 +43,10 @@ pub fn cmd_still( engine::renderer::load_custom_fonts(&scenario.fonts); } + for view in &scenario.views { + engine::prefetch_icons(&view.scenes)?; + } + let config = &scenario.video; let fps = config.fps; diff --git a/crates/rustmotion/src/encode/video/ffmpeg.rs b/crates/rustmotion/src/encode/video/ffmpeg.rs index d8facdb..be6863a 100644 --- a/crates/rustmotion/src/encode/video/ffmpeg.rs +++ b/crates/rustmotion/src/encode/video/ffmpeg.rs @@ -313,7 +313,7 @@ fn encode_with_ffmpeg_hw_impl( let fps = config.fps; for view in &scenario.views { - prefetch_icons(&view.scenes); + prefetch_icons(&view.scenes)?; } for failure in analyze_scenario_audio(scenario) { eprintln!("rustmotion: audio-reactive: {failure} — waveform/audio_spectrum will render flat for this track."); diff --git a/crates/rustmotion/src/encode/video/formats.rs b/crates/rustmotion/src/encode/video/formats.rs index 55275ba..4d7765a 100644 --- a/crates/rustmotion/src/encode/video/formats.rs +++ b/crates/rustmotion/src/encode/video/formats.rs @@ -53,7 +53,7 @@ fn encode_png_sequence_to_dir( let height = config.height; for view in &scenario.views { - prefetch_icons(&view.scenes); + prefetch_icons(&view.scenes)?; } let tasks = build_frame_tasks(scenario); @@ -146,7 +146,7 @@ fn encode_gif_to_path( let fps = config.fps; for view in &scenario.views { - prefetch_icons(&view.scenes); + prefetch_icons(&view.scenes)?; } let tasks = build_frame_tasks(scenario); @@ -232,7 +232,7 @@ fn encode_raw_frames(scenario: &Scenario, quiet: bool, writer: &mut dyn Write) - let config = &scenario.video; for view in &scenario.views { - prefetch_icons(&view.scenes); + prefetch_icons(&view.scenes)?; } let tasks = build_frame_tasks(scenario); diff --git a/crates/rustmotion/src/encode/video/h264.rs b/crates/rustmotion/src/encode/video/h264.rs index bb05aef..0e0ccdf 100644 --- a/crates/rustmotion/src/encode/video/h264.rs +++ b/crates/rustmotion/src/encode/video/h264.rs @@ -56,7 +56,7 @@ fn encode_video_impl( for view in &scenario.views { preextract_video_frames(&view.scenes, fps); - prefetch_icons(&view.scenes); + prefetch_icons(&view.scenes)?; } for failure in analyze_scenario_audio(scenario) { eprintln!("rustmotion: audio-reactive: {failure} — waveform/audio_spectrum will render flat for this track."); @@ -158,7 +158,7 @@ pub fn encode_video_incremental( for view in &scenario.views { preextract_video_frames(&view.scenes, fps); - prefetch_icons(&view.scenes); + prefetch_icons(&view.scenes)?; } for failure in analyze_scenario_audio(scenario) { eprintln!("rustmotion: audio-reactive: {failure} — waveform/audio_spectrum will render flat for this track."); diff --git a/crates/rustmotion/src/engine/preload.rs b/crates/rustmotion/src/engine/preload.rs index 8d2af53..9ee24fc 100644 --- a/crates/rustmotion/src/engine/preload.rs +++ b/crates/rustmotion/src/engine/preload.rs @@ -34,7 +34,7 @@ fn video_frame_cache_bytes() -> u64 { .sum() } -pub fn prefetch_icons(scenes: &[Scene]) { +pub fn prefetch_icons(scenes: &[Scene]) -> crate::error::Result<()> { use std::collections::HashSet; let mut seen = HashSet::new(); @@ -128,16 +128,14 @@ pub fn prefetch_icons(scenes: &[Scene]) { } if !unresolved.is_empty() { - panic!( - "rustmotion: {} icon(s) could not be preloaded — checked the disk cache at \ - {} and the network, both failed:\n - {}\n\ - A render must not silently omit an icon: fix the identifier(s), or connect to \ - the network so they can be downloaded once and cached for offline use.", - unresolved.len(), - icon_cache_dir().display(), - unresolved.join("\n - ") - ); + return Err(crate::error::RustmotionError::IconsUnresolved { + count: unresolved.len(), + cache_dir: icon_cache_dir().display().to_string(), + details: unresolved.join("\n - "), + }); } + + Ok(()) } pub fn preextract_video_frames(scenes: &[Scene], fps: u32) { @@ -358,14 +356,14 @@ mod tests { })) .expect("scene must deserialize"); - let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { - prefetch_icons(std::slice::from_ref(&scene)); - })); + let result = prefetch_icons(std::slice::from_ref(&scene)); assert!( result.is_err(), - "prefetch_icons must panic (or otherwise hard-fail) when an icon cannot be \ - resolved via disk cache or network, instead of silently continuing" + "prefetch_icons must hard-fail when an icon cannot be resolved via disk cache or \ + network, instead of silently continuing. It reports a named error rather than \ + panicking now, so a caller can decide — the studio keeps its preview alive, the \ + render commands exit." ); } } diff --git a/crates/rustmotion/src/studio/app/mod.rs b/crates/rustmotion/src/studio/app/mod.rs index 2305566..c74d1c0 100644 --- a/crates/rustmotion/src/studio/app/mod.rs +++ b/crates/rustmotion/src/studio/app/mod.rs @@ -108,7 +108,9 @@ pub fn spawn_scenario_warmup(shared: Shared) { (m.scenario.clone(), m.scenario.video.fps) }; for view in &scenario.views { - engine::prefetch_icons(&view.scenes); + if let Err(icons_unresolved) = engine::prefetch_icons(&view.scenes) { + eprintln!("Warning: {icons_unresolved}"); + } engine::preextract_video_frames(&view.scenes, fps); } let failures = rustmotion::encode::audio_analysis::analyze_scenario_audio(&scenario);