dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Come back from develop on the photograph you were editing

Browse Source 
Leaving develop returned to where the *grid* was, which after a walk
along the photo roll can be a thousand rows from the frame you had just
finished. So the one photograph you were certainly interested in was the
one the grid came back without.

Two positions, and the rule is not to pick one of them. The grid seeks to
the remembered position, then reveals the keyboard cursor -- which is now
put on the open photograph, and which moves the viewport as little as
will bring its row into view. A frame inside the remembered screenful
moves nothing at all; one outside it scrolls exactly far enough. One
rule, both behaviours.

The cursor rather than the selection, deliberately: `place_cursor` also
rewrites the selection, and a set of forty photographs assembled in the
grid must survive having one of them opened.

`reveal()` now also runs on the grid's `init`, since `cursor-row` is
initialised rather than changed when the subtree is rebuilt and no
handler would otherwise fire. Both it and the roll's centring defer while
the element has no height yet -- `init` runs before layout, where a
height of zero makes every row look off screen -- and a latch brings the
first real height back to the cursor without letting every later resize
haul the viewport around.

The capture-time marker follows the same move, for the same reason:
`load_window` rebuilds the axis only when the scope, the filter or the
total has changed, and none of them has. It is the same library seen from
a different row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-30 20:35:23 +02:00
co-authored by Claude Opus 5
parent 53dc2d171e
commit 0ff1e01ec3
4 changed files with 229 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/library_ui.rs
+112 -24
View File
@@ -4806,6 +4806,106 @@ fn report_position(window: &AppWindow, ctl: &Rc<LibraryController>, row: usize)
window.set_total(window.get_library_total());
}
/// Bring the grid back to where the photographer left it, and — coming out of
/// develop — put the keyboard cursor on the photograph they were editing.
///
/// # Two positions, not one
///
/// `resume_at` is where the *grid* was when it was left. `focus` is the
/// photograph develop was showing, which after a walk along the photo roll can
/// be a thousand rows away from it. Restoring only the first is what made
/// leaving develop feel like losing the frame you had just finished: the grid
/// came back to a screenful that no longer had it in.
///
/// So both are honoured, and which one wins is decided by whether they
/// disagree. If the open photograph is inside the screenful the grid was left
/// showing — the ordinary case, where develop was opened and closed on the same
/// frame — the remembered position is used unchanged and nothing appears to
/// move. If it is not, the grid goes to the photograph, because that is the one
/// the user was last looking at.
///
/// # Why the cursor rather than the selection
///
/// Deliberately not [`place_cursor`], which also rewrites the selection. A
/// selection of forty photographs assembled in the grid, then one of them
/// opened to check it, must survive the trip back — discarding it is the kind
/// of silent loss that stops people using the develop view mid-cull. The
/// cursor is a position, not a judgement, so it is safe to move.
///
/// The viewport is left to the grid's own `reveal()`, which moves as little as
/// will bring the cursor's row into view. That is what makes the two cases
/// above one rule rather than two: a cursor already on screen moves nothing.
fn resume_position(
window: &AppWindow,
ctl: &Rc<LibraryController>,
coll: &Rc<crate::collections_ui::CollectionsController>,
focus: Option<usize>,
) {
let total = window.get_library_total().max(0) as usize;
if total == 0 {
// Files named on the command line: there is no grid behind them, and
// an ordinal read off the readout would name a row of nothing.
return;
}
let resume = ctl.resume_at.get().min(total - 1);
let focus = focus.map(|at| at.min(total - 1));
// A screenful, as the grid last measured itself. The comparison has to be
// against what is *visible*, not against the loaded window, which is
// several screenfuls wide — an open photograph loaded but scrolled past is
// still one the user cannot see.
let viewport = ctl.viewport_cells.get().max(1);
let anchor = match focus {
Some(at) if at >= resume && at < resume + viewport => resume,
Some(at) => at,
None => resume,
};
// Centred like `on_library_scrolled` does it, so scrolling either way from
// the restored position has loaded rows to move into.
let window_size = *ctl.window.borrow();
*ctl.offset.borrow_mut() = window_start(anchor, window_size, total);
load_window(window, ctl);
// Before the grid is shown, not after: the markup gates it on an `if`, and
// the rebuilt Flickable reads `scroll-to` in its `init`. Setting these
// afterwards would leave that init to run against the previous position.
window.set_library_scroll_to(anchor as i32);
window.set_library_scroll_token(window.get_library_scroll_token() + 1);
if let Some(at) = focus {
coll.set_cursor(Some(at));
window.set_library_cursor(at as i32);
}
// The view has moved, so the capture-time marker has to move with it.
//
// `load_window` will not do it: it rebuilds the axis only when the scope,
// the filter or the total has changed, and none of them has — this is the
// same library seen from a different row. Without this the marker stayed
// wherever it was when develop was entered, which after a walk along the
// photo roll can be months away from the photographs now on screen.
reanchor_timeline(window, ctl, anchor);
}
/// Put the capture-time marker where the view now is.
///
/// Clearing the anchor and re-running the axis rather than computing a fraction
/// here: `refresh_timeline` seeds itself from the view's position when nothing
/// has claimed one, and it owns every other thing the sidebar draws — the bars,
/// the range band, the lit bucket. A second place that positioned the marker
/// would be a second chance to disagree with the bars under it, which is the
/// bug the `current-fraction` note in `library.slint` records.
fn reanchor_timeline(window: &AppWindow, ctl: &Rc<LibraryController>, at: usize) {
ctl.resume_at.set(at);
*ctl.current_bucket.borrow_mut() = None;
let borrow = ctl.catalog.borrow();
if let Some(catalog) = borrow.as_ref() {
refresh_timeline(window, catalog, ctl);
}
}
/// Connect the grid's callbacks.
pub fn wire<F>(
window: &AppWindow,
@@ -5352,12 +5452,15 @@ pub fn wire<F>(
// Develop → grid.
//
// Returns to where the user left rather than to the top. `show-library`
// gates an `if` in the markup, so the grid is rebuilt from nothing and its
// Flickable starts at row 0; the position has to be replayed explicitly.
// Returns to where the user left, on the photograph they were editing,
// rather than to the top. The grid is gated on an `if` in the markup, so it
// is rebuilt from nothing and its Flickable starts at row 0; the position
// has to be replayed explicitly. See [`resume_position`] for which of the
// two positions wins when they disagree.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let coll_ctl = coll_ctl.clone();
window.on_back_to_library(move || {
let Some(w) = weak.upgrade() else { return };
@@ -5369,27 +5472,12 @@ pub fn wire<F>(
// the last moment its edit can be written.
on_leave_develop();
let resume = ctl.resume_at.get();
if resume > 0 {
// Through the same channel a scrub uses, and for the same
// reason: the viewport and the loaded window both have to move,
// or the cells are drawn thousands of rows from where the view
// sits.
//
// Centred like `on_library_scrolled` does it, so scrolling up
// from the restored position has loaded rows above it.
let window_size = *ctl.window.borrow();
let total = w.get_library_total().max(0) as usize;
*ctl.offset.borrow_mut() = window_start(resume, window_size, total);
load_window(&w, &ctl);
// Before the grid is shown, not after: the markup gates it on
// an `if`, and the rebuilt Flickable reads `scroll-to` in its
// `init`. Setting these afterwards would leave that init to run
// against the previous position.
w.set_library_scroll_to(resume as i32);
w.set_library_scroll_token(w.get_library_scroll_token() + 1);
}
// Where the open photograph sits in the library, which is exactly
// what `report_position` keeps `index` holding — including after
// every step along the photo roll, which is the case that makes
// this differ from `resume_at`.
let open = w.get_index().max(0) as usize;
resume_position(&w, &ctl, &coll_ctl, Some(open));
w.set_show_library(true);
});