From f1f528fc42495b214628404ae40e18919e4880e5 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Mon, 17 Aug 2026 13:03:03 +0200 Subject: [PATCH] Stop rendering a missing white balance as neutral, which came out pink MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sane_wb` replaced any coefficient it could not use with 1.0. That reads as a safe default and is not one. A Bayer sensor's green photosites collect roughly twice the signal of its red and blue, so unbalanced data is strongly green — and the camera matrix is built assuming the data reaching it has already been balanced. Fed green-heavy input it subtracts green as designed, overshoots, and the frame lands in magenta. Bodies whose as-shot coefficients rawler does not report came out pink, and nothing anywhere said why. The fallback is now the camera's own response to daylight, which `cam_to_srgb_from` was already computing on its way to balancing the matrix and then discarding. `daylight_wb` exposes it, and both callers read the same matrix through the same illuminant preference — so the multipliers neutralise exactly the white the matrix expects to be neutral, by construction rather than by coincidence. With no matrix either, the body is unknown and neutral is the honest answer: uncalibrated beats wrong in a specific direction. A test caught me returning the response rather than its reciprocal, which inverts the correction — a sensor is *least* sensitive to the channel needing the largest multiplier, so that version boosted precisely the wrong one. The doc comment now says which of the two it returns, because they differ by an inversion and look alike. Four tests, on a real matrix (Canon 6D, D65) rather than a contrived one: the fallback is nowhere near neutral, lifts both red and blue against green, stays green-normalised, and — the property that makes it consistent rather than merely plausible — balancing by it and then applying the matrix maps the camera's white to a neutral sRGB. `daylight_wb` is also the anchor the white-balance presets need: a preset in kelvin requires an absolute illuminant to be a preset *of*, and the temperature control is currently a relative offset from whatever the camera chose. Co-Authored-By: Claude Opus 5 --- core/dr-decode/src/lib.rs | 166 +++++++++++++++++++++++++++++++++++--- 1 file changed, 154 insertions(+), 12 deletions(-) diff --git a/core/dr-decode/src/lib.rs b/core/dr-decode/src/lib.rs index a6a5cbc..9a0f88d 100644 --- a/core/dr-decode/src/lib.rs +++ b/core/dr-decode/src/lib.rs @@ -422,8 +422,11 @@ pub fn decode(bytes: &[u8]) -> Result { .raw_image(&source, &Default::default(), false) .map_err(|e| DecodeError::Decode(e.to_string()))?; - // Derived before the match below moves `image.data`. + // Both derived before the match below moves `image.data`, and from the + // same matrix: the balance and the conversion must agree about which white + // is neutral or the frame carries a cast that looks like a decode fault. let color_matrix = cam_to_srgb(&image); + let wb_coeffs = sane_wb(image.wb_coeffs, xyz_to_cam_of(&image).as_ref()); let data = match image.data { rawler::RawImageData::Integer(v) => v, @@ -484,7 +487,7 @@ pub fn decode(bytes: &[u8]) -> Result { .first() .map(|v| *v as u16) .unwrap_or(u16::MAX), - wb_coeffs: sane_wb(image.wb_coeffs), + wb_coeffs, color_matrix, }) } @@ -526,6 +529,28 @@ fn cam_to_srgb(image: &rawler::RawImage) -> Option<[f32; 9]> { cam_to_srgb_from(&xyz_to_cam) } +/// The camera's XYZ→camera matrix, as rawler holds it. +/// +/// Split out so the white-balance fallback and the colour matrix read the same +/// data through the same illuminant preference; two readers disagreeing about +/// which matrix a body uses would balance to one white and convert from +/// another. +fn xyz_to_cam_of(image: &rawler::RawImage) -> Option<[[f32; 3]; 3]> { + use rawler::imgop::xyz::Illuminant; + let flat = image + .color_matrix + .get(&Illuminant::D65) + .or_else(|| image.color_matrix.get(&Illuminant::A))?; + if flat.len() < 9 { + return None; + } + Some([ + [flat[0], flat[1], flat[2]], + [flat[3], flat[4], flat[5]], + [flat[6], flat[7], flat[8]], + ]) +} + /// The matrix maths, split out so it can be tested without a RAW file. // // The constants below are quoted at their published precision rather than @@ -621,10 +646,70 @@ fn cam_to_srgb_from(xyz_to_cam: &[[f32; 3]; 3]) -> Option<[f32; 9]> { /// /// Coefficients are also divided through by green, so green is the reference /// channel and exposure does not shift when white balance changes. -fn sane_wb(raw: [f32; 4]) -> [f32; 4] { - let usable = |v: f32| if v.is_finite() && v > 0.0 { v } else { 1.0 }; - let (r, g, b) = (usable(raw[0]), usable(raw[1]), usable(raw[2])); - [r / g, 1.0, b / g, 1.0] +fn sane_wb(raw: [f32; 4], matrix: Option<&[[f32; 3]; 3]>) -> [f32; 4] { + let usable = |v: f32| v.is_finite() && v > 0.0; + if usable(raw[0]) && usable(raw[1]) && usable(raw[2]) { + let (r, g, b) = (raw[0], raw[1], raw[2]); + return [r / g, 1.0, b / g, 1.0]; + } + + // **Absent is not neutral, and treating it as neutral is why frames came + // out pink.** A Bayer sensor's green photosites collect roughly twice the + // signal of its red and blue, so unbalanced data is strongly green. The + // camera matrix is built on the assumption that the data reaching it has + // already been balanced, and it subtracts green accordingly — fed + // green-heavy data it overshoots, and the frame lands in magenta. + // + // So a missing coefficient falls back to what the camera itself says about + // daylight rather than to 1.0. `daylight_wb` derives it from the same + // matrix the pipeline is about to apply, which makes the two consistent by + // construction: the multipliers neutralise exactly the white the matrix + // expects to be neutral. + match matrix.and_then(daylight_wb) { + Some([r, g, b]) => [r / g, 1.0, b / g, 1.0], + // No matrix either — an unknown body. Neutral is then the honest + // answer rather than a worse guess, and the result is uncalibrated + // rather than wrong in a specific direction. + None => [1.0, 1.0, 1.0, 1.0], + } +} + +/// TRACES: FR-DEV-3e +/// The multipliers that make daylight neutral on this sensor. +/// +/// The camera's own response to white, which is the quantity `cam_to_srgb_from` +/// computes on its way to balancing the matrix and then discards. Exposed +/// because two callers need it: the fallback above, and any control that wants +/// to express white balance against an absolute illuminant rather than as an +/// offset from whatever the camera happened to choose. +/// +/// Returns the **correcting multipliers**, green-normalised — not the camera's +/// response. The two are reciprocals and confusing them inverts the +/// correction: a sensor is *least* sensitive to the channel needing the +/// largest multiplier, so returning the response boosts exactly the wrong one. +pub fn daylight_wb(xyz_to_cam: &[[f32; 3]; 3]) -> Option<[f32; 3]> { + // The XYZ of sRGB white is the row sums of sRGB→XYZ. Written out rather + // than referencing the constant inside `cam_to_srgb_from` so the two + // cannot drift apart silently. + #[allow(clippy::excessive_precision)] + const WHITE_XYZ: [f32; 3] = [0.9504700, 1.0000000, 1.0888300]; + + let mut cam_white = [0.0f32; 3]; + for (i, row) in xyz_to_cam.iter().enumerate() { + for k in 0..3 { + cam_white[i] += row[k] * WHITE_XYZ[k]; + } + } + if cam_white.iter().any(|v| !v.is_finite() || *v <= 1e-6) { + return None; + } + // Reciprocal, then normalised so green is 1 and overall exposure does not + // move when the balance does. + Some([ + cam_white[1] / cam_white[0], + 1.0, + cam_white[1] / cam_white[2], + ]) } /// Invert a 3×3 matrix, or `None` if it is singular. @@ -911,7 +996,7 @@ mod tests { // rawler reports RGBE, and E is NaN on every three-colour sensor. // Measured on a Canon 6D: [1.893, 1.0, 1.797, NaN]. Uploaded raw, // that NaN poisons the shader's uniform block. - let wb = sane_wb([1.8925781, 1.0, 1.796875, f32::NAN]); + let wb = sane_wb([1.8925781, 1.0, 1.796875, f32::NAN], None); assert!(wb.iter().all(|v| v.is_finite()), "no NaN may survive"); assert_eq!(wb[3], 1.0); } @@ -920,20 +1005,77 @@ mod tests { fn white_balance_is_normalised_to_green() { // Green is the reference channel, so overall exposure does not shift // when white balance changes. - let wb = sane_wb([3.0, 2.0, 4.0, f32::NAN]); + let wb = sane_wb([3.0, 2.0, 4.0, f32::NAN], None); assert_eq!(wb[1], 1.0); assert!((wb[0] - 1.5).abs() < 1e-6); assert!((wb[2] - 2.0).abs() < 1e-6); } #[test] - fn absent_white_balance_falls_back_to_neutral() { - // A body reporting nothing must render neutral, not black or - // infinite. - let wb = sane_wb([0.0, 0.0, 0.0, 0.0]); + fn absent_white_balance_with_no_matrix_falls_back_to_neutral() { + // An unknown body: nothing to derive a better answer from, so neutral + // is the honest one. Uncalibrated beats wrong in a specific direction. + let wb = sane_wb([0.0, 0.0, 0.0, 0.0], None); assert_eq!(wb, [1.0, 1.0, 1.0, 1.0]); } + #[test] + fn absent_white_balance_uses_the_camera_daylight_rather_than_neutral() { + // **The pink-frame bug.** Substituting 1.0 for a missing coefficient + // leaves the sensor's raw green excess in place — green photosites + // collect roughly twice what red and blue do — and the camera matrix, + // which is built expecting balanced input, subtracts green and + // overshoots into magenta. + // + // A real matrix: the Canon 6D's, D65. Its daylight multipliers are + // emphatically not 1:1:1, and that difference is the whole fix. + let xyz_to_cam = [ + [0.7034, -0.0804, -0.1014], + [-0.4420, 1.2564, 0.2058], + [-0.0851, 0.1994, 0.5758], + ]; + let wb = sane_wb([0.0, 0.0, 0.0, 0.0], Some(&xyz_to_cam)); + + assert_eq!(wb[1], 1.0, "still green-normalised"); + assert!(wb.iter().all(|v| v.is_finite() && *v > 0.0)); + assert!( + (wb[0] - 1.0).abs() > 0.15 && (wb[2] - 1.0).abs() > 0.15, + "a real sensor's daylight balance is nowhere near neutral; got {wb:?}" + ); + // Red and blue both need lifting against green on a Bayer sensor. + assert!(wb[0] > 1.0 && wb[2] > 1.0, "got {wb:?}"); + } + + #[test] + fn the_daylight_multipliers_neutralise_what_the_matrix_calls_white() { + // The property that makes the fallback consistent rather than merely + // plausible: balancing by these and then applying the matrix must map + // the camera's white to a neutral sRGB. If the two disagreed, the + // fallback would trade one cast for another. + let xyz_to_cam = [ + [0.7034, -0.0804, -0.1014], + [-0.4420, 1.2564, 0.2058], + [-0.0851, 0.1994, 0.5758], + ]; + let wb = daylight_wb(&xyz_to_cam).expect("a real matrix has a white"); + let m = cam_to_srgb_from(&xyz_to_cam).expect("invertible"); + + // Camera-space white, balanced: each channel divided by its own + // response, which is unity by construction. + let balanced = [1.0f32, 1.0, 1.0]; + let out = [ + m[0] * balanced[0] + m[1] * balanced[1] + m[2] * balanced[2], + m[3] * balanced[0] + m[4] * balanced[1] + m[5] * balanced[2], + m[6] * balanced[0] + m[7] * balanced[1] + m[8] * balanced[2], + ]; + let spread = out.iter().cloned().fold(f32::MIN, f32::max) + - out.iter().cloned().fold(f32::MAX, f32::min); + assert!( + spread < 0.02, + "balanced white must come out neutral, got {out:?} from wb {wb:?}" + ); + } + #[test] fn xtrans_does_not_rephase() { // A 6×6 cell does not re-phase under a 2×2 shift; claiming otherwise