diff --git a/crates/catalog/src/keywords.rs b/crates/catalog/src/keywords.rs index 92a531951..619967f65 100644 --- a/crates/catalog/src/keywords.rs +++ b/crates/catalog/src/keywords.rs @@ -236,63 +236,85 @@ pub struct KeywordNode { impl Catalog { /// The keyword tree: every keyword level with photo counts, sorted by name (case-insensitive). pub fn keyword_tree(&self) -> Vec { - use std::collections::{HashMap, HashSet}; - // every keyword level (`travel`, `travel|italy`…) by its lower-case path: the name and + use std::collections::HashMap; + // every keyword level (`travel`, `travel|italy`…), one per lower-case path: the name and // path as first written, and the photos counted (once per photo, even when two of its - // keywords share a parent) + // keywords share a parent: `seen` is the last photo counted) struct Level { name: String, path: String, count: usize, - parent: Option, + parent: Option, + seen: usize, } - let mut levels: HashMap = HashMap::new(); - let mut this_photo: HashSet = HashSet::new(); + let mut levels: Vec = Vec::new(); + let mut by_lower: HashMap = HashMap::new(); // the keyword list first (no photos counted): a listed keyword is there without photos let listed: Vec = self.listed_keywords().map(|k| k.path.clone()).collect(); + // each keyword as written → its levels (its own and those above it), worked out once: a + // large library repeats a few thousand keywords tens of thousands of times (issue #1011) + let mut chains: HashMap<&str, Vec> = HashMap::new(); let photos = self.photos().filter(|p| p.in_library()).map(|p| (true, p.meta.keywords.as_slice())); - for (counts, keywords) in std::iter::once((false, listed.as_slice())).chain(photos) { - this_photo.clear(); + for (photo, (counts, keywords)) in std::iter::once((false, listed.as_slice())).chain(photos).enumerate() { for k in keywords { - let mut path = String::new(); - let mut lower = String::new(); - let mut parent: Option = None; - for part in k.split(SEP).map(str::trim).filter(|s| !s.is_empty()) { - if !path.is_empty() { - path.push(SEP); - lower.push(SEP); + let chain = chains.entry(k.as_str()).or_insert_with(|| { + let mut chain = Vec::new(); + let mut path = String::new(); + let mut lower = String::new(); + for part in k.split(SEP).map(str::trim).filter(|s| !s.is_empty()) { + if !path.is_empty() { + path.push(SEP); + lower.push(SEP); + } + path.push_str(part); + lower.push_str(&part.to_lowercase()); + let i = match by_lower.get(&lower) { + Some(&i) => i, + None => { + let i = levels.len(); + let parent = chain.last().copied(); + levels.push(Level { name: part.to_string(), path: path.clone(), count: 0, parent, seen: 0 }); + by_lower.insert(lower.clone(), i); + i + } + }; + chain.push(i); } - path.push_str(part); - lower.push_str(&part.to_lowercase()); - let l = levels.entry(lower.clone()).or_insert_with(|| Level { - name: part.to_string(), - path: path.clone(), - count: 0, - parent: parent.clone(), - }); - if counts && this_photo.insert(lower.clone()) { - l.count += 1; + chain + }); + if counts { + for &i in chain.iter() { + let Some(l) = levels.get_mut(i) else { continue }; + // photo 0 is the keyword list, which counts nothing + if l.seen != photo { + l.seen = photo; + l.count += 1; + } } - parent = Some(lower.clone()); } } } - let mut kids: HashMap, Vec> = HashMap::new(); - for (lower, l) in &levels { - kids.entry(l.parent.clone()).or_default().push(lower.clone()); + // each level's children; `roots` the top level + let mut kids: Vec> = vec![Vec::new(); levels.len()]; + let mut roots = Vec::new(); + for (i, l) in levels.iter().enumerate() { + match l.parent.and_then(|p| kids.get_mut(p)) { + Some(k) => k.push(i), + None => roots.push(i), + } } - fn build(at: Option, levels: &HashMap, kids: &mut HashMap, Vec>) -> Vec { - let Some(mut list) = kids.remove(&at) else { return Vec::new() }; - list.sort_by_key(|l| levels[l].name.to_lowercase()); + fn build(mut list: Vec, levels: &[Level], kids: &mut [Vec]) -> Vec { + list.sort_by_cached_key(|&i| levels.get(i).map(|l| l.name.to_lowercase())); list.into_iter() - .map(|l| { - let lv = &levels[&l]; - let children = build(Some(l.clone()), levels, kids); - KeywordNode { name: lv.name.clone(), path: lv.path.clone(), count: lv.count, children } + .filter_map(|i| { + let lv = levels.get(i)?; + let below = kids.get_mut(i).map(std::mem::take).unwrap_or_default(); + let children = build(below, levels, kids); + Some(KeywordNode { name: lv.name.clone(), path: lv.path.clone(), count: lv.count, children }) }) .collect() } - build(None, &levels, &mut kids) + build(roots, &levels, &mut kids) } /// `SetMeta` ops for every photo whose keywords `f` changes. @@ -804,6 +826,15 @@ mod tests { assert_eq!(c.query(&f, &Default::default()).len(), 3); } + /// A keyword written twice on a photo, or in two spellings, counts the photo once; the next + /// photo with it counts again. + #[test] + fn tree_counts_a_repeated_keyword_once_per_photo() { + let (c, _) = lib(&[&["sea|Baltic", "sea|Baltic", "Sea|baltic"], &["sea|baltic"], &[]]); + let t = c.keyword_tree(); + assert_eq!(tree_paths(&t), [("sea".to_string(), 2), ("sea|Baltic".to_string(), 2)]); + } + #[test] fn rename_delete_merge_are_single_undoable_batches() { let (mut c, ids) = lib(&[&["travel|italy|rome", "beach"], &["Travel|Italy"], &["italia", "travel|italy"], &["sea"]]); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 9a387c77c..749d70f3b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -80,6 +80,8 @@ mod tests_keyword_list; #[cfg(test)] mod tests_keyword_set; #[cfg(test)] +mod tests_keyword_sidebar; +#[cfg(test)] mod tests_keywording; #[cfg(test)] mod tests_labels; diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index a14332c82..cec20e7df 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -21,6 +21,12 @@ pub(crate) fn content_width(ctx: &egui::Context) -> f32 { /// The most a row asks for its name: longer ones are cut when there is no more room. const MAX_NAME_NEED: f32 = 140.0; +/// Height of a sidebar row. +const ROW_HEIGHT: f32 = 29.0; + +/// Temp-data key of the keyword (lower-case path) the sidebar scrolls to, and the frame it was asked for. +const KEYWORD_SCROLL_TO: &str = "left-keyword-scroll-to"; + /// A row says how wide it needs to be. fn note_width(ui: &egui::Ui, w: f32) { ui.data_mut(|d| { @@ -75,7 +81,7 @@ fn row_sensed( ) -> egui::Response { let label = if matches!(id, "all" | "recentlyAdded" | "picks" | "missing" | "recentlyDeleted") { crate::i18n::tr(label) } else { label }; let t = Tokens::get(ui.ctx()); - let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 29.0), sense); + let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), ROW_HEIGHT), sense); register(ui.ctx(), format!("source:{id}"), r); let said = spoken.unwrap_or(label); let name = match count { @@ -638,6 +644,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { above.push_str(name); set_tree_open(ui, egui::Id::new(("kw-open", above.to_lowercase())), true); } + // …and the sidebar scrolls to it when its row is drawn (`keyword_rows`) + let frame = ui.ctx().cumulative_frame_nr(); + ui.data_mut(|d| d.insert_temp(egui::Id::new(KEYWORD_SCROLL_TO), (path.to_lowercase(), frame))); } // Local keeps track of the folder being browsed whether or not it is drawn let local = local_now(app, ui); @@ -1829,14 +1838,35 @@ fn folder_menu_for_library(app: &mut LightcraftApp, resp: &egui::Response, n: &F /// filters the grid by the keyword (children included), the triangle opens a level, and the /// context menu renames, merges or deletes the keyword across the library. fn keyword_rows(app: &mut LightcraftApp, ui: &mut egui::Ui, nodes: &[KeywordNode], indent: f32) { + // the keyword just made from the header, to scroll to (asked for a frame or two ago) + let scroll_id = egui::Id::new(KEYWORD_SCROLL_TO); + let frame = ui.ctx().cumulative_frame_nr(); + let scroll_to = ui.data(|d| d.get_temp::<(String, u64)>(scroll_id)).filter(|(_, asked)| frame.saturating_sub(*asked) <= 2).map(|(p, _)| p); for n in nodes { - let open_id = egui::Id::new(("kw-open", n.path.to_lowercase())); + let lower = n.path.to_lowercase(); + let reveal = scroll_to.as_deref() == Some(lower.as_str()); + let open_id = egui::Id::new(("kw-open", lower)); let mut open = tree_open(ui, open_id, false); + // a row out of sight only takes its room: a library can have thousands of keywords, and + // drawing them all every frame slows the whole app down (issue #1011) + let next = Rect::from_min_size(ui.cursor().min, vec2(ui.available_width(), ROW_HEIGHT)); + if !reveal && !ui.is_rect_visible(next) { + ui.allocate_space(next.size()); + note_width(ui, 42.0 + indent + 18.0); + if open && !n.children.is_empty() { + keyword_rows(app, ui, &n.children, indent + 16.0); + } + continue; + } let sel = app.session.filter.keyword.as_deref().is_some_and(|k| k.eq_ignore_ascii_case(&n.path)); let resp = row(app, ui, &format!("keyword:{}", n.path), Icon::Tag, &n.name, Some(n.count), sel, indent); if !n.children.is_empty() { tree_toggle(ui, &resp, indent, open_id, &mut open, format!("keywordToggle:{}", n.path)); } + if reveal { + ui.scroll_to_rect_animation(resp.rect, None, egui::style::ScrollAnimation::none()); + ui.data_mut(|d| d.remove::<(String, u64)>(scroll_id)); + } let resp = resp.on_hover_text(if n.children.is_empty() { n.path.clone() } else { format!("{} (includes the keywords below it)", n.path) }); if resp.clicked() { let v = if sel { serde_json::Value::Null } else { json!(n.path) }; diff --git a/crates/ui-egui/src/tests_keyword_sidebar.rs b/crates/ui-egui/src/tests_keyword_sidebar.rs new file mode 100644 index 000000000..1d7457cd6 --- /dev/null +++ b/crates/ui-egui/src/tests_keyword_sidebar.rs @@ -0,0 +1,92 @@ +//! Headless tests of the left sidebar's Keywords section on a large library (issue #1011): with +//! thousands of keywords listed, a frame draws only the rows near the screen, and the rows out +//! of sight still take their room, so scrolling reaches the last keyword. + +use std::time::{Duration, Instant}; + +use lightcraft_catalog::{Op, Photo, PhotoId, Source}; +use lightcraft_engine::Session; +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(30); +const SETTLE: Duration = Duration::from_secs(120); + +/// `photos` photos with six keywords each out of `keywords` top-level ones (each with a child). +fn library(photos: u64, keywords: u64) -> Session { + let mut s = Session::new(); + for i in 0..photos { + let path = format!("/lightcraft-kw-test/IMG_{i:05}.jpg"); + let mut p = Photo::new(PhotoId(i + 1), Source::File { path }, &format!("IMG_{i:05}.jpg"), "JPEG", 60, 40, "2026-01-01T00:00:00"); + p.meta.keywords = (0..6).map(|k| format!("Keyword {:05}|Child", (i * 6 + k) % keywords)).collect(); + s.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + } + s +} + +fn app(photos: u64, keywords: u64) -> Headless { + let app = LightcraftApp::new(library(photos, keywords), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1400.0, 900.0], 1.0); + let r = h.request("ui.set", json!({"view": "photoGrid", "leftPanel": true}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + h +} + +/// The keyword rows drawn this frame. +fn keyword_rows(h: &Headless) -> Vec { + h.app.widgets.iter().filter_map(|(w, _)| w.strip_prefix("source:keyword:")).map(str::to_string).collect() +} + +/// Given a library with 2,000 keywords, when the sidebar's Keywords section is open, then a frame +/// draws only the few dozen rows that fit on screen (not all 2,000), and scrolling to the end +/// draws the last keyword: the rows out of sight keep their height. +#[test] +fn keywords_section_draws_only_the_rows_near_the_screen() { + const KEYWORDS: u64 = 2000; + let mut h = app(KEYWORDS / 6 + 1, KEYWORDS); + assert_eq!(h.app.caches.keyword_tree(&h.app.session.catalog).len(), KEYWORDS as usize); + let rows = keyword_rows(&h); + assert!(!rows.is_empty() && rows.len() < 100, "{} of {KEYWORDS} keyword rows drawn", rows.len()); + assert!(rows.contains(&"Keyword 00000".to_string()), "{rows:?}"); + // all the way down + h.request("ui.move", json!({"x": 120.0, "y": 400.0}), T); + for _ in 0..40 { + let r = h.request("ui.scroll", json!({"dy": -5000.0}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + } + h.settle(SETTLE); + let rows = keyword_rows(&h); + assert!(!rows.is_empty() && rows.len() < 100, "{} of {KEYWORDS} keyword rows drawn", rows.len()); + assert!(rows.contains(&"Keyword 01999".to_string()), "the last keyword is reached: {rows:?}"); + assert!(!rows.contains(&"Keyword 00000".to_string()), "{rows:?}"); +} + +/// Timings on a library the size of the report's (58,000 photos): `cargo test --release -p +/// lightcraft-ui-egui --lib probe_keyword_sidebar_cost -- --ignored --nocapture`. +#[test] +#[ignore = "timing probe"] +fn probe_keyword_sidebar_cost() { + let mut h = app(58_000, 4000); + let t = Instant::now(); + let tree = h.app.session.catalog.keyword_tree(); + eprintln!("keyword_tree: {:?} ({} top level)", t.elapsed(), tree.len()); + eprintln!("keyword rows drawn: {}", keyword_rows(&h).len()); + let t = Instant::now(); + for _ in 0..10 { + h.step(); + } + eprintln!("unchanged frame: {:?}", t.elapsed() / 10); + let t = Instant::now(); + for i in 0..10u64 { + let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [i + 1]}}), T); + assert_eq!(r["ok"], true, "{r}"); + let r = h.request("engine.execute", json!({"command": "photo.rate", "params": {"rating": (i % 5) + 1}}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + } + eprintln!("frame after a rating: {:?}", t.elapsed() / 10); +} diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index 25edf3bfa..a94f613a5 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -889,6 +889,10 @@ fn the_keywords_header_creates_a_keyword() { h.step(); h.step(); assert!(has(&h, "source:keyword:zebra crossing"), "the new keyword is listed"); + // the sidebar scrolls to it: it is listed last, below what the panel shows + h.settle(SETTLE); + let (row, panel) = (widget(&h, "source:keyword:zebra crossing"), widget(&h, "panel:left_panel")); + assert!(panel.contains_rect(row), "the new keyword is in sight: {row:?} in {panel:?}"); } /// A folded section's row count sits to the left of its header button; a section without a button