Place the timeline marker where the pointer actually is
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 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+109
-11
@@ -1320,20 +1320,25 @@ fn refresh_timeline(window: &AppWindow, catalog: &Catalog, ctl: &Rc<LibraryContr
|
||||
})
|
||||
.collect();
|
||||
|
||||
// Where the grid currently sits, as an index among these bars. Slint
|
||||
// cannot search an array, and a component guessing the position would put
|
||||
// the marker somewhere plausible and wrong.
|
||||
// Where the grid currently sits, as a fraction of the visible span.
|
||||
//
|
||||
// This is the exact inverse of what a click produces: the widget maps a y
|
||||
// to a fraction of its track and `on_library_scrub_fraction` interpolates
|
||||
// `from + (to - from) * f`, so the marker must invert that same expression
|
||||
// or it lands somewhere other than the pointer.
|
||||
//
|
||||
// The earlier version sent a *bar index* instead. Bars occupy one equal
|
||||
// slot each regardless of how much time they cover, and `rposition` snaps
|
||||
// to the bucket's start edge, so the marker sat at the top of whichever
|
||||
// slot contained the instant — near enough on a dense uniform axis, plainly
|
||||
// wrong on a sparse one, and never under the click.
|
||||
let current = *ctl.current_bucket.borrow();
|
||||
let index = current
|
||||
.and_then(|t| {
|
||||
bars.iter()
|
||||
.rposition(|b| (b.start as i64) <= t)
|
||||
.map(|i| i as i32)
|
||||
})
|
||||
.unwrap_or(-1);
|
||||
let fraction = current
|
||||
.map(|t| ((t - from) as f64 / (to - from).max(1) as f64).clamp(0.0, 1.0) as f32)
|
||||
.unwrap_or(-1.0);
|
||||
|
||||
window.set_library_current_bucket(current.unwrap_or(0) as i32);
|
||||
window.set_library_current_index(index);
|
||||
window.set_library_current_fraction(fraction);
|
||||
window.set_library_timeline_anchored(current.is_some());
|
||||
window.set_library_timeline_label(
|
||||
format!("{} – {}", format_date(from), format_date(to)).into(),
|
||||
@@ -1418,6 +1423,31 @@ fn catalog_span(catalog: &Catalog) -> 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<i64> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user