Stop re-answering questions about the library every time the window moves
The timeline's bars, the filter chips' counts, the "on this device" count and whether the scoped collection is pinned all describe the *library*. None of them can change because the view scrolled. `load_window` recomputed all four every time the window moved, which is several times per screenful. Together that is a `MIN`/`MAX`, a `GROUP BY`, two counts, and under a collection three more queries — about 8 ms of SQLite on the thread that is trying to draw the frame, for four answers that were already on screen and already right. They are now keyed on what they actually depend on: the scope, the filter, whether this is the trash, and the total. The total earns its place as the change detector as much as for the scrollbar — a scan landing, a delete or a restore all move it, and it was already read on every load. What a total cannot see is a rating edited under an unchanged count. That is covered, and deliberately not by widening the key: `apply_judgement` already refreshes the chips itself, because it has to report what actually landed rather than what was asked for. Same for the axis — a zoom, a pan, a scrub and dates arriving from the thumbnail worker each call `refresh_timeline` directly. Skipping the recompute here cannot leave anything stale on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+74
-18
@@ -61,6 +61,30 @@ const MIN_WINDOW: usize = 24;
|
||||
const MIN_CELL_SIZE: f32 = 90.0;
|
||||
const MAX_CELL_SIZE: f32 = 420.0;
|
||||
|
||||
/// TRACES: NFR-P5
|
||||
/// What the grid's whole-library readouts are an answer about.
|
||||
///
|
||||
/// The timeline's bars, the filter chips' counts, the "on this device" count
|
||||
/// and whether the scoped collection is pinned all describe the *library*, not
|
||||
/// the window over it — and [`load_window`] recomputed every one of them each
|
||||
/// time the window moved, which is several times per screenful of scrolling.
|
||||
/// Together they are a `MIN`/`MAX`, a `GROUP BY`, two counts and, under a
|
||||
/// collection, three more queries: about 8 ms of SQLite on the thread that is
|
||||
/// trying to draw the frame, for four answers that scrolling cannot change.
|
||||
///
|
||||
/// So they are recomputed when this changes and not otherwise. `total` is in
|
||||
/// here as the change detector as much as anything else: it is already read on
|
||||
/// every load for the scrollbar, and a scan landing, a delete or a restore all
|
||||
/// move it. What it cannot see — a rating edited under an unchanged count — is
|
||||
/// covered because the paths that do that refresh the chips themselves.
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
struct LibraryFacts {
|
||||
scope: Option<dr_types::CollectionId>,
|
||||
filter: library::RatingFilter,
|
||||
trash: bool,
|
||||
total: usize,
|
||||
}
|
||||
|
||||
/// Library state for the running window.
|
||||
pub struct LibraryController {
|
||||
/// Shared with [`crate::collections_ui`], which edits collections against
|
||||
@@ -109,6 +133,9 @@ pub struct LibraryController {
|
||||
/// window, wasteful on a narrow one. The grid measures itself and reports
|
||||
/// its screenful; this is [`SCREENFULS`] of them.
|
||||
window: RefCell<usize>,
|
||||
/// What the whole-library readouts on screen were last computed for, so a
|
||||
/// window that merely moved does not recompute them. See [`LibraryFacts`].
|
||||
library_facts: std::cell::Cell<Option<LibraryFacts>>,
|
||||
/// How many cells the viewport shows at once, as the grid last reported.
|
||||
///
|
||||
/// Kept beside `window` rather than divided back out of it, because
|
||||
@@ -313,6 +340,7 @@ impl LibraryController {
|
||||
resume_at: std::cell::Cell::new(0),
|
||||
window: RefCell::new((INITIAL_VIEWPORT_CELLS * SCREENFULS).max(MIN_WINDOW)),
|
||||
viewport_cells: std::cell::Cell::new(INITIAL_VIEWPORT_CELLS),
|
||||
library_facts: std::cell::Cell::new(None),
|
||||
requested: RefCell::new(Default::default()),
|
||||
scan_timer: RefCell::new(None),
|
||||
thumb_timer: RefCell::new(None),
|
||||
@@ -2004,24 +2032,39 @@ fn load_window(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
// query rather than another predicate threaded through the scoped one.
|
||||
let trash = ctl.viewing_trash.get();
|
||||
|
||||
// TRACES: FR-NC-6a
|
||||
// Whether the newly scoped collection is already pinned. Read here rather
|
||||
// than remembered, because a pin outlives the session that made it — on
|
||||
// reopening the library the button has to show what the catalog says, not
|
||||
// what this run happens to have done.
|
||||
window.set_library_scope_pinned(match scope {
|
||||
Some(id) => dr_catalog::collections::descendants(catalog.connection(), id)
|
||||
.map(|ids| collection_images(catalog, &ids))
|
||||
.map(|images| scope_is_pinned(catalog, &images))
|
||||
.unwrap_or(false),
|
||||
None => false,
|
||||
});
|
||||
|
||||
let total = if trash {
|
||||
library::total_trashed(catalog).unwrap_or(0)
|
||||
} else {
|
||||
library::total_images_scoped(catalog, scope, &filter).unwrap_or(0)
|
||||
};
|
||||
|
||||
// What the whole-library readouts below describe. See [`LibraryFacts`].
|
||||
let facts = LibraryFacts {
|
||||
scope,
|
||||
filter,
|
||||
trash,
|
||||
total,
|
||||
};
|
||||
let describes_something_new = ctl.library_facts.get() != Some(facts);
|
||||
ctl.library_facts.set(Some(facts));
|
||||
|
||||
// TRACES: FR-NC-6a
|
||||
// Whether the newly scoped collection is already pinned. Read here rather
|
||||
// than remembered, because a pin outlives the session that made it — on
|
||||
// reopening the library the button has to show what the catalog says, not
|
||||
// what this run happens to have done.
|
||||
//
|
||||
// Three queries deep — descendants, their images, and whether every one of
|
||||
// them is pinned — and the answer cannot change by scrolling.
|
||||
if describes_something_new {
|
||||
window.set_library_scope_pinned(match scope {
|
||||
Some(id) => dr_catalog::collections::descendants(catalog.connection(), id)
|
||||
.map(|ids| collection_images(catalog, &ids))
|
||||
.map(|images| scope_is_pinned(catalog, &images))
|
||||
.unwrap_or(false),
|
||||
None => false,
|
||||
});
|
||||
}
|
||||
// Read before it is overwritten: the property still holds what the library
|
||||
// was last time this ran, and a library that has become shorter is the
|
||||
// signal that something was deleted out from under the view.
|
||||
@@ -2036,8 +2079,17 @@ fn load_window(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
*ctl.offset.borrow_mut() = offset;
|
||||
window.set_library_offset(offset as i32);
|
||||
|
||||
// The date range this window covers, so the scrubber can label itself.
|
||||
refresh_timeline(window, catalog, ctl);
|
||||
// The axis the scrubber draws. A `MIN`/`MAX` and a `GROUP BY` over the
|
||||
// whole scope — 5.7 ms on 24,000 images — and the bars do not change as the
|
||||
// grid scrolls, only the marker on them does. The marker is moved by the
|
||||
// scroll handler on every event, without coming through here.
|
||||
//
|
||||
// Every other thing that *does* change the bars — a zoom, a pan, a scrub,
|
||||
// dates landing from the thumbnail worker — calls `refresh_timeline`
|
||||
// itself, so this being skipped cannot leave a stale axis on screen.
|
||||
if describes_something_new {
|
||||
refresh_timeline(window, catalog, ctl);
|
||||
}
|
||||
|
||||
let cells = if trash {
|
||||
library::read_trashed_cells(catalog, offset, window_size)
|
||||
@@ -2178,9 +2230,13 @@ fn load_window(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
if let Some(coll) = ctl.coll_ctl.borrow().as_ref().and_then(|w| w.upgrade()) {
|
||||
crate::collections_ui::sync_selection(window, &coll, &ids);
|
||||
}
|
||||
// The filter chips' counts describe the whole library, not this window, so
|
||||
// they are refreshed here rather than per cell.
|
||||
refresh_rating_counts(window, catalog);
|
||||
// The filter chips' counts describe the whole library, not this window —
|
||||
// which is exactly why they are not recomputed for a window that moved.
|
||||
// A judgement changes them and calls this itself (see `apply_judgement`),
|
||||
// so a cull still watches its own chips move.
|
||||
if describes_something_new {
|
||||
refresh_rating_counts(window, catalog);
|
||||
}
|
||||
|
||||
// The library got shorter while the view was looking at it — a delete.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user