diff --git a/crates/codecs/src/encode.rs b/crates/codecs/src/encode.rs index e3d83ac33..c002abd98 100644 --- a/crates/codecs/src/encode.rs +++ b/crates/codecs/src/encode.rs @@ -395,7 +395,7 @@ pub fn pq_decode(e: f32) -> f32 { pub fn encode_avif_pq(width: u32, height: u32, rgb: &[f32], quality: u8, speed: u8, meta: &EncodeMeta) -> Result> { let (w, h) = (width as usize, height as usize); let n = w.checked_mul(h).ok_or_else(|| Error::Encode("image too large".into()))?; - if n == 0 || rgb.len() < n * 3 { + if n == 0 || n.checked_mul(3).is_none_or(|need| rgb.len() < need) { return Err(Error::Encode("HDR AVIF: sample buffer too short".into())); } #[cfg(all(feature = "avif", not(target_arch = "wasm32")))] diff --git a/crates/develop/src/lib.rs b/crates/develop/src/lib.rs index b7c952511..f963f6e28 100644 --- a/crates/develop/src/lib.rs +++ b/crates/develop/src/lib.rs @@ -162,6 +162,27 @@ mod tests { assert_ne!(back.hash64(), DevelopSettings::default().hash64()); } + #[test] + fn default_hdr_is_not_serialized_so_existing_hashes_stay() { + let s = DevelopSettings::default(); + assert!(s.to_json().get("hdr").is_none(), "HDR off must not change the settings JSON or hash64"); + let mut on = s.clone(); + on.hdr.enabled = true; + assert!(on.to_json().get("hdr").is_some()); + assert_ne!(on.hash64(), s.hash64()); + assert_eq!(DevelopSettings::from_json(&on.to_json()).unwrap(), on); + } + + #[test] + fn hdr_peak_is_finite_for_a_non_finite_headroom() { + let mut h = Hdr { enabled: true, max_ev: f64::NAN, ..Hdr::default() }; + assert_eq!(h.peak(), (Hdr::DEFAULT_MAX_EV as f32).exp2()); + h.max_ev = f64::INFINITY; + assert_eq!(h.peak(), (Hdr::DEFAULT_MAX_EV as f32).exp2()); + h.max_ev = -3.0; + assert_eq!(h.peak(), 1.0); + } + #[test] fn missing_fields_take_defaults_and_unknown_ignored() { let s = DevelopSettings::from_json(&json!({"light": {"exposure": 0.5}, "futureThing": 3})).unwrap(); diff --git a/crates/develop/src/settings.rs b/crates/develop/src/settings.rs index d7ce79f9f..6ed1a5491 100644 --- a/crates/develop/src/settings.rs +++ b/crates/develop/src/settings.rs @@ -14,7 +14,9 @@ pub struct DevelopSettings { pub treatment: Treatment, pub wb: WhiteBalance, pub light: Light, - /// HDR editing: highlights above SDR white, and the SDR rendition derived from them. + /// HDR editing: highlights above SDR white, and the SDR rendition derived from them. Not written while it is + /// the default, so settings without HDR keep their JSON and `hash64` (and every cached preview stays valid). + #[serde(skip_serializing_if = "Hdr::is_default")] pub hdr: Hdr, pub curve: ToneCurve, pub color: ColorAdj, @@ -973,9 +975,19 @@ impl Hdr { pub const DEFAULT_MAX_EV: f64 = 3.0; pub const MAX_EV_LIMIT: f64 = 5.0; - /// Peak linear value of an HDR render relative to SDR white (1.0 when HDR is off). + /// Whether these are the default HDR settings (HDR off, nothing changed). + pub fn is_default(&self) -> bool { + *self == Hdr::default() + } + + /// Peak linear value of an HDR render relative to SDR white (1.0 when HDR is off). A non-finite headroom + /// (a damaged file or argument) falls back to the default, so the peak is always finite and at least 1. pub fn peak(&self) -> f32 { - if self.enabled { (self.max_ev.clamp(0.0, Hdr::MAX_EV_LIMIT) as f32).exp2() } else { 1.0 } + if !self.enabled { + return 1.0; + } + let ev = if self.max_ev.is_finite() { self.max_ev } else { Hdr::DEFAULT_MAX_EV }; + (ev.clamp(0.0, Hdr::MAX_EV_LIMIT) as f32).exp2() } }