Merge PR #927

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<usize>) -> Vec<RenderJob> {
+pub(crate) fn jobs_for(s: &mut Session, id: lightcraft_catalog::PhotoId, edge: Option<usize>) -> Vec<RenderJob> {
     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<Vec<RenderJob>>, 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<RenderJob>) -> bool {
+    let mut decoded: Option<crate::media::DecodedSource> = 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<Vec<RenderJob>>, 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<Value> {
     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<Value> {
         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<CommandSpec> {
         }),
     ]
 }
+
+#[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<u64> = 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));
+    }
+}

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)
     }