diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 613b17e7..33a7a497 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -73,6 +73,7 @@ pub fn inspect(app: &LightcraftApp, ctx: &egui::Context) -> Value { "pixelsPerPoint": ctx.pixels_per_point(), "canvasRect": app.canvas_rect.map(rect_json), "imageRect": app.image_rect.map(rect_json), + "scroll": {"grid": app.grid_scroll, "filmstrip": app.film_scroll}, "active": app.session.active().map(|p| p.0), "selection": app.session.selection.ids.iter().map(|p| p.0).collect::>(), "activeMask": app.session.active_mask, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6a67bd16..13a0e09d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -24,6 +24,8 @@ pub mod widgets; #[cfg(test)] mod tests_masking; +#[cfg(test)] +mod tests_scroll; use std::sync::mpsc::{Receiver, Sender}; @@ -114,6 +116,10 @@ pub struct LightcraftApp { /// Rect of the photo canvas and the displayed image (screen points) from the last frame. pub canvas_rect: Option, pub image_rect: Option, + /// Scroll offsets (points) of the photo grid (vertical) and the filmstrip (horizontal) as + /// drawn last (`ui.inspect` → `scroll`). + pub grid_scroll: Option, + pub film_scroll: Option, /// Widget registry from the last frame (automation ids → rects). pub widgets: Vec<(String, egui::Rect)>, /// In-progress on-canvas gesture (brush stroke points, gradient drag…). @@ -169,6 +175,8 @@ impl LightcraftApp { last_time: 0.0, canvas_rect: None, image_rect: None, + grid_scroll: None, + film_scroll: None, widgets: vec![], gesture: None, loupe_shown: None, diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index a869dbd6..81cee455 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1201,17 +1201,20 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { let cell_w = 120.0; let ppp = ui.ctx().pixels_per_point(); let mut child = ui.new_child(egui::UiBuilder::new().max_rect(r.shrink2(vec2(0.0, 4.0)))); - let scroll_key = egui::Id::new("film-last-active"); - let last: Option = child.data(|d| d.get_temp(scroll_key)); - child.data_mut(|d| d.insert_temp(scroll_key, active)); + // a vertical mouse wheel scrolls the strip sideways (horizontal trackpad scrolls still work) + child.style_mut().always_scroll_the_only_direction = true; + // only a new active photo (or the strip appearing) scrolls it; see `grid::follow_active` + let follow = super::grid::follow_active(child.ctx(), egui::Id::new("film-follow-active"), active); egui::ScrollArea::horizontal().id_salt("filmstrip").auto_shrink([false, false]).show_viewport(&mut child, |ui, vp| { let (area, _) = ui.allocate_exact_size(vec2(ids.len() as f32 * cell_w, r.height() - 16.0), Sense::hover()); + app.film_scroll = Some(vp.left()); for (i, id) in ids.iter().enumerate() { let cr = Rect::from_min_size(pos2(area.left() + i as f32 * cell_w, area.top()), vec2(cell_w, area.height())); - if Some(*id) == active && last != active { + let local = Rect::from_min_size(pos2(i as f32 * cell_w, 0.0), cr.size()); + // centred when it is (partly) off screen; a click on a visible thumbnail leaves the strip alone + if follow && Some(*id) == active && !(local.left() >= vp.left() && local.right() <= vp.right()) { ui.scroll_to_rect(cr, Some(egui::Align::Center)); } - let local = Rect::from_min_size(pos2(i as f32 * cell_w, 0.0), cr.size()); if !local.intersects(vp.expand2(vec2(cell_w * 4.0, 0.0))) { continue; } diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index 115f6c77..23db3687 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -102,18 +102,19 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let cells: Vec = ids.iter().zip(&lay.cells).map(|(id, r)| Cell { id: *id, rect: *r }).collect(); let total_h = lay.height + 12.0; let active = app.session.selection.active; - // keep the active photo in view when it changes by keyboard - let scroll_to: Option = { - let key = egui::Id::new("grid-last-active"); - let last: Option = ui.data(|d| d.get_temp(key)); - ui.data_mut(|d| d.insert_temp(key, active)); - if last != active { active.and_then(|a| cells.iter().find(|c| c.id == a).map(|c| c.rect)) } else { None } + // bring the active photo into view when it changes (keyboard, click, command) or the grid + // comes back on screen; otherwise the scroll position is the user's + let scroll_to: Option = if follow_active(ui.ctx(), egui::Id::new("grid-follow-active"), active) { + active.and_then(|a| cells.iter().find(|c| c.id == a).map(|c| c.rect)) + } else { + None }; let stacks = app.session.catalog.stack_index(); let mut visible_ids = HashSet::new(); egui::ScrollArea::vertical().id_salt("grid-scroll").auto_shrink([false, false]).show_viewport(ui, |ui, viewport| { let (area, _) = ui.allocate_exact_size(vec2(ui.available_width(), total_h), Sense::hover()); let origin = area.min; + app.grid_scroll = Some(viewport.top()); if let Some(r) = scroll_to { ui.scroll_to_rect(r.translate(origin.to_vec2()), None); } @@ -154,6 +155,21 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let _ = (Color32::BLACK, StrokeKind::Inside, Stroke::NONE); } +/// Should a scrolling photo strip (`key`: the grid, the filmstrip) bring the active photo into +/// view on this pass? Yes when the active photo changed since the strip was last drawn, or when +/// the strip was not drawn on the previous pass (view switch, panel shown again). Otherwise the +/// scroll position is the user's: time passing, finished renders, edits to the photo or a released +/// scroll bar never move it (issue #11). +pub(crate) fn follow_active(ctx: &egui::Context, key: egui::Id, active: Option) -> bool { + let pass = ctx.cumulative_pass_nr(); + let last: Option<(Option, u64)> = ctx.data(|d| d.get_temp(key)); + ctx.data_mut(|d| d.insert_temp(key, (active, pass))); + match last { + Some((was, drawn)) => was != active || drawn + 1 < pass, + None => true, + } +} + /// Height of a date header row (points). pub const HEADER_H: f32 = 40.0; diff --git a/crates/ui-egui/src/tests_scroll.rs b/crates/ui-egui/src/tests_scroll.rs new file mode 100644 index 00000000..05167c47 --- /dev/null +++ b/crates/ui-egui/src/tests_scroll.rs @@ -0,0 +1,117 @@ +//! Headless tests of the photo grid's and the filmstrip's scrolling (issue #11): the user's +//! scroll position stays put until the active photo changes; the mouse wheel scrolls the +//! filmstrip sideways. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn demo(view: &str) -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + // big thumbnails: the demo library is several screens tall + let r = h.request("ui.set", json!({"view": view, "thumbSize": 480.0}), T); + assert_eq!(r["ok"], true, "{r}"); + let first = h.app.session.visible_cloned()[0].0; + let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [first]}}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + h +} + +/// Run `n` frames (plus a settle: thumbnails and renders finishing must not move anything). +fn idle(h: &mut Headless, n: usize) { + h.settle(SETTLE); + for _ in 0..n { + h.step(); + } +} + +fn grid_y(h: &Headless) -> f32 { + h.app.grid_scroll.expect("grid drawn") +} + +fn film_x(h: &Headless) -> f32 { + h.app.film_scroll.expect("filmstrip drawn") +} + +fn widget(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("no widget {id}")) +} + +#[test] +fn grid_keeps_the_users_scroll_position() { + let mut h = demo("photoGrid"); + assert_eq!(grid_y(&h), 0.0); + let canvas = h.app.canvas_rect.expect("grid canvas"); + // the mouse wheel scrolls the grid away from the active (first) photo… + h.request("ui.move", json!({"x": canvas.center().x, "y": canvas.center().y}), T); + for _ in 0..4 { + h.request("ui.scroll", json!({"dy": -300.0}), T); + } + idle(&mut h, 120); + let y = grid_y(&h); + assert!(y > 600.0, "the wheel scrolled the grid ({y})"); + // …and it stays there: no snap back after a second and more of frames and finished renders + idle(&mut h, 120); + assert_eq!(grid_y(&h), y, "the grid snapped back"); + // dragging the scroll bar back up a bit, then releasing it + let x = canvas.right() - 3.0; + let r = h.request("ui.drag", json!({"x": x, "y": canvas.top() + 300.0, "toX": x, "toY": canvas.top() + 200.0}), T); + assert_eq!(r["ok"], true, "{r}"); + idle(&mut h, 120); + let y2 = grid_y(&h); + assert!(y2 > 0.0 && (y2 - y).abs() > 1.0, "the scroll bar moved the grid ({y} → {y2})"); + // editing the active photo does not yank the grid back either + let r = h.request("engine.execute", json!({"command": "develop.set", "params": {"control": "light.exposure", "value": 0.5}}), T); + assert_eq!(r["ok"], true, "{r}"); + idle(&mut h, 60); + assert_eq!(grid_y(&h), y2, "an edit snapped the grid back"); + // a new active photo (keyboard) brings it into view + h.request("ui.key", json!({"key": "right"}), T); + idle(&mut h, 60); + let second = h.app.session.visible_cloned()[1].0; + assert_eq!(h.app.session.active().map(|p| p.0), Some(second)); + let cell = widget(&h, &format!("thumb:{second}")); + assert!(canvas.contains_rect(cell) || cell.intersects(canvas), "the new active photo is in view ({cell:?} vs {canvas:?})"); + assert!(grid_y(&h) < y2, "scrolled up to the second photo"); +} + +#[test] +fn filmstrip_wheel_scrolls_and_keeps_its_position() { + let mut h = demo("detail"); + let first = h.app.session.visible_cloned()[0].0; + let x0 = film_x(&h); + let film = widget(&h, &format!("film:{first}")); + // a vertical wheel over the filmstrip scrolls it sideways + h.request("ui.move", json!({"x": 600.0, "y": film.center().y}), T); + for _ in 0..3 { + h.request("ui.scroll", json!({"dy": -200.0}), T); + } + idle(&mut h, 60); + let x = film_x(&h); + assert!(x > x0 + 300.0, "the wheel scrolled the filmstrip ({x0} → {x})"); + // a horizontal (trackpad) scroll still works + h.request("ui.scroll", json!({"dx": 100.0}), T); + idle(&mut h, 60); + let x1 = film_x(&h); + assert!(x1 < x - 50.0, "a horizontal scroll moved it back ({x} → {x1})"); + // it stays there + idle(&mut h, 120); + assert_eq!(film_x(&h), x1, "the filmstrip snapped back"); + // the next photo by keyboard brings the filmstrip to it + h.request("ui.key", json!({"key": "right"}), T); + idle(&mut h, 60); + let second = h.app.session.visible_cloned()[1].0; + assert_eq!(h.app.session.active().map(|p| p.0), Some(second)); + assert!(film_x(&h) < x1, "scrolled back to the active photo"); + let cell = widget(&h, &format!("film:{second}")); + assert!(cell.left() >= 0.0 && cell.right() <= 1200.0, "the new active photo is in view ({cell:?})"); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 2b4c9884..186adf40 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -21,7 +21,7 @@ mode ([mcp.md](mcp.md)) is a thin layer over this channel. Implementation: | `engine.commands` | — | Engine + UI commands: id, label, menu, shortcut, params doc, enabled | | `ui.menu.list` | — | Menu entries (flat: id, label, menu path, shortcut, enabled) | | `ui.menu.tree` | — | The menu bar as shown (File … Help): items `{id, params?, label, shortcut?, enabled, checked?}`, separators, submenus — the model behind the native macOS menu bar and the in-window menus | -| `ui.inspect` | — | UI state, window, canvas/image rects, active photo, selection, perf, status, memory (bytes per cache, see `library.memory`; plus stage caches and textures), `export: {running: {total, done, current} \| null, last}` | +| `ui.inspect` | — | UI state, window, canvas/image rects, `scroll: {grid, filmstrip}` (scroll offsets in points, `null` until drawn), active photo, selection, perf, status, memory (bytes per cache, see `library.memory`; plus stage caches and textures), `export: {running: {total, done, current} \| null, last}` | | `ui.widgets` | `{filter?}` | On-screen widgets `{id, rect: [x, y, w, h]}` (screen points) | | `ui.clickWidget` / `ui.dragWidget` / `ui.hoverWidget` | `{id, count?, fx?, fy?}` / `{id, toX?, toY?, dx?, dy?, steps?}` / `{id, fx?, fy?}` | Real egui input on a widget (hover: the pointer rests on it, e.g. for preset/profile previews) | | `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points | diff --git a/docs/parity.md b/docs/parity.md index 171b5aa0..be341962 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -149,12 +149,12 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-VIEW-PHOTOGRID | Justified photo grid | P0 | ✅ | `cmd:view.photoGrid`, `cmd:library.groups`, `cmd:library.sort` (`group`), `crates/ui-egui/src/panels/grid.rs`, `crates/catalog/src/dates.rs` | grouped by capture / import / edit date (day headers “Wednesday, 30 September 2026 · 12 photos”, months or years when zoomed out, or a fixed day/month/year/none choice in the sort menu); sticky header; clicking a header selects its photos; stacks never split | +| LR-VIEW-PHOTOGRID | Justified photo grid | P0 | ✅ | `cmd:view.photoGrid`, `cmd:library.groups`, `cmd:library.sort` (`group`), `crates/ui-egui/src/panels/grid.rs`, `crates/catalog/src/dates.rs` | grouped by capture / import / edit date (day headers “Wednesday, 30 September 2026 · 12 photos”, months or years when zoomed out, or a fixed day/month/year/none choice in the sort menu); sticky header; clicking a header selects its photos; stacks never split; scrolls to the active photo only when it changes (or the grid comes back on screen), never on its own | | LR-VIEW-SQUAREGRID | Square grid | P0 | ✅ | `cmd:view.squareGrid` | same date headers as the photo grid | | LR-VIEW-DETAIL | Single-photo view | P0 | ✅ | `cmd:view.detail`, `crates/ui-egui/src/panels/detail.rs` | | | LR-VIEW-EDIT | Edit view | P0 | ✅ | `cmd:panel.edit` | | | LR-VIEW-FULLSCREEN | Full-screen preview | P1 | ✅ | `cmd:view.fullScreenPreview`, `cmd:view.enterFullScreen` | photo on black, arrows step, Esc exits; ⇧⌘F window full screen | -| LR-VIEW-FILMSTRIP | Filmstrip | P0 | ✅ | `cmd:view.filmstrip` | | +| LR-VIEW-FILMSTRIP | Filmstrip | P0 | ✅ | `cmd:view.filmstrip`, `crates/ui-egui/src/panels/detail.rs` | the mouse wheel scrolls it sideways; it follows the active photo only when that changes (centred if off screen), so a scrolled strip stays put | | LR-VIEW-ZOOM | Zoom & pan | P0 | ✅ | `cmd:view.zoomFit`, `cmd:view.zoom100`, `cmd:view.zoomIn`, `cmd:view.zoomOut`, `cmd:view.zoomToggle`, `cmd:view.clickZoom` | steps 25–800 % (not 6–1600 %); Fill only in the bottom bar; a click on the photo (and Z / Space) eases to the click-zoom ratio (1:1 default, 2:1, 3:1, 4:1, 8:1 in the bottom bar) | | LR-VIEW-NAVIGATOR | Navigator mini map | P1 | ✅ | `cmd:view.navigator`, `crates/ui-egui/src/panels/detail.rs` | shown while zoomed (bottom right); click/drag pans | | LR-VIEW-BEFOREAFTER | Before / after | P0 | ✅ | `cmd:view.showOriginal`, `cmd:view.beforeAfter`, `cmd:view.beforeAfterSplit`, `cmd:view.beforeAfterTopBottom`, `cmd:view.beforeAfterSplitTopBottom`, `cmd:beforeAfter.setBefore`, `cmd:beforeAfter.copyAfterToBefore`, `cmd:beforeAfter.copyBeforeToAfter`, `cmd:beforeAfter.swap`, `cmd:beforeAfter.resetBefore` | all four layouts; before = the import state (defaults + import preset) or a chosen history step / version / the current settings; copy and swap (View → Before/After Settings, History row menu). The chosen before lasts for the session |