From 6a0ced426d32d55fed392a185358d1f4c115801f Mon Sep 17 00:00:00 2001 From: tylerphotos <93777540+tylerphotos@users.noreply.github.com> Date: Sun, 18 Oct 2026 08:03:05 -0500 Subject: [PATCH 1/2] LR-IMP-ADD-DIALOG: add import thumbnail-size slider Remember an independent 80-320 point size for import review thumbnails. Reflow the grid without changing candidate checks, request density-aware previews, and retain only nearby import textures and pending work. Add control-channel access, interaction and persistence regressions, and update the import guide, parity row and remaining loupe gap. This is the thumbnail-size portion of #2056; pre-import loupe remains separate. Correct Windows test portability issues exposed by the required gate: a Unix-only import, export path comparisons and folder widget path ids. Validation: cargo xtask ci; native control-channel acceptance including small/large sizes, selection, short-window layout and restart persistence. --- crates/catalog/src/store.rs | 1 + crates/engine/src/import.rs | 5 +- crates/engine/src/tests_export.rs | 10 +- crates/ui-egui/src/import.rs | 73 +++++++-- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/menus.rs | 13 ++ crates/ui-egui/src/render.rs | 20 ++- crates/ui-egui/src/state.rs | 4 + crates/ui-egui/src/tests_folder_drop.rs | 15 +- crates/ui-egui/src/tests_import_thumbnails.rs | 149 ++++++++++++++++++ docs/control-protocol.md | 6 + docs/gaps.md | 7 +- docs/parity-checklist.md | 5 +- docs/user/guides/import-photos.md | 2 +- 14 files changed, 282 insertions(+), 30 deletions(-) create mode 100644 crates/ui-egui/src/tests_import_thumbnails.rs diff --git a/crates/catalog/src/store.rs b/crates/catalog/src/store.rs index 2f3dc4912..a5976c14c 100644 --- a/crates/catalog/src/store.rs +++ b/crates/catalog/src/store.rs @@ -244,6 +244,7 @@ impl Store for MemStore { #[cfg(test)] mod tests_staging { + #[cfg(unix)] use super::*; /// A link planted at a staging name (`.tmp`) is removed, never followed: the file it points to keeps its diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 9aa847b01..e30137bbe 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -1544,8 +1544,9 @@ impl Session { p.preview_only = info.preview_only.clone(); } p.develop = std::sync::Arc::new(p.import_defaults()); - let edge = edge.clamp(64, crate::media::SourceLevel::Thumb.max_edge()); - let level = crate::media::SourceLevel::Thumb; + // Large import tiles on high-density displays need more than a 512-pixel source. + let edge = edge.clamp(64, 1024); + let level = crate::media::SourceLevel::for_size(edge); let source = self.media.origin_ref(&p.source, level.max_edge()); let key = lightcraft_preview::Hasher128::new().str(&c.path).u64(c.file_size).u64(edge as u64).finish().0 as u64; let small = crate::media::RenderJob { diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 0ac8df804..2f70ae208 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -991,15 +991,19 @@ fn same_folder_as_original_exports_next_to_each_photo() { let exists = |p: &str| std::path::Path::new(p).exists(); let files = crate::export::export_batch(&mut s, &ids, &o, &to, &mut crate::export::write_file, &exists).unwrap(); let written: Vec<&str> = files.iter().map(|f| f["path"].as_str().unwrap()).collect(); - let mut want = vec![a.join("Exports/IMG_1.jpg").to_string_lossy().to_string(), b.join("Exports/IMG_1.jpg").to_string_lossy().to_string()]; - let mut got: Vec = written.iter().map(|p| p.to_string()).collect(); + // Compare path components: Windows accepts both separators in returned paths. + let mut want = vec![a.join("Exports").join("IMG_1.jpg"), b.join("Exports").join("IMG_1.jpg")]; + let mut got: Vec = written.iter().map(std::path::PathBuf::from).collect(); want.sort(); got.sort(); assert_eq!(got, want); assert!(!dir.join("unused").exists(), "nothing written to the specific folder"); // again: each folder's own collision gets the next free name let files = crate::export::export_batch(&mut s, &ids, &o, &to, &mut crate::export::write_file, &exists).unwrap(); - assert!(files.iter().all(|f| f["path"].as_str().unwrap().ends_with("Exports/IMG_1-2.jpg")), "{files:?}"); + assert!( + files.iter().all(|f| std::path::Path::new(f["path"].as_str().unwrap()).ends_with(std::path::Path::new("Exports").join("IMG_1-2.jpg"))), + "{files:?}" + ); // no subfolder: next to the original, which is never written over let beside = ExportOptions { subfolder: String::new(), conflict: Conflict::Overwrite, ..o.clone() }; let err = crate::export::export_batch(&mut s, &ids, &beside, &to, &mut crate::export::write_file, &exists).unwrap_err(); diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index e851463d1..65f0f1467 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -8,7 +8,7 @@ //! catalog between frames, with a row and ✕ in the activity stack (issue #345); the whole import is //! one undo step. -use egui::{Align2, Color32, Rect, Sense, Stroke, StrokeKind, Vec2, pos2, vec2}; +use egui::{Color32, Rect, Sense, Stroke, StrokeKind, Vec2, pos2, vec2}; use lightcraft_engine::import::{ ImportCandidate, ImportUprightMode, ImportWarning, ImportWarningCode, ScanInput, ScanOutput, ScanProgress, scan_with, }; @@ -35,6 +35,23 @@ const CONTENT_BOTTOM: &str = "import-content-bottom"; /// Room below the grid before it has been measured (more than the options and buttons take). const DEFAULT_BELOW_GRID: f32 = 360.0; +pub(crate) const THUMB_SIZE_MIN: f32 = 80.0; +pub(crate) const THUMB_SIZE_MAX: f32 = 320.0; +pub(crate) const THUMB_SIZE_DEFAULT: f32 = 116.0; + +/// Bound values from saved preferences and the control channel before laying out the grid. +pub(crate) fn thumb_size(size: f32) -> f32 { + if size.is_finite() { size.clamp(THUMB_SIZE_MIN, THUMB_SIZE_MAX) } else { THUMB_SIZE_DEFAULT } +} + +/// A few physical-pixel sizes avoid decoding on every point of a slider drag. The engine caps +/// candidate previews at 1024 pixels; this covers the largest tile at up to 3× display density. +fn thumb_edge(size: f32, pixels_per_point: f32) -> usize { + let scale = if pixels_per_point.is_finite() && pixels_per_point > 0.0 { pixels_per_point } else { 1.0 }; + let pixels = (thumb_size(size) - 8.0) * scale; + [192, 384, 512, 768, 1024].into_iter().find(|edge| *edge as f32 >= pixels).unwrap_or(1024) +} + /// Height of the photo grid: what is `available` once `below` (options and buttons) is left free, /// in whole points so the window never grows by a fraction each frame, and at least one `row`. pub(crate) fn grid_height(available: f32, below: f32, row: f32) -> f32 { @@ -1026,8 +1043,18 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { } }); } + ui.horizontal(|ui| { + let mut size = thumb_size(app.ui.import_thumb_size); + let response = + ui.add(egui::Slider::new(&mut size, THUMB_SIZE_MIN..=THUMB_SIZE_MAX).text(crate::i18n::tr("Thumbnail size")).show_value(false)); + // Slider's response includes its label; automation addresses the track itself. + register(ui.ctx(), "slider:importThumbSize", Rect::from_min_size(response.rect.min, vec2(ui.spacing().slider_width, response.rect.height()))); + if response.changed() { + let _ = app.run("view.importThumbSize", json!({"size": size})); + } + }); // candidate grid - let cell = 116.0; + let cell = thumb_size(app.ui.import_thumb_size).min(ui.available_width().max(1.0)); let avail = ui.available_width(); let cols = ((avail + 6.0) / (cell + 6.0)).floor().max(1.0) as usize; let rows = n.div_ceil(cols); @@ -1038,6 +1065,7 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { y(CONTENT_BOTTOM).zip(y(GRID_BOTTOM)).map(|(content, grid)| content - grid) }); let grid_h = grid_height(ui.available_height(), below.unwrap_or(DEFAULT_BELOW_GRID), cell + 26.0); + let mut visible = std::collections::HashSet::new(); let grid = egui::ScrollArea::vertical().id_salt("import-grid").max_height(grid_h).auto_shrink([false, false]).show_viewport(ui, |ui, viewport| { let (area, _) = ui.allocate_exact_size(vec2(avail, rows as f32 * (cell + 26.0)), Sense::hover()); @@ -1047,10 +1075,15 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { if !local.intersects(viewport.expand(cell)) { continue; } + let Ok(slot) = u32::try_from(i) else { continue }; + visible.insert(slot); let rect = local.translate(area.min.to_vec2()); candidate_cell(app, ui, d, i, rect); } }); + if !ui.is_sizing_pass() { + app.renderer.retain_imports(&visible); + } if !ui.is_sizing_pass() { ui.ctx().data_mut(|m| m.insert_temp(egui::Id::new(GRID_BOTTOM), grid.inner_rect.bottom())); } @@ -1314,7 +1347,8 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog, i: usize, rect: Rect) { let t = Tokens::get(ui.ctx()); - let c = d.candidates[i].clone(); + let Some(c) = d.candidates.get(i).cloned() else { return }; + let Ok(slot_index) = u32::try_from(i) else { return }; let ok = d.importable(i); let img = Rect::from_min_size(rect.min, vec2(rect.width(), rect.width())); let resp = ui.interact(img, egui::Id::new(("import-cell", i)), Sense::click()); @@ -1323,9 +1357,10 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial let p = ui.painter(); p.rect_filled(img, 3.0, Color32::from_gray(30)); // thumbnail (background job; the grid only asks for the cells in view) - let slot = Slot::Import(i as u32); - if !app.renderer.textures.contains_key(&slot) - && let Some(job) = app.session.candidate_thumb_job(&c, 192, i as u64) + let slot = Slot::Import(slot_index); + let edge = thumb_edge(img.width(), ui.ctx().pixels_per_point()); + if app.renderer.textures.get(&slot).is_none_or(|tex| tex.size.into_iter().max().unwrap_or(0) < edge) + && let Some(job) = app.session.candidate_thumb_job(&c, edge, u64::from(slot_index)) { app.renderer.request_quick(slot, job, 4); } @@ -1336,7 +1371,7 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial let tint = if ok { Color32::WHITE } else { Color32::from_gray(110) }; p.image(tex.tex.id(), fit, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), tint); } - let on = d.checked[i] && ok; + let on = d.checked.get(i).copied().unwrap_or(false) && ok; if on { p.rect_stroke(img, 3.0, Stroke::new(2.0, t.accent), StrokeKind::Inside); } @@ -1358,13 +1393,20 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial _ => None, }; if let Some(b) = badge { - let g = p.layout_no_wrap(crate::i18n::tr(b).to_string(), t.semibold(10.0), Color32::WHITE); + let mut label = egui::text::LayoutJob::simple_singleline(crate::i18n::tr(b).to_string(), t.semibold(10.0), Color32::WHITE); + label.wrap.max_width = (img.width() - 16.0).max(1.0); + label.wrap.max_rows = 1; + let g = p.layout_job(label); let br = badge_rect(img, cb, g.size()); p.rect_filled(br, 3.0, Color32::from_rgba_unmultiplied(170, 60, 50, 220)); p.galley(br.min + vec2(4.0, 2.0), g, Color32::WHITE); } - let name = if c.name.chars().count() > 18 { format!("{}…", c.name.chars().take(17).collect::()) } else { c.name.clone() }; - p.text(pos2(rect.left() + 2.0, img.bottom() + 9.0), Align2::LEFT_CENTER, name, t.font(10.5), if ok { t.text_label } else { t.text_dim }); + let color = if ok { t.text_label } else { t.text_dim }; + let mut name = egui::text::LayoutJob::simple_singleline(c.name.clone(), t.font(10.5), color); + name.wrap.max_width = (rect.width() - 4.0).max(1.0); + name.wrap.max_rows = 1; + let name = p.layout_job(name); + p.galley(pos2(rect.left() + 2.0, img.bottom() + 2.0), name, color); let tip = format!( "{}\n{} × {} · {} · {:.1} MB{}", c.path, @@ -1581,6 +1623,17 @@ fn field(ui: &mut egui::Ui, label: &str, add: impl FnOnce(&mut egui::Ui) -> R mod tests { use super::*; + #[test] + fn import_thumb_edges_follow_size_and_display_density() { + assert_eq!(thumb_edge(116.0, 1.0), 192); + assert_eq!(thumb_edge(320.0, 1.0), 384); + assert_eq!(thumb_edge(320.0, 2.0), 768); + assert_eq!(thumb_edge(320.0, 3.0), 1024); + assert_eq!(thumb_edge(320.0, f32::MAX), 1024); + assert_eq!(thumb_edge(f32::NAN, f32::NAN), 192); + assert_eq!(thumb_edge(116.0, -1.0), 192); + } + #[test] fn grid_height_follows_the_window() { // the grid takes what the options and buttons leave, in whole points diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index bb290bd86..ca1a8ab0e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -98,6 +98,8 @@ mod tests_grading; #[cfg(test)] mod tests_grid; #[cfg(test)] +mod tests_import_thumbnails; +#[cfg(test)] mod tests_info_dialogs; #[cfg(test)] mod tests_keymap; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index d575d7490..f3ec4fbc5 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -100,6 +100,7 @@ pub const UI_COMMANDS: &[UiCommand] = &[ // Library grids only (Photo Grid, Square Grid, People): the bottom-bar slider's step by key ("view.thumbSizeUp", "Increase Grid Thumbnail Size", Some("="), "View"), ("view.thumbSizeDown", "Decrease Grid Thumbnail Size", Some("-"), "View"), + ("view.importThumbSize", "Import Thumbnail Size", None, ""), ("view.clipping", "Show Clipping", Some("J"), "View"), ("view.visualizeHdr", "Visualize HDR", None, "View"), // in grids S expands/collapses stacks (the engine command it shadows) @@ -866,6 +867,18 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { + // {size?: points}; without a size, read the current preference. + let size = match p.get("size") { + None => app.ui.import_thumb_size, + Some(value) => match value.as_f64().map(|n| n as f32).filter(|n| n.is_finite()) { + Some(size) => size, + None => return Some(Err("view.importThumbSize: size must be a finite number".into())), + }, + }; + app.ui.import_thumb_size = crate::import::thumb_size(size); + Ok(json!({"size": app.ui.import_thumb_size})) + } "view.navigate" => { // {zoom?: "fit"|"fill"|{percent: number}, pan?: [x, y]} (normalized image centre). // Validate the complete request before changing either part of the viewport. diff --git a/crates/ui-egui/src/render.rs b/crates/ui-egui/src/render.rs index 64ab067b7..5dee1bc1c 100644 --- a/crates/ui-egui/src/render.rs +++ b/crates/ui-egui/src/render.rs @@ -954,12 +954,20 @@ impl Renderer { /// Drop the import review thumbnails (a new review, or the dialog closed). pub fn forget_imports(&mut self) { - self.pool.reprioritize(|s, p| if matches!(s, Slot::Import(_)) { None } else { Some(p) }); - self.queue.retain(|q| !matches!(q.slot, Slot::Import(_))); - self.pending.retain(|s, _| !matches!(s, Slot::Import(_))); - self.request_ids.retain(|s, _| !matches!(s, Slot::Import(_))); - self.textures.retain(|s, _| !matches!(s, Slot::Import(_))); - self.quick_tried.retain(|s, _| !matches!(s, Slot::Import(_))); + self.retain_imports(&Default::default()); + } + + /// Keep only the import tiles near the viewport, including queued work. Large thumbnails + /// must not accumulate for every candidate while scrolling through a whole card. + pub fn retain_imports(&mut self, keep: &std::collections::HashSet) { + let retained = |s: &Slot| !matches!(s, Slot::Import(i) if !keep.contains(i)); + self.pool.reprioritize(|s, p| retained(s).then_some(p)); + self.queue.retain(|q| retained(&q.slot)); + self.pending.retain(|s, _| retained(s)); + self.request_ids.retain(|s, _| retained(s)); + self.textures.retain(|s, _| retained(s)); + self.quick_tried.retain(|s, _| retained(s)); + self.failed.retain(|s, _| retained(s)); } /// Drop thumbnails that aren't in `keep` (bounded memory for huge libraries), and queued diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index f8c8cc0d4..0b6b28ccc 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -662,6 +662,8 @@ pub struct UiState { pub zoom_anim: bool, pub before_after: BeforeAfter, pub thumb_size: f32, + /// Import review thumbnail size in points, independent of the library grid. + pub import_thumb_size: f32, /// Open Edit sections by id. pub open_sections: Vec, /// Open flyouts (curve, mixer, grading…). @@ -1283,6 +1285,7 @@ impl Default for UiState { zoom_anim: false, before_after: BeforeAfter::Off, thumb_size: 220.0, + import_thumb_size: crate::import::THUMB_SIZE_DEFAULT, open_sections: vec!["light".into()], open_flyouts: vec![], single_panel: false, @@ -1467,6 +1470,7 @@ impl UiState { /// Clamp values restored from disk. pub fn sanitized(mut self) -> Self { self.thumb_size = self.thumb_size.clamp(THUMB_SIZE_MIN, THUMB_SIZE_MAX); + self.import_thumb_size = crate::import::thumb_size(self.import_thumb_size); self.left_width = LEFT_WIDTH.clamp(self.left_width); self.right_width = RIGHT_WIDTH.clamp(self.right_width); self.brush_size = self.brush_size.clamp(0.002, 0.5); diff --git a/crates/ui-egui/src/tests_folder_drop.rs b/crates/ui-egui/src/tests_folder_drop.rs index 9b4cb860a..4d37f5709 100644 --- a/crates/ui-egui/src/tests_folder_drop.rs +++ b/crates/ui-egui/src/tests_folder_drop.rs @@ -50,8 +50,13 @@ fn widget(h: &Headless, id: &str) -> Option { h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r) } +fn folder_id(prefix: &str, dir: &Path) -> String { + // FolderNode paths (and their widget ids) use forward slashes on every platform. + format!("{prefix}:{}", dir.to_string_lossy().replace('\\', "/")) +} + fn folder_row(h: &Headless, dir: &Path) -> Option { - widget(h, &format!("source:libfolder:{}", dir.to_string_lossy())) + widget(h, &folder_id("source:libfolder", dir)) } fn id_of(h: &Headless, file: &str) -> u64 { @@ -180,7 +185,7 @@ fn photos_already_in_the_folder_stay_put() { let (root, trip, home) = folders("here"); let mut h = app(&[(trip.join("beach.png"), false), (home.join("garden.png"), false)]); // open the folder holding both - let toggle = widget(&h, &format!("libraryFolderToggle:{}", root.to_string_lossy())).unwrap(); + let toggle = widget(&h, &folder_id("libraryFolderToggle", &root)).unwrap(); h.app.synthetic.push(egui::Event::PointerMoved(toggle.center())); h.app.synthetic.push(egui::Event::PointerButton { pos: toggle.center(), @@ -284,7 +289,7 @@ fn cancelling_the_question_moves_nothing() { let target = if folder_row(&h, &home).is_some() { target } else { - click(&mut h, &format!("libraryFolderToggle:{}", root.to_string_lossy())); + click(&mut h, &folder_id("libraryFolderToggle", &root)); folder_row(&h, &home).unwrap() }; let undo = h.app.session.undo.len(); @@ -302,7 +307,7 @@ fn dont_ask_again_moves_the_next_drop_at_once() { let (root, trip, home) = folders("dont-ask"); std::fs::copy(trip.join("beach.png"), trip.join("dunes.png")).unwrap(); let mut h = app(&[(trip.join("beach.png"), false), (trip.join("dunes.png"), false), (home.join("garden.png"), false)]); - click(&mut h, &format!("libraryFolderToggle:{}", root.to_string_lossy())); + click(&mut h, &folder_id("libraryFolderToggle", &root)); let target = folder_row(&h, &home).unwrap(); let beach = id_of(&h, "beach.png"); h.app.session.selection = lightcraft_engine::Selection::single(lightcraft_catalog::PhotoId(beach)); @@ -327,7 +332,7 @@ fn dont_ask_again_moves_the_next_drop_at_once() { fn a_question_left_open_while_another_library_opens_moves_nothing() { let (root, trip, home) = folders("other-library"); let mut h = app(&[(trip.join("beach.png"), false), (home.join("garden.png"), false)]); - click(&mut h, &format!("libraryFolderToggle:{}", root.to_string_lossy())); + click(&mut h, &folder_id("libraryFolderToggle", &root)); let beach = id_of(&h, "beach.png"); let target = folder_row(&h, &home).unwrap(); drop_beach(&mut h, beach, target); diff --git a/crates/ui-egui/src/tests_import_thumbnails.rs b/crates/ui-egui/src/tests_import_thumbnails.rs new file mode 100644 index 000000000..cdf178826 --- /dev/null +++ b/crates/ui-egui/src/tests_import_thumbnails.rs @@ -0,0 +1,149 @@ +//! Import review sizes change the real grid and its preview resolution, preserving culling. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::import::ImportDialog; +use crate::render::Slot; +use crate::state::{Dialog, UiState}; +use crate::{LightcraftApp, Services}; +use lightcraft_engine::import::ImportCandidate; + +const T: Duration = Duration::from_secs(30); + +struct Review { + h: Headless, + dir: std::path::PathBuf, +} + +impl Review { + fn new(scale: f32) -> Self { + let dir = std::env::temp_dir().join(format!("lc-import-thumbnails-{}-{}", std::process::id(), lightcraft_preview::next_tick())); + std::fs::create_dir_all(&dir).unwrap(); + let image = lightcraft_raster::Rgba8 { + width: 1600, + height: 1200, + data: (0..1600 * 1200).map(|i| [((i % 1600) / 7) as u8, ((i / 1600) / 5) as u8, 100, 255]).collect(), + }; + let bytes = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&image), &Default::default()).unwrap(); + let path = dir.join("original.png"); + std::fs::write(&path, &bytes).unwrap(); + let mut candidates: Vec<_> = (0..60) + .map(|i| ImportCandidate { + path: path.to_string_lossy().into(), + name: format!("a-long-photo-name-{i:02}.png"), + format: "PNG".into(), + width: 1600, + height: 1200, + file_size: bytes.len() as u64, + ..Default::default() + }) + .collect(); + candidates[1].duplicate = Some("content".into()); + candidates[2].error = Some("Unreadable test candidate".into()); + let mut app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + app.ui.dialog = Some(Dialog::Import { opts: Box::new(ImportDialog::new(candidates)) }); + let mut h = Headless::new(app, [1366.0, 1000.0], scale); + assert!(h.settle(T)); + Self { h, dir } + } + + fn rect(&self, id: &str) -> egui::Rect { + self.h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("missing {id}")) + } + + fn checks(&self) -> Vec { + let Some(Dialog::Import { opts }) = &self.h.app.ui.dialog else { panic!("import review closed") }; + opts.checked.clone() + } + + fn request(&mut self, method: &str, params: serde_json::Value) { + let response = self.h.request(method, params, T); + assert_eq!(response["ok"], true, "{method}: {response}"); + } + + fn size(&mut self, size: f32) { + self.request("engine.execute", json!({"command": "view.importThumbSize", "params": {"size": size}})); + assert!(self.h.settle(T)); + } +} + +impl Drop for Review { + fn drop(&mut self) { + self.h.app.ui.dialog = None; + self.h.app.renderer.forget_imports(); + self.h.settle(T); + let _ = std::fs::remove_dir_all(&self.dir); + } +} + +#[test] +fn import_thumbnail_slider_reflows_without_changing_selection_or_badges() { + let mut review = Review::new(1.0); + assert_eq!(review.rect("import:0").width(), 116.0); + review.request("ui.clickWidget", json!({"id": "import:0"})); + let checked = review.checks(); + assert!(!checked[0] && !checked[1] && !checked[2]); + // Real pointer input on the slider, including its end stops. + review.request("ui.clickWidget", json!({"id": "slider:importThumbSize", "fx": 0.0})); + assert!(review.h.settle(T)); + assert_eq!(review.rect("import:0").width(), 80.0); + let slider = review.rect("slider:importThumbSize"); + review.request("ui.dragWidget", json!({"id": "slider:importThumbSize", "toX": slider.right(), "toY": slider.center().y})); + assert!(review.h.settle(T)); + assert_eq!(review.rect("import:0").width(), 320.0); + assert_eq!(review.checks(), checked); + let text = review.h.painted_text().join("\n"); + assert!(text.contains("Duplicate") && text.contains("Unreadable"), "badges remain visible: {text}"); + assert!(review.rect("import:3").top() > review.rect("import:0").top(), "larger tiles wrap into more rows"); + review.size(80.0); + assert_eq!(review.rect("import:3").top(), review.rect("import:0").top()); + assert_eq!(review.checks(), checked); + // The preference survives closing/reopening the review and a settings round trip. + let saved = serde_json::to_value(&review.h.app.ui).unwrap(); + let restored: UiState = serde_json::from_value(saved).unwrap(); + assert_eq!(restored.sanitized().import_thumb_size, 80.0); +} + +#[test] +fn import_thumbnail_previews_upgrade_for_density_and_stay_bounded_when_scrolling() { + let mut review = Review::new(2.0); + assert_eq!(review.h.app.renderer.textures[&Slot::Import(0)].size, [384, 288]); + review.size(320.0); + assert_eq!(review.h.app.renderer.textures[&Slot::Import(0)].size, [768, 576]); + // Shrinking keeps the sharp preview instead of replacing it with a smaller decode. + let sharp = review.h.app.renderer.textures[&Slot::Import(0)].key; + review.size(116.0); + assert_eq!(review.h.app.renderer.textures[&Slot::Import(0)].key, sharp); + review.size(320.0); + // Scrolling a large card retains only the nearby tiles and drops obsolete requests. + review.request("ui.hoverWidget", json!({"id": "import:0"})); + review.request("ui.scroll", json!({"dy": -20000.0})); + assert!(review.h.settle(T)); + assert!(!review.h.app.renderer.textures.contains_key(&Slot::Import(0))); + let imports: Vec<_> = review.h.app.renderer.textures.keys().filter_map(|s| if let Slot::Import(i) = s { Some(*i) } else { None }).collect(); + assert!(!imports.is_empty() && imports.len() < 20, "bounded visible previews: {imports:?}"); + for i in imports { + assert!(review.h.app.widgets.iter().any(|(w, _)| w == &format!("import:{i}"))); + } +} + +#[test] +fn import_thumbnail_size_rejects_bad_commands_and_restores_old_preferences() { + let mut app = LightcraftApp::new(lightcraft_engine::Session::new(), Services::default()); + assert_eq!(app.run("view.importThumbSize", json!({})).unwrap()["size"], 116.0); + for bad in [json!("large"), json!(null), json!([120]), json!(1e300)] { + assert!(app.run("view.importThumbSize", json!({"size": bad})).is_err()); + assert_eq!(app.ui.import_thumb_size, 116.0); + } + assert_eq!(app.run("view.importThumbSize", json!({"size": -10})).unwrap()["size"], 80.0); + assert_eq!(app.run("view.importThumbSize", json!({"size": 1e6})).unwrap()["size"], 320.0); + let legacy: UiState = serde_json::from_value(json!({})).unwrap(); + assert_eq!(legacy.import_thumb_size, 116.0); + for bad in [f32::NAN, f32::INFINITY, f32::NEG_INFINITY] { + let state = UiState { import_thumb_size: bad, ..Default::default() }.sanitized(); + assert_eq!(state.import_thumb_size, 116.0); + } +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 31ecbf8c2..f67bd4fcd 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -239,6 +239,12 @@ For MCP, use `lightcraft-cli mcp --connect 127.0.0.1:7980`: `run_command` reache `lightcraft://controls` ([mcp.md](mcp.md#resources)). Headless CLI/MCP mode owns a separate engine session; use connect mode for feedback from edits in the desktop app. +The import review's **Thumbnail size** slider is `slider:importThumbSize` in `ui.widgets`. +`engine.execute {command: "view.importThumbSize", params: {size: 240}}` sets its size in points +(80–320, clamped); omitting `size` reads it. Both return `{size}`. Non-numeric or non-finite +values are refused. The preference is `importThumbSize` in the UI state, independent of the +library grid's `thumbSize`; resizing keeps the import checkboxes and duplicate markers. + ## Headless rendering (no window, no GPU) The egui UI can be rasterized on the CPU (`crates/ui-egui/src/softpaint.rs`, driven by diff --git a/docs/gaps.md b/docs/gaps.md index f11a74f7a..aae2389fb 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-14 · **Change:** minor (gap 10: 515 of 1,446 models verified, ~84% of photographers on a verified body) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-17 · **Change:** minor (import grid thumbnail size adjustable; gap 30 tracks the remaining pre-import loupe, #2056) 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 @@ -44,6 +44,7 @@ Hours are Opus 5.5 agent wall-clock hours (see [target-app-parity.md](target-app | 27 | **Windows / Linux runtime coverage**: Windows installer UI unverified on Windows (#79); GPU path proven on Apple and on one AMD GPU on Linux (Radeon 8060S, Vulkan); NVIDIA, Intel and DX12 unverified; the Linux riscv64 tarball (new in 0.6.0, #844) is cross-compiled and never run beyond a QEMU smoke test | ROADMAP, issues, `release.yml` | non-Mac users find bugs first | 13–27 | [hardware-parity](hardware-parity.md) | | 29 | **HDR merge leftovers** (LR-MERGE-HDR): AI Denoise doesn't run on merged (linear) DNGs; JPEG brackets are treated as linear (no camera response curve); every frame is held as 32-bit float, ~0.86 GB per 67 MP frame (half-float frames would halve it) | tested on Sony A7R VI brackets 2026-10-14 ([merge.md](merge.md)); Lightroom's Denoise supports HDR DNGs | HDR shooters who denoise their merges, merge JPEGs, or merge 7+ frames of high-resolution raws on 16 GB machines | 10–20 (+ denoise linear-RGB contract, gap 11) | [merge](merge.md) | | 28 | **Small Classic shortcuts and keys**: rating `[` `]`, flag cycling, filter-bar keys, F1 help, panel keys (Tab, F5, F8; F6 and F7 toggle the filmstrip and the left panel) | [ui-parity](ui-parity.md#shortcuts) | Classic muscle memory | 4–8 | [ui-parity](ui-parity.md) | +| 30 | **Pre-import loupe and keyboard culling** (#2056): the import review now has an 80–320 pt thumbnail slider, with suitably sized previews and preserved selection; opening a candidate large and next/previous culling before import remain | `crates/ui-egui/src/import.rs`, `crates/ui-egui/src/tests_import_thumbnails.rs` | photographers culling a large card before adding it to their library | 6–10 | [import guide](user/guides/import-photos.md) | Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform work they need around them, **~340–600 h** to beta (incl. 40–80 h for the ~92 open core-path user issues) (see [target-app-parity.md](target-app-parity.md#stage)). @@ -60,6 +61,9 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w ## Closed recently (kept for calibration) +- 2026-10-17: import review grid thumbnail-size slider (part of #2056, LR-IMP-ADD-DIALOG): 80–320 pt, + remembered separately from the library grid, density-aware previews and bounded visible textures. + Gap 30 retains the loupe request; headline readiness estimates unchanged. - 2026-10-12: original-pixel detail at 100% (gap 4, LR-VIEW-ZOOM, #323): soft proofing gets zoom windows like Detail, Before/After, Compare, Reference and mask overlays; 1:1 tested equal to the full-size export. - 2026-10-11: Sony ILCA-99M2 profile. @@ -77,6 +81,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-17 | minor | Import thumbnail-size slider (part of #2056); gap 30 records the remaining pre-import loupe and keyboard culling | | 2026-10-14 | minor | Gap 26: at 100k photos stack and album lookups are indexed, so an "In a Stack" or "Album" smart album counts in ~14 ms (was 0.6–2.7 s); open time unchanged | | 2026-10-14 | minor | Gap 10: Camera-coverage sweep (#1327): 30 models verified on 40 CC0 files (coverage.py 515 → 545 verified); sensor verification does not establish colour parity (#1277) | | 2026-10-14 | minor | Gap 21: de, es, fr, ja and uk catalogs filled (97–99%); 55–90 h → 40–70 h | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index de7ba697f..da25ec019 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-14 · **Change:** minor (integration batch 36: slideshow module, print-ready PDF fixes, local adjustment presets, Layout Image loupe overlay, Delete in an album removes from it, new languages Czech, Polish, Italian-ready catalogs; many rows' notes updated by their PRs) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-17 · **Change:** minor (import grid thumbnail-size slider, #2056; status and summary counts unchanged) · **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 @@ -71,7 +71,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). |---|---|---|---|---|---| | 2026-10-12 | minor | LR-IMP-FORMATS: Mamiya MEF decodes from sensor data and is imported (`vendor/mef.rs`) | | 2026-10-12 | minor | LR-IMP-FORMATS: Epson ERF decodes from sensor data (`vendor/erf.rs`), no longer preview-only | -| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with thumbnails and checkboxes (duplicates by path/content marked and unchecked; with several file types, a toggle per type checks or unchecks all of its files, e.g. the raws without the JPEGs beside them, issue #344) filling a resizable window (issue #337), transfer mode (add in place / copy / move, each explained), album (existing/new), preset, keywords; the folder scan runs on a background thread, shown in the activity stack with ✕ to cancel (a NAS folder no longer freezes the window); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, shown in the activity stack with ✕ to cancel; one undo step. Drag-and-drop goes through the same background import. File dialogs list the importable extensions (`lightcraft_engine::import::EXTENSIONS`) in any letter case on Linux, where GTK and the desktop portal match globs case-sensitively (`apps/lightcraft/src/dialog_filter.rs`, issue #342; 0.4.0 hid `.JPG` / `.NEF`, issue #705). The folder scan, Browse Folder, drag-and-drop and the sidecar lookup match extensions in any case (`crates/engine/src/tests_import.rs`) | +| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with an 80–320 pt Thumbnail size slider (`cmd:view.importThumbSize`, `crates/ui-egui/src/tests_import_thumbnails.rs`; remembered separately from the library grid; density-aware previews, bounded visible textures), thumbnails and checkboxes (duplicates by path/content marked and unchecked; with several file types, a toggle per type checks or unchecks all of its files, e.g. the raws without the JPEGs beside them, issue #344) filling a resizable window (issue #337), transfer mode (add in place / copy / move, each explained), album (existing/new), preset, keywords; the folder scan runs on a background thread, shown in the activity stack with ✕ to cancel (a NAS folder no longer freezes the window); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, shown in the activity stack with ✕ to cancel; one undo step. Drag-and-drop goes through the same background import. File dialogs list the importable extensions (`lightcraft_engine::import::EXTENSIONS`) in any letter case on Linux, where GTK and the desktop portal match globs case-sensitively (`apps/lightcraft/src/dialog_filter.rs`, issue #342; 0.4.0 hid `.JPG` / `.NEF`, issue #705). The folder scan, Browse Folder, drag-and-drop and the sidecar lookup match extensions in any case (`crates/engine/src/tests_import.rs`) | | LR-IMP-DRAGDROP | Drop files/folders to import | P0 | ✅ | `crates/ui-egui/src/lib.rs` (dropped files → `cmd:library.import`) | dropping onto a specific album not supported | | LR-IMP-DUPES | Skip duplicates by content | P1 | ✅ | `crates/engine/src/import.rs`, `crates/engine/src/tests_import.rs` | | | LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Import from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); copies are verified (a new file, synced, checked against the content hash the scan computed — byte compare with the source when there is none, issue #134; a bad copy is removed and reported as failed, never counted as imported; taken names get -1, -2…, never replaced — issue #96, `crates/engine/src/import_move.rs`); no PTP/MTP (cameras that don't mount as a disk); Settings → Import → Cameras and Cards can open the import review when a camera or card is connected (off by default; a card that was already in doesn't ask, and an open dialog or running import is finished first; `crates/ui-egui/src/import.rs`) | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-17 | minor | LR-IMP-ADD-DIALOG: import grid thumbnail-size slider, density-aware previews, bounded visible textures and preserved culling; remaining loupe tracked in gap 30 (#2056); status and summary counts unchanged | | 2026-10-12 | minor | LR-EDIT-OPTICS-PROFILE: Sony ARW shading, CA and distortion tables decoded with settings and provenance (raw IFD, `SR2SubIFD` fallback); applied corrections unchanged (ILCE-7RM4A and ILCE-7RM6 distortion); status unchanged (🟡) | | 2026-10-14 | minor | LR-IMP-CAMERA-COVERAGE: Camera-coverage sweep (#1327): 30 models verified on 40 CC0 files (coverage.py 515 → 545 verified); sensor verification does not establish colour parity (#1277); status unchanged (🟡) | | 2026-10-13 | minor | LR-MERGE-PANO: a panorama's DNG takes its `AsShotNeutral` from the median of its frames' vendor white-balance multipliers (CR2, NEF, ARW…) instead of the first frame's tags, and frames now carry the multipliers (issue #1144); status unchanged (✅) | diff --git a/docs/user/guides/import-photos.md b/docs/user/guides/import-photos.md index 8130272ae..b688f3d16 100644 --- a/docs/user/guides/import-photos.md +++ b/docs/user/guides/import-photos.md @@ -10,7 +10,7 @@ Use **File** > **Import Photos…** to pick files, or **File** > **Import from F ## Steps 1. Open the **File** menu and choose **Import Photos…** or **Import from Folder…**, then pick the files or the folder. -2. Look at the thumbnails. Every photo that can be imported has a checkbox. Click a photo to check or uncheck it. Shift-click sets a range. **Check All** and **Uncheck All** change every photo at once. +2. Look at the thumbnails. Use **Thumbnail size** above the grid to make them smaller or larger; the size is remembered for the next import. Every photo that can be imported has a checkbox. Click a photo to check or uncheck it. Shift-click sets a range. **Check All** and **Uncheck All** change every photo at once. 3. Photos already in your library are marked **Duplicate** or **In library** and start unchecked. A file LightCraft cannot read is marked **Unreadable**. A photo you deleted that is still in **Recently Deleted** is marked **Will be restored**. **In Recently Deleted** sets what happens to those: **Restore them** (the default) brings the photos back with their edits, ratings and albums, **Import as new** deletes the old record first, and **Leave them** skips them. 4. Under **Transfer**, choose how the files get into the library: - **Add in place** keeps the files where they are. The library only points to them. Nothing is copied or moved. From 6b1fc20053f96318b51091025cfef95a352d2a49 Mon Sep 17 00:00:00 2001 From: tylerphotos <93777540+tylerphotos@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:59:23 -0500 Subject: [PATCH 2/2] LR-IMP-ADD-DIALOG: render import previews when the camera JPEG is too small Require a covering embedded image only for renderable import candidates. Keep the available camera JPEG for preview-only raws and failed or unavailable render fallbacks; ordinary quick views and grid thumbnails retain their immediate stand-in behavior. Also release import preview state when canceled through the control channel. Co-Authored-By: Codex --- crates/engine/src/import/job.rs | 14 ++- crates/engine/src/import/job/tests.rs | 92 +++++++++++++++++++ crates/engine/src/media/quick.rs | 17 +++- crates/engine/src/media/session.rs | 3 +- .../engine/src/media/thumbnail_hash_tests.rs | 1 + crates/ui-egui/src/control.rs | 3 + crates/ui-egui/src/tests/import_thumbnails.rs | 14 +-- docs/control-protocol.md | 2 +- docs/gaps.md | 2 +- docs/parity-checklist.md | 4 +- 10 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 crates/engine/src/import/job/tests.rs diff --git a/crates/engine/src/import/job.rs b/crates/engine/src/import/job.rs index 25dc392de..a178035da 100644 --- a/crates/engine/src/import/job.rs +++ b/crates/engine/src/import/job.rs @@ -483,7 +483,16 @@ impl Session { (Some(l), MediaKind::Raw) => Some((c.path.clone(), l.clone(), edge)), _ => None, }; - Some(crate::media::QuickJob { request_id: 0, photo: id, key, cached: Vec::new(), embedded, small: Some(Box::new(small)) }) + let minimum_embedded_edge = (c.preview_only.is_none() && p.preview_only.is_none()).then_some(render_edge); + Some(crate::media::QuickJob { + request_id: 0, + photo: id, + key, + cached: Vec::new(), + embedded, + minimum_embedded_edge, + small: Some(Box::new(small)), + }) } /// Use the system clock for import/edit times (native hosts). @@ -492,3 +501,6 @@ impl Session { self } } + +#[cfg(test)] +mod tests; diff --git a/crates/engine/src/import/job/tests.rs b/crates/engine/src/import/job/tests.rs new file mode 100644 index 000000000..82490915b --- /dev/null +++ b/crates/engine/src/import/job/tests.rs @@ -0,0 +1,92 @@ +//! Import previews must cover the tile when possible, retaining camera JPEGs as a safe fallback. + +use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, +}; + +use crate::Session; +use crate::import::ImportCandidate; +use crate::media::QuickSource; + +/// Synthetic raw, bounded decoder and camera JPEG, with reads counted independently of pixels. +fn fixture(preview_edge: Option, renderable: bool, preview_only: bool) -> (Session, ImportCandidate, Arc) { + let mut session = Session::new(); + let reads = Arc::new(AtomicUsize::new(0)); + let count = reads.clone(); + session.media.file_loader = Some(Arc::new(move |_, edge| { + count.fetch_add(1, Ordering::SeqCst); + if !renderable { + return Err("synthetic raw cannot be rendered".into()); + } + let width = edge.min(1024); + let height = width * 3 / 4; + Ok((lightcraft_raster::Rgb32f { width, height, data: vec![[0.1, 0.2, 0.3]; width * height] }, Default::default())) + })); + session.media.preview_loader = Some(Arc::new(move |_, requested| { + let width = preview_edge?.min(requested); + Some(lightcraft_raster::Rgba8 { width, height: width * 3 / 4, data: vec![[30, 60, 90, 255]; width * (width * 3 / 4)] }) + })); + let candidate = ImportCandidate { + path: "synthetic-import.nef".into(), + name: "synthetic-import.nef".into(), + format: "NEF".into(), + kind: lightcraft_catalog::MediaKind::Raw, + width: 1024, + height: 768, + preview_only: preview_only.then(|| "unsupported synthetic raw variant".into()), + ..Default::default() + }; + (session, candidate, reads) +} + +#[test] +/// A renderable raw fills the requested tile when its embedded JPEG is too small. +fn import_preview_renders_when_the_camera_jpeg_cannot_cover_the_tile() { + let (mut session, candidate, reads) = fixture(Some(64), true, false); + let mut job = session.candidate_thumb_job(&candidate, 768, 0).unwrap(); + job.request_id = 42; + let key = job.key; + let result = job.run(); + assert_eq!(result.quick, Some(QuickSource::Small)); + let image = result.rendered.unwrap().image; + assert_eq!((image.width, image.height), (768, 576)); + assert_eq!(reads.load(Ordering::SeqCst), 1); + assert_eq!((result.request_id, result.key), (42, key)); +} + +#[test] +/// A sufficient camera JPEG avoids decoding the raw, retaining the fast path. +fn import_preview_keeps_a_camera_jpeg_that_already_covers_the_tile() { + let (mut session, candidate, reads) = fixture(Some(1024), true, false); + let result = session.candidate_thumb_job(&candidate, 768, 0).unwrap().run(); + assert_eq!(result.quick, Some(QuickSource::Embedded)); + assert_eq!(result.rendered.unwrap().image.width, 768); + assert_eq!(reads.load(Ordering::SeqCst), 0); +} + +#[test] +/// Preview-only raws never waste a render attempt behind their available camera JPEG. +fn import_preview_only_raw_uses_the_available_camera_jpeg_without_a_render() { + let (mut session, candidate, reads) = fixture(Some(64), false, true); + let result = session.candidate_thumb_job(&candidate, 768, 0).unwrap().run(); + assert_eq!(result.quick, Some(QuickSource::Embedded)); + assert_eq!(result.rendered.unwrap().image.width, 64); + assert_eq!(reads.load(Ordering::SeqCst), 0); +} + +#[test] +/// A failed or absent render never discards the usable, if undersized, camera JPEG. +fn import_preview_retains_the_camera_jpeg_if_the_render_fails_or_is_absent() { + for no_render in [false, true] { + let (mut session, candidate, reads) = fixture(Some(64), false, false); + let mut job = session.candidate_thumb_job(&candidate, 768, 0).unwrap(); + if no_render { + job.small = None; + } + let result = job.run(); + assert_eq!(result.quick, Some(QuickSource::Embedded)); + assert_eq!(result.rendered.unwrap().image.width, 64); + assert_eq!(reads.load(Ordering::SeqCst), usize::from(!no_render)); + } +} diff --git a/crates/engine/src/media/quick.rs b/crates/engine/src/media/quick.rs index ca399b561..21edb2bed 100644 --- a/crates/engine/src/media/quick.rs +++ b/crates/engine/src/media/quick.rs @@ -25,6 +25,10 @@ pub struct QuickJob { pub cached: Vec<(Arc, Hash128)>, /// Embedded preview of an unedited raw: (path, loader, max edge). pub embedded: Option<(String, PreviewLoader, usize)>, + /// Import tiles need this many pixels when a render can provide them. `None` accepts any + /// camera JPEG immediately (ordinary transient stand-ins and preview-only imports). + /// An undersized JPEG remains the fallback if the render fails or is unavailable. + pub minimum_embedded_edge: Option, /// Last resort: run this thumbnail job. pub small: Option>, } @@ -72,14 +76,23 @@ impl QuickJob { return done(rendered(Arc::unwrap_or_clone(img)), Some(QuickSource::Cached)); } } + let mut embedded_fallback = None; if let Some((path, loader, edge)) = &self.embedded && let Some(image) = loader(path, *edge) { - return done(rendered(image), Some(QuickSource::Embedded)); + if self.minimum_embedded_edge.is_none_or(|minimum| image.width.max(image.height) >= minimum) { + return done(rendered(image), Some(QuickSource::Embedded)); + } + embedded_fallback = Some(image); } if let Some(job) = self.small { let r = job.run(); - return RenderResult { request_id, key, quick: Some(QuickSource::Small), ..r }; + if r.rendered.is_ok() || embedded_fallback.is_none() { + return RenderResult { request_id, key, quick: Some(QuickSource::Small), ..r }; + } + } + if let Some(image) = embedded_fallback { + return done(rendered(image), Some(QuickSource::Embedded)); } done(Err("no quick preview".into()), None) } diff --git a/crates/engine/src/media/session.rs b/crates/engine/src/media/session.rs index b222739fe..ea69c4907 100644 --- a/crates/engine/src/media/session.rs +++ b/crates/engine/src/media/session.rs @@ -342,7 +342,7 @@ impl crate::Session { let h = Hasher128::new().str(&content_key(&p)).str("quick").u64(p.develop.hash64()).u64(apply_crop as u64).finish(); let key = h.0 as u64; let embedded = self.embedded_of(&p).map(|(path, l)| (path, l, max_edge.clamp(1, SourceLevel::Preview.max_edge()))); - Some(QuickJob { request_id: 0, photo: id, key, cached, embedded, small: apply_crop.then(|| Box::new(small)) }) + Some(QuickJob { request_id: 0, photo: id, key, cached, embedded, minimum_embedded_edge: None, small: apply_crop.then(|| Box::new(small)) }) } /// A first grid thumbnail for an unedited raw: the cached render for `job` (the real @@ -361,6 +361,7 @@ impl crate::Session { key: job.key, cached: job.cache.clone().into_iter().collect(), embedded: Some((path, l, edge)), + minimum_embedded_edge: None, small: Some(Box::new(job.clone())), }) } diff --git a/crates/engine/src/media/thumbnail_hash_tests.rs b/crates/engine/src/media/thumbnail_hash_tests.rs index fdbd20f80..3c23e9db9 100644 --- a/crates/engine/src/media/thumbnail_hash_tests.rs +++ b/crates/engine/src/media/thumbnail_hash_tests.rs @@ -107,6 +107,7 @@ fn a_render_that_panics_fails_instead_of_never_answering() { key: 9, cached: Vec::new(), embedded: Some(("x.arw".into(), Arc::new(|_: &str, _: usize| -> Option { panic!("preview reader bug") }), 256)), + minimum_embedded_edge: None, small: None, }; let r = quick.run(); diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index caacbc590..c32434569 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -434,6 +434,9 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest None => err("no dialog open"), }, "ui.dialog.cancel" => { + if matches!(app.ui.dialog, Some(crate::state::Dialog::Import { .. })) { + app.renderer.forget_imports(); + } app.ui.dialog = None; ok(Value::Null) } diff --git a/crates/ui-egui/src/tests/import_thumbnails.rs b/crates/ui-egui/src/tests/import_thumbnails.rs index ba670cd5e..6a1b0c104 100644 --- a/crates/ui-egui/src/tests/import_thumbnails.rs +++ b/crates/ui-egui/src/tests/import_thumbnails.rs @@ -171,16 +171,16 @@ fn import_thumbnail_small_sources_do_not_rebuild_jobs_every_frame() { } } -/// Closing the real review releases large previews through both user-facing dismissal paths. +/// Closing the review releases large previews through the button, Escape and control channel. #[test] -fn import_thumbnail_close_releases_previews_for_cancel_and_escape() { - for escape in [false, true] { +fn import_thumbnail_close_releases_previews_for_all_cancel_paths() { + for dismissal in ["button", "escape", "control"] { let mut review = Review::new(1.0); assert!(review.h.app.renderer.textures.contains_key(&Slot::Import(0))); - if escape { - review.request("ui.key", json!({"key": "Escape"})); - } else { - review.request("ui.clickWidget", json!({"id": "button:dialogCancel"})); + match dismissal { + "escape" => review.request("ui.key", json!({"key": "Escape"})), + "control" => review.request("ui.dialog.cancel", json!({})), + _ => review.request("ui.clickWidget", json!({"id": "button:dialogCancel"})), } assert!(review.h.settle(T)); assert!(review.h.app.ui.dialog.is_none()); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 129726699..22d0723b2 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -320,7 +320,7 @@ library grid's `thumbSize`; resizing keeps the import checkboxes and duplicate m The shared slider also exposes `sliderValue:importThumbSize` for exact entry and `sliderLabel:importThumbSize` for double-click reset to 116 pt. Up/Down nudges the selected slider by 1 pt; Return commits typed values and Escape cancels them. Slider and command -labels, including edit/reset feedback, are localized in every shipped catalog. Previews use bounded physical-pixel buckets through 1024 px. A smaller source is tried once per bucket; scrolling releases offscreen textures, queued work and request identities. +labels, including edit/reset feedback, are localized in every shipped catalog. Previews use bounded physical-pixel buckets through 1024 px. A smaller source is tried once per bucket; scrolling releases offscreen textures, queued work and request identities. A renderable raw uses a render when its camera JPEG cannot cover the tile; preview-only raws, unavailable renders and failed renders retain the available camera JPEG. ## Headless rendering (no window, no GPU) diff --git a/docs/gaps.md b/docs/gaps.md index d9289a63e..fc1a3abd6 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -86,7 +86,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| -| 2026-10-29 | minor | import review: remembered thumbnail size and density-aware previews (#2237, part of #2056); pre-import loupe/keyboard culling remains open; no ranked gap closes | +| 2026-10-29 | minor | Import thumbnails render when the camera JPEG is too small, retaining preview-only/error fallbacks; import review: remembered thumbnail size and density-aware previews (#2237, part of #2056); pre-import loupe/keyboard culling remains open; no ranked gap closes | | 2026-10-24 | minor | Gap 28: ⌘⌥⇧C / ⌘⌥⇧V (Ctrl+Alt+Shift+C / V on Windows and Linux) copy and paste metadata; no key leaves the entry; hours unchanged | | 2026-10-24 | minor | Gap 30 shrinks: the Info panel's Show Metadata For (Target Photo / Selected Photos, `view.infoSelectedPhotos`) shows the selected photos' shared values and marks fields that differ as Mixed; the All Metadata dialog remains; 1–3 h (was 4–8) | | 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 | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index f8aaaf4a2..b102e754a 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -69,7 +69,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:view.importThumbSize`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/dialogs/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with a remembered 80–320 pt thumbnail-size slider (default 116, independent of the library grid), density-aware previews capped at 1024 px and retention limited to nearby cells; tiny sources are tried once per requested size bucket; the pre-import loupe and keyboard culling remain open under #2056; thumbnails and checkboxes (duplicates by path/content marked and unchecked; with several file types, a toggle per type checks or unchecks all of its files, e.g. the raws without the JPEGs beside them, issue #344) filling a resizable window (issue #337), transfer mode (add in place / copy / move, each explained), album (existing/new): the destination picker follows the sidebar folder hierarchy and order, searches full paths, and distinguishes duplicate names by their folder path; folders navigate and smart albums are excluded, with None, New album and album-context preselection retained (#2362; `crates/ui-egui/src/widgets/album_picker.rs`, `crates/ui-egui/src/tests/album_picker.rs`); preset, keywords; the folder scan runs on a background thread, shown in the activity stack with ✕ to cancel (a NAS folder no longer freezes the window); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, shown in the activity stack with ✕ to cancel; one undo step. Drag-and-drop goes through the same background import. File dialogs list the importable extensions (`lightcraft_engine::import::EXTENSIONS`) in any letter case on Linux, where GTK and the desktop portal match globs case-sensitively (`apps/lightcraft/src/dialog_filter.rs`, issue #342; 0.4.0 hid `.JPG` / `.NEF`, issue #705). The folder scan, Browse Folder, drag-and-drop and the sidecar lookup match extensions in any case (`crates/engine/src/tests/import.rs`) | +| LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:view.importThumbSize`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/dialogs/import.rs` | review dialog (Import Photos): the scanned source (a scanned folder is not added to Local), candidate grid with a remembered 80–320 pt thumbnail-size slider (default 116, independent of the library grid), density-aware previews capped at 1024 px and retention limited to nearby cells; tiny sources are tried once per requested size bucket; renderable raws use a sharper render when the camera JPEG is undersized, retaining the JPEG for preview-only raws or an unavailable/failed render; the pre-import loupe and keyboard culling remain open under #2056; thumbnails and checkboxes (duplicates by path/content marked and unchecked; with several file types, a toggle per type checks or unchecks all of its files, e.g. the raws without the JPEGs beside them, issue #344) filling a resizable window (issue #337), transfer mode (add in place / copy / move, each explained), album (existing/new): the destination picker follows the sidebar folder hierarchy and order, searches full paths, and distinguishes duplicate names by their folder path; folders navigate and smart albums are excluded, with None, New album and album-context preselection retained (#2362; `crates/ui-egui/src/widgets/album_picker.rs`, `crates/ui-egui/src/tests/album_picker.rs`); preset, keywords; the folder scan runs on a background thread, shown in the activity stack with ✕ to cancel (a NAS folder no longer freezes the window); the import itself (probing, sidecars, copying / moving files) runs on a worker thread and its batches join the library between frames, shown in the activity stack with ✕ to cancel; one undo step. Drag-and-drop goes through the same background import. File dialogs list the importable extensions (`lightcraft_engine::import::EXTENSIONS`) in any letter case on Linux, where GTK and the desktop portal match globs case-sensitively (`apps/lightcraft/src/dialog_filter.rs`, issue #342; 0.4.0 hid `.JPG` / `.NEF`, issue #705). The folder scan, Browse Folder, drag-and-drop and the sidecar lookup match extensions in any case (`crates/engine/src/tests/import.rs`) | | LR-IMP-DRAGDROP | Drop files/folders to import | P0 | ✅ | `crates/ui-egui/src/lib.rs` (dropped files → the import review), `crates/ui-egui/src/drop_overlay.rs`, `crates/ui-egui/src/tests/drop_import.rs` | while files are dragged over the window a veil and a card say what dropping does (review N items for import, import N presets, open the face model; a note to close the open dialog first when one is open); photos and folders open the import review as File ▸ Import does (Settings ▸ Import ▸ Import dropped files without review imports them at once; the web build always does); with no library open on the desktop (the launch list, a library that couldn't open) photos wait for the library opened and presets are refused, and the card says so; a drop while a dialog is open (also in the web build) or while an import is running is refused with a toast, and the card says so; a finished scan waits for an open dialog to close before its review opens; the control channel's `ui.dropFiles` hovers or drops files for scripts; dropping onto a specific album not supported | | LR-IMP-DUPES | Skip duplicates by content | P1 | ✅ | `crates/engine/src/import.rs`, `crates/engine/src/tests/import.rs` | | | LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Import from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); copies are verified (a new file, synced, checked against the content hash the scan computed — byte compare with the source when there is none, issue #134; a bad copy is removed and reported as failed, never counted as imported; taken names get -1, -2…, never replaced — issue #96, `crates/engine/src/import_move.rs`); no PTP/MTP (cameras that don't mount as a disk); Settings → Import → Cameras and Cards can open the import review when a camera or card is connected (off by default; a card that was already in doesn't ask, and an open dialog or running import is finished first; `crates/ui-egui/src/dialogs/import.rs`) | @@ -799,7 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| -| 2026-10-29 | minor | LR-IMP-ADD-DIALOG: remembered import thumbnail size, density-aware bounded previews and one job per candidate/size bucket (#2237, part of #2056); import loupe remains open; counts unchanged | +| 2026-10-29 | minor | LR-IMP-ADD-DIALOG: remembered import thumbnail size, density-aware bounded previews and one job per candidate/size bucket, with sharper raw fallback and the available camera JPEG retained when rendering cannot help (#2237, part of #2056); import loupe remains open; counts unchanged | | 2026-10-25 | patch | LR-BEHAV-TITLEBAR: the top bar's title-bar gestures come from craft-window's `title_bar`: a double-click follows the system setting, and a press on a button, the search field or a panel floating over the bar never moves the window; status counts unchanged | | 2026-10-24 | minor | KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 (`keyword.nextSet`, `keyword.previousSet`) go to the next / previous keyword set; status counts unchanged (KEYC-METADATA stays 🟡) | | 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 🟡) |