Synchronize Folder: progress counts the photo checks too (review fix)

The dialog's "Scanning… n / m" counted only the files probed, so it
could sit at m / m while the library's photos were still being checked.
SyncProgress counts both halves (its `files.cancel` still stops the
whole scan).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford
2026-10-09 18:18:33 -04:00
co-authored by Claude Opus 5.5
parent c98010cffa
commit 82d46b14cd
3 changed files with 55 additions and 17 deletions
+35 -9
View File
@@ -39,6 +39,24 @@ use crate::{EngineError, Result, Session};
/// The largest XMP sidecar read when looking for metadata updates; a bigger one is skipped.
const MAX_SIDECAR_BYTES: u64 = 16 * 1024 * 1024;
/// How far a scan is: the folder's files probed ([`ScanProgress`], whose `cancel` stops the
/// whole scan) and the library's photos checked.
#[derive(Default)]
pub struct SyncProgress {
pub files: ScanProgress,
pub checked: std::sync::atomic::AtomicUsize,
pub to_check: std::sync::atomic::AtomicUsize,
}
impl SyncProgress {
/// (done, of how many), both halves together.
pub fn counts(&self) -> (usize, usize) {
use std::sync::atomic::Ordering::Relaxed;
let done = self.files.done.load(Relaxed).saturating_add(self.checked.load(Relaxed));
(done, self.files.total.load(Relaxed).saturating_add(self.to_check.load(Relaxed)))
}
}
/// What a scan of a folder needs, taken from the session so it can run on a worker thread.
pub struct SyncInput {
folder: String,
@@ -155,8 +173,9 @@ impl SyncInput {
///
/// Both halves wait on the disk (a network share answers each file slowly), so they run at once:
/// the look for new files (which probes them on its own threads) beside the checks of the
/// library's photos, which run on several threads too. Stops early when `progress.cancel` is set.
pub fn scan_with(input: SyncInput, progress: &ScanProgress) -> FolderChanges {
/// library's photos, which run on several threads too. Stops early when `progress.files.cancel`
/// is set.
pub fn scan_with(input: SyncInput, progress: &SyncProgress) -> FolderChanges {
let SyncInput { folder, scan, photos, labels } = input;
let seen: Vec<Arc<Photo>> = photos.iter().map(|(p, _)| Arc::clone(p)).collect();
if !cfg!(target_arch = "wasm32") && !Path::new(&folder).is_dir() {
@@ -164,13 +183,13 @@ pub fn scan_with(input: SyncInput, progress: &ScanProgress) -> FolderChanges {
}
// (no threads on the web: one after the other there)
#[cfg(target_arch = "wasm32")]
let (out, checks) = (crate::import::scan_with(scan, std::slice::from_ref(&folder), progress), check_photos(&photos, &labels, progress));
let (out, checks) = (crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files), check_photos(&photos, &labels, progress));
#[cfg(not(target_arch = "wasm32"))]
let (out, checks) = std::thread::scope(|sc| {
let photos = &photos;
let labels = &labels;
let checks = sc.spawn(move || check_photos(photos, labels, progress));
let out = crate::import::scan_with(scan, std::slice::from_ref(&folder), progress);
let out = crate::import::scan_with(scan, std::slice::from_ref(&folder), &progress.files);
// (a check that panicked counts as nothing found; the scan still reports the rest)
(out, checks.join().unwrap_or_default())
});
@@ -245,8 +264,15 @@ fn check_photo(p: &Photo, naming: SidecarNaming, labels: &Catalog) -> Check {
/// [`check_photo`] for each photo, in order, on several threads (each file is a wait on the
/// disk, not work for the CPU). Photos not reached before a cancel count as nothing found.
fn check_photos(photos: &[(Arc<Photo>, SidecarNaming)], labels: &Catalog, progress: &ScanProgress) -> Vec<Check> {
fn check_photos(photos: &[(Arc<Photo>, SidecarNaming)], labels: &Catalog, progress: &SyncProgress) -> Vec<Check> {
use std::sync::atomic::Ordering::Relaxed;
progress.to_check.store(photos.len(), Relaxed);
let cancel = &progress.files.cancel;
let check = |p: &Photo, naming: SidecarNaming| {
let c = check_photo(p, naming, labels);
progress.checked.fetch_add(1, Relaxed);
c
};
if cfg!(target_arch = "wasm32") {
return vec![Check::Nothing; photos.len()];
}
@@ -259,10 +285,10 @@ fn check_photos(photos: &[(Arc<Photo>, SidecarNaming)], labels: &Catalog, progre
std::thread::scope(|sc| {
for _ in 0..n {
sc.spawn(|| {
while !progress.cancel.load(Relaxed) {
while !cancel.load(Relaxed) {
let i = next.fetch_add(1, Relaxed);
let Some((p, naming)) = photos.get(i) else { break };
let c = check_photo(p, *naming, labels);
let c = check(p, *naming);
if let Some(slot) = found.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get_mut(i) {
*slot = c;
}
@@ -273,13 +299,13 @@ fn check_photos(photos: &[(Arc<Photo>, SidecarNaming)], labels: &Catalog, progre
return found.into_inner().unwrap_or_else(std::sync::PoisonError::into_inner);
}
}
photos.iter().map(|(p, naming)| if progress.cancel.load(Relaxed) { Check::Nothing } else { check_photo(p, *naming, labels) }).collect()
photos.iter().map(|(p, naming)| if cancel.load(Relaxed) { Check::Nothing } else { check(p, *naming) }).collect()
}
/// [`scan_with`] on the calling thread.
pub fn scan(s: &mut Session, path: &str, disk: bool) -> Result<FolderChanges> {
let input = SyncInput::new(s, path, disk)?;
let changes = scan_with(input, &ScanProgress::default());
let changes = scan_with(input, &SyncProgress::default());
if changes.offline {
return Err(not_there(&changes.path));
}
+15 -2
View File
@@ -244,8 +244,8 @@ fn a_cancelled_scan_stops_checking_files() {
let mut s = library(&dir);
std::fs::remove_file(dir.path("trip/day1/b.png")).unwrap();
let input = crate::sync::SyncInput::new(&mut s, &dir.path("trip"), false).unwrap();
let progress = crate::import::ScanProgress::default();
progress.cancel.store(true, std::sync::atomic::Ordering::Relaxed);
let progress = crate::sync::SyncProgress::default();
progress.files.cancel.store(true, std::sync::atomic::Ordering::Relaxed);
let c = crate::sync::scan_with(input, &progress);
assert!(c.missing.is_empty() && c.metadata.is_empty(), "{c:?}");
}
@@ -393,3 +393,16 @@ fn a_change_to_the_folders_photos_makes_the_scan_stale() {
let r = s.execute("folder.synchronize", &json!({"path": dir.path("trip")})).unwrap();
assert_eq!(r["imported"], 1, "{r}");
}
#[test]
fn the_progress_counts_the_photos_checked_too() {
use std::sync::atomic::Ordering::Relaxed;
let dir = Scratch::new("progress");
let mut s = library(&dir);
let input = crate::sync::SyncInput::new(&mut s, &dir.path("trip"), false).unwrap();
let progress = crate::sync::SyncProgress::default();
crate::sync::scan_with(input, &progress);
assert_eq!((progress.checked.load(Relaxed), progress.to_check.load(Relaxed)), (2, 2));
let (done, total) = progress.counts();
assert!(done == total && total >= 2, "{done}/{total}");
}
+5 -6
View File
@@ -6,8 +6,7 @@
use std::sync::Arc;
use std::sync::atomic::Ordering;
use lightcraft_engine::import::ScanProgress;
use lightcraft_engine::sync::{FolderChanges, SyncChoice, SyncInput, scan_with};
use lightcraft_engine::sync::{FolderChanges, SyncChoice, SyncInput, SyncProgress, scan_with};
use serde_json::{Value, json};
use crate::LightcraftApp;
@@ -17,7 +16,7 @@ use crate::theme::Tokens;
/// A running scan of the dialog's folder.
pub struct SyncTask {
path: String,
progress: Arc<ScanProgress>,
progress: Arc<SyncProgress>,
rx: std::sync::mpsc::Receiver<FolderChanges>,
}
@@ -25,7 +24,7 @@ pub struct SyncTask {
pub fn open(app: &mut LightcraftApp, path: &str, name: &str, disk: bool) -> Result<(), String> {
cancel(app);
let input = SyncInput::new(&mut app.session, path, disk).map_err(|e| e.to_string())?;
let progress = Arc::new(ScanProgress::default());
let progress = Arc::new(SyncProgress::default());
let (tx, rx) = std::sync::mpsc::channel();
let p = progress.clone();
let job = move || {
@@ -54,7 +53,7 @@ pub fn open(app: &mut LightcraftApp, path: &str, name: &str, disk: bool) -> Resu
/// `folder.scanChanges` is the agent's).
fn cancel(app: &mut LightcraftApp) {
if let Some(t) = app.sync.take() {
t.progress.cancel.store(true, Ordering::Relaxed);
t.progress.files.cancel.store(true, Ordering::Relaxed);
}
if std::mem::take(&mut app.sync_owns_changes) {
app.session.folder_changes = None;
@@ -109,7 +108,7 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) {
/// How far the scan is (files probed, of how many).
fn progress(app: &LightcraftApp) -> Option<(usize, usize)> {
app.sync.as_ref().map(|t| (t.progress.done.load(Ordering::Relaxed), t.progress.total.load(Ordering::Relaxed)))
app.sync.as_ref().map(|t| t.progress.counts())
}
/// The dialog's body.