Step along the roll by library ordinal, and keep its mark on the open photograph

In a library's develop view the arrows, space, A and D opened
`library-roll-pick(library-roll-current ± 1)`. `roll-current` is a row
of the loaded window, set when a photograph was opened and never again.
Two things followed on any library larger than one window:

- Holding D stopped dead at the end of the loaded window: the pick of a
  row past `ctl.paths` found no path and did nothing, a few screenfuls
  into a library of thousands. A stopped at its start the same way.
- Any reload of the window — a background sync, a judgement that drops
  the open frame out of a filter — left `roll-current` on a row that now
  held another photograph. The roll marked it, develop's rating keys
  judged it, and the next step walked on from it.

The keys now call `library-roll-step(delta)`, and Rust steps as
`move_cursor` does in the grid: from the open photograph's ordinal
(`index`, which `report_position` already keeps), clamped to the
library, loading the window around the target only when it lies
outside. The window-bringing half of `place_cursor` is shared for this
as `bring_window_to`, so the grid cursor and the roll move the window
the same way. Catalog reads stay proportional to the window: one
`load_window` per window crossed, none per step within it.

The open photograph is also remembered by image id. Every
`load_window` finds it again in the rows it just read (a scan of one
window, no query), puts `roll-current` and `index` back on it, or takes
the mark off when it is not there. When it is missing but its ordinal
is still inside the window, it has left the grid and its successor
moved up into its place, so the next step forward lands on that ordinal
instead of skipping the successor.

A click on the roll still reports a row; it is right because the cells
it is drawn from and `ctl.paths` are replaced together, and it now goes
through the same open path as a step.

Fixes #64.
This commit is contained in:
2026-09-25 23:00:55 -04:00
parent ce72fe49a0
commit a030bfd239
8 changed files with 422 additions and 101 deletions
+18
View File
@@ -128,6 +128,22 @@ pub struct LibraryController {
/// quarter-window above the view. Restoring that as a viewport position
/// would land the user consistently short of where they were.
pub(super) resume_at: std::cell::Cell<usize>,
/// The photograph open in develop, by catalog id, when it was opened from
/// this library.
///
/// **An id, not a row of the loaded window.** The roll marks a row and the
/// keyboard steps from one, but a row only names a photograph until the
/// window is next re-read — a step past its edge, a background sync, a
/// judgement that drops the frame out of a filter. Each reload finds this
/// id again in the new window (see `window::follow_open`) and puts the
/// roll's mark, and the ordinal `index` reports, back on it.
pub(super) roll_open: std::cell::Cell<Option<i64>>,
/// The open photograph was not in the window last read, although the
/// ordinal it held still was: it has left the grid — a judgement under a
/// filter, most often — and the photograph after it has moved up into its
/// place. A step forward is then onto that ordinal, not past it, or the
/// frame that closed the gap would be skipped.
pub(super) roll_left: std::cell::Cell<bool>,
/// How many cells to load, derived from what the viewport can show.
///
/// A fixed count is wrong in both directions: too small on a maximised 4K
@@ -430,6 +446,8 @@ impl LibraryController {
thumb_class: RefCell::new(Vec::new()),
offset: RefCell::new(0),
resume_at: std::cell::Cell::new(0),
roll_open: std::cell::Cell::new(None),
roll_left: std::cell::Cell::new(false),
window: RefCell::new((INITIAL_VIEWPORT_CELLS * SCREENFULS).max(MIN_WINDOW)),
viewport_cells: std::cell::Cell::new(INITIAL_VIEWPORT_CELLS),
library_facts: RefCell::new(None),
+232 -36
View File
@@ -9,7 +9,7 @@
use std::rc::Rc;
use slint::{ComponentHandle, Model as _};
use slint::ComponentHandle;
use crate::AppWindow;
use crate::{GestureRow, Library, View};
@@ -27,7 +27,7 @@ use super::timeline::{
capture_time_at, catalog_span, note_place, resume_position, wire_timeline, write_place,
zoomed_span,
};
use super::window::{load_window, window_move, window_start};
use super::window::{bring_window_to, load_window, mark_open, window_move};
/// Walk the keyboard cursor through the library — the arrow keys.
///
@@ -89,29 +89,16 @@ fn place_cursor(
at: usize,
extend: bool,
) {
// Bring the window to the cursor if it has walked out of it. Centred a
// quarter in, exactly as a scroll does it, so continuing in the same
// direction has loaded cells to move into rather than another reload on
// the very next press.
let loaded = window.global::<Library>().get_library_cells().row_count();
let offset = *ctl.offset.borrow();
if at < offset || at >= offset + loaded {
let size = *ctl.window.borrow();
let total = window.global::<Library>().get_library_total().max(0) as usize;
*ctl.offset.borrow_mut() = window_start(at, size, total);
load_window(window, ctl);
}
// Re-read: `load_window` clamps the offset against the library's end, so
// the window may not start where it was asked to.
let offset = *ctl.offset.borrow();
let ids = ctl.visible_ids();
let Some(row) = at.checked_sub(offset).filter(|r| *r < ids.len()) else {
// Bring the window to the cursor if it has walked out of it, which is the
// same thing scrolling does.
let Some(row) = bring_window_to(window, ctl, at) else {
// The window could not be brought to the cursor — an empty or
// shrinking library. Leaving the cursor where it was is better than
// pointing it at nothing.
return;
};
let offset = *ctl.offset.borrow();
let ids = ctl.visible_ids();
if !extend {
// An arrow key collapses the selection onto the cursor. A *click* on
@@ -125,6 +112,69 @@ fn place_cursor(
window.global::<Library>().set_library_cursor(at as i32);
}
/// Where a step of `delta` along the roll lands, as a library ordinal, from
/// the photograph open at ordinal `from`; `None` for a step that goes nowhere.
///
/// Clamped at both ends, so holding a key at the last frame stays there rather
/// than reopening it on every repeat. `left` is
/// [`LibraryController::roll_left`]: the open photograph has dropped out of the
/// grid and the next one has moved up into its ordinal, so a step forward
/// lands *on* `from` — the photograph that now follows — and not past it.
fn roll_target(from: usize, delta: i32, total: usize, left: bool) -> Option<usize> {
if total == 0 {
return None;
}
let step = if left && delta > 0 { delta - 1 } else { delta };
let next = (from as isize)
.saturating_add(step as isize)
.clamp(0, total as isize - 1) as usize;
(next != from || left).then_some(next)
}
/// Step along the roll by library ordinal, bringing the window with it.
///
/// **By ordinal, as [`move_cursor`] walks the grid, and not by row.** The keys
/// used to pick the roll's row either side of the marked one. The roll is the
/// loaded window, so a row past its end picked nothing and holding D stopped
/// dead at the edge of whatever was loaded — a few screenfuls into a library
/// of thousands. Stepping from the open photograph's ordinal, and loading the
/// window around the next one when it lies outside, walks the whole library in
/// grid order, one photograph per press, and reads the catalog only when the
/// window has to move.
fn roll_step(window: &AppWindow, ctl: &Rc<LibraryController>, delta: i32, on_open: &OpenImage) {
let total = window.global::<Library>().get_library_total().max(0) as usize;
let from = window.get_index().max(0) as usize;
let Some(to) = roll_target(from, delta, total, ctl.roll_left.get()) else {
return;
};
if let Some(row) = bring_window_to(window, ctl, to) {
open_from_roll(window, ctl, row, on_open);
}
}
/// Open the photograph at `row` of the loaded window without leaving develop:
/// a pick from the roll, or a step along it.
///
/// `open_from_library` persists the outgoing edit before it loads the next
/// one, which is what makes this safe to fire as fast as a key repeats.
fn open_from_roll(
window: &AppWindow,
ctl: &Rc<LibraryController>,
row: usize,
on_open: &OpenImage,
) {
let Some(path) = ctl.paths.borrow().get(row).cloned() else {
return;
};
mark_open(window, ctl, row);
on_open(path);
report_position(window, ctl, row);
// TRACES: FR-UI-8
// Debounced, unlike the two entries into develop: the roll is walked
// frame by frame and every step is a different photograph.
note_place(window, ctl);
}
/// Say where in the library the photograph now open sits.
///
/// **After `on_open_image`, never before.** The generic open path resets the
@@ -256,7 +306,7 @@ pub fn wire<F>(
w.set_active_view(View::Develop);
// Which cell the develop view is now showing, so the photo
// roll opens marking it rather than marking nothing.
w.global::<Library>().set_library_roll_current(i);
mark_open(&w, &ctl, i as usize);
// A develop session begins here, so the roll centres on this
// photograph the first time it settles rather than merely
// scrolling it into view at one edge. Raised here and not in
@@ -277,30 +327,37 @@ pub fn wire<F>(
// TRACES: FR-UI-4
// A photograph chosen from the photo roll.
//
// The same row-to-path lookup a cell click does, without the selection
// The roll draws the loaded window, so what it reports is a row of it —
// the same row-to-path lookup a cell click does, without the selection
// rules: the roll is a way of moving between photographs, not of building
// a set, so there is no modified press to honour and no reason to leave
// develop. `open_from_library` persists the outgoing edit before it loads
// the next one, which is what makes this safe to fire repeatedly.
// develop.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let on_open_image = on_open_image.clone();
window.global::<Library>().on_library_roll_pick(move |i| {
let Some(w) = weak.upgrade() else { return };
let Some(path) = ctl.paths.borrow().get(i as usize).cloned() else {
return;
};
w.global::<Library>().set_library_roll_current(i);
on_open_image(path);
report_position(&w, &ctl, i as usize);
// TRACES: FR-UI-8
// Debounced, unlike the two entries into develop: the roll is walked
// frame by frame and every step is a different photograph.
note_place(&w, &ctl);
if let Ok(row) = usize::try_from(i) {
open_from_roll(&w, &ctl, row, &on_open_image);
}
});
}
// TRACES: FR-UI-5
// One photograph along, from the keyboard in develop.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let on_open_image = on_open_image.clone();
window
.global::<Library>()
.on_library_roll_step(move |delta| {
let Some(w) = weak.upgrade() else { return };
roll_step(&w, &ctl, delta, &on_open_image);
});
}
wire_grid_cursor_and_zoom(window, &ctl, &coll_ctl, &on_open_image);
wire_grid_sync_and_load(window, &ctl);
wire_timeline(window, &ctl);
@@ -362,8 +419,7 @@ fn wire_grid_cursor_and_zoom(
w.set_active_view(View::Develop);
// As on a click: the roll marks what is open, and centres on it
// because this too begins a session.
w.global::<Library>()
.set_library_roll_current(row.unwrap_or(0) as i32);
mark_open(&w, &ctl, row.unwrap_or(0));
w.global::<Library>().set_library_roll_centre(true);
on_open(path);
report_position(&w, &ctl, row.unwrap_or(0));
@@ -748,6 +804,146 @@ fn wire_grid_routes(
#[cfg(test)]
mod tests {
use super::*;
use crate::library_ui::window::{locate_open, window_for, window_start};
/// A library larger than one loaded window, and the window over it as
/// `load_window` leaves it: offset clamped so the window stays full, and
/// holding the photographs from there on. A photograph's id is its
/// ordinal plus a thousand, so a row can be checked for *which* one it is.
struct Roll {
total: usize,
size: usize,
offset: usize,
}
impl Roll {
fn id(ordinal: usize) -> i64 {
ordinal as i64 + 1000
}
fn ids(&self) -> Vec<i64> {
let end = (self.offset + self.size).min(self.total);
(self.offset..end).map(Self::id).collect()
}
fn load(&mut self, offset: usize) {
self.offset = offset.min(self.total.saturating_sub(self.size.min(self.total)));
}
/// What `roll_step` does: the target ordinal, the window brought to
/// it, and the photograph at the resulting row. `None` for no step.
fn step(&mut self, from: usize, delta: i32, left: bool) -> Option<(usize, i64)> {
let to = roll_target(from, delta, self.total, left)?;
if let Some(o) = window_for(to, self.offset, self.ids().len(), self.size, self.total) {
self.load(o);
}
let row = to
.checked_sub(self.offset)
.filter(|r| *r < self.ids().len())?;
Some((self.offset + row, self.ids()[row]))
}
}
/// Hold D from the first frame: every photograph, once each, in grid
/// order, across every window boundary — and then nothing, at the end.
#[test]
fn stepping_forward_crosses_window_boundaries_one_photograph_per_press() {
let mut lib = Roll {
total: 300,
size: 96,
offset: 0,
};
let mut at = 0;
let mut opened = vec![Roll::id(0)];
let mut moves = 0;
for _ in 0..400 {
let before = lib.offset;
let Some((next, id)) = lib.step(at, 1, false) else {
break;
};
moves += usize::from(lib.offset != before);
assert_eq!(next, at + 1, "one photograph per press");
at = next;
opened.push(id);
}
let expected: Vec<i64> = (0..300).map(Roll::id).collect();
assert_eq!(opened, expected);
assert!(
moves >= 2,
"the walk must have crossed window boundaries ({moves})"
);
assert_eq!(lib.step(at, 1, false), None, "the last frame stays put");
}
/// The same backwards with A, from the last frame, starting from a window
/// placed at the end of the library as a scrub there would place it.
#[test]
fn stepping_backward_crosses_window_boundaries_one_photograph_per_press() {
let mut lib = Roll {
total: 300,
size: 96,
offset: 0,
};
lib.load(window_start(299, 96, 300));
let mut at = 299;
let mut opened = vec![Roll::id(299)];
for _ in 0..400 {
let Some((next, id)) = lib.step(at, -1, false) else {
break;
};
assert_eq!(next + 1, at, "one photograph per press");
at = next;
opened.push(id);
}
let expected: Vec<i64> = (0..300).rev().map(Roll::id).collect();
assert_eq!(opened, expected);
assert_eq!(lib.offset, 0);
}
/// The mark follows the photograph, not the row: after a step that moves
/// the window, the row the open photograph had is someone else's, and
/// the reload has to find it again.
#[test]
fn the_roll_mark_follows_the_open_photograph_across_a_reload() {
let mut lib = Roll {
total: 300,
size: 96,
offset: 0,
};
let open = Roll::id(95);
assert_eq!(locate_open(open, &lib.ids(), lib.offset, 95), Ok(95));
lib.load(window_start(96, 96, 300));
assert_ne!(
lib.ids()[95],
open,
"the old row now holds another photograph"
);
let row = locate_open(open, &lib.ids(), lib.offset, 95).unwrap();
assert_eq!(lib.ids()[row], open);
assert_eq!(lib.offset + row, 95);
// Scrolled well away: not here, and not gone either.
lib.load(200);
assert_eq!(locate_open(open, &lib.ids(), lib.offset, 95), Err(false));
}
/// A judgement under a filter drops the open photograph out of the grid,
/// and the next one moves up into its ordinal. Stepping forward opens that
/// one; stepping back opens the one before, as it always would have.
#[test]
fn a_photograph_that_left_the_grid_steps_onto_its_successor() {
let ids: Vec<i64> = (0..10).filter(|i| *i != 5).map(|i| i + 1000).collect();
assert_eq!(locate_open(1005, &ids, 0, 5), Err(true));
assert_eq!(roll_target(5, 1, 9, true), Some(5));
assert_eq!(roll_target(5, -1, 9, true), Some(4));
// The last frame left: forward lands on the new last one.
assert_eq!(roll_target(9, 1, 9, true), Some(8));
assert_eq!(roll_target(5, 1, 9, false), Some(6));
assert_eq!(roll_target(0, -1, 9, false), None);
assert_eq!(roll_target(0, 1, 0, false), None);
}
/// One zoom step, as the handler applies it.
fn zoom_cell(current: f32, delta: i32) -> f32 {
+2 -4
View File
@@ -18,7 +18,7 @@ use crate::{AppWindow, Library, TimelineBar, View};
use super::controller::LibraryController;
use super::filter_bar::{push_people_chips, refilter};
use super::window::{load_window, window_start};
use super::window::{load_window, mark_open, window_start};
/// Move the timeline's zoom by whole levels.
///
@@ -814,9 +814,7 @@ pub(crate) fn apply_place(
window.global::<Library>().set_library_roll_centre(true);
let offset = *ctl.offset.borrow();
if let Some(row) = at.checked_sub(offset) {
window
.global::<Library>()
.set_library_roll_current(row as i32);
mark_open(window, ctl, row);
}
open(place.path.clone());
// After the open, which resets the readout to "1 of 1" on its
+102
View File
@@ -187,6 +187,105 @@ pub(super) fn window_move(
Some(desired)
}
/// Where the loaded window has to move to so that it holds ordinal `at`, or
/// `None` if it already does.
///
/// Placed by [`window_start`], a quarter in, as a scroll places it — so a walk
/// that carries on in the same direction has loaded cells to move into rather
/// than another reload on the very next step.
pub(super) fn window_for(
at: usize,
offset: usize,
loaded: usize,
window_size: usize,
total: usize,
) -> Option<usize> {
(at < offset || at >= offset + loaded).then(|| window_start(at, window_size, total))
}
/// Load the window around library ordinal `at` if it is not already in it,
/// and say which row of it `at` now is.
///
/// `None` when the window could not be brought there — an empty or shrinking
/// library — which callers treat as "do nothing" rather than guess at a row.
pub(super) fn bring_window_to(
window: &AppWindow,
ctl: &Rc<LibraryController>,
at: usize,
) -> Option<usize> {
let loaded = ctl.paths.borrow().len();
let size = *ctl.window.borrow();
let total = window.global::<Library>().get_library_total().max(0) as usize;
if let Some(offset) = window_for(at, *ctl.offset.borrow(), loaded, size, total) {
*ctl.offset.borrow_mut() = offset;
load_window(window, ctl);
}
// Re-read: `load_window` clamps the offset against the library's end, so
// the window may not start where it was asked to.
let offset = *ctl.offset.borrow();
at.checked_sub(offset)
.filter(|row| *row < ctl.paths.borrow().len())
}
/// Record that the photograph at `row` of the loaded window is the one now
/// open in develop, and mark it on the roll.
pub(super) fn mark_open(window: &AppWindow, ctl: &LibraryController, row: usize) {
ctl.roll_open.set(ctl.image_ids.borrow().get(row).copied());
ctl.roll_left.set(false);
window
.global::<Library>()
.set_library_roll_current(row as i32);
}
/// Put the roll's mark back on the open photograph after the window was
/// re-read.
///
/// **The mark is a row, and a reload changes what every row holds.** It used
/// to be set when a photograph was opened and never again, so once the window
/// moved — a step off its end, a background sync, a judgement under a filter —
/// the roll marked whichever photograph had taken that row, the rating keys in
/// develop judged it, and the next step walked on from it. Found by id in the
/// window just read: a scan of one window, not a query.
///
/// Where it is found, `index` is corrected with it, since that ordinal is what
/// the next step and the return to the grid start from. Where it is not, the
/// mark comes off; and if its ordinal is still inside the window, it is a
/// photograph that has left the grid rather than one the window has moved away
/// from — see [`LibraryController::roll_left`].
fn follow_open(window: &AppWindow, ctl: &LibraryController, offset: usize) {
let Some(open) = ctl.roll_open.get() else {
return;
};
let index = window.get_index().max(0) as usize;
match locate_open(open, &ctl.image_ids.borrow(), offset, index) {
Ok(row) => {
ctl.roll_left.set(false);
window
.global::<Library>()
.set_library_roll_current(row as i32);
window.set_index((offset + row) as i32);
}
Err(left) => {
ctl.roll_left.set(left);
window.global::<Library>().set_library_roll_current(-1);
}
}
}
/// Which row of a window starting at `offset` holds photograph `open`, or, if
/// none does, whether it has left the grid: its last known ordinal `index` is
/// still inside the window, so something else now sits there.
pub(super) fn locate_open(
open: i64,
ids: &[i64],
offset: usize,
index: usize,
) -> Result<usize, bool> {
ids.iter()
.position(|id| *id == open)
.ok_or(index >= offset && index < offset + ids.len())
}
/// Fill the model from the catalog and start fetching thumbnails.
///
/// Reads the window starting at the controller's current offset, which the
@@ -399,6 +498,9 @@ pub(super) fn load_window(window: &AppWindow, ctl: &Rc<LibraryController>) {
window
.global::<Library>()
.set_library_cells(slint::ModelRc::new(slint::VecModel::from(rows)));
// The rows have just changed meaning, so the roll's mark — a row — has to
// be found again rather than left pointing at whoever sits there now.
follow_open(window, ctl, offset);
// The model is fresh, so the "in this many collections" badges are all zero
// until refilled. One query for the whole window, not one per cell.