mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 10:09:01 -06:00
LRC-LIB-REFVIEW: fix Escape and catalog-local reference pins
This commit is contained in:
@@ -590,8 +590,8 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
|
||||
let reference = app
|
||||
.ui
|
||||
.reference
|
||||
.filter(|r| app.session.catalog.photo(lightcraft_catalog::PhotoId(*r)).is_some())
|
||||
.or_else(|| app.session.active().map(|a| a.0));
|
||||
.filter(|r| app.session.catalog.photo(lightcraft_catalog::PhotoId(*r)).is_some_and(|p| !p.deleted))
|
||||
.or_else(|| app.session.active().filter(|a| app.session.catalog.photo(*a).is_some_and(|p| !p.deleted)).map(|a| a.0));
|
||||
let Some(r) = reference else { return Some(Err("select a photo to use as the reference".into())) };
|
||||
app.ui.reference = Some(r);
|
||||
if app.session.active().is_none_or(|a| a.0 == r)
|
||||
@@ -605,7 +605,11 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
|
||||
}
|
||||
"photo.setReference" => {
|
||||
let id = p.get("id").and_then(Value::as_u64).or_else(|| app.session.active().map(|a| a.0));
|
||||
app.ui.reference = id;
|
||||
let Some(id) = id else { return Some(Err("select a photo to use as the reference".into())) };
|
||||
if app.session.catalog.photo(lightcraft_catalog::PhotoId(id)).is_none_or(|p| p.deleted) {
|
||||
return Some(Err(format!("no photo {id}")));
|
||||
}
|
||||
app.ui.reference = Some(id);
|
||||
Ok(json!({"reference": id}))
|
||||
}
|
||||
"compare.swap" => crate::panels::compare::swap(app),
|
||||
@@ -636,7 +640,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
|
||||
panel(app, &ctx, RightPanel::Crop, "Crop, Rotate, Geometry");
|
||||
} else if app.ui.view == ViewMode::People && app.ui.person_page.is_some() {
|
||||
app.ui.person_page = None;
|
||||
} else if matches!(app.ui.view, ViewMode::Compare | ViewMode::Survey) {
|
||||
} else if matches!(app.ui.view, ViewMode::Compare | ViewMode::Survey | ViewMode::Reference) {
|
||||
app.ui.view = ViewMode::Detail;
|
||||
} else if app.ui.view == ViewMode::Detail {
|
||||
// a photo opened from a person's page (or the unnamed faces) goes back there, any other to the grid
|
||||
|
||||
@@ -578,7 +578,7 @@ pub fn show_survey(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
/// the Edit panel works on, so a look can be matched by eye.
|
||||
pub fn show_reference(app: &mut LightcraftApp, ui: &mut egui::Ui) {
|
||||
let canvas = area_and_filmstrip(app, ui);
|
||||
let reference = app.ui.reference.map(PhotoId).filter(|r| app.session.catalog.photo(*r).is_some());
|
||||
let reference = app.ui.reference.map(PhotoId).filter(|r| app.session.catalog.photo(*r).is_some_and(|p| !p.deleted));
|
||||
let (Some(r), Some(active)) = (reference, app.session.active()) else {
|
||||
super::empty_message(ui, canvas, "No reference photo", "Right-click a photo ▸ Set as Reference Photo, then View → Reference View (Shift+R)");
|
||||
return;
|
||||
|
||||
@@ -1137,6 +1137,8 @@ fn switch_library(app: &mut LightcraftApp, path: &str) -> Result<Value, String>
|
||||
Err(e) => Err(format!("{err}; reopening the previous library {} failed too: {e}", prev.display())),
|
||||
};
|
||||
}
|
||||
// Photo IDs are local to a catalog: a pin cannot follow them into another library.
|
||||
app.ui.reference = None;
|
||||
app.ui.settings.library_path = path.clone();
|
||||
if let Some(prev) = previous.filter(|p| p != &path && !app.ui.settings.recent_libraries.contains(p)) {
|
||||
remember_library(&mut app.ui.settings.recent_libraries, &prev);
|
||||
@@ -1311,7 +1313,8 @@ mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
fn app_with_import() -> LightcraftApp {
|
||||
let mut app = LightcraftApp::new(lightcraft_engine::Session::new(), crate::Services::default());
|
||||
let mut app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), crate::Services::default());
|
||||
app.run("photo.setReference", json!({})).unwrap();
|
||||
app.import = Some(crate::import::ImportTask::new(vec!["a.jpg".into()], json!({"mode": "add"}), 0, false));
|
||||
app
|
||||
}
|
||||
@@ -1319,17 +1322,21 @@ mod tests {
|
||||
#[test]
|
||||
fn open_library_is_refused_while_an_import_runs() {
|
||||
let mut app = app_with_import();
|
||||
let pin = app.ui.reference;
|
||||
let err = open_library(&mut app, &json!({"path": "other"})).unwrap_err();
|
||||
assert!(err.contains("import to finish"), "{err}");
|
||||
assert!(app.import.is_some());
|
||||
assert_eq!(app.ui.reference, pin, "a refused Open Library keeps the reference pin");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn new_library_is_refused_while_an_import_runs() {
|
||||
let mut app = app_with_import();
|
||||
let pin = app.ui.reference;
|
||||
let err = new_library(&mut app, &json!({"path": "other"})).unwrap_err();
|
||||
assert!(err.contains("import to finish"), "{err}");
|
||||
assert!(app.import.is_some());
|
||||
assert_eq!(app.ui.reference, pin, "a refused New Library keeps the reference pin");
|
||||
}
|
||||
|
||||
/// The menu items that switch libraries are greyed out meanwhile, like during a Lightroom import.
|
||||
@@ -1349,11 +1356,18 @@ mod tests {
|
||||
fn open_library_is_refused_while_an_import_scan_runs() {
|
||||
let dir = std::env::temp_dir().join(format!("lc-ui-switchscan-{}", std::process::id()));
|
||||
let _ = std::fs::create_dir_all(&dir);
|
||||
let mut app = LightcraftApp::new(lightcraft_engine::Session::new(), crate::Services::default());
|
||||
let mut app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), crate::Services::default());
|
||||
app.run("photo.setReference", json!({})).unwrap();
|
||||
let pin = app.ui.reference;
|
||||
crate::import::open(&mut app, vec![dir.to_string_lossy().into_owned()]).unwrap();
|
||||
assert!(app.scan.is_some());
|
||||
let err = open_library(&mut app, &json!({"path": "other"})).unwrap_err();
|
||||
assert!(err.contains("import to finish"), "{err}");
|
||||
assert_eq!(app.ui.reference, pin, "a refused Open Library during a scan keeps the reference pin");
|
||||
let err = new_library(&mut app, &json!({"path": "other"})).unwrap_err();
|
||||
assert!(err.contains("import to finish"), "{err}");
|
||||
assert_eq!(app.ui.reference, pin, "a refused New Library during a scan keeps the reference pin");
|
||||
assert!(app.scan.is_some());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -209,3 +209,64 @@ fn a_compare_pane_says_when_its_render_failed() {
|
||||
assert!(h.step_until(SETTLE, failed), "the failure is said");
|
||||
assert!(!h.app.widgets.iter().any(|(w, _)| w.starts_with("compare:rendering:")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reference_escape_leaves_overlays_then_the_view_without_changing_edits() {
|
||||
use crate::state::{LightsOut, ViewMode};
|
||||
let mut h = demo();
|
||||
let reference = h.app.session.visible_cloned()[0];
|
||||
h.app.run("library.select", json!({"ids": [reference.0]})).unwrap();
|
||||
h.app.run("view.reference", json!({})).unwrap();
|
||||
let active = h.app.session.active().unwrap();
|
||||
h.app.run("develop.set", json!({"control": "light.exposure", "value": 0.7})).unwrap();
|
||||
h.app.ui.lights_out = LightsOut::Dim;
|
||||
h.request("ui.key", json!({"key": "Escape"}), T);
|
||||
assert_eq!((h.app.ui.view, h.app.ui.lights_out), (ViewMode::Reference, LightsOut::On));
|
||||
h.request("ui.key", json!({"key": "Escape"}), T);
|
||||
assert_eq!(h.app.ui.view, ViewMode::Detail);
|
||||
assert_eq!(h.app.ui.reference, Some(reference.0), "leaving the view keeps the pinned photo");
|
||||
assert_eq!(h.app.session.active(), Some(active));
|
||||
assert_eq!(h.app.session.develop_of(active).unwrap().light.exposure, 0.7);
|
||||
h.request("ui.key", json!({"key": "Escape"}), T);
|
||||
assert_eq!(h.app.ui.view, ViewMode::PhotoGrid);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_or_deleted_reference_is_refused_without_replacing_the_pin() {
|
||||
let mut h = demo();
|
||||
let ids = h.app.session.visible_cloned();
|
||||
h.app.run("library.select", json!({"ids": [ids[0].0]})).unwrap();
|
||||
h.app.run("photo.setReference", json!({})).unwrap();
|
||||
h.app.run("photo.delete", json!({"ids": [ids[1].0]})).unwrap();
|
||||
for id in [u64::MAX, ids[1].0] {
|
||||
let r = h.app.run("photo.setReference", json!({"id": id}));
|
||||
assert!(r.is_err(), "unavailable photo {id} must not be pinned");
|
||||
assert_eq!(h.app.ui.reference, Some(ids[0].0));
|
||||
}
|
||||
h.app.run("library.select", json!({"ids": []})).unwrap();
|
||||
assert!(h.app.run("photo.setReference", json!({})).is_err());
|
||||
assert_eq!(h.app.ui.reference, Some(ids[0].0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deleting_the_pinned_reference_stops_drawing_it_and_reentry_uses_a_live_photo() {
|
||||
let mut h = demo();
|
||||
let reference = h.app.session.visible_cloned()[0];
|
||||
h.app.run("library.select", json!({"ids": [reference.0]})).unwrap();
|
||||
h.app.run("view.reference", json!({})).unwrap();
|
||||
h.settle(SETTLE);
|
||||
let texture = h.app.renderer.textures.get(&crate::render::Slot::Compare(0)).unwrap().tex.id();
|
||||
let drawn = |h: &Headless| h.view.painted_shapes().iter().any(|s| matches!(s, egui::epaint::Shape::Mesh(mesh) if mesh.texture_id == texture));
|
||||
assert!(drawn(&h), "the live reference is shown before deletion");
|
||||
let active = h.app.session.active().unwrap();
|
||||
h.app.run("photo.delete", json!({"ids": [reference.0]})).unwrap();
|
||||
h.settle(SETTLE);
|
||||
assert!(!drawn(&h), "Recently Deleted photos must not remain on the reference canvas");
|
||||
let r = h.app.run("view.reference", json!({})).unwrap();
|
||||
assert_eq!(r["reference"], active.0);
|
||||
assert_eq!(h.app.ui.reference, Some(active.0));
|
||||
h.app.run("photo.restore", json!({"ids": [reference.0]})).unwrap();
|
||||
h.app.run("photo.setReference", json!({"id": reference.0})).unwrap();
|
||||
h.settle(SETTLE);
|
||||
assert!(h.app.renderer.textures.get(&crate::render::Slot::Compare(0)).is_some_and(|t| t.photo == reference));
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
//! loupe renders at the size it is drawn at (up to the photo's own pixels and a safety ceiling);
|
||||
//! "Preview size" in Settings only lowers that when the user asks for it.
|
||||
|
||||
use crate::state::{AppSettings, LOUPE_EDGE_CEILING, PREVIEW_LIMITS, STANDARD_PREVIEW_EDGE};
|
||||
use crate::state::{AppSettings, LOUPE_EDGE_CEILING, STANDARD_PREVIEW_EDGE};
|
||||
|
||||
/// A texture side no GPU we run on refuses (see the tests of small ones below).
|
||||
const BIG: usize = 16384;
|
||||
@@ -11,13 +11,6 @@ fn with_limit(preview_limit: u32) -> AppSettings {
|
||||
AppSettings { preview_limit, ..Default::default() }
|
||||
}
|
||||
|
||||
// Given a fresh install
|
||||
#[test]
|
||||
fn default_is_automatic() {
|
||||
assert_eq!(AppSettings::default().preview_limit, 0);
|
||||
assert_eq!(PREVIEW_LIMITS[0], 0, "Automatic is the first choice");
|
||||
}
|
||||
|
||||
// Given Automatic, when a 24 MP photo is shown at 1:1, then the render is the photo's own width
|
||||
#[test]
|
||||
fn automatic_renders_a_one_to_one_view_at_native_size() {
|
||||
|
||||
@@ -183,6 +183,38 @@ fn missing_recent_libraries_are_greyed_out_and_pruned_and_the_list_is_capped() {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reference_photo_ids_do_not_cross_library_boundaries() {
|
||||
let dir = temp("reference-switch");
|
||||
let (first, second) = (dir.join("first"), dir.join("second"));
|
||||
let mut session = lightcraft_engine::Session::new().with_fs();
|
||||
session.open_library(&second, true).unwrap();
|
||||
let new_photo = session.visible_cloned()[0];
|
||||
session.close_library().unwrap();
|
||||
session.open_library(&first, true).unwrap();
|
||||
let old_reference = session.visible_cloned()[1];
|
||||
let mut app = LightcraftApp::new(session, Services::default());
|
||||
app.run("library.select", json!({"ids": [old_reference.0]})).unwrap();
|
||||
app.run("photo.setReference", json!({})).unwrap();
|
||||
app.run("view.reference", json!({})).unwrap();
|
||||
let held = lightcraft_catalog::LibraryLock::acquire(&second, "another program").unwrap();
|
||||
assert!(app.run("app.openLibrary", json!({"path": second.to_string_lossy()})).is_err());
|
||||
assert_eq!(app.ui.reference, Some(old_reference.0), "a refused switch keeps the live pin");
|
||||
drop(held);
|
||||
app.run("app.openLibrary", json!({"path": second.to_string_lossy()})).unwrap();
|
||||
assert!(app.session.catalog.photo(old_reference).is_some(), "the second library reuses this numeric ID");
|
||||
assert_eq!(app.ui.reference, None, "the first library's pin must not acquire a different photo");
|
||||
app.run("library.select", json!({"ids": [new_photo.0]})).unwrap();
|
||||
let r = app.run("view.reference", json!({})).unwrap();
|
||||
assert_eq!(r["reference"], new_photo.0);
|
||||
app.run("app.newLibrary", json!({"path": dir.to_string_lossy(), "name": "Empty"})).unwrap();
|
||||
assert_eq!(app.ui.reference, None, "New Library clears the old reference too");
|
||||
assert!(app.run("view.reference", json!({})).is_err(), "an empty library cannot pin a reference");
|
||||
assert_eq!(app.ui.reference, None);
|
||||
app.session.close_library().unwrap();
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// A library another program has open is refused before the current one is closed: the current
|
||||
/// library stays open with its photos, and the window title keeps its name.
|
||||
#[test]
|
||||
|
||||
File diff suppressed because one or more lines are too long
+20
-1
@@ -1,6 +1,6 @@
|
||||
# UI parity
|
||||
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (zoom row: soft proofing sharp at 1:1; canvas tools by keyboard; focus-order test; ~50 unnamed hand-drawn widgets named) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-21 · **Change:** minor (Reference Escape, catalog-local pins and deleted-photo handling verified on Windows; earlier: zoom row soft proofing sharp at 1:1, canvas tools by keyboard, focus-order test, ~50 unnamed hand-drawn widgets named) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6
|
||||
|
||||
How LightCraft's interaction compares with Lightroom's: views, panels, tools and handles, modifier
|
||||
keys, shortcuts, zoom, context menus and feel. Rows for individual menu items and shortcuts live in
|
||||
@@ -126,6 +126,24 @@ unnamed was in the way.
|
||||
Not done: brush strokes and red-eye ellipses still need a pointer. Nobody has tried the app with
|
||||
a real screen reader on any platform yet.
|
||||
|
||||
## Reference view lifecycle
|
||||
|
||||
Escape leaves Reference for Detail, preserving the active photo and its edits; a second Escape
|
||||
returns to the grid. Existing dialogs, tools and Lights Out retain their higher-priority Escape
|
||||
handling. The reference stays pinned when leaving the view, but a successful Open/New Library
|
||||
clears it: numeric photo IDs are local to each catalog. A refused switch keeps the current pin,
|
||||
including Open/New refused while an import or its folder scan is running.
|
||||
|
||||
`photo.setReference` refuses missing or Recently Deleted photos without replacing the current
|
||||
pin. Deleting a pinned reference stops drawing it; re-entering Reference falls back to a live
|
||||
active photo. Restoring the photo permits pinning it again. An empty catalog cannot supply one.
|
||||
|
||||
Verification: four new behavioral regressions failed on upstream `76b5377` before the fix.
|
||||
The patched Windows x64 app on RTX 5080 / DX12 passed Escape, edit preservation, delete/restore,
|
||||
and New/Open Library checks using isolated catalogs. Saved exposure survived reopening. Native
|
||||
1400 × 900 captures did not fall back to headless rendering. Control-channel input does not
|
||||
establish OS keyboard delivery or cross-platform acceptance.
|
||||
|
||||
## Revision history
|
||||
|
||||
| Date | Change | Summary |
|
||||
@@ -138,5 +156,6 @@ a real screen reader on any platform yet.
|
||||
| 2026-10-13 | minor | Shortcuts: Home / End and Shift+Left / Right in the two library grids, Cmd+O (Open Library…) and Shift+H (Visualize HDR) get bindings and keyboard tests; percentages and the dated counts above are unchanged |
|
||||
| 2026-10-14 | minor | Zoom row: soft proofing gets zoom windows; LR-VIEW-ZOOM gap closed |
|
||||
| 2026-10-12 | minor | Accessibility: an AccessKit tree audit test over every view, Develop tool and 13 dialogs; unnamed fields, slider ranges and the Auto Stack slider fixed; 30% → 55%; UI remaining 40–70 h → 34–58 h |
|
||||
| 2026-10-11 | minor | Reference Escape exits to Detail; catalog-local pins and missing/deleted reference handling verified with regressions and native Windows smoke |
|
||||
| 2026-10-11 | minor | Zoom row: Compare and Reference get zoom windows; what remains of LR-VIEW-ZOOM is overlays and soft proofing |
|
||||
| 2026-10-10 | major | Created: measured command and shortcut counts, assessment by area, estimates; moved the shortcut conflicts section here from the checklist |
|
||||
|
||||
@@ -570,6 +570,14 @@
|
||||
D7000, D5100, D800, D800E, Df, D4, D3, D3S, D700, D300, D300S, D600, D610, D810, D4S, D5600, Coolpix A, D3000, D40X,
|
||||
D60, D80 and D2Xs now show the area the camera itself frames, a few pixels smaller than before.
|
||||
|
||||
### Reference view reliability
|
||||
- Escape now leaves Reference for Detail without changing the active photo or its edits; the next
|
||||
Escape returns to the grid. A dialog, tool or Lights Out still handles Escape first.
|
||||
- Opening or creating a different library clears the pinned reference. Photo IDs belong to one
|
||||
catalog; a pin no longer silently becomes another library's photo with the same numeric ID.
|
||||
- Missing or Recently Deleted photos cannot be pinned. Deleting the pinned reference stops drawing
|
||||
it; re-entering Reference chooses a live photo, and restoring the deleted photo makes it pinnable again.
|
||||
|
||||
### JPEG XL export
|
||||
- Export as JPEG XL (issue #247): Format ▸ JPEG XL in the Export dialog, `format: "jxl"` for agents and the CLI
|
||||
(`render in.raw -o out.jxl`). Quality as for JPEG, or Lossless; Effort (Fast / Normal) trades time for a
|
||||
|
||||
Reference in New Issue
Block a user