diff --git a/Cargo.toml b/Cargo.toml index 88ec615..d0e8e4b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,9 +57,11 @@ target_sdk_version = 35 [package.metadata.android.application] label = "Gallery" -# JANK-HUNT ONLY: debuggable so `adb shell simpleperf`/run-as can attach to -# the process (off-CPU profiling of the UI thread). REMOVE before shipping. -debuggable = true +# Not debuggable: a debuggable app shows the system 16 KB compatibility +# dialog on EVERY cold launch — that dialog is its own activity, it pauses +# us, and its lifecycle was a source of lost surfaces / black windows. +# (Jank-hunt is over; simpleperf no longer needed.) +debuggable = false [[package.metadata.android.application.activity]] name = "android.app.NativeActivity" diff --git a/src/lib.rs b/src/lib.rs index 20c26d5..6404e86 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -209,6 +209,9 @@ const MAX_PAINTED: usize = 225; /// model mutation (one cell property change; texture upload only when the /// pixels actually change) — cheap enough for 4+4 per tick. const MAX_PAINT_CELLS_PER_TICK: usize = 2; +/// Hold-queue cap for finished results that missed the per-tick paint cap +/// (~two viewports); overflow drops back to the disk cache. +const READY_CAP: usize = 48; const MAX_EVICT_CELLS_PER_TICK: usize = 4; /// Reconcile tick period. 8 ms ≈ one 120 Hz frame; Slint's event loop is /// VSync-paced on Android, so the actual cadence quantizes to the display @@ -307,6 +310,15 @@ struct AppState { /// set: a full decode can fail where the 400 px thumb succeeds, e.g. /// the GPU texture cap). UI thread only. full_failed: Mutex>, + /// Jank fix (pop-in): finished decode results that missed the + /// per-tick paint cap. Previously they were DROPPED (out of inflight, + /// not in painted) and FILL re-submitted them → the same cell + /// re-decoded 3–6× and a new viewport took 500–850 ms to fill after a + /// flick. Held results paint on a later tick at zero re-decode cost; + /// `ready_paths` keeps FILL from re-submitting them. Bounded by + /// READY_CAP. UI thread only. + ready: Mutex>, + ready_paths: Mutex>, /// 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 @@ -314,7 +326,7 @@ struct AppState { /// 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)>, + 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, @@ -435,17 +447,22 @@ static REAPER_TX: std::sync::LazyLock>> = let (tx, rx) = std::sync::mpsc::channel::>(); std::thread::Builder::new() .name("full-reaper".into()) - .spawn(move || loop { - // 500 ms batching: coalesce drops, never touch the UI path. - match rx.recv_timeout(std::time::Duration::from_millis(500)) { - Ok(first) => { - drop(first); - // Drain whatever else is waiting (no extra sleep). - while let Ok(b) = rx.try_recv() { - drop(b); + .spawn(move || { + // Garbage: never compete with the decode workers (nice +2) + // or the UI thread (nice -10) for CPU. + prio::set_self_nice(5); + loop { + // 500 ms batching: coalesce drops, never touch the UI path. + match rx.recv_timeout(std::time::Duration::from_millis(500)) { + Ok(first) => { + drop(first); + // Drain whatever else is waiting (no extra sleep). + while let Ok(b) = rx.try_recv() { + drop(b); + } } + Err(_) => {} } - Err(_) => {} } }) .ok(); @@ -484,6 +501,8 @@ fn state() -> &'static AppState { full_inflight: Mutex::new(HashMap::new()), full_evicted_recent: Mutex::new(HashMap::new()), full_failed: Mutex::new(HashSet::new()), + ready: Mutex::new(Vec::new()), + ready_paths: Mutex::new(HashSet::new()), grid_motion: Mutex::new((0.0, Instant::now())), tap_suppressed: AtomicBool::new(false), thumb_dims: Mutex::new(HashMap::new()), @@ -540,20 +559,52 @@ fn set_list(weak: &slint::Weak
, entries: Vec, status: String) { let _ = slint::invoke_from_event_loop(move || { let Some(w) = weak.upgrade() else { return }; let Some(app) = STATE.get() else { return }; - app.engine.lock().unwrap().bump_generation(); - app.inflight.lock().unwrap().clear(); - app.painted.lock().unwrap().clear(); - app.failed.lock().unwrap().clear(); - app.full_cache.lock().unwrap().clear(); - app.full_inflight.lock().unwrap().clear(); - app.full_evicted_recent.lock().unwrap().clear(); - app.full_failed.lock().unwrap().clear(); - app.thumb_dims.lock().unwrap().clear(); + + // STREAMING APPEND vs REPLACE: during the initial scan the list grows + // in 512-entry batches. A reset (set_vec + generation bump + state + // clears) on every batch orphaned every queued/in-flight decode + // (results arrived marked stale → dropped → FILL re-submitted) — a + // 17× decode storm on cold start, plus full row churn. Appends must + // be silent to the pipeline: extend the model, keep all state. + let is_append = { + let cur = app.entries.lock().unwrap(); + !entries.is_empty() + && entries.len() > cur.len() + && cur.len() > 0 + && entries[..cur.len()].iter().zip(cur.iter()).all(|(a, b)| a == b) + }; + + if !is_append { + app.engine.lock().unwrap().bump_generation(); + app.inflight.lock().unwrap().clear(); + app.painted.lock().unwrap().clear(); + app.failed.lock().unwrap().clear(); + app.full_cache.lock().unwrap().clear(); + app.full_inflight.lock().unwrap().clear(); + app.full_evicted_recent.lock().unwrap().clear(); + app.full_failed.lock().unwrap().clear(); + app.thumb_dims.lock().unwrap().clear(); + app.ready.lock().unwrap().clear(); + app.ready_paths.lock().unwrap().clear(); + } *app.entries.lock().unwrap() = entries.clone(); let n = entries.len(); *app.viewport.lock().unwrap() = 0..INITIAL_VIEWPORT_CELLS.min(n); if let Some(m) = w.get_items().as_any().downcast_ref::>() { - m.set_vec(chunk(&entries)); + if is_append { + let old_rows = m.row_count(); + let new_rows = n.div_ceil(GRID_COLS); + if new_rows > old_rows { + // Re-chunk only the appended suffix; existing rows keep + // their textures untouched. + let mut rows = chunk(&entries); + rows.truncate(new_rows); + rows.drain(..old_rows.min(rows.len())); + m.extend_from_slice(&rows); + } + } else { + m.set_vec(chunk(&entries)); + } } w.set_status(status.into()); }); @@ -579,20 +630,20 @@ fn set_status(weak: &slint::Weak
, status: String) { /// was the scroll hitch + content pulse.) Slint's ListView for-loop keeps /// every row alive, so the live row's model is always reachable. /// Returns the number of paint cells applied. Paints are capped at -/// `MAX_PAINT_CELLS_PER_TICK`; unapplied paints are dropped on purpose — -/// their cells are no longer painted and no longer in flight, so the fill -/// step re-submits them (disk-cache hit) and they land next tick. +/// `MAX_PAINT_CELLS_PER_TICK` (cap); excess finished results are held in +/// `app.ready` (pop-in fix) and paint on a later tick without re-decoding. fn apply_cell_updates( app: &AppState, m: &VecModel, entries: &[Entry], paints: &[JobResult], clears: &[usize], + paint_cap: usize, ) -> usize { let mut painted_now = 0usize; for r in paints { - if painted_now >= MAX_PAINT_CELLS_PER_TICK { - break; // the rest is re-covered by the fill step + if painted_now >= paint_cap { + break; // the rest is held / re-covered by the fill step } if !r.ok || r.id >= entries.len() || entries[r.id].path != r.path { continue; // list moved on; drop @@ -1225,6 +1276,8 @@ fn reset_grid_in_place(w: &Main, app: &AppState) { app.full_evicted_recent.lock().unwrap().clear(); app.full_failed.lock().unwrap().clear(); app.thumb_dims.lock().unwrap().clear(); + app.ready.lock().unwrap().clear(); + app.ready_paths.lock().unwrap().clear(); // The viewer's image (and its load) also belonged to the dying device: // close it; the tick/grid re-fill on the new one. close_viewer(w, app); @@ -1296,6 +1349,14 @@ fn install_render_notifier(window: &Main, wid: u32) { } else if gputex::ready() { log::info!("gputex: [W{wid}] wgpu device captured"); } + // Always repaint on (re)setup: the surface may have been + // invalidated while the window was away (a system dialog, + // an app switch, screen lock) with NO model change pending + // — without this the window comes back black and stays + // black until the next content mutation. + if let Some(w) = weak.upgrade() { + w.window().request_redraw(); + } } // Jank-hunt instrumentation (`perf` feature; see // docs/BUGS/scroll-jank-120hz.md). FR = app-side render cost @@ -1307,6 +1368,7 @@ fn install_render_notifier(window: &Main, wid: u32) { FR_BEFORE_US.store(now_us(), std::sync::atomic::Ordering::Relaxed); } slint::RenderingState::AfterRendering => { + LAST_PRESENT_US.store(now_us(), std::sync::atomic::Ordering::Relaxed); #[cfg(feature = "perf")] { let now = now_us(); @@ -1346,8 +1408,7 @@ static FR_BEFORE_US: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64 #[cfg(feature = "perf")] static FR_LAST_US: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); -/// Monotonic ms since process start (wall clock could jump on NTP). -#[cfg(feature = "perf")] +/// Monotonic µs since process start (wall clock could jump on NTP). static NOW_T0: std::sync::OnceLock = std::sync::OnceLock::new(); #[cfg(feature = "perf")] #[inline] @@ -1356,7 +1417,8 @@ fn now_ms() -> f64 { t0.elapsed().as_secs_f64() * 1000.0 } -#[cfg(feature = "perf")] +// Unconditional (not `perf`-gated): the window-liveness watchdog needs it +// in every build. fn now_us() -> u64 { NOW_T0.get_or_init(std::time::Instant::now).elapsed().as_micros() as u64 } @@ -1542,6 +1604,13 @@ fn on_scroll(app: &AppState, y: f32, height: f32, width: f32) { /// lines are only visible at log level Debug). static DBG_TICKS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); +/// App clock (µs) of the last completed frame (AfterRendering). Feeds the +/// window-liveness watchdog in the drain tick. Updated unconditionally +/// (not behind the `perf` feature) — it is the only signal we have that +/// the surface is still presenting. +static LAST_PRESENT_US: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(0); + fn start_drain_timer(weak: &slint::Weak
, wid: u32) { let weak = weak.clone(); let timer = Timer::default(); @@ -1557,6 +1626,22 @@ fn start_drain_timer(weak: &slint::Weak
, wid: u32) { let Some(app) = STATE.get() else { return }; let Some(w) = weak.upgrade() else { return }; + // WINDOW LIVENESS WATCHDOG: the OS can invalidate our surface + // (a system dialog activity, an app switch, screen lock) with + // NO window event at all — the last presented buffer is lost + // and, at rest, nothing ever sets pending_redraw again, so the + // window sits black forever. If the window is visible but no + // frame has completed in a while, force one redraw (one 8 ms + // frame, once per >2.5 s, negligible). + if w.window().is_visible() + && now_us() + .saturating_sub(LAST_PRESENT_US.load(std::sync::atomic::Ordering::Relaxed)) + > 2_500_000 + { + log::info!("gallery: [W{wid}] no frame for >2.5s while visible — forcing redraw"); + w.window().request_redraw(); + } + // Pause thumbnail painting and model mutations during active drag/fling // (motion within the last 150ms) to ensure zero layout thrashing during scroll. // Thumbnails paint instantly the moment scrolling pauses/settles. @@ -1652,9 +1737,44 @@ fn start_drain_timer(weak: &slint::Weak
, wid: u32) { // done). Paint viewport results (capped); the disk cache owns // the rest — a dropped result costs a ~10 ms cache hit on // re-entry, never a re-decode. + // Paint cap: 2 during grid motion (protect the 120 Hz frame + // budget — each paint uploads a texture), 4 while the grid is + // settled (fast pop-in right after a flick). + let paint_cap = { + let gm = app.grid_motion.lock().unwrap(); + if gm.1.elapsed().as_millis() < 150 { + MAX_PAINT_CELLS_PER_TICK + } else { + MAX_PAINT_CELLS_PER_TICK * 2 + } + }; let mut paints: Vec = Vec::new(); + // 1a. HOLD QUEUE: results that missed the paint cap on a + // previous tick paint first this tick (already decoded — + // zero re-decode cost). Stale ones (scrolled away) drop + // back to the disk cache. + { + let mut rd = app.ready.lock().unwrap(); + if !rd.is_empty() { + let mut kept: Vec = Vec::new(); + let mut rp = app.ready_paths.lock().unwrap(); + for r in rd.drain(..) { + if paints.len() < paint_cap && viewport.contains(&r.id) { + rp.remove(&r.path); + paints.push(r); + } else if kept.len() < READY_CAP { + kept.push(r); + } else { + rp.remove(&r.path); // overflow: disk cache owns it + } + } + rd.extend(kept); + } + } { let mut inflight = app.inflight.lock().unwrap(); + let mut rd = app.ready.lock().unwrap(); + let mut rp = app.ready_paths.lock().unwrap(); for r in results { inflight.remove(&r.path); if r.skipped { @@ -1674,11 +1794,18 @@ fn start_drain_timer(weak: &slint::Weak
, wid: u32) { } continue; } - if viewport.contains(&r.id) - && paints.len() < MAX_PAINT_CELLS_PER_TICK - { + if viewport.contains(&r.id) && paints.len() < paint_cap { paints.push(r); + } else if viewport.contains(&r.id) && rd.len() < READY_CAP { + // Cap full: HOLD for the next tick instead of + // dropping (drop ⇒ FILL re-submits ⇒ re-decode + // storm + slow pop-in). Mark the path so FILL + // skips it while held. + rp.insert(r.path.clone()); + rd.push(r); } + // Out-of-viewport results: drop (the disk cache owns + // them; a re-entry is a ~2 ms hit, never a re-decode). } } @@ -1716,7 +1843,7 @@ fn start_drain_timer(weak: &slint::Weak
, wid: u32) { // 3. APPLY: in-place nested cell updates (≤4 paints + ≤4 // evictions; each = one cell property change). - let painted_now = apply_cell_updates(app, m, &entries, &paints, &to_clear); + let painted_now = apply_cell_updates(app, m, &entries, &paints, &to_clear, paint_cap); // 4. FILL: every viewport cell without pixels and without an // in-flight job gets submitted now, center-first so the eye is @@ -1732,6 +1859,7 @@ fn start_drain_timer(weak: &slint::Weak
, wid: u32) { { let painted = app.painted.lock().unwrap(); let failed = app.failed.lock().unwrap(); + let ready_paths = app.ready_paths.lock().unwrap(); let mut inflight = app.inflight.lock().unwrap(); let center = (viewport.start + viewport.end) / 2; let mut missing: Vec = viewport @@ -1741,6 +1869,9 @@ fn start_drain_timer(weak: &slint::Weak
, wid: u32) { let e = &entries[*id]; !painted.contains(id) && !inflight.contains_key(&e.path) + // A held (already-decoded) result paints on a + // later tick — do not re-submit it. + && !ready_paths.contains(&e.path) && !failed.contains(&(e.path.clone(), e.mtime, e.size)) }) .collect(); @@ -1989,14 +2120,28 @@ fn scan_thread(weak: &slint::Weak
) { // Warm start: keep the cached list, only update the status line. let t0 = std::time::Instant::now(); let roots = scan::roots(); + // STREAMING ORDER: append-only (each 512-file group mtime-sorted, + // groups concatenated). A stable prefix is what lets set_list take its + // APPEND path (no model reset, no generation bump, no orphaned decode + // jobs) — the old code re-sorted the WHOLE accumulated list per batch, + // so every batch's "sorted prefix" differed and every batch was a full + // reset: a 17× decode storm on cold start. The final global sort lands + // as ONE reset at scan end (viewport re-fills from disk cache, ~2 ms + // per cell). + let acc = std::cell::RefCell::new(Vec::::new()); + let last = std::cell::Cell::new(0usize); let entries = scan::walk_stream(&roots, &|batch| { let n = batch.len(); if db_warm { set_status(weak, format!("Updating… {n}")); + last.set(n); } else { - let mut sorted = batch.to_vec(); - sorted.sort_by(|a, b| b.mtime.cmp(&a.mtime).then_with(|| a.path.cmp(&b.path))); - set_list(weak, sorted, format!("Scanning… {n}")); + let from = last.get(); + last.set(n); + let mut group: Vec = batch[from..].to_vec(); + group.sort_by(|a, b| b.mtime.cmp(&a.mtime).then_with(|| a.path.cmp(&b.path))); + acc.borrow_mut().extend(group); + set_list(weak, acc.borrow().clone(), format!("Scanning… {n}")); } });