Fetch what is on screen before what is not
The other half of the blank bottom row, on a library still filling its thumbnail store: the cells were in the model, and nobody had asked the server for them yet. A batch is fetched one image at a time, two round trips each, and it is abandoned wholesale the moment the window moves. Issued in model order, the front of that queue was the quarter-window of cells sitting *above* the view — which nobody is looking at — and the back of it was the bottom of the screen and the screenfuls below. So the last rows of the grid waited behind three quarters of a window's worth of fetches for photographs off screen, and every scroll threw the queue away and started over from above the view again. For as long as the scrolling continued, the bottom of the grid could be starved. The rows still address the model they were built against; only the order they are asked for in changes. On screen first, in reading order, then the rows below the view, then the rows above it. Below before above because that is where the view is going — scrolling back over cells already fetched is served from the store, and from `requested` without a fetch at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2771,6 +2771,37 @@ fn flag_from_code(v: i32) -> dr_types::FlagState {
|
||||
}
|
||||
}
|
||||
|
||||
/// Where a row of the loaded window sits in the fetch queue.
|
||||
///
|
||||
/// On screen first, top to bottom; then the rows below the view, nearest
|
||||
/// first; then the rows above it, nearest first.
|
||||
///
|
||||
/// # Why the queue has an order at all
|
||||
///
|
||||
/// The batch is fetched one image at a time, two round trips each, and it is
|
||||
/// abandoned wholesale the moment the window moves — so whatever is at the
|
||||
/// back of it on a remote library is not slow to appear, it never appears.
|
||||
/// Issued in model order, the back of the queue was the bottom of the screen
|
||||
/// and everything below it, and the *front* was the quarter-window of cells
|
||||
/// above the view that nobody was looking at. A scroll then abandoned the
|
||||
/// batch and the new one started over, again from above the view: on a
|
||||
/// library still filling its store, the last rows of the grid could be
|
||||
/// starved for as long as the scrolling continued.
|
||||
///
|
||||
/// Below before above because that is where the view is going. Scrolling back
|
||||
/// over cells already fetched is served from the store, and from `requested`
|
||||
/// without a fetch at all.
|
||||
fn fetch_rank(row: usize, first_on_screen: usize, on_screen: usize) -> (u8, usize) {
|
||||
let past = first_on_screen + on_screen;
|
||||
if row >= first_on_screen && row < past {
|
||||
(0, row - first_on_screen)
|
||||
} else if row >= past {
|
||||
(1, row - past)
|
||||
} else {
|
||||
(2, first_on_screen - row)
|
||||
}
|
||||
}
|
||||
|
||||
/// Fetch thumbnails for rows in the model that do not have one yet.
|
||||
fn request_thumbnails(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
let Some((creds, session, _)) = ctl.session.borrow().clone() else {
|
||||
@@ -2783,7 +2814,7 @@ fn request_thumbnails(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
let cell_pixels = window.get_library_cell_size().max(1.0) as u32;
|
||||
let class = dr_thumbs::ThumbSize::for_cell(cell_pixels);
|
||||
|
||||
let wanted: Vec<library::ThumbnailRequest> = {
|
||||
let mut wanted: Vec<library::ThumbnailRequest> = {
|
||||
let paths = ctl.paths.borrow();
|
||||
let file_ids = ctl.file_ids.borrow();
|
||||
let sizes = ctl.sizes.borrow();
|
||||
@@ -2820,6 +2851,15 @@ fn request_thumbnails(window: &AppWindow, ctl: &Rc<LibraryController>) {
|
||||
return;
|
||||
}
|
||||
|
||||
// What is on screen, first — see [`fetch_rank`]. The rows keep addressing
|
||||
// the model they were built against; only the order they are asked for in
|
||||
// changes.
|
||||
{
|
||||
let first_on_screen = ctl.resume_at.get().saturating_sub(*ctl.offset.borrow());
|
||||
let on_screen = ctl.viewport_cells.get().max(1);
|
||||
wanted.sort_by_key(|r| fetch_rank(r.row, first_on_screen, on_screen));
|
||||
}
|
||||
|
||||
let requested = wanted.len();
|
||||
let rx = library::spawn_thumbnails(
|
||||
creds,
|
||||
@@ -5821,6 +5861,52 @@ mod tests {
|
||||
assert_eq!(window_move(39, 0, W, ON_SCREEN, 40), None);
|
||||
}
|
||||
|
||||
// --- which cells are fetched first -------------------------------------
|
||||
|
||||
#[test]
|
||||
fn the_visible_cells_are_fetched_before_anything_else() {
|
||||
// The window holds a quarter of itself above the view, and in model
|
||||
// order those cells — which nobody is looking at — were fetched first,
|
||||
// ahead of the whole screen.
|
||||
let window: Vec<usize> = (0..W).collect();
|
||||
let first_on_screen = W / 4;
|
||||
let mut order = window.clone();
|
||||
order.sort_by_key(|row| fetch_rank(*row, first_on_screen, ON_SCREEN));
|
||||
|
||||
assert_eq!(
|
||||
&order[..ON_SCREEN],
|
||||
&window[first_on_screen..first_on_screen + ON_SCREEN],
|
||||
"the screen is not fetched first, in reading order"
|
||||
);
|
||||
// The bottom row of the grid — the one reported missing — comes before
|
||||
// every offscreen cell rather than after all of them.
|
||||
let bottom = first_on_screen + ON_SCREEN - 1;
|
||||
assert!(
|
||||
order.iter().position(|r| *r == bottom).unwrap() < ON_SCREEN,
|
||||
"the bottom row of the grid is still behind offscreen cells"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn offscreen_cells_are_fetched_nearest_first_below_before_above() {
|
||||
let first_on_screen = W / 4;
|
||||
let past = first_on_screen + ON_SCREEN;
|
||||
// The row just below the view beats the row just above it, and both
|
||||
// beat rows further out on their own side.
|
||||
assert!(
|
||||
fetch_rank(past, first_on_screen, ON_SCREEN)
|
||||
< fetch_rank(first_on_screen - 1, first_on_screen, ON_SCREEN)
|
||||
);
|
||||
assert!(
|
||||
fetch_rank(past, first_on_screen, ON_SCREEN)
|
||||
< fetch_rank(past + 1, first_on_screen, ON_SCREEN)
|
||||
);
|
||||
assert!(
|
||||
fetch_rank(first_on_screen - 1, first_on_screen, ON_SCREEN)
|
||||
< fetch_rank(0, first_on_screen, ON_SCREEN)
|
||||
);
|
||||
}
|
||||
|
||||
// --- carrying thumbnails across a reload -------------------------------
|
||||
//
|
||||
// The black flash: `load_window` rebuilds every row, and until these it
|
||||
|
||||
Reference in New Issue
Block a user