mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-01 05:57:09 -06:00
Merge PR #2849: LR-EXP-METADATA: refuse Originals with embedded place names
crates/engine/src/export.rs: main moved the code to export/prepare.rs; hunk ported there by hand docs/gaps.md, docs/parity-checklist.md: header keeps the newer line, revision rows are the union
This commit is contained in:
aiongg
@@ -227,9 +227,22 @@ impl PreparedExport {
|
||||
if remove_location {
|
||||
let metadata = lightcraft_meta::try_extract(&bytes)
|
||||
.map_err(|e| format!("{label}: can't verify location metadata in this original file: {e}"))?;
|
||||
if metadata.gps.is_some() {
|
||||
if !bytes.starts_with(b"8BPS") {
|
||||
let embedded = lightcraft_meta::embedded(&bytes);
|
||||
if let Some(xmp) = embedded.xmp.as_deref() {
|
||||
lightcraft_meta::parse_xmp(xmp).map_err(|e| {
|
||||
format!("{label}: can't verify location metadata in this original file: malformed embedded XMP: {e}")
|
||||
})?;
|
||||
}
|
||||
}
|
||||
let embeds_place_name =
|
||||
[metadata.sublocation.as_deref(), metadata.city.as_deref(), metadata.state.as_deref(), metadata.country.as_deref()]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.any(|value| !value.trim().is_empty());
|
||||
if metadata.gps.is_some() || embeds_place_name {
|
||||
return Err(format!(
|
||||
"{label}: can't remove the location from an original file that embeds GPS; export as JPEG, TIFF or DNG instead"
|
||||
"{label}: can't remove the location from an original file that embeds GPS or place names; export as JPEG or TIFF, or as DNG for a raw photo"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,14 @@ use crate::Session;
|
||||
use crate::export::{ExportFormat, ExportOptions, export_photo};
|
||||
use crate::tests_xmp::{synthetic_dng_with, synthetic_psd_with_metadata, temp_dir};
|
||||
|
||||
fn jpeg_with_xmp(metadata: &lightcraft_meta::Metadata) -> Vec<u8> {
|
||||
let pixels = [96u8; 12];
|
||||
let image = lightcraft_codecs::EncodeImage::new(2, 2, 3, lightcraft_codecs::Samples::U8(&pixels));
|
||||
let xmp = lightcraft_meta::write_xmp(metadata, None);
|
||||
let encoded = lightcraft_codecs::EncodeMeta { xmp: Some(&xmp), ..Default::default() };
|
||||
lightcraft_codecs::encode_jpeg(&image, 90, lightcraft_codecs::ChromaSubsampling::S444, &encoded).unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn full_size_export_of_a_cropped_photo_is_not_upscaled() {
|
||||
let mut s = Session::with_demo();
|
||||
@@ -640,6 +648,168 @@ fn remove_location_holds_for_original_and_dng_exports() {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_location_refuses_original_with_embedded_place_names() {
|
||||
let dir = temp_dir("export-noloc-place-fields");
|
||||
let value = "Embedded Place";
|
||||
let cases = [
|
||||
("sublocation", lightcraft_meta::Metadata { sublocation: Some(value.into()), ..Default::default() }),
|
||||
("city", lightcraft_meta::Metadata { city: Some(value.into()), ..Default::default() }),
|
||||
("state", lightcraft_meta::Metadata { state: Some(value.into()), ..Default::default() }),
|
||||
("country", lightcraft_meta::Metadata { country: Some(value.into()), ..Default::default() }),
|
||||
];
|
||||
let mut paths = Vec::new();
|
||||
for (name, metadata) in &cases {
|
||||
let path = dir.join(format!("{name}.jpg"));
|
||||
std::fs::write(&path, jpeg_with_xmp(metadata)).unwrap();
|
||||
paths.push(path.to_string_lossy().to_string());
|
||||
}
|
||||
|
||||
let mut session = Session::new().with_fs();
|
||||
session.execute("library.import", &json!({"paths": paths})).unwrap();
|
||||
let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() };
|
||||
for (name, _) in cases {
|
||||
let photo_id = session.catalog.photos().find(|p| p.file_name == format!("{name}.jpg")).unwrap().id;
|
||||
let original = std::fs::read(dir.join(format!("{name}.jpg"))).unwrap();
|
||||
let embedded = lightcraft_meta::try_extract(&original).unwrap();
|
||||
assert!(embedded.gps.is_none(), "{name} fixture should isolate place names");
|
||||
let place_name = match name {
|
||||
"sublocation" => embedded.sublocation.as_deref(),
|
||||
"city" => embedded.city.as_deref(),
|
||||
"state" => embedded.state.as_deref(),
|
||||
"country" => embedded.country.as_deref(),
|
||||
_ => None,
|
||||
};
|
||||
assert_eq!(place_name, Some(value), "{name} fixture should be parsed");
|
||||
let error = export_photo(&mut session, photo_id, &options, 1).err().expect("embedded place name should refuse Original export");
|
||||
assert!(error.contains("place names") && error.contains("export as JPEG or TIFF"), "{name}: {error}");
|
||||
}
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_location_refuses_original_with_unreadable_non_psd_xmp() {
|
||||
let dir = temp_dir("export-noloc-malformed-xmp");
|
||||
let path = dir.join("Unreadable.jpg");
|
||||
let pixels = [96u8; 12];
|
||||
let image = lightcraft_codecs::EncodeImage::new(2, 2, 3, lightcraft_codecs::Samples::U8(&pixels));
|
||||
let xmp = "<x:xmpmeta><broken";
|
||||
let encoded = lightcraft_codecs::EncodeMeta { xmp: Some(xmp), ..Default::default() };
|
||||
let bytes = lightcraft_codecs::encode_jpeg(&image, 90, lightcraft_codecs::ChromaSubsampling::S444, &encoded).unwrap();
|
||||
assert!(lightcraft_meta::parse_xmp(xmp).is_err(), "fixture XMP should be malformed");
|
||||
let metadata = lightcraft_meta::try_extract(&bytes).unwrap();
|
||||
assert!(metadata.gps.is_none() && metadata.city.is_none(), "best-effort extraction omits malformed XMP");
|
||||
std::fs::write(&path, bytes).unwrap();
|
||||
|
||||
let mut session = Session::new().with_fs();
|
||||
session.execute("library.import", &json!({"paths": [path.to_string_lossy()]})).unwrap();
|
||||
let id = session.catalog.photos().next().unwrap().id;
|
||||
let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() };
|
||||
let error = export_photo(&mut session, id, &options, 1).err().expect("unreadable XMP must not pass location removal");
|
||||
assert!(error.contains("can't verify location metadata") && error.contains("malformed embedded XMP"), "{error}");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_location_original_place_refusal_writes_no_files() {
|
||||
use crate::export::{Destination, prepare_batch, run_batch};
|
||||
|
||||
let dir = temp_dir("export-noloc-place-no-write");
|
||||
let source = dir.join("City.jpg");
|
||||
let original = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some("Embedded Place".into()), ..Default::default() });
|
||||
std::fs::write(&source, &original).unwrap();
|
||||
let output = dir.join("out");
|
||||
std::fs::create_dir_all(&output).unwrap();
|
||||
|
||||
let mut session = Session::new().with_fs();
|
||||
session.execute("library.import", &json!({"paths": [source.to_string_lossy()]})).unwrap();
|
||||
let id = session.catalog.photos().next().unwrap().id;
|
||||
let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() };
|
||||
let items = prepare_batch(&mut session, &[id], &options).unwrap();
|
||||
let destination = Destination { dir: output.to_string_lossy().to_string(), exact: None };
|
||||
let mut writes = Vec::new();
|
||||
let mut write = |path: &str, bytes: &[u8]| -> Result<(), String> {
|
||||
writes.push(path.to_string());
|
||||
std::fs::write(path, bytes).map_err(|error| error.to_string())
|
||||
};
|
||||
let exists = |path: &str| std::path::Path::new(path).exists();
|
||||
let result = run_batch(items, &options, &destination, &mut write, &exists, true, &mut |_, _| true);
|
||||
|
||||
let error = result.unwrap_err();
|
||||
assert!(error.contains("place names"), "{error}");
|
||||
assert!(writes.is_empty(), "refused export called the writer: {writes:?}");
|
||||
assert!(!output.join("City.jpg").exists());
|
||||
assert!(!output.join("City.xmp").exists());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_location_checks_embedded_places_after_catalog_clear() {
|
||||
let dir = temp_dir("export-noloc-place-catalog-clear");
|
||||
let source = dir.join("City.jpg");
|
||||
let original = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some("Embedded Place".into()), ..Default::default() });
|
||||
std::fs::write(&source, &original).unwrap();
|
||||
|
||||
let mut session = Session::new().with_fs();
|
||||
session.execute("library.import", &json!({"paths": [source.to_string_lossy()]})).unwrap();
|
||||
let id = session.catalog.photos().next().unwrap().id;
|
||||
assert_eq!(session.catalog.photo(id).unwrap().meta.city, "Embedded Place");
|
||||
session.execute("photo.setMeta", &json!({"ids": [id.0], "city": ""})).unwrap();
|
||||
assert!(session.catalog.photo(id).unwrap().meta.city.is_empty());
|
||||
assert_eq!(lightcraft_meta::try_extract(&std::fs::read(&source).unwrap()).unwrap().city.as_deref(), Some("Embedded Place"));
|
||||
|
||||
let options = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() };
|
||||
let error = export_photo(&mut session, id, &options, 1).err().expect("embedded place name should refuse Original export");
|
||||
assert!(error.contains("place names"), "{error}");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_location_allows_original_without_embedded_location() {
|
||||
let dir = temp_dir("export-noloc-place-controls");
|
||||
let embedded_path = dir.join("Embedded.jpg");
|
||||
let no_location_path = dir.join("NoLocation.jpg");
|
||||
let blank_path = dir.join("Blank.jpg");
|
||||
let embedded = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some("Embedded Place".into()), ..Default::default() });
|
||||
let no_location = jpeg_with_xmp(&Default::default());
|
||||
let blank = jpeg_with_xmp(&lightcraft_meta::Metadata { city: Some(" \t".into()), ..Default::default() });
|
||||
std::fs::write(&embedded_path, &embedded).unwrap();
|
||||
std::fs::write(&no_location_path, &no_location).unwrap();
|
||||
std::fs::write(&blank_path, &blank).unwrap();
|
||||
assert!(lightcraft_meta::try_extract(&blank).unwrap().city.is_none(), "whitespace-only XMP place names are blank");
|
||||
|
||||
let mut session = Session::new().with_fs();
|
||||
session
|
||||
.execute(
|
||||
"library.import",
|
||||
&json!({"paths": [embedded_path.to_string_lossy(), no_location_path.to_string_lossy(), blank_path.to_string_lossy()]}),
|
||||
)
|
||||
.unwrap();
|
||||
let photo_id = |name: &str| session.catalog.photos().find(|p| p.file_name == name).unwrap().id;
|
||||
let embedded_id = photo_id("Embedded.jpg");
|
||||
let no_location_id = photo_id("NoLocation.jpg");
|
||||
let blank_id = photo_id("Blank.jpg");
|
||||
|
||||
let keep = ExportOptions { format: ExportFormat::Original, ..Default::default() };
|
||||
let kept = export_photo(&mut session, embedded_id, &keep, 1).unwrap();
|
||||
assert_eq!(kept.bytes, embedded, "removeLocation=false still copies the source byte for byte");
|
||||
assert_eq!(lightcraft_meta::try_extract(&kept.bytes).unwrap().city.as_deref(), Some("Embedded Place"));
|
||||
|
||||
let remove = ExportOptions { format: ExportFormat::Original, remove_location: true, ..Default::default() };
|
||||
for (id, source) in [(no_location_id, &no_location), (blank_id, &blank)] {
|
||||
let exported = export_photo(&mut session, id, &remove, 1).unwrap();
|
||||
assert_eq!(&exported.bytes, source);
|
||||
}
|
||||
|
||||
session.execute("photo.setMeta", &json!({"ids": [no_location_id.0], "city": "Catalog-only Place"})).unwrap();
|
||||
let catalog_only = export_photo(&mut session, no_location_id, &remove, 1).unwrap();
|
||||
let sidecar = catalog_only.sidecars.first().unwrap().1.as_slice();
|
||||
let packet = std::str::from_utf8(sidecar).unwrap();
|
||||
assert!(lightcraft_meta::parse_xmp(packet).unwrap().metadata.city.is_none(), "catalog-only place name is removed from the sidecar");
|
||||
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// `metadata: "NONE"` as `app.export` sends it: the JPEG carries no location or camera data
|
||||
/// (it used to be read as `all` and carry both).
|
||||
#[test]
|
||||
|
||||
@@ -87,6 +87,7 @@ Beta-blocking gaps (B) total **~230–430 h**; with the stability and platform w
|
||||
| 2026-10-24 | minor | Gap 8: lens profile work is tracked in #329; hours unchanged |
|
||||
| 2026-10-24 | minor | Gap 28: ⌘⌥⇧C / ⌘⌥⇧V (Ctrl+Alt+Shift+C / V on Windows and Linux) copy and paste metadata; no key leaves the entry; hours unchanged |
|
||||
| 2026-10-24 | minor | Gap 30 shrinks: the Info panel's Show Metadata For (Target Photo / Selected Photos, `view.infoSelectedPhotos`) shows the selected photos' shared values and marks fields that differ as Mixed; the All Metadata dialog remains; 1–3 h (was 4–8) |
|
||||
| 2026-10-24 | patch | LR-EXP-METADATA: Original export refuses embedded GPS or nonblank place names when Remove Location Info is on; ranked gaps, estimates and readiness unchanged |
|
||||
| 2026-10-23 | minor | Gap 13: before a Lightroom catalog import the app asks whether it merges into the open library or goes into a new one (opt-out in Settings, #2672); hours unchanged |
|
||||
| 2026-10-22 | patch | Gap 28: View shows the current grid-cycle key beside Photo Grid and Square Grid (#2500); no new binding and no change to the remaining shortcut gap or hours |
|
||||
| 2026-10-22 | minor | Gap 15: the Print page setup and PDF gain the Japanese photo and postcard paper sizes (L判 89×127, 2L判 127×178, KG判 102×152, はがき 100×148, 往復はがき 148×200, 六切 203×254, 四切 254×305, 名刺 55×91 mm), by their customary names; gap not closed |
|
||||
|
||||
@@ -398,7 +398,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
|
||||
| LR-EXP-COLORSPACE | Output colour space | P0 | ✅ | `cmd:app.export` (`colorSpace`), `crates/pipeline/src/output.rs`, `crates/engine/src/export.rs` | sRGB, Display P3, Adobe RGB (1998) compatible, ProPhoto RGB, Rec. 2020: rendered from the working space with gamut mapping into the target gamut (CPU + GPU), own ICC profile embedded; AVIF stays sRGB (muxer has no ICC); lossy JPEG XL declares sRGB / P3 / Rec. 2020 (Adobe RGB and ProPhoto written as Rec. 2020), lossless JPEG XL embeds the profile |
|
||||
| LR-EXP-HDR | HDR output | P2 | 🟡 | `cmd:app.export`, `crates/ui-egui/src/dialogs.rs` | HDR output checkbox in the export dialog: gain map JPEG, PQ or HLG AVIF, PQ PNG, PQ JPEG XL, float TIFF; no HDR presets yet |
|
||||
| LR-EXP-SHARPEN | Output sharpening | P1 | ✅ | `cmd:app.export` (`sharpen`, `sharpenAmount`) | |
|
||||
| LR-EXP-METADATA | Metadata policy | P1 | ✅ | `cmd:app.export` (`metadata`, `removeLocation`) | |
|
||||
| LR-EXP-METADATA | Metadata policy | P1 | ✅ | `cmd:app.export` (`metadata`, `removeLocation`) | Original export is refused when Remove Location Info is on and the source embeds GPS or a nonblank sublocation, city, state or country; rendered exports and XMP sidecars remove those modeled fields |
|
||||
| LR-EXP-WATERMARK | Watermark | P1 | 🟡 | `cmd:app.export` (`watermark`: text, or `{text, vertical, size, opacity, anchor, insetX, insetY, inset, rotation, color, shadow, image, imageWidth}`), `crates/engine/src/export.rs` (`Watermark`, `WATERMARK_PARAMS`) | text uses Inter SemiBold, the existing Mincho-first CJK chain, then Thai faces from `CRAFT_FONTS_DIR`, shared with contact-sheet captions and slideshow overlays (#2052, partial). Horizontal Thai mark positioning remains incomplete. Arabic export fallback is deferred until joining and bidi reordering exist; UI font fallback is unchanged. Text (`size` = height as a fraction of the short edge, 0.005..0.5, default 0.035; `color` sRGB `[r,g,b]`; `shadow`; Japanese vertical glyph forms and origins, tested by `biz_ud_vertical_watermarks_place_punctuation_at_the_top_right`; extended grapheme cells keep decomposed dakuten/handakuten and Latin accents together, tested by `vertical_watermarks_keep_combining_marks_in_one_cell` and `upright_watermark_cell_retains_multiple_glyphs` (see `docs/vertical-watermarks.md`)) or a graphic with transparency (EXIF orientation applied before sizing; oversized graphics fit the rotated image bounds without stretching, tested by `graphic_watermark_honours_exif_orientation` and `graphic_watermark_keeps_aspect_when_fitted_and_rotated`; `image` path, `imageWidth` = fraction of the photo's width, 0.01..1, converted to the output colour space); `anchor` (nine positions), `insetX` / `insetY` (margins from the side and from the top/bottom edges, each a fraction of the short edge, 0..0.4; the Export dialog's Horizontal and Vertical inset sliders, as in Lightroom Classic; the older single `inset` still reads as both; tested by `watermark_insets_are_horizontal_and_vertical`), `rotation` (0, 90, 180 or 270 degrees clockwise, the Export dialog's Rotate row, as Lightroom Classic's rotate buttons; sized before it is turned, placed and inset by the turned box; tested by `watermark_rotation_turns_the_text_in_its_corner` and `graphic_watermark_is_placed_scaled_and_blended`), `opacity` 0..1. Unknown keys and out-of-range values are errors (issue #183). The Export dialog previews it on the first exported photo's thumbnail (`draw_watermark_preview` scales the 6 px minimum text size to the preview so it matches the file), tested by `watermark_preview_scales_the_minimum_text_size` and `export_dialog_previews_the_watermark` |
|
||||
| LR-EXP-NAMING | File naming | P1 | ✅ | `cmd:app.export` (`naming`, `startNumber`), `crates/engine/src/rename.rs` (`expand_tokens`) | one template language for export, Rename Photos and import renaming: `{name}`, `{num}` (number at the end of the name), `{seq}` / `{seq:N}`, `{date}` / `{date:%Y-%m-%d}`, `{folder}`, `{camera}`, `{lens}`, `{iso}`, `{rating}`, `{title}`, `{creator}`, `{ext}`; free-form template rather than a list of named schemes |
|
||||
| LR-EXP-LOCATION | Destination folder | P0 | ✅ | `cmd:app.export` (`dir`, `destinationMode`: specificFolder / sameAsOriginal, `subfolder`, `conflict`: unique / overwrite / skip), `cmd:export.checkTarget`, `crates/ui-egui/src/dialogs.rs` (Choose… folder picker), `crates/engine/src/originals.rs`, `crates/catalog/src/safe_file.rs` | folder picker on the desktop; Export to: Specific folder or Same folder as original photo (each photo into its original's folder + the subfolder; a demo/browser photo or a missing folder is that photo's error, issue #956); existing files get `-2`, `-3`… by default; the policy covers a file and its sidecars together; a catalogued original (or its sidecar) is never written over, whatever the policy or an exact `path` (export, `ui.render`, MCP `render_photo`, merge `previewPath`, `lightcraft-cli render`); files are written to a temp file and renamed into place (issue #93; not synced file by file — exports can be made again, issue #134) |
|
||||
@@ -802,6 +802,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove
|
||||
| 2026-10-25 | patch | LR-BEHAV-TITLEBAR: the top bar's title-bar gestures come from craft-window's `title_bar`: a double-click follows the system setting, and a press on a button, the search field or a panel floating over the bar never moves the window; status counts unchanged |
|
||||
| 2026-10-24 | minor | KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 (`keyword.nextSet`, `keyword.previousSet`) go to the next / previous keyword set; status counts unchanged (KEYC-METADATA stays 🟡) |
|
||||
| 2026-10-24 | minor | KEYC-METADATA: ⌘K / Ctrl+K (`panel.addKeywords`, Add Keywords) shows the Keywords panel, opens Keywording and puts the cursor in the Add keyword box; status counts unchanged (KEYC-METADATA stays 🟡) |
|
||||
| 2026-10-24 | patch | LR-EXP-METADATA: Original export refuses embedded GPS or nonblank place names when Remove Location Info is on; status and feature counts unchanged |
|
||||
| 2026-10-24 | patch | LR-LIB-RATING, LR-LIB-DELETE: a slider drag still open when a rating, flag or label takes the photo out of the view, or when the photo is deleted, ends as its own undo step before the selection moves on, as moving to the next photo does; LR-BEHAV-LOCALIZE lists all 18 languages; status unchanged |
|
||||
| 2026-10-24 | patch | Integration batch 43: Integrated main 98702b5c, preserving the new fail-closed GPS metadata check for Original export. Watermark geometry/orientation and HDR JPEG size-limit fixes remain in their shared helpers; no feature-count changes. |
|
||||
| 2026-10-23 | patch | LR-MERGE-HDR / LR-MERGE-PANO / LR-MERGE-HDRPANO: the merge dialog makes a new preview each time it opens, also after a failed one (#2708); status unchanged |
|
||||
|
||||
@@ -13,6 +13,9 @@
|
||||
|
||||
## 2026-10-24
|
||||
|
||||
### Original export checks embedded place names when removing location
|
||||
- Original copies are refused when “Remove Location Info” is enabled and the source contains GPS or a nonblank sublocation, city, state or country. Use JPEG or TIFF for rendered output; DNG is available for raw photos.
|
||||
|
||||
### Relative-date filters and smart albums follow the clock
|
||||
- A current view or source count using “in the last” rules now refreshes when the session clock moves into a new
|
||||
minute, even when no catalog edit occurred. This fixes stale photos remaining in the grid and stale counts for
|
||||
|
||||
Reference in New Issue
Block a user