From ca7683e47fae9ca3355948ac2a3f507254c21195 Mon Sep 17 00:00:00 2001 From: OneSlips Date: Thu, 22 Oct 2026 20:28:24 +1000 Subject: [PATCH 01/25] Tests: discovery tests build their expected paths with the platform's separator `recursive_discovery_includes_sony_sr2_and_srf_case_insensitively` (engine) and `folder_discovery_matches_engine_extensions` (mcp) joined "trip/day2/..." as one string, so on Windows the expected paths kept `/` while discovery returns `\`, and both failed. They now join one component at a time. Test-only change. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/tests_import.rs | 4 +++- crates/mcp/src/headless.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 191595ebe..b8a66161a 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -35,7 +35,9 @@ fn recursive_discovery_includes_sony_sr2_and_srf_case_insensitively() { let src = temp_dir("sony-discovery"); std::fs::create_dir_all(src.join("trip/day2")).unwrap(); let names = ["r1.SR2", "f828.srf", "trip/r1.sr2", "trip/f828.SRF", "trip/day2/r1.Sr2", "trip/day2/f828.SrF", "control.JPG"]; - let mut expected: Vec = names.iter().map(|name| src.join(name).to_string_lossy().into_owned()).collect(); + // one component at a time: `join("trip/r1.sr2")` keeps the `/`, discovery yields the platform's separator + let mut expected: Vec = + names.iter().map(|name| name.split('/').fold(src.clone(), |p, c| p.join(c)).to_string_lossy().into_owned()).collect(); for path in &expected { std::fs::write(path, b"discovery only, not a decoded photo").unwrap(); } diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index d62a9a707..e281437c5 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -207,7 +207,7 @@ mod tests { fn folder_discovery_matches_engine_extensions() { let dir = std::env::temp_dir().join(format!("lc-mcp-discovery-{}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); - let nested = dir.join("trip/day2"); + let nested = dir.join("trip").join("day2"); // not "trip/day2": discovery yields the platform's separator std::fs::create_dir_all(&nested).unwrap(); let mut expected = Vec::new(); for ext in lightcraft_engine::import::EXTENSIONS { From e5ea8043f64b794b165bed90e4ba5ed14c26f96e Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 23 Oct 2026 22:15:18 +0200 Subject: [PATCH 02/25] LR-MERGE-PANO: the merge dialog makes a new preview each time it opens, also after a failed one (#2708) The dialog remembered the options of a failed preview and never retried them, and opening it again cleared the error but not that memory. With the same options, a reopened dialog showed neither a preview nor an error until an option changed, so Auto Select looked stuck while picking another projection worked. Opening the dialog now forgets the failure, and a failure only holds for the photos it was made with. --- crates/ui-egui/src/merge.rs | 17 +++++++++++------ crates/ui-egui/src/tests_activity.rs | 25 +++++++++++++++++++++++++ docs/parity-checklist.md | 5 +++-- docs/whats-new.md | 7 +++++++ 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/merge.rs b/crates/ui-egui/src/merge.rs index 61485a271..4d840ae6d 100644 --- a/crates/ui-egui/src/merge.rs +++ b/crates/ui-egui/src/merge.rs @@ -86,6 +86,8 @@ pub struct MergeTask { pub job: MergeJob, /// The dialog options it was started with. pub options: MergeDialog, + /// The photos it merges. + pub ids: Vec, pub progress: Arc>, pub cancel: Arc, rx: Receiver>, @@ -102,8 +104,9 @@ pub struct MergeState { pub preview: Option<(egui::TextureHandle, [usize; 2], MergeDialog, Value)>, pub preview_pixels: Option>, pub error: Option, - /// Options whose preview failed (not retried until they change). - pub failed_options: Option, + /// The photos and options whose preview failed (not retried until either changes or the dialog + /// opens again). + pub failed_options: Option<(Vec, MergeDialog)>, /// The last finished merge (`{id, path, …}`). pub last_result: Option, /// Photos the dialog merges (the selection when it opened). @@ -143,7 +146,7 @@ fn spawn(app: &LightcraftApp, options: &MergeDialog, ids: &[PhotoId], preview: b std::thread::Builder::new().name("photo-merge".into()).spawn(work).map_err(|e| e.to_string())?; #[cfg(target_arch = "wasm32")] work(); - Ok(MergeTask { preview, job, options: options.clone(), progress, cancel, rx, guard: None }) + Ok(MergeTask { preview, job, options: options.clone(), ids: ids.to_vec(), progress, cancel, rx, guard: None }) } /// The options of the last merge of kind `command` started (kept in the library preferences), or @@ -162,6 +165,8 @@ pub fn open(app: &mut LightcraftApp, command: &str) -> Result { } app.merge.ids = ids; app.merge.error = None; + // a preview that failed before is tried again, or the dialog would show neither a preview nor its error (#2708) + app.merge.failed_options = None; app.merge.preview = None; app.merge.preview_pixels = None; if let Some(t) = app.merge.preview_task.take() { @@ -253,7 +258,7 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) { } Err(e) => { app.merge.error = Some(e); - app.merge.failed_options = Some(opts.clone()); + app.merge.failed_options = Some((ids, opts.clone())); } } } @@ -285,7 +290,7 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) { let Some(t) = app.merge.preview_task.take() else { return }; if e != "cancelled" { app.merge.error = Some(e); - app.merge.failed_options = Some(t.options); + app.merge.failed_options = Some((t.ids, t.options)); } } Err(_) => ctx.request_repaint_after(std::time::Duration::from_millis(50)), @@ -362,7 +367,7 @@ fn added_message(what: &str, info: &Value) -> String { } fn last_failed_matches(app: &LightcraftApp, opts: &MergeDialog) -> bool { - app.merge.failed_options.as_ref() == Some(opts) + app.merge.failed_options.as_ref().is_some_and(|(ids, o)| ids == &app.merge.ids && o == opts) } /// The dialog body (options on the left, preview on the right). diff --git a/crates/ui-egui/src/tests_activity.rs b/crates/ui-egui/src/tests_activity.rs index bce9d2eb1..9fa533145 100644 --- a/crates/ui-egui/src/tests_activity.rs +++ b/crates/ui-egui/src/tests_activity.rs @@ -394,6 +394,31 @@ fn merge_dialog_remembers_the_last_settings_per_kind() { let _ = std::fs::remove_dir_all(dir); } +/// Issue #2708: a preview that failed (a photo's file missing) is tried again when the dialog opens again; it used to +/// show neither a preview nor an error until an option changed. +#[test] +fn merge_dialog_retries_a_failed_preview_when_it_opens_again() { + let (mut h, dir) = bracket("merge-retry"); + let (file, away) = (dir.join("IMG_1.dng"), dir.join("IMG_1.away")); + std::fs::rename(&file, &away).unwrap(); + crate::merge::open(&mut h.app, "merge.panorama").unwrap(); + h.step(); + assert!(h.step_until(T, |h| h.app.merge.preview_task.is_none())); + assert!(h.app.merge.error.is_some(), "the preview failed"); + h.app.ui.dialog = None; + h.step(); + // the file is back (a drive reconnected); the same photos and options again + std::fs::rename(&away, &file).unwrap(); + crate::merge::open(&mut h.app, "merge.panorama").unwrap(); + h.step(); + assert!(h.app.merge.preview_task.is_some() || h.app.merge.error.is_some(), "the dialog shows a preview being made, or why not"); + assert!(h.step_until(T, |h| h.app.merge.preview_task.is_none())); + assert!(h.app.merge.preview.is_some(), "{:?}", h.app.merge.error); + h.app.ui.dialog = None; + h.step(); + let _ = std::fs::remove_dir_all(dir); +} + /// HDR for Each Bracket Set runs the merges one after the other, each with its activity row; cancelling one stops the /// rest. #[test] diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 21140ae26..4b038ffbd 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-23 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO: the dialog makes a new preview each time it opens, also after a failed one, #2708; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -353,7 +353,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| | LR-MERGE-HDR | HDR merge | P2 | 🟡 | `cmd:merge.hdr`, `cmd:dialog.mergeHdr`, `crates/merge/src/frame.rs` (`Frame::lens`), `crates/merge/src/hdr.rs` | auto align (Lanczos-3 warp), deghost None–High + overlay, auto settings, float DNG, Create Stack; the DNG keeps the reference frame's embedded lens corrections (tested by `hdr_merge_keeps_the_reference_frames_lens_corrections`); raws without colour matrices of their own (CR2, NEF, ARW…) keep the colour they open with, written as DNG tags (`starting_colour_tags`; the look's chroma curve has no tag; panoramas too), issue #965; the DNG's `AsShotNeutral` is the median of the inputs' vendor white-balance multipliers, issue #1144. Tested on real Sony A7R VI brackets (67 MP, 2026-10-14): frames agree with the merged radiance within ~1 % in all channels, exposure steps match EXIF within 0.03 EV. A clipped highlight of the reference that moved in other frames (a star at 200 mm) stays instead of becoming a black hole, and deghost (≤ 2048 px maps) catches a brighter frame's ghost in a dark sky (`a_clipped_star_of_the_reference_is_not_erased_by_frames_where_it_moved`, `deghost_removes_a_bright_frames_ghost_in_a_dark_sky`). 🟡 because: AI Denoise doesn't run on merged DNGs (Bayer only; Lightroom's does), JPEG brackets are treated as linear (no camera response curve), and every frame is held as 32-bit float (~0.86 GB per 67 MP frame, so 7 frames need ~7 GB); gap 29 | -| LR-MERGE-PANO | Panorama | P2 | ✅ | `cmd:merge.panorama`, `cmd:dialog.mergePanorama` | spherical/cylindrical/perspective + auto (projected with Lanczos-3), boundary warp, auto crop, fill edges (diffusion), DNG whose `AsShotNeutral` is the median of its frames' as-shot white balance (vendor multipliers for CR2, NEF, ARW…), issue #1144; no lens model / 360° wrap | +| LR-MERGE-PANO | Panorama | P2 | ✅ | `cmd:merge.panorama`, `cmd:dialog.mergePanorama` | spherical/cylindrical/perspective + auto (projected with Lanczos-3), boundary warp, auto crop, fill edges (diffusion), DNG whose `AsShotNeutral` is the median of its frames' as-shot white balance (vendor multipliers for CR2, NEF, ARW…), issue #1144; the dialog makes a new preview each time it opens, also after a failed one (issue #2708, `merge_dialog_retries_a_failed_preview_when_it_opens_again`); no lens model / 360° wrap | | LR-MERGE-HDRPANO | HDR panorama | P2 | ✅ | `cmd:merge.hdrPanorama`, `cmd:dialog.mergeHdrPanorama` | brackets grouped by EXIF | | LR-MERGE-HEADLESS | Merge with last settings | P2 | ✅ | `cmd:merge.hdrLast`, `cmd:merge.panoramaLast`, `cmd:merge.hdrPanoramaLast`, `cmd:merge.hdrBatch`, `crates/catalog/src/stacks.rs` (`bracket_sets`) | no dialog; the options of the last merge of that kind (defaults the first time), kept in the library's prefs.json so they survive a restart; the HDR / Panorama / HDR Panorama dialogs open with them too (`merge_dialog_remembers_the_last_settings_per_kind`). HDR for Each Bracket Set merges a batch like Lightroom's merge of stacks: one HDR per stack among the photos, then per bracket (photos taken less than a second apart, after the shutter time, at exposures ≥ ½ EV apart; bursts at one exposure are skipped), one after the other in the activity stack (cancelling one stops the rest; a failed set doesn't); CLI `merge hdr-batch FILES…` | @@ -798,6 +798,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-23 | patch | LR-MERGE-HDR / LR-MERGE-PANO: the merge dialog makes a new preview each time it opens, also after a failed one (#2708); status unchanged | | 2026-10-21 | patch | LR-IMP-ADD-DIALOG: reconcile #2362 with current main; deleted-album guard and hierarchical picker retained; counts unchanged | | 2026-10-20 | minor | LR-IMP-ADD-DIALOG: #2362 gives the import review the shared hierarchy/search album picker while retaining None/New album and existing destination IDs; status and feature counts unchanged | | 2026-10-21 | patch | KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the slider's row; leaving it (even between two frames) or Tab focus alone does not arm them (issue #1355); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..bc0da0188 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-23 + +### Merge previews after a failed one +- The HDR, Panorama and HDR Panorama dialogs make a new preview each time they open (issue #2708). After a preview + had failed once, for example because a photo's file was missing, opening the dialog again with the same options + showed neither a preview nor an error until an option was changed. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges From 650edf66f8d524c7c01f9fb8125a37db6bab1e36 Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 22:27:28 +0100 Subject: [PATCH 03/25] LR-EXP-METADATA: refuse Originals with embedded place names Co-authored-by: Codex --- crates/engine/src/export.rs | 9 +- crates/engine/src/tests_export.rs | 147 ++++++++++++++++++++++++++++++ docs/gaps.md | 3 +- docs/parity-checklist.md | 5 +- docs/whats-new.md | 3 + 5 files changed, 162 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index b1a5bce0f..53279ccc5 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -1837,9 +1837,14 @@ impl PreparedExport { if remove_location { let metadata = lightcraft_meta::try_extract(&bytes) .map_err(|e| format!("{label}: can't verify location metadata in this original file: {e}"))?; - if metadata.gps.is_some() { + let embeds_place_name = + [metadata.sublocation.as_deref(), metadata.city.as_deref(), metadata.state.as_deref(), metadata.country.as_deref()] + .into_iter() + .flatten() + .any(|value| !value.trim().is_empty()); + if metadata.gps.is_some() || embeds_place_name { return Err(format!( - "{label}: can't remove the location from an original file that embeds GPS; export as JPEG, TIFF or DNG instead" + "{label}: can't remove the location from an original file that embeds GPS or place names; export as JPEG, TIFF or DNG instead" )); } } diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index d0f174245..1df6702a8 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -6,6 +6,14 @@ use crate::Session; use crate::export::{ExportFormat, ExportOptions, export_photo}; use crate::tests_xmp::{synthetic_dng_with, synthetic_psd_with_metadata, temp_dir}; +fn jpeg_with_xmp(metadata: &lightcraft_meta::Metadata) -> Vec { + let pixels = [96u8; 12]; + let image = lightcraft_codecs::EncodeImage::new(2, 2, 3, lightcraft_codecs::Samples::U8(&pixels)); + let xmp = lightcraft_meta::write_xmp(metadata, None); + let encoded = lightcraft_codecs::EncodeMeta { xmp: Some(&xmp), ..Default::default() }; + lightcraft_codecs::encode_jpeg(&image, 90, lightcraft_codecs::ChromaSubsampling::S444, &encoded).unwrap() +} + #[test] fn full_size_export_of_a_cropped_photo_is_not_upscaled() { let mut s = Session::with_demo(); @@ -640,6 +648,145 @@ fn remove_location_holds_for_original_and_dng_exports() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn remove_location_refuses_original_with_embedded_place_names() { + let dir = temp_dir("export-noloc-place-fields"); + let value = "Embedded Place"; + let cases = [ + ("sublocation", lightcraft_meta::Metadata { sublocation: Some(value.into()), ..Default::default() }), + ("city", lightcraft_meta::Metadata { city: Some(value.into()), ..Default::default() }), + ("state", lightcraft_meta::Metadata { state: Some(value.into()), ..Default::default() }), + ("country", lightcraft_meta::Metadata { country: Some(value.into()), ..Default::default() }), + ]; + let mut paths = Vec::new(); + for (name, metadata) in &cases { + let path = dir.join(format!("{name}.jpg")); + std::fs::write(&path, jpeg_with_xmp(metadata)).unwrap(); + paths.push(path.to_string_lossy().to_string()); + } + + let mut session = Session::new().with_fs(); + session.execute("library.import", &json!({"paths": paths})).unwrap(); + let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() }; + for (name, _) in cases { + let photo_id = session.catalog.photos().find(|p| p.file_name == format!("{name}.jpg")).unwrap().id; + let original = std::fs::read(dir.join(format!("{name}.jpg"))).unwrap(); + let embedded = lightcraft_meta::try_extract(&original).unwrap(); + assert!(embedded.gps.is_none(), "{name} fixture should isolate place names"); + let place_name = match name { + "sublocation" => embedded.sublocation.as_deref(), + "city" => embedded.city.as_deref(), + "state" => embedded.state.as_deref(), + "country" => embedded.country.as_deref(), + _ => None, + }; + assert_eq!(place_name, Some(value), "{name} fixture should be parsed"); + let error = export_photo(&mut session, photo_id, &options, 1).err().expect("embedded place name should refuse Original export"); + assert!(error.contains("place names") && error.contains("export as JPEG"), "{name}: {error}"); + } + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn remove_location_original_place_refusal_writes_no_files() { + use crate::export::{Destination, prepare_batch, run_batch}; + + let dir = temp_dir("export-noloc-place-no-write"); + let source = dir.join("City.jpg"); + let original = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some("Embedded Place".into()), ..Default::default() }); + std::fs::write(&source, &original).unwrap(); + let output = dir.join("out"); + std::fs::create_dir_all(&output).unwrap(); + + let mut session = Session::new().with_fs(); + session.execute("library.import", &json!({"paths": [source.to_string_lossy()]})).unwrap(); + let id = session.catalog.photos().next().unwrap().id; + let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() }; + let items = prepare_batch(&mut session, &[id], &options).unwrap(); + let destination = Destination { dir: output.to_string_lossy().to_string(), exact: None }; + let mut writes = Vec::new(); + let mut write = |path: &str, bytes: &[u8]| -> Result<(), String> { + writes.push(path.to_string()); + std::fs::write(path, bytes).map_err(|error| error.to_string()) + }; + let exists = |path: &str| std::path::Path::new(path).exists(); + let result = run_batch(items, &options, &destination, &mut write, &exists, true, &mut |_, _| true); + + let error = result.unwrap_err(); + assert!(error.contains("place names"), "{error}"); + assert!(writes.is_empty(), "refused export called the writer: {writes:?}"); + assert!(!output.join("City.jpg").exists()); + assert!(!output.join("City.xmp").exists()); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn remove_location_checks_embedded_places_after_catalog_clear() { + let dir = temp_dir("export-noloc-place-catalog-clear"); + let source = dir.join("City.jpg"); + let original = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some("Embedded Place".into()), ..Default::default() }); + std::fs::write(&source, &original).unwrap(); + + let mut session = Session::new().with_fs(); + session.execute("library.import", &json!({"paths": [source.to_string_lossy()]})).unwrap(); + let id = session.catalog.photos().next().unwrap().id; + assert_eq!(session.catalog.photo(id).unwrap().meta.city, "Embedded Place"); + session.execute("photo.setMeta", &json!({"ids": [id.0], "city": ""})).unwrap(); + assert!(session.catalog.photo(id).unwrap().meta.city.is_empty()); + assert_eq!(lightcraft_meta::try_extract(&std::fs::read(&source).unwrap()).unwrap().city.as_deref(), Some("Embedded Place")); + + let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() }; + let error = export_photo(&mut session, id, &options, 1).err().expect("embedded place name should refuse Original export"); + assert!(error.contains("place names"), "{error}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn remove_location_allows_original_without_embedded_location() { + let dir = temp_dir("export-noloc-place-controls"); + let embedded_path = dir.join("Embedded.jpg"); + let no_location_path = dir.join("NoLocation.jpg"); + let blank_path = dir.join("Blank.jpg"); + let embedded = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some("Embedded Place".into()), ..Default::default() }); + let no_location = jpeg_with_xmp(&Default::default()); + let blank = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some(" \t".into()), ..Default::default() }); + std::fs::write(&embedded_path, &embedded).unwrap(); + std::fs::write(&no_location_path, &no_location).unwrap(); + std::fs::write(&blank_path, &blank).unwrap(); + assert!(lightcraft_meta::try_extract(&blank).unwrap().city.is_none(), "whitespace-only XMP place names are blank"); + + let mut session = Session::new().with_fs(); + session + .execute( + "library.import", + &json!({"paths": [embedded_path.to_string_lossy(), no_location_path.to_string_lossy(), blank_path.to_string_lossy()]}), + ) + .unwrap(); + let photo_id = |name: &str| session.catalog.photos().find(|p| p.file_name == name).unwrap().id; + let embedded_id = photo_id("Embedded.jpg"); + let no_location_id = photo_id("NoLocation.jpg"); + let blank_id = photo_id("Blank.jpg"); + + let keep = ExportOptions { format: ExportFormat::Original, ..Default::default() }; + let kept = export_photo(&mut session, embedded_id, &keep, 1).unwrap(); + assert_eq!(kept.bytes, embedded, "removeLocation=false still copies the source byte for byte"); + assert_eq!(lightcraft_meta::try_extract(&kept.bytes).unwrap().city.as_deref(), Some("Embedded Place")); + + let remove = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() }; + for (id, source) in [(no_location_id, &no_location), (blank_id, &blank)] { + let exported = export_photo(&mut session, id, &remove, 1).unwrap(); + assert_eq!(&exported.bytes, source); + } + + session.execute("photo.setMeta", &json!({"ids": [no_location_id.0], "city": "Catalog-only Place"})).unwrap(); + let catalog_only = export_photo(&mut session, no_location_id, &remove, 1).unwrap(); + let sidecar = catalog_only.sidecars.first().unwrap().1.as_slice(); + let packet = std::str::from_utf8(sidecar).unwrap(); + assert!(lightcraft_meta::parse_xmp(packet).unwrap().metadata.city.is_none(), "catalog-only place name is removed from the sidecar"); + + let _ = std::fs::remove_dir_all(&dir); +} + /// `metadata: "NONE"` as `app.export` sends it: the JPEG carries no location or camera data /// (it used to be read as `all` and carry both). #[test] diff --git a/docs/gaps.md b/docs/gaps.md index ceea3724f..fbef7abc8 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps: where LightCraft falls short of Lightroom -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-23 · **Change:** minor (gap 13: before a Lightroom catalog import the app asks merge or new library, #2672; gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains; gap 28: ⌘L Enable Filters switches the filter off and on; gap 15: Japanese photo and postcard paper sizes in the Print page setup and PDF) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-24 · **Change:** patch (LR-EXP-METADATA: Original export refuses embedded GPS or nonblank place names when Remove Location Info is on; readiness, ranked gaps and estimates unchanged) Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is @@ -84,6 +84,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-24 | patch | LR-EXP-METADATA: Original export refuses embedded GPS or nonblank place names when Remove Location Info is on; ranked gaps, estimates and readiness unchanged | | 2026-10-23 | minor | Gap 13: before a Lightroom catalog import the app asks whether it merges into the open library or goes into a new one (opt-out in Settings, #2672); hours unchanged | | 2026-10-22 | patch | Gap 28: View shows the current grid-cycle key beside Photo Grid and Square Grid (#2500); no new binding and no change to the remaining shortcut gap or hours | | 2026-10-22 | minor | Gap 15: the Print page setup and PDF gain the Japanese photo and postcard paper sizes (L判 89×127, 2L判 127×178, KG判 102×152, はがき 100×148, 往復はがき 148×200, 六切 203×254, 四切 254×305, 名刺 55×91 mm), by their customary names; gap not closed | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d059494ce..60da38e94 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-24 · **Change:** minor (LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-24 · **Change:** patch (LR-EXP-METADATA: Original export refuses embedded GPS or nonblank place names when Remove Location Info is on; statuses and feature counts unchanged; see revision history for earlier updates) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -398,7 +398,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EXP-COLORSPACE | Output colour space | P0 | ✅ | `cmd:app.export` (`colorSpace`), `crates/pipeline/src/output.rs`, `crates/engine/src/export.rs` | sRGB, Display P3, Adobe RGB (1998) compatible, ProPhoto RGB, Rec. 2020: rendered from the working space with gamut mapping into the target gamut (CPU + GPU), own ICC profile embedded; AVIF stays sRGB (muxer has no ICC); lossy JPEG XL declares sRGB / P3 / Rec. 2020 (Adobe RGB and ProPhoto written as Rec. 2020), lossless JPEG XL embeds the profile | | LR-EXP-HDR | HDR output | P2 | 🟡 | `cmd:app.export`, `crates/ui-egui/src/panels/dialogs.rs` | HDR output checkbox in the export dialog: gain map JPEG, PQ or HLG AVIF, PQ PNG, PQ JPEG XL, float TIFF; no HDR presets yet | | LR-EXP-SHARPEN | Output sharpening | P1 | ✅ | `cmd:app.export` (`sharpen`, `sharpenAmount`) | | -| LR-EXP-METADATA | Metadata policy | P1 | ✅ | `cmd:app.export` (`metadata`, `removeLocation`) | | +| LR-EXP-METADATA | Metadata policy | P1 | ✅ | `cmd:app.export` (`metadata`, `removeLocation`) | Original export is refused when Remove Location Info is on and the source embeds GPS or a nonblank sublocation, city, state or country; rendered exports and XMP sidecars remove those modeled fields | | LR-EXP-WATERMARK | Watermark | P1 | 🟡 | `cmd:app.export` (`watermark`: text, or `{text, vertical, size, opacity, anchor, insetX, insetY, inset, rotation, color, shadow, image, imageWidth}`), `crates/engine/src/export.rs` (`Watermark`, `WATERMARK_PARAMS`) | text uses Inter SemiBold, the existing Mincho-first CJK chain, then Thai faces from `CRAFT_FONTS_DIR`, shared with contact-sheet captions and slideshow overlays (#2052, partial). Horizontal Thai mark positioning remains incomplete. Arabic export fallback is deferred until joining and bidi reordering exist; UI font fallback is unchanged. Text (`size` = height as a fraction of the short edge, 0.005..0.5, default 0.035; `color` sRGB `[r,g,b]`; `shadow`; Japanese vertical glyph forms and origins, tested by `biz_ud_vertical_watermarks_place_punctuation_at_the_top_right`; extended grapheme cells keep decomposed dakuten/handakuten and Latin accents together, tested by `vertical_watermarks_keep_combining_marks_in_one_cell` and `upright_watermark_cell_retains_multiple_glyphs` (see `docs/vertical-watermarks.md`)) or a graphic with transparency (EXIF orientation applied before sizing; oversized graphics fit the rotated image bounds without stretching, tested by `graphic_watermark_honours_exif_orientation` and `graphic_watermark_keeps_aspect_when_fitted_and_rotated`; `image` path, `imageWidth` = fraction of the photo's width, 0.01..1, converted to the output colour space); `anchor` (nine positions), `insetX` / `insetY` (margins from the side and from the top/bottom edges, each a fraction of the short edge, 0..0.4; the Export dialog's Horizontal and Vertical inset sliders, as in Lightroom Classic; the older single `inset` still reads as both; tested by `watermark_insets_are_horizontal_and_vertical`), `rotation` (0, 90, 180 or 270 degrees clockwise, the Export dialog's Rotate row, as Lightroom Classic's rotate buttons; sized before it is turned, placed and inset by the turned box; tested by `watermark_rotation_turns_the_text_in_its_corner` and `graphic_watermark_is_placed_scaled_and_blended`), `opacity` 0..1. Unknown keys and out-of-range values are errors (issue #183). The Export dialog previews it on the first exported photo's thumbnail (`draw_watermark_preview` scales the 6 px minimum text size to the preview so it matches the file), tested by `watermark_preview_scales_the_minimum_text_size` and `export_dialog_previews_the_watermark` | | LR-EXP-NAMING | File naming | P1 | ✅ | `cmd:app.export` (`naming`, `startNumber`), `crates/engine/src/rename.rs` (`expand_tokens`) | one template language for export, Rename Photos and import renaming: `{name}`, `{num}` (number at the end of the name), `{seq}` / `{seq:N}`, `{date}` / `{date:%Y-%m-%d}`, `{folder}`, `{camera}`, `{lens}`, `{iso}`, `{rating}`, `{title}`, `{creator}`, `{ext}`; free-form template rather than a list of named schemes | | LR-EXP-LOCATION | Destination folder | P0 | ✅ | `cmd:app.export` (`dir`, `destinationMode`: specificFolder / sameAsOriginal, `subfolder`, `conflict`: unique / overwrite / skip), `cmd:export.checkTarget`, `crates/ui-egui/src/panels/dialogs.rs` (Choose… folder picker), `crates/engine/src/originals.rs`, `crates/catalog/src/safe_file.rs` | folder picker on the desktop; Export to: Specific folder or Same folder as original photo (each photo into its original's folder + the subfolder; a demo/browser photo or a missing folder is that photo's error, issue #956); existing files get `-2`, `-3`… by default; the policy covers a file and its sidecars together; a catalogued original (or its sidecar) is never written over, whatever the policy or an exact `path` (export, `ui.render`, MCP `render_photo`, merge `previewPath`, `lightcraft-cli render`); files are written to a temp file and renamed into place (issue #93; not synced file by file — exports can be made again, issue #134) | @@ -799,6 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-24 | patch | LR-EXP-METADATA: Original export refuses embedded GPS or nonblank place names when Remove Location Info is on; status and feature counts unchanged | | 2026-10-24 | patch | LR-LIB-RATING, LR-LIB-DELETE: a slider drag still open when a rating, flag or label takes the photo out of the view, or when the photo is deleted, ends as its own undo step before the selection moves on, as moving to the next photo does; LR-BEHAV-LOCALIZE lists all 18 languages; status unchanged | | 2026-10-24 | patch | Integration batch 43: Integrated main 98702b5c, preserving the new fail-closed GPS metadata check for Original export. Watermark geometry/orientation and HDR JPEG size-limit fixes remain in their shared helpers; no feature-count changes. | | 2026-10-23 | patch | Export integration: Integrated main 5ca49b3e, retaining Width & Height resizing, export format/bit-depth validation and batch panic cancellation alongside the existing watermark and HDR JPEG size-limit fixes. No feature-count changes. | diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..92a640d56 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,9 @@ ## 2026-10-24 +### Original export checks embedded place names when removing location +- Original copies are refused when “Remove Location Info” is enabled and the source contains GPS or a nonblank sublocation, city, state or country. Use a rendered format or DNG to remove that embedded location. + ### Relative-date filters and smart albums follow the clock - A current view or source count using “in the last” rules now refreshes when the session clock moves into a new minute, even when no catalog edit occurred. This fixes stale photos remaining in the grid and stale counts for From 77471ca376d95ba621aa0558806e29ca98673577 Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:18:57 +0100 Subject: [PATCH 04/25] LR-EXP-METADATA: reject Originals with unreadable XMP Co-authored-by: Codex --- crates/engine/src/export.rs | 10 +++++++++- crates/engine/src/tests_export.rs | 25 ++++++++++++++++++++++++- docs/whats-new.md | 2 +- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index 53279ccc5..4c593beab 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -1837,6 +1837,14 @@ impl PreparedExport { if remove_location { let metadata = lightcraft_meta::try_extract(&bytes) .map_err(|e| format!("{label}: can't verify location metadata in this original file: {e}"))?; + if !bytes.starts_with(b"8BPS") { + let embedded = lightcraft_meta::embedded(&bytes); + if let Some(xmp) = embedded.xmp.as_deref() { + lightcraft_meta::parse_xmp(xmp).map_err(|e| { + format!("{label}: can't verify location metadata in this original file: malformed embedded XMP: {e}") + })?; + } + } let embeds_place_name = [metadata.sublocation.as_deref(), metadata.city.as_deref(), metadata.state.as_deref(), metadata.country.as_deref()] .into_iter() @@ -1844,7 +1852,7 @@ impl PreparedExport { .any(|value| !value.trim().is_empty()); if metadata.gps.is_some() || embeds_place_name { return Err(format!( - "{label}: can't remove the location from an original file that embeds GPS or place names; export as JPEG, TIFF or DNG instead" + "{label}: can't remove the location from an original file that embeds GPS or place names; export as JPEG or TIFF, or as DNG for a raw photo" )); } } diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 1df6702a8..c0377409f 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -682,11 +682,34 @@ fn remove_location_refuses_original_with_embedded_place_names() { }; assert_eq!(place_name, Some(value), "{name} fixture should be parsed"); let error = export_photo(&mut session, photo_id, &options, 1).err().expect("embedded place name should refuse Original export"); - assert!(error.contains("place names") && error.contains("export as JPEG"), "{name}: {error}"); + assert!(error.contains("place names") && error.contains("export as JPEG or TIFF"), "{name}: {error}"); } let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn remove_location_refuses_original_with_unreadable_non_psd_xmp() { + let dir = temp_dir("export-noloc-malformed-xmp"); + let path = dir.join("Unreadable.jpg"); + let pixels = [96u8; 12]; + let image = lightcraft_codecs::EncodeImage::new(2, 2, 3, lightcraft_codecs::Samples::U8(&pixels)); + let xmp = " Date: Sun, 25 Oct 2026 12:12:11 +0800 Subject: [PATCH 05/25] docs(linux): document rustc 1.99 ThinLTO SIGILL workaround (#2811) --- README.md | 30 ++++++++++++++++++++++++++++++ README.zh-hans.md | 28 ++++++++++++++++++++++++++++ README.zh-hant.md | 28 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/README.md b/README.md index 78b3b3234..0706d4901 100644 --- a/README.md +++ b/README.md @@ -451,6 +451,36 @@ information combined with LTO and one codegen unit on low-memory builders. A PKG its own profile settings can override inherited variables; adjust the packaging recipe locally or contact its maintainer rather than assuming an environment prefix on `makepkg` will take precedence. +**Linux/Arch build troubleshooting: rustc SIGILL with ThinLTO.** +In [#2811](https://github.com/storytold/lightcraft/issues/2811), an Arch Linux x86_64 build on an +AMD Ryzen 9 7900X3D with 64 GB RAM failed with rustc 1.99.0 / LLVM 23.1.1, AUR `lightcraft` +`0.7.1-1` and `0.13.0-1`, `RUSTFLAGS="-C opt-level=3 -C target-cpu=native"`, and release settings +`opt-level=3`, `lto=thin`, `codegen-units=1`. `cargo build --frozen --release` reached the executable +crates before rustc crashed with signal 4 (`SIGILL` / `ILL_ILLOPN`). The core dumps point into LLVM's +SelectionDAG / vector-combine optimization path; the exact compiler root cause has not been isolated. +This does not mean every machine with that hardware or toolchain will fail. It is a different observed +failure from the 16 GB OOM kills in [#1196](https://github.com/storytold/lightcraft/issues/1196) above; +there is no evidence establishing a shared root cause. + +For a direct Cargo source build, the equivalent command-local LTO override is: + +```sh +CARGO_PROFILE_RELEASE_LTO=off cargo build --frozen --release +``` + +Reporter @rwarmstr verified the workaround by exporting `CARGO_PROFILE_RELEASE_LTO=off` **inside the +AUR PKGBUILD's `_srcenv()` function** and rebuilding both package versions, without changing +`RUSTFLAGS`. The reporter tested the AUR recipe, not the direct Cargo invocation above. +A variable prefixed to `makepkg` alone may be overwritten by the PKGBUILD's own exports; the recipe +must honor or set the override. The AUR `!lto` opt-out behavior is tracked separately in +[#2813](https://github.com/storytold/lightcraft/issues/2813). + +[Cargo's LTO documentation](https://doc.rust-lang.org/cargo/reference/profiles.html#lto) distinguishes +`false` from `off`: `false` can still perform thin local LTO across codegen units (unless +`codegen-units=1` or `opt-level=0`); `off` disables LTO explicitly. The low-memory recipe above is +unchanged. This workaround is for affected builds, not a new default; disabling LTO may change +performance, binary size and build behavior. + Tests that need a GPU, the corpus, the craft-fonts faces or a model pass without checking anything when those are missing (libtest has no "skipped" status), so `cargo xtask ci` lists them: tests record a skip with `lightcraft_testkit::skip!`, and the run ends with what the machine offered (OS, CPUs, RAM, GPU diff --git a/README.zh-hans.md b/README.zh-hans.md index 2800db410..cc7bd6a0b 100644 --- a/README.zh-hans.md +++ b/README.zh-hans.md @@ -384,6 +384,34 @@ CARGO_BUILD_JOBS=1 CARGO_PROFILE_RELEASE_DEBUG=0 CARGO_PROFILE_RELEASE_LTO=false 设置可以覆盖继承的环境变量；应在本地调整打包配方或联系维护者，而不要假定给 `makepkg` 添加环境变量前缀就能优先生效。 +**Linux/Arch 构建故障：rustc 在 ThinLTO 阶段发生 SIGILL。** +在 [#2811](https://github.com/storytold/lightcraft/issues/2811) 中，报告者的 Arch Linux x86_64 +机器配备 AMD Ryzen 9 7900X3D 和 64 GB RAM，使用 rustc 1.99.0 / LLVM 23.1.1、AUR +`lightcraft` `0.7.1-1` 和 `0.13.0-1`、`RUSTFLAGS="-C opt-level=3 -C target-cpu=native"`， +以及 release 设置 `opt-level=3`、`lto=thin`、`codegen-units=1`。 +`cargo build --frozen --release` 在编译到可执行 crate 时，rustc 因 signal 4 +（`SIGILL` / `ILL_ILLOPN`）崩溃。core dump 指向 LLVM 的 SelectionDAG / vector-combine +优化路径；编译器的具体根因尚未确定。这不表示所有使用这些硬件或工具链的机器都会失败。 +此故障与上文 [#1196](https://github.com/storytold/lightcraft/issues/1196) 中 16 GB 机器上的 +OOM 终止表现不同，没有证据表明两者根因相同。 + +直接使用 Cargo 从源码构建时，等价的单条命令 LTO 覆盖方式是： + +```sh +CARGO_PROFILE_RELEASE_LTO=off cargo build --frozen --release +``` + +报告者 @rwarmstr 验证成功的是：在 **AUR PKGBUILD 的 `_srcenv()` 函数内部**导出 +`CARGO_PROFILE_RELEASE_LTO=off` 后重新构建上述两个版本，无需更改 `RUSTFLAGS`。 +报告者测试的是 AUR 配方，并未单独验证上面的直接 Cargo 命令。仅在 `makepkg` 前添加变量， +可能会被 PKGBUILD 自己的 export 覆盖；打包配方必须保留或设置该覆盖值。 +AUR 的 `!lto` 选项行为由 [#2813](https://github.com/storytold/lightcraft/issues/2813) 单独跟踪。 + +[Cargo 的 LTO 文档](https://doc.rust-lang.org/cargo/reference/profiles.html#lto) 区分了 `false` +和 `off`：`false` 仍可能在代码生成单元之间执行 thin local LTO（除非 `codegen-units=1` +或 `opt-level=0`）；`off` 则明确禁用 LTO。上文的低内存配方保持不变。 +此绕过方法仅针对受影响的构建，不是新的默认建议；禁用 LTO 可能改变性能、二进制大小和构建行为。 + **中文和日文文字**需要共享字体仓库，这是一个可选的构建输入（所有正式发布版都会包含它）： ```sh diff --git a/README.zh-hant.md b/README.zh-hant.md index 12fcfb11b..2325e5ee6 100644 --- a/README.zh-hant.md +++ b/README.zh-hant.md @@ -402,6 +402,34 @@ CARGO_BUILD_JOBS=1 CARGO_PROFILE_RELEASE_DEBUG=0 CARGO_PROFILE_RELEASE_LTO=false PKGBUILD 可能會覆寫繼承的環境變數；請在本機調整打包腳本或聯絡維護者，而不要假設替 `makepkg` 加上環境變數前綴就能優先生效。 +**Linux/Arch 建置故障：rustc 在 ThinLTO 階段發生 SIGILL。** +在 [#2811](https://github.com/storytold/lightcraft/issues/2811) 中，回報者的 Arch Linux x86_64 +機器配備 AMD Ryzen 9 7900X3D 與 64 GB RAM，使用 rustc 1.99.0 / LLVM 23.1.1、AUR +`lightcraft` `0.7.1-1` 與 `0.13.0-1`、`RUSTFLAGS="-C opt-level=3 -C target-cpu=native"`， +以及 release 設定 `opt-level=3`、`lto=thin`、`codegen-units=1`。 +`cargo build --frozen --release` 在編譯到可執行 crate 時，rustc 因 signal 4 +（`SIGILL` / `ILL_ILLOPN`）當機。core dump 指向 LLVM 的 SelectionDAG / vector-combine +最佳化路徑；編譯器的具體根因尚未確定。這不表示所有使用這些硬體或工具鏈的機器都會失敗。 +此故障與上文 [#1196](https://github.com/storytold/lightcraft/issues/1196) 中 16 GB 機器上的 +OOM 終止表現不同，沒有證據顯示兩者根因相同。 + +直接使用 Cargo 從原始碼建置時，等價的單條指令 LTO 覆寫方式是： + +```sh +CARGO_PROFILE_RELEASE_LTO=off cargo build --frozen --release +``` + +回報者 @rwarmstr 驗證成功的是：在 **AUR PKGBUILD 的 `_srcenv()` 函式內部**匯出 +`CARGO_PROFILE_RELEASE_LTO=off` 後重新建置上述兩個版本，無須變更 `RUSTFLAGS`。 +回報者測試的是 AUR 腳本，並未單獨驗證上面的直接 Cargo 指令。僅在 `makepkg` 前加上變數， +可能會被 PKGBUILD 自己的 export 覆寫；打包腳本必須保留或設定該覆寫值。 +AUR 的 `!lto` 選項行為由 [#2813](https://github.com/storytold/lightcraft/issues/2813) 另行追蹤。 + +[Cargo 的 LTO 文件](https://doc.rust-lang.org/cargo/reference/profiles.html#lto) 區分了 `false` +與 `off`：`false` 仍可能在程式碼產生單元之間執行 thin local LTO（除非 `codegen-units=1` +或 `opt-level=0`）；`off` 則明確停用 LTO。上文的低記憶體建置配方維持不變。 +此繞過方法僅適用於受影響的建置，不是新的預設建議；停用 LTO 可能改變效能、二進位檔大小與建置行為。 + 測試在缺少 GPU、語料庫、craft-fonts 字型或模型時會直接通過而不檢查任何內容（libtest 沒有「已跳過」 這個狀態），所以 `cargo xtask ci` 會把它們列出來：測試會用 `lightcraft_testkit::skip!` 記錄跳過原因， 執行結束時會列出機器提供的環境（作業系統、CPU、記憶體、每個後端的 GPU 配接器、語料庫、字型）以及每一項 From dc6433179b6a62be41dffbbd65669e1003898320 Mon Sep 17 00:00:00 2001 From: radialmonster Date: Sun, 25 Oct 2026 00:26:49 -0400 Subject: [PATCH 06/25] Export: note on LZW TIFF size, never larger ZIP at 16 bit (#2882) 16-bit LZW TIFFs come out larger than uncompressed ones and slower than ZIP. The export keeps LZW exactly as picked (some software reads only LZW), and says what it costs where it is chosen: - Export dialog: a note under Compression when LZW is selected, worded more strongly at 16 bit, a milder one at 8 bit and 32-bit float. - Export presets that write LZW TIFFs carry the same hint on hover, in the dialog's Preset list and the grid's Export with Preset menu. - tiffCompression docs: MCP schema and headless params, the user export guide and the parity checklist. Tests: the note shows for LZW only and under the Compression choice (headless), presets get the same hint, the dialog keeps LZW; 16-bit ZIP is never larger than uncompressed on a noisy synthetic render (noise 0 to 0.1) and on the demo photo's export. Fixes #2882 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_014ti8KemZbjE2ZgHgV1q1BM --- crates/engine/src/export.rs | 45 +++++++++ crates/engine/src/tests_export.rs | 23 +++++ crates/mcp/src/headless.rs | 2 +- crates/mcp/src/tools.rs | 2 +- crates/ui-egui/src/panels/dialogs.rs | 141 ++++++++++++++++++++++++++- crates/ui-egui/src/panels/grid.rs | 7 +- docs/parity-checklist.md | 2 +- docs/user/guides/export-photos.md | 11 ++- 8 files changed, 223 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index b1a5bce0f..571c45c76 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -3259,6 +3259,51 @@ mod tests { (v.len(), d) } + /// A 16-bit render of a photo-like scene: smooth colour gradients plus sensor-like noise of + /// `noise` (linear, about 0.002 to 0.03 for clean to high-ISO raws), the case where the low + /// byte of every 16-bit sample is mostly noise. + fn noisy_render(noise: f32) -> lightcraft_pipeline::Rendered { + let (w, h) = (512, 384); + // a hash of the position and channel: the same noise every run + let hash = |x: usize, y: usize, c: usize| { + let mut z = (x as u64) << 32 ^ (y as u64) << 2 ^ c as u64 ^ 0x2882; + z = (z ^ (z >> 33)).wrapping_mul(0xff51_afd7_ed55_8ccd); + z = (z ^ (z >> 33)).wrapping_mul(0xc4ce_b9fe_1a85_ec53); + (z ^ (z >> 33)) as f32 / u64::MAX as f32 - 0.5 + }; + let src = lightcraft_raster::Rgb32f::from_fn(w, h, |x, y| { + let (u, v) = (x as f32 / w as f32, y as f32 / h as f32); + let base = [0.05 + 0.4 * u, 0.1 + 0.3 * v, 0.2 + 0.2 * (u * v)]; + [0, 1, 2].map(|c| (base[c] + noise * hash(x, y, c)).max(0.0)) + }); + let req = lightcraft_pipeline::RenderRequest { depth: OutputDepth::U16, ..lightcraft_pipeline::RenderRequest::fit(w, h) }; + lightcraft_pipeline::render(&src, &Default::default(), &Default::default(), &req) + } + + /// Issue #2882: a 16-bit TIFF with ZIP (the default) is never larger than the same render + /// uncompressed, from smooth to noisy. (LZW can be: it is printed, not checked, and the export + /// dialog says so when it is picked.) + #[test] + fn sixteen_bit_zip_tiff_is_never_larger_than_uncompressed() { + assert_eq!(ExportOptions::default().tiff_compression, TiffCompression::Deflate, "ZIP is the default"); + for noise in [0.0, 0.002, 0.01, 0.03, 0.1] { + let r = noisy_render(noise); + assert!(matches!(r.deep.as_ref().unwrap().samples, DeepSamples::U16(_))); + let size = |tiff_compression| { + let o = ExportOptions { format: ExportFormat::Tiff, bit_depth: Some(16), tiff_compression, ..Default::default() }; + assert_eq!(o.effective_depth(), OutputDepth::U16); + encode_rendered(&r, &o, None).unwrap().len() + }; + let (none, zip, lzw) = (size(TiffCompression::None), size(TiffCompression::Deflate), size(TiffCompression::Lzw)); + eprintln!( + "noise {noise}: none {none} B, ZIP {zip} B ({:+.1} %), LZW {lzw} B ({:+.1} %)", + 100.0 * (zip as f64 / none as f64 - 1.0), + 100.0 * (lzw as f64 / none as f64 - 1.0) + ); + assert!(zip <= none, "noise {noise}: 16-bit ZIP {zip} B is larger than uncompressed {none} B"); + } + } + #[test] fn sixteen_bit_tiff_and_png_have_no_banding() { let tiff = |bit_depth| ExportOptions { format: ExportFormat::Tiff, bit_depth, ..Default::default() }; diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index d0f174245..2dc35626b 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -1203,3 +1203,26 @@ fn same_folder_as_original_refuses_photos_without_a_folder() { assert!(e.contains("folder is missing") && e.contains(&gone), "{e}"); let _ = std::fs::remove_dir_all(&dir); } + +/// Issue #2882: the demo photo's 16-bit TIFF export (2000 px long edge) with ZIP (the default) is +/// never larger than the same export uncompressed. +#[test] +fn sixteen_bit_zip_tiff_export_is_never_larger_than_uncompressed() { + use crate::export::TiffCompression; + let mut s = Session::with_demo(); + let id = s.active().unwrap(); + let size = |s: &mut Session, tiff_compression| { + let o = ExportOptions { + format: ExportFormat::Tiff, + bit_depth: Some(16), + tiff_compression, + ..ExportOptions::from_json(&json!({"longEdge": 2000})) + }; + export_photo(s, id, &o, 1).unwrap().bytes.len() + }; + let none = size(&mut s, TiffCompression::None); + let zip = size(&mut s, TiffCompression::Deflate); + let lzw = size(&mut s, TiffCompression::Lzw); + eprintln!("demo photo, 16-bit TIFF: none {none} B, ZIP {zip} B, LZW {lzw} B"); + assert!(zip <= none, "16-bit ZIP {zip} B is larger than uncompressed {none} B"); +} diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index d62a9a707..f4f32bb00 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -132,7 +132,7 @@ impl Backend for Headless { "engine.commands" => { let mut v: Vec = self.session.commands().into_iter().map(|c| serde_json::to_value(c).unwrap_or_default()).collect(); v.push(json!({"id": "app.export", "label": "Export Now", "menu": [], "shortcut": null, - "params": "{path?: output file (.jpg/.png/.tif/.webp/.avif/.jxl/.psd/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|jxl|psd|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), fit?: dimensions|widthHeight (with width + height: dimensions fits either orientation, widthHeight limits the width to `width` and the height to `height`), dontEnlarge?, ppi?, quality?: 1..100, jxlLossless?, jxlEffort?: fast|normal, jxlProgressive?, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, hdr? (HDR output for HDR edits: gain map JPEG, PQ AVIF / PNG / JPEG XL, float TIFF), sharpen?: none|screen|matte|glossy, sharpenAmount?: low|standard|high, metadata?: all|allExceptCamera|copyright|none, removeLocation?, naming?, startNumber?, destinationMode?: specificFolder|sameAsOriginal (sameAsOriginal = each photo into its original's folder; `dir` unused), subfolder?, conflict?: unique|overwrite|skip, tiffCompression?: none|lzw|zip, dngCompression?: lossless|deflate|uncompressed, addToPhotos?: bool (macOS: then add the files written to Apple Photos; outcome in `applePhotos`), photosAlbum?: top-level album name (made when missing), watermark?: text | {text?, vertical? (upright columns, right to left), size? (text height, 0.005..0.5 of the short edge; default 0.035), opacity? (0..1; 0.7), anchor?: topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight, insetX? (margin from the left/right edge, 0..0.4 of the short edge; 0.025), insetY? (from the top/bottom edge; 0.025), inset? (sets both), rotation?: 0|90|180|270 (degrees clockwise; 0), color? [r,g,b] sRGB, shadow?, image? (graphic drawn instead of the text), imageWidth? (0.01..1 of the photo's width; 0.2)}} — an unknown parameter, an out-of-range watermark size or a value of the wrong kind is an error, not a default", + "params": "{path?: output file (.jpg/.png/.tif/.webp/.avif/.jxl/.psd/.dng) | dir?, ids?, preset?, format?: jpeg|png|tiff|webp|avif|jxl|psd|dng|original, longEdge?|shortEdge?|width?|height?|megapixels?|percent? (default longEdge 3000; longEdge 0 = full size), fit?: dimensions|widthHeight (with width + height: dimensions fits either orientation, widthHeight limits the width to `width` and the height to `height`), dontEnlarge?, ppi?, quality?: 1..100, jxlLossless?, jxlEffort?: fast|normal, jxlProgressive?, limitKb?, colorSpace?: srgb|displayP3|adobeRgb|proPhoto|rec2020, bitDepth?: 8|10|16|32, hdr? (HDR output for HDR edits: gain map JPEG, PQ AVIF / PNG / JPEG XL, float TIFF), sharpen?: none|screen|matte|glossy, sharpenAmount?: low|standard|high, metadata?: all|allExceptCamera|copyright|none, removeLocation?, naming?, startNumber?, destinationMode?: specificFolder|sameAsOriginal (sameAsOriginal = each photo into its original's folder; `dir` unused), subfolder?, conflict?: unique|overwrite|skip, tiffCompression?: none|lzw|zip (default zip; lzw is usually larger and slower than zip, at 16 bit often larger than none: only for software that needs LZW), dngCompression?: lossless|deflate|uncompressed, addToPhotos?: bool (macOS: then add the files written to Apple Photos; outcome in `applePhotos`), photosAlbum?: top-level album name (made when missing), watermark?: text | {text?, vertical? (upright columns, right to left), size? (text height, 0.005..0.5 of the short edge; default 0.035), opacity? (0..1; 0.7), anchor?: topLeft|top|topRight|left|center|right|bottomLeft|bottom|bottomRight, insetX? (margin from the left/right edge, 0..0.4 of the short edge; 0.025), insetY? (from the top/bottom edge; 0.025), inset? (sets both), rotation?: 0|90|180|270 (degrees clockwise; 0), color? [r,g,b] sRGB, shadow?, image? (graphic drawn instead of the text), imageWidth? (0.01..1 of the photo's width; 0.2)}} — an unknown parameter, an out-of-range watermark size or a value of the wrong kind is an error, not a default", "enabled": self.session.active().is_some()})); Ok(Value::Array(v)) } diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 8f782bb9d..b542b2b2f 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -238,7 +238,7 @@ pub fn helper_tools(has_ui: bool) -> Vec { "destinationMode": {"type": "string", "enum": ["specificFolder", "sameAsOriginal"], "description": "specificFolder (default) = everything into `dir`; sameAsOriginal = each photo into the folder of its original (+ `subfolder`); a photo with no folder on disk (demo, browser) or whose folder is gone gets an error"}, "subfolder": {"type": "string", "description": "Created inside the destination folder"}, "conflict": {"type": "string", "enum": ["unique", "overwrite", "skip"]}, - "tiffCompression": {"type": "string", "enum": ["none", "lzw", "zip"]}, + "tiffCompression": {"type": "string", "enum": ["none", "lzw", "zip"], "description": "zip (default); lzw is usually larger and slower than zip, and at 16 bit often larger than none: use it only when other software needs LZW"}, "dngCompression": {"type": "string", "enum": ["lossless", "deflate", "uncompressed", "lossy"]}, "format": {"type": "string", "enum": ["jpeg", "png", "tiff", "psd", "webp", "avif", "jxl", "original", "dng", "rendereddng"], "description": "psd = a flat PSD (no layers); rendereddng = Render to DNG, a linear 16-bit float DNG of the rendered photo with the edit baked in; original = the file as is + an XMP sidecar with the edits; dng = raw photos as DNG with the edits embedded"}, "longEdge": {"type": "integer", "description": "Output long edge in px. When no size param is given: 3000; 0 = full size (cropped, native resolution)"}, diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index f14d7338b..dac47376c 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -798,7 +798,12 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { ui.separator(); } builtin = b; - if ui.selectable_label(false, crate::i18n::builtin_label(&p.name, b)).clicked() { + let r = ui.selectable_label(false, crate::i18n::builtin_label(&p.name, b)); + let r = match preset_lzw_note(&p.params) { + Some(note) => r.on_hover_text(crate::i18n::tr(note)), + None => r, + }; + if r.clicked() { chosen = Some(p.name); } } @@ -1031,6 +1036,11 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { if opts.format == F::Tiff { use lightcraft_engine::export::TiffCompression as Z; choices(ui, "Compression", "exportTiffCompression", &[(Z::None, "None"), (Z::Lzw, "LZW"), (Z::Deflate, "ZIP")], &mut opts.tiff_compression); + // issue #2882: LZW stays as picked, with a note on what it costs + if let Some(note) = tiff_lzw_note(opts) { + let r = ui.label(egui::RichText::new(crate::i18n::tr(note)).color(t.text_dim)); + crate::widgets::register(ui.ctx(), "note:exportTiffLzw", r.rect); + } } if opts.format == F::Dng { use lightcraft_engine::export::DngCompression as Z; @@ -2118,6 +2128,29 @@ pub fn prompt(app: &mut LightcraftApp, title: &str, hint: &str, value: &str, com } /// The Export dialog's choices as `app.export` params (without the folder). +/// The 16-bit note: LZW's byte dictionary does worse on 16-bit samples than storing them, so +/// 16-bit LZW TIFFs came out 10-33 % larger than uncompressed ones (issue #2882). +pub(crate) const TIFF_LZW_NOTE_16: &str = + "At 16 bit, LZW is often larger than no compression, and larger and slower than ZIP. Use it only if other software needs LZW."; +/// The note at 8 bit and 32-bit float: one 8-bit file in #2882 was larger with LZW too. +pub(crate) const TIFF_LZW_NOTE: &str = + "LZW is usually larger and slower than ZIP, and can be larger than no compression. Use it only if other software needs LZW."; + +/// The note shown under Compression when a TIFF export uses LZW (none otherwise). The export +/// keeps LZW: some software reads only LZW TIFFs. +pub(crate) fn tiff_lzw_note(o: &lightcraft_engine::export::ExportOptions) -> Option<&'static str> { + use lightcraft_engine::export::{ExportFormat, OutputDepth, TiffCompression}; + if o.format != ExportFormat::Tiff || o.tiff_compression != TiffCompression::Lzw { + return None; + } + Some(if o.effective_depth() == OutputDepth::U16 { TIFF_LZW_NOTE_16 } else { TIFF_LZW_NOTE }) +} + +/// [`tiff_lzw_note`] for an export preset's params: the hint on presets that write LZW TIFFs. +pub(crate) fn preset_lzw_note(params: &serde_json::Value) -> Option<&'static str> { + tiff_lzw_note(&lightcraft_engine::export::ExportOptions::from_json(params)) +} + fn export_dialog_params( opts: &lightcraft_engine::export::ExportOptions, full_size: bool, @@ -2623,3 +2656,109 @@ mod cull_dialog_tests { assert_eq!(burst(&h), (1800.0, 1.0)); } } + +#[cfg(test)] +mod tiff_lzw_note_tests { + use std::time::Duration; + + use lightcraft_engine::export::{ExportFormat, ExportOptions, TiffCompression}; + use serde_json::json; + + use super::{TIFF_LZW_NOTE, TIFF_LZW_NOTE_16, export_dialog_params, preset_lzw_note, tiff_lzw_note}; + use crate::headless::Headless; + use crate::state::Dialog; + use crate::{LightcraftApp, Services}; + + const T: Duration = Duration::from_secs(20); + + fn tiff(compression: TiffCompression, bit_depth: Option) -> ExportOptions { + ExportOptions { format: ExportFormat::Tiff, tiff_compression: compression, bit_depth, ..Default::default() } + } + + // Issue #2882: the note shows for LZW only, worded more strongly at 16 bit (TIFF's default depth) + #[test] + fn lzw_gets_a_note_stronger_at_16_bit() { + assert_eq!(tiff_lzw_note(&tiff(TiffCompression::Lzw, None)), Some(TIFF_LZW_NOTE_16), "TIFF defaults to 16-bit"); + assert_eq!(tiff_lzw_note(&tiff(TiffCompression::Lzw, Some(16))), Some(TIFF_LZW_NOTE_16)); + assert_eq!(tiff_lzw_note(&tiff(TiffCompression::Lzw, Some(8))), Some(TIFF_LZW_NOTE)); + assert_eq!(tiff_lzw_note(&tiff(TiffCompression::Lzw, Some(32))), Some(TIFF_LZW_NOTE)); + for compression in [TiffCompression::None, TiffCompression::Deflate, TiffCompression::PackBits] { + for depth in [None, Some(8), Some(16), Some(32)] { + assert_eq!(tiff_lzw_note(&tiff(compression, depth)), None, "{compression:?} {depth:?}"); + } + } + // the setting only matters for TIFF: a JPEG preset that carries it gets no note + let jpeg = ExportOptions { format: ExportFormat::Jpeg, tiff_compression: TiffCompression::Lzw, ..Default::default() }; + assert_eq!(tiff_lzw_note(&jpeg), None); + assert!(TIFF_LZW_NOTE_16.contains("16 bit") && TIFF_LZW_NOTE.contains("other software needs LZW")); + } + + // Presets that write LZW TIFFs carry the same hint; the built-in ones don't write LZW + #[test] + fn presets_with_lzw_get_the_same_hint() { + assert_eq!(preset_lzw_note(&json!({"format": "tiff", "tiffCompression": "lzw"})), Some(TIFF_LZW_NOTE_16)); + assert_eq!(preset_lzw_note(&json!({"format": "tiff", "tiffCompression": "LZW", "bitDepth": 8})), Some(TIFF_LZW_NOTE)); + assert_eq!(preset_lzw_note(&json!({"format": "tiff", "tiffCompression": "zip"})), None); + assert_eq!(preset_lzw_note(&json!({"format": "tiff"})), None, "ZIP is the default"); + for p in lightcraft_engine::export::builtin_presets() { + assert_eq!(preset_lzw_note(&p.params), None, "{}", p.name); + } + } + + // The note never changes the choice: the dialog exports exactly the compression picked + #[test] + fn the_dialog_keeps_lzw_as_picked() { + for depth in [None, Some(8), Some(16), Some(32)] { + let o = tiff(TiffCompression::Lzw, depth); + let params = export_dialog_params(&o, true, &Default::default(), 0); + assert_eq!(ExportOptions::from_json(¶ms).tiff_compression, TiffCompression::Lzw, "{depth:?}: {params}"); + } + } + + fn set(h: &mut Headless, compression: TiffCompression, bit_depth: Option) { + if let Some(Dialog::Export { opts, .. }) = &mut h.app.ui.dialog { + opts.format = ExportFormat::Tiff; + opts.tiff_compression = compression; + opts.bit_depth = bit_depth; + } else { + panic!("no Export dialog"); + } + h.settle(T); + } + + fn note_rect(h: &Headless) -> Option { + h.app.widgets.iter().find(|(w, _)| w == "note:exportTiffLzw").map(|(_, r)| *r) + } + + fn widget(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("no widget {id}")) + } + + // Scenario: in the Export dialog, picking LZW for a TIFF shows the note under Compression; ZIP + // and None show none, and the choice stays LZW + #[test] + fn export_dialog_shows_the_lzw_note_under_compression() { + let services = Services { png: None, write_shared: Some(std::sync::Arc::new(|_: &str, _: &[u8]| Ok(()))), ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1600.0, 1000.0], 1.0); + h.settle(T); + let r = h.request("engine.execute", json!({"command": "dialog.export", "params": {}}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(T); + for compression in [TiffCompression::Deflate, TiffCompression::None] { + set(&mut h, compression, None); + assert!(note_rect(&h).is_none(), "{compression:?} has no note"); + } + for depth in [None, Some(8), Some(32)] { + set(&mut h, TiffCompression::Lzw, depth); + let note = note_rect(&h).unwrap_or_else(|| panic!("LZW at {depth:?} shows the note")); + // under the Compression choice, inside the dialog + assert!(note.top() >= widget(&h, "button:exportTiffCompression-1").bottom() - 1.0, "{note:?}"); + assert!(widget(&h, "dialog:window").contains_rect(note), "{note:?}"); + let Some(Dialog::Export { opts, .. }) = &h.app.ui.dialog else { panic!("dialog closed") }; + assert_eq!(opts.tiff_compression, TiffCompression::Lzw, "the note leaves the choice alone"); + } + set(&mut h, TiffCompression::Deflate, None); + assert!(note_rect(&h).is_none(), "back to ZIP: the note goes"); + } +} diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index 883f47f8b..a904f328c 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -1413,7 +1413,12 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } crate::widgets::menu_button(ui, crate::i18n::tr("Export with Preset"), |ui| { for (p, _) in app.session.all_export_presets() { - if ui.button(&p.name).clicked() { + let r = ui.button(&p.name); + let r = match crate::panels::dialogs::preset_lzw_note(&p.params) { + Some(note) => r.on_hover_text(crate::i18n::tr(note)), + None => r, + }; + if r.clicked() { match app.run("app.export", json!({"preset": p.name, "background": true})) { Ok(r) if r.get("background").is_some() => {} Ok(r) => { diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d059494ce..c02e9c686 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -394,7 +394,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EXP-DIM | Output size | P0 | ✅ | `cmd:app.export` (`longEdge`, `shortEdge`, `width`, `height`, `fit`, `megapixels`, `percent`, `dontEnlarge`, `ppi`), `crates/engine/src/export.rs` (`Resize`), `crates/ui-egui/src/panels/dialogs.rs` | full size = the cropped native size (no longer upscaled); both of Lightroom's boxes: Dimensions (W × H fits either orientation) and Width & Height (`fit: widthHeight`, each edge bound to its own number, #1620); ppi written to JFIF / pHYs / TIFF tags; at most 65,535 px an edge, and an export enlarged past the photo's size stops at 512 megapixels (Lightroom's ceiling too) and is refused, with the size it would need, when it needs more memory than the computer has (it aborted the process before) | | LR-EXP-QUALITY | JPEG quality | P0 | ✅ | `cmd:app.export` (`quality`, `limitKb`), `crates/codecs/src/jpeg_par.rs` | 4:4:4 chroma (4:2:0 when a file-size limit is set); Huffman tables made for each image from its own symbol counts (T.81 Annex K.2) instead of the example tables: the same pixels, 12–14 % smaller files at quality 80–100 on a 24 MP photo-like image, still interleaved baseline (issue #1373); ordinary and HDR gain map JPEGs search quality up to the requested maximum to fit `limitKb`, including gain map and metadata, or return an error. Tested by `size_limit_respected` and `hdr_jpeg_size_limit_respected`. | | LR-EXP-BITDEPTH | Bit depth | P1 | ✅ | `cmd:app.export` (`bitDepth`), `crates/pipeline/src/output.rs` (`OutputDepth`), `crates/engine/src/export.rs` | 16-bit PNG/TIFF/JPEG XL rendered at 16 bits (TIFF defaults to 16), 32-bit float linear TIFF with a linear profile, 10-bit AVIF; high-bit-depth renders run on the CPU | -| LR-EXP-COMPRESSION | TIFF compression | P1 | ✅ | `cmd:app.export` (`tiffCompression`: none / lzw / zip) | | +| LR-EXP-COMPRESSION | TIFF compression | P1 | ✅ | `cmd:app.export` (`tiffCompression`: none / lzw / zip) | ZIP is the default. LZW is usually larger and slower than ZIP, and at 16 bit often larger than none (#2882); the export dialog notes this when LZW is picked | | LR-EXP-COLORSPACE | Output colour space | P0 | ✅ | `cmd:app.export` (`colorSpace`), `crates/pipeline/src/output.rs`, `crates/engine/src/export.rs` | sRGB, Display P3, Adobe RGB (1998) compatible, ProPhoto RGB, Rec. 2020: rendered from the working space with gamut mapping into the target gamut (CPU + GPU), own ICC profile embedded; AVIF stays sRGB (muxer has no ICC); lossy JPEG XL declares sRGB / P3 / Rec. 2020 (Adobe RGB and ProPhoto written as Rec. 2020), lossless JPEG XL embeds the profile | | LR-EXP-HDR | HDR output | P2 | 🟡 | `cmd:app.export`, `crates/ui-egui/src/panels/dialogs.rs` | HDR output checkbox in the export dialog: gain map JPEG, PQ or HLG AVIF, PQ PNG, PQ JPEG XL, float TIFF; no HDR presets yet | | LR-EXP-SHARPEN | Output sharpening | P1 | ✅ | `cmd:app.export` (`sharpen`, `sharpenAmount`) | | diff --git a/docs/user/guides/export-photos.md b/docs/user/guides/export-photos.md index 71bca3241..985263644 100644 --- a/docs/user/guides/export-photos.md +++ b/docs/user/guides/export-photos.md @@ -16,11 +16,12 @@ Select the photos, then choose **File** > **Export…**. Pick a format, a size a 5. Under the size options, tick **Full size**, or clear it and choose **Resize to** one of **Long Edge**, **Short Edge**, **Width**, **Height**, **Width × Height**, **Megapixels** or **Percentage**, then enter the value. 6. Set **Color space** (**sRGB**, **P3**, **Adobe RGB**, **ProPhoto** or **Rec.2020**) if you need something other than the default. Lossy **JPEG XL** writes **Adobe RGB** and **ProPhoto** as **Rec.2020**. Tick **Lossless** to keep them. 7. Optional: set **Sharpen** for **Screen**, **Matte** or **Glossy** output, set **Metadata** to **All**, **No camera**, **Copyright** or **None**, and tick **Remove location info** or **Watermark**. -8. Next to **Folder**, click **Choose…** and pick where the files go. **Subfolder** adds a folder inside it. -9. Set **File name** to a template such as `{name}-{seq}`. Click **Tags** to see the tags you can insert. -10. Set **If file exists** to **Add number**, **Overwrite** or **Skip**. -11. On macOS, tick **Add to Apple Photos** to add the exported files to Photos as well. Type an **Album** name to put them in that album. The first time, macOS asks whether LightCraft may control Photos. -12. Click **OK**. The export shows its progress in the activity stack at the top left. +8. For **TIFF**, set **Compression** to **ZIP** (the default), **LZW** or **None**. **LZW** is usually larger and slower than **ZIP**, and at 16 bit it can be larger than **None**. Use it only if other software needs LZW. +9. Next to **Folder**, click **Choose…** and pick where the files go. **Subfolder** adds a folder inside it. +10. Set **File name** to a template such as `{name}-{seq}`. Click **Tags** to see the tags you can insert. +11. Set **If file exists** to **Add number**, **Overwrite** or **Skip**. +12. On macOS, tick **Add to Apple Photos** to add the exported files to Photos as well. Type an **Album** name to put them in that album. The first time, macOS asks whether LightCraft may control Photos. +13. Click **OK**. The export shows its progress in the activity stack at the top left. If you did not choose a folder, the window stays open and shows an error, so your choices are not lost. From d2e6222a22f2456c93de6353f667fb618cc31d70 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 25 Oct 2026 06:57:27 +0100 Subject: [PATCH 07/25] Reject signed chunk sizes in the HTTP chunked decoder Found while testing crates/fetch/src/http.rs:522 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/fetch/src/http.rs | 3 ++- crates/fetch/src/tests.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/fetch/src/http.rs b/crates/fetch/src/http.rs index c95478388..1002e5205 100644 --- a/crates/fetch/src/http.rs +++ b/crates/fetch/src/http.rs @@ -519,7 +519,8 @@ impl Response { if *left == 0 { let line = self.conn.line(1024, limits)?; let hex = line.split(';').next().unwrap_or_default().trim(); - let size = u64::from_str_radix(hex, 16).map_err(|_| HttpError::Protocol(format!("bad chunk size `{hex}`")))?; + let size = if hex.bytes().all(|b| b.is_ascii_hexdigit()) { u64::from_str_radix(hex, 16).ok() } else { None } + .ok_or_else(|| HttpError::Protocol(format!("bad chunk size `{hex}`")))?; if size == 0 { // trailers, up to the empty line let mut n = 0; diff --git a/crates/fetch/src/tests.rs b/crates/fetch/src/tests.rs index 08f869dcc..e1e7a5679 100644 --- a/crates/fetch/src/tests.rs +++ b/crates/fetch/src/tests.rs @@ -86,6 +86,9 @@ fn handle(mut s: TcpStream, hits: &AtomicUsize) { b.extend_from_slice(&[7u8; 5000]); send(&mut s, "200 OK", "", &b) } + "signedchunk" => { + let _ = s.write_all(b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n+5\r\nhello\r\n0\r\n\r\n"); + } "chunked" => { let _ = s.write_all(b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\nConnection: close\r\n\r\n"); for c in body.chunks(70_001) { @@ -249,6 +252,16 @@ fn a_416_or_a_bad_complete_part_starts_over() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn signed_chunk_size_is_rejected() { + let srv = server(); + let dir = tmp("signedchunk"); + let f = FileSpec { name: "vocab.json", size: None, sha256: None, max: 1 << 20 }; + let r = run(&[f], &[format!("{}/signedchunk", srv.base)], &dir, &AtomicBool::new(false)).0; + assert!(r.is_err(), "a chunk size of `+5` was accepted"); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn chunked_bodies_and_unpinned_files() { let srv = server(); From 0b3810192497cda493af37475149fedcfbfc2679 Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Mon, 26 Oct 2026 11:22:28 +1100 Subject: [PATCH 08/25] Never crash: an oversize export size is an error, not a multiply overflow render_export / export_job now refuse an output box past 65535 px per side or 2^30 pixels (lightcraft_pipeline::check_output_size) before anything is sized from it, and the GPU render checks w*h (and its 12-byte buffers) with checked_mul, failing with a Limit error instead of overflowing. Sources: own design. (cherry picked from commit e0b279ad8e83d115be22ad978188f060308171fc) Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/media.rs | 1 + crates/engine/src/tests_export.rs | 11 +++++++++++ crates/gpu/src/render.rs | 5 ++++- crates/pipeline/src/lib.rs | 15 +++++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index e76ef8169..f17ec7703 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -1675,6 +1675,7 @@ impl crate::Session { space: lightcraft_pipeline::OutputSpace, depth: lightcraft_pipeline::OutputDepth, ) -> Result { + lightcraft_pipeline::check_output_size(max_w, max_h)?; let mut job = self.render_job(id, max_w, max_h, false, true).ok_or("no such photo")?; // An export renders the full image: never a thumbnail's source developed from a smaller copy of the raw // image that the file keeps (Lightroom's merged DNGs) diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index d0f174245..dbb7cd562 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -1203,3 +1203,14 @@ fn same_folder_as_original_refuses_photos_without_a_folder() { assert!(e.contains("folder is missing") && e.contains(&gone), "{e}"); let _ = std::fs::remove_dir_all(&dir); } + +/// An absurd export size is refused, not a multiply overflow (it panicked before the fix). +#[test] +fn huge_export_size_is_an_error() { + use lightcraft_pipeline::{OutputDepth, OutputSpace}; + let mut s = Session::with_demo(); + let id = s.active().unwrap(); + let huge = usize::MAX >> 8; + assert!(s.render_export(id, huge, huge, OutputSpace::Srgb, OutputDepth::U8).is_err()); + assert!(s.render_export(id, 70_000, 10, OutputSpace::Srgb, OutputDepth::U8).is_err()); +} diff --git a/crates/gpu/src/render.rs b/crates/gpu/src/render.rs index 64990db0e..009452699 100644 --- a/crates/gpu/src/render.rs +++ b/crates/gpu/src/render.rs @@ -599,7 +599,10 @@ pub fn render( }; let plan = lightcraft_pipeline::plan(src, info, s, req); let (w, h) = (plan.w, plan.h); - let n = w * h; + let Some(n) = w.checked_mul(h).filter(|n| n.checked_mul(12).is_some()) else { + fail(FailKind::Limit, format!("{w}×{h} output is too large")); + return None; + }; let _limit = match fault { Some(crate::Fault::Limit(bytes)) => Some(crate::ctx::LimitOverride::new(bytes)), _ => None, diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index 8f2702760..42cfedbba 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -173,6 +173,21 @@ impl PixelWindow { } } +/// The longest output side a render may be asked for (TIFF/PNG/JPEG all fit within it). +pub const MAX_OUTPUT_SIDE: usize = 65_535; +/// The most output pixels a render may be asked for (the box it fits into, ≈ 1.07 Gpx). +pub const MAX_OUTPUT_PIXELS: usize = 1 << 30; + +/// Rejects an output box past [`MAX_OUTPUT_SIDE`] or [`MAX_OUTPUT_PIXELS`] before anything is +/// sized from it. +pub fn check_output_size(max_w: usize, max_h: usize) -> Result<(), String> { + let pixels = max_w.checked_mul(max_h); + if max_w > MAX_OUTPUT_SIDE || max_h > MAX_OUTPUT_SIDE || pixels.is_none_or(|n| n > MAX_OUTPUT_PIXELS) { + return Err(format!("output size {max_w}×{max_h} is too large (at most {MAX_OUTPUT_SIDE} px per side and {MAX_OUTPUT_PIXELS} pixels)")); + } + Ok(()) +} + impl RenderRequest { pub fn fit(max_w: usize, max_h: usize) -> Self { Self { From 5725dccb1db905b1ff644420fa9c467f46437671 Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Mon, 26 Oct 2026 11:41:51 +1100 Subject: [PATCH 09/25] Never crash: render_now refuses an oversize output size too Review follow-up: the synchronous render_now path checks the output box with check_output_size like export_job; the hostile-input test covers it. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/media.rs | 1 + crates/engine/src/tests_export.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index f17ec7703..10857d6fa 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -1645,6 +1645,7 @@ impl crate::Session { /// Synchronous render (CLI, MCP, tests). pub fn render_now(&mut self, id: PhotoId, max_w: usize, max_h: usize) -> Result { + lightcraft_pipeline::check_output_size(max_w, max_h)?; let job = self.render_job(id, max_w, max_h, false, true).ok_or("no such photo")?; let r = job.run(); self.accept(&r); diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index dbb7cd562..217e68ea1 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -1213,4 +1213,6 @@ fn huge_export_size_is_an_error() { let huge = usize::MAX >> 8; assert!(s.render_export(id, huge, huge, OutputSpace::Srgb, OutputDepth::U8).is_err()); assert!(s.render_export(id, 70_000, 10, OutputSpace::Srgb, OutputDepth::U8).is_err()); + assert!(s.render_now(id, huge, huge).is_err()); + assert!(s.render_now(id, 10, 70_000).is_err()); } From ae55b94287dab15911796d51e45209d444e9df69 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 26 Oct 2026 17:06:08 -0500 Subject: [PATCH 10/25] perf(ui-egui): #3067 test apps render on two workers, not six Every LightcraftApp built by the ui-egui lib tests started the product's render pool: cores - 1, at most six workers. Several tests run at once and most build an app, so 4 test threads ran about 24 render workers and 16 ran about 96. Under cfg(test) Renderer::default now starts two (TEST_WORKERS); a test that needs more asks with with_worker_threads. The product default is unchanged. a_test_app_renders_on_two_workers fails without it (6 workers on a 16-core machine). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/render.rs | 22 ++++++++++++++++++-- crates/ui-egui/src/render/thumbnail_tests.rs | 19 +++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/render.rs b/crates/ui-egui/src/render.rs index 31117b22e..48d6cdde4 100644 --- a/crates/ui-egui/src/render.rs +++ b/crates/ui-egui/src/render.rs @@ -267,10 +267,28 @@ impl Slot { } } +/// Render workers of an app built by this crate's own tests. Most of them build an app, several +/// run at once, and a test needs a render or two at a time, not the machine's six (issue #3067: +/// sixteen tests at once ran about 96 render threads on 16 cores). A test that needs more asks +/// for them with [`Renderer::with_worker_threads`]. +#[cfg(test)] +pub(crate) const TEST_WORKERS: usize = 2; + +/// Render workers an app starts with: the machine's job-pool size, at most six (none on wasm). +#[cfg(not(test))] +fn default_workers() -> usize { + if cfg!(target_arch = "wasm32") { 0 } else { JobPool::::default_threads().min(6) } +} + +/// Render workers an app starts with in this crate's tests: [`TEST_WORKERS`]. +#[cfg(test)] +fn default_workers() -> usize { + TEST_WORKERS +} + impl Default for Renderer { fn default() -> Self { - let threads = if cfg!(target_arch = "wasm32") { 0 } else { JobPool::::default_threads().min(6) }; - Self::with_worker_threads(threads) + Self::with_worker_threads(default_workers()) } } diff --git a/crates/ui-egui/src/render/thumbnail_tests.rs b/crates/ui-egui/src/render/thumbnail_tests.rs index 984ce0462..cbb7ae70f 100644 --- a/crates/ui-egui/src/render/thumbnail_tests.rs +++ b/crates/ui-egui/src/render/thumbnail_tests.rs @@ -446,6 +446,25 @@ fn a_panicking_render_is_a_failure_not_a_dead_worker() { assert_eq!(app.renderer.live_workers(), workers); } +/// An app built by this crate's tests renders on two workers, not the machine's six: several +/// tests run at once, and each of them used to start six (issue #3067). +#[test] +fn a_test_app_renders_on_two_workers() { + let (mut app, _, ctx) = fixture_with(Session::new(), (*photo()).clone()); + app.renderer.offload = None; // the real job pool + let slot = Slot::Thumb(PhotoId(1)); + let job = app.session.thumb_job(PhotoId(1), 256).unwrap(); + app.renderer.request(slot, job, 30); + assert_eq!(app.renderer.live_workers(), TEST_WORKERS, "the pool starts all its workers with its first job"); + let t0 = std::time::Instant::now(); + while app.renderer.is_pending(slot) { + assert!(t0.elapsed() < std::time::Duration::from_secs(20), "the render never answered"); + std::thread::sleep(std::time::Duration::from_millis(5)); + app.renderer.poll(&ctx, &mut app.session); + } + assert!(app.renderer.textures.contains_key(&slot), "{:?}", app.renderer.failure(slot)); +} + #[test] fn raw_quick_path_frames_build_no_jobs() { // an unedited raw: the grid shows its embedded preview (held here until released) and stops From 3c96fdc9de1eb345a5039c64d9d273915492851b Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 26 Oct 2026 12:53:11 -0500 Subject: [PATCH 11/25] test(ui-egui): #3065 put the memory budget back as it was found, one changing test at a time settings_dialog_by_keyboard_and_clicks changes the process-wide memory budget through Settings > Performance (app.memoryBudget) and then reset it to default_budget(), undoing a budget set from outside (LIGHTCRAFT_MEMORY_MB, a harness cap), and its assertion on the global could race with another test changing it. A test-only BudgetGuard (crates/ui-egui/src/test_budget.rs) now takes a shared lock, so tests that change the budget run one at a time, and puts the budget back to exactly the value it found when dropped, also on a failed assertion (drop runs on unwind; a poisoned lock is taken anyway). The settings test holds it only for the few frames of the change, and also checks the session's own cache share, which only the command can change. Unit tests cover restoring the outside value (not the default), restoring on panic, and a failed holder not blocking the next. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/headless/tests.rs | 27 +++++--- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/test_budget.rs | 93 ++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 crates/ui-egui/src/test_budget.rs diff --git a/crates/ui-egui/src/headless/tests.rs b/crates/ui-egui/src/headless/tests.rs index 80d559efc..52f1c917b 100644 --- a/crates/ui-egui/src/headless/tests.rs +++ b/crates/ui-egui/src/headless/tests.rs @@ -4268,13 +4268,26 @@ fn settings_dialog_by_keyboard_and_clicks() { assert_eq!(h.app.session.cache_mb, 512); h.request("ui.clickWidget", json!({"id": "button:settingsPreview-3"}), t); assert_eq!(h.app.ui.settings.preview_limit, 3840); - h.request("ui.clickWidget", json!({"id": "button:settingsMemory-2"}), t); - assert_eq!(h.app.ui.settings.memory_mb, 1024); - h.step(); - assert_eq!(lightcraft_engine::memory::budget(), 1024 << 20, "applied through app.memoryBudget"); - h.request("ui.clickWidget", json!({"id": "button:settingsMemory-0"}), t); - h.step(); - assert_eq!(lightcraft_engine::memory::budget(), lightcraft_engine::memory::default_budget(), "back to automatic"); + { + // the memory budget is process-wide: this test's turn to change it, and it goes back + // to what it was (not the default) when the guard drops, also on a failed assertion + // (issue #3065). The change lasts these few frames. + use lightcraft_engine::memory::{budget, cache_share, default_budget}; + let _budget = crate::test_budget::BudgetGuard::hold(); + // the session's own share, set apart from the 1024 MB choice's: only the command + // changes it, whatever another test does to the process-wide budget + h.app.session.media.set_budget(cache_share(1024 << 20) + (1 << 20)); + h.request("ui.clickWidget", json!({"id": "button:settingsMemory-2"}), t); + assert_eq!(h.app.ui.settings.memory_mb, 1024); + h.step(); + assert_eq!(h.app.session.media.budget(), cache_share(1024 << 20), "the session's caches follow"); + assert_eq!(budget(), 1024 << 20, "applied through app.memoryBudget"); + h.request("ui.clickWidget", json!({"id": "button:settingsMemory-0"}), t); + h.step(); + let automatic = (default_budget() >> 20) << 20; + assert_eq!(h.app.session.media.budget(), cache_share(automatic), "back to automatic"); + assert_eq!(budget(), automatic, "back to automatic"); + } // Import: per-camera defaults h.request("ui.clickWidget", json!({"id": "button:settingsTab-import"}), t); h.request("ui.clickWidget", json!({"id": "check:settings.perCamera"}), t); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8da88f9ae..3ab3f6bcd 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -39,6 +39,8 @@ pub mod theme; pub mod widgets; pub mod window_place; +#[cfg(test)] +mod test_budget; #[cfg(test)] mod tests; #[cfg(test)] diff --git a/crates/ui-egui/src/test_budget.rs b/crates/ui-egui/src/test_budget.rs new file mode 100644 index 000000000..1442cd1a0 --- /dev/null +++ b/crates/ui-egui/src/test_budget.rs @@ -0,0 +1,93 @@ +//! For tests that change the process-wide memory budget ([`lightcraft_engine::memory::budget`]). +//! +//! The budget is one number for the whole test process: the decode, export and GPU gates, every +//! new session's caches and the demo scene cache size from it, and other tests run at the same +//! time. A test that changes it (through `app.memoryBudget` or Settings ▸ Performance) holds a +//! [`BudgetGuard`] for as long as the change lasts. Tests that change the budget then run one at +//! a time, so one can't change it under another's assertion. The budget goes back to exactly what +//! it was before, also when the test fails: putting back the value found, not `default_budget()`, +//! keeps a budget set from outside (`LIGHTCRAFT_MEMORY_MB`, a test harness) (issue #3065). +//! +//! Tests that only read the budget don't take the lock; keep the change short. +//! The lock is per process, so per test binary, which is all the budget is shared by. + +use std::sync::{Mutex, MutexGuard, PoisonError}; + +use lightcraft_engine::memory; + +/// Taken by every test that changes the process-wide budget. +static CHANGING_BUDGET: Mutex<()> = Mutex::new(()); + +/// Held while a test changes the process-wide memory budget: the other tests that change it wait, +/// and the budget is put back as it was found when the guard drops. +pub(crate) struct BudgetGuard { + // fields drop in order: the budget is put back before the next test may take its turn + _restore: Restore, + _turn: MutexGuard<'static, ()>, +} + +impl BudgetGuard { + /// Wait for the other tests that change the budget, then remember the budget as it is now. + pub(crate) fn hold() -> BudgetGuard { + // poisoned: a test failed holding it, and its guard put the budget back on unwind + let turn = CHANGING_BUDGET.lock().unwrap_or_else(PoisonError::into_inner); + BudgetGuard { _restore: Restore::new(), _turn: turn } + } +} + +/// Puts the process-wide budget back to what it was when made. +struct Restore(usize); + +impl Restore { + fn new() -> Restore { + Restore(memory::budget()) + } +} + +impl Drop for Restore { + fn drop(&mut self) { + // doesn't panic; the value came from `budget()`, so it's within what `set_budget` keeps + memory::set_budget(self.0); + } +} + +mod tests { + use super::*; + + // The budget is put back as it was found, not to the default, also when the test fails + // (issue #3065) + #[test] + fn the_budget_is_put_back_as_it_was_found() { + let _turn = BudgetGuard::hold(); + // a budget set from outside the test, unlike the default + let outside = if memory::budget() == 700 << 20 { 600 << 20 } else { 700 << 20 }; + memory::set_budget(outside); + + { + let _r = Restore::new(); + memory::set_budget(1024 << 20); + assert_eq!(memory::budget(), 1024 << 20); + } + assert_eq!(memory::budget(), outside, "the budget found, not the default"); + + let failed = std::panic::catch_unwind(|| { + let _r = Restore::new(); + memory::set_budget(96 << 20); + panic!("an assertion fails while the budget is changed"); + }); + assert!(failed.is_err()); + assert_eq!(memory::budget(), outside, "put back on unwind too"); + } + + // A test that fails holding the lock doesn't make the next one fail + #[test] + fn a_failed_test_does_not_block_the_next() { + let failed = std::thread::spawn(|| { + let _g = BudgetGuard::hold(); + panic!("an assertion fails while holding the turn"); + }) + .join(); + assert!(failed.is_err()); + let _g = BudgetGuard::hold(); + } +} From 1b9b5e1549c3c7c88c8e18d58f522ac1a120d0b1 Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Mon, 26 Oct 2026 11:40:12 +0200 Subject: [PATCH 12/25] Windows: declare the gvfs test path only where it is used The doc_path test declares `gvfs` on every platform, but only the `#[cfg(not(windows))]` assertion below it uses it. A Windows build therefore reports `unused variable: gvfs`, `cargo clippy --all-targets -- -D warnings` fails there, and `cargo xtask ci` stops at its clippy step, so none of the later steps run. The declaration now carries the same cfg as its only use. Nothing changes on other platforms. --- crates/engine/src/doc_path.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/engine/src/doc_path.rs b/crates/engine/src/doc_path.rs index d3f455b81..b1c48762f 100644 --- a/crates/engine/src/doc_path.rs +++ b/crates/engine/src/doc_path.rs @@ -63,6 +63,7 @@ mod tests { assert_eq!(resolve("Photos/2024:05/a.jpg", Some(&doc)), Err(NotFollowed::Refused(Refused::Invalid))); #[cfg(not(windows))] assert_eq!(resolve(r"C:\Photos\a.jpg", Some(&doc)), Err(NotFollowed::NotHere)); + #[cfg(not(windows))] let gvfs = Path::new("/run/user/1000/gvfs/smb-share:server=tokyo,share=photos/catalog.lrcat"); #[cfg(not(windows))] assert_eq!(resolve("a.jpg", Some(gvfs)), Ok(gvfs.with_file_name("a.jpg"))); From 13849307a9343504339d68b086f1a32cdcbbb457 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:57:39 +0000 Subject: [PATCH 13/25] Reject signed ports in Url::parse Found while testing crates/fetch/src/http.rs:140 (Url::parse) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/fetch/src/http.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/fetch/src/http.rs b/crates/fetch/src/http.rs index c95478388..ee1141652 100644 --- a/crates/fetch/src/http.rs +++ b/crates/fetch/src/http.rs @@ -137,7 +137,8 @@ impl Url { } }; let port = match port { - Some(p) => p.parse::().ok().filter(|p| *p != 0).ok_or_else(|| bad("bad port"))?, + Some(p) if p.bytes().all(|b| b.is_ascii_digit()) => p.parse::().ok().filter(|p| *p != 0).ok_or_else(|| bad("bad port"))?, + Some(_) => return Err(bad("bad port")), None if tls => 443, None => 80, }; @@ -713,6 +714,15 @@ mod tests { } } + #[test] + fn rejects_signed_ports() { + for bad in ["http://example.com:+80/x", "http://example.com:-80/x", "http://[::1]:+80/x"] { + assert!(Url::parse(bad).is_err(), "{bad}"); + } + assert!(Url::parse("http://a.test/").unwrap().join("http://example.com:+80/x").is_err()); + assert_eq!(Url::parse("http://example.com:80/x").unwrap().port, 80); + } + #[test] fn rejects_text_after_a_bracketed_ipv6_host() { for bad in ["http://[::1]junk/x", "https://[::1]junk/x", "http://[::1]8080/x", "http://[::1]8080"] { From 73f0d25405125afa0dba907b8c1fde1c19e8d410 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:09:41 +0000 Subject: [PATCH 14/25] Reject signed numbers in Content-Range parsing Found while testing crates/fetch/src/lib.rs:465 (parse_content_range) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/fetch/src/lib.rs | 13 +++++++++++-- crates/fetch/src/tests.rs | 4 ++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/crates/fetch/src/lib.rs b/crates/fetch/src/lib.rs index 12daff541..07552e4bb 100644 --- a/crates/fetch/src/lib.rs +++ b/crates/fetch/src/lib.rs @@ -466,16 +466,25 @@ fn parse_content_range(v: &str) -> Option<(u64, Option)> { let v = v.trim().strip_prefix("bytes")?.trim(); let (range, total) = v.split_once('/')?; let (a, b) = range.split_once('-')?; - let (a, b) = (a.trim().parse::().ok()?, b.trim().parse::().ok()?); + let (a, b) = (digits(a)?, digits(b)?); if b < a { return None; } let total = match total.trim() { "*" => None, - t => Some(t.parse::().ok()?), + t => Some(digits(t)?), }; Some((a, total)) } +/// A non-empty run of ASCII digits as a `u64` (no sign, unlike `str::parse`). +fn digits(s: &str) -> Option { + let s = s.trim(); + if s.is_empty() || !s.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + s.parse().ok() +} + #[cfg(test)] mod tests; diff --git a/crates/fetch/src/tests.rs b/crates/fetch/src/tests.rs index 08f869dcc..2a9c91f4f 100644 --- a/crates/fetch/src/tests.rs +++ b/crates/fetch/src/tests.rs @@ -335,6 +335,10 @@ fn content_ranges() { assert_eq!(parse_content_range("bytes 0-0/*"), Some((0, None))); assert_eq!(parse_content_range("bytes 9-1/300"), None); assert_eq!(parse_content_range("items 1-2/3"), None); + assert_eq!(parse_content_range("bytes +100-+299/+300"), None); + assert_eq!(parse_content_range("bytes 100-+299/300"), None); + assert_eq!(parse_content_range("bytes 100-299/+300"), None); + assert_eq!(parse_content_range("bytes 1-/3"), None); } /// A bearer token goes only to https, only to the host the download began at, and only when there is one. From 8a6907877ea1a12d8a88f1da0224497b9497bafc Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:35:11 +0000 Subject: [PATCH 15/25] Develop exports from a source at least twice the output size Fixes #3271 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/media/session.rs | 9 +++++++++ crates/engine/src/media/tests.rs | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/crates/engine/src/media/session.rs b/crates/engine/src/media/session.rs index b222739fe..63dfa0b14 100644 --- a/crates/engine/src/media/session.rs +++ b/crates/engine/src/media/session.rs @@ -430,6 +430,15 @@ impl crate::Session { depth: lightcraft_pipeline::OutputDepth, ) -> Result { let mut job = self.render_job(id, max_w, max_h, false, true).ok_or("no such photo")?; + // An export develops from a source at least twice its size, so it is resized down with a good filter + // rather than made from a preview just covering it (for a raw, a binned and box-reduced image) + let p = self.catalog.photo(id).ok_or("no such photo")?.clone(); + let level = SourceLevel::for_size(source_edge_needed(&p, &job.settings, true, max_w.max(max_h)).saturating_mul(2)); + if level.max_edge() > job.level.max_edge() { + job.source = self.media.source_ref(&p, level); + job.key ^= ((job.level as u64) << 60) ^ ((level as u64) << 60); + job.level = level; + } // An export renders the full image: never a thumbnail's source developed from a smaller copy of the raw // image that the file keeps (Lightroom's merged DNGs) let reduced = match &job.source { diff --git a/crates/engine/src/media/tests.rs b/crates/engine/src/media/tests.rs index d36284262..15744f025 100644 --- a/crates/engine/src/media/tests.rs +++ b/crates/engine/src/media/tests.rs @@ -517,3 +517,22 @@ fn variant_jobs_have_their_own_keys_and_hit_the_cache() { assert_ne!(pv.key, s.render_job(id, 800, 600, false, true).unwrap().key); assert_eq!(*pv.settings, a); } + +/// An export develops from a source at least twice its long edge (a 2000 px export of a 6000 px +/// photo from the original, not the 2560 px preview, which for a raw is binned and box-reduced), +/// while on-screen views keep the cheap preview. +#[test] +fn an_export_develops_from_a_source_twice_its_size() { + use lightcraft_pipeline::{OutputDepth, OutputSpace}; + let mut s = crate::Session::with_demo(); + let id = s.active().unwrap(); + let p = s.catalog.photo(id).unwrap().clone(); + assert!(p.width.max(p.height) >= 6000, "a camera-sized demo: {}×{}", p.width, p.height); + let job = s.export_job(id, 2000, 2000, OutputSpace::Srgb, OutputDepth::U8).unwrap(); + assert_eq!(job.level, SourceLevel::Full, "2000 px export"); + for edge in [200, 600, 1280, 1281, 2560] { + let job = s.export_job(id, edge, edge, OutputSpace::Srgb, OutputDepth::U8).unwrap(); + assert!(job.level.max_edge() >= 2 * edge, "{edge} px export from {:?}", job.level); + } + assert_eq!(s.render_job(id, 1600, 1600, false, true).unwrap().level, SourceLevel::Preview, "a view keeps the preview"); +} From 04a6224dabde929aa87261a7bd352fce20ec0296 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:06:22 +0000 Subject: [PATCH 16/25] Refuse a signed or malformed Content-Length in HTTP responses Found while testing crates/fetch/src/http.rs:598 (get) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/fetch/src/http.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/fetch/src/http.rs b/crates/fetch/src/http.rs index c95478388..3eca33df3 100644 --- a/crates/fetch/src/http.rs +++ b/crates/fetch/src/http.rs @@ -595,7 +595,10 @@ pub fn get(url: &Url, headers: &[(&str, String)], limits: &Limits) -> Result().map_err(|_| HttpError::Protocol(format!("bad Content-Length `{n}`")))?) + // RFC 9110: `1*DIGIT`; `u64::from_str` alone would also take a leading '+' + let v = n.trim(); + let len = if !v.is_empty() && v.bytes().all(|b| b.is_ascii_digit()) { v.parse::().ok() } else { None }; + BodyMode::Length(len.ok_or_else(|| HttpError::Protocol(format!("bad Content-Length `{n}`")))?) } else { BodyMode::Close }; @@ -652,6 +655,29 @@ mod tests_connect { assert!(r.is_err()); assert!(t.elapsed() < Duration::from_millis(1500), "{:?}", t.elapsed()); } + + /// Content-Length is `1*DIGIT`: a sign, hex prefix, trailing junk or empty value is refused, not read as a length. + #[test] + fn signed_or_malformed_content_length_is_refused() { + for value in ["+3", "-3", "3x", "", "0x10", "+0"] { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let server = std::thread::spawn(move || { + let (mut s, _) = listener.accept().unwrap(); + let mut buf = [0u8; 1024]; + let _ = s.read(&mut buf); + let _ = s.write_all(format!("HTTP/1.1 200 OK\r\nContent-Length: {value}\r\n\r\nabc").as_bytes()); + }); + let cancel = AtomicBool::new(false); + let url = Url::parse(&format!("http://127.0.0.1:{port}/f")).unwrap(); + match get(&url, &[], &limits(&cancel)) { + Err(HttpError::Protocol(m)) => assert!(m.contains("bad Content-Length"), "{value:?}: {m}"), + Err(e) => panic!("{value:?}: unexpected error {e:?}"), + Ok(r) => panic!("{value:?}: malformed Content-Length accepted (status {})", r.status), + } + server.join().unwrap(); + } + } } #[cfg(test)] From 27b47c1d6d1ab257cebe67b03b0a19e0e8567993 Mon Sep 17 00:00:00 2001 From: Brian Wang <82687988+Peaperfish@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:46:32 +0800 Subject: [PATCH 17/25] Contributors: add my entry (Peaperfish) --- contributors/people.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 95e58421a..218c8908e 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -74,3 +74,7 @@ sync_github_name = true [people.dotCooCoo] real_name = "Robert Lee" sync_github_name = true + +[people.Peaperfish] +real_name = "Yule Wang" +display_name = "Brian Wang" From 0aca56a1b8e6fcf8027b9a9e52f34f63440e0589 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 23 Oct 2026 17:54:24 -0400 Subject: [PATCH 18/25] engine tests: clear a left-over '.offline' sibling and clean up after the smart-previews test Tests that take a folder offline rename it to '.offline'. temp_dir only cleared '', and smart_previews_folder_is_chosen_per_library never removed anything, so a non-empty '.offline' left by an earlier run whose process id was reused made the rename fail with DirectoryNotEmpty (1 failure in 300 runs under load). --- crates/engine/src/tests/import.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/engine/src/tests/import.rs b/crates/engine/src/tests/import.rs index dc1c6c8f2..b3b36e758 100644 --- a/crates/engine/src/tests/import.rs +++ b/crates/engine/src/tests/import.rs @@ -13,6 +13,9 @@ use crate::Session; fn temp_dir(tag: &str) -> std::path::PathBuf { let d = std::env::temp_dir().join(format!("lc-import-{tag}-{}", std::process::id())); let _ = std::fs::remove_dir_all(&d); + // Tests that take a folder "offline" rename it to this sibling; a left-over one (from a run + // whose process id is reused) is non-empty and would make that rename fail. + let _ = std::fs::remove_dir_all(d.with_extension("offline")); std::fs::create_dir_all(&d).unwrap(); d } @@ -1229,6 +1232,11 @@ fn smart_previews_folder_is_chosen_per_library() { let r = again.execute("library.smartPreviewsLocation", &json!({"reset": true})).unwrap(); assert_eq!(r["custom"], false, "{r}"); assert_eq!(r["path"], lib.join("Smart Previews").to_string_lossy().as_ref()); + // (the open library's files are removed below: on Windows an open file cannot be) + drop(again); + for d in [&lib, &away, &src.with_extension("offline")] { + let _ = std::fs::remove_dir_all(d); + } } /// Missing Photos covers library photos only: a small library next to many Local browse From f4cb103120ca72dbabd9eb65902ce40a34866b9d Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 30 Oct 2026 15:36:03 +0100 Subject: [PATCH 19/25] Export sizes keep the pixels typed (#3300) The pixel fields of Export > Image Sizing had a step of 16, and a typed value is snapped to the step, so 1080 became 1088 and 3000 became 3008. Use whole pixels; keyboard nudges stay about 100 px (range / 200). --- crates/ui-egui/src/dialogs/export.rs | 7 +++--- crates/ui-egui/src/headless/tests.rs | 36 ++++++++++++++++++++++++++++ docs/whats-new.md | 6 +++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs/export.rs b/crates/ui-egui/src/dialogs/export.rs index ce9cffe77..9d7ebcf28 100644 --- a/crates/ui-egui/src/dialogs/export.rs +++ b/crates/ui-egui/src/dialogs/export.rs @@ -179,11 +179,12 @@ pub(super) const QUALITY: ControlSpec = spec("export.quality", "Quality", 1.0, 1 pub(super) const LIMIT_KB: ControlSpec = spec("export.limitKb", "Limit file size (KB, 0 = off)", 0.0, 20_000.0, 0.0, 10.0); -const SIZE_PX: ControlSpec = spec("export.sizePx", "Pixels", 16.0, 20_000.0, 2048.0, 16.0); +// whole pixels: a typed size is snapped to the step, and 1080 or 1350 must stay what was typed +const SIZE_PX: ControlSpec = spec("export.sizePx", "Pixels", 16.0, 20_000.0, 2048.0, 1.0); -const SIZE_W: ControlSpec = spec("export.sizeW", "Width (px)", 16.0, 20_000.0, 2048.0, 16.0); +const SIZE_W: ControlSpec = spec("export.sizeW", "Width (px)", 16.0, 20_000.0, 2048.0, 1.0); -const SIZE_H: ControlSpec = spec("export.sizeH", "Height (px)", 16.0, 20_000.0, 2048.0, 16.0); +const SIZE_H: ControlSpec = spec("export.sizeH", "Height (px)", 16.0, 20_000.0, 2048.0, 1.0); const SIZE_MP: ControlSpec = ControlSpec { decimals: 1, ..spec("export.sizeMp", "Megapixels", 0.1, 100.0, 12.0, 0.1) }; diff --git a/crates/ui-egui/src/headless/tests.rs b/crates/ui-egui/src/headless/tests.rs index 13bd7a2ae..f273edf6f 100644 --- a/crates/ui-egui/src/headless/tests.rs +++ b/crates/ui-egui/src/headless/tests.rs @@ -4953,3 +4953,39 @@ fn screenshot_request_is_answered_without_a_window() { let r = h.request("ui.screenshot", json!({"headless": true}), SETTLE); assert_eq!(r["result"]["width"], 640, "{r}"); } + +/// A size typed in the Export dialog is the size exported: the pixel fields kept only multiples +/// of 16, so 1080 became 1088 and 3000 became 3008 (issue #3300). +#[test] +fn export_sizes_keep_the_pixels_typed() { + let mut h = demo([1400.0, 900.0]); + let t = Duration::from_secs(10); + h.request("engine.execute", json!({"command": "dialog.export", "params": {}}), t); + h.settle(SETTLE); + let type_value = |h: &mut Headless, id: &str, text: &str| { + let r = h.request("ui.clickWidget", json!({"id": id}), t); + assert_eq!(r["ok"], true, "{id}: {r}"); + h.request("ui.text", json!({"text": text}), t); + h.request("ui.key", json!({"key": "Enter"}), t); + h.settle(SETTLE); + }; + let resize = |h: &Headless| match &h.app.ui.dialog { + Some(crate::state::Dialog::Export { resize, .. }) => *resize, + _ => panic!("the dialog closed"), + }; + if let Some(crate::state::Dialog::Export { full_size, resize, .. }) = &mut h.app.ui.dialog { + *full_size = false; + *resize = lightcraft_engine::export::Resize::long_edge(2048); + } + h.settle(SETTLE); + type_value(&mut h, "sliderValue:export.sizePx", "3000"); + assert_eq!(resize(&h).value, 3000.0); + if let Some(crate::state::Dialog::Export { resize, .. }) = &mut h.app.ui.dialog { + resize.mode = lightcraft_engine::export::ResizeMode::WidthHeight; + resize.height = 2048; + } + h.settle(SETTLE); + type_value(&mut h, "sliderValue:export.sizeW", "1080"); + type_value(&mut h, "sliderValue:export.sizeH", "1350"); + assert_eq!((resize(&h).value, resize(&h).height), (1080.0, 1350)); +} diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..cdfb62ac6 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,11 @@ # What's new in LightCraft +## 2026-10-30 + +### Export sizes keep the pixels typed +- Export ▸ Image Sizing kept pixel sizes only in steps of 16, so a typed 1080 became 1088 and 3000 became 3008. The + long edge, short edge, width and height fields now keep any whole number of pixels. + ## 2026-10-24 ### Relative-date filters and smart albums follow the clock From 2d15ae77185b841d5b8cccca80d5c6d1e15777a9 Mon Sep 17 00:00:00 2001 From: Bruno-Alumn <171815746+Bruno-Alumn@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:16:57 +0100 Subject: [PATCH 20/25] LR-MERGE-HDR / LR-MERGE-HDRPANO rows record the preview retry; the test accepts a preview that finished in one frame --- crates/ui-egui/src/tests/activity.rs | 5 ++++- docs/parity-checklist.md | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/tests/activity.rs b/crates/ui-egui/src/tests/activity.rs index da3b934dd..c988ba884 100644 --- a/crates/ui-egui/src/tests/activity.rs +++ b/crates/ui-egui/src/tests/activity.rs @@ -431,7 +431,10 @@ fn merge_dialog_retries_a_failed_preview_when_it_opens_again() { std::fs::rename(&away, &file).unwrap(); crate::merge::open(&mut h.app, "merge.panorama").unwrap(); h.step(); - assert!(h.app.merge.preview_task.is_some() || h.app.merge.error.is_some(), "the dialog shows a preview being made, or why not"); + assert!( + h.app.merge.preview.is_some() || h.app.merge.preview_task.is_some() || h.app.merge.error.is_some(), + "the dialog shows a preview, one being made, or why not" + ); assert!(h.step_until(T, |h| h.app.merge.preview_task.is_none())); assert!(h.app.merge.preview.is_some(), "{:?}", h.app.merge.error); h.app.ui.dialog = None; diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index d8d251359..aaaa625a4 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO: the dialog makes a new preview each time it opens, also after a failed one, #2708; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO / LR-MERGE-HDRPANO: the dialog makes a new preview each time it opens, also after a failed one, #2708; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -352,9 +352,9 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-MERGE-HDR | HDR merge | P2 | 🟡 | `cmd:merge.hdr`, `cmd:dialog.mergeHdr`, `crates/merge/src/frame.rs` (`Frame::lens`), `crates/merge/src/hdr.rs` | auto align (Lanczos-3 warp), deghost None–High + overlay, click-to-zoom preview (drag pans; the zoom stays while options change), auto settings, float DNG, Create Stack; the DNG keeps the reference frame's embedded lens corrections (tested by `hdr_merge_keeps_the_reference_frames_lens_corrections`); raws without colour matrices of their own (CR2, NEF, ARW…) keep the colour they open with, written as DNG tags (`starting_colour_tags`; the look's chroma curve has no tag; panoramas too), issue #965; the DNG's `AsShotNeutral` is the median of the inputs' vendor white-balance multipliers, issue #1144. Tested on real Sony A7R VI brackets (67 MP, 2026-10-14): frames agree with the merged radiance within ~1 % in all channels, exposure steps match EXIF within 0.03 EV. A clipped highlight of the reference that moved in other frames (a star at 200 mm) stays instead of becoming a black hole, and deghost (≤ 2048 px maps) catches a brighter frame's ghost in a dark sky (`a_clipped_star_of_the_reference_is_not_erased_by_frames_where_it_moved`, `deghost_removes_a_bright_frames_ghost_in_a_dark_sky`). 🟡 because: AI Denoise doesn't run on merged DNGs (Bayer only; Lightroom's does), JPEG brackets are treated as linear (no camera response curve), and every frame is held as 32-bit float (~0.86 GB per 67 MP frame, so 7 frames need ~7 GB); gap 29 | +| LR-MERGE-HDR | HDR merge | P2 | 🟡 | `cmd:merge.hdr`, `cmd:dialog.mergeHdr`, `crates/merge/src/frame.rs` (`Frame::lens`), `crates/merge/src/hdr.rs` | auto align (Lanczos-3 warp), deghost None–High + overlay, click-to-zoom preview (drag pans; the zoom stays while options change), auto settings, float DNG, Create Stack; the DNG keeps the reference frame's embedded lens corrections (tested by `hdr_merge_keeps_the_reference_frames_lens_corrections`); raws without colour matrices of their own (CR2, NEF, ARW…) keep the colour they open with, written as DNG tags (`starting_colour_tags`; the look's chroma curve has no tag; panoramas too), issue #965; the DNG's `AsShotNeutral` is the median of the inputs' vendor white-balance multipliers, issue #1144; the dialog makes a new preview each time it opens, also after a failed one (issue #2708, `merge_dialog_retries_a_failed_preview_when_it_opens_again`). Tested on real Sony A7R VI brackets (67 MP, 2026-10-14): frames agree with the merged radiance within ~1 % in all channels, exposure steps match EXIF within 0.03 EV. A clipped highlight of the reference that moved in other frames (a star at 200 mm) stays instead of becoming a black hole, and deghost (≤ 2048 px maps) catches a brighter frame's ghost in a dark sky (`a_clipped_star_of_the_reference_is_not_erased_by_frames_where_it_moved`, `deghost_removes_a_bright_frames_ghost_in_a_dark_sky`). 🟡 because: AI Denoise doesn't run on merged DNGs (Bayer only; Lightroom's does), JPEG brackets are treated as linear (no camera response curve), and every frame is held as 32-bit float (~0.86 GB per 67 MP frame, so 7 frames need ~7 GB); gap 29 | | LR-MERGE-PANO | Panorama | P2 | ✅ | `cmd:merge.panorama`, `cmd:dialog.mergePanorama` | spherical/cylindrical/perspective + auto (projected with Lanczos-3), boundary warp, auto crop, fill edges (diffusion), DNG whose `AsShotNeutral` is the median of its frames' as-shot white balance (vendor multipliers for CR2, NEF, ARW…), issue #1144; the dialog makes a new preview each time it opens, also after a failed one (issue #2708, `merge_dialog_retries_a_failed_preview_when_it_opens_again`); no lens model / 360° wrap | -| LR-MERGE-HDRPANO | HDR panorama | P2 | ✅ | `cmd:merge.hdrPanorama`, `cmd:dialog.mergeHdrPanorama` | brackets grouped by EXIF | +| LR-MERGE-HDRPANO | HDR panorama | P2 | ✅ | `cmd:merge.hdrPanorama`, `cmd:dialog.mergeHdrPanorama` | brackets grouped by EXIF; the dialog makes a new preview each time it opens, also after a failed one (issue #2708, `merge_dialog_retries_a_failed_preview_when_it_opens_again`) | | LR-MERGE-HEADLESS | Merge with last settings | P2 | ✅ | `cmd:merge.hdrLast`, `cmd:merge.panoramaLast`, `cmd:merge.hdrPanoramaLast`, `cmd:merge.hdrBatch`, `crates/catalog/src/stacks.rs` (`bracket_sets`) | no dialog; the options of the last merge of that kind (defaults the first time), kept in the library's prefs.json so they survive a restart; the HDR / Panorama / HDR Panorama dialogs open with them too (`merge_dialog_remembers_the_last_settings_per_kind`). HDR for Each Bracket Set merges a batch like Lightroom's merge of stacks: one HDR per stack among the photos, then per bracket (photos taken less than a second apart, after the shutter time, at exposures ≥ ½ EV apart; bursts at one exposure are skipped), one after the other in the activity stack (cancelling one stops the rest; a failed set doesn't); CLI `merge hdr-batch FILES…` | ## P. Enhance (ENH) @@ -804,7 +804,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-24 | minor | KEYC-METADATA: ⌘K / Ctrl+K (`panel.addKeywords`, Add Keywords) shows the Keywords panel, opens Keywording and puts the cursor in the Add keyword box; status counts unchanged (KEYC-METADATA stays 🟡) | | 2026-10-24 | patch | LR-LIB-RATING, LR-LIB-DELETE: a slider drag still open when a rating, flag or label takes the photo out of the view, or when the photo is deleted, ends as its own undo step before the selection moves on, as moving to the next photo does; LR-BEHAV-LOCALIZE lists all 18 languages; status unchanged | | 2026-10-24 | patch | Integration batch 43: Integrated main 98702b5c, preserving the new fail-closed GPS metadata check for Original export. Watermark geometry/orientation and HDR JPEG size-limit fixes remain in their shared helpers; no feature-count changes. | -| 2026-10-23 | patch | LR-MERGE-HDR / LR-MERGE-PANO: the merge dialog makes a new preview each time it opens, also after a failed one (#2708); status unchanged | +| 2026-10-23 | patch | LR-MERGE-HDR / LR-MERGE-PANO / LR-MERGE-HDRPANO: the merge dialog makes a new preview each time it opens, also after a failed one (#2708); status unchanged | | 2026-10-23 | patch | Export integration: Integrated main 5ca49b3e, retaining Width & Height resizing, export format/bit-depth validation and batch panic cancellation alongside the existing watermark and HDR JPEG size-limit fixes. No feature-count changes. | | 2026-10-23 | minor | LRC-LIB-CATALOG-SWITCH, LR-PREF-GENERAL: Settings ▸ General ▸ At launch (Open the last library / Ask me) with a launch window listing the recent libraries, Alt/Option at launch asks once, File ▸ Open Recent Library ▸ Remove from List and `app.recentLibraries {remove}` (issue #1290); photos dropped while no library is open wait for the one opened (presets are refused then); status and counts unchanged (✅) | | 2026-10-23 | minor | LRC-LIB-CATALOG-IMPORT: Import Lightroom Catalog… (and Open Library's offer for a Lightroom catalog) asks whether the catalog merges into the open library or goes into a new library made next to it; Settings ▸ General or "Don't ask again" switches the question off; `file.importLightroom` takes `into` / `name` / `location` (#2672); status unchanged (🟡) | From 61bcf0f506677def7ebc460aaf8780ade6b96226 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Fri, 30 Oct 2026 01:01:56 -0500 Subject: [PATCH 21/25] test(ui-egui): drop_import's launch list waits for its library checks before a click (#3368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The launch list's rows say "Checking…" until a worker answers; the answer drops that line, the centred window shrinks and its buttons move. draw_launch_list only waited for the list to be drawn, so on a loaded runner the answer could land between a click's press and release and ui.clickWidget refused it ("moved before its release"), as on a windows-latest run of a_queued_photo_drop_survives_the_choosers_new_library_prompt_and_cancel. The helper now waits until every row is answered and the window holds still, as the launch_library tests already do. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests/drop_import.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/tests/drop_import.rs b/crates/ui-egui/src/tests/drop_import.rs index 3d8031a69..80c0b5462 100644 --- a/crates/ui-egui/src/tests/drop_import.rs +++ b/crates/ui-egui/src/tests/drop_import.rs @@ -131,7 +131,10 @@ fn app_with_library(dir: &Scratch) -> Headless { } /// Draw the launch list (`over_open_library`: Alt-at-launch over the library that opened, else the -/// list with no library behind it). +/// list with no library behind it), then wait until it holds still. Each recent library's row says +/// "Checking…" until a worker answers; the answer drops that line, so the centred window shrinks and +/// its buttons move. A click aimed before then can find its button gone from under the pointer by +/// the release, and `ui.clickWidget` refuses it. fn draw_launch_list(h: &mut Headless, over_open_library: bool) { let mut problem = crate::panels::library_problem::LibraryProblem::choose_later(&h.app.ui.settings); if over_open_library && let Some(c) = problem.chooser.as_mut() { @@ -139,6 +142,26 @@ fn draw_launch_list(h: &mut Headless, over_open_library: bool) { } h.app.library_problem = Some(problem); assert!(h.step_until(T, |h| h.app.widgets.iter().any(|(w, _)| w == "button:libraryOpen")), "the launch list is drawn"); + let answered = + |h: &Headless| h.app.library_problem.as_ref().and_then(|p| p.chooser.as_ref()).is_some_and(|c| c.libraries.iter().all(|l| !l.checking)); + assert!(h.step_until(T, answered), "every recent library is checked"); + wait_until_still(h, "button:libraryOpen"); +} + +/// Run frames until widget `id` keeps its rect from one frame to the next (an anchored window +/// takes a frame to move to its new size). +fn wait_until_still(h: &mut Headless, id: &str) { + let rect = |h: &Headless| h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r); + let mut last = rect(h); + for _ in 0..60 { + h.step(); + let now = rect(h); + if now.is_some() && now == last { + return; + } + last = now; + } + panic!("{id} never held still"); } /// The launch list as Alt-at-launch shows it over the library that opened. From ea2e4bf6299049f2d36704a6ba3512d0afbf221a Mon Sep 17 00:00:00 2001 From: Antonio Sokic <5701985+finaler1x@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:09:24 +0100 Subject: [PATCH 22/25] Issue #3412: accessibility audit allows missing glyphs without craft-fonts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without CRAFT_FONTS_DIR the Reference view's "▸" has no glyph and egui_kittest panicked, failing the audit on every dev build. It now allows boxes then and records a skip!(Fonts) note; with craft-fonts a missing glyph still fails. --- crates/ui-egui/tests/it/accessibility.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/tests/it/accessibility.rs b/crates/ui-egui/tests/it/accessibility.rs index 04d64f3e5..30196048b 100644 --- a/crates/ui-egui/tests/it/accessibility.rs +++ b/crates/ui-egui/tests/it/accessibility.rs @@ -14,7 +14,14 @@ fn harness(view: ViewMode, right: RightPanel) -> Harness<'static, LightcraftApp> app.ui.left_panel = true; app.ui.view = view; app.ui.right = right; - let mut h = Harness::builder().with_size(vec2(1500.0, 950.0)).build_ui_state( + let mut builder = Harness::builder().with_size(vec2(1500.0, 950.0)); + // text such as "▸" in the Reference view's hint has its glyph only in the craft-fonts faces (releases + // always have them): without them it is drawn as a box, which this audit of names doesn't need + if lightcraft_engine::CRAFT_FONTS.is_empty() { + lightcraft_testkit::skip!(Fonts, "built without CRAFT_FONTS_DIR: glyphs only craft-fonts has are drawn as boxes, not checked"); + builder = builder.allow_missing_glyphs(); + } + let mut h = builder.build_ui_state( |ui, app: &mut LightcraftApp| { app.logic(&ui.ctx().clone()); app.ui(ui); From cda73d235e36ef85bcd18d59e0b9611251f11733 Mon Sep 17 00:00:00 2001 From: Antonio Sokic <5701985+finaler1x@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:25:18 +0100 Subject: [PATCH 23/25] Issue #3412: record the craft-fonts skip once per accessibility audit --- crates/ui-egui/tests/it/accessibility.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/tests/it/accessibility.rs b/crates/ui-egui/tests/it/accessibility.rs index 30196048b..39b707cd3 100644 --- a/crates/ui-egui/tests/it/accessibility.rs +++ b/crates/ui-egui/tests/it/accessibility.rs @@ -18,7 +18,6 @@ fn harness(view: ViewMode, right: RightPanel) -> Harness<'static, LightcraftApp> // text such as "▸" in the Reference view's hint has its glyph only in the craft-fonts faces (releases // always have them): without them it is drawn as a box, which this audit of names doesn't need if lightcraft_engine::CRAFT_FONTS.is_empty() { - lightcraft_testkit::skip!(Fonts, "built without CRAFT_FONTS_DIR: glyphs only craft-fonts has are drawn as boxes, not checked"); builder = builder.allow_missing_glyphs(); } let mut h = builder.build_ui_state( @@ -87,6 +86,10 @@ fn audit(h: &Harness<'_, LightcraftApp>, place: &str) -> Vec { #[test] fn every_interactive_node_has_a_name_in_every_view_and_tool() { + // said once per audit, not once per harness + if lightcraft_engine::CRAFT_FONTS.is_empty() { + lightcraft_testkit::skip!(Fonts, "built without CRAFT_FONTS_DIR: glyphs only craft-fonts has are drawn as boxes, not checked"); + } let mut problems = Vec::new(); for view in [ViewMode::PhotoGrid, ViewMode::SquareGrid, ViewMode::Detail, ViewMode::Compare, ViewMode::Survey, ViewMode::Reference] { let h = harness(view, RightPanel::None); From ec3cada7110d92582e80a006b5dbaface8f69f30 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 31 Oct 2026 02:19:19 +0000 Subject: [PATCH 24/25] Integration: #2890 re-export preset_lzw_note for the grid context menu --- crates/ui-egui/src/dialogs.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index c9cb13cf6..ed9e6fb30 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -16,11 +16,12 @@ mod rename; mod sam_model; pub(crate) use confirm::album_entries; +pub(crate) use export::preset_lzw_note; pub use confirm::confirm_dialog; pub use cull::{MAX_BURST_GAP, clamp_cull_burst, cull_params, fmt_gap, parse_gap}; use export::{ LIMIT_KB, QUALITY, START_NUMBER, WM_IMAGE_WIDTH, WM_INSET_X, WM_INSET_Y, WM_OPACITY, WM_POSITIONS, WM_SIZE, export_dialog_params, export_size, - group_checklist, preset_lzw_note, tiff_lzw_note, watermark_preview, + group_checklist, tiff_lzw_note, watermark_preview, }; pub use frame::{ConfirmStyle, confirm_style, list_height, prompt}; use frame::{ From c1ce177f0f4274c531fbce156eea5625aa04df89 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 31 Oct 2026 02:19:24 +0000 Subject: [PATCH 25/25] Integration: cargo fmt --- crates/ui-egui/src/dialogs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index ed9e6fb30..8ac10e42d 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -16,9 +16,9 @@ mod rename; mod sam_model; pub(crate) use confirm::album_entries; -pub(crate) use export::preset_lzw_note; pub use confirm::confirm_dialog; pub use cull::{MAX_BURST_GAP, clamp_cull_burst, cull_params, fmt_gap, parse_gap}; +pub(crate) use export::preset_lzw_note; use export::{ LIMIT_KB, QUALITY, START_NUMBER, WM_IMAGE_WIDTH, WM_INSET_X, WM_INSET_Y, WM_OPACITY, WM_POSITIONS, WM_SIZE, export_dialog_params, export_size, group_checklist, tiff_lzw_note, watermark_preview,