diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6b5033e0..31003a54 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -41,6 +41,8 @@ mod tests_activity; #[cfg(test)] mod tests_album_picker; #[cfg(test)] +mod tests_compare; +#[cfg(test)] mod tests_crop_rotate; #[cfg(test)] mod tests_curve; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index aff0326a..798c1ce7 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -66,6 +66,7 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("view.reference", "Reference View", Some("Shift+R"), "View"), ("photo.setReference", "Set as Reference Photo", None, ""), ("compare.swap", "Swap Compare Photos", None, "View"), + ("compare.set", "Show in Compare", None, ""), ("compare.makeSelect", "Make Candidate the Select", None, "View"), ("view.autoAdvance", "Auto Advance", None, "Photo"), ("view.filmstrip", "Filmstrip", Some("/"), "View"), @@ -385,6 +386,10 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option crate::panels::compare::swap(app), + "compare.set" => { + let Some(id) = p.get("id").and_then(Value::as_u64) else { return Some(Err("compare.set needs `id`".into())) }; + crate::panels::compare::set_photo(app, lightcraft_catalog::PhotoId(id), p.get("side").and_then(Value::as_str)) + } "compare.makeSelect" => crate::panels::compare::make_select(app), "view.autoAdvance" => { app.ui.auto_advance = !app.ui.auto_advance; @@ -1614,7 +1619,7 @@ pub fn ui_enabled(app: &LightcraftApp, id: &str) -> bool { "file.backupLibrary" => app.services.backup_library.is_some(), "app.openLogFolder" => app.services.reveal.is_some() && app.services.log_file.is_some(), "file.restoreLibrary" => app.services.restore_library.is_some(), - "compare.swap" | "compare.makeSelect" => app.ui.view == ViewMode::Compare, + "compare.swap" | "compare.makeSelect" | "compare.set" => app.ui.view == ViewMode::Compare, s if s.starts_with("dialog.merge") || (s.starts_with("merge.") && s.ends_with("Last")) => { app.session.targets(&serde_json::json!({})).len() >= 2 && app.merge.final_task.is_none() } diff --git a/crates/ui-egui/src/panels/compare.rs b/crates/ui-egui/src/panels/compare.rs index 77ee4242..d758a148 100644 --- a/crates/ui-egui/src/panels/compare.rs +++ b/crates/ui-egui/src/panels/compare.rs @@ -73,6 +73,30 @@ pub fn swap(app: &mut LightcraftApp) -> Result { Ok(Value::Null) } +/// Show photo `id` on one side of the pair: `side` "select" or "candidate", else the side that is +/// active (the one the Edit panel and the rating keys act on). A photo already on the other side +/// swaps the two. The photo becomes the active one, so what is edited is always on screen. +pub fn set_photo(app: &mut LightcraftApp, id: PhotoId, side: Option<&str>) -> Result { + if app.session.catalog.photo(id).is_none_or(|p| p.deleted) { + return Err(format!("no photo {}", id.0)); + } + let (sel, cand) = compare_pair(app).ok_or("nothing to compare")?; + let to_select = match side { + Some("select") => true, + Some("candidate") => false, + Some(other) => return Err(format!("unknown side `{other}` (select or candidate)")), + None => app.session.active() == Some(sel), + }; + let (a, b) = match (to_select, id == sel, id == cand) { + (true, _, true) | (false, true, _) => (cand, sel), + (_, true, _) | (_, _, true) => (sel, cand), + (true, false, false) => (id, cand), + (false, false, false) => (sel, id), + }; + select_pair(app, a, b, id); + Ok(json!({"select": a.0, "candidate": b.0})) +} + /// The candidate becomes the select; the next photo becomes the candidate. pub fn make_select(app: &mut LightcraftApp) -> Result { let (_, b) = compare_pair(app).ok_or("nothing to compare")?; diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index c3678ae2..0f5573dc 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -2030,7 +2030,12 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { p.galley(pos2(br.min.x + 17.0, br.center().y - g.size().y / 2.0), g, Color32::WHITE); } } - if resp.clicked() { + if resp.clicked() && app.ui.view == crate::state::ViewMode::Compare { + // Compare: the clicked photo replaces the active side (or swaps with the other one) + if let Err(e) = app.run("compare.set", json!({"id": id.0})) { + app.toast(ui.ctx(), e); + } + } else if resp.clicked() { let m = ui.input(|i| i.modifiers); let mode = if m.command { "toggle" diff --git a/crates/ui-egui/src/tests_compare.rs b/crates/ui-egui/src/tests_compare.rs new file mode 100644 index 00000000..42f5fc02 --- /dev/null +++ b/crates/ui-egui/src/tests_compare.rs @@ -0,0 +1,84 @@ +//! Headless tests of Compare with the filmstrip. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::Zoom; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn demo() -> Headless { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + Headless::new(app, [1400.0, 900.0], 1.0) +} + +/// In Compare, the photo the Edit panel and the rating keys act on is always one of the two on +/// screen: a filmstrip click must not leave the pair showing while a third photo becomes active. +#[test] +fn filmstrip_click_in_compare_keeps_the_active_photo_on_screen() { + let mut h = demo(); + let vis: Vec = h.app.session.visible_cloned().iter().map(|p| p.0).collect(); + let rating = |h: &Headless, id: u64| h.app.session.catalog.photo(lightcraft_catalog::PhotoId(id)).map(|p| p.rating); + for zoom in [Zoom::Fit, Zoom::Percent(100.0)] { + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [vis[0], vis[1]], "active": vis[0]}}), T); + let r = h.request("engine.execute", json!({"command": "view.compare"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.app.ui.zoom = zoom; + h.settle(SETTLE); + let target = vis[4]; + let before = rating(&h, target); + let r = h.request("ui.clickWidget", json!({"id": format!("film:{target}")}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let (sel, cand) = h.app.ui.compare.expect("still comparing"); + let active = h.app.session.selection.active.map(|p| p.0).expect("an active photo"); + // a rating key must not change a photo that isn't shown + h.request("ui.key", json!({"key": "3"}), T); + if target != sel && target != cand { + assert_eq!(rating(&h, target), before, "zoom {zoom:?}: the hidden photo {target} was rated"); + } + assert!(active == sel || active == cand, "zoom {zoom:?}: active photo {active} is not on screen (compare shows {sel} | {cand})"); + } +} + +/// A filmstrip click in Compare puts the photo into the active side: the candidate after +/// entering Compare, the select after clicking the select's pane; a photo already on the other +/// side swaps the two. `compare.set` does the same for agents, with an explicit `side`. +#[test] +fn filmstrip_click_replaces_the_active_side_in_compare() { + let mut h = demo(); + let vis: Vec = h.app.session.visible_cloned().iter().map(|p| p.0).collect(); + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [vis[0], vis[1]], "active": vis[0]}}), T); + let r = h.request("engine.execute", json!({"command": "view.compare"}), T); + let (sel, cand) = (r["result"]["select"].as_u64().unwrap(), r["result"]["candidate"].as_u64().unwrap()); + h.settle(SETTLE); + let click_film = |h: &mut Headless, id: u64| { + let r = h.request("ui.clickWidget", json!({"id": format!("film:{id}")}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + }; + let active = |h: &Headless| h.app.session.selection.active.map(|p| p.0); + let others: Vec = vis.iter().copied().filter(|v| *v != sel && *v != cand).take(3).collect(); + // the candidate is active after entering Compare: it is replaced + click_film(&mut h, others[0]); + assert_eq!(h.app.ui.compare, Some((sel, others[0]))); + assert_eq!(active(&h), Some(others[0])); + // the select's photo, clicked in the filmstrip: the two swap, it stays active + click_film(&mut h, sel); + assert_eq!(h.app.ui.compare, Some((others[0], sel))); + assert_eq!(active(&h), Some(sel)); + // a side chosen explicitly + let r = h.request("engine.execute", json!({"command": "compare.set", "params": {"id": others[1], "side": "select"}}), T); + assert_eq!(r["ok"], true, "{r}"); + assert_eq!(h.app.ui.compare, Some((others[1], sel))); + assert_eq!(active(&h), Some(others[1])); + // the select is active now: the next filmstrip click replaces it + click_film(&mut h, others[2]); + assert_eq!(h.app.ui.compare, Some((others[2], sel))); + let r = h.request("engine.execute", json!({"command": "compare.set", "params": {"id": others[0], "side": "left"}}), T); + assert_eq!(r["ok"], false, "unknown side is an error: {r}"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 09941108..2ffbe891 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -131,7 +131,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-ZOOM | Zoom & pan | P0 | 🟑 | `cmd:view.zoomFit`, `cmd:view.zoom100`, `cmd:view.zoomIn`, `cmd:view.zoomOut`, `cmd:view.zoomToggle`, `cmd:view.clickZoom`, `cmd:view.navigate`, `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/headless.rs` | continuous native pinch / modifier-wheel zoom (Fit–800%, anchored under the pointer), two-finger scroll / touch pan with edge limits; Detail (including tools, Before/After and full-screen preview), Compare and Reference; fractional percentages preserve small gestures; image, picker and targeted-adjustment cursors stay steady between gesture events; keyboard steps 25–800 % (not 6–1600 %); Fill in the bottom bar; click / Z / Space eases to the chosen ratio (1:1 default, 2:1, 3:1, 4:1, 8:1). The loupe's cost follows the canvas, not the zoom (issue #323): the whole frame is rendered at about canvas size (at most the 2560 px preview source level unless a larger Preview size is chosen, and never above the GPU's texture side), and once the view is zoomed well past that (more than 25 %) the window on screen is rendered too, cut from a frame of at most the photo's own pixels, so beyond 100 % the GPU magnifies it (`region::plan`, `RenderRequest::window`, `Session::region_job`, `crates/ui-egui/src/region.rs`); 1:1 and 2:1–8:1 are sharp at any photo size. A slider tick takes 7–12 ms on a 1400 Γ— 900 canvas and 11–25 ms on a 2800 Γ— 1700 one at 100 % and 400 % of a 24 MP photo (M1 Max, GPU, `profile_slider_drag`); about 20–25 ms at 100 % on the big canvas is over a 16 ms frame. A continuous pinch holds the render sizes (and the windows there are) until it is quiet. A fit view on a canvas up to about 3200 px (a Retina Mac) never decodes the original; larger displays get a window at fit, which does. Modes that draw another picture over the loupe (mask overlays, soft proofing) and photos whose spots or Auto Mask strokes read too far for one window (only photos longer than 8192 px) render the whole frame at the size it is drawn, as before windows; while a preset or profile is hovered, or over a picture that is not the frame (an unsupported raw's embedded JPEG), the whole frame stays canvas-sized. Before/After (side by side, stacked, `\\` and both wipes) gets the same windows on both sides, clipped to their pane or side of the line. Windows render on the GPU like whole frames (equivalence-tested against the CPU), in the browser build too (windows no wider than its GPU's texture side, at most 3072 px), and are not made while hovering a look, soft-proofing or showing an overlay, nor over a picture that is not the frame (an unsupported raw's embedded JPEG with another crop). No texture is larger than the GPU allows. Spots and Auto Mask brushes work while what a window reads fits one render (8192 px along an axis, 4096 in the browser): the window grows to hold the spot and its source patch, or the dabs near it, and if that would take more (a clone from far away at 8:1) the loupe keeps the whole-frame render instead, and spots with an automatic source pick it from a fixed-size render of the whole frame (so a window's choice can differ from a whole-frame render's, which picks at its own resolution). The wide stages (dehaze, clarity, highlights/shadows base) and Refine Edges on masks read a window margin capped at 768 px; measured on hard-edged blocks that costs at most a few levels at 8:1. the histogram and Navigator come from the whole-frame render | | 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 | -| LR-VIEW-COMPARE | Compare two photos | P1 | βœ… | `cmd:view.compare`, `cmd:compare.swap`, `cmd:compare.makeSelect`, `crates/ui-egui/src/panels/compare.rs` | select / candidate, synced zoom + pan, arrows move the candidate; no zoom-link toggle | +| LR-VIEW-COMPARE | Compare two photos | P1 | βœ… | `cmd:view.compare`, `cmd:compare.swap`, `cmd:compare.makeSelect`, `cmd:compare.set`, `crates/ui-egui/src/panels/compare.rs`, `crates/ui-egui/src/tests_compare.rs` | select / candidate, synced zoom + pan, arrows move the candidate; a filmstrip click puts the photo into the active side (a photo on the other side swaps), so the active photo is always one of the two shown; no zoom-link toggle | | LR-VIEW-SURVEY | Survey view [Classic] | P2 | βœ… | `cmd:view.survey`, `crates/ui-egui/src/panels/compare.rs` | selection tiled (≀ 48), keys act on the active photo, hover Γ— removes | | LR-VIEW-INFOOVERLAY | Info overlay on the photo | P1 | βœ… | `cmd:view.infoOverlay` | off / file + date + size / exposure + camera; ⌘I cycles (I in full screen; elsewhere I stays the Info panel) | | LR-VIEW-SLIDESHOW | Slideshow | P2 | βœ… | `cmd:view.slideshow` | View β–Έ Slideshow (βŒ₯βŒ˜β†©): the photos in view full screen, every 4 s (`interval`), wrapping; Space pauses, ←/β†’ step, Esc ends | @@ -675,7 +675,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-AUTOIMPORT | Watched-folder import | P2 | βœ… | `cmd:library.autoImport` | see LR-IMP-AUTO | | LRC-LIB-TETHER | Tethered capture | P2 | ⬜ | | | | LRC-LIB-VIEWS | Grid / loupe / compare / survey / people | P1 | 🟑 | `cmd:view.photoGrid`, `cmd:view.squareGrid`, `cmd:view.detail`, `cmd:view.compare`, `cmd:view.survey`, `cmd:view.gridInfo` | grid, square grid, loupe, compare, survey, second window; square-grid captions: file name, exposure or capture date (View β–Έ Grid Info); no people view | -| LRC-LIB-COMPARE | Compare view | P1 | βœ… | `cmd:view.compare` | ⇧C (C is Crop here); swap, make select; zoom always linked | +| LRC-LIB-COMPARE | Compare view | P1 | βœ… | `cmd:view.compare` | ⇧C (C is Crop here); swap, make select; a filmstrip click replaces the active side; zoom always linked | | LRC-LIB-SURVEY | Survey view | P2 | βœ… | `cmd:view.survey` | N | | LRC-LIB-REFVIEW | Reference view | P2 | βœ… | `cmd:view.reference`, `cmd:photo.setReference`, `crates/ui-egui/src/panels/compare.rs` (`show_reference`) | View β–Έ Reference View (⇧R): the reference photo beside the active one (edited with the Edit panel); photo menu β–Έ Set as Reference Photo | | LRC-LIB-CATALOG-PANEL | Catalog sets | P1 | βœ… | `cmd:library.source` (`missing`) | all, recently added, picks, recently deleted, missing photos (when any) |