diff --git a/ui/dr-ui/src/identity_ui.rs b/ui/dr-ui/src/identity_ui.rs index b543b97..c61aad8 100644 --- a/ui/dr-ui/src/identity_ui.rs +++ b/ui/dr-ui/src/identity_ui.rs @@ -13,7 +13,7 @@ use std::time::Duration; use dr_catalog::faces::{FaceId, PersonId}; use dr_catalog::Catalog; use dr_thumbs::ThumbStore; -use slint::{ComponentHandle, ModelRc, VecModel}; +use slint::{ComponentHandle, Model as _, ModelRc, VecModel}; use crate::faces::FaceSweepMessage; use crate::identity::{self, FaceCell, PersonRow}; @@ -120,20 +120,28 @@ impl IdentityController { /// long as the person exists. On a library with a few hundred named people /// that is worth tens of megabytes of nothing but a saved decode. /// - /// Costless to lose. `refresh` rebuilds any portrait it does not find, so - /// the only consequence is the JPEG decode this cache exists to skip, and - /// only for the people the rail is actually showing at the time. + /// Costless to lose. The next reload hands whatever it does not find to + /// `fill_covers`, so the only consequence is the JPEG decode this cache + /// exists to skip — paid off the blocking path, a slice at a time. pub fn clear_covers(&self) { self.covers.borrow_mut().clear(); } } /// Push the people rail and the face grid into the window. +/// +/// **Draws with the portraits it already has and cuts the rest afterwards.** +/// Cutting one costs a JPEG decode, and where the face predates the stored-crop +/// column it costs a whole 1024px proxy decode — 3.2 seconds of them on the +/// reference library, every one of which used to be spent between the click on +/// "Identity" and the screen appearing. `fill_covers` does that work a slice at +/// a time, in rail order, so the rail is on screen immediately and fills in +/// from the top. pub fn refresh( window: &AppWindow, - ctl: &IdentityController, + ctl: &Rc, catalog: &Rc>>, - store: Option<&ThumbStore>, + store: Option>, ) { let borrow = catalog.borrow(); let Some(cat) = borrow.as_ref() else { @@ -166,20 +174,22 @@ pub fn refresh( // see the rule in `widgets.slint`. The toggle reloads either way, so this // costs a pass over a list that was already in hand. let show_ignored = ctl.show_ignored.get(); + // Whoever the rail is missing a portrait for, as (row, person) in rail + // order — which is the order `fill_covers` works in, so the rows the user + // is looking at are cut first. + let mut pending: Vec<(usize, PersonId)> = Vec::new(); let rows: Vec = view .people .iter() .filter(|p| show_ignored || !p.ignored) - .map(|p| { - let cover = store.and_then(|store| { - if let Some(img) = ctl.covers.borrow().get(&p.id) { - return Some(img.clone()); - } - let crop = identity::load_cover(cat, store, p.id).ok().flatten()?; - let img = to_slint_image(crop.width, crop.height, &crop.rgba); - ctl.covers.borrow_mut().insert(p.id, img.clone()); - Some(img) - }); + .enumerate() + .map(|(row, p)| { + // The cache only. Cutting a portrait here is what used to hold the + // screen shut; anything not already in hand is left to `fill_covers`. + let cover = ctl.covers.borrow().get(&p.id).cloned(); + if cover.is_none() && store.is_some() { + pending.push((row, p.id)); + } IdentityPerson { id: p.id.0 as i32, @@ -215,7 +225,7 @@ pub fn refresh( } } - match (ctl.selected.get(), store) { + match (ctl.selected.get(), store.as_deref()) { (Some(person), Some(store)) => { let cells = identity::load_faces(cat, store, person).unwrap_or_else(|e| { log::warn!("identity: reading faces: {e}"); @@ -252,7 +262,107 @@ pub fn refresh( window.set_identity_picked(ctl.picked.borrow().len() as i32); drop(borrow); - refresh_coverage(window, catalog, store); + refresh_coverage(window, catalog, store.as_deref()); + + // Last, so a portrait cannot delay anything above it. + if let Some(store) = store { + fill_covers(window, ctl, catalog, store, pending); + } +} + +/// How long one slice of portrait-cutting may hold the UI thread. +/// +/// Under half a 60Hz frame. The unit of work is one portrait and it is not +/// divisible, so a slice overruns by whatever the last one cost — a stored crop +/// is well under a millisecond, and the proxy fallback is about three and a +/// half, which is a frame that renders late rather than a frame that is missed. +const COVER_SLICE: Duration = Duration::from_millis(8); + +/// Cut the rail portraits that were not already cached, a slice per tick. +/// +/// # Why a timer and not a thread +/// +/// The work is a `Catalog` query and a decode against a `ThumbStore`, and both +/// handles live on this thread — a worker would need its own connection to the +/// same SQLite file, which is what the sweep and the regrouping pass do because +/// they run for minutes and would otherwise be unbounded. This is seconds of +/// small, independent pieces, so slicing it is the cheaper answer to the same +/// question: nothing here ever holds the thread for longer than a frame. +/// +/// # Ordering +/// +/// `pending` arrives in rail order, and the rail is sorted by confirmed faces, +/// so the portraits the user is looking at are cut first and the tail fills in +/// behind them. With the rail virtualised, the rows past the fold are not even +/// drawn until they are scrolled to. +/// +/// # Why it patches rather than reloads +/// +/// A portrait changes one cell. Calling `refresh` for it would re-read the +/// catalog and rebuild the model on every tick, and the model being replaced +/// underneath the `ListView` is exactly what the slicing exists to avoid. +fn fill_covers( + window: &AppWindow, + ctl: &Rc, + catalog: &Rc>>, + store: Rc, + pending: Vec<(usize, PersonId)>, +) { + if pending.is_empty() { + // Parking `None` stops whatever the last refresh left running: its + // pending list is about a model that no longer exists. + park_cover_timer(None); + return; + } + + let timer = slint::Timer::default(); + let weak = window.as_weak(); + let ctl = ctl.clone(); + let catalog = catalog.clone(); + // Where the last tick got to. The list is only read forwards. + let mut next = 0usize; + + timer.start(slint::TimerMode::Repeated, COVER_SLICE, move || { + let Some(w) = weak.upgrade() else { return }; + let borrow = catalog.borrow(); + let Some(cat) = borrow.as_ref() else { + park_cover_timer(None); + return; + }; + let rows = w.get_identity_people(); + + let started = std::time::Instant::now(); + while next < pending.len() && started.elapsed() < COVER_SLICE { + let (row, person) = pending[next]; + next += 1; + + // The row index came from the model this fill was started for, and + // a reload between ticks replaces that model. Checked rather than + // trusted, because the failure it prevents is silent and wrong: a + // face drawn beside somebody else's name. A mismatch — or a model + // too short to hold the row — means this fill is about a rail that + // no longer exists, and the reload that replaced it started its own. + let Some(mut cell) = rows.row_data(row).filter(|c| c.id == person.0 as i32) else { + park_cover_timer(None); + return; + }; + + let Ok(Some(crop)) = identity::load_cover(cat, &store, person) else { + continue; + }; + let img = to_slint_image(crop.width, crop.height, &crop.rgba); + ctl.covers.borrow_mut().insert(person, img.clone()); + + cell.has_cover = true; + cell.cover = img; + rows.set_row_data(row, cell); + } + + if next >= pending.len() { + park_cover_timer(None); + } + }); + park_cover_timer(Some(timer)); } /// Run the batch check and put its answer on screen. @@ -467,7 +577,7 @@ pub fn wire( // drift from the catalog in exactly the cases that matter. macro_rules! reload { ($w:expr, $ctl:expr, $catalog:expr, $store:expr) => { - refresh(&$w, &$ctl, &$catalog, $store().as_deref()) + refresh(&$w, &$ctl, &$catalog, $store()) }; } @@ -895,7 +1005,7 @@ pub fn wire( // makes new people; a stale cache would draw the // previous pass's faces beside the new groups. ctl_tick.covers.borrow_mut().clear(); - refresh(&w, &ctl_tick, &catalog_tick, store_tick().as_deref()); + refresh(&w, &ctl_tick, &catalog_tick, store_tick()); } }, ); @@ -999,7 +1109,7 @@ pub fn wire( // grouped, so a sweep ending with an unchanged people // rail is expected. The coverage line is what shows it // worked, and Regroup is the next step. - refresh(&w, &ctl_tick, &catalog_tick, store_tick().as_deref()); + refresh(&w, &ctl_tick, &catalog_tick, store_tick()); } }, ); @@ -1127,6 +1237,24 @@ fn park_preview_timer(timer: slint::Timer) { PREVIEW_TIMER.with(|slot| *slot.borrow_mut() = Some(timer)); } +/// Keep the portrait fill's slice timer alive — and stop it. +/// +/// A third slot, because a fill runs at the same time as anything else the +/// screen is doing: it starts on every reload, and a reload is how a sweep and +/// a regrouping pass report progress. +/// +/// Takes an `Option` where the others do not, because this is the one that ends +/// on its own. `None` drops the parked timer, which stops it — including from +/// inside its own callback, which is where the last slice does it. Slint +/// supports that directly: a timer removed while its callback is running is +/// marked and dropped afterwards. +fn park_cover_timer(timer: Option) { + thread_local! { + static COVER_TIMER: RefCell> = const { RefCell::new(None) }; + } + COVER_TIMER.with(|slot| *slot.borrow_mut() = timer); +} + #[cfg(test)] mod tests { use super::*;