Keep the overlay on the photograph when the view moves
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.
This commit is contained in:
+156
-5
@@ -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<DevelopSession> {
|
||||
let rgba: Vec<u8> = (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<GpuContext> {
|
||||
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<u8> = (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() {
|
||||
|
||||
@@ -1084,6 +1084,13 @@ pub fn run(paths: Vec<PathBuf>) -> 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.**
|
||||
|
||||
@@ -84,6 +84,25 @@ pub(crate) fn sync(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSessio
|
||||
}
|
||||
None => 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.
|
||||
|
||||
@@ -806,6 +806,15 @@ export component AppWindow inherits Window {
|
||||
/// opaque map hides the thing being judged.
|
||||
in property <float> overlay-strength: 0.55;
|
||||
in property <bool> 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 <int> overlay-clip-x;
|
||||
in property <int> overlay-clip-y;
|
||||
in property <int> overlay-clip-w;
|
||||
in property <int> overlay-clip-h;
|
||||
/// The straightening angle, so the overlay turns with the frame.
|
||||
in property <float> overlay-angle: 0.0;
|
||||
/// Clicking the canvas selects a region instead of panning.
|
||||
in property <bool> region-picking: false;
|
||||
/// Whether shift is down, tracked by the develop key scope below.
|
||||
@@ -1336,6 +1345,16 @@ in property <bool> 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
|
||||
|
||||
Reference in New Issue
Block a user