From b273f8e6087ae9f20ebfc2f4fb3708fd263f0171 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:53:36 +0200 Subject: [PATCH 01/99] #798: read large Lightroom catalogs by page Generated with Codex. --- crates/engine/src/lightroom_catalog.rs | 37 ++++- crates/engine/src/lightroom_sqlite.rs | 220 ++++++++++++++++++++++++- docs/gaps.md | 2 +- docs/lightroom-catalog-import.md | 13 +- docs/parity-checklist.md | 2 +- 5 files changed, 257 insertions(+), 17 deletions(-) diff --git a/crates/engine/src/lightroom_catalog.rs b/crates/engine/src/lightroom_catalog.rs index 6c1c7724d..a727491fb 100644 --- a/crates/engine/src/lightroom_catalog.rs +++ b/crates/engine/src/lightroom_catalog.rs @@ -8,7 +8,7 @@ use std::sync::{ atomic::{AtomicBool, AtomicUsize, Ordering}, }; -use crate::lightroom_sqlite::{Database, LiveTable, Value as SqlValue}; +use crate::lightroom_sqlite::{Database, FileSource, LiveTable, Value as SqlValue}; use lightcraft_catalog::{Album, Flag, Op, Photo, PhotoId, Source}; use lightcraft_geom::{Affine, Orientation, Point, Rect}; use serde::{Deserialize, Serialize}; @@ -352,15 +352,15 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, let before = std::fs::metadata(path).map_err(|e| e.to_string())?; let wal_path = PathBuf::from(format!("{}-wal", path.display())); let wal_before = std::fs::metadata(&wal_path).ok().map(|m| (m.len(), m.modified().ok())); - let main = bytes(path)?; + let main = FileSource::open(path)?; let wal = match std::fs::metadata(&wal_path) { Ok(_) => bytes(&wal_path)?, Err(e) if e.kind() == std::io::ErrorKind::NotFound => Vec::new(), Err(e) => return Err(e.to_string()), }; - let wal_len = committed_wal_len(&main, &wal)?; + let wal_len = committed_wal_len(main.header(), &wal)?; ensure_read_active(cancel)?; - let db = Database::open_with_wal(main, &wal[..wal_len]).map_err(|e| format!("invalid SQLite catalog: {e:?}"))?; + let db = Database::open_file_with_wal(main, &wal[..wal_len]).map_err(|e| format!("invalid SQLite catalog: {e:?}"))?; let after = std::fs::metadata(path).map_err(|e| e.to_string())?; let wal_after = std::fs::metadata(&wal_path).ok().map(|m| (m.len(), m.modified().ok())); if before.len() != after.len() || before.modified().ok() != after.modified().ok() || wal_before != wal_after { @@ -471,6 +471,11 @@ pub fn read_with_progress(path: &Path, cancel: &AtomicBool, total: &AtomicUsize, }); } ensure_read_active(cancel)?; + reader.db.ensure_unchanged()?; + let wal_after = std::fs::metadata(&wal_path).ok().map(|m| (m.len(), m.modified().ok())); + if wal_before != wal_after { + return Err("catalog changed during reading; close Lightroom and retry".into()); + } Ok(out) } @@ -1302,6 +1307,30 @@ mod tests { assert_eq!(catalog.photos[0].uuid, "fixture-image"); assert_eq!(number(&catalog.photos[0].image, "rating"), 5); assert!(catalog.photos[0].path.ends_with("test.ARW")); + // Put the required image table beyond the old 1 GiB cutoff without allocating it. + // A main-file-sized allocation would defeat the purpose of the page-backed reader. + // The large fixture relies on Unix sparse-file behavior; small parity runs everywhere. + #[cfg(unix)] + { + let far_offset = 1u64 << 30; + schema.last_mut().unwrap()[3] = I((far_offset / 2048 + 1) as i64); + leaf(&mut bytes[..2048], 100, &schema); + use std::io::{Seek, SeekFrom, Write}; + let mut sparse = std::fs::OpenOptions::new().write(true).truncate(true).open(&path).unwrap(); + sparse.write_all(&bytes).unwrap(); + sparse.set_len(far_offset + 2048).unwrap(); + sparse.seek(SeekFrom::Start(far_offset)).unwrap(); + sparse.write_all(&bytes[4 * 2048..5 * 2048]).unwrap(); + drop(sparse); + let large = read(&path).unwrap(); + assert_eq!(serde_json::to_value(&large.photos).unwrap(), serde_json::to_value(&catalog.photos).unwrap()); + let mut session = crate::Session::new(); + session.execute("library.importLightroom", &json!({"path": path.to_string_lossy()})).unwrap(); + assert_eq!(session.catalog.len(), 1); + assert_eq!(session.catalog.photos().next().unwrap().rating, 5); + session.execute("edit.undo", &json!({})).unwrap(); + assert!(session.catalog.is_empty()); + } std::fs::write(&path, b"bad catalog").unwrap(); assert!(read(&path).is_err()); std::fs::remove_file(path).unwrap(); diff --git a/crates/engine/src/lightroom_sqlite.rs b/crates/engine/src/lightroom_sqlite.rs index 900fe4a26..f00cb111f 100644 --- a/crates/engine/src/lightroom_sqlite.rs +++ b/crates/engine/src/lightroom_sqlite.rs @@ -6,7 +6,79 @@ //! than a SQL engine. It never writes its input and has no SQLite runtime //! dependency. +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::fs::{File, Metadata}; +use std::io::{Read, Seek, SeekFrom}; +use std::path::{Path, PathBuf}; +use std::sync::Mutex; + +const MAX_DATABASE_BYTES: u64 = 16 << 30; + +/// A bounded, read-only page source. Only referenced pages are loaded, never the whole file. +pub struct FileSource { + file: Mutex, + path: PathBuf, + before: Metadata, + header: Vec, +} + +impl FileSource { + pub fn open(path: &Path) -> Result { + if !std::fs::metadata(path).map_err(|e| e.to_string())?.is_file() { + return Err("SQLite catalog must be a regular file".into()); + } + let mut file = File::open(path).map_err(|e| e.to_string())?; + let before = file.metadata().map_err(|e| e.to_string())?; + if !before.is_file() { + return Err("SQLite catalog must be a regular file".into()); + } + if before.len() > MAX_DATABASE_BYTES { + return Err("catalog exceeds 16 GiB import limit".into()); + } + let mut header = vec![0; 100]; + file.read_exact(&mut header).map_err(|e| format!("cannot read SQLite header: {e}"))?; + let source = Self { file: Mutex::new(file), path: path.into(), before, header }; + source.ensure_unchanged()?; + Ok(source) + } + + pub fn header(&self) -> &[u8] { + &self.header + } + + pub fn ensure_unchanged(&self) -> Result<(), String> { + let file = self.file.lock().map_err(|_| "SQLite page source lock poisoned")?; + let handle = file.metadata().map_err(|e| e.to_string())?; + let path = std::fs::metadata(&self.path).map_err(|_| "catalog changed during reading; close Lightroom and retry")?; + if !same_metadata(&self.before, &handle) || !same_metadata(&self.before, &path) { + return Err("catalog changed during reading; close Lightroom and retry".into()); + } + Ok(()) + } + + fn page(&self, page_id: u32, size: usize) -> Result, String> { + let start = u64::from(page_id).checked_sub(1).and_then(|n| n.checked_mul(size as u64)).ok_or("page offset overflow")?; + if start.checked_add(size as u64).is_none_or(|end| end > self.before.len()) { + return Err(format!("page {page_id} is truncated")); + } + let mut out = vec![0; size]; + let mut file = self.file.lock().map_err(|_| "SQLite page source lock poisoned")?; + file.seek(SeekFrom::Start(start)).and_then(|_| file.read_exact(&mut out)).map_err(|e| format!("cannot read SQLite page {page_id}: {e}"))?; + Ok(out) + } +} + +fn same_metadata(a: &Metadata, b: &Metadata) -> bool { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if a.dev() != b.dev() || a.ino() != b.ino() { + return false; + } + } + a.len() == b.len() && a.modified().ok() == b.modified().ok() && a.created().ok() == b.created().ok() +} const MAX_PAGE_ID: u32 = 16_777_216; const MAX_COLUMNS: usize = 16_384; @@ -44,6 +116,7 @@ pub struct LiveTable { /// Read-only SQLite database image, with a committed WAL overlay if supplied. pub struct Database { main: Vec, + file: Option, overlay: HashMap>, page_size: usize, reserved: usize, @@ -84,17 +157,34 @@ impl Database { /// The WAL argument must include its 32-byte header and only complete /// frames through a commit. Frames are overlaid by page number, with the /// final commit's database-size field defining the visible page count. + #[cfg(test)] pub fn open_with_wal(main: Vec, committed_wal: &[u8]) -> Result { + Self::open(main, None, committed_wal) + } + + pub fn open_file_with_wal(source: FileSource, committed_wal: &[u8]) -> Result { + Self::open(source.header().to_vec(), Some(source), committed_wal) + } + + pub fn ensure_unchanged(&self) -> Result<(), String> { + if let Some(file) = &self.file { + file.ensure_unchanged()?; + } + Ok(()) + } + + fn open(main: Vec, file: Option, committed_wal: &[u8]) -> Result { let main_header = main.get(..100).ok_or("truncated SQLite header")?; if main_header.get(..16) != Some(b"SQLite format 3\0") { return Err("invalid SQLite header signature".into()); } let page_size = sqlite_page_size(be_u16(main_header, 16)?)?; validate_header_fields(main_header, page_size)?; - if main.len() < page_size || !main.len().is_multiple_of(page_size) { + let main_len = file.as_ref().map_or(main.len() as u64, |source| source.before.len()); + if main_len < page_size as u64 || !main_len.is_multiple_of(page_size as u64) { return Err("SQLite image is not a whole number of pages".into()); } - let main_pages = u32::try_from(main.len() / page_size).map_err(|_| "SQLite image is too large")?; + let main_pages = u32::try_from(main_len / page_size as u64).map_err(|_| "SQLite image is too large")?; if main_pages == 0 || main_pages > MAX_PAGE_ID { return Err("invalid SQLite image page count".into()); } @@ -140,7 +230,7 @@ impl Database { _ => return Err("unsupported SQLite text encoding".into()), }; - let mut db = Self { main, overlay, page_size, reserved, encoding, page_count, tables: Vec::new(), rowid_aliases: HashMap::new() }; + let mut db = Self { main, file, overlay, page_size, reserved, encoding, page_count, tables: Vec::new(), rowid_aliases: HashMap::new() }; let schema = db.parse_schema()?; db.tables = schema.tables; db.rowid_aliases = schema.aliases; @@ -166,17 +256,20 @@ impl Database { Ok(state.out) } - fn page(&self, page_id: u32) -> Result<&[u8], String> { + fn page(&self, page_id: u32) -> Result, String> { if page_id == 0 || page_id > self.page_count { return Err(format!("page {page_id} is outside database")); } if let Some(page) = self.overlay.get(&page_id) { - return Ok(page); + return Ok(Cow::Borrowed(page)); + } + if let Some(file) = &self.file { + return file.page(page_id, self.page_size).map(Cow::Owned); } let id = usize::try_from(page_id).map_err(|_| "page number does not fit host")?; let start = id.checked_sub(1).and_then(|n| n.checked_mul(self.page_size)).ok_or("page offset overflow")?; let end = start.checked_add(self.page_size).ok_or("page offset overflow")?; - self.main.get(start..end).ok_or_else(|| format!("page {page_id} is truncated")) + self.main.get(start..end).map(Cow::Borrowed).ok_or_else(|| format!("page {page_id} is truncated")) } fn visit_table(&self, page_id: u32, ncols: usize, alias: Option, depth: usize, state: &mut WalkState) -> Result<(), String> { @@ -219,7 +312,7 @@ impl Database { if state.payload_total > MAX_TABLE_PAYLOAD { return Err("SQLite table payload exceeds bounded import size".into()); } - let payload = self.payload(page, at, payload_len)?; + let payload = self.payload(&page, at, payload_len)?; let mut values = decode_record(&payload, ncols, self.encoding)?; if let Some(index) = alias { if index >= values.len() { @@ -325,7 +418,7 @@ impl Database { out.extend_from_slice(overflow.get(4..end).ok_or("truncated SQLite overflow page")?); remaining -= take; if remaining > 0 { - next = be_u32_at(overflow, 0)?; + next = be_u32_at(&overflow, 0)?; } } Ok(out) @@ -871,6 +964,72 @@ fn be_u32_at(bytes: &[u8], at: usize) -> Result { mod tests { use super::*; + struct Scratch(PathBuf); + impl Scratch { + fn new(bytes: &[u8]) -> Self { + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let path = std::env::temp_dir().join(format!("lightcraft-page-source-{}-{n}.lrcat", std::process::id())); + std::fs::write(&path, bytes).unwrap(); + Self(path) + } + } + impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.0); + } + } + + fn file_backed(mut db: Database) -> (Scratch, Database) { + let scratch = Scratch::new(&db.main); + db.file = Some(FileSource::open(&scratch.0).unwrap()); + db.main.truncate(100); + (scratch, db) + } + + #[test] + fn file_source_bounds_and_mutation_detection() { + let scratch = Scratch::new(&[0; 512]); + let source = FileSource::open(&scratch.0).unwrap(); + assert_eq!(source.page(1, 512).unwrap().len(), 512); + assert!(source.page(2, 512).unwrap_err().contains("truncated")); + File::options().write(true).open(&scratch.0).unwrap().set_len(100).unwrap(); + assert!(source.page(1, 512).is_err()); + assert!(source.ensure_unchanged().unwrap_err().contains("changed during reading")); + drop(source); + // Unix set_len creates a hole. Do not reserve a 16 GiB normal file on Windows. + #[cfg(unix)] + { + File::options().write(true).open(&scratch.0).unwrap().set_len(MAX_DATABASE_BYTES + 1).unwrap(); + assert_eq!(FileSource::open(&scratch.0).err().unwrap(), "catalog exceeds 16 GiB import limit"); + } + } + + #[cfg(unix)] + #[test] + fn file_source_detects_same_sized_path_replacement() { + let original = Scratch::new(&[0; 512]); + let replacement = Scratch::new(&[0; 512]); + let source = FileSource::open(&original.0).unwrap(); + std::fs::rename(&replacement.0, &original.0).unwrap(); + assert!(source.ensure_unchanged().unwrap_err().contains("changed during reading")); + } + + #[test] + fn file_source_detects_same_length_content_changes() { + let scratch = Scratch::new(&[0; 512]); + let source = FileSource::open(&scratch.0).unwrap(); + std::fs::write(&scratch.0, [1; 512]).unwrap(); + // Make the timestamp change deterministic even on filesystems with coarse timestamps. + File::options() + .write(true) + .open(&scratch.0) + .unwrap() + .set_modified(source.before.modified().unwrap() + std::time::Duration::from_secs(2)) + .unwrap(); + assert!(source.ensure_unchanged().unwrap_err().contains("changed during reading")); + } + fn varint(mut value: u64) -> Vec { let mut out = vec![u8::try_from(value & 0x7f).unwrap_or(0)]; value >>= 7; @@ -996,6 +1155,40 @@ mod tests { assert!(tables.iter().any(|table| table.name == "t")); let rows = db.read_table(2, 2).unwrap(); assert_eq!(rows.first().and_then(|row| row.values.first()), Some(&Value::Integer(1))); + let scratch = Scratch::new(&db.main); + let file_db = Database::open_file_with_wal(FileSource::open(&scratch.0).unwrap(), &[]).unwrap(); + assert_eq!(file_db.live_tables().unwrap(), tables); + assert_eq!(file_db.read_table(2, 2).unwrap(), rows); + assert_eq!(file_db.main.len(), 100); + file_db.ensure_unchanged().unwrap(); + // The same committed WAL page overrides the lazily read main page. + let mut wal = vec![0; 32 + 24 + 512]; + wal[..4].copy_from_slice(&0x377f0682u32.to_be_bytes()); + wal[4..8].copy_from_slice(&3_007_000u32.to_be_bytes()); + wal[8..12].copy_from_slice(&512u32.to_be_bytes()); + wal[32..36].copy_from_slice(&2u32.to_be_bytes()); + wal[36..40].copy_from_slice(&2u32.to_be_bytes()); + leaf(&mut wal[56..], 0, 2, &[3, 0, 15, b'y']); + let memory_wal = Database::open_with_wal(db.main.clone(), &wal).unwrap(); + let file_wal = Database::open_file_with_wal(FileSource::open(&scratch.0).unwrap(), &wal).unwrap(); + assert_eq!(memory_wal.read_table(2, 2).unwrap(), file_wal.read_table(2, 2).unwrap()); + assert_eq!(file_wal.read_table(2, 2).unwrap()[0].values[0], Value::Integer(2)); + let mut malformed = db.main.clone(); + malformed.push(0); + let partial = Scratch::new(&malformed); + assert_eq!( + Database::open_file_with_wal(FileSource::open(&partial.0).unwrap(), &[]).err().unwrap(), + "SQLite image is not a whole number of pages" + ); + let mut malformed = db.main.clone(); + malformed[24..28].copy_from_slice(&1u32.to_be_bytes()); + malformed[92..96].copy_from_slice(&1u32.to_be_bytes()); + malformed[28..32].copy_from_slice(&3u32.to_be_bytes()); + let oversized = Scratch::new(&malformed); + assert_eq!( + Database::open_file_with_wal(FileSource::open(&oversized.0).unwrap(), &[]).err().unwrap(), + "SQLite header page count exceeds image" + ); } #[test] @@ -1012,6 +1205,7 @@ mod tests { leaf(&mut main[1024..1536], 0, 1, &[2, 0]); leaf(&mut main[1536..], 0, 2, &[2, 0]); let db = Database { + file: None, main, overlay: HashMap::new(), page_size: 512, @@ -1043,6 +1237,7 @@ mod tests { let mut aliases = HashMap::new(); aliases.insert(1, 0); let db = Database { + file: None, main, overlay: HashMap::new(), page_size: 512, @@ -1063,6 +1258,7 @@ mod tests { main[103..105].copy_from_slice(&1u16.to_be_bytes()); main[108..110].copy_from_slice(&490u16.to_be_bytes()); let db = Database { + file: None, main, overlay: HashMap::new(), page_size: 512, @@ -1073,6 +1269,8 @@ mod tests { rowid_aliases: HashMap::new(), }; assert!(db.read_table(1, 1).is_err()); + let (_scratch, db) = file_backed(db); + assert!(db.read_table(1, 1).is_err()); } #[test] @@ -1085,6 +1283,7 @@ mod tests { let mut main = vec![0u8; 2 * 512]; let db = Database { + file: None, main: std::mem::take(&mut main), overlay: HashMap::new(), page_size: 512, @@ -1111,6 +1310,10 @@ mod tests { assert_eq!(bytes.len(), 600); assert_eq!(&bytes[..92], &[b'x'; 92]); assert_eq!(&bytes[92..], &[b'y'; 508]); + let (_scratch, mut db) = file_backed(db); + db.overlay.clear(); + let bytes = db.payload(&cell_page, 0, 600).unwrap(); + assert_eq!(&bytes[92..], &[b'y'; 508]); } #[test] @@ -1134,6 +1337,7 @@ mod tests { let mut page = vec![0u8; 512]; page[16..18].copy_from_slice(&512u16.to_be_bytes()); let db = Database { + file: None, main: vec![0; 512], overlay: HashMap::new(), page_size: 512, diff --git a/docs/gaps.md b/docs/gaps.md index 64b1cb8ac..98cc17548 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -27,7 +27,7 @@ Hours are Opus 5.5 agent wall-clock hours (see [target-app-parity.md](target-app | 10 B | **Per-model verification thin, plus cheap per-body decode fixes**: 234 of 1,446 Adobe models verified (16%), ~40% of photographers on a verified body; Nikon 5 bodies verified, Canon 11. Cheap fixes: Z 6II packed 14-bit NEF (#769, PR #671), Sony A100 / F828 regression (#702), DSLR-A200…A390 / A850 12-bit ARW (#535) | measured: [raw/coverage.tsv](raw/coverage.tsv) | per-body bugs (levels, crops, WB) surface only from users | 25–45 | [raw-parity](raw-parity.md#by-maker-measured) | | 11 | **Enhance: Raw Details, Super Resolution; AI denoise without shippable weights or X-Trans** | ENH rows 0%; denoise Bayer only, RawNIND weights GPL and opt-in | Fujifilm users get no AI denoise; nobody gets it out of the box | 30–60 + model decision | [raw-parity](raw-parity.md#demosaic-levels-crops-and-lens-metadata) | | 12 | **Content-aware / generative remove, detect distractions, reflections removal** | REM rows 75%; patch synthesis missing | cleanup work goes back to Photoshop | 25–45 (+ generative model) | [target-app-parity](target-app-parity.md#ai) | -| 13 | **Lightroom catalog migration is one-way and approximate**: unsupported Adobe profiles / AI settings archived, history kept as data | lightroom-catalog-import.md | switching users lose some looks | 8–12 | [file-format-parity](file-format-parity.md) | +| 13 | **Lightroom catalog migration is one-way and approximate**: unsupported Adobe profiles / AI settings archived, history kept as data; main-file pages read on demand up to 16 GiB, with dense-table and WAL bounds retained (#798) | lightroom-catalog-import.md | switching users lose some looks | 8–12 | [file-format-parity](file-format-parity.md) | | 14 | **HDR display** (EDR / Windows HDR); HLG; HDR settings from `crs` XMP | HDR rows 70%; LR-VIEW-HDR-DISPLAY ⬜ | HDR editing works blind on HDR monitors | 12–25 | [hardware-parity](hardware-parity.md) | | 15 | **Print module**: custom packages, printer colour management (contact-sheet PDF only) | LRC-PRINT rows | Classic users who print | 25–40 | [target-app-parity](target-app-parity.md#features) | | 16 | **Map view, Book, Slideshow module, publish services** | LRC-MAP / BOOK / SS rows ⬜ | Classic users; publish services matter for Flickr / SmugMug workflows | 60–100 | [target-app-parity](target-app-parity.md#features) | diff --git a/docs/lightroom-catalog-import.md b/docs/lightroom-catalog-import.md index 0c18a6521..3ae13c75e 100644 --- a/docs/lightroom-catalog-import.md +++ b/docs/lightroom-catalog-import.md @@ -28,9 +28,16 @@ retained. Oldest managed archives are retired; legacy `.json` archives remain un `Interop/lightroom-index.json` records imported identities. Source catalogs and originals are never overwritten. -Native import is bounded to a 1 GiB database/WAL, 16 MiB XMP packets and one million rows per -table. The in-tree, read-only Rust SQLite B-tree reader follows SQLite’s public file-format -specification; it introduces no dependency or C runtime. Table payloads are capped at 256 MiB. +Native import reads referenced database pages on demand (at most 64 KiB per page), rather than +loading the whole `.lrcat`. Database files are capped at 16 GiB and 16,777,216 pages; the WAL +remains capped at 1 GiB. Tables still have a 256 MiB encoded-payload budget and one million rows, +individual records are capped at 64 MiB, and XMP packets at 16 MiB. These limits can still reject +large catalogs with dense tables; file size alone no longer determines the memory allocation. +The open file and source path are checked again after all tables are read. Close Lightroom before +importing: metadata checks detect ordinary concurrent changes, but are not SQLite snapshot isolation. + +The in-tree, read-only Rust SQLite B-tree reader follows SQLite's public file-format +specification; it introduces no dependency or C runtime. Optional history, keyword and collection tables that exceed limits are skipped with warnings; required photo tables still report errors. Unrelated unsupported schemas are ignored. A needed WITHOUT ROWID or virtual table produces an explicit error, handled as a warning when optional. diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 099411089..2b6cf4814 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -679,7 +679,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-SURVEY | Survey view | P2 | ✅ | `cmd:view.survey` | N | | LRC-LIB-REFVIEW | Reference view | P2 | ✅ | `cmd:view.reference`, `cmd:photo.setReference`, `crates/ui-egui/src/panels/compare.rs` (`show_reference`) | View ▸ Reference View (⇧R): the reference photo beside the active one (edited with the Edit panel); photo menu ▸ Set as Reference Photo | | LRC-LIB-CATALOG-PANEL | Catalog sets | P1 | ✅ | `cmd:library.source` (`missing`) | all, recently added, picks, recently deleted, missing photos (when any) | -| LRC-LIB-CATALOG-IMPORT | Native Lightroom Classic catalog import | P1 | 🟡 | `cmd:library.inspectLightroom`, `cmd:library.importLightroom`, `crates/engine/src/lightroom_catalog.rs`, `docs/lightroom-catalog-import.md` | read-only `.lrcat` plus committed WAL pages, in-tree read-only Rust SQLite B-tree reader; background inspection/import with progress/cancellation, per-open library identity checks; originals in place, ratings/flags/labels, XMP, hierarchical keywords, collection sets/regular collections, virtual copies, mapped develop settings; stable reimport identities, undo, missing-file placeholders, existing edits preserved. Best-effort deduplicated recovery archives retain settings/history/snapshots/collection rules (32 MiB each, eight archives/128 MiB aggregate); archive limits/write failures and optional-table limits warn without blocking photo import; unrelated unsupported tables and opaque SQL BLOBs skipped. Custom white balance on relative-WB raws (`RAW` in Lightroom's terms) stays As Shot and is reported, since the catalog keeps no as-shot white to shift from (issue #510). Adobe rendering, AI data and native history replay remain incomplete; no writes to Lightroom catalogs | +| LRC-LIB-CATALOG-IMPORT | Native Lightroom Classic catalog import | P1 | 🟡 | `cmd:library.inspectLightroom`, `cmd:library.importLightroom`, `crates/engine/src/lightroom_catalog.rs`, `docs/lightroom-catalog-import.md` | read-only `.lrcat` plus committed WAL pages, in-tree read-only Rust SQLite B-tree reader; on-demand main-file pages up to 16 GiB with existing table/record bounds (WAL still capped at 1 GiB, issue #798); background inspection/import with progress/cancellation, per-open library identity checks; originals in place, ratings/flags/labels, XMP, hierarchical keywords, collection sets/regular collections, virtual copies, mapped develop settings; stable reimport identities, undo, missing-file placeholders, existing edits preserved. Best-effort deduplicated recovery archives retain settings/history/snapshots/collection rules (32 MiB each, eight archives/128 MiB aggregate); archive limits/write failures and optional-table limits warn without blocking photo import; unrelated unsupported tables and opaque SQL BLOBs skipped. Custom white balance on relative-WB raws (`RAW` in Lightroom's terms) stays As Shot and is reported, since the catalog keeps no as-shot white to shift from (issue #510). Adobe rendering, AI data and native history replay remain incomplete; no writes to Lightroom catalogs | | LRC-LIB-CATALOG-LOCK | Catalog open in one program at a time | P1 | ✅ | `cmd:app.openLibrary`, `crates/catalog/src/lock.rs`, `crates/engine/src/library.rs` (`open_library`) | exclusive OS lock on `catalog.lock` for the session (app and `lightcraft-cli --library`); a second opener is refused with who has it (`catalog.lock.owner`: program, process, computer); released by the OS on a crash, so never stale; file systems without locks open unprotected with a warning (the app's notices, `library.info` → `settingsWarnings`, and the CLI's stderr; issue #171); no read-only mode; the browser build (OPFS) has no guard across tabs yet. A library that can't be opened at launch (locked, unreadable, newer format, drive missing) shows a window — Try Again, Choose Another Library…, Continue Without Saving (then a banner; nothing written), Quit — instead of a silent demo session (`crates/ui-egui/src/panels/library_problem.rs`) | | LRC-LIB-FOLDERS | Disk folder tree | P1 | ✅ | `cmd:library.browse`, `cmd:folder.rename`, `cmd:folder.move`, `crates/ui-egui/src/panels/left.rs` (`local_section`, `folder_tree`) | standard places + the browsed folder + Browse Folder…, each an expandable tree of subfolders, breadcrumb navigation; context menu: Rename Folder…, Move Folder To…, Show in Finder (on disk, photos relinked; one undo step each: undo / redo move the folder back and forth, refused when the destination is taken) | | LRC-LIB-LIBFOLDERS | Library folders | P1 | ✅ | `cmd:library.folders`, `cmd:library.source` (`libraryFolder`), `cmd:library.filter` (`libraryFolder`), `cmd:library.removeFolder`, `cmd:folder.rename`, `cmd:folder.move`, `cmd:folder.label`, `cmd:folder.scanChanges`, `cmd:folder.synchronize`, `crates/catalog/src/folders.rs`, `crates/engine/src/sync.rs`, `crates/ui-egui/src/sync.rs`, `crates/ui-egui/src/panels/left.rs` (`folders_section`, `folder_menu_for_library`, `row_menu`), `crates/ui-egui/src/tests_panels.rs` | sidebar Folders: the folders the library's photos were imported from, with photo counts (subfolders included), as another way to explore the library next to By Date and Keywords; Local stays the way to browse any folder on disk. Each disk is a row of its own and starts open: a drive letter, a Windows share (`\\server\share`, `\\?\UNC\…`, `\\wsl$\…`), a macOS share or disk (`/Volumes/tokyo`), a Linux mount (`/media`, `/run/media`, `/mnt`, gvfs `smb-share:…`, `/net/`), NFS (`/Network/Servers/`), else “This Computer” — told apart from the stored paths alone, so an offline share lists like a mounted one and the sidebar never waits on a network. Only folders that hold imported photos are listed (browsed, deleted and demo photos don't count); every folder is a row of its own (`Users` → `me` → `Pictures`), and a folder that holds nothing itself and leads to one folder starts open; every level indents one step more than its parent however deep it goes: the whole sidebar (Albums, Local, By Date, Folders, Keywords) scrolls sideways when its widest row is wider than the panel (rows are measured one frame behind; Create Album and the counts stay at the visible edge); a name that does not fit ends in an ellipsis at the count (the full name on hover; a row asks for room for at most 140 pt of its name, so depth, not one long name, makes the sidebar scroll), as does the grid title; selection and hover bars end at the panel's edge; click makes the folder the source, like an album or a Local folder (its photos and those of the folders inside it fill the grid, titled “photos/travel”; the filter bar narrows it further; `library.source` `libraryFolder` + `path`; kept with the library's view), and its row is always opened into view; the shown folder follows a rename or move and their undo (engine-tested; the sidebar test covers rename), is dropped back to All Photos when it is removed or no longer holds a photo, and a smart album made from the view keeps it, also when its rules are edited (the editor has no folder field). Only a folder that shows exactly its own photos can be shown (not the startup disk, not `/Volumes`-like folders above disks, not an empty one). A row that would cover other disks' photos too (the startup disk, `/Volumes`, `/mnt`…) only opens. Right-click (row or triangle): Rename Folder…, Move Folder To…, Show in Finder (on disk, photos relinked, as in Local), Set Color Label ▸ (the five colours with their custom names, None, Edit Label Names…; `folder.label`, one undo step; the row shows the label's dot before its count and says it with its name; kept in the catalog as a folder record under the folder's identity, so it follows Rename / Move and their undo and redo, the folders inside going along), Synchronize Folder… (the dialog opens at once and scans the folder and the folders inside it on a worker thread — the look for new files beside parallel checks of the library's photos — then offers Import new photos (checked; files only browsed in Local count as new), Relink photos whose file was renamed or moved within the folder (checked; matched by content — a Duplicate's too — or, for a photo with no hash on record such as one from a Lightroom catalog, by file size and dimensions when exactly one file agrees), Remove missing photos from the library (to Recently Deleted) and Read metadata updates from XMP sidecars (saved after the photo came in or was last edited here, and saying something the library doesn't; the sidecar wins), with counts and skipped duplicates / unreadable files; `folder.scanChanges` / `folder.synchronize`, which acts on exactly the scan shown — a scan whose folder's photos changed meanwhile is refused and made again in the background — and is one undo step, also with a full history; a folder that isn't there (an unplugged disk) is reported as such, not as every photo missing; a whole disk or a folder holding disks needs `disk: true`; Esc stops the scan; Synchronize hands everything to a worker thread — the import, the checks that moved and missing files are still as found, the sidecar reads — and commits its steps between frames, shown in the activity stack with ✕ to stop, so no file is touched on the UI thread) and Remove from Library… (asks; moves the folder's photos to Recently Deleted, one undo step, no file touched); a disk row (not the startup disk) offers Set Color Label ▸, Synchronize Folder… and Remove Disk from Library… (the command needs `disk: true` for a whole disk or a folder spanning several). Limits: names are compared as written (macOS `/Volumes/tokyo` vs `/Volumes/TOKYO` are two disks; `/Volumes/Macintosh HD/…` and `/…` two rows); no disk capacity / online status; no “include subfolders” switch (a folder always includes the ones inside it); creating a folder and moving photos between folders are not here; folders can't be filtered or sorted by label, and a label stays with the old path when the folder is moved outside LightCraft (Find Missing relinks photos, not folder records); Synchronize has no “show import dialog first” or “show missing photos” option, and only knows when a photo came in or was last developed here: a rating, label or metadata change made here (auto-write off) after another app saved the sidecar is listed as an update, and reading it restores the sidecar's older value (reading is never the default); the walk for new files isn't cancelled midway and its depth/entry limits are only logged | From a2911a508621273c0c6f4cfed0b8387824ce844e Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Tue, 13 Oct 2026 12:56:50 +0200 Subject: [PATCH 02/99] LR-VIEW-ZOOM: drag-scale follow-ups from review (#330) - A full set of slow drags forgets only the one remembered longest ago instead of all 256 (each forgotten pair costs one more slow full-size drag). - DragScale's doc says a slow drag is remembered for its photo only. - A host frame time over 1 s (a hidden, occluded or stalled window) counts as unknown, like no frame time at all, so it can't mark a drag slow. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/region.rs | 69 ++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/region.rs b/crates/ui-egui/src/region.rs index 4fb2e587b..f02f80df5 100644 --- a/crates/ui-egui/src/region.rs +++ b/crates/ui-egui/src/region.rs @@ -107,14 +107,20 @@ const GIVE_UP_FACTOR: f64 = 2.0; /// The full-size drafts (after the first) a drag times before it may give up on full size. const GIVE_UP_AFTER: usize = 3; -/// The most (photo, control) pairs [`DragScale`] remembers as too slow for full-size drafts. +/// The most (photo, control) pairs [`DragScale`] remembers as too slow for full-size drafts; past +/// it, the one remembered longest ago is forgotten. const SLOW_DRAGS_KEPT: usize = 256; +/// A host frame time (ms) longer than this is not a texture upload but a window that was hidden, +/// occluded or stalled: it counts as unknown, like a frame time from no host at all. +const FRAME_MS_KNOWN: f64 = 1000.0; + /// The size of a slider drag's whole-frame draft, chosen when the drag starts and kept until it /// ends (a change in the middle would make the resolution jump), unless its full-size drafts are /// far over budget. A drag renders at full size when the GPU draws the view and no earlier drag of /// the same control on this photo went over budget at that size; otherwise at [`CPU_DRAFT_SCALE`] -/// (issue #330). Whether the GPU drew a render is not enough on its own: some views (a LUT +/// (issue #330). A slow drag is remembered for its photo only: the same control on another photo +/// (another size, other edits) starts at full size again and is timed afresh. Whether the GPU drew a render is not enough on its own: some views (a LUT /// profile, refined masks, a photo past a GPU limit) fall back to the CPU per render, and a large /// draft costs a texture upload too, which only the time a draft took to show tells. #[derive(Clone, Debug, Default)] @@ -128,8 +134,8 @@ pub struct DragScale { /// The render time (ms) of a draft that reached the screen this frame: the time of the frame /// that uploads it is only known on the next one. pending: Option, - /// Drags whose full-size drafts went over budget. - slow: std::collections::HashSet<(lightcraft_catalog::PhotoId, String)>, + /// Drags whose full-size drafts went over budget, the one remembered longest ago first. + slow: std::collections::VecDeque<(lightcraft_catalog::PhotoId, String)>, } impl DragScale { @@ -165,7 +171,7 @@ impl DragScale { return; } if let Some(render) = self.pending.take() { - let frame = last_frame_ms.filter(|ms| ms.is_finite() && *ms >= 0.0).unwrap_or(0.0); + let frame = last_frame_ms.filter(|ms| ms.is_finite() && (0.0..=FRAME_MS_KNOWN).contains(ms)).unwrap_or(0.0); if self.samples.len() < 1024 { self.samples.push(render + frame); } @@ -189,7 +195,19 @@ impl DragScale { /// Remember drags of `control` on `photo` as too slow for full-size drafts (tests). #[cfg(test)] pub fn remember_slow(&mut self, photo: lightcraft_catalog::PhotoId, control: &str) { - self.slow.insert((photo, control.to_string())); + self.mark_slow((photo, control.to_string())); + } + + /// Remember a slow drag, forgetting the one remembered longest ago when the set is full (not + /// all of them: each forgotten pair costs one more slow full-size drag). + fn mark_slow(&mut self, key: (lightcraft_catalog::PhotoId, String)) { + if self.slow.contains(&key) { + return; + } + while self.slow.len() >= SLOW_DRAGS_KEPT { + self.slow.pop_front(); + } + self.slow.push_back(key); } /// The drag ended: remember it if its full-size drafts were over budget. @@ -197,10 +215,7 @@ impl DragScale { if let Some((key, _)) = self.current.take() && median_after_first(&self.samples).is_some_and(|m| m > DRAFT_BUDGET_MS) { - if self.slow.len() >= SLOW_DRAGS_KEPT { - self.slow.clear(); - } - self.slow.insert(key); + self.mark_slow(key); } self.samples.clear(); self.seen = None; @@ -522,6 +537,40 @@ mod tests { assert_eq!(hostile.scale(drag, true, None), Some(1.0)); } + // Given more slow drags than are remembered, only the one remembered longest ago is forgotten: + // clearing them all would bring back a slow full-size drag for every pair + #[test] + fn a_full_slow_set_forgets_only_the_oldest_drag() { + use lightcraft_catalog::PhotoId; + let mut d = DragScale::default(); + for photo in 0..=SLOW_DRAGS_KEPT as u64 { + d.remember_slow(PhotoId(photo), "Exposure"); + } + assert_eq!(d.scale(Some((PhotoId(0), "Exposure")), true, None), Some(1.0), "the oldest is forgotten"); + d.scale(None, true, None); + for photo in [1, SLOW_DRAGS_KEPT as u64] { + assert_eq!(d.scale(Some((PhotoId(photo), "Exposure")), true, None), Some(CPU_DRAFT_SCALE), "photo {photo}"); + d.scale(None, true, None); + } + d.remember_slow(PhotoId(1), "Exposure"); // already remembered: nothing is forgotten + assert_eq!(d.scale(Some((PhotoId(2), "Exposure")), true, None), Some(CPU_DRAFT_SCALE)); + } + + // Given draft frames that took seconds (the window hidden or occluded mid-drag), they count as + // unknown: fast renders don't mark the drag slow because the window wasn't drawing + #[test] + fn a_hidden_windows_frame_times_dont_mark_a_drag_slow() { + use lightcraft_catalog::PhotoId; + let mut d = DragScale::default(); + let drag = Some((PhotoId(1), "Exposure")); + d.scale(drag, true, None); + for key in 1..=6 { + d.observe(key, 6.0, Some(4000.0)); + } + d.scale(None, true, None); + assert_eq!(d.scale(drag, true, None), Some(1.0)); + } + // Given full-size drafts far over budget (a refined mask the CPU renders at 190 ms a step), // the drag drops to the small draft after a few of them instead of at its end, and the next // drag starts small From 8d91abe60330d28e9ebce3e37e9170b93832be11 Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Tue, 13 Oct 2026 13:33:38 +0200 Subject: [PATCH 03/99] LR-VIEW-ZOOM: a slow drag verdict needs two strikes and expires (#330) A busy moment (another app, a backup) could make a slider draft small for the rest of the session. DragScale keeps timing in wall-clock time, what the user waits for, but now: - a (photo, control) pair is slow only after two drags in a row over 20 ms (1.25x the 16 ms budget); the first makes it suspect, and a drag within 20 ms forgets the verdict; - a slow verdict lasts 30 s, then a drag tries full size again; a slow retry doubles the wait, up to 240 s (after WebRTC's CPU overuse detector: 40 s ramp-up delay, doubled after a failed up-switch, capped at 240 s). The time comes from egui's input clock, which the web build has too. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/detail.rs | 4 +- crates/ui-egui/src/region.rs | 319 ++++++++++++++++++++-------- 2 files changed, 235 insertions(+), 88 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index ba14091a7..6661ce92f 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -445,7 +445,8 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { && lightcraft_engine::gpu::available(); let shown = app.renderer.textures.get(&Slot::Main).filter(|t| t.photo == id && t.quick.is_none()).map(|t| (t.key, t.ms)); let drag = app.session.interaction.as_ref().map(|i| (i.photo, i.label.as_str())); - let draft = app.drag_scale.scale(drag, gpu, shown.map(|(key, _)| key)); + let now = ui.input(|i| i.time); + let draft = app.drag_scale.scale(drag, gpu, shown.map(|(key, _)| key), now); // render at the final size: a click-zoom animation only changes how the result is drawn let native_long = native[0].max(native[1]); let drawn_long = target_rect.width().max(target_rect.height()) * ppp; @@ -471,7 +472,6 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { app.drag_scale.observe(key, ms, app.perf.host_frame_ms); } // a pinch changes the zoom every frame: keep the sizes of before it until it is quiet - let now = ui.input(|i| i.time); let plan = app.size_hold.apply(id, now, navigating, plan); if app.size_hold.holding(now) { ui.ctx().request_repaint_after(std::time::Duration::from_secs_f64(crate::region::HOLD_SECS)); diff --git a/crates/ui-egui/src/region.rs b/crates/ui-egui/src/region.rs index f02f80df5..1c2c78407 100644 --- a/crates/ui-egui/src/region.rs +++ b/crates/ui-egui/src/region.rs @@ -107,22 +107,50 @@ const GIVE_UP_FACTOR: f64 = 2.0; /// The full-size drafts (after the first) a drag times before it may give up on full size. const GIVE_UP_AFTER: usize = 3; -/// The most (photo, control) pairs [`DragScale`] remembers as too slow for full-size drafts; past -/// it, the one remembered longest ago is forgotten. -const SLOW_DRAGS_KEPT: usize = 256; +/// The most (photo, control) pairs [`DragScale`] keeps a verdict on; past it, the one remembered +/// longest ago is forgotten. +const VERDICTS_KEPT: usize = 256; /// A host frame time (ms) longer than this is not a texture upload but a window that was hidden, /// occluded or stalled: it counts as unknown, like a frame time from no host at all. const FRAME_MS_KNOWN: f64 = 1000.0; +/// A drag counts against its (photo, control) pair only when its full-size drafts took longer than +/// this (ms): a quarter over the budget, so a drag that only just misses it keeps full size. +const VERDICT_MS: f64 = DRAFT_BUDGET_MS * 1.25; + +/// How long (s) a pair found slow drafts small before a drag of it tries full size again: the +/// machine may only have been busy. Each retry that is slow too doubles the wait, up to +/// [`RETRY_MAX_S`]. A retry costs at most a few slow drafts before the drag gives up on full size. +const RETRY_AFTER_S: f64 = 30.0; + +/// The longest wait (s) before a slow pair tries full size again. +const RETRY_MAX_S: f64 = 240.0; + +/// What [`DragScale`] remembers about the drags of one control on one photo. +#[derive(Clone, Copy, Debug, PartialEq)] +enum Verdict { + /// One drag was over budget at full size: maybe a busy moment, so the next stays full size. + Suspect, + /// Two drags in a row were: drafts are small until `until` (the host's clock, s), then a drag + /// tries full size again; `wait` is how long this verdict holds (s). + Slow { until: f64, wait: f64 }, +} + /// The size of a slider drag's whole-frame draft, chosen when the drag starts and kept until it /// ends (a change in the middle would make the resolution jump), unless its full-size drafts are -/// far over budget. A drag renders at full size when the GPU draws the view and no earlier drag of -/// the same control on this photo went over budget at that size; otherwise at [`CPU_DRAFT_SCALE`] -/// (issue #330). A slow drag is remembered for its photo only: the same control on another photo -/// (another size, other edits) starts at full size again and is timed afresh. Whether the GPU drew a render is not enough on its own: some views (a LUT -/// profile, refined masks, a photo past a GPU limit) fall back to the CPU per render, and a large -/// draft costs a texture upload too, which only the time a draft took to show tells. +/// far over budget. A drag renders at full size when the GPU draws the view and the same control +/// on this photo isn't known to be too slow at that size; otherwise at [`CPU_DRAFT_SCALE`] (issue +/// #330). Whether the GPU drew a render is not enough on its own: some views (a LUT profile, +/// refined masks, a photo past a GPU limit) fall back to the CPU per render, and a large draft +/// costs a texture upload too, which only the time a draft took to show tells. +/// +/// A drag's time is wall-clock time, what the user waits for. So that a moment the machine was busy +/// (another app, a backup) can't make a slider draft small for the rest of the session, a pair is +/// slow only after two drags in a row over [`VERDICT_MS`], and only for a while: after +/// [`RETRY_AFTER_S`] a drag tries full size again, a fast one forgets the verdict and a slow one +/// doubles the wait. A verdict is for its photo only: the same control on another photo (another +/// size, other edits) is timed afresh. #[derive(Clone, Debug, Default)] pub struct DragScale { /// The drag running now: (photo, control) and its scale. @@ -134,26 +162,28 @@ pub struct DragScale { /// The render time (ms) of a draft that reached the screen this frame: the time of the frame /// that uploads it is only known on the next one. pending: Option, - /// Drags whose full-size drafts went over budget, the one remembered longest ago first. - slow: std::collections::VecDeque<(lightcraft_catalog::PhotoId, String)>, + /// Pairs whose full-size drafts went over budget, the one remembered longest ago first. + verdicts: std::collections::VecDeque<((lightcraft_catalog::PhotoId, String), Verdict)>, } impl DragScale { /// The draft scale for this frame, or `None` without a drag. `drag` is the photo and control /// being dragged; `gpu` whether the GPU draws this view; `shown` the key of the whole-frame - /// render on screen (a render from before the drag is not one of its drafts). - pub fn scale(&mut self, drag: Option<(lightcraft_catalog::PhotoId, &str)>, gpu: bool, shown: Option) -> Option { + /// render on screen (a render from before the drag is not one of its drafts); `now` the host's + /// clock (s; egui's `input.time`). + pub fn scale(&mut self, drag: Option<(lightcraft_catalog::PhotoId, &str)>, gpu: bool, shown: Option, now: f64) -> Option { let Some((photo, control)) = drag else { - self.finish(); + self.finish(now); return None; }; match &self.current { Some(((p, c), s)) if *p == photo && c == control => return Some(*s), - Some(_) => self.finish(), + Some(_) => self.finish(now), None => {} } let key = (photo, control.to_string()); - let s = if gpu && !self.slow.contains(&key) { 1.0 } else { CPU_DRAFT_SCALE }; + let small = matches!(self.verdict(&key), Some(Verdict::Slow { until, .. }) if now < until); + let s = if gpu && !small { 1.0 } else { CPU_DRAFT_SCALE }; self.current = Some((key, s)); self.seen = shown; Some(s) @@ -192,30 +222,58 @@ impl DragScale { } } - /// Remember drags of `control` on `photo` as too slow for full-size drafts (tests). + /// Remember drags of `control` on `photo` as too slow for full-size drafts, for good (tests). #[cfg(test)] pub fn remember_slow(&mut self, photo: lightcraft_catalog::PhotoId, control: &str) { - self.mark_slow((photo, control.to_string())); + self.set_verdict((photo, control.to_string()), Some(Verdict::Slow { until: f64::INFINITY, wait: RETRY_MAX_S })); } - /// Remember a slow drag, forgetting the one remembered longest ago when the set is full (not - /// all of them: each forgotten pair costs one more slow full-size drag). - fn mark_slow(&mut self, key: (lightcraft_catalog::PhotoId, String)) { - if self.slow.contains(&key) { - return; - } - while self.slow.len() >= SLOW_DRAGS_KEPT { - self.slow.pop_front(); - } - self.slow.push_back(key); + fn verdict(&self, key: &(lightcraft_catalog::PhotoId, String)) -> Option { + self.verdicts.iter().find(|(k, _)| k == key).map(|(_, v)| *v) } - /// The drag ended: remember it if its full-size drafts were over budget. - fn finish(&mut self) { + /// Set or (`None`) forget a pair's verdict. A new pair forgets the one remembered longest ago + /// when the list is full (not all of them: each forgotten pair costs one more slow drag). + fn set_verdict(&mut self, key: (lightcraft_catalog::PhotoId, String), verdict: Option) { + let at = self.verdicts.iter().position(|(k, _)| *k == key); + match (at, verdict) { + (Some(i), Some(v)) => { + if let Some(entry) = self.verdicts.get_mut(i) { + entry.1 = v; + } + } + (Some(i), None) => { + self.verdicts.remove(i); + } + (None, Some(v)) => { + while self.verdicts.len() >= VERDICTS_KEPT { + self.verdicts.pop_front(); + } + self.verdicts.push_back((key, v)); + } + (None, None) => {} + } + } + + /// The drag ended. A drag timed at full size updates its pair's verdict: over [`VERDICT_MS`] + /// a first time makes it suspect, a second slow for [`RETRY_AFTER_S`], and a slow retry + /// doubles the wait; within it, the verdict is forgotten. A drag never timed at full size (it + /// drafted small, or was too short) changes nothing. + fn finish(&mut self, now: f64) { if let Some((key, _)) = self.current.take() - && median_after_first(&self.samples).is_some_and(|m| m > DRAFT_BUDGET_MS) + && let Some(median) = median_after_first(&self.samples) { - self.mark_slow(key); + let verdict = if median > VERDICT_MS { + let wait = match self.verdict(&key) { + None => None, + Some(Verdict::Suspect) => Some(RETRY_AFTER_S), + Some(Verdict::Slow { wait, .. }) => Some((wait * 2.0).min(RETRY_MAX_S)), + }; + Some(wait.map_or(Verdict::Suspect, |wait| Verdict::Slow { until: now + wait, wait })) + } else { + None + }; + self.set_verdict(key, verdict); } self.samples.clear(); self.seen = None; @@ -442,10 +500,10 @@ mod tests { fn a_drag_shrinks_the_draft_only_on_the_cpu() { use lightcraft_catalog::PhotoId; let mut gpu = DragScale::default(); - assert_eq!(gpu.scale(None, true, None), None, "no drag, no draft"); - assert_eq!(gpu.scale(Some((PhotoId(1), "Exposure")), true, None), Some(1.0)); + assert_eq!(gpu.scale(None, true, None, 0.0), None, "no drag, no draft"); + assert_eq!(gpu.scale(Some((PhotoId(1), "Exposure")), true, None, 0.0), Some(1.0)); let mut cpu = DragScale::default(); - assert_eq!(cpu.scale(Some((PhotoId(1), "Exposure")), false, None), Some(CPU_DRAFT_SCALE)); + assert_eq!(cpu.scale(Some((PhotoId(1), "Exposure")), false, None, 0.0), Some(CPU_DRAFT_SCALE)); } // Given a drag that started before the GPU was ready, it keeps its size until it ends: the @@ -455,33 +513,120 @@ mod tests { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let drag = Some((PhotoId(1), "Exposure")); - assert_eq!(d.scale(drag, false, None), Some(CPU_DRAFT_SCALE)); - assert_eq!(d.scale(drag, true, None), Some(CPU_DRAFT_SCALE), "the GPU came up mid-drag"); - assert_eq!(d.scale(None, true, None), None); - assert_eq!(d.scale(drag, true, None), Some(1.0), "the next drag gets full size"); + assert_eq!(d.scale(drag, false, None, 0.0), Some(CPU_DRAFT_SCALE)); + assert_eq!(d.scale(drag, true, None, 0.0), Some(CPU_DRAFT_SCALE), "the GPU came up mid-drag"); + assert_eq!(d.scale(None, true, None, 0.0), None); + assert_eq!(d.scale(drag, true, None, 0.0), Some(1.0), "the next drag gets full size"); // and the other way: the GPU lost mid-drag keeps the drag at full size until it ends - assert_eq!(d.scale(drag, false, None), Some(1.0)); + assert_eq!(d.scale(drag, false, None, 0.0), Some(1.0)); } - // Given full-size drafts over the slider budget (a LUT profile or refined mask the CPU renders, - // a photo past a GPU limit, a slow GPU), the next drag of that control on that photo drafts - // small; other controls and photos keep full size + /// One whole drag of `drag` at full size whose drafts all take `ms` to show, ending at `now`. + fn timed_drag(d: &mut DragScale, drag: Option<(lightcraft_catalog::PhotoId, &str)>, ms: f64, now: f64) -> Option { + let scale = d.scale(drag, true, None, now); + for key in 1..=6 { + d.observe(key, ms, None); + } + d.scale(None, true, None, now); + scale + } + + // Given full-size drafts over the slider budget twice in a row (a LUT profile or refined mask + // the CPU renders, a photo past a GPU limit, a slow GPU), the next drag of that control on + // that photo drafts small; once could be a busy moment. Other controls and photos keep full size #[test] - fn a_drag_over_budget_drafts_small_the_next_time() { + fn a_drag_over_budget_twice_drafts_small_the_next_time() { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let exposure = Some((PhotoId(1), "Exposure")); - assert_eq!(d.scale(exposure, true, Some(7)), Some(1.0)); + assert_eq!(d.scale(exposure, true, Some(7), 0.0), Some(1.0)); d.observe(7, 400.0, None); // the render from before the drag: not a draft of it - for (key, ms) in [(8, 300.0), (9, 20.0), (10, 25.0), (11, 9.0)] { + for (key, ms) in [(8, 300.0), (9, 30.0), (10, 25.0), (11, 9.0)] { d.observe(key, ms, None); d.observe(key, ms, None); // a render is timed once, however many frames show it } - assert_eq!(d.scale(exposure, true, None), Some(1.0), "over budget, but not far enough to give up mid-drag"); - assert_eq!(d.scale(None, true, None), None); - assert_eq!(d.scale(exposure, true, None), Some(CPU_DRAFT_SCALE)); - assert_eq!(d.scale(Some((PhotoId(1), "Contrast")), true, None), Some(1.0)); - assert_eq!(d.scale(Some((PhotoId(2), "Exposure")), true, None), Some(1.0)); + assert_eq!(d.scale(exposure, true, None, 0.0), Some(1.0), "over budget, but not far enough to give up mid-drag"); + assert_eq!(d.scale(None, true, None, 0.0), None); + assert_eq!(timed_drag(&mut d, exposure, 25.0, 1.0), Some(1.0), "one slow drag: maybe the machine was busy"); + assert_eq!(d.scale(exposure, true, None, 2.0), Some(CPU_DRAFT_SCALE)); + d.scale(None, true, None, 2.0); + assert_eq!(d.scale(Some((PhotoId(1), "Contrast")), true, None, 2.0), Some(1.0)); + assert_eq!(d.scale(Some((PhotoId(2), "Exposure")), true, None, 2.0), Some(1.0)); + } + + // Given a pair found slow, it drafts small only for a while: then a drag tries full size + // again; a slow retry doubles the wait (up to its cap), a fast one forgets the verdict + #[test] + fn a_slow_verdict_expires_and_is_retried() { + use lightcraft_catalog::PhotoId; + let mut d = DragScale::default(); + let drag = Some((PhotoId(1), "Exposure")); + timed_drag(&mut d, drag, 25.0, 0.0); + timed_drag(&mut d, drag, 25.0, 0.0); + assert_eq!(timed_drag(&mut d, drag, 25.0, RETRY_AFTER_S - 1.0), Some(CPU_DRAFT_SCALE), "slow until the wait is over"); + assert_eq!(timed_drag(&mut d, drag, 25.0, RETRY_AFTER_S), Some(1.0), "then a drag tries full size: still slow"); + let mut at = RETRY_AFTER_S; + let mut wait = RETRY_AFTER_S * 2.0; + for _ in 0..6 { + assert_eq!(timed_drag(&mut d, drag, 25.0, at + wait - 1.0), Some(CPU_DRAFT_SCALE), "the wait doubled to {wait} s"); + assert_eq!(timed_drag(&mut d, drag, 25.0, at + wait), Some(1.0), "a retry after {wait} s, slow again"); + at += wait; + wait = (wait * 2.0).min(RETRY_MAX_S); + } + assert_eq!(wait, RETRY_MAX_S, "the wait stops growing"); + assert_eq!(timed_drag(&mut d, drag, 8.0, at + wait), Some(1.0), "a retry, fast this time"); + assert_eq!(timed_drag(&mut d, drag, 25.0, at + wait + 1.0), Some(1.0), "forgotten: one slow drag is a suspicion again"); + assert_eq!(timed_drag(&mut d, drag, 25.0, at + wait + 2.0), Some(1.0), "suspect: still full size"); + assert_eq!(d.scale(drag, true, None, at + wait + 3.0), Some(CPU_DRAFT_SCALE), "slow from the start again: wait 30 s"); + } + + // Given a drag only just over budget, it doesn't count against the pair (it would flip between + // the two sizes); one clearly over does + #[test] + fn only_a_drag_clearly_over_budget_counts() { + use lightcraft_catalog::PhotoId; + let mut d = DragScale::default(); + let drag = Some((PhotoId(1), "Exposure")); + for t in 0..4 { + timed_drag(&mut d, drag, 18.0, f64::from(t)); + } + assert_eq!(d.scale(drag, true, None, 4.0), Some(1.0), "18 ms drafts: just over the 16 ms budget"); + d.scale(None, true, None, 4.0); + timed_drag(&mut d, drag, VERDICT_MS + 5.0, 5.0); + timed_drag(&mut d, drag, VERDICT_MS + 5.0, 6.0); + assert_eq!(d.scale(drag, true, None, 7.0), Some(CPU_DRAFT_SCALE)); + } + + // Given one slow drag while the machine was busy, then fast ones, the slider keeps full size + #[test] + fn a_busy_moment_doesnt_make_a_slider_draft_small() { + use lightcraft_catalog::PhotoId; + let mut d = DragScale::default(); + let drag = Some((PhotoId(1), "Exposure")); + timed_drag(&mut d, drag, 40.0, 0.0); + for t in 1..=3 { + assert_eq!(timed_drag(&mut d, drag, 8.0, f64::from(t)), Some(1.0)); + } + timed_drag(&mut d, drag, 40.0, 4.0); + assert_eq!(d.scale(drag, true, None, 5.0), Some(1.0), "the fast drags forgot the first slow one"); + } + + // Given a hostile host clock (NaN, infinite, going backwards), a drag never panics; a clock it + // can't read tries full size + #[test] + fn a_hostile_clock_never_panics() { + use lightcraft_catalog::PhotoId; + let mut d = DragScale::default(); + let drag = Some((PhotoId(1), "Exposure")); + timed_drag(&mut d, drag, 25.0, 100.0); + timed_drag(&mut d, drag, 25.0, 100.0); + assert_eq!(d.scale(drag, true, None, 50.0), Some(CPU_DRAFT_SCALE), "the clock went backwards: still slow"); + d.scale(None, true, None, 50.0); + assert_eq!(d.scale(drag, true, None, f64::NAN), Some(1.0)); + d.scale(None, true, None, f64::NAN); + for now in [f64::INFINITY, f64::NEG_INFINITY, f64::NAN, -1.0] { + timed_drag(&mut d, drag, 25.0, now); + } } // Given full-size drafts within budget (the first one, rebuilding the stage caches, may be @@ -491,18 +636,18 @@ mod tests { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let drag = Some((PhotoId(1), "Exposure")); - d.scale(drag, true, None); + d.scale(drag, true, None, 0.0); for (key, ms) in [(1, 120.0), (2, 4.0), (3, 6.0), (4, 30.0), (5, 5.0)] { d.observe(key, ms, None); } - d.scale(None, true, None); - assert_eq!(d.scale(drag, true, None), Some(1.0)); + d.scale(None, true, None, 0.0); + assert_eq!(d.scale(drag, true, None, 0.0), Some(1.0)); // hostile times are ignored, never a panic for (key, ms) in [(6, f64::NAN), (7, f64::INFINITY), (8, -1.0)] { d.observe(key, ms, None); } - d.scale(None, true, None); - assert_eq!(d.scale(drag, true, None), Some(1.0)); + d.scale(None, true, None, 0.0); + assert_eq!(d.scale(drag, true, None, 0.0), Some(1.0)); } // Given renders within budget whose frames (the texture upload) take it over, the drag counts @@ -512,29 +657,30 @@ mod tests { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let drag = Some((PhotoId(1), "Exposure")); - d.scale(drag, true, None); - for key in 1..=5 { - d.observe(key, 8.0, Some(12.0)); // the previous frame, which uploaded the last draft + for _ in 0..2 { + d.scale(drag, true, None, 0.0); + for key in 1..=5 { + d.observe(key, 8.0, Some(14.0)); // the previous frame, which uploaded the last draft + } + d.observe(5, 8.0, Some(14.0)); + d.scale(None, true, None, 0.0); } - d.observe(5, 8.0, Some(12.0)); - d.scale(None, true, None); - assert_eq!(d.scale(drag, true, None), Some(CPU_DRAFT_SCALE), "8 ms renders + 12 ms frames: over 16 ms"); + assert_eq!(d.scale(drag, true, None, 0.0), Some(CPU_DRAFT_SCALE), "8 ms renders + 14 ms frames: over budget"); // the same renders without the host's frame times (headless) stay within budget let mut headless = DragScale::default(); - headless.scale(drag, true, None); - for key in 1..=6 { - headless.observe(key, 8.0, None); + headless.scale(drag, true, None, 0.0); + for _ in 0..2 { + timed_drag(&mut headless, drag, 8.0, 0.0); } - headless.scale(None, true, None); - assert_eq!(headless.scale(drag, true, None), Some(1.0)); + assert_eq!(headless.scale(drag, true, None, 0.0), Some(1.0)); // a hostile frame time counts as nothing, never a panic let mut hostile = DragScale::default(); - hostile.scale(drag, true, None); + hostile.scale(drag, true, None, 0.0); for (key, frame) in [(1, f64::NAN), (2, f64::INFINITY), (3, -1.0), (4, f64::NAN)] { hostile.observe(key, 8.0, Some(frame)); } - hostile.scale(None, true, None); - assert_eq!(hostile.scale(drag, true, None), Some(1.0)); + hostile.scale(None, true, None, 0.0); + assert_eq!(hostile.scale(drag, true, None, 0.0), Some(1.0)); } // Given more slow drags than are remembered, only the one remembered longest ago is forgotten: @@ -543,17 +689,17 @@ mod tests { fn a_full_slow_set_forgets_only_the_oldest_drag() { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); - for photo in 0..=SLOW_DRAGS_KEPT as u64 { + for photo in 0..=VERDICTS_KEPT as u64 { d.remember_slow(PhotoId(photo), "Exposure"); } - assert_eq!(d.scale(Some((PhotoId(0), "Exposure")), true, None), Some(1.0), "the oldest is forgotten"); - d.scale(None, true, None); - for photo in [1, SLOW_DRAGS_KEPT as u64] { - assert_eq!(d.scale(Some((PhotoId(photo), "Exposure")), true, None), Some(CPU_DRAFT_SCALE), "photo {photo}"); - d.scale(None, true, None); + assert_eq!(d.scale(Some((PhotoId(0), "Exposure")), true, None, 0.0), Some(1.0), "the oldest is forgotten"); + d.scale(None, true, None, 0.0); + for photo in [1, VERDICTS_KEPT as u64] { + assert_eq!(d.scale(Some((PhotoId(photo), "Exposure")), true, None, 0.0), Some(CPU_DRAFT_SCALE), "photo {photo}"); + d.scale(None, true, None, 0.0); } d.remember_slow(PhotoId(1), "Exposure"); // already remembered: nothing is forgotten - assert_eq!(d.scale(Some((PhotoId(2), "Exposure")), true, None), Some(CPU_DRAFT_SCALE)); + assert_eq!(d.scale(Some((PhotoId(2), "Exposure")), true, None, 0.0), Some(CPU_DRAFT_SCALE)); } // Given draft frames that took seconds (the window hidden or occluded mid-drag), they count as @@ -563,33 +709,34 @@ mod tests { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let drag = Some((PhotoId(1), "Exposure")); - d.scale(drag, true, None); + d.scale(drag, true, None, 0.0); for key in 1..=6 { d.observe(key, 6.0, Some(4000.0)); } - d.scale(None, true, None); - assert_eq!(d.scale(drag, true, None), Some(1.0)); + d.scale(None, true, None, 0.0); + assert_eq!(d.scale(drag, true, None, 0.0), Some(1.0)); } // Given full-size drafts far over budget (a refined mask the CPU renders at 190 ms a step), - // the drag drops to the small draft after a few of them instead of at its end, and the next - // drag starts small + // the drag drops to the small draft after a few of them instead of at its end; after a second + // such drag, the next starts small #[test] fn a_drag_far_over_budget_drafts_small_before_it_ends() { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let drag = Some((PhotoId(1), "Exposure")); - assert_eq!(d.scale(drag, true, None), Some(1.0)); + assert_eq!(d.scale(drag, true, None, 0.0), Some(1.0)); let mut scales = Vec::new(); for key in 1..=8 { d.observe(key, 190.0, None); - scales.push(d.scale(drag, true, None)); + scales.push(d.scale(drag, true, None, 0.0)); } // drafts 1–4 are timed one frame late; the median of 2–4 decides on the fifth frame assert_eq!(scales[..4], [Some(1.0); 4], "{scales:?}"); assert_eq!(scales[4..], [Some(CPU_DRAFT_SCALE); 4], "{scales:?}"); - d.scale(None, true, None); - assert_eq!(d.scale(drag, true, None), Some(CPU_DRAFT_SCALE)); + d.scale(None, true, None, 0.0); + assert_eq!(timed_drag(&mut d, drag, 190.0, 1.0), Some(1.0), "one such drag: maybe a busy moment"); + assert_eq!(d.scale(drag, true, None, 2.0), Some(CPU_DRAFT_SCALE)); } // Given a full-size drag zoomed in (a window covers the view), the hidden whole-frame draft is From 6a66860ea9db6fa85206f59453a7e84dd1793a32 Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Tue, 13 Oct 2026 14:11:27 +0200 Subject: [PATCH 04/99] LR-VIEW-ZOOM: hidden-window test drags twice so it catches a missing frame-time bound (#330) One slow drag only makes the pair suspect, so a single hidden-window drag passed even without the FRAME_MS_KNOWN upper bound. Two such drags followed by a full-size third fail (0.6) when the bound is removed. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/region.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/region.rs b/crates/ui-egui/src/region.rs index 1c2c78407..23b21bc19 100644 --- a/crates/ui-egui/src/region.rs +++ b/crates/ui-egui/src/region.rs @@ -703,18 +703,21 @@ mod tests { } // Given draft frames that took seconds (the window hidden or occluded mid-drag), they count as - // unknown: fast renders don't mark the drag slow because the window wasn't drawing + // unknown: fast renders don't mark the drag slow because the window wasn't drawing. Two such + // drags, since one slow drag only makes the pair suspect #[test] fn a_hidden_windows_frame_times_dont_mark_a_drag_slow() { use lightcraft_catalog::PhotoId; let mut d = DragScale::default(); let drag = Some((PhotoId(1), "Exposure")); - d.scale(drag, true, None, 0.0); - for key in 1..=6 { - d.observe(key, 6.0, Some(4000.0)); + for t in 0..2u32 { + assert_eq!(d.scale(drag, true, None, f64::from(t)), Some(1.0), "drag {}", t + 1); + for key in 1..=6 { + d.observe(u64::from(t) * 10 + key, 6.0, Some(4000.0)); + } + d.scale(None, true, None, f64::from(t)); } - d.scale(None, true, None, 0.0); - assert_eq!(d.scale(drag, true, None, 0.0), Some(1.0)); + assert_eq!(d.scale(drag, true, None, 2.0), Some(1.0), "after two hidden-window drags: still full size"); } // Given full-size drafts far over budget (a refined mask the CPU renders at 190 ms a step), From 9a0f0761057aabd468336289a351697f51ea1e1d Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Sun, 11 Oct 2026 16:53:11 +0200 Subject: [PATCH 05/99] =?UTF-8?q?LR-VIEW-CLIPPING:=20=E2=8C=A5-drag=20prev?= =?UTF-8?q?iews=20for=20Blacks,=20Whites,=20Exposure,=20Sharpening=20and?= =?UTF-8?q?=20NR=20(#937)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Holding ⌥ (Option) while dragging a slider shows a diagnostic view in the loupe, as in Lightroom; releasing ⌥ or the slider brings the photo back and nothing is saved: - Blacks: shadow clipping (white; black where every channel is 0, the colour of the unclipped channels where only some are). - Whites / Exposure: highlight clipping (black; white where every channel is 255, the clipped channels' colour where only some are). - Sharpening Masking: the weight sharpening gets at each pixel, white on black. - Sharpening Amount / Radius / Detail, luminance NR: the image in grey. New `Overlay` variants (ShadowClip, HighlightClip, SharpenMask, Grey) are drawn by the shared `visualize::apply`, so CPU and GPU agree; the sharpening mask needs the detail planes and renders on the CPU (like HdrRange). The mask uses the same weight function as the render (`finish::sharpen_weight`). With a display profile the clipping views are tested in sRGB, like the histogram, and diagnostic views keep the photo's histogram instead of the view's. Tracker: LR-VIEW-CLIPPING ✅ → 🟡 (J only frames the photo, it doesn't paint clipped pixels) and LR-EDIT-DETAIL-SHARPEN ✅ → 🟡 (Radius and Detail don't reach the render). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/display.rs | 19 +++ crates/engine/src/media.rs | 15 ++- crates/gpu/src/lib.rs | 10 +- crates/pipeline/src/finish.rs | 37 +++++- crates/pipeline/src/lib.rs | 5 +- crates/pipeline/src/local.rs | 7 +- crates/pipeline/src/visualize.rs | 139 ++++++++++++++++++++-- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/detail.rs | 41 ++++++- crates/ui-egui/src/tests_alt_preview.rs | 93 +++++++++++++++ crates/ui-egui/src/tests_masking.rs | 12 +- crates/ui-egui/src/tests_preview_limit.rs | 7 +- docs/parity-checklist.md | 23 ++-- 13 files changed, 368 insertions(+), 42 deletions(-) create mode 100644 crates/ui-egui/src/tests_alt_preview.rs diff --git a/crates/engine/src/display.rs b/crates/engine/src/display.rs index f47e099b6..8a91a6644 100644 --- a/crates/engine/src/display.rs +++ b/crates/engine/src/display.rs @@ -152,6 +152,25 @@ mod tests { } } + /// ⌥-drag views on a profiled display (issue #937): the histogram stays the photo's (not the + /// view's), and the clipping view is tested and drawn in sRGB, so it stays black and white. + #[test] + fn diagnostic_views_keep_the_photos_histogram() { + use lightcraft_pipeline::Overlay; + let mut session = crate::Session::with_demo(); + let id = session.visible_cloned()[0]; + let d = std::sync::Arc::new(p3()); + let mut run = |o| session.loupe_job(id, 320, 240, true).unwrap().with_display(Some(d.clone())).with_overlay(o).run().rendered.unwrap(); + let plain = run(Overlay::None); + let clip = run(Overlay::ShadowClip); + let (a, b) = (plain.histogram.mean_luma(), clip.histogram.mean_luma()); + assert!((a - b).abs() < 0.02, "the photo's histogram: {a} vs {b}"); + let px = &clip.image.data; + // (the profile may round white to 254) + let binary = px.iter().filter(|p| p[..3].iter().all(|v| *v <= 2 || *v >= 253)).count(); + assert!(binary as f64 > 0.99 * px.len() as f64, "{binary} of {}", px.len()); + } + #[test] fn load_reports_and_rejects() { let dir = std::env::temp_dir().join(format!("lightcraft-display-{}", std::process::id())); diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 6d1bfe0de..886c2f21c 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -968,11 +968,24 @@ impl RenderJob { // Thumbnails (many small jobs side by side) stay on the CPU; views and exports use // the GPU when there is one. let gpu = self.cache.is_none(); - let (mut rendered, gpu_fallback) = develop_reporting(src, &info, &self.settings, &self.request, self.stages.as_deref(), gpu); + // a clipping view tests sRGB values, like the histogram: rendered in sRGB, then + // converted for the display below + let clip_view = self.request.overlay.is_clipping(); + let request = if clip_view { RenderRequest { display: None, ..self.request } } else { self.request }; + let (mut rendered, gpu_fallback) = develop_reporting(src, &info, &self.settings, &request, self.stages.as_deref(), gpu); // for a display profile: the image goes to the display's values, the histogram // and the cached view preview stay sRGB let mut srgb = None; let display = match (&self.display, self.request.display) { + // a diagnostic view: the histogram stays the photo's (taken before the overlay + // was drawn), and nothing goes to the view cache + (Some(d), Some(_)) if self.request.overlay != lightcraft_pipeline::Overlay::None => { + let converted = if clip_view { d.profile.from_srgb(&mut rendered.image) } else { d.profile.correct(&mut rendered.image) }; + if let Err(e) = converted { + log::warn!("display profile: {e}"); + } + Some(d.id()) + } (Some(d), Some(_)) => { let keep = self.view_cache.is_some() && self.request.quality == Quality::Full; let (s, h) = crate::display::finish_render(&mut rendered.image, d, keep); diff --git a/crates/gpu/src/lib.rs b/crates/gpu/src/lib.rs index 76fb0f2c1..89315a344 100644 --- a/crates/gpu/src/lib.rs +++ b/crates/gpu/src/lib.rs @@ -118,7 +118,8 @@ pub fn last_fallback() -> Option { #[derive(Clone, Debug, PartialEq, Eq)] pub enum Fallback { /// The GPU was not tried: it is unavailable, disabled or closing ([`unavailable_reason`]), or - /// the request is one only the CPU renders (high bit depth, soft proof, HDR range overlay). + /// the request is one only the CPU renders (high bit depth, soft proof, HDR range or sharpening + /// mask overlay). NotTried, /// The render was tried on the GPU and failed, for this reason (the same text /// [`last_fallback`] reports). The CPU renders it instead. @@ -439,10 +440,13 @@ pub fn render(src: &Arc, info: &SourceInfo, s: &DevelopSettings, req: &R /// Whether the GPU renders requests of this kind at all (a given render can still fall back to /// the CPU): the kernel writes 8-bit output, so high-bit-depth exports and soft proofs render on -/// the CPU, and so does Visualize HDR, which needs the float HDR render. [`try_render`] follows it, +/// the CPU, and so does Visualize HDR, which needs the float HDR render, and the sharpening mask, +/// which needs the detail planes. [`try_render`] follows it, /// and the loupe sizes its slider-drag drafts by it (issue #330), so the two can't drift apart. pub fn handles(depth: lightcraft_pipeline::OutputDepth, proofed: bool, overlay: lightcraft_pipeline::Overlay) -> bool { - depth == lightcraft_pipeline::OutputDepth::U8 && !proofed && overlay != lightcraft_pipeline::Overlay::HdrRange + depth == lightcraft_pipeline::OutputDepth::U8 + && !proofed + && !matches!(overlay, lightcraft_pipeline::Overlay::HdrRange | lightcraft_pipeline::Overlay::SharpenMask) } /// [`render`], reporting why this render made no GPU image: [`Fallback::NotTried`] when the GPU diff --git a/crates/pipeline/src/finish.rs b/crates/pipeline/src/finish.rs index eb65d51a2..1fc68be00 100644 --- a/crates/pipeline/src/finish.rs +++ b/crates/pipeline/src/finish.rs @@ -5,7 +5,7 @@ use lightcraft_color::spline::{Lut1, MonotoneCurve}; use lightcraft_color::transfer::linear_to_srgb; use lightcraft_develop::{DevelopSettings, LocalAdjustments, Process, ToneCurve, VignetteStyle}; use lightcraft_geom::Point; -use lightcraft_raster::Rgba8; +use lightcraft_raster::{Plane, Rgba8}; use crate::colorops::ColorOps; use crate::geometry::Frame; @@ -14,6 +14,36 @@ use crate::output::{DeepImage, DeepSamples, OutputDepth, OutputSpace, OutputTrc} use crate::tone::ToneMap; use crate::{Prepared, SourceInfo, for_rows}; +/// Sharpening's Masking as 0..1. +fn sharpen_masking(s: &DevelopSettings) -> f32 { + (s.detail.sharpen_masking / 100.0) as f32 +} + +/// How much sharpening applies at a pixel whose fine detail (log luminance minus its texture +/// blur) is `det`, with Masking `masking` (0..1): everywhere at 0, only on stronger edges above. +#[inline] +fn sharpen_weight(masking: f32, det: f32) -> f32 { + if masking > 0.0 { smooth(masking * 0.25, masking * 0.25 + 0.15, det.abs()) } else { 1.0 } +} + +/// [`sharpen_weight`] at every pixel of `p` (the view [`crate::Overlay::SharpenMask`] shows). +pub(crate) fn sharpen_weight_plane(p: &Prepared, s: &DevelopSettings) -> Plane { + let masking = sharpen_masking(s); + if masking <= 0.0 { + return Plane::filled(p.log_l.width, p.log_l.height, 1.0); + } + // the detail band the render sharpens (made here when sharpening is off) + let fresh; + let blur = match &p.texture_blur { + Some(b) if (b.width, b.height) == (p.log_l.width, p.log_l.height) => &**b, + _ => { + fresh = lightcraft_raster::blur::gaussian(&p.log_l, crate::local::texture_sigma(p.px_per_long)); + &fresh + } + }; + p.log_l.zip_map(blur, |l, b| sharpen_weight(masking, l - b)) +} + #[inline] fn smooth(e0: f32, e1: f32, x: f32) -> f32 { let t = ((x - e0) / (e1 - e0)).clamp(0.0, 1.0); @@ -244,7 +274,7 @@ impl FinishParams { tex, dehaze, sharpen: (s.detail.sharpen_amount / 150.0) as f32, - sharpen_mask: (s.detail.sharpen_masking / 100.0) as f32, + sharpen_mask: sharpen_masking(s), air: air_pre * gain, air_pre, gain, @@ -509,8 +539,7 @@ pub(crate) fn finish_with( let tame = 1.0 - 0.6 * smooth(0.4, 1.6, det.abs()); delta += tx * 1.1 * det.clamp(-1.0, 1.0) * tame; if sp != 0.0 { - let m = if sharpen_mask > 0.0 { smooth(sharpen_mask * 0.25, sharpen_mask * 0.25 + 0.15, det.abs()) } else { 1.0 }; - delta += sp * 1.3 * det.clamp(-0.8, 0.8) * m; + delta += sp * 1.3 * det.clamp(-0.8, 0.8) * sharpen_weight(sharpen_mask, det); } } // local Noise: smooth (or, negative, boost) small-amplitude detail, keep edges diff --git a/crates/pipeline/src/lib.rs b/crates/pipeline/src/lib.rs index 455cec022..c87efaf0f 100644 --- a/crates/pipeline/src/lib.rs +++ b/crates/pipeline/src/lib.rs @@ -663,8 +663,11 @@ pub fn color_range_sample(src: &Rgb32f, info: &SourceInfo, s: &DevelopSettings, } /// The alpha plane a mask overlay shows: the one the render evaluated, or (for a hidden mask) a -/// fresh evaluation. +/// fresh evaluation. For [`Overlay::SharpenMask`], the weight sharpening gets at each pixel. fn overlay_alpha(o: Overlay, plan: &Plan<'_>, prep: &Prepared) -> Option { + if o == Overlay::SharpenMask { + return Some(finish::sharpen_weight_plane(prep, &plan.settings)); + } let m = o.mask(&plan.settings)?; if let Some(e) = prep.masks.iter().find(|e| e.id == m.id) { return Some(e.alpha.clone()); diff --git a/crates/pipeline/src/local.rs b/crates/pipeline/src/local.rs index 679acabfe..b6180086c 100644 --- a/crates/pipeline/src/local.rs +++ b/crates/pipeline/src/local.rs @@ -488,6 +488,11 @@ pub struct PlaneSigmas { pub chroma: Option, } +/// Gaussian sigma of the texture / sharpening detail band at `px_per_long` pixels per long edge. +pub(crate) fn texture_sigma(px_per_long: f64) -> f32 { + (0.0018 * px_per_long as f32).max(0.6) +} + pub fn plane_sigmas(s: &DevelopSettings, px_per_long: f64, q: Quality) -> PlaneSigmas { let ppl = px_per_long as f32; let local_any = |f: fn(&lightcraft_develop::LocalAdjustments) -> f64| s.masks.iter().any(|m| f(&m.adjust) != 0.0); @@ -508,7 +513,7 @@ pub fn plane_sigmas(s: &DevelopSettings, px_per_long: f64, q: Quality) -> PlaneS || local_any(|a| a.sharpness) || local_any(|a| a.noise) || s.masks.iter().any(|m| m.adjust.defringe > 0.0)) - .then(|| (0.0018 * ppl).max(0.6)); + .then(|| texture_sigma(px_per_long)); let dark = (s.effects.dehaze != 0.0 || local_any(|a| a.dehaze)).then(|| (0.02 * ppl).max(1.0)); let chroma = (local_any(|a| a.moire) || s.masks.iter().any(|m| m.adjust.noise > 0.0)).then(|| (CHROMA_SIGMA * ppl).max(1.0)); let base_detail = if base.is_some() { tone_base_detail(s, px_per_long) } else { 0 }; diff --git a/crates/pipeline/src/visualize.rs b/crates/pipeline/src/visualize.rs index 8f3cf1884..3f1ab19fc 100644 --- a/crates/pipeline/src/visualize.rs +++ b/crates/pipeline/src/visualize.rs @@ -9,12 +9,20 @@ //! - **Mask overlay** (Masking): the evaluated alpha of one mask, drawn as a colour tint, a colour //! tint on a black-and-white image, the image on black / white, or the alpha as white on black //! ([`MaskView`]). Both renderers hand the same alpha plane (the one the render used) to [`apply`]. +//! - **Clipping while ⌥-dragging** Blacks / Whites / Exposure ([`Overlay::ShadowClip`], +//! [`Overlay::HighlightClip`]): every output channel at 0 (or 255) shown, everything else blank, +//! so the clipped areas, and the channels that clip, are all that's left to see. +//! - **Sharpening mask while ⌥-dragging Masking** ([`Overlay::SharpenMask`]): the weight the +//! render gives sharpening at each pixel, white on black (the CPU renderer draws it: it needs the +//! detail planes). +//! - **Grey while ⌥-dragging** Sharpening or luminance Noise Reduction ([`Overlay::Grey`]): the +//! render in grey, so detail and noise are judged without colour. use std::borrow::Cow; use lightcraft_color::perceptual::{lab_to_lch, oklab_from_2020}; use lightcraft_color::transfer::srgb_to_linear; -use lightcraft_color::{REC2020, SRGB}; +use lightcraft_color::{LUMA_709, REC2020, SRGB}; use lightcraft_develop::DevelopSettings; use lightcraft_raster::{Plane, Rgba8}; @@ -39,6 +47,17 @@ pub enum Overlay { /// Visualize HDR (photos edited in HDR): the SDR image dimmed to grey, with every pixel the HDR /// rendition puts above SDR white painted by how far above it is ([`HDR_BANDS`]). HdrRange, + /// Shadow clipping (⌥ on Blacks): white, black where every channel is 0, and where only some + /// are, the colour of the channels that aren't. + ShadowClip, + /// Highlight clipping (⌥ on Whites / Exposure): black, white where every channel is 255, and + /// where only some are, the colour of those channels. + HighlightClip, + /// Sharpening's weight after Masking (⌥ on Masking): white where it sharpens, black where + /// Masking holds it back. + SharpenMask, + /// The render in grey (⌥ on the Sharpening and luminance Noise Reduction sliders). + Grey, } /// [`Overlay::HdrRange`] colours (sRGB) for 0–1, 1–2, 2–3 and ≥ 3 stops above SDR white. @@ -125,6 +144,10 @@ impl Overlay { Overlay::Mask { id, view, color, opacity } => (3, pack_mask(id, view, color, opacity) as f64), Overlay::AllMasks { view, color, opacity } => (5, pack_mask(0, view, color, opacity) as f64), Overlay::HdrRange => (4, 0.0), + Overlay::ShadowClip => (6, 0.0), + Overlay::HighlightClip => (7, 0.0), + Overlay::SharpenMask => (8, 0.0), + Overlay::Grey => (9, 0.0), } } @@ -143,6 +166,10 @@ impl Overlay { opacity: ((v as u64 >> 44) & 0x7f).min(100) as u8, } } + 6 => Overlay::ShadowClip, + 7 => Overlay::HighlightClip, + 8 => Overlay::SharpenMask, + 9 => Overlay::Grey, _ => Overlay::None, } } @@ -156,9 +183,19 @@ impl Overlay { Overlay::Mask { id, view, color, opacity } => 3 << 60 | pack_mask(id, view, color, opacity), Overlay::AllMasks { view, color, opacity } => 5 << 60 | pack_mask(0, view, color, opacity), Overlay::HdrRange => 0x4000, + Overlay::ShadowClip => 0x5000, + Overlay::HighlightClip => 0x6000, + Overlay::SharpenMask => 0x7000, + Overlay::Grey => 0x8000, } } + /// The ⌥-drag clipping views, which test the sRGB values the histogram counts (a display + /// render draws them in sRGB, then converts). + pub fn is_clipping(self) -> bool { + matches!(self, Overlay::ShadowClip | Overlay::HighlightClip) + } + /// The mask an [`Overlay::Mask`] shows (if it exists in `s` and has components). pub fn mask(self, s: &DevelopSettings) -> Option<&lightcraft_develop::Mask> { match self { @@ -181,12 +218,21 @@ pub fn adjust_settings(o: Overlay, s: &mut Cow<'_, DevelopSettings>) { } /// Draw overlay `o` over `img` (rendered with `plan`). `mask` is the evaluated alpha of the mask -/// an [`Overlay::Mask`] shows, at the image's size (see [`Overlay::mask`]); `masks` is the set of -/// alphas an [`Overlay::AllMasks`] shows, one per visible mask with components. +/// an [`Overlay::Mask`] shows, or the sharpening weight of an [`Overlay::SharpenMask`], at the +/// image's size (see [`Overlay::mask`]); `masks` is the set of alphas an [`Overlay::AllMasks`] +/// shows, one per visible mask with components. pub fn apply(img: &mut Rgba8, o: Overlay, plan: &Plan<'_>, mask: Option<&Plane>, masks: Option<&[Plane]>) { match o { // drawn by the renderer from the HDR rendition ([`hdr_range`]) Overlay::None | Overlay::HdrRange => {} + Overlay::ShadowClip => clipping(img, false), + Overlay::HighlightClip => clipping(img, true), + Overlay::Grey => grey(img), + Overlay::SharpenMask => { + if let Some(a) = mask.filter(|a| (a.width, a.height) == (img.width, img.height)) { + mask_view(img, a, MaskView::WhiteOnBlack, [255; 3], 100); + } + } Overlay::PointColorRange(i) => { if let Some(p) = plan.settings.point_colors.get(i as usize) { point_color_range(img, &PointK::new(p)); @@ -224,7 +270,7 @@ pub fn mask_view(img: &mut Rgba8, a: &Plane, view: MaskView, color: [u8; 3], opa for (x, px) in row.iter_mut().enumerate() { let a = a.data[y * w + x].clamp(0.0, 1.0); let p = [px[0] as f32, px[1] as f32, px[2] as f32]; - let grey = 0.2126 * p[0] + 0.7152 * p[1] + 0.0722 * p[2]; + let grey = luma8(*px); let out: [f32; 3] = match view { MaskView::Color => std::array::from_fn(|c| p[c] + (col[c] - p[c]) * a * op), MaskView::ColorOnBw => std::array::from_fn(|c| grey + (col[c] - grey) * a * op), @@ -238,6 +284,25 @@ pub fn mask_view(img: &mut Rgba8, a: &Plane, view: MaskView, color: [u8; 3], opa }); } +/// Rec. 709 luma of an 8-bit pixel (0..255). +fn luma8(p: [u8; 4]) -> f32 { + LUMA_709[0] * p[0] as f32 + LUMA_709[1] * p[1] as f32 + LUMA_709[2] * p[2] as f32 +} + +/// The clipping view: with `high`, each channel at 255 on, the rest off (black where nothing +/// clips); otherwise each channel at 0 off, the rest on (white where nothing clips). +fn clipping(img: &mut Rgba8, high: bool) { + let on = move |v: u8| if (high && v == 255) || (!high && v != 0) { 255 } else { 0 }; + img.map_in_place(|p| [on(p[0]), on(p[1]), on(p[2]), 255]); +} + +fn grey(img: &mut Rgba8) { + img.map_in_place(|p| { + let g = luma8(p).round().clamp(0.0, 255.0) as u8; + [g, g, g, 255] + }); +} + /// Blur radius (Gaussian sigma) of the spot view, as a fraction of the long edge. const SPOT_SIGMA: f64 = 0.006; @@ -255,7 +320,7 @@ fn spots(img: &mut Rgba8, t: u8, ppl: f64) { } let l = lightcraft_raster::Plane::from_fn(w, h, |x, y| { let p = img.data[y * w + x]; - (0.2126 * p[0] as f32 + 0.7152 * p[1] as f32 + 0.0722 * p[2] as f32) / 255.0 + luma8(p) / 255.0 }); let b = lightcraft_raster::blur::gaussian(&l, (SPOT_SIGMA * ppl).max(0.8) as f32); let thr = spot_threshold(t); @@ -275,7 +340,7 @@ fn point_color_range(img: &mut Rgba8, k: &PointK) { let c = std::array::from_fn(|r| m[r][0] * lin[0] + m[r][1] * lin[1] + m[r][2] * lin[2]); let [l, ch, h] = lab_to_lch(oklab_from_2020(c)); let a = k.weight(l, ch, h); - let grey = 0.2126 * px[0] as f32 + 0.7152 * px[1] as f32 + 0.0722 * px[2] as f32; + let grey = luma8(*px); for c in 0..3 { px[c] = (grey + (px[c] as f32 - grey) * a).round().clamp(0.0, 255.0) as u8; } @@ -296,7 +361,7 @@ pub fn hdr_range(img: &mut Rgba8, hdr: &crate::DeepImage, luma: [f32; 3]) { let i = (y * w + x) * 3; let Some(c) = v.get(i..i + 3) else { continue }; let lum = luma[0] * c[0] + luma[1] * c[1] + luma[2] * c[2]; - let grey = (0.2126 * p[0] as f32 + 0.7152 * p[1] as f32 + 0.0722 * p[2] as f32) * 0.55; + let grey = luma8(*p) * 0.55; if lum > 1.0 { let stops = lum.log2(); let band = HDR_BANDS[(stops as usize).min(HDR_BANDS.len() - 1)]; @@ -343,6 +408,66 @@ mod tests { assert_eq!(Overlay::from_parts(k, v), Overlay::Spots(37)); } + #[test] + fn clipping_views_show_only_the_clipped_channels() { + // left to right: black, a red that clips only in red, mid grey, white + let src = Rgb32f::from_fn(40, 10, |x, _| match x / 10 { + 0 => [0.0; 3], + 1 => [0.9, 0.02, 0.02], + 2 => [0.18; 3], + _ => [8.0; 3], + }); + let (info, s) = (SourceInfo::default(), DevelopSettings::default()); + let shot = |o: Overlay| render(&src, &info, &s, &RenderRequest { overlay: o, ..RenderRequest::fit(40, 10) }); + let plain = shot(Overlay::None).image; + let at = |i: &Rgba8, x: usize| i.data[5 * 40 + x]; + assert_eq!((at(&plain, 5), at(&plain, 35)), ([0, 0, 0, 255], [255; 4]), "the scene clips at both ends"); + assert_eq!(at(&plain, 15)[0], 255, "the red clips in red"); + let hi = shot(Overlay::HighlightClip); + assert_eq!([5, 15, 25, 35].map(|x| at(&hi.image, x)), [[0, 0, 0, 255], [255, 0, 0, 255], [0, 0, 0, 255], [255; 4]]); + let lo = shot(Overlay::ShadowClip); + assert_eq!(at(&lo.image, 5), [0, 0, 0, 255], "black where every channel clips"); + assert_eq!([25, 35].map(|x| at(&lo.image, x)), [[255; 4]; 2], "white where none does"); + // only the view changes: the histogram is the photo's + assert_eq!(hi.histogram.clipping(), shot(Overlay::None).histogram.clipping()); + for o in [Overlay::ShadowClip, Overlay::HighlightClip, Overlay::SharpenMask, Overlay::Grey] { + let (k, v) = o.to_parts(); + assert_eq!(Overlay::from_parts(k, v), o); + assert_ne!(o.key(), Overlay::None.key()); + } + } + + #[test] + fn sharpen_mask_view_follows_masking() { + // a hard vertical edge between two flat halves + let src = Rgb32f::from_fn(200, 100, |x, _| if x < 100 { [0.05; 3] } else { [0.4; 3] }); + let info = SourceInfo::default(); + let mut s = DevelopSettings::default(); + s.detail.sharpen_amount = 60.0; + let shot = + |s: &DevelopSettings| render(&src, &info, s, &RenderRequest { overlay: Overlay::SharpenMask, ..RenderRequest::fit(200, 100) }).image; + let at = |i: &Rgba8, x: usize| i.data[50 * 200 + x][0]; + // Masking 0: sharpening applies everywhere + let open = shot(&s); + assert!(open.data.iter().all(|p| *p == [255; 4]), "all white at Masking 0"); + // Masking 100: only the edge is sharpened + s.detail.sharpen_masking = 100.0; + let edge = shot(&s); + assert_eq!((at(&edge, 20), at(&edge, 180)), (0, 0), "flat areas held back"); + assert!(at(&edge, 100).max(at(&edge, 99)) > 200, "the edge sharpened: {}", at(&edge, 100)); + // the mask shows with sharpening off too + s.detail.sharpen_amount = 0.0; + assert_eq!(shot(&s).data, edge.data); + } + + #[test] + fn grey_view_drops_the_colour() { + let src = Rgb32f::from_fn(8, 8, |_, _| [0.4, 0.1, 0.05]); + let req = RenderRequest { overlay: Overlay::Grey, ..RenderRequest::fit(8, 8) }; + let v = render(&src, &SourceInfo::default(), &DevelopSettings::default(), &req).image; + assert!(v.data.iter().all(|p| p[0] == p[1] && p[1] == p[2] && p[0] > 0)); + } + #[test] fn point_color_range_keeps_the_selected_colour_only() { // left half orange, right half blue diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index bb290bd86..f9577b224 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -54,6 +54,8 @@ mod tests_album_picker; #[cfg(test)] mod tests_album_remove; #[cfg(test)] +mod tests_alt_preview; +#[cfg(test)] mod tests_appearance; #[cfg(test)] mod tests_before_after; diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 68b5346e3..2bae040ba 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -320,11 +320,11 @@ fn animated_rect(ctx: &egui::Context, anim: &mut bool, target: Rect) -> Rect { /// Whether an overlay can be rendered on a region tile without changing what the tile means. /// Mask overlays (one mask, or every visible mask at once) are evaluated from the same mask -/// alphas as the full render; the other diagnostic overlays either need whole-frame context or -/// deliberately replace the loupe. +/// alphas as the full render, and the clipping and grey views work per pixel of it; the other +/// diagnostic overlays need whole-frame context or deliberately replace the loupe. pub(crate) fn overlay_supports_window(overlay: lightcraft_pipeline::Overlay) -> bool { use lightcraft_pipeline::Overlay; - matches!(overlay, Overlay::None | Overlay::Mask { .. } | Overlay::AllMasks { .. }) + matches!(overlay, Overlay::None | Overlay::Mask { .. } | Overlay::AllMasks { .. } | Overlay::ShadowClip | Overlay::HighlightClip | Overlay::Grey) } /// What a window render of the loupe needs to know about the view it belongs to. @@ -518,7 +518,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { app.image_rect = Some(img_rect); // request renders: the loupe at display resolution (drafts during drags) let interacting = app.session.interaction.is_some(); - let overlay = view_overlay(app, &d); + let overlay = view_overlay(app, &d, ui.input(|i| i.modifiers.alt && i.pointer.primary_down())); // a drag drafts at full size where the GPU draws the view (by the GPU's own rule, `handles`; // `ready` first: `available` would wait for a device still being created) let gpu = interacting @@ -1338,13 +1338,42 @@ fn targeted_drag(app: &mut LightcraftApp, resp: &egui::Response, map: &CanvasMap } } -/// The diagnostic overlay the loupe shows (the selected mask, Point Color's visualized range, +/// The view holding ⌥ (Option) while dragging slider `control` shows in the loupe, as in Lightroom +/// (issue #937): clipping on Blacks (shadows) and Whites / Exposure (highlights), the sharpening +/// mask on Masking, the image in grey on the other Sharpening and the luminance Noise Reduction +/// sliders. Releasing ⌥ or the slider brings the photo back; nothing is saved. +pub(crate) fn slider_preview(control: &str) -> Option { + SLIDER_PREVIEWS.iter().find(|(id, _)| *id == control).map(|(_, o)| *o) +} + +/// The controls with a [`slider_preview`], and the view each shows. +pub(crate) const SLIDER_PREVIEWS: [(&str, lightcraft_pipeline::Overlay); 10] = { + use lightcraft_pipeline::Overlay::{Grey, HighlightClip, ShadowClip, SharpenMask}; + [ + ("light.blacks", ShadowClip), + ("light.whites", HighlightClip), + ("light.exposure", HighlightClip), + ("detail.sharpenMasking", SharpenMask), + ("detail.sharpenAmount", Grey), + ("detail.sharpenRadius", Grey), + ("detail.sharpenDetail", Grey), + ("detail.nrLuminance", Grey), + ("detail.nrDetail", Grey), + ("detail.nrContrast", Grey), + ] +}; + +/// The diagnostic overlay the loupe shows (the ⌥-drag preview of the slider being dragged while +/// `alt`: ⌥ is held with the button down; the selected mask, Point Color's visualized range, /// Visualize Spots). -pub(crate) fn view_overlay(app: &LightcraftApp, d: &DevelopSettings) -> lightcraft_pipeline::Overlay { +pub(crate) fn view_overlay(app: &LightcraftApp, d: &DevelopSettings, alt: bool) -> lightcraft_pipeline::Overlay { use lightcraft_pipeline::{MaskView, Overlay}; if app.ui.fullscreen { return Overlay::None; } + if alt && let Some(o) = app.ui.dragging_control.as_deref().and_then(slider_preview) { + return o; + } // Show every visible mask's region at once (Ctrl+Shift+O / Cmd+Shift+O), ahead of the selected-mask overlay // so the shortcut is not shadowed by the always-on single-mask overlay. if app.ui.right == RightPanel::Masking && app.ui.mask_overlay_all && d.masks.iter().any(|m| m.visible && !m.components.is_empty()) { diff --git a/crates/ui-egui/src/tests_alt_preview.rs b/crates/ui-egui/src/tests_alt_preview.rs new file mode 100644 index 000000000..24abcd7ce --- /dev/null +++ b/crates/ui-egui/src/tests_alt_preview.rs @@ -0,0 +1,93 @@ +//! Headless tests of the ⌥-drag previews (issue #937): holding ⌥ (Option) while dragging Blacks, +//! Whites, Exposure or a Sharpening / Noise Reduction slider shows a diagnostic view in the loupe. + +use std::time::Duration; + +use egui::{Event, Modifiers, PointerButton}; +use lightcraft_pipeline::Overlay; +use serde_json::json; + +use crate::headless::Headless; +use crate::panels::detail::{SLIDER_PREVIEWS, slider_preview}; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn detail() -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1400.0, 1400.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail", "right": "edit"}), T); + assert_eq!(r["ok"], true, "{r}"); + if !h.app.ui.section_open("light") { + h.app.ui.toggle_section("light"); + } + h.app.renderer.keep_pixels = true; + h.settle(SETTLE); + h +} + +/// Share of the loupe render's pixels whose channels are all 0 or 255 (a clipping view). +fn binary_share(h: &Headless) -> f64 { + let t = h.app.renderer.textures.get(&crate::render::Slot::Main).expect("loupe render"); + let px = &t.pixels.as_ref().expect("pixels kept").pixels; + let binary = px.iter().filter(|c| [c.r(), c.g(), c.b()].iter().all(|v| *v == 0 || *v == 255)).count(); + binary as f64 / px.len().max(1) as f64 +} + +#[test] +fn alt_during_a_blacks_drag_shows_shadow_clipping_until_released() { + let mut h = detail(); + assert!(binary_share(&h) < 0.5, "the photo itself is no clipping view"); + let track = h.app.widgets.iter().find(|(w, _)| w == "slider:light.blacks").map(|(_, r)| *r).expect("Blacks slider on screen"); + let from = track.center(); + let before = h.app.session.develop_of(h.app.session.active().expect("photo")).expect("develop").light.blacks; + // a plain drag to the left, button held: the photo as usual + h.app.synthetic.push(Event::PointerMoved(from)); + h.app.synthetic.push(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + for i in 1..=8 { + h.app.synthetic.push(Event::PointerMoved(from - egui::vec2(6.0 * i as f32, 0.0))); + } + h.settle(SETTLE); + assert_eq!(h.app.ui.dragging_control.as_deref(), Some("light.blacks")); + assert!(binary_share(&h) < 0.5, "no preview without ⌥"); + // ⌥ pressed mid-drag: the shadow clipping view + h.app.synthetic.push(Event::ModifiersChanged(Modifiers::ALT)); + h.settle(SETTLE); + assert!(binary_share(&h) > 0.99, "the clipping view: {}", binary_share(&h)); + // ⌥ released: the photo again, the drag still on + h.app.synthetic.push(Event::ModifiersChanged(Modifiers::NONE)); + h.settle(SETTLE); + assert!(binary_share(&h) < 0.5, "the photo is back"); + h.app.synthetic.push(Event::PointerButton { + pos: from - egui::vec2(48.0, 0.0), + button: PointerButton::Primary, + pressed: false, + modifiers: Modifiers::NONE, + }); + h.settle(SETTLE); + assert_eq!(h.app.ui.dragging_control, None); + let after = h.app.session.develop_of(h.app.session.active().expect("photo")).expect("develop").light.blacks; + assert!(after < before, "the drag itself still moved Blacks: {before} → {after}"); +} + +#[test] +fn each_slider_has_its_preview() { + assert_eq!(slider_preview("light.blacks"), Some(Overlay::ShadowClip)); + for c in ["light.whites", "light.exposure"] { + assert_eq!(slider_preview(c), Some(Overlay::HighlightClip), "{c}"); + } + assert_eq!(slider_preview("detail.sharpenMasking"), Some(Overlay::SharpenMask)); + for c in ["detail.sharpenAmount", "detail.sharpenRadius", "detail.sharpenDetail", "detail.nrLuminance", "detail.nrDetail", "detail.nrContrast"] { + assert_eq!(slider_preview(c), Some(Overlay::Grey), "{c}"); + } + for c in ["light.contrast", "color.saturation", "detail.nrColor"] { + assert_eq!(slider_preview(c), None, "{c}"); + } + // every mapped id is a real control, once (a rename would silently drop its preview) + for (i, (c, _)) in SLIDER_PREVIEWS.iter().enumerate() { + assert!(lightcraft_develop::controls::find(c).is_some(), "{c}"); + assert!(!SLIDER_PREVIEWS[..i].iter().any(|(d, _)| d == c), "{c} twice"); + } +} diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index bbea5356e..5e7beba9b 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -159,23 +159,23 @@ fn mask_adjustments_hide_overlay_without_disabling_the_mask() { let d = develop(&h); assert!(d.masks[0].adjust.exposure > 0.0); assert!(d.masks[0].visible, "hiding the overlay must not disable the mask"); - assert_eq!(crate::panels::detail::view_overlay(&h.app, &d), Overlay::None); + assert_eq!(crate::panels::detail::view_overlay(&h.app, &d, false), Overlay::None); let mask = d.masks[0].clone(); let r = h.request("ui.key", json!({"key": "o"}), T); assert_eq!(r["ok"], true, "{r}"); assert!(h.app.ui.mask_overlay, "O can restore the overlay"); assert_eq!(develop(&h).masks[0], mask, "toggling the overlay leaves the mask and its effect unchanged"); - assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::Mask { .. })); + assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h), false), Overlay::Mask { .. })); // Ctrl+Shift+O / Cmd+Shift+O shows every visible mask's region at once (the new command), ahead of the selected // mask overlay so it is not shadowed by the always-on single-mask overlay. exec(&mut h, "view.maskOverlayAll", json!({})); assert!(h.app.ui.mask_overlay_all, "the all-masks overlay command turns it on"); - assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::AllMasks { .. })); + assert!(matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h), false), Overlay::AllMasks { .. })); exec(&mut h, "view.maskOverlayAll", json!({})); assert!(!h.app.ui.mask_overlay_all, "the all-masks overlay command toggles it off"); - assert!(!matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h)), Overlay::AllMasks { .. })); + assert!(!matches!(crate::panels::detail::view_overlay(&h.app, &develop(&h), false), Overlay::AllMasks { .. })); // Failed edits and global adjustments must not hide the mask overlay. assert!(h.app.run("mask.adjust", json!({"id": 999, "values": {"exposure": 1.0}})).is_err()); @@ -274,12 +274,12 @@ fn mask_overlay_keys_and_pins() { assert_eq!(h.app.session.active_mask, Some(2)); // the loupe asks the renderer for the selected mask's overlay let d = develop(&h); - let o = crate::panels::detail::view_overlay(&h.app, &d); + let o = crate::panels::detail::view_overlay(&h.app, &d, false); assert_eq!(o, Overlay::Mask { id: 2, view: MaskView::Color, color: [230, 30, 40], opacity: 50 }); // O toggles it, Shift+O cycles the colour (and leaves the crop overlay alone) h.request("ui.key", json!({"key": "o"}), T); assert!(!h.app.ui.mask_overlay); - assert_eq!(crate::panels::detail::view_overlay(&h.app, &d), Overlay::None); + assert_eq!(crate::panels::detail::view_overlay(&h.app, &d, false), Overlay::None); h.request("ui.key", json!({"key": "o"}), T); let crop = h.app.ui.crop_overlay; let colour = h.app.ui.mask_overlay_color; diff --git a/crates/ui-egui/src/tests_preview_limit.rs b/crates/ui-egui/src/tests_preview_limit.rs index dfddb9c4f..4be0ad35f 100644 --- a/crates/ui-egui/src/tests_preview_limit.rs +++ b/crates/ui-egui/src/tests_preview_limit.rs @@ -212,7 +212,10 @@ mod in_the_loupe { h.settle(SETTLE); let id = h.app.session.active().expect("a photo"); let d = h.app.session.develop_of(id).expect("settings"); - assert!(matches!(crate::panels::detail::view_overlay(&h.app, &d), lightcraft_pipeline::Overlay::Mask { .. }), "the mask overlay is shown"); + assert!( + matches!(crate::panels::detail::view_overlay(&h.app, &d, false), lightcraft_pipeline::Overlay::Mask { .. }), + "the mask overlay is shown" + ); assert_eq!((h.app.ui.zoom, h.app.ui.pan), (zoom, pan), "showing the overlay keeps zoom and pan"); let main = rendered_long_edge(&h); assert!(main <= 2560 && main < native, "native {native}: the whole-frame render is {main}"); @@ -250,7 +253,7 @@ mod in_the_loupe { let id = h.app.session.active().expect("a photo"); let d = h.app.session.develop_of(id).expect("settings"); assert!( - matches!(crate::panels::detail::view_overlay(&h.app, &d), lightcraft_pipeline::Overlay::AllMasks { .. }), + matches!(crate::panels::detail::view_overlay(&h.app, &d, false), lightcraft_pipeline::Overlay::AllMasks { .. }), "the all-masks overlay is shown" ); let main = rendered_long_edge(&h); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 043aebb50..8c1fa715e 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-11 · **Change:** minor (⌥-drag previews on Blacks / Whites / Exposure / Sharpening / luminance NR, #937; LR-VIEW-CLIPPING and LR-EDIT-DETAIL-SHARPEN downgraded to 🟡: J doesn't paint clipped pixels, Radius / Detail unused; ⌥-drag previews on Blacks / Whites / Exposure / Sharpening / luminance NR, #937; LR-VIEW-CLIPPING and LR-EDIT-DETAIL-SHARPEN downgraded to 🟡: J doesn't paint clipped pixels, Radius / Detail unused; LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **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 @@ -28,10 +28,10 @@ cited here still exists. Formerly docs/parity.md. |---|---:|---:|---:|---:|---:|---:| | A. Import (IMP) | 12 | 3 | 1 | 1 | 4/6 (67%) | 5/5 (100%) | | B. Library management (LIB) | 22 | 2 | 1 | 2 | 9/9 (100%) | 9/9 (100%) | -| C. Views & navigation (VIEW) | 15 | 2 | 1 | 0 | 8/9 (89%) | 4/4 (100%) | +| C. Views & navigation (VIEW) | 14 | 3 | 1 | 0 | 7/9 (78%) | 4/4 (100%) | | D. Search & filter (FILT) | 11 | 1 | 1 | 0 | 4/4 (100%) | 4/4 (100%) | | E. Metadata (META) | 5 | 1 | 0 | 0 | 2/2 (100%) | 2/2 (100%) | -| F. Edit panel — global adjustments (EDIT) | 44 | 2 | 3 | 1 | 28/28 (100%) | 13/14 (93%) | +| F. Edit panel — global adjustments (EDIT) | 43 | 3 | 3 | 1 | 27/28 (96%) | 13/14 (93%) | | G. Profiles (PROF) | 7 | 1 | 3 | 0 | 3/4 (75%) | 3/3 (100%) | | H. Crop & rotate (CROP) | 9 | 0 | 0 | 1 | 6/6 (100%) | 3/3 (100%) | | I. Remove / healing (REM) | 8 | 1 | 2 | 2 | 4/4 (100%) | 2/3 (67%) | @@ -53,9 +53,9 @@ cited here still exists. Formerly docs/parity.md. | Y. Menus | 84 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 78 | 4 | 2 | 1 | 49/52 (94%) | 24/26 (92%) | | Lightroom Classic extras | 40 | 23 | 23 | 8 | — | 24/26 (92%) | -| **Total** | 418 | 62 | 59 | 35 | 193/201 (96%) | 148/161 (92%) | +| **Total** | 416 | 64 | 59 | 35 | 191/201 (95%) | 148/161 (92%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **83.3%** of 539 in-scope rows — P0 98.0% of 201 · P1 95.0% of 161 · P2 55.7% of 176. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **83.1%** of 539 in-scope rows — P0 97.5% of 201 · P1 95.0% of 161 · P2 55.7% of 176. ## Top gaps and shortcut notes @@ -140,7 +140,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-INFOOVERLAY | Info overlay on the photo | P1 | ✅ | `cmd:view.infoOverlay` | off / file + date + size / exposure + camera; ⌘I cycles (I in full screen; elsewhere I stays the Info panel) | | LR-VIEW-SLIDESHOW | Slideshow | P2 | ✅ | `cmd:view.slideshow` | View ▸ Slideshow (⌥⌘↩): the photos in view full screen from the selected one, played with the library's playback settings (see LRC-SS-PLAYBACK: fades, random order, repeat, previews prepared ahead; `interval` / any setting overrides one run); Space pauses, ←/→ step (a cut), Esc ends; playback controls on pointer movement | | LR-VIEW-SECONDWINDOW | Second display window [Classic] | P2 | ✅ | `cmd:view.secondWindow`, `crates/ui-egui/src/panels/second.rs` | Window ▸ Second Window (⌘F11): the active photo fitted in its own native window with its own render (a floating panel where there are no native windows); loupe view only (no grid / compare / survey there) | -| LR-VIEW-CLIPPING | Clipping indicators | P0 | ✅ | `cmd:view.clipping` | | +| LR-VIEW-CLIPPING | Clipping indicators | P0 | 🟡 | `cmd:view.clipping`, `crates/ui-egui/src/panels/detail.rs` (`clipping_overlay`, `slider_preview`) | J only frames the photo red (highlights) / blue (shadows) when any pixel clips; it doesn't paint the clipped pixels. ⌥-dragging Blacks / Whites / Exposure shows the clipped pixels and channels (`Overlay::ShadowClip` / `HighlightClip`, issue #937) | | LR-VIEW-HISTOGRAM | Histogram | P0 | ✅ | `cmd:view.histogram`, `crates/ui-egui/src/panels/edit.rs`, `crates/pipeline/src/lib.rs` (`hdr_histogram`) | HDR edits: SDR on the left half, the HDR range (0 to 5 stops above SDR white, a fixed scale) on the right with the headroom limit marked; colour bar (white, or Visualize HDR range's colours when that is on) and SDR / HDR labels; no drag-to-adjust on the histogram; it follows slider drags live (a small render beside the drafts, `Slot::HdrHistogram`) and never drops to the SDR layout mid-drag | | LR-VIEW-HDR-DISPLAY | HDR display output | P2 | ⬜ | |, `apps/lightcraft/src/hdr_display.rs` (`choose`, `preferred_hdr_format`) | SDR/HDR decision (platform, surface format, OS headroom, `LIGHTCRAFT_HDR_DISPLAY=0`) is unit-tested and logged at start; always SDR today because eframe configures an 8-bit surface; no OS headroom query yet; | @@ -180,12 +180,12 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-AUTO | Auto settings | P0 | ✅ | `cmd:develop.auto`, `crates/pipeline/src/auto.rs` | a Lightroom-like recipe calibrated black-box on Lightroom's Auto results for 24 Sony raws: Exposure from the scene's scene-linear log-average luminance (Lightroom's observed slope, -0.41 EV per EV), Highlights -66 and Shadows +52 (Lightroom's average Auto values), Contrast +68 and Vibrance +30 fitted on our scales. Held out one photo at a time: on the 16 photos whose start Sony's DRO doesn't brighten, a median dE76 of 6.4 from Lightroom's Auto results against 8.3 unedited; on all 24, 7.7 against 7.9 (issues #318, #209). Calibrated on Sony files only; Whites and Blacks stay 0 | | LR-EDIT-BW | Black & white | P0 | ✅ | `cmd:develop.treatment` | | | LR-EDIT-HDR-MODE | HDR editing | P2 | ✅ | `cmd:develop.hdr`, `ctl:hdr.maxEv`, `crates/ui-egui/src/panels/edit.rs` (`hdr_row`) | HDR button next to B&W in the Edit panel header, as Lightroom (Visualize HDR and the headroom limit in the Light panel while it's on); see Q. HDR | -| LR-EDIT-LIGHT-EXPOSURE | Exposure | P0 | ✅ | `ctl:light.exposure` | | +| LR-EDIT-LIGHT-EXPOSURE | Exposure | P0 | ✅ | `ctl:light.exposure` | ⌥-drag: highlight clipping view (#937) | | LR-EDIT-LIGHT-CONTRAST | Contrast | P0 | ✅ | `ctl:light.contrast` | | | LR-EDIT-LIGHT-HIGHLIGHTS | Highlights | P0 | ✅ | `ctl:light.highlights`, `crates/raw/src/highlight.rs` | sensor-clipped highlights stay neutral when Highlights (or Exposure, Whites) darkens them: fully clipped content is rebuilt to the neutral of the colour model applied after it, partly clipped content takes its colour from reliable unclipped content nearby (not from dark objects, lens fringes or another surface beside it) and white where there is none, and pale partly clipped content (a white sky) turns white on its way to clipping (#523); binned previews and thumbnails decide clipping per sample before averaging (`develop_binned_masked`, `highlight::reconstruct_masked`), so a block with one clipped specular sample keeps its mean instead of jumping to the clip level, which reconstruction turned into green speckles at 500 px (#548) From process V2 the Highlights/Shadows base is the edge-aware filter of the 0.15 %-of-long-edge median of log luminance: twigs and fine texture take their surroundings' shift, so bright areas keep their local contrast as with Exposure (V1's base kept ~0.6 of it at −100, #632); region edges stay sharp | | LR-EDIT-LIGHT-SHADOWS | Shadows | P0 | ✅ | `ctl:light.shadows` | From process V2, the same base as Highlights: dark areas keep their local contrast when lifted (#632) | -| LR-EDIT-LIGHT-WHITES | Whites | P0 | ✅ | `ctl:light.whites` | | -| LR-EDIT-LIGHT-BLACKS | Blacks | P0 | ✅ | `ctl:light.blacks` | | +| LR-EDIT-LIGHT-WHITES | Whites | P0 | ✅ | `ctl:light.whites` | ⌥-drag: highlight clipping view, black where nothing clips (#937) | +| LR-EDIT-LIGHT-BLACKS | Blacks | P0 | ✅ | `ctl:light.blacks` | ⌥-drag: shadow clipping view, white where nothing clips (#937) | | LR-EDIT-LIGHT-CURVE-PARAM | Parametric curve | P0 | ✅ | `ctl:curve.highlights`, `ctl:curve.lights`, `ctl:curve.darks`, `ctl:curve.shadows`, `ctl:curve.split*` | | | LR-EDIT-LIGHT-CURVE-POINT | Point curve | P0 | ✅ | `cmd:develop.curve`, `cmd:curve.reset`, `cmd:curve.presets`, `cmd:curve.applyPreset`, `cmd:curve.savePreset`, `cmd:curve.deletePreset`, `cmd:curve.importPresets`, `cmd:curve.exportPresets`, `cmd:file.importCurvePresets`, `cmd:file.exportCurvePresets`, `crates/engine/src/cmd/curves.rs`, `crates/ui-egui/src/panels/edit.rs` | click to add a point; drag a point in both axes (between its neighbours, input / output readout, one undo step per drag; ⌥-drag moves it a tenth as far for fine adjustment, issue #1363); drag empty space to add and drag; double-click removes; reset: double-click a channel selector (that channel), the Reset button under the graph (every curve incl. parametric) or right-click the graph (channel / all); Point Curve preset dropdown: own Linear / Medium Contrast / Strong Contrast, user presets (save the current point curves, delete by right-click; saved with the library), import / export as `.lccurve` JSON | | LR-EDIT-LIGHT-CURVE-RGB | Per-channel curves | P0 | ✅ | `cmd:develop.curve` (`channel`) | | @@ -206,8 +206,8 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-EFFECTS-DEHAZE | Dehaze | P0 | ✅ | `ctl:effects.dehaze` | | | LR-EDIT-EFFECTS-VIGNETTE | Post-crop vignette | P0 | ✅ | `ctl:vignette.*`, `crates/pipeline/src/finish.rs`, `crates/ui-egui/src/panels/edit.rs` | style picker (Highlight / Color / Paint) in the Effects section | | LR-EDIT-EFFECTS-GRAIN | Grain | P1 | ✅ | `ctl:grain.*` | | -| LR-EDIT-DETAIL-SHARPEN | Sharpening | P0 | ✅ | `ctl:detail.sharpenAmount`, `ctl:detail.sharpenRadius`, `ctl:detail.sharpenDetail`, `ctl:detail.sharpenMasking` | no Alt-drag mask preview | -| LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | radii in the original's pixels, so grid, loupe and smaller exports look like the full-size render scaled down (`local::nr_params`, `Plan::detail_scale`, #960) | +| LR-EDIT-DETAIL-SHARPEN | Sharpening | P0 | 🟡 | `ctl:detail.sharpenAmount`, `ctl:detail.sharpenRadius`, `ctl:detail.sharpenDetail`, `ctl:detail.sharpenMasking` | ⌥-drag: Masking shows the sharpening mask white on black, the other sliders the image in grey (#937). Radius and Detail are saved but the render doesn't use them yet (fixed band at 0.18% of the long edge, `local::texture_sigma`) | +| LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | radii in the original's pixels, so grid, loupe and smaller exports look like the full-size render scaled down (`local::nr_params`, `Plan::detail_scale`, #960); ⌥-drag: the image in grey (#937) | | LR-EDIT-DETAIL-CNR | Colour noise reduction | P0 | ✅ | `ctl:detail.nrColor`, `ctl:detail.nrColorDetail`, `ctl:detail.nrColorSmoothness` | preview-size independent like luminance NR (#960) | | LR-EDIT-DETAIL-DENOISE | AI denoise | P2 | 🟡 | cmd:denoise.toggle, cmd:denoise.models.install, cmd:denoise.queue, ctl:enhance.denoise, `crates/denoise/`, `docs/denoise.md` | non-destructive Bayer RAW, opt-in weights, pure-Rust CPU/GPU; GPU convolution uses independently written shared scalars (Metal equivalence regression, issue #479); tiles can also run on another computer's graphics card (`lightcraft-cli denoise-serve`, `crates/denoise/src/remote.rs`); X-Trans/linear RGB, model policy and Lightroom fidelity remain; closing the model setup without installing turns the switch back off (#922) | | LR-EDIT-DETAIL-RAWDETAILS | Improved demosaic toggle | P2 | ⬜ | | | @@ -871,6 +871,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-13 | minor | LR-META-INFO: Nikon DSLR raws show their lens from the maker note; status unchanged (✅) | | 2026-10-13 | patch | LR-IMP-LOCAL: browsing a Local folder again forgets the untouched records of files deleted or moved by another app (issue #1392); status unchanged | | 2026-10-13 | minor | LR-MERGE-PANO: a panorama keeps its photos' resolution (up to 512 MP / 65,000 px) instead of a fixed 40 MP, held to what fits in memory (issue #1455); status unchanged | +| 2026-10-11 | minor | ⌥-drag previews (issue #937) noted on LR-EDIT-LIGHT-EXPOSURE / WHITES / BLACKS and LR-EDIT-DETAIL-NR; LR-VIEW-CLIPPING ✅ → 🟡 (J frames the photo, doesn't paint clipped pixels), LR-EDIT-DETAIL-SHARPEN ✅ → 🟡 (Radius and Detail don't reach the render) | | 2026-10-11 | minor | KEY-SLIDER-TYPE: honor immediate Return/Escape and keep typed keys in the value editor; add regression references | | 2026-10-11 | minor | LR-PROF-CAMERACOLOR: raws with a black-and-white camera JPEG take its tone curve (issue #577) | | 2026-10-11 | minor | LR-VIEW-ZOOM and LR-VIEW-COMPARE: Compare and Reference are sharp at 1:1 (zoom windows per pane, two originals kept decoded while they are open); statuses unchanged | From 342afcb81eecc30250c85bbe9506940350c2c510 Mon Sep 17 00:00:00 2001 From: witkowski01 Date: Thu, 15 Oct 2026 10:18:46 +0200 Subject: [PATCH 06/99] =?UTF-8?q?i18n(pl):=20menu=20command=20names,=20the?= =?UTF-8?q?=20formats=20left=20in=20English,=20and=20Vibrance=20as=20Jaskr?= =?UTF-8?q?awo=C5=9B=C4=87?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/ui-egui/locales/pl-formats.json | 84 +++++++++--------- crates/ui-egui/locales/pl.json | 114 ++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 44 deletions(-) diff --git a/crates/ui-egui/locales/pl-formats.json b/crates/ui-egui/locales/pl-formats.json index 1886098df..19f2c1a38 100644 --- a/crates/ui-egui/locales/pl-formats.json +++ b/crates/ui-egui/locales/pl-formats.json @@ -248,50 +248,50 @@ "{n} photos opened for editing; each is stacked with its original": "Zdjęcia otwarte do edycji: {n}; każde połączono w stos z oryginałem", "· {in_source} identical copies": "· identyczne kopie: {in_source}", "· {trashed} in Recently Deleted": "· w Ostatnio usuniętych: {trashed}", - " in {g} group{}": " in {g} group{}", - " · kept, not applied: {}": " · kept, not applied: {}", - " — kept but not applied: {}": " — kept but not applied: {}", - "Apply to “{mask}” on Selected Photos": "Apply to “{mask}” on Selected Photos", - "Delete the effect preset “{name}”?": "Delete the effect preset “{name}”?", - "Delete these {n} effect presets?": "Delete these {n} effect presets?", - "Delete {n} effect preset{}? This can't be undone.": "Delete {n} effect preset{}? This can't be undone.", - "Effect: {name}": "Effect: {name}", - "Exported {} effect preset{}": "Exported {} effect preset{}", + " in {g} group{}": " w grupach: {g}{:.0}", + " · kept, not applied: {}": " · zachowane, niezastosowane: {}", + " — kept but not applied: {}": " — zachowane, ale niezastosowane: {}", + "Apply to “{mask}” on Selected Photos": "Zastosuj do „{mask}” na zaznaczonych zdjęciach", + "Delete the effect preset “{name}”?": "Usunąć ustawienie wstępne efektu „{name}”?", + "Delete these {n} effect presets?": "Usunąć te ustawienia wstępne efektów ({n})?", + "Delete {n} effect preset{}? This can't be undone.": "Usunąć ustawienia wstępne efektów ({n})?{:.0} Tej operacji nie można cofnąć.", + "Effect: {name}": "Efekt: {name}", + "Exported {} effect preset{}": "Wyeksportowane ustawienia wstępne efektów: {}{:.0}", "Imported {e} mask effect{} (Masking ▸ Effect)": "Imported {e} mask effect{} (Masking → Effect)", - "Update “{name}” with Current Settings": "Update “{name}” with Current Settings", - "{n} preset{}": "{n} preset{}", - "{count} more warnings.": "{count} more warnings.", - "{photos} catalog records processed; {collections} collections.": "{photos} catalog records processed; {collections} collections.", - "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.", - "Applied {name}": "Applied {name}", - "Delete “{name}”?": "Delete “{name}”?", - "Updated {name}": "Updated {name}", - "Exported to {path}": "Exported to {path}", - "Saved template {name}": "Saved template {name}", - "Imported {name}": "Imported {name}", - "Skipped {problem}": "Skipped {problem}", - "…and {more} more": "…and {more} more", - "Template preview: {name}": "Template preview: {name}", - "{n} photos in view · {slides} slides · {time}": "{n} photos in view · {slides} slides · {time}", - "{n} pages": "{n} pages", - "{n} JPEG files": "{n} JPEG files", - "{s} s of AV1 video": "{s} s of AV1 video", - "Slideshow exported: {what}": "Slideshow exported: {what}", - " · {n} skipped (could not be rendered)": " · {n} skipped (could not be rendered)", - "{n} raws can now be developed from their raw data": "{n} raws can now be developed from their raw data", - "The XMP sidecars of {n} photos couldn't be written (is their folder offline?). LightCraft keeps trying.": "The XMP sidecars of {n} photos couldn't be written (is their folder offline?). LightCraft keeps trying.", - "Create the keyword “{name}”": "Create the keyword “{name}”", - "new, inside {parent}": "new, inside {parent}", - "Keywords of {n} selected photos": "Keywords of {n} selected photos", + "Update “{name}” with Current Settings": "Aktualizuj „{name}” bieżącymi ustawieniami", + "{n} preset{}": "Ustawienia wstępne: {n}{:.0}", + "{count} more warnings.": "Pozostałe ostrzeżenia: {count}.", + "{photos} catalog records processed; {collections} collections.": "Przetworzone rekordy katalogu: {photos}; kolekcje: {collections}.", + "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Brakujące oryginały: {missing}. Ich rekordy w katalogu są dostępne; użyj polecenia „Znajdź brakujące zdjęcia”, aby ponownie je połączyć.", + "Applied {name}": "Zastosowano: {name}", + "Delete “{name}”?": "Usunąć „{name}”?", + "Updated {name}": "Zaktualizowano: {name}", + "Exported to {path}": "Wyeksportowano do {path}", + "Saved template {name}": "Zapisano szablon {name}", + "Imported {name}": "Zaimportowano: {name}", + "Skipped {problem}": "Pominięto: {problem}", + "…and {more} more": "…i jeszcze {more}", + "Template preview: {name}": "Podgląd szablonu: {name}", + "{n} photos in view · {slides} slides · {time}": "Zdjęcia w widoku: {n} · slajdy: {slides} · {time}", + "{n} pages": "Strony: {n}", + "{n} JPEG files": "Pliki JPEG: {n}", + "{s} s of AV1 video": "{s} s wideo AV1", + "Slideshow exported: {what}": "Wyeksportowano pokaz slajdów: {what}", + " · {n} skipped (could not be rendered)": " · pominięte (nie udało się wyrenderować): {n}", + "{n} raws can now be developed from their raw data": "Pliki RAW, które można teraz wywołać z danych surowych: {n}", + "The XMP sidecars of {n} photos couldn't be written (is their folder offline?). LightCraft keeps trying.": "Nie udało się zapisać plików pomocniczych XMP zdjęć ({n}). Czy ich folder jest offline? LightCraft ponawia próby.", + "Create the keyword “{name}”": "Utwórz słowo kluczowe „{name}”", + "new, inside {parent}": "nowe, wewnątrz {parent}", + "Keywords of {n} selected photos": "Słowa kluczowe zaznaczonych zdjęć ({n})", "Removed “{name}” from {n} photos. Edit ▸ Undo puts it back.": "Removed “{name}” from {n} photos. Edit → Undo puts it back.", - "LightCraft stopped unexpectedly: {e}\n\nThe log file has the details: {path}": "LightCraft stopped unexpectedly: {e}\n\nThe log file has the details: {path}", - "LightCraft stopped unexpectedly: {e}": "LightCraft stopped unexpectedly: {e}", - "Renames {n} face{} across the library. The files are not changed.": "Renames {n} face{} across the library. The files are not changed.", - "“{to}” is already a person. Combining them makes one person; Undo separates them again.": "“{to}” is already a person. Combining them makes one person; Undo separates them again.", - "{what} merge added at {mp}: its full size, {full}, needs more memory than this computer has": "{what} merge added at {mp}: its full size, {full}, needs more memory than this computer has", - "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})": "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})", - "Color space: Rec. 2020 {curve} (HDR AVIF)": "Color space: Rec. 2020 {curve} (HDR AVIF)", - "{n} selected photos · {pages} pages": "{n} selected photos · {pages} pages", + "LightCraft stopped unexpectedly: {e}\n\nThe log file has the details: {path}": "LightCraft nieoczekiwanie zakończył działanie: {e}\n\nSzczegóły są w pliku dziennika: {path}", + "LightCraft stopped unexpectedly: {e}": "LightCraft nieoczekiwanie zakończył działanie: {e}", + "Renames {n} face{} across the library. The files are not changed.": "Zmienia nazwę twarzy w całej bibliotece (liczba twarzy: {n}).{:.0} Pliki nie są zmieniane.", + "“{to}” is already a person. Combining them makes one person; Undo separates them again.": "„{to}” to już istniejąca osoba. Połączenie utworzy jedną osobę; Cofnij ponownie je rozdzieli.", + "{what} merge added at {mp}: its full size, {full}, needs more memory than this computer has": "Dodano połączenie {what} w rozdzielczości {mp}: pełny rozmiar, {full}, wymaga więcej pamięci, niż ma ten komputer", + "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})": "Dodano połączenie {what} w rozdzielczości {mp}, największej możliwej dla panoramy (pełny rozmiar to {full})", + "Color space: Rec. 2020 {curve} (HDR AVIF)": "Przestrzeń kolorów: Rec. 2020 {curve} (HDR AVIF)", + "{n} selected photos · {pages} pages": "Zaznaczone zdjęcia: {n} · strony: {pages}", "Already in “{}”": "Już w „{}”", "Move {n} photo{} to “{}”": "Przenieś do „{1}” — zdjęcia: {n}{0:.0}", "Move {n} photo{} to “{}”?": "Przenieść do „{1}” — zdjęcia: {n}{0:.0}?", diff --git a/crates/ui-egui/locales/pl.json b/crates/ui-egui/locales/pl.json index 7416ce241..81a2ab60b 100644 --- a/crates/ui-egui/locales/pl.json +++ b/crates/ui-egui/locales/pl.json @@ -1511,7 +1511,7 @@ "Versions (Shift+V)": "Wersje (Shift+V)", "Vertical": "Pionowo", "Vertical text": "Tekst pionowy", - "Vibrance": "Dynamika", + "Vibrance": "Jaskrawość", "Videos": "Wideo", "View": "Widok", "Viewing and sharing": "Wyświetlanie i udostępnianie", @@ -2157,5 +2157,115 @@ "This is a Lightroom catalog, not a LightCraft library. Import it into a LightCraft library instead?": "To jest katalog Lightrooma, a nie biblioteka LightCraft. Zaimportować go zamiast tego do biblioteki LightCraft?", "Unnamed Faces": "Nienazwane twarze", "Vertical inset (% of short edge)": "Margines pionowy (% krótszego boku)", - "not added to Apple Photos (cancelled)": "nie dodano do aplikacji Zdjęcia (anulowano)" + "not added to Apple Photos (cancelled)": "nie dodano do aplikacji Zdjęcia (anulowano)", + "Set Folder Color Label": "Ustaw etykietę kolorystyczną folderu", + "Refresh View": "Odśwież widok", + "Hold View Until Refreshed": "Wstrzymaj widok do odświeżenia", + "Photo Below": "Zdjęcie poniżej", + "Photo Above": "Zdjęcie powyżej", + "Cycle Flag": "Przełącz oznaczenie", + "Increase Flag Status": "Podnieś status oznaczenia", + "Decrease Flag Status": "Obniż status oznaczenia", + "Update to Current Process": "Aktualizuj do bieżącej wersji procesu", + "HDR": "HDR", + "Previous History Step": "Poprzedni krok historii", + "Next History Step": "Następny krok historii", + "Find Blemishes": "Znajdź skazy", + "Keyword Info": "Informacje o słowie kluczowym", + "Create Keyword": "Utwórz słowo kluczowe", + "Edit Keyword": "Edytuj słowo kluczowe", + "Move Keyword": "Przenieś słowo kluczowe", + "Put New Keywords Inside": "Umieszczaj nowe słowa kluczowe wewnątrz", + "Mask Effect Presets": "Ustawienia wstępne efektów maski", + "Apply Mask Effect": "Zastosuj efekt maski", + "Save Mask Effect": "Zapisz efekt maski", + "Update Mask Effect": "Aktualizuj efekt maski", + "Rename Mask Effect": "Zmień nazwę efektu maski", + "Duplicate Mask Effect": "Duplikuj efekt maski", + "Delete Mask Effect": "Usuń efekt maski", + "Favorite Mask Effect": "Oznacz efekt maski jako ulubiony", + "Move Mask Effect to Group": "Przenieś efekt maski do grupy", + "Rename Mask Effect Group": "Zmień nazwę grupy efektów maski", + "Set Effect for New Masks": "Ustaw efekt dla nowych masek", + "Import Mask Effects": "Importuj efekty maski", + "Export Mask Effects": "Eksportuj efekty maski", + "HDR for Each Bracket Set": "HDR dla każdej serii ekspozycji", + "Write Waiting XMP Sidecars": "Zapisz oczekujące pliki pomocnicze XMP", + "Slideshow Playback Settings": "Ustawienia odtwarzania pokazu slajdów", + "Slideshow Layout Settings": "Ustawienia układu pokazu slajdów", + "Slideshow Overlay Settings": "Ustawienia nakładek pokazu slajdów", + "Slideshow Title Settings": "Ustawienia tytułów pokazu slajdów", + "Slideshow Templates": "Szablony pokazu slajdów", + "Apply Slideshow Template": "Zastosuj szablon pokazu slajdów", + "Save Slideshow Template": "Zapisz szablon pokazu slajdów", + "Update Slideshow Template": "Aktualizuj szablon pokazu slajdów", + "Rename Slideshow Template": "Zmień nazwę szablonu pokazu slajdów", + "Delete Slideshow Template": "Usuń szablon pokazu slajdów", + "Import Slideshow Template": "Importuj szablon pokazu slajdów", + "Export Slideshow Template": "Eksportuj szablon pokazu slajdów", + "Slideshow Slide Preview": "Podgląd slajdu pokazu", + "Export Slideshow": "Eksportuj pokaz slajdów", + "Slideshow Export Status": "Stan eksportu pokazu slajdów", + "Export Contact Sheet PDF": "Eksportuj arkusz stykowy PDF", + "Apple Photos Imports": "Importy z aplikacji Zdjęcia", + "Print Settings": "Ustawienia drukowania", + "Set Print Settings": "Zmień ustawienia drukowania", + "Reset Print Settings": "Przywróć ustawienia drukowania", + "Add Print Cell": "Dodaj komórkę wydruku", + "Move Print Cell": "Przenieś komórkę wydruku", + "Resize Print Cell": "Zmień rozmiar komórki wydruku", + "Delete Print Cell": "Usuń komórkę wydruku", + "Arrange Print Cell": "Rozmieść komórkę wydruku", + "Set Print Cell Photo": "Ustaw zdjęcie komórki wydruku", + "Move Print Cell to Page": "Przenieś komórkę wydruku na stronę", + "Auto Layout": "Układ automatyczny", + "New Print Page": "Nowa strona wydruku", + "Clear Print Layout": "Wyczyść układ wydruku", + "Print to PDF": "Drukuj do PDF", + "Print Templates": "Szablony wydruku", + "Save Print Template": "Zapisz szablon wydruku", + "Apply Print Template": "Zastosuj szablon wydruku", + "Delete Print Template": "Usuń szablon wydruku", + "Recheck Preview-Only Raws": "Sprawdź ponownie pliki RAW dostępne tylko jako podgląd", + "Scan Folder for Changes": "Skanuj folder w poszukiwaniu zmian", + "Rename Person": "Zmień nazwę osoby", + "Read Focus Points": "Odczytaj punkty ostrości", + "Read Focus Points in the Background": "Odczytuj punkty ostrości w tle", + "Background Tasks": "Zadania w tle", + "Cancel Background Task": "Anuluj zadanie w tle", + "Select Only Active Photo": "Zaznacz tylko aktywne zdjęcie", + "Show Focus Point": "Pokaż punkt ostrości", + "Zoom to Focus Point": "Powiększ do punktu ostrości", + "Show in Compare": "Pokaż w porównaniu", + "Increase Grid Thumbnail Size": "Zwiększ rozmiar miniatur w siatce", + "Decrease Grid Thumbnail Size": "Zmniejsz rozmiar miniatur w siatce", + "Visualize HDR": "Wizualizuj HDR", + "Cycle Tool Overlay": "Przełącz nakładkę narzędzia", + "Choose Layout Image…": "Wybierz obraz układu…", + "Show Layout Image": "Pokaż obraz układu", + "Layout Image Opacity": "Krycie obrazu układu", + "Layout Image Matte": "Matowanie obrazu układu", + "Solo Mode": "Tryb solo", + "Slideshow Options…": "Opcje pokazu slajdów…", + "Export Slideshow…": "Eksportuj pokaz slajdów…", + "Pause / Resume Slideshow": "Wstrzymaj / wznów pokaz slajdów", + "Next Slide": "Następny slajd", + "Previous Slide": "Poprzedni slajd", + "End Slideshow": "Zakończ pokaz slajdów", + "Contact Sheet PDF…": "Arkusz stykowy PDF…", + "Edit in PhotoCraft": "Edytuj w PhotoCraft", + "Edit in Other Editor": "Edytuj w innym edytorze", + "Open Original in Other Editor (RAW, no copy)": "Otwórz oryginał w innym edytorze (RAW, bez kopii)", + "Save Current Settings as New Effect Preset…": "Zapisz bieżące ustawienia jako nowe ustawienie wstępne efektu…", + "Manage Effect Presets…": "Zarządzaj ustawieniami wstępnymi efektów…", + "Export Effect Presets…": "Eksportuj ustawienia wstępne efektów…", + "Display Profile": "Profil ekranu", + "HDR Headroom": "Zapas jasności HDR", + "SDR Brightness": "Jasność SDR", + "SDR Contrast": "Kontrast SDR", + "SDR Highlights": "Światła SDR", + "SDR Shadows": "Cienie SDR", + "SDR Whites": "Biele SDR", + "SDR Clarity": "Przejrzystość SDR", + "Face Models on the GPU": "Modele twarzy przetwarzane na GPU" } From c990dec0f0f15acb345f9340865eabe24a16272c Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Thu, 15 Oct 2026 11:27:07 +0200 Subject: [PATCH 07/99] LR-VIEW-ZOOM: tracker row describes the two-strike, expiring slow-drag verdict (#330) Co-Authored-By: Claude Opus 5.5 --- docs/parity-checklist.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 95dcd2aef..b2d471526 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-15 · **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 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 @@ -131,7 +131,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-VIEW-EDIT | Edit view | P0 | ✅ | `cmd:panel.edit` | | | LR-VIEW-FULLSCREEN | Full-screen preview | P1 | ✅ | `cmd:view.fullScreenPreview`, `cmd:view.enterFullScreen` | photo on black, arrows step, Esc exits; ⇧⌘F window full screen | | LR-VIEW-FILMSTRIP | Filmstrip | P0 | ✅ | `cmd:view.filmstrip`, `crates/ui-egui/src/panels/detail.rs` | the photo context menu on right-click; the mouse wheel scrolls it sideways, a third of its width per notch as in Lightroom Classic (trackpads scroll the points they report); it follows the active photo only when that changes (centred if off screen), so a scrolled strip stays put; every selected photo is highlighted, the active one framed in white, as in the grid | -| LR-VIEW-ZOOM | Zoom & pan | P0 | ✅ | `cmd:view.zoomFit`, `cmd:view.zoom100`, `cmd:view.zoomIn`, `cmd:view.zoomOut`, `cmd:view.zoomToggle`, `cmd:view.clickZoom`, `cmd:view.navigate`, `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/headless.rs` | continuous native pinch / modifier-wheel zoom (Fit–800%, anchored under the pointer), two-finger scroll / touch pan with edge limits; Detail (including tools, Before/After and full-screen preview), Compare and Reference; fractional percentages preserve small gestures; image, picker and targeted-adjustment cursors stay steady between gesture events; keyboard steps 25–800 % (not 6–1600 %); Fill in the bottom bar; click / Z / Space eases to the chosen ratio (1:1 default, 2:1, 3:1, 4:1, 8:1). The loupe's cost follows the canvas, not the zoom (issue #323): the whole frame is rendered at about canvas size (at most the 2560 px preview source level unless a larger Preview size is chosen, and never above the GPU's texture side), and once the view is zoomed well past that (more than 25 %) the window on screen is rendered too, cut from a frame of at most the photo's own pixels, so beyond 100 % the GPU magnifies it (`region::plan`, `RenderRequest::window`, `Session::region_job`, `crates/ui-egui/src/region.rs`); 1:1 and 2:1–8:1 are sharp at any photo size. A slider tick takes 7–12 ms on a 1400 × 900 canvas and 11–25 ms on a 2800 × 1700 one at 100 % and 400 % of a 24 MP photo (M1 Max, GPU, `profile_slider_drag`); about 20–25 ms at 100 % on the big canvas is over a 16 ms frame. A slider drag renders the whole frame at full size where the GPU draws the view, so the fit view stays sharp while dragging, and at 0.6 of it where that would be too slow: on the CPU, with soft proofing or the HDR range shown, or for a control whose full-size drafts on this photo went over the 16 ms slider budget in an earlier drag (a LUT profile or refined mask the CPU renders, a photo past a GPU limit, a slow GPU); a draft's time is its render plus the frame that uploads and shows it. The size is chosen when a drag starts and kept until it ends, unless its full-size drafts take more than twice the budget, when it drops to 0.6 after three of them (`region::DragScale`, issue #330). Zoomed in, the whole-frame draft under the window is the 0.6 one (only the histogram and the window's stand-in use it). Render workers convert the pixels for their textures, not the UI thread. Handing GPU renders to egui without a CPU readback is #1228. A continuous pinch holds the render sizes (and the windows there are) until it is quiet. A fit view on a canvas up to about 3200 px (a Retina Mac) never decodes the original; larger displays get a window at fit, which does. Diagnostic overlays that need whole-frame context (Visualize Spots, Point Color range, HDR range) and photos whose spots or Auto Mask strokes read too far for one window (only photos longer than 8192 px) render the whole frame at the size it is drawn, as before windows; while a preset or profile is hovered, or over a picture that is not the frame (an unsupported raw's embedded JPEG), the whole frame stays canvas-sized. Before/After (side by side, stacked, `\\` and both wipes) gets the same windows on both sides, clipped to their pane or side of the line. Windows render on the GPU like whole frames (equivalence-tested against the CPU), in the browser build too (windows no wider than its GPU's texture side, at most 3072 px), and are not made while hovering a look or showing a diagnostic overlay, nor over a picture that is not the frame (an unsupported raw's embedded JPEG with another crop). No texture is larger than the GPU allows. Spots and Auto Mask brushes work while what a window reads fits one render (8192 px along an axis, 4096 in the browser): the window grows to hold the spot and its source patch, or the dabs near it, and if that would take more (a clone from far away at 8:1) the loupe keeps the whole-frame render instead, and spots with an automatic source pick it from a fixed-size render of the whole frame (so a window's choice can differ from a whole-frame render's, which picks at its own resolution). The wide stages (dehaze, clarity, highlights/shadows base) and Refine Edges on masks read a window margin capped at 768 px; measured on hard-edged blocks that costs at most a few levels at 8:1. the histogram and Navigator come from the whole-frame render. Compare and Reference get the same windows, one per pane (`Slot::CompareRegion`, `panels/compare.rs`), over each pane's whole-frame render (at most 2560 px); while either view is open two originals stay decoded (`MediaCache::set_full_capacity`), so one pane's window does not evict the other's source; a pinch, or a slider drag on the active photo in Reference, keeps the windows there are until it ends. Mask overlays and soft proofing get the same windows (the window carries the overlay or the proof; the proof renders on the CPU, so a tick at 100 % on a 1400 × 900 canvas takes about 36 ms instead of a full-size 24 MP proof). Verified: a 1:1 window of a 24 MP photo with tone and colour edits is the full-size export to within one level (`one_to_one_window_pixels_are_export_pixels`). Measured on Linux (Ryzen AI Max+ 395, Radeon 8060S, GPU, `profile_slider_drag`): median tick→shown 7 ms at fit, 33 ms at 100 % and 6 ms at 400 % on 1400 × 900; 21 ms at 100 % and 9 ms at 400 % on 2800 × 1700; 36 ms at 100 % with soft proof. Not windowed: the diagnostic overlays named above | +| LR-VIEW-ZOOM | Zoom & pan | P0 | ✅ | `cmd:view.zoomFit`, `cmd:view.zoom100`, `cmd:view.zoomIn`, `cmd:view.zoomOut`, `cmd:view.zoomToggle`, `cmd:view.clickZoom`, `cmd:view.navigate`, `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/headless.rs` | continuous native pinch / modifier-wheel zoom (Fit–800%, anchored under the pointer), two-finger scroll / touch pan with edge limits; Detail (including tools, Before/After and full-screen preview), Compare and Reference; fractional percentages preserve small gestures; image, picker and targeted-adjustment cursors stay steady between gesture events; keyboard steps 25–800 % (not 6–1600 %); Fill in the bottom bar; click / Z / Space eases to the chosen ratio (1:1 default, 2:1, 3:1, 4:1, 8:1). The loupe's cost follows the canvas, not the zoom (issue #323): the whole frame is rendered at about canvas size (at most the 2560 px preview source level unless a larger Preview size is chosen, and never above the GPU's texture side), and once the view is zoomed well past that (more than 25 %) the window on screen is rendered too, cut from a frame of at most the photo's own pixels, so beyond 100 % the GPU magnifies it (`region::plan`, `RenderRequest::window`, `Session::region_job`, `crates/ui-egui/src/region.rs`); 1:1 and 2:1–8:1 are sharp at any photo size. A slider tick takes 7–12 ms on a 1400 × 900 canvas and 11–25 ms on a 2800 × 1700 one at 100 % and 400 % of a 24 MP photo (M1 Max, GPU, `profile_slider_drag`); about 20–25 ms at 100 % on the big canvas is over a 16 ms frame. A slider drag renders the whole frame at full size where the GPU draws the view, so the fit view stays sharp while dragging, and at 0.6 of it where that would be too slow: on the CPU, with soft proofing or the HDR range shown, or for a control whose full-size drafts on this photo went clearly over the 16 ms slider budget (more than 20 ms) in two drags in a row (a LUT profile or refined mask the CPU renders, a photo past a GPU limit, a slow GPU); a draft's time is its render plus the frame that uploads and shows it, and a host frame time over 1 s (a hidden or stalled window) counts as unknown. A slow verdict is per photo and expires: after 30 s a drag tries full size again, the wait doubling after each slow retry up to 240 s; a drag within 20 ms forgets it. At most 256 (photo, control) pairs are remembered, the oldest forgotten first. The size is chosen when a drag starts and kept until it ends, unless its full-size drafts take more than twice the budget, when it drops to 0.6 after three of them (`region::DragScale`, issue #330). Zoomed in, the whole-frame draft under the window is the 0.6 one (only the histogram and the window's stand-in use it). Render workers convert the pixels for their textures, not the UI thread. Handing GPU renders to egui without a CPU readback is #1228. A continuous pinch holds the render sizes (and the windows there are) until it is quiet. A fit view on a canvas up to about 3200 px (a Retina Mac) never decodes the original; larger displays get a window at fit, which does. Diagnostic overlays that need whole-frame context (Visualize Spots, Point Color range, HDR range) and photos whose spots or Auto Mask strokes read too far for one window (only photos longer than 8192 px) render the whole frame at the size it is drawn, as before windows; while a preset or profile is hovered, or over a picture that is not the frame (an unsupported raw's embedded JPEG), the whole frame stays canvas-sized. Before/After (side by side, stacked, `\\` and both wipes) gets the same windows on both sides, clipped to their pane or side of the line. Windows render on the GPU like whole frames (equivalence-tested against the CPU), in the browser build too (windows no wider than its GPU's texture side, at most 3072 px), and are not made while hovering a look or showing a diagnostic overlay, nor over a picture that is not the frame (an unsupported raw's embedded JPEG with another crop). No texture is larger than the GPU allows. Spots and Auto Mask brushes work while what a window reads fits one render (8192 px along an axis, 4096 in the browser): the window grows to hold the spot and its source patch, or the dabs near it, and if that would take more (a clone from far away at 8:1) the loupe keeps the whole-frame render instead, and spots with an automatic source pick it from a fixed-size render of the whole frame (so a window's choice can differ from a whole-frame render's, which picks at its own resolution). The wide stages (dehaze, clarity, highlights/shadows base) and Refine Edges on masks read a window margin capped at 768 px; measured on hard-edged blocks that costs at most a few levels at 8:1. the histogram and Navigator come from the whole-frame render. Compare and Reference get the same windows, one per pane (`Slot::CompareRegion`, `panels/compare.rs`), over each pane's whole-frame render (at most 2560 px); while either view is open two originals stay decoded (`MediaCache::set_full_capacity`), so one pane's window does not evict the other's source; a pinch, or a slider drag on the active photo in Reference, keeps the windows there are until it ends. Mask overlays and soft proofing get the same windows (the window carries the overlay or the proof; the proof renders on the CPU, so a tick at 100 % on a 1400 × 900 canvas takes about 36 ms instead of a full-size 24 MP proof). Verified: a 1:1 window of a 24 MP photo with tone and colour edits is the full-size export to within one level (`one_to_one_window_pixels_are_export_pixels`). Measured on Linux (Ryzen AI Max+ 395, Radeon 8060S, GPU, `profile_slider_drag`): median tick→shown 7 ms at fit, 33 ms at 100 % and 6 ms at 400 % on 1400 × 900; 21 ms at 100 % and 9 ms at 400 % on 2800 × 1700; 36 ms at 100 % with soft proof. Not windowed: the diagnostic overlays named above | | LR-VIEW-FOCUSPOINT | Camera focus point overlay and zoom (beyond Lightroom) | P2 | 🟡 | `cmd:view.focusPoint`, `cmd:view.zoomToFocus`, `cmd:view.zoomToggle`, `cmd:photo.reload`, `cmd:photo.readFocusPoints`, `cmd:focus.pump`, `crates/meta/src/focus.rs`, `crates/meta/src/focus_read.rs`, `crates/engine/src/focus_scan.rs`, `crates/engine/src/tests_focus_scan.rs`, `crates/ui-egui/src/panels/detail.rs` (`focus_box`, `draw_focus_point`), `crates/ui-egui/src/panels/compare.rs`, `crates/ui-egui/src/tests_focus_point.rs`, `docs/focus-points.md` | an extra (Lightroom shows no focus points): the AF frame read from the file at import; photos imported earlier get it in the background from a header-only read of each file (about 32 KB, ~100 photos/s from a spinning disk; activity row, cancellable, resumable; Photo ▸ Read Focus Points) as a box on the upright photo, drawn in Detail, Compare and Survey through rotation and crop (View ▸ Show Focus Point, ⌥F); Toggle Zoom centres on it (View ▸ Zoom to Focus Point, on by default); Info panel row `AF-C · Wide · Animal Eye Tracking`; `meta.focus` in `photo.inspect`. Partial: Sony ILCE-1 and ILCE-7SM3 only (layout checked against 333 files); other Sony bodies and other makers not yet; manual focus and centre-only Wide reports show nothing; Compare panes share one pan, so Z centres on the active photo's point only | | LR-VIEW-NAVIGATOR | Navigator mini map | P1 | ✅ | `cmd:view.navigator`, `crates/ui-egui/src/panels/detail.rs` | shown while zoomed (bottom right); click/drag pans | | LR-VIEW-BEFOREAFTER | Before / after | P0 | ✅ | `cmd:view.showOriginal`, `cmd:view.beforeAfter`, `cmd:view.beforeAfterSplit`, `cmd:view.beforeAfterTopBottom`, `cmd:view.beforeAfterSplitTopBottom`, `cmd:beforeAfter.setBefore`, `cmd:beforeAfter.copyAfterToBefore`, `cmd:beforeAfter.copyBeforeToAfter`, `cmd:beforeAfter.swap`, `cmd:beforeAfter.resetBefore` | all four layouts; before = the import state (defaults + import preset) or a chosen history step / version / the current settings; copy and swap (View → Before/After Settings, History row menu). The chosen before lasts for the session; in side by side and top / bottom, clicks with the white-balance selector, colour samplers and targeted adjustment work in the Before view too (`crates/ui-egui/src/tests_before_after.rs`). Zoomed in, the side by side and stacked views each stay in their pane, with their labels under it. Click zoom animates both panes together, including cached window tiles; input follows the displayed image and render requests use the final size (#1821) | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | LR-VIEW-ZOOM: a slow slider-drag verdict needs two drags over 20 ms and expires after 30 s (doubling to 240 s); hidden-window frame times are ignored; the slow set forgets its oldest entry, not all (issue #330); status 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 (✅) | From d770c5231377ccc1fd8c5c046665b6274ee8b3df Mon Sep 17 00:00:00 2001 From: Adrian D'souza <102761952+bogusyogi@users.noreply.github.com> Date: Thu, 15 Oct 2026 16:30:36 +0530 Subject: [PATCH 08/99] Masks: bound the total dabs of one brush stroke (#1520) Co-Authored-By: Claude Opus 5.5 --- crates/pipeline/src/masks.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/pipeline/src/masks.rs b/crates/pipeline/src/masks.rs index f2aac5881..6db0e2e52 100644 --- a/crates/pipeline/src/masks.rs +++ b/crates/pipeline/src/masks.rs @@ -484,6 +484,11 @@ pub fn push_pressures(pressure: &mut Vec, added: usize, from: f32, to: f32) /// pointer is held still (the same screen position mapped again, up to float rounding). const STATIONARY_PX: f64 = 1e-6; +/// Most dabs one stroke holds (32 MiB of centres). A segment is capped on its own, but a stroke of +/// many hostile segments (an agent, a crafted XMP or catalog) would otherwise add up without +/// bound; past this the rest of the stroke lays nothing (#1520). +pub const MAX_STROKE_DABS: usize = 2_000_000; + /// The dabs of stroke `s` in a `w × h` output. pub fn brush_dabs(s: &BrushStroke, frame: &Frame, w: usize, h: usize) -> BrushDabs { let to_out = frame.norm_to_out(w, h); @@ -537,9 +542,10 @@ pub fn dab_spacing(r: f64) -> f64 { /// their dabs with this, so they cover the same pixels (issue #517). A sample where the pointer /// hasn't moved (a stationary hold records one per frame) or that is not a finite point lays /// nothing, so holding the brush still adds no coverage (#518). Returns whether `q` was stamped; -/// callers move `prev` on only then. +/// callers move `prev` on only then. A stroke holding [`MAX_STROKE_DABS`] takes no more. pub fn push_dabs(dabs: &mut Vec, prev: Option, q: Point, r: f64) -> bool { - if !(q.x.is_finite() && q.y.is_finite()) { + let room = MAX_STROKE_DABS.saturating_sub(dabs.len()); + if room == 0 || !(q.x.is_finite() && q.y.is_finite()) { return false; } if let Some(prev) = prev { @@ -551,6 +557,8 @@ pub fn push_dabs(dabs: &mut Vec, prev: Option, q: Point, r: f64) - // a hostile or degenerate segment (huge) gets no in-between dabs rather than a huge // allocation: 1 M dabs is far more than any on-screen stroke needs let n = if n.is_finite() { n.clamp(0.0, 1_000_000.0) as usize } else { 0 }; + // the in-between dabs and `q` together fit what is left of the stroke's budget + let n = n.min(room); for k in 1..n { dabs.push(prev.lerp(q, k as f64 / n as f64)); } @@ -838,6 +846,17 @@ mod tests { } assert_eq!(committed.dabs, again); assert!(committed.dabs.windows(2).all(|w| w[0].dist(w[1]) <= dab_spacing(committed.r) + 1e-9)); + // many huge segments stop at the stroke budget instead of adding up without bound + let mut big = Vec::new(); + let mut at = Point::new(0.0, 0.0); + for i in 0..5 { + let q = Point::new(if i % 2 == 0 { 1e12 } else { 0.0 }, 0.0); + if push_dabs(&mut big, Some(at), q, 1.0) { + at = q; + } + } + assert!(big.len() <= MAX_STROKE_DABS, "{} dabs", big.len()); + assert!(!push_dabs(&mut big, Some(at), Point::new(5.0, 5.0), 1.0), "a full stroke takes no more"); // a hostile segment places no in-between dabs let mut v = Vec::new(); assert!(!push_dabs(&mut v, Some(Point::new(f64::NAN, 0.0)), Point::new(1.0, 1.0), r)); From b9b80aee8a499f11f71a28c1f16d27c69a82750d Mon Sep 17 00:00:00 2001 From: Matthew James Sivak Date: Thu, 15 Oct 2026 15:35:11 +0200 Subject: [PATCH 09/99] LR-BEHAV-BG-TASKS: Reload from Disk reads its files in the background photo.reload decoded every targeted file on the UI thread, so a selection of raws froze the window until all were read. The command is now split into plan / probe / apply (engine `cmd::reload`); the app intercepts photo.reload and probes the files on a worker thread with a cancellable activity row (kind `reload`, counted progress), applying the result as one undo step. `wait: true` keeps the old in-place behaviour for agents; CLI/MCP unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/convert.rs | 33 +++------- crates/engine/src/cmd/mod.rs | 1 + crates/engine/src/cmd/reload.rs | 93 +++++++++++++++++++++++++++++ crates/engine/src/import.rs | 2 +- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/menus.rs | 4 ++ crates/ui-egui/src/panels/edit.rs | 13 ++-- crates/ui-egui/src/panels/grid.rs | 5 +- crates/ui-egui/src/reload.rs | 67 +++++++++++++++++++++ crates/ui-egui/src/tasks.rs | 29 ++++++++- crates/ui-egui/src/tests_recheck.rs | 64 +++++++++++++++++++- docs/background-tasks.md | 1 + docs/parity-checklist.md | 5 +- 13 files changed, 283 insertions(+), 35 deletions(-) create mode 100644 crates/engine/src/cmd/reload.rs create mode 100644 crates/ui-egui/src/reload.rs diff --git a/crates/engine/src/cmd/convert.rs b/crates/engine/src/cmd/convert.rs index eae955cce..0871e4352 100644 --- a/crates/engine/src/cmd/convert.rs +++ b/crates/engine/src/cmd/convert.rs @@ -313,30 +313,15 @@ pub(crate) fn reload_ops( } /// Re-read photos whose files changed on disk (an external editor saved them): new size, -/// dimensions and content hash, cached sources dropped. → {reloaded: [ids]} +/// dimensions and content hash, cached sources dropped. The app runs the same steps +/// ([`crate::cmd::reload`]) with the files read on a worker thread. → {reloaded: [ids]} fn reload(s: &mut Session, p: &Value) -> Result { - let ids = s.targets(p)?; - let paths: Vec<(lightcraft_catalog::PhotoId, String)> = ids - .iter() - .filter_map(|id| match s.catalog.photo(*id).map(|ph| ph.source.clone()) { - Some(Source::File { path }) => Some((*id, path)), - _ => None, - }) - .collect(); - let probed = crate::import::probe_paths(s, &paths.iter().map(|(_, p)| p.clone()).collect::>()); - let mut ops = Vec::new(); - let mut reloaded = Vec::new(); - for ((id, _), info) in paths.into_iter().zip(probed) { - let Ok(info) = info else { continue }; - reload_ops(&s.catalog, id, &info, &mut ops, &mut reloaded); - } - for id in &reloaded { - s.media.forget(*id); - } - if !ops.is_empty() { - s.commit("Reload Changed Files", Op::Batch { ops })?; - } - Ok(json!({"reloaded": reloaded.iter().map(|i| i.0).collect::>()})) + use crate::cmd::reload as r; + let items = r::plan(s, p)?; + let probe = r::file_probe(s); + let results = r::probe(probe.as_ref(), &items, &std::sync::atomic::AtomicBool::new(false), &|_| {}); + let reloaded = r::apply(s, items, results)?; + Ok(r::to_json(&reloaded)) } /// Duplicate: copy the file next to itself (`-copy`, never overwriting) and add it with the @@ -401,7 +386,7 @@ pub fn edit_specs() -> Vec { "Reload from Disk", ["Photo"], None, - "{ids?} — re-read photos whose files changed on disk (e.g. saved by an external editor), or raws shown from their embedded preview that can be decoded now; camera fields the catalog lacks are filled in, and so is the lens data the file carries (an untouched photo then gets its lens correction turned on as import would; an edited one keeps its settings) → {reloaded}", + "{ids?} — re-read photos whose files changed on disk (e.g. saved by an external editor), or raws shown from their embedded preview that can be decoded now; camera fields the catalog lacks are filled in, and so is the lens data the file carries (an untouched photo then gets its lens correction turned on as import would; an edited one keeps its settings); in the app the files are read in the background (activity row `reload`, cancellable) and the reply is {background: true, count} unless `wait: true` → {reloaded}", has_selection, reload ), diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 4b86d4012..bf6733a7a 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -38,6 +38,7 @@ pub mod previews; mod print; mod query; pub mod recheck; +pub mod reload; mod slideshow; mod sync; mod xmp; diff --git a/crates/engine/src/cmd/reload.rs b/crates/engine/src/cmd/reload.rs new file mode 100644 index 000000000..9fc0a590f --- /dev/null +++ b/crates/engine/src/cmd/reload.rs @@ -0,0 +1,93 @@ +//! Reload from Disk (`photo.reload`), split so the app can read the files on a worker thread: a +//! raw re-decode takes a second or more per file, and a selection of them froze the window while +//! the command ran. [`plan`] lists the photos to re-read (no file-system calls), [`probe`] reads +//! their files without the session (cancellable, with progress), and [`apply`] commits what came +//! of it as one undo step. The command itself (agents, CLI, MCP) does all three at once. + +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + +use lightcraft_catalog::{Op, PhotoId, Source}; +use serde_json::{Value, json}; + +use crate::media::{FileProbe, ProbeInfo}; +use crate::{Result, Session}; + +/// A photo to re-read: its id and the file it had when it was listed (a photo relinked meanwhile +/// is skipped by [`apply`]). +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Item { + pub id: PhotoId, + pub path: String, +} + +/// The photos `p` targets (`ids`, else the selection) that have a file. No file-system calls. +pub fn plan(s: &Session, p: &Value) -> Result> { + Ok(s.targets(p)? + .into_iter() + .filter_map(|id| match s.catalog.photo(id).map(|ph| &ph.source) { + Some(Source::File { path }) => Some(Item { id, path: path.clone() }), + _ => None, + }) + .collect()) +} + +/// The session's file probe, to [`probe`] with on a worker thread (`None` in a session without file +/// access, where a reload reads nothing but the file name, as the command does). +pub fn file_probe(s: &Session) -> Option { + s.media.file_probe.clone() +} + +/// Read each listed photo's file as Reload does, several at once. Doesn't touch the session. +/// `progress(done)` is called while it runs (from this thread); once `cancel` is set no further +/// file is started, and the files not reached come back as errors (left unchanged by [`apply`]). +pub fn probe( + probe: Option<&FileProbe>, + items: &[Item], + cancel: &AtomicBool, + progress: &dyn Fn(usize), +) -> Vec> { + let paths: Vec = items.iter().map(|i| i.path.clone()).collect(); + let done = AtomicUsize::new(0); + #[cfg(not(target_arch = "wasm32"))] + let r = std::thread::scope(|sc| { + let h = sc.spawn(|| crate::import::probe_all(probe, &paths, &done, cancel)); + while !h.is_finished() { + progress(done.load(Ordering::Relaxed)); + std::thread::sleep(std::time::Duration::from_millis(50)); + } + // a probe that panicked (already logged): nothing is known about any file + h.join().unwrap_or_else(|_| paths.iter().map(|p| Err(format!("{p}: could not be read"))).collect()) + }); + #[cfg(target_arch = "wasm32")] + let r = crate::import::probe_all(probe, &paths, &done, cancel); + progress(done.load(Ordering::Relaxed)); + r +} + +/// Commit [`probe`]'s results as one undo step ("Reload Changed Files"): each photo and its virtual +/// copies get what their file is now. A file that couldn't be read changes nothing; a photo removed +/// or relinked since it was listed is skipped. Returns the photos that changed. +pub fn apply(s: &mut Session, items: Vec, results: Vec>) -> Result> { + let mut ops = Vec::new(); + let mut reloaded = Vec::new(); + for (item, info) in items.into_iter().zip(results) { + let Ok(info) = info else { continue }; + let current = s.catalog.photo(item.id).is_some_and(|ph| matches!(&ph.source, Source::File { path } if *path == item.path)); + if !current { + continue; + } + crate::cmd::convert::reload_ops(&s.catalog, item.id, &info, &mut ops, &mut reloaded); + } + for id in &reloaded { + s.media.forget(*id); + } + if !ops.is_empty() { + s.commit("Reload Changed Files", Op::Batch { ops })?; + } + Ok(reloaded) +} + +/// `{reloaded: [ids]}`. +pub fn to_json(reloaded: &[PhotoId]) -> Value { + json!({"reloaded": reloaded.iter().map(|i| i.0).collect::>()}) +} diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 9aa847b01..b98dcd591 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -494,7 +494,7 @@ pub(crate) fn probe_with(probe: &crate::media::FileProbe, paths: &[String]) -> V /// Probe `paths` (several at once), counting each in `done`. Stops before the next file once /// `cancel` is set: the files not reached come back as errors. -fn probe_all( +pub(crate) fn probe_all( probe: Option<&crate::media::FileProbe>, paths: &[String], done: &std::sync::atomic::AtomicUsize, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index bb290bd86..080996420 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -33,6 +33,7 @@ pub mod pick; pub mod print_view; pub mod recheck; pub mod region; +pub mod reload; pub mod render; pub mod shortcuts; pub mod sidebar; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9fcfc8b9d..7918d4f8a 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -417,6 +417,10 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option app.toast_error(ctx, e), - Ok(_) if decoded => app.toast(ctx, crate::i18n::tr("This raw is now developed from its raw data")), + Ok(r) if r["background"] == true => {} + // nothing to read in the background (a photo without a file): answered in place + Ok(_) if app.session.catalog.photo(id).is_some_and(|p| p.preview_only.is_none()) => { + app.toast(ctx, crate::i18n::tr("This raw is now developed from its raw data")) + } Ok(_) => app.toast(ctx, crate::i18n::tr("This raw variant still can't be decoded")), } } diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index c6365b784..10d12ccb6 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -1162,7 +1162,10 @@ pub fn context_menu(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } } if ui.button(crate::i18n::tr("Reload from Disk")).clicked() { - let _ = app.run("photo.reload", json!({})); + // the files are read in the background (activity row `reload`) + if let Err(e) = app.run("photo.reload", json!({})) { + app.toast_error(ui.ctx(), e); + } } if ui.button(crate::i18n::tr("Create Virtual Copy")).clicked() { let _ = app.run("photo.virtualCopy", json!({})); diff --git a/crates/ui-egui/src/reload.rs b/crates/ui-egui/src/reload.rs new file mode 100644 index 000000000..32823cf16 --- /dev/null +++ b/crates/ui-egui/src/reload.rs @@ -0,0 +1,67 @@ +//! Reload from Disk in the app: the files are read on a worker thread (a raw's decode takes a +//! second or more, and a selection of them froze the window), with a row in the activity stack +//! that counts the files and can stop. What came of it is applied back here, as one undo step, +//! unless another library was opened meanwhile. Agents get the same with `wait: true`, which runs +//! the engine's command in place. + +use std::sync::Arc; +use std::sync::atomic::AtomicBool; + +use lightcraft_engine::cmd::reload; +use serde_json::{Value, json}; + +use crate::LightcraftApp; + +/// The activity stack's label while the files are read (the menu item's name, already translated). +pub const LABEL: &str = "Reload from Disk"; + +/// `photo.reload` from the app: start reading the targeted files in the background → +/// `{background: true, count}`. `None` hands the command to the engine: with `wait: true`, or when +/// no targeted photo has a file (it then answers at once, refusing an empty selection as usual). +pub fn command(app: &mut LightcraftApp, p: &Value) -> Option> { + if p.get("wait").and_then(Value::as_bool) == Some(true) { + return None; + } + let items = match reload::plan(&app.session, p) { + Ok(items) => items, + Err(e) => return Some(Err(format!("photo.reload: {e}"))), + }; + if items.is_empty() { + return None; + } + let count = items.len(); + // the photos shown from their embedded preview, to say afterwards whether they decode now + let preview_only: Vec<_> = + items.iter().filter(|i| app.session.catalog.photo(i.id).is_some_and(|ph| ph.preview_only.is_some())).map(|i| i.id).collect(); + let library = app.session.library_generation(); + let probe = reload::file_probe(&app.session); + let cancel = Arc::new(AtomicBool::new(false)); + let flag = cancel.clone(); + let work = move |task: lightcraft_engine::activity::TaskHandle| { + let total = items.len() as u64; + task.progress(0, total); + let results = reload::probe(probe.as_ref(), &items, &flag, &|done| task.progress(done as u64, total)); + (items, results) + }; + let done = move |app: &mut LightcraftApp, ctx: &egui::Context, (items, results)| { + if app.session.library_generation() != library { + return; + } + if let Err(e) = reload::apply(&mut app.session, items, results) { + return app.toast_error(ctx, e.to_string()); + } + let cleared = preview_only.iter().filter(|id| app.session.catalog.photo(**id).is_some_and(|ph| ph.preview_only.is_none())).count(); + let message = match (count, preview_only.len(), cleared) { + (1, 1, 1) => crate::i18n::tr("This raw is now developed from its raw data").to_string(), + (1, 1, _) => crate::i18n::tr("This raw variant still can't be decoded").to_string(), + (_, _, 0) => return, + (_, _, 1) => crate::i18n::tr("1 raw can now be developed from its raw data").to_string(), + (_, _, n) => crate::i18n::tr_format!("{n} raws can now be developed from their raw data", n = n), + }; + app.toast(ctx, message); + }; + if let Err(e) = crate::tasks::spawn_tracked(app, LABEL, "reload", cancel, work, done) { + return Some(Err(e)); + } + Some(Ok(json!({"background": true, "count": count}))) +} diff --git a/crates/ui-egui/src/tasks.rs b/crates/ui-egui/src/tasks.rs index 70abd629a..86dc79381 100644 --- a/crates/ui-egui/src/tasks.rs +++ b/crates/ui-egui/src/tasks.rs @@ -50,6 +50,34 @@ pub fn spawn( kind: Option<&'static str>, work: impl FnOnce() -> T + Send + 'static, done: impl FnOnce(&mut LightcraftApp, &egui::Context, T) + Send + 'static, +) -> Result<(), String> { + let guard = kind.map(|k| app.session.activity.start(k, label, lightcraft_engine::activity::Cancel::No)); + start(app, label, guard, work, done) +} + +/// [`spawn`] for a task with progress that can stop: its row (`kind`, `label`) adopts `cancel`, +/// the flag `work` checks, so the row's ✕ and `activity.cancel` stop it; `work` reports progress +/// through the handle it is given. +pub fn spawn_tracked( + app: &mut LightcraftApp, + label: &str, + kind: &'static str, + cancel: std::sync::Arc, + work: impl FnOnce(lightcraft_engine::activity::TaskHandle) -> T + Send + 'static, + done: impl FnOnce(&mut LightcraftApp, &egui::Context, T) + Send + 'static, +) -> Result<(), String> { + let guard = app.session.activity.start(kind, label, lightcraft_engine::activity::Cancel::Flag(cancel)); + let handle = guard.handle(); + start(app, label, Some(guard), move || work(handle), done) +} + +/// Run `work` on a worker thread and list it in `app.tasks` with its activity row (if any). +fn start( + app: &mut LightcraftApp, + label: &str, + guard: Option, + work: impl FnOnce() -> T + Send + 'static, + done: impl FnOnce(&mut LightcraftApp, &egui::Context, T) + Send + 'static, ) -> Result<(), String> { let (tx, rx) = channel::(); let repaint = app.tasks.repaint.clone(); @@ -64,7 +92,6 @@ pub fn spawn( std::thread::Builder::new().name(format!("lc-task-{label}")).spawn(job).map_err(|e| format!("{label}: could not start: {e}"))?; #[cfg(target_arch = "wasm32")] job(); - let guard = kind.map(|k| app.session.activity.start(k, label, lightcraft_engine::activity::Cancel::No)); app.tasks.running.push(Task { label: label.to_string(), rx, _guard: guard }); Ok(()) } diff --git a/crates/ui-egui/src/tests_recheck.rs b/crates/ui-egui/src/tests_recheck.rs index 88149e464..c343050fc 100644 --- a/crates/ui-egui/src/tests_recheck.rs +++ b/crates/ui-egui/src/tests_recheck.rs @@ -78,13 +78,75 @@ fn recheck_result_is_dropped_after_switching_libraries() { assert!(app.session.catalog.photo(id).is_none(), "the other library is untouched"); } -/// The notice's Reload from Disk re-reads the file and says what came of it. +/// The notice's Reload from Disk re-reads the file in the background (a row in the activity stack, +/// the window not waiting for the decode) and says what came of it. #[test] fn notice_reload_button_rereads_the_file() { let dir = Dir(std::env::temp_dir().join(format!("lc-ui-recheck-reload-{}", std::process::id()))); let (mut app, id) = app_with_stale_photo(&dir.0); let ctx = egui::Context::default(); crate::panels::edit::reload_preview_only(&mut app, &ctx, id); + let rows = app.session.activity.list(); + let row = rows.iter().find(|r| r.kind == "reload").unwrap_or_else(|| panic!("no reload row: {rows:?}")); + assert_eq!(row.label, crate::reload::LABEL); + assert!(row.cancellable); + assert!(crate::tasks::wait(&mut app, &ctx, Duration::from_secs(10))); + assert!(app.session.activity.list().iter().all(|r| r.kind != "reload"), "the row goes with the task"); assert_eq!(app.session.catalog.photo(id).unwrap().preview_only, None); assert!(app.ui.toast.as_ref().is_some_and(|(t, _, _)| t == "This raw is now developed from its raw data"), "{:?}", app.ui.toast); + // one undo step + assert_eq!(app.session.undo.last().map(|u| u.label.as_str()), Some("Reload Changed Files")); +} + +/// Reload from Disk on a selection returns at once and reads the files on a worker thread; `wait` +/// runs it in place; a cancelled reload changes nothing it didn't read. +#[test] +fn reload_of_a_selection_runs_in_the_background() { + let dir = Dir(std::env::temp_dir().join(format!("lc-ui-reload-sel-{}", std::process::id()))); + let (mut app, id) = app_with_stale_photo(&dir.0); + let ctx = egui::Context::default(); + app.session.selection = lightcraft_engine::Selection { ids: vec![id], active: Some(id) }; + let r = app.run("photo.reload", json!({})).unwrap(); + assert_eq!(r, json!({"background": true, "count": 1})); + assert!(app.tasks.is_running(crate::reload::LABEL)); + assert!(app.session.activity.list().iter().any(|r| r.kind == "reload")); + assert!(crate::tasks::wait(&mut app, &ctx, Duration::from_secs(10))); + assert_eq!(app.session.catalog.photo(id).unwrap().preview_only, None); + + // agents can wait for the result: the engine's reply, nothing left running + let r = app.run("photo.reload", json!({"ids": [id.0], "wait": true})).unwrap(); + assert!(r.get("reloaded").is_some(), "{r}"); + assert!(app.tasks.is_empty()); + + // cancelled before any file is read: nothing changes + let p = app.session.catalog.photo(id).unwrap().clone(); + app.session + .catalog + .apply(lightcraft_catalog::Op::SetContent { + id, + width: p.width, + height: p.height, + file_size: p.file_size, + content_hash: p.content_hash.clone(), + preview_only: Some("Cr3 files are not decoded yet".into()), + preview_checked: "0.5.0".into(), + }) + .unwrap(); + let items = lightcraft_engine::cmd::reload::plan(&app.session, &json!({"ids": [id.0]})).unwrap(); + let probe = lightcraft_engine::cmd::reload::file_probe(&app.session); + let cancelled = std::sync::atomic::AtomicBool::new(true); + let results = lightcraft_engine::cmd::reload::probe(probe.as_ref(), &items, &cancelled, &|_| {}); + assert!(lightcraft_engine::cmd::reload::apply(&mut app.session, items, results).unwrap().is_empty()); + assert!(app.session.catalog.photo(id).unwrap().preview_only.is_some()); +} + +/// With no photo selected the command is refused at once, as before (no task, no row). +#[test] +fn reload_with_nothing_selected_starts_nothing() { + let dir = Dir(std::env::temp_dir().join(format!("lc-ui-reload-none-{}", std::process::id()))); + let (mut app, _) = app_with_stale_photo(&dir.0); + app.session.selection = lightcraft_engine::Selection::default(); + let _ = app.run("photo.reload", json!({})); + assert!(app.tasks.is_empty()); + assert!(app.session.activity.list().iter().all(|r| r.kind != "reload")); } diff --git a/docs/background-tasks.md b/docs/background-tasks.md index 8ef8789d8..58fd3d6c5 100644 --- a/docs/background-tasks.md +++ b/docs/background-tasks.md @@ -60,6 +60,7 @@ guard.detail(&file_name); | Lightroom catalog import | `crates/ui-egui/src/lightroom_import.rs` | stop being cancellable for a phase that must finish | | Face scan, AI Denoise | `crates/engine/src/cmd/face_recognize.rs`, `crates/engine/src/denoise.rs` | a continuous job with no ✕, kept in session state | | Find Missing Photos | `crates/ui-egui/src/tasks.rs` | a generic background task with a row (`kind: Some(..)`) | +| Reload from Disk | `crates/ui-egui/src/reload.rs`, `crates/engine/src/cmd/reload.rs` | a command split into plan / probe / apply so the app reads the files on a worker thread (`tasks::spawn_tracked`: progress and `Cancel::Flag`); `wait: true` runs it in place | | Reading focus points | `crates/engine/src/focus_scan.rs` | a worker paced by the UI's per-frame pump (`focus.pump {pace}`); `Cancel::Flag`; commits in batches without undo steps | ## Testing diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 95dcd2aef..4473ed8e8 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-15 · **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 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 @@ -461,7 +461,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | | | LR-BEHAV-PREVIEW-HOVER | Hover previews | P1 | ✅ | `crates/ui-egui/src/panels/presets.rs`, `crates/ui-egui/src/panels/profiles.rs`, `crates/ui-egui/src/panels/right.rs` (`versions`), `crates/ui-egui/src/panels/detail.rs` | presets, profile menu, profile browser and versions preview in the loupe | | LR-BEHAV-PROGRESSIVE | Progressive rendering | P0 | ✅ | `crates/engine/src/media.rs`, `crates/preview/src/lib.rs` | | -| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `cmd:activity.list`, `cmd:activity.cancel`, `crates/engine/src/activity.rs`, `crates/ui-egui/src/panels/activity.rs`, `docs/background-tasks.md`, `crates/preview/src/lib.rs` (`JobPool`) | renders off the UI thread; exports run several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); one activity stack in the top-left corner under the top bar, as in Lightroom Classic (issue #345): imports, folder scans, exports (and the contact sheet PDF), Synchronize Folder (its scan and its run), Build / Smart Previews, Lightroom catalog inspect / import, Photo Merge, model downloads (SAM 3, face, AI Denoise), the face scan, AI Denoise and Find Missing Photos each show a row (name, progress bar or a sweep while the amount isn't known, count and current item) once 0.5 s old, three rows then "+N more"; ✕ cancels the jobs that can stop ("Stopping…" until they do); quitting asks while a cancellable task runs; agents and `ui.inspect` (`activity`) see the same list. No pause (Lightroom can pause some tasks) | +| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `cmd:activity.list`, `cmd:activity.cancel`, `crates/engine/src/activity.rs`, `crates/ui-egui/src/panels/activity.rs`, `docs/background-tasks.md`, `crates/preview/src/lib.rs` (`JobPool`) | renders off the UI thread; exports run several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); one activity stack in the top-left corner under the top bar, as in Lightroom Classic (issue #345): imports, folder scans, exports (and the contact sheet PDF), Synchronize Folder (its scan and its run), Build / Smart Previews, Lightroom catalog inspect / import, Photo Merge, model downloads (SAM 3, face, AI Denoise), the face scan, AI Denoise, Find Missing Photos and Reload from Disk (`cmd:photo.reload`, its files decoded on a worker thread, cancellable; `wait: true` for agents) each show a row (name, progress bar or a sweep while the amount isn't known, count and current item) once 0.5 s old, three rows then "+N more"; ✕ cancels the jobs that can stop ("Stopping…" until they do); quitting asks while a cancellable task runs; agents and `ui.inspect` (`activity`) see the same list. No pause (Lightroom can pause some tasks) | | LR-BEHAV-OFFLINE | Offline editing | P1 | ✅ | | local-first: everything works offline | | LR-BEHAV-WEB-SAFETY | Library safety in the browser build | P1 | 🟡 | `cmd:file.backupLibrary`, `cmd:file.restoreLibrary`, `apps/lightcraft-web/src/safety.rs`, `apps/lightcraft-web/src/backup.rs`, `apps/lightcraft-web/src/files.rs` | web only (experimental): Back Up Library (zip of the catalog and every stored original) and a non-destructive Restore; failed saves show as unsaved and are retried; a photo that can't be stored isn't added; one tab per library (Web Locks); notices for storage that isn't persistent and for a library that can't open; `?reset` asks first; a panic shows a message. Missing: `?safe` start, deleting originals of removed photos, zip64 (backups > 4 GB) | | LR-BEHAV-GPU | GPU acceleration | P0 | ✅ | `crates/gpu/src/render.rs`, `cmd:app.gpu`, `docs/gpu-pipeline.md`, `crates/ui-egui/src/panels/notices.rs` | CPU fallback per render on device limits, out of memory, device errors / loss, incomplete or black results (`perf.gpuReason`, `perf.gpuFallback`; each render reports its own reason — `lightcraft_gpu::try_render`, `gpuFallback` per exported file — issue #675); short submissions for slow iGPUs; backend choice (DX12 only on Windows, `LIGHTCRAFT_GPU_BACKEND` / `WGPU_BACKEND`), device created after the window opens, crash sentinel (issue #136); a GPU lost while LightCraft runs (driver reset) stops it with that message and turns GPU rendering off for the next start, which says why (issue #1337; the window's renderer can't survive the lost device); quitting waits (up to 2 s) for renders in flight and starts no new GPU work, so none is inside the driver when the process exits (`crates/gpu/src/exit.rs`, issue #620; seen crashing only on Linux + NVIDIA, other drivers unverified); with GPU rendering on and no adapter found at all (e.g. no Vulkan loader on NixOS outside `nix develop`), the desktop app says once per session that it renders on the CPU and points to Help ▸ System Info (`gpu_notice`; not when the GPU is turned off or only a software adapter exists); no tiling beyond the buffer limit (CPU) | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | LR-BEHAV-BG-TASKS: Reload from Disk reads its files on a worker thread with a cancellable activity row instead of freezing the window on a selection of raws; status 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 (✅) | From c806debada90b2e89199271794515cb49f55ab21 Mon Sep 17 00:00:00 2001 From: Pierre-Edouard Honnet Date: Thu, 15 Oct 2026 20:12:46 +0200 Subject: [PATCH 10/99] LR-IMP-FORMATS: CRX high-frequency band width from the tile geometry (#899) The decoder tried each high-frequency band at its synthesis width and up to two more columns, keeping the first width whose entropy stream ended exactly. Two widths can both do that with different coefficients (#899). Measured on every C-RAW plane in cr3_corpus.rs (CC0 raw.pixls.us files): each band decodes at exactly one width. A tile with a neighbour on its right codes bands 5 and 8 (the vertical high-pass of the level-2 and level-1 low-pass columns) at an odd width, so an even band gains one unused column; no other band is extended. The width now comes from that rule, and a stream that does not fit it is corrupt (the CR3 falls back to its preview). All corpus sensor hashes are unchanged. The property-test encoder now codes bands 5 and 8 the same way. Middle tiles (three or more C-RAW tiles) follow the left-tile rule; no public file has one. Co-Authored-By: Claude Opus 5.5 --- crates/raw/src/vendor/crx.rs | 110 ++++++++++++++++++++++------- crates/raw/tests/crx_properties.rs | 14 +++- docs/cr3.md | 12 ++++ 3 files changed, 109 insertions(+), 27 deletions(-) diff --git a/crates/raw/src/vendor/crx.rs b/crates/raw/src/vendor/crx.rs index 853efdffe..13f63a4a3 100644 --- a/crates/raw/src/vendor/crx.rs +++ b/crates/raw/src/vendor/crx.rs @@ -366,6 +366,39 @@ fn split_dimension(length: usize, before: bool, after: bool) -> (usize, usize) { (length.div_ceil(2) + usize::from(after && length.is_multiple_of(2)), length / 2 + usize::from(before) + usize::from(after)) } +/// The width at which high-frequency band `index` (1–9, coarsest level first) is entropy coded, +/// given its synthesis width `width`. +/// +/// A tile with a neighbour on its right codes the vertical high-pass band of the level-1 and level-2 +/// low-pass columns (bands 8 and 5, whose low-pass columns the next level transforms again) at an odd +/// width: an even synthesis width gains one column, which synthesis does not use. Every other band, +/// including band 2 of the last level, is coded at its synthesis width. Measured on every C-RAW plane +/// that `tests/cr3_corpus.rs` decodes (CC0 raw.pixls.us files): two-tile files with tile widths 2816, +/// 3144, 3192, 3444 and 3564 (G5 X Mark II, M50, R100, 250D, RP, R, 90D, M6 Mark II), where bands 5 +/// and 8 gain a column in left tiles exactly when the rule says so, and single-tile `0x100` and +/// `0x200` files, which never extend a band. In every one of those planes, each band's entropy stream +/// ends exactly at that width and at no other width from `width` to `width + 2`. The width used to be +/// probed by trying `width`, `width + 1` and `width + 2` in turn, but two widths can both consume a +/// stream exactly and give different coefficients (#899), so it is now taken from the geometry. +/// No public file has a middle tile (three or more C-RAW tiles); it is assumed to follow the left tile. +fn coded_band_width(index: usize, width: usize, edges: TileEdges) -> usize { + if edges.right && matches!(index, 5 | 8) { width | 1 } else { width } +} + +/// A high-frequency band entropy coded `coded_width` columns wide, cropped to its `width` synthesis columns. +fn coded_high_frequency_band(src: &[u8], width: usize, coded_width: usize, height: usize) -> Result> { + let values = high_frequency_band(src, coded_width, height)?; + if coded_width == width { + return Ok(values); + } + let mut cropped = Vec::new(); + cropped.try_reserve_exact(width.saturating_mul(height)).map_err(|_| RawError::Limit("CRX cropped band allocation"))?; + for row in values.chunks_exact(coded_width) { + cropped.extend_from_slice(row.get(..width).ok_or_else(|| corrupt("band crop outside row"))?); + } + Ok(cropped) +} + fn wavelet_plane(sample: &[u8], plane: &Plane, width: usize, height: usize, edges: TileEdges, qp: Option<&QpMap>) -> Result> { let mut dimensions = vec![(width, height)]; let mut high_dimensions = Vec::new(); @@ -383,32 +416,7 @@ fn wavelet_plane(sample: &[u8], plane: &Plane, width: usize, height: usize, edge let mut bands = Vec::with_capacity(10); for (i, (&(w, h), band)) in shapes.iter().zip(&plane.bands).enumerate() { let bytes = band_bytes(sample, band)?; - let mut data = if i == 0 { - lossless_plane(bytes, w, h)? - } else { - // Some CRX high-frequency bands retain a redundant right column - // beyond synthesis support (LH2 in the M50/R100 fixtures). Infer - // that small extension only from a complete, correctly aligned - // entropy stream, then crop each row to the synthesis support. - let mut decoded = None; - for extra in 0..=2 { - let coded_width = w.checked_add(extra).ok_or(RawError::Limit("CRX band width"))?; - if let Ok(values) = high_frequency_band(bytes, coded_width, h) { - if extra == 0 { - decoded = Some(values); - } else { - let mut cropped = Vec::new(); - cropped.try_reserve_exact(w * h).map_err(|_| RawError::Limit("CRX cropped band allocation"))?; - for row in values.chunks_exact(coded_width) { - cropped.extend_from_slice(row.get(..w).ok_or_else(|| corrupt("band crop outside row"))?); - } - decoded = Some(cropped); - } - break; - } - } - decoded.ok_or_else(|| RawError::Corrupt(format!("Canon CRX band {i}: unsupported geometry or corrupt entropy")))? - }; + let mut data = if i == 0 { lossless_plane(bytes, w, h)? } else { coded_high_frequency_band(bytes, w, coded_band_width(i, w, edges), h)? }; for (position, value) in data.iter_mut().enumerate() { let step = match band.quant { Quantization::Uniform(value) => quant_step(value, MAX_UNIFORM_QUANT)?, @@ -951,6 +959,56 @@ mod tests { } } + #[test] + fn coded_band_width_follows_the_measured_tile_geometry() { + let left_tile = TileEdges { right: true, ..TileEdges::default() }; + let right_tile = TileEdges { left: true, ..TileEdges::default() }; + let single = TileEdges::default(); + // Bands 5 and 8 of a left tile: an even width gains a column (RP 400 -> 401, R 862 -> 863, + // 90D / M6 Mark II 892 -> 893), an odd one does not (90D / M6 Mark II band 5 at 447). + for (band, width, coded) in [(5, 394, 395), (5, 400, 401), (5, 432, 433), (5, 447, 447), (8, 862, 863), (8, 892, 893), (8, 705, 705)] { + assert_eq!(coded_band_width(band, width, left_tile), coded, "band {band} at {width}"); + } + // The last level's low-pass columns are not transformed again: band 2 keeps an even width. + assert_eq!(coded_band_width(2, 198, left_tile), 198); + for band in [1, 3, 4, 6, 7, 9] { + assert_eq!(coded_band_width(band, 394, left_tile), 394, "band {band}"); + } + for band in 1..=9 { + assert_eq!(coded_band_width(band, 446, right_tile), 446, "band {band}"); + assert_eq!(coded_band_width(band, 446, single), 446, "band {band}"); + } + } + + #[test] + fn a_band_that_decodes_at_two_widths_uses_the_coded_width() { + // #899: entropy alignment alone does not identify the coded width. This 2-row stream is + // consumed exactly both 2 and 3 columns wide, with different retained coefficients. + let src = [0xc0, 0x0f]; + assert_eq!(high_frequency_band(&src, 2, 2).unwrap(), [0, 0, 5, -2]); + assert_eq!(high_frequency_band(&src, 3, 2).unwrap(), [0, 0, 5, 0, 0, 0]); + // The width probe took the first width that decoded (2). Band 8 of a left tile at an even + // width is coded one column wider, so the geometry selects 3 and crops it. + let coded = coded_band_width(8, 2, TileEdges { right: true, ..TileEdges::default() }); + assert_eq!(coded, 3); + assert_eq!(coded_high_frequency_band(&src, 2, coded, 2).unwrap(), [0, 0, 0, 0]); + // Elsewhere the same stream is read at its synthesis width. + assert_eq!(coded_high_frequency_band(&src, 2, coded_band_width(7, 2, TileEdges::default()), 2).unwrap(), [0, 0, 5, -2]); + // The #899 reproducer: an odd width is never extended, so the narrow reading is kept. + let src = [0x00, 0x33]; + assert_eq!(coded_band_width(5, 3, TileEdges { right: true, ..TileEdges::default() }), 3); + assert_eq!(coded_high_frequency_band(&src, 3, 3, 1).unwrap(), [5, 0, -1]); + assert_eq!(high_frequency_band(&src, 4, 1).unwrap(), [5, 0, 0, 0]); + } + + #[test] + fn a_band_that_fails_at_its_coded_width_is_an_error() { + // No fallback to another width: a stream that only fits 2 columns is corrupt where 3 are coded. + let src = [0xc0, 0x0f]; + assert!(high_frequency_band(&src, 4, 2).is_err()); + assert!(coded_high_frequency_band(&src, 3, 4, 2).is_err()); + } + #[test] fn craw_overshoot_at_clipped_highlights_is_clamped_to_the_bit_depth() { // As `three_level_craw_reconstructs_sensor_samples`, but each LL3 coefficient is +8200 diff --git a/crates/raw/tests/crx_properties.rs b/crates/raw/tests/crx_properties.rs index 32c9f65d3..804970de4 100644 --- a/crates/raw/tests/crx_properties.rs +++ b/crates/raw/tests/crx_properties.rs @@ -250,6 +250,7 @@ fn encode_qp_map(values: &[i32], width: usize, height: usize) -> Vec { // --- exact algebraic inverse of the crate's 5/3 synthesis (the forward transform under test) --- +#[derive(Clone)] struct Band { width: usize, height: usize, @@ -345,6 +346,17 @@ fn band_shapes(plane_width: usize, plane_height: usize, before: bool, after: boo [(lw3, lh3), (hw3, lh3), (lw3, hh3), (hw3, hh3), (hw2, lh2), (lw2, hh2), (hw2, hh2), (hw1, lh1), (lw1, hh1), (hw1, hh1)] } +/// The band as the camera codes it (crx.rs `coded_band_width`): with a tile on the right, bands 5 and 8 +/// are coded at an odd width, so an even band gains a column that synthesis ignores (zero here). +fn coded_band(band: &Band, index: usize, after: bool) -> Band { + if !(after && matches!(index, 5 | 8) && band.width.is_multiple_of(2)) { + return band.clone(); + } + let width = band.width + 1; + let data = band.data.chunks_exact(band.width).flat_map(|row| row.iter().copied().chain([0])).collect(); + Band { width, height: band.height, data } +} + fn encode_band(band: &Band, lowpass: bool) -> Vec { let mut bits = BitWriter::default(); if lowpass { @@ -568,7 +580,7 @@ fn encode_tile_planes( *plane_bytes = vec![encode_band(&band, true)]; } else { let bands = forward_bands(values, plane_w, plane_h, before, after, halo); - *plane_bytes = bands.iter().enumerate().map(|(i, band)| encode_band(band, i == 0)).collect(); + *plane_bytes = bands.iter().enumerate().map(|(i, band)| encode_band(&coded_band(band, i, after), i == 0)).collect(); } } encoded diff --git a/docs/cr3.md b/docs/cr3.md index 79babaea4..b133dfe6a 100644 --- a/docs/cr3.md +++ b/docs/cr3.md @@ -127,6 +127,18 @@ tiles. All ten sensor comparisons pass. R8 HEVC preview tracks are distinguished the primary Bayer RAW; the HEVC preview itself is not decoded. Missing corpus files are reported as skipped by the tests. +In a tile with a neighbour on its right, the vertical high-pass bands of the level-1 and level-2 +low-pass columns (bands 8 and 5) are entropy coded at an odd width: an even band carries one extra +column that synthesis does not use. No other band is extended, including the last level's (band 2). +The decoder takes each band's coded width from this rule. It used to try the synthesis width and up to +two more columns and keep the first that decoded to the exact end of the stream, but two widths can +both do that and give different coefficients (#899). The rule was measured on every C-RAW plane in +`cr3_corpus.rs`: two-tile files at tile widths 2816, 3144, 3192, 3444 and 3564 (G5 X Mark II, M50, +R100, 250D, RP, R, 90D, M6 Mark II), whose left tiles extend bands 5 and 8 exactly where the rule says, +and single-tile `0x100` and `0x200` files, which never extend a band. In every plane, each band decodes +at its coded width and at no other width up to two columns wider, so all sensor hashes are unchanged. +No public file has three or more C-RAW tiles; a middle tile follows the same rule, which is unverified. + The version `0x200` QP predictor and entropy framing were checked independently. Controlled input mutations establish floor averaging of paired QP rows before the nonlinear integer table, then multiplication, division by eight, addition From 5bf0d45ba87be9e99cfaaf14dd1a1b41d04e14da Mon Sep 17 00:00:00 2001 From: fanyzp Date: Fri, 16 Oct 2026 02:20:40 +0800 Subject: [PATCH 11/99] Do not read a sibling's shared stem sidecar on import (#1905) With stem naming, `IMG_0001.xmp` is the sidecar of every file of that stem and, in a folder Lightroom has worked on, it is the raw's. On import the packet was looked up with the library naming alone, so a TIFF or JPEG arriving beside a raw of the same name read the raw's packet and took its rating, label, keywords and `crs:` develop settings (the case radialmonster reported in #92; the read side of a catalogued photo is #1523). `ImportStems` ranks the files an import is adding as if they were catalogued already, in the order `StemOwners` uses for writing: the library's files and the arriving ones together, a raw first, then the file name in lower case. A file that does not own the stem sidecar reads only its own `.xmp`, never the sibling's packet. A file that is not catalogued yet has no id, and the id it draws when the import commits is above every id in the library, so it loses a tie against a catalogued file; between arriving files the order they arrive in stands for it. Folder synchronize and the Lightroom catalog import go through the same function, so a file arriving there is ranked the same way. --- crates/engine/src/import.rs | 21 +++- crates/engine/src/sidecar.rs | 101 ++++++++++++++++-- .../engine/src/sidecar/tests_stem_owners.rs | 78 +++++++++++++- crates/engine/src/tests_import.rs | 91 ++++++++++++++++ docs/parity-checklist.md | 5 +- docs/xmp-interop.md | 6 +- 6 files changed, 283 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 9aa847b01..f3c7dc9c0 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -965,9 +965,11 @@ impl ImportJob { } /// What the library held when the import was set up, by path and by content: made once, with - /// the first files (on the thread that prepares them), from the library as it was then. - fn look_up_library(&mut self) { - let Some((library, fresh)) = self.library.take() else { return }; + /// the first files (on the thread that prepares them), from the library as it was then. The + /// snapshot comes back: the files the import is adding are ranked against it for the ownership + /// of the shared stem sidecars they read ([`crate::sidecar::ImportStems`]). + fn look_up_library(&mut self) -> Option { + let (library, fresh) = self.library.take()?; for p in library.photos() { // a trashed photo is replaced by a fresh import, so it doesn't count as a duplicate if p.deleted && fresh { @@ -980,12 +982,14 @@ impl ImportJob { self.by_hash.insert(h.clone(), Some(p.id)); } } + Some(library) } /// [`ImportJob::prepare`] for files [`ImportJob::expand`] returned. pub fn prepare_files(&mut self, files: Vec, cancel: &std::sync::atomic::AtomicBool) -> Prepared { use std::sync::atomic::Ordering::Relaxed; - self.look_up_library(); + // the library as it was, kept for the ownership of the shared stem sidecars below + let library = self.look_up_library(); let opts = self.opts.clone(); let mode = self.mode; let mut out = Prepared { scanned: files.len(), items: Vec::new() }; @@ -1025,6 +1029,13 @@ impl ImportJob { let probed: Vec> = cached.into_iter().map(|c| c.map(Ok).unwrap_or_else(|| fresh.next().unwrap_or_else(|| Err("not probed".into())))).collect(); crate::memory::release(); + // Who owns the shared stem sidecar (`IMG_0001.xmp`) among the library's files and the ones + // arriving with this import: a JPEG or a TIFF arriving beside its raw reads its own sidecar + // and never the raw's packet (issue #1905). + let stems = crate::sidecar::ImportStems::for_import( + library.as_ref(), + todo.iter().zip(&probed).filter_map(|(path, info)| info.as_ref().ok().map(|i| (path.as_str(), i.kind))), + ); // Files are planned one by one, in order (duplicates, sidecars, names, folders); their // copies or moves — the slow part, reading and writing every byte and verifying it — are // collected and run on several threads (issue #367), then their outcomes take their places. @@ -1063,7 +1074,7 @@ impl ImportJob { // the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files // and names the copy when the file itself has none let raw = info.kind == MediaKind::Raw; - let packet = crate::sidecar::find_sidecar(&path, self.naming) + let packet = crate::sidecar::find_sidecar_for_import(&stems, self.naming, &path) .and_then(|f| std::fs::read_to_string(f).ok()) .or_else(|| info.xmp.clone().filter(|_| raw)); let target = crate::crs::Target::for_file(info.kind, &info.format, info.preview_only.is_some(), &info.meta.camera); diff --git a/crates/engine/src/sidecar.rs b/crates/engine/src/sidecar.rs index ea06d69a7..88b2f1d12 100644 --- a/crates/engine/src/sidecar.rs +++ b/crates/engine/src/sidecar.rs @@ -15,7 +15,10 @@ //! **Shared names** (Stem naming): `IMG_0001.CR3` and `IMG_0001.JPG` would share `IMG_0001.xmp`. //! When two catalogued files map to one stem sidecar, it belongs to one of them (a raw first, then //! the first by file name — [`Session::sidecar_naming`]); the others use `.xmp` -//! (`IMG_0001.JPG.xmp`) for writing and reading, so neither overwrites the other. +//! (`IMG_0001.JPG.xmp`) for writing and reading, so neither overwrites the other. An import ranks +//! the files it is adding against the library the same way ([`ImportStems`]): a JPEG or a TIFF +//! arriving beside its raw's `IMG_0001.xmp` reads its own `.xmp`, never the raw's packet +//! (issue #1905). //! //! **Read** (on import, and `photo.readMetadataFromFile`): the sidecar wins for metadata, except //! the capture time: a time embedded in the file wins, and the sidecar's (`exif:DateTimeOriginal`, @@ -23,7 +26,7 @@ //! settings are restored from `lc:settings` when present, else mapped from interoperable `crs:` //! fields ([`crate::crs`]). For raw/DNG files without a sidecar the file's embedded XMP is used. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -69,6 +72,18 @@ pub fn find_sidecar(original: &str, naming: SidecarNaming) -> Option { sidecar_candidates(original, naming).find(|p| p.is_file()) } +/// Find the sidecar for a file an import is adding, by the ownership [`ImportStems`] worked out +/// (issue #1905): [`find_sidecar`] with the library's naming, except that a file which does not +/// own its stem sidecar reads its own `.xmp` (the name it writes) and never the packet of +/// the sibling that owns the stem one. +pub(crate) fn find_sidecar_for_import(stems: &ImportStems, naming: SidecarNaming, original: &str) -> Option { + if stems.owns(original) { + return find_sidecar(original, naming); + } + let shared = xmp_cases(sidecar_path(original, SidecarNaming::Stem)); + sidecar_candidates(original, SidecarNaming::Full).filter(|p| !shared.contains(p)).find(|p| p.is_file()) +} + /// The names `original`'s sidecar can have, in the order they are looked for: under `naming`, /// then under the other naming (the full name also with the photo extension in the other case), /// each as `.xmp`, `.XMP` and `.Xmp`. @@ -87,7 +102,12 @@ pub(crate) fn sidecar_candidates(original: &str, naming: SidecarNaming) -> impl } v }) - .flat_map(|p| [p.clone(), p.with_extension("XMP"), p.with_extension("Xmp")]) + .flat_map(xmp_cases) +} + +/// The three ways one sidecar name can be written: `.xmp`, `.XMP` and `.Xmp`. +fn xmp_cases(p: PathBuf) -> [PathBuf; 3] { + [p.clone(), p.with_extension("XMP"), p.with_extension("Xmp")] } /// What a sidecar (or embedded XMP packet) says about a photo. `None` = not stated. @@ -526,6 +546,16 @@ pub(crate) fn op_photos(op: &Op, out: &mut Vec) { } } +/// Where a file stands in the order a shared stem sidecar is owned by: a raw first, then the file +/// name in lower case, then the catalog id (a file an import is adding has no id yet, and ranks +/// last among files of its kind and name: see [`ImportStems`]). +type Rank = (bool, String, u64); + +/// The rank of the file at `path`, of kind `kind`, catalogued as `id`. +fn rank_of(path: &str, kind: MediaKind, id: u64) -> Rank { + (kind != MediaKind::Raw, path.to_lowercase(), id) +} + /// Which photo owns each stem sidecar that several catalogued files map to (see /// [`Session::sidecar_naming`]). pub(crate) struct StemOwners(HashMap); @@ -536,10 +566,10 @@ impl StemOwners { /// owners were worked out before, for every write). #[cfg(test)] pub(crate) fn of(cat: &lightcraft_catalog::Catalog) -> StemOwners { - let mut best: HashMap = HashMap::new(); + let mut best: HashMap = HashMap::new(); for p in cat.photos().filter(|p| p.copy_of.is_none()) { let Some(path) = file_path(p) else { continue }; - let rank = (p.kind != MediaKind::Raw, path.to_lowercase(), p.id.0); + let rank = rank_of(path, p.kind, p.id.0); let e = best.entry(stem_key(path)).or_insert_with(|| (rank.clone(), p.id, 0)); e.2 += 1; if rank < e.0 { @@ -564,7 +594,7 @@ impl StemOwners { /// The owners of the shared stems in `wanted`. fn where_wanted(cat: &lightcraft_catalog::Catalog, wanted: &std::collections::HashSet) -> StemOwners { // per stem: (raw first, then file name, then id) of the best candidate; and how many - let mut best: HashMap = HashMap::new(); + let mut best: HashMap = HashMap::new(); let mut key = String::new(); for p in cat.photos().filter(|p| p.copy_of.is_none()) { let Some(path) = file_path(p) else { continue }; @@ -572,7 +602,7 @@ impl StemOwners { if !wanted.contains(key.as_str()) { continue; } - let rank = (p.kind != MediaKind::Raw, path.to_lowercase(), p.id.0); + let rank = rank_of(path, p.kind, p.id.0); let e = best.entry(key.clone()).or_insert_with(|| (rank.clone(), p.id, 0)); e.2 += 1; if rank < e.0 { @@ -591,6 +621,63 @@ impl StemOwners { } } +/// Which of the files an import is adding may read the shared stem sidecar (`IMG_0001.xmp`) of +/// their stem, and which read their own `.xmp` (`IMG_0001.TIF.xmp`) instead (issue #1905: +/// before, a JPEG or a TIFF imported beside its raw read the raw's packet and took its rating, +/// metadata and develop settings). The order is the one [`StemOwners`] applies to writing, with +/// the arriving files ranked against the library and against each other: a raw first, then the +/// file name in lower case. A file that is not catalogued yet has no id, and the id it draws when +/// the import commits is above every id in the library, so it loses a tie against a catalogued +/// file; between arriving files the order they arrive in stands for it. +pub(crate) struct ImportStems { + /// The arriving files that own the stem sidecar of their stem, by path as it arrives. + owners: HashSet, +} + +impl ImportStems { + /// Work out [`ImportStems`] for the files `incoming`, in the order they arrive. `None` for a + /// library already looked over (a job prepared twice): there is nothing to rank against, so + /// every arriving file reads as it did before. The library is passed over once, and only the + /// stems the arriving files map to are looked at closer. + pub(crate) fn for_import<'a>(cat: Option<&lightcraft_catalog::Catalog>, incoming: impl Iterator) -> ImportStems { + // per stem: the best of the library's files, and the best arriving file with its path + let mut best: HashMap, Option<(Rank, &'a str)>)> = HashMap::new(); + for (arriving, (path, kind)) in incoming.enumerate() { + let e = best.entry(stem_key(path)).or_default(); + let rank = rank_of(path, kind, arriving as u64); + if e.1.as_ref().is_none_or(|(b, _)| rank < *b) { + e.1 = Some((rank, path)); + } + } + if let Some(cat) = cat { + let mut key = String::new(); + for p in cat.photos().filter(|p| p.copy_of.is_none()) { + let Some(path) = file_path(p) else { continue }; + stem_key_into(&mut key, path); + let Some(e) = best.get_mut(key.as_str()) else { continue }; + let rank = rank_of(path, p.kind, p.id.0); + if e.0.as_ref().is_none_or(|b| rank < *b) { + e.0 = Some(rank); + } + } + } + let owners = best + .into_values() + .filter_map(|(in_library, arriving)| { + let (rank, path) = arriving?; + let wins = in_library.as_ref().is_none_or(|b| (rank.0, rank.1.as_str()) < (b.0, b.1.as_str())); + wins.then(|| path.to_string()) + }) + .collect(); + ImportStems { owners } + } + + /// Whether the arriving file at `original` owns the stem sidecar of its stem. + pub(crate) fn owns(&self, original: &str) -> bool { + self.owners.contains(original) + } +} + /// How the sidecar of each of photos `ids` is named in library `cat`, whose naming is `naming` /// ([`Session::sidecar_namings`], for a thread that has the library and not the session). pub(crate) fn namings_in(cat: &lightcraft_catalog::Catalog, naming: SidecarNaming, ids: &[PhotoId]) -> Vec { diff --git a/crates/engine/src/sidecar/tests_stem_owners.rs b/crates/engine/src/sidecar/tests_stem_owners.rs index 43170f4f6..0d86fe92d 100644 --- a/crates/engine/src/sidecar/tests_stem_owners.rs +++ b/crates/engine/src/sidecar/tests_stem_owners.rs @@ -1,4 +1,5 @@ -//! Who owns a stem sidecar, worked out for the photos asked about only (`StemOwners::among`). +//! Who owns a stem sidecar, worked out for the photos asked about only (`StemOwners::among`), and +//! for the files an import is adding (`ImportStems`). //! //! A library can hold millions of photos; a sidecar that is written for one of them, or a folder //! that is synchronized, must not cost a table of them all. @@ -37,6 +38,11 @@ const PATHS: [&str; 26] = [ "/pics/dots/a.jpg", ]; +/// The kind a file of this name is catalogued as: the `.cr3` ones are raws. +fn kind_of(path: &str) -> MediaKind { + if path.to_lowercase().ends_with("cr3") { MediaKind::Raw } else { MediaKind::Image } +} + fn library() -> (Catalog, Vec) { let mut c = Catalog::new(); let mut ids = Vec::new(); @@ -44,9 +50,7 @@ fn library() -> (Catalog, Vec) { let id = c.alloc_photo_id(); let name = path.rsplit('/').next().unwrap_or(path); let mut p = Photo::new(id, Source::File { path: path.to_string() }, name, "X", 6, 4, "2026-01-01"); - if path.to_lowercase().ends_with("cr3") { - p.kind = MediaKind::Raw; - } + p.kind = kind_of(path); c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); ids.push(id); } @@ -114,6 +118,72 @@ fn owners_among_some_photos_hold_nothing_of_the_rest() { assert_eq!(namings(&some, &c, &first), [SidecarNaming::Stem, SidecarNaming::Full, SidecarNaming::Stem]); } +/// Given a library where files share stems in every way a name can, when files arrive in an +/// import, then each of them may read the shared stem sidecar exactly when it would own it once +/// it is catalogued: the arriving files are ranked against the library's files and against each +/// other, as the write rule ranks them (issue #1905). +#[test] +fn an_arriving_file_owns_the_stem_sidecar_exactly_when_it_would_once_catalogued() { + let (c, _) = library(); + // the arrivals: the same names under an upper-case folder, so each shares its stem with the + // library's files (the key lower-cases the path) without being one of them + let incoming: Vec<(String, MediaKind)> = PATHS + .iter() + .map(|p| { + let (dir, name) = p.rsplit_once('/').unwrap_or(("", p)); + (format!("{}/{name}", dir.to_uppercase()), kind_of(p)) + }) + .collect(); + // the library with these arrivals in it, the ids drawn in the order they arrive + let arriving_in = |base: &Catalog, add: &[usize]| -> Catalog { + let mut cat = base.clone(); + for &k in add { + let (path, kind) = &incoming[k]; + let id = cat.alloc_photo_id(); + let name = path.rsplit('/').next().unwrap_or(path); + let mut p = Photo::new(id, Source::File { path: path.clone() }, name, "X", 6, 4, "2026-01-01"); + p.kind = *kind; + cat.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + } + cat + }; + let owns_once_catalogued = |cat: &Catalog, k: usize| -> bool { + let path = &incoming[k].0; + let p = cat.photos().find(|p| matches!(&p.source, Source::File { path: x } if x == path)).unwrap(); + StemOwners::of(cat).naming(p, SidecarNaming::Stem) == SidecarNaming::Stem + }; + let as_arriving = |ks: &[usize]| -> Vec<(&str, MediaKind)> { ks.iter().map(|k| (incoming[*k].0.as_str(), incoming[*k].1)).collect() }; + let all: Vec = (0..incoming.len()).collect(); + for i in 0..incoming.len() { + let alone = ImportStems::for_import(Some(&c), as_arriving(&[i]).into_iter()); + assert_eq!(alone.owns(&incoming[i].0), owns_once_catalogued(&arriving_in(&c, &[i]), i), "{} alone", incoming[i].0); + for j in 0..incoming.len() { + // every pair arriving together + if i == j { + continue; + } + let pair = ImportStems::for_import(Some(&c), as_arriving(&[i, j]).into_iter()); + let cat = arriving_in(&c, &[i, j]); + for k in [i, j] { + assert_eq!( + pair.owns(&incoming[k].0), + owns_once_catalogued(&cat, k), + "{} with {}", + incoming[k].0, + incoming[if k == i { j } else { i }].0 + ); + } + } + } + // a job prepared twice (a library already looked over) has nothing to rank against: every + // one of them is ranked among the arrivals alone, as it would be in a library of just them + let alone = ImportStems::for_import(None, as_arriving(&all).into_iter()); + let only_arrivals = arriving_in(&Catalog::new(), &all); + for k in &all { + assert_eq!(alone.owns(&incoming[*k].0), owns_once_catalogued(&only_arrivals, *k), "{} without a library", incoming[*k].0); + } +} + /// The key written into a buffer (no allocation for a plain path) is the key: for every path /// above, for shapes a path from a file can have, and for every string of up to six of the /// characters that decide a key (a letter in each case, a dot, both separators, a letter that is diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 3d6469deb..ce8ef3171 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -1748,3 +1748,94 @@ fn presets_apply_the_imported_profile_they_name() { assert_eq!(apply(&mut s, "Portra Soft").profile.id, "lc.vivid"); let _ = std::fs::remove_dir_all(&src); } + +/// A raw container LightCraft shows by its embedded JPEG: a TIFF whose IFD0 points at one. +fn write_raw(path: &Path) { + use lightcraft_codecs::{ChromaSubsampling, EncodeImage, EncodeMeta, Samples, encode_jpeg}; + let px: Vec = (0..48 * 32).flat_map(|i| [(i % 251) as u8, 91, 201]).collect(); + let j = encode_jpeg(&EncodeImage::new(48, 32, 3, Samples::U8(&px)), 90, ChromaSubsampling::S444, &EncodeMeta::default()).unwrap(); + let off = 8 + 2 + 24 + 4; + let mut f = b"II*\0\x08\0\0\0\x02\0".to_vec(); + for (tag, v) in [(513u16, off as u32), (514, j.len() as u32)] { + f.extend_from_slice(&tag.to_le_bytes()); + f.extend_from_slice(&[4, 0, 1, 0, 0, 0]); + f.extend_from_slice(&v.to_le_bytes()); + } + f.extend_from_slice(&[0, 0, 0, 0]); + f.extend_from_slice(&j); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, f).unwrap(); +} + +/// An edited TIFF of a raw: the same name, a TIFF (not a raw) to LightCraft. +fn write_tiff(path: &Path, seed: u8) { + use lightcraft_codecs::{EncodeImage, EncodeMeta, Samples, TiffCompression, encode_tiff}; + let (w, h) = (48usize, 32usize); + let px: Vec = (0..w * h).flat_map(|i| [(i % w * 5) as u8, (i / w * 7) as u8, seed]).collect(); + let bytes = encode_tiff(&EncodeImage::new(w as u32, h as u32, 3, Samples::U8(&px)), TiffCompression::Deflate, &EncodeMeta::default()).unwrap(); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, bytes).unwrap(); +} + +/// Issue #1905 (the import side of #1523): the stem sidecar `IMG_1.xmp` belongs to the raw first, +/// so a TIFF or JPEG arriving beside a raw of the same name reads its own `IMG_1.TIF.xmp`, never +/// the raw's packet, with or without the raw. Before, the arriving file took the raw's rating, +/// label, metadata and develop settings. +#[test] +fn a_file_arriving_beside_its_raws_stem_reads_its_own_sidecar() { + let sidecar = |rating: u8| { + format!( + r#" +"# + ) + }; + let read = |s: &Session, name: &str| -> u8 { s.catalog.photos().find(|p| p.file_name == name).map(|p| p.rating).unwrap_or_default() }; + + // the raw and the edited TIFF of it, same name, in one import + let src = temp_dir("stem-pair"); + write_raw(&src.join("IMG_1.NEF")); + write_tiff(&src.join("IMG_1.TIF"), 7); + std::fs::write(src.join("IMG_1.xmp"), sidecar(5)).unwrap(); + std::fs::write(src.join("IMG_1.TIF.xmp"), sidecar(2)).unwrap(); + let mut s = Session::new().with_fs(); + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap(); + assert_eq!((ids(&r, "imported"), ids(&r, "failed")), (2, 0), "{r}"); + assert_eq!(r["sidecars"], 2, "{r}"); + assert_eq!(read(&s, "IMG_1.NEF"), 5, "the raw reads the stem sidecar, which is its own"); + assert_eq!(read(&s, "IMG_1.TIF"), 2, "the TIFF reads its own sidecar, never the raw's"); + let _ = std::fs::remove_dir_all(&src); + + // the TIFF with no sidecar of its own: nothing of the raw's packet comes across + let src = temp_dir("stem-none"); + write_raw(&src.join("IMG_2.NEF")); + write_tiff(&src.join("IMG_2.TIF"), 8); + std::fs::write(src.join("IMG_2.xmp"), sidecar(4)).unwrap(); + let mut s = Session::new().with_fs(); + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap(); + assert_eq!((ids(&r, "imported"), ids(&r, "failed")), (2, 0), "{r}"); + assert_eq!(r["sidecars"], 1, "{r}"); + assert_eq!(read(&s, "IMG_2.NEF"), 4); + assert_eq!(read(&s, "IMG_2.TIF"), 0, "the raw's packet is not the TIFF's"); + let _ = std::fs::remove_dir_all(&src); + + // the raw already in the library, the edited TIFF arriving later + use lightcraft_catalog::{MediaKind, Op, Photo, Source}; + let src = temp_dir("stem-after"); + write_raw(&src.join("IMG_3.NEF")); + write_tiff(&src.join("IMG_3.TIF"), 9); + std::fs::write(src.join("IMG_3.xmp"), sidecar(3)).unwrap(); + let mut s = Session::new().with_fs(); + let raw = { + let id = s.catalog.alloc_photo_id(); + let path = src.join("IMG_3.NEF").to_string_lossy().into_owned(); + let mut p = Photo::new(id, Source::File { path }, "IMG_3.NEF", "NEF", 48, 32, "2026-01-14T05:58:48"); + p.kind = MediaKind::Raw; + p + }; + s.catalog.apply(Op::AddPhoto { photo: Box::new(raw) }).unwrap(); + let r = s.execute("library.import", &json!({"paths": [src.join("IMG_3.TIF").to_string_lossy()]})).unwrap(); + assert_eq!((ids(&r, "imported"), ids(&r, "failed")), (1, 0), "{r}"); + assert_eq!(r["sidecars"], 0, "{r}"); + assert_eq!(read(&s, "IMG_3.TIF"), 0, "the catalogued raw owns IMG_3.xmp"); + let _ = std::fs::remove_dir_all(&src); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 95dcd2aef..1fcddcbbd 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-16 · **Change:** patch (LR-META-XMP: a file imported beside another one of its stem reads its own sidecar, never the sibling's shared stem packet, issue #1905) · **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 @@ -170,7 +170,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-META-COPYRIGHT-DEFAULT | Default copyright on import | P1 | ✅ | `cmd:library.preferences` (`import.copyright`, `import.creator`), `crates/ui-egui/src/panels/settings.rs` | Settings → Import → Metadata; fills only photos without their own | | LR-META-LOCATION | Location editing | P2 | 🟡 | `cmd:photo.setMeta` (`location`, `city`, `state`, `country`, `gps`), `cmd:metadata.reverseGeocode` | place fields and GPS (decimal or degrees / minutes / seconds) edited in Info, read/written as IPTC Core / Photoshop / EXIF XMP; Show on Map opens OpenStreetMap; Fill Location fills empty City / State / Country from GPS offline (GeoNames, downloaded on request: `cmd:metadata.downloadPlaceNames`); no embedded map | | LR-META-COPYPASTE | Copy / paste metadata | P2 | ✅ | `cmd:photo.copyMetadata`, `cmd:photo.pasteMetadata` | title, caption, alt text, extended description, copyright (notice, status, usage terms, info URL), creator, place fields, keywords; `fields` picks a subset | -| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none; saving merges into an existing sidecar (`crates/meta/src/xmp_merge.rs`): another app's `crs:`, `xmpMM:History` and unknown namespaces are kept byte for byte, an unreadable sidecar is backed up first; with stem naming, files sharing a stem (raw + JPEG) get separate sidecars (`.xmp` for all but the raw); a `crs:` custom white balance on a relative-WB raw is shifted from `crs:AsShotTemperature` rather than read as Kelvin, or kept As Shot and reported when the packet has no as-shot white (issue #510); heal and clone spots (`crs:RetouchAreas`, legacy `crs:RetouchInfo`) are read into our spots (`crates/engine/src/crs_masks.rs`, see `docs/xmp-interop.md` → Spot removal); radial masks (`Mask/CircularGradient`) keep Lightroom's inside / outside: `crs:Flipped="true"` is the effect-inside flag of current masks and of the legacy Radial Filter, `crs:MaskInverted` the inversion, checked against Lightroom Classic's renders (issue #1815); Read Metadata from File never writes the sidecar back, even with auto-write on, so another app's later `crs:` edits are still read (issue #934) | +| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none; saving merges into an existing sidecar (`crates/meta/src/xmp_merge.rs`): another app's `crs:`, `xmpMM:History` and unknown namespaces are kept byte for byte, an unreadable sidecar is backed up first; with stem naming, files sharing a stem (raw + JPEG) get separate sidecars (`.xmp` for all but the raw; an import ranks the files it is adding the same way, so a TIFF or JPEG arriving beside its raw reads its own `.xmp` and never the raw's `IMG_0001.xmp`, issue #1905); a `crs:` custom white balance on a relative-WB raw is shifted from `crs:AsShotTemperature` rather than read as Kelvin, or kept As Shot and reported when the packet has no as-shot white (issue #510); heal and clone spots (`crs:RetouchAreas`, legacy `crs:RetouchInfo`) are read into our spots (`crates/engine/src/crs_masks.rs`, see `docs/xmp-interop.md` → Spot removal); radial masks (`Mask/CircularGradient`) keep Lightroom's inside / outside: `crs:Flipped="true"` is the effect-inside flag of current masks and of the legacy Radial Filter, `crs:MaskInverted` the inversion, checked against Lightroom Classic's renders (issue #1815); Read Metadata from File never writes the sidecar back, even with auto-write on, so another app's later `crs:` edits are still read (issue #934) | | LR-META-EXIF-FULL | Full EXIF/IPTC [Classic] | P1 | ✅ | `cmd:photo.allMetadata`, `cmd:dialog.allMetadata`, `crates/meta/src/tags.rs`, `crates/meta/src/exif.rs`, `crates/meta/src/iptc.rs` | read and written on export; Info ▸ All Metadata…: every TIFF / EXIF / GPS / interop tag (named, common values spelled out) and the XMP fields, searchable; metadata presets | ## F. Edit panel — global adjustments (EDIT) @@ -887,3 +887,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-13 | minor | LRC-LIB-CATALOG-IMPORT review: offline Local promotion and result dismissal across library switches covered; redundant operation/settings copies removed. Status and summary counts unchanged. | | 2026-10-14 | patch | LRC-LIB-CATALOG-IMPORT: reconciled with upstream phased progress, cancellation kinds, WAL-only import, translations and current progress tracking. Status and summary counts unchanged. | | 2026-10-13 | minor | KEYC-COMPARE: `=` / `-` (also `+`) increase / decrease the grid thumbnail size (`view.thumbSizeUp` / `view.thumbSizeDown`, View menu), grids only; status unchanged (🟡) | +| 2026-10-16 | patch | LR-META-XMP: a TIFF or JPEG imported beside a raw of its stem reads its own `.xmp` and takes nothing from the raw's `IMG_0001.xmp`, whether it arrives with the raw or after it (a stem sidecar belongs to one file: a raw first, then the first by name); status unchanged (✅) | diff --git a/docs/xmp-interop.md b/docs/xmp-interop.md index c467a471a..c2ce80ea4 100644 --- a/docs/xmp-interop.md +++ b/docs/xmp-interop.md @@ -10,7 +10,7 @@ other tools), and let LightCraft pick up edits made elsewhere. |---|---| | Name | `.xmp` by default (`IMG_0001.CR3` → `IMG_0001.xmp`); `.xmp` (`IMG_0001.CR3.xmp`) with `naming: "full"`. Reading accepts either (preferred first) and `.XMP`. With stem naming, files sharing a stem don't share a sidecar: see *Shared names* below. | | Write | `photo.saveMetadataToFile {ids?}` (Photo ▸ Save Metadata to File, ⌘S), or automatically after every change with `library.xmpPreferences {autoWrite: true}` (File ▸ Automatically Write Changes into XMP). Slider drags are written once, when the drag ends; undo/redo rewrite the sidecar. An existing sidecar is **merged into, never replaced** (see *Saving into an existing sidecar*). Written atomically (temp file, fsync, rename). The result lists `written`, `merged` and `backups`. | -| Read | On import (`library.import`, the report counts `sidecars`), and `photo.readMetadataFromFile {ids?}` (one undo step). For raw/DNG files without a sidecar, the XMP embedded in the file is used. `photo.readMetadataFromFile` never writes the sidecar, even with auto-write on: writing back what it has just read would add `lc:settings`, which later reads prefer over another app's `crs:` edits (issue #934). | +| Read | On import (`library.import`, the report counts `sidecars`), and `photo.readMetadataFromFile {ids?}` (one undo step). For raw/DNG files without a sidecar, the XMP embedded in the file is used. `photo.readMetadataFromFile` never writes the sidecar, even with auto-write on: writing back what it has just read would add `lc:settings`, which later reads prefer over another app's `crs:` edits (issue #934). A file arriving in an import is ranked as if it were catalogued already: it reads its own `.xmp` when another file of its stem (a raw first) owns the shared stem sidecar (issue #1905). | | Preferences | `library.xmpPreferences {autoWrite?, naming?: stem\|full}`, stored in the library's `prefs.json`. | What we write (standard namespaces, so other tools can read the metadata): @@ -74,6 +74,10 @@ library share a stem, the stem sidecar belongs to one of them — a raw first, o the others write and read `.xmp` (`IMG_0001.JPG.xmp`), falling back to the stem sidecar for reading when they have none. So saving one photo never overwrites the other's metadata (`Session::sidecar_naming`). +An import ranks the files it is adding the same way, before they are catalogued: a TIFF or JPEG arriving beside a +raw of its stem reads its own `.xmp` and takes nothing from the raw's `IMG_0001.xmp`, whether it arrives +with the raw or after it (issue #1905). + ## Face regions (MWG) LightCraft reads face, pet, focus and barcode regions in the Metadata Working Group's region schema From abf29ea392061b3ae81a5541c5afbef03c365d0e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 21:53:46 +0000 Subject: [PATCH 12/99] #1508: web backup refuses to start while changes are unsaved The browser backup copies the stored library files, so while a save to browser storage is failing it reported success but left out the queued changes. It now refuses with the number of unsaved changes and why (Session::unsaved), and is allowed again once they are written. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AbegtYaPuwqZ81pGfELsFK --- apps/lightcraft-web/src/backup.rs | 12 ++++++++++++ apps/lightcraft-web/src/files.rs | 26 ++++++++++++++++++++++++++ apps/lightcraft-web/src/web.rs | 3 +++ docs/web.md | 4 +++- docs/whats-new.md | 8 ++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) diff --git a/apps/lightcraft-web/src/backup.rs b/apps/lightcraft-web/src/backup.rs index 00ec12a41..281a32296 100644 --- a/apps/lightcraft-web/src/backup.rs +++ b/apps/lightcraft-web/src/backup.rs @@ -52,6 +52,18 @@ pub fn valid_hash(h: &str) -> bool { !h.is_empty() && h.len() <= 128 && h.chars().all(|c| c.is_ascii_alphanumeric()) } +/// Why a backup can't start yet, given [`lightcraft_engine::Session::unsaved`]: changes applied in +/// the app but not yet written to browser storage (a save failed). The zip copies the stored +/// library files, so it would report success while missing them; `None` when everything is saved. +pub fn unsaved_refusal(unsaved: Option<(usize, &str)>) -> Option { + let (ops, why) = unsaved?; + let changes = if ops == 1 { "1 change".to_string() } else { format!("{ops} changes") }; + Some(format!( + "Backup not started: {changes} could not be saved to browser storage yet ({why}), so the backup would miss them. \ + Free up storage space; LightCraft retries the save with every change. Back up once nothing is left unsaved." + )) +} + /// Zip entry name for an original. pub fn original_entry(hash: &str, file_name: &str) -> String { let name: String = file_name.chars().map(|c| if c == '/' || c == '\\' || c.is_control() { '_' } else { c }).collect(); diff --git a/apps/lightcraft-web/src/files.rs b/apps/lightcraft-web/src/files.rs index d0c9f348b..4b5f6289d 100644 --- a/apps/lightcraft-web/src/files.rs +++ b/apps/lightcraft-web/src/files.rs @@ -282,6 +282,32 @@ mod tests { assert_eq!(s2.catalog.photo(id).unwrap().rating, 5, "the rating survived"); } + /// A backup copies the stored library files, so while a save is failing it would miss the + /// queued changes: it is refused with the reason until they are written (#1508). + #[test] + fn backup_is_refused_while_changes_are_unsaved() { + let files = Files::default(); + let mut s = Session::new(); + let stores = LibraryStores { dir: "browser".into(), catalog: Box::new(files.store()), files: Box::new(files.store()), on_disk: false }; + s.open_library_in(stores, true).unwrap(); + assert_eq!(crate::backup::unsaved_refusal(s.unsaved()), None, "everything saved: the backup may start"); + files.set_error(Some("QuotaExceededError".into())); + let _ = s.execute("photo.rate", &json!({"rating": 5})); + let refusal = crate::backup::unsaved_refusal(s.unsaved()).expect("refused while the rating is unsaved"); + assert!(refusal.contains("could not be saved") && refusal.contains("QuotaExceeded"), "{refusal}"); + files.set_error(None); + s.persist_if_dirty(); + s.persist().unwrap(); + assert_eq!(crate::backup::unsaved_refusal(s.unsaved()), None, "allowed again once the change is written"); + } + + #[test] + fn unsaved_refusal_counts_changes() { + use crate::backup::unsaved_refusal; + assert!(unsaved_refusal(Some((1, "quota"))).unwrap().contains("1 change could not")); + assert!(unsaved_refusal(Some((3, "quota"))).unwrap().contains("3 changes could not")); + } + #[test] fn library_survives_reload_through_flushes() { let mut disk = BTreeMap::new(); diff --git a/apps/lightcraft-web/src/web.rs b/apps/lightcraft-web/src/web.rs index 6a4487d09..3ce8eb66c 100644 --- a/apps/lightcraft-web/src/web.rs +++ b/apps/lightcraft-web/src/web.rs @@ -173,6 +173,9 @@ fn services(originals: Originals, backend: Option, files: Files, frozen screen_profile: None, backup_library: Some(Box::new(move |session: &mut Session| { let Some(b) = backup_backend.clone() else { return Err("nothing is stored in this browser session (?store=memory)".into()) }; + if let Some(why) = crate::backup::unsaved_refusal(session.unsaved()) { + return Err(why); + } // the photos' own names for the originals in the zip let names: std::collections::HashMap = session .catalog diff --git a/docs/web.md b/docs/web.md index b275e837d..a755bfd80 100644 --- a/docs/web.md +++ b/docs/web.md @@ -111,7 +111,9 @@ are the same in every version, so serve them with `Cache-Control: no-cache` (not grant it, a notice says the library may be evicted under storage pressure. - **File ▸ Back Up Library…** downloads a zip of the library files and every stored original (`originals//`, so an unzipped backup is browsable; entries are stored, not - compressed; at most 4 GB). **File ▸ Restore Library from Backup…** reads such a zip into a + compressed; at most 4 GB). While a save is failing it refuses to start and says how many + changes are unsaved and why: the zip copies the stored library files, so it would miss them + (issue #1508). **File ▸ Restore Library from Backup…** reads such a zip into a new library folder (`library-restored-