From f80c3778bd3be51d8fc943fdf469db8de0ea027d Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Sun, 11 Oct 2026 17:16:00 -0500 Subject: [PATCH 1/3] perf(catalog): #1057 count develop-only ops in a separate library revision Catalog::library_revision is bumped by every op except develop-only ones (SetDevelop, SetHistory, PushHistory, SetVersions and batches of them); revision keeps counting every op. Filter/Sort/RuleSet::reads_develop and Catalog::{filter,album,smart_albums}_read(s)_develop say whether a view can change with a develop edit (through Album rules, loops ended by a seen-set), and Catalog::view_revision gives the key such a view caches on. Rule fields are classified by an allow-list, so a new field counts as reading develop state until it is classified on purpose. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/journal.rs | 2 +- crates/catalog/src/lib.rs | 11 ++ crates/catalog/src/revision.rs | 275 +++++++++++++++++++++++++++++++++ crates/engine/src/library.rs | 2 +- 4 files changed, 288 insertions(+), 2 deletions(-) create mode 100644 crates/catalog/src/revision.rs diff --git a/crates/catalog/src/journal.rs b/crates/catalog/src/journal.rs index 697ddf01..7849640f 100644 --- a/crates/catalog/src/journal.rs +++ b/crates/catalog/src/journal.rs @@ -454,7 +454,7 @@ impl Journal { Err(e) => log::warn!("catalog: can't write the format v{VERSION} snapshot: {e}"), } } - catalog.revision = 0; + catalog.set_revision(0); Ok((j, catalog, report)) } diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs index cc24266f..e15341ae 100644 --- a/crates/catalog/src/lib.rs +++ b/crates/catalog/src/lib.rs @@ -21,6 +21,7 @@ pub mod local; pub mod lock; pub mod model; pub mod query; +pub mod revision; pub mod rules; pub mod safe_file; pub mod stacks; @@ -38,6 +39,7 @@ pub use local::{DEFAULT_FORGET_DAYS, ForgetPlan, folder_of}; pub use lock::{LibraryLock, LockError, LockOwner}; pub use model::*; pub use query::{DateGroup, Filter, Person, RatingOp, Sort, SortKey, mix64}; +pub use revision::ViewRevision; pub use rules::{Match, Rule, RuleSet}; use serde::{Deserialize, Serialize}; pub use store::{FsStore, MemStore, Store}; @@ -275,6 +277,11 @@ pub struct Catalog { /// Increments on every applied op. #[serde(skip)] pub revision: u64, + /// Increments on every applied op that changes more than photos' develop state + /// ([`Op::is_develop_only`]): what views that don't read develop state key on (see + /// [`revision`]), so a slider drag doesn't work them out again on every tick. + #[serde(skip)] + pub library_revision: u64, } impl Catalog { @@ -479,8 +486,12 @@ impl Catalog { /// Apply an op; returns its inverse. On error nothing changes. pub fn apply(&mut self, op: Op) -> Result { + let develop_only = op.is_develop_only(); let inv = self.apply_inner(op)?; self.revision += 1; + if !develop_only { + self.library_revision += 1; + } Ok(inv) } diff --git a/crates/catalog/src/revision.rs b/crates/catalog/src/revision.rs new file mode 100644 index 00000000..04f31261 --- /dev/null +++ b/crates/catalog/src/revision.rs @@ -0,0 +1,275 @@ +//! What changed, for caches: [`Catalog::revision`] counts every applied op, and +//! [`Catalog::library_revision`] every op except those that only change photos' develop state +//! ([`Op::is_develop_only`]: settings, edit time, History, snapshots). A slider drag applies a +//! develop op per tick; views that don't read develop state (most filters and sorts, the sidebar's +//! counts and trees) key on the library revision and aren't worked out again on every tick. +//! +//! Some views *do* read develop state: "edited" filters and rules, Edit Date, Cropped, Treatment +//! and the size rules (crop and orientation change a photo's shown size). Those key on the full +//! revision ([`Catalog::view_revision`] with `reads_develop`), through the smart albums a filter +//! or rule tests too. + +use std::collections::HashSet; + +use crate::{AlbumId, Catalog, Filter, Op, Rule, RuleSet, Sort, SortKey}; + +/// What a cached view of the catalog was worked out from: the library revision, and the full +/// revision as well when the view reads develop state. Equal values mean the view is unchanged. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] +pub struct ViewRevision { + pub library: u64, + /// [`Catalog::revision`] for views that read develop state; `None` for the others. + pub all: Option, +} + +/// Smart-album rule fields whose matches don't depend on a photo's develop state. Every other +/// field (including one added later and not listed here) counts as reading it, so a forgotten +/// field costs a query per slider tick, never a stale view. +const FIELDS_WITHOUT_DEVELOP: &[&str] = &[ + "rating", + "flag", + "label", + "text", + // an album rule reads what that album holds: see `Catalog::filter_reads_develop` + "album", + "virtualCopy", + "copyName", + "stacked", + "fileName", + "extension", + "filePath", + "kind", + "format", + "duration", + "captureDate", + "importDate", + "keywords", + "keywordCount", + "person", + "personCount", + "title", + "caption", + "altText", + "creator", + "copyright", + "copyrightStatus", + "camera", + "lens", + "focalLength", + "aperture", + "shutterSpeed", + "iso", + "location", + "city", + "state", + "country", + "hasGps", + "sharpness", + "bestOfGroup", +]; + +/// Whether a rule on `field` can match differently after a develop edit (Has Edits, Edit Date, +/// Cropped, Treatment, and the sizes a crop or rotation changes). +pub fn field_reads_develop(field: &str) -> bool { + !FIELDS_WITHOUT_DEVELOP.contains(&field) +} + +impl Op { + /// Whether the op only changes photos' develop state: their settings and edit time, History + /// and snapshots. Such ops bump [`Catalog::revision`] but not [`Catalog::library_revision`]. + pub fn is_develop_only(&self) -> bool { + let mut next = vec![self]; + while let Some(op) = next.pop() { + match op { + Op::SetDevelop { .. } | Op::SetHistory { .. } | Op::PushHistory { .. } | Op::SetVersions { .. } => {} + Op::Batch { ops } => next.extend(ops), + _ => return false, + } + } + true + } +} + +impl RuleSet { + /// Whether any of the rules (nested groups included) tests develop state itself; the albums + /// an Album rule tests are [`Catalog::filter_reads_develop`]'s concern. + pub fn reads_develop(&self) -> bool { + let mut next = vec![self]; + while let Some(set) = next.pop() { + for rule in &set.rules { + match rule { + Rule::Group { group } => next.push(group), + Rule::Field { field, .. } if field_reads_develop(field) => return true, + Rule::Field { .. } => {} + } + } + } + false + } +} + +impl Filter { + /// Whether the filter itself tests develop state: Edited, an `edited:` search token or a rule + /// that does. The albums it tests are [`Catalog::filter_reads_develop`]'s concern. + pub fn reads_develop(&self) -> bool { + self.edited.is_some() + || self.text.split_whitespace().any(|t| t.to_lowercase().split_once(':').is_some_and(|(field, _)| field == "edited")) + || self.rule_set.as_ref().is_some_and(RuleSet::reads_develop) + } +} + +impl Sort { + /// Whether the order depends on develop state (Edit Date: every edit moves the photo). + pub fn reads_develop(&self) -> bool { + self.key == SortKey::EditDate + } +} + +impl Catalog { + /// Whether which photos `filter` lets through can change with a develop edit: the filter's + /// own fields and rules, and those of every smart album it tests, directly or through other + /// smart albums (loops in saved rules end the search). + pub fn filter_reads_develop(&self, filter: &Filter) -> bool { + if filter.reads_develop() { + return true; + } + self.albums_read_develop(filter.albums_tested()) + } + + /// Whether what album `id` holds can change with a develop edit (a smart album whose rules, + /// or those of the smart albums they test, read develop state). Plain albums never do. + pub fn album_reads_develop(&self, id: AlbumId) -> bool { + self.albums_read_develop(vec![id]) + } + + /// Whether any smart album's contents can change with a develop edit (the sidebar's counts). + pub fn smart_albums_read_develop(&self) -> bool { + self.albums_read_develop(self.albums().filter(|a| a.is_smart()).map(|a| a.id).collect()) + } + + fn albums_read_develop(&self, mut next: Vec) -> bool { + let mut seen: HashSet = HashSet::new(); + while let Some(id) = next.pop() { + if !seen.insert(id) { + continue; + } + let Some(rules) = self.album(id).and_then(|a| a.smart.as_deref()) else { continue }; + if rules.reads_develop() { + return true; + } + next.extend(rules.albums_tested()); + } + false + } + + /// The revision a cached view keys on: the library revision, plus the full revision when the + /// view reads develop state (see the module docs). + pub fn view_revision(&self, reads_develop: bool) -> ViewRevision { + ViewRevision { library: self.library_revision, all: reads_develop.then_some(self.revision) } + } + + /// Count revisions from `at` (both of them), as a library just loaded does. + pub fn set_revision(&mut self, at: u64) { + self.revision = at; + self.library_revision = at; + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use serde_json::json; + + use super::*; + use crate::{Album, Photo, PhotoId, Source}; + + fn photo(id: u64) -> Photo { + Photo::new(PhotoId(id), Source::Demo { scene: 0 }, &format!("IMG_{id}.jpg"), "JPEG", 6000, 4000, "2026-01-01T00:00:00") + } + + fn rules(v: serde_json::Value) -> RuleSet { + serde_json::from_value(v).unwrap() + } + + fn smart(id: u64, rules: RuleSet) -> Op { + let mut a = Album::new(AlbumId(id), format!("Smart {id}")); + a.smart = Some(Box::new(Filter { rule_set: Some(rules), ..Default::default() })); + Op::AddAlbum { album: a } + } + + #[test] + fn develop_ops_bump_only_the_full_revision() { + let mut c = Catalog::new(); + c.apply(Op::AddPhoto { photo: Box::new(photo(1)) }).unwrap(); + let (rev, lib) = (c.revision, c.library_revision); + let mut s = (*c.photo(PhotoId(1)).unwrap().develop).clone(); + s.light.exposure = 1.0; + let set = + Op::SetDevelop { id: PhotoId(1), settings: Arc::new(s.clone()), label: "Exposure".into(), edited: Some("2026-02-01T00:00:00".into()) }; + let step = Op::PushHistory { id: PhotoId(1), step: crate::HistoryStep { label: "Exposure".into(), settings: Arc::new(s) } }; + c.apply(Op::Batch { ops: vec![set, step] }).unwrap(); + assert_eq!((c.revision, c.library_revision), (rev + 1, lib), "a develop edit leaves the library revision"); + c.apply(Op::SetRating { id: PhotoId(1), rating: 3 }).unwrap(); + assert_eq!((c.revision, c.library_revision), (rev + 2, lib + 1)); + // a batch with any library op is a library change + let mixed = + Op::Batch { ops: vec![Op::SetHistory { id: PhotoId(1), history: Vec::new() }, Op::SetFlag { id: PhotoId(1), flag: crate::Flag::Pick }] }; + c.apply(mixed).unwrap(); + assert_eq!(c.library_revision, lib + 2); + // a failed op changes neither + assert!(c.apply(Op::SetRating { id: PhotoId(9), rating: 3 }).is_err()); + assert_eq!((c.revision, c.library_revision), (rev + 3, lib + 2)); + } + + #[test] + fn filters_sorts_and_rules_that_read_develop_state() { + assert!(!Filter::default().reads_develop()); + assert!(!Filter { text: "harbour rating:3".into(), rating: 2, keyword: Some("travel".into()), ..Default::default() }.reads_develop()); + assert!(Filter { edited: Some(false), ..Default::default() }.reads_develop()); + assert!(Filter { text: "harbour Edited:yes".into(), ..Default::default() }.reads_develop()); + assert!(Sort { key: SortKey::EditDate, ..Default::default() }.reads_develop()); + assert!(!Sort { key: SortKey::FileName, ..Default::default() }.reads_develop()); + for field in ["edited", "cropped", "treatment", "editDate", "longEdge", "shortEdge", "aspect", "megapixels"] { + let nested = rules( + json!({"rules": [{"field": "rating", "op": ">=", "value": 1}, {"group": {"match": "any", "rules": [{"field": field, "op": "is", "value": true}]}}]}), + ); + assert!(nested.reads_develop(), "{field}"); + } + assert!( + !rules(json!({"rules": [{"field": "rating", "op": ">=", "value": 1}, {"field": "keywords", "op": "contains", "value": "x"}]})) + .reads_develop() + ); + } + + /// Every rule field is either known not to read develop state or counts as reading it: a + /// new field must be put on one side on purpose. + #[test] + fn every_rule_field_is_classified() { + let reads: Vec<&str> = crate::rules::FIELDS.iter().map(|f| f.0).filter(|f| field_reads_develop(f)).collect(); + assert_eq!(reads, ["editDate", "longEdge", "shortEdge", "aspect", "megapixels", "edited", "cropped", "treatment"]); + for f in FIELDS_WITHOUT_DEVELOP { + assert!(crate::rules::FIELDS.iter().any(|x| x.0 == *f), "{f} is not a rule field"); + } + } + + #[test] + fn smart_albums_read_develop_through_the_albums_they_test() { + let mut c = Catalog::new(); + c.apply(smart(1, rules(json!({"rules": [{"field": "rating", "op": ">=", "value": 3}]})))).unwrap(); + c.apply(smart(2, rules(json!({"rules": [{"field": "edited", "op": "is", "value": true}]})))).unwrap(); + // 3 tests 2 (which reads develop); 4 and 5 test each other (a loop) and 1 + c.apply(smart(3, rules(json!({"rules": [{"field": "album", "op": "is", "value": 2}]})))).unwrap(); + c.apply(smart(4, rules(json!({"rules": [{"field": "album", "op": "is", "value": 5}, {"field": "album", "op": "is", "value": 1}]})))).unwrap(); + c.apply(smart(5, rules(json!({"rules": [{"field": "album", "op": "is", "value": 4}]})))).unwrap(); + c.apply(Op::AddAlbum { album: Album::new(AlbumId(6), "Plain") }).unwrap(); + let reads = |id: u64| c.album_reads_develop(AlbumId(id)); + assert_eq!([1, 2, 3, 4, 5, 6, 7].map(reads), [false, true, true, false, false, false, false]); + assert!(c.filter_reads_develop(&Filter { album: Some(AlbumId(3)), ..Default::default() })); + assert!(!c.filter_reads_develop(&Filter { album: Some(AlbumId(4)), ..Default::default() })); + assert!(c.smart_albums_read_develop()); + c.apply(Op::RemoveAlbum { id: AlbumId(2) }).unwrap(); + c.apply(Op::RemoveAlbum { id: AlbumId(3) }).unwrap(); + assert!(!c.smart_albums_read_develop()); + } +} diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index 2bb5511c..5b47f354 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -370,7 +370,7 @@ impl Session { // in this process its own range: otherwise an empty library opened after one that was // never edited this session would look unchanged and show the old library's numbers. static LOADS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); - catalog.revision = LOADS.fetch_add(1, std::sync::atomic::Ordering::Relaxed).wrapping_add(1) << 32; + catalog.set_revision(LOADS.fetch_add(1, std::sync::atomic::Ordering::Relaxed).wrapping_add(1) << 32); self.catalog = catalog; self.library_generation = crate::LIBRARY_GEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed); for entry in self.undo.iter_mut().chain(self.redo.iter_mut()) { From dfd6cbeea6591a0476642953d1c1c2633557ef8d Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Sun, 11 Oct 2026 17:16:00 -0500 Subject: [PATCH 2/3] perf(engine): #1057 slider ticks don't redo the grid query Session::visible() and source_total() key on a structured ViewKey compared field by field (no per-frame Debug format of the filter) and on the view revision, so a develop edit re-runs the query only when the filter, sort or smart-album source reads develop state. At 100k photos a drag tick drops from 68 ms (capture-date sort) / 27 ms (file name) to under 0.01 ms. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/lib.rs | 79 ++++++++++-- crates/engine/src/tests_invalidation.rs | 162 ++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 13 deletions(-) create mode 100644 crates/engine/src/tests_invalidation.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3bc6c431..b5aa050d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -175,15 +175,16 @@ pub struct Session { pub filter: Filter, pub sort: Sort, pub selection: Selection, - /// The grid order for the current source/filter/sort (cached by catalog revision). + /// The grid order for the current source/filter/sort (cached by library revision, and by the + /// full catalog revision when the filter, sort or source reads develop state). /// Shared, so frontends can hold it across frames without copying ([`Session::visible_shared`]). visible: Arc<[PhotoId]>, - visible_key: Option<(u64, String)>, + visible_key: Option, /// Identifies the current `visible` list: a new value (unique in the process) every time it /// is recomputed, so frontends can key their per-view caches on it instead of hashing the ids. visible_gen: u64, /// Photos in the current source with no filter on (cached like `visible`). - total: Option<((u64, String), usize)>, + total: Option<(ViewKey, usize)>, pub undo: Vec, /// Undo steps committed in this session, ever (the history drops its oldest past 1000, so /// its length can't tell how many steps a command just made; see [`Session::commits`]). @@ -839,13 +840,14 @@ impl Session { /// Photos shown in the grid/filmstrip for the current source, filter and sort. pub fn visible(&mut self) -> &[PhotoId] { - let mut key = - (self.catalog.revision, format!("{:?}|{:?}|{:?}|{:?}|{:?}", self.source, self.filter, self.sort, self.browse, self.library_folder)); - if self.source == LibrarySource::Missing && self.media.availability.is_background() { - // the view fills in as the background checks find files gone - key.1.push_str(&format!("|{}", self.media.availability.generation())); - } - if self.visible_key.as_ref() != Some(&key) { + // a develop edit (a slider tick) only redoes the query when the filter, sort or source reads develop state + let reads_develop = self.source_reads_develop() || self.sort.reads_develop() || self.catalog.filter_reads_develop(&self.filter); + let revision = self.catalog.view_revision(reads_develop); + // the Missing Photos view fills in as the background checks find files gone + let availability = + (self.source == LibrarySource::Missing && self.media.availability.is_background()).then(|| self.media.availability.generation()); + if !self.visible_key.as_ref().is_some_and(|k| k.is(self, revision, true, availability)) { + let key = ViewKey::of(self, revision, true, availability); // "in the last N days" rules count back from the session's clock lightcraft_catalog::rules::set_now(Some((self.clock)())); let mut f = self.source.to_filter(&self.filter, &self.catalog); @@ -916,8 +918,9 @@ impl Session { if self.source == LibrarySource::Missing { return None; } - let key = (self.catalog.revision, format!("{:?}|{:?}|{:?}", self.source, self.browse, self.library_folder)); - if self.total.as_ref().map(|t| &t.0) != Some(&key) { + let revision = self.catalog.view_revision(self.source_reads_develop()); + if !self.total.as_ref().is_some_and(|t| t.0.is(self, revision, false, None)) { + let key = ViewKey::of(self, revision, false, None); let mut f = self.source.to_filter(&Filter::default(), &self.catalog); if self.source == LibrarySource::Folder { let b = self.browse.clone().unwrap_or_default(); @@ -933,12 +936,21 @@ impl Session { self.total.as_ref().map(|t| t.1) } + /// Whether what the current source holds can change with a develop edit (a smart album + /// whose rules read develop state; see [`lightcraft_catalog::revision`]). + fn source_reads_develop(&self) -> bool { + match self.source { + LibrarySource::Album(a) => self.catalog.album_reads_develop(a), + _ => false, + } + } + pub fn visible_cloned(&mut self) -> Vec { self.visible().to_vec() } /// The visible photos without copying them, with their generation: equal generations mean - /// the very same list (same catalog revision, source, filter, sort), so a frontend can cache + /// the very same list (same library, source, filter, sort), so a frontend can cache /// whatever it derives from the list under that number. pub fn visible_shared(&mut self) -> (u64, Arc<[PhotoId]>) { self.visible(); @@ -957,6 +969,45 @@ impl Session { } } +/// What [`Session::visible`] (or [`Session::source_total`]) was worked out from. Compared every +/// frame, so without formatting or copying anything. +#[derive(Clone, Debug, PartialEq)] +struct ViewKey { + revision: lightcraft_catalog::ViewRevision, + source: LibrarySource, + /// The filter bar and sort (`None` for the source total, which ignores them). + view: Option<(Filter, Sort)>, + browse: Option, + library_folder: Option, + availability: Option, +} + +impl ViewKey { + fn of(s: &Session, revision: lightcraft_catalog::ViewRevision, filtered: bool, availability: Option) -> ViewKey { + ViewKey { + revision, + source: s.source, + view: filtered.then(|| (s.filter.clone(), s.sort)), + browse: s.browse.clone(), + library_folder: s.library_folder.clone(), + availability, + } + } + + fn is(&self, s: &Session, revision: lightcraft_catalog::ViewRevision, filtered: bool, availability: Option) -> bool { + let view = match &self.view { + Some((f, sort)) => filtered && *f == s.filter && *sort == s.sort, + None => !filtered, + }; + self.revision == revision + && self.source == s.source + && view + && self.browse == s.browse + && self.library_folder == s.library_folder + && self.availability == availability + } +} + /// Automatic versions kept per photo. pub const AUTO_VERSIONS: usize = 20; @@ -1003,6 +1054,8 @@ mod tests_import; #[cfg(test)] mod tests_import_move; #[cfg(test)] +mod tests_invalidation; +#[cfg(test)] mod tests_keyword_list; #[cfg(test)] mod tests_libops; diff --git a/crates/engine/src/tests_invalidation.rs b/crates/engine/src/tests_invalidation.rs new file mode 100644 index 00000000..024a7e4f --- /dev/null +++ b/crates/engine/src/tests_invalidation.rs @@ -0,0 +1,162 @@ +//! What a develop edit invalidates: a slider tick changes one photo's develop settings, so the +//! library-wide grid query only runs again when the current filter, sort or smart album reads +//! develop state (see `lightcraft_catalog::revision`). + +use lightcraft_catalog::{Album, AlbumId, Filter, Op, Photo, PhotoId, RuleSet, Sort, SortKey, Source}; +use serde_json::json; + +use crate::{LibrarySource, Session}; + +/// A library of `n` photos with capture dates and a keyword each. +fn library(n: u64) -> Session { + let mut s = Session::new(); + let ops = (0..n) + .map(|i| { + let mut p = Photo::new( + PhotoId(i + 1), + Source::Demo { scene: (i % 20) as u32 }, + &format!("IMG_{i:06}.jpg"), + "JPEG", + 6000, + 4000, + "2026-01-01T00:00:00", + ); + p.captured = Some(format!("20{:02}-{:02}-{:02}T10:00:00", 10 + i % 16, 1 + i % 12, 1 + i % 28)); + p.meta.keywords = vec![format!("kw{}", i % 300)]; + Op::AddPhoto { photo: Box::new(p) } + }) + .collect(); + s.commit("Add", Op::Batch { ops }).unwrap(); + s +} + +/// Change photo `id`'s exposure (a committed edit, or a slider tick inside an interaction). +fn expose(s: &mut Session, id: PhotoId, ev: f64) { + let mut d = (*s.develop_of(id).unwrap()).clone(); + d.light.exposure = ev; + s.set_develop(id, d, "Exposure").unwrap(); +} + +fn smart(s: &mut Session, id: u64, rules: serde_json::Value) { + let mut a = Album::new(AlbumId(id), format!("Smart {id}")); + let rules: RuleSet = serde_json::from_value(rules).unwrap(); + a.smart = Some(Box::new(Filter { rule_set: Some(rules), ..Default::default() })); + s.commit("New Smart Album", Op::AddAlbum { album: a }).unwrap(); +} + +/// Slider ticks and committed develop edits leave the grid's list as it is (same generation: the +/// query didn't run) when nothing in view reads develop state; a library change still redoes it. +#[test] +fn develop_edits_dont_redo_the_library_query() { + let mut s = library(50); + let (generation, _) = s.visible_shared(); + let total = s.source_total(); + let id = s.visible()[7]; + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + s.execute("develop.beginInteraction", &json!({"label": "Exposure"})).unwrap(); + for i in 0..10 { + s.execute("develop.set", &json!({"control": "light.exposure", "value": i as f64 / 10.0})).unwrap(); + assert_eq!(s.visible_shared().0, generation, "tick {i}"); + } + s.execute("develop.endInteraction", &json!({})).unwrap(); + expose(&mut s, id, 2.0); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.visible_shared().0, generation, "a committed edit and its undo"); + assert_eq!(s.source_total(), total); + // a library change does + s.execute("photo.rate", &json!({"rating": 4})).unwrap(); + assert_ne!(s.visible_shared().0, generation); +} + +/// Filters, sorts and smart albums that read develop state follow every develop edit, slider +/// ticks included. +#[test] +fn views_that_read_develop_state_follow_develop_edits() { + let mut s = library(20); + let ids = s.visible().to_vec(); + let (a, b) = (ids[3], ids[11]); + s.execute("library.select", &json!({"ids": [a.0]})).unwrap(); + + // the filter bar's Edited, during a drag + s.filter = Filter { edited: Some(true), ..Default::default() }; + assert!(s.visible().is_empty()); + s.execute("develop.beginInteraction", &json!({"label": "Exposure"})).unwrap(); + s.execute("develop.set", &json!({"control": "light.exposure", "value": 0.5})).unwrap(); + assert_eq!(s.visible(), [a], "a slider tick makes the photo edited"); + s.execute("develop.set", &json!({"control": "light.exposure", "value": 0.0})).unwrap(); + assert!(s.visible().is_empty(), "and back"); + s.execute("develop.endInteraction", &json!({})).unwrap(); + // a search token + s.filter = Filter { text: "edited:yes".into(), ..Default::default() }; + expose(&mut s, b, 1.0); + assert_eq!(s.visible(), [b]); + + // Edit Date: the photo edited last comes first + s.filter = Filter::default(); + s.sort = Sort { key: SortKey::EditDate, ascending: false, ..Default::default() }; + s.clock = Box::new(|| "2100-05-01T10:00:00".to_string()); + expose(&mut s, a, 0.3); + assert_eq!(s.visible()[0], a); + s.clock = Box::new(|| "2100-05-01T11:00:00".to_string()); + expose(&mut s, b, 0.7); + assert_eq!(s.visible()[0], b); + + // a smart album testing Cropped, and one testing it through an Album rule + s.sort = Sort::default(); + smart(&mut s, 100, json!({"rules": [{"field": "cropped", "op": "is", "value": true}]})); + smart(&mut s, 101, json!({"rules": [{"field": "album", "op": "is", "value": 100}]})); + for album in [100, 101] { + s.source = LibrarySource::Album(AlbumId(album)); + assert!(s.visible().is_empty()); + assert_eq!(s.source_total(), Some(0)); + } + let mut d = (*s.develop_of(a).unwrap()).clone(); + d.crop.geometry.rect = lightcraft_geom::Rect::new(0.1, 0.1, 0.9, 0.9); + s.set_develop(a, d, "Crop").unwrap(); + for album in [100, 101] { + s.source = LibrarySource::Album(AlbumId(album)); + assert_eq!(s.visible(), [a], "album {album}"); + assert_eq!(s.source_total(), Some(1), "album {album}"); + } + // and the size rules (a crop changes the shown shape) + s.source = LibrarySource::All; + let square: RuleSet = serde_json::from_value(json!({"rules": [{"field": "aspect", "op": "is", "value": "square"}]})).unwrap(); + s.filter = Filter { rule_set: Some(square), ..Default::default() }; + assert!(s.visible().is_empty()); + let mut d = (*s.develop_of(b).unwrap()).clone(); + d.crop.geometry.rect = lightcraft_geom::Rect::new(0.0, 0.0, 4000.0 / 6000.0, 1.0); + s.set_develop(b, d, "Crop").unwrap(); + assert_eq!(s.visible(), [b]); +} + +/// Per-tick cost of a slider drag on a big library (the develop op plus the grid query the +/// filmstrip asks for every frame): +/// `cargo test --release -p lightcraft-engine -- --ignored drag_tick_100k --nocapture`. +#[test] +#[ignore] +fn drag_tick_100k() { + let mut s = library(100_000); + let id = s.visible()[50_000]; + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + s.execute("develop.beginInteraction", &json!({"label": "Exposure"})).unwrap(); + let n = 60; + let tick = |s: &mut Session, name: &str| { + s.visible(); + let t = std::time::Instant::now(); + for i in 0..n { + s.execute("develop.set", &json!({"control": "light.exposure", "value": i as f64 / 100.0})).unwrap(); + s.visible(); + } + let ms = t.elapsed().as_secs_f64() * 1e3 / n as f64; + eprintln!("slider tick at 100k photos, {name}: {ms:.3} ms (develop.set + visible)"); + }; + tick(&mut s, "warm-up"); + tick(&mut s, "by capture date"); + s.sort = Sort { key: SortKey::FileName, ..Default::default() }; + tick(&mut s, "by file name"); + // a view that reads develop state still asks again on every tick + s.sort = Sort::default(); + s.filter = Filter { edited: Some(false), ..Default::default() }; + tick(&mut s, "Unedited filter"); + s.execute("develop.endInteraction", &json!({})).unwrap(); +} From f6b41efb168cfe1fb463ea7221af4357e38e51b8 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Sun, 11 Oct 2026 17:16:00 -0500 Subject: [PATCH 3/3] perf(ui-egui): #1057 sidebar caches survive develop edits Library-wide caches that don't read develop state (folder/keyword trees, keyword chips/ticks/names, counts, person names, By Date, filter-bar values, suggestions, smart-album problems, the Missing count, face hints) key on the library revision; people, album counts and the rule dialog's count key on the view revision. The justified grid layout adds the catalog revision to its key since crops and rotations change cell shapes without changing the list. The filmstrip shares the visible list instead of copying it per frame. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 118 +++++++++++++++++++++------- crates/ui-egui/src/panels/detail.rs | 5 +- crates/ui-egui/src/panels/faces.rs | 3 +- crates/ui-egui/src/panels/grid.rs | 16 ++-- crates/ui-egui/src/panels/left.rs | 3 +- crates/ui-egui/src/tests_grid.rs | 26 ++++++ docs/parity-checklist.md | 5 +- 7 files changed, 135 insertions(+), 41 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 9a387c77..c14ec2c6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1454,7 +1454,10 @@ mod drop_tests { } } -/// Results recomputed only when the catalog (or their inputs) change. +/// Results recomputed only when the catalog (or their inputs) change. Views that don't read +/// develop state key on [`lightcraft_catalog::Catalog::library_revision`], so a slider drag (a +/// develop edit per tick) doesn't redo them; the others on +/// [`lightcraft_catalog::Catalog::view_revision`] or the full revision. #[derive(Default)] pub struct Caches { keyword_tree: Option<(u64, std::sync::Arc>)>, @@ -1466,7 +1469,7 @@ pub struct Caches { /// The Keyword List's tick boxes, by library revision and selection. keyword_ticks: Option<(u64, u64, std::sync::Arc)>, folder_tree: Option<(u64, std::sync::Arc>)>, - people: Option<(u64, lightcraft_catalog::Filter, std::sync::Arc>)>, + people: Option<(lightcraft_catalog::ViewRevision, lightcraft_catalog::Filter, std::sync::Arc>)>, suggestions: Option<(u64, std::sync::Arc>)>, counts: Option<(u64, LibraryCounts)>, date_groups: Option<(u64, std::sync::Arc>)>, @@ -1546,10 +1549,10 @@ impl Caches { /// The folders the library's photos were imported from. pub fn folder_tree(&mut self, cat: &lightcraft_catalog::Catalog) -> std::sync::Arc> { match &self.folder_tree { - Some((r, t)) if *r == cat.revision => t.clone(), + Some((r, t)) if *r == cat.library_revision => t.clone(), _ => { let t = std::sync::Arc::new(cat.folder_tree()); - self.folder_tree = Some((cat.revision, t.clone())); + self.folder_tree = Some((cat.library_revision, t.clone())); t } } @@ -1557,10 +1560,10 @@ impl Caches { /// The library's keyword tree. pub fn keyword_tree(&mut self, cat: &lightcraft_catalog::Catalog) -> std::sync::Arc> { match &self.keyword_tree { - Some((r, t)) if *r == cat.revision => t.clone(), + Some((r, t)) if *r == cat.library_revision => t.clone(), _ => { let t = std::sync::Arc::new(cat.keyword_tree()); - self.keyword_tree = Some((cat.revision, t.clone())); + self.keyword_tree = Some((cat.library_revision, t.clone())); t } } @@ -1575,14 +1578,14 @@ impl Caches { ) -> std::sync::Arc> { let key = key_of((selection, export)); match &self.keyword_names { - Some((r, k, c)) if *r == cat.revision && *k == key => c.clone(), + Some((r, k, c)) if *r == cat.library_revision && *k == key => c.clone(), _ => { let c = std::sync::Arc::new(if export { panels::keywording::will_export(cat, selection) } else { panels::keywording::with_containing(cat, selection) }); - self.keyword_names = Some((cat.revision, key, c.clone())); + self.keyword_names = Some((cat.library_revision, key, c.clone())); c } } @@ -1595,10 +1598,10 @@ impl Caches { ) -> std::sync::Arc> { let key = key_of(selection); match &self.keyword_chips { - Some((r, k, c)) if *r == cat.revision && *k == key => c.clone(), + Some((r, k, c)) if *r == cat.library_revision && *k == key => c.clone(), _ => { let c = std::sync::Arc::new(panels::keywording::chips(cat, selection)); - self.keyword_chips = Some((cat.revision, key, c.clone())); + self.keyword_chips = Some((cat.library_revision, key, c.clone())); c } } @@ -1611,10 +1614,10 @@ impl Caches { ) -> std::sync::Arc { let key = key_of(selection); match &self.keyword_ticks { - Some((r, k, t)) if *r == cat.revision && *k == key => t.clone(), + Some((r, k, t)) if *r == cat.library_revision && *k == key => t.clone(), _ => { let t = std::sync::Arc::new(panels::keyword_list::Ticks::of(cat, selection)); - self.keyword_ticks = Some((cat.revision, key, t.clone())); + self.keyword_ticks = Some((cat.library_revision, key, t.clone())); t } } @@ -1627,11 +1630,13 @@ impl Caches { filter: &lightcraft_catalog::Filter, ) -> std::sync::Arc> { let key = lightcraft_catalog::Filter { person: None, ..filter.clone() }; + // a develop edit only counts again when the filter reads develop state (Edited, …) + let rev = cat.view_revision(cat.filter_reads_develop(&key)); match &self.people { - Some((r, f, t)) if *r == cat.revision && *f == key => t.clone(), + Some((r, f, t)) if *r == rev && *f == key => t.clone(), _ => { let t = std::sync::Arc::new(cat.people_in(&key)); - self.people = Some((cat.revision, key, t.clone())); + self.people = Some((rev, key, t.clone())); t } } @@ -1639,7 +1644,7 @@ impl Caches { /// All Photos / Picks / Recently Deleted counts. pub fn counts(&mut self, cat: &lightcraft_catalog::Catalog) -> LibraryCounts { if let Some((r, c)) = self.counts - && r == cat.revision + && r == cat.library_revision { return c; } @@ -1654,16 +1659,16 @@ impl Caches { c.deleted += 1; } } - self.counts = Some((cat.revision, c)); + self.counts = Some((cat.library_revision, c)); c } /// Everyone named on a face in the library (for completing a name as it is typed). pub fn person_names(&mut self, cat: &lightcraft_catalog::Catalog) -> std::sync::Arc> { match &self.person_names { - Some((r, v)) if *r == cat.revision => v.clone(), + Some((r, v)) if *r == cat.library_revision => v.clone(), _ => { let v = std::sync::Arc::new(cat.people().into_iter().map(|p| p.name).collect::>()); - self.person_names = Some((cat.revision, v.clone())); + self.person_names = Some((cat.library_revision, v.clone())); v } } @@ -1671,10 +1676,10 @@ impl Caches { /// The By Date tree. pub fn date_groups(&mut self, cat: &lightcraft_catalog::Catalog) -> std::sync::Arc> { match &self.date_groups { - Some((r, g)) if *r == cat.revision => g.clone(), + Some((r, g)) if *r == cat.library_revision => g.clone(), _ => { let g = std::sync::Arc::new(cat.date_groups()); - self.date_groups = Some((cat.revision, g.clone())); + self.date_groups = Some((cat.library_revision, g.clone())); g } } @@ -1682,7 +1687,7 @@ impl Caches { /// Cameras, lenses and keywords in the library (filter bar pickers). pub fn filter_values(&mut self, cat: &lightcraft_catalog::Catalog) -> std::sync::Arc { match &self.filter_values { - Some((r, v)) if *r == cat.revision => v.clone(), + Some((r, v)) if *r == cat.library_revision => v.clone(), _ => { let distinct = |mut v: Vec| { v.retain(|s| !s.trim().is_empty()); @@ -1696,14 +1701,14 @@ impl Caches { lenses: distinct(lib().map(|p| p.meta.lens.clone()).collect()), keywords: cat.keywords().into_iter().map(|(k, _)| k).collect(), }); - self.filter_values = Some((cat.revision, v.clone())); + self.filter_values = Some((cat.library_revision, v.clone())); v } } } /// Keyword suggestions for a photo with `current` keywords and the typed `prefix`. pub fn suggestions(&mut self, cat: &lightcraft_catalog::Catalog, current: &[String], prefix: &str, n: usize) -> std::sync::Arc> { - let k = key_of((cat.revision, current, prefix, n)); + let k = key_of((cat.library_revision, current, prefix, n)); match &self.suggestions { Some((h, v)) if *h == k => v.clone(), _ => { @@ -1714,8 +1719,9 @@ impl Caches { } } /// Every album's photo count for the sidebar (a smart album evaluates its rules, which - /// scans the catalog). Recomputed when the catalog changes (photos, metadata, album rules - /// all bump its revision) and — only while some smart album has an "in the last…" rule — + /// scans the catalog). Recomputed when the library changes (photos, metadata, album rules + /// all bump its revision; a develop edit only while some smart album's rules read develop + /// state) 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 @@ -1731,7 +1737,10 @@ impl Caches { ) -> std::sync::Arc { 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())); + // the count only changes with a develop edit when the rules (or the albums they test) read develop state + let reads_develop = rules.reads_develop() || rules.albums_tested().into_iter().any(|a| cat.album_reads_develop(a)); + let rev = cat.view_revision(reads_develop); + let k = key_of((rev, serde_json::to_string(rules).unwrap_or_default(), editing, &folder, minute, i18n::language().code())); if let Some((key, v)) = &self.rules_view && *key == k { @@ -1756,7 +1765,7 @@ impl Caches { cat: &lightcraft_catalog::Catalog, ) -> std::sync::Arc>> { if let Some((rev, v)) = &self.smart_problems - && *rev == cat.revision + && *rev == cat.library_revision { return v.clone(); } @@ -1764,7 +1773,7 @@ impl Caches { 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())); + self.smart_problems = Some((cat.library_revision, v.clone())); v } @@ -1775,7 +1784,8 @@ impl Caches { ) -> std::sync::Arc> { let relative = cat.albums().any(|a| a.smart.as_ref().is_some_and(|f| f.depends_on_now())); let minute = if relative { now.get(..16).unwrap_or(now) } else { "" }; - let k = key_of((cat.revision, minute)); + // plain albums' counts never change with a develop edit; smart albums' do when their rules read develop state + let k = key_of((cat.view_revision(cat.smart_albums_read_develop()), minute)); match &self.album_counts { Some((h, v)) if *h == k => v.clone(), _ => { @@ -1879,6 +1889,56 @@ mod cache_tests { assert_eq!(c.smart_problem_scans, 2); } + /// A develop edit (one slider tick: settings, edit time and a History step) changes no + /// library-wide view unless it reads develop state: the sidebar's counts and rule-dialog count + /// aren't worked out again, while a smart album and a rule testing Has Edits / Cropped follow + /// the edit. + #[test] + fn develop_edits_only_redo_views_that_read_develop_state() { + 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(); + } + cat.apply(smart(10, serde_json::json!({"rules": [{"field": "rating", "op": "gte", "value": 3}]}))).unwrap(); + let now = "2026-09-30T12:00:00"; + let rating = + |v: u64| -> RuleSet { serde_json::from_value(serde_json::json!({"rules": [{"field": "rating", "op": "gte", "value": v}]})).unwrap() }; + let cropped: RuleSet = serde_json::from_value(serde_json::json!({"rules": [{"field": "cropped", "op": "is", "value": true}]})).unwrap(); + let develop = |cat: &mut Catalog, id: u64, f: &dyn Fn(&mut lightcraft_develop::DevelopSettings)| { + let mut s = (*cat.photo(PhotoId(id)).unwrap().develop).clone(); + f(&mut s); + let settings = std::sync::Arc::new(s); + let step = lightcraft_catalog::HistoryStep { label: "Edit".into(), settings: settings.clone() }; + let set = Op::SetDevelop { id: PhotoId(id), settings, label: "Edit".into(), edited: Some(now.into()) }; + cat.apply(Op::Batch { ops: vec![set, Op::PushHistory { id: PhotoId(id), step }] }).unwrap(); + }; + c.album_counts(&cat, now); + c.rules_view(&cat, &rating(3), None, None, now); + let (tree, counts) = (c.keyword_tree(&cat), c.counts(&cat).total); + for i in 0..20 { + develop(&mut cat, 2, &|s| s.light.exposure = i as f64 / 10.0); + c.album_counts(&cat, now); + c.rules_view(&cat, &rating(3), None, None, now); + assert!(std::sync::Arc::ptr_eq(&tree, &c.keyword_tree(&cat))); + assert_eq!(c.counts(&cat).total, counts); + } + assert_eq!((c.album_count_scans, c.rules_view_scans), (1, 1), "slider ticks don't recount"); + // views that read develop state follow the edit + assert_eq!(c.rules_view(&cat, &cropped, None, None, now).count, 0); + cat.apply(smart(11, serde_json::json!({"rules": [{"field": "edited", "op": "is", "value": true}]}))).unwrap(); + assert_eq!(c.album_counts(&cat, now)[&AlbumId(11)], 1, "photo 2 has edits"); + develop(&mut cat, 3, &|s| s.crop.geometry.rect = lightcraft_geom::Rect::new(0.1, 0.1, 0.9, 0.9)); + assert_eq!(c.album_counts(&cat, now)[&AlbumId(11)], 2, "a smart album testing Has Edits counts the new edit"); + assert_eq!(c.rules_view(&cat, &cropped, None, None, now).count, 1, "a rule testing Cropped sees the crop"); + // a rule that tests such an album through an Album rule follows it too + let via: RuleSet = serde_json::from_value(serde_json::json!({"rules": [{"field": "album", "op": "is", "value": 11}]})).unwrap(); + assert_eq!(c.rules_view(&cat, &via, None, None, now).count, 2); + develop(&mut cat, 1, &|s| s.light.exposure = 1.0); + assert_eq!(c.rules_view(&cat, &via, None, None, now).count, 3); + assert_eq!(c.album_counts(&cat, now)[&AlbumId(11)], 3); + } + /// Without "in the last…" rules the clock never causes a rescan. #[test] fn absolute_rules_ignore_the_clock() { diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 23447616..431c954c 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -487,7 +487,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { // once this photo is on screen: prepare its neighbours in filmstrip order (source decoded and // kept, view render cached) so stepping to them is instant if !interacting && !app.renderer.is_pending(Slot::Main) && app.renderer.textures.get(&Slot::Main).is_some_and(|t| t.photo == id) { - let ids = app.session.visible_cloned(); + let ids = app.session.visible_shared().1; if let Some(i) = ids.iter().position(|p| *p == id) { let next = ids.get(i + 1).copied(); let prev = i.checked_sub(1).and_then(|j| ids.get(j)).copied(); @@ -2132,7 +2132,8 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { let t = Tokens::get(ui.ctx()); ui.painter().rect_filled(r, 0.0, t.canvas); ui.painter().rect_filled(Rect::from_min_size(r.min, vec2(r.width(), 4.0)), 0.0, Color32::from_gray(0x20)); - let ids = app.session.visible_cloned(); + // shared, not copied: the strip is drawn every frame + let ids = app.session.visible_shared().1; let active = app.session.selection.active; let cell_w = 120.0; let ppp = ui.ctx().pixels_per_point(); diff --git a/crates/ui-egui/src/panels/faces.rs b/crates/ui-egui/src/panels/faces.rs index 2b3fe032..cd540f76 100644 --- a/crates/ui-egui/src/panels/faces.rs +++ b/crates/ui-egui/src/panels/faces.rs @@ -555,7 +555,8 @@ pub fn hints_for(app: &mut LightcraftApp, photo: u64) -> Option, GroupBy); /// What the grid derives from the visible photos, kept across frames. Everything is keyed by the /// session's visible-list generation ([`lightcraft_engine::Session::visible_shared`]), which -/// changes whenever the list is recomputed (any catalog change: crops, orientation, dates, stacks; -/// or another source, filter or sort) — so an unchanged frame neither copies nor hashes the ids. +/// changes whenever the list is recomputed (a library change: dates, stacks, photos; a develop +/// edit when the filter or sort reads it; or another source, filter or sort) — so an unchanged +/// frame neither copies nor hashes the ids. Photo shapes (crop, orientation) are develop state: +/// the justified layout keys on the catalog revision too. #[derive(Default)] pub struct GridCache { runs: Option<(RunsKey, Arc>)>, - /// Layout by (runs key, width, thumbnail size, square). - layout: Option<((RunsKey, u32, u32, bool), Arc)>, + /// Layout by (runs key, width, thumbnail size, square, catalog revision for photo shapes). + layout: Option<((RunsKey, u32, u32, bool, u64), Arc)>, /// Stack index by generation. stacks: Option<(u64, Arc)>, /// How many of the photos are browsed (Local) ones, by generation. @@ -211,8 +213,10 @@ fn show_inner(app: &mut LightcraftApp, ui: &mut egui::Ui) { let stats = &mut app.caches.grid_stats; let runs = app.caches.grid.runs(stats, &app.session.catalog, &ids, runs_key); // the layout only changes with the photos (and their shapes), the grouping, the width and - // the thumbnail size - let lay_key = (runs_key, avail_w.to_bits(), target.to_bits(), square); + // the thumbnail size; a develop edit (crop, rotation) changes a shape without changing the + // list, so justified rows also key on the catalog revision + let shapes = if square { 0 } else { app.session.catalog.revision }; + let lay_key = (runs_key, avail_w.to_bits(), target.to_bits(), square, shapes); let lay = match &app.caches.grid.layout { Some((k, l)) if *k == lay_key => l.clone(), _ => { diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index a14332c8..33e0eab9 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -732,7 +732,8 @@ fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize { } let id = egui::Id::new("missing-count"); let now = ui.input(|i| i.time); - let rev = app.session.catalog.revision; + // which files the library expects is not develop state: a slider drag doesn't check them all again + let rev = app.session.catalog.library_revision; let mut job: Job = ui.data(|d| d.get_temp(id)).unwrap_or(Job { rev: u64::MAX, at: f64::MIN, ..Default::default() }); let fresh = now - job.at < 5.0 && job.rev == rev; if !fresh && !job.running.load(Relaxed) { diff --git a/crates/ui-egui/src/tests_grid.rs b/crates/ui-egui/src/tests_grid.rs index 7aafb212..c832f00f 100644 --- a/crates/ui-egui/src/tests_grid.rs +++ b/crates/ui-egui/src/tests_grid.rs @@ -132,6 +132,32 @@ fn grid_headers_selection_scrolling_stacks_and_shape_changes() { assert_cheap_frames(&mut h); } +/// A develop edit (a Quick Develop click, a slider tick) leaves the list in view as it is: the +/// grid's date runs and stack index aren't rebuilt, and the square grid's layout neither; the +/// justified grid lays out again since a crop or rotation can change a photo's shape. +#[test] +fn develop_edits_keep_the_grid_view() { + for view in [ViewMode::PhotoGrid, ViewMode::SquareGrid] { + let mut h = grid(view); + let (generation, _) = h.app.session.visible_shared(); + let before = h.app.caches.grid_stats.clone(); + let id = h.app.session.visible()[10]; + select(&mut h, id); + for v in [0.5, 1.0] { + let r = h.request("engine.execute", json!({"command": "develop.set", "params": {"control": "light.exposure", "value": v}}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + } + assert_eq!(h.app.session.visible_shared().0, generation, "{view:?}: the query didn't run again"); + let s = &h.app.caches.grid_stats; + assert_eq!((s.view_builds, s.stack_index_builds), (before.view_builds, before.stack_index_builds), "{view:?}"); + if view == ViewMode::SquareGrid { + assert_eq!(s.layout_builds, before.layout_builds, "square cells don't depend on shapes"); + } + assert_cheap_frames(&mut h); + } +} + /// Selected photos whose thumbnails have not loaded (or can't) are still marked selected in the /// justified grid (issue #298): the active one white, the others grey. #[test] diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 77ae26e4..a439822e 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (LR-LIB-SMARTALBUM: develop edits no longer recount the sidebar unless a rule reads develop state, #1057; rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -96,7 +96,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-BYDATE | Browse by date | P1 | ✅ | `cmd:library.filter` (`date`), `crates/ui-egui/src/panels/left.rs` (`date_row`), `crates/catalog/src/query.rs` (`date_groups`) | year → month → day tree with library-wide counts; a row filters the current source by that date. Active filters show as removable chips under the grid header (`crates/ui-egui/src/panels/chips.rs`, with “+N more” and Clear all), the header counts “N of M photos” (matching of the source's total), a badge on the top-bar filter icon counts them, and an empty result says no photos match the active filters | | LR-LIB-ALBUM | Albums | P0 | ✅ | `cmd:album.create`, `cmd:album.rename`, `cmd:album.delete`, `cmd:album.addPhotos`, `cmd:album.removePhotos`, `cmd:dialog.newAlbum`, `crates/engine/src/tests_albums_local.rs`, `crates/ui-egui/src/tests_panels.rs` | photos dragged from the grid onto an album row are added to it (LR-BEHAV-DRAGDROP); albums hold library photos (issue #684): `album.create {addSelected}`, `album.addPhotos` and `album.toggleTarget` (B) skip browsed Local photos and return `skipped` (a toast says so), a Local-only selection is an error and leaves Photo ▸ Add to Album, the grid's right-click Add to Album, a folder row's Add Selected Photos and a drop onto an album row off with the hint to Add to My Photos first (`button:contextAddToAlbum`, `button:contextAlbum:`); right-click a photo ▸ Add to Album lists the albums and ends with New Album…, which creates one holding the selection — also when the library has no album yet (issue #676; `button:contextNewAlbum`) | | LR-LIB-FOLDER | Folders of albums | P0 | ✅ | `cmd:album.create` (`folder`, `parent`), `cmd:album.move`, `cmd:album.reorder`, `cmd:album.sort`, `cmd:dialog.newAlbum` / `cmd:dialog.newFolder` / `cmd:dialog.smartAlbum` / `cmd:dialog.newSmartAlbum` (`parent`), `crates/ui-egui/src/panels/left.rs` (`albums_tree`, `folder_menu`, `disclosure_triangle`), `crates/ui-egui/src/tests_panels.rs` | Delete removes a folder and its nested albums as one undoable edit, keeps photos in the library and returns a deleted album view to All Photos (issue #501); folders nest and fold with the same disclosure triangle as Local / By Date / Folders / Keywords (`albumToggle:`; the row folds them too; open by default); right-click a folder ▸ New ▸ Create Album… / Create Smart Album… / Create Smart Album from Filter… / Create Folder… creates inside it (the `dialog.new*` commands take `parent`), opening the folders down to the new row; creating is only allowed inside folders; moving: drag an album or folder row onto a folder (outlined when allowed; a closed folder opens after 0.6 s under the pointer; the "Albums" header takes an item that sits in a folder back to the top level while it is dragged), the Move to menu or `album.move` — one undo step each; not onto a plain album, itself or something inside it; drop on the top / bottom half of an album or the edge of a folder to place it there by hand (`cmd:album.reorder`, a line shows the place; ordered per folder, folders stay first, new albums go after the ordered ones; right-click ▸ Sort Contents A–Z or ＋ ▸ Sort Albums A–Z = `cmd:album.sort`) | -| LR-LIB-SMARTALBUM | Smart albums | P1 | ✅ | `cmd:album.createSmart`, `cmd:album.setRules`, `crates/catalog/src/query.rs`, `crates/ui-egui/src/panels/filterbar.rs`, `crates/ui-egui/src/lib.rs` (`album_counts`) | saved filters (rating/flag/label/kind/edited/keyword/camera/lens/date range/text/album) or a rule set from the rule editor (match all / any / none, nested groups; see LRC-LIB-SMARTCOLL), live; sidebar counts are cached per catalog revision (and per minute while an “in the last…” rule is used), counted without building id lists | +| LR-LIB-SMARTALBUM | Smart albums | P1 | ✅ | `cmd:album.createSmart`, `cmd:album.setRules`, `crates/catalog/src/query.rs`, `crates/ui-egui/src/panels/filterbar.rs`, `crates/ui-egui/src/lib.rs` (`album_counts`) | saved filters (rating/flag/label/kind/edited/keyword/camera/lens/date range/text/album) or a rule set from the rule editor (match all / any / none, nested groups; see LRC-LIB-SMARTCOLL), live; sidebar counts are cached per library revision (develop edits recount only while a smart album's rules read develop state: Has Edits, Edit Date, Cropped, Treatment, sizes; #1057) (and per minute while an “in the last…” rule is used), counted without building id lists | | LR-LIB-SHARED-ALBUM | Shared albums | P2 | ⬜ | | needs a sharing service | | LR-LIB-OFFLINE | Keep album offline | P2 | 🚫 | | not applicable: local-first library | | LR-LIB-TARGET | Target album | P2 | ✅ | `cmd:album.setTarget`, `cmd:album.toggleTarget` | see LRC-LIB-COLLECTIONS | @@ -787,6 +787,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-11 | patch | LR-LIB-SMARTALBUM: sidebar counts key on the library revision, so develop edits recount them only while a smart album reads develop state (issue #1057); status unchanged | | 2026-10-11 | minor | KEY-SLIDER-TYPE: honor immediate Return/Escape and keep typed keys in the value editor; add regression references | | 2026-10-11 | minor | LR-PROF-CAMERACOLOR: raws with a black-and-white camera JPEG take its tone curve (issue #577) | | 2026-10-11 | minor | LR-VIEW-ZOOM and LR-VIEW-COMPARE: Compare and Reference are sharp at 1:1 (zoom windows per pane, two originals kept decoded while they are open); statuses unchanged |