LRC-LIB-SMARTCOLL: rule dialog and sidebar problems cached per change, not per frame

From final review: while the rule dialog was open it ran the rule check
several times, the full query for the live count (sorting every match
only to count them) and rebuilt the album list with a loop walk per
album, on every repaint, including pointer moves over a picker. The
sidebar re-checked every smart album's rules on every frame.

Caches::rules_view keeps the dialog's problems, count and album list
until the catalog revision, the rules, the language or (for "in the
last…" rules) the minute changes. Caches::smart_album_problems keeps
the sidebar's ⚠ problems per catalog revision. Both count their scans,
like the album counts, and a test checks that unchanged frames reuse
them and that rule and catalog changes don't.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford
2026-10-09 10:54:58 -04:00
co-authored by Claude Opus 5.5
parent a484c66c02
commit e8678bb28e
3 changed files with 110 additions and 14 deletions
+100
View File
@@ -1068,6 +1068,12 @@ pub struct Caches {
album_counts: Option<(u64, std::sync::Arc<std::collections::HashMap<lightcraft_catalog::AlbumId, usize>>)>,
/// How often the album counts were recomputed (tests check that unchanged frames don't).
pub album_count_scans: usize,
/// What the open smart-album rule dialog shows besides the rules ([`Caches::rules_view`]).
rules_view: Option<(u64, std::sync::Arc<RulesView>)>,
pub rules_view_scans: usize,
/// The problems of every saved smart album that has some ([`Caches::smart_album_problems`]).
smart_problems: Option<(u64, std::sync::Arc<std::collections::HashMap<lightcraft_catalog::AlbumId, Vec<lightcraft_catalog::rules::Problem>>>)>,
pub smart_problem_scans: usize,
/// AI denoise: what the pump last saw, the model list and the downloads being watched.
pub denoise: panels::denoise::Ui,
/// Bumped when a face model is installed, removed or chosen, so Settings re-reads the list at once.
@@ -1117,6 +1123,14 @@ pub struct FilterValues {
pub keywords: Vec<String>,
}
/// What the smart-album rule dialog shows besides the rules themselves: their problems (each row
/// marks its own), how many photos they match and the albums an Album rule picks from.
pub struct RulesView {
pub problems: Vec<lightcraft_catalog::rules::Problem>,
pub count: usize,
pub albums: Vec<album_picker::AlbumEntry>,
}
pub(crate) fn key_of(parts: impl std::hash::Hash) -> u64 {
use std::hash::{BuildHasher, BuildHasherDefault, DefaultHasher};
BuildHasherDefault::<DefaultHasher>::default().hash_one(parts)
@@ -1244,6 +1258,56 @@ impl Caches {
/// all bump its revision) and — only while some smart album has an "in the last…" rule —
/// when `now` (the session clock, ISO) enters a new minute, so such counts follow the clock
/// within a minute without rescanning every frame.
/// [`RulesView`] for `rules` edited in smart album `editing` (`None`: a new album), within
/// `folder` when it was made from a folder view: worked out again only when the catalog, the
/// rules, the language or (for "in the last…" rules) the minute changes, not every frame.
pub fn rules_view(
&mut self,
cat: &lightcraft_catalog::Catalog,
rules: &lightcraft_catalog::RuleSet,
editing: Option<u64>,
folder: Option<String>,
now: &str,
) -> std::sync::Arc<RulesView> {
let relative = rules.depends_on_now();
let minute = if relative { now.get(..16).unwrap_or(now) } else { "" };
let k = key_of((cat.revision, serde_json::to_string(rules).unwrap_or_default(), editing, &folder, minute, i18n::language().code()));
if let Some((key, v)) = &self.rules_view
&& *key == k
{
return v.clone();
}
if relative {
lightcraft_catalog::rules::set_now(Some(now.to_string()));
}
let problems = rules.check_for(cat, editing.map(lightcraft_catalog::AlbumId));
let filter = lightcraft_catalog::Filter { rule_set: Some(rules.clone()), library_folder: folder, ..Default::default() };
let count = if problems.is_empty() { cat.query(&filter, &Default::default()).len() } else { 0 };
let v = std::sync::Arc::new(RulesView { problems, count, albums: panels::dialogs::album_entries(cat, editing) });
self.rules_view_scans += 1;
self.rules_view = Some((k, v.clone()));
v
}
/// The problems of every saved smart album that has some (`Catalog::smart_album_problems`),
/// for the sidebar's ⚠ marks: worked out again only when the catalog changes.
pub fn smart_album_problems(
&mut self,
cat: &lightcraft_catalog::Catalog,
) -> std::sync::Arc<std::collections::HashMap<lightcraft_catalog::AlbumId, Vec<lightcraft_catalog::rules::Problem>>> {
if let Some((rev, v)) = &self.smart_problems
&& *rev == cat.revision
{
return v.clone();
}
let v: std::sync::Arc<std::collections::HashMap<_, _>> = std::sync::Arc::new(
cat.albums().filter(|a| a.is_smart()).map(|a| (a.id, cat.smart_album_problems(a.id))).filter(|(_, p)| !p.is_empty()).collect(),
);
self.smart_problem_scans += 1;
self.smart_problems = Some((cat.revision, v.clone()));
v
}
pub fn album_counts(
&mut self,
cat: &lightcraft_catalog::Catalog,
@@ -1319,6 +1383,42 @@ mod cache_tests {
lightcraft_catalog::rules::set_now(None);
}
/// The rule dialog's problems, live count and album list are worked out once per change to the
/// catalog or the rules, not every frame; so are the sidebar's smart-album problems.
#[test]
fn rule_dialog_and_problems_are_cached_until_something_changes() {
let mut cat = Catalog::default();
let mut c = super::Caches::default();
for i in 0..3u64 {
cat.apply(photo(i + 1, "2026-09-30T11:00:00", if i == 0 { 5 } else { 1 })).unwrap();
}
let rules =
|v: u64| -> RuleSet { serde_json::from_value(serde_json::json!({"rules": [{"field": "rating", "op": "gte", "value": v}]})).unwrap() };
let now = "2026-09-30T12:00:00";
let view = c.rules_view(&cat, &rules(3), None, None, now);
assert_eq!((view.count, view.problems.len()), (1, 0));
for _ in 0..10 {
c.rules_view(&cat, &rules(3), None, None, now);
}
assert_eq!(c.rules_view_scans, 1, "unchanged frames reuse it");
assert_eq!(c.rules_view(&cat, &rules(1), None, None, now).count, 3, "a rule change");
assert_eq!(c.rules_view(&cat, &rules(9), None, None, now).problems.len(), 1);
cat.apply(Op::SetRating { id: PhotoId(2), rating: 4 }).unwrap();
assert_eq!(c.rules_view(&cat, &rules(3), None, None, now).count, 2, "a catalog change");
assert_eq!(c.rules_view_scans, 4);
// the sidebar's problems
cat.apply(Op::AddAlbum { album: lightcraft_catalog::Album::new(AlbumId(20), "Trip") }).unwrap();
cat.apply(smart(21, serde_json::json!({"rules": [{"field": "album", "op": "is", "value": 20}]}))).unwrap();
assert!(c.smart_album_problems(&cat).get(&AlbumId(21)).is_none_or(Vec::is_empty));
for _ in 0..10 {
c.smart_album_problems(&cat);
}
assert_eq!(c.smart_problem_scans, 1);
cat.apply(Op::RemoveAlbum { id: AlbumId(20) }).unwrap();
assert!(c.smart_album_problems(&cat).get(&AlbumId(21)).is_some_and(|p| !p.is_empty()), "its album is gone");
assert_eq!(c.smart_problem_scans, 2);
}
/// Without "in the last…" rules the clock never causes a rescan.
#[test]
fn absolute_rules_ignore_the_clock() {
+9 -13
View File
@@ -247,22 +247,18 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
let r = ui.add(egui::TextEdit::singleline(name).hint_text(crate::i18n::tr("Name")).desired_width(f32::INFINITY));
crate::widgets::register(ui.ctx(), "field:smartName", r.rect);
ui.add_space(6.0);
// checked before drawing, so each row marks its own problem
let env = crate::panels::rules_editor::Env {
problems: rules.check_for(&app.session.catalog, id.map(lightcraft_catalog::AlbumId)),
today: (app.session.clock)(),
albums: album_entries(&app.session.catalog, *id),
};
// the folder an album made from a folder view carries is not in the editor, but it counts
let folder = id.and_then(|id| app.session.catalog.album(lightcraft_catalog::AlbumId(id))).and_then(|a| a.smart.as_deref().and_then(|f| f.library_folder.clone()));
// problems, count and albums: cached until the catalog or the rules change (not per frame)
let now = (app.session.clock)();
let view = app.caches.rules_view(&app.session.catalog, rules, *id, folder, &now);
let env = crate::panels::rules_editor::Env { problems: view.problems.clone(), today: now, albums: view.albums.clone() };
egui::ScrollArea::vertical().max_height(360.0).auto_shrink([false, true]).show(ui, |ui| {
crate::panels::rules_editor::edit(ui, rules, "rules", 0, &[], &env);
});
let problems = rules.check_for(&app.session.catalog, id.map(lightcraft_catalog::AlbumId));
// the folder an album made from a folder view carries is not in the editor, but it counts
let folder = id.and_then(|id| app.session.catalog.album(lightcraft_catalog::AlbumId(id))).and_then(|a| a.smart.as_deref().and_then(|f| f.library_folder.clone()));
let f = lightcraft_catalog::Filter { rule_set: Some(rules.clone()), library_folder: folder, ..Default::default() };
let n = if problems.is_empty() { app.session.catalog.query(&f, &Default::default()).len() } else { 0 };
let n = view.count;
ui.add_space(4.0);
if problems.is_empty() {
if view.problems.is_empty() {
ui.label(
egui::RichText::new(crate::i18n::tr_format!("{n} photo{} match · updates automatically as photos change", if n == 1 { "" } else { "s" }, n = n))
.color(t.text_dim),
@@ -1034,7 +1030,7 @@ fn created_in(app: &mut LightcraftApp, command: &str, params: serde_json::Value)
/// The albums an Album rule in smart album `editing` picks from, in the sidebar's order: the album
/// itself and any smart album that tests it (testing it back would loop) are blocked, with why.
fn album_entries(cat: &lightcraft_catalog::Catalog, editing: Option<u64>) -> Vec<crate::album_picker::AlbumEntry> {
pub(crate) fn album_entries(cat: &lightcraft_catalog::Catalog, editing: Option<u64>) -> Vec<crate::album_picker::AlbumEntry> {
let editing = editing.map(lightcraft_catalog::AlbumId);
crate::album_picker::entries_from(cat, |a| match editing {
Some(e) if a.id == e => Some(crate::i18n::tr("This is the album you're editing").to_string()),
+1 -1
View File
@@ -728,7 +728,7 @@ fn albums_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, all: &AlbumKids, pare
app.session.target_album.filter(|t| app.session.catalog.album(*t).is_some()).or_else(|| app.session.catalog.quick_collection());
let label = if target == Some(a.id) { format!("{} +", a.name) } else { a.name.clone() };
// a smart album whose rules no longer check (an album they test was deleted) says so
let problems = app.session.catalog.smart_album_problems(a.id);
let problems = app.caches.smart_album_problems(&app.session.catalog).get(&a.id).cloned().unwrap_or_default();
let label = if problems.is_empty() { label } else { format!("⚠ {label}") };
let mut resp = row_sensed(app, ui, &format!("album:{}", a.id.0), icon, &label, None, Some(n), sel, indent, Sense::click_and_drag());
if !problems.is_empty() {