mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-02 14:39:41 -06:00
LR-VIEW-BEFOREAFTER: click tools work in the Before view of side by side / top-bottom
In the side-by-side and top/bottom Before/After layouts a click in the Before view was mapped to image coordinates through the After view's rectangle, so the White Balance Selector sampled the wrong point (at Fit, the photo's edge). The click tools in general_interaction now use the Before view's image rectangle when the pointer is in the Before view's pane; the pane decides, since zoomed in an image reaches past its pane. New headless test tests_before_after.rs clicks the same image point in the After and Before views (side by side, top/bottom; Fit and 100 %) and expects the same white balance. Without the fix: Before 8202 K / +116, After 2710 K / +33. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
964d9338ba
commit
40d7b86ecb
@@ -41,6 +41,8 @@ mod tests_activity;
|
||||
#[cfg(test)]
|
||||
mod tests_album_picker;
|
||||
#[cfg(test)]
|
||||
mod tests_before_after;
|
||||
#[cfg(test)]
|
||||
mod tests_crop_rotate;
|
||||
#[cfg(test)]
|
||||
mod tests_curve;
|
||||
|
||||
@@ -685,7 +685,19 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
RightPanel::Masking => mask_overlay(app, ui, &resp, &map, &d),
|
||||
RightPanel::Remove => remove_overlay(app, ui, &resp, &map, &d),
|
||||
RightPanel::RedEye => eye_overlay(app, ui, &resp, &map, &d),
|
||||
_ => general_interaction(app, ui, &resp, &map, img_rect, main_area, native, aspect),
|
||||
_ => {
|
||||
// side by side / stacked: Before shows the same frame, so a click in its view (white
|
||||
// balance, colour samples, targeted adjustment) means the same point of the photo.
|
||||
// Zoomed in, a view's image reaches past its pane: what counts is the pane clicked.
|
||||
let pointer = resp.interact_pointer_pos().or(resp.hover_pos());
|
||||
let map = match (split, areas.first(), pointer) {
|
||||
(true, Some(before), Some(q)) if before.contains(q) => {
|
||||
CanvasMap::new(&frame, fit_rect(*before, aspect, app.ui.zoom, native, ppp, app.ui.pan))
|
||||
}
|
||||
_ => map,
|
||||
};
|
||||
general_interaction(app, ui, &resp, &map, img_rect, main_area, native, aspect)
|
||||
}
|
||||
}
|
||||
}
|
||||
// drawn and hit-tested above the loupe and its tools: clicks on it pan
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
//! Headless tests of clicks in the two-view Before / After layouts.
|
||||
|
||||
use std::time::Duration;
|
||||
|
||||
use serde_json::json;
|
||||
|
||||
use crate::headless::Headless;
|
||||
use crate::state::{BeforeAfter, ViewMode, Zoom};
|
||||
use crate::{LightcraftApp, Services};
|
||||
|
||||
const T: Duration = Duration::from_secs(20);
|
||||
const SETTLE: Duration = Duration::from_secs(120);
|
||||
|
||||
/// Click at a screen position with real egui pointer events (as `ui.pointer` sends them).
|
||||
fn click_at(h: &mut Headless, pos: egui::Pos2) {
|
||||
h.app.synthetic.push(egui::Event::PointerMoved(pos));
|
||||
h.app.synthetic.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() });
|
||||
h.app.synthetic.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() });
|
||||
h.settle(SETTLE);
|
||||
}
|
||||
|
||||
/// The white-balance selector picks the same point of the photo whichever view of a side-by-side
|
||||
/// or top/bottom Before / After layout is clicked, at Fit and zoomed in. Clicks on the Before
|
||||
/// view used to be mapped through the After view's rectangle, so they sampled the photo's edge.
|
||||
#[test]
|
||||
fn wb_selector_picks_the_same_point_in_the_before_view() {
|
||||
let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() });
|
||||
let mut h = Headless::new(app, [1400.0, 900.0], 1.0);
|
||||
let id = h.app.session.visible_cloned()[2];
|
||||
h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [id.0], "active": id.0}}), T);
|
||||
h.app.ui.view = ViewMode::Detail;
|
||||
let wb = |h: &Headless| h.app.session.catalog.photo(id).map(|p| (p.develop.wb.temp, p.develop.wb.tint));
|
||||
// lay the view out and arm the selector; returns the After view's image rect and the canvas
|
||||
let arm = |h: &mut Headless, mode: BeforeAfter, zoom: Zoom| {
|
||||
h.request("engine.execute", json!({"command": "develop.reset"}), T);
|
||||
h.app.ui.before_after = mode;
|
||||
h.app.ui.zoom = zoom;
|
||||
h.app.ui.pan = Default::default();
|
||||
h.settle(SETTLE);
|
||||
let r = h.request("engine.execute", json!({"command": "tool.wbPicker"}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.settle(SETTLE);
|
||||
(h.app.image_rect.expect("After view on screen"), h.app.canvas_rect.expect("canvas"))
|
||||
};
|
||||
for mode in [BeforeAfter::SideBySide, BeforeAfter::TopBottom] {
|
||||
// the Before view is the After view moved by one pane: measured at Fit, where both are
|
||||
// centred in their panes and so mirror each other about the canvas centre
|
||||
let (img, canvas) = arm(&mut h, mode, Zoom::Fit);
|
||||
let shift = match mode {
|
||||
BeforeAfter::SideBySide => egui::vec2(img.left() + img.right() - 2.0 * canvas.center().x, 0.0),
|
||||
_ => egui::vec2(0.0, img.top() + img.bottom() - 2.0 * canvas.center().y),
|
||||
};
|
||||
assert!(shift.length() > 100.0, "{mode:?}: two views, {shift:?} apart");
|
||||
h.request("engine.execute", json!({"command": "tool.wbPicker"}), T);
|
||||
for zoom in [Zoom::Fit, Zoom::Percent(100.0)] {
|
||||
let mut picked = Vec::new();
|
||||
for before_view in [false, true] {
|
||||
let (img, canvas) = arm(&mut h, mode, zoom);
|
||||
// the middle of the After pane (on the photo at Fit and zoomed in), or the same
|
||||
// point of the photo in the Before view
|
||||
let after = canvas.center() + shift / 2.0;
|
||||
assert!(img.contains(after), "{mode:?} {zoom:?}: {after:?} on the photo {img:?}");
|
||||
click_at(&mut h, if before_view { after - shift } else { after });
|
||||
assert_eq!(h.app.ui.tool, "", "{mode:?} {zoom:?}: the selector is done after one click");
|
||||
picked.push(wb(&h));
|
||||
}
|
||||
assert_ne!(picked[0], Some((6500.0, 0.0)), "{mode:?} {zoom:?}: the click picked a white balance");
|
||||
assert_eq!(picked[1], picked[0], "{mode:?} {zoom:?}: the Before view picks the After view's point");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -130,7 +130,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
|
||||
| LR-VIEW-FILMSTRIP | Filmstrip | P0 | ✅ | `cmd:view.filmstrip`, `crates/ui-egui/src/panels/detail.rs` | the photo context menu on right-click; 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; every selected photo is highlighted, the active one framed in white, as in the grid |
|
||||
| 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-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; in side by side and top / bottom, clicks with the white-balance selector, colour samplers and targeted adjustment work in the Before view too (`crates/ui-egui/src/tests_before_after.rs`) |
|
||||
| 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-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) |
|
||||
|
||||
Reference in New Issue
Block a user