diff --git a/crates/engine/src/cmd/previews.rs b/crates/engine/src/cmd/previews.rs index d4969db80..a3b56dee0 100644 --- a/crates/engine/src/cmd/previews.rs +++ b/crates/engine/src/cmd/previews.rs @@ -178,7 +178,7 @@ fn build(s: &mut Session, p: &Value) -> Result { /// One photo's smart preview to build or discard: (photo, its file in the smart previews folder, /// how to load the original at preview size, whether the photo takes the per-file camera look — /// `camera_preview::file_local_look`, [`lightcraft_catalog::Photo::fitted_look`]: only those -/// proxies go stale when the fit changes). +/// proxies go stale when the fit changes; an older encoding makes any proxy stale). #[cfg(not(target_arch = "wasm32"))] type SmartJob = (lightcraft_catalog::PhotoId, std::path::PathBuf, Option, bool); @@ -205,15 +205,16 @@ struct SmartCounts { failed: Vec, } -/// `source` if it reads the photo's original, else `None`: a cached source decoded from a smart -/// preview (it has no decoder facts), a proxy standing in for an offline original, and the proxy -/// fallback of a file source are not the original. +/// `source` if it reads the photo's original, else `None`. A source decoded from a smart preview +/// (it now carries the proxy's stored facts), a proxy standing in for an offline original, and the +/// proxy fallback of a file source are not the original: refreshing a stale proxy from one of them +/// would stamp its older look as current. #[cfg(not(target_arch = "wasm32"))] fn original_only(source: crate::media::SourceRef) -> Option { use crate::media::SourceRef; match source { SourceRef::File { path, max_edge, loader, denoise, .. } => Some(SourceRef::File { path, max_edge, loader, fallback: None, denoise }), - SourceRef::Loaded(d) if d.info.is_some() => Some(SourceRef::Loaded(d)), + SourceRef::Loaded(d) if d.info.is_some() && !d.from_proxy => Some(SourceRef::Loaded(d)), _ => None, } } @@ -252,10 +253,11 @@ fn smart_run( } } else { // a complete, current proxy is kept; a missing or damaged one (cut short by a crash or a - // full drive) is (re)built, and so is one written by an older camera-look fit (see - // `camera_preview::LOOK_VERSION`) when its original can be read + // full drive) is (re)built, and so is one written by an older encoding or camera-look + // fit (see `smart::ENCODE_VERSION` and `camera_preview::LOOK_VERSION`) when its + // original can be read let damaged = path.exists(); - let stale = look && damaged && crate::smart::is_stale(&path); + let stale = damaged && crate::smart::is_stale_for(&path, look); if damaged && !stale && crate::smart::is_valid(&path) { n.built += 1; } else { @@ -363,14 +365,14 @@ fn smart(s: &mut Session, p: &Value) -> Result { ) } -/// Bring smart previews written by an older camera-look fit up to date, once per fit version: when -/// the smart previews folder has not been checked at this `LOOK_VERSION` (its marker, -/// [`crate::smart::look_checked`]), a worker thread (the UI never waits on the drives) looks at the -/// proxies of the raws that take the per-file camera look, the only ones the fit reaches (JPEG, -/// PNG… proxies are never rebuilt), and rebuilds the stale ones from their originals. One whose -/// original can't be read (offline) is kept as it is, still marked stale in its header: Build -/// Smart Previews rebuilds it once the original is back. Only existing proxies are touched: none -/// is built, and a missing folder is never created. Returns the thread, if started. +/// Bring smart previews up to date, once per fit-and-encoding version: when the smart previews +/// folder has not been checked at this build's versions (its marker, [`crate::smart::look_checked`]), +/// a worker thread (the UI never waits on the drives) looks at the proxies of the photos on disk +/// and rebuilds the stale ones from their originals — one whose stored pixels are an older encoding +/// (every photo), or, for a raw that takes the per-file camera look, one written by an older fit. +/// One whose original can't be read (offline) is kept as it is, still marked stale in its header: +/// Build Smart Previews rebuilds it once the original is back. Only existing proxies are touched: +/// none is built, and a missing folder is never created. Returns the thread, if started. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn refresh_stale_smart_previews(s: &mut Session) -> Option> { let dir = s.media.smart_dir.clone()?; @@ -378,9 +380,11 @@ pub(crate) fn refresh_stale_smart_previews(s: &mut Session) -> Option = s.catalog.photos().filter(|p| matches!(p.source, lightcraft_catalog::Source::File { .. }) && p.fitted_look()).collect(); - let jobs: Vec = - photos.iter().map(|p| (p.id, crate::smart::sharded_file_path(&dir, p), Some(s.media.origin_ref(&p.source, max_edge)), true)).collect(); + let photos: Vec<_> = s.catalog.photos().filter(|p| matches!(p.source, lightcraft_catalog::Source::File { .. })).collect(); + let jobs: Vec = photos + .iter() + .map(|p| (p.id, crate::smart::sharded_file_path(&dir, p), Some(s.media.origin_ref(&p.source, max_edge)), p.fitted_look())) + .collect(); std::thread::Builder::new() .name("lc-smart-look".into()) .spawn(move || refresh_stale(&dir, jobs)) @@ -394,7 +398,7 @@ fn refresh_stale(dir: &std::path::Path, jobs: Vec) { let stale: Vec = jobs .into_iter() .map(|(id, path, source, look)| (id, crate::smart::existing_file_path(dir, path), source, look)) - .filter(|(_, path, _, _)| crate::smart::is_stale(path)) + .filter(|(_, path, _, look)| crate::smart::is_stale_for(path, *look)) .collect(); if !stale.is_empty() { // `custom`: the folder is there already, and must not be recreated if it went away since diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 0688410ed..bed1a9835 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -183,11 +183,14 @@ pub struct DecodedSource { pub camera_tone: Option, /// The same picture developed from the AI-denoised mosaic, when the photo has one cached. pub denoised: Option>, + /// These pixels came from the photo's smart preview (an offline original), not from its file. + /// A refresh must never rebuild a proxy from itself: that would stamp its older look as current. + pub from_proxy: bool, } impl DecodedSource { pub fn new(image: Arc, info: Option) -> Self { - DecodedSource { image, info, camera_tone: None, denoised: None } + DecodedSource { image, info, camera_tone: None, denoised: None, from_proxy: false } } /// The picture to develop for a Denoise `amount` (0 to 1, see [`DevelopSettings::denoise_amount`]): the plain one, @@ -302,6 +305,7 @@ impl SourceRef { info: Some(info), camera_tone: None, denoised: twin.map(|t| Arc::new(Twin::new(Arc::new(t)))), + from_proxy: false, }) }) } else { diff --git a/crates/engine/src/smart.rs b/crates/engine/src/smart.rs index 5a610aa52..d3e042a55 100644 --- a/crates/engine/src/smart.rs +++ b/crates/engine/src/smart.rs @@ -423,12 +423,15 @@ fn header_encode_version(head: &serde_json::Value) -> u64 { head.get(ENC_FIELD).and_then(serde_json::Value::as_u64).unwrap_or(0) } -/// Whether the complete proxy at `path` is out of date for this build: written by an older -/// camera-look fit than this build's (so its stored curve, and the colour baked into its pixels, are -/// out of date), or by an older encoding of its own pixels ([`ENCODE_VERSION`], i.e. a lower JPEG -/// quality). A proxy from a newer build is left alone (rebuilding it here would only undo that -/// build's work), as is one that can't be read: those are [`is_valid`]'s business. -pub fn is_stale(path: &Path) -> bool { +/// Whether the complete proxy at `path` is out of date for a photo and must be rebuilt: written by +/// an older encoding of its own pixels ([`ENCODE_VERSION`], i.e. a lower JPEG quality) — this +/// reaches every proxy — or, when the photo takes the per-file camera look (`look`, +/// [`lightcraft_catalog::Photo::fitted_look`]), by an older camera-look fit than this build's (so +/// its stored curve, and the colour baked into its pixels, are out of date; the fit only reaches +/// those raws, so a JPEG's or DNG's proxy is never stale for it). A proxy from a newer build is +/// left alone (rebuilding it here would only undo that build's work), as is one that can't be +/// read: those are [`is_valid`]'s business. +pub fn is_stale_for(path: &Path, look: bool) -> bool { use std::io::Read; if !is_valid(path) { return false; @@ -440,9 +443,14 @@ pub fn is_stale(path: &Path) -> bool { } let Some(rest) = head.strip_prefix(MAGIC) else { return false }; let Some(nl) = rest.iter().position(|b| *b == b'\n') else { return false }; - rest.get(..nl) - .and_then(|h| serde_json::from_slice::(h).ok()) - .is_some_and(|h| header_look_version(&h) < u64::from(crate::camera_preview::LOOK_VERSION) || header_encode_version(&h) < ENCODE_VERSION) + rest.get(..nl).and_then(|h| serde_json::from_slice::(h).ok()).is_some_and(|h| { + header_encode_version(&h) < ENCODE_VERSION || (look && header_look_version(&h) < u64::from(crate::camera_preview::LOOK_VERSION)) + }) +} + +/// [`is_stale_for`] for a photo that takes the per-file camera look: both conditions apply. +pub fn is_stale(path: &Path) -> bool { + is_stale_for(path, true) } /// The marker file in a smart previews folder: the camera-look fit version and encode version its @@ -480,7 +488,13 @@ pub fn mark_look_checked(dir: &Path) -> Result<(), String> { /// Load the proxy at `path`. pub fn load(path: &Path) -> Result { let b = std::fs::read(path).map_err(|e| format!("{}: {e}", path.display()))?; - decode(&b).map(|(image, camera_tone, info)| crate::media::DecodedSource { image: Arc::new(image), info, camera_tone, denoised: None }) + decode(&b).map(|(image, camera_tone, info)| crate::media::DecodedSource { + image: Arc::new(image), + info, + camera_tone, + denoised: None, + from_proxy: true, + }) } #[cfg(test)] diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index f0b50db37..be34b6f23 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -1555,6 +1555,99 @@ fn smart_previews_from_an_older_look_fit_are_refreshed() { let _ = std::fs::remove_dir_all(&lib); } +/// Issue #1534 (review): a proxy's encoding version is migrated independently of the per-file +/// camera look. A DNG keeps an absolute white balance, so it never takes that look; at head its +/// existing proxy was kept at an older encoding even with its original online, so the decoder +/// facts the newer encoding carries never reached it through a normal rebuild. +#[test] +fn a_proxy_with_an_older_encoding_is_refreshed_without_the_camera_look() { + let src = temp_dir("smartenc-src"); + let lib = temp_dir("smartenc-lib"); + write_png(&src.join("a.png"), 3); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + // the test loader decodes the catalogue photo's "DNG" pixels + let pixels = lightcraft_raster::Rgb32f::from_fn(48, 32, |x, y| [0.02 + (x % 9) as f32 * 0.05, 0.03 + (y % 7) as f32 * 0.04, 0.06]); + let info = lightcraft_pipeline::SourceInfo { raw: true, as_shot_temp: 5200.0, as_shot_tint: 6.0, ..Default::default() }; + let (p, i) = (pixels.clone(), info.clone()); + s.media.file_loader = Some(std::sync::Arc::new(move |_path, _max_edge| Ok((p.clone(), i.clone())))); + s.execute("library.import", &json!({"paths": [src.join("a.png").to_string_lossy()]})).unwrap(); + let id = s.active().unwrap(); + let mut raw = (**s.catalog.photo(id).unwrap()).clone(); + raw.format = "DNG".into(); + raw.kind = lightcraft_catalog::MediaKind::Raw; + s.catalog.apply(lightcraft_catalog::Op::RemovePhoto { id }).unwrap(); + s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(raw) }).unwrap(); + assert!(s.catalog.photo(id).unwrap().develops_raw() && !s.catalog.photo(id).unwrap().fitted_look(), "a DNG does not take the camera look"); + s.execute("library.select", &json!({"ids": [id.0], "active": id.0})).unwrap(); + let dir = s.media.smart_dir.clone().unwrap(); + let file = crate::smart::sharded_file_path(&dir, s.catalog.photo(id).unwrap()); + let r = s.execute("library.smartPreviews", &json!({})).unwrap(); + assert_eq!((r["built"].as_u64(), r["refreshed"].as_u64()), (Some(1), Some(0)), "{r}"); + assert_eq!(smart_header(&file)["enc"], json!(crate::smart::ENCODE_VERSION)); + + // an existing proxy from the previous encoding, its original online: the migration must reach + // it through a normal rebuild even though the photo never takes the camera look + edit_smart_header(&file, |h| h["enc"] = json!(crate::smart::ENCODE_VERSION - 1)); + let stale_bytes = std::fs::read(&file).unwrap(); + assert!(crate::smart::is_stale(&file), "the older encoding is stale even without the camera look"); + let r = s.execute("library.smartPreviews", &json!({})).unwrap(); + assert_eq!((r["built"].as_u64(), r["refreshed"].as_u64(), r["staleKept"].as_u64()), (Some(1), Some(1), Some(0)), "{r}"); + assert_eq!(smart_header(&file)["enc"], json!(crate::smart::ENCODE_VERSION), "migrated to the current encoding"); + assert!(!crate::smart::is_stale(&file)); + assert_ne!(std::fs::read(&file).unwrap(), stale_bytes, "the proxy was rebuilt"); + let _ = std::fs::remove_dir_all(&src); + let _ = std::fs::remove_dir_all(&lib); +} + +/// Issue #1534 (review): a cached proxy source is not the original. After an offline edit rendered +/// from a stale proxy, Build Smart Previews read that cached source back and refreshed the proxy +/// from itself, stamping its older look as current. A stale refresh reads the original only, so an +/// offline original leaves the stored look — and the bytes — as they are. +#[test] +fn a_stale_refresh_never_rebuilds_a_proxy_from_itself() { + let src = temp_dir("smartself-src"); + let lib = temp_dir("smartself-lib"); + write_png(&src.join("a.png"), 3); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + let pixels = lightcraft_raster::Rgb32f::from_fn(48, 32, |x, y| [0.02 + (x % 9) as f32 * 0.05, 0.03 + (y % 7) as f32 * 0.04, 0.06]); + let info = lightcraft_pipeline::SourceInfo { raw: true, as_shot_temp: 5200.0, as_shot_tint: 6.0, ..Default::default() }; + let (p, i) = (pixels.clone(), info.clone()); + s.media.file_loader = Some(std::sync::Arc::new(move |_path, _max_edge| Ok((p.clone(), i.clone())))); + s.execute("library.import", &json!({"paths": [src.join("a.png").to_string_lossy()]})).unwrap(); + let id = s.active().unwrap(); + // a raw that takes the per-file camera look, so an older fit makes its proxy stale + let mut raw = (**s.catalog.photo(id).unwrap()).clone(); + raw.format = "ARW".into(); + raw.kind = lightcraft_catalog::MediaKind::Raw; + s.catalog.apply(lightcraft_catalog::Op::RemovePhoto { id }).unwrap(); + s.catalog.apply(lightcraft_catalog::Op::AddPhoto { photo: Box::new(raw) }).unwrap(); + assert!(s.catalog.photo(id).unwrap().fitted_look()); + s.execute("library.select", &json!({"ids": [id.0], "active": id.0})).unwrap(); + let dir = s.media.smart_dir.clone().unwrap(); + let file = crate::smart::sharded_file_path(&dir, s.catalog.photo(id).unwrap()); + let r = s.execute("library.smartPreviews", &json!({})).unwrap(); + assert_eq!(r["built"], 1, "{r}"); + edit_smart_header(&file, |h| h["look_version"] = json!(0)); + let stale_bytes = std::fs::read(&file).unwrap(); + assert!(crate::smart::is_stale(&file), "the stored look is behind this build's fit"); + + // the original goes offline, then a preview-size render loads the proxy into the source cache + std::fs::rename(&src, src.with_extension("offline")).unwrap(); + s.media.forget(id); + let _ = s.render_now(id, 600, 600).expect("renders from the smart preview"); + + // Build Smart Previews must not take that cached proxy for the original: it can't refresh a + // stale proxy from itself + let r = s.execute("library.smartPreviews", &json!({})).unwrap(); + assert_eq!((r["refreshed"].as_u64(), r["staleKept"].as_u64()), (Some(0), Some(1)), "{r}"); + assert_eq!(std::fs::read(&file).unwrap(), stale_bytes, "the offline proxy is preserved"); + assert!(crate::smart::is_stale(&file), "still stale: the original was not read"); + let _ = std::fs::remove_dir_all(src.with_extension("offline")); + let _ = std::fs::remove_dir_all(&lib); +} + /// The look marker: missing, damaged or older than this build's fit version counts as unchecked. #[test] fn smart_preview_look_marker_is_read_safely() {