From 07ae88e923239d5de3a3af257a17f984f9c78f15 Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Sun, 11 Oct 2026 10:00:45 +1100 Subject: [PATCH 1/2] Perf: preview builds render photos side by side and make the thumbnail from the view's source library.buildPreviews rendered one photo at a time, and each photo's thumbnail job decoded the file again at thumbnail level after (before, in fact) the view job had decoded it at preview level. Now the view renders first and its decoded source serves the thumbnail, and photos render in as many lanes as an export batch (two for 1:1 previews, a full-size source each); a panic in one photo counts as that photo's failure. 100 24 MP ARWs, standard previews, loaded machine: 49.4 s -> 38.4 s (CPU 3m28 -> 2m49). Sources: own design. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/engine/src/cmd/previews.rs | 111 ++++++++++++++++++++++++++---- 1 file changed, 98 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/cmd/previews.rs b/crates/engine/src/cmd/previews.rs index 50ccb4bd6..08dae2644 100644 --- a/crates/engine/src/cmd/previews.rs +++ b/crates/engine/src/cmd/previews.rs @@ -54,13 +54,14 @@ impl PreviewBuild { /// The jobs that build `id`'s previews: the largest grid thumbnail, then the view render at /// `edge` (`None` = 1:1, the photo's full size). -fn jobs_for(s: &mut Session, id: lightcraft_catalog::PhotoId, edge: Option) -> Vec { +pub(crate) fn jobs_for(s: &mut Session, id: lightcraft_catalog::PhotoId, edge: Option) -> Vec { let Some(p) = s.catalog.photo(id) else { return Vec::new() }; let full = p.width.max(p.height) as usize; let e = edge.unwrap_or(full).min(full.max(1)).max(1); let mut v = Vec::new(); - v.extend(s.thumb_job(id, THUMB_SIZES[THUMB_SIZES.len() - 1])); v.extend(s.loupe_job(id, e, e, true)); + // (after the view: it renders from the source the view loaded, see `run_photo`) + v.extend(s.thumb_job(id, THUMB_SIZES[THUMB_SIZES.len() - 1])); v } @@ -76,17 +77,66 @@ fn task_for(s: &Session, state: &PreviewBuild) -> TaskGuard { task } -fn run_all(jobs: Vec>, state: &PreviewBuild, task: &TaskGuard) { - for photo_jobs in jobs { - if state.cancel.load(Ordering::Relaxed) { - break; +/// Render one photo's jobs in order. The source the first job decodes serves the ones after it +/// (the grid thumbnail is made from the view's source instead of decoding the file again). +pub(crate) fn run_photo(jobs: Vec) -> bool { + let mut decoded: Option = None; + let mut ok = true; + for mut j in jobs { + if let Some(src) = &decoded + && !matches!(j.source, crate::media::SourceRef::Loaded(_)) + { + j.source = crate::media::SourceRef::Loaded(Box::new(src.clone())); } - let ok = photo_jobs.into_iter().all(|j| j.run().rendered.is_ok()); - if !ok { - state.failed.fetch_add(1, Ordering::Relaxed); + let r = j.run(); + ok &= r.rendered.is_ok(); + if decoded.is_none() { + decoded = r.loaded; } - let done = state.done.fetch_add(1, Ordering::Relaxed).saturating_add(1); - task.progress(done as u64, state.total as u64); + } + ok +} + +/// How many photos render side by side: as many as an export batch for standard previews (each +/// render spreads over the cores already; the others fill them while one decodes), two for 1:1 +/// previews (a full-size source each). +fn lanes(n: usize, full: bool) -> usize { + let l = crate::export::export_parallelism(n); + if full { l.min(2) } else { l } +} + +fn run_all(jobs: Vec>, state: &PreviewBuild, task: &TaskGuard, lanes: usize) { + let total = jobs.len(); + let queue = std::sync::Mutex::new(jobs.into_iter()); + let work = || { + loop { + if state.cancel.load(Ordering::Relaxed) { + break; + } + let next = queue.lock().unwrap_or_else(std::sync::PoisonError::into_inner).next(); + let Some(photo_jobs) = next else { break }; + let ok = crate::guard::catch("building previews", || run_photo(photo_jobs)).unwrap_or(false); + if !ok { + state.failed.fetch_add(1, Ordering::Relaxed); + } + let done = state.done.fetch_add(1, Ordering::Relaxed).saturating_add(1); + task.progress(done as u64, total as u64); + } + }; + #[cfg(not(target_arch = "wasm32"))] + std::thread::scope(|sc| { + for k in 1..lanes.max(1) { + // a lane that can't start leaves its photos to the others (and to this thread) + if std::thread::Builder::new().name(format!("previews-{k}")).spawn_scoped(sc, work).is_err() { + break; + } + } + work(); + }); + #[cfg(target_arch = "wasm32")] + { + let _ = lanes; + work(); } state.finished.store(true, Ordering::Relaxed); } @@ -107,9 +157,10 @@ fn build(s: &mut Session, p: &Value) -> Result { let state = Arc::new(PreviewBuild { total: jobs.len(), ..Default::default() }); s.preview_build = Some(state.clone()); let task = task_for(s, &state); + let lanes = lanes(jobs.len(), edge.is_none()); let wait = p.get("wait").and_then(Value::as_bool).unwrap_or(false) || cfg!(target_arch = "wasm32"); if wait { - run_all(jobs, &state, &task); + run_all(jobs, &state, &task, lanes); return Ok(state.json()); } #[cfg(not(target_arch = "wasm32"))] @@ -118,7 +169,7 @@ fn build(s: &mut Session, p: &Value) -> Result { let st = state.clone(); std::thread::Builder::new() .name("lc-build-previews".into()) - .spawn(move || run_all(jobs, &st, &task)) + .spawn(move || run_all(jobs, &st, &task, lanes)) .map_err(|e| bad(C, format!("could not start: {e}")))?; } Ok(state.json()) @@ -465,3 +516,37 @@ pub fn specs() -> Vec { }), ] } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::{jobs_for, run_photo}; + use crate::Session; + use crate::media::SourceRef; + + // Given a photo's preview jobs, the grid thumbnail is made from the source the view decoded, not + // by reading the file again (here: a thumbnail job pointing at a file that is not there) + #[test] + fn preview_build_makes_the_thumbnail_from_the_views_source() { + let mut s = Session::with_demo(); + let id = s.active().unwrap(); + let mut jobs = jobs_for(&mut s, id, Some(512)); + assert_eq!(jobs.len(), 2, "view and thumbnail"); + assert!(jobs[0].view_cache.is_some(), "the view renders first"); + jobs[1].source = SourceRef::File { path: "/nonexistent/photo.arw".into(), max_edge: 512, loader: None, fallback: None, denoise: None }; + assert!(run_photo(jobs), "the thumbnail rendered from the view's decoded source"); + } + + // Given many photos, a standard preview build renders them side by side and counts every one + #[test] + fn preview_build_of_many_photos_counts_each_once() { + let mut s = Session::with_demo(); + let ids: Vec = s.catalog.photos().map(|p| p.id.0).collect(); + assert!(ids.len() > 4); + let r = s.execute("library.buildPreviews", &json!({"size": "standard", "edge": 512, "ids": ids, "wait": true})).unwrap(); + assert_eq!(r["done"], json!(ids.len()), "{r}"); + assert_eq!(r["total"], json!(ids.len())); + assert_eq!(r["failed"], json!(0)); + } +} From ab240d6ecea04cc7afb0e3951bf25c43fd956eef Mon Sep 17 00:00:00 2001 From: Andrew Frahn Date: Sun, 11 Oct 2026 10:00:45 +1100 Subject: [PATCH 2/2] Perf: hue/saturation tables skip the sRGB round trip and fmodf in their per-pixel path HsvTable::apply took the value through linear -> sRGB -> linear even when the table's value scale is 1 (every hue/saturation-only table, as fitted camera looks are), and wrapped hues with rem_euclid (an fmodf call) three times per pixel. The value is now kept as it is for a unit scale and the sRGB-encoded value is reused otherwise; hues are wrapped by a helper that equals rem_euclid bit for bit (tested) without the call for values within one period. 2048 px render of a 24 MP ARW, single thread: 16.4 -> 15.1 G instructions, 4.94 -> 4.76 G cycles. Sources: own design. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/raw/src/profile.rs | 56 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/raw/src/profile.rs b/crates/raw/src/profile.rs index 582bf9446..566c2b1c3 100644 --- a/crates/raw/src/profile.rs +++ b/crates/raw/src/profile.rs @@ -76,7 +76,7 @@ impl HsvTable { /// Tri-linear lookup at hue `h` (degrees), saturation `s` and value `v` (table coordinates). pub fn lookup(&self, h: f32, s: f32, v: f32) -> [f32; 3] { - let hf = (h.rem_euclid(360.0) / 360.0 * self.hue_divisions as f32).clamp(0.0, self.hue_divisions as f32); + let hf = (wrap(h, 360.0) / 360.0 * self.hue_divisions as f32).clamp(0.0, self.hue_divisions as f32); let h0 = (hf as usize).min(self.hue_divisions - 1); let h1 = (h0 + 1) % self.hue_divisions; let th = (hf - h0 as f32).clamp(0.0, 1.0); @@ -113,7 +113,9 @@ impl HsvTable { // already above 1 (out-of-ProPhoto colours, highlight headroom) are only ever lowered. let s2 = limited(sat, ss); let v2 = if self.srgb_value && max <= 1.0 { - srgb_to_linear((linear_to_srgb(max) * vs).min(1.0)) + // (a value scale of 1 — every hue/saturation-only table — leaves the value as it is: + // no round trip through the sRGB curve, two `powf` per pixel) + if vs == 1.0 { max } else { srgb_to_linear((v_index * vs).min(1.0)) } } else if self.srgb_value { max * srgb_to_linear(vs.min(1.0)).min(1.0) } else { @@ -128,12 +130,27 @@ fn limited(x: f32, k: f32) -> f32 { if x <= 1.0 { (x * k).min(1.0) } else { x * k.min(1.0) } } +/// `x.rem_euclid(p)` (`p > 0`), bit for bit, without the `fmodf` call for the values it nearly always +/// gets here (within one period of `[0, p)`; both subtractions are exact there). +#[inline] +fn wrap(x: f32, p: f32) -> f32 { + if (0.0..p).contains(&x) { + x + } else if x > -p && x < 0.0 { + x + p + } else if (p..2.0 * p).contains(&x) { + x - p + } else { + x.rem_euclid(p) + } +} + fn hue_of(c: [f32; 3], max: f32, d: f32) -> f32 { if d <= 0.0 { return 0.0; } let h = if max == c[0] { - ((c[1] - c[2]) / d).rem_euclid(6.0) + wrap((c[1] - c[2]) / d, 6.0) } else if max == c[1] { (c[2] - c[0]) / d + 2.0 } else { @@ -145,7 +162,7 @@ fn hue_of(c: [f32; 3], max: f32, d: f32) -> f32 { /// HSV → RGB for any saturation ≥ 0 (above 1 yields negative components, the inverse of /// [`hue_of`] for out-of-gamut inputs). fn hsv_to_rgb(h: f32, s: f32, v: f32) -> [f32; 3] { - let hp = h.rem_euclid(360.0) / 60.0; + let hp = wrap(h, 360.0) / 60.0; let sector = (hp as u32).min(5); let f = hp - sector as f32; let (p, q, t) = (v * (1.0 - s), v * (1.0 - s * f), v * (1.0 - s * (1.0 - f))); @@ -282,6 +299,37 @@ mod tests { HsvTable { hue_divisions: h, sat_divisions: s, val_divisions: v, data: vec![[0.0, 1.0, 1.0]; h * s * v], srgb_value: false } } + #[test] + fn wrap_is_rem_euclid_bit_for_bit() { + for p in [6.0f32, 360.0] { + let mut x = -3.0 * p; + while x < 3.0 * p { + for v in [x, x.next_up(), x.next_down(), -0.0, 0.0, p, -p, 2.0 * p, p.next_down(), (-p).next_up()] { + assert_eq!(wrap(v, p).to_bits(), v.rem_euclid(p).to_bits(), "{v} mod {p}"); + } + x += p / 97.0; + } + for v in [f32::NAN, f32::INFINITY, f32::NEG_INFINITY, 1e30, -1e30] { + assert_eq!(wrap(v, p).is_nan(), v.rem_euclid(p).is_nan()); + } + } + } + + #[test] + fn unit_value_scale_with_srgb_values_keeps_the_value() { + let mut t = identity(6, 2, 3); + t.srgb_value = true; + for (i, e) in t.data.iter_mut().enumerate() { + *e = [(i % 7) as f32 - 3.0, 1.1, 1.0]; + } + for c in [[0.2f32, 0.1, 0.05], [0.9, 0.5, 0.7], [0.01, 0.02, 0.015], [1.5, 0.4, 0.2]] { + let max = c[0].max(c[1]).max(c[2]); + let out = t.apply(c); + let got = out[0].max(out[1]).max(out[2]); + assert!((got - max).abs() <= 1e-6 * max.max(1.0), "{c:?} -> {out:?}"); + } + } + fn close(a: [f32; 3], b: [f32; 3], tol: f32) -> bool { a.iter().zip(&b).all(|(x, y)| (x - y).abs() <= tol) }