Answer a thumbnail miss with the other stored class before the network

Offline, a grid zoomed past 256px was blank wherever it had not been
zoomed over before. The store was asked only for the exact class the cell
wanted, and the sweep stores only the grid class, so every zoomed cell
missed and went to a server that was not there — with its 256px thumbnail
sitting in the store the whole time. Online it cost the same round trip,
just without the blank cell at the end of it.

The split now tries the other class on a miss. A smaller one stands in
and the fetch for the real class still goes out; a larger one answers
the request outright, since there is nothing a fetch would improve on.
`ThumbnailReady` carries the class its pixels are, and the drain records
that rather than the batch's class, so a stand-in is replaced on the next
reload instead of being counted as served. `already_served` counts a
held large thumbnail as serving the grid class too, so zooming back out
does not re-read the store for pixels already on screen.

The split moves into `split_by_store` so it can be tested without a
worker thread or a network.

ui/dr-ui/src/library/cells.rs

@@ -1015,6 +1015,7 @@ mod tests {
             width,
             height,
             rgba,
+            class: dr_thumbs::ThumbSize::Grid,
             from_cache: true,
         };
 

ui/dr-ui/src/library/thumbnails_gen.rs

@@ -33,6 +33,11 @@ pub struct ThumbnailReady {
     pub width: u32,
     pub height: u32,
     pub rgba: Vec<u8>,
+    /// Which class these pixels are, which is not always the class asked for:
+    /// the store answers a miss with the other class where it holds one. The
+    /// grid records this, so a stand-in is replaced once the real class can be
+    /// fetched rather than counted as served.
+    pub class: dr_thumbs::ThumbSize,
     /// Whether these pixels came off local disk rather than the server.
     ///
     /// The grid paints both identically, so this exists solely for
@@ -61,6 +66,9 @@ pub enum ThumbnailMessage {
         cached: usize,
         fetching: usize,
         dating: usize,
+        /// Of `cached`, how many are the other class standing in for one
+        /// still being fetched. Each of those rows is delivered twice.
+        standing_in: usize,
     },
     /// One header-only date read is starting.
     ///
@@ -115,46 +123,12 @@ pub fn spawn_thumbnails(
 
         // Split the batch before delivering anything, so the plan can be
         // reported first and the UI knows the shape of the work up front.
-        // Decoding happens here rather than in the split, because a corrupt
+        let Split {
-        // blob turns a hit into a miss.
+            hits,
-        let mut hits = Vec::new();
+            to_fetch,
-        let mut to_fetch = Vec::new();
+            metadata_only,
-        // Images whose thumbnail is cached but whose date is still unknown.
+            standing_in,
-        //
+        } = split_by_store(store.as_ref(), wanted);
-        // These need a header read even though no pixels are wanted. Without
-        // this pass an image is dated *only* on the one visit that produced
-        // its thumbnail — so a library browsed once before the EXIF code
-        // existed, or synced from another device's shards, stays permanently
-        // undated and never appears on the timeline.
-        let mut metadata_only = Vec::new();
-
-        for req in wanted {
-            let stored = req
-                .file_id
-                .zip(store.as_ref())
-                .and_then(|(id, s)| s.get(id, req.thumb_size).ok().flatten());
-
-            match stored.map(|t| dr_thumbs::decode_rgba(&t.bytes)) {
-                Some(Ok((width, height, rgba))) => {
-                    if req.needs_metadata {
-                        metadata_only.push(req.clone());
-                    }
-                    hits.push(ThumbnailReady {
-                        row: req.row,
-                        width,
-                        height,
-                        rgba,
-                        from_cache: true,
-                    });
-                }
-                // A corrupt stored blob is a miss, not a failure.
-                Some(Err(e)) => {
-                    log::debug!("stored thumbnail unreadable, refetching: {e}");
-                    to_fetch.push(req);
-                }
-                None => to_fetch.push(req),
-            }
-        }
 
         log::info!(
             "thumbnails: {} from store, {} to fetch{}",
@@ -171,6 +145,7 @@ pub fn spawn_thumbnails(
                 cached: hits.len(),
                 fetching: to_fetch.len(),
                 dating: metadata_only.len(),
+                standing_in,
             })
             .is_err()
         {
@@ -269,6 +244,105 @@ pub fn spawn_thumbnails(
     rx
 }
 
+/// A batch divided by what the store can answer.
+#[derive(Default)]
+struct Split {
+    /// Decoded off local disk, ready to deliver.
+    hits: Vec<ThumbnailReady>,
+    /// Needing the network: a miss, or a smaller class standing in.
+    to_fetch: Vec<ThumbnailRequest>,
+    /// Images whose thumbnail is cached but whose date is still unknown.
+    ///
+    /// These need a header read even though no pixels are wanted. Without
+    /// this pass an image is dated *only* on the one visit that produced
+    /// its thumbnail — so a library browsed once before the EXIF code
+    /// existed, or synced from another device's shards, stays permanently
+    /// undated and never appears on the timeline.
+    metadata_only: Vec<ThumbnailRequest>,
+    /// Rows in both `hits` and `to_fetch`: the grid class shown while the
+    /// large one is fetched.
+    standing_in: usize,
+}
+
+/// Divide a batch into what the store holds and what must be fetched.
+///
+/// Decoding happens here rather than after, because a corrupt blob turns a
+/// hit into a miss.
+fn split_by_store(store: Option<&ThumbStore>, wanted: Vec<ThumbnailRequest>) -> Split {
+    let mut split = Split::default();
+
+    for req in wanted {
+        if let Some(hit) = read_stored(store, &req, req.thumb_size) {
+            if req.needs_metadata {
+                split.metadata_only.push(req.clone());
+            }
+            split.hits.push(hit);
+            continue;
+        }
+
+        // The other class, before the network. The sweep stores only the
+        // grid class, so a zoomed grid asked the server for every cell it had
+        // not zoomed over before — and offline, left each one blank with its
+        // 256px thumbnail sitting in the store. A softer cell is better than
+        // an empty one, and a sharper one is simply better.
+        // A wide panorama cell falls back to the grid class too: the sweep
+        // always stores it, and smaller, it only stands in while the wide
+        // class is fetched.
+        let other = match req.thumb_size {
+            dr_thumbs::ThumbSize::Grid => dr_thumbs::ThumbSize::Large,
+            dr_thumbs::ThumbSize::Large
+            | dr_thumbs::ThumbSize::Wide2
+            | dr_thumbs::ThumbSize::Wide3
+            | dr_thumbs::ThumbSize::Wide4 => dr_thumbs::ThumbSize::Grid,
+        };
+        match read_stored(store, &req, other) {
+            // Larger than asked for: nothing a fetch would improve on.
+            Some(hit) if other > req.thumb_size => {
+                if req.needs_metadata {
+                    split.metadata_only.push(req.clone());
+                }
+                split.hits.push(hit);
+            }
+            // Smaller: shown now, and the fetch still goes out to replace it.
+            // The fetch reads the header, so it dates the image too.
+            Some(stand_in) => {
+                split.standing_in += 1;
+                split.hits.push(stand_in);
+                split.to_fetch.push(req);
+            }
+            None => split.to_fetch.push(req),
+        }
+    }
+
+    split
+}
+
+/// A stored thumbnail of one class, decoded for the grid.
+///
+/// `None` for a miss, for a request with no file id to key on, and for a
+/// corrupt blob — which is a miss, not a failure, and gets refetched.
+fn read_stored(
+    store: Option<&ThumbStore>,
+    req: &ThumbnailRequest,
+    class: dr_thumbs::ThumbSize,
+) -> Option<ThumbnailReady> {
+    let stored = store?.get(req.file_id?, class).ok().flatten()?;
+    match dr_thumbs::decode_rgba(&stored.bytes) {
+        Ok((width, height, rgba)) => Some(ThumbnailReady {
+            row: req.row,
+            width,
+            height,
+            rgba,
+            class,
+            from_cache: true,
+        }),
+        Err(e) => {
+            log::debug!("stored thumbnail unreadable, refetching: {e}");
+            None
+        }
+    }
+}
+
 pub(super) async fn fetch_one(
     backend: &dyn RemoteBackend,
     decoder: &dyn dr_decode::Decoder,
@@ -302,6 +376,7 @@ pub(super) async fn fetch_one(
         width: preview.width,
         height: preview.height,
         rgba: preview.rgba,
+        class: req.thumb_size,
         from_cache: false,
     }))
 }
@@ -546,6 +621,86 @@ pub fn camera_label(make: Option<&str>, model: Option<&str>) -> Option<String> {
 mod tests {
     use super::*;
 
+    /// A store holding one image at one class, and a request for it.
+    fn store_with(tag: &str, file_id: u64, edge: u32, class: dr_thumbs::ThumbSize) -> ThumbStore {
+        let dir = std::env::temp_dir().join(format!("dr-ui-split-{tag}-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        let rgba: Vec<u8> = std::iter::repeat_n([10u8, 20, 30, 255], (edge * edge) as usize)
+            .flatten()
+            .collect();
+        let mut store = ThumbStore::open(&dir).unwrap();
+        let bytes = dr_thumbs::encode_rgba(edge, edge, &rgba).unwrap();
+        store
+            .put(
+                file_id,
+                class,
+                &dr_thumbs::Thumbnail {
+                    width: edge,
+                    height: edge,
+                    bytes,
+                },
+            )
+            .unwrap();
+        store
+    }
+
+    fn asking(file_id: u64, thumb_size: dr_thumbs::ThumbSize) -> ThumbnailRequest {
+        ThumbnailRequest {
+            row: 0,
+            path: "a.CR2".into(),
+            file_id: Some(file_id),
+            size: 0,
+            image_id: 1,
+            thumb_size,
+            needs_metadata: false,
+            full_resolution: false,
+        }
+    }
+
+    #[test]
+    fn a_zoomed_cell_is_drawn_from_the_grid_class_while_the_large_one_is_fetched() {
+        // The offline symptom: the sweep stores only the grid class, so a grid
+        // zoomed past 256px found nothing at its class and went to a server
+        // that was not there — every cell blank, its thumbnail in the store.
+        use dr_thumbs::ThumbSize::{Grid, Large};
+        let store = store_with("standin", 7, 8, Grid);
+
+        let split = split_by_store(Some(&store), vec![asking(7, Large)]);
+
+        assert_eq!(split.hits.len(), 1, "drawn from the store now");
+        assert_eq!(split.hits[0].class, Grid, "and recorded as what it is");
+        assert_eq!(split.to_fetch.len(), 1, "the large class is still wanted");
+        assert_eq!(split.standing_in, 1);
+    }
+
+    #[test]
+    fn a_large_thumbnail_answers_a_grid_request_without_a_fetch() {
+        use dr_thumbs::ThumbSize::{Grid, Large};
+        let store = store_with("larger", 7, 16, Large);
+
+        let split = split_by_store(Some(&store), vec![asking(7, Grid)]);
+
+        assert_eq!(split.hits.len(), 1);
+        assert_eq!(split.hits[0].class, Large);
+        assert!(
+            split.to_fetch.is_empty(),
+            "nothing a fetch would improve on"
+        );
+        assert_eq!(split.standing_in, 0);
+    }
+
+    #[test]
+    fn an_exact_hit_is_not_a_stand_in() {
+        use dr_thumbs::ThumbSize::Large;
+        let store = store_with("exact", 7, 16, Large);
+
+        let split = split_by_store(Some(&store), vec![asking(7, Large)]);
+
+        assert_eq!(split.hits.len(), 1);
+        assert!(split.to_fetch.is_empty());
+        assert_eq!(split.standing_in, 0);
+    }
+
     #[test]
     fn the_thumbnail_pass_asks_only_for_what_is_missing() {
         // The work list is the whole point of the pass being resumable and of

ui/dr-ui/src/library_ui/window.rs

@@ -96,8 +96,20 @@ fn already_served(
     held: &std::collections::HashMap<i64, Held>,
     ids: impl Iterator<Item = i64>,
 ) -> std::collections::HashSet<(i64, dr_thumbs::ThumbSize)> {
-    ids.filter_map(|id| Some((id, held.get(&id)?.class?)))
+    use dr_thumbs::ThumbSize;
-        .collect()
+    let mut served = std::collections::HashSet::new();
+    for id in ids {
+        let Some(class) = held.get(&id).and_then(|h| h.class) else {
+            continue;
+        };
+        served.insert((id, class));
+        // The large class is everything the grid class would be, and more: a
+        // cell the store answered with it has nothing left to ask for.
+        if class == ThumbSize::Large {
+            served.insert((id, ThumbSize::Grid));
+        }
+    }
+    served
 }
 
 /// Where the loaded window starts for a view whose first visible cell is
@@ -1014,11 +1026,13 @@ fn drain_thumbnails(
                         cached,
                         fetching,
                         dating,
+                        standing_in,
                     } => {
                         // Date reads produce no cell, so they are counted into
                         // the bar's denominator or it finishes while work is
-                        // still running.
+                        // still running. A stand-in is delivered once from the
-                        job.add_total(dating);
+                        // store and again from the fetch, so it counts twice.
+                        job.add_total(dating + standing_in);
 
                         let mut parts = Vec::new();
                         if cached > 0 {
@@ -1084,10 +1098,11 @@ fn drain_thumbnails(
                             row.has_thumb = true;
                             model.set_row_data(t.row, row);
                             // What this cell is now showing, so the next reload
-                            // can carry it over and know not to ask again.
+                            // can carry it over and know not to ask again. The
-                            if let Some(class) = classes.get(t.row) {
+                            // pixels' own class, not the one the row asked for:
-                                record_class(&ctl_cb, t.row, *class);
+                            // a stand-in recorded as the class it stands in for
-                            }
+                            // would never be replaced.
+                            record_class(&ctl_cb, t.row, t.class);
                         }
                     }
                     ThumbnailMessage::Unavailable { row, reason } => {
@@ -1542,6 +1557,22 @@ mod tests {
         );
     }
 
+    #[test]
+    fn a_large_thumbnail_satisfies_the_grid_class() {
+        // Zooming out past 256px reloads the window at the grid class. A cell
+        // already showing the large class has every pixel the small one would
+        // give, so asking the store for it again is a read for nothing.
+        let held = hold_thumbnails(
+            &model_of(&[5], &[5]),
+            &[5],
+            &[Some(dr_thumbs::ThumbSize::Large)],
+        );
+        let served = already_served(&held, [5i64].into_iter());
+
+        assert!(served.contains(&(5, dr_thumbs::ThumbSize::Large)));
+        assert!(served.contains(&(5, dr_thumbs::ThumbSize::Grid)));
+    }
+
     #[test]
     fn a_cell_whose_class_was_never_recorded_is_fetched_again() {
         // Pixels with no class are pixels from before this bookkeeping existed