Merge pull request #1259 from arnav-hiwarkar/perf/media-browser-listing

perf(media-browser): reuse listings and cache filename sort keys

crates/engine/src/media_browser.rs

@@ -173,7 +173,7 @@ impl VirtualTree {
                 })
             })
             .collect();
-        out.sort_by(|a, b| b.is_dir.cmp(&a.is_dir).then_with(|| a.name.to_lowercase().cmp(&b.name.to_lowercase())));
+        sort_entries(&mut out);
         Ok(out)
     }
 }
@@ -193,6 +193,10 @@ pub struct Entry {
     pub kind: Option<&'static str>,
 }
 
+fn sort_entries(entries: &mut [Entry]) {
+    entries.sort_by_cached_key(|e| (!e.is_dir, e.name.to_lowercase()));
+}
+
 const VIDEO: &[&str] = &["mp4", "m4v", "mov", "webm", "mkv", "avi", "mxf", "gif"];
 const IMAGE: &[&str] = &["png", "jpg", "jpeg", "tif", "tiff", "exr", "bmp", "webp", "psd", "psb", "svg", "tga", "dpx", "cin", "hdr"];
 const AUDIO: &[&str] = &["wav", "aif", "aiff", "mp3", "m4a", "aac", "flac", "ogg", "opus"];
@@ -251,7 +255,7 @@ pub fn list(dir: &str, importable_only: bool) -> Result<Vec<Entry>, String> {
             Some(Entry { name, path, is_dir: md.is_dir(), size: if md.is_dir() { 0 } else { md.len() }, modified: unix(md.modified()), kind })
         })
         .collect();
-    out.sort_by(|a, b| b.is_dir.cmp(&a.is_dir).then_with(|| a.name.to_lowercase().cmp(&b.name.to_lowercase())));
+    sort_entries(&mut out);
     Ok(out)
 }
 
@@ -407,6 +411,10 @@ mod tests {
         assert_eq!(media.len(), 3);
         assert_eq!(parent(&dir.join("Shots").to_string_lossy()).as_deref(), Some(d.as_str()));
         assert!(list(&dir.join("nope").to_string_lossy(), false).is_err());
+        std::fs::write(dir.join("Ω.csv"), b"x").unwrap();
+        std::fs::write(dir.join("作品.csv"), b"x").unwrap();
+        let all = list(&d, false).unwrap();
+        assert_eq!(all.iter().map(|e| e.name.as_str()).collect::<Vec<_>>(), ["Shots", "A.mov", "b.png", "notes.txt", "Ω.csv", "作品.csv"]);
         std::fs::remove_dir_all(&dir).unwrap();
     }
 
@@ -436,6 +444,61 @@ mod tests {
         assert_eq!(vparent("/"), None);
     }
 
+    #[test]
+    fn listings_keep_folders_first_and_stable_unicode_order() {
+        let mut tree = VirtualTree::default();
+        for name in ["作品.csv", "ä.csv", "Ä.csv", "İ.csv", "B.csv", "a.csv", "A.csv"] {
+            tree.add_file(&format!("/Shoot/{name}"), &format!("/files/{name}"), 1, None);
+        }
+        for name in ["zFolder", "Alpha"] {
+            tree.add_dir(&format!("/Shoot/{name}"));
+        }
+        let entries = tree.list("/Shoot", true).unwrap();
+        assert_eq!(
+            entries.iter().map(|e| e.name.as_str()).collect::<Vec<_>>(),
+            ["Alpha", "zFolder", "A.csv", "a.csv", "B.csv", "İ.csv", "Ä.csv", "ä.csv", "作品.csv"]
+        );
+        assert_eq!(entries[2].path, "/files/A.csv");
+        let mut reversed = entries.iter().rev().cloned().collect::<Vec<_>>();
+        sort_entries(&mut reversed);
+        assert_eq!(
+            reversed.iter().map(|e| e.name.as_str()).collect::<Vec<_>>(),
+            ["Alpha", "zFolder", "a.csv", "A.csv", "B.csv", "İ.csv", "ä.csv", "Ä.csv", "作品.csv"]
+        );
+    }
+
+    #[test]
+    #[ignore = "Media Browser filename sorting benchmark"]
+    fn directory_sort_benchmark() {
+        let entries: Vec<_> = (0..1000)
+            .map(|i| Entry {
+                name: format!("作品 {} {:04}.mov", if i % 2 == 0 { "Clip" } else { "clip" }, i * 7919 % 1000),
+                path: format!("/Shoot/{i}.mov"),
+                is_dir: i % 7 == 0,
+                size: 1,
+                modified: None,
+                kind: Some("video"),
+            })
+            .collect();
+        let mut before = Vec::new();
+        let mut after = Vec::new();
+        for _ in 0..5 {
+            let mut old = entries.clone();
+            let start = std::time::Instant::now();
+            old.sort_by(|a, b| b.is_dir.cmp(&a.is_dir).then_with(|| a.name.to_lowercase().cmp(&b.name.to_lowercase())));
+            before.push(start.elapsed());
+            let mut new = entries.clone();
+            let start = std::time::Instant::now();
+            sort_entries(&mut new);
+            after.push(start.elapsed());
+            assert_eq!(old, new);
+            std::hint::black_box(new);
+        }
+        before.sort();
+        after.sort();
+        println!("1,000 filename sort, median: {:?} -> {:?}", before[2], after[2]);
+    }
+
     #[test]
     fn iso_dates() {
         assert_eq!(iso_date(0), "1970-01-01T00:00:00Z");

crates/ui-egui/src/panels/media_panels.rs

@@ -15,6 +15,7 @@ use effectcraft_engine::media_browser as mb;
 use effectcraft_engine::project::ItemId;
 use egui::{Align2, Color32, Rect, Sense, Stroke, pos2, vec2};
 use serde_json::{Value, json};
+use std::sync::Arc;
 
 use super::DragPayload;
 use super::panel_kit as kit;
@@ -24,6 +25,7 @@ use crate::{EffectcraftApp, widgets};
 // ---------------------------------------------------------------- Media Browser
 
 type Thumb = Option<(egui::TextureHandle, [u32; 2])>;
+type Listing = (f64, Arc<Result<Vec<mb::Entry>, String>>);
 
 /// Thumbnail of a file (cached per path; `None` while not decoded or not decodable).
 fn thumbnail(app: &EffectcraftApp, ctx: &egui::Context, path: &str, budget: &mut u32) -> Thumb {
@@ -76,11 +78,11 @@ pub fn media_browser(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
     // Listing cached per folder for a second.
     let lid = egui::Id::new(("mb-list", dir.clone(), st.importable_only));
     let now = ctx.input(|i| i.time);
-    let listing: Option<(f64, Result<Vec<mb::Entry>, String>)> = ctx.data(|d| d.get_temp(lid));
+    let listing: Option<Listing> = ctx.data(|d| d.get_temp(lid));
     let entries = match listing {
         Some((at, l)) if now - at < 1.0 => l,
         _ => {
-            let l = b.list(&dir, st.importable_only);
+            let l = Arc::new(b.list(&dir, st.importable_only));
             ctx.data_mut(|d| d.insert_temp(lid, (now, l.clone())));
             l
         }
@@ -115,7 +117,7 @@ pub fn media_browser(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
         }
         if kit::button(app, ui, Rect::from_min_size(pos2(ax, y), vec2(w, 20.0)), &format!("mediaBrowser.action.{id}"), label, false) {
             kit::exec(app, "mediaBrowser.action", json!({"action": id}));
-            ctx.data_mut(|d| d.remove::<(f64, Result<Vec<mb::Entry>, String>)>(lid));
+            ctx.data_mut(|d| d.remove::<Listing>(lid));
         }
         ax -= 6.0;
     }
@@ -150,7 +152,7 @@ pub fn media_browser(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
     }
     // Files.
     let list = Rect::from_min_max(pos2(side.max.x + 1.0, side.min.y), rect.max);
-    let entries = match entries {
+    let entries = match entries.as_ref() {
         Ok(e) => e,
         Err(e) => {
             p.text(list.center(), Align2::CENTER_CENTER, e, Tokens::ui(12.0), t.danger);
@@ -417,3 +419,52 @@ pub fn import_card(app: &mut EffectcraftApp, ui: &mut egui::Ui, full: Rect) -> f
     }
     full.max.y - 16.0 - bottom
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    #[ignore = "Media Browser warm listing cache benchmark"]
+    fn listing_cache_benchmark() {
+        let ctx = egui::Context::default();
+        let entries: Vec<_> = (0..1000)
+            .map(|i| mb::Entry {
+                name: format!("Clip {i:04}.mov"),
+                path: format!("/Shoot/作品 Clip {i:04}.mov"),
+                is_dir: false,
+                size: 1,
+                modified: None,
+                kind: Some("video"),
+            })
+            .collect();
+        let old_id = egui::Id::new("old-list");
+        let new_id = egui::Id::new("new-list");
+        ctx.data_mut(|d| {
+            d.insert_temp(old_id, (0.0, Ok::<_, String>(entries.clone())));
+            d.insert_temp(new_id, (0.0, Arc::new(Ok::<_, String>(entries))));
+        });
+        let (_, original) = ctx.data(|d| d.get_temp::<Listing>(new_id)).unwrap();
+        assert_eq!(original.as_ref().as_ref().unwrap()[0].name, "Clip 0000.mov");
+        let mut before = Vec::new();
+        let mut after = Vec::new();
+        for _ in 0..5 {
+            let start = std::time::Instant::now();
+            for _ in 0..1000 {
+                let _ = std::hint::black_box(ctx.data(|d| d.get_temp::<(f64, Result<Vec<mb::Entry>, String>)>(old_id)).unwrap());
+            }
+            before.push(start.elapsed());
+            let start = std::time::Instant::now();
+            for _ in 0..1000 {
+                std::hint::black_box(ctx.data(|d| d.get_temp::<Listing>(new_id)).unwrap());
+            }
+            after.push(start.elapsed());
+        }
+        let (_, cached) = ctx.data(|d| d.get_temp::<Listing>(new_id)).unwrap();
+        assert!(Arc::ptr_eq(&original, &cached));
+        assert_eq!(cached.as_ref().as_ref().unwrap().last().unwrap().name, "Clip 0999.mov");
+        before.sort();
+        after.sort();
+        println!("1,000 Media Browser entries, median per warm cache lookup: {:?} -> {:?}", before[2] / 1000, after[2] / 1000);
+    }
+}

crates/ui-egui/tests/ui_media_browser_cache.rs

@@ -0,0 +1,145 @@
+use std::sync::{
+    Arc, Mutex,
+    atomic::{AtomicBool, Ordering},
+};
+
+use effectcraft_engine::{
+    Session,
+    media_browser::{Browser, Entry, VirtualTree},
+};
+use effectcraft_ui_egui::{EffectcraftApp, dock::PanelKind};
+use egui::{Event, Modifiers, PointerButton, pos2};
+use egui_kittest::Harness;
+use serde_json::{Value, json};
+
+#[derive(Default)]
+struct MutableBrowser {
+    tree: Mutex<VirtualTree>,
+    offline: AtomicBool,
+}
+
+impl Browser for MutableBrowser {
+    fn home(&self) -> String {
+        "/Shoot".into()
+    }
+    fn list(&self, dir: &str, only: bool) -> Result<Vec<Entry>, String> {
+        if self.offline.load(Ordering::Relaxed) {
+            return Err("Folder temporarily unavailable".into());
+        }
+        self.tree.lock().unwrap().list(dir, only)
+    }
+    fn actions(&self) -> Vec<(String, String)> {
+        vec![("refresh".into(), "Refresh".into())]
+    }
+    fn action(&self, id: &str) -> Result<Value, String> {
+        assert_eq!(id, "refresh");
+        self.tree.lock().unwrap().add_file("/Shoot/From action.csv", "/files/action.csv", 1, None);
+        Ok(json!({}))
+    }
+}
+
+type Listing = (f64, Arc<Result<Vec<Entry>, String>>);
+
+fn listing(h: &Harness<'_, EffectcraftApp>, only: bool) -> Listing {
+    let dir = h.state().session.state.media_browser.folder.clone().unwrap_or_else(|| "/Shoot".into());
+    h.ctx.data(|d| d.get_temp(egui::Id::new(("mb-list", dir, only)))).unwrap()
+}
+
+fn names(list: &Arc<Result<Vec<Entry>, String>>) -> Vec<&str> {
+    list.as_ref().as_ref().unwrap().iter().map(|e| e.name.as_str()).collect()
+}
+
+fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str) {
+    let e = h.state().auto.find(id).unwrap_or_else(|| panic!("missing {id}"));
+    let pos = pos2(e.rect[0] + 8.0, e.rect[1] + e.rect[3] / 2.0);
+    h.input_mut().events.push(Event::PointerMoved(pos));
+    h.step();
+    for pressed in [true, false] {
+        h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE });
+        h.step();
+    }
+    h.run_steps(2);
+}
+
+fn at(h: &mut Harness<'_, EffectcraftApp>, time: f64) {
+    h.input_mut().time = Some(time);
+    h.step();
+    h.input_mut().time = Some(time);
+    h.step();
+    h.input_mut().time = None;
+}
+
+#[test]
+fn browser_cache_refreshes_filters_navigates_and_recovers_from_errors() {
+    let browser = Arc::new(MutableBrowser::default());
+    {
+        let mut tree = browser.tree.lock().unwrap();
+        tree.add_file("/Shoot/Clip.csv", "/files/clip.csv", 1, None);
+        tree.add_file("/Shoot/notes.txt", "/files/notes.txt", 1, None);
+        tree.add_file("/Shoot/Sub/Child.csv", "/files/child.csv", 1, None);
+    }
+    let mut session = Session { browser: Some(browser.clone()), ..Default::default() };
+    session.prefs.general.language = "en".into();
+    let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).with_step_dt(1.0 / 60.0).build_eframe(|_| {
+        let mut app = EffectcraftApp::new(session);
+        app.ui.start_screen = false;
+        app.show_panel(PanelKind::MediaBrowser);
+        app.ui.maximized = Some(PanelKind::MediaBrowser);
+        app
+    });
+    h.run_steps(3);
+    let (time, first) = listing(&h, false);
+    assert_eq!(names(&first), ["Sub", "Clip.csv", "notes.txt"]);
+    assert!(h.state().auto.find("mediaBrowser.entry.Clip.csv").is_some());
+    browser.tree.lock().unwrap().add_file("/Shoot/Added.csv", "/files/added.csv", 2, Some(5));
+    at(&mut h, time + 0.999);
+    assert!(Arc::ptr_eq(&first, &listing(&h, false).1));
+    assert!(h.state().auto.find("mediaBrowser.entry.Added.csv").is_none());
+    at(&mut h, time + 1.0);
+    let refreshed = listing(&h, false).1;
+    assert_eq!(names(&refreshed), ["Sub", "Added.csv", "Clip.csv", "notes.txt"]);
+    assert!(h.state().auto.find("mediaBrowser.entry.Added.csv").is_some());
+    assert_eq!(names(&first), ["Sub", "Clip.csv", "notes.txt"]);
+    click(&mut h, "mediaBrowser.importableOnly");
+    assert!(h.state().session.state.media_browser.importable_only);
+    assert_eq!(names(&listing(&h, true).1), ["Sub", "Added.csv", "Clip.csv"]);
+    assert!(h.state().auto.find("mediaBrowser.entry.notes.txt").is_none());
+    click(&mut h, "mediaBrowser.importableOnly");
+    assert!(!h.state().session.state.media_browser.importable_only);
+    assert!(Arc::ptr_eq(&refreshed, &listing(&h, false).1));
+    click(&mut h, "mediaBrowser.action.refresh");
+    assert_eq!(names(&listing(&h, false).1), ["Sub", "Added.csv", "Clip.csv", "From action.csv", "notes.txt"]);
+    assert!(h.state().auto.find("mediaBrowser.entry.From action.csv").is_some());
+    assert_eq!(names(&refreshed), ["Sub", "Added.csv", "Clip.csv", "notes.txt"]);
+    h.run_steps(40);
+    click(&mut h, "mediaBrowser.entry.Sub");
+    click(&mut h, "mediaBrowser.entry.Sub");
+    assert_eq!(h.state().session.state.media_browser.folder.as_deref(), Some("/Shoot/Sub"));
+    assert_eq!(names(&listing(&h, false).1), ["Child.csv"]);
+    assert!(h.state().auto.find("mediaBrowser.entry.Child.csv").is_some());
+    browser.tree.lock().unwrap().remove("/Shoot/Sub/Child.csv");
+    let time = listing(&h, false).0;
+    at(&mut h, time + 1.0);
+    assert_eq!(names(&listing(&h, false).1), Vec::<&str>::new());
+    assert!(h.state().auto.find("mediaBrowser.entry.Child.csv").is_none());
+    click(&mut h, "mediaBrowser.up");
+    assert_eq!(h.state().session.state.media_browser.folder.as_deref(), Some("/Shoot"));
+    assert!(h.state().auto.find("mediaBrowser.entry.Clip.csv").is_some());
+    browser.offline.store(true, Ordering::Relaxed);
+    let time = listing(&h, false).0;
+    at(&mut h, time + 1.0);
+    let (time, error) = listing(&h, false);
+    assert_eq!(error.as_ref(), &Err("Folder temporarily unavailable".into()));
+    assert!(h.output().shapes.iter().any(|s| matches!(&s.shape, egui::Shape::Text(t) if t.galley.text() == "Folder temporarily unavailable")));
+    assert!(h.state().auto.find("mediaBrowser.entry.Clip.csv").is_none());
+    browser.offline.store(false, Ordering::Relaxed);
+    browser.tree.lock().unwrap().remove("/Shoot/notes.txt");
+    at(&mut h, time + 0.999);
+    assert!(Arc::ptr_eq(&error, &listing(&h, false).1));
+    at(&mut h, time + 1.0);
+    assert_eq!(names(&listing(&h, false).1), ["Sub", "Added.csv", "Clip.csv", "From action.csv"]);
+    assert!(h.state().auto.find("mediaBrowser.entry.Clip.csv").is_some());
+    if let Ok(path) = std::env::var("MEDIA_BROWSER_SNAPSHOT") {
+        h.render().unwrap().save(path).unwrap();
+    }
+}