diff --git a/apps/lightcraft-web/src/store.rs b/apps/lightcraft-web/src/store.rs index 8f7c8e2839..7f32051c2e 100644 --- a/apps/lightcraft-web/src/store.rs +++ b/apps/lightcraft-web/src/store.rs @@ -12,7 +12,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex}; -use lightcraft_engine::files::{load_bytes_with, probe_bytes}; +use lightcraft_engine::files::{load_bytes_decoded, probe_bytes}; use lightcraft_engine::media::{FileLoader, FileProbe}; /// Prefix of the catalog paths of files kept in browser storage. @@ -146,9 +146,9 @@ impl Originals { let s = self.clone(); // (an imported camera profile is not applied on the web, as in the render worker: a raw keeps its own colour, // whether it decodes in a worker or inline when workers are unavailable) - let loader: FileLoader = Arc::new(move |path: &str, max_edge: usize, _dcp: Option<&lightcraft_engine::media::DcpProfile>| { + let loader: FileLoader = Arc::new(move |path: &str, max_edge: usize, with: lightcraft_engine::media::RawDecode<'_>| { let bytes = s.get(path).ok_or_else(|| format!("{path}: the original is still loading from browser storage"))?; - load_bytes_with(&bytes, max_edge, None) + load_bytes_decoded(&bytes, max_edge, lightcraft_engine::media::RawDecode { dcp: None, outliers: with.outliers }) }); let s = self.clone(); let probe: FileProbe = Arc::new(move |path: &str| { @@ -285,15 +285,15 @@ mod tests { color: ColorData { illuminant: [21, 0], color_matrix: [Some(swap), None], ..Default::default() }, }; assert_ne!( - load_bytes_with(&dng, 64, Some(&profile)).unwrap().0.data, - load_bytes_with(&dng, 64, None).unwrap().0.data, + lightcraft_engine::files::load_bytes_with(&dng, 64, Some(&profile)).unwrap().0.data, + lightcraft_engine::files::load_bytes_with(&dng, 64, None).unwrap().0.data, "the profile changes a raw's colour where it is applied" ); let store = Originals::default(); store.insert("h1", Arc::from(dng)); let (loader, _) = store.hooks(); - let with = loader("web/h1/a.dng", 64, Some(&profile)).unwrap(); - let without = loader("web/h1/a.dng", 64, None).unwrap(); + let with = loader("web/h1/a.dng", 64, lightcraft_engine::media::RawDecode { dcp: Some(&profile), outliers: false }).unwrap(); + let without = loader("web/h1/a.dng", 64, Default::default()).unwrap(); assert_eq!(with.0.data, without.0.data, "the web loader leaves it out"); } diff --git a/apps/lightcraft-web/src/wire.rs b/apps/lightcraft-web/src/wire.rs index 53d389b6f7..bad3507783 100644 --- a/apps/lightcraft-web/src/wire.rs +++ b/apps/lightcraft-web/src/wire.rs @@ -129,7 +129,9 @@ impl WireJob { /// Identifies the decoded source this job needs. fn source_key(&self) -> String { match &self.origin { - Source::File { path } => format!("{}:{:?}:{}:{path}", self.cache_generation, self.source_identity, self.max_edge), + Source::File { path } => { + format!("{}:{:?}:{}:{}:{path}", self.cache_generation, self.source_identity, self.max_edge, self.settings.removes_hot_pixels()) + } Source::Demo { scene } => format!("{}:{:?}:{}:demo:{scene}", self.cache_generation, self.source_identity, self.max_edge), } } @@ -181,8 +183,10 @@ impl WorkerCore { None => { let src = match (&job.origin, original) { (Source::File { .. }, Some(bytes)) => { - let (image, info) = lightcraft_engine::files::load_bytes(bytes, job.max_edge)?; - DecodedSource::new(Arc::new(image), Some(info)) + let outliers = job.settings.removes_hot_pixels(); + let with = lightcraft_engine::media::RawDecode { dcp: None, outliers }; + let (image, info) = lightcraft_engine::files::load_bytes_decoded(bytes, job.max_edge, with)?; + DecodedSource { outliers, ..DecodedSource::new(Arc::new(image), Some(info)) } } (Source::File { path }, None) => return Err(format!("{path}: original not found in browser storage")), (origin @ Source::Demo { .. }, _) => self.media.origin_ref(origin, job.max_edge).load_source()?, diff --git a/crates/develop/src/lib.rs b/crates/develop/src/lib.rs index cb6caa3ba0..324fe29a02 100644 --- a/crates/develop/src/lib.rs +++ b/crates/develop/src/lib.rs @@ -27,6 +27,7 @@ impl DevelopSettings { s.wb.tint = as_shot_tint; s.detail.sharpen_amount = 40.0; s.detail.nr_color = 25.0; + s.detail.nr_outliers = true; s } @@ -38,6 +39,18 @@ impl DevelopSettings { serde_json::from_value(v.clone()) } + /// [`Self::to_json`] with Remove Hot Pixels (`detail.nr_outliers`) stated even when off, for JSON that is copied, + /// merged, scaled or compared key by key (copy / paste, presets, Auto Sync). The stored JSON leaves it out when + /// off, so settings saved before it keep their hash; a merge needs the `false`, or copying Detail would not turn + /// the box off and a preset applied at an amount would not find its base value. + pub fn to_json_for_merge(&self) -> Value { + let mut v = self.to_json(); + if let Some(detail) = v.get_mut("detail").and_then(Value::as_object_mut) { + detail.insert("nr_outliers".into(), Value::Bool(self.detail.nr_outliers)); + } + v + } + /// [`Self::from_json`] for settings from outside the app, such as a sidecar's `lc:settings` /// (issue #2555): kept in range as [`Self::merged`] keeps them. pub fn from_untrusted_json(v: &Value) -> Result { @@ -141,6 +154,12 @@ impl DevelopSettings { std::borrow::Cow::Owned(d) } + /// Whether a raw is decoded with its single outlier photosites replaced: Remove Hot Pixels is on and the Detail + /// section's eye is too (an Edit-panel section that is off contributes nothing, as in [`Self::effective`]). + pub fn removes_hot_pixels(&self) -> bool { + self.detail.nr_outliers && self.section_enabled("detail") + } + /// How much of the AI-denoised picture to mix in, 0 (none) to 1 (all of it): the Denoise amount, unless the Detail /// section is switched off or the value is not a number. pub fn denoise_amount(&self) -> f32 { @@ -167,7 +186,11 @@ impl DevelopSettings { self.curve = d; } Section::Color => self.wb.mode = WbMode::AsShot, - Section::Detail => self.enhance.denoise_on = None, + Section::Detail => { + self.enhance.denoise_on = None; + // not a slider: reset like them, to the general default (off) + self.detail.nr_outliers = false; + } // the creative profile is not a control: the chosen look goes back to the default one, // and so does the camera's own local tone mapping Section::Profile => { @@ -202,6 +225,37 @@ mod tests { use super::*; use serde_json::json; + /// Remove Hot Pixels (#1345) is on for a newly imported raw and off for everything else, and an edit saved + /// before the box existed reads as off, so it renders as it did. + #[test] + fn hot_pixel_box_is_on_for_new_raws_only() { + assert!(DevelopSettings::for_raw(5500.0, 10.0).detail.nr_outliers); + assert!(!DevelopSettings::default().detail.nr_outliers); + let mut saved = DevelopSettings::for_raw(5500.0, 10.0).to_json(); + let detail = saved.get_mut("detail").and_then(|d| d.as_object_mut()).unwrap(); + assert!(detail.remove("nr_outliers").is_some()); + let read = DevelopSettings::from_json(&saved).unwrap(); + assert!(!read.detail.nr_outliers); + assert_eq!(read.detail.nr_color, 25.0, "the rest of the saved edit is kept"); + assert_ne!(read.hash64(), DevelopSettings::for_raw(5500.0, 10.0).hash64(), "cached renders are told apart"); + // off is omitted: an edit saved before the box keeps its JSON, and so its hash and cached thumbnails + assert_eq!(read.to_json(), saved); + // copying Detail still carries the box when it is off + let copied = crate::extract_groups(&read, &[crate::SettingsGroup::Detail]); + assert_eq!(copied["detail"]["nr_outliers"], false); + let mut target = DevelopSettings::for_raw(5500.0, 10.0).to_json(); + crate::presets::deep_merge(&mut target, &copied); + assert!(!DevelopSettings::from_json(&target).unwrap().detail.nr_outliers); + // the Detail eye switches it off for the decode too, and Reset Detail resets it like the sliders + let mut raw = DevelopSettings::for_raw(5500.0, 10.0); + assert!(raw.removes_hot_pixels()); + raw.set_section_enabled("detail", false); + assert!(!raw.removes_hot_pixels()); + raw.set_section_enabled("detail", true); + raw.reset_section(Section::Detail); + assert!(!raw.detail.nr_outliers && !raw.removes_hot_pixels()); + } + #[test] fn serde_roundtrip_default_and_edited() { let mut s = DevelopSettings::default(); diff --git a/crates/develop/src/presets.rs b/crates/develop/src/presets.rs index d5a658f51c..b69f2df460 100644 --- a/crates/develop/src/presets.rs +++ b/crates/develop/src/presets.rs @@ -126,7 +126,7 @@ impl SettingsGroup { /// The subset of `s` covering `groups`, as a partial JSON object. pub fn extract_groups(s: &DevelopSettings, groups: &[SettingsGroup]) -> Value { - let full = s.to_json(); + let full = s.to_json_for_merge(); let mut out = Map::new(); for g in groups { for k in g.keys() { @@ -186,7 +186,7 @@ fn scale_patch(base: &Value, patch: &Value, t: f64) -> Value { /// Apply a partial settings object with an amount (1.0 = 100 %). Out-of-range values are clamped /// through the control specs, as every merge does ([`DevelopSettings::merged`]). pub fn apply_partial(s: &DevelopSettings, partial: &Value, amount: f64) -> DevelopSettings { - let patch = if (amount - 1.0).abs() < 1e-9 { Cow::Borrowed(partial) } else { Cow::Owned(scale_patch(&s.to_json(), partial, amount)) }; + let patch = if (amount - 1.0).abs() < 1e-9 { Cow::Borrowed(partial) } else { Cow::Owned(scale_patch(&s.to_json_for_merge(), partial, amount)) }; // (a white balance without a scale keeps its old meaning: `DevelopSettings::merged`) s.merged(&patch).unwrap_or_else(|_| s.clone()) } diff --git a/crates/develop/src/settings.rs b/crates/develop/src/settings.rs index 201b33d5f0..82de362e5f 100644 --- a/crates/develop/src/settings.rs +++ b/crates/develop/src/settings.rs @@ -632,6 +632,13 @@ pub struct Detail { pub nr_color: f64, pub nr_color_detail: f64, pub nr_color_smoothness: f64, + /// Replace single outlier photosites (noise spikes far above all their same-colour neighbours) in the + /// raw mosaic before demosaicing, so they do not become coloured dots (#1345). On for newly imported + /// raws ([`crate::DevelopSettings::for_raw`]); off for edits saved before it existed, so they look the + /// same. Raw files only. Omitted when off, so settings saved before it keep their JSON (and hash, and with it + /// their cached thumbnails). + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub nr_outliers: bool, } impl Default for Detail { @@ -649,6 +656,7 @@ impl Default for Detail { nr_color: 0.0, nr_color_detail: 50.0, nr_color_smoothness: 50.0, + nr_outliers: false, } } } diff --git a/crates/engine/examples/render_bench.rs b/crates/engine/examples/render_bench.rs index 789867c665..23e577b010 100644 --- a/crates/engine/examples/render_bench.rs +++ b/crates/engine/examples/render_bench.rs @@ -144,7 +144,7 @@ fn benchmark_load(args: &[String], n: usize, only: &str) { } println!(" {:<36} {}", "load_bytes(2560):", best(n, || drop(lightcraft_engine::files::load_bytes(&bytes, 2560).expect("load")))); let (loader, _) = lightcraft_engine::files::fs_hooks(); - println!(" {:<36} {}", "fs loader (2560, read+load):", best(n, || drop(loader(path, 2560, None).expect("load")))); + println!(" {:<36} {}", "fs loader (2560, read+load):", best(n, || drop(loader(path, 2560, Default::default()).expect("load")))); } } diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index 292ad27795..e78407f5ea 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -1166,7 +1166,7 @@ pub fn specs() -> Vec { |s, p| { let c = "preset.update"; let id = active(s, c)?; - let current = s.develop_of(id).unwrap_or_default().to_json(); + let current = s.develop_of(id).unwrap_or_default().to_json_for_merge(); let groups = groups_param(p, c)?; let pr = user_preset(s, p, c)?; pr.settings = match groups { diff --git a/crates/engine/src/cmd/previews.rs b/crates/engine/src/cmd/previews.rs index e9c40232e9..0d506b5adf 100644 --- a/crates/engine/src/cmd/previews.rs +++ b/crates/engine/src/cmd/previews.rs @@ -619,8 +619,8 @@ fn original_only(source: crate::media::SourceRef) -> Option { - Some(SourceRef::File { path, max_edge, loader, fallback: None, denoise, camera_profile: None, reduced: None }) + SourceRef::File { path, max_edge, loader, denoise, outliers, .. } => { + Some(SourceRef::File { path, max_edge, loader, fallback: None, denoise, camera_profile: None, outliers, reduced: None }) } SourceRef::Loaded(d) if d.info.is_some() && !d.from_proxy && !d.reduced => Some(SourceRef::Loaded(d)), _ => None, @@ -1042,6 +1042,7 @@ mod tests { fallback: None, denoise: None, camera_profile: None, + outliers: false, reduced: None, }; assert!(run_photo(jobs), "the thumbnail rendered from the view's decoded source"); diff --git a/crates/engine/src/cmd/query.rs b/crates/engine/src/cmd/query.rs index 5ceacb9539..8b96430248 100644 --- a/crates/engine/src/cmd/query.rs +++ b/crates/engine/src/cmd/query.rs @@ -273,6 +273,21 @@ pub fn specs() -> Vec { } Ok(serde_json::to_value(s.memory_report()).unwrap_or_default()) }), + cmd!(query "app.hotPixelLimit", "Hot Pixel Limit", [], None, "{percent?: number} — Remove Hot Pixels leaves a raw alone when more than this share of its photosites would be replaced (0.001..100 %, default 0.2 %; real light it keeps doesn't count); returns {percent, default}", always, |s, p| { + if let Some(v) = p.get("percent").filter(|v| !v.is_null()) { + let Some(percent) = v.as_f64().filter(|x| (0.001..=100.0).contains(x)) else { + return Err(bad("app.hotPixelLimit", "percent must be a number in 0.001..100")); + }; + // (the default as an `f32` from the app's settings, 0.2000000029…, is the default: its keys stay) + let default = lightcraft_raw::HOT_PIXEL_MAX_SHARE; + let share = if (percent / 100.0 / default - 1.0).abs() < 1e-6 { default } else { percent / 100.0 }; + if crate::files::set_hot_pixel_limit(share) { + // decoded with the old limit, and their render keys change + s.media.clear_sources(); + } + } + Ok(json!({"percent": crate::files::hot_pixel_limit() * 100.0, "default": lightcraft_raw::HOT_PIXEL_MAX_SHARE * 100.0})) + }), cmd!(query "journal.list", "Command Journal", [], None, "{limit?}", always, |s, p| { let lim = p.get("limit").and_then(Value::as_u64).unwrap_or(100) as usize; let start = s.journal.len().saturating_sub(lim); diff --git a/crates/engine/src/files.rs b/crates/engine/src/files.rs index 4f09be75ba..0e40d0fc5f 100644 --- a/crates/engine/src/files.rs +++ b/crates/engine/src/files.rs @@ -27,7 +27,7 @@ pub(crate) use colour::clipped_white; use fs::quick_image_srgb; pub use fs::{fs_hooks, fs_pair_loader, fs_reduced_loader}; pub(crate) use load::{DngWhite, WbScaleOf, dng_tone_curve}; -pub use load::{load_bytes, load_bytes_with, load_vec, load_vec_pair, load_vec_reduced}; +pub use load::{load_bytes, load_bytes_decoded, load_bytes_with, load_vec, load_vec_pair, load_vec_reduced}; #[cfg(test)] use preview::PLAN_PREFIX; pub(crate) use preview::decode_raw_preview; @@ -109,5 +109,33 @@ impl CameraColourModel<'_> { } } +/// Remove Hot Pixels leaves a raw alone when more than this share of its photosites would be replaced (Settings ▸ +/// Performance ▸ Hot pixel limit, `app.hotPixelLimit`), as `f64` bits. Process-wide, like the GPU switch. +static HOT_PIXEL_LIMIT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(lightcraft_raw::HOT_PIXEL_MAX_SHARE.to_bits()); + +/// The share of a raw's photosites Remove Hot Pixels may replace (0..1). +pub fn hot_pixel_limit() -> f64 { + f64::from_bits(HOT_PIXEL_LIMIT.load(std::sync::atomic::Ordering::Relaxed)) +} + +/// Set [`hot_pixel_limit`]; false (nothing changed) unless `share` is a new value in 0..=1. Sources decoded before +/// keep the old limit: the caller forgets them ([`crate::media::MediaCache::clear_sources`]). +pub fn set_hot_pixel_limit(share: f64) -> bool { + if !(0.0..=1.0).contains(&share) { + return false; + } + HOT_PIXEL_LIMIT.swap(share.to_bits(), std::sync::atomic::Ordering::Relaxed) != share.to_bits() +} + +/// What [`hot_pixel_limit`] adds to the render cache keys of `settings`: nothing at the default limit or when they +/// don't remove hot pixels, so those renders stay valid whatever the limit. +pub fn hot_pixel_key(settings: &lightcraft_develop::DevelopSettings) -> u64 { + let limit = hot_pixel_limit(); + if !settings.removes_hot_pixels() || limit.to_bits() == lightcraft_raw::HOT_PIXEL_MAX_SHARE.to_bits() { + return 0; + } + limit.to_bits().wrapping_mul(0x9e37_79b9_7f4a_7c15).rotate_left(29) +} + #[cfg(test)] pub(crate) mod tests; diff --git a/crates/engine/src/files/fs.rs b/crates/engine/src/files/fs.rs index ce8347d701..e839873cf0 100644 --- a/crates/engine/src/files/fs.rs +++ b/crates/engine/src/files/fs.rs @@ -1,7 +1,8 @@ use super::load::{load_vec, load_vec_pair, load_vec_reduced}; use super::preview::{fit_box, fs_preview_loader, preview_orientation}; use super::probe::probe_bytes; -use super::{Arc, DcpProfile, DenoiseSpec, FileLoader, FileProbe, Filter, PairLoader, ReducedLoader, fit}; +use super::{Arc, DenoiseSpec, FileLoader, FileProbe, Filter, PairLoader, ReducedLoader, fit}; +use crate::media::RawDecode; /// The file's first [`lightcraft_raw::PREVIEW_PREFIX`] bytes (or the whole file when it is shorter), /// which is what every format [`lightcraft_raw::bounded_preview`] covers keeps its pointer in. @@ -60,13 +61,13 @@ pub(super) fn quick_image_srgb(header: &[u8], window: &[u8], adobe_rgb: bool, ma /// The filesystem-backed [`PairLoader`]: the file is read and decoded once, and developed as usual and from the photo's /// denoised picture. It holds two pictures at once, so it counts for more at the memory gate than a plain load. pub fn fs_pair_loader() -> PairLoader { - Arc::new(|path: &str, max_edge: usize, spec: &DenoiseSpec, dcp: Option<&DcpProfile>| { + Arc::new(|path: &str, max_edge: usize, spec: &DenoiseSpec, with: RawDecode<'_>| { let len = std::fs::metadata(path).map(|m| m.len() as usize).unwrap_or(0); let weight = len * if max_edge <= crate::media::SourceLevel::Thumb.max_edge() { 4 } else { 10 }; let gate = crate::memory::work_gate(); let _permit = if crate::memory::is_background() { gate.acquire(weight) } else { gate.acquire_urgent(weight) }; let bytes = std::fs::read(path).map_err(|e| format!("{path}: {e}"))?; - let r = load_vec_pair(bytes, max_edge, spec, dcp); + let r = load_vec_pair(bytes, max_edge, spec, with); crate::memory::release(); r }) @@ -81,7 +82,7 @@ pub fn fs_pair_loader() -> PairLoader { /// yields to them. pub fn fs_hooks() -> (FileLoader, FileProbe) { let loader: FileLoader = - Arc::new(|path: &str, max_edge: usize, dcp: Option<&DcpProfile>| fs_load(path, max_edge, |bytes| load_vec(bytes, max_edge, dcp))); + Arc::new(|path: &str, max_edge: usize, with: RawDecode<'_>| fs_load(path, max_edge, |bytes| load_vec(bytes, max_edge, with))); let probe: FileProbe = Arc::new(|path: &str| { use std::io::Read; // one open, then everything from the open file: its size and modification time are not @@ -101,7 +102,7 @@ pub fn fs_hooks() -> (FileLoader, FileProbe) { /// The filesystem [`ReducedLoader`]: [`load_vec_reduced`] of the file, gated like [`fs_hooks`]' loader. pub fn fs_reduced_loader() -> ReducedLoader { - Arc::new(|path: &str, max_edge: usize, dcp: Option<&DcpProfile>| fs_load(path, max_edge, |bytes| load_vec_reduced(bytes, max_edge, dcp))) + Arc::new(|path: &str, max_edge: usize, with: RawDecode<'_>| fs_load(path, max_edge, |bytes| load_vec_reduced(bytes, max_edge, with))) } /// Read `path` and `load` it, waiting on [`crate::memory::work_gate`] as [`fs_hooks`] explains. diff --git a/crates/engine/src/files/load.rs b/crates/engine/src/files/load.rs index 004723dca8..0e396dd815 100644 --- a/crates/engine/src/files/load.rs +++ b/crates/engine/src/files/load.rs @@ -2,6 +2,7 @@ use super::colour::bin_factor; use super::preview::{fit_box, load_embedded_preview}; use super::probe::preview_reason; use super::{Arc, CameraColourModel, DcpProfile, DenoiseSpec, Filter, HIGHLIGHT_CLIP, Orientation, Rgb32f, SourceInfo, fit, xy_to_temp_tint}; +use crate::media::RawDecode; /// Decode a file into a linear Rec.2020 image no larger than `max_edge`, oriented. /// @@ -9,26 +10,31 @@ use super::{Arc, CameraColourModel, DcpProfile, DenoiseSpec, Filter, HIGHLIGHT_C /// instead of each one waking the pool from outside and waiting for it (which costs more than the /// loops themselves when the machine is busy). pub fn load_bytes(bytes: &[u8], max_edge: usize) -> Result<(Rgb32f, SourceInfo), String> { - rayon::scope(|_| load_bytes_now(std::borrow::Cow::Borrowed(bytes), max_edge, None, false, None).map(|(img, _, info, _)| (img, info))) + load_bytes_decoded(bytes, max_edge, RawDecode::default()) } /// [`load_bytes`] developing a raw file with an imported camera profile (`dcp`) in place of its own colour model. pub fn load_bytes_with(bytes: &[u8], max_edge: usize, dcp: Option<&DcpProfile>) -> Result<(Rgb32f, SourceInfo), String> { - rayon::scope(|_| load_bytes_now(std::borrow::Cow::Borrowed(bytes), max_edge, None, false, dcp).map(|(img, _, info, _)| (img, info))) + load_bytes_decoded(bytes, max_edge, RawDecode { dcp, outliers: false }) +} + +/// [`load_bytes`] decoding a raw file as `with` says. +pub fn load_bytes_decoded(bytes: &[u8], max_edge: usize, with: RawDecode<'_>) -> Result<(Rgb32f, SourceInfo), String> { + rayon::scope(|_| load_bytes_now(std::borrow::Cow::Borrowed(bytes), max_edge, None, false, with).map(|(img, _, info, _)| (img, info))) } /// [`load_bytes`] taking the file's bytes: a raw file's bytes are freed as soon as it is decoded /// (less memory held while it is developed). -pub fn load_vec(bytes: Vec, max_edge: usize, dcp: Option<&DcpProfile>) -> Result<(Rgb32f, SourceInfo), String> { - rayon::scope(move |_| load_bytes_now(std::borrow::Cow::Owned(bytes), max_edge, None, false, dcp).map(|(img, _, info, _)| (img, info))) +pub fn load_vec(bytes: Vec, max_edge: usize, with: RawDecode<'_>) -> Result<(Rgb32f, SourceInfo), String> { + rayon::scope(move |_| load_bytes_now(std::borrow::Cow::Owned(bytes), max_edge, None, false, with).map(|(img, _, info, _)| (img, info))) } /// [`load_vec`] for a thumbnail: a DNG that keeps reduced-resolution copies of its raw image (Lightroom's HDR and /// panorama merges) develops the smallest that covers `max_edge` instead of the whole image. The flag says whether /// it did: such a picture is not the full image, and an export never renders from it. -pub fn load_vec_reduced(bytes: Vec, max_edge: usize, dcp: Option<&DcpProfile>) -> Result<(Rgb32f, SourceInfo, bool), String> { +pub fn load_vec_reduced(bytes: Vec, max_edge: usize, with: RawDecode<'_>) -> Result<(Rgb32f, SourceInfo, bool), String> { rayon::scope(move |_| { - load_bytes_now(std::borrow::Cow::Owned(bytes), max_edge, None, true, dcp).map(|(img, _, info, reduced)| (img, info, reduced)) + load_bytes_now(std::borrow::Cow::Owned(bytes), max_edge, None, true, with).map(|(img, _, info, reduced)| (img, info, reduced)) }) } @@ -39,10 +45,10 @@ pub fn load_vec_pair( bytes: Vec, max_edge: usize, spec: &DenoiseSpec, - dcp: Option<&DcpProfile>, + with: RawDecode<'_>, ) -> Result<(Rgb32f, Option, SourceInfo), String> { rayon::scope(move |_| { - load_bytes_now(std::borrow::Cow::Owned(bytes), max_edge, Some(spec), false, dcp).map(|(img, twin, info, _)| (img, twin, info)) + load_bytes_now(std::borrow::Cow::Owned(bytes), max_edge, Some(spec), false, with).map(|(img, twin, info, _)| (img, twin, info)) }) } @@ -73,8 +79,9 @@ fn load_bytes_now( max_edge: usize, denoise: Option<&DenoiseSpec>, allow_reduced: bool, - dcp: Option<&DcpProfile>, + with: RawDecode<'_>, ) -> Result<(Rgb32f, Option, SourceInfo, bool), String> { + let dcp = with.dcp; if lightcraft_raw::probe(&bytes).is_some() { // a thumbnail (`allow_reduced`, never an export) of a DNG that keeps smaller copies of its raw image // (Lightroom's merges) develops the smallest that covers it; a denoised photo's twin needs the full mosaic @@ -96,6 +103,14 @@ fn load_bytes_now( } Err(Err(e)) => return Err(e), }; + // single outlier photosites, replaced in the mosaic before anything is developed from it (#1345) + if with.outliers { + let t = std::time::Instant::now(); + let n = raw.suppress_hot_pixels_with(super::hot_pixel_limit()); + if lightcraft_pipeline::profiling() { + eprintln!("[profile] raw outliers: {n} photosites replaced in {:.1} ms", t.elapsed().as_secs_f64() * 1e3); + } + } // Embedded lens corrections are applied by the pipeline ("Enable Profile Corrections"), not baked in // (removed before the camera look's binned sensor proxy too, which needs an empty `OpcodeList3`). let info = raw.info(); diff --git a/crates/engine/src/files/tests.rs b/crates/engine/src/files/tests.rs index 4fab917c82..e8eabc72db 100644 --- a/crates/engine/src/files/tests.rs +++ b/crates/engine/src/files/tests.rs @@ -591,7 +591,7 @@ fn cached_denoise_uses_the_same_white_balance_profile_and_orientation_as_plain_r product::write(&path, &camera_rgb, key).unwrap(); let spec = DenoiseSpec { product: path, key: key.into(), make: None }; let (ordinary, _) = load_bytes(&bytes, 2048).unwrap(); - let (plain, cached, _) = load_vec_pair(bytes, 2048, &spec, None).unwrap(); + let (plain, cached, _) = load_vec_pair(bytes, 2048, &spec, Default::default()).unwrap(); assert_eq!(plain.data, ordinary.data, "requesting a twin must not change the ordinary path"); let cached = cached.unwrap(); assert_eq!((plain.width, plain.height), (24, 32)); @@ -1255,10 +1255,10 @@ fn exports_never_develop_a_dngs_reduced_copy() { let mean = |img: &lightcraft_raster::Rgba8| img.data.iter().map(|p| p[1] as f64).sum::() / img.data.len().max(1) as f64; // the loaders: only the thumbnail's develops the copy, and only when it covers the size - let (copy, _, used) = load_vec_reduced(bytes.clone(), 512, None).unwrap(); + let (copy, _, used) = load_vec_reduced(bytes.clone(), 512, Default::default()).unwrap(); assert!(used, "a thumbnail develops the copy"); - assert!(!load_vec_reduced(bytes.clone(), 1024, None).unwrap().2, "no copy covers 1024 px"); - let (full, _) = load_vec(bytes.clone(), 512, None).unwrap(); + assert!(!load_vec_reduced(bytes.clone(), 1024, Default::default()).unwrap().2, "no copy covers 1024 px"); + let (full, _) = load_vec(bytes.clone(), 512, Default::default()).unwrap(); assert!(g(©) > 4.0 * g(&full), "the fixture's copy is much brighter: {} vs {}", g(©), g(&full)); let dir = std::env::temp_dir().join(format!("lc-reduced-export-{}", std::process::id())); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index bade3892d3..b0e3d2b9e5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1016,8 +1016,9 @@ impl Session { return Vec::new(); } let Some(old) = self.develop_of(id) else { return Vec::new() }; - let mut old_json = old.to_json(); - let mut new_json = new.to_json(); + // (Remove Hot Pixels, omitted when off, is stated: switching it off is a change) + let mut old_json = old.to_json_for_merge(); + let mut new_json = new.to_json_for_merge(); // False is omitted from stored profile JSON. Keep it explicit for this comparison so // switching camera local tone mapping off is a change without copying untouched values. old_json["profile"]["camera_local_tone"] = Value::Bool(old.profile.camera_local_tone); diff --git a/crates/engine/src/media.rs b/crates/engine/src/media.rs index 9c6de8dd2c..dcba02494d 100644 --- a/crates/engine/src/media.rs +++ b/crates/engine/src/media.rs @@ -44,8 +44,8 @@ use settings_hash::SETTINGS_HASH_ENTRIES; use settings_hash::SettingsHashes; pub use source::{ DISK_CACHE_BYTES, DISK_CACHE_PER_PHOTO, DecodedSource, DenoiseSpec, FACE_EDGE_STEP, FileLoader, FileProbe, MakeProduct, PairLoader, - PreviewLoader, ProbeInfo, RENDER_CACHE_VERSION, ReducedLoader, SourceLevel, SourceRef, THUMB_SIZES, Twin, default_disk_cache_bytes, face_edge, - thumb_bucket, + PreviewLoader, ProbeInfo, RENDER_CACHE_VERSION, RawDecode, ReducedLoader, SourceLevel, SourceRef, THUMB_SIZES, Twin, default_disk_cache_bytes, + face_edge, thumb_bucket, }; pub struct MediaCache { diff --git a/crates/engine/src/media/cache.rs b/crates/engine/src/media/cache.rs index 036b4730a4..acba8b85be 100644 --- a/crates/engine/src/media/cache.rs +++ b/crates/engine/src/media/cache.rs @@ -334,11 +334,12 @@ impl MediaCache { } fn source_ref_plain(&mut self, p: &Photo, level: SourceLevel, cached: bool, wanted: Option<(String, Arc)>) -> SourceRef { + let outliers = p.develop.removes_hot_pixels(); // (a source developed with another camera profile, or none, doesn't stand in; a smart preview // has no camera profile, and decoding it again gives the same picture, so it stands in while the // original is offline) if cached - && let Some(a) = self.get_source(p.id, level) + && let Some(a) = self.get_source(p.id, level).filter(|a| a.fits_outliers(outliers)) && (a.camera_profile == wanted.as_ref().map(|(id, _)| id.clone()) || (a.from_proxy && self.original_offline(p))) { return SourceRef::Loaded(Box::new(a)); @@ -348,7 +349,7 @@ impl MediaCache { if cached && level == SourceLevel::Thumb && self.denoise.spec(p.id).is_none() - && let Some(big) = self.get_source(p.id, SourceLevel::Preview) + && let Some(big) = self.get_source(p.id, SourceLevel::Preview).filter(|b| b.fits_outliers(outliers)) && (big.camera_profile == wanted.as_ref().map(|(tag, _)| tag.clone()) || (big.from_proxy && self.original_offline(p))) { return SourceRef::Fit { from: Box::new(big), max_edge: level.max_edge() }; @@ -372,15 +373,17 @@ impl MediaCache { return SourceRef::Smart { path: sp }; } let mut r = self.origin_ref(&p.source, max_edge); - if let SourceRef::File { fallback, camera_profile, .. } = &mut r { + if let SourceRef::File { fallback, camera_profile, outliers: o, .. } = &mut r { *fallback = Some(sp); *camera_profile = wanted; + *o = outliers; } return r; } let mut r = self.origin_ref(&p.source, max_edge); - if let SourceRef::File { camera_profile, .. } = &mut r { + if let SourceRef::File { camera_profile, outliers: o, .. } = &mut r { *camera_profile = wanted; + *o = outliers; } r } @@ -421,6 +424,7 @@ impl MediaCache { fallback: None, denoise: None, camera_profile: None, + outliers: false, reduced: None, }, } @@ -432,6 +436,7 @@ impl MediaCache { fallback: None, denoise: None, camera_profile: None, + outliers: false, reduced: None, }, } diff --git a/crates/engine/src/media/session.rs b/crates/engine/src/media/session.rs index ed472972bd..a3d5069177 100644 --- a/crates/engine/src/media/session.rs +++ b/crates/engine/src/media/session.rs @@ -58,7 +58,8 @@ impl crate::Session { let content = Hasher128::new().str(&content_key).finish().0 as u64; let source_key = super::quick::source_identity(&p); // Temporary "before" settings must not replace the photo's cached develop hash. - let settings_hash = if before { settings.hash64() } else { self.media.settings_hashes.get(id, &settings) }; + let settings_hash = + if before { settings.hash64() ^ crate::files::hot_pixel_key(&settings) } else { self.media.settings_hashes.get(id, &settings) }; let salt = self.media.denoise_salt(id, &settings); let profile_salt = self.media.camera_profile_salt(&settings.profile.id); let lens = super::quick::lens_key(&p); @@ -131,6 +132,7 @@ impl crate::Session { // (the photo's database lens profile stays part of it: a profile looked up again renders afresh) let lens = self.catalog.photo(id).map_or(0, |p| super::quick::lens_key(p)); job.key = settings.hash64() + ^ crate::files::hot_pixel_key(settings) ^ ((max_w as u64) << 40) ^ ((max_h as u64) << 20) ^ (apply_crop as u64) @@ -144,7 +146,11 @@ impl crate::Session { /// `profile_salt`: [`MediaCache::camera_profile_salt`] of `settings`' profile. pub fn variant_key(p: &Photo, settings: &DevelopSettings, edge: usize, profile_salt: u64) -> Hash128 { let mut k = Hasher128::new(); - k.str(&content_key(p)).str("variant").u64(settings.hash64()).u64(edge as u64).u64(RENDER_CACHE_VERSION); + k.str(&content_key(p)) + .str("variant") + .u64(settings.hash64() ^ crate::files::hot_pixel_key(settings)) + .u64(edge as u64) + .u64(RENDER_CACHE_VERSION); with_process(&mut k, settings).u64(crate::camera_profiles::cache_key()); if profile_salt != 0 { k.str("dcp").u64(profile_salt); @@ -272,7 +278,11 @@ impl crate::Session { let salt = self.media.denoise_salt(p.id, &p.develop); let profile_salt = self.media.camera_profile_salt(&p.develop.profile.id); let mut k = Hasher128::new(); - k.str(&content_key(p)).str("view").u64(p.develop.hash64()).u64(apply_crop as u64).u64(RENDER_CACHE_VERSION); + k.str(&content_key(p)) + .str("view") + .u64(p.develop.hash64() ^ crate::files::hot_pixel_key(&p.develop)) + .u64(apply_crop as u64) + .u64(RENDER_CACHE_VERSION); with_process(&mut k, &p.develop).u64(crate::camera_profiles::cache_key()); if salt != 0 { k.u64(salt); @@ -404,8 +414,14 @@ impl crate::Session { let profile_salt = self.media.camera_profile_salt(&p.develop.profile.id); let cached = vec![(self.media.rendered.clone(), self.view_key(&p, apply_crop))]; // (the photo too: a virtual copy shares the file and, unedited, the settings) - let h = - Hasher128::new().str(&content_key(&p)).str("quick").u64(p.develop.hash64()).u64(apply_crop as u64).u64(id.0).u64(profile_salt).finish(); + let h = Hasher128::new() + .str(&content_key(&p)) + .str("quick") + .u64(p.develop.hash64() ^ crate::files::hot_pixel_key(&p.develop)) + .u64(apply_crop as u64) + .u64(id.0) + .u64(profile_salt) + .finish(); let key = h.0 as u64; let embedded = self.embedded_of(&p).map(|(path, l)| (path, l, max_edge.clamp(1, SourceLevel::Preview.max_edge()))); let embedded_cache = embedded.as_ref().map(|(_, _, edge)| { diff --git a/crates/engine/src/media/settings_hash.rs b/crates/engine/src/media/settings_hash.rs index 39f76d52bb..66598924dc 100644 --- a/crates/engine/src/media/settings_hash.rs +++ b/crates/engine/src/media/settings_hash.rs @@ -17,7 +17,7 @@ impl SettingsHashes { { return *hash; } - let hash = settings.hash64(); + let hash = settings.hash64() ^ crate::files::hot_pixel_key(settings); #[cfg(test)] { self.computations += 1; diff --git a/crates/engine/src/media/source.rs b/crates/engine/src/media/source.rs index 50cedabe5a..ce2e89862f 100644 --- a/crates/engine/src/media/source.rs +++ b/crates/engine/src/media/source.rs @@ -87,15 +87,22 @@ pub(super) fn source_edge_needed(p: &Photo, settings: &DevelopSettings, apply_cr if needed.is_finite() { needed.min(usize::MAX as f64 / 2.0) as usize } else { out_long } } -/// Decodes a file into a linear Rec.2020 image no larger than `max_edge` (set by the app). -/// The third argument is the photo's imported camera profile ([`crate::cmd::dcp_profiles`]): a raw file is developed -/// with its colour model. -pub type FileLoader = Arc) -> Result<(Rgb32f, SourceInfo), String> + Send + Sync>; +/// How a raw file is decoded for a photo. +#[derive(Clone, Copy, Default)] +pub struct RawDecode<'a> { + /// The photo's imported camera profile ([`crate::cmd::dcp_profiles`]): a raw file is developed with its colour model. + pub dcp: Option<&'a DcpProfile>, + /// Replace a raw's single outlier photosites first ([`DevelopSettings`] `detail.nr_outliers`). + pub outliers: bool, +} + +/// Decodes a file into a linear Rec.2020 image no larger than `max_edge` (set by the app), as the third argument says. +pub type FileLoader = Arc) -> Result<(Rgb32f, SourceInfo), String> + Send + Sync>; /// [`FileLoader`] for a thumbnail: a raw file that keeps smaller copies of its raw image (Lightroom's merged DNGs) /// develops the smallest that covers `max_edge` (set by the app); the flag says it did. Never used for an export, /// which must render the full image. -pub type ReducedLoader = Arc) -> Result<(Rgb32f, SourceInfo, bool), String> + Send + Sync>; +pub type ReducedLoader = Arc) -> Result<(Rgb32f, SourceInfo, bool), String> + Send + Sync>; /// A raw file's embedded (camera-rendered) preview as display sRGB, oriented, no larger than /// `max_edge` (set by the app). `None`: no usable preview. @@ -118,8 +125,7 @@ pub struct DenoiseSpec { /// Decodes a raw file once and develops it twice: as usual, and from the denoised picture in `spec` when that can be /// read (`None` when it cannot). Both are at most `max_edge` long and the same size. -pub type PairLoader = - Arc) -> Result<(Rgb32f, Option, SourceInfo), String> + Send + Sync>; +pub type PairLoader = Arc) -> Result<(Rgb32f, Option, SourceInfo), String> + Send + Sync>; /// A photo's denoised picture next to its plain one, and the last mix of the two that was asked for. pub struct Twin { @@ -152,6 +158,8 @@ pub struct DecodedSource { /// The imported camera profile (`dcp:…` id) the raw file was developed with, if any: a cached source made with /// another one (or none) doesn't stand in for it. pub camera_profile: Option, + /// Decoded with a raw's single outlier photosites replaced (`detail.nr_outliers`). + pub outliers: bool, /// 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, @@ -162,7 +170,14 @@ pub struct DecodedSource { impl DecodedSource { pub fn new(image: Arc, info: Option) -> Self { - DecodedSource { image, info, camera_tone: None, denoised: None, camera_profile: None, from_proxy: false, reduced: false } + DecodedSource { image, info, camera_tone: None, denoised: None, camera_profile: None, outliers: false, from_proxy: false, reduced: false } + } + + /// Whether this source can stand for one decoded with `outliers`: a raw decode must match it; anything without a + /// mosaic (a rendered file, a smart preview, a demo scene) has nothing the flag would change. A smart preview's + /// facts say `raw` for a raw photo, so it is told apart by `from_proxy`. + pub fn fits_outliers(&self, outliers: bool) -> bool { + self.outliers == outliers || self.from_proxy || !self.info.as_ref().is_some_and(|i| i.raw) } /// The picture to develop for a Denoise `amount` (0 to 1, see [`DevelopSettings::denoise_amount`]): the plain one, @@ -256,6 +271,8 @@ pub enum SourceRef { denoise: Option<(DenoiseSpec, PairLoader)>, /// The photo's imported camera profile (id and profile): its raw file is developed with it. camera_profile: Option<(String, Arc)>, + /// Replace a raw's single outlier photosites before developing it (`detail.nr_outliers`). + outliers: bool, /// A thumbnail's loader, used instead of `loader` (see [`ReducedLoader`]); `None` for every other job. reduced: Option, }, @@ -283,17 +300,21 @@ impl SourceRef { return Ok(DecodedSource { image: Arc::new(image), denoised: None, ..(**from).clone() }); } SourceRef::Demo { scene, max_edge } => crate::demo::scene_pixels(scene, *max_edge)?, - SourceRef::File { path, max_edge, loader, fallback, denoise, camera_profile, reduced } => { - let dcp = camera_profile.as_ref().map(|(_, p)| p.as_ref()); + SourceRef::File { path, max_edge, loader, fallback, denoise, camera_profile, outliers, reduced } => { + let with = RawDecode { dcp: camera_profile.as_ref().map(|(_, p)| p.as_ref()), outliers: *outliers }; let made_with = camera_profile.as_ref().map(|(id, _)| id.clone()); let plain = || match (reduced, loader) { - (Some(l), _) => l(path, *max_edge, dcp).map(|(image, info, reduced)| DecodedSource { + (Some(l), _) => l(path, *max_edge, with).map(|(image, info, reduced)| DecodedSource { reduced, camera_profile: made_with.clone(), + outliers: *outliers, + ..DecodedSource::new(Arc::new(image), Some(info)) + }), + (None, Some(l)) => l(path, *max_edge, with).map(|(image, info)| DecodedSource { + camera_profile: made_with.clone(), + outliers: *outliers, ..DecodedSource::new(Arc::new(image), Some(info)) }), - (None, Some(l)) => l(path, *max_edge, dcp) - .map(|(image, info)| DecodedSource { camera_profile: made_with.clone(), ..DecodedSource::new(Arc::new(image), Some(info)) }), (None, None) => Err(format!("no decoder available for {path}")), }; // the denoised picture comes with the plain one from a single decode; when it cannot be read the @@ -307,7 +328,7 @@ impl SourceRef { None => true, }; if made { - pair(path, *max_edge, spec, dcp).and_then(|(image, twin, info)| { + pair(path, *max_edge, spec, with).and_then(|(image, twin, info)| { if spec.make.is_some() && twin.is_none() { return Err("the cached AI Denoise picture could not be read; clear the denoise cache and retry".into()); } @@ -317,6 +338,7 @@ impl SourceRef { camera_tone: None, denoised: twin.map(|t| Arc::new(Twin::new(Arc::new(t)))), camera_profile: made_with.clone(), + outliers: *outliers, from_proxy: false, reduced: false, }) diff --git a/crates/engine/src/media/tests.rs b/crates/engine/src/media/tests.rs index 7d5f80b6e5..0db6e0a958 100644 --- a/crates/engine/src/media/tests.rs +++ b/crates/engine/src/media/tests.rs @@ -91,6 +91,7 @@ fn decoder_info_survives_render_jobs_cache_and_eviction() { fallback: None, denoise: None, camera_profile: None, + outliers: false, reduced: None, }; job.info = SourceInfo::default(); // Header facts cannot override decoder facts. @@ -148,6 +149,7 @@ fn old_relative_white_balance_on_a_kelvin_camera_keeps_its_look_and_learns_the_a fallback: None, denoise: None, camera_profile: None, + outliers: false, reduced: None, }; let undo_before = s.undo.len(); diff --git a/crates/engine/src/media/thumbnail_hash_tests.rs b/crates/engine/src/media/thumbnail_hash_tests.rs index 6f9fcab339..9379288e26 100644 --- a/crates/engine/src/media/thumbnail_hash_tests.rs +++ b/crates/engine/src/media/thumbnail_hash_tests.rs @@ -281,6 +281,83 @@ fn a_render_that_panics_fails_instead_of_never_answering() { assert_eq!(r.rendered.err().as_deref(), Some("the render failed unexpectedly: preview reader bug")); } +/// A raw photo's smart preview has no mosaic left: it stands in with the box on or off, so an offline raw keeps +/// reusing the decoded proxy instead of reading the file again for every render. +#[test] +fn a_smart_preview_fits_either_hot_pixel_setting() { + let raw = SourceInfo { raw: true, ..Default::default() }; + let decoded = DecodedSource::new(Arc::new(Rgb32f::new(8, 8)), Some(raw)); + assert!(decoded.fits_outliers(false) && !decoded.fits_outliers(true), "a raw decode must match the box"); + let proxy = DecodedSource { from_proxy: true, ..decoded }; + assert!(proxy.fits_outliers(false) && proxy.fits_outliers(true)); +} + +/// The Remove Hot Pixels box decides how a raw is decoded (#1345): the loader is told, and a source decoded with +/// the other setting is not reused. A rendered file has no mosaic, so its source stays. +#[test] +fn hot_pixel_box_decodes_the_raw_again() { + let mut s = session(); + let asked = Arc::new(std::sync::Mutex::new(Vec::new())); + let log = asked.clone(); + let raw = Arc::new(std::sync::atomic::AtomicBool::new(true)); + let is_raw = raw.clone(); + s.media.file_loader = Some(Arc::new(move |_, _, with: crate::media::RawDecode<'_>| { + log.lock().unwrap().push(with.outliers); + Ok((Rgb32f::new(8, 8), SourceInfo { raw: is_raw.load(std::sync::atomic::Ordering::Relaxed), ..Default::default() })) + })); + let set = |s: &mut crate::Session, on: bool| { + let mut d = (*s.catalog.photo(PhotoId(1)).unwrap().develop).clone(); + d.detail.nr_outliers = on; + // a new edit each time, so a render cached for earlier settings is not simply shown again + d.light.exposure += 0.1; + s.catalog.apply(Op::SetDevelop { id: PhotoId(1), settings: Arc::new(d), label: "box".into(), edited: None }).unwrap(); + }; + let render = |s: &mut crate::Session| { + let r = s.thumb_job(PhotoId(1), 256).unwrap().run(); + s.accept(&r); + }; + render(&mut s); + render(&mut s); + assert_eq!(*asked.lock().unwrap(), [false], "decoded once, then cached"); + set(&mut s, true); + render(&mut s); + render(&mut s); + assert_eq!(*asked.lock().unwrap(), [false, true], "switched on: decoded again with the filter, then cached"); + set(&mut s, false); + render(&mut s); + assert_eq!(*asked.lock().unwrap(), [false, true, false]); + // not a raw: the box changes nothing, the cached source is kept + raw.store(false, std::sync::atomic::Ordering::Relaxed); + s.media.forget(PhotoId(1)); + set(&mut s, false); + render(&mut s); + set(&mut s, true); + render(&mut s); + assert_eq!(*asked.lock().unwrap(), [false, true, false, false]); +} + +/// With the Detail section's eye off, Remove Hot Pixels contributes nothing either: the raw is decoded without it. +#[test] +fn detail_eye_off_decodes_the_raw_without_hot_pixel_removal() { + let mut s = session(); + let asked = Arc::new(std::sync::Mutex::new(Vec::new())); + let log = asked.clone(); + s.media.file_loader = Some(Arc::new(move |_, _, with: crate::media::RawDecode<'_>| { + log.lock().unwrap().push(with.outliers); + Ok((Rgb32f::new(8, 8), SourceInfo { raw: true, ..Default::default() })) + })); + for eye in [true, false] { + let mut d = (*s.catalog.photo(PhotoId(1)).unwrap().develop).clone(); + d.detail.nr_outliers = true; + d.set_section_enabled("detail", eye); + d.light.exposure += 0.1; + s.catalog.apply(Op::SetDevelop { id: PhotoId(1), settings: Arc::new(d), label: "eye".into(), edited: None }).unwrap(); + let r = s.thumb_job(PhotoId(1), 256).unwrap().run(); + s.accept(&r); + } + assert_eq!(*asked.lock().unwrap(), [true, false]); +} + /// CodeRabbit on #3544: a stand-in job decoding a camera JPEG when Discard Preview Cache clears the /// cache doesn't put the picture back: it writes for the generation it was made in (memory and /// disk alike), as render jobs do. diff --git a/crates/engine/src/smart.rs b/crates/engine/src/smart.rs index 77f78bd299..b44d1f35a1 100644 --- a/crates/engine/src/smart.rs +++ b/crates/engine/src/smart.rs @@ -503,6 +503,7 @@ pub fn load(path: &Path) -> Result { camera_tone, denoised: None, camera_profile: None, + outliers: false, from_proxy: true, reduced: false, }) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3393eca3fc..03592df41b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1041,6 +1041,48 @@ fn auto_sync_carries_only_the_changed_settings() { assert_eq!(s.develop_of(PhotoId(ids[1])).unwrap().light.exposure, 0.7); } +#[test] +fn auto_sync_carries_remove_hot_pixels_off() { + use lightcraft_catalog::PhotoId; + let mut s = demo(); + let ids: Vec = s.catalog.photos().take(2).map(|p| p.id).collect(); + for id in &ids { + let mut d = (*s.develop_of(*id).unwrap()).clone(); + d.detail.nr_outliers = true; + s.set_develop(*id, d, "Setup").unwrap(); + } + s.execute("library.select", &json!({"ids": [ids[0].0, ids[1].0], "active": ids[0].0})).unwrap(); + s.execute("develop.autoSync", &json!({"on": true})).unwrap(); + // off is omitted from the stored JSON: switching it off is still a change Auto Sync carries + let mut d = (*s.develop_of(ids[0]).unwrap()).clone(); + d.detail.nr_outliers = false; + s.set_develop(ids[0], d, "Remove Hot Pixels").unwrap(); + assert!(!s.develop_of(ids[1]).unwrap().detail.nr_outliers); +} + +#[test] +fn the_hot_pixel_limit_renders_afresh_only_the_photos_that_remove_hot_pixels() { + use lightcraft_catalog::PhotoId; + let mut s = demo(); + let ids: Vec = s.catalog.photos().take(2).map(|p| p.id).collect(); + let mut d = (*s.develop_of(ids[0]).unwrap()).clone(); + d.detail.nr_outliers = true; + s.set_develop(ids[0], d, "Setup").unwrap(); + let keys = |s: &mut crate::Session| ids.iter().map(|id| s.thumb_job(*id, 256).unwrap().cache.unwrap().1).collect::>(); + let before = keys(&mut s); + assert_eq!(s.execute("app.hotPixelLimit", &json!({})).unwrap()["default"], 0.2); + assert!(s.execute("app.hotPixelLimit", &json!({"percent": 0})).is_err()); + assert!(s.execute("app.hotPixelLimit", &json!({"percent": "1"})).is_err()); + let r = s.execute("app.hotPixelLimit", &json!({"percent": 1.5})); + let after = keys(&mut s); + // (process-wide: back to the default before anything can fail) + s.execute("app.hotPixelLimit", &json!({"percent": 0.2f32})).unwrap(); + assert_eq!(r.unwrap()["percent"], 1.5); + assert_ne!(after[0], before[0], "a photo that removes hot pixels renders with the new limit"); + assert_eq!(after[1], before[1], "the other photo's thumbnail stays valid"); + assert_eq!(keys(&mut s), before, "the default limit keeps the keys it always had"); +} + #[test] fn auto_sync_camera_local_tone_off_preserves_other_settings() { use lightcraft_catalog::PhotoId; diff --git a/crates/engine/src/tests/face_tiles.rs b/crates/engine/src/tests/face_tiles.rs index b166dcd859..d3b50aefc4 100644 --- a/crates/engine/src/tests/face_tiles.rs +++ b/crates/engine/src/tests/face_tiles.rs @@ -91,7 +91,7 @@ fn count_loads(s: &mut Session) -> Loads { let loads = Loads::default(); let file = s.media.file_loader.clone().unwrap(); let n = loads.files.clone(); - s.media.file_loader = Some(Arc::new(move |p: &str, e: usize, dcp: Option<&crate::media::DcpProfile>| { + s.media.file_loader = Some(Arc::new(move |p: &str, e: usize, dcp: crate::media::RawDecode<'_>| { n.fetch_add(1, Ordering::SeqCst); file(p, e, dcp) })); diff --git a/crates/raw/src/hotpixel.rs b/crates/raw/src/hotpixel.rs new file mode 100644 index 0000000000..aa6131b18b --- /dev/null +++ b/crates/raw/src/hotpixel.rs @@ -0,0 +1,519 @@ +//! Hot pixels on sensors that don't list their defects. +//! +//! A hot pixel is a single photosite that reads far too bright: a leaky or stuck cell, most visible +//! at high ISO and in dark areas. Demosaicing turns it into a small saturated dot of its colour +//! (red, green or blue), and colour noise reduction can only make that dot fainter, since the photosite +//! is too bright, not just the wrong colour. DNGs list such pixels in their opcodes and Panasonic marks +//! them in the data (both repaired elsewhere); other raws, such as Sony ARWs, carry no list. +//! +//! [`RawImage::suppress_hot_pixels`] finds them in the mosaic, before demosaicing: +//! +//! 1. **Noise model.** On a sparse sample of the image, each photosite is compared with the median of +//! its same-colour neighbours. The spread of that difference, per brightness, gives the noise +//! σ(level) as `σ² = a · level + b` (shot noise plus read noise), fitted to the image itself. +//! 2. **Candidates.** A photosite more than [`K_HOT`] σ above *every* same-colour neighbour (the +//! nearest eight, two photosites away on a Bayer grid, the nearest greens for green). +//! 3. **Real light is kept.** A star, a spark or a specular point lights more than one photosite: its +//! direct neighbours are raised too. A candidate with at least [`MIN_LIT`] neighbours more than +//! [`K_SPOT`] σ above their own surroundings is left alone. Hot photosites also come in adjacent +//! pairs, so a single lit neighbour doesn't count. +//! 4. The rest are replaced by the median of their same-colour neighbours. +//! +//! If more than a share of the photosites would be replaced (by default [`HOT_PIXEL_MAX_SHARE`]; the caller may +//! choose another), the image doesn't fit the model (a synthetic pattern, a star field shot through a pinhole) and +//! nothing is changed. Real light kept in step 3 doesn't count: a dense star field still has its hot pixels removed. + +use crate::{RawData, RawImage}; +use rayon::prelude::*; + +/// A candidate stands more than this many σ above the brightest of its same-colour neighbours (pure noise +/// hardly reaches 4.5 σ above the brightest of eight; measured on two ILCE-7M4 files at ISO 3200 and 25600). +const K_HOT: f32 = 5.0; +/// A direct neighbour this many σ above its own surroundings means the light is real. +const K_SPOT: f32 = 4.0; +/// ...and it takes this many lit neighbours (a hot pair has one). +const MIN_LIT: usize = 2; +/// Every this many rows and columns are sampled for the noise model. +const SAMPLE_STEP: usize = 5; +/// Brightness bins of the noise model (on a square-root scale), and the samples a bin needs. +const BINS: usize = 32; +const MIN_BIN_SAMPLES: usize = 64; +/// Photosites this close to the active area's edge are left alone (their neighbourhood is cut). +const MARGIN: usize = 4; +/// More replaced than this share of the photosites and the result is discarded (unless the caller chooses another). +pub const HOT_PIXEL_MAX_SHARE: f64 = 2e-3; + +/// For one position in the CFA pattern: the offsets of the nearest same-colour photosites (at most 8, +/// within two photosites) and of the 8 direct neighbours. +#[derive(Clone)] +struct Phase { + same: Vec<(isize, isize)>, + around: [(isize, isize); 8], +} + +fn phases(cfa: &crate::Cfa) -> Vec { + let around = [(-1, -1), (0, -1), (1, -1), (-1, 0), (1, 0), (-1, 1), (0, 1), (1, 1)]; + let (pw, ph) = (cfa.width, cfa.height); + (0..ph) + .flat_map(|j| (0..pw).map(move |i| (i, j))) + .map(|(i, j)| { + // anchored far enough from 0 that i + dx stays positive + let (x0, y0) = (i + 2 * pw, j + 2 * ph); + let c = cfa.color_at(x0, y0); + let mut same: Vec<(isize, isize)> = (-2isize..=2) + .flat_map(|dy| (-2isize..=2).map(move |dx| (dx, dy))) + .filter(|&(dx, dy)| (dx, dy) != (0, 0) && cfa.color_at((x0 as isize + dx) as usize, (y0 as isize + dy) as usize) == c) + .collect(); + same.sort_by_key(|&(dx, dy)| dx * dx + dy * dy); + same.truncate(8); + Phase { same, around } + }) + .collect() +} + +/// The noise σ at a brightness (in raw counts above black): `sqrt(a · level + b)`. +#[derive(Clone, Copy, Debug)] +struct Noise { + a: f32, + b: f32, +} + +impl Noise { + fn sigma(&self, level: f32) -> f32 { + (self.a * level.max(0.0) + self.b).max(1.0).sqrt() + } +} + +fn median(v: &mut [f32]) -> f32 { + if v.is_empty() { + return 0.0; + } + v.sort_by(f32::total_cmp); + let n = v.len(); + if n % 2 == 1 { v[n / 2] } else { 0.5 * (v[n / 2 - 1] + v[n / 2]) } +} + +/// The sample view the passes work on: the u16 mosaic, its width, the active area, the CFA phases. +struct View<'a> { + d: &'a [u16], + w: usize, + pw: usize, + ph: usize, + phases: &'a [Phase], +} + +impl View<'_> { + /// Value at full-image `(x, y)`; callers keep `(x, y)` inside the image. + #[inline] + fn at(&self, x: isize, y: isize) -> f32 { + self.d[y as usize * self.w + x as usize] as f32 + } + #[inline] + fn phase(&self, x: usize, y: usize) -> &Phase { + &self.phases[(y % self.ph) * self.pw + (x % self.pw)] + } + /// The same-colour neighbours' values around `(x, y)` into `buf`, and their maximum. + #[inline] + fn neighbours(&self, x: usize, y: usize, buf: &mut Vec) -> f32 { + buf.clear(); + let mut max = f32::MIN; + for &(dx, dy) in &self.phase(x, y).same { + let v = self.at(x as isize + dx, y as isize + dy); + max = max.max(v); + buf.push(v); + } + max + } +} + +impl RawImage { + /// Replace isolated hot photosites by the median of their same-colour neighbours (see the module docs). + /// Returns how many were replaced. Only single-plane integer CFA data is examined. + pub fn suppress_hot_pixels(&mut self) -> usize { + self.suppress_hot_pixels_with(HOT_PIXEL_MAX_SHARE) + } + + /// [`Self::suppress_hot_pixels`], giving up when more than `max_share` of the photosites (0..1) would be replaced. + pub fn suppress_hot_pixels_with(&mut self, max_share: f64) -> usize { + let Some(cfa) = self.cfa.clone() else { return 0 }; + let (w, h, a) = (self.width, self.height, self.active_area); + let black = self.black.mean(); + let RawData::U16(d) = &mut self.data else { return 0 }; + if self.cpp != 1 + || !cfa.valid() + || w.checked_mul(h) != Some(d.len()) + || a.width < 4 * MARGIN + || a.height < 4 * MARGIN + || a.x + a.width > w + || a.y + a.height > h + || !black.is_finite() + { + return 0; + } + let phases = phases(&cfa); + if phases.iter().any(|p| p.same.len() < 4) { + return 0; + } + let (x0, x1, y0, y1) = (a.x + MARGIN, a.x + a.width - MARGIN, a.y + MARGIN, a.y + a.height - MARGIN); + let range = (self.white.iter().copied().fold(0.0f32, f32::max) - black).max(1.0); + let fixes = { + let v = View { d: d.as_slice(), w, pw: cfa.width, ph: cfa.height, phases: &phases }; + let Some(noise) = noise_model(&v, (x0, x1, y0, y1), black, range) else { return 0 }; + let candidates = candidates(&v, (x0, x1, y0, y1), black, noise); + let mut buf = Vec::with_capacity(8); + let n_cand = candidates.len(); + let kept: Vec<_> = candidates.into_iter().filter(|&(x, y, _)| !lit_around(&v, x, y, black, noise, &mut buf)).collect(); + // (real light kept above doesn't count against the limit) + let sites = (x1 - x0) * (y1 - y0); + let limit = max_share * sites as f64; + if limit.is_nan() || kept.len() as f64 > limit { + return 0; + } + if std::env::var_os("LIGHTCRAFT_PROFILE").is_some() { + eprintln!( + " hot pixels: noise σ² = {:.3}·L + {:.1} (σ at 0: {:.1}, at 1 %: {:.1}), {n_cand} candidates, {} real light", + noise.a, + noise.b, + noise.sigma(0.0), + noise.sigma(0.01 * range), + n_cand - kept.len() + ); + // the strongest excesses below the threshold, to see how far the dots are + let mut hist = [0usize; 12]; + let mut buf2 = Vec::with_capacity(8); + for y in (y0..y1).step_by(1) { + for x in x0..x1 { + let s = v.at(x as isize, y as isize); + let max = v.neighbours(x, y, &mut buf2); + if s <= max { + continue; + } + let m = median(&mut buf2); + let k = ((s - max) / noise.sigma(m - black)) as usize; + hist[k.min(11)] += 1; + } + } + eprintln!(" hot pixels: photosites by (s - max neighbour) / σ, 0..11+: {hist:?}"); + // very bright photosites in dark surroundings, whatever the verdict + let mut bright = 0; + let mut buf4 = Vec::with_capacity(8); + for y in y0..y1 { + for x in x0..x1 { + let s = v.at(x as isize, y as isize) - black; + if s < 1500.0 { + continue; + } + v.neighbours(x, y, &mut buf4); + let m = median(&mut buf4) - black; + if m > 300.0 { + continue; + } + bright += 1; + if bright <= 4 { + eprintln!( + " bright at ({x}, {y}), colour {}, s {s:.0}, median {m:.0}, lit {}:", + cfa.color_at(x, y), + lit_around(&v, x, y, black, noise, &mut buf4) + ); + for dy in -3isize..=3 { + let row: Vec = + (-3isize..=3).map(|dx| format!("{:6.0}", v.at(x as isize + dx, y as isize + dy) - black)).collect(); + eprintln!(" {}", row.join(" ")); + } + } + } + } + eprintln!(" bright photosites in dark surroundings: {bright}"); + // a few candidates rejected as real light: their 5 × 5 neighbourhood above black + let mut shown = 0; + let mut buf3 = Vec::with_capacity(8); + for y in y0..y1 { + for x in x0..x1 { + if shown >= 6 { + break; + } + let s = v.at(x as isize, y as isize); + let max = v.neighbours(x, y, &mut buf3); + if s <= max { + continue; + } + let m = median(&mut buf3); + if s - max > 8.0 * noise.sigma(m - black) && lit_around(&v, x, y, black, noise, &mut buf3) { + shown += 1; + eprintln!(" rejected at ({x}, {y}), colour {}:", cfa.color_at(x, y)); + for dy in -2isize..=2 { + let row: Vec = + (-2isize..=2).map(|dx| format!("{:6.0}", v.at(x as isize + dx, y as isize + dy) - black)).collect(); + eprintln!(" {}", row.join(" ")); + } + } + } + } + } + kept + }; + for &(x, y, r) in &fixes { + d[y * w + x] = r; + } + fixes.len() + } +} + +/// Fit `σ² = a · level + b` to the spread of (photosite − median of its same-colour neighbours) on a sample. +fn noise_model(v: &View<'_>, (x0, x1, y0, y1): (usize, usize, usize, usize), black: f32, range: f32) -> Option { + let bin_of = |level: f32| (((level.max(0.0) / range).sqrt() * BINS as f32) as usize).min(BINS - 1); + let rows: Vec = (y0..y1).step_by(SAMPLE_STEP).collect(); + let per_row: Vec> = rows + .par_iter() + .map(|&y| { + let mut buf = Vec::with_capacity(8); + (x0..x1) + .step_by(SAMPLE_STEP) + .map(|x| { + v.neighbours(x, y, &mut buf); + let m = median(&mut buf); + let level = m - black; + (bin_of(level), level, (v.at(x as isize, y as isize) - m).abs()) + }) + .collect() + }) + .collect(); + let mut bins: Vec<(Vec, f64, usize)> = vec![(Vec::new(), 0.0, 0); BINS]; + for (b, level, dev) in per_row.into_iter().flatten() { + bins[b].0.push(dev); + bins[b].1 += level as f64; + bins[b].2 += 1; + } + // per bin: mean level and σ from the median absolute deviation (robust to the hot pixels themselves) + let pts: Vec<(f64, f64, f64)> = bins + .into_iter() + .filter(|(dev, _, n)| *n >= MIN_BIN_SAMPLES && !dev.is_empty()) + .map(|(mut dev, sum, n)| { + let s = 1.4826 * median(&mut dev) as f64; + (sum / n as f64, s * s, n as f64) + }) + .collect(); + if pts.is_empty() { + return None; + } + // weighted least squares for σ² = a·L + b, both kept ≥ 0 + let (sw, sl, sv) = pts.iter().fold((0.0, 0.0, 0.0), |(w, l, s), &(lv, var, n)| (w + n, l + n * lv, s + n * var)); + let (ml, mv) = (sl / sw, sv / sw); + let (cov, varl) = pts.iter().fold((0.0, 0.0), |(c, vl), &(lv, var, n)| (c + n * (lv - ml) * (var - mv), vl + n * (lv - ml) * (lv - ml))); + let a = if varl > 0.0 { (cov / varl).max(0.0) } else { 0.0 }; + let b = (mv - a * ml).max(0.0); + (a.is_finite() && b.is_finite()).then_some(Noise { a: a as f32, b: b as f32 }) +} + +/// Photosites far above every same-colour neighbour, with the value to replace them by. +fn candidates(v: &View<'_>, (x0, x1, y0, y1): (usize, usize, usize, usize), black: f32, noise: Noise) -> Vec<(usize, usize, u16)> { + (y0..y1) + .into_par_iter() + .flat_map_iter(|y| { + let mut buf = Vec::with_capacity(8); + let mut out = Vec::new(); + for x in x0..x1 { + let s = v.at(x as isize, y as isize); + let max = v.neighbours(x, y, &mut buf); + if s <= max { + continue; + } + let m = median(&mut buf); + if s - max > K_HOT * noise.sigma(m - black) { + out.push((x, y, m.round().clamp(0.0, u16::MAX as f32) as u16)); + } + } + out + }) + .collect() +} + +/// Are at least [`MIN_LIT`] direct neighbours of `(x, y)` clearly above their own same-colour surroundings +/// (real light)? A single one is not enough: hot photosites also come in adjacent pairs. +fn lit_around(v: &View<'_>, x: usize, y: usize, black: f32, noise: Noise, buf: &mut Vec) -> bool { + v.phase(x, y) + .around + .iter() + .filter(|&&(dx, dy)| { + let (nx, ny) = ((x as isize + dx) as usize, (y as isize + dy) as usize); + let s = v.at(nx as isize, ny as isize); + v.neighbours(nx, ny, buf); + let m = median(buf); + s - m > K_SPOT * noise.sigma(m - black) + }) + .count() + >= MIN_LIT +} + +#[cfg(test)] +mod tests { + use crate::*; + + /// A `w × h` RGGB mosaic, black 512, white 16383, flat grey 2000 counts above black with + /// deterministic noise of about σ ≈ 30 counts. + fn mosaic(w: usize, h: usize) -> RawImage { + let mut seed = 0x2545_f491_4f6c_dd1du64; + let mut noise = || { + // sum of 4 uniforms, about N(0, 1) + (0..4) + .map(|_| { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + (seed >> 11) as f32 / (1u64 << 53) as f32 - 0.5 + }) + .sum::() + * 1.732 + }; + let data = (0..w * h).map(|_| (512.0 + 2000.0 + 30.0 * noise()).round() as u16).collect(); + RawImage { + format: RawFormat::Arw, + width: w, + height: h, + cpp: 1, + data: RawData::U16(data), + cfa: Cfa::bayer("RGGB"), + bits: 14, + black: BlackLevel::uniform(512.0), + white: vec![16383.0], + active_area: Rect::new(0, 0, w, h), + crop: Rect::new(0, 0, w, h), + orientation: Orientation::Normal, + color: ColorData::default(), + wb_multipliers: None, + linearized: false, + opcodes: OpcodeLists::default(), + lens_metadata: Default::default(), + pixel_aspect: 1.0, + metadata: Metadata::default(), + } + } + + fn get(r: &RawImage, x: usize, y: usize) -> u16 { + match &r.data { + RawData::U16(d) => d[y * r.width + x], + RawData::F32(_) => 0, + } + } + + fn set(r: &mut RawImage, x: usize, y: usize, v: u16) { + let w = r.width; + if let RawData::U16(d) = &mut r.data { + d[y * w + x] = v; + } + } + + #[test] + fn flat_noise_is_left_alone() { + let mut r = mosaic(256, 256); + let before = r.data.clone(); + assert_eq!(r.suppress_hot_pixels(), 0); + assert_eq!(r.data, before); + } + + #[test] + fn single_hot_photosites_of_each_colour_are_replaced() { + let mut r = mosaic(256, 256); + // red (even, even), green (odd, even), blue (odd, odd) + for &(x, y) in &[(100, 100), (101, 60), (151, 151)] { + set(&mut r, x, y, 9000); + } + assert_eq!(r.suppress_hot_pixels(), 3); + for &(x, y) in &[(100, 100), (101, 60), (151, 151)] { + let v = get(&r, x, y) as f32; + assert!((v - 2512.0).abs() < 120.0, "({x}, {y}) = {v}"); + } + } + + #[test] + fn an_adjacent_hot_pair_is_replaced() { + let mut r = mosaic(256, 256); + set(&mut r, 80, 80, 9000); + set(&mut r, 80, 81, 8000); + assert_eq!(r.suppress_hot_pixels(), 2); + assert!(get(&r, 80, 80) < 2700 && get(&r, 80, 81) < 2700); + } + + #[test] + fn a_small_star_is_kept() { + let mut r = mosaic(256, 256); + // a point of light over a 3 × 3 block: brightest in the middle, its neighbours lit too + for dy in -1i32..=1 { + for dx in -1i32..=1 { + let v = if dx == 0 && dy == 0 { 12000 } else { 5000 }; + set(&mut r, (120 + dx) as usize, (90 + dy) as usize, v); + } + } + let before = r.data.clone(); + assert_eq!(r.suppress_hot_pixels(), 0); + assert_eq!(r.data, before); + } + + #[test] + fn a_hot_photosite_in_a_dark_area_is_replaced() { + let mut r = mosaic(256, 256); + // a dark patch near black (noise is then read noise, smaller) + for y in 32..96 { + for x in 32..96 { + set(&mut r, x, y, 512 + 20 + ((x * 7 + y * 13) % 11) as u16); + } + } + set(&mut r, 64, 64, 1500); + assert_eq!(r.suppress_hot_pixels(), 1); + assert!(get(&r, 64, 64) < 600); + } + + #[test] + fn a_dense_star_field_keeps_its_stars_and_loses_its_hot_pixels() { + let mut r = mosaic(256, 256); + // 225 small stars: more candidates than 0.2 % of the photosites, every one of them real light + for sy in 0..15 { + for sx in 0..15 { + let (cx, cy) = (16 + 16 * sx, 16 + 16 * sy); + for dy in -1i32..=1 { + for dx in -1i32..=1 { + let v = if dx == 0 && dy == 0 { 12000 } else { 5000 }; + set(&mut r, (cx + dx) as usize, (cy + dy) as usize, v); + } + } + } + } + let hot = [(24, 24), (105, 72), (201, 153)]; + for &(x, y) in &hot { + set(&mut r, x, y, 9000); + } + assert_eq!(r.suppress_hot_pixels(), 3); + for &(x, y) in &hot { + assert!(get(&r, x, y) < 2700, "({x}, {y}) = {}", get(&r, x, y)); + } + assert_eq!(get(&r, 16, 16), 12000, "a star is left alone"); + } + + #[test] + fn the_limit_is_the_callers() { + let mut r = mosaic(256, 256); + let hot = [(40, 40), (101, 60), (151, 151), (200, 30)]; + for &(x, y) in &hot { + set(&mut r, x, y, 9000); + } + let before = r.data.clone(); + // 4 of 248 × 248 photosites: above a limit of 0.005 %, within 0.01 % + assert_eq!(r.suppress_hot_pixels_with(5e-5), 0); + assert_eq!(r.data, before); + assert_eq!(r.suppress_hot_pixels_with(1e-4), 4); + assert_eq!(r.clone().suppress_hot_pixels_with(f64::NAN), 0, "a limit that is not a number replaces nothing"); + } + + #[test] + fn too_many_candidates_change_nothing() { + let mut r = mosaic(128, 128); + for y in (8..120).step_by(4) { + for x in (8..120).step_by(4) { + set(&mut r, x, y, 12000); + } + } + let before = r.data.clone(); + assert_eq!(r.suppress_hot_pixels(), 0); + assert_eq!(r.data, before); + } +} diff --git a/crates/raw/src/lib.rs b/crates/raw/src/lib.rs index e8052c3bbb..371891ddf7 100644 --- a/crates/raw/src/lib.rs +++ b/crates/raw/src/lib.rs @@ -34,6 +34,7 @@ mod dng; pub mod dngwrite; pub mod gaintable; pub mod highlight; +mod hotpixel; #[cfg(feature = "jxl")] mod jxl; pub mod kelvin_builtin; @@ -55,6 +56,7 @@ mod wb_estimate; pub use demosaic::{Method, demosaic}; pub use dngwrite::{DngCompression, DngWriteOptions, write_dng, write_rendered_dng}; +pub use hotpixel::HOT_PIXEL_MAX_SHARE; pub use lens_metadata::LensMetadata; pub use lightcraft_color::Mat3; pub use lightcraft_geom::Orientation; diff --git a/crates/ui-egui/examples/thumb_request_bench.rs b/crates/ui-egui/examples/thumb_request_bench.rs index ada42356de..980434b317 100644 --- a/crates/ui-egui/examples/thumb_request_bench.rs +++ b/crates/ui-egui/examples/thumb_request_bench.rs @@ -168,7 +168,7 @@ fn verify_texture(h: &Headless, id: PhotoId) { let edge = tex.size[0].max(tex.size[1]); assert!([128, 256, 384, 512].contains(&edge), "thumbnail must use a supported size bucket"); let Source::File { path } = &p.source else { unreachable!() }; - let (source, info) = h.app.session.media.file_loader.as_ref().unwrap()(path, 512, None).unwrap(); + let (source, info) = h.app.session.media.file_loader.as_ref().unwrap()(path, 512, Default::default()).unwrap(); println!("decode,path={path},raw={},preview_only={:?}", info.raw, p.preview_only); if std::env::var_os("REQUIRE_RAW").is_some() { assert!(p.preview_only.is_none(), "embedded-preview fallback does not validate RAW decoding"); diff --git a/crates/ui-egui/locales/de.json b/crates/ui-egui/locales/de.json index 5a5b8e9a2b..0310ef5f20 100644 --- a/crates/ui-egui/locales/de.json +++ b/crates/ui-egui/locales/de.json @@ -1229,6 +1229,7 @@ "Remove (H)": "Entfernen (H)", "Remove CA": "Chromatische Aberration entfernen", "Remove Chromatic Aberration": "Chromatische Aberration entfernen", + "Remove Hot Pixels": "Hotpixel entfernen", "Camera local tone mapping": "Lokale Tonwertzuordnung der Kamera", "Apply the local tone mapping stored in the raw (Apple ProRAW), as the camera renders it": "Die in der RAW-Datei gespeicherte lokale Tonwertzuordnung (Apple ProRAW) anwenden, wie die Kamera sie rendert", "Remove Disk from Library": "Datenträger aus Bibliothek entfernen", diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 34ffc3811b..6c2f1d7d9d 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -132,6 +132,7 @@ impl LightcraftApp { disk_space_notice: false, disk_notice: None, memory_applied: None, + hot_pixels_applied: None, display_applied: None, display_error: None, screen_profile: None, diff --git a/crates/ui-egui/src/app/prefs.rs b/crates/ui-egui/src/app/prefs.rs index 9a5c612f12..64331a0995 100644 --- a/crates/ui-egui/src/app/prefs.rs +++ b/crates/ui-egui/src/app/prefs.rs @@ -48,6 +48,12 @@ impl LightcraftApp { let _ = self.session.execute("app.memoryBudget", &serde_json::json!({"mb": mb})); } self.memory_applied = Some(mb); + let limit = self.ui.settings.hot_pixel_limit; + if self.hot_pixels_applied != Some(limit) { + self.hot_pixels_applied = Some(limit); + // (a value the engine refuses, from a hand-edited file, leaves its limit as it was) + let _ = self.session.execute("app.hotPixelLimit", &serde_json::json!({"percent": limit})); + } // the monitor profile (Settings ▸ Display): previews follow it from the next frame if self.display_applied.as_ref() != Some(&self.ui.settings.display_profile) { let path = self.ui.settings.display_profile.clone(); diff --git a/crates/ui-egui/src/dialogs/settings.rs b/crates/ui-egui/src/dialogs/settings.rs index cada6c7fba..900ee796ca 100644 --- a/crates/ui-egui/src/dialogs/settings.rs +++ b/crates/ui-egui/src/dialogs/settings.rs @@ -16,7 +16,7 @@ use egui::RichText; use serde_json::{Value, json}; use crate::LightcraftApp; -use crate::state::{GridBadges, PREVIEW_LIMITS, StartupView}; +use crate::state::{GridBadges, HOT_PIXEL_LIMIT_DEFAULT, HOT_PIXEL_LIMITS, PREVIEW_LIMITS, StartupView}; use crate::theme::Tokens; use crate::widgets::register; diff --git a/crates/ui-egui/src/dialogs/settings/performance.rs b/crates/ui-egui/src/dialogs/settings/performance.rs index 214dd38243..4fe223f696 100644 --- a/crates/ui-egui/src/dialogs/settings/performance.rs +++ b/crates/ui-egui/src/dialogs/settings/performance.rs @@ -1,7 +1,9 @@ use super::widgets::{check, choices, heading, hint, row}; #[cfg(not(target_arch = "wasm32"))] use super::widgets::{label_width, path_label}; -use super::{CACHE_SIZES, LABEL_W, LightcraftApp, PREVIEW_LIMITS, RichText, Tokens, Value, json, register}; +use super::{ + CACHE_SIZES, HOT_PIXEL_LIMIT_DEFAULT, HOT_PIXEL_LIMITS, LABEL_W, LightcraftApp, PREVIEW_LIMITS, RichText, Tokens, Value, json, register, +}; // --------------------------------------------------------------------------------- Performance @@ -49,6 +51,21 @@ pub(super) fn performance_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &To let opts = [(0u32, auto.as_str()), (512, "512 MB"), (1024, "1 GB"), (2048, "2 GB"), (4096, "4 GB")]; choices(ui, "settingsMemory", &opts, &mut app.ui.settings.memory_mb); }); + row(ui, t, crate::i18n::tr("Hot pixel limit"), |ui| { + let opts: Vec<(f32, String)> = HOT_PIXEL_LIMITS + .iter() + .map(|p| (*p, if *p == HOT_PIXEL_LIMIT_DEFAULT { format!("{p} % ({})", crate::i18n::tr("default")) } else { format!("{p} %") })) + .collect(); + let opts: Vec<(f32, &str)> = opts.iter().map(|(p, l)| (*p, l.as_str())).collect(); + choices(ui, "settingsHotPixels", &opts, &mut app.ui.settings.hot_pixel_limit); + }); + hint( + ui, + t, + crate::i18n::tr( + "Remove Hot Pixels (Detail) leaves a raw unchanged when more than this share of its photosites would be replaced: such a photo doesn't look like hot pixels on a sensor.", + ), + ); heading(ui, t, crate::i18n::tr("Preview cache")); let cur = app.session.cache_mb; row(ui, t, crate::i18n::tr("Size limit"), |ui| { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 009238d2bb..0c5e9b2cf8 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -375,6 +375,8 @@ pub struct LightcraftApp { pub(crate) disk_notice: Option<(u64, String)>, /// The memory budget setting last applied (MB, 0 = automatic). memory_applied: Option, + /// The hot pixel limit last applied (percent). + hot_pixels_applied: Option, /// The display profile setting last applied (`app.displayProfile`). pub(crate) display_applied: Option, /// Why the display profile setting could not be applied (shown in Settings ▸ Display). diff --git a/crates/ui-egui/src/panels/edit.rs b/crates/ui-egui/src/panels/edit.rs index 632bad5cbb..195bcb6488 100644 --- a/crates/ui-egui/src/panels/edit.rs +++ b/crates/ui-egui/src/panels/edit.rs @@ -308,6 +308,15 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { { control(app, ui, d, c, true); } + // single outlier photosites, replaced in the raw mosaic (#1345): raws only + if app.session.catalog.photo(id).is_some_and(|p| p.develops_raw()) { + egui::Frame::NONE.inner_margin(egui::Margin { left: 24, right: 22, top: 4, bottom: 0 }).show(ui, |ui| { + let mut on = d.detail.nr_outliers; + if crate::dialogs::settings::check(ui, "detail.nrOutliers", &mut on, "Remove Hot Pixels") { + let _ = app.run("develop.merge", json!({"settings": {"detail": {"nr_outliers": on}}, "label": "Remove Hot Pixels"})); + } + }); + } ui.add_space(8.0); }); section(app, ui, &d, "optics", "Optics", |app, ui, d| { diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 6f4b42723f..6d25426298 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -8,9 +8,9 @@ mod view; pub use dialog::{Dialog, NameEdit, SyncCounts, default_preset_groups}; pub use settings::{ - AppSettings, AppearanceMode, DarkTheme, EditFile, EditFormat, EditSpace, GridBadges, KeywordChips, LOUPE_EDGE_CEILING, LOUPE_EDGE_STEP, - LaunchLibrary, LightTheme, LightroomImport, MIN_TEXTURE_SIDE, PHOTOCRAFT, PREVIEW_LIMITS, RECENT_LIBRARIES, STANDARD_PREVIEW_EDGE, StartupView, - WINDOW_FRAME_CEILING, + AppSettings, AppearanceMode, DarkTheme, EditFile, EditFormat, EditSpace, GridBadges, HOT_PIXEL_LIMIT_DEFAULT, HOT_PIXEL_LIMITS, KeywordChips, + LOUPE_EDGE_CEILING, LOUPE_EDGE_STEP, LaunchLibrary, LightTheme, LightroomImport, MIN_TEXTURE_SIDE, PHOTOCRAFT, PREVIEW_LIMITS, RECENT_LIBRARIES, + STANDARD_PREVIEW_EDGE, StartupView, WINDOW_FRAME_CEILING, }; pub use view::{ BRIEF_INFO_SECS, BeforeAfter, BriefInfo, CLICK_ZOOMS, CropOverlay, FOCUS_KEYWORDS_PASSES, InfoOverlay, KeywordingView, LEFT_WIDTH, LayoutImage, diff --git a/crates/ui-egui/src/state/settings.rs b/crates/ui-egui/src/state/settings.rs index 0fae2ed8e7..62b50557cc 100644 --- a/crates/ui-egui/src/state/settings.rs +++ b/crates/ui-egui/src/state/settings.rs @@ -153,6 +153,9 @@ pub struct AppSettings { pub photocraft_file: EditFile, /// Memory the caches may hold together, in MB (0 = automatic; `app.memoryBudget`). pub memory_mb: u32, + /// Remove Hot Pixels leaves a raw alone when more than this share of its photosites, in percent, would be + /// replaced (`app.hotPixelLimit`). + pub hot_pixel_limit: f32, /// The Info panel's metadata fields with several photos selected: true (the default) shows the /// values the selected photos share and marks each field where they differ (Selected Photos), /// false shows the active photo's values (Target Photo) (`view.infoSelectedPhotos`). @@ -238,6 +241,7 @@ impl Default for AppSettings { photocraft_path: String::new(), photocraft_file: EditFile { format: EditFormat::Psd, ..EditFile::default() }, memory_mb: 0, + hot_pixel_limit: HOT_PIXEL_LIMIT_DEFAULT, info_selected_photos: true, film_names: true, film_badges: true, @@ -321,6 +325,10 @@ impl EditFile { /// Preview size limits offered in Settings → Performance (0 = Automatic). pub const PREVIEW_LIMITS: [u32; 5] = [0, 1600, 2560, 3840, 5120]; +/// Hot pixel limits offered in Settings → Performance (percent of a raw's photosites), and the default. +pub const HOT_PIXEL_LIMITS: [f32; 5] = [0.05, 0.1, 0.2, 0.5, 1.0]; +pub const HOT_PIXEL_LIMIT_DEFAULT: f32 = 0.2; + /// The most the loupe ever renders in one go (memory and GPU texture size), whatever the setting. pub const LOUPE_EDGE_CEILING: u32 = 8192; diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 811c1b4b22..8a857e58d8 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -211,6 +211,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-DETAIL-RAWDETAILS | Improved demosaic toggle | P2 | ⬜ | | | | LR-EDIT-DETAIL-SUPERRES | Super resolution | P2 | ⬜ | | | | LR-EDIT-DETAIL-AISHARPEN | AI sharpen | OOS | 🚫 | | | +| LR-EDIT-DETAIL-HOTPIXELS | Hot pixel removal | P1 | ✅ | `check:detail.nrOutliers`, `crates/raw/src/hotpixel.rs`, `crates/engine/src/files/load.rs` (`load_bytes_now`), `cmd:app.hotPixelLimit` | Lightroom has no control for this. Here a box under Noise Reduction (raws only), on for newly imported raws and off for edits saved before it existed (issue #1345): single photosites far above all their same-colour neighbours are replaced in the mosaic before demosaicing; a point lit across neighbouring photosites (a star, a spark) is kept. A raw with more than 0.2 % of its photosites to replace (real light kept doesn't count) is left unchanged; Settings ▸ Performance ▸ Hot pixel limit sets the share (test `the_hot_pixel_limit_renders_afresh_only_the_photos_that_remove_hot_pixels`). Switching it decodes the raw again (tests `hot_pixel_box_is_on_for_new_raws_only`, `hot_pixel_box_decodes_the_raw_again`) | | LR-EDIT-OPTICS-CA | Remove chromatic aberration | P1 | ✅ | `crates/ui-egui/src/panels/edit.rs` (checkbox), `ctl:optics.caRed`, `ctl:optics.caBlue` | | | LR-EDIT-OPTICS-PROFILE | Lens profile corrections | P1 | 🟡 | `ctl:optics.profileDistortion`, `ctl:optics.profileVignetting`, `crates/pipeline/src/optics.rs`, `crates/raw/src/vendor/rw2.rs`, `crates/raw/src/vendor/raf_lens.rs`, `crates/engine/src/crs.rs` | uses corrections embedded in the file: DNG `WarpRectilinear` / `WarpRectilinear2` (opcode 14, DNG 1.6: degree-14 radial polynomial, valid-radius clamp, reciprocal option, applied in list order with the spec's skip rule for a `WarpRectilinear` after an optional one, so Pixel DNGs' two-stage warp is corrected; up to 3 chained stages, sampled on the CPU because the polynomials need `f64`; issue #635) / `FixVignetteRadial`, and the camera's own distortion correction in Panasonic / Leica RW2 / RWL files (tag `0x0119`, read as the equivalent `WarpRectilinear`; on 79 photos from 13 bodies and 11 lenses the corrected render lines up with the camera JPEG within 0.75 px at 1440 px at the corners, from up to 13 % of the half diagonal uncorrected; issue #256); an imported preset's `LensProfileEnable=0` leaves them on, as Lightroom applies built-in corrections regardless (inferred); Sony ILCE-7RM4A Bayer ARW raw-IFD distortion tables (`0x7037`, 16-sample layout, native 3:2 framing) use the same warp path, enabled for the independently validated ILCE-7RM4A with FE 24–105 and FE 200–600 lenses, ILCE-7M3 (FE 16–35 F4 ZA, FE 28–70, FE 50 F1.8; 26 shots within 0.06–0.44 px median of the camera JPEG at 6000 px, from up to 26 px uncorrected), and ILCE-7CR full-frame Bayer 3:2 files with correction Auto or forced Auto (a separate corrected-radius conversion validated against camera JPEGs with FE 24-50mm, FE 16-25mm and FE 50mm lenses); A7CR Off, APS-C, aspect crops and lossless M/S remain uncorrected; ILCE-7RM6 verified against the camera JPEG on 54 photos with three lenses (`docs/sony-ilce7rm6-lens-corrections.md`); other Sony models remain uncorrected (see `docs/sony-lens-corrections.md`); Nikon NEF distortion from NikonNEFInfo (`0xc7d5`: polynomial, frame-filling zoom and the optical-centre offset left by in-body stabilisation; `crates/raw/src/vendor/nef_lens.rs`, `docs/nikon-lens-corrections.md`) for the Z 6II, Z 8 and Z f, full-frame image area only: F-mount lenses on the FTZ match the camera JPEG within 0.2 px median, Z lenses within 0.5–1.5 px except the 24-70 f/4's wide end, where the camera applies a further correction that is not in the readable data (7 px median at 24 mm, from 16 px / 106 px p95 uncorrected); Fujifilm RAF raw-IFD tables (`0xf00b` distortion, `0xf010` vignetting; `crates/raw/src/vendor/raf_lens.rs`, `docs/fuji-lens-corrections.md`) become a `WarpRectilinear` with the camera's frame-filling zoom and a `FixVignetteRadial` gain for 59 body models with sample files checked against the camera JPEG (the table indexed by the source radius with last knot at the corner; on 67 body/lens combinations the corrected render's radial shape error against the camera JPEG falls from a median of 0.72 % to 0.05 % of the half diagonal, largest 0.42 %, and the vignetting gain of the JPEG tracks the table at a median correlation of 0.987); files whose table scale is not the recorded image's half diagonal (X-T30 III, X-M5, X-T5 crops; X-S20 supported) and six unmatched bodies stay uncorrected; Fujifilm's lateral CA table (`0xf00f`) is real but is not applied, because our demosaic already registers the planes and the stored shift over-corrects; vignetting and lateral CA data of Olympus raws not read yet; Sony ARW shading, lateral CA and distortion tables (raw IFD `0x7031`–`0x7037`, with the encrypted `SR2SubIFD` copies as a checked fallback) are decoded with their camera settings, provenance and framing evidence for every body that records them (`crates/raw/src/lens_metadata.rs`), but only the ILCE-7RM4A, ILCE-7M3, ILCE-7RM6 and ILCE-7CR use the separate distortion conversions above; Sony shading and CA semantics are not validated beyond the ILCE-7RM6's (lateral CA off) (`docs/sony-lens-corrections.md`); Reload from Disk reads the file's lens data for photos catalogued without it and turns the correction on for those still at import settings (issue #314); lens-profile database: a lensfun-format XML database in `LIGHTCRAFT_LENSFUN_DIR` (none is bundled) supplies distortion (poly3 / poly5 / PTLens), TCA and vignetting for lenses it lists (`crates/lensdb`, `crates/engine/src/lens_profiles.rs`, `docs/lens-profiles.md`); raws that name no lens model (Nikon DSLRs: maker-note lens specification only) look it up by a name built from the specification (`24-70mm f/2.8`) and take only a profile named exactly that; lens database: the desktop app offers lensfun's `data/db` once on the first launch (Download / Not Now with its size; Settings ▸ General ▸ Lens profiles downloads it later; nothing is fetched without that click; CC BY-SA 3.0, pinned by commit, size and SHA-256, never bundled; `cmd:lens.downloadDatabase`, `crates/engine/src/lensfun_download.rs`) and downloads it with an activity row, and when it arrives every library raw whose lens it knows is looked up again in the background (`cmd:library.refreshLensProfiles`, `crates/engine/src/cmd/lens_refresh.rs`), as is a photo whose Enable Lens Corrections is ticked; the Optics panel names the loaded profile and its source, or says "No lens profile found for " (`crates/ui-egui/src/lens_db.rs`); presets, Copy / Paste and Sync carry only the switch and amounts, never a profile (`crates/engine/src/tests/lens_refresh.rs`); no source picker or own calibration yet | ) diff --git a/docs/whats-new.md b/docs/whats-new.md index e1ebcc7827..f9b9d6b73c 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,17 @@ # What's new in LightCraft +## Unreleased + +### Remove Hot Pixels +- Detail ▸ Noise Reduction has a new box, Remove Hot Pixels (raws only), on for newly imported raws (issue #1345). + At high ISO single photosites read far brighter than all their neighbours and turn into small red, green or blue + dots that noise reduction only makes fainter. They are now replaced before the raw is developed. A point of light + spread over neighbouring photosites, such as a star, is kept; untick the box for star fields that need every + single-photosite point. Photos edited earlier keep their look: the box is off for them. +- A raw that would have more than 0.2 % of its photosites replaced is left unchanged: such a photo doesn't look like + hot pixels on a sensor. Stars and other points of light that are kept don't count, so a dense star field still + loses its hot pixels. Settings ▸ Performance ▸ Hot pixel limit sets the share (0.05 % to 1 %). + ## 2026-11-01 ### Korean and Vietnamese text in exports draws whole