From 15e308c3eb99a293e76a184ec3db79a5424ce3a1 Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Sun, 11 Oct 2026 14:58:38 +0200 Subject: [PATCH] Sort ties by file name, then id Photos that tie on the sort key (equal ratings, one import batch, a burst within a second) now sort by file name before id, in Catalog::query, Recently Added and auto-stack. Split out of #641. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/query.rs | 15 +++++++++-- crates/catalog/src/stacks.rs | 29 +++++++++++++++++---- crates/catalog/src/tests.rs | 43 +++++++++++++++++++++++++++++++ crates/engine/src/lib.rs | 10 +++++-- crates/engine/src/tests_import.rs | 21 +++++++++++++++ docs/whats-new.md | 5 ++++ 6 files changed, 114 insertions(+), 9 deletions(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index 8b694a02..be6ab6cd 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -1,5 +1,7 @@ //! Filtering, search and sorting. +use std::cmp::Ordering::Equal; + use serde::{Deserialize, Serialize}; use crate::{AlbumId, Catalog, ColorLabel, Flag, MediaKind, Photo, PhotoId}; @@ -359,8 +361,13 @@ impl Filter { } } +/// Case-insensitive file name order without allocating (compares per-character lowercase via `char::to_lowercase`). +pub fn cmp_name(a: &str, b: &str) -> std::cmp::Ordering { + a.chars().flat_map(char::to_lowercase).cmp(b.chars().flat_map(char::to_lowercase)) +} + impl Catalog { - /// Photos matching `filter`, in `sort` order (ties broken by id for stability). + /// Photos matching `filter`, in `sort` order (ties broken by file name, then id). pub fn query(&self, filter: &Filter, sort: &Sort) -> Vec { let root = filter.library_root(); let mut v: Vec<&Photo> = self.photos().map(|p| p.as_ref()).filter(|p| filter.matches_in(p, self, root.as_deref())).collect(); @@ -369,11 +376,15 @@ impl Catalog { SortKey::CaptureDate => a.captured.cmp(&b.captured).then_with(|| a.imported.cmp(&b.imported)), SortKey::ImportDate => a.imported.cmp(&b.imported), SortKey::EditDate => a.edited.cmp(&b.edited), - SortKey::FileName => a.file_name.to_lowercase().cmp(&b.file_name.to_lowercase()), + SortKey::FileName => cmp_name(&a.file_name, &b.file_name), SortKey::Rating => a.rating.cmp(&b.rating), SortKey::FileSize => a.file_size.cmp(&b.file_size), SortKey::Random => shuffle_rank(sort.seed, a.id).cmp(&shuffle_rank(sort.seed, b.id)), } + // Ties (one import batch, a burst within a second, equal ratings) sort by file name, the + // order cameras number frames, so the order doesn't depend on how ids were allocated; + // the id only keeps it stable. + .then_with(|| if sort.key == SortKey::FileName { Equal } else { cmp_name(&a.file_name, &b.file_name) }) .then(a.id.cmp(&b.id)); if sort.ascending { o } else { o.reverse() } }); diff --git a/crates/catalog/src/stacks.rs b/crates/catalog/src/stacks.rs index 304106ae..04a9f0be 100644 --- a/crates/catalog/src/stacks.rs +++ b/crates/catalog/src/stacks.rs @@ -9,6 +9,7 @@ use std::collections::{HashMap, HashSet}; +use crate::query::cmp_name; use crate::{Catalog, CatalogError, Op, PhotoId, Result, Stack, StackId}; impl Catalog { @@ -203,17 +204,19 @@ impl Catalog { /// Only runs of two or more are returned. pub fn auto_stack_groups(&self, ids: &[PhotoId], gap_secs: f64) -> Vec> { let index = self.stack_index(); - let mut timed: Vec<(i64, PhotoId)> = ids + let mut timed: Vec<(i64, &str, PhotoId)> = ids .iter() .filter_map(|id| self.photos.get(id)) .filter(|p| !p.deleted && !index.contains_key(&p.id)) - .filter_map(|p| Some((iso_seconds(p.captured.as_deref()?)?, p.id))) + .filter_map(|p| Some((iso_seconds(p.captured.as_deref()?)?, p.file_name.as_str(), p.id))) .collect(); - timed.sort(); - timed.dedup_by_key(|(_, id)| *id); + // a burst within one second stays in file name order (the first file is the stack's cover) + timed.sort_by(|a, b| a.0.cmp(&b.0).then_with(|| cmp_name(a.1, b.1)).then(a.2.cmp(&b.2))); + // equal ids are the same photo, so same time and name: adjacent after the sort + timed.dedup_by_key(|(_, _, id)| *id); let mut groups: Vec> = Vec::new(); let mut last: Option = None; - for (t, id) in timed { + for (t, _, id) in timed { match (last, groups.last_mut()) { (Some(l), Some(g)) if (t - l) as f64 <= gap_secs => g.push(id), _ => groups.push(vec![id]), @@ -264,6 +267,22 @@ mod tests { (c, ids) } + #[test] + fn auto_stack_keeps_a_burst_in_file_order() { + let mut c = Catalog::new(); + let mut by_name = Vec::new(); + for (n, name) in ["DSC_0001.ARW", "dsc_0002.ARW", "DSC_0003.ARW"].iter().enumerate() { + let id = PhotoId(1000 - n as u64); + let mut p = Photo::new(id, Source::Demo { scene: 1 }, name, "ARW", 60, 40, "2026-01-01T00:00:00"); + p.captured = Some("2026-04-01T10:00:00".into()); + c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + by_name.push(id); + } + let groups = c.auto_stack_groups(&[by_name[2], by_name[0], by_name[1]], 1.0); + assert_eq!(groups, vec![by_name.clone()]); + assert_eq!(groups[0][0], by_name[0], "the first file is the stack's cover"); + } + #[test] fn iso_seconds_parses() { assert_eq!(iso_seconds("1970-01-01T00:00:00"), Some(0)); diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index bedc94e7..acfbb1bb 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -694,3 +694,46 @@ fn undated_photos_group_under_unknown_date_and_sort_together() { let filter_year = Filter { date: Some("2026".into()), ..Default::default() }; assert_eq!(c.query(&filter_year, &Sort { key: SortKey::CaptureDate, ascending: false, ..Default::default() }), vec![p_dated2, p_dated1]); } + +/// Photos with one import time and capture time, as one import batch or a burst gives them, added +/// with ids that run opposite to their names. +fn batch_with_reversed_ids(c: &mut Catalog) -> Vec { + let names = ["DSC_0001.ARW", "DSC_0002.ARW", "DSC_0003.ARW", "DSC_0004.ARW"]; + let mut ids = Vec::new(); + for (n, name) in names.iter().enumerate() { + let id = PhotoId(1000 - n as u64); + let mut p = Photo::new(id, Source::Demo { scene: 1 }, name, "ARW", 10, 10, "2026-10-10T09:00:00"); + p.captured = Some("2026-10-01T12:00:00".into()); + c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + ids.push(id); + } + ids // in name order +} + +#[test] +fn one_import_batch_sorts_by_file_name_not_id() { + let mut c = Catalog::new(); + let by_name = batch_with_reversed_ids(&mut c); + let sort = Sort { key: SortKey::ImportDate, ascending: true, ..Default::default() }; + assert_eq!(c.query(&Filter::default(), &sort), by_name); +} + +#[test] +fn burst_in_one_second_sorts_by_file_name_not_id() { + let mut c = Catalog::new(); + let by_name = batch_with_reversed_ids(&mut c); + let sort = Sort { key: SortKey::CaptureDate, ascending: true, ..Default::default() }; + assert_eq!(c.query(&Filter::default(), &sort), by_name); +} + +#[test] +fn cmp_name_ignores_case_without_allocating() { + use crate::query::cmp_name; + use std::cmp::Ordering::*; + assert_eq!(cmp_name("IMG_1.JPG", "img_1.jpg"), Equal); + assert_eq!(cmp_name("Ärger.jpg", "ärger.jpg"), Equal); + assert_eq!(cmp_name("a", "B"), Less); + assert_eq!(cmp_name("B", "a"), Greater); + assert_eq!(cmp_name("DSC_0001", "DSC_0002"), Less); + assert_eq!(cmp_name("a", "ab"), Less); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3bc6c431..a024eb02 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -881,11 +881,17 @@ impl Session { } } if self.source == LibrarySource::RecentlyAdded { - // newest import first, whatever the sort (the grid groups by import day) + // newest import first, whatever the sort direction (the grid groups by import day); + // a batch stays in file order (name, then id) let cat = &self.catalog; visible.sort_by(|a, b| { let key = |id: &PhotoId| cat.photo(*id).map(|p| p.imported.clone()).unwrap_or_default(); - key(b).cmp(&key(a)).then(a.cmp(b)) + let name = |id: &PhotoId| cat.photo(*id).map(|p| p.file_name.as_str()); + let by_name = match (name(a), name(b)) { + (Some(x), Some(y)) => lightcraft_catalog::query::cmp_name(x, y), + _ => std::cmp::Ordering::Equal, + }; + key(b).cmp(&key(a)).then(by_name).then(a.cmp(b)) }); } if self.source == LibrarySource::Missing { diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 18599349..bdebb3cb 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -309,6 +309,27 @@ fn recently_added_covers_recent_imports_newest_first() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn recently_added_keeps_an_import_batch_in_file_order() { + use lightcraft_catalog::{Op, Photo, PhotoId, Source}; + let mut s = Session::new(); + // one import batch: one `imported` string, ids running opposite to the names + let mut by_name = Vec::new(); + for (n, name) in ["IMG_0001.png", "IMG_0002.png", "IMG_0003.png", "IMG_0004.png"].iter().enumerate() { + let id = PhotoId(5000 - n as u64); + let p = Photo::new(id, Source::File { path: name.to_string() }, name, "PNG", 16, 12, "2026-09-30T09:00:00"); + s.catalog.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + by_name.push(name.to_string()); + } + s.clock = Box::new(|| "2026-10-01T09:00:00".to_string()); + for ascending in [true, false] { + s.execute("library.sort", &serde_json::json!({"key": "captureDate", "ascending": ascending})).unwrap(); + s.execute("library.source", &serde_json::json!({"kind": "recentlyAdded"})).unwrap(); + let names: Vec = s.visible_cloned().iter().map(|id| s.catalog.photo(*id).unwrap().file_name.clone()).collect(); + assert_eq!(names, by_name, "ascending: {ascending}"); + } +} + /// Copy imports: a destination folder, flat / by-month folders, renamed copies (numbered in /// import order), and a metadata preset on every photo. #[test] diff --git a/docs/whats-new.md b/docs/whats-new.md index fb8b285a..cfb2d9ec 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -46,6 +46,11 @@ in, instead of moving it back to Program Files (issue #399). If that folder has been deleted since, the upgrade installs to Program Files. +### Library sorting +- Photos that tie on the sort key (equal ratings, one import batch, a burst shot within the same second) now sort by + file name, then id, instead of import order. Recently Added keeps a batch in file name order, and Auto-Stack orders a + burst by file name, so the first file is the cover. + ## 2026-10-10 ### Dialogs fit the window