From 206680fb22044773131d2cf361c6b013fc4f0a49 Mon Sep 17 00:00:00 2001 From: Alex Chambers Date: Thu, 15 Oct 2026 02:24:48 +0200 Subject: [PATCH] LR-VIEW-BEFOREAFTER: animate both panes during click zoom Translate the shared animated image rectangle into the Before pane for whole-frame drawing, cached window tiles and input mapping. Translate the final target separately so render requests stay stable during the motion. Cover intermediate mesh bounds and final geometry in both layouts, including clicks in either pane, return to Fit and cached region tiles. Fixes #1821 Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/detail.rs | 16 +- crates/ui-egui/src/tests_before_after.rs | 182 ++++++++++++++++++++++- docs/parity-checklist.md | 5 +- 3 files changed, 190 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 22820ac6b..3681bb56c 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -426,9 +426,10 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let resp = ui.interact(canvas, egui::Id::new("loupe"), Sense::click_and_drag()); let mut target_rect = fit_rect(main_area, display_aspect, app.ui.zoom, native, ppp, app.ui.pan); let mut img_rect = animated_rect(ui.ctx(), &mut app.ui.zoom_anim, target_rect); + let in_pane = |pane: Rect, image: Rect| image.translate(pane.center() - main_area.center()); let mut navigating = false; for &view_area in &areas { - let view_img = if view_area == main_area { img_rect } else { fit_rect(view_area, display_aspect, app.ui.zoom, native, ppp, app.ui.pan) }; + let view_img = in_pane(view_area, img_rect); if navigate_gesture(app, ui, &resp, view_area, view_img, native) { navigating = true; target_rect = fit_rect(main_area, display_aspect, app.ui.zoom, native, ppp, app.ui.pan); @@ -436,6 +437,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { break; } } + let before_img = in_pane(areas[0], img_rect); + let before_target = in_pane(areas[0], target_rect); app.image_rect = Some(img_rect); // request renders: the loupe at display resolution (drafts during drags) let interacting = app.session.interaction.is_some(); @@ -551,11 +554,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { // (slot, before side?, rect to draw in, rect it is requested for, area whose pixels show) let mut window_views: Vec = Vec::new(); if window_ctx.frame_edge.is_some() { - let before_rect = |r: Rect, area: Rect| WindowView { slot: Slot::RegionBefore, before: true, img: r, target: r, visible: area }; let after = WindowView { slot: Slot::Region, before: false, img: img_rect, target: target_rect, visible: main_area }; if split { - let br = fit_rect(areas[0], aspect, app.ui.zoom, native, ppp, app.ui.pan); - window_views.push(before_rect(br, areas[0])); + window_views.push(WindowView { slot: Slot::RegionBefore, before: true, img: before_img, target: before_target, visible: areas[0] }); window_views.push(after); } else if show_before { window_views.push(WindowView { slot: Slot::RegionBefore, before: true, img: img_rect, target: target_rect, visible: canvas }); @@ -612,7 +613,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { if split { // each view stays in its pane: zoomed in, a photo reaches past it (and would cover the // other view's margins with its whole-frame render, magnified and blurred) - let br = fit_rect(areas[0], aspect, app.ui.zoom, native, ppp, app.ui.pan); + let br = before_img; draw(Slot::Before, br, areas[0]); shown = draw(Slot::Main, img_rect, main_area); // the labels under each photo, or under its pane when the photo reaches past it @@ -753,10 +754,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { // A click-zoom there centres both views on the point clicked (#1742). let pointer = resp.interact_pointer_pos().or(resp.hover_pos()); let (map, img, pane) = match (split, areas.first(), pointer) { - (true, Some(before), Some(q)) if before.contains(q) => { - let br = fit_rect(*before, aspect, app.ui.zoom, native, ppp, app.ui.pan); - (CanvasMap::new(&frame, br), br, *before) - } + (true, Some(before), Some(q)) if before.contains(q) => (CanvasMap::new(&frame, before_img), before_img, *before), _ => (map, img_rect, main_area), }; general_interaction(app, ui, &resp, &map, img, pane, native, aspect) diff --git a/crates/ui-egui/src/tests_before_after.rs b/crates/ui-egui/src/tests_before_after.rs index 8a89098fe..02ee879a1 100644 --- a/crates/ui-egui/src/tests_before_after.rs +++ b/crates/ui-egui/src/tests_before_after.rs @@ -1,10 +1,11 @@ -//! Headless tests of clicks in the two-view Before / After layouts. +//! Headless tests of input and zoom animation in the two-view Before / After layouts. use std::time::Duration; use serde_json::json; -use crate::headless::Headless; +use crate::headless::{Headless, RenderGate}; +use crate::render::Slot; use crate::state::{BeforeAfter, ViewMode, Zoom}; use crate::{LightcraftApp, Services}; @@ -109,6 +110,183 @@ fn a_click_in_the_before_view_zooms_in_on_the_point_clicked() { } } +/// A texture mesh in its pane, excluding the Navigator's copy of the Main texture. +fn pane_mesh(h: &Headless, slot: Slot, pane: egui::Rect) -> egui::Rect { + let tex = &h.app.renderer.textures[&slot]; + let meshes: Vec<_> = h + .view + .shapes + .iter() + .filter_map(|c| match &c.shape { + egui::epaint::Shape::Mesh(m) if m.texture_id == tex.tex.id() && c.clip_rect == pane => Some(m.calc_bounds()), + _ => None, + }) + .collect(); + assert_eq!(meshes.len(), 1, "{slot:?}: one image clipped to {pane:?}, got {meshes:?}"); + meshes[0] +} + +fn assert_rect(actual: egui::Rect, expected: egui::Rect) { + assert!( + (actual.min - expected.min).abs().max_elem() < 1.0 && (actual.max - expected.max).abs().max_elem() < 1.0, + "image bounds {actual:?}, expected {expected:?}" + ); +} + +/// Start with both textures ready, then hold replacement renders so worker timing cannot hide +/// a frame of the transition or replace a cached tile under the assertions. +fn animation_view(mode: BeforeAfter, zoom: Zoom) -> (Headless, [egui::Rect; 2], std::rc::Rc>) { + 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 r = h.request("ui.set", json!({"view": "detail", "right": "none", "filmstrip": false}), T); + assert_eq!(r["ok"], true, "{r}"); + h.app.ui.before_after = mode; + h.app.ui.zoom = zoom; + h.app.ui.pan = (0.5, 0.5); + h.app.ui.click_zoom = 300; + assert!(h.settle(SETTLE), "baseline renders settled"); + let before_id = h.app.renderer.textures[&Slot::Before].tex.id(); + let before = h + .view + .shapes + .iter() + .find_map(|c| match &c.shape { + egui::epaint::Shape::Mesh(m) if m.texture_id == before_id => Some(c.clip_rect), + _ => None, + }) + .expect("Before image on screen"); + let canvas = h.app.canvas_rect.expect("canvas"); + let shift = 2.0 * (canvas.center() - before.center()); + let after = before.translate(shift); + assert!(canvas.contains_rect(before) && canvas.contains_rect(after), "both clips stay in the canvas"); + assert!(!before.intersects(after), "pane clips do not overlap"); + let open = std::rc::Rc::new(std::cell::Cell::new(false)); + h.app.renderer.set_offload(Box::new(RenderGate::new(open.clone()))); + (h, [before, after], open) +} + +fn queue_click(h: &mut Headless, pos: egui::Pos2) { + h.app.synthetic.extend([ + egui::Event::PointerMoved(pos), + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }, + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }, + ]); + while !h.app.synthetic.is_empty() { + h.step(); + } + assert!(h.app.ui.zoom_anim, "the click started an animation"); +} + +fn check_click_animation(mode: BeforeAfter) { + let (mut h, [before, after], open) = animation_view(mode, Zoom::Fit); + let shift = before.center() - after.center(); + let fit = h.app.image_rect.expect("Fit image"); + let id = h.app.session.active().expect("photo"); + let photo = h.app.session.catalog.photo(id).expect("photo"); + let native = [photo.width as usize, photo.height as usize]; + let aspect = native[0] as f32 / native[1] as f32; + // Exercise clicks in both panes, including their return to Fit, without settling the motion. + for before_click in [false, true] { + for zoom_in in [true, false] { + let pos = if zoom_in { fit.center() + egui::vec2(fit.width() * 0.3, fit.height() * -0.25) } else { after.center() }; + queue_click(&mut h, if before_click { pos + shift } else { pos }); + assert_eq!(h.app.ui.zoom, if zoom_in { Zoom::Percent(300.0) } else { Zoom::Fit }); + if zoom_in { + let pan = h.app.ui.pan; + assert!((pan.0 - 0.8).abs() < 0.02 && (pan.1 - 0.25).abs() < 0.02, "{mode:?}: clicked point {pan:?}"); + } + let target = crate::panels::detail::fit_rect(after, aspect, h.app.ui.zoom, native, 1.0, h.app.ui.pan); + let mut intermediate = 0; + let mut keys = std::collections::HashSet::new(); + for _ in 0..40 { + h.step(); + keys.insert((h.app.renderer.wanted(Slot::Before), h.app.renderer.wanted(Slot::Main))); + let img = h.app.image_rect.expect("animated image"); + let between = img.width() > fit.width() + 1.0 && img.width() < native[0] as f32 * 3.0 - 1.0; + if between { + intermediate += 1; + } + if between || !h.app.ui.zoom_anim { + let a = pane_mesh(&h, Slot::Main, after); + let b = pane_mesh(&h, Slot::Before, before); + assert_rect(b, a.translate(shift)); + } + } + assert!(intermediate > 0, "{mode:?}: inspected intermediate animation frames"); + assert!(!h.app.ui.zoom_anim, "animation finished"); + assert_rect(h.app.image_rect.expect("settled image"), target); + assert_rect(pane_mesh(&h, Slot::Main, after), crate::panels::detail::fit_texture_rect(target, h.app.renderer.textures[&Slot::Main].size)); + assert_eq!(keys.len(), 1, "both panes request stable final-size renders throughout the animation: {keys:?}"); + } + } + open.set(true); + assert!(h.settle(SETTLE)); +} + +#[test] +fn both_before_after_panes_use_the_animated_rect_side_by_side() { + check_click_animation(BeforeAfter::SideBySide); +} + +#[test] +fn both_before_after_panes_use_the_animated_rect_top_bottom() { + check_click_animation(BeforeAfter::TopBottom); +} + +fn check_region_animation(mode: BeforeAfter) { + let (mut h, [before, after], open) = animation_view(mode, Zoom::Percent(200.0)); + let shift = before.center() - after.center(); + let start = h.app.image_rect.expect("200 percent image"); + let tiles = [(Slot::RegionBefore, before, true), (Slot::Region, after, false)].map(|(slot, pane, is_before)| { + let tex = &h.app.renderer.textures[&slot]; + let tile = h.app.region_tiles[&(is_before, tex.key)]; + pane_mesh(&h, slot, pane); + (slot, pane, tile, tex.key) + }); + h.app.ui.zoom = Zoom::Percent(400.0); + h.app.ui.zoom_anim = true; + let mut intermediate = 0; + let mut keys = std::collections::HashSet::new(); + for _ in 0..40 { + h.step(); + keys.insert((h.app.renderer.wanted(Slot::RegionBefore), h.app.renderer.wanted(Slot::Region))); + let img = h.app.image_rect.expect("animated image"); + if img.width() > start.width() + 1.0 && img.width() < start.width() * 2.0 - 1.0 { + intermediate += 1; + } + if img.width() <= start.width() + 1.0 { + continue; + } + for (slot, pane, tile, key) in tiles { + assert_eq!(h.app.renderer.textures[&slot].key, key, "the cached tile stays on screen while renders wait"); + let full = if tile.before { img.translate(shift) } else { img }; + let (fw, fh) = (tile.full.0 as f32, tile.full.1 as f32); + let w = tile.window; + let expected = egui::Rect::from_min_size( + full.min + egui::vec2(w.x as f32 / fw * full.width(), w.y as f32 / fh * full.height()), + egui::vec2(w.w as f32 / fw * full.width(), w.h as f32 / fh * full.height()), + ); + assert_rect(pane_mesh(&h, slot, pane), expected); + } + } + assert!(intermediate > 0, "{mode:?}: inspected intermediate region frames"); + assert!(!h.app.ui.zoom_anim); + assert!((h.app.image_rect.expect("settled image").width() - start.width() * 2.0).abs() < 1.0); + assert_eq!(keys.len(), 1, "region requests use the final target throughout the animation: {keys:?}"); + open.set(true); + assert!(h.settle(SETTLE)); +} + +#[test] +fn before_after_region_tiles_use_the_animated_rect_side_by_side() { + check_region_animation(BeforeAfter::SideBySide); +} + +#[test] +fn before_after_region_tiles_use_the_animated_rect_top_bottom() { + check_region_animation(BeforeAfter::TopBottom); +} + /// Mean (r, g, b) of the 41 × 41 screen pixels around `at`. fn mean_around(shot: &egui::ColorImage, at: egui::Pos2) -> [f32; 3] { let mut sum = [0.0f32; 3]; diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 7bb187811..97bdc4b9c 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (integration batch 35: KEYC-SELECTONLYACTIVE, Select Only Active Photo ⌘⇧D; LR-BEHAV-AUTOSAVE, damaged snapshots repaired on open; LR-EDIT-OPTICS-PROFILE, Nikon Z 6II / Z 8 / Z f in-file distortion; LRC-LIB-CATALOG-IMPORT, WAL-only catalogs open; LR-EXP-DIM, enlargements stop at 512 MP; LR-IMP-CAMERA-COVERAGE, Canon EOS RP CR3 verified; model folder retry) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-15 · **Change:** patch (LR-VIEW-BEFOREAFTER click-zoom animation, #1821) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -131,7 +131,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. Compare and Reference get the same windows, one per pane (`Slot::CompareRegion`, `panels/compare.rs`), over each pane's whole-frame render (at most 2560 px); while either view is open two originals stay decoded (`MediaCache::set_full_capacity`), so one pane's window does not evict the other's source; a pinch, or a slider drag on the active photo in Reference, keeps the windows there are until it ends. Still 🟡: mask overlays and soft proofing have no window (a full-size whole-frame render at 1:1) | | 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; 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`). Zoomed in, the side by side and stacked views each stay in their pane, with their labels under it | +| 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`). Zoomed in, the side by side and stacked views each stay in their pane, with their labels under it. Click zoom animates both panes together, including cached window tiles; input follows the displayed image and render requests use the final size (#1821) | | 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; a ⌘/Ctrl- or Shift-click on the filmstrip changes the selection as in the grid while the two photos shown stay selected and the active photo is unchanged (#187); 1:1 to 8:1 sharp on both sides (zoom windows, see LR-VIEW-ZOOM); no zoom-link toggle; each pane says “Rendering…” (widget `compare:rendering:`) or “This photo can't be opened” (`compare:renderFailed:`) as the loupe does | | 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) | @@ -790,6 +790,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-15 | patch | LR-VIEW-BEFOREAFTER: side by side and stacked panes share click-zoom animation for whole-frame images, window tiles and input; render requests keep the final target (#1821); status unchanged | | 2026-10-13 | patch | LR-VIEW-COMPARE, LRC-LIB-COMPARE: ⌘/Ctrl- and Shift-clicks on the filmstrip in Compare change the selection as in the grid; the two photos shown stay selected and the active photo is unchanged (#187); statuses unchanged | | 2026-10-13 | minor | KEYC-SELECTONLYACTIVE: Edit → Select Only Active Photo (⌘⇧D) lands as a new row; status 🟡 (headless-tested, native macOS menu path unverified) | | 2026-10-13 | patch | LRC-LIB-CATALOG-IMPORT: a catalog whose tables are all still in its `-wal` file (main file schema format 0) opens instead of failing as an unsupported schema format; status unchanged |