fix(icons): repair the attribute rewrite that stopped every icon rendering

crates/rustmotion-core/src/engine/renderer/assets.rs

@@ -67,7 +67,7 @@ pub fn icon_cache_dir() -> PathBuf {
     base.join("rustmotion").join("icons")
 }
 
-fn icon_source_cache_file(cache_dir: &Path, icon: &str) -> PathBuf {
+pub fn icon_source_cache_file(cache_dir: &Path, icon: &str) -> PathBuf {
     cache_dir.join(format!("{}.svg", icon.replace(':', "_")))
 }
 
@@ -89,7 +89,7 @@ fn set_root_attribute(svg: &str, attribute: &str, value: &str) -> String {
                     "{}{}\"{}",
                     &tag[..value_start],
                     value,
-                    &tag[value_start + value_len..]
+                    &tag[value_start + value_len + 1..]
                 ),
                 None => tag.to_string(),
             }
@@ -112,6 +112,42 @@ fn recolour_and_resize(source: &[u8], hex_color: &str, width: u32, height: u32)
     set_root_attribute(&sized, "height", &height.to_string()).into_bytes()
 }
 
+fn legacy_icon_source(cache_dir: &Path, icon: &str) -> Option<Vec<u8>> {
+    let slug = icon.replace(':', "_");
+    let prefix = format!("{slug}-");
+    let entries = std::fs::read_dir(cache_dir).ok()?;
+    for entry in entries.flatten() {
+        let name = entry.file_name();
+        let name = name.to_str()?;
+        let Some(rest) = name.strip_prefix(&prefix) else {
+            continue;
+        };
+        let Some(rest) = rest.strip_suffix(".svg") else {
+            continue;
+        };
+        let Some((colour, size)) = rest.split_once('-') else {
+            continue;
+        };
+        if colour.len() != 6 || !colour.chars().all(|c| c.is_ascii_hexdigit()) {
+            continue;
+        }
+        let Some((w, h)) = size.split_once('x') else {
+            continue;
+        };
+        if w.parse::<u32>().is_err() || h.parse::<u32>().is_err() {
+            continue;
+        }
+        let baked = std::fs::read_to_string(entry.path()).ok()?;
+        let restored = baked
+            .replace(&format!("#{}", colour.to_lowercase()), "currentColor")
+            .replace(&format!("#{}", colour.to_uppercase()), "currentColor");
+        if restored.contains("currentColor") {
+            return Some(restored.into_bytes());
+        }
+    }
+    None
+}
+
 const ICON_FETCH_ATTEMPTS: u32 = 4;
 
 fn status_is_worth_retrying(error: &ureq::Error) -> bool {
@@ -177,13 +213,19 @@ pub fn fetch_icon_svg_in(
 
     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);
+        _ => match legacy_icon_source(cache_dir, icon) {
+            Some(migrated) => {
+                let _ = std::fs::write(&source_file, &migrated);
+                migrated
             }
-            fetched
-        }
+            None => {
+                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
+            }
+        },
     };
 
     Ok(recolour_and_resize(&source, &hex_color, width, height))
@@ -741,6 +783,94 @@ mod icon_source_cache_tests {
         );
     }
 
+    fn a_cache_dir(tag: &str) -> PathBuf {
+        let dir = std::env::temp_dir().join(format!(
+            "rm_legacy_icons_{}_{}_{tag}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        ));
+        std::fs::create_dir_all(&dir).expect("cache dir");
+        dir
+    }
+
+    #[test]
+    fn a_file_left_by_the_old_naming_is_migrated_instead_of_refetched() {
+        let dir = a_cache_dir("migrate");
+        let baked = LUCIDE_CHECK.replace("currentColor", "#3B6FD4");
+        std::fs::write(dir.join("lucide_check-3b6fd4-26x26.svg"), &baked).expect("seed");
+
+        let migrated = legacy_icon_source(&dir, "lucide:check")
+            .expect("674 of these sit in a real cache; an offline render must be able to use one");
+        let text = String::from_utf8(migrated).unwrap();
+        assert!(
+            text.contains("currentColor"),
+            "the baked colour has to come back out, or the icon can never be recoloured: {text}"
+        );
+        assert!(!text.contains("3B6FD4"), "got {text}");
+        std::fs::remove_dir_all(&dir).ok();
+    }
+
+    #[test]
+    fn migration_survives_an_icon_whose_own_name_contains_a_dash() {
+        let dir = a_cache_dir("dashed");
+        let baked = LUCIDE_CHECK.replace("currentColor", "#8b5cf6");
+        std::fs::write(dir.join("lucide_arrow-down-8b5cf6-176x176.svg"), &baked).expect("seed");
+        assert!(
+            legacy_icon_source(&dir, "lucide:arrow-down").is_some(),
+            "the colour and size have to be parsed from the right, not from the first dash"
+        );
+        std::fs::remove_dir_all(&dir).ok();
+    }
+
+    #[test]
+    fn an_unrelated_file_is_not_mistaken_for_a_legacy_entry() {
+        let dir = a_cache_dir("unrelated");
+        std::fs::write(dir.join("lucide_check-notes.svg"), LUCIDE_CHECK).expect("seed");
+        std::fs::write(dir.join("lucide_checkmark.svg"), LUCIDE_CHECK).expect("seed");
+        assert!(
+            legacy_icon_source(&dir, "lucide:check").is_none(),
+            "only a name ending in -{{6 hex}}-{{w}}x{{h}} is a legacy entry"
+        );
+        std::fs::remove_dir_all(&dir).ok();
+    }
+
+    #[test]
+    fn the_rewritten_icon_still_parses_as_svg() {
+        let out = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "2563eb", 84, 84);
+        let text = String::from_utf8(out.clone()).unwrap();
+        assert!(
+            !text.contains("\"\""),
+            "the rewrite must not leave the previous value's closing quote behind: {text}"
+        );
+        usvg::Tree::from_data(&out, &usvg::Options::default()).unwrap_or_else(|e| {
+            panic!(
+                "a rewritten icon has to parse, or nothing renders and the only sign is a \
+                 warning on stderr: {e}\n{text}"
+            )
+        });
+    }
+
+    #[test]
+    fn rewriting_an_attribute_that_already_has_a_value_replaces_it_exactly_once() {
+        let source = r#"<svg xmlns="http://www.w3.org/2000/svg" width="1em" height="1em" viewBox="0 0 24 24"><path d="M4 12h16"/></svg>"#;
+        let out = String::from_utf8(recolour_and_resize(source.as_bytes(), "000000", 84, 96))
+            .expect("utf8");
+        assert_eq!(
+            out.matches("width=").count(),
+            1,
+            "one width attribute, not two: {out}"
+        );
+        assert!(out.contains(r#"width="84""#), "got {out}");
+        assert!(out.contains(r#"height="96""#), "got {out}");
+        assert!(
+            !out.contains("1em"),
+            "the 1em Iconify ships must be gone: {out}"
+        );
+    }
+
     #[test]
     fn recolouring_substitutes_current_color_everywhere() {
         let out = recolour_and_resize(LUCIDE_CHECK.as_bytes(), "2a2f6b", 84, 84);

crates/rustmotion/src/cli/commands/render.rs

@@ -62,7 +62,9 @@ pub fn cmd_render(
 
     let start = std::time::Instant::now();
 
-    if !scenario.fonts.is_empty() {
+    if frame.is_some() {
+        engine::preload::preload_scenario_assets(&scenario)?;
+    } else if !scenario.fonts.is_empty() {
         engine::renderer::load_custom_fonts(&scenario.fonts);
     }
 

crates/rustmotion/src/tests.rs

@@ -3822,3 +3822,100 @@ mod still_runs_the_audio_analysis_tests {
         );
     }
 }
+
+#[cfg(test)]
+mod icon_render_tests {
+    use crate::encode::video::{build_frame_tasks, render_frame_task, FrameTask};
+    use crate::loader::load_scenario_from_source;
+    use rustmotion_core::engine::renderer::{icon_cache_dir, icon_source_cache_file};
+
+    const W: usize = 160;
+    const H: usize = 160;
+
+    const A_BAR: &str = r#"<svg xmlns="http://www.w3.org/2000/svg" width="1em" height="1em" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="4"><path d="M2 12h20"/></svg>"#;
+
+    struct SeededIcon {
+        name: String,
+        file: std::path::PathBuf,
+    }
+
+    impl SeededIcon {
+        fn new(tag: &str) -> Self {
+            let name = format!("rmtest{tag}:bar");
+            let dir = icon_cache_dir();
+            std::fs::create_dir_all(&dir).expect("cache dir");
+            let file = icon_source_cache_file(&dir, &name);
+            std::fs::write(&file, A_BAR).expect("seed the icon so the test needs no network");
+            Self { name, file }
+        }
+    }
+
+    impl Drop for SeededIcon {
+        fn drop(&mut self) {
+            std::fs::remove_file(&self.file).ok();
+        }
+    }
+
+    fn render_first_frame(icon: &str) -> Vec<u8> {
+        let json = format!(
+            r##"{{
+              "version": "1.0",
+              "video": {{ "width": {W}, "height": {H}, "fps": 30, "background": "#FFFFFF" }},
+              "scenes": [{{ "duration": 1.0, "children": [
+                {{ "type": "icon", "icon": "{icon}",
+                   "style": {{ "position": "absolute", "left": 38, "top": 38,
+                              "width": 84, "height": 84, "color": "#2563EB" }} }}
+              ]}}]
+            }}"##
+        );
+        let scenario = load_scenario_from_source(None, Some(&json)).expect("load");
+        crate::engine::preload::preload_scenario_assets(&scenario).expect("preload");
+        let tasks = build_frame_tasks(&scenario);
+        let task = tasks
+            .iter()
+            .find(|t| matches!(t, FrameTask::Normal { .. }))
+            .expect("a normal frame");
+        render_frame_task(&scenario.video, &scenario, task).expect("render")
+    }
+
+    fn blue_pixels(frame: &[u8]) -> usize {
+        (0..W * H)
+            .filter(|i| {
+                let p = i * 4;
+                frame[p] < 120 && frame[p + 2] > 150
+            })
+            .count()
+    }
+
+    #[test]
+    fn an_icon_actually_paints_its_glyph_into_the_frame() {
+        let seeded = SeededIcon::new("paint");
+        let frame = render_first_frame(&seeded.name);
+        let painted = blue_pixels(&frame);
+        assert!(
+            painted > 100,
+            "the icon painted {painted} coloured pixels. Rewriting its width left a stray quote, \
+             so the SVG did not parse, so nothing was drawn — and the only sign was a warning on \
+             stderr with a zero exit code"
+        );
+    }
+
+    #[test]
+    fn the_icon_takes_the_colour_the_scenario_asks_for() {
+        let seeded = SeededIcon::new("colour");
+        let frame = render_first_frame(&seeded.name);
+        let mut found = None;
+        for i in 0..W * H {
+            let p = i * 4;
+            if frame[p] < 120 && frame[p + 2] > 150 && frame[p + 3] > 250 {
+                found = Some((frame[p], frame[p + 1], frame[p + 2]));
+                break;
+            }
+        }
+        let (r, g, b) = found.expect("an opaque coloured pixel");
+        assert!(
+            r < 80 && g < 140 && b > 200,
+            "currentColor must become the declared #2563EB, got ({r}, {g}, {b})"
+        );
+    }
+}