Upstream review r3: grid Shift+Up/Down, date-folder rename, small fixes

Cargo.toml

@@ -92,6 +92,7 @@ getrandom = "0.3"
 # All date/time conversion goes through chrono (no hand-rolled calendar code beyond the 400-year range extension; CLAUDE.md).
 chrono = { version = "0.4.45", default-features = false, features = ["std"] }
 rayon = "1"
+same-file = "1" # lcc-catalog reads a file's volume serial number and file index with it on Windows
 proptest = "1"
 bytemuck = { version = "1", features = ["derive"] }
 half = { version = "2", default-features = false } # f32 → f16 for the HDR loupe texture

crates/catalog/Cargo.toml

@@ -29,6 +29,9 @@ siphasher = { workspace = true }
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 redb = { workspace = true }
 
+[target.'cfg(windows)'.dependencies]
+same-file = { workspace = true }
+
 [dev-dependencies]
 lcc-fuzzkit = { workspace = true }
 lcc-geom = { workspace = true }

crates/catalog/src/safe_file.rs

@@ -23,6 +23,7 @@
 //!   replacing anything** (a hard link, which fails on a taken name; where the volume has no hard
 //!   links, an exclusive create that claims the name, then a rename over that placeholder).
 //! - [`same_file`] tells whether two paths name the same file on disk (device + inode on Unix,
+//!   volume serial number + file index on Windows,
 //!   resolved paths elsewhere), for "is this target an original?" checks.
 //! - [`open_regular`] / [`open_or_create_regular`] open a library file in place (to read, append
 //!   or cut it), never through a link planted at its name, nor a FIFO or a device (issue #2552).
@@ -389,6 +390,14 @@ pub fn same_file(a: &Path, b: &Path) -> bool {
     }
     #[cfg(not(unix))]
     {
+        // A hard link has its own path but shares the volume serial number and file index with
+        // its original. A volume that does not report them is compared by resolved path below.
+        #[cfg(windows)]
+        {
+            if ::same_file::is_same_file(a, b).unwrap_or(false) {
+                return true;
+            }
+        }
         if ma.len() != mb.len() {
             return false;
         }
@@ -611,6 +620,20 @@ mod tests {
         let _ = fs::remove_dir_all(&d);
     }
 
+    #[test]
+    fn same_file_sees_a_hard_link_under_another_name() {
+        let d = temp("same-link");
+        let p = d.join("IMG_1.jpg");
+        fs::write(&p, b"original").unwrap();
+        let linked = d.join("linked.png");
+        fs::hard_link(&p, &linked).unwrap();
+        fs::write(d.join("copy.png"), b"original").unwrap();
+        assert!(same_file(&p, &linked), "a hard link is the same file");
+        assert!(same_file(&linked, &p));
+        assert!(!same_file(&p, &d.join("copy.png")), "a copy with the same content and length is another file");
+        let _ = fs::remove_dir_all(&d);
+    }
+
     #[test]
     fn same_file_sees_through_other_spellings() {
         let d = temp("same");

crates/engine/src/cmd/browse.rs

@@ -539,7 +539,13 @@ pub(crate) fn move_folder(s: &mut Session, from: &str, to: &str) -> std::result:
     if !src.is_dir() {
         return Err(format!("{from} is not a folder"));
     }
-    if s.library.as_ref().is_some_and(|l| l.dir.starts_with(src) || src == l.dir) {
+    // `a/../b` names a folder without spelling it, which the checks below and the relink can't see
+    if src.components().any(|c| matches!(c, std::path::Component::ParentDir)) {
+        return Err(format!("{from}: give the folder's path without '..'"));
+    }
+    // the library folder itself, and its own top-level folders (`Originals`, `thumbs`), stay where
+    // the library expects them; folders below those (an import's date folders) can be renamed
+    if s.library.as_ref().is_some_and(|l| l.dir.starts_with(src) || src.parent() == Some(l.dir.as_path())) {
         return Err("the library's own folders can't be moved here".into());
     }
     // Repairs retain their exact old paths, including those on surviving history entries.

crates/engine/src/faces_index.rs

@@ -599,6 +599,8 @@ impl FacesState {
         self.in_flight.clear();
         self.retry_at = None;
         self.saved_at = None;
+        self.task = None;
+        self.peak = 0;
         self.epoch = self.epoch.wrapping_add(1);
     }
 

crates/engine/src/merge.rs

@@ -911,6 +911,8 @@ impl Session {
         info["id"] = json!(id.0);
         info["path"] = json!(path);
         info["bytes"] = json!(out.dng.len());
+        // a repeated identical merge gives the verified result it already made (no new file, edits kept)
+        info["reused"] = json!(reused);
         Ok(info)
     }
 

crates/engine/src/tests_face_recognize.rs

@@ -524,6 +524,24 @@ fn face_scan_row_follows_pending_photos() {
     let _ = std::fs::remove_dir_all(&d);
 }
 
+#[test]
+fn face_scan_row_starts_over_in_another_library() {
+    // issue #2995: the scan's whole was kept from the library opened before, so a smaller one showed most of it done
+    let d = temp("peak-library");
+    let (mut s, _) = setup(&d, 64);
+    s.faces.peak = 442_717;
+    s.open_library(d.join("other"), true).unwrap();
+    let id = s.catalog.photos().map(|p| p.id).max().unwrap();
+    set_regions(&mut s, id, vec![region(0.1, None)]);
+    let last = s.execute("faces.pump", &json!({})).unwrap();
+    let pending = last["pendingPhotos"].as_u64().unwrap();
+    assert!(pending > 0, "{last}");
+    assert_eq!(last["peak"].as_u64().unwrap(), pending, "{last}");
+    let rows = s.activity.list().into_iter().filter(|t| t.kind == "faces").collect::<Vec<_>>();
+    assert_eq!(rows.iter().map(|t| (t.total, t.done)).collect::<Vec<_>>(), vec![(pending, 0)]);
+    let _ = std::fs::remove_dir_all(&d);
+}
+
 fn face_names(s: &Session, id: PhotoId) -> Vec<Option<String>> {
     s.catalog.photo(id).unwrap().meta.regions.iter().map(|r| r.name.clone()).collect()
 }

crates/engine/src/tests_folders.rs

@@ -16,6 +16,7 @@
 
 use lcc_catalog::{Op, Photo, Source};
 use serde_json::json;
+use std::path::Path;
 
 use crate::{LibrarySource, Session, filter_chips};
 
@@ -841,3 +842,29 @@ fn a_destination_without_write_permission_reports_an_error_and_keeps_everything(
     }
     std::fs::set_permissions(dir.0.join("ro"), std::fs::Permissions::from_mode(0o755)).unwrap();
 }
+
+#[test]
+fn a_date_folder_inside_the_library_can_be_renamed_but_the_librarys_own_folders_cannot() {
+    let dir = Scratch::new("in-library");
+    let lib = dir.0.join("lib");
+    let mut s = Session::new();
+    s.open_library(&lib, false).unwrap();
+    let day = lib.join("Originals").join("2026-10-16");
+    std::fs::create_dir_all(&day).unwrap();
+    add(&mut s, &day.join("a.jpg").to_string_lossy());
+    let day = day.to_string_lossy().to_string();
+    s.execute("folder.rename", &json!({"path": day, "name": "Beach"})).unwrap();
+    assert!(lib.join("Originals/Beach").is_dir(), "the imported date folder is renamed");
+    assert!(!Path::new(&day).exists());
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(Path::new(&day).is_dir(), "undo names it back");
+    for own in ["Originals", "thumbs"] {
+        std::fs::create_dir_all(lib.join(own)).unwrap();
+        let path = lib.join(own).to_string_lossy().to_string();
+        assert!(s.execute("folder.rename", &json!({"path": path, "name": "x"})).is_err(), "{own} stays put");
+    }
+    assert!(s.execute("folder.rename", &json!({"path": lib.to_string_lossy(), "name": "x"})).is_err(), "and so does the library");
+    let alias = lib.join("Originals").join("..").join("Originals").to_string_lossy().to_string();
+    assert!(s.execute("folder.rename", &json!({"path": alias, "name": "x"})).is_err(), "even named through '..'");
+    assert!(lib.join("Originals").is_dir());
+}

crates/engine/src/tests_merge.rs

@@ -145,6 +145,7 @@ fn hdr_merge_command_creates_and_imports_a_dng() {
     let r = s.execute("merge.hdr", &json!({"ids": ids, "autoSettings": true, "stack": true})).unwrap();
     let path = r["path"].as_str().unwrap();
     assert!(path.ends_with("IMG_0-HDR.dng"), "{path}");
+    assert_eq!(r["reused"].as_bool(), Some(false));
     let id = lcc_catalog::PhotoId(r["id"].as_u64().unwrap());
     let p = s.catalog.photo(id).unwrap().clone();
     assert_eq!((p.width, p.height), (480, 320));
@@ -168,6 +169,7 @@ fn hdr_merge_command_creates_and_imports_a_dng() {
     let undo_before = s.undo.len();
     let r2 = s.execute("merge.hdr", &json!({"ids": ids, "autoSettings": true, "stack": true})).unwrap();
     assert_eq!(r2["id"].as_u64(), Some(id.0));
+    assert_eq!(r2["reused"].as_bool(), Some(true));
     assert_eq!(r2["path"].as_str(), Some(path));
     assert!(!dir.join("IMG_0-HDR-2.dng").exists(), "duplicate merge left an orphan");
     assert_eq!(s.catalog.photos().count(), 4);

crates/fetch/src/lib.rs

@@ -57,7 +57,7 @@ pub fn mirrors(env: Option<&str>, file: Option<&Path>, defaults: &[&str]) -> Vec
     let mut add = |s: &str| {
         let s = s.trim();
         // a local folder (an absolute path) is the same as its `file://` URL
-        let s = if Path::new(s).is_absolute() { format!("file://{s}") } else { s.to_string() };
+        let s = if Path::new(s).is_absolute() { format!("file://{}", s.replace('%', "%25")) } else { s.to_string() };
         let s = s.trim_end_matches('/');
         let usable = local_path(s).is_some() || Url::parse(s).is_ok();
         if !s.is_empty() && usable && !out.iter().any(|m| m == s) && out.len() < MAX_MIRRORS {
@@ -522,10 +522,29 @@ fn fetch_file(
 fn local_path(url: &str) -> Option<PathBuf> {
     let rest = url.strip_prefix("file://")?;
     let rest = if rest.get(2..3) == Some(":") { rest.strip_prefix('/').unwrap_or(rest) } else { rest };
-    let p = PathBuf::from(rest);
+    let rest = percent_decode(rest)?;
+    let p = PathBuf::from(&rest);
     if p.is_absolute() && !rest.chars().any(char::is_control) { Some(p) } else { None }
 }
 
+/// Decode `%XX` escapes (a `file://` URL's path); `None` for a broken escape or non-UTF-8 result.
+fn percent_decode(s: &str) -> Option<String> {
+    let b = s.as_bytes();
+    let mut out = Vec::with_capacity(b.len());
+    let mut i = 0;
+    while let Some(&c) = b.get(i) {
+        if c == b'%' {
+            let hex = std::str::from_utf8(b.get(i + 1..i + 3)?).ok()?;
+            out.push(u8::from_str_radix(hex, 16).ok()?);
+            i += 3;
+        } else {
+            out.push(c);
+            i += 1;
+        }
+    }
+    String::from_utf8(out).ok()
+}
+
 /// Copy a file from a local mirror into `part` (from the start; it is checked afterwards like a download).
 fn copy_local(src: &Path, part: &Path, limit: u64, size: Option<u64>, cancel: &AtomicBool, report: &mut dyn FnMut(u64, u64)) -> Result<(), Fail> {
     let disk = |e: std::io::Error| Fail::Disk(format!("{}: {e}", part.display()));

crates/fetch/src/tests.rs

@@ -491,3 +491,14 @@ fn the_overall_deadline_stops_a_trickling_server() {
     assert!(!dir.join("model.bin").exists());
     let _ = std::fs::remove_dir_all(&dir);
 }
+
+#[test]
+fn file_urls_are_percent_decoded_but_bare_paths_keep_their_percent_signs() {
+    let bare = mirrors(Some("/models/50%20off"), None, &[]);
+    assert_eq!(bare.len(), 1);
+    assert_eq!(local_path(&bare[0]), Some(std::path::PathBuf::from("/models/50%20off")));
+    assert_eq!(local_path("file:///models/my%20sam3"), Some(std::path::PathBuf::from("/models/my sam3")));
+    for hostile in ["file:///x%", "file:///x%2", "file:///x%zz", "file:///x%ff", "file:///x%0a"] {
+        assert_eq!(local_path(hostile), None, "{hostile}");
+    }
+}

crates/lensdb/src/lib.rs

@@ -161,20 +161,24 @@ impl LensDb {
     /// The camera with this EXIF make and model.
     pub fn find_camera(&self, make: &str, model: &str) -> Option<&Camera> {
         let mk = tokens(make);
-        let md = strip_maker(tokens(model), &mk);
+        let raw = tokens(model);
+        let md = strip_maker(raw.clone(), &mk);
         if md.is_empty() {
             return None;
         }
         self.cameras.iter().find(|c| {
             let cm = tokens(&c.maker);
+            // with no EXIF make, the model may still start with the maker's word
+            let md = if mk.is_empty() { strip_maker(raw.clone(), &cm) } else { md.clone() };
             maker_matches(&cm, &mk) && strip_maker(tokens(&c.model), &cm) == md
         })
     }
 
     /// The best profile for an EXIF lens model (and lens make, when the file has one), preferring lenses for the
     /// camera's mount. `None` when no name is a convincing match.
     pub fn find_lens(&self, lens_make: Option<&str>, lens_model: &str, camera: Option<&Camera>) -> Option<&Profile> {
-        self.best_lens(lens_make, lens_model, camera, |_| true)
+        // a compact camera's built-in lens is named after the camera, not the EXIF focal range
+        self.best_lens(lens_make, lens_model, camera, |_| true).or_else(|| self.find_fixed_lens(&tokens(lens_model), camera?))
     }
 
     /// [`find_lens`](Self::find_lens) among the profiles one of whose names is exactly `name`
@@ -207,10 +211,7 @@ impl LensDb {
                 best = Some((score, lens));
             }
         }
-        match best {
-            Some((_, l)) => Some(l),
-            None => self.find_fixed_lens(&exif, camera?),
-        }
+        best.map(|(_, l)| l)
     }
 
     /// Compact cameras with a built-in lens: lensfun names the lens after the camera and gives it the camera's own
@@ -234,6 +235,9 @@ impl LensDb {
             return (cands.len() == 1).then(|| cands[0]);
         };
         let (lo, hi) = (lo * 0.9, hi * 1.1);
+        if !lo.is_finite() || !hi.is_finite() {
+            return None;
+        }
         let mut best: Option<(f64, &Profile)> = None;
         for p in cands {
             let focals: Vec<f64> =
@@ -263,7 +267,7 @@ fn strip_maker(model: Vec<String>, maker: &[String]) -> Vec<String> {
 /// The numbers before the first "mm" in tokenised EXIF lens text: (shortest, longest) focal length.
 fn exif_focal_range(exif: &[String]) -> Option<(f64, f64)> {
     let end = exif.iter().position(|t| t == "mm")?;
-    let nums: Vec<f64> = exif.get(..end)?.iter().filter_map(|t| t.parse::<f64>().ok()).filter(|v| *v > 0.0).collect();
+    let nums: Vec<f64> = exif.get(..end)?.iter().filter_map(|t| t.parse::<f64>().ok()).filter(|v| v.is_finite() && *v > 0.0).collect();
     let lo = nums.iter().copied().fold(f64::INFINITY, f64::min);
     let hi = nums.iter().copied().fold(0.0, f64::max);
     (lo.is_finite() && hi > 0.0).then_some((lo, hi))

crates/lensdb/src/tests.rs

@@ -251,6 +251,14 @@ fn built_in_lens_matches_by_camera_mount_and_focal_range() {
     let l = db.find_lens(None, "6.0-72.0 mm f/2.7-3.5", cam).unwrap();
     assert_eq!(l.mounts, ["canonS5"]);
     assert!(db.find_lens(None, "6.0-72.0 mm f/2.7-3.5", None).is_none());
+    // a file with no make still finds the camera by its full model name
+    assert!(db.find_camera("", "Canon PowerShot S5 IS").is_some());
+    // the exact-name lookup never falls back to the camera's mount
+    assert!(db.find_lens_named(None, "6.0-72.0 mm f/2.7-3.5", cam).is_none());
+    // a focal length that overflows to infinity is ignored, not taken as an unbounded range
+    let huge = format!("6.0-{}9 mm f/2.7-3.5", "9".repeat(400));
+    assert_eq!(db.find_lens(None, &huge, cam).unwrap().mounts, ["canonS5"]);
+    assert_eq!(exif_focal_range(&tokens(&huge)), Some((6.0, 6.0)));
 }
 
 /// A hand-edited or corrupt database may carry any number: infinities, NaN, enormous or negative

crates/pipeline/src/geometry.rs

@@ -92,7 +92,7 @@ impl Frame {
     /// corrections (profile, embedded camera warp, manual distortion, lateral CA) always do this, as
     /// a corrected lens never shows the blank past the photo's edge (issue #245); perspective only
     /// with "Constrain Crop" (`with_persp`), so a keystone fix can keep its corners.
-    fn constrain_to_image(&mut self, with_persp: bool) {
+    pub(crate) fn constrain_to_image(&mut self, with_persp: bool) {
         let Some(full) = self.warp.as_ref().filter(|w| w.moves_pixels()) else { return };
         let optics;
         let wp = if with_persp || full.persp == Homography::IDENTITY {

crates/pipeline/src/lib.rs

@@ -296,6 +296,10 @@ pub fn frame_for(src: &Rgb32f, info: &SourceInfo, s: &DevelopSettings, apply_cro
     let mut frame = geometry::Frame::with_lens(src.width, src.height, s, apply_crop, info.lens.as_ref());
     if s.optics.remove_ca && s.section_enabled("optics") {
         frame.add_lateral_ca(optics::estimate_lateral_ca(src));
+        if apply_crop {
+            // The estimated CA moves the colour planes too: keep the crop inside every one of them.
+            frame.constrain_to_image(s.geometry.constrain_crop);
+        }
     }
     frame
 }