From 86deb5b0046f53976aad7ed266cd0c97504bffe9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 20:42:01 +0000 Subject: [PATCH] Fix #367: import copies and verifies files on several threads Import -> Copy (and Move, and Copy as DNG) handled one file after another on the import worker: copy, fsync, read back and hash, optionally decode and re-encode the raw as DNG. Every other CPU-heavy step of an import was already parallel (the probes, up to 8 threads), so with the probes cached by the review dialog's scan the transfer phase ran on one core. ImportJob::prepare_files now plans each file in order, as before (duplicates by path and content, sidecars, rename sequence numbers, folders, Kept notes) and collects the copies/moves, which then run on up to 8 threads (std::thread::scope; sequential on wasm32). Outcomes keep their places, so results, order, names and failure messages are those of a one-by-one import: - a file whose content matches a transfer still pending, or whose name could collide with one (same folder, same name family: IMG.jpg, IMG-1.jpg, img.dng), waits until that one has finished; - cancel stops before the next file; transfers not started leave no item; - a panic in one transfer fails that file instead of the batch; - Copy as DNG holds a memory-gate permit (6x the file size) while it runs. Copy verification and crash safety are unchanged (same copy_new / place). The desktop's import worker prepares batches of 2 x workers files (8..16, lightcraft_engine::import::batch_size) instead of 8, so a batch's slowest file holds back fewer threads; the browser build keeps 8. Measured (4-core shared VM, 200 procedural 3.7 MB JPEGs = 754 MB, warm page cache, throwaway benchmark not committed): - probe: ~80 ms CPU per file is the JPEG decode (DCT-scaled to 64 px), 1.2 ms the content hash; the dialog's scan: 4.4 s, 3.6 cores busy. - Copy, probes cached: copy+fsync+verify 1.59 s at 0.43 cores before, 0.94 s at 0.81 cores after. Copy, not cached: 6.45 s -> 5.51 s. - Add, probes cached: 2 ms prepare; nothing to parallelize. - commit of a batch on the UI thread (journal append + fsync): ~4 ms, flat for libraries of 0, 5k, 20k and 50k photos (~0.7 s for 1300). Copy as DNG could not be measured (no raw corpus here); it is the most CPU-heavy per-file step and was sequential. Test: copies_run_side_by_side_with_the_same_outcome (copies under the test fault Slow run on more than one thread; names, order and in-batch duplicates match a one-by-one copy; the outcome assertions also pass on the old code, the thread assertion fails there). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8 --- crates/engine/src/import.rs | 295 ++++++++++++++++++++----- crates/engine/src/import_move.rs | 23 ++ crates/engine/src/tests_import_move.rs | 67 ++++++ crates/ui-egui/src/headless.rs | 11 +- crates/ui-egui/src/import.rs | 8 +- 5 files changed, 340 insertions(+), 64 deletions(-) diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 5df94988..5ab462a1 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -365,7 +365,7 @@ fn probe_all(probe: Option<&crate::media::FileProbe>, paths: &[String], progress }; #[cfg(not(target_arch = "wasm32"))] { - let n = std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(1, 8).min(paths.len().max(1)); + let n = workers(paths.len()); if n > 1 { let mut results: Vec>> = vec![None; paths.len()]; let next = std::sync::atomic::AtomicUsize::new(0); @@ -404,18 +404,16 @@ fn probe_all(probe: Option<&crate::media::FileProbe>, paths: &[String], progress .collect() } -/// Copy `src` into `root`/`folders` as `name` (default: its own name), made unique with -1, -2…; -/// returns the new path. The copy is verified ([`crate::import_move::copy_new`]: a new file, -/// synced and checked against `probe_hash`, the content hash the probe computed — or compared -/// byte for byte with the source when there is none); a bad one is removed and the photo -/// reported as failed. -fn copy_into(root: &Path, src: &str, folders: &[String], name: Option<&str>, probe_hash: Option<&str>) -> Result { - let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f)); +/// Copy `src` into `dir` as `name` (default: its own name), made unique with -1, -2…; returns the +/// new path. The copy is verified ([`crate::import_move::copy_new`]: a new file, synced and +/// checked against `probe_hash`, the content hash the probe computed — or compared byte for byte +/// with the source when there is none); a bad one is removed and the photo reported as failed. +fn copy_into(dir: &Path, src: &str, name: Option<&str>, probe_hash: Option<&str>) -> Result { let name = name .map(str::to_string) .unwrap_or_else(|| Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "photo".into())); let expect = probe_hash.and_then(lightcraft_preview::Hash128::parse); - crate::import_move::copy_new(Path::new(src), &dir, &name, expect).map(|p| p.to_string_lossy().to_string()) + crate::import_move::copy_new(Path::new(src), dir, &name, expect).map(|p| p.to_string_lossy().to_string()) } /// What a [`scan`] needs from the session, so it can run on another thread (a folder on a network @@ -699,8 +697,9 @@ impl ImportJob { } /// The file-system half of importing `paths`: expand folders, skip known paths, probe, skip - /// duplicates by content, read sidecars, copy or place (Move) the files. Nothing in the - /// catalog changes. Stops before the next file once `cancel` is set. + /// duplicates by content, read sidecars, copy or place (Move) the files — several at once, + /// with the names and outcomes of one after another. Nothing in the catalog changes. Stops + /// before the next file once `cancel` is set. pub fn prepare(&mut self, paths: &[String], cancel: &std::sync::atomic::AtomicBool) -> Prepared { let files = self.expand(paths); self.prepare_files(files, cancel) @@ -750,24 +749,39 @@ impl ImportJob { let probed: Vec> = cached.into_iter().map(|c| c.map(Ok).unwrap_or_else(|| fresh.next().unwrap_or_else(|| Err("not probed".into())))).collect(); crate::memory::release(); + // Files are planned one by one, in order (duplicates, sidecars, names, folders); their + // copies or moves — the slow part, reading and writing every byte and verifying it — are + // collected and run on several threads (issue #367), then their outcomes take their places. + let mut plan = Plan::default(); for (path, info) in todo.into_iter().zip(probed) { if cancel.load(Relaxed) { break; } - self.done.fetch_add(1, Relaxed); let mut info = match info { Ok(i) => i, Err(e) => { log::warn!("import {path}: {e}"); - out.items.push(PreparedItem::Failed(path, e)); + self.done.fetch_add(1, Relaxed); + plan.slots.push(Slot::Item(PreparedItem::Failed(path, e))); continue; } }; + // a copy of a file whose transfer is still pending: a duplicate only if that one succeeds + if !opts.local + && let Some(h) = &info.content_hash + && plan.transfers.iter().any(|t| t.info.content_hash.as_ref() == Some(h)) + { + self.run_plan(&mut plan, &mut out, cancel); + if cancel.load(Relaxed) { + break; + } + } if let Some(h) = &info.content_hash && !opts.local && let Some(existing) = self.by_hash.get(h) { - out.items.push(PreparedItem::Duplicate { path, existing: *existing, reason: "content", hash: Some(h.clone()) }); + self.done.fetch_add(1, Relaxed); + plan.slots.push(Slot::Item(PreparedItem::Duplicate { path, existing: *existing, reason: "content", hash: Some(h.clone()) })); continue; } // the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files @@ -786,7 +800,6 @@ impl ImportJob { if info.captured.is_none() { info.captured = sidecar.as_ref().and_then(|sc| sc.captured.clone()); } - let mut placed = None; let stored = match (mode, &self.copy_root) { (ImportMode::Copy | ImportMode::Move, Some(root)) if !crate::import_move::inside(Path::new(&path), root) @@ -803,37 +816,35 @@ impl ImportJob { n }); let folders = opts.organize.folders(&q); - if mode == ImportMode::Move && !crate::import_move::is_symlink(Path::new(&path)) { - let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f)); - let name = name.unwrap_or(own); - match crate::import_move::place(Path::new(&path), &dir, &name) { - Ok(pl) => { - let dst = pl.dst.to_string_lossy().to_string(); - placed = Some(pl); - dst - } - Err(e) => { - out.items.push(PreparedItem::Failed(path, e)); - continue; - } + let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f)); + // the name this file gets depends on what a pending transfer of a like-named file + // into the same folder ends up with: that one goes first + let as_named = name.as_deref().unwrap_or(if own.is_empty() { "photo" } else { own.as_str() }); + let family = (dir.to_string_lossy().to_lowercase(), name_family(as_named)); + if plan.transfers.iter().any(|t| t.family == family) { + self.run_plan(&mut plan, &mut out, cancel); + if cancel.load(Relaxed) { + break; } + } + let how = if mode == ImportMode::Move && !crate::import_move::is_symlink(Path::new(&path)) { + How::Place { name: name.unwrap_or(own) } } else { if mode == ImportMode::Move { let reason = "a symbolic link: the file it points to was copied, the link and its target stay"; - out.items.push(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() })); + plan.slots.push(Slot::Item(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() }))); } - match copy_into(root, &path, &folders, name.as_deref(), info.content_hash.as_deref()) { - Ok(p) => p, - Err(e) => { - out.items.push(PreparedItem::Failed(path, e)); - continue; - } - } - } + How::Copy { name } + }; + // Copy as DNG: the copied raw becomes a DNG (the copy is ours to replace) + let dng = opts.convert_dng && mode == ImportMode::Copy && raw && !info.format.eq_ignore_ascii_case("DNG"); + plan.slots.push(Slot::Transfer); + plan.transfers.push(Transfer { path, info, sidecar, dir, how, dng, family }); + continue; } (ImportMode::Move, _) => { let reason = "already inside the destination or the library: added where it is"; - out.items.push(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() })); + plan.slots.push(Slot::Item(PreparedItem::Kept(Kept { path: path.clone(), reason: reason.into() }))); path.clone() } _ => path.clone(), @@ -841,27 +852,199 @@ impl ImportJob { if let Some(h) = &info.content_hash { self.by_hash.insert(h.clone(), None); } - // Copy as DNG: the copied raw becomes a DNG (the copy is ours to replace) - let mut stored = stored; - if opts.convert_dng - && mode == ImportMode::Copy - && stored != path - && info.kind == MediaKind::Raw - && !info.format.eq_ignore_ascii_case("DNG") - { - match crate::cmd::convert::write_dng_with(self.file_bytes.as_ref(), &stored, String::new()) { - Ok(dng) => { - let _ = std::fs::remove_file(&stored); - stored = dng; - info.format = "DNG".into(); - } - Err(e) => log::warn!("import {path}: copy as DNG: {e}"), - } - } - out.items.push(PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed }))); + self.done.fetch_add(1, Relaxed); + plan.slots.push(Slot::Item(PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed: None })))); } + self.run_plan(&mut plan, &mut out, cancel); out } + + /// Run the transfers `plan` collected (on several threads) and append its items to `out` in + /// file order. Transfers not started because `cancel` was set leave no item. + fn run_plan(&mut self, plan: &mut Plan, out: &mut Prepared, cancel: &std::sync::atomic::AtomicBool) { + let mut done = run_transfers(std::mem::take(&mut plan.transfers), self.file_bytes.as_ref(), cancel, &self.done).into_iter(); + for slot in std::mem::take(&mut plan.slots) { + let item = match slot { + Slot::Item(it) => it, + Slot::Transfer => match done.next().flatten() { + Some(it) => it, + None => continue, + }, + }; + if let PreparedItem::Ready(r) = &item + && let Some(h) = &r.info.content_hash + { + self.by_hash.insert(h.clone(), None); + } + out.items.push(item); + } + } +} + +/// A batch's items in file order while its transfers are pending ([`ImportJob::run_plan`]). +#[derive(Default)] +struct Plan { + slots: Vec, + transfers: Vec, +} + +enum Slot { + Item(PreparedItem), + /// The outcome of the next of [`Plan::transfers`]. + Transfer, +} + +/// A file to copy or move into `dir`, planned in order and run on any thread ([`run_transfer`]). +struct Transfer { + path: String, + info: ProbeInfo, + sidecar: Option, + dir: PathBuf, + how: How, + /// Copy as DNG. + dng: bool, + /// The folder (lowercase) and [`name_family`]: transfers of one family can take each other's + /// names, so they never run at the same time. + family: (String, String), +} + +enum How { + /// A verified copy, under `name` (default: the file's own). + Copy { name: Option }, + /// Move: placed (linked or copied) without touching the source. + Place { name: String }, +} + +/// The names a file called `name` could end up with, as one key: the transfers number a taken name +/// `IMG-1.JPG`, `IMG-2.JPG`…, and Copy as DNG names its DNG `IMG.dng`, `IMG-2.dng`…, so `IMG.JPG`, +/// `IMG-1.jpg` and `img.dng` are all `img` (case-insensitive: so are some file systems). +fn name_family(name: &str) -> String { + let mut stem = match name.rsplit_once('.') { + Some((s, _)) if !s.is_empty() => s, + _ => name, + }; + while let Some((head, tail)) = stem.rsplit_once('-') + && !head.is_empty() + && !tail.is_empty() + && tail.bytes().all(|b| b.is_ascii_digit()) + { + stem = head; + } + stem.to_lowercase() +} + +/// Threads for `jobs` file reads (probes, copies) at once. +#[cfg(not(target_arch = "wasm32"))] +fn workers(jobs: usize) -> usize { + std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(1, 8).min(jobs.max(1)) +} + +/// Files per batch for a caller that prepares an import in batches ([`ImportJob::prepare_files`]): +/// enough to keep every probe and copy thread busy (a batch waits for its slowest file), few +/// enough that each batch soon joins the catalog. +pub fn batch_size() -> usize { + #[cfg(not(target_arch = "wasm32"))] + { + workers(usize::MAX).saturating_mul(2).clamp(8, 16) + } + #[cfg(target_arch = "wasm32")] + { + 8 + } +} + +/// Run `transfers` (several at once where threads exist); their outcomes in order, `None` for one +/// not started because `cancel` was set. `done` counts each one finished. +fn run_transfers( + transfers: Vec, + file_bytes: Option<&crate::merge::ByteReader>, + cancel: &std::sync::atomic::AtomicBool, + done: &std::sync::atomic::AtomicUsize, +) -> Vec> { + use std::sync::atomic::Ordering::Relaxed; + let one = |t: Transfer| { + if cancel.load(Relaxed) { + return None; + } + let path = t.path.clone(); + // last-resort guard: a panic is this file's failure, not the import's + let item = crate::guard::catch("import", || run_transfer(t, file_bytes)).unwrap_or_else(|e| PreparedItem::Failed(path, e)); + done.fetch_add(1, Relaxed); + Some(item) + }; + #[cfg(not(target_arch = "wasm32"))] + { + let n = workers(transfers.len()); + if n > 1 { + let mut results: Vec> = std::iter::repeat_with(|| None).take(transfers.len()).collect(); + let queue = std::sync::Mutex::new(transfers.into_iter().enumerate()); + let out = std::sync::Mutex::new(&mut results); + // (the tests' injected faults hold for the workers too) + #[cfg(test)] + let fault = crate::import_move::injected_fault(); + let work = &|| { + #[cfg(test)] + crate::import_move::inject(fault); + loop { + let next = queue.lock().unwrap_or_else(std::sync::PoisonError::into_inner).next(); + let Some((i, t)) = next else { break }; + let r = one(t); + if let Some(slot) = out.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get_mut(i) { + *slot = r; + } + } + }; + std::thread::scope(|sc| { + let handles: Vec<_> = + (0..n).filter_map(|i| std::thread::Builder::new().name(format!("lc-import-copy-{i}")).spawn_scoped(sc, work).ok()).collect(); + if handles.is_empty() { + // no thread to be had: copy here, one after another + work(); + } + for h in handles { + if h.join().is_err() { + log::error!("import: a copy worker stopped unexpectedly"); + } + } + }); + return results; + } + } + transfers.into_iter().map(one).collect() +} + +/// Copy or move one planned file (and Copy as DNG); its item. +fn run_transfer(t: Transfer, file_bytes: Option<&crate::merge::ByteReader>) -> PreparedItem { + let Transfer { path, mut info, sidecar, dir, how, dng, .. } = t; + let mut placed = None; + let mut stored = match how { + How::Place { name } => match crate::import_move::place(Path::new(&path), &dir, &name) { + Ok(pl) => { + let dst = pl.dst.to_string_lossy().to_string(); + placed = Some(pl); + dst + } + Err(e) => return PreparedItem::Failed(path, e), + }, + How::Copy { name } => match copy_into(&dir, &path, name.as_deref(), info.content_hash.as_deref()) { + Ok(p) => p, + Err(e) => return PreparedItem::Failed(path, e), + }, + }; + if dng { + // decoding the raw twice and encoding it takes several times the file's size in memory + let weight = usize::try_from(info.file_size).unwrap_or(usize::MAX).saturating_mul(6); + let _permit = crate::memory::work_gate().acquire(weight); + match crate::cmd::convert::write_dng_with(file_bytes, &stored, String::new()) { + Ok(dng) => { + let _ = std::fs::remove_file(&stored); + stored = dng; + info.format = "DNG".into(); + } + Err(e) => log::warn!("import {path}: copy as DNG: {e}"), + } + } + PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed })) } /// The catalog half of an import: add the photos [`ImportJob::prepare`] readied (one undoable op; diff --git a/crates/engine/src/import_move.rs b/crates/engine/src/import_move.rs index 3465eaa7..db442136 100644 --- a/crates/engine/src/import_move.rs +++ b/crates/engine/src/import_move.rs @@ -56,6 +56,9 @@ pub(crate) enum Fault { Corrupt, /// Removing the source fails (as on a read-only card). FailRemove, + /// Each copy takes a little longer and notes the thread it ran on ([`copy_threads`]), to see + /// an import's copies run side by side. + Slow, } #[cfg(test)] @@ -69,6 +72,21 @@ pub(crate) fn inject(f: Fault) { FAULT.with(|c| c.set(f)); } +/// The calling thread's injected fault (tests only), for the workers it starts to inherit. +#[cfg(test)] +pub(crate) fn injected_fault() -> Fault { + FAULT.with(|c| c.get()) +} + +/// The threads copies made under [`Fault::Slow`] ran on (tests only). +#[cfg(test)] +static COPY_THREADS: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); + +#[cfg(test)] +pub(crate) fn copy_threads() -> Vec { + COPY_THREADS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).clone() +} + fn injected(f: Fault) -> bool { #[cfg(test)] { @@ -284,6 +302,11 @@ pub(crate) fn copy_verified(src: &Path, dst: &Path, expect: Option> 20); let _ = std::fs::remove_dir_all(&base); } + +/// Issue #367: Import → Copy wrote and verified one file after another on one thread. The copies +/// now run side by side, with the names, order, duplicates and reports a one-by-one copy gives: +/// like-named files take -1, -2… in file order (also against a file already there and a literal +/// `IMG_1-1`), and a file with the content of one copied earlier in the batch is its duplicate +/// (also while that copy is still under way). +#[test] +fn copies_run_side_by_side_with_the_same_outcome() { + let base = temp_dir("parallel-copy"); + let (card, dest) = (base.join("card"), base.join("out")); + write_png(&card.join("a/IMG_1.png"), 1); + write_png(&card.join("a/IMG_2.png"), 2); + write_png(&card.join("b/IMG_1.png"), 3); + write_png(&card.join("b/IMG_1-1.png"), 4); + write_png(&card.join("c/IMG_1.png"), 5); + for i in 0..8u8 { + write_png(&card.join(format!("d/p{i}.png")), 10 + i); + } + // the content of a file whose copy is still under way + std::fs::create_dir_all(card.join("e")).unwrap(); + std::fs::copy(card.join("d/p3.png"), card.join("e/dup.png")).unwrap(); + std::fs::create_dir_all(&dest).unwrap(); + std::fs::write(dest.join("IMG_2.png"), b"someone else's file").unwrap(); + + let mut s = session(); + inject(Fault::Slow); + let r = s + .execute( + "library.import", + &json!({"paths": [card.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "flat"}), + ) + .unwrap(); + inject(Fault::None); + let threads: std::collections::HashSet<_> = crate::import_move::copy_threads().into_iter().collect(); + if std::thread::available_parallelism().map_or(1, |n| n.get()) > 1 { + assert!(threads.len() > 1, "copies ran on {} thread(s)", threads.len()); + } + + assert_eq!(len(&r, "imported"), 13, "{r}"); + assert_eq!(len(&r, "failed"), 0, "{r}"); + assert_eq!(len(&r, "duplicates"), 1, "{r}"); + assert_eq!(r["duplicates"][0]["reason"], "content"); + assert!(r["duplicates"][0]["path"].as_str().unwrap().ends_with("dup.png"), "{r}"); + // the order of a one-by-one copy (files in name order), each verified + let expect = [ + ("a/IMG_1.png", "IMG_1.png"), + ("a/IMG_2.png", "IMG_2-1.png"), + ("b/IMG_1-1.png", "IMG_1-1.png"), + ("b/IMG_1.png", "IMG_1-2.png"), + ("c/IMG_1.png", "IMG_1-3.png"), + ]; + for (src, dst) in expect { + assert_eq!(std::fs::read(dest.join(dst)).unwrap(), std::fs::read(card.join(src)).unwrap(), "{src} → {dst}"); + } + for i in 0..8 { + assert_eq!(std::fs::read(dest.join(format!("p{i}.png"))).unwrap(), std::fs::read(card.join(format!("d/p{i}.png"))).unwrap()); + } + assert_eq!(std::fs::read(dest.join("IMG_2.png")).unwrap(), b"someone else's file"); + assert_eq!(files_under(&dest).len(), 14, "{:?}", files_under(&dest)); + // the duplicate names the photo copied from the same content (d/p3); ids follow the file order + let ids: Vec = r["imported"].as_array().unwrap().iter().map(|v| v.as_u64().unwrap()).collect(); + assert!(ids.windows(2).all(|w| w[0] < w[1]), "{ids:?}"); + assert_eq!(r["duplicates"][0]["existing"], ids[8], "{r}"); + let first = s.catalog.photo(lightcraft_catalog::PhotoId(ids[0])).unwrap(); + assert_eq!(first.file_name, "IMG_1.png"); + let _ = std::fs::remove_dir_all(&base); +} diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 787e0d1c..4411c35a 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1635,8 +1635,9 @@ mod tests { let _ = std::fs::remove_dir_all(&base); let (src, dest) = (base.join("card"), base.join("out")); std::fs::create_dir_all(&src).unwrap(); - for i in 0..10u8 { - let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[i * 20, 3, 9, 255]; 64] }; + // more files than a batch holds (see `batch_size`) + for i in 0..20u8 { + let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[i * 12, 3, 9, 255]; 64] }; let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap(); std::fs::write(src.join(format!("IMG_{i:02}.png")), png).unwrap(); } @@ -1651,7 +1652,7 @@ mod tests { h.settle(SETTLE); h.step_until(SETTLE, |h| matches!(h.app.ui.dialog, Some(crate::state::Dialog::Import { .. }))); let Some(crate::state::Dialog::Import { opts }) = &h.app.ui.dialog else { panic!("no import review") }; - assert_eq!(opts.candidates.len(), 10); + assert_eq!(opts.candidates.len(), 20); for id in ["button:importCopy", "button:importDest"] { let r = h.request("ui.clickWidget", json!({"id": id}), t); assert_eq!(r["ok"], true, "{id}: {r}"); @@ -1670,8 +1671,8 @@ mod tests { assert!(h.app.import.is_none(), "finished"); let mut names: Vec = std::fs::read_dir(&dest).unwrap().flatten().map(|e| e.file_name().to_string_lossy().to_string()).collect(); names.sort(); - let want: Vec = (1..=10).map(|i| format!("Trip-{i:02}.png")).collect(); - assert_eq!(names, want, "numbered across batches of {}", crate::import::BATCH); + let want: Vec = (1..=20).map(|i| format!("Trip-{i:02}.png")).collect(); + assert_eq!(names, want, "numbered across batches of {}", lightcraft_engine::import::batch_size()); let _ = std::fs::remove_dir_all(&base); } diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 0e330722..143947d5 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -18,8 +18,10 @@ use crate::render::Slot; use crate::theme::Tokens; use crate::widgets::register; -/// Files per batch (each batch joins the catalog as it is ready, so the progress window updates). -pub(crate) const BATCH: usize = 8; +/// Files per batch in the browser build (each batch joins the catalog as it is ready, so the +/// progress window updates); the desktop's worker uses [`lightcraft_engine::import::batch_size`]. +#[cfg(target_arch = "wasm32")] +const BATCH: usize = 8; /// The review dialog's first size (points). It can be resized; the photo grid takes the height. pub(crate) const DIALOG_SIZE: [f32; 2] = [960.0, 720.0]; @@ -273,7 +275,7 @@ impl ImportRun { let ctx = ctx.clone(); let work = move || { let files = job.expand(&queue); - for chunk in files.chunks(BATCH) { + for chunk in files.chunks(lightcraft_engine::import::batch_size()) { if cancel.load(Ordering::Relaxed) { break; }