From 1c815fb3f823083e96b67bf10ec5a68470383b31 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 11:00:13 +0200 Subject: [PATCH 01/47] Show an empty view when Find Similar has no matches Found while testing crates/engine/src/cmd/cull.rs:115 (find_similar; empty-`only` semantics at crates/catalog/src/query.rs:328) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/cull.rs | 4 ++++ crates/engine/src/tests.rs | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/crates/engine/src/cmd/cull.rs b/crates/engine/src/cmd/cull.rs index 676519d29..c5d9a1854 100644 --- a/crates/engine/src/cmd/cull.rs +++ b/crates/engine/src/cmd/cull.rs @@ -113,6 +113,10 @@ fn find_similar(s: &mut Session, p: &Value) -> Result { hits.sort_by(|a, b| b.1.total_cmp(&a.1)); if p.get("filter").and_then(Value::as_bool).unwrap_or(true) { s.filter.only = hits.iter().map(|h| h.0).collect(); + if s.filter.only.is_empty() { + // an empty `only` means "no constraint": use an id no photo has so no hits show no photos + s.filter.only = vec![PhotoId(u64::MAX)]; + } } Ok(json!({"photos": hits.iter().map(|(i, v)| json!({"id": i.0, "similarity": (v * 1000.0).round() / 1000.0})).collect::>()})) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 400135ce0..f4ac5ae5f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1102,6 +1102,16 @@ fn find_similar_filters_to_look_alikes() { assert!(s.visible_cloned().len() > vis.len()); } +#[test] +fn find_similar_with_no_hits_shows_no_photos() { + let mut s = demo(); + let r = s.execute("library.findSimilar", &json!({"similarity": 1.5})).unwrap(); + assert_eq!(r["photos"].as_array().map(Vec::len), Some(0)); + assert_eq!(s.visible_cloned().len(), 0, "an empty Find Similar result must show no photos, not the whole library"); + s.execute("library.clearFilter", &json!({})).unwrap(); + assert_eq!(s.visible_cloned().len(), s.catalog.len(), "Clear Filters brings everything back"); +} + #[test] fn quick_develop_adds_to_each_photo() { use lightcraft_catalog::PhotoId; From 5879f07dfdd6255e7eb1d25745fa5e9aaee0b768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Sun, 18 Oct 2026 10:36:42 +0800 Subject: [PATCH 02/47] Masking: outline every visible mask when the all-masks overlay is on Ctrl+Shift+O (the all-masks overlay) shows each visible mask's painted region, but only the selected mask drew its white outline. Draw the outline for every visible mask while mask_overlay_all is active so the other Radial/Linear masks read like the selected one. Handles and the rotation grip stay on the selected mask, so nothing else becomes draggable. Extends the all-masks overlay added in #919. --- crates/ui-egui/src/panels/detail.rs | 60 +++++++++++++++++------------ 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 44427565e..3d0297831 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -2011,9 +2011,14 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons } for m in &d.masks { let sel = Some(m.id) == active; + // Ctrl+Shift+O (`mask_overlay_all`) shows every visible mask's region; outline each one + // too, not just the selected mask. Handles and the rotation grip stay on the selected + // mask so nothing else becomes draggable. + let show_outline = sel || (app.ui.mask_overlay_all && m.visible); + let stroke_a: u8 = if sel { 230 } else { 170 }; for (ci, c) in m.components.iter().enumerate() { match &c.shape { - MaskShape::Radial { center, rx, ry, angle, .. } if sel => { + MaskShape::Radial { center, rx, ry, angle, .. } if show_outline => { let pts: Vec = (0..64) .map(|i| { let a = i as f64 / 64.0 * std::f64::consts::TAU; @@ -2023,39 +2028,44 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons map.screen(Point::new(center.x + (x * co - y * s) * l.0, center.y + (x * s + y * co) * l.1)) }) .collect(); - p.add(egui::Shape::closed_line(pts, Stroke::new(1.5, Color32::from_white_alpha(230)))); + p.add(egui::Shape::closed_line(pts, Stroke::new(1.5, Color32::from_white_alpha(stroke_a)))); + + if sel { + let l = frame_long_norm(map); + for (i, a_deg) in [0.0_f64, 90.0_f64, 180.0_f64, 270.0_f64].into_iter().enumerate() { + let a_local = a_deg.to_radians(); + let (s, co) = angle.to_radians().sin_cos(); + let (x, y) = (rx * a_local.cos(), ry * a_local.sin()); + let q = map.screen(Point::new(center.x + (x * co - y * s) * l.0, center.y + (x * s + y * co) * l.1)); + p.circle_stroke(q, 5.0, Stroke::new(1.5, Color32::WHITE)); + register(ui.ctx(), format!("maskHandle:{}:{ci}:{}", m.id, i + 1), Rect::from_center_size(q, vec2(14.0, 14.0))); + grips.push((m.id, ci, i as u8 + 1, q)); + } - let l = frame_long_norm(map); - for (i, a_deg) in [0.0_f64, 90.0_f64, 180.0_f64, 270.0_f64].into_iter().enumerate() { - let a_local = a_deg.to_radians(); let (s, co) = angle.to_radians().sin_cos(); - let (x, y) = (rx * a_local.cos(), ry * a_local.sin()); - let q = map.screen(Point::new(center.x + (x * co - y * s) * l.0, center.y + (x * s + y * co) * l.1)); - p.circle_stroke(q, 5.0, Stroke::new(1.5, Color32::WHITE)); - register(ui.ctx(), format!("maskHandle:{}:{ci}:{}", m.id, i + 1), Rect::from_center_size(q, vec2(14.0, 14.0))); - grips.push((m.id, ci, i as u8 + 1, q)); + let rot_r = rx + 0.08; + let rot_q = map.screen(Point::new(center.x + (rot_r * co) * l.0, center.y + (rot_r * s) * l.1)); + let h1_q = map.screen(Point::new(center.x + (rx * co) * l.0, center.y + (rx * s) * l.1)); + p.line_segment([h1_q, rot_q], Stroke::new(1.0, Color32::from_white_alpha(200))); + p.circle_stroke(rot_q, 4.0, Stroke::new(1.5, Color32::WHITE)); + register(ui.ctx(), format!("maskHandle:{}:{ci}:5", m.id), Rect::from_center_size(rot_q, vec2(14.0, 14.0))); + grips.push((m.id, ci, 5, rot_q)); } - - let (s, co) = angle.to_radians().sin_cos(); - let rot_r = rx + 0.08; - let rot_q = map.screen(Point::new(center.x + (rot_r * co) * l.0, center.y + (rot_r * s) * l.1)); - let h1_q = map.screen(Point::new(center.x + (rx * co) * l.0, center.y + (rx * s) * l.1)); - p.line_segment([h1_q, rot_q], Stroke::new(1.0, Color32::from_white_alpha(200))); - p.circle_stroke(rot_q, 4.0, Stroke::new(1.5, Color32::WHITE)); - register(ui.ctx(), format!("maskHandle:{}:{ci}:5", m.id), Rect::from_center_size(rot_q, vec2(14.0, 14.0))); - grips.push((m.id, ci, 5, rot_q)); } - MaskShape::Linear { start, end } if sel => { + MaskShape::Linear { start, end } if show_outline => { let (a, b) = (map.screen(*start), map.screen(*end)); let dir = (b - a).normalized(); let perp = vec2(-dir.y, dir.x) * 3000.0; - for (q, alpha) in [(a, 230), (b, 110)] { + let (a1, a2): (u8, u8) = if sel { (230, 110) } else { (170, 80) }; + for (q, alpha) in [(a, a1), (b, a2)] { p.line_segment([q - perp, q + perp], Stroke::new(1.0, Color32::from_white_alpha(alpha))); } - for (i, q) in [a, b].iter().enumerate() { - p.circle_stroke(*q, 5.0, Stroke::new(1.5, Color32::WHITE)); - register(ui.ctx(), format!("maskHandle:{}:{ci}:{i}", m.id), Rect::from_center_size(*q, vec2(14.0, 14.0))); - grips.push((m.id, ci, i as u8 + 1, *q)); + if sel { + for (i, q) in [a, b].iter().enumerate() { + p.circle_stroke(*q, 5.0, Stroke::new(1.5, Color32::WHITE)); + register(ui.ctx(), format!("maskHandle:{}:{ci}:{i}", m.id), Rect::from_center_size(*q, vec2(14.0, 14.0))); + grips.push((m.id, ci, i as u8 + 1, *q)); + } } } _ => {} From eb009d1bd6e010184d711b814dc00ee2816d53c8 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sun, 18 Oct 2026 14:58:17 +0000 Subject: [PATCH 03/47] fix: give Find Similar an explicit empty-result filter state and clamp similarity Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/catalog/src/query.rs | 7 +++++++ crates/engine/src/cmd/cull.rs | 8 +++----- crates/engine/src/tests.rs | 27 +++++++++++++++++++++++++++ crates/engine/src/view.rs | 5 +++-- crates/ui-egui/src/panels/chips.rs | 2 +- 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index d1548f8bd..fae34ccb7 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -29,6 +29,10 @@ pub struct Filter { /// Only these photos (Find Similar results…); empty = no constraint. #[serde(skip_serializing_if = "Vec::is_empty")] pub only: Vec, + /// Show nothing: an explicit empty result (Find Similar with no matches), since an empty + /// `only` means "no constraint". + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub only_none: bool, /// Any of these labels (the filter bar's multi-select); empty = no constraint. #[serde(skip_serializing_if = "Vec::is_empty")] pub labels: Vec, @@ -325,6 +329,9 @@ impl Filter { if !self.labels.is_empty() && !p.label.is_some_and(|l| self.labels.contains(&l)) { return false; } + if self.only_none { + return false; + } if !self.only.is_empty() && !prepared.only.as_ref().map_or_else(|| self.only.contains(&p.id), |only| only.contains(&p.id)) { return false; } diff --git a/crates/engine/src/cmd/cull.rs b/crates/engine/src/cmd/cull.rs index c5d9a1854..27e7a4fa1 100644 --- a/crates/engine/src/cmd/cull.rs +++ b/crates/engine/src/cmd/cull.rs @@ -106,17 +106,15 @@ fn find_similar(s: &mut Session, p: &Value) -> Result { if let Some(v) = sig(s, q) { let sim = lightcraft_pipeline::cull::similarity(&me, &v); if sim >= min { - hits.push((q, sim)); + hits.push((q, sim.clamp(0.0, 1.0))); } } } hits.sort_by(|a, b| b.1.total_cmp(&a.1)); if p.get("filter").and_then(Value::as_bool).unwrap_or(true) { s.filter.only = hits.iter().map(|h| h.0).collect(); - if s.filter.only.is_empty() { - // an empty `only` means "no constraint": use an id no photo has so no hits show no photos - s.filter.only = vec![PhotoId(u64::MAX)]; - } + // an empty `only` means "no constraint": no hits is its own state that shows no photos + s.filter.only_none = s.filter.only.is_empty(); } Ok(json!({"photos": hits.iter().map(|(i, v)| json!({"id": i.0, "similarity": (v * 1000.0).round() / 1000.0})).collect::>()})) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f4ac5ae5f..e59c21a11 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1112,6 +1112,33 @@ fn find_similar_with_no_hits_shows_no_photos() { assert_eq!(s.visible_cloned().len(), s.catalog.len(), "Clear Filters brings everything back"); } +#[test] +fn find_similar_of_a_flat_photo_shows_an_empty_view_and_counts_zero() { + let mut s = demo(); + let photo = s.catalog.photos().next().unwrap().clone(); + // a flat photo's signature is all zeros: it is not similar even to itself + s.signatures.insert(crate::media::content_key(&photo), [0.0; 64]); + let r = s.execute("library.findSimilar", &json!({"id": photo.id.0})).unwrap(); + assert_eq!(r["photos"].as_array().map(Vec::len), Some(0)); + assert_eq!(s.visible_cloned().len(), 0, "no matches show nothing, not the whole library"); + assert_eq!(s.catalog.count(&s.filter), 0); + let chips = crate::filter_chips(&s.filter, &s.catalog); + assert_eq!(chips.iter().map(|c| c.label.as_str()).collect::>(), ["Only 0 photos"]); + s.execute("library.filter", &chips[0].clear).unwrap(); + assert_eq!(s.filter, Default::default(), "the chip clears the empty state"); + assert_eq!(s.visible_cloned().len(), s.catalog.len()); +} + +#[test] +fn find_similar_values_stay_within_zero_and_one() { + let mut s = demo(); + let id = s.catalog.photos().next().unwrap().id; + let r = s.execute("library.findSimilar", &json!({"id": id.0, "similarity": -1.0})).unwrap(); + let sims: Vec = r["photos"].as_array().unwrap().iter().map(|h| h["similarity"].as_f64().unwrap()).collect(); + assert_eq!(sims.len(), s.catalog.len()); + assert!(sims.iter().all(|v| (0.0..=1.0).contains(v)), "{sims:?}"); +} + #[test] fn quick_develop_adds_to_each_photo() { use lightcraft_catalog::PhotoId; diff --git a/crates/engine/src/view.rs b/crates/engine/src/view.rs index 5b32999bb..0d5cde3cf 100644 --- a/crates/engine/src/view.rs +++ b/crates/engine/src/view.rs @@ -214,8 +214,9 @@ pub fn filter_chips(f: &Filter, cat: &Catalog) -> Vec { if let Some(rs) = f.rule_set.as_ref().filter(|r| !r.rules.is_empty()) { add(format!("Rules: {}", rs.describe_with(cat)), json!({"ruleSet": Null})); } - if !f.only.is_empty() { - add(format!("Only {} photos", f.only.len()), json!({"only": []})); + if f.only_none || !f.only.is_empty() { + let n = if f.only_none { 0 } else { f.only.len() }; + add(format!("Only {n} photos"), json!({"only": [], "onlyNone": false})); } v } diff --git a/crates/ui-egui/src/panels/chips.rs b/crates/ui-egui/src/panels/chips.rs index 30f03971b..3a8f0ce8e 100644 --- a/crates/ui-egui/src/panels/chips.rs +++ b/crates/ui-egui/src/panels/chips.rs @@ -55,7 +55,7 @@ pub(crate) fn display_label(chip: &FilterChip, filter: &lightcraft_catalog::Filt }; } if chip.clear.get("only").is_some() { - return format!("{} {}", tr("Only"), crate::i18n::tr_format!("{n} photos", n = filter.only.len())); + return format!("{} {}", tr("Only"), crate::i18n::tr_format!("{n} photos", n = if filter.only_none { 0 } else { filter.only.len() })); } if let Some((prefix, value)) = chip.label.split_once(": ") { let value = if matches!(prefix, "Flag" | "Type") { From 195a519c0788fc2a24327f9834649ce06ed905e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 19 Oct 2026 07:57:06 +0800 Subject: [PATCH 04/47] Address review: regression tests and overlay outline parity - Add two regression tests in tests_masking.rs (next to mask_overlay_keys_and_pins) covering #2210: every visible mask is outlined under the all-masks overlay while only the selected mask registers maskHandle: widgets, and an unselected mask's outline is painted on screen. - Match the outline condition to the painted region: only outline masks with components, like view_overlay already does. - Add examples/mask_overlay_shot.rs (+ dev-dep png) to render a screenshot of the overlay for the PR. - Revert the earlier standalone test in headless.rs; make mask_overlay and CanvasMap::new pub(crate) so the masking tests can reach them. --- Cargo.lock | 1 + crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/examples/mask_overlay_shot.rs | 49 +++++++++++++++++ crates/ui-egui/src/panels/detail.rs | 9 ++-- crates/ui-egui/src/tests_masking.rs | 57 ++++++++++++++++++++ 5 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 crates/ui-egui/examples/mask_overlay_shot.rs diff --git a/Cargo.lock b/Cargo.lock index a33b59a81..8bb819d53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3525,6 +3525,7 @@ dependencies = [ "lightcraft-slideshow", "lightcraft-testkit", "log", + "png", "serde", "serde_json", "toml", diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 1ae33a03d..3b423f8e8 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -35,6 +35,7 @@ lightcraft-denoise = { workspace = true, features = ["runtime"] } lightcraft-codecs = { workspace = true } lightcraft-faces = { workspace = true } lightcraft-merge = { workspace = true } +png = { workspace = true } [build-dependencies] serde_json = { workspace = true } diff --git a/crates/ui-egui/examples/mask_overlay_shot.rs b/crates/ui-egui/examples/mask_overlay_shot.rs new file mode 100644 index 000000000..36875aee9 --- /dev/null +++ b/crates/ui-egui/examples/mask_overlay_shot.rs @@ -0,0 +1,49 @@ +//! Render a screenshot of the masking view with the all-masks overlay on: several visible masks, +//! each outlined, only the selected one draggable. Used to attach a visual to PR #2210. +//! +//! Run: `cargo run -p lightcraft-ui-egui --example mask_overlay_shot -- ` + +use std::time::Duration; + +use lightcraft_raster::Rgba8; +use serde_json::json; + +use lightcraft_ui_egui::headless::Headless; +use lightcraft_ui_egui::{LightcraftApp, Services}; + +fn main() { + let out = std::env::args().nth(1).unwrap_or_else(|| "mask-overlay.png".into()); + let t = Duration::from_secs(20); + + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + let _ = h.request("ui.set", json!({"view": "detail"}), t); + let _ = h.request("engine.execute", json!({"command": "panel.masking"}), t); + + // A few visible masks; mask.add selects the newest, so the last one is selected. + for params in [ + json!({"kind": "radial", "center": [0.3, 0.4], "rx": 0.12, "ry": 0.12}), + json!({"kind": "linear", "start": [0.65, 0.25], "end": [0.65, 0.6]}), + json!({"kind": "radial", "center": [0.7, 0.65], "rx": 0.1, "ry": 0.14, "angle": 30}), + ] { + let _ = h.request("engine.execute", json!({"command": "mask.add", "params": params}), t); + } + let _ = h.request("engine.execute", json!({"command": "view.maskOverlayAll"}), t); + let _ = h.request("engine.execute", json!({"command": "view.maskPins", "params": {"show": false}}), t); + h.settle(Duration::from_secs(120)); + + let px = h.view.paint(&Default::default()); + let rgba = Rgba8::from_fn(px.width(), px.height(), |x, y| { + let p = px.pixels[y * px.width() + x]; + [p.r(), p.g(), p.b(), p.a()] + }); + + let file = std::fs::File::create(&out).expect("create png"); + let mut enc = png::Encoder::new(file, rgba.width as u32, rgba.height as u32); + enc.set_color(png::ColorType::Rgba); + enc.set_depth(png::BitDepth::Eight); + let mut w = enc.write_header().expect("png header"); + w.write_image_data(&rgba.as_bytes()).expect("encode png"); + println!("wrote {out} ({}, {})", rgba.width, rgba.height); +} diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 3d0297831..1f24f7a43 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -81,7 +81,7 @@ pub struct CanvasMap { } impl CanvasMap { - fn new(frame: &Frame, rect: Rect) -> CanvasMap { + pub(crate) fn new(frame: &Frame, rect: Rect) -> CanvasMap { let k = 8.0f32; let (w, h) = ((rect.width() * k).round().max(1.0) as usize, (rect.height() * k).round().max(1.0) as usize); let to_norm = frame.out_to_norm(w, h); @@ -1996,7 +1996,7 @@ fn color_range_pick(app: &mut LightcraftApp, ui: &egui::Ui, resp: &egui::Respons /// The Masking tool on the photo: outlines and handles of the selected mask, a pin per mask /// component (click selects its mask, drag moves the component), and brush painting. The mask /// itself shows as a rendered overlay ([`view_overlay`]). -fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, d: &DevelopSettings) { +pub(crate) fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, map: &CanvasMap, d: &DevelopSettings) { let long = (map.rect.width().max(map.rect.height())) as f64; let active = app.session.active_mask; let clip = app.canvas_rect.unwrap_or(map.rect); @@ -2013,8 +2013,9 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons let sel = Some(m.id) == active; // Ctrl+Shift+O (`mask_overlay_all`) shows every visible mask's region; outline each one // too, not just the selected mask. Handles and the rotation grip stay on the selected - // mask so nothing else becomes draggable. - let show_outline = sel || (app.ui.mask_overlay_all && m.visible); + // mask so nothing else becomes draggable. The component check keeps this in step with the + // painted region (`view_overlay` at detail.rs:1354 only covers masks with components). + let show_outline = sel || (app.ui.mask_overlay_all && m.visible && !m.components.is_empty()); let stroke_a: u8 = if sel { 230 } else { 170 }; for (ci, c) in m.components.iter().enumerate() { match &c.shape { diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 4e397208e..c78183a6e 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -317,6 +317,63 @@ fn mask_overlay_keys_and_pins() { h.settle(SETTLE); } +/// #2210: Ctrl+Shift+O (the all-masks overlay) draws the white outline of every visible +/// Radial/Linear mask, not just the selected one — but only the selected mask registers +/// `maskHandle:` elements, so nothing else becomes draggable. +#[test] +fn all_masks_overlay_outlines_every_visible_mask_but_only_the_selected_one_is_draggable() { + let mut h = detail("panel.masking"); + // Two visible radial masks; mask 2 ends up selected (mask.add selects the new one). + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.3, 0.5], "rx": 0.12, "ry": 0.12})); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.7, 0.5], "rx": 0.12, "ry": 0.12})); + exec(&mut h, "view.maskOverlayAll", json!({})); + assert!(h.app.ui.mask_overlay_all); + assert_eq!(h.app.session.active_mask, Some(2)); + // Pins add their own elements; keep them out of the handle count. + exec(&mut h, "view.maskPins", json!({"show": false})); + h.settle(SETTLE); + + let handles_of = |h: &Headless, id: usize| h.app.widgets.iter().filter(|(w, _)| w.starts_with(&format!("maskHandle:{id}:"))).count(); + // The selected mask keeps its 4 edge handles and its rotation grip. + assert_eq!(handles_of(&h, 2), 5, "the selected mask keeps its handles"); + // The other visible mask draws its outline but registers no handle: it is not draggable. + assert_eq!(handles_of(&h, 1), 0, "an unselected mask must not become draggable"); + // …while a mask with no components has nothing to outline and nothing to grab either. + exec(&mut h, "mask.add", json!({"kind": "luminanceRange"})); + exec(&mut h, "mask.addComponent", json!({"kind": "radial", "op": "add", "center": [0.5, 0.8], "rx": 0.08, "ry": 0.08})); + assert_eq!(h.app.session.active_mask, Some(3)); + assert_eq!(handles_of(&h, 1), 0); + h.settle(SETTLE); +} + +/// #2210: the outlines of *unselected* masks are painted, not registered as widgets, so the way to +/// observe them is the rendered pixels. With the all-masks overlay on, the unselected mask's +/// ellipse edge must carry white stroke pixels the photo alone does not have. +#[test] +fn all_masks_overlay_paints_the_outline_of_an_unselected_mask() { + let mut h = detail("panel.masking"); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.3, 0.5], "rx": 0.12, "ry": 0.12})); + exec(&mut h, "mask.add", json!({"kind": "radial", "center": [0.7, 0.5], "rx": 0.12, "ry": 0.12})); + assert_eq!(h.app.session.active_mask, Some(2)); + exec(&mut h, "view.maskPins", json!({"show": false})); + + // Sample the whole canvas: the outline is white (alpha 170) over the photo, so the overlay adds + // near-white pixels along the unselected mask's ellipse edge. + let white_pixels = |h: &mut Headless, on: bool| -> usize { + let overlay = h.app.ui.mask_overlay_all; + if overlay != on { + exec(h, "view.maskOverlayAll", json!({})); + } + assert_eq!(h.app.ui.mask_overlay_all, on); + h.settle(SETTLE); + let px = h.view.paint(&Default::default()); + px.pixels.iter().filter(|p| p.r() > 200 && p.g() > 200 && p.b() > 200).count() + }; + let off = white_pixels(&mut h, false); + let on = white_pixels(&mut h, true); + assert!(on > off, "the unselected mask's outline must add white pixels: off {off}, on {on}"); +} + /// #518: holding the brush still, however long, paints the same feathered spot as a quick press. #[test] fn holding_the_brush_still_does_not_harden_the_spot() { From 794e307c67cccea19a1dc9bf96f9964d9c60c9b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=A0=91=E6=B4=8B=20=E9=99=88?= <205215139@qq.com> Date: Mon, 19 Oct 2026 08:18:54 +0800 Subject: [PATCH 05/47] Address review: fail the screenshot example if a mask command errors --- crates/ui-egui/examples/mask_overlay_shot.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/examples/mask_overlay_shot.rs b/crates/ui-egui/examples/mask_overlay_shot.rs index 36875aee9..7a8675d9d 100644 --- a/crates/ui-egui/examples/mask_overlay_shot.rs +++ b/crates/ui-egui/examples/mask_overlay_shot.rs @@ -18,8 +18,15 @@ fn main() { let services = Services { png: None, ..Default::default() }; let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); let mut h = Headless::new(app, [1200.0, 800.0], 1.0); - let _ = h.request("ui.set", json!({"view": "detail"}), t); - let _ = h.request("engine.execute", json!({"command": "panel.masking"}), t); + // Run a control command and stop with a clear message if it does not report ok, so the + // generated PNG never silently omits a mask it was meant to show (CodeRabbit review, line 30). + let mut run = |method: &str, params: serde_json::Value| { + let r = h.request(method, params, t); + assert_eq!(r["ok"], true, "mask_overlay_shot: `{method}` failed: {r}"); + }; + + run("ui.set", json!({"view": "detail"})); + run("engine.execute", json!({"command": "panel.masking"})); // A few visible masks; mask.add selects the newest, so the last one is selected. for params in [ @@ -27,10 +34,10 @@ fn main() { json!({"kind": "linear", "start": [0.65, 0.25], "end": [0.65, 0.6]}), json!({"kind": "radial", "center": [0.7, 0.65], "rx": 0.1, "ry": 0.14, "angle": 30}), ] { - let _ = h.request("engine.execute", json!({"command": "mask.add", "params": params}), t); + run("engine.execute", json!({"command": "mask.add", "params": params})); } - let _ = h.request("engine.execute", json!({"command": "view.maskOverlayAll"}), t); - let _ = h.request("engine.execute", json!({"command": "view.maskPins", "params": {"show": false}}), t); + run("engine.execute", json!({"command": "view.maskOverlayAll"})); + run("engine.execute", json!({"command": "view.maskPins", "params": {"show": false}})); h.settle(Duration::from_secs(120)); let px = h.view.paint(&Default::default()); From 8d4e6e1e018d52c557481edc495b2057a2384134 Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Thu, 22 Oct 2026 06:42:11 +0100 Subject: [PATCH 06/47] LR-META-XMP: preserve foreign namespace collisions --- crates/engine/src/tests_xmp.rs | 32 +++++++++++++ crates/meta/README.md | 7 +++ crates/meta/src/xmp.rs | 82 +++++++++++++++++++++++++++++----- crates/meta/src/xmp_merge.rs | 55 ++++++++++++++++++++--- docs/gaps.md | 5 ++- docs/parity-checklist.md | 5 ++- docs/whats-new.md | 6 +++ docs/xmp-interop.md | 3 ++ 8 files changed, 176 insertions(+), 19 deletions(-) diff --git a/crates/engine/src/tests_xmp.rs b/crates/engine/src/tests_xmp.rs index 1a4d86fa3..998bd985a 100644 --- a/crates/engine/src/tests_xmp.rs +++ b/crates/engine/src/tests_xmp.rs @@ -1375,6 +1375,38 @@ const OTHER_APP_SIDECAR: &str = r#" "#; +/// A foreign namespace may use a familiar prefix without becoming LightCraft-owned metadata. +#[test] +fn saving_preserves_foreign_namespace_prefix_collisions() { + let src = temp_dir("namespace-src"); + let lib = temp_dir("namespace-lib"); + write_png(&src.join("shot.png"), 1); + let foreign = r#"also keep"#; + let packet = format!(r#"{foreign}"#); + let path = src.join("shot.xmp"); + std::fs::write(&path, packet).unwrap(); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + s.execute("library.import", &json!({"paths": [src.join("shot.png")]})).unwrap(); + let id = only(&s); + assert_eq!(s.catalog.photo(id).unwrap().rating, 0, "a foreign rating must not become catalog metadata"); + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + s.execute("photo.rate", &json!({"rating": 5})).unwrap(); + for _ in 0..2 { + let result = s.execute("photo.saveMetadataToFile", &json!({})).unwrap(); + assert_eq!(result["failed"], json!([]), "{result}"); + assert_eq!(result["backups"], json!([]), "a valid packet must merge: {result}"); + assert_eq!(result["merged"], json!([path.to_string_lossy()]), "{result}"); + let saved = std::fs::read_to_string(&path).unwrap(); + assert!(saved.contains(foreign), "lost foreign bytes: {saved}"); + assert_eq!(lightcraft_meta::parse_xmp(&saved).unwrap().metadata.rating, Some(5)); + } + drop(s); + std::fs::remove_dir_all(src).unwrap(); + std::fs::remove_dir_all(lib).unwrap(); +} + /// Issue #92: saving merges into another app's sidecar instead of replacing it. #[test] fn saving_merges_into_another_apps_sidecar() { diff --git a/crates/meta/README.md b/crates/meta/README.md index 83ee9e601..5c002ff1b 100644 --- a/crates/meta/README.md +++ b/crates/meta/README.md @@ -27,6 +27,13 @@ prefixes by namespace URI, handles attribute and element forms, `rdf:Seq`/`Bag`/ `rdf:parseType="Resource"` and nested-description structs, predefined and numeric entities. Nesting depth and node count are limited. +An explicitly foreign namespace using a recognized prefix (for example `lc` or `xmp`) cannot supply +recognized properties. Its keys use `Q{hex-encoded namespace URI}local` in `properties` and `values`; +hex encoding keeps `/` reserved for struct paths. Other unknown prefixes keep their existing QName keys. +Merging preserves the original foreign XML rather than writing these opaque keys into the packet. +The parser and merge scanner each cap generated opaque names at 8 MiB per packet, returning `TooComplex` +if repeated use of a long namespace URI would exceed that budget. + ## Robustness No panics on malformed input: proptests feed random bytes to every container walker and random text to the diff --git a/crates/meta/src/xmp.rs b/crates/meta/src/xmp.rs index 1fb789164..2dbf59482 100644 --- a/crates/meta/src/xmp.rs +++ b/crates/meta/src/xmp.rs @@ -69,7 +69,8 @@ pub struct XmpData { /// The opaque `lc:settings` JSON string, if present. pub lc_settings: Option, /// Every property found, keyed `prefix:name` (struct fields as `prefix:name/prefix:field`); arrays keep - /// item order, language alternatives put `x-default` first. + /// item order, language alternatives put `x-default` first. Foreign namespaces that reuse a recognized + /// prefix use `Q{hex-encoded namespace URI}local` instead, so they cannot impersonate known properties. pub properties: BTreeMap>, /// Every top-level property with its full structure (arrays of structs such as local /// corrections, which [`XmpData::properties`] flattens). @@ -150,26 +151,46 @@ impl Node { const MAX_DEPTH: usize = 64; const MAX_NODES: usize = 200_000; +// Expanding a long namespace URI on many short names must not amplify a small packet without bound. +pub(crate) const MAX_OPAQUE_NAME_BYTES: usize = 8 * 1024 * 1024; -/// `prefix:local` with the prefix canonicalised by namespace URI (see [`NAMESPACES`]). -pub(crate) fn canonical_name(qname: &str, scopes: &[Vec<(String, String)>]) -> String { - canonical(qname, scopes) +/// Names canonicalised by namespace URI (see [`NAMESPACES`]); conflicting foreign prefixes are opaque. +pub(crate) fn canonical_name(qname: &str, scopes: &[Vec<(String, String)>], opaque_budget: &mut usize) -> Result { + canonical(qname, scopes, opaque_budget) } -fn canonical(qname: &str, scopes: &[Vec<(String, String)>]) -> String { +fn canonical(qname: &str, scopes: &[Vec<(String, String)>], opaque_budget: &mut usize) -> Result { let (prefix, local) = qname.split_once(':').unwrap_or(("", qname)); if prefix == "xml" || prefix == "xmlns" { - return qname.to_string(); + return Ok(qname.to_string()); } let uri = scopes.iter().rev().flat_map(|s| s.iter().rev()).find(|(p, _)| p == prefix).map(|(_, u)| u.as_str()); - match uri { + Ok(match uri { Some(u) => match NAMESPACES.iter().find(|(_, nu)| *nu == u) { Some((p, _)) => format!("{p}:{local}"), + None if NAMESPACES.iter().any(|(p, _)| *p == prefix) => { + // A foreign `lc:settings` must never match our `lc:*` ownership rule (nor may a + // foreign `xmp:Rating` become a rating). Braces cannot occur in an XML QName; + // hex keeps URI slashes out of the flattened property's `/` path separators. + let size = + u.len().checked_mul(2).and_then(|n| n.checked_add(3)).and_then(|n| n.checked_add(local.len())).ok_or(XmpError::TooComplex)?; + *opaque_budget = opaque_budget.checked_sub(size).ok_or(XmpError::TooComplex)?; + let mut key = String::with_capacity(size); + key.push_str("Q{"); + for byte in u.bytes() { + for nibble in [byte >> 4, byte & 15] { + key.push(char::from(if nibble < 10 { b'0' + nibble } else { b'a' + (nibble - 10) })); + } + } + key.push('}'); + key.push_str(local); + key + } None if prefix.is_empty() => local.to_string(), None => qname.to_string(), }, None => qname.to_string(), - } + }) } fn entity(name: &str) -> Option { @@ -202,7 +223,7 @@ fn xml_safe(s: &str) -> String { } /// Unescape attribute values (predefined + numeric references; unknown references kept verbatim). -fn unescape_attr(s: &str) -> String { +pub(crate) fn unescape_attr(s: &str) -> String { let mut out = String::with_capacity(s.len()); let mut rest = s; while let Some(i) = rest.find('&') { @@ -229,6 +250,7 @@ fn parse_tree(s: &str) -> Result { let mut stack: Vec = vec![Node { name: "#root".into(), ..Default::default() }]; let mut scopes: Vec> = vec![vec![("xml".into(), "http://www.w3.org/XML/1998/namespace".into())]]; let mut count = 0usize; + let mut opaque_budget = MAX_OPAQUE_NAME_BYTES; loop { let ev = reader.read_event().map_err(|e| XmpError::Xml(e.to_string()))?; match ev { @@ -251,8 +273,9 @@ fn parse_tree(s: &str) -> Result { } } scopes.push(scope); - let name = canonical(&String::from_utf8_lossy(e.name().as_ref()), &scopes); - let attrs = raw_attrs.into_iter().map(|(k, v)| (canonical(&k, &scopes), v)).collect(); + let name = canonical(&String::from_utf8_lossy(e.name().as_ref()), &scopes, &mut opaque_budget)?; + let attrs = + raw_attrs.into_iter().map(|(k, v)| Ok((canonical(&k, &scopes, &mut opaque_budget)?, v))).collect::>()?; let node = Node { name, attrs, ..Default::default() }; if matches!(ev, Event::Empty(_)) { scopes.pop(); @@ -767,6 +790,43 @@ mod tests { assert_eq!(d.lc_settings, None); } + #[test] + fn foreign_namespace_cannot_impersonate_known_metadata() { + for body in [ + r#"xmp:Rating="2" lc:settings="foreign settings"/>"#, + r#">2foreign settings"#, + ] { + let packet = format!( + r#""# + ); + let d = parse_xmp(&packet).unwrap(); + assert_eq!(d.metadata.rating, None, "foreign rating: {packet}"); + assert_eq!(d.lc_settings, None, "foreign settings: {packet}"); + assert!(!d.properties.contains_key("xmp:Rating")); + assert!(!d.values.contains_key("lc:settings")); + assert!(d.properties.values().any(|v| v == &["foreign settings"])); + assert!(d.properties.keys().all(|key| !key.contains('/')), "opaque names must not create struct paths"); + } + } + + #[test] + fn foreign_namespace_names_are_distinct_and_scoped() { + let packet = r#" + + 1 + 2 + 4 + "#; + let d = parse_xmp(packet).unwrap(); + assert_eq!(d.metadata.rating, Some(4), "a local rebinding must not escape its scope"); + assert_eq!(d.properties.len(), 3, "different foreign URIs must not collide"); + assert_eq!(d.values.len(), 3); + assert_eq!(d.properties["xmp:Rating"], vec!["4"]); + assert!(d.properties.values().any(|v| v == &["1"])); + assert!(d.properties.values().any(|v| v == &["2"])); + } + #[test] fn attribute_form_foreign_prefixes_and_structs() { let x = r#" diff --git a/crates/meta/src/xmp_merge.rs b/crates/meta/src/xmp_merge.rs index 022f962ec..e1e4bca5e 100644 --- a/crates/meta/src/xmp_merge.rs +++ b/crates/meta/src/xmp_merge.rs @@ -23,7 +23,7 @@ use std::collections::{BTreeSet, HashSet}; use quick_xml::events::Event; -use crate::xmp::{XmpError, canonical_name, parse_xmp}; +use crate::xmp::{MAX_OPAQUE_NAME_BYTES, XmpError, canonical_name, parse_xmp, unescape_attr}; /// Which properties [`merge_xmp`] takes from the fresh packet. Names are canonical /// `prefix:name` (prefixes as [`crate::parse_xmp`] reports them); `prefix:*` is a whole namespace. @@ -101,6 +101,7 @@ fn scan(s: &str) -> Result { let mut stack: Vec = Vec::new(); let mut scopes: Vec> = vec![vec![("xml".into(), "http://www.w3.org/XML/1998/namespace".into())]]; let mut count = 0usize; + let mut opaque_budget = MAX_OPAQUE_NAME_BYTES; loop { let before = pos(&reader); let ev = reader.read_event().map_err(|e| XmpError::Xml(e.to_string()))?; @@ -124,18 +125,21 @@ fn scan(s: &str) -> Result { } let v = String::from_utf8_lossy(&a.value).into_owned(); if k == "xmlns" { - scope.push((String::new(), v.clone())); + scope.push((String::new(), unescape_attr(&v))); } else if let Some(p) = k.strip_prefix("xmlns:") { - scope.push((p.to_string(), v.clone())); + scope.push((p.to_string(), unescape_attr(&v))); } raw_attrs.push((k, v)); } scopes.push(scope); let raw_name = String::from_utf8_lossy(e.name().as_ref()).into_owned(); - let name = canonical_name(&raw_name, &scopes); + let name = canonical_name(&raw_name, &scopes, &mut opaque_budget)?; let kind = match stack.last() { Some(Kind::Rdf) if name == "rdf:Description" => { - let attrs = raw_attrs.into_iter().map(|(raw, value)| Attr { name: canonical_name(&raw, &scopes), raw, value }).collect(); + let attrs = raw_attrs + .into_iter() + .map(|(raw, value)| Ok(Attr { name: canonical_name(&raw, &scopes, &mut opaque_budget)?, raw, value })) + .collect::>()?; out.descs.push(Desc { tag_start: before, tag_end: after, raw_name, empty, attrs, props: Vec::new(), end: after }); Kind::Desc(out.descs.len() - 1) } @@ -399,6 +403,47 @@ mod tests { assert_eq!(d.properties.get("exif:DateTimeOriginal"), Some(&vec!["2024-05-06T07:08:09".to_string()])); } + #[test] + fn preserves_foreign_properties_using_known_prefixes() { + for property in [r#"lc:Secret="keep-me"/>"#, r#">keep-me"#] { + let desc = format!(r#"{desc}"#); + let out = merge_xmp(&old, &fresh(4, &[]), RULES).unwrap(); + assert!(out.contains(&desc), "lost foreign description: {out}"); + assert_eq!(parse_xmp(&out).unwrap().metadata.rating, Some(4)); + assert_eq!(merge_xmp(&out, &fresh(4, &[]), RULES).unwrap(), out); + } + } + + #[test] + fn namespace_aliases_remain_distinct_during_merge() { + // The alias URI includes a character reference: scanning and parsing must resolve it identically. + let foreign = r#""#; + let old = format!( + r#"{foreign} + + "# + ); + let out = merge_xmp(&old, &fresh(5, &[]), RULES).unwrap(); + assert!(out.contains(foreign), "lost foreign rating: {out}"); + assert!(!out.contains("a:Rating="), "genuine alias must be replaced: {out}"); + assert_eq!(parse_xmp(&out).unwrap().metadata.rating, Some(5)); + assert_eq!(merge_xmp(&out, &fresh(5, &[]), RULES).unwrap(), out); + } + + #[test] + fn opaque_namespace_expansion_is_bounded() { + // A roughly 40 KiB packet must not expand the same long URI into every property name indefinitely. + let packet = format!( + r#"{}"#, + "a".repeat(1024), + "".repeat(5000) + ); + assert_eq!(parse_xmp(&packet).unwrap_err(), XmpError::TooComplex); + assert!(matches!(scan(&packet), Err(XmpError::TooComplex))); + } + #[test] fn many_attributes_scan_in_linear_time() { // quick-xml's duplicate-name check compares each attribute with all earlier ones: 200 000 attributes on diff --git a/docs/gaps.md b/docs/gaps.md index be411a205..4c7223599 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps: where LightCraft falls short of Lightroom -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-21 · **Change:** minor (gap 28: F8 shows / hides the right panel; import review album hierarchy, #2362; gap 20: Minolta MRW decodes; gap 10: 551 models verified; gap 15: Print Page Options — page numbers, page info, crop marks; gap 15: Print Page Options — page numbers, page info, crop marks; gap 28: `\` shows / hides the Filter Bar in the grids; gap 21: Indonesian interface shipped, every message but What's New; gap 27: the Windows x64 installer verified on Windows 11; the Settings → Apps uninstall confirmation remains) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-22 · **Change:** patch (XMP namespace-prefix collision regression fixed; ranked gaps unchanged) Every known shortfall, one entry each, ranked by user impact (how many Lightroom users hit it, and how hard it blocks them), then by effort. **Agents pick work from the top** (the first entry nobody is @@ -60,6 +60,8 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w ## Closed recently (kept for calibration) +- 2026-10-22: saving XMP preserves foreign namespaces even when they use `lc:` or `xmp:` prefixes; importing them no longer interprets their properties as native settings or ratings. Parser, merge and save-command regressions cover the correction; no ranked gap or readiness estimate changes. + - 2026-10-20: the import review album picker follows the sidebar folder hierarchy and searches album paths (#2362, LR-IMP-ADD-DIALOG), retaining None/New album and existing destination IDs. This closes a usability subgap; no ranked gap or headline feature count closes. - 2026-10-12: original-pixel detail at 100% (gap 4, LR-VIEW-ZOOM, #323): soft proofing gets zoom windows like @@ -79,6 +81,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w | Date | Change | Summary | |---|---|---| +| 2026-10-22 | patch | XMP namespace-prefix collisions no longer overwrite foreign metadata; ranked gaps and estimates unchanged | | 2026-10-21 | minor | Gap 28 shrinks: F8 shows / hides the right panel (`view.rightPanel`); rating `[` `]`, flag cycling and F1 help were already bound, so the entry lists only filter-bar keys, Tab / ⇧Tab and F5; 2–4 h (was 4–8) | | 2026-10-20 | minor | #2362: import destination albums use the shared hierarchy/search picker instead of a flat name list; existing import workflows retained; ranked gaps/readiness counts unchanged | | 2026-10-19 | minor | Gap 21: Indonesian (`id`) shipped with every catalog message but What's New; four new languages left (Hindi, Arabic, Korean, Vietnamese), 135–239 h | diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 99f17561e..58dafb43b 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** minor (LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-22 · **Change:** patch (LR-META-XMP: foreign namespaces cannot impersonate recognized prefixes) · **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 @@ -168,7 +168,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` | manual rereading also reads sidecar-free JPEG embedded XMP (first complete standard APP1 before SOS/EOI, within 16 MiB of headers and 4096 marker/fill steps; no extended assembly or between-scan packets, not strict whole-JPEG validation; reader cases in `crates/engine/src/sidecar.rs`); a discovered unreadable sidecar fails without embedded fallback; procedural refresh/undo, non-UTF-8 sidecar and two explicit malformed fixtures in `crates/engine/src/tests_xmp.rs` (file-write checks use auto-write off); 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); settings Lightroom writes only for itself (`CompatibleVersion`, the profile digests, the lens profile's name / file / setup, `CropConstrainToUnitSquare`, the lens profile's strength at 100 %) are not reported as unimported (`crates/engine/src/crs.rs`) | +| 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` | manual rereading also reads sidecar-free JPEG embedded XMP (first complete standard APP1 before SOS/EOI, within 16 MiB of headers and 4096 marker/fill steps; no extended assembly or between-scan packets, not strict whole-JPEG validation; reader cases in `crates/engine/src/sidecar.rs`); a discovered unreadable sidecar fails without embedded fallback; procedural refresh/undo, non-UTF-8 sidecar and two explicit malformed fixtures in `crates/engine/src/tests_xmp.rs` (file-write checks use auto-write off); 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 (ownership follows namespace URI: foreign namespaces using `lc:` or `xmp:` cannot supply native settings or ratings and survive saving), 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); settings Lightroom writes only for itself (`CompatibleVersion`, the profile digests, the lens profile's name / file / setup, `CropConstrainToUnitSquare`, the lens profile's strength at 100 %) are not reported as unimported (`crates/engine/src/crs.rs`) | | 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` | `photo.allMetadata` reuses loaded original bytes for embedded XMP and keeps sidecar precedence (`crates/engine/src/tests_xmp.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) @@ -798,6 +798,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-22 | patch | LR-META-XMP: namespace URI determines ownership; foreign namespaces using recognized prefixes survive saves and cannot supply native settings or ratings; status and counts unchanged | | 2026-10-21 | patch | LR-IMP-ADD-DIALOG: reconcile #2362 with current main; deleted-album guard and hierarchical picker retained; counts unchanged | | 2026-10-20 | minor | LR-IMP-ADD-DIALOG: #2362 gives the import review the shared hierarchy/search album picker while retaining None/New album and existing destination IDs; status and feature counts unchanged | | 2026-10-21 | patch | KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the slider's row; leaving it (even between two frames) or Tab focus alone does not arm them (issue #1355); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..52835604e 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,11 @@ # What's new in LightCraft +## 2026-10-22 + +### Foreign XMP metadata survives namespace-prefix collisions +- Saving a sidecar keeps another application's properties even when its namespace uses prefixes such as `lc` + or `xmp`. Those foreign properties no longer become LightCraft settings or ratings when imported. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges diff --git a/docs/xmp-interop.md b/docs/xmp-interop.md index b90e76c40..7fd11a172 100644 --- a/docs/xmp-interop.md +++ b/docs/xmp-interop.md @@ -81,6 +81,9 @@ the only copy of those edits outside that application's catalog. Saving never dr ones), `xmp:CreatorTool`, comments, the packet wrapper and padding. LightCraft's properties go into one `rdf:Description` of their own; owned properties written by another application (in attribute or element form) are removed from its description, which otherwise stays as it was. +- Ownership follows the namespace URI, not the prefix spelling. A foreign namespace bound to `lc:` or `xmp:` + stays foreign: its properties are preserved and do not become LightCraft settings or ratings. Genuine namespace + aliases remain interchangeable, including namespace declarations containing XML character references. - A sidecar that can't be read as XMP (not well-formed, not UTF-8, no `rdf:RDF`) is first copied to `.xmp.bak-