diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 483976a..3ef9090 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -1285,6 +1285,12 @@ pub fn run(paths: Vec) -> Result<()> { { // What the cache actually holds, so the ceiling above it is a figure // the user can judge rather than an abstract one. + // + // Empty at this point on a launch and that is expected: the catalog it + // queries is being opened on a worker (`library_ui::open_catalog_soon`) + // and is not there yet. The figure is filled in when the settings page + // is opened, which is the only time it is looked at — see the `on_open` + // closure passed to `settings_ui::wire`. settings.set_usage_label(describe_cache_usage(&library)); // Rendered once up front so the page is correct the first time it is // opened, rather than on the second open after a callback has run. @@ -1461,6 +1467,7 @@ pub fn run(paths: Vec) -> Result<()> { // they are only ever looked at while this page is on screen. let lib = library.clone(); let catalog = library.catalog(); + let ctl = settings.clone(); move |w: &AppWindow| { let store = lib.session().and_then(|c| { dr_thumbs::ThumbStore::open(&library::thumbs_dir(&c.account)).ok() @@ -1471,6 +1478,14 @@ pub fn run(paths: Vec) -> Result<()> { .and_then(|c| library::face_models(&c.account)) .is_none(), ); + // Here rather than at startup, on exactly the reasoning + // above: the catalog this reads is opened on a worker now, + // so a launch has nothing to describe, and the figure is + // only ever read while this page is on screen. `render` + // again because `settings_ui::wire` renders *before* it + // calls this, and the label is one of the things it draws. + ctl.set_usage_label(describe_cache_usage(&lib)); + settings_ui::render(w, &ctl); } }, ); diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index 89075c7..28c4ce3 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -165,6 +165,11 @@ pub struct LibraryController { /// fetched independently: holding the 256px one says nothing about whether /// the large one has been asked for. requested: RefCell>, + /// Drains the worker that opens and verifies the catalog — see + /// [`spawn_catalog_open`]. Held for the same reason every other timer here + /// is: a `slint::Timer` stops when it is dropped, so a local one would + /// never fire. + catalog_timer: RefCell>, scan_timer: RefCell>, thumb_timer: RefCell>, /// The whole-library sweep, which outlives any one grid window. @@ -371,6 +376,7 @@ impl LibraryController { viewport_cells: std::cell::Cell::new(INITIAL_VIEWPORT_CELLS), library_facts: RefCell::new(None), requested: RefCell::new(Default::default()), + catalog_timer: RefCell::new(None), scan_timer: RefCell::new(None), thumb_timer: RefCell::new(None), sweep_timer: RefCell::new(None), @@ -849,6 +855,199 @@ pub fn open( } let path = library::catalog_path(&conn.account); + + // Before the scan, not after it: the grid can be filled from disk now and + // the scan is only ever going to add to it. + // + // And it is the gate on the scan, not merely a prelude to it — a damaged + // catalog has a question on screen, and a scan writing into it while that + // question is unanswered is how the last good copy gets destroyed. That + // gate is why the scan starts from the drain below rather than from here: + // the answer no longer arrives on the next line. + open_catalog_soon(window, ctl, coll_ctl, conn, filter, path); +} + +/// Open the catalog on a worker, then show it and start the scan. +/// +/// # Why this is not `show_catalog_now` on the spot +/// +/// It was, and it was the largest blocking call on the launch path. +/// [`Catalog::open_verified`] runs `PRAGMA quick_check`, which reads **every +/// page** of the database, and then `Catalog::open`, which takes a full copy of +/// the file before a migration and rewrites its structure. On a 50,000-image +/// library that is tens of megabytes of I/O, and all of it happened before the +/// window had painted anything. +/// +/// On Android that is not a stutter but an ANR. `dr_ui::run` is called from +/// `android_main` and does not reach `window.run()` — the first `poll_events`, +/// and so the first time anything drains the activity's input channel — until +/// every line above it has finished. Five seconds of that and the system offers +/// to kill the app. +/// +/// [`crate::recovery_ui`] and `recovery`'s own module documentation both say the +/// check belongs "at startup, where a failure has a user in front of it who can +/// answer a question". That was always the intent; this is what makes it true, +/// because the question can only be asked once there is an interface to ask it +/// in. +/// +/// # What the user sees meanwhile +/// +/// `library-opening`, which the grid's empty state draws as "Opening the +/// library…" rather than as "Scanning…". They are different answers — one is +/// reading a file on this device, the other is walking a tree over the network +/// — and the grid already refuses to conflate the two states it had. +/// +/// `library-scanning` stays true throughout, which is what keeps the gate above +/// from being merely advisory: the header hides Rescan while it is set, so +/// there is no button that could start a scan into a catalog this has not +/// finished checking. +fn open_catalog_soon( + window: &AppWindow, + ctl: Rc, + coll_ctl: Rc, + conn: Connection, + filter: FormatFilter, + path: PathBuf, +) { + // Already open: a library being re-opened within one session, which is the + // case `show_catalog_now` short-circuited. There is nothing to wait for and + // nothing to say, so the scan starts immediately as it always did. + if ctl.catalog.borrow().is_some() { + begin_scan(window, &ctl, &coll_ctl, conn, filter, path); + return; + } + + window.set_library_opening(true); + window.set_library_status("Reading the catalog on this device…".into()); + + let rx = spawn_catalog_open(path.clone()); + let weak = window.as_weak(); + let held = ctl.clone(); + let timer = slint::Timer::default(); + timer.start( + slint::TimerMode::Repeated, + // Tighter than the scan's 120 ms: this one lands once, and everything + // the grid can show is waiting behind it. + std::time::Duration::from_millis(30), + move || { + let Some(w) = weak.upgrade() else { return }; + let got = match rx.try_recv() { + Ok(got) => got, + Err(std::sync::mpsc::TryRecvError::Empty) => return, + // The worker died without answering — a panic inside SQLite, + // realistically. Treated as "no catalog to show", which is what + // the synchronous path did with any error it could not + // classify: the scan is the thing that has to work. + Err(std::sync::mpsc::TryRecvError::Disconnected) => { + CatalogOpen::Failed("the catalog worker stopped without answering".into()) + } + }; + stop(&held.catalog_timer); + w.set_library_opening(false); + + match got { + CatalogOpen::Opened(cat) => { + adopt_catalog(&w, &held, &coll_ctl, cat); + begin_scan( + &w, + &held, + &coll_ctl, + conn.clone(), + filter.clone(), + path.clone(), + ); + } + CatalogOpen::Corrupt(detail) => { + // No scan, and this is the whole reason the scan waits for + // this answer: `Catalog::open` succeeds on a file whose + // header survived, so a scan would write ETags and image + // rows into damaged pages while the user is still reading + // the question — turning a file that had a backup into one + // where the backup is the only copy left. + // + // `offer` clears the scanning state itself, for exactly + // that reason, so nothing here does it twice. + crate::recovery_ui::offer(&w, &path, &detail); + } + CatalogOpen::Failed(why) => { + // Not surfaced: this is a first run more often than it is + // anything else, the empty state already says the scan is + // running, and an error here would contradict a scan that + // is working perfectly. If the scan fails too, it reports + // for both of them. + log::info!("no catalog to show before the scan: {why}"); + begin_scan( + &w, + &held, + &coll_ctl, + conn.clone(), + filter.clone(), + path.clone(), + ); + } + } + }, + ); + *ctl.catalog_timer.borrow_mut() = Some(timer); +} + +/// What the catalog worker found. +enum CatalogOpen { + /// Checked, opened, migrated forward and backfilled. + Opened(Catalog), + /// `PRAGMA quick_check` failed. Carries SQLite's own words, because the + /// message the user is shown is the diagnosis rather than a paraphrase. + Corrupt(String), + /// Anything else: no catalog file yet, a permission, a locked database. + Failed(String), +} + +/// Check, open and migrate the catalog on a thread of its own. +/// +/// `Catalog` holds a `rusqlite::Connection`, which is `Send` and not `Sync` — +/// exactly the shape that can be built on a worker and handed over once, which +/// is what this does. Nothing else touches the file while it runs: this is the +/// only opener on the launch path, and the scan that opens its own connection +/// does not start until the answer has landed. +fn spawn_catalog_open(path: PathBuf) -> Receiver { + let (tx, rx) = std::sync::mpsc::channel(); + + std::thread::spawn(move || { + let started = std::time::Instant::now(); + let message = match Catalog::open_verified(&path) { + Ok(cat) => { + // The number this change exists for. Worth an `info` line on + // every launch: it is the one figure that says whether a slow + // start is the catalog or something else, and it is not + // measurable from anywhere else. + log::info!( + "catalog checked and opened in {} ms", + started.elapsed().as_millis() + ); + CatalogOpen::Opened(cat) + } + Err(dr_catalog::CatalogError::Corrupt { detail }) => CatalogOpen::Corrupt(detail), + Err(e) => CatalogOpen::Failed(e.to_string()), + }; + let _ = tx.send(message); + }); + + rx +} + +/// Start the scan that brings the catalog up to date. +/// +/// Split out of [`open`] because it no longer runs there — see +/// [`open_catalog_soon`] — and shared with the two paths that reach it once the +/// catalog has been answered for. +fn begin_scan( + window: &AppWindow, + ctl: &Rc, + coll_ctl: &Rc, + conn: Connection, + filter: FormatFilter, + path: PathBuf, +) { log::info!( "scanning {} for {} format(s) → {}", if conn.account.root.is_empty() { @@ -859,16 +1058,7 @@ pub fn open( filter.iter().count(), path.display() ); - - // Before the scan, not after it: the grid can be filled from disk now and - // the scan is only ever going to add to it. - // - // And it is the gate on the scan, not merely a prelude to it — a damaged - // catalog has a question on screen, and a scan writing into it while that - // question is unanswered is how the last good copy gets destroyed. - if !show_catalog_now(window, &ctl, &path, &coll_ctl) { - return; - } + window.set_library_status("Starting…".into()); let rx = library::spawn_scan( conn.clone(), @@ -877,7 +1067,7 @@ pub fn open( path.clone(), ); - drain_scan(window.as_weak(), ctl, coll_ctl, rx, path); + drain_scan(window.as_weak(), ctl.clone(), coll_ctl.clone(), rx, path); } /// Drain scan progress on the UI thread. @@ -1904,6 +2094,15 @@ fn schedule_reload(window: &AppWindow, ctl: &Rc) { /// Show what the catalog already holds, without waiting for the scan. /// +/// **Blocking, and no longer on the launch path.** [`open_catalog_soon`] is +/// what a launch goes through; this is what [`crate::recovery_ui`] goes through +/// once the user has answered the damage question, where the event loop is +/// already running, the file has just been replaced under a `forget_catalog`, +/// and there is a `recovery-busy` state on screen saying so. Same reasoning as +/// `recovery_ui::answer` gives for doing the file copy itself in place: nothing +/// else is on screen to block, and a worker would buy a spinner and cost the +/// guarantee that nothing else touches the file while it is being replaced. +/// /// # Why a launch should not be a scan /// /// A launch does not have to discover the library. The catalog from the last @@ -1939,10 +2138,10 @@ pub(crate) fn show_catalog_now( return true; } - // Verified rather than plain: this is the once-per-launch moment where a - // full check is affordable and there is a user in front of it who can - // answer the question. See `dr_catalog::recovery` for why it is not on - // every open. + // Verified rather than plain: this is a moment where a full check is + // affordable and there is a user in front of it who can answer the + // question — they have just answered one. See `dr_catalog::recovery` for + // why it is not on every open. let cat = match Catalog::open_verified(catalog_path) { Ok(cat) => cat, Err(dr_catalog::CatalogError::Corrupt { detail }) => { @@ -1957,11 +2156,32 @@ pub(crate) fn show_catalog_now( } }; + adopt_catalog(window, ctl, coll_ctl, cat); + true +} + +/// Take an opened catalog into the interface. +/// +/// The UI half of opening a catalog, separated from the opening itself so that +/// [`open_catalog_soon`] — which does the opening on a worker — and +/// [`show_catalog_now`] — which still does it in place, for +/// [`crate::recovery_ui`], where the event loop is already running and the file +/// has just been replaced — describe the grid the same way. +/// +/// Everything here is UI-thread work by necessity: it fills the sidebar and the +/// grid model. +fn adopt_catalog( + window: &AppWindow, + ctl: &Rc, + coll_ctl: &Rc, + cat: Catalog, +) { // Before anything else can see this catalog, and exactly once per open — - // the early return above is what makes it once. The job queue is durable, - // so a run that was killed mid-job left its row marked `Running` with - // nobody holding it; recovery hands those back to be resumed rather than - // lost, and drops jobs naming photographs that have since been deleted. + // the callers' early return on an already-open catalog is what makes it + // once. The job queue is durable, so a run that was killed mid-job left its + // row marked `Running` with nobody holding it; recovery hands those back to + // be resumed rather than lost, and drops jobs naming photographs that have + // since been deleted. // // Here rather than wherever a runner starts, because there is no owner // column in `jobs`: a second recovery pass while a worker held a claim @@ -1985,7 +2205,6 @@ pub(crate) fn show_catalog_now( crate::collections_ui::refresh_tree(window, coll_ctl, &cat); *ctl.catalog.borrow_mut() = Some(cat); load_window(window, ctl); - true } /// Drop the open catalog, so the next `show_catalog_now` opens the file diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 0e23ca2..841086e 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -294,6 +294,15 @@ export component AppWindow inherits Window { callback activity-clear-finished(); in property library-scanning: false; + /// The catalog on this device is being opened and checked, on a worker, + /// before the scan starts (`library_ui::open_catalog_soon`). + /// + /// Separate from `library-scanning` because they are different answers to + /// the same question: this one is reading a file that is already here, and + /// that one is walking a tree over the network. Both are true at once for + /// the first moment of a launch, and only the more specific of them is + /// worth putting on an empty grid. + in property library-opening: false; in property library-status: ""; in property library-error: ""; @@ -1517,6 +1526,7 @@ in property panel-visible: true; cells: root.library-cells; total: root.library-total; scanning: root.library-scanning; + opening: root.library-opening; scan-status: root.library-status; scan-error: root.library-error; diff --git a/ui/dr-ui/ui/library.slint b/ui/dr-ui/ui/library.slint index ce673d1..ae76c71 100644 --- a/ui/dr-ui/ui/library.slint +++ b/ui/dr-ui/ui/library.slint @@ -982,6 +982,10 @@ export component LibraryGrid inherits Rectangle { in property <[LibraryCell]> cells; in property total: 0; in property scanning: false; + /// The catalog on this device is being opened and checked before the scan + /// starts. Distinct from `scanning` for the reason the empty state below + /// gives: they are two different waits and only one of them is the network. + in property opening: false; in property scan-status: ""; in property scan-error: ""; /// Which folder is being shown. Visible at all times: two similarly-named @@ -2415,9 +2419,20 @@ export component LibraryGrid inherits Rectangle { // // "Still scanning" and "scanned, found nothing" are different answers. // Conflating them is how a working scan looks broken. + // + // And "still opening" is a third, which is the state a launch is in + // for as long as it takes to read and check the catalog on this + // device. It used to be spent before the window had painted at all, + // where it read as a frozen application — on Android, as an ANR. + // Now it is a worker, so it needs a sentence: a grid saying + // "Scanning…" while nothing is on the network is the same kind of + // lie the two answers below were separated to avoid. if root.total == 0: EmptyState { - headline: root.scanning ? "Scanning…" : "No images found"; - detail: root.scanning ? root.scan-status + headline: root.opening + ? "Opening the library…" + : (root.scanning ? "Scanning…" : "No images found"); + detail: (root.opening || root.scanning) + ? root.scan-status : "Check the library folder and which formats are ticked."; }