From f6a100863e5ac6b1c180749c2d1649df730596b0 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Tue, 11 Aug 2026 20:08:32 +0200 Subject: [PATCH] Place the timeline marker where the pointer actually is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The marker was drawn from a bucket index while a click reported a fraction of the track. Those are different quantities: bars each occupy one equal slot whatever span of time they cover, and the index snapped to the containing bucket's start edge, so the marker landed at the top of whichever slot held the instant — close enough to pass on a dense uniform axis, plainly wrong on a sparse one, and never under the click. Send the fraction instead, computed as the exact inverse of the interpolation the scrub handler applies, and position the marker from it. Marker and click are now the same quantity by construction. Scrolling the grid also left the marker behind: only an explicit scrub ever wrote the position, so the axis claimed to say "when you are" and stopped being true the moment the wheel moved. Map the first visible row back to a capture time and move the marker with it. That runs on every scroll event rather than behind the window-reload guard, which fires a few times per screenful and would make the marker advance in jerks. Only the marker moves, not the bars: rebuilding those means a GROUP BY aggregate over the library, far too much for a flick, and they do not change as the grid scrolls anyway. Co-Authored-By: Claude Opus 5 --- ui/dr-ui/src/library_ui.rs | 120 +++++++++++++++++++++++++++++++++---- ui/dr-ui/ui/app.slint | 4 +- ui/dr-ui/ui/library.slint | 26 ++++---- 3 files changed, 127 insertions(+), 23 deletions(-) diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index f1b3850..63be01f 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -1320,20 +1320,25 @@ fn refresh_timeline(window: &AppWindow, catalog: &Catalog, ctl: &Rc Option<(i64, i64)> { .and_then(|(lo, hi)| Some((lo?, hi?))) } +/// Capture time of the image at row `ordinal` in the grid's own ordering. +/// +/// The inverse of the count in [`scrub_to`], and it must stay the inverse: the +/// same `shadowed_by IS NULL` exclusion and the same ordering, or scrolling +/// would report an instant the scrub would never produce for that row. +/// +/// `None` for an ordinal that lands among the undated tail, which sorts last +/// and has no place on a capture-time axis. +/// +/// Called on every scroll event, so it has to stay cheap: the `images_captured` +/// index makes it a seek along already-ordered rows rather than a sort. +fn capture_time_at(catalog: &Catalog, ordinal: usize) -> Option { + catalog + .connection() + .query_row( + "SELECT captured_at FROM images + WHERE shadowed_by IS NULL AND captured_at IS NOT NULL + ORDER BY captured_at + LIMIT 1 OFFSET ?1", + [ordinal as i64], + |r| r.get::<_, i64>(0), + ) + .ok() +} + /// Jump the grid to the first image at or after `when`. /// /// This is the scrub: the window moves, the library does not narrow. Every @@ -1648,6 +1678,44 @@ where let Some(w) = weak.upgrade() else { return }; let first_visible = first_visible.max(0) as usize; + // Move the timeline marker with the view. Scrolling the grid is a + // way of moving through time just as scrubbing is, and a marker + // that only ever moved on a scrub sat still while the photographs + // beside it advanced by months — the axis said "when you are" and + // was wrong the moment the user touched the wheel. + // + // This runs before the reload guard below, which fires only a few + // times per screenful; the marker has to follow every event or it + // would advance in visible jerks. + // + // Only the marker is moved, not the whole histogram: rebuilding + // the bars means a `GROUP BY strftime` aggregate over the library, + // far too much for every event of a flick. The bars do not change + // as the grid scrolls anyway — only where the marker sits on them. + // + // Re-running the scrub would be wrong for a second reason: it sets + // `scroll-to`, which would drive the grid from its own scroll. + { + let borrow = ctl.catalog.borrow(); + if let Some(catalog) = borrow.as_ref() { + if let Some(when) = capture_time_at(catalog, first_visible) { + *ctl.current_bucket.borrow_mut() = Some(when); + + let zoom = *ctl.timeline_zoom.borrow(); + let centre = *ctl.timeline_centre.borrow(); + if let Some(full) = catalog_span(catalog) { + let (from, to) = zoomed_span(full, zoom, centre); + w.set_library_current_bucket(when as i32); + w.set_library_current_fraction( + ((when - from) as f64 / (to - from).max(1) as f64).clamp(0.0, 1.0) + as f32, + ); + w.set_library_timeline_anchored(true); + } + } + } + } + // Centre the window on the view, so scrolling either way has // loaded rows ahead of it rather than only below. let window_size = *ctl.window.borrow(); @@ -1976,6 +2044,36 @@ mod tests { assert_ne!(instant_at(span, 0.10), instant_at(span, 0.11)); } + /// Where the marker is drawn for an instant, as `refresh_timeline` + /// computes it. + fn marker_at(span: (i64, i64), t: i64) -> f32 { + ((t - span.0) as f64 / (span.1 - span.0).max(1) as f64).clamp(0.0, 1.0) as f32 + } + + #[test] + fn the_marker_lands_on_the_fraction_that_was_clicked() { + // The bug this guards: the marker was placed from a *bar index* while + // the click was a fraction of the track, so it never appeared under + // the pointer. Marker and click must be inverses. + let span = (1_000, 9_000); + for f in [0.0_f32, 0.1, 0.25, 0.5, 0.75, 1.0] { + let round_trip = marker_at(span, instant_at(span, f)); + assert!( + (round_trip - f).abs() < 1e-3, + "clicked {f}, marker drawn at {round_trip}" + ); + } + } + + #[test] + fn an_instant_outside_the_visible_span_pins_the_marker_to_an_end() { + // Zooming in leaves the grid's instant outside the axis. The marker + // belongs at the edge it went past, not off the widget. + let span = (1_000, 2_000); + assert_eq!(marker_at(span, 0), 0.0); + assert_eq!(marker_at(span, 5_000), 1.0); + } + #[test] fn a_scrub_fraction_outside_the_axis_is_clamped() { // A drag that leaves the widget still reports a position; it must land diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 234ee96..6ec0c51 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -214,7 +214,7 @@ export component AppWindow inherits Window { in property library-sweep-total: 0; in property library-current-bucket: 0; - in property library-current-index: -1; + in property library-current-fraction: -1; in property library-timeline-anchored: false; callback library-scrub-fraction(float); @@ -468,7 +468,7 @@ export component AppWindow inherits Window { sweep-total: root.library-sweep-total; current-bucket: root.library-current-bucket; - current-bucket-index: root.library-current-index; + current-bucket-fraction: root.library-current-fraction; timeline-anchored: root.library-timeline-anchored; scrub-fraction(f) => { root.library-scrub-fraction(f); } diff --git a/ui/dr-ui/ui/library.slint b/ui/dr-ui/ui/library.slint index 5871a61..4974a1f 100644 --- a/ui/dr-ui/ui/library.slint +++ b/ui/dr-ui/ui/library.slint @@ -52,10 +52,15 @@ export component Timeline inherits Rectangle { /// The bucket the grid is currently showing, highlighted so the position is /// visible when the grid is moved by scrolling instead. in property current-start: 0; - /// Index of that bucket among `bars`, supplied by Rust. Slint has no array - /// search, and a component that quietly returned the wrong index would put - /// the marker in a plausible but false position. - in property current-index: -1; + /// Where the grid sits along the visible span, 0..1, supplied by Rust. + /// + /// **A fraction, not a bucket index.** The marker has to be placed by the + /// same quantity a click produces: `fraction-at` maps a y to a fraction of + /// the track and Rust interpolates an instant from it, so positioning the + /// marker from a bar index instead put it wherever that bucket's *slot* + /// happened to fall — never under the pointer. Negative means "not + /// anchored". + in property current-fraction: -1; /// True once the user has taken control. Until then the marker rests at the /// middle rather than pinning to either end, which would imply a selection /// that has not been made. @@ -147,8 +152,8 @@ export component Timeline inherits Rectangle { height: 1px; background: Theme.active; opacity: root.anchored ? 1.0 : 0.35; - y: root.anchored && root.current-index >= 0 - ? root.track-top + root.current-index * root.slot + y: root.anchored && root.current-fraction >= 0 + ? root.track-top + clamp(root.current-fraction, 0.0, 1.0) * root.track-height : root.track-top + root.track-height / 2; } @@ -502,10 +507,11 @@ export component LibraryGrid inherits Rectangle { in property scroll-to: 0; in property scroll-token: 0; - /// Which bucket the grid currently sits in, and where that is among the - /// bars. Slint cannot search an array, so Rust supplies both. + /// Which bucket the grid currently sits in, and how far along the visible + /// span that is. Rust supplies both: it owns the span, so only it can turn + /// an instant into the fraction that places the marker. in property current-bucket: 0; - in property current-bucket-index: -1; + in property current-bucket-fraction: -1; /// False until the user has moved the timeline themselves, so the marker /// rests at the middle rather than implying a choice not yet made. in property timeline-anchored: false; @@ -917,7 +923,7 @@ export component LibraryGrid inherits Rectangle { bars: root.timeline; range-label: root.timeline-label; current-start: root.current-bucket; - current-index: root.current-bucket-index; + current-fraction: root.current-bucket-fraction; anchored: root.timeline-anchored; scrub-to(f) => { root.scrub-fraction(f); }