From e7dbdeb21f1b5e570c059c1e5a524d77f6b93fef Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 22 Aug 2026 08:53:02 +0200 Subject: [PATCH] Keep the overlay on the photograph when the view moves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay is a source-space picture; the canvas beside it shows whatever the crop, the zoom and the pan selected out of that same space. Drawn whole it stayed frame-sized while the photograph moved underneath, so zooming in left a map of the whole picture stretched over a detail of it. It now reports the visible rectangle as a clip, which the compositor applies for nothing. Resampling on the CPU instead would mean rebuilding a megapixel image on every frame of a drag, and putting it on the GPU would add a second texture to keep in step with the view. Pushed from the render path rather than the panel's sync: a pan changes no mask and no row, so nothing else needs to run, and rebuilding the row models on every frame of a drag would be waste. Straightening is handled by rotating the image. A quarter turn or a flip permutes the axes and a clip rectangle cannot say that — noted where it happens rather than left to be discovered. The proper fix is to run the overlay through the same shader prologue the photograph goes through, which is the right answer and a larger one than this. Four tests, and the one that matters asserts the clip *narrows* when zoomed — which is precisely what it failed to do. --- ui/dr-ui/src/develop.rs | 161 +++++++++++++++++++++++++++++++++++++-- ui/dr-ui/src/lib.rs | 7 ++ ui/dr-ui/src/masks_ui.rs | 19 +++++ ui/dr-ui/ui/app.slint | 19 +++++ 4 files changed, 201 insertions(+), 5 deletions(-) diff --git a/ui/dr-ui/src/develop.rs b/ui/dr-ui/src/develop.rs index 7007883..02ad81f 100644 --- a/ui/dr-ui/src/develop.rs +++ b/ui/dr-ui/src/develop.rs @@ -858,14 +858,49 @@ impl DevelopSession { self.show_overlay = on; } + /// The part of the overlay the view is currently showing, in overlay + /// pixels: `(x, y, width, height)`. + /// + /// The overlay is a **source-space** picture, and the canvas beside it + /// shows whatever the crop, the zoom and the pan selected out of that same + /// space. Drawn whole, it stays the size of the frame while the photograph + /// moves underneath — which is exactly the fault this exists to fix. + /// + /// Reported as a clip rectangle rather than resampled here: the compositor + /// crops and scales a texture for nothing, where doing it on the CPU would + /// mean rebuilding a megapixel image on every frame of a drag. + /// + /// **Known gap.** A quarter turn or a flip permutes the axes, and a clip + /// rectangle cannot express that — the straightening angle is handled + /// alongside this, but a quarter-turned frame shows the overlay unturned. + /// Fixing it properly means running the overlay through the same shader + /// prologue the image goes through, which is the right answer and a larger + /// one than this. + pub fn overlay_clip(&self) -> (i32, i32, i32, i32) { + let Some(seg) = self.segmentation.as_ref() else { + return (0, 0, 0, 0); + }; + let (w, h) = seg.proxy_size(); + let rect = self.graph.framing().visible_rect(); + + // Rounded outward, so half a pixel of rounding never shows as a strip + // of missing overlay along an edge. + let x = (rect.x * w as f32).floor().max(0.0) as i32; + let y = (rect.y * h as f32).floor().max(0.0) as i32; + let right = ((rect.x + rect.width) * w as f32).ceil().min(w as f32) as i32; + let bottom = ((rect.y + rect.height) * h as f32).ceil().min(h as f32) as i32; + + (x, y, (right - x).max(1), (bottom - y).max(1)) + } + /// TRACES: FR-DEV-3 - /// A false-coloured picture of the current grouping, for the canvas. + /// A false-coloured picture of what a click can select, for the canvas. /// /// Returned as a CPU image rather than a texture, and deliberately: it is - /// regenerated only when the level changes, it is proxy-sized rather than - /// viewport-sized, and Slint scales and composites it for free. Putting it - /// on the GPU would buy nothing and add a second texture to keep in step - /// with the view. + /// regenerated only when the segmentation changes, it is proxy-sized + /// rather than viewport-sized, and the compositor scales and clips it for + /// free. Putting it on the GPU would buy nothing and add a second texture + /// to keep in step with the view. /// /// `None` when the overlay is off or nothing has been segmented, so the /// caller can bind this straight to an image source. @@ -1865,6 +1900,122 @@ mod tests { out } + // ---------------------------------------------------------------------- + // The overlay's clip rectangle + // ---------------------------------------------------------------------- + // + // The overlay is a source-space picture and the canvas shows whatever the + // crop, the zoom and the pan selected out of that space. Drawn whole it + // stays frame-sized while the photograph moves underneath, which is what + // these pin down. + + /// A session with a segmentation, so the clip has a proxy to measure + /// against. + /// + /// The model finds nothing in flat grey, and that is fine: the clip is + /// computed from the framing and the proxy size, neither of which depends + /// on what was detected. + fn segmented_session(ctx: &GpuContext) -> Option { + let rgba: Vec = (0..100 * 100).flat_map(|_| [128, 128, 128, 255]).collect(); + let mut session = + DevelopSession::open_rgb(ctx, &rgba, 100, 100, dr_types::Orientation::NORMAL) + .expect("session"); + session + .segment(ctx, &crate::segmentation::Options::default()) + .ok()?; + Some(session) + } + + fn headless() -> Option { + pollster::block_on(dr_gpu::GpuContext::new_headless()).ok() + } + + #[test] + fn an_unzoomed_overlay_shows_the_whole_frame() { + let Some(ctx) = headless() else { return }; + let Some(session) = segmented_session(&ctx) else { + eprintln!("no model; skipping"); + return; + }; + + let (x, y, w, h) = session.overlay_clip(); + assert_eq!((x, y), (0, 0)); + assert!(w > 1 && h > 1, "the whole proxy: {w}x{h}"); + } + + /// The bug this exists for: zooming must narrow the clip, or the overlay + /// keeps showing the whole picture at frame size while the canvas shows a + /// detail of it. + #[test] + fn zooming_narrows_the_overlay_to_what_is_visible() { + let Some(ctx) = headless() else { return }; + let Some(mut session) = segmented_session(&ctx) else { + eprintln!("no model; skipping"); + return; + }; + + let (_, _, full_w, full_h) = session.overlay_clip(); + session.zoom_about(4.0, 0.5, 0.5); + let (_, _, zoomed_w, zoomed_h) = session.overlay_clip(); + + assert!( + zoomed_w < full_w && zoomed_h < full_h, + "zoomed in, the overlay should show less: {zoomed_w}x{zoomed_h} \ + against {full_w}x{full_h}" + ); + } + + #[test] + fn panning_moves_the_overlay_with_the_photograph() { + let Some(ctx) = headless() else { return }; + let Some(mut session) = segmented_session(&ctx) else { + eprintln!("no model; skipping"); + return; + }; + + session.zoom_about(4.0, 0.5, 0.5); + let (before_x, _, _, _) = session.overlay_clip(); + session.pan_by(0.3, 0.0); + let (after_x, _, _, _) = session.overlay_clip(); + + assert!( + after_x > before_x, + "panning right moves the visible window right: {before_x} then {after_x}" + ); + } + + #[test] + fn cropping_narrows_the_overlay_too() { + let Some(ctx) = headless() else { return }; + let Some(mut session) = segmented_session(&ctx) else { + eprintln!("no model; skipping"); + return; + }; + + let (_, _, full_w, _) = session.overlay_clip(); + session.set_crop(dr_pipeline::CropRect { + x: 0.25, + y: 0.25, + width: 0.5, + height: 0.5, + }); + let (x, y, w, _) = session.overlay_clip(); + + assert!(w < full_w, "a half-width crop shows half the overlay"); + assert!(x > 0 && y > 0, "and it starts inside the frame"); + } + + /// Nothing segmented means no overlay, and no rectangle a caller might + /// divide by. + #[test] + fn no_segmentation_means_no_clip() { + let Some(ctx) = headless() else { return }; + let rgba: Vec = (0..16 * 16).flat_map(|_| [128, 128, 128, 255]).collect(); + let session = DevelopSession::open_rgb(&ctx, &rgba, 16, 16, dr_types::Orientation::NORMAL) + .expect("session"); + assert_eq!(session.overlay_clip(), (0, 0, 0, 0)); + } + /// TRACES: FR-DSP-1 | AC-8 #[test] fn the_displayed_frame_is_a_texture_and_not_a_pixel_buffer() { diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index b21eaf7..c41036b 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -1084,6 +1084,13 @@ pub fn run(paths: Vec) -> Result<()> { window.set_can_undo(s.can_undo()); window.set_can_redo(s.can_redo()); + // TRACES: FR-DEV-3 + // Which part of the region overlay the view is showing. Here + // rather than in the panel's own sync because a pan or a zoom + // changes it while changing no mask and no row — and every one of + // those ends in a redraw. + masks_ui::sync_overlay_view(window, s); + let (mut w, mut h) = *viewport.borrow(); // **Half resolution while the gesture is still moving.** diff --git a/ui/dr-ui/src/masks_ui.rs b/ui/dr-ui/src/masks_ui.rs index 30118a6..d94f5f2 100644 --- a/ui/dr-ui/src/masks_ui.rs +++ b/ui/dr-ui/src/masks_ui.rs @@ -84,6 +84,25 @@ pub(crate) fn sync(window: &AppWindow, session: &Rc window.set_overlay_on(false), } + + // Which part of it the view is showing. Pushed on every sync *and* on + // every redraw, because zooming and panning change this without changing + // anything else the panel shows. + sync_overlay_view(window, s); +} + +/// Push the overlay's clip rectangle and angle. +/// +/// Separate from [`sync`] because it is called from the render path too: a pan +/// changes no mask and no row, so nothing else in `sync` needs to run, and +/// rebuilding the row models on every frame of a drag would be wasteful. +pub(crate) fn sync_overlay_view(window: &AppWindow, session: &DevelopSession) { + let (x, y, w, h) = session.overlay_clip(); + window.set_overlay_clip_x(x); + window.set_overlay_clip_y(y); + window.set_overlay_clip_w(w); + window.set_overlay_clip_h(h); + window.set_overlay_angle(session.angle()); } /// Install the panel's callbacks. diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index c9565bc..53e3963 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -806,6 +806,15 @@ export component AppWindow inherits Window { /// opaque map hides the thing being judged. in property overlay-strength: 0.55; in property overlay-on: false; + /// Which part of the source-space overlay the view is showing, in overlay + /// pixels. Without it the overlay stays frame-sized while the photograph + /// moves under it. + in property overlay-clip-x; + in property overlay-clip-y; + in property overlay-clip-w; + in property overlay-clip-h; + /// The straightening angle, so the overlay turns with the frame. + in property overlay-angle: 0.0; /// Clicking the canvas selects a region instead of panning. in property region-picking: false; /// Whether shift is down, tracked by the develop key scope below. @@ -1336,6 +1345,16 @@ in property panel-visible: true; width: parent.shown-w; height: parent.shown-h; source: root.region-overlay; + // The visible part of a source-space picture, which is + // what makes the overlay follow a zoom, a pan and a + // crop. The compositor does the crop and the scale; + // resampling on the CPU would mean rebuilding a + // megapixel image on every frame of a drag. + source-clip-x: root.overlay-clip-x; + source-clip-y: root.overlay-clip-y; + source-clip-width: root.overlay-clip-w; + source-clip-height: root.overlay-clip-h; + rotation-angle: -root.overlay-angle * 1deg; image-fit: fill; opacity: root.overlay-strength; // Nearest-neighbour, always. The map is proxy-sized