From d729c80a5adc603b163810436c2d34df1b74e0d0 Mon Sep 17 00:00:00 2001 From: PiMinusMax <121135695+PiMinusMax@users.noreply.github.com> Date: Thu, 5 Nov 2026 09:26:47 +0100 Subject: [PATCH] Detail: Remove Hot Pixels, on for newly imported raws (#1345) (#1350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * WIP: suppress isolated hot photosites in the raw mosaic (#820) Noise model fitted to the image, candidates 5 sigma above every same-colour neighbour, kept when two or more direct neighbours are lit (real light); adjacent hot pairs are replaced too. LIGHTCRAFT_HOT_PIXELS=0 turns it off, LIGHTCRAFT_HP_DEBUG=1 prints statistics. Renders are cached apart from older ones. Co-Authored-By: Claude Opus 5.5 * WIP: hot pixel suppression off by default (LIGHTCRAFT_HOT_PIXELS=1 turns it on) Co-Authored-By: Claude Opus 5.5 * #1345: mosaic outlier filter: cache key no longer names the #858 comparison switch (merged upstream) Co-Authored-By: Claude Opus 5.5 * #1345: mosaic outlier filter: fixed thresholds (no LIGHTCRAFT_HP_K / _SPOT knobs), statistics under LIGHTCRAFT_PROFILE Co-Authored-By: Claude Opus 5.5 * #1345: Remove Hot Pixels, a box in Detail ▸ Noise Reduction, on for newly imported raws `Detail::nr_outliers` replaces single outlier photosites in the raw mosaic before demosaicing. It is true in `DevelopSettings::for_raw` and false by default, so edits saved before the box existed keep their look. The loaders take the flag (no more `LIGHTCRAFT_HOT_PIXELS`), a decoded source records it, and a cached raw source decoded with the other setting is decoded again. The box shows for raws only (`check:detail.nrOutliers`). * #1345: Remove Hot Pixels off keeps old settings' JSON; smart previews fit either setting - `detail.nr_outliers` is left out of the stored JSON when off, so settings saved before it keep their hash and cached thumbnails. Copy / paste, presets (also at an amount), preset updates and Auto Sync work on `DevelopSettings::to_json_for_merge`, which states it, so turning the box off still carries over. - `DecodedSource::fits_outliers` accepts a smart preview (`from_proxy`): it has no mosaic, so an offline raw with the box on keeps reusing the decoded proxy. (review of #1350) Co-Authored-By: Claude Opus 5.5 * #1345: the Detail eye and Reset Detail switch Remove Hot Pixels off `DevelopSettings::removes_hot_pixels` (the box and the Detail section's eye) decides the raw decode in the app, the web worker and the thumbnail bench, so a Detail section that is off contributes nothing here either. Reset Detail resets the box to the general default (off), like the sliders it resets. (review of #1350) Co-Authored-By: Claude Opus 5.5 * #1345: the hot pixel limit counts only replaced photosites, and can be set The 0.2 % limit was checked against the candidates before real light was filtered out, so a dense star field reached it with its stars alone and kept its hot pixels too. It now counts the photosites that would be replaced, as the module docs say. The limit is now a parameter (RawImage::suppress_hot_pixels_with, default HOT_PIXEL_MAX_SHARE) and a setting: Settings > Performance > Hot pixel limit, applied with the new command app.hotPixelLimit. Changing it forgets the decoded sources and changes the render cache keys of photos that remove hot pixels; at the default every key stays as it was. Co-Authored-By: Claude Opus 5.5 * #1345: the app's default hot pixel limit is the default The setting is an f32: 0.2 % read back as 0.2000000029… % was a new limit, so every start forgot the decoded sources and changed the render keys of photos that remove hot pixels. A value within a millionth of the default now is the default. Co-Authored-By: Claude Opus 5.5 * Format the imports * Web loader test states what it decodes with * Format the media exports --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> --- apps/lightcraft-web/src/store.rs | 14 +- apps/lightcraft-web/src/wire.rs | 10 +- crates/develop/src/lib.rs | 56 +- crates/develop/src/presets.rs | 4 +- crates/develop/src/settings.rs | 8 + crates/engine/examples/render_bench.rs | 2 +- crates/engine/src/cmd/develop.rs | 2 +- crates/engine/src/cmd/previews.rs | 5 +- crates/engine/src/cmd/query.rs | 15 + crates/engine/src/files.rs | 30 +- crates/engine/src/files/fs.rs | 11 +- crates/engine/src/files/load.rs | 33 +- crates/engine/src/files/tests.rs | 8 +- crates/engine/src/lib.rs | 5 +- crates/engine/src/media.rs | 4 +- crates/engine/src/media/cache.rs | 13 +- crates/engine/src/media/session.rs | 26 +- crates/engine/src/media/settings_hash.rs | 2 +- crates/engine/src/media/source.rs | 50 +- crates/engine/src/media/tests.rs | 2 + .../engine/src/media/thumbnail_hash_tests.rs | 77 +++ crates/engine/src/smart.rs | 1 + crates/engine/src/tests.rs | 42 ++ crates/engine/src/tests/face_tiles.rs | 2 +- crates/raw/src/hotpixel.rs | 519 ++++++++++++++++++ crates/raw/src/lib.rs | 2 + .../ui-egui/examples/thumb_request_bench.rs | 2 +- crates/ui-egui/locales/de.json | 1 + crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/app/prefs.rs | 6 + crates/ui-egui/src/dialogs/settings.rs | 2 +- .../src/dialogs/settings/performance.rs | 19 +- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/edit.rs | 9 + crates/ui-egui/src/state.rs | 6 +- crates/ui-egui/src/state/settings.rs | 8 + docs/parity-checklist.md | 1 + docs/whats-new.md | 12 + 38 files changed, 941 insertions(+), 71 deletions(-) create mode 100644 crates/raw/src/hotpixel.rs 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 faec08ca98..eea56f1265 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 fb535f1c43..eedd25a87b 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