diff --git a/crates/engine/src/cmd/geocode.rs b/crates/engine/src/cmd/geocode.rs index 085183d58..c9ffb6b7e 100644 --- a/crates/engine/src/cmd/geocode.rs +++ b/crates/engine/src/cmd/geocode.rs @@ -115,6 +115,12 @@ fn fill(meta: &mut lightcraft_catalog::Meta, place: &lightcraft_geocode::Place, fn reverse(s: &mut Session, p: &Value) -> Result { const C: &str = "metadata.reverseGeocode"; let overwrite = bool_or(p, "overwrite", false); + // the place list holds towns of 15 000 people or more: a photo in a smaller one finds the nearest + // larger town, which may be another municipality; past this many metres City is left empty (#2242) + let max_distance = match p.get("maxDistance") { + None | Some(Value::Null) => None, + Some(v) => Some(v.as_f64().filter(|m| m.is_finite() && *m > 0.0).ok_or_else(|| bad(C, "`maxDistance` must be a positive number of metres"))?), + }; let ids = s.targets(p)?; let todo: Vec<(PhotoId, LatLon)> = ids .iter() @@ -140,14 +146,24 @@ fn reverse(s: &mut Session, p: &Value) -> Result { } t.progress(n as u64, todo.len() as u64); } - let Some(place) = names.reverse(*at) else { continue }; + let Some(mut place) = names.reverse(*at) else { continue }; + // too far from the town found: State and Country still hold, City (and below) doesn't + let too_far = max_distance.is_some_and(|m| place.distance > m); + let nearest_city = too_far.then(|| std::mem::take(&mut place.city)); + if too_far { + place.sublocation.clear(); + } let Some(ph) = s.catalog.photo(*id) else { continue }; let mut meta = ph.meta.clone(); fill(&mut meta, &place, overwrite); if meta != ph.meta { ops.push(Op::SetMeta { id: *id, meta: Box::new(meta) }); } - found.push(json!({"id": id.0, "place": place})); + let mut entry = json!({"id": id.0, "place": place}); + if let Some(city) = nearest_city { + entry["nearestCity"] = json!(city); + } + found.push(entry); } drop(task); let changed = ops.len(); @@ -271,7 +287,7 @@ pub fn specs() -> Vec { "Fill Location from GPS", ["Photo"], None, - "{ids?, overwrite?: bool (also replace place fields that already have text; default false: only empty ones are filled), dryRun?, dataset?: folder} → {looked, found, changed, photos: [{id, place: {sublocation, city, state, country, iso, distance}}], attribution} — fills City / State / Country (and Sublocation when known) from GPS with the offline GeoNames place names (download them first); one undo step", + "{ids?, overwrite?: bool (also replace place fields that already have text; default false: only empty ones are filled), dryRun?, dataset?: folder, maxDistance?: metres (past this distance from the town found, City is left empty and the town is given as nearestCity)} → {looked, found, changed, photos: [{id, place: {sublocation, city, state, country, iso, distance}, nearestCity?}], attribution} — fills City / State / Country (and Sublocation when known) from GPS with the offline GeoNames place names (download them first); one undo step. The place names list towns of 15 000 people or more (GeoNames cities15000): a photo taken in a smaller town gets the nearest larger one, up to 50 km away (`distance` says how far); `maxDistance` keeps such a town out of City", has_selection, reverse ), @@ -343,6 +359,36 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Issue #2242: the place names hold only towns of 15 000 people or more, so a photo in a + /// village finds the nearest larger town. `maxDistance` leaves City empty past that distance + /// (State and Country still fill) and says which town it was. + #[test] + fn max_distance_leaves_a_far_town_out_of_city() { + let mut s = Session::with_demo(); + let ids = s.visible_cloned(); + let dir = dataset("maxdist"); + // about 5 km north of Paris (like Portofino and Rapallo in the issue), and right in Paris + set_gps(&mut s, ids[0], Some((48.90, 2.35)), ""); + set_gps(&mut s, ids[1], Some((48.855, 2.35)), ""); + let r = s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0, ids[1].0], "dataset": dir, "maxDistance": 2000})).unwrap(); + let m0 = s.catalog.photo(ids[0]).unwrap().meta.clone(); + assert_eq!((m0.city.as_str(), m0.state.as_str(), m0.country.as_str()), ("", "Île-de-France", "France")); + assert_eq!(s.catalog.photo(ids[1]).unwrap().meta.city, "Paris", "close enough"); + let far = r["photos"].as_array().unwrap().iter().find(|p| p["id"] == json!(ids[0].0)).unwrap(); + assert_eq!((far["place"]["city"].as_str(), far["nearestCity"].as_str()), (Some(""), Some("Paris")), "{far}"); + assert!(far["place"]["distance"].as_f64().unwrap() > 2000.0); + // without it, the nearest town fills City as before + s.undo_step().unwrap(); + s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0], "dataset": dir})).unwrap(); + assert_eq!(s.catalog.photo(ids[0]).unwrap().meta.city, "Paris"); + // hostile values are refused, not taken as "no limit" + for bad in [json!(-1), json!(0), json!("far"), json!(true)] { + let e = s.execute("metadata.reverseGeocode", &json!({"ids": [ids[0].0], "dataset": dir, "maxDistance": bad})).unwrap_err(); + assert!(e.to_string().contains("maxDistance"), "{e}"); + } + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn explains_missing_data_and_gps() { let mut s = Session::with_demo(); diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index 537330810..6f4c8da3f 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -771,7 +771,11 @@ fn info(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } }); if text_button(ui, "fillLocation", crate::i18n::tr("Fill Location"), false) - .on_hover_text(crate::i18n::tr("Fill empty City, State and Country from GPS (offline GeoNames place names)")) + .on_hover_text(format!( + "{}\n{}", + crate::i18n::tr("Fill empty City, State and Country from GPS (offline GeoNames place names)"), + crate::i18n::tr("The place names list towns of 15,000 people or more: in a smaller one, City gets the nearest larger town.") + )) .clicked() && let Err(e) = app.run("metadata.reverseGeocode", json!({})) { diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 4e0531793..0e8d83169 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -737,7 +737,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-MAP-LOCATIONS | Saved locations | P2 | ⬜ | | | | LRC-MAP-TRACKLOG | GPS track logs | P2 | ✅ | `cmd:photo.autoTagTracklog`, `cmd:photo.tagFromTracklog`, `crates/meta/src/gpx.rs` | Photo ▸ Auto-Tag from Tracklog…: a GPX 1.0 / 1.1 track log sets the GPS of the selected photos by capture time — interpolated between the points of a track segment, else the nearest point within `maxGap` (10 min); never across segment breaks. The camera's time zone comes from the photo (Exif offset) or is asked for; photos that already have a location keep it unless `replace`; `dryRun` previews; one undo step. No track drawn on a map (no Map module) | | LRC-MAP-FILTER | Location filter bar | P2 | ⬜ | | | -| LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); no online provider | +| LRC-MAP-REVGEO | Reverse geocoding | P2 | 🟡 | `cmd:metadata.reverseGeocode` | offline from GeoNames (City / State / Country, no sublocation); the list holds towns of 15 000 people or more, so a smaller town gets the nearest larger one (the tooltip and command help say so); `maxDistance` leaves City empty past a distance and names the town as `nearestCity` (issue #2242); no online provider | | LRC-BOOK-SETTINGS | Book settings | P2 | ⬜ | | | | LRC-BOOK-AUTOLAYOUT | Book auto layout | P2 | ⬜ | | | | LRC-BOOK-PAGE | Book pages & templates | P2 | ⬜ | | | diff --git a/docs/whats-new.md b/docs/whats-new.md index 2f02ffb91..1c5c9db60 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,13 @@ # What's new in LightCraft +## 2026-10-18 + +### Fill Location from GPS: no more neighbouring town in City +- The offline place names list towns of 15,000 people or more, so a photo taken in a smaller one got the nearest larger + town in City, sometimes another municipality (issue #2242). The Fill Location tooltip and the command help now say so, + and `metadata.reverseGeocode` takes `maxDistance` (metres): past it, City stays empty (State and Country still fill) + and the reply names the town it found as `nearestCity`. + ## 2026-10-14 ### Model downloads time out on a name that doesn't resolve