mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-13 17:31:09 -06:00
Fix mismatched source/destination rects in before/after zoom window renders (#3842)
* Fix mismatched source/destination rects in before/after zoom window renders Both panes of Before/After cut the zoom window from one rect but paste it over another, which is exactly the inconsistent-offset and artifact behavior reported in issue #1035; a headless test exercising click-zoom in both panes with a fixed camera raw would catch the regression. Detail view: compute `visible` relative to `v.img` instead of `v.target` so the window's source and destination share the animated rect. Compare/Reference view: compute `visible` relative to `display` (the rect actually drawn) instead of `img` so texture-aspect fits land in the right place. Test: compare_zoom_window_tracks_the_zoomed_pane failed before the change and passes after. * Compare: keep before_target, return both rects so Survey's remove button stays on the photo Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Format Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Compare: the early return gives both rects Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: eliets2 <287273517+eliets2@users.noreply.github.com> Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
eliets2
FredApps
parent
a856df5f42
commit
d2bf0810bd
@@ -210,12 +210,20 @@ fn request_tile_renders(app: &mut LightcraftApp, id: PhotoId, slot: Slot, want:
|
||||
|
||||
/// Draw one photo fitted into `area` (rendered at its display size into `slot`), with its caption
|
||||
/// strip below. Returns the image rect and the click/drag response.
|
||||
fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slot, area: Rect, label: &str, zoom: Zoom) -> (Rect, egui::Response) {
|
||||
fn photo_tile(
|
||||
app: &mut LightcraftApp,
|
||||
ui: &mut egui::Ui,
|
||||
id: PhotoId,
|
||||
slot: Slot,
|
||||
area: Rect,
|
||||
label: &str,
|
||||
zoom: Zoom,
|
||||
) -> (Rect, Rect, egui::Response) {
|
||||
let t = Tokens::get(ui.ctx());
|
||||
let ppp = ui.ctx().pixels_per_point();
|
||||
let resp = ui.interact(area, egui::Id::unique(("cull-tile", slot_index(slot), id.0)), Sense::click_and_drag());
|
||||
crate::widgets::announce(&resp, app.session.catalog.photo(id).map(|p| p.file_name.clone()).unwrap_or_default());
|
||||
let Some(photo) = app.session.catalog.photo(id).cloned() else { return (area, resp) };
|
||||
let Some(photo) = app.session.catalog.photo(id).cloned() else { return (area, area, resp) };
|
||||
let caption_h = 26.0;
|
||||
let img_area = Rect::from_min_max(area.min, pos2(area.right(), area.bottom() - caption_h));
|
||||
let frame = lightcraft_pipeline::geometry::Frame::with_lens(
|
||||
@@ -279,7 +287,7 @@ fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slo
|
||||
display = super::detail::fit_rect(img_area, texture_aspect, zoom, native, ppp, app.ui.pan);
|
||||
p.image(tex.id(), display, uv, Color32::WHITE);
|
||||
if let Some(i) = window {
|
||||
draw_window(&p, app, i, id, photo.develop.hash64(), display);
|
||||
draw_window(&p, app, i, id, photo.develop.hash64(), img);
|
||||
}
|
||||
} else if let Some(tex) = app.renderer.textures.get(&Slot::Thumb(id)) {
|
||||
let texture_aspect = tex.size[0].max(1) as f32 / tex.size[1].max(1) as f32;
|
||||
@@ -369,7 +377,10 @@ fn photo_tile(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, slot: Slo
|
||||
register(ui, format!("cull:{}:label", id.0), lr);
|
||||
}
|
||||
register(ui, format!("cull:{}", id.0), display);
|
||||
(display, resp)
|
||||
// The frame rect the window was requested in (`img`) and the texture's rect (`display`): they
|
||||
// differ when the texture's aspect differs from the frame's. The first is the window's
|
||||
// reference, the second is the photo as painted.
|
||||
(img, display, resp)
|
||||
}
|
||||
|
||||
/// Panes that get window renders: Compare's Select and Candidate, Reference's two sides.
|
||||
@@ -510,7 +521,7 @@ pub fn show_compare(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
];
|
||||
let zoom = app.ui.zoom;
|
||||
for (id, r, label, i) in panes {
|
||||
let (img, resp) = photo_tile(app, ui, id, Slot::Compare(i), r, label, zoom);
|
||||
let (img, _, resp) = photo_tile(app, ui, id, Slot::Compare(i), r, label, zoom);
|
||||
if i == 1 {
|
||||
app.image_rect = Some(img);
|
||||
}
|
||||
@@ -565,7 +576,8 @@ pub fn show_survey(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
for (i, id) in photos.iter().enumerate() {
|
||||
let (c, r) = (i % cols, i / cols);
|
||||
let cell = Rect::from_min_size(pos2(area.left() + c as f32 * cw, area.top() + r as f32 * ch), vec2(cw, ch)).shrink(6.0);
|
||||
let (img, resp) = photo_tile(app, ui, *id, Slot::Compare(i as u8), cell, "", Zoom::Fit);
|
||||
// (Survey tiles are fit views with no windows: what counts is the photo as painted)
|
||||
let (_, img, resp) = photo_tile(app, ui, *id, Slot::Compare(i as u8), cell, "", Zoom::Fit);
|
||||
if app.session.selection.active == Some(*id) {
|
||||
app.image_rect = Some(img);
|
||||
}
|
||||
@@ -620,8 +632,8 @@ pub fn show_reference(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
let zoom = app.ui.zoom;
|
||||
let left = Rect::from_min_size(area.min, vec2(half, area.height()));
|
||||
let right = Rect::from_min_size(pos2(area.right() - half, area.top()), vec2(half, area.height()));
|
||||
let (_, lresp) = photo_tile(app, ui, r, Slot::Compare(0), left, "Reference", zoom);
|
||||
let (img, _) = photo_tile(app, ui, active, Slot::Compare(1), right, "Active", zoom);
|
||||
let (_, _, lresp) = photo_tile(app, ui, r, Slot::Compare(0), left, "Reference", zoom);
|
||||
let (img, _, _) = photo_tile(app, ui, active, Slot::Compare(1), right, "Active", zoom);
|
||||
app.image_rect = Some(img);
|
||||
crate::widgets::context_menu(&lresp, |ui| {
|
||||
if ui.button(crate::i18n::tr("Clear Reference")).clicked() {
|
||||
|
||||
@@ -285,6 +285,86 @@ fn deleting_the_pinned_reference_stops_drawing_it_and_reentry_uses_a_live_photo(
|
||||
assert!(h.app.renderer.textures.get(&crate::render::Slot::Compare(0)).is_some_and(|t| t.photo == reference));
|
||||
}
|
||||
|
||||
/// The zoom window is checked against the reference frame the request was made for: a window is a
|
||||
/// part of the frame, so its bounds are the window's pixels laid over that frame, not over whatever
|
||||
/// rect the whole-frame texture happens to be fitted into. In the candidate pane that frame is the
|
||||
/// one recorded for the pane: the tile is laid over it, so any other frame rect is the texture's.
|
||||
#[test]
|
||||
fn compare_zoom_window_tracks_the_zoomed_pane() {
|
||||
use crate::render::Slot;
|
||||
let mut h = demo();
|
||||
let vis: Vec<u64> = h.app.session.visible_cloned().iter().map(|p| p.0).collect();
|
||||
h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [vis[0], vis[1]], "active": vis[0]}}), T);
|
||||
let r = h.request("engine.execute", json!({"command": "view.compare"}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.app.ui.zoom = Zoom::Percent(100.0);
|
||||
h.settle(SETTLE);
|
||||
let candidate = vis[1];
|
||||
let photo = h.app.session.catalog.photo(lightcraft_catalog::PhotoId(candidate)).cloned().expect("photo");
|
||||
let frame = lightcraft_pipeline::geometry::Frame::with_lens(
|
||||
photo.width.max(1) as usize,
|
||||
photo.height.max(1) as usize,
|
||||
&photo.develop,
|
||||
true,
|
||||
photo.embedded_lens.as_ref(),
|
||||
);
|
||||
let aspect = frame.aspect() as f32;
|
||||
let native = crate::panels::detail::output_px(&frame);
|
||||
let view = h.app.compare_windows[1].expect("a zoom window");
|
||||
assert_eq!(view.photo, lightcraft_catalog::PhotoId(candidate));
|
||||
let tex = h.app.renderer.textures.get(&Slot::CompareRegion(1)).expect("window texture");
|
||||
assert_eq!(tex.photo, lightcraft_catalog::PhotoId(candidate));
|
||||
let tex_id = tex.tex.id();
|
||||
// Lay the whole-frame texture over the same reference frame with an aspect just inside the
|
||||
// same-frame tolerance: the drawn rect moves, the request stays valid, and the window is
|
||||
// drawn over the moved texture rect. Hold the replacement renders so the existing window is
|
||||
// the one drawn, magnified, over the texture.
|
||||
let texture_size = {
|
||||
let whole = h.app.renderer.textures.get_mut(&Slot::Compare(1)).expect("whole-frame texture");
|
||||
let target_aspect = aspect * 1.015;
|
||||
whole.size[1] = (whole.size[0] as f32 / target_aspect).round().max(8.0) as usize;
|
||||
whole.size
|
||||
};
|
||||
let texture_aspect = texture_size[0].max(1) as f32 / texture_size[1].max(1) as f32;
|
||||
assert!(crate::region::same_aspect(texture_aspect, aspect), "a trace off the frame's aspect stays in tolerance: {texture_aspect} {}", aspect);
|
||||
let open = std::rc::Rc::new(std::cell::Cell::new(false));
|
||||
h.app.renderer.set_offload(Box::new(crate::headless::RenderGate::new(open.clone())));
|
||||
h.app.session.begin_interaction("Exposure").unwrap();
|
||||
h.step();
|
||||
let pane_img = h.app.image_rect.expect("the candidate pane's frame");
|
||||
let img_area = egui::Rect::from_min_max(pane_img.min, egui::pos2(pane_img.right(), pane_img.bottom()));
|
||||
let img = pane_img;
|
||||
let display = crate::panels::detail::fit_rect(img_area, texture_aspect, h.app.ui.zoom, native, 1.0, h.app.ui.pan);
|
||||
let expected = egui::Rect::from_min_size(
|
||||
img.min + egui::vec2(view.window.x as f32 / view.full.0 as f32 * img.width(), view.window.y as f32 / view.full.1 as f32 * img.height()),
|
||||
egui::vec2(view.window.w as f32 / view.full.0 as f32 * img.width(), view.window.h as f32 / view.full.1 as f32 * img.height()),
|
||||
);
|
||||
let wrong = egui::Rect::from_min_size(
|
||||
display.min
|
||||
+ egui::vec2(view.window.x as f32 / view.full.0 as f32 * display.width(), view.window.y as f32 / view.full.1 as f32 * display.height()),
|
||||
egui::vec2(view.window.w as f32 / view.full.0 as f32 * display.width(), view.window.h as f32 / view.full.1 as f32 * display.height()),
|
||||
);
|
||||
assert!(
|
||||
(wrong.min - expected.min).abs().max_elem() > 3.0,
|
||||
"the frame rect and the texture rect must differ for this check: {img:?} vs {display:?}"
|
||||
);
|
||||
let drawn: Vec<egui::Rect> = h
|
||||
.view
|
||||
.shapes
|
||||
.iter()
|
||||
.filter_map(|c| match &c.shape {
|
||||
egui::epaint::Shape::Mesh(m) if m.texture_id == tex_id => Some(m.calc_bounds()),
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
let bounds = drawn.iter().copied().find(|b| (b.min - wrong.min).abs().max_elem() < 1.0).or_else(|| drawn.first().copied());
|
||||
let bounds = bounds.expect("the window tile is painted");
|
||||
assert!(
|
||||
(bounds.min - expected.min).abs().max_elem() < 1.0 && (bounds.max - expected.max).abs().max_elem() < 1.0,
|
||||
"the tile {bounds:?} must sit in the zoomed pane's frame {expected:?}, not in the texture's rect {wrong:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Clicking a colour label (or stars) in the Info panel while culling labels the active photo
|
||||
/// only, as the label keys and the bottom bar's stars and flags do, and the label shows under that
|
||||
/// photo in Compare (#2398: "Can't assign color to photo when culling").
|
||||
|
||||
Reference in New Issue
Block a user