diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index 1fb3fcf300..e86e4252a7 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -758,6 +758,18 @@ fn short_num(v: f64, decimals: usize) -> String { if s.contains('.') { s.trim_end_matches('0').trim_end_matches('.').to_string() } else { s } } +/// `v` to three significant digits, with no trailing zeros (0.031714 → "0.0317", 0.4 → "0.4"). +/// Two decimals threw away most of a slow shutter speed: a stored 0.031714 s printed as "0.03 s", +/// which is 5 % off, and a stored 0.0215 s printed as "0.02 s", 7 % off (#6958). +fn sig3(v: f64) -> String { + let magnitude = v.abs().log10().floor(); + // Clamped at both ends: a value at the large end must not ask for a negative field width, and + // at the small end the field is widened rather than truncated, so a positive exposure never + // reads as "0 s". 320 is beyond f64's decimal range, so it always terminates. + let decimals = if magnitude.is_finite() { (2.0 - magnitude).clamp(0.0, 320.0) } else { 0.0 }; + short_num(v, decimals as usize) +} + /// Exposure time as cameras print it: "1/250 s" for fractions of a second, "0.4 s" or "2 s" /// when the reciprocal isn't a whole number or the exposure is a second or longer. fn exposure_text(t: f64) -> Option { @@ -765,10 +777,18 @@ fn exposure_text(t: f64) -> Option { return None; } let r = 1.0 / t; - if t < 1.0 && (r - r.round()).abs() <= r * 0.01 { - return Some(format!("1/{} s", r.round())); + let n = r.round(); + // Below 1/10 s the reciprocal is above 10, so its nearest whole number puts the shown time + // within 5 % of the stored one, and is the form a photographer expects ("1/32 s"). The test + // used to be relative to the reciprocal, so it shrank as the exposure slowed: under 1/50 s it + // fell below half a rounding step and nearly every fraction fell through to the decimal form + // (#6958). Above 1/10 s the fraction is only right when the reciprocal is very nearly whole + // (1/2 s, 1/4 s) — 0.4 s stays "0.4 s" rather than becoming "1/3 s". + let fraction = n.is_finite() && (1.0..=1e9).contains(&n) && (t < 0.1 || (t < 1.0 && (r - n).abs() <= r * 0.01)); + if fraction { + return Some(format!("1/{} s", n as u64)); } - Some(format!("{} s", short_num(t, if t < 1.0 { 2 } else { 1 }))) + Some(format!("{} s", sig3(t))) } /// EXIF (`2024:02:17 12:40:19`) or XMP (`2024-02-17T12:40:19.00+01:00`) dates as diff --git a/crates/engine/src/file_cmds/tests.rs b/crates/engine/src/file_cmds/tests.rs index 3ab50c4a77..bd8ccc9925 100644 --- a/crates/engine/src/file_cmds/tests.rs +++ b/crates/engine/src/file_cmds/tests.rs @@ -494,6 +494,54 @@ fn camera_data_formats_like_photoshop_and_falls_back_to_xmp() { assert_eq!(camera_data(Some(b"II*\0\xff\xff\xff\xff"), Some("0/0 exif:ExposureTime=\"-1/0\"")), json!({})); } +/// A slow shutter speed whose reciprocal is not whole prints as the nearest fraction a camera +/// would write, not as two decimals. The old 1 %-of-the-reciprocal test shrank as the exposure +/// slowed, so below 1/50 s it fell below half a rounding step and nearly every such fraction fell +/// through to the decimal form (#6958). +#[test] +fn camera_data_prints_a_slow_exposure_as_the_fraction_a_camera_writes() { + let text = |t: f64| exposure_text(t).expect("an exposure"); + // The two values from the report: 31714/1000000 s and 215/10000 s. + assert_eq!(text(0.031_714), "1/32 s", "a stored 0.031714 s is about 1/32 s"); + assert_eq!(text(0.0215), "1/47 s", "a stored 0.0215 s is about 1/47 s"); + // The rest of a slow shutter scale, and the exact reciprocals that already worked. + assert_eq!(text(0.125), "1/8 s"); + assert_eq!(text(1.0 / 60.0), "1/60 s"); + assert_eq!(text(1.0 / 4000.0), "1/4000 s"); + assert_eq!(text(0.04), "1/25 s"); + assert_eq!(text(0.09), "1/11 s"); + // Every shown time stays within 5 % of the stored one — the point of the report. + for t in [0.031_714, 0.0215, 0.04, 0.09, 0.125, 1.0 / 60.0, 1.0 / 4000.0] { + let n: f64 = text(t).split('/').nth(1).and_then(|d| d.split(' ').next()).expect("1/n s").parse().expect("a number"); + assert!((1.0 / n / t - 1.0).abs() <= 0.05, "{t} shown as {} is more than 5 % off", text(t)); + } + // A tenth of a second and longer keeps the Photoshop forms: a near-whole reciprocal is a + // fraction, anything else is the decimal at three significant digits. + assert_eq!(text(0.5), "1/2 s"); + assert_eq!(text(0.25), "1/4 s"); + assert_eq!(text(0.4), "0.4 s"); + assert_eq!(text(0.105), "0.105 s", "two decimals would read 0.11 s"); + assert_eq!(text(1.25), "1.25 s", "one decimal would read 1.3 s"); + assert_eq!(text(2.0), "2 s"); + // A hostile value cannot panic, and a positive exposure never reads as "0 s" (never-crash, rule 9). + assert_ne!(exposure_text(f64::MIN_POSITIVE).as_deref(), Some("0 s")); + assert_ne!(exposure_text(f64::MIN_POSITIVE), None); + assert_eq!(exposure_text(0.0), None); + assert_eq!(exposure_text(-1.0), None); + assert_eq!(exposure_text(f64::NAN), None); + assert_eq!(exposure_text(f64::INFINITY), None); + assert_ne!(exposure_text(f64::MAX), None); + // The same through a real EXIF block, so the round trip to File Info is covered too. + let camera = |t: f64| { + let exif = photocraft_algo::exif::build(&photocraft_algo::exif::CameraInfo { exposure_time: Some(t), ..Default::default() }); + camera_data(Some(&exif), None)["exposure"].as_str().map(str::to_owned) + }; + assert_eq!(camera(0.031_714), Some("1/32 s".to_owned())); + assert_eq!(camera(0.0215), Some("1/47 s".to_owned())); + // An absurd exposure is capped to one dialog line, as every other camera field is. + assert!(camera(f64::MAX).expect("an exposure").len() <= 200, "a huge exposure must stay on one dialog line"); +} + const MULTILINGUAL_XMP: &str = r#"