From dfd6cbeea6591a0476642953d1c1c2633557ef8d Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Sun, 11 Oct 2026 17:16:00 -0500 Subject: [PATCH] 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(); +}