Viewer input fixes: tap during fling stops the fling (no open); zoom sticks at native resolution

1) Tap/touch during a fling: the ListView's Flickable already stops the
   fling when a press lands (its press handler resets the fling
   animation — slint core items/flickable.rs, remove_binding =
   set_animated_value(current, none)); the bug was that the press is
   then forwarded to the cell (~100 ms later, FORWARD_DELAY) and the
   release opened the viewer. The cell TouchArea now reports its
   pointer-event down (1.18 has no pressed callback) via
   cell-pressed → on_cell_pressing, which marks the tap as a
   fling-stopper when the grid moved within the last 150 ms (the tick
   tracks raw list-y changes in grid_motion); on_open_image then
   swallows the tap.

2) Zooming past max resolution moved the image: zoom_at applied the
   raw factor to tx/ty BEFORE clamping s — at the cap s stayed put
   but the translation still shifted about the (moving) pinch
   center, so the image followed the fingers like a two-finger drag.
   zoom_at now clamps s first (factor derived from the clamped s:
   at the cap factor = 1, a true no-op), and Input::pinch sticks at
   both caps (the handler reports a CUMULATIVE scale from the pinch
   start, so past the cap every later update would otherwise drift).
   The zoom cap is now NATIVE resolution: max(fit, 1.0) — 1 device
   pixel per image pixel (small images keep fit); zooming further
   only pixelates, so it is not allowed (was 20× fit).

Tests: zoom_at_past_cap_is_noop, pinch_sticks_at_caps (new),
clamp_zoom_bounds (native cap + small-image fit), updated
pinch_handler_sequence (4× fit now clamps at native). 36/36.
Emulator: fling+tap → 0 viewer opens, grid settled; normal tap
still opens (cached path).
This commit is contained in:
Greg Pomerantz 2026-10-29 12:30:42 -04:00
parent 9a1654136f
commit e10a71b00a
3 changed files with 161 additions and 13 deletions

View File

@ -160,7 +160,7 @@ pub mod prio {
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use std::ops::Range; use std::ops::Range;
use std::path::Path; use std::path::Path;
use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
use std::sync::{Mutex, OnceLock}; use std::sync::{Mutex, OnceLock};
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
@ -213,6 +213,12 @@ const MAX_EVICT_CELLS_PER_TICK: usize = 4;
/// refresh (8 ms on 120 Hz, ~16 ms on 60 Hz). Per-tick work is tiny and /// refresh (8 ms on 120 Hz, ~16 ms on 60 Hz). Per-tick work is tiny and
/// idempotent, so jitter is harmless. /// idempotent, so jitter is harmless.
const DRAIN_INTERVAL_MS: u64 = 8; const DRAIN_INTERVAL_MS: u64 = 8;
/// A cell press younger than this window after the last grid motion is a
/// fling-stopper, not a tap. The Flickable delays forwarding the press to
/// the cell ~100 ms (FORWARD_DELAY in slint core), and the fling stops at
/// the touch-down itself, so the window must cover delay + tick latency.
const TAP_VS_FLING_MS: u64 = 150;
/// A viewport cell waiting longer than this for its in-flight job is a /// A viewport cell waiting longer than this for its in-flight job is a
/// stall; log a diagnostic (rate-limited). /// stall; log a diagnostic (rate-limited).
const STALL_ALARM_MS: u64 = 500; const STALL_ALARM_MS: u64 = 500;
@ -291,6 +297,17 @@ struct AppState {
/// set: a full decode can fail where the 400 px thumb succeeds, e.g. /// set: a full decode can fail where the 400 px thumb succeeds, e.g.
/// the GPU texture cap). UI thread only. /// the GPU texture cap). UI thread only.
full_failed: Mutex<HashSet<String>>, full_failed: Mutex<HashSet<String>>,
/// Grid motion: (last list-y seen, when it last CHANGED by more than
/// half a px). The tick polls list-y every ~8 ms, so a fling updates
/// the time every frame until it settles (even below one row of
/// scroll — the viewport RANGE change alone would miss slow tails).
/// A cell press younger than 150 ms is a touch DURING A FLING: the
/// Flickable's own press handling stops the fling, so the release
/// must not open the viewer (`tap_suppressed`). UI thread only.
grid_motion: Mutex<(f32, Instant)>,
/// Set by `on_cell_pressing` when the grid was still moving; consumed
/// (swap) by `on_open_image`, which then does not open the viewer.
tap_suppressed: AtomicBool,
/// Decoded sizes of painted cells: entry index → (w, h). Mirrors /// Decoded sizes of painted cells: entry index → (w, h). Mirrors
/// `painted` (insert on paint, drop on eviction) so a tapped cell's /// `painted` (insert on paint, drop on eviction) so a tapped cell's
/// viewer can be fitted to its thumbnail — the Image itself is read /// viewer can be fitted to its thumbnail — the Image itself is read
@ -406,6 +423,8 @@ fn state() -> &'static AppState {
full_cache: Mutex::new(FullCache { lru: Vec::new(), pixels: 0 }), full_cache: Mutex::new(FullCache { lru: Vec::new(), pixels: 0 }),
full_inflight: Mutex::new(HashMap::new()), full_inflight: Mutex::new(HashMap::new()),
full_failed: Mutex::new(HashSet::new()), full_failed: Mutex::new(HashSet::new()),
grid_motion: Mutex::new((0.0, Instant::now())),
tap_suppressed: AtomicBool::new(false),
thumb_dims: Mutex::new(HashMap::new()), thumb_dims: Mutex::new(HashMap::new()),
} }
}) })
@ -1431,6 +1450,17 @@ fn start_drain_timer(weak: &slint::Weak<Main>, wid: u32) {
); );
} }
*app.viewport.lock().unwrap() = vp.clone(); *app.viewport.lock().unwrap() = vp.clone();
// Grid motion (drag or fling): remember the last change
// of the RAW scroll position — a cell press within the
// next 150 ms is a fling-stopper, not a tap
// (on_cell_pressing). Half a px beats float noise.
{
let y = w.get_list_y();
let mut gm = app.grid_motion.lock().unwrap();
if (y - gm.0).abs() > 0.5 {
*gm = (y, Instant::now());
}
}
(vp, stable) (vp, stable)
} }
None => (app.viewport.lock().unwrap().clone(), true), None => (app.viewport.lock().unwrap().clone(), true),
@ -1684,12 +1714,37 @@ pub fn start_scan(window: &Main, wid: u32) {
window.on_scroll_changed(move |y: f32, h: f32, w: f32| on_scroll(app, y, h, w)); window.on_scroll_changed(move |y: f32, h: f32, w: f32| on_scroll(app, y, h, w));
// A finger went down on a cell. If the grid was still moving (a fling
// in progress), this touch's job is to STOP the fling — and that
// already happens in the ListView's Flickable, whose press handler
// resets the fling animation (slint core items/flickable.rs:
// `remove_binding` = set the content position to its current value
// with no animation). What must NOT happen is the release opening
// the viewer: mark it so `on_open_image` swallows the tap.
window.on_cell_pressing(move || {
let Some(app) = STATE.get() else { return };
let moving = app
.grid_motion
.lock()
.unwrap()
.1
.elapsed()
< Duration::from_millis(TAP_VS_FLING_MS);
app.tap_suppressed.store(moving, Ordering::Relaxed);
});
// M4 viewer wiring (each handler gets its own clone of the weak ref). // M4 viewer wiring (each handler gets its own clone of the weak ref).
window.on_open_image({ window.on_open_image({
let weak = weak.clone(); let weak = weak.clone();
move |path| { move |path| {
let Some(w) = weak.upgrade() else { return }; let Some(w) = weak.upgrade() else { return };
let Some(app) = STATE.get() else { return }; let Some(app) = STATE.get() else { return };
if app.tap_suppressed.swap(false, Ordering::Relaxed) {
// Tap during a fling: the touch stopped the fling; do not
// open the image.
log::debug!("gallery: tap during fling — suppressed");
return;
}
open_viewer(&w, app, path.to_string()); open_viewer(&w, app, path.to_string());
} }
}); });

View File

@ -30,15 +30,26 @@ export struct Item {
component CellView inherits Rectangle { component CellView inherits Rectangle {
in property <Cell> cell; in property <Cell> cell;
callback cell-clicked(); callback cell-clicked();
callback cell-pressed();
background: #1a1a1a; background: #1a1a1a;
border-radius: 4px; border-radius: 4px;
clip: true; clip: true;
// Whole-cell tap target (M4: opens the viewer). A drag past the slop is // Whole-cell tap target (M4: opens the viewer). A drag past the slop is
// taken over by the ListView's Flickable, which cancels the press, so // taken over by the ListView's Flickable, which cancels the press, so
// scrolling is unaffected. // scrolling is unaffected. The `pressed` hook lets lib.rs detect a
// touch DURING A FLING: the Flickable's own press handling already
// stops the fling (it resets the fling animation on press — slint core
// items/flickable.rs); the tap on release must then NOT open the
// viewer (lib.rs suppresses it when the grid was still moving).
TouchArea { TouchArea {
enabled: cell.path != ""; enabled: cell.path != "";
// 1.18's TouchArea has no `pressed` callback — use pointer-event.
pointer-event(e) => {
if (e.kind == PointerEventKind.down) {
root.cell-pressed();
}
}
clicked => { clicked => {
root.cell-clicked(); root.cell-clicked();
} }
@ -73,6 +84,7 @@ component CellView inherits Rectangle {
component GridRow inherits Rectangle { component GridRow inherits Rectangle {
in property <Item> row-data; in property <Item> row-data;
callback cell-clicked(path: string); callback cell-clicked(path: string);
callback cell-pressed();
property <length> cw: (self.width - 8px) / 3; property <length> cw: (self.width - 8px) / 3;
width: 100%; width: 100%;
height: cw + 4px; height: cw + 4px;
@ -86,6 +98,9 @@ component GridRow inherits Rectangle {
cell-clicked => { cell-clicked => {
root.cell-clicked(cell.path); root.cell-clicked(cell.path);
} }
cell-pressed => {
root.cell-pressed();
}
} }
} }
@ -260,6 +275,9 @@ export component Main inherits Window {
in property <string> v-status: ""; in property <string> v-status: "";
/// A grid cell was tapped: path of the entry to open. /// A grid cell was tapped: path of the entry to open.
callback open-image(path: string); callback open-image(path: string);
// A finger went down on a cell (before the tap is known): lets lib.rs
// mark the tap as "fling-stopper" when the grid is still moving.
callback cell-pressing();
/// Viewer overlay pointer event: (finger, x, y, kind). Single finger /// Viewer overlay pointer event: (finger, x, y, kind). Single finger
/// (pan / double-tap) — pinch goes through the handler callbacks below. /// (pan / double-tap) — pinch goes through the handler callbacks below.
callback viewer-pointer(finger: int, x: float, y: float, kind: int); callback viewer-pointer(finger: int, x: float, y: float, kind: int);
@ -308,6 +326,9 @@ export component Main inherits Window {
cell-clicked(path) => { cell-clicked(path) => {
root.open-image(path); root.open-image(path);
} }
cell-pressed => {
root.cell-pressing();
}
} }
} }
} }

View File

@ -14,9 +14,12 @@
//! Transform model: the image's natural (post-EXIF) size is (iw, ih) //! Transform model: the image's natural (post-EXIF) size is (iw, ih)
//! "content units" (decoded pixels). `Transform { s, tx, ty }` maps content //! "content units" (decoded pixels). `Transform { s, tx, ty }` maps content
//! → viewer: a content point p lands at (tx + p.x·s, ty + p.y·s). `s` is //! → viewer: a content point p lands at (tx + p.x·s, ty + p.y·s). `s` is
//! clamped to [fit, fit·MAX_ZOOM]; `tx`/`ty` are centered when the scaled //! clamped to [fit, max(fit, 1.0)] — the upper bound is NATIVE resolution
//! image is smaller than the viewport and otherwise clamped so the image //! (1 device pixel per image pixel; small images keep fit): zooming past
//! always covers the viewer (no panning into blank areas). //! it only pixelates, so the pinch must STICK there (do nothing, no drift
//! — see `zoom_at`/`Input::pinch`); `tx`/`ty` are centered when the
//! scaled image is smaller than the viewport and otherwise clamped so the
//! image always covers the viewer (no panning into blank areas).
//! //!
//! Gestures: //! Gestures:
//! - single finger: pan (after a 12 px slop so taps never drift). //! - single finger: pan (after a 12 px slop so taps never drift).
@ -32,8 +35,6 @@ use turbojpeg::ScalingFactor;
use crate::images; use crate::images;
/// Maximum pinch zoom, relative to the fit scale.
pub const MAX_ZOOM: f32 = 20.0;
/// A second tap within this time (and distance) of the first is a double /// A second tap within this time (and distance) of the first is a double
/// tap. /// tap.
const DOUBLE_TAP_MS: u64 = 300; const DOUBLE_TAP_MS: u64 = 300;
@ -70,11 +71,12 @@ pub fn fit(iw: f32, ih: f32, vw: f32, vh: f32) -> Transform {
} }
} }
/// Clamp zoom to [fit, fit·MAX_ZOOM] and translation to "image covers the /// Clamp zoom to [fit, max(fit, 1.0)] (native resolution — see module
/// viewport" (centered while smaller than the viewport). /// docs) and translation to "image covers the viewport" (centered while
/// smaller than the viewport).
pub fn clamp(t: Transform, iw: f32, ih: f32, vw: f32, vh: f32) -> Transform { pub fn clamp(t: Transform, iw: f32, ih: f32, vw: f32, vh: f32) -> Transform {
let fit_s = fit_scale(iw, ih, vw, vh); let fit_s = fit_scale(iw, ih, vw, vh);
let s = t.s.clamp(fit_s, fit_s * MAX_ZOOM); let s = t.s.clamp(fit_s, fit_s.max(1.0));
let w = iw * s; let w = iw * s;
let h = ih * s; let h = ih * s;
let tx = if w <= vw { (vw - w) / 2.0 } else { t.tx.clamp(vw - w, 0.0) }; let tx = if w <= vw { (vw - w) / 2.0 } else { t.tx.clamp(vw - w, 0.0) };
@ -83,6 +85,12 @@ pub fn clamp(t: Transform, iw: f32, ih: f32, vw: f32, vh: f32) -> Transform {
} }
/// Zoom by `factor` so the content point under (cx, cy) stays under (cx, cy). /// Zoom by `factor` so the content point under (cx, cy) stays under (cx, cy).
///
/// `s` is clamped BEFORE computing the effective factor: zooming past a
/// cap must be a no-op (s unchanged → factor 1 → tx/ty unchanged). Applying
/// the raw factor first and clamping `s` afterwards would shift tx/ty about
/// the (moving) gesture center even though `s` didn't change — the image
/// follows the fingers like a two-finger drag.
pub fn zoom_at( pub fn zoom_at(
t: Transform, t: Transform,
cx: f32, cx: f32,
@ -93,7 +101,8 @@ pub fn zoom_at(
vw: f32, vw: f32,
vh: f32, vh: f32,
) -> Transform { ) -> Transform {
let s = t.s * factor.max(f32::MIN); let fit_s = fit_scale(iw, ih, vw, vh);
let s = (t.s * factor.max(f32::MIN)).clamp(fit_s, fit_s.max(1.0));
let f = if t.s > 0.0 { s / t.s } else { 1.0 }; let f = if t.s > 0.0 { s / t.s } else { 1.0 };
let tx = cx - (cx - t.tx) * f; let tx = cx - (cx - t.tx) * f;
let ty = cy - (cy - t.ty) * f; let ty = cy - (cy - t.ty) * f;
@ -166,7 +175,23 @@ impl Input {
/// The `ScaleRotateGestureHandler`'s cumulative `scale` about `center`, /// The `ScaleRotateGestureHandler`'s cumulative `scale` about `center`,
/// applied to `base` (a snapshot taken at `started`). Pure — the caller /// applied to `base` (a snapshot taken at `started`). Pure — the caller
/// commits via `set_transform` when the gesture ends. /// commits via `set_transform` when the gesture ends.
///
/// STICKS at both zoom caps: the handler reports a CUMULATIVE scale
/// from the pinch start, so once the transform has hit a cap, later
/// updates keep requesting past it while the gesture center moves.
/// Applying those (even with the clamped s — see `zoom_at`) would
/// drift the image like a two-finger drag; at the cap the correct
/// behavior is: nothing.
pub fn pinch(&self, base: Transform, scale: f32, cx: f32, cy: f32) -> Transform { pub fn pinch(&self, base: Transform, scale: f32, cx: f32, cy: f32) -> Transform {
let fit_s = fit_scale(self.iw, self.ih, self.vw, self.vh);
let max_s = fit_s.max(1.0);
let target = base.s * scale.max(f32::MIN);
let eps = max_s * 1e-3;
if self.t.s >= max_s - eps && target >= max_s
|| self.t.s <= fit_s + eps && target <= fit_s
{
return self.t;
}
zoom_at(base, cx, cy, scale.max(f32::MIN), self.iw, self.ih, self.vw, self.vh) zoom_at(base, cx, cy, scale.max(f32::MIN), self.iw, self.ih, self.vw, self.vh)
} }
@ -404,8 +429,53 @@ mod tests {
let fit_s = fit_scale(IW, IH, VW, VH); let fit_s = fit_scale(IW, IH, VW, VH);
let low = clamp(Transform { s: fit_s / 2.0, tx: 0.0, ty: 0.0 }, IW, IH, VW, VH); let low = clamp(Transform { s: fit_s / 2.0, tx: 0.0, ty: 0.0 }, IW, IH, VW, VH);
assert!((low.s - fit_s).abs() < 1e-4); assert!((low.s - fit_s).abs() < 1e-4);
// Upper bound = native resolution (1.0); fit (0.27) is below it.
let high = clamp(Transform { s: fit_s * 1000.0, tx: 0.0, ty: 0.0 }, IW, IH, VW, VH); let high = clamp(Transform { s: fit_s * 1000.0, tx: 0.0, ty: 0.0 }, IW, IH, VW, VH);
assert!((high.s - fit_s * MAX_ZOOM).abs() < 1e-2); assert!((high.s - 1.0).abs() < 1e-3);
// A SMALL image whose fit is already above native: fit wins (the
// image must be able to fill the screen).
let small = clamp(Transform { s: 10.0, tx: 0.0, ty: 0.0 }, 500.0, 400.0, VW, VH);
assert!((small.s - (VW / 500.0).min(VH / 400.0)).abs() < 1e-3);
}
#[test]
fn zoom_at_past_cap_is_noop() {
// At native resolution (s = 1.0, the cap for this photo), zooming
// FURTHER must leave the transform untouched — including tx/ty, no
// matter where the (moving) gesture center is.
let t = Transform { s: 1.0, tx: -500.0, ty: -300.0 };
let t = clamp(t, IW, IH, VW, VH);
let t2 = zoom_at(t, 100.0, 200.0, 1.5, IW, IH, VW, VH);
assert_eq!(t2, t);
// Zooming out from the cap works as usual.
let t3 = zoom_at(t, 540.0, 1170.0, 0.5, IW, IH, VW, VH);
assert!((t3.s - 0.5).abs() < 1e-4);
}
#[test]
fn pinch_sticks_at_caps() {
let mut i = Input::new(IW, IH, VW, VH);
let fit_s = fit_scale(IW, IH, VW, VH);
// Zoom to the native cap (a pinch from fit, base = fit).
let fit_t = i.transform();
let at_cap = i.pinch(fit_t, 1.0 / fit_s, 300.0, 400.0);
i.set_transform(at_cap);
assert!((i.transform().s - 1.0).abs() < 1e-3);
// Further pinch (cumulative scale keeps growing, center moves):
// NOTHING happens — no zoom, no drift.
for (scale, cx, cy) in [(2.0 / fit_s, 300.0, 400.0), (3.0 / fit_s, 900.0, 1900.0)] {
let t = i.pinch(fit_t, scale, cx, cy);
assert_eq!(t, i.transform());
}
// Zooming out again works.
let t = i.pinch(i.transform(), 0.5, 540.0, 1170.0);
assert!(t.s < i.transform().s);
// Same at the FIT cap (zoomed all the way out): pinching smaller
// is a no-op.
let mut i2 = Input::new(IW, IH, VW, VH);
let t2 = i2.transform();
assert_eq!(i2.pinch(t2, 0.5, 100.0, 100.0), t2);
assert_eq!(i2.pinch(t2, 0.01, 100.0, 100.0), t2);
} }
#[test] #[test]
@ -550,8 +620,10 @@ mod tests {
assert!((t1.ty + py * t1.s - 1170.0).abs() < 1e-2); assert!((t1.ty + py * t1.s - 1170.0).abs() < 1e-2);
// A later event reports 4.0 CUMULATIVE (not ×4 of the last): the // A later event reports 4.0 CUMULATIVE (not ×4 of the last): the
// transform is always base × scale — no incremental compounding. // transform is always base × scale — no incremental compounding.
// 4× fit (1.08) exceeds the NATIVE-resolution cap (1.0): clamped,
// and the fixation about the center still holds.
let t2 = i.pinch(base, 4.0, 540.0, 1170.0); let t2 = i.pinch(base, 4.0, 540.0, 1170.0);
assert!((t2.s - base.s * 4.0).abs() < 1e-2); assert!((t2.s - 1.0).abs() < 1e-3);
assert!((t2.tx + px * t2.s - 540.0).abs() < 1e-2); assert!((t2.tx + px * t2.s - 540.0).abs() < 1e-2);
} }