PERF-SMART: test a smart album's rules cheapest first

A rule set is now tested cheapest rule first (numbers, choices and yes/no, then lists and
dates, then text and paths, then smart albums), worked out once per prepared filter and per
smart album, not per photo. Photos matched are the same; the editor still shows the user's order.
While a smart album tests itself, or a chain of smart albums reaches the depth limit, rules are
tested as written, since there an answer can depend on which album is asked first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford
2026-10-29 09:14:26 -04:00
co-authored by Claude Opus 5.5
parent 77c0a6adfa
commit fec89459e0
5 changed files with 517 additions and 16 deletions
+87 -3
View File
@@ -294,6 +294,10 @@ pub struct Catalog {
/// Lookups derived from the above (never saved), see [`index`].
#[serde(skip)]
index: index::Indexes,
/// Each smart album's rules ready to run (cheapest rule first), worked out when first asked and
/// forgotten when an op adds or removes an album or changes rules.
#[serde(skip)]
plans: PlansMemo,
}
/// What of a photo an op changed: enough to tell which tallies of the library an edit leaves as they were (a
@@ -332,6 +336,32 @@ impl Aspects {
/// The most photos remembered as touched; older changes are then answered with "anything may have changed".
const MAX_TOUCHED: usize = 8192;
/// [`Catalog::plans`] as last worked out: derived from the albums, like [`index::Indexes`], so no
/// part of what a catalog is; a copy starts without it.
#[derive(Default)]
struct PlansMemo {
plans: std::sync::OnceLock<std::collections::HashMap<AlbumId, query::Prepared>>,
guard: std::sync::OnceLock<bool>,
}
impl Clone for PlansMemo {
fn clone(&self) -> Self {
Self::default()
}
}
impl PartialEq for PlansMemo {
fn eq(&self, _: &PlansMemo) -> bool {
true
}
}
impl std::fmt::Debug for PlansMemo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("PlansMemo")
}
}
/// A short memory of what the latest ops changed, so that what is derived from the catalog (the grid's list) can
/// follow a small edit without being worked out again from every photo.
#[derive(Clone, Debug, Default)]
@@ -442,12 +472,61 @@ impl Catalog {
/// no smart album).
pub fn album_contains(&self, id: AlbumId, p: &Photo) -> bool {
match self.albums.get(&id) {
Some(Album { smart: Some(rules), .. }) => self.smart_album_has(id, rules, &rules.prepare_one(), p),
Some(Album { smart: Some(rules), .. }) => match self.plans().get(&id) {
Some(plan) => self.smart_album_has(id, rules, plan, p),
None => self.smart_album_has(id, rules, &rules.prepare_one(self), p),
},
Some(a) => self.album_lists(a.id, p.id),
None => false,
}
}
/// Every smart album's [`Filter::prepare_one`], worked out once for the albums as they are
/// (so a photo asked about does not prepare the album's rules again).
fn plans(&self) -> &std::collections::HashMap<AlbumId, query::Prepared> {
self.plans.plans.get_or_init(|| self.albums.values().filter_map(|a| a.smart.as_ref().map(|rules| (a.id, rules.prepare_one(self)))).collect())
}
/// Whether rules must be tested as written, not cheapest first ([`Filter::prepare`]): some smart
/// album tests itself (a saved or damaged catalog; the app refuses to make one), or a chain of
/// smart albums testing smart albums is as long as [`rules::MAX_ALBUM_DEPTH`], where the
/// innermost hold nothing. There an answer can depend on which album was asked first (the
/// answers memo keeps what was worked out under the cut), so reordering could change who
/// matches. Worked out once for the albums as they are.
pub(crate) fn keeps_written_order(&self) -> bool {
*self.plans.guard.get_or_init(|| {
self.albums.values().any(|a| a.smart.is_some() && self.album_reaches(a.id, a.id)) || self.longest_chain() >= rules::MAX_ALBUM_DEPTH
})
}
/// The most smart albums one smart album leads through, itself included, by the smart albums
/// its rules and album filter test. Only asked of albums that lead back to none.
fn longest_chain(&self) -> usize {
let tests = |id: &AlbumId| -> Vec<AlbumId> {
let tested = self.albums.get(id).and_then(|a| a.smart.as_deref()).map(Filter::albums_tested).unwrap_or_default();
tested.into_iter().filter(|t| self.albums.get(t).is_some_and(|a| a.smart.is_some())).collect()
};
let mut chain: std::collections::HashMap<AlbumId, usize> = std::collections::HashMap::new();
for root in self.albums.values().filter(|a| a.smart.is_some()).map(|a| a.id) {
let mut stack = vec![root];
while let Some(a) = stack.last().copied() {
if chain.contains_key(&a) {
stack.pop();
continue;
}
let kids = tests(&a);
let pending: Vec<AlbumId> = kids.iter().copied().filter(|k| !chain.contains_key(k)).collect();
if pending.is_empty() {
chain.insert(a, 1 + kids.iter().filter_map(|k| chain.get(k)).copied().max().unwrap_or(0));
stack.pop();
} else {
stack.extend(pending);
}
}
}
chain.values().copied().max().unwrap_or(0)
}
/// Whether smart album `id` with `rules` (as [`Filter::prepare`]d) holds `p`.
fn smart_album_has(&self, id: AlbumId, rules: &Filter, prepared: &query::Prepared, p: &Photo) -> bool {
// guarded: a smart album testing smart albums can't loop or recurse without end
@@ -502,7 +581,7 @@ impl Catalog {
pub fn album_photos(&self, id: AlbumId) -> Vec<PhotoId> {
match self.albums.get(&id) {
Some(Album { smart: Some(rules), .. }) => {
let prepared = rules.prepare();
let prepared = rules.prepare(self);
self.photos.values().filter(|p| self.smart_album_has(id, rules, &prepared, p)).map(|p| p.id).collect()
}
Some(a) => a.photos.clone(),
@@ -515,7 +594,7 @@ impl Catalog {
match self.albums.get(&id) {
// counted in place: no id list is built just for its length
Some(Album { smart: Some(rules), .. }) => {
let prepared = rules.prepare();
let prepared = rules.prepare(self);
self.photos.values().filter(|p| self.smart_album_has(id, rules, &prepared, p)).count()
}
Some(a) => a.photos.iter().filter(|p| self.photos.get(p).is_some_and(|p| !p.deleted)).count(),
@@ -779,6 +858,7 @@ impl Catalog {
Op::SetHistoryCursor { id, cursor: std::mem::replace(&mut p.history_cursor, cursor) }
}
Op::AddAlbum { album } => {
self.plans = PlansMemo::default();
if self.albums.contains_key(&album.id) {
return Err(CatalogError::Invalid(format!("album {:?} exists", album.id)));
}
@@ -800,6 +880,7 @@ impl Catalog {
Op::RemoveAlbum { id }
}
Op::RemoveAlbum { id } => {
self.plans = PlansMemo::default();
if self.albums.values().any(|a| a.parent == Some(id)) {
return Err(CatalogError::Invalid("folder is not empty".into()));
}
@@ -860,6 +941,7 @@ impl Catalog {
Op::SetAlbumCover { id, cover: std::mem::replace(&mut a.cover, cover) }
}
Op::SetAlbumRules { id, rules } => {
self.plans = PlansMemo::default();
self.validate_rules(&rules)?;
let a = self.album_mut(id)?;
let Some(old) = a.smart.as_mut() else {
@@ -1209,6 +1291,8 @@ mod tests_lock;
#[cfg(test)]
mod tests_repair;
#[cfg(test)]
mod tests_rule_order;
#[cfg(test)]
mod tests_scale;
#[cfg(test)]
mod tests_tally;
+33 -13
View File
@@ -143,6 +143,9 @@ fn shuffle_rank(seed: u64, id: PhotoId) -> u64 {
pub(crate) struct Prepared {
root: Option<String>,
only: Option<std::collections::HashSet<PhotoId>>,
/// [`Filter::rule_set`] with its rules in the order to test them ([`crate::RuleSet::by_cost`]);
/// `None` tests them as written.
rules: Option<crate::RuleSet>,
}
/// The tokens of a search text: split at whitespace, except inside double quotes, so
@@ -327,18 +330,35 @@ impl Filter {
}
pub fn matches(&self, p: &Photo, cat: &Catalog) -> bool {
self.matches_in(p, cat, &self.prepare_one())
// one photo: its rules are tested as written (ordering them would cost more than it saves)
self.matches_in(p, cat, &Prepared { root: self.library_root(), only: None, rules: None })
}
/// [`Filter::prepare`] for testing one photo (the `only` ids are looked through, not hashed).
pub(crate) fn prepare_one(&self) -> Prepared {
Prepared { root: self.library_root(), only: None }
/// [`Filter::prepare`] for testing one photo at a time, with what each needs worked out once
/// (the `only` ids are looked through, not hashed).
pub(crate) fn prepare_one(&self, cat: &Catalog) -> Prepared {
Prepared { root: self.library_root(), only: None, rules: self.ordered_rules(cat) }
}
/// The rule set to test, cheapest rule first: once per prepared filter, not per photo. `None`
/// (as written) while any smart album tests itself or a chain is too deep ([`Catalog::keeps_written_order`]), where the
/// order could change which photos match.
fn ordered_rules(&self, cat: &Catalog) -> Option<crate::RuleSet> {
if cat.keeps_written_order() {
return None;
}
self.rule_set.as_ref().map(|rs| rs.by_cost(cat))
}
/// What [`Filter::matches_in`] needs worked out once for a pass over many photos rather than
/// once per photo: the library folder's identity and the `only` ids as a set.
pub(crate) fn prepare(&self) -> Prepared {
Prepared { root: self.library_root(), only: (!self.only.is_empty()).then(|| self.only.iter().copied().collect()) }
/// once per photo: the library folder's identity, the `only` ids as a set and the rules in the
/// order to test them.
pub(crate) fn prepare(&self, cat: &Catalog) -> Prepared {
Prepared {
root: self.library_root(),
only: (!self.only.is_empty()).then(|| self.only.iter().copied().collect()),
rules: self.ordered_rules(cat),
}
}
/// [`Filter::library_folder`] as a folder identity ([`folder_key`]); `None` without a choice.
@@ -382,7 +402,7 @@ impl Filter {
return false;
}
if let Some(rs) = &self.rule_set
&& !rs.matches(p, cat)
&& !prepared.rules.as_ref().unwrap_or(rs).matches(p, cat)
{
return false;
}
@@ -482,9 +502,9 @@ impl Catalog {
/// album's rules, the filter bar), as long as they are still where the view looks (`within`: in the library,
/// the album, the folder; not deleted).
pub fn query_keeping(&self, filter: &Filter, sort: &Sort, keep: &std::collections::HashSet<PhotoId>, within: &Filter) -> Vec<PhotoId> {
let prepared = filter.prepare();
let prepared = filter.prepare(self);
// `within` only decides for the few kept photos: worked out per photo, not hashed
let within_prepared = within.prepare_one();
let within_prepared = within.prepare_one(self);
let mut v: Vec<&Photo> = self
.photos()
.map(|p| p.as_ref())
@@ -502,7 +522,7 @@ impl Catalog {
pub fn follow(&self, was: &[PhotoId], touched: &[PhotoId], filter: &Filter, sort: &Sort, within: Option<&Filter>) -> Vec<PhotoId> {
let listed: std::collections::HashSet<PhotoId> = was.iter().copied().filter(|id| touched.binary_search(id).is_ok()).collect();
let mut ids: Vec<PhotoId> = was.iter().copied().filter(|id| !listed.contains(id)).collect();
let (prepared, within_prepared) = (filter.prepare_one(), within.map(Filter::prepare_one));
let (prepared, within_prepared) = (filter.prepare_one(self), within.map(|w| w.prepare_one(self)));
for id in touched {
let Some(p) = self.photo(*id) else { continue };
let kept = within.zip(within_prepared.as_ref()).is_some_and(|(w, wp)| listed.contains(id) && w.matches_in(p, self, wp));
@@ -517,7 +537,7 @@ impl Catalog {
/// How many photos match `filter`: [`Catalog::query`]'s length without its sorting.
pub fn count(&self, filter: &Filter) -> usize {
let prepared = filter.prepare();
let prepared = filter.prepare(self);
self.photos().filter(|p| filter.matches_in(p, self, &prepared)).count()
}
@@ -574,7 +594,7 @@ impl Catalog {
pub fn people_in(&self, filter: &Filter) -> Vec<Person> {
let filter = Filter { person: None, ..filter.clone() };
let mut m: std::collections::HashMap<String, (Person, f64)> = Default::default();
let prepared = filter.prepare();
let prepared = filter.prepare(self);
for p in self.photos().filter(|p| filter.matches_in(p, self, &prepared)) {
let mut seen: Vec<String> = Vec::new();
for r in p.meta.regions.iter().filter(|r| r.kind == lightcraft_meta::RegionKind::Face) {
+64
View File
@@ -488,6 +488,12 @@ fn pass_answer(id: crate::AlbumId, photo: crate::PhotoId) -> Option<bool> {
})
}
#[cfg(test)]
thread_local! {
/// Tests: test rules in the order they were written, as before [`RuleSet::by_cost`].
pub(crate) static WRITTEN_ORDER: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
}
#[cfg(test)]
thread_local! {
/// How many times a smart album's rules were worked out on this thread (tests count them).
@@ -1048,6 +1054,48 @@ impl Rule {
}
}
/// How much work one rule is, in tiers: [`RuleSet::by_cost`] tests the cheaper first.
const COST_SCALAR: u8 = 0;
const COST_SMALL: u8 = 1;
const COST_TEXT: u8 = 2;
const COST_ALBUM: u8 = 3;
impl Rule {
/// A rule's tier: 0 numbers, choices and yes/no questions about one photo's own record (and
/// anything that is false at once: an unknown field or a missing album); 1 dates, keyword and
/// people lists, short single fields, an index lookup and a plain album's list (a folder holds no photos here: an empty lookup); 2 text built
/// from several fields and file paths (they copy and lower-case the text); 3 a smart album,
/// which runs a whole rule set for the photo. A group costs as much as its dearest rule.
fn cost(&self, cat: &Catalog) -> u8 {
match self {
Rule::Group { group } => group.rules.iter().map(|r| r.cost(cat)).max().unwrap_or(COST_SCALAR),
Rule::Field { field, value, .. } => match field.as_str() {
"album" => album_rule_id(value).map_or(COST_SCALAR, |id| match cat.albums.get(&id) {
None => COST_SCALAR,
Some(a) if a.smart.is_some() => COST_ALBUM,
Some(_) => COST_SMALL,
}),
"text" | "filePath" | "location" => COST_TEXT,
"keywords" | "person" | "personCount" | "stacked" | "sameContent" | "copyName" | "fileName" | "extension" | "format" | "title"
| "caption" | "altText" | "city" | "state" | "country" | "camera" | "lens" | "creator" | "copyright" | "captureDate"
| "importDate" | "editDate" | "shutterSpeed" | "aspect" => COST_SMALL,
_ => COST_SCALAR,
},
}
}
/// This rule with its nested groups ordered by cost, and its tier.
fn by_cost(&self, cat: &Catalog) -> (u8, Rule) {
match self {
Rule::Group { group } => {
let group = group.by_cost(cat);
(group.rules.iter().map(|r| r.cost(cat)).max().unwrap_or(COST_SCALAR), Rule::Group { group })
}
field => (field.cost(cat), field.clone()),
}
}
}
impl RuleSet {
pub fn matches(&self, p: &Photo, cat: &Catalog) -> bool {
if self.rules.is_empty() {
@@ -1060,6 +1108,22 @@ impl RuleSet {
}
}
/// This rule set with its rules (and those of nested groups) in the order to test them: the
/// cheapest first ([`Rule::cost`]), the user's order among equals. Rules are pure (they read
/// the photo and the catalog; a smart album's answer is the same however it is reached as long
/// as no smart album tests itself and no chain is cut by the depth limit, which callers check first: [`Catalog::keeps_written_order`]),
/// so for all, any and none the photos that match are the same in any order; only the work
/// differs. Worked out once for a pass, never per photo. The saved rules are untouched.
pub(crate) fn by_cost(&self, cat: &Catalog) -> RuleSet {
#[cfg(test)]
if WRITTEN_ORDER.get() {
return self.clone();
}
let mut rules: Vec<(u8, Rule)> = self.rules.iter().map(|r| r.by_cost(cat)).collect();
rules.sort_by_key(|(cost, _)| *cost);
RuleSet { mode: self.mode, rules: rules.into_iter().map(|(_, r)| r).collect() }
}
/// Whether matches depend on the clock ("in the last…" rules, nested groups included): the
/// same photos can enter or leave the set without any catalog change.
pub fn depends_on_now(&self) -> bool {
+324
View File
@@ -0,0 +1,324 @@
//! A rule set is tested cheapest rule first ([`RuleSet::by_cost`]), once per pass; what matches
//! is what testing the rules in the order they were written gives.
//!
//! Scenarios:
//! - Given random libraries (Unicode and Windows paths, nested groups, album rules that test
//! smart albums and missing albums), then every album, query and count gives what the written
//! order gives, for all, any and none.
//! - Given a library where smart albums test themselves (the app refuses to make one; a saved or
//! damaged catalog can hold it, and there the answer can depend on which album is asked first),
//! then the rules are tested as written, so the answers are the written order's, as before.
//! - Given `all[album is <smart album>, <a rule no photo meets>]`, then the smart album is never
//! asked about a photo: the cheap rule ends it first (written first, it ran for every photo).
//! - Given rules in any order, then the saved rules and the order the editor shows are untouched.
use serde_json::json;
use crate::rules::{EVALUATED, WRITTEN_ORDER};
use crate::*;
/// A small generator: the same sequence every run.
struct Rng(u64);
impl Rng {
fn next(&mut self, n: u64) -> u64 {
self.0 = self.0.wrapping_mul(6364136223846793005).wrapping_add(1442695040888963407);
(self.0 >> 33) % n
}
fn pick<'a, T>(&mut self, of: &'a [T]) -> &'a T {
&of[self.next(of.len() as u64) as usize]
}
}
const PATHS: &[&str] = &[
"/Volumes/share/photos/Asie/Japon/Été/IMG_0001.jpg",
"/Volumes/share/photos/ASIE/japon/IMG_0002.JPG",
"C:\\Users\\me\\Pictures\\Ünïcode\\Tokyo\\IMG_0003.jpg",
"/Volumes/share/photos/Straße/İstanbul/IMG_0004.cr2",
"/Volumes/share/photos/東京/渋谷/IMG_0005.jpg",
"/Volumes/share/photos/Asie/Chine/clip.mp4",
"relative/dir/IMG_0007.jpg",
];
const WORDS: &[&str] =
&["asie", "Japon", "été", "ÜNÏCODE", "tokyo", "straße", "i̇stanbul", "東京", "photos\\asie", "photos/Asie/", " ", "", "img_000"];
fn library(rng: &mut Rng) -> (Catalog, Vec<PhotoId>) {
let mut c = Catalog::new();
let ids: Vec<PhotoId> = (0..24)
.map(|i| {
let id = c.alloc_photo_id();
let path = if i == 5 { None } else { Some(rng.pick(PATHS).to_string()) };
let source = path.map_or(Source::Demo { scene: 1 }, |path| Source::File { path });
let name = format!("IMG_{i:04}.{}", ["jpg", "cr2", "mp4"][rng.next(3) as usize]);
let mut p = Photo::new(id, source, &name, "JPEG", 6000, 4000, "2026-09-30T10:00:00");
p.rating = rng.next(6) as u8;
p.flag = [Flag::None, Flag::Pick, Flag::Reject][rng.next(3) as usize];
p.label = [None, Some(ColorLabel::Red), Some(ColorLabel::Blue)][rng.next(3) as usize];
p.meta.keywords = (0..rng.next(4)).map(|k| format!("kw{}", k + rng.next(2))).collect();
p.captured = Some(format!("20{}-0{}-1{}T10:00:00", 10 + rng.next(16), 1 + rng.next(9), rng.next(9)));
c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
id
})
.collect();
(c, ids)
}
fn leaf(rng: &mut Rng, albums: &[AlbumId]) -> serde_json::Value {
let field = |f: &str, op: &str, v: serde_json::Value| json!({"field": f, "op": op, "value": v});
match rng.next(14) {
0 => field("rating", ["gte", "lte", "is", "gt", "lt"][rng.next(5) as usize], json!(rng.next(6))),
1 => field("flag", ["is", "isNot"][rng.next(2) as usize], json!(["pick", "reject", "none"][rng.next(3) as usize])),
2 => field("label", ["is", "isNot"][rng.next(2) as usize], json!(["red", "blue", "none"][rng.next(3) as usize])),
3 => field("kind", ["is", "isNot"][rng.next(2) as usize], json!(["image", "raw", "video"][rng.next(3) as usize])),
4 => field("keywordCount", ["lte", "gte"][rng.next(2) as usize], json!(rng.next(4))),
5 => field("keywords", ["contains", "notContains", "isEmpty"][rng.next(3) as usize], json!(format!("kw{}", rng.next(3)))),
6 => field("captureDate", ["before", "after"][rng.next(2) as usize], json!(format!("20{}", 10 + rng.next(16)))),
7 | 8 => field("filePath", ["contains", "notContains", "startsWith", "is", "isEmpty"][rng.next(5) as usize], json!(*rng.pick(WORDS))),
9 => field("fileName", "contains", json!(["img_000", ".cr2", "mp4"][rng.next(3) as usize])),
10 => field("text", ["contains", "notContains"][rng.next(2) as usize], json!(*rng.pick(WORDS))),
// an album that may not exist (99) or may be on a loop
_ => {
let album = if rng.next(8) == 0 { AlbumId(99) } else { *rng.pick(albums) };
field("album", ["is", "isNot"][rng.next(2) as usize], json!(album.0))
}
}
}
fn rule_set(rng: &mut Rng, albums: &[AlbumId], depth: u32) -> serde_json::Value {
let rules: Vec<serde_json::Value> = (0..rng.next(5))
.map(|_| if depth < 2 && rng.next(4) == 0 { json!({"group": rule_set(rng, albums, depth + 1)}) } else { leaf(rng, albums) })
.collect();
let mode = ["all", "any", "none"][rng.next(3) as usize];
json!({"match": mode, "rules": rules})
}
fn filter(rules: serde_json::Value) -> Filter {
Filter { rule_set: Some(serde_json::from_value(rules).unwrap()), ..Default::default() }
}
/// What a catalog answers for every album, photo and a few queries.
fn answers(c: &Catalog, albums: &[AlbumId], photos: &[PhotoId], queries: &[Filter]) -> String {
let mut out = String::new();
for a in albums {
let held: Vec<bool> = photos.iter().map(|p| c.album_contains(*a, c.photo(*p).unwrap())).collect();
out += &format!("{a:?} {held:?} {:?} {}\n", c.album_photos(*a), c.album_count(*a));
}
for p in photos {
out += &format!("{p:?} {:?}\n", c.albums_of(*p));
}
for q in queries {
out += &format!("{:?} {}\n", c.query(q, &Sort::default()), c.count(q));
}
out
}
#[test]
fn the_cheapest_first_order_matches_what_the_written_order_matches() {
let chained = same_answers_in_random_libraries(Rng(11), false);
assert!(chained > 20, "the generator makes smart albums that test smart albums: {chained}");
}
#[test]
fn a_library_with_loops_answers_as_written_and_does_not_hang() {
let looping = same_answers_in_random_libraries(Rng(5), true);
assert!(looping > 20, "the generator makes loops: {looping}");
}
/// Albums of random libraries and queries answer the same cheapest first and as written; returns
/// how many smart albums tested a smart album (`loops`: one that leads back to itself).
fn same_answers_in_random_libraries(mut rng: Rng, loops: bool) -> usize {
let mut chained = 0;
for round in 0..150 {
let (mut c, photos) = library(&mut rng);
let plain = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { photos: photos.iter().copied().step_by(3).collect(), ..Album::new(plain, "Plain") } }).unwrap();
let folder = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { folder: true, ..Album::new(folder, "F") } }).unwrap();
let smarts: Vec<AlbumId> = (0..6)
.map(|i| {
let id = c.alloc_album_id();
let parent = (i % 3 == 0).then_some(folder);
c.apply(Op::AddAlbum { album: Album { parent, smart: Some(Box::default()), ..Album::new(id, format!("S{i}")) } }).unwrap();
id
})
.collect();
let albums: Vec<AlbumId> = [plain, folder].into_iter().chain(smarts.iter().copied()).collect();
for (i, s) in smarts.iter().enumerate() {
// (without loops a smart album tests only the albums made before it)
let testable = if loops { &albums[..] } else { &albums[..2 + i] };
let _ = c.apply(Op::SetAlbumRules { id: *s, rules: Box::new(filter(rule_set(&mut rng, testable, 0))) });
}
let queries: Vec<Filter> = (0..4).map(|_| filter(rule_set(&mut rng, &albums, 0))).collect();
// (the copy starts without what the catalog worked out, so each is worked out under its own order)
let written = c.clone();
let ordered_answers = answers(&c, &albums, &photos, &queries);
WRITTEN_ORDER.set(true);
let written_answers = answers(&written, &albums, &photos, &queries);
WRITTEN_ORDER.set(false);
assert_eq!(ordered_answers, written_answers, "round {round}: {}", c.to_snapshot());
let reaching = |s: &AlbumId| {
smarts.iter().any(|t| if loops { c.album_reaches(*t, *s) && c.album_reaches(*s, *t) } else { t != s && c.album_reaches(*s, *t) })
};
chained += smarts.iter().filter(|s| reaching(s)).count();
assert_eq!(c.keeps_written_order(), smarts.iter().any(|s| c.album_reaches(*s, *s)));
assert!(loops || !c.keeps_written_order(), "round {round}: {}", c.to_snapshot());
}
chained
}
#[test]
fn a_cheap_rule_that_fails_ends_the_test_before_a_smart_album_runs() {
let mut c = Catalog::new();
let photos: Vec<PhotoId> = (0..20)
.map(|i| {
let id = c.alloc_photo_id();
let p = Photo::new(id, Source::File { path: format!("/a/b/{i}.jpg") }, &format!("{i}.jpg"), "JPEG", 6000, 4000, "2026-09-30T10:00:00");
c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
id
})
.collect();
let inner = c.alloc_album_id();
c.apply(Op::AddAlbum {
album: Album {
smart: Some(Box::new(filter(json!({"rules": [{"field": "filePath", "op": "contains", "value": "/a/"}]})))),
..Album::new(inner, "Inner")
},
})
.unwrap();
let rules = |first: serde_json::Value, second: serde_json::Value| json!({"match": "all", "rules": [first, second]});
let album = json!({"field": "album", "op": "is", "value": inner.0});
// no photo is rated 6 stars (a rating is at most 5)
let never = json!({"field": "rating", "op": "gte", "value": 6});
let outer = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { smart: Some(Box::new(filter(rules(album.clone(), never.clone())))), ..Album::new(outer, "Outer") } })
.unwrap();
EVALUATED.set(0);
assert_eq!(c.album_count(outer), 0);
// (the outer album's own rules count as one run for each photo; its inner album must run for none)
assert_eq!(EVALUATED.get(), photos.len(), "only the outer album ran, once per photo");
// the same photos whichever way round the rules are written
let swapped = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { smart: Some(Box::new(filter(rules(never, album)))), ..Album::new(swapped, "Swapped") } }).unwrap();
assert_eq!((c.album_count(swapped), c.album_photos(swapped)), (0, vec![]));
}
#[test]
fn rules_are_ordered_by_cost_without_touching_the_saved_ones() {
let mut c = Catalog::new();
let plain = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album::new(plain, "Plain") }).unwrap();
let smart = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..Album::new(smart, "Smart") } }).unwrap();
let written: RuleSet = serde_json::from_value(json!({"match": "all", "rules": [
{"field": "album", "op": "is", "value": smart.0},
{"field": "filePath", "op": "contains", "value": "x"},
{"group": {"match": "any", "rules": [
{"field": "album", "op": "isNot", "value": smart.0},
{"field": "rating", "op": "gte", "value": 3}]}},
{"field": "keywords", "op": "contains", "value": "k"},
{"field": "album", "op": "is", "value": plain.0},
{"field": "rating", "op": "gte", "value": 3},
{"field": "album", "op": "is", "value": 99},
{"field": "flag", "op": "is", "value": "pick"}]}))
.unwrap();
let before = written.clone();
let ordered = written.by_cost(&c);
assert_eq!(written, before, "the saved rules stay as the user wrote them");
let names: Vec<String> = ordered
.rules
.iter()
.map(|r| match r {
Rule::Group { .. } => "group".to_string(),
Rule::Field { field, op, .. } => format!("{field} {op}"),
})
.collect();
// scalars (a missing album is false at once), then lists and a plain album, then text, then
// smart albums and the group that holds one; equals keep the written order
assert_eq!(names, ["rating gte", "album is", "flag is", "keywords contains", "album is", "filePath contains", "album is", "group"]);
let inner: Vec<&str> = match ordered.rules.get(7) {
Some(Rule::Group { group }) => group.rules.iter().map(|r| if let Rule::Field { field, .. } = r { field.as_str() } else { "group" }).collect(),
_ => Vec::new(),
};
assert_eq!(inner, ["rating", "album"], "nested groups are ordered too");
}
/// B = all[album is C], C = all[album isNot B], A = all[any[album is C, flag is pick], album is B]:
/// asked in the order the rules are written, a picked photo is not in A; asked cheapest first
/// (the flag, then the albums) it was. With a loop the order stays as written.
#[test]
fn smart_albums_that_test_each_other_keep_the_written_order() {
let mut c = Catalog::new();
let mut p = Photo::new(c.alloc_photo_id(), Source::File { path: "/a/1.jpg".into() }, "1.jpg", "JPEG", 6000, 4000, "2026-09-30T10:00:00");
p.flag = Flag::Pick;
let photo = p.id;
c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
let ids: Vec<AlbumId> = ["A", "B", "C"]
.iter()
.map(|name| {
let id = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..Album::new(id, *name) } }).unwrap();
id
})
.collect();
let (a, b, cc) = (ids[0], ids[1], ids[2]);
let album = |op: &str, id: AlbumId| json!({"field": "album", "op": op, "value": id.0});
let set = |c: &mut Catalog, id: AlbumId, rules: serde_json::Value| c.apply(Op::SetAlbumRules { id, rules: Box::new(filter(rules)) }).unwrap();
set(&mut c, b, json!({"match": "all", "rules": [album("is", cc)]}));
set(&mut c, cc, json!({"match": "all", "rules": [album("isNot", b)]}));
set(
&mut c,
a,
json!({"match": "all", "rules": [
{"group": {"match": "any", "rules": [album("is", cc), {"field": "flag", "op": "is", "value": "pick"}]}},
album("is", b)]}),
);
let written = c.clone();
let ask = |c: &Catalog| {
ids.iter().map(|id| (c.album_contains(*id, c.photo(photo).unwrap()), c.album_photos(*id), c.album_count(*id))).collect::<Vec<_>>()
};
WRITTEN_ORDER.set(true);
let as_written = ask(&written);
WRITTEN_ORDER.set(false);
assert!(c.keeps_written_order());
assert_eq!(ask(&c), as_written);
}
/// A = all[any[album is D1, flag is pick], album is W], D1 tests D2 … D30, D30 tests W, W tests Z,
/// Z = rating ≥ 0: asked from A, the chain is cut by the depth limit, and asked cheapest first (the
/// flag, then the albums) the cut falls elsewhere. Chains that long keep the written order.
#[test]
fn a_chain_as_deep_as_the_limit_keeps_the_written_order() {
let mut c = Catalog::new();
let mut p = Photo::new(c.alloc_photo_id(), Source::File { path: "/a/1.jpg".into() }, "1.jpg", "JPEG", 6000, 4000, "2026-09-30T10:00:00");
p.flag = Flag::Pick;
let photo = p.id;
c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap();
let new_smart = |c: &mut Catalog, name: &str| {
let id = c.alloc_album_id();
c.apply(Op::AddAlbum { album: Album { smart: Some(Box::default()), ..Album::new(id, name) } }).unwrap();
id
};
let a = new_smart(&mut c, "A");
let chain: Vec<AlbumId> = (1..=30).map(|i| new_smart(&mut c, &format!("D{i}"))).collect();
let (w, z) = (new_smart(&mut c, "W"), new_smart(&mut c, "Z"));
let album = |id: AlbumId| json!({"field": "album", "op": "is", "value": id.0});
let set = |c: &mut Catalog, id: AlbumId, rules: serde_json::Value| c.apply(Op::SetAlbumRules { id, rules: Box::new(filter(rules)) }).unwrap();
set(
&mut c,
a,
json!({"match": "all", "rules": [{"group": {"match": "any", "rules": [album(chain[0]), {"field": "flag", "op": "is", "value": "pick"}]}}, album(w)]}),
);
for pair in chain.windows(2) {
set(&mut c, pair[0], json!({"match": "all", "rules": [album(pair[1])]}));
}
set(&mut c, chain[29], json!({"match": "all", "rules": [album(w)]}));
set(&mut c, w, json!({"match": "all", "rules": [album(z)]}));
set(&mut c, z, json!({"match": "all", "rules": [{"field": "rating", "op": "gte", "value": 0}]}));
let written = c.clone();
let ask = |c: &Catalog| [a, w, z, chain[0]].map(|id| (c.album_contains(id, c.photo(photo).unwrap()), c.album_photos(id), c.album_count(id)));
WRITTEN_ORDER.set(true);
let as_written = ask(&written);
WRITTEN_ORDER.set(false);
assert!(c.keeps_written_order());
assert_eq!(ask(&c), as_written);
}
+9
View File
@@ -157,6 +157,15 @@
the full count took about 14 s instead of about 23 s on a heavily loaded machine; the rest is the text rules of
the albums themselves. The counts are the same as before, including for albums that loop through each other.
### Smart albums open and count faster on large libraries
- A smart album that tests another smart album and also a file path or other text (for example "in Rejects, and the
path contains …") now checks the quick rules first and the slow ones last, so most photos are ruled out before the
other album is asked about them. The photos in the album and their order are the same, and the rules still show in
the order you wrote them. (A library whose smart albums test each other in a circle, which LightCraft doesn't let
you make, or test each other 32 deep, keeps testing the rules as written.) On a 312,000-photo test library, opening
one such album went from about 224 ms to about 100 ms, and opening 51 of them in turn from about 19 s to about
5.8 s (a busy machine, so rough numbers).
### Sharper resampling in HDR and panorama merges
- Merged photos (HDR and panorama) are resampled with a sharper filter (Lanczos-3 instead of bilinear). Moving a
photo by half a pixel used to lose a third of its finest detail; now it keeps most of it, without halos around