diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1c84db43..90cff182 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -207,7 +207,10 @@ pub struct LightcraftApp { pub region_before_view: Option, /// The loupe's render sizes while a pinch or two-finger scroll runs. pub(crate) size_hold: region::SizeHold, - pub(crate) region_tiles: std::collections::HashMap, + /// What the loupe drew last frame, in order (tests check layering). + #[cfg(test)] + pub(crate) draw_order: Vec<&'static str>, + pub(crate) region_tiles: std::collections::HashMap<(bool, u64), region::RegionView>, /// Photo Merge dialog previews and background merges. pub merge: merge::MergeState, /// An import in progress (the import review dialog's batches). @@ -278,6 +281,8 @@ impl LightcraftApp { region_view: None, region_before_view: None, size_hold: Default::default(), + #[cfg(test)] + draw_order: Vec::new(), region_tiles: Default::default(), merge: merge::MergeState::default(), import: None, diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 3d03899c..bf33336c 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -267,8 +267,8 @@ fn request_window(app: &mut LightcraftApp, c: &WindowCtx, v: &WindowView) -> Opt let look = job.settings.hash64(); let view = crate::region::RegionView { photo: c.id, before: v.before, key: job.key, full: (fw, fh), window: win, settings: look }; // windows of other photos, looks and zooms of this side can't be drawn any more - app.region_tiles.retain(|_, t| t.before != v.before || t.is_current(c.id, (fw, fh), look, c.interacting)); - app.region_tiles.insert(job.key, view); + app.region_tiles.retain(|(before, _), t| *before != v.before || t.is_current(c.id, (fw, fh), look, c.interacting)); + app.region_tiles.insert((v.before, job.key), view); app.renderer.request(v.slot, job, 99); Some(view) } @@ -277,9 +277,7 @@ fn request_window(app: &mut LightcraftApp, c: &WindowCtx, v: &WindowView) -> Opt /// drag, an earlier draft of it). fn draw_window(p: &egui::Painter, app: &LightcraftApp, c: &WindowCtx, v: &WindowView, wanted: &crate::region::RegionView) { let Some(tex) = app.renderer.textures.get(&v.slot).filter(|t| t.photo == c.id) else { return }; - let Some(tile) = - app.region_tiles.get(&tex.key).filter(|t| t.before == v.before && t.is_current(c.id, wanted.full, wanted.settings, c.interacting)) - else { + let Some(tile) = app.region_tiles.get(&(v.before, tex.key)).filter(|t| t.is_current(c.id, wanted.full, wanted.settings, c.interacting)) else { return; }; let (fw, fh) = (tile.full.0 as f32, tile.full.1 as f32); @@ -487,7 +485,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { app.region_view = shown; } if shown.is_none() { - app.region_tiles.retain(|_, v| v.before != before); + app.region_tiles.retain(|(b, _), _| *b != before); app.renderer.release(slot); } } @@ -555,35 +553,43 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { } } } - // the window renders, drawn over the whole-frame ones where they belong in the frame (the - // Before side of a wipe only on its side of the line) - for (v, view) in &windows { - let clip = match (app.ui.before_after, v.before) { - (BeforeAfter::Split, true) => Rect::from_min_max(canvas.min, pos2(img_rect.center().x, canvas.bottom())), - (BeforeAfter::SplitTopBottom, true) => Rect::from_min_max(canvas.min, pos2(canvas.right(), img_rect.center().y)), - (BeforeAfter::Split, false) => Rect::from_min_max(pos2(img_rect.center().x, canvas.top()), canvas.max), - (BeforeAfter::SplitTopBottom, false) => Rect::from_min_max(pos2(canvas.left(), img_rect.center().y), canvas.max), - _ => canvas, - }; - draw_window(&p.with_clip_rect(clip.intersect(canvas)), app, &window_ctx, v, view); - } app.loupe_shown = Some((id, shown)); + #[cfg(test)] + app.draw_order.clear(); + // a wipe shows the Before whole-frame render on its side of the line… + if app.ui.before_after == BeforeAfter::Split + && let Some(tex) = app.renderer.textures.get(&Slot::Before).filter(|t| t.photo == id) + { + let before_rect = fit_texture_rect(img_rect, tex.size); + let left = Rect::from_min_max(before_rect.min, pos2(before_rect.center().x, before_rect.bottom())); + p.image(tex.tex.id(), left, Rect::from_min_max(pos2(0.0, 0.0), pos2(0.5, 1.0)), Color32::WHITE); + } + if app.ui.before_after == BeforeAfter::SplitTopBottom + && let Some(tex) = app.renderer.textures.get(&Slot::Before).filter(|t| t.photo == id) + { + let before_rect = fit_texture_rect(img_rect, tex.size); + let top = Rect::from_min_max(before_rect.min, pos2(before_rect.right(), before_rect.center().y)); + p.image(tex.tex.id(), top, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 0.5)), Color32::WHITE); + } + #[cfg(test)] + if matches!(app.ui.before_after, BeforeAfter::Split | BeforeAfter::SplitTopBottom) { + app.draw_order.push("wipe stand-in"); + } + // …and the window renders go over the whole-frame ones, all of them, where they belong in the + // frame (clipped to their pane, or their side of the line) + for (v, view) in &windows { + let clip = crate::region::window_clip(app.ui.before_after, v.before, v.visible, canvas, img_rect); + draw_window(&p.with_clip_rect(clip), app, &window_ctx, v, view); + #[cfg(test)] + app.draw_order.push(if v.before { "window RegionBefore" } else { "window Region" }); + } + // the line of a wipe over both if app.ui.before_after == BeforeAfter::Split { let mid = img_rect.center().x; - if let Some(tex) = app.renderer.textures.get(&Slot::Before).filter(|t| t.photo == id) { - let before_rect = fit_texture_rect(img_rect, tex.size); - let left = Rect::from_min_max(before_rect.min, pos2(before_rect.center().x, before_rect.bottom())); - p.image(tex.tex.id(), left, Rect::from_min_max(pos2(0.0, 0.0), pos2(0.5, 1.0)), Color32::WHITE); - } p.line_segment([pos2(mid, img_rect.top()), pos2(mid, img_rect.bottom())], Stroke::new(1.5, Color32::WHITE)); } if app.ui.before_after == BeforeAfter::SplitTopBottom { let mid = img_rect.center().y; - if let Some(tex) = app.renderer.textures.get(&Slot::Before).filter(|t| t.photo == id) { - let before_rect = fit_texture_rect(img_rect, tex.size); - let top = Rect::from_min_max(before_rect.min, pos2(before_rect.right(), before_rect.center().y)); - p.image(tex.tex.id(), top, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 0.5)), Color32::WHITE); - } p.line_segment([pos2(img_rect.left(), mid), pos2(img_rect.right(), mid)], Stroke::new(1.5, Color32::WHITE)); } if app.ui.show_clipping && !show_before && !fullscreen { diff --git a/crates/ui-egui/src/region.rs b/crates/ui-egui/src/region.rs index 41bd79e8..a3fb86d5 100644 --- a/crates/ui-egui/src/region.rs +++ b/crates/ui-egui/src/region.rs @@ -142,6 +142,23 @@ impl SizeHold { } } +/// Where a window of one side of a Before/After view may be drawn: its own pane in side by side +/// and stacked views (a window carries margin beyond what its pane shows, which would otherwise +/// spill into the neighbour), its side of the line in a wipe, else the canvas. +pub fn window_clip(mode: crate::state::BeforeAfter, before_side: bool, pane: egui::Rect, canvas: egui::Rect, img: egui::Rect) -> egui::Rect { + use crate::state::BeforeAfter; + let (min, max) = (canvas.min, canvas.max); + let clip = match (mode, before_side) { + (BeforeAfter::Split, true) => egui::Rect::from_min_max(min, egui::pos2(img.center().x, max.y)), + (BeforeAfter::SplitTopBottom, true) => egui::Rect::from_min_max(min, egui::pos2(max.x, img.center().y)), + (BeforeAfter::Split, false) => egui::Rect::from_min_max(egui::pos2(img.center().x, min.y), max), + (BeforeAfter::SplitTopBottom, false) => egui::Rect::from_min_max(egui::pos2(min.x, img.center().y), max), + (BeforeAfter::SideBySide | BeforeAfter::TopBottom, _) => pane, + _ => canvas, + }; + clip.intersect(canvas) +} + /// Whether the picture on screen has the frame's aspect (to the one pixel a texture's whole-pixel /// size is off by): a window is a part of the frame, so it can only be placed over a picture that /// is the frame (an unsupported raw's embedded JPEG may be cropped differently). @@ -313,6 +330,27 @@ mod tests { assert_eq!(hold.apply(PhotoId(2), 0.1, true, zoomed), zoomed); } + // Given side by side, a window is drawn in its pane only; given a wipe, on its side of the line + #[test] + fn a_window_is_clipped_to_its_pane_or_its_side_of_the_wipe() { + use crate::state::BeforeAfter::*; + let canvas = egui::Rect::from_min_max(egui::pos2(0.0, 0.0), egui::pos2(1000.0, 600.0)); + let (left, right) = ( + egui::Rect::from_min_max(egui::pos2(24.0, 24.0), egui::pos2(494.0, 576.0)), + egui::Rect::from_min_max(egui::pos2(506.0, 24.0), egui::pos2(976.0, 576.0)), + ); + assert_eq!(window_clip(SideBySide, true, left, canvas, left), left); + assert_eq!(window_clip(SideBySide, false, right, canvas, right), right); + let img = egui::Rect::from_min_max(egui::pos2(100.0, 50.0), egui::pos2(900.0, 550.0)); + let before = window_clip(Split, true, canvas, canvas, img); + let after = window_clip(Split, false, canvas, canvas, img); + assert_eq!((before.max.x, after.min.x), (500.0, 500.0), "the line is the image's middle"); + assert_eq!(window_clip(SplitTopBottom, true, canvas, canvas, img).max.y, 300.0); + assert_eq!(window_clip(SplitTopBottom, false, canvas, canvas, img).min.y, 300.0); + assert_eq!(window_clip(Off, false, canvas, canvas, img), canvas); + assert_eq!(window_clip(Original, true, canvas, canvas, img), canvas); + } + // Hostile numbers give a plan, never a panic #[test] fn hostile_sizes_give_a_usable_plan() { diff --git a/crates/ui-egui/src/tests_preview_limit.rs b/crates/ui-egui/src/tests_preview_limit.rs index 08c43dec..6efbba35 100644 --- a/crates/ui-egui/src/tests_preview_limit.rs +++ b/crates/ui-egui/src/tests_preview_limit.rs @@ -526,4 +526,33 @@ mod in_the_loupe { assert!(h.app.renderer.textures.contains_key(&Slot::Region), "a window arrived"); assert!(h.app.session.memory_report().full_source.bytes > 0, "the original stayed in the cache while the drag went on"); } + + // Given a wipe at 1:1, the Before window is drawn after (over) the low-resolution Before + // stand-in on its side of the line, never under it + #[test] + fn a_wipe_draws_the_before_window_over_its_stand_in() { + for mode in [crate::state::BeforeAfter::Split, crate::state::BeforeAfter::SplitTopBottom] { + let (mut h, _) = detail(); + h.app.ui.before_after = mode; + h.request("engine.execute", json!({"command": "view.zoom100"}), T); + h.settle(SETTLE); + h.step(); + let order = &h.app.draw_order; + let pos = |what: &str| order.iter().position(|d| *d == what); + let (stand_in, before, after) = (pos("wipe stand-in"), pos("window RegionBefore"), pos("window Region")); + assert!(stand_in.is_some() && before.is_some() && after.is_some(), "{mode:?}: {order:?}"); + assert!(before > stand_in, "{mode:?}: the Before window is under its stand-in: {order:?}"); + } + } + + // Given a Before and an After with the same look (an unedited photo), both windows are kept + // apart: the job key is the same, and one tile must not replace the other + #[test] + fn identical_before_and_after_keep_a_tile_each() { + let (mut h, _) = detail(); + h.app.ui.before_after = crate::state::BeforeAfter::SideBySide; + h.request("engine.execute", json!({"command": "view.zoom100"}), T); + h.settle(SETTLE); + assert_eq!(h.app.region_tiles.len(), 2, "{:?}", h.app.region_tiles.keys().collect::>()); + } }