diff --git a/Cargo.lock b/Cargo.lock index e68254b..f59a50a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1183,6 +1183,8 @@ name = "dr-gpu" version = "0.1.0" dependencies = [ "bytemuck", + "dr-decode", + "dr-pipeline", "dr-types", "env_logger", "log", @@ -1191,6 +1193,14 @@ dependencies = [ "wgpu", ] +[[package]] +name = "dr-pipeline" +version = "0.1.0" +dependencies = [ + "dr-types", + "log", +] + [[package]] name = "dr-sync" version = "0.1.0" @@ -1206,6 +1216,7 @@ name = "dr-sync-nextcloud" version = "0.1.0" dependencies = [ "async-trait", + "dr-decode", "dr-sync", "dr-types", "env_logger", @@ -1234,6 +1245,7 @@ dependencies = [ "anyhow", "dr-decode", "dr-gpu", + "dr-pipeline", "dr-types", "log", "pollster", diff --git a/Cargo.toml b/Cargo.toml index d9c906d..6c58e38 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ "core/dr-types", "core/dr-decode", "core/dr-gpu", + "core/dr-pipeline", "core/dr-sync", "core/dr-sync-nextcloud", "ui/dr-ui", @@ -23,6 +24,7 @@ repository = "https://github.com/dtourolle/DarkRoom" dr-types = { path = "core/dr-types" } dr-decode = { path = "core/dr-decode" } dr-gpu = { path = "core/dr-gpu" } +dr-pipeline = { path = "core/dr-pipeline" } dr-sync = { path = "core/dr-sync" } dr-sync-nextcloud = { path = "core/dr-sync-nextcloud" } dr-ui = { path = "ui/dr-ui" } diff --git a/core/dr-decode/examples/rawinfo.rs b/core/dr-decode/examples/rawinfo.rs new file mode 100644 index 0000000..2e1376f --- /dev/null +++ b/core/dr-decode/examples/rawinfo.rs @@ -0,0 +1,61 @@ +//! Print what `decode` extracts from a RAW file. +//! +//! A sanity check on the pipeline's inputs: black and white levels, the CFA +//! pattern after re-phasing, as-shot white balance, and the camera→sRGB +//! matrix. Wrong values here produce a wrong image no shader can fix, so it +//! is worth being able to see them directly. +//! +//! ```sh +//! cargo run -p dr-decode --example rawinfo -- IMG.CR2 +//! ``` + +fn main() { + let Some(path) = std::env::args().nth(1) else { + eprintln!("usage: rawinfo "); + std::process::exit(2); + }; + + let bytes = std::fs::read(&path).expect("read file"); + let raw = dr_decode::decode(&bytes).expect("decode"); + + println!("file {path}"); + println!("readout {} × {}", raw.width, raw.height); + println!( + "crop {} × {} at ({}, {})", + raw.crop.width, raw.crop.height, raw.crop.x, raw.crop.y + ); + let (dx, dy) = raw.crop.shifts_cfa_phase(); + println!( + "cfa {:?} (rephased: {dx}, {dy})", + raw.cfa_pattern + ); + println!("black {:?}", raw.black_level); + println!("white {}", raw.white_level); + println!("wb_coeffs {:?}", raw.wb_coeffs); + + match raw.color_matrix { + Some(m) => { + println!("cam→srgb"); + for row in m.chunks(3) { + println!( + " [{:>8.4} {:>8.4} {:>8.4}]", + row[0], row[1], row[2] + ); + } + // Each row should sum to roughly 1: a neutral camera-space colour + // must stay neutral in sRGB. Far from 1 means the normalisation + // or the matrix composition is wrong. + let sums: Vec = m.chunks(3).map(|r| r.iter().sum()).collect(); + println!("row sums {sums:.4?} (≈1.0 each if correct)"); + } + None => println!("cam→srgb none — uncalibrated body"), + } + + // Sample the actual data range, which reveals a black-level or bit-depth + // mistake faster than any amount of staring at metadata. + let (min, max) = raw + .data + .iter() + .fold((u16::MAX, 0u16), |(lo, hi), &v| (lo.min(v), hi.max(v))); + println!("sample range {min} … {max}"); +} diff --git a/core/dr-decode/src/lib.rs b/core/dr-decode/src/lib.rs index d8e0dd6..5c23c4f 100644 --- a/core/dr-decode/src/lib.rs +++ b/core/dr-decode/src/lib.rs @@ -46,6 +46,8 @@ pub struct Metadata { /// them. #[derive(Debug, Clone)] pub struct RawImage { + /// Width of `data` in samples — the *full* sensor row stride, including + /// any masked border. Not the width the user sees; see [`Self::crop`]. pub width: u32, pub height: u32, /// One sample per photosite, in sensor order. @@ -55,8 +57,42 @@ pub struct RawImage { pub white_level: u16, /// As-shot white balance, as per-channel multipliers. pub wb_coeffs: [f32; 4], - /// Camera-to-XYZ colour matrix (FR-DEV-3e). + /// Camera RGB to linear sRGB (D65), row-major 3×3 (FR-DEV-3e). + /// + /// `None` where the body is unknown to the decoder, in which case the + /// pipeline falls back to identity and the result is uncalibrated rather + /// than wrong-by-a-guess. pub color_matrix: Option<[f32; 9]>, + /// The usable region of `data`, excluding masked and border photosites. + pub crop: CropRect, +} + +/// TRACES: FR-RAW-3 +/// The usable region of a sensor readout. +/// +/// RAW files carry photosites the image does not include: optically black +/// columns used to measure the black level, and a few border rows most +/// demosaics need as context but no viewer should display. Cropping is +/// therefore not an edit — it is part of reading the file correctly. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CropRect { + pub x: u32, + pub y: u32, + pub width: u32, + pub height: u32, +} + +impl CropRect { + /// Whether the crop origin shifts the CFA phase. + /// + /// A Bayer pattern repeats every 2×2, so a crop starting at an odd + /// coordinate makes the top-left photosite of the *visible* image a + /// different colour than the pattern names. Demosaicing without + /// accounting for it swaps red and blue — the classic symptom being a + /// correctly-exposed image with wildly wrong colour. + pub fn shifts_cfa_phase(&self) -> (bool, bool) { + (self.x % 2 == 1, self.y % 2 == 1) + } } /// TRACES: FR-RAW-5 @@ -78,6 +114,61 @@ impl CfaPattern { pub fn is_xtrans(self) -> bool { matches!(self, CfaPattern::XTrans) } + + /// The pattern as seen from an origin shifted by `(dx, dy)` photosites. + /// + /// Used to re-phase the pattern after cropping to the active area + /// ([`CropRect::shifts_cfa_phase`]). X-Trans is returned unchanged: its + /// 6×6 cell does not re-phase under a 2×2 shift, so the X-Trans demosaic + /// handles the offset itself. + pub fn shifted(self, dx: bool, dy: bool) -> Self { + use CfaPattern::*; + if matches!(self, XTrans | Unknown) { + return self; + } + // Shifting one column swaps the pair horizontally; one row swaps + // vertically. Both together is the diagonal opposite. + let after_x = if dx { + match self { + Rggb => Grbg, + Grbg => Rggb, + Bggr => Gbrg, + Gbrg => Bggr, + other => other, + } + } else { + self + }; + if dy { + match after_x { + Rggb => Gbrg, + Gbrg => Rggb, + Grbg => Bggr, + Bggr => Grbg, + other => other, + } + } else { + after_x + } + } + + /// The colour of the photosite at `(x, y)` within the pattern. + /// + /// Channel indices are 0=R, 1=G, 2=B, matching the shader's convention. + pub fn colour_at(self, x: u32, y: u32) -> u8 { + use CfaPattern::*; + // Each 2×2 cell listed row-major from its own origin. + let cell: [u8; 4] = match self { + Rggb => [0, 1, 1, 2], + Bggr => [2, 1, 1, 0], + Grbg => [1, 0, 2, 1], + Gbrg => [1, 2, 0, 1], + // Not meaningful for a 6×6 pattern or an unknown one; the caller + // must not be on the Bayer path at all. + XTrans | Unknown => [1, 1, 1, 1], + }; + cell[((y % 2) * 2 + (x % 2)) as usize] + } } /// TRACES: FR-RAW-1 | M-9 @@ -164,6 +255,9 @@ 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`. + let color_matrix = cam_to_srgb(&image); + let data = match image.data { rawler::RawImageData::Integer(v) => v, rawler::RawImageData::Float(v) => { @@ -175,7 +269,6 @@ pub fn decode(bytes: &[u8]) -> Result { } }; - let cfa = cfa_from_rawler(&image.camera.cfa, image.camera.model.as_str()); // Black levels are rationals; the pipeline wants plain u16 samples. let bl = &image.blacklevel.levels; let level_at = |i: usize| -> u16 { @@ -185,9 +278,36 @@ pub fn decode(bytes: &[u8]) -> Result { }; let black_level = [level_at(0), level_at(1), level_at(2), level_at(3)]; + // Prefer the recommended crop, falling back to the active area, then to + // the whole readout. `crop_area` is what the camera itself would show; + // `active_area` merely excludes the masked border. + let rect = image.crop_area.or(image.active_area); + let crop = match rect { + Some(r) => CropRect { + x: r.p.x as u32, + y: r.p.y as u32, + width: r.d.w as u32, + height: r.d.h as u32, + }, + None => CropRect { + x: 0, + y: 0, + width: image.width as u32, + height: image.height as u32, + }, + }; + + // Re-phase the CFA to the crop origin, or the demosaic swaps R and B on + // any body whose active area starts at an odd coordinate. Applied exactly + // once — shifting twice returns the original pattern and reintroduces the + // very bug it exists to prevent. + let (dx, dy) = crop.shifts_cfa_phase(); + let cfa = cfa_from_rawler(&image.camera.cfa, image.camera.model.as_str()).shifted(dx, dy); + Ok(RawImage { width: image.width as u32, height: image.height as u32, + crop, data, cfa_pattern: cfa, black_level, @@ -197,11 +317,177 @@ pub fn decode(bytes: &[u8]) -> Result { .first() .map(|v| *v as u16) .unwrap_or(u16::MAX), - wb_coeffs: image.wb_coeffs, - color_matrix: None, + wb_coeffs: sane_wb(image.wb_coeffs), + color_matrix, }) } +/// TRACES: FR-DEV-3e +/// Compose the camera→sRGB-linear matrix from rawler's XYZ→camera. +/// +/// **Two rawler traps this avoids**, both measured on a Canon 6D CR2 +/// (2026-08-09): +/// +/// 1. `RawImage::xyz_to_cam` is **all zeros** — it carries an upstream +/// deprecation note and 0.7.2 no longer fills it. The live data is +/// `color_matrix`, keyed by illuminant. Reading the old field silently +/// yields no colour transform at all. +/// 2. `cam_to_xyz_normalized()` divides each of four rows by its own sum, and +/// the fourth row (emerald/white, unused on any Bayer body) sums to zero. +/// Every element came back `NaN`. Inverting the 3×3 ourselves avoids the +/// fourth channel entirely. +fn cam_to_srgb(image: &rawler::RawImage) -> Option<[f32; 9]> { + use rawler::imgop::xyz::Illuminant; + + // Prefer D65 — it matches sRGB's white point, so no chromatic adaptation + // is needed. Illuminant A (tungsten) is a distant fallback for bodies + // that ship only one matrix; adapting it properly is a v0.2 colour- + // management concern (ARCH §5.2), not something to fake here. + let flat = image + .color_matrix + .get(&Illuminant::D65) + .or_else(|| image.color_matrix.get(&Illuminant::A))?; + if flat.len() < 9 { + return None; + } + + let xyz_to_cam: [[f32; 3]; 3] = [ + [flat[0], flat[1], flat[2]], + [flat[3], flat[4], flat[5]], + [flat[6], flat[7], flat[8]], + ]; + cam_to_srgb_from(&xyz_to_cam) +} + +/// 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 +// trimmed to what f32 can represent. Truncating a standard matrix to satisfy +// a linter makes it harder to check against the specification, and the +// rounding happens identically either way. +#[allow(clippy::excessive_precision)] +fn cam_to_srgb_from(xyz_to_cam: &[[f32; 3]; 3]) -> Option<[f32; 9]> { + // XYZ (D65) → linear sRGB, the standard primaries. + const XYZ_TO_SRGB: [[f32; 3]; 3] = [ + [3.2404542, -1.5371385, -0.4985314], + [-0.9692660, 1.8760108, 0.0415560], + [0.0556434, -0.2040259, 1.0572252], + ]; + // sRGB (D65) → XYZ, for finding the camera response to white. + const SRGB_TO_XYZ: [[f32; 3]; 3] = [ + [0.4124564, 0.3575761, 0.1804375], + [0.2126729, 0.7151522, 0.0721750], + [0.0193339, 0.1191920, 0.9503041], + ]; + + // An absent or unpopulated matrix is all zeros. Using it would render + // black, so report absence and let the caller fall back to identity. + if xyz_to_cam.iter().flatten().all(|v| v.abs() < f32::EPSILON) { + return None; + } + if xyz_to_cam.iter().flatten().any(|v| !v.is_finite()) { + return None; + } + + // White balance to D65: find what the camera reports for sRGB white, so + // the composed matrix maps neutral to neutral. Without this the image + // carries a strong cast even with correct primaries. + let mut cam_white = [0.0f32; 3]; + for (i, row) in xyz_to_cam.iter().enumerate() { + // xyz_to_cam · (XYZ of sRGB white) — the row sums of SRGB_TO_XYZ. + for k in 0..3 { + let white_k: f32 = SRGB_TO_XYZ[k].iter().sum(); + cam_white[i] += row[k] * white_k; + } + } + if cam_white.iter().any(|v| v.abs() < 1e-6 || !v.is_finite()) { + return None; + } + + // Scale each row so the camera's own white becomes unity, then invert. + let balanced = [ + [ + xyz_to_cam[0][0] / cam_white[0], + xyz_to_cam[0][1] / cam_white[0], + xyz_to_cam[0][2] / cam_white[0], + ], + [ + xyz_to_cam[1][0] / cam_white[1], + xyz_to_cam[1][1] / cam_white[1], + xyz_to_cam[1][2] / cam_white[1], + ], + [ + xyz_to_cam[2][0] / cam_white[2], + xyz_to_cam[2][1] / cam_white[2], + xyz_to_cam[2][2] / cam_white[2], + ], + ]; + + let cam_to_xyz = invert3(&balanced)?; + + let mut out = [0.0f32; 9]; + for i in 0..3 { + for j in 0..3 { + let mut sum = 0.0; + for k in 0..3 { + sum += XYZ_TO_SRGB[i][k] * cam_to_xyz[k][j]; + } + out[i * 3 + j] = sum; + } + } + + if out.iter().any(|v| !v.is_finite()) { + return None; + } + Some(out) +} + +/// TRACES: FR-DEV-3e +/// Normalise as-shot white balance into usable multipliers. +/// +/// rawler reports coefficients in RGBE order, and the fourth is `NaN` on +/// every three-colour sensor — *measured on a Canon 6D CR2 (2026-08-09): +/// `[1.893, 1.0, 1.797, NaN]`*. Uploaded to the GPU unchecked, that NaN +/// contaminates the shader's uniform block. It is normalised to 1.0 here, +/// where the reason can be written down, rather than being defended against +/// at every use site. +/// +/// 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] +} + +/// Invert a 3×3 matrix, or `None` if it is singular. +fn invert3(m: &[[f32; 3]; 3]) -> Option<[[f32; 3]; 3]> { + let det = m[0][0] * (m[1][1] * m[2][2] - m[1][2] * m[2][1]) + - m[0][1] * (m[1][0] * m[2][2] - m[1][2] * m[2][0]) + + m[0][2] * (m[1][0] * m[2][1] - m[1][1] * m[2][0]); + if det.abs() < 1e-12 || !det.is_finite() { + return None; + } + let inv = 1.0 / det; + Some([ + [ + (m[1][1] * m[2][2] - m[1][2] * m[2][1]) * inv, + (m[0][2] * m[2][1] - m[0][1] * m[2][2]) * inv, + (m[0][1] * m[1][2] - m[0][2] * m[1][1]) * inv, + ], + [ + (m[1][2] * m[2][0] - m[1][0] * m[2][2]) * inv, + (m[0][0] * m[2][2] - m[0][2] * m[2][0]) * inv, + (m[0][2] * m[1][0] - m[0][0] * m[1][2]) * inv, + ], + [ + (m[1][0] * m[2][1] - m[1][1] * m[2][0]) * inv, + (m[0][1] * m[2][0] - m[0][0] * m[2][1]) * inv, + (m[0][0] * m[1][1] - m[0][1] * m[1][0]) * inv, + ], + ]) +} + fn cfa_from_rawler(cfa: &rawler::CFA, model: &str) -> CfaPattern { // rawler exposes the pattern as a string; X-Trans is 6x6 rather than 2x2. let name = cfa.name.to_ascii_uppercase(); @@ -263,4 +549,171 @@ mod tests { assert!(CfaPattern::XTrans.is_xtrans()); assert!(!CfaPattern::Rggb.is_xtrans()); } + + #[test] + fn cfa_colours_follow_the_named_pattern() { + // RGGB: red at the origin, blue diagonally opposite. + let p = CfaPattern::Rggb; + assert_eq!(p.colour_at(0, 0), 0, "top-left is red"); + assert_eq!(p.colour_at(1, 0), 1, "top-right is green"); + assert_eq!(p.colour_at(0, 1), 1, "bottom-left is green"); + assert_eq!(p.colour_at(1, 1), 2, "bottom-right is blue"); + } + + #[test] + fn cfa_pattern_repeats_every_two_photosites() { + let p = CfaPattern::Bggr; + for (x, y) in [(0u32, 0u32), (1, 0), (0, 1), (1, 1)] { + assert_eq!(p.colour_at(x, y), p.colour_at(x + 2, y + 2)); + assert_eq!(p.colour_at(x, y), p.colour_at(x + 100, y + 64)); + } + } + + #[test] + fn an_odd_crop_origin_rephases_the_pattern() { + // The bug this prevents: a body whose active area starts at an odd + // column renders with red and blue swapped, because the visible + // top-left photosite is not the one the pattern names. + let shifted = CfaPattern::Rggb.shifted(true, false); + assert_eq!(shifted, CfaPattern::Grbg); + // Reading the shifted pattern at the origin must agree with reading + // the original one column across. + assert_eq!(shifted.colour_at(0, 0), CfaPattern::Rggb.colour_at(1, 0)); + assert_eq!(shifted.colour_at(1, 0), CfaPattern::Rggb.colour_at(2, 0)); + } + + #[test] + fn shifting_both_axes_gives_the_diagonal_opposite() { + let s = CfaPattern::Rggb.shifted(true, true); + assert_eq!(s, CfaPattern::Bggr); + assert_eq!(s.colour_at(0, 0), CfaPattern::Rggb.colour_at(1, 1)); + } + + #[test] + fn shifting_is_its_own_inverse() { + for p in [ + CfaPattern::Rggb, + CfaPattern::Bggr, + CfaPattern::Grbg, + CfaPattern::Gbrg, + ] { + assert_eq!(p.shifted(true, false).shifted(true, false), p); + assert_eq!(p.shifted(false, true).shifted(false, true), p); + assert_eq!(p.shifted(true, true).shifted(true, true), p); + } + } + + #[test] + fn an_even_crop_origin_leaves_the_pattern_alone() { + let c = CropRect { + x: 0, + y: 0, + width: 100, + height: 100, + }; + assert_eq!(c.shifts_cfa_phase(), (false, false)); + assert_eq!(CfaPattern::Rggb.shifted(false, false), CfaPattern::Rggb); + + let even = CropRect { + x: 84, + y: 50, + width: 100, + height: 100, + }; + assert_eq!(even.shifts_cfa_phase(), (false, false)); + } + + #[test] + fn neutral_stays_neutral_through_the_colour_matrix() { + // The property that makes a camera matrix correct: a neutral camera + // colour must land on a neutral sRGB colour, so every row sums to 1. + // A matrix that fails this renders a strong global cast. + // + // Values are the D65 matrix rawler reports for a Canon EOS 6D. + let xyz_to_cam = [ + [0.7034, -0.0804, -0.1014], + [-0.4420, 1.2564, 0.2058], + [-0.0851, 0.1994, 0.5758], + ]; + let m = cam_to_srgb_from(&xyz_to_cam).expect("a well-formed matrix inverts"); + + for (i, row) in m.chunks(3).enumerate() { + let sum: f32 = row.iter().sum(); + assert!( + (sum - 1.0).abs() < 1e-4, + "row {i} sums to {sum}, not 1.0 — neutral would not stay neutral" + ); + } + } + + #[test] + fn an_all_zero_matrix_is_absent_rather_than_black() { + // rawler's deprecated `xyz_to_cam` is all zeros in 0.7.2. Treating it + // as a real matrix renders a black image; the pipeline needs to know + // to fall back to identity instead. + assert_eq!(cam_to_srgb_from(&[[0.0; 3]; 3]), None); + } + + #[test] + fn a_singular_matrix_is_rejected() { + // Two identical rows cannot be inverted; returning garbage here would + // surface as an unexplained colour failure much later. + let singular = [[1.0, 2.0, 3.0], [1.0, 2.0, 3.0], [4.0, 5.0, 6.0]]; + assert_eq!(cam_to_srgb_from(&singular), None); + } + + #[test] + // Indexing by i/j is how the matrix identity is written down; iterators + // would obscure what is being asserted. + #[allow(clippy::needless_range_loop)] + fn inversion_round_trips() { + let m = [[2.0, 0.0, 1.0], [1.0, 3.0, 0.0], [0.0, 1.0, 4.0]]; + let inv = invert3(&m).expect("invertible"); + // m · inv should be the identity. + for i in 0..3 { + for j in 0..3 { + let mut sum = 0.0; + for k in 0..3 { + sum += m[i][k] * inv[k][j]; + } + let expected = if i == j { 1.0 } else { 0.0 }; + assert!((sum - expected).abs() < 1e-5, "element ({i},{j}) = {sum}"); + } + } + } + + #[test] + fn white_balance_drops_the_nan_fourth_channel() { + // 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]); + assert!(wb.iter().all(|v| v.is_finite()), "no NaN may survive"); + assert_eq!(wb[3], 1.0); + } + + #[test] + 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]); + 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]); + assert_eq!(wb, [1.0, 1.0, 1.0, 1.0]); + } + + #[test] + fn xtrans_does_not_rephase() { + // A 6×6 cell does not re-phase under a 2×2 shift; claiming otherwise + // would corrupt the X-Trans path rather than fix it. + assert_eq!(CfaPattern::XTrans.shifted(true, true), CfaPattern::XTrans); + } } diff --git a/core/dr-gpu/Cargo.toml b/core/dr-gpu/Cargo.toml index 7831b5f..7d7a6dd 100644 --- a/core/dr-gpu/Cargo.toml +++ b/core/dr-gpu/Cargo.toml @@ -7,19 +7,27 @@ license.workspace = true [dependencies] dr-types.workspace = true +dr-decode.workspace = true +dr-pipeline.workspace = true wgpu.workspace = true thiserror.workspace = true log.workspace = true bytemuck.workspace = true +# Needed outside tests: shader compilation errors are collected through an +# async error scope, which must be resolved before the pipeline is returned. +pollster.workspace = true [dev-dependencies] -pollster.workspace = true env_logger.workspace = true [[example]] name = "bench" required-features = ["readback"] +[[example]] +name = "develop" +required-features = ["readback"] + [features] default = [] # Exposes read_pixels outside tests. Production must not enable this. diff --git a/core/dr-gpu/examples/develop.rs b/core/dr-gpu/examples/develop.rs new file mode 100644 index 0000000..50abc8f --- /dev/null +++ b/core/dr-gpu/examples/develop.rs @@ -0,0 +1,136 @@ +//! Render a RAW file through the full pipeline and write a PPM. +//! +//! The end-to-end check: decode → demosaic → adjust → display encode, on a +//! real file rather than a synthetic fixture. Unit tests prove each stage in +//! isolation; this proves they compose into an image a person would accept. +//! +//! ```sh +//! cargo run -p dr-gpu --example develop --features readback -- IMG.CR2 out.ppm +//! ``` +//! +//! PPM because it needs no encoder dependency and every image viewer reads +//! it. This is a diagnostic, not the export path (FR-EXP-*). + +use dr_gpu::{AdjustPass, Demosaicer, GpuContext}; +use dr_pipeline::ops::{colour, exposure, tone, white_balance}; +use dr_pipeline::EditGraph; + +fn main() { + env_logger::init(); + + let mut args = std::env::args().skip(1); + let Some(input) = args.next() else { + eprintln!("usage: develop [out.ppm] [preset]"); + eprintln!(" preset: neutral (default) | punchy | recover"); + std::process::exit(2); + }; + let output = args.next().unwrap_or_else(|| "develop.ppm".into()); + let preset = args.next().unwrap_or_else(|| "neutral".into()); + + let bytes = std::fs::read(&input).expect("read file"); + + let t0 = std::time::Instant::now(); + let raw = dr_decode::decode(&bytes).expect("decode"); + let decode_ms = t0.elapsed().as_secs_f32() * 1000.0; + println!( + "decoded {} × {} ({:?}), {decode_ms:.0} ms", + raw.crop.width, raw.crop.height, raw.cfa_pattern + ); + + let ctx = pollster::block_on(GpuContext::new_headless()).expect("gpu"); + println!("adapter {} ({:?})", ctx.adapter_name(), ctx.backend()); + + let t1 = std::time::Instant::now(); + let demosaicer = Demosaicer::new(&ctx).expect("demosaicer"); + let image = demosaicer.run(&raw).expect("demosaic"); + ctx.device.poll(wgpu::Maintain::Wait); + println!("demosaiced {:.0} ms", t1.elapsed().as_secs_f32() * 1000.0); + + // Build an edit. The presets exist so the output can be eyeballed for + // each operation actually doing something, not merely compiling. + let mut graph = EditGraph::default_chain(); + match preset.as_str() { + "punchy" => { + graph.set_param(exposure::ID, exposure::EXPOSURE, 0.3); + graph.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::HIGHLIGHTS, -40.0); + graph.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::SHADOWS, 30.0); + graph.set_param(tone::BLACKS_WHITES_ID, tone::BLACKS, -20.0); + graph.set_param(tone::BLACKS_WHITES_ID, tone::WHITES, 25.0); + graph.set_param(colour::VIBRANCE_ID, colour::VIBRANCE, 35.0); + } + "recover" => { + graph.set_param(exposure::ID, exposure::EXPOSURE, -0.5); + graph.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::HIGHLIGHTS, -80.0); + graph.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::SHADOWS, 60.0); + graph.set_param(colour::BRILLIANCE_ID, colour::BRILLIANCE, 40.0); + graph.set_param(white_balance::ID, white_balance::TEMPERATURE, 15.0); + } + _ => {} + } + + let shader = graph.compose(); + println!( + "shader {} active op(s), {} uniform floats, structure {:016x}", + shader.source.matches("---- ").count(), + shader.uniforms.len(), + shader.structure_hash + ); + + let mut adjust = AdjustPass::new(&ctx); + let (w, h) = image.size(); + + let t2 = std::time::Instant::now(); + adjust.render(&image, &shader, w, h).expect("adjust"); + ctx.device.poll(wgpu::Maintain::Wait); + println!("adjusted {:.2} ms", t2.elapsed().as_secs_f32() * 1000.0); + + // Time a second render with only a value changed: this is the slider + // path, and it must not recompile. + graph.set_param(exposure::ID, exposure::EXPOSURE, 0.31); + let again = graph.compose(); + let t3 = std::time::Instant::now(); + adjust.render(&image, &again, w, h).expect("adjust"); + ctx.device.poll(wgpu::Maintain::Wait); + println!( + "re-render {:.2} ms ({} pipeline(s) compiled)", + t3.elapsed().as_secs_f32() * 1000.0, + adjust.cached_pipelines() + ); + + let (pixels, pw, ph) = adjust.read_output().expect("readback"); + + // Sanity: an all-black or all-white result means something upstream + // failed silently, and it is far easier to see here than in a viewer. + let mut sum = 0u64; + let mut min = 255u8; + let mut max = 0u8; + for px in pixels.chunks_exact(4) { + let l = px[0].max(px[1]).max(px[2]); + sum += u64::from(l); + min = min.min(l); + max = max.max(l); + } + let mean = sum as f64 / (pixels.len() / 4) as f64; + println!("levels min {min}, mean {mean:.1}, max {max}"); + if max == 0 { + eprintln!("WARNING: the image is entirely black"); + } + + write_ppm(&output, &pixels, pw, ph); + println!("wrote {output} ({pw} × {ph})"); +} + +/// Write binary PPM (P6): a three-line header then RGB triples. +fn write_ppm(path: &str, rgba: &[u8], w: u32, h: u32) { + use std::io::Write; + + let mut out = Vec::with_capacity((w * h * 3) as usize + 32); + out.extend_from_slice(format!("P6\n{w} {h}\n255\n").as_bytes()); + for px in rgba.chunks_exact(4) { + out.extend_from_slice(&px[..3]); + } + std::fs::File::create(path) + .expect("create output") + .write_all(&out) + .expect("write output"); +} diff --git a/core/dr-gpu/src/adjust.rs b/core/dr-gpu/src/adjust.rs new file mode 100644 index 0000000..22f629b --- /dev/null +++ b/core/dr-gpu/src/adjust.rs @@ -0,0 +1,768 @@ +//! The adjust pass — runs `dr-pipeline`'s generated shader. +//! +//! Takes the demosaiced texture, applies the composed operation chain, and +//! writes a display-ready RGBA8 texture. One dispatch, whatever the number of +//! active operations, because the operations were fused into one shader +//! before they got here. +//! +//! # The pipeline cache +//! +//! Compiling a shader takes milliseconds — fine once, ruinous per frame while +//! a slider is moving. Pipelines are therefore cached by the composed +//! shader's `structure_hash`, which covers the operation set and their order +//! but not their values. Dragging a slider re-uploads a uniform buffer and +//! reuses the compiled pipeline; enabling an operation compiles once and then +//! also reuses. + +use std::collections::HashMap; + +use dr_pipeline::ComposedShader; +use wgpu::util::DeviceExt; + +use crate::{DemosaicedImage, GpuContext, GpuError}; + +/// Number of leading floats in the generated uniform block that the composer +/// reserves for base parameters — three padded matrix rows and the as-shot +/// white balance. Must match `BASE_UNIFORM_FIELDS` in dr-pipeline. +const BASE_FIELDS: usize = 16; + +/// Runs composed operation chains against demosaiced images. +pub struct AdjustPass { + ctx: GpuContext, + bind_group_layout: wgpu::BindGroupLayout, + pipeline_layout: wgpu::PipelineLayout, + /// Compiled pipelines by structure hash (ARCH §5.6). + cache: HashMap, + /// Output texture, reallocated only when the size changes. + target: Option, +} + +struct Target { + texture: wgpu::Texture, + view: wgpu::TextureView, + width: u32, + height: u32, +} + +impl AdjustPass { + pub const FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba8Unorm; + + pub fn new(ctx: &GpuContext) -> Self { + let bind_group_layout = + ctx.device + .create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor { + label: Some("adjust-bgl"), + entries: &[ + // The demosaiced source. + wgpu::BindGroupLayoutEntry { + binding: 0, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::Texture { + sample_type: wgpu::TextureSampleType::Float { filterable: true }, + view_dimension: wgpu::TextureViewDimension::D2, + multisampled: false, + }, + count: None, + }, + wgpu::BindGroupLayoutEntry { + binding: 1, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::Buffer { + ty: wgpu::BufferBindingType::Uniform, + has_dynamic_offset: false, + min_binding_size: None, + }, + count: None, + }, + wgpu::BindGroupLayoutEntry { + binding: 2, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::StorageTexture { + access: wgpu::StorageTextureAccess::WriteOnly, + format: Self::FORMAT, + view_dimension: wgpu::TextureViewDimension::D2, + }, + count: None, + }, + ], + }); + + let pipeline_layout = ctx + .device + .create_pipeline_layout(&wgpu::PipelineLayoutDescriptor { + label: Some("adjust-layout"), + bind_group_layouts: &[&bind_group_layout], + push_constant_ranges: &[], + }); + + Self { + ctx: ctx.clone(), + bind_group_layout, + pipeline_layout, + cache: HashMap::new(), + target: None, + } + } + + /// Compile a composed shader, or return the cached pipeline. + /// + /// Compilation errors carry the generated source, since a stray line + /// number against code nobody wrote is otherwise very hard to act on. + fn pipeline(&mut self, shader: &ComposedShader) -> Result<&wgpu::ComputePipeline, GpuError> { + if !self.cache.contains_key(&shader.structure_hash) { + // A validation error here is a codegen bug, not a user error. + // Push an error scope so it surfaces as a Result rather than a + // panic from wgpu's default handler. + self.ctx + .device + .push_error_scope(wgpu::ErrorFilter::Validation); + + let module = self + .ctx + .device + .create_shader_module(wgpu::ShaderModuleDescriptor { + label: Some("adjust-generated"), + source: wgpu::ShaderSource::Wgsl(shader.source.as_str().into()), + }); + + let pipeline = + self.ctx + .device + .create_compute_pipeline(&wgpu::ComputePipelineDescriptor { + label: Some("adjust-pipeline"), + layout: Some(&self.pipeline_layout), + module: &module, + entry_point: Some("main"), + compilation_options: Default::default(), + cache: None, + }); + + if let Some(err) = pollster::block_on(self.ctx.device.pop_error_scope()) { + return Err(GpuError::ShaderCompilation(format!( + "{err}\n\n--- generated source ---\n{}", + numbered(&shader.source) + ))); + } + + self.cache.insert(shader.structure_hash, pipeline); + } + + Ok(self + .cache + .get(&shader.structure_hash) + .expect("just inserted")) + } + + /// Ensure the output texture matches the requested size. + fn ensure_target(&mut self, width: u32, height: u32) { + let matches = self + .target + .as_ref() + .is_some_and(|t| t.width == width && t.height == height); + if matches { + return; + } + + let texture = self.ctx.device.create_texture(&wgpu::TextureDescriptor { + label: Some("adjust-output"), + size: wgpu::Extent3d { + width, + height, + depth_or_array_layers: 1, + }, + mip_level_count: 1, + sample_count: 1, + dimension: wgpu::TextureDimension::D2, + format: Self::FORMAT, + usage: wgpu::TextureUsages::STORAGE_BINDING + | wgpu::TextureUsages::TEXTURE_BINDING + | wgpu::TextureUsages::COPY_SRC, + view_formats: &[], + }); + let view = texture.create_view(&Default::default()); + self.target = Some(Target { + texture, + view, + width, + height, + }); + } + + /// Render one frame at the requested output size. + /// + /// `width`/`height` are the *display* size, which is normally far smaller + /// than the image. Rendering at viewport resolution rather than sensor + /// resolution is what keeps slider interaction inside the frame budget + /// (FR-DSP-1). + pub fn render( + &mut self, + source: &DemosaicedImage, + shader: &ComposedShader, + width: u32, + height: u32, + ) -> Result<&wgpu::Texture, GpuError> { + let (width, height) = (width.max(1), height.max(1)); + self.ensure_target(width, height); + + // Base uniforms: the camera matrix and as-shot white balance, which + // every generated shader reads regardless of which operations are + // active. + let mut uniforms = shader.uniforms.clone(); + if uniforms.len() < BASE_FIELDS { + uniforms.resize(BASE_FIELDS, 0.0); + } + let m = source.color_matrix(); + let wb = source.as_shot_wb(); + // Rows padded to vec4 for std140 alignment. + uniforms[0..4].copy_from_slice(&[m[0], m[1], m[2], 0.0]); + uniforms[4..8].copy_from_slice(&[m[3], m[4], m[5], 0.0]); + uniforms[8..12].copy_from_slice(&[m[6], m[7], m[8], 0.0]); + uniforms[12..16].copy_from_slice(&[wb[0], wb[1], wb[2], 0.0]); + + let params_buf = self + .ctx + .device + .create_buffer_init(&wgpu::util::BufferInitDescriptor { + label: Some("adjust-params"), + contents: bytemuck::cast_slice(&uniforms), + usage: wgpu::BufferUsages::UNIFORM, + }); + + // Borrow order: compile first, since `pipeline` takes &mut self. + let _ = self.pipeline(shader)?; + let pipeline = self + .cache + .get(&shader.structure_hash) + .expect("compiled above"); + let target = self.target.as_ref().expect("ensured above"); + + let bind_group = self + .ctx + .device + .create_bind_group(&wgpu::BindGroupDescriptor { + label: Some("adjust-bg"), + layout: &self.bind_group_layout, + entries: &[ + wgpu::BindGroupEntry { + binding: 0, + resource: wgpu::BindingResource::TextureView(source.view()), + }, + wgpu::BindGroupEntry { + binding: 1, + resource: params_buf.as_entire_binding(), + }, + wgpu::BindGroupEntry { + binding: 2, + resource: wgpu::BindingResource::TextureView(&target.view), + }, + ], + }); + + let mut enc = self + .ctx + .device + .create_command_encoder(&wgpu::CommandEncoderDescriptor { + label: Some("adjust-encoder"), + }); + { + let mut pass = enc.begin_compute_pass(&wgpu::ComputePassDescriptor { + label: Some("adjust-pass"), + timestamp_writes: None, + }); + pass.set_pipeline(pipeline); + pass.set_bind_group(0, &bind_group, &[]); + pass.dispatch_workgroups(width.div_ceil(8), height.div_ceil(8), 1); + } + self.ctx.queue.submit(Some(enc.finish())); + + Ok(&self.target.as_ref().expect("ensured above").texture) + } + + /// How many distinct pipelines are compiled. Exposed for tests asserting + /// that slider movement does not recompile. + pub fn cached_pipelines(&self) -> usize { + self.cache.len() + } + + pub fn output(&self) -> Option<&wgpu::Texture> { + self.target.as_ref().map(|t| &t.texture) + } + + /// Copy the output to the CPU as tightly packed RGBA8. + /// + /// **A temporary bridge, not the display path.** ARCH §6.1 forbids this + /// round-trip in production and AC-8 asserts it does not happen; it + /// exists only because Slint's texture-import path is unwired until + /// spike S1. Measured cost at 4K is ~7 ms against a 0.28 ms compute pass + /// — 96% of the frame — so this must go, and the `readback` feature gate + /// keeps it out of a shipping build. + #[cfg(any(test, feature = "readback"))] + pub fn read_output(&self) -> Result<(Vec, u32, u32), GpuError> { + let Some(target) = self.target.as_ref() else { + return Err(GpuError::Readback("nothing rendered yet".into())); + }; + let (w, h) = (target.width, target.height); + + let unpadded = w * 4; + let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT; + let padded = unpadded.div_ceil(align) * align; + + let buf = self.ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("adjust-readback"), + size: (padded * h) as u64, + usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ, + mapped_at_creation: false, + }); + + let mut enc = self.ctx.device.create_command_encoder(&Default::default()); + enc.copy_texture_to_buffer( + wgpu::ImageCopyTexture { + texture: &target.texture, + mip_level: 0, + origin: wgpu::Origin3d::ZERO, + aspect: wgpu::TextureAspect::All, + }, + wgpu::ImageCopyBuffer { + buffer: &buf, + layout: wgpu::ImageDataLayout { + offset: 0, + bytes_per_row: Some(padded), + rows_per_image: Some(h), + }, + }, + wgpu::Extent3d { + width: w, + height: h, + depth_or_array_layers: 1, + }, + ); + self.ctx.queue.submit(Some(enc.finish())); + + let slice = buf.slice(..); + let (tx, rx) = std::sync::mpsc::channel(); + slice.map_async(wgpu::MapMode::Read, move |r| { + let _ = tx.send(r); + }); + self.ctx.device.poll(wgpu::Maintain::Wait); + rx.recv() + .map_err(|e| GpuError::Readback(e.to_string()))? + .map_err(|e| GpuError::Readback(e.to_string()))?; + + let data = slice.get_mapped_range(); + let mut out = Vec::with_capacity((unpadded * h) as usize); + for row in 0..h { + let start = (row * padded) as usize; + out.extend_from_slice(&data[start..start + unpadded as usize]); + } + drop(data); + buf.unmap(); + Ok((out, w, h)) + } +} + +/// Number the lines of generated source, so a compiler error can be located. +fn numbered(src: &str) -> String { + src.lines() + .enumerate() + .map(|(i, l)| format!("{:>4} | {l}", i + 1)) + .collect::>() + .join("\n") +} + +#[cfg(test)] +mod tests { + use super::*; + use dr_decode::{CfaPattern, CropRect, RawImage}; + use dr_pipeline::ops::{colour, exposure, tone, white_balance}; + use dr_pipeline::EditGraph; + + use crate::Demosaicer; + + fn ctx() -> Option { + match pollster::block_on(GpuContext::new_headless()) { + Ok(c) => Some(c), + Err(e) => { + eprintln!("skipping: no GPU adapter ({e})"); + None + } + } + } + + /// A flat mid-grey image, so an operation's effect is unambiguous. + fn grey_image(ctx: &GpuContext, level: u16) -> DemosaicedImage { + let size = 16u32; + let mut data = vec![0u16; (size * size) as usize]; + for v in data.iter_mut() { + *v = level; + } + let raw = RawImage { + width: size, + height: size, + data, + cfa_pattern: CfaPattern::Rggb, + black_level: [0; 4], + white_level: 16383, + wb_coeffs: [1.0, 1.0, 1.0, 1.0], + // Identity, so the test reasons about the operations alone + // rather than about a camera's colour response. + color_matrix: Some([1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]), + crop: CropRect { + x: 0, + y: 0, + width: size, + height: size, + }, + }; + Demosaicer::new(ctx) + .expect("demosaicer") + .run(&raw) + .expect("demosaic") + } + + fn read_centre(ctx: &GpuContext, tex: &wgpu::Texture) -> [u8; 4] { + let w = tex.width(); + let h = tex.height(); + let unpadded = w * 4; + let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT; + let padded = unpadded.div_ceil(align) * align; + + let buf = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("adjust-readback"), + size: (padded * h) as u64, + usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ, + mapped_at_creation: false, + }); + + let mut enc = ctx.device.create_command_encoder(&Default::default()); + enc.copy_texture_to_buffer( + wgpu::ImageCopyTexture { + texture: tex, + mip_level: 0, + origin: wgpu::Origin3d::ZERO, + aspect: wgpu::TextureAspect::All, + }, + wgpu::ImageCopyBuffer { + buffer: &buf, + layout: wgpu::ImageDataLayout { + offset: 0, + bytes_per_row: Some(padded), + rows_per_image: Some(h), + }, + }, + wgpu::Extent3d { + width: w, + height: h, + depth_or_array_layers: 1, + }, + ); + ctx.queue.submit(Some(enc.finish())); + + let slice = buf.slice(..); + let (tx, rx) = std::sync::mpsc::channel(); + slice.map_async(wgpu::MapMode::Read, move |r| { + let _ = tx.send(r); + }); + ctx.device.poll(wgpu::Maintain::Wait); + rx.recv().expect("map").expect("map ok"); + + let data = slice.get_mapped_range(); + let off = ((h / 2) * padded + (w / 2) * 4) as usize; + let px = [data[off], data[off + 1], data[off + 2], data[off + 3]]; + drop(data); + buf.unmap(); + px + } + + #[test] + fn a_neutral_graph_produces_a_compilable_shader() { + // The first thing that could go wrong with codegen: the empty case. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + let shader = EditGraph::default_chain().compose(); + + pass.render(&img, &shader, 16, 16) + .expect("a neutral chain must compile"); + } + + #[test] + fn every_operation_generates_compilable_wgsl() { + // The test that justifies the whole codegen approach. Each operation + // is compiled on its own, so a WGSL error names the operation that + // caused it rather than surfacing only in some combination. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + + // A name paired with the edit that activates one operation. Boxed + // closures rather than a type alias: the list is read top to bottom + // as a table of what is covered. + #[allow(clippy::type_complexity)] + let cases: Vec<(&str, Box)> = vec![ + ( + "white_balance.temperature", + Box::new(|g: &mut EditGraph| { + g.set_param(white_balance::ID, white_balance::TEMPERATURE, 60.0) + }), + ), + ( + "white_balance.tint", + Box::new(|g: &mut EditGraph| { + g.set_param(white_balance::ID, white_balance::TINT, -40.0) + }), + ), + ( + "exposure", + Box::new(|g: &mut EditGraph| g.set_param(exposure::ID, exposure::EXPOSURE, 1.5)), + ), + ( + "highlights", + Box::new(|g: &mut EditGraph| { + g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::HIGHLIGHTS, -70.0) + }), + ), + ( + "shadows", + Box::new(|g: &mut EditGraph| { + g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::SHADOWS, 70.0) + }), + ), + ( + "blacks", + Box::new(|g: &mut EditGraph| { + g.set_param(tone::BLACKS_WHITES_ID, tone::BLACKS, -50.0) + }), + ), + ( + "whites", + Box::new(|g: &mut EditGraph| { + g.set_param(tone::BLACKS_WHITES_ID, tone::WHITES, 50.0) + }), + ), + ( + "brilliance", + Box::new(|g: &mut EditGraph| { + g.set_param(colour::BRILLIANCE_ID, colour::BRILLIANCE, 60.0) + }), + ), + ( + "vibrance", + Box::new(|g: &mut EditGraph| { + g.set_param(colour::VIBRANCE_ID, colour::VIBRANCE, 60.0) + }), + ), + ( + "saturation", + Box::new(|g: &mut EditGraph| { + g.set_param(colour::SATURATION_ID, colour::SATURATION, 60.0) + }), + ), + ]; + + for (name, apply) in cases { + let mut g = EditGraph::default_chain(); + apply(&mut g); + let shader = g.compose(); + pass.render(&img, &shader, 16, 16) + .unwrap_or_else(|e| panic!("{name} generated invalid WGSL:\n{e}")); + } + } + + #[test] + fn the_whole_chain_at_once_compiles() { + // Individually-valid fragments can still collide when combined — + // duplicate helpers, clashing locals, a malformed uniform block. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + + let mut g = EditGraph::default_chain(); + g.set_param(white_balance::ID, white_balance::TEMPERATURE, 30.0); + g.set_param(white_balance::ID, white_balance::TINT, -20.0); + g.set_param(exposure::ID, exposure::EXPOSURE, 0.8); + g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::HIGHLIGHTS, -60.0); + g.set_param(tone::HIGHLIGHTS_SHADOWS_ID, tone::SHADOWS, 40.0); + g.set_param(tone::BLACKS_WHITES_ID, tone::BLACKS, -25.0); + g.set_param(tone::BLACKS_WHITES_ID, tone::WHITES, 35.0); + g.set_param(colour::BRILLIANCE_ID, colour::BRILLIANCE, 45.0); + g.set_param(colour::VIBRANCE_ID, colour::VIBRANCE, 55.0); + g.set_param(colour::SATURATION_ID, colour::SATURATION, 15.0); + + let shader = g.compose(); + assert_eq!(shader.source.matches("---- ").count(), 7); + pass.render(&img, &shader, 32, 32) + .expect("the full chain must compile"); + } + + #[test] + fn exposure_brightens_the_image() { + // Proves the uniforms actually reach the shader, not merely that it + // compiles. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 2000); + + let neutral = EditGraph::default_chain().compose(); + let before = { + let t = pass.render(&img, &neutral, 16, 16).expect("render"); + read_centre(&ctx, t) + }; + + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 2.0); + let brighter = g.compose(); + let after = { + let t = pass.render(&img, &brighter, 16, 16).expect("render"); + read_centre(&ctx, t) + }; + + assert!( + after[0] > before[0], + "+2 stops should brighten: {before:?} -> {after:?}" + ); + } + + #[test] + fn negative_exposure_darkens_the_image() { + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 8000); + + let neutral = EditGraph::default_chain().compose(); + let before = { + let t = pass.render(&img, &neutral, 16, 16).expect("render"); + read_centre(&ctx, t) + }; + + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, -2.0); + let darker = g.compose(); + let after = { + let t = pass.render(&img, &darker, 16, 16).expect("render"); + read_centre(&ctx, t) + }; + + assert!(after[0] < before[0], "-2 stops should darken"); + } + + #[test] + fn full_negative_saturation_produces_grey() { + // A neutral grey source cannot show this, so use a coloured one: + // a strongly red-weighted image must come out with equal channels. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + + let size = 16u32; + let mut data = vec![0u16; (size * size) as usize]; + for y in 0..size { + for x in 0..size { + // RGGB: make red photosites bright, others dim. + let c = CfaPattern::Rggb.colour_at(x, y); + data[(y * size + x) as usize] = if c == 0 { 12000 } else { 3000 }; + } + } + let raw = RawImage { + width: size, + height: size, + data, + cfa_pattern: CfaPattern::Rggb, + black_level: [0; 4], + white_level: 16383, + wb_coeffs: [1.0, 1.0, 1.0, 1.0], + color_matrix: Some([1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]), + crop: CropRect { + x: 0, + y: 0, + width: size, + height: size, + }, + }; + let img = Demosaicer::new(&ctx) + .expect("demosaicer") + .run(&raw) + .expect("demosaic"); + + let mut g = EditGraph::default_chain(); + g.set_param(colour::SATURATION_ID, colour::SATURATION, -100.0); + let shader = g.compose(); + let px = { + let t = pass.render(&img, &shader, 16, 16).expect("render"); + read_centre(&ctx, t) + }; + + let spread = px[0].abs_diff(px[1]).max(px[1].abs_diff(px[2])); + assert!( + spread <= 2, + "-100 saturation must produce grey, got {px:?} (spread {spread})" + ); + } + + #[test] + fn moving_a_slider_does_not_recompile() { + // The property the pipeline cache exists for. Recompiling per frame + // would make slider interaction unusable regardless of shader cost. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + + let mut g = EditGraph::default_chain(); + for i in 1..=10 { + g.set_param(exposure::ID, exposure::EXPOSURE, i as f32 * 0.2); + let shader = g.compose(); + pass.render(&img, &shader, 16, 16).expect("render"); + } + + assert_eq!( + pass.cached_pipelines(), + 1, + "ten slider positions must share one compiled pipeline" + ); + } + + #[test] + fn a_different_operation_set_compiles_its_own_pipeline() { + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 1.0); + pass.render(&img, &g.compose(), 16, 16).expect("render"); + assert_eq!(pass.cached_pipelines(), 1); + + g.set_param(colour::SATURATION_ID, colour::SATURATION, 40.0); + pass.render(&img, &g.compose(), 16, 16).expect("render"); + assert_eq!(pass.cached_pipelines(), 2); + + // Returning to the earlier state must reuse, not compile a third. + g.set_param(colour::SATURATION_ID, colour::SATURATION, 0.0); + pass.render(&img, &g.compose(), 16, 16).expect("render"); + assert_eq!(pass.cached_pipelines(), 2); + } + + #[test] + fn output_is_opaque_everywhere() { + // A zero alpha would composite as an invisible image, which reads as + // "nothing rendered" rather than as a bug in this pass. + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + let shader = EditGraph::default_chain().compose(); + let t = pass.render(&img, &shader, 16, 16).expect("render"); + assert_eq!(read_centre(&ctx, t)[3], 255); + } + + #[test] + fn the_output_resizes_with_the_viewport() { + let Some(ctx) = ctx() else { return }; + let mut pass = AdjustPass::new(&ctx); + let img = grey_image(&ctx, 4000); + let shader = EditGraph::default_chain().compose(); + + let t = pass.render(&img, &shader, 64, 48).expect("render"); + assert_eq!((t.width(), t.height()), (64, 48)); + + let t = pass.render(&img, &shader, 32, 96).expect("render"); + assert_eq!((t.width(), t.height()), (32, 96)); + } +} diff --git a/core/dr-gpu/src/demosaic.rs b/core/dr-gpu/src/demosaic.rs new file mode 100644 index 0000000..fd97207 --- /dev/null +++ b/core/dr-gpu/src/demosaic.rs @@ -0,0 +1,749 @@ +//! Raw upload, black/white normalisation, and Bayer demosaic. +//! +//! The first real pipeline stage (ARCH §5.2). It takes CFA sensor data from +//! `dr-decode`, uploads it once, and produces a linear scene-referred +//! RGBA16Float texture in *camera* colour space. Everything downstream — the +//! camera matrix, white balance, the tone operations — works on that texture +//! and never sees the CFA pattern. +//! +//! Uploaded once per image, not per frame. Moving a slider re-runs the adjust +//! pass over this texture; it does not re-demosaic, which is what keeps the +//! interaction budget (NFR-P9) reachable on a 24 MP file. + +use dr_decode::{CfaPattern, RawImage}; +use wgpu::util::DeviceExt; + +use crate::{GpuContext, GpuError}; + +/// Uniform block for the demosaic pass. Layout must match `demosaic.wgsl`. +#[repr(C)] +#[derive(Copy, Clone, Debug, bytemuck::Pod, bytemuck::Zeroable)] +struct DemosaicParams { + width: u32, + height: u32, + crop_x: u32, + crop_y: u32, + stride: u32, + pattern: u32, + _pad0: u32, + _pad1: u32, + black: [f32; 4], + inv_range: [f32; 4], +} + +/// A demosaiced image living on the GPU. +/// +/// Linear, scene-referred, camera colour space, RGBA16Float. This is the +/// input every adjustment operates on, and the reason the ops need no +/// knowledge of sensors or CFA patterns. +pub struct DemosaicedImage { + texture: wgpu::Texture, + view: wgpu::TextureView, + width: u32, + height: u32, + /// Carried through for the camera→sRGB transform in the adjust pass. + color_matrix: [f32; 9], + /// As-shot white balance, the neutral starting point for the WB control. + as_shot_wb: [f32; 3], +} + +impl DemosaicedImage { + pub const FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba16Float; + + pub fn texture(&self) -> &wgpu::Texture { + &self.texture + } + + pub fn view(&self) -> &wgpu::TextureView { + &self.view + } + + pub fn size(&self) -> (u32, u32) { + (self.width, self.height) + } + + /// Camera RGB → linear sRGB, row-major. Identity where the body is + /// uncalibrated, so the image renders uncalibrated rather than black. + pub fn color_matrix(&self) -> [f32; 9] { + self.color_matrix + } + + /// As-shot white balance multipliers, green-normalised. + /// + /// The white balance control is expressed *relative* to these, so its + /// neutral position reproduces what the camera chose. + pub fn as_shot_wb(&self) -> [f32; 3] { + self.as_shot_wb + } +} + +/// Runs the demosaic pass. Holds the pipeline so repeated images reuse it. +pub struct Demosaicer { + ctx: GpuContext, + pipeline: wgpu::ComputePipeline, + bind_group_layout: wgpu::BindGroupLayout, +} + +impl Demosaicer { + pub fn new(ctx: &GpuContext) -> Result { + let shader = ctx + .device + .create_shader_module(wgpu::ShaderModuleDescriptor { + label: Some("demosaic"), + source: wgpu::ShaderSource::Wgsl(include_str!("shaders/demosaic.wgsl").into()), + }); + + let bind_group_layout = + ctx.device + .create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor { + label: Some("demosaic-bgl"), + entries: &[ + // Raw samples, packed two u16 per u32. + wgpu::BindGroupLayoutEntry { + binding: 0, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::Buffer { + ty: wgpu::BufferBindingType::Storage { read_only: true }, + has_dynamic_offset: false, + min_binding_size: None, + }, + count: None, + }, + wgpu::BindGroupLayoutEntry { + binding: 1, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::Buffer { + ty: wgpu::BufferBindingType::Uniform, + has_dynamic_offset: false, + min_binding_size: None, + }, + count: None, + }, + wgpu::BindGroupLayoutEntry { + binding: 2, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::StorageTexture { + access: wgpu::StorageTextureAccess::WriteOnly, + format: DemosaicedImage::FORMAT, + view_dimension: wgpu::TextureViewDimension::D2, + }, + count: None, + }, + ], + }); + + let layout = ctx + .device + .create_pipeline_layout(&wgpu::PipelineLayoutDescriptor { + label: Some("demosaic-layout"), + bind_group_layouts: &[&bind_group_layout], + push_constant_ranges: &[], + }); + + let pipeline = ctx + .device + .create_compute_pipeline(&wgpu::ComputePipelineDescriptor { + label: Some("demosaic-pipeline"), + layout: Some(&layout), + module: &shader, + entry_point: Some("main"), + compilation_options: Default::default(), + cache: None, + }); + + Ok(Self { + ctx: ctx.clone(), + pipeline, + bind_group_layout, + }) + } + + /// Upload and demosaic one image. + pub fn run(&self, raw: &RawImage) -> Result { + if raw.cfa_pattern.is_xtrans() { + // Better to say so than to render a maze of artefacts that reads + // as a corrupt file (FR-RAW-5). + return Err(GpuError::UnsupportedCfa( + "X-Trans demosaic not yet implemented".into(), + )); + } + let pattern = match raw.cfa_pattern { + CfaPattern::Rggb => 0u32, + CfaPattern::Bggr => 1, + CfaPattern::Grbg => 2, + CfaPattern::Gbrg => 3, + other => { + return Err(GpuError::UnsupportedCfa(format!("{other:?}"))); + } + }; + + let (width, height) = (raw.crop.width.max(1), raw.crop.height.max(1)); + + let limits = self.ctx.device.limits(); + if width > limits.max_texture_dimension_2d || height > limits.max_texture_dimension_2d { + return Err(GpuError::TooLarge(format!( + "{width}×{height} exceeds the device limit of {}", + limits.max_texture_dimension_2d + ))); + } + + // Pack the u16 samples two per u32. WGSL has no u16 storage type, so + // unpacking happens in the shader. + let packed = pack_samples(&raw.data); + let raw_buf = self + .ctx + .device + .create_buffer_init(&wgpu::util::BufferInitDescriptor { + label: Some("raw-samples"), + contents: bytemuck::cast_slice(&packed), + usage: wgpu::BufferUsages::STORAGE, + }); + + let params = DemosaicParams { + width, + height, + crop_x: raw.crop.x, + crop_y: raw.crop.y, + stride: raw.width, + pattern, + _pad0: 0, + _pad1: 0, + black: black_per_cell(raw), + inv_range: inv_range_per_cell(raw), + }; + let params_buf = self + .ctx + .device + .create_buffer_init(&wgpu::util::BufferInitDescriptor { + label: Some("demosaic-params"), + contents: bytemuck::bytes_of(¶ms), + usage: wgpu::BufferUsages::UNIFORM, + }); + + let texture = self.ctx.device.create_texture(&wgpu::TextureDescriptor { + label: Some("demosaiced"), + size: wgpu::Extent3d { + width, + height, + depth_or_array_layers: 1, + }, + mip_level_count: 1, + sample_count: 1, + dimension: wgpu::TextureDimension::D2, + format: DemosaicedImage::FORMAT, + // STORAGE to write here, TEXTURE_BINDING so the adjust pass can + // sample it. COPY_SRC only for tests. + usage: wgpu::TextureUsages::STORAGE_BINDING + | wgpu::TextureUsages::TEXTURE_BINDING + | wgpu::TextureUsages::COPY_SRC, + view_formats: &[], + }); + let view = texture.create_view(&Default::default()); + + let bind_group = self + .ctx + .device + .create_bind_group(&wgpu::BindGroupDescriptor { + label: Some("demosaic-bg"), + layout: &self.bind_group_layout, + entries: &[ + wgpu::BindGroupEntry { + binding: 0, + resource: raw_buf.as_entire_binding(), + }, + wgpu::BindGroupEntry { + binding: 1, + resource: params_buf.as_entire_binding(), + }, + wgpu::BindGroupEntry { + binding: 2, + resource: wgpu::BindingResource::TextureView(&view), + }, + ], + }); + + let mut enc = self + .ctx + .device + .create_command_encoder(&wgpu::CommandEncoderDescriptor { + label: Some("demosaic-encoder"), + }); + { + let mut pass = enc.begin_compute_pass(&wgpu::ComputePassDescriptor { + label: Some("demosaic-pass"), + timestamp_writes: None, + }); + pass.set_pipeline(&self.pipeline); + pass.set_bind_group(0, &bind_group, &[]); + pass.dispatch_workgroups(width.div_ceil(8), height.div_ceil(8), 1); + } + self.ctx.queue.submit(Some(enc.finish())); + + Ok(DemosaicedImage { + texture, + view, + width, + height, + // Identity where the body is uncalibrated: the image renders with + // no colour transform rather than not at all. + color_matrix: raw.color_matrix.unwrap_or(IDENTITY_3X3), + as_shot_wb: [raw.wb_coeffs[0], raw.wb_coeffs[1], raw.wb_coeffs[2]], + }) + } +} + +const IDENTITY_3X3: [f32; 9] = [1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0]; + +/// Pack u16 samples two per u32, little-endian within the word. +/// +/// WGSL has no 16-bit storage type without an optional feature, so the shader +/// unpacks. An odd sample count pads with a zero, which is never addressed: +/// the shader indexes by pixel, not by word. +fn pack_samples(data: &[u16]) -> Vec { + let mut out = Vec::with_capacity(data.len().div_ceil(2)); + let mut chunks = data.chunks_exact(2); + for pair in &mut chunks { + out.push(u32::from(pair[0]) | (u32::from(pair[1]) << 16)); + } + if let Some(&last) = chunks.remainder().first() { + out.push(u32::from(last)); + } + out +} + +/// Black level per CFA cell position, indexed `(y & 1) * 2 + (x & 1)`. +/// +/// `dr-decode` reports four levels in CFA order, which is already this +/// layout. Bodies reporting a single level get it broadcast. +fn black_per_cell(raw: &RawImage) -> [f32; 4] { + let b = raw.black_level; + if b[1] == 0 && b[2] == 0 && b[3] == 0 { + return [f32::from(b[0]); 4]; + } + [ + f32::from(b[0]), + f32::from(b[1]), + f32::from(b[2]), + f32::from(b[3]), + ] +} + +/// Reciprocal of the usable range per cell, so the shader avoids a division. +/// +/// A white level at or below black would divide by zero; such a file is +/// malformed, and falling back to full scale renders something inspectable +/// rather than a NaN texture. +fn inv_range_per_cell(raw: &RawImage) -> [f32; 4] { + let white = f32::from(raw.white_level); + let black = black_per_cell(raw); + let mut out = [0.0f32; 4]; + for (i, &b) in black.iter().enumerate() { + let range = white - b; + out[i] = if range > 1.0 { + 1.0 / range + } else { + 1.0 / 65535.0 + }; + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + use dr_decode::CropRect; + + fn raw_for(black: [u16; 4], white: u16) -> RawImage { + RawImage { + width: 4, + height: 4, + data: vec![0; 16], + cfa_pattern: CfaPattern::Rggb, + black_level: black, + white_level: white, + wb_coeffs: [1.0, 1.0, 1.0, 1.0], + color_matrix: None, + crop: CropRect { + x: 0, + y: 0, + width: 4, + height: 4, + }, + } + } + + #[test] + fn samples_pack_two_per_word() { + let packed = pack_samples(&[0x1234, 0xABCD]); + assert_eq!(packed, vec![0xABCD_1234]); + } + + #[test] + fn an_odd_sample_count_does_not_lose_the_last_value() { + // A sensor with an odd sample count would otherwise drop its final + // photosite, or worse, read past the buffer. + let packed = pack_samples(&[0x0001, 0x0002, 0x0003]); + assert_eq!(packed.len(), 2); + assert_eq!(packed[1] & 0xFFFF, 3); + } + + #[test] + fn packing_preserves_every_sample() { + let data: Vec = (0..64).map(|i| i * 1000).collect(); + let packed = pack_samples(&data); + for (i, &expected) in data.iter().enumerate() { + let word = packed[i / 2]; + let got = if i % 2 == 0 { + word & 0xFFFF + } else { + word >> 16 + }; + assert_eq!(got as u16, expected, "sample {i}"); + } + } + + #[test] + fn a_single_black_level_is_broadcast_to_every_cell() { + // Many bodies report one level rather than four; treating the absent + // three as zero would leave three quarters of the image lifted. + let raw = raw_for([512, 0, 0, 0], 16383); + assert_eq!(black_per_cell(&raw), [512.0; 4]); + } + + #[test] + fn per_cell_black_levels_are_kept_distinct() { + let raw = raw_for([2047, 2048, 2048, 2049], 15070); + assert_eq!(black_per_cell(&raw), [2047.0, 2048.0, 2048.0, 2049.0]); + } + + #[test] + fn normalisation_maps_white_to_one() { + let raw = raw_for([2048, 2048, 2048, 2048], 15070); + let inv = inv_range_per_cell(&raw); + let normalised = (15070.0 - 2048.0) * inv[0]; + assert!( + (normalised - 1.0).abs() < 1e-5, + "the white level must land on 1.0, got {normalised}" + ); + } + + #[test] + fn a_degenerate_range_does_not_divide_by_zero() { + // A malformed file reporting white <= black must not produce NaN + // across the whole texture. + let raw = raw_for([5000, 5000, 5000, 5000], 4000); + let inv = inv_range_per_cell(&raw); + assert!(inv.iter().all(|v| v.is_finite() && *v > 0.0)); + } + + // ---- GPU tests ----------------------------------------------------- + // + // These exercise the shader itself. The CPU tests above cover the + // parameter maths; only running the pass proves the kernels and the CFA + // indexing are right. + + fn ctx() -> Option { + match pollster::block_on(GpuContext::new_headless()) { + Ok(c) => Some(c), + Err(e) => { + eprintln!("skipping: no GPU adapter ({e})"); + None + } + } + } + + /// Build a synthetic CFA image of a uniform colour. + /// + /// Each photosite carries its own channel's value, which is what a + /// sensor looking at a flat patch would record. A correct demosaic must + /// return that colour at every pixel. + fn flat_cfa(pattern: CfaPattern, size: u32, rgb: [u16; 3], black: u16, white: u16) -> RawImage { + let mut data = vec![0u16; (size * size) as usize]; + for y in 0..size { + for x in 0..size { + let c = pattern.colour_at(x, y) as usize; + data[(y * size + x) as usize] = black + rgb[c]; + } + } + RawImage { + width: size, + height: size, + data, + cfa_pattern: pattern, + black_level: [black; 4], + white_level: white, + wb_coeffs: [1.0, 1.0, 1.0, 1.0], + color_matrix: None, + crop: CropRect { + x: 0, + y: 0, + width: size, + height: size, + }, + } + } + + /// Read back the demosaiced texture as f32 RGBA. + fn read_rgba(ctx: &GpuContext, img: &DemosaicedImage) -> Vec<[f32; 4]> { + let (w, h) = img.size(); + let unpadded = w * 8; // RGBA16Float = 8 bytes per pixel + let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT; + let padded = unpadded.div_ceil(align) * align; + + let buf = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("demosaic-readback"), + size: (padded * h) as u64, + usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ, + mapped_at_creation: false, + }); + + let mut enc = ctx.device.create_command_encoder(&Default::default()); + enc.copy_texture_to_buffer( + wgpu::ImageCopyTexture { + texture: img.texture(), + mip_level: 0, + origin: wgpu::Origin3d::ZERO, + aspect: wgpu::TextureAspect::All, + }, + wgpu::ImageCopyBuffer { + buffer: &buf, + layout: wgpu::ImageDataLayout { + offset: 0, + bytes_per_row: Some(padded), + rows_per_image: Some(h), + }, + }, + wgpu::Extent3d { + width: w, + height: h, + depth_or_array_layers: 1, + }, + ); + ctx.queue.submit(Some(enc.finish())); + + let slice = buf.slice(..); + let (tx, rx) = std::sync::mpsc::channel(); + slice.map_async(wgpu::MapMode::Read, move |r| { + let _ = tx.send(r); + }); + ctx.device.poll(wgpu::Maintain::Wait); + rx.recv().expect("map").expect("map ok"); + + let data = slice.get_mapped_range(); + let mut out = Vec::with_capacity((w * h) as usize); + for y in 0..h { + let row = (y * padded) as usize; + for x in 0..w { + let px = row + (x * 8) as usize; + let mut c = [0.0f32; 4]; + for (i, slot) in c.iter_mut().enumerate() { + let o = px + i * 2; + let bits = u16::from_le_bytes([data[o], data[o + 1]]); + *slot = half_to_f32(bits); + } + out.push(c); + } + } + drop(data); + buf.unmap(); + out + } + + /// Decode an IEEE 754 binary16 value. + fn half_to_f32(bits: u16) -> f32 { + let sign = f32::from_bits(u32::from(bits & 0x8000) << 16); + let exp = (bits >> 10) & 0x1F; + let mant = bits & 0x03FF; + let magnitude = match exp { + 0 => f32::from(mant) * 2f32.powi(-24), + 0x1F => { + if mant == 0 { + f32::INFINITY + } else { + f32::NAN + } + } + _ => (1.0 + f32::from(mant) / 1024.0) * 2f32.powi(i32::from(exp) - 15), + }; + magnitude.copysign(sign) + } + + #[test] + fn a_flat_patch_demosaics_to_that_colour() { + // The fundamental correctness property: a sensor looking at a uniform + // colour must reconstruct that colour everywhere. Any error in the + // kernels, the CFA indexing, or the normalisation breaks it. + let Some(ctx) = ctx() else { return }; + let d = Demosaicer::new(&ctx).expect("demosaicer"); + + let black = 2048u16; + let white = 16383u16; + let range = f32::from(white - black); + // A colour with three clearly distinct channels, so a swap is loud. + let rgb = [8000u16, 4000, 2000]; + let expected = [ + f32::from(rgb[0]) / range, + f32::from(rgb[1]) / range, + f32::from(rgb[2]) / range, + ]; + + let raw = flat_cfa(CfaPattern::Rggb, 32, rgb, black, white); + let img = d.run(&raw).expect("demosaic"); + let px = read_rgba(&ctx, &img); + + // Interior pixels only: the border reflects, and a 2-pixel margin is + // where that shows. + let (w, _) = img.size(); + for y in 2..30u32 { + for x in 2..30u32 { + let p = px[(y * w + x) as usize]; + for (ch, &want) in expected.iter().enumerate() { + assert!( + (p[ch] - want).abs() < 0.01, + "pixel ({x},{y}) channel {ch}: got {}, want {want}", + p[ch] + ); + } + } + } + } + + #[test] + fn every_bayer_layout_reconstructs_the_same_colour() { + // The packed CFA constants in the shader are easy to get wrong — two + // of the four were wrong on the first attempt, and a wrong one swaps + // red and blue. Each layout describes the same scene, so each must + // produce the same output. + let Some(ctx) = ctx() else { return }; + let d = Demosaicer::new(&ctx).expect("demosaicer"); + + let (black, white) = (0u16, 16383u16); + let rgb = [9000u16, 5000, 1500]; + let expected = [ + f32::from(rgb[0]) / f32::from(white), + f32::from(rgb[1]) / f32::from(white), + f32::from(rgb[2]) / f32::from(white), + ]; + + for pattern in [ + CfaPattern::Rggb, + CfaPattern::Bggr, + CfaPattern::Grbg, + CfaPattern::Gbrg, + ] { + let raw = flat_cfa(pattern, 32, rgb, black, white); + let img = d.run(&raw).expect("demosaic"); + let px = read_rgba(&ctx, &img); + let (w, _) = img.size(); + + let p = px[(16 * w + 16) as usize]; + for (ch, &want) in expected.iter().enumerate() { + assert!( + (p[ch] - want).abs() < 0.01, + "{pattern:?} channel {ch}: got {}, want {want} — \ + a wrong CFA constant swaps channels", + p[ch] + ); + } + } + } + + #[test] + fn an_odd_crop_origin_still_reconstructs_correctly() { + // The case CropRect::shifts_cfa_phase exists for. Cropping to an odd + // origin re-phases the pattern; if the shader is given the unshifted + // one, red and blue swap. + let Some(ctx) = ctx() else { return }; + let d = Demosaicer::new(&ctx).expect("demosaicer"); + + let (black, white) = (0u16, 16383u16); + let rgb = [9000u16, 5000, 1500]; + let mut raw = flat_cfa(CfaPattern::Rggb, 34, rgb, black, white); + + // Crop one photosite in on both axes, as a body with an odd active + // area would. The visible top-left is now green-on-a-red-row. + raw.crop = CropRect { + x: 1, + y: 1, + width: 32, + height: 32, + }; + let (dx, dy) = raw.crop.shifts_cfa_phase(); + assert!(dx && dy, "the fixture must actually shift the phase"); + raw.cfa_pattern = raw.cfa_pattern.shifted(dx, dy); + + let img = d.run(&raw).expect("demosaic"); + let px = read_rgba(&ctx, &img); + let (w, _) = img.size(); + let p = px[(16 * w + 16) as usize]; + + let expected = [ + f32::from(rgb[0]) / f32::from(white), + f32::from(rgb[1]) / f32::from(white), + f32::from(rgb[2]) / f32::from(white), + ]; + for (ch, &want) in expected.iter().enumerate() { + assert!( + (p[ch] - want).abs() < 0.01, + "channel {ch}: got {}, want {want} — the crop origin \ + re-phases the CFA and the shader must see the shifted pattern", + p[ch] + ); + } + } + + #[test] + fn output_is_free_of_nan_and_negatives() { + // f16 NaN propagates silently through every later stage; a negative + // value breaks the ratio-based operations downstream. + let Some(ctx) = ctx() else { return }; + let d = Demosaicer::new(&ctx).expect("demosaicer"); + + // A high-contrast checkerboard, which is where the gradient + // correction overshoots hardest. + let size = 32u32; + let mut data = vec![0u16; (size * size) as usize]; + for y in 0..size { + for x in 0..size { + data[(y * size + x) as usize] = if (x / 2 + y / 2) % 2 == 0 { 16000 } else { 40 }; + } + } + let raw = RawImage { + width: size, + height: size, + data, + cfa_pattern: CfaPattern::Rggb, + black_level: [32; 4], + white_level: 16383, + wb_coeffs: [1.0, 1.0, 1.0, 1.0], + color_matrix: None, + crop: CropRect { + x: 0, + y: 0, + width: size, + height: size, + }, + }; + + let img = d.run(&raw).expect("demosaic"); + for (i, p) in read_rgba(&ctx, &img).iter().enumerate() { + for (ch, v) in p.iter().take(3).enumerate() { + assert!(v.is_finite(), "pixel {i} channel {ch} is {v}"); + assert!(*v >= 0.0, "pixel {i} channel {ch} is negative: {v}"); + } + } + } + + #[test] + fn xtrans_is_refused_rather_than_rendered_wrong() { + let Some(ctx) = ctx() else { return }; + let d = Demosaicer::new(&ctx).expect("demosaicer"); + let mut raw = flat_cfa(CfaPattern::Rggb, 8, [100, 100, 100], 0, 1000); + raw.cfa_pattern = CfaPattern::XTrans; + + assert!( + matches!(d.run(&raw), Err(GpuError::UnsupportedCfa(_))), + "an X-Trans file must report the gap, not render artefacts" + ); + } +} diff --git a/core/dr-gpu/src/error.rs b/core/dr-gpu/src/error.rs index 138c2e7..b3cc8ed 100644 --- a/core/dr-gpu/src/error.rs +++ b/core/dr-gpu/src/error.rs @@ -21,4 +21,13 @@ pub enum GpuError { #[error("readback failed: {0}")] Readback(String), + + /// The X-Trans demosaic is not implemented (FR-RAW-5). Reported rather + /// than approximated with the Bayer path, which would produce a maze of + /// colour artefacts and look like a corrupt file. + #[error("unsupported CFA pattern: {0}")] + UnsupportedCfa(String), + + #[error("image too large for this device: {0}")] + TooLarge(String), } diff --git a/core/dr-gpu/src/lib.rs b/core/dr-gpu/src/lib.rs index b0520cd..58ce16e 100644 --- a/core/dr-gpu/src/lib.rs +++ b/core/dr-gpu/src/lib.rs @@ -11,7 +11,11 @@ use std::sync::Arc; use wgpu::util::DeviceExt; +mod adjust; +mod demosaic; mod error; +pub use adjust::AdjustPass; +pub use demosaic::{DemosaicedImage, Demosaicer}; pub use error::GpuError; /// Owns the wgpu device and queue. diff --git a/core/dr-gpu/src/shaders/demosaic.wgsl b/core/dr-gpu/src/shaders/demosaic.wgsl new file mode 100644 index 0000000..53dd18d --- /dev/null +++ b/core/dr-gpu/src/shaders/demosaic.wgsl @@ -0,0 +1,183 @@ +// Black/white normalisation and Bayer demosaic, in one pass. +// +// Input is the raw sensor readout as packed u16 samples — one per photosite, +// in CFA order. Output is linear scene-referred RGBA16Float in *camera* +// colour space; the camera→sRGB matrix belongs to the adjust pass, so this +// stage is purely about reconstructing three channels from one. +// +// The algorithm is Malvar-He-Cutler (ICASSP 2004): bilinear interpolation +// plus a Laplacian correction taken from the channel that *is* sampled at +// each site. One 5x5 neighbourhood per pixel, and dramatically better than +// bilinear on edges — bilinear leaves visible zippering on any high-contrast +// boundary, which on a 24 MP file is the first thing seen at 1:1. +// +// Kernel coefficients below are the paper's, all over 8. + +struct DemosaicParams { + // Dimensions of the *cropped* output, in pixels. + width: u32, + height: u32, + // Origin of the crop within the sensor readout, in photosites. Added to + // every read so the masked border is never sampled. + crop_x: u32, + crop_y: u32, + // Row stride of the input, in samples. + stride: u32, + // CFA layout of the *cropped* image, already re-phased for the crop + // origin by dr-decode: 0=RGGB, 1=BGGR, 2=GRBG, 3=GBRG. + pattern: u32, + _pad0: u32, + _pad1: u32, + // Per-CFA-position black levels, indexed by (y&1)*2 + (x&1). + black: vec4, + // Reciprocal of (white - black) per position, precomputed on the CPU so + // the shader does no division. + inv_range: vec4, +} + +@group(0) @binding(0) var raw: array; +@group(0) @binding(1) var params: DemosaicParams; +@group(0) @binding(2) var output: texture_storage_2d; + +// Colour of the photosite at (x, y): 0=R, 1=G, 2=B. +// +// Each pattern is its 2x2 cell read row-major, packed two bits per entry so +// the lookup is an index and a shift rather than a branch. +fn colour_at(x: u32, y: u32) -> u32 { + let cell = (y & 1u) * 2u + (x & 1u); + // RGGB = R,G,G,B -> 0,1,1,2 ; BGGR = 2,1,1,0 ; GRBG = 1,0,2,1 ; GBRG = 1,2,0,1 + // Entry i occupies bits [2i, 2i+1], so the cell order reads + // right-to-left in hex. Verified against a table rather than derived by + // eye — two of these were wrong on the first attempt. + var packed: u32; + switch params.pattern { + case 0u: { packed = 0x94u; } // RGGB -> [0,1,1,2] + case 1u: { packed = 0x16u; } // BGGR -> [2,1,1,0] + case 2u: { packed = 0x61u; } // GRBG -> [1,0,2,1] + default: { packed = 0x49u; } // GBRG -> [1,2,0,1] + } + return (packed >> (cell * 2u)) & 3u; +} + +// Whether the row through (x, y) is one carrying red photosites. +// +// Needed at green sites, where red lies along one axis and blue along the +// other, and which is which depends on the pattern. +fn red_is_horizontal(x: u32, y: u32) -> bool { + // The horizontal neighbour of a green site. + return colour_at(x + 1u, y) == 0u; +} + +// Read one photosite, normalised to [0, 1] against its own black level. +// +// Coordinates are relative to the crop origin. A 5x5 window at the image edge +// reflects rather than reading masked photosites or running off the buffer. +fn sample(ix: i32, iy: i32) -> f32 { + let w = i32(params.width); + let h = i32(params.height); + + // Reflect at the borders, preserving CFA parity: reflecting by an even + // distance keeps the mirrored sample the same colour as the one it + // stands in for. Clamping instead would flatten the correction term and + // leave a visible one-pixel seam along each edge. + var cx = ix; + var cy = iy; + if (cx < 0) { cx = -cx; } + if (cy < 0) { cy = -cy; } + if (cx > w - 1) { cx = 2 * (w - 1) - cx; } + if (cy > h - 1) { cy = 2 * (h - 1) - cy; } + cx = clamp(cx, 0, w - 1); + cy = clamp(cy, 0, h - 1); + + let sx = u32(cx) + params.crop_x; + let sy = u32(cy) + params.crop_y; + let index = sy * params.stride + sx; + + // Samples are u16, packed two per u32 word. + let word = raw[index >> 1u]; + let raw_value = select(word & 0xFFFFu, word >> 16u, (index & 1u) == 1u); + + // Black level and range are per CFA position. Subtracting black can go + // negative on sensor noise — real signal below the black point — so the + // result is clamped rather than allowed to wrap. + let cell = (u32(cy) & 1u) * 2u + (u32(cx) & 1u); + let value = (f32(raw_value) - params.black[cell]) * params.inv_range[cell]; + return max(value, 0.0); +} + +@compute @workgroup_size(8, 8, 1) +fn main(@builtin(global_invocation_id) gid: vec3) { + if (gid.x >= params.width || gid.y >= params.height) { + return; + } + + let x = i32(gid.x); + let y = i32(gid.y); + + let c = sample(x, y); + + // 5x5 neighbourhood. + let n1 = sample(x, y - 1); + let s1 = sample(x, y + 1); + let w1 = sample(x - 1, y); + let e1 = sample(x + 1, y); + + let n2 = sample(x, y - 2); + let s2 = sample(x, y + 2); + let w2 = sample(x - 2, y); + let e2 = sample(x + 2, y); + + let nw = sample(x - 1, y - 1); + let ne = sample(x + 1, y - 1); + let sw = sample(x - 1, y + 1); + let se = sample(x + 1, y + 1); + + let axial1 = n1 + s1 + w1 + e1; + let diag1 = nw + ne + sw + se; + let vert2 = n2 + s2; + let horiz2 = w2 + e2; + + let colour = colour_at(gid.x, gid.y); + var rgb: vec3; + + if (colour == 1u) { + // ---- Green site ---------------------------------------------- + // Green is measured. Red and blue are interpolated from their own + // axis, with a correction from the green Laplacian. + // + // Malvar "G at R/B locations" kernels, transposed per axis: + // chroma along the row: (5c + 4(w1+e1) - (nw+ne+sw+se) - (n2+s2) + 0.5(w2+e2)) / 8 + let along_row = + (5.0 * c + 4.0 * (w1 + e1) - diag1 - vert2 + 0.5 * horiz2) * 0.125; + let along_col = + (5.0 * c + 4.0 * (n1 + s1) - diag1 - horiz2 + 0.5 * vert2) * 0.125; + + let red_horizontal = red_is_horizontal(gid.x, gid.y); + let r = select(along_col, along_row, red_horizontal); + let b = select(along_row, along_col, red_horizontal); + rgb = vec3(r, c, b); + } else { + // ---- Red or blue site ---------------------------------------- + // Green at an R/B site: bilinear on the axial neighbours, corrected + // by the centre channel's Laplacian. + // (4c + 2(n1+s1+w1+e1) - (n2+s2+w2+e2)) / 8 + let green = (4.0 * c + 2.0 * axial1 - (vert2 + horiz2)) * 0.125; + + // The opposite chroma sits on the diagonals. + // (6c + 2(nw+ne+sw+se) - 1.5(n2+s2+w2+e2)) / 8 + let opposite = (6.0 * c + 2.0 * diag1 - 1.5 * (vert2 + horiz2)) * 0.125; + + if (colour == 0u) { + rgb = vec3(c, green, opposite); + } else { + rgb = vec3(opposite, green, c); + } + } + + // The correction term can overshoot below zero near clipped highlights. + // Negative light is not meaningful, and carrying it forward makes the + // ratio-based operations downstream (white balance, saturation) misbehave. + rgb = max(rgb, vec3(0.0)); + + textureStore(output, vec2(x, y), vec4(rgb, 1.0)); +} diff --git a/core/dr-pipeline/Cargo.toml b/core/dr-pipeline/Cargo.toml new file mode 100644 index 0000000..ac20719 --- /dev/null +++ b/core/dr-pipeline/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "dr-pipeline" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true + +# No GPU dependency, deliberately. This crate describes operations and +# generates their WGSL; dr-gpu compiles and runs it. Keeping wgpu out means +# the descriptor and codegen logic is testable without a device (ARCH §6.5a). +[dependencies] +dr-types.workspace = true +log.workspace = true diff --git a/core/dr-pipeline/src/descriptor.rs b/core/dr-pipeline/src/descriptor.rs new file mode 100644 index 0000000..b7a9174 --- /dev/null +++ b/core/dr-pipeline/src/descriptor.rs @@ -0,0 +1,215 @@ +//! Parameter descriptors — operations described as data (ARCH §3.3). +//! +//! The core never builds a control. It publishes what its parameters *are*, +//! and `dr-ui` maps each `ParamKind` to a widget appropriate to the current +//! input modality (ARCH §4.3). Adding an operation therefore needs no UI +//! change (FR-DEV-3c). +//! +//! Labels are keys, not strings: resolving them needs a localiser, and +//! `core/` must not depend on one (NFR-A11Y-1). + +use std::fmt; + +/// Identifies a parameter within an operation. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct ParamId(pub &'static str); + +impl fmt::Display for ParamId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.0) + } +} + +/// Identifies an operation kind. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct OpId(pub &'static str); + +impl fmt::Display for OpId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.0) + } +} + +/// A localisation key. The UI resolves it; the core never sees the string. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LocalizedKey(pub &'static str); + +/// What a slider's travel means. +/// +/// Photographic controls are rarely linear in their underlying quantity: +/// exposure is linear in stops but exponential in light, and a temperature +/// slider that is linear in kelvin feels wrong at both ends. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Scale { + Linear, + /// Even *perceptual* steps across the range, for controls whose effect + /// concentrates near one end. + Perceptual, +} + +/// The unit a value carries, for display. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Unit { + None, + /// Exposure value — photographers think in stops, not multipliers. + Stops, + Kelvin, + Percent, +} + +/// The shape of a parameter's value. +#[derive(Debug, Clone, PartialEq)] +pub enum ParamKind { + Scalar { + min: f32, + max: f32, + scale: Scale, + unit: Unit, + precision: u8, + }, + Bool, +} + +/// One parameter of an operation. +#[derive(Debug, Clone, PartialEq)] +pub struct ParamDescriptor { + pub id: ParamId, + pub label: LocalizedKey, + pub kind: ParamKind, + pub default: f32, +} + +impl ParamDescriptor { + /// A scalar in stops — the exposure-like controls. + pub const fn stops(id: &'static str, label: &'static str, min: f32, max: f32) -> Self { + Self { + id: ParamId(id), + label: LocalizedKey(label), + kind: ParamKind::Scalar { + min, + max, + scale: Scale::Linear, + unit: Unit::Stops, + precision: 2, + }, + default: 0.0, + } + } + + /// A symmetric −100…+100 control, the familiar shape for tone and colour + /// adjustments. Neutral at zero, so a double-tap reset is meaningful. + pub const fn amount(id: &'static str, label: &'static str) -> Self { + Self { + id: ParamId(id), + label: LocalizedKey(label), + kind: ParamKind::Scalar { + min: -100.0, + max: 100.0, + scale: Scale::Linear, + unit: Unit::None, + precision: 0, + }, + default: 0.0, + } + } + + /// A general scalar with an explicit range and default. + // + // Eight arguments, and a builder would be the usual answer — but this has + // to be `const` so descriptors can be `static`, and `const` functions + // cannot use a builder's method chain. The two common shapes have their + // own constructors above; this is the escape hatch for the rest. + #[allow(clippy::too_many_arguments)] + pub const fn scalar( + id: &'static str, + label: &'static str, + min: f32, + max: f32, + default: f32, + unit: Unit, + scale: Scale, + precision: u8, + ) -> Self { + Self { + id: ParamId(id), + label: LocalizedKey(label), + kind: ParamKind::Scalar { + min, + max, + scale, + unit, + precision, + }, + default, + } + } + + /// Clamp a value into this parameter's declared range. + /// + /// Applied before the value reaches a shader: a slider dragged past its + /// bounds, or a sidecar written by a newer version with a wider range, + /// must not produce out-of-range uniforms. + pub fn clamp(&self, value: f32) -> f32 { + match self.kind { + ParamKind::Scalar { min, max, .. } => { + if value.is_finite() { + value.clamp(min, max) + } else { + // A NaN from a corrupt sidecar would otherwise poison the + // uniform block and blank the image. + self.default + } + } + ParamKind::Bool => { + if value != 0.0 { + 1.0 + } else { + 0.0 + } + } + } + } +} + +/// The static description of an operation. +#[derive(Debug, Clone, PartialEq)] +pub struct OpDescriptor { + pub id: OpId, + pub label: LocalizedKey, + pub params: &'static [ParamDescriptor], +} + +impl OpDescriptor { + pub fn param(&self, id: ParamId) -> Option<&ParamDescriptor> { + self.params.iter().find(|p| p.id == id) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const P: ParamDescriptor = ParamDescriptor::amount("test", "test.label"); + + #[test] + fn values_clamp_into_range() { + assert_eq!(P.clamp(150.0), 100.0); + assert_eq!(P.clamp(-150.0), -100.0); + assert_eq!(P.clamp(42.0), 42.0); + } + + #[test] + fn a_nan_falls_back_to_the_default_rather_than_poisoning_the_uniform() { + // A corrupt sidecar must not blank the image: one NaN in a uniform + // block propagates through every pixel. + assert_eq!(P.clamp(f32::NAN), P.default); + assert_eq!(P.clamp(f32::INFINITY), P.default); + } + + #[test] + fn amount_controls_are_neutral_at_zero() { + // Double-tap-to-reset and "is this op doing anything" both depend on + // neutral being zero. + assert_eq!(P.default, 0.0); + } +} diff --git a/core/dr-pipeline/src/graph.rs b/core/dr-pipeline/src/graph.rs new file mode 100644 index 0000000..b4b0f69 --- /dev/null +++ b/core/dr-pipeline/src/graph.rs @@ -0,0 +1,426 @@ +//! The edit graph — an ordered set of operations (ARCH §3.4). +//! +//! CPU-side state, deliberately. The GPU device can be lost and rebuilt at any +//! moment on Android (ARCH §6.10), and recovery is only tractable because +//! everything needed to re-render lives here rather than in GPU memory. +//! +//! Order is data, not code: operations run in the sequence this holds them, +//! so reordering the pipeline needs no code change. + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamId, ParamKind}; +use crate::operation::{compose, ComposedShader, Operation}; +use crate::ops; + +/// TRACES: FR-DEV-3a +/// What one operation offers, as plain data. +/// +/// Deliberately owned rather than borrowed, and free of any trait objects: +/// the UI receives a snapshot it can hold across a frame without borrowing +/// the graph, and nothing in it hints at how the operation is implemented. +#[derive(Debug, Clone, PartialEq)] +pub struct OpCapability { + pub id: OpId, + /// A key for the UI's own catalogue. Never a display string — resolving + /// it needs a localiser, which `core/` must not depend on. + pub label: LocalizedKey, + /// Whether this operation currently alters the image. A UI may use it to + /// mark a section as modified, or to offer a per-operation reset. + pub active: bool, + pub params: Vec, +} + +/// TRACES: FR-DEV-3a | FR-DEV-3b +/// What one parameter offers. +/// +/// [`Self::kind`] is what selects the control: the UI maps each `ParamKind` +/// to a widget appropriate to the current input modality (ARCH §4.3), and +/// never switches on the parameter's identity. +#[derive(Debug, Clone, PartialEq)] +pub struct ParamCapability { + pub id: ParamId, + pub label: LocalizedKey, + pub kind: ParamKind, + pub default: f32, + /// The current setting, so the control opens where the edit actually is. + pub value: f32, +} + +impl ParamCapability { + /// Whether this parameter is away from its default. + pub fn is_modified(&self) -> bool { + self.value != self.default + } +} + +/// An ordered pipeline of operations. +pub struct EditGraph { + ops: Vec>, +} + +impl EditGraph { + /// The default develop chain, in pipeline order (ARCH §5.2). + /// + /// Order is not arbitrary. White balance and exposure come first because + /// they are corrections to how the scene was captured, and the tonal + /// operations that follow should act on a correctly exposed image. + /// Colour comes last, so vibrance responds to the tones the user has + /// actually settled on rather than the ones they started with. + pub fn default_chain() -> Self { + Self { + ops: vec![ + Box::new(ops::WhiteBalance::new()), + Box::new(ops::Exposure::new()), + Box::new(ops::HighlightsShadows::new()), + Box::new(ops::BlacksWhites::new()), + Box::new(ops::Brilliance::new()), + Box::new(ops::Vibrance::new()), + Box::new(ops::Saturation::new()), + ], + } + } + + /// Descriptors for every operation, in order. Drives panel generation + /// (FR-DEV-3a). + pub fn descriptors(&self) -> Vec<&'static OpDescriptor> { + self.ops.iter().map(|o| o.descriptor()).collect() + } + + /// TRACES: FR-DEV-3a | FR-DEV-3c + /// Everything a UI needs to build its controls. + /// + /// **This is the only thing the UI should read.** It must not know that + /// exposure exists, that saturation is implemented with a mix, or that + /// any of this becomes a shader — it walks this list and instantiates a + /// control per entry according to the [`ParamKind`]. A new operation + /// therefore appears in the interface with no UI change at all + /// (FR-DEV-3c), and an operation removed from the chain disappears from + /// it just as automatically. + /// + /// Current values are included so the UI has no separate initialisation + /// step, and so reopening an edited image shows where the sliders + /// actually are. + pub fn capabilities(&self) -> Vec { + self.ops + .iter() + .map(|op| { + let desc = op.descriptor(); + OpCapability { + id: desc.id, + label: desc.label, + active: op.is_active(), + params: desc + .params + .iter() + .map(|p| ParamCapability { + id: p.id, + label: p.label, + kind: p.kind.clone(), + default: p.default, + value: op.param(p.id), + }) + .collect(), + } + }) + .collect() + } + + /// Set a parameter, clamping to the descriptor's declared range. + /// + /// Clamping here rather than in each operation means an operation never + /// has to defend against an out-of-range value, and a corrupt sidecar + /// cannot reach a shader. + pub fn set_param(&mut self, op: OpId, param: ParamId, value: f32) { + let Some(operation) = self.ops.iter_mut().find(|o| o.descriptor().id == op) else { + // A sidecar naming an operation this build does not have. The + // rest of the edit must still apply. + log::warn!("unknown operation {op}; ignoring"); + return; + }; + let clamped = match operation.descriptor().param(param) { + Some(d) => d.clamp(value), + None => { + log::warn!("unknown parameter {param} on {op}; ignoring"); + return; + } + }; + operation.set_param(param, clamped); + } + + /// Read a parameter back. + pub fn param(&self, op: OpId, param: ParamId) -> Option { + self.ops + .iter() + .find(|o| o.descriptor().id == op) + .map(|o| o.param(param)) + } + + /// Reset every parameter of every operation to its default. + pub fn reset(&mut self) { + for op in &mut self.ops { + for p in op.descriptor().params { + op.set_param(p.id, p.default); + } + } + } + + /// Whether any operation currently changes the image. + pub fn is_neutral(&self) -> bool { + !self.ops.iter().any(|o| o.is_active()) + } + + /// Generate the fused shader for the current state. + pub fn compose(&self) -> ComposedShader { + compose(&self.ops) + } +} + +impl Default for EditGraph { + fn default() -> Self { + Self::default_chain() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ops::{exposure, white_balance}; + + #[test] + fn a_fresh_graph_is_neutral() { + // Opening an unedited image must produce the image, not an + // interpretation of it. + let g = EditGraph::default_chain(); + assert!(g.is_neutral()); + assert_eq!( + g.compose().source.matches("---- ").count(), + 0, + "a neutral graph must generate no operation blocks" + ); + } + + #[test] + fn the_default_chain_exposes_every_operation() { + let g = EditGraph::default_chain(); + let ids: Vec<&str> = g.descriptors().iter().map(|d| d.id.0).collect(); + for expected in [ + "white_balance", + "exposure", + "highlights_shadows", + "blacks_whites", + "brilliance", + "vibrance", + "saturation", + ] { + assert!(ids.contains(&expected), "{expected} missing from the chain"); + } + } + + #[test] + fn white_balance_and_exposure_precede_the_tonal_operations() { + // Corrections to capture must come before interpretation of tone, or + // the tonal controls act on a wrongly exposed image. + let g = EditGraph::default_chain(); + let ids: Vec<&str> = g.descriptors().iter().map(|d| d.id.0).collect(); + let pos = |id: &str| ids.iter().position(|x| *x == id).expect(id); + assert!(pos("white_balance") < pos("highlights_shadows")); + assert!(pos("exposure") < pos("highlights_shadows")); + assert!(pos("highlights_shadows") < pos("vibrance")); + } + + #[test] + fn setting_a_parameter_activates_its_operation() { + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 1.5); + assert!(!g.is_neutral()); + assert_eq!(g.param(exposure::ID, exposure::EXPOSURE), Some(1.5)); + assert!(g.compose().source.contains("---- exposure ----")); + } + + #[test] + fn values_are_clamped_to_the_descriptor() { + // The guarantee that lets each operation skip range checks. + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 99.0); + assert_eq!(g.param(exposure::ID, exposure::EXPOSURE), Some(5.0)); + } + + #[test] + fn an_unknown_operation_is_ignored_rather_than_panicking() { + // A sidecar from a newer version names operations this build lacks. + // The rest of the edit must still load. + let mut g = EditGraph::default_chain(); + g.set_param(OpId("time_machine"), ParamId("year"), 1994.0); + assert!(g.is_neutral()); + } + + #[test] + fn an_unknown_parameter_is_ignored() { + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, ParamId("nonexistent"), 3.0); + assert!(g.is_neutral()); + } + + #[test] + fn reset_returns_every_operation_to_neutral() { + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 2.0); + g.set_param(white_balance::ID, white_balance::TEMPERATURE, 50.0); + assert!(!g.is_neutral()); + + g.reset(); + assert!(g.is_neutral(), "reset must clear every operation"); + } + + #[test] + fn only_active_operations_reach_the_shader() { + // The composition property, end to end: two adjustments out of seven + // available must generate a shader doing exactly two things. + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 1.0); + g.set_param(white_balance::ID, white_balance::TINT, 25.0); + + let shader = g.compose(); + assert_eq!(shader.source.matches("---- ").count(), 2); + assert!(shader.source.contains("---- exposure ----")); + assert!(shader.source.contains("---- white_balance ----")); + assert!(!shader.source.contains("---- saturation ----")); + } + + #[test] + fn moving_a_slider_does_not_change_the_shader_structure() { + // What makes the pipeline cache worth having: dragging a slider must + // reuse the compiled pipeline and upload uniforms only. + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 1.0); + let first = g.compose(); + + g.set_param(exposure::ID, exposure::EXPOSURE, 2.0); + let second = g.compose(); + + assert_eq!(first.structure_hash, second.structure_hash); + assert_eq!(first.source, second.source); + assert_ne!(first.uniforms, second.uniforms); + } + + #[test] + fn capabilities_describe_every_operation_and_parameter() { + // The UI builds its whole panel from this. Anything missing here is + // something the UI would have to hardcode. + let g = EditGraph::default_chain(); + let caps = g.capabilities(); + assert_eq!(caps.len(), g.descriptors().len()); + + for cap in &caps { + assert!(!cap.params.is_empty(), "{} exposes no parameters", cap.id); + for p in &cap.params { + // A control cannot be built without a range. + match &p.kind { + ParamKind::Scalar { min, max, .. } => { + assert!(min < max, "{}.{} has an empty range", cap.id, p.id); + assert!( + (*min..=*max).contains(&p.default), + "{}.{} default is outside its range", + cap.id, + p.id + ); + } + ParamKind::Bool => {} + } + } + } + } + + #[test] + fn capabilities_report_current_values_not_just_defaults() { + // So reopening an edited image shows the sliders where the edit left + // them, with no separate initialisation path in the UI. + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 1.25); + + let cap = g + .capabilities() + .into_iter() + .find(|c| c.id == exposure::ID) + .expect("exposure is in the chain"); + let p = &cap.params[0]; + assert_eq!(p.value, 1.25); + assert_eq!(p.default, 0.0); + assert!(p.is_modified()); + assert!(cap.active); + } + + #[test] + fn a_fresh_graph_reports_nothing_modified() { + for cap in EditGraph::default_chain().capabilities() { + assert!(!cap.active, "{} should start inactive", cap.id); + for p in &cap.params { + assert!( + !p.is_modified(), + "{}.{} should start at default", + cap.id, + p.id + ); + } + } + } + + #[test] + fn capabilities_survive_a_round_trip_through_set_param() { + // The UI reads a capability, writes the value back, and must get the + // same thing out — no hidden scaling between the two. + let mut g = EditGraph::default_chain(); + for cap in g.capabilities() { + for p in &cap.params { + if let ParamKind::Scalar { max, .. } = p.kind { + let target = max * 0.5; + g.set_param(cap.id, p.id, target); + assert_eq!(g.param(cap.id, p.id), Some(target)); + } + } + } + } + + #[test] + fn adding_an_operation_needs_no_ui_change() { + // FR-DEV-3c, asserted structurally: everything a control needs is + // reachable from the capability list, so a new operation appears + // without the UI naming it. If this test needs editing to add an + // operation, the abstraction has leaked. + let g = EditGraph::default_chain(); + let rendered: Vec = g + .capabilities() + .iter() + .flat_map(|c| { + c.params.iter().map(move |p| match p.kind { + ParamKind::Scalar { + min, + max, + precision, + .. + } => format!( + "{}/{}: slider {min}..{max} @{precision} = {}", + c.label.0, p.label.0, p.value + ), + ParamKind::Bool => format!("{}/{}: switch", c.label.0, p.label.0), + }) + }) + .collect(); + + assert_eq!(rendered.len(), 10, "seven operations, ten parameters"); + assert!(rendered.iter().all(|r| !r.is_empty())); + } + + #[test] + fn enabling_another_operation_does_change_the_structure() { + let mut g = EditGraph::default_chain(); + g.set_param(exposure::ID, exposure::EXPOSURE, 1.0); + let before = g.compose().structure_hash; + + g.set_param( + crate::ops::colour::SATURATION_ID, + crate::ops::colour::SATURATION, + 30.0, + ); + assert_ne!(before, g.compose().structure_hash); + } +} diff --git a/core/dr-pipeline/src/lib.rs b/core/dr-pipeline/src/lib.rs new file mode 100644 index 0000000..db81a8b --- /dev/null +++ b/core/dr-pipeline/src/lib.rs @@ -0,0 +1,175 @@ +//! The develop pipeline — operations, descriptors, and shader composition. +//! +//! # What this crate is +//! +//! An operation here is three things: a **descriptor** saying what its +//! parameters are, a **WGSL fragment** saying what it does to a colour, and +//! the **state** of its current settings. It is not a shader, not a pipeline, +//! and not a control — those belong to `dr-gpu` and `dr-ui` respectively. +//! +//! That split is what lets this crate have no GPU dependency at all: the +//! generated WGSL is a string, and everything about it can be tested without +//! a device (ARCH §6.5a). +//! +//! # Composable shaders +//! +//! The interesting property. Operations are separate in Rust but *fused* on +//! the GPU: [`operation::compose`] concatenates the enabled operations' +//! fragments into one compute shader, so an edit with three active +//! adjustments runs as one dispatch with one texture read and one write. +//! +//! An operation at neutral settings contributes nothing — no code, no +//! uniform, no branch. The shader for a given op-set compiles once and is +//! cached by [`operation::ComposedShader::structure_hash`], which covers the +//! operations and their order but not their values; moving a slider uploads +//! uniforms and reuses the pipeline. +//! +//! # Where this sits +//! +//! Input is the demosaiced texture from `dr-gpu`: linear, scene-referred, +//! camera colour space. Working in linear light is what makes exposure a +//! single multiply and white balance a per-channel scale; on gamma-encoded +//! data neither would be physically meaningful (ARCH §5.2). + +pub mod descriptor; +pub mod graph; +pub mod operation; +pub mod ops; + +pub use descriptor::{ + LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, ParamKind, Scale, Unit, +}; +pub use graph::{EditGraph, OpCapability, ParamCapability}; +pub use operation::{compose, Affects, ComposedShader, Helper, Operation, Uniform}; + +#[cfg(test)] +mod tests { + use super::*; + + /// Every operation in the default chain, activated. + fn fully_active() -> EditGraph { + let mut g = EditGraph::default_chain(); + for desc in g.descriptors() { + for p in desc.params { + // A value away from the default, within range. + let v = match p.kind { + ParamKind::Scalar { max, .. } => max * 0.5, + ParamKind::Bool => 1.0, + }; + g.set_param(desc.id, p.id, v); + } + } + g + } + + #[test] + fn every_operation_can_be_activated_together() { + let g = fully_active(); + assert!(!g.is_neutral()); + let shader = g.compose(); + assert_eq!( + shader.source.matches("---- ").count(), + 7, + "all seven operations should appear" + ); + } + + #[test] + fn the_full_chain_generates_a_well_formed_uniform_block() { + let shader = fully_active().compose(); + assert_eq!( + shader.uniforms.len() % 4, + 0, + "the uniform block must be 16-byte aligned" + ); + assert!(shader.uniforms.iter().all(|v| v.is_finite())); + } + + #[test] + fn no_operation_declares_a_duplicate_id() { + // Two operations sharing an id would collide in the generated + // uniform struct and produce a shader that does not compile. + let g = EditGraph::default_chain(); + let mut ids: Vec<&str> = g.descriptors().iter().map(|d| d.id.0).collect(); + let before = ids.len(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(before, ids.len(), "operation ids must be unique"); + } + + #[test] + fn no_operation_declares_a_duplicate_parameter() { + for desc in EditGraph::default_chain().descriptors() { + let mut ids: Vec<&str> = desc.params.iter().map(|p| p.id.0).collect(); + let before = ids.len(); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(before, ids.len(), "{} has a duplicate parameter", desc.id); + } + } + + #[test] + fn every_default_is_within_its_declared_range() { + // A default outside its own range would mean a fresh image opens + // with a value the UI cannot represent. + for desc in EditGraph::default_chain().descriptors() { + for p in desc.params { + assert_eq!( + p.clamp(p.default), + p.default, + "{}.{} default {} is outside its range", + desc.id, + p.id, + p.default + ); + } + } + } + + #[test] + fn defaults_leave_every_operation_inactive() { + // The invariant behind "opening an image shows the image": every + // operation must read its own default as neutral. + let g = EditGraph::default_chain(); + for desc in g.descriptors() { + for p in desc.params { + assert_eq!( + g.param(desc.id, p.id), + Some(p.default), + "{}.{} does not start at its default", + desc.id, + p.id + ); + } + } + assert!(g.is_neutral()); + } + + #[test] + fn generated_uniform_names_are_valid_wgsl_identifiers() { + let shader = fully_active().compose(); + for line in shader.source.lines() { + let trimmed = line.trim(); + let Some((name, _)) = trimmed.split_once(": f32,") else { + continue; + }; + assert!( + name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') + && !name.starts_with(|c: char| c.is_ascii_digit()), + "{name} is not a valid WGSL identifier" + ); + } + } + + #[test] + fn a_full_chain_declares_each_helper_once() { + // Six of the seven operations want `luminance`. A duplicate function + // definition fails to compile, so this is the property that keeps + // helper sharing safe as operations are added. + let source = fully_active().compose().source; + for helper in ["luminance", "tone_position", "colour_saturation"] { + let count = source.matches(&format!("fn {helper}(")).count(); + assert!(count <= 1, "{helper} declared {count} times"); + } + } +} diff --git a/core/dr-pipeline/src/operation.rs b/core/dr-pipeline/src/operation.rs new file mode 100644 index 0000000..42ff530 --- /dev/null +++ b/core/dr-pipeline/src/operation.rs @@ -0,0 +1,564 @@ +//! The `Operation` trait and WGSL fragment composition. +//! +//! # Composable shaders +//! +//! Each operation contributes a **WGSL fragment**: a function taking a linear +//! RGB colour and returning one. The pipeline concatenates the fragments of +//! the enabled operations into a single generated shader, run as one compute +//! dispatch. This buys the performance of a fused pass without the coupling: +//! +//! - **One texture read and one write per frame**, not one pair per operation. +//! At 24 MP the difference is the whole frame budget. +//! - **Operations stay independent.** Adding one is a new file implementing +//! this trait; no central shader to edit and no ordering table to update. +//! - **A disabled operation vanishes from the source** rather than costing a +//! branch, so an image with two active adjustments compiles to a shader +//! doing exactly two things. +//! - **Each distinct op-set compiles once** and is cached by the hash of its +//! generated source (ARCH §5.6). +//! +//! The cost is that WGSL compile errors point at generated source, so the +//! generator emits readable, commented output — see [`compose`]. + +use std::fmt::Write as _; + +use crate::descriptor::{OpDescriptor, ParamId}; + +/// What an operation's parameters affect, for cache invalidation scoping. +/// +/// Adjusting exposure must not invalidate the demosaic result; this is what +/// lets the tile cache reuse everything up to the first changed stage +/// (ARCH §5.3). +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum Affects { + /// Per-pixel colour only. Everything in this milestone. + Colour, + /// Pixel positions — crop, rotate. Invalidates geometry-dependent caches. + Geometry, +} + +/// A single scalar a fragment reads from the generated uniform block. +/// +/// Operations declare uniforms by name and value; the composer assigns them +/// slots and emits the struct. An operation never knows its own offset, which +/// is what allows fragments to be reordered or omitted freely. +#[derive(Debug, Clone, PartialEq)] +pub struct Uniform { + /// Field name as it appears in WGSL. Prefixed with the op id by the + /// composer, so two operations may both declare `amount`. + pub name: &'static str, + pub value: f32, +} + +/// A develop operation. +/// +/// Object-safe: the pipeline holds `Box` in graph order, so +/// order is data rather than code (ARCH §3.4). +pub trait Operation: Send + Sync { + /// Static description, driving UI generation (FR-DEV-3a). + fn descriptor(&self) -> &'static OpDescriptor; + + /// Set a parameter. Values arrive already clamped to the descriptor. + fn set_param(&mut self, id: ParamId, value: f32); + + /// Read a parameter back, for the sidecar and for the UI's initial state. + fn param(&self, id: ParamId) -> f32; + + /// Whether this operation currently changes the image. + /// + /// An operation at its neutral settings returns `false` and is omitted + /// from the generated shader entirely. This is what makes the common case + /// — a handful of active adjustments out of many available — cost only + /// what is actually used. + fn is_active(&self) -> bool; + + /// The WGSL body of this operation's transform. + /// + /// Receives `c` (a `vec3` of linear RGB) and must produce the + /// result in `c`. Uniforms are addressed by the names declared in + /// [`Self::uniforms`], accessed as `u.`; the composer + /// rewrites them, so a fragment writes the bare name. + /// + /// The fragment runs inside its own block, so locals need no unique + /// names. + fn wgsl_body(&self) -> String; + + /// Uniform values this operation's fragment reads. + fn uniforms(&self) -> Vec; + + /// What this operation's parameters affect. + fn affects(&self) -> Affects { + Affects::Colour + } + + /// Any WGSL helper functions the fragment calls. + /// + /// Emitted once per *distinct* function name even if several operations + /// request it, so shared helpers (luminance, soft clipping) are declared + /// exactly once. + fn helpers(&self) -> &'static [Helper] { + &[] + } +} + +/// A named WGSL helper function, deduplicated across operations. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Helper { + pub name: &'static str, + pub source: &'static str, +} + +/// The result of composing a set of operations into one shader. +#[derive(Debug, Clone, PartialEq)] +pub struct ComposedShader { + /// Complete, compilable WGSL. + pub source: String, + /// Uniform values in the order the generated struct declares them. + pub uniforms: Vec, + /// Identifies this shader's *structure* — the op-set and their order, + /// not their values. Two edits differing only in slider positions share + /// a compiled pipeline and differ only in the uniform upload. + pub structure_hash: u64, +} + +/// Fields the generated uniform struct always carries, before op uniforms. +/// +/// WGSL requires a uniform struct to be non-empty and 16-byte aligned; these +/// are needed by every generated shader in any case. +const BASE_UNIFORM_FIELDS: usize = 16; + +/// Compose enabled operations into a single compute shader. +/// +/// Inactive operations are skipped entirely — they contribute no code, no +/// uniforms, and nothing to the structure hash. +pub fn compose(ops: &[Box]) -> ComposedShader { + let active: Vec<&dyn Operation> = ops + .iter() + .map(|o| o.as_ref()) + .filter(|o| o.is_active()) + .collect(); + + let mut uniform_fields = String::new(); + let mut uniform_values: Vec = Vec::new(); + let mut body = String::new(); + let mut helpers: Vec = Vec::new(); + + // The base block: the camera matrix and output settings every generated + // shader needs. Declared first so their slots are fixed regardless of + // which operations are present. + uniform_fields.push_str( + " // Camera RGB -> linear sRGB. Rows padded to vec4 for std140\n\ + \x20 // alignment; a bare mat3x3 is laid out as three vec4 anyway.\n\ + \x20 cam_to_srgb_0: vec4,\n\ + \x20 cam_to_srgb_1: vec4,\n\ + \x20 cam_to_srgb_2: vec4,\n\ + \x20 // As-shot white balance, the neutral point for the WB control.\n\ + \x20 as_shot_wb: vec4,\n", + ); + uniform_values.resize(BASE_UNIFORM_FIELDS, 0.0); + + for op in &active { + let id = op.descriptor().id.0; + let prefix = sanitise(id); + + // Each op's uniforms are prefixed, so two operations may both declare + // a field called `amount` without colliding. + let op_uniforms = op.uniforms(); + if !op_uniforms.is_empty() { + let _ = writeln!(uniform_fields, " // {id}"); + } + for u in &op_uniforms { + let _ = writeln!(uniform_fields, " {prefix}_{}: f32,", u.name); + uniform_values.push(u.value); + } + + for h in op.helpers() { + if !helpers.iter().any(|existing| existing.name == h.name) { + helpers.push(*h); + } + } + + // Rewrite bare uniform names to their prefixed struct fields, so a + // fragment is written without knowing about any other operation. + let mut fragment = op.wgsl_body(); + for u in &op_uniforms { + fragment = rewrite_uniform(&fragment, u.name, &format!("u.{prefix}_{}", u.name)); + } + + let _ = writeln!(body, "\n // ---- {id} ----"); + let _ = writeln!(body, " {{"); + for line in fragment.lines() { + let _ = writeln!(body, " {line}"); + } + let _ = writeln!(body, " }}"); + } + + // Pad the uniform block to a 16-byte boundary. A struct whose size is not + // a multiple of 16 is rejected by the WGSL uniform address space rules. + let pad = (4 - (uniform_values.len() % 4)) % 4; + for i in 0..pad { + let _ = writeln!(uniform_fields, " _pad{i}: f32,"); + uniform_values.push(0.0); + } + + let mut helper_src = String::new(); + for h in &helpers { + let _ = writeln!(helper_src, "{}\n", h.source.trim_end()); + } + + let source = format!( + "// GENERATED — do not edit. +// +// Composed by dr-pipeline from {} active operation(s). Each block below is +// one operation's fragment, run in graph order over a linear scene-referred +// colour. Operations at neutral settings are omitted rather than branched +// over, so this shader does exactly the work the current edit requires. + +struct Params {{ +{uniform_fields}}} + +@group(0) @binding(0) var source: texture_2d; +@group(0) @binding(1) var u: Params; +@group(0) @binding(2) var output: texture_storage_2d; + +{helper_src}// Linear sRGB to the display transfer function. +// +// The one place quantisation happens: everything above runs in linear f16, +// and this is the final encode (ARCH §5.2). +fn encode_srgb(c: vec3) -> vec3 {{ + let lo = c * 12.92; + let hi = 1.055 * pow(max(c, vec3(0.0031308)), vec3(1.0 / 2.4)) - 0.055; + return select(hi, lo, c <= vec3(0.0031308)); +}} + +@compute @workgroup_size(8, 8, 1) +fn main(@builtin(global_invocation_id) gid: vec3) {{ + let dims = textureDimensions(output); + if (gid.x >= dims.x || gid.y >= dims.y) {{ + return; + }} + + // Source is the demosaiced image: linear, scene-referred, camera space. + let src_dims = textureDimensions(source); + let coord = vec2( + i32(gid.x * src_dims.x / dims.x), + i32(gid.y * src_dims.y / dims.y), + ); + var c = textureLoad(source, coord, 0).rgb; + + // As-shot white balance. Applied unconditionally, before any operation, + // because it is part of *interpreting* the sensor rather than an edit: a + // Bayer sensor's green photosites collect far more signal than its red + // and blue, so raw camera-space values are strongly green and no amount + // of later correction recovers a neutral image from them. The white + // balance operation, when active, applies its own offset on top of this. + c = c * u.as_shot_wb.rgb; +{body} + // Camera space -> linear sRGB. Applied after the adjustments so white + // balance and exposure act on sensor-native values, which is where they + // are physically meaningful. + c = vec3( + dot(u.cam_to_srgb_0.rgb, c), + dot(u.cam_to_srgb_1.rgb, c), + dot(u.cam_to_srgb_2.rgb, c), + ); + + // Clip to the display gamut and encode. + c = clamp(c, vec3(0.0), vec3(1.0)); + textureStore(output, vec2(gid.xy), vec4(encode_srgb(c), 1.0)); +}} +", + active.len() + ); + + let structure_hash = hash_structure(&active); + + ComposedShader { + source, + uniforms: uniform_values, + structure_hash, + } +} + +/// Hash the op-set and order — the structure, not the values. +/// +/// Two edits with the same operations at different slider positions produce +/// the same hash and reuse one compiled pipeline (ARCH §6.13: the hash is +/// over integer state only, so it is exactly deterministic). +fn hash_structure(active: &[&dyn Operation]) -> u64 { + // FNV-1a: no dependency, stable across runs and platforms, which the + // shader cache key requires. + let mut h: u64 = 0xcbf2_9ce4_8422_2325; + for op in active { + for byte in op.descriptor().id.0.as_bytes() { + h ^= u64::from(*byte); + h = h.wrapping_mul(0x100_0000_01b3); + } + // A separator, so ["ab", "c"] and ["a", "bc"] differ. + h ^= 0xff; + h = h.wrapping_mul(0x100_0000_01b3); + } + h +} + +/// Replace whole-word occurrences of `name` with `replacement`. +/// +/// Whole-word matching matters: an operation with uniforms `amount` and +/// `amount_hi` must not have the first rewrite corrupt the second. +fn rewrite_uniform(src: &str, name: &str, replacement: &str) -> String { + let mut out = String::with_capacity(src.len()); + let bytes = src.as_bytes(); + let mut i = 0; + + while i < src.len() { + if src[i..].starts_with(name) { + let before_ok = i == 0 || !is_ident_byte(bytes[i - 1]); + let after = i + name.len(); + let after_ok = after >= src.len() || !is_ident_byte(bytes[after]); + if before_ok && after_ok { + out.push_str(replacement); + i = after; + continue; + } + } + // Push one full character, not one byte, so non-ASCII in a comment + // does not split a UTF-8 sequence. + let ch = src[i..].chars().next().expect("in bounds"); + out.push(ch); + i += ch.len_utf8(); + } + out +} + +fn is_ident_byte(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'_' +} + +/// Make an operation id safe to embed in a WGSL identifier. +fn sanitise(id: &str) -> String { + id.chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::descriptor::{LocalizedKey, OpId, ParamDescriptor}; + + static DESC_A: OpDescriptor = OpDescriptor { + id: OpId("op_a"), + label: LocalizedKey("a"), + params: &[ParamDescriptor::amount("amount", "a.amount")], + }; + static DESC_B: OpDescriptor = OpDescriptor { + id: OpId("op_b"), + label: LocalizedKey("b"), + params: &[ParamDescriptor::amount("amount", "b.amount")], + }; + + struct Fake { + desc: &'static OpDescriptor, + amount: f32, + helper: Option, + } + + impl Operation for Fake { + fn descriptor(&self) -> &'static OpDescriptor { + self.desc + } + fn set_param(&mut self, _id: ParamId, value: f32) { + self.amount = value; + } + fn param(&self, _id: ParamId) -> f32 { + self.amount + } + fn is_active(&self) -> bool { + self.amount != 0.0 + } + fn wgsl_body(&self) -> String { + "c = c * amount;".into() + } + fn uniforms(&self) -> Vec { + vec![Uniform { + name: "amount", + value: self.amount, + }] + } + fn helpers(&self) -> &'static [Helper] { + match self.helper { + Some(_) => SHARED, + None => &[], + } + } + } + + static SHARED: &[Helper] = &[Helper { + name: "luma", + source: "fn luma(c: vec3) -> f32 { return c.g; }", + }]; + + fn fake(desc: &'static OpDescriptor, amount: f32, helper: bool) -> Box { + Box::new(Fake { + desc, + amount, + helper: helper.then_some(SHARED[0]), + }) + } + + #[test] + fn an_inactive_operation_contributes_nothing() { + // The point of composing rather than branching: an op at neutral + // must not appear in the source at all. + let ops = vec![fake(&DESC_A, 0.0, false)]; + let shader = compose(&ops); + assert!( + !shader.source.contains("op_a"), + "a neutral operation must not reach the generated shader" + ); + assert_eq!( + shader.uniforms.len(), + BASE_UNIFORM_FIELDS, + "it must contribute no uniforms either" + ); + } + + #[test] + fn an_active_operation_appears_once() { + let ops = vec![fake(&DESC_A, 2.0, false)]; + let shader = compose(&ops); + assert!(shader.source.contains("---- op_a ----")); + assert!(shader.source.contains("u.op_a_amount")); + } + + #[test] + fn uniforms_are_prefixed_so_operations_cannot_collide() { + // Both fakes declare a uniform called `amount`. Without prefixing, + // the generated struct would have a duplicate field and fail to + // compile — the failure mode that makes naive concatenation fragile. + let ops = vec![fake(&DESC_A, 1.0, false), fake(&DESC_B, 2.0, false)]; + let shader = compose(&ops); + assert!(shader.source.contains("op_a_amount: f32")); + assert!(shader.source.contains("op_b_amount: f32")); + assert!(shader.source.contains("c = c * u.op_a_amount;")); + assert!(shader.source.contains("c = c * u.op_b_amount;")); + } + + #[test] + fn uniform_values_follow_declaration_order() { + let ops = vec![fake(&DESC_A, 1.5, false), fake(&DESC_B, 2.5, false)]; + let shader = compose(&ops); + assert_eq!(shader.uniforms[BASE_UNIFORM_FIELDS], 1.5); + assert_eq!(shader.uniforms[BASE_UNIFORM_FIELDS + 1], 2.5); + } + + #[test] + fn a_shared_helper_is_emitted_once() { + // Two operations wanting the same helper must not produce a + // duplicate function definition. + let ops = vec![fake(&DESC_A, 1.0, true), fake(&DESC_B, 1.0, true)]; + let shader = compose(&ops); + assert_eq!( + shader.source.matches("fn luma(").count(), + 1, + "a helper requested twice must be declared once" + ); + } + + #[test] + fn the_uniform_block_is_16_byte_aligned() { + // WGSL rejects a uniform struct whose size is not a multiple of 16. + for n in 0..6 { + let ops: Vec> = (0..n) + .map(|i| fake(if i % 2 == 0 { &DESC_A } else { &DESC_B }, 1.0, false)) + .collect(); + let shader = compose(&ops); + assert_eq!( + shader.uniforms.len() % 4, + 0, + "{n} operations produced {} floats, not a multiple of 4", + shader.uniforms.len() + ); + } + } + + #[test] + fn structure_hash_ignores_values_but_tracks_the_op_set() { + // The property the shader cache depends on: moving a slider must not + // trigger a recompile, but enabling an operation must. + let a1 = compose(&[fake(&DESC_A, 1.0, false)]).structure_hash; + let a2 = compose(&[fake(&DESC_A, 9.0, false)]).structure_hash; + assert_eq!(a1, a2, "a value change must reuse the compiled pipeline"); + + let both = compose(&[fake(&DESC_A, 1.0, false), fake(&DESC_B, 1.0, false)]); + assert_ne!(a1, both.structure_hash, "a different op-set must recompile"); + } + + #[test] + fn structure_hash_is_order_sensitive() { + // Operation order is data (ARCH §3.4); two orders are different + // shaders and must not share a cache entry. + let ab = compose(&[fake(&DESC_A, 1.0, false), fake(&DESC_B, 1.0, false)]); + let ba = compose(&[fake(&DESC_B, 1.0, false), fake(&DESC_A, 1.0, false)]); + assert_ne!(ab.structure_hash, ba.structure_hash); + } + + #[test] + fn rewriting_respects_word_boundaries() { + // `amount` must not corrupt `amount_hi` — the bug a naive + // string replace would introduce. + let got = rewrite_uniform("x = amount + amount_hi;", "amount", "u.p_amount"); + assert_eq!(got, "x = u.p_amount + amount_hi;"); + } + + #[test] + fn rewriting_leaves_substrings_alone() { + let got = rewrite_uniform("total_amount = 1.0;", "amount", "u.a"); + assert_eq!(got, "total_amount = 1.0;"); + } + + #[test] + fn as_shot_white_balance_is_applied_even_with_no_operations() { + // The bug this catches, seen on a real CR2: a Bayer sensor's green + // photosites collect roughly twice the signal of its red and blue, + // so an image rendered without the as-shot multipliers comes out + // violently green. It must not depend on the white balance operation + // being active — that one carries only the user's offset. + let shader = compose(&[]); + assert!( + shader.source.contains("u.as_shot_wb"), + "a neutral edit must still apply as-shot white balance" + ); + } + + #[test] + fn white_balance_is_applied_before_the_operations() { + // Exposure and the tonal controls act on white-balanced values; if + // the multiply came afterwards, every operation would be reasoning + // about a green-cast image. + let ops = vec![fake(&DESC_A, 2.0, false)]; + let source = compose(&ops).source; + let wb = source.find("u.as_shot_wb").expect("wb applied"); + let op = source.find("---- op_a ----").expect("op present"); + assert!(wb < op, "as-shot white balance must precede the operations"); + } + + #[test] + fn the_camera_matrix_is_applied_after_the_operations() { + // Adjustments are meaningful in sensor-native space, where highlight + // headroom still exists; converting first would clip it away. + let ops = vec![fake(&DESC_A, 2.0, false)]; + let source = compose(&ops).source; + let op = source.find("---- op_a ----").expect("op present"); + let matrix = source.find("u.cam_to_srgb_0").expect("matrix applied"); + assert!(op < matrix, "the camera matrix must come after operations"); + } + + #[test] + fn generated_source_carries_a_do_not_edit_banner() { + // Someone will eventually find this in a debugger and try to fix it + // in place. + let shader = compose(&[fake(&DESC_A, 1.0, false)]); + assert!(shader.source.starts_with("// GENERATED")); + } +} diff --git a/core/dr-pipeline/src/ops/colour.rs b/core/dr-pipeline/src/ops/colour.rs new file mode 100644 index 0000000..bac85f2 --- /dev/null +++ b/core/dr-pipeline/src/ops/colour.rs @@ -0,0 +1,397 @@ +//! Colour operations: vibrance, saturation, and brilliance. +//! +//! # Vibrance versus saturation +//! +//! Saturation scales every colour's distance from grey equally. Vibrance +//! scales it *more for muted colours than for already-saturated ones*, and +//! protects skin tones. The difference matters: pushing saturation on a +//! portrait turns faces orange long before the background improves, which is +//! precisely the problem vibrance was invented to solve. +//! +//! # Brilliance +//! +//! Apple's control, and a genuinely different idea from either: it lifts +//! shadows and pulls highlights *simultaneously*, applying the opposite +//! correction at each end of the range while leaving mid-tones alone. The +//! result reads as "more light in the scene" rather than "less contrast", +//! because local relationships survive where a plain contrast reduction +//! flattens them. +//! +//! It overlaps with highlights/shadows deliberately — one control doing both +//! in a fixed relationship is easier to reach for than two controls needing +//! to be balanced against each other. + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId}; +use crate::operation::{Helper, Operation, Uniform}; +use crate::ops::tone::TONE_HELPERS; + +// --------------------------------------------------------------------------- +// Saturation +// --------------------------------------------------------------------------- + +pub const SATURATION_ID: OpId = OpId("saturation"); +pub const SATURATION: ParamId = ParamId("saturation"); + +static SAT_DESCRIPTOR: OpDescriptor = OpDescriptor { + id: SATURATION_ID, + label: LocalizedKey("op.saturation"), + params: &[ParamDescriptor::amount("saturation", "param.saturation")], +}; + +#[derive(Debug, Default, Clone)] +pub struct Saturation { + amount: f32, +} + +impl Saturation { + pub fn new() -> Self { + Self::default() + } +} + +impl Operation for Saturation { + fn descriptor(&self) -> &'static OpDescriptor { + &SAT_DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + SATURATION => self.amount = value, + _ => log::warn!("saturation: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + SATURATION => self.amount, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.amount != 0.0 + } + + fn wgsl_body(&self) -> String { + "\ +// Interpolate away from the luminance-preserving grey. A factor of 0 is +// monochrome, 1 is unchanged, above 1 is more saturated. +let luma = luminance(c); +c = mix(vec3(luma), c, factor); +c = max(c, vec3(0.0));" + .into() + } + + fn uniforms(&self) -> Vec { + // -100 reaches exactly monochrome; +100 doubles the distance from + // grey. The floor at zero matters: a negative factor would push a + // colour past grey into its complement, inverting hues. + vec![Uniform { + name: "factor", + value: (1.0 + self.amount / 100.0).max(0.0), + }] + } + + fn helpers(&self) -> &'static [Helper] { + TONE_HELPERS + } +} + +// --------------------------------------------------------------------------- +// Vibrance +// --------------------------------------------------------------------------- + +pub const VIBRANCE_ID: OpId = OpId("vibrance"); +pub const VIBRANCE: ParamId = ParamId("vibrance"); + +static VIB_DESCRIPTOR: OpDescriptor = OpDescriptor { + id: VIBRANCE_ID, + label: LocalizedKey("op.vibrance"), + params: &[ParamDescriptor::amount("vibrance", "param.vibrance")], +}; + +#[derive(Debug, Default, Clone)] +pub struct Vibrance { + amount: f32, +} + +impl Vibrance { + pub fn new() -> Self { + Self::default() + } +} + +impl Operation for Vibrance { + fn descriptor(&self) -> &'static OpDescriptor { + &VIB_DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + VIBRANCE => self.amount = value, + _ => log::warn!("vibrance: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + VIBRANCE => self.amount, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.amount != 0.0 + } + + fn wgsl_body(&self) -> String { + "\ +let luma = luminance(c); +let sat = colour_saturation(c); + +// The vibrance curve: full effect on grey, tapering to nothing on colours +// that are already saturated. Squaring the falloff keeps the mid-range +// responsive while still protecting the extremes. +let falloff = (1.0 - sat) * (1.0 - sat); + +// Skin protection. Skin sits in a narrow band of hue where red leads green +// leads blue; pushing it is what makes vibrance look wrong on portraits. +// Detected by channel ordering rather than a hue angle, which costs a +// conversion and buys nothing here. +let is_skin = f32(c.r > c.g && c.g > c.b); +let skin_guard = 1.0 - is_skin * 0.5; + +let strength = amount * falloff * skin_guard; +c = mix(vec3(luma), c, 1.0 + strength); +c = max(c, vec3(0.0));" + .into() + } + + fn uniforms(&self) -> Vec { + vec![Uniform { + name: "amount", + value: self.amount / 100.0, + }] + } + + fn helpers(&self) -> &'static [Helper] { + // Needs both luminance (from tone) and the saturation measure. + // Duplicates across the two lists are deduplicated by the composer. + COLOUR_AND_TONE + } +} + +/// The helper set vibrance needs: luminance plus the saturation measure. +static COLOUR_AND_TONE: &[Helper] = crate::ops::helpers::COLOUR; + +// --------------------------------------------------------------------------- +// Brilliance +// --------------------------------------------------------------------------- + +pub const BRILLIANCE_ID: OpId = OpId("brilliance"); +pub const BRILLIANCE: ParamId = ParamId("brilliance"); + +static BRIL_DESCRIPTOR: OpDescriptor = OpDescriptor { + id: BRILLIANCE_ID, + label: LocalizedKey("op.brilliance"), + params: &[ParamDescriptor::amount("brilliance", "param.brilliance")], +}; + +#[derive(Debug, Default, Clone)] +pub struct Brilliance { + amount: f32, +} + +impl Brilliance { + pub fn new() -> Self { + Self::default() + } +} + +impl Operation for Brilliance { + fn descriptor(&self) -> &'static OpDescriptor { + &BRIL_DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + BRILLIANCE => self.amount = value, + _ => log::warn!("brilliance: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + BRILLIANCE => self.amount, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.amount != 0.0 + } + + fn wgsl_body(&self) -> String { + "\ +let luma = luminance(c); +let pos = tone_position(luma); + +// Opposite corrections at the two ends: shadows up, highlights down, both +// tapering to nothing at the mid-point. This is what separates brilliance +// from a contrast control — mid-tones keep their local relationships, so +// the image gains apparent light rather than losing structure. +let lift = (1.0 - smoothstep(0.0, 0.5, pos)) * amount; +let pull = smoothstep(0.5, 1.0, pos) * amount; + +// A mild saturation compensation. Flattening the tonal range washes colour +// out; without this, brilliance looks faded at useful settings. +let gain = exp2(lift - pull); +c = c * gain; + +let luma_after = luminance(c); +c = mix(vec3(luma_after), c, 1.0 + max(amount, 0.0) * 0.15); +c = max(c, vec3(0.0));" + .into() + } + + fn uniforms(&self) -> Vec { + vec![Uniform { + name: "amount", + // Half a stop at each end at full travel — the two ends move + // apart by a stop in total, which is a strong but not + // destructive flattening. + value: self.amount / 100.0 * 0.5, + }] + } + + fn helpers(&self) -> &'static [Helper] { + TONE_HELPERS + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::operation::compose; + + #[test] + fn all_three_start_neutral() { + assert!(!Saturation::new().is_active()); + assert!(!Vibrance::new().is_active()); + assert!(!Brilliance::new().is_active()); + } + + #[test] + fn full_negative_saturation_reaches_monochrome() { + // The property that makes -100 meaningful: it must land exactly on + // grey, not merely near it. + let mut s = Saturation::new(); + s.set_param(SATURATION, -100.0); + assert_eq!(s.uniforms()[0].value, 0.0); + } + + #[test] + fn positive_saturation_increases_the_factor() { + let mut s = Saturation::new(); + s.set_param(SATURATION, 100.0); + assert!((s.uniforms()[0].value - 2.0).abs() < 1e-6); + } + + #[test] + fn the_saturation_factor_never_goes_negative() { + // A negative factor would invert hues — a colour past monochrome + // becomes its complement, which is never wanted here. + let mut s = Saturation::new(); + s.set_param(SATURATION, -200.0); + assert!(s.uniforms()[0].value >= 0.0); + } + + #[test] + fn vibrance_protects_skin_in_its_fragment() { + // The distinguishing behaviour; if the guard is dropped, portraits + // go orange and the control is indistinguishable from saturation. + let mut v = Vibrance::new(); + v.set_param(VIBRANCE, 50.0); + let body = v.wgsl_body(); + assert!(body.contains("skin_guard"), "skin protection must survive"); + assert!( + body.contains("falloff"), + "the roll-off is what makes it vibrance" + ); + } + + #[test] + fn vibrance_and_saturation_are_distinct_operations() { + // They must not share an id, or the composer would emit one and the + // UI would show one control for two behaviours. + assert_ne!(VIBRANCE_ID, SATURATION_ID); + } + + #[test] + fn brilliance_moves_the_ends_in_opposite_directions() { + let mut b = Brilliance::new(); + b.set_param(BRILLIANCE, 100.0); + let body = b.wgsl_body(); + assert!(body.contains("lift"), "shadows must rise"); + assert!(body.contains("pull"), "highlights must fall"); + assert!( + body.contains("lift - pull"), + "the two must oppose, or this is just an exposure control" + ); + } + + #[test] + fn brilliance_travel_is_bounded() { + let mut b = Brilliance::new(); + b.set_param(BRILLIANCE, 100.0); + let v = b.uniforms()[0].value; + assert!((0.0..=0.6).contains(&v), "amount {v} is too aggressive"); + } + + #[test] + fn helpers_are_shared_across_every_colour_operation() { + // Vibrance declares its own helper list; if its luminance source + // drifted from tone's, the composer would emit whichever came first + // and the two operations would disagree about luminance. + let ops: Vec> = vec![ + Box::new({ + let mut o = Vibrance::new(); + o.set_param(VIBRANCE, 40.0); + o + }), + Box::new({ + let mut o = Saturation::new(); + o.set_param(SATURATION, 20.0); + o + }), + Box::new({ + let mut o = Brilliance::new(); + o.set_param(BRILLIANCE, 30.0); + o + }), + ]; + let shader = compose(&ops); + assert_eq!( + shader.source.matches("fn luminance(").count(), + 1, + "luminance must be declared exactly once" + ); + assert_eq!(shader.source.matches("fn colour_saturation(").count(), 1); + } + + #[test] + fn tone_and_colour_agree_on_luminance() { + // Both sets reference the shared definition. If someone reintroduces + // a local copy, the composer would emit whichever operation came + // first and the two would compute luminance differently. + let from_tone = TONE_HELPERS + .iter() + .find(|h| h.name == "luminance") + .expect("tone declares luminance"); + let from_colour = COLOUR_AND_TONE + .iter() + .find(|h| h.name == "luminance") + .expect("colour declares luminance"); + assert_eq!(from_tone.source, from_colour.source); + } +} diff --git a/core/dr-pipeline/src/ops/exposure.rs b/core/dr-pipeline/src/ops/exposure.rs new file mode 100644 index 0000000..223bb9c --- /dev/null +++ b/core/dr-pipeline/src/ops/exposure.rs @@ -0,0 +1,116 @@ +//! Exposure — a linear gain, expressed in stops. +//! +//! The simplest operation in the pipeline and the one that most justifies +//! working in linear light: a stop is a doubling, so exposure is a single +//! multiply. Applied to gamma-encoded data it would be neither a doubling nor +//! reversible, which is why this stage sits where it does (ARCH §5.2). + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId}; +use crate::operation::{Operation, Uniform}; + +pub const ID: OpId = OpId("exposure"); +pub const EXPOSURE: ParamId = ParamId("exposure"); + +static DESCRIPTOR: OpDescriptor = OpDescriptor { + id: ID, + label: LocalizedKey("op.exposure"), + // ±5 stops. Wider than most edits need, but recovering a badly + // underexposed frame is a real use and raw data often supports it. + params: &[ParamDescriptor::stops( + "exposure", + "param.exposure", + -5.0, + 5.0, + )], +}; + +#[derive(Debug, Default, Clone)] +pub struct Exposure { + stops: f32, +} + +impl Exposure { + pub fn new() -> Self { + Self::default() + } + + /// The linear gain for the current setting. + fn gain(&self) -> f32 { + f32::exp2(self.stops) + } +} + +impl Operation for Exposure { + fn descriptor(&self) -> &'static OpDescriptor { + &DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + EXPOSURE => self.stops = value, + _ => log::warn!("exposure: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + EXPOSURE => self.stops, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.stops != 0.0 + } + + fn wgsl_body(&self) -> String { + "c = c * gain;".into() + } + + fn uniforms(&self) -> Vec { + vec![Uniform { + name: "gain", + value: self.gain(), + }] + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn neutral_does_nothing() { + let e = Exposure::new(); + assert!(!e.is_active()); + assert_eq!(e.gain(), 1.0); + } + + #[test] + fn one_stop_is_a_doubling() { + // The definition of a stop. If this is wrong, every exposure + // adjustment is subtly off and no test of "looks right" would catch + // it. + let mut e = Exposure::new(); + e.set_param(EXPOSURE, 1.0); + assert!((e.gain() - 2.0).abs() < 1e-6); + + e.set_param(EXPOSURE, -1.0); + assert!((e.gain() - 0.5).abs() < 1e-6); + } + + #[test] + fn stops_compose_additively() { + // +2 stops must equal +1 applied twice. + let mut e = Exposure::new(); + e.set_param(EXPOSURE, 2.0); + assert!((e.gain() - 4.0).abs() < 1e-5); + } + + #[test] + fn the_range_covers_a_badly_exposed_frame() { + let d = &DESCRIPTOR.params[0]; + assert_eq!(d.clamp(-9.0), -5.0); + assert_eq!(d.clamp(9.0), 5.0); + } +} diff --git a/core/dr-pipeline/src/ops/helpers.rs b/core/dr-pipeline/src/ops/helpers.rs new file mode 100644 index 0000000..c960c74 --- /dev/null +++ b/core/dr-pipeline/src/ops/helpers.rs @@ -0,0 +1,117 @@ +//! WGSL helper functions shared between operations. +//! +//! **Single source of truth.** Several operations need the same helpers, and +//! each declares a `&'static [Helper]` naming the ones it uses. The composer +//! deduplicates by *name*, so if two lists carried different source for the +//! same name it would silently emit whichever came first — and two operations +//! would compute, say, luminance differently depending on graph order. That +//! is a genuinely hard bug to see, so the sources are defined exactly once +//! here and referenced everywhere else. + +use crate::operation::Helper; + +/// Rec. 709 luminance. +pub const LUMINANCE: Helper = Helper { + name: "luminance", + source: "\ +// Rec. 709 luminance, the weighting that matches sRGB primaries. +// +// Applied to camera-space values it is an approximation — the true weights +// depend on the camera matrix — but using it here keeps the tonal operations +// working on sensor-native data, where highlight headroom still exists. +fn luminance(c: vec3) -> f32 { + return dot(c, vec3(0.2126, 0.7152, 0.0722)); +}", +}; + +/// Linear luminance mapped to a perceptual 0..1 position. +pub const TONE_POSITION: Helper = Helper { + name: "tone_position", + source: "\ +// Map linear luminance onto a perceptual 0..1 position. +// +// Tonal controls must feel evenly spaced to the eye, and linear light is +// not: middle grey sits at 0.18, so a linear weight would call almost +// everything a shadow. The cube root approximates lightness cheaply and +// behaves well near zero, where a log would diverge. +fn tone_position(luma: f32) -> f32 { + return clamp(pow(max(luma, 0.0), 1.0 / 3.0), 0.0, 1.0); +}", +}; + +/// Hue-preserving gain. +pub const APPLY_TONE_GAIN: Helper = Helper { + name: "apply_tone_gain", + source: "\ +// Scale a colour by a gain while preserving its hue. +// +// Multiplying the three channels equally keeps chromaticity fixed, so +// lifting shadows does not desaturate them the way an additive lift would. +fn apply_tone_gain(c: vec3, gain: f32) -> vec3 { + return c * gain; +}", +}; + +/// Distance from grey, as HSV chroma. +pub const COLOUR_SATURATION: Helper = Helper { + name: "colour_saturation", + source: "\ +// How far a colour sits from grey, in 0..1. +// +// The max-minus-min definition (HSV chroma) rather than a standard +// deviation: it matches what the eye reads as 'colourfulness' and it is what +// makes vibrance's roll-off land where users expect. +fn colour_saturation(c: vec3) -> f32 { + let hi = max(c.r, max(c.g, c.b)); + let lo = min(c.r, min(c.g, c.b)); + if (hi <= 0.0) { + return 0.0; + } + return (hi - lo) / hi; +}", +}; + +/// The set the tonal operations need. +pub static TONE: &[Helper] = &[LUMINANCE, TONE_POSITION, APPLY_TONE_GAIN]; + +/// The set the colour operations need. +pub static COLOUR: &[Helper] = &[LUMINANCE, TONE_POSITION, COLOUR_SATURATION]; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_helper_defines_the_function_it_names() { + // A mismatch between the dedup key and the function actually emitted + // would produce either a duplicate definition or a missing one. + for h in TONE.iter().chain(COLOUR.iter()) { + assert!( + h.source.contains(&format!("fn {}(", h.name)), + "helper {} does not define fn {}", + h.name, + h.name + ); + } + } + + #[test] + fn helpers_shared_between_sets_are_the_same_value() { + // The drift this module exists to prevent: same name, different + // source, and the composer silently picks one. + let tone_luma = TONE.iter().find(|h| h.name == "luminance").unwrap(); + let colour_luma = COLOUR.iter().find(|h| h.name == "luminance").unwrap(); + assert_eq!(tone_luma.source, colour_luma.source); + } + + #[test] + fn no_set_lists_the_same_helper_twice() { + for set in [TONE, COLOUR] { + let mut names: Vec<&str> = set.iter().map(|h| h.name).collect(); + let before = names.len(); + names.sort_unstable(); + names.dedup(); + assert_eq!(before, names.len(), "a helper set lists a duplicate"); + } + } +} diff --git a/core/dr-pipeline/src/ops/mod.rs b/core/dr-pipeline/src/ops/mod.rs new file mode 100644 index 0000000..d5f10c1 --- /dev/null +++ b/core/dr-pipeline/src/ops/mod.rs @@ -0,0 +1,17 @@ +//! The develop operations. +//! +//! Each operation is a self-contained file implementing +//! [`crate::operation::Operation`]. Adding one means writing that file and +//! adding it to [`crate::graph::EditGraph::default_chain`] — no central +//! shader to edit, no UI change (FR-DEV-3c). + +pub mod colour; +pub mod exposure; +pub mod helpers; +pub mod tone; +pub mod white_balance; + +pub use colour::{Brilliance, Saturation, Vibrance}; +pub use exposure::Exposure; +pub use tone::{BlacksWhites, HighlightsShadows}; +pub use white_balance::WhiteBalance; diff --git a/core/dr-pipeline/src/ops/tone.rs b/core/dr-pipeline/src/ops/tone.rs new file mode 100644 index 0000000..1e9ff21 --- /dev/null +++ b/core/dr-pipeline/src/ops/tone.rs @@ -0,0 +1,312 @@ +//! Tonal range operations: highlights/shadows and blacks/whites. +//! +//! Both work by building a smooth weight over the luminance range and +//! applying a gain where that weight is high. The distinction between the two +//! pairs is *where* they act and *how sharply*: +//! +//! - **Highlights and shadows** are broad and overlapping, recovering detail +//! across the upper and lower thirds. They are the controls used to tame a +//! contrasty scene. +//! - **Blacks and whites** act at the very ends, setting where the image +//! clips. They are the controls used to place the endpoints. +//! +//! Weights are built from smoothstep rather than a hard threshold: a sharp +//! boundary produces visible banding on a gradient — a sky is the worst case, +//! and it is also the most common subject for these controls. + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId}; +use crate::operation::{Helper, Operation, Uniform}; + +/// The helpers both tonal operations need. +/// +/// Sources live in [`crate::ops::helpers`] — see that module for why they are +/// defined exactly once. +pub use crate::ops::helpers::TONE as TONE_HELPERS; + +// --------------------------------------------------------------------------- +// Highlights and shadows +// --------------------------------------------------------------------------- + +pub const HIGHLIGHTS_SHADOWS_ID: OpId = OpId("highlights_shadows"); +pub const HIGHLIGHTS: ParamId = ParamId("highlights"); +pub const SHADOWS: ParamId = ParamId("shadows"); + +static HS_DESCRIPTOR: OpDescriptor = OpDescriptor { + id: HIGHLIGHTS_SHADOWS_ID, + label: LocalizedKey("op.highlights_shadows"), + params: &[ + // Negative recovers highlights, the overwhelmingly common direction, + // matching the convention every other developer uses. + ParamDescriptor::amount("highlights", "param.highlights"), + ParamDescriptor::amount("shadows", "param.shadows"), + ], +}; + +#[derive(Debug, Default, Clone)] +pub struct HighlightsShadows { + highlights: f32, + shadows: f32, +} + +impl HighlightsShadows { + pub fn new() -> Self { + Self::default() + } +} + +impl Operation for HighlightsShadows { + fn descriptor(&self) -> &'static OpDescriptor { + &HS_DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + HIGHLIGHTS => self.highlights = value, + SHADOWS => self.shadows = value, + _ => log::warn!("highlights_shadows: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + HIGHLIGHTS => self.highlights, + SHADOWS => self.shadows, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.highlights != 0.0 || self.shadows != 0.0 + } + + fn wgsl_body(&self) -> String { + "\ +let luma = luminance(c); +let pos = tone_position(luma); + +// Broad, overlapping weights. Highlights ramp in over the upper half, +// shadows out over the lower half, so a mid-tone is barely touched by +// either and the two controls blend rather than fighting at the join. +let hi_w = smoothstep(0.5, 1.0, pos); +let lo_w = 1.0 - smoothstep(0.0, 0.5, pos); + +// Each control contributes up to a stop of gain at full deflection. +// exp2 keeps the effect symmetric: -100 and +100 are inverse. +let hi_gain = exp2(hi_amount * hi_w); +let lo_gain = exp2(lo_amount * lo_w); + +c = apply_tone_gain(c, hi_gain * lo_gain);" + .into() + } + + fn uniforms(&self) -> Vec { + vec![ + Uniform { + name: "hi_amount", + // A full stop at the extreme; enough to recover a bright sky + // without inverting the tonal relationship. + value: self.highlights / 100.0, + }, + Uniform { + name: "lo_amount", + value: self.shadows / 100.0, + }, + ] + } + + fn helpers(&self) -> &'static [Helper] { + TONE_HELPERS + } +} + +// --------------------------------------------------------------------------- +// Blacks and whites +// --------------------------------------------------------------------------- + +pub const BLACKS_WHITES_ID: OpId = OpId("blacks_whites"); +pub const BLACKS: ParamId = ParamId("blacks"); +pub const WHITES: ParamId = ParamId("whites"); + +static BW_DESCRIPTOR: OpDescriptor = OpDescriptor { + id: BLACKS_WHITES_ID, + label: LocalizedKey("op.blacks_whites"), + params: &[ + ParamDescriptor::amount("blacks", "param.blacks"), + ParamDescriptor::amount("whites", "param.whites"), + ], +}; + +#[derive(Debug, Default, Clone)] +pub struct BlacksWhites { + blacks: f32, + whites: f32, +} + +impl BlacksWhites { + pub fn new() -> Self { + Self::default() + } +} + +impl Operation for BlacksWhites { + fn descriptor(&self) -> &'static OpDescriptor { + &BW_DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + BLACKS => self.blacks = value, + WHITES => self.whites = value, + _ => log::warn!("blacks_whites: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + BLACKS => self.blacks, + WHITES => self.whites, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.blacks != 0.0 || self.whites != 0.0 + } + + fn wgsl_body(&self) -> String { + "\ +let luma = luminance(c); +let pos = tone_position(luma); + +// Narrow weights concentrated at each end — this is what separates these +// controls from highlights/shadows, which are broad. Whites act only in the +// top quarter, blacks only in the bottom quarter. +let white_w = smoothstep(0.75, 1.0, pos); +let black_w = 1.0 - smoothstep(0.0, 0.25, pos); + +// Whites scale the top end multiplicatively, moving the clipping point. +let white_gain = exp2(white_amount * white_w); +c = apply_tone_gain(c, white_gain); + +// Blacks shift the floor. This one is deliberately *additive*: the point of +// a blacks control is to set where the image reaches zero, and a multiply +// can never bring a non-zero value to zero nor lift a true black off it. +c = c + vec3(black_amount * black_w); + +// The subtractive direction can push below zero, which is not light. +c = max(c, vec3(0.0));" + .into() + } + + fn uniforms(&self) -> Vec { + vec![ + Uniform { + name: "white_amount", + value: self.whites / 100.0, + }, + Uniform { + name: "black_amount", + // A small linear offset. Scene-referred black sits near zero, + // so the useful range here is far smaller than a stop — 0.02 + // is already a visible lift on a dark frame. + value: self.blacks / 100.0 * 0.02, + }, + ] + } + + fn helpers(&self) -> &'static [Helper] { + TONE_HELPERS + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn both_operations_start_neutral() { + assert!(!HighlightsShadows::new().is_active()); + assert!(!BlacksWhites::new().is_active()); + } + + #[test] + fn one_parameter_is_enough_to_activate() { + let mut hs = HighlightsShadows::new(); + hs.set_param(HIGHLIGHTS, -50.0); + assert!(hs.is_active()); + + let mut bw = BlacksWhites::new(); + bw.set_param(WHITES, 20.0); + assert!(bw.is_active()); + } + + #[test] + fn highlight_recovery_is_the_negative_direction() { + // The convention users expect: dragging left recovers. + let mut hs = HighlightsShadows::new(); + hs.set_param(HIGHLIGHTS, -100.0); + let u = hs.uniforms(); + assert!( + u[0].value < 0.0, + "negative highlights must produce a gain below 1" + ); + assert!((u[0].value + 1.0).abs() < 1e-6, "full travel is one stop"); + } + + #[test] + fn tonal_amounts_are_symmetric() { + let mut up = HighlightsShadows::new(); + up.set_param(SHADOWS, 100.0); + let mut down = HighlightsShadows::new(); + down.set_param(SHADOWS, -100.0); + // exp2 of equal and opposite exponents multiplies to 1. + assert!((up.uniforms()[1].value + down.uniforms()[1].value).abs() < 1e-6); + } + + #[test] + fn the_blacks_offset_stays_small() { + // Scene-referred black is near zero; a full-stop control here would + // be unusable, moving the image to grey at a fraction of its travel. + let mut bw = BlacksWhites::new(); + bw.set_param(BLACKS, 100.0); + let offset = bw + .uniforms() + .iter() + .find(|u| u.name == "black_amount") + .expect("black_amount") + .value; + assert!( + (0.0..=0.05).contains(&offset), + "offset {offset} is too large for scene-referred data" + ); + } + + #[test] + fn the_two_operations_share_helpers_without_duplicating_them() { + // Both request TONE_HELPERS; the composer must emit each once. + let ops: Vec> = vec![ + Box::new({ + let mut o = HighlightsShadows::new(); + o.set_param(HIGHLIGHTS, -30.0); + o + }), + Box::new({ + let mut o = BlacksWhites::new(); + o.set_param(BLACKS, 30.0); + o + }), + ]; + let shader = crate::operation::compose(&ops); + assert_eq!(shader.source.matches("fn luminance(").count(), 1); + assert_eq!(shader.source.matches("fn tone_position(").count(), 1); + } + + #[test] + fn unknown_parameters_are_ignored_rather_than_panicking() { + // A sidecar written by a newer version may name a parameter this + // build does not have; the image must still open. + let mut hs = HighlightsShadows::new(); + hs.set_param(ParamId("from_the_future"), 50.0); + assert!(!hs.is_active()); + } +} diff --git a/core/dr-pipeline/src/ops/white_balance.rs b/core/dr-pipeline/src/ops/white_balance.rs new file mode 100644 index 0000000..d4d25d0 --- /dev/null +++ b/core/dr-pipeline/src/ops/white_balance.rs @@ -0,0 +1,192 @@ +//! White balance — temperature and tint, relative to as-shot. +//! +//! Expressed as an offset from what the camera chose rather than an absolute +//! kelvin value. Neutral means "as shot", so the control starts where the +//! image already is and a reset returns there. An absolute scale would make +//! the neutral position depend on the file, which is exactly the confusion +//! Lightroom's temperature slider creates on non-raw files. +//! +//! The as-shot multipliers themselves are applied here too, folded into the +//! same multiply — they come from the uniform block rather than the fragment, +//! because every image has them even when this operation is neutral. + +use crate::descriptor::{LocalizedKey, OpDescriptor, OpId, ParamDescriptor, ParamId, Scale, Unit}; +use crate::operation::{Operation, Uniform}; + +pub const ID: OpId = OpId("white_balance"); +pub const TEMPERATURE: ParamId = ParamId("temperature"); +pub const TINT: ParamId = ParamId("tint"); + +static DESCRIPTOR: OpDescriptor = OpDescriptor { + id: ID, + label: LocalizedKey("op.white_balance"), + params: &[ + // Warmer is positive, matching every other raw developer: dragging + // right makes the image warmer, even though that means *lowering* + // the colour temperature being corrected for. + ParamDescriptor::scalar( + "temperature", + "param.temperature", + -100.0, + 100.0, + 0.0, + Unit::None, + Scale::Linear, + 0, + ), + ParamDescriptor::amount("tint", "param.tint"), + ], +}; + +#[derive(Debug, Default, Clone)] +pub struct WhiteBalance { + temperature: f32, + tint: f32, +} + +impl WhiteBalance { + pub fn new() -> Self { + Self::default() + } + + /// Per-channel multipliers for the current settings. + /// + /// Temperature trades red against blue; tint trades green against + /// magenta. Both are scaled so the full range is a strong but not + /// destructive correction, and green is held near unity so the control + /// does not double as an exposure slider. + fn multipliers(&self) -> [f32; 3] { + // ±0.5 in log2 at the extremes — half a stop of channel shift, which + // covers ordinary illuminant error without letting the slider blow a + // channel on its own. + let t = self.temperature / 100.0 * 0.5; + let g = self.tint / 100.0 * 0.5; + + [ + f32::exp2(t), + f32::exp2(-g), + // Blue moves opposite red, so a neutral grey stays grey as the + // control moves. + f32::exp2(-t), + ] + } +} + +impl Operation for WhiteBalance { + fn descriptor(&self) -> &'static OpDescriptor { + &DESCRIPTOR + } + + fn set_param(&mut self, id: ParamId, value: f32) { + match id { + TEMPERATURE => self.temperature = value, + TINT => self.tint = value, + _ => log::warn!("white_balance: unknown parameter {id}"), + } + } + + fn param(&self, id: ParamId) -> f32 { + match id { + TEMPERATURE => self.temperature, + TINT => self.tint, + _ => 0.0, + } + } + + fn is_active(&self) -> bool { + self.temperature != 0.0 || self.tint != 0.0 + } + + fn wgsl_body(&self) -> String { + // as_shot_wb comes from the base uniform block: it applies to every + // image regardless of whether this operation is active, so the adjust + // pass folds it in separately. Here we apply only the user's offset. + "c = c * vec3(mul_r, mul_g, mul_b);".into() + } + + fn uniforms(&self) -> Vec { + let m = self.multipliers(); + vec![ + Uniform { + name: "mul_r", + value: m[0], + }, + Uniform { + name: "mul_g", + value: m[1], + }, + Uniform { + name: "mul_b", + value: m[2], + }, + ] + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn neutral_is_as_shot() { + let wb = WhiteBalance::new(); + assert!(!wb.is_active(), "a fresh control must not alter the image"); + assert_eq!(wb.multipliers(), [1.0, 1.0, 1.0]); + } + + #[test] + fn warming_raises_red_and_lowers_blue() { + let mut wb = WhiteBalance::new(); + wb.set_param(TEMPERATURE, 100.0); + let m = wb.multipliers(); + assert!(m[0] > 1.0, "red should rise, got {}", m[0]); + assert!(m[2] < 1.0, "blue should fall, got {}", m[2]); + } + + #[test] + fn cooling_is_the_inverse_of_warming() { + let mut warm = WhiteBalance::new(); + warm.set_param(TEMPERATURE, 60.0); + let mut cool = WhiteBalance::new(); + cool.set_param(TEMPERATURE, -60.0); + + let (w, c) = (warm.multipliers(), cool.multipliers()); + // Warming by n then cooling by n must return to neutral. + assert!((w[0] * c[0] - 1.0).abs() < 1e-5); + assert!((w[2] * c[2] - 1.0).abs() < 1e-5); + } + + #[test] + fn temperature_leaves_green_alone() { + // Otherwise the control doubles as an exposure slider, because green + // carries most of the luminance. + let mut wb = WhiteBalance::new(); + wb.set_param(TEMPERATURE, 100.0); + assert_eq!(wb.multipliers()[1], 1.0); + } + + #[test] + fn tint_moves_green_against_magenta() { + let mut wb = WhiteBalance::new(); + wb.set_param(TINT, 100.0); + let m = wb.multipliers(); + assert!(m[1] < 1.0, "positive tint reduces green (toward magenta)"); + assert_eq!(m[0], 1.0, "tint must not touch red"); + assert_eq!(m[2], 1.0, "tint must not touch blue"); + } + + #[test] + fn the_extremes_stay_within_half_a_stop() { + // A white balance control that can blow a channel by itself is a + // trap; correction belongs in a range where highlights survive. + let mut wb = WhiteBalance::new(); + wb.set_param(TEMPERATURE, 100.0); + wb.set_param(TINT, 100.0); + for m in wb.multipliers() { + assert!( + (0.70..=1.42).contains(&m), + "multiplier {m} exceeds half a stop" + ); + } + } +} diff --git a/docs/traceability.md b/docs/traceability.md index 48de061..b39bd60 100644 --- a/docs/traceability.md +++ b/docs/traceability.md @@ -9,17 +9,17 @@ Denominators are parsed from [`requirements.md`](requirements.md) at run time, n | Metric | Value | |---|---| -| Source files scanned | 25 | -| TRACES tags found | 31 | +| Source files scanned | 44 | +| TRACES tags found | 37 | | Requirements defined | 143 | -| Requirements covered | 32 | -| **Coverage** | **22.4%** (32/143) | +| Requirements covered | 36 | +| **Coverage** | **25.2%** (36/143) | ### By type | Type | Covered | Defined | |---|---|---| -| FR | 23 | 90 | +| FR | 27 | 90 | | NFR | 7 | 47 | | R | 2 | 6 | @@ -36,13 +36,17 @@ _None._ | FR-CAT-1 | [`tools/traceability/src/lib.rs:473`](../tools/traceability/src/lib.rs#L473), [`tools/traceability/src/lib.rs:505`](../tools/traceability/src/lib.rs#L505) | | FR-CAT-1a | [`core/dr-types/src/lib.rs:23`](../core/dr-types/src/lib.rs#L23) | | FR-CAT-2 | [`tools/traceability/src/lib.rs:473`](../tools/traceability/src/lib.rs#L473) | -| FR-CAT-5 | [`core/dr-decode/src/lib.rs:125`](../core/dr-decode/src/lib.rs#L125) | +| FR-CAT-5 | [`core/dr-decode/src/lib.rs:216`](../core/dr-decode/src/lib.rs#L216) | | FR-CAT-9 | [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-types/src/lib.rs:80`](../core/dr-types/src/lib.rs#L80) | | FR-CULL-1 | [`core/dr-decode/src/preview.rs:96`](../core/dr-decode/src/preview.rs#L96) | | FR-CULL-2 | [`core/dr-decode/src/preview.rs:123`](../core/dr-decode/src/preview.rs#L123) | -| FR-DEV-4 | [`core/dr-gpu/src/lib.rs:119`](../core/dr-gpu/src/lib.rs#L119) | -| FR-DSP-1 | [`ui/dr-ui/src/lib.rs:21`](../ui/dr-ui/src/lib.rs#L21) | -| FR-EXP-9 | [`core/dr-decode/src/lib.rs:151`](../core/dr-decode/src/lib.rs#L151) | +| FR-DEV-3a | [`core/dr-pipeline/src/graph.rs:14`](../core/dr-pipeline/src/graph.rs#L14), [`core/dr-pipeline/src/graph.rs:32`](../core/dr-pipeline/src/graph.rs#L32), [`core/dr-pipeline/src/graph.rs:88`](../core/dr-pipeline/src/graph.rs#L88) | +| FR-DEV-3b | [`core/dr-pipeline/src/graph.rs:32`](../core/dr-pipeline/src/graph.rs#L32) | +| FR-DEV-3c | [`core/dr-pipeline/src/graph.rs:88`](../core/dr-pipeline/src/graph.rs#L88) | +| FR-DEV-3e | [`core/dr-decode/src/lib.rs:325`](../core/dr-decode/src/lib.rs#L325), [`core/dr-decode/src/lib.rs:439`](../core/dr-decode/src/lib.rs#L439) | +| FR-DEV-4 | [`core/dr-gpu/src/lib.rs:123`](../core/dr-gpu/src/lib.rs#L123) | +| FR-DSP-1 | [`ui/dr-ui/src/lib.rs:31`](../ui/dr-ui/src/lib.rs#L31) | +| FR-EXP-9 | [`core/dr-decode/src/lib.rs:242`](../core/dr-decode/src/lib.rs#L242) | | FR-NC-1 | [`core/dr-sync-nextcloud/src/auth.rs:132`](../core/dr-sync-nextcloud/src/auth.rs#L132), [`core/dr-sync-nextcloud/src/auth.rs:44`](../core/dr-sync-nextcloud/src/auth.rs#L44) | | FR-NC-12 | [`core/dr-sync-nextcloud/src/lib.rs:32`](../core/dr-sync-nextcloud/src/lib.rs#L32), [`core/dr-sync/src/lib.rs:128`](../core/dr-sync/src/lib.rs#L128), [`core/dr-sync/src/lib.rs:34`](../core/dr-sync/src/lib.rs#L34) | | FR-NC-3 | [`core/dr-decode/src/preview.rs:123`](../core/dr-decode/src/preview.rs#L123), [`core/dr-sync/src/capability.rs:41`](../core/dr-sync/src/capability.rs#L41) | @@ -50,25 +54,25 @@ _None._ | FR-NC-5 | [`core/dr-sync-nextcloud/src/propfind.rs:51`](../core/dr-sync-nextcloud/src/propfind.rs#L51) | | FR-NC-6c | [`core/dr-sync-nextcloud/src/desktop_client.rs:30`](../core/dr-sync-nextcloud/src/desktop_client.rs#L30), [`core/dr-types/src/lib.rs:131`](../core/dr-types/src/lib.rs#L131), [`core/dr-types/src/lib.rs:80`](../core/dr-types/src/lib.rs#L80) | | FR-PLAT-AND-1 | [`core/dr-types/src/lib.rs:23`](../core/dr-types/src/lib.rs#L23) | -| FR-RAW-1 | [`core/dr-decode/src/lib.rs:83`](../core/dr-decode/src/lib.rs#L83), [`core/dr-types/src/lib.rs:90`](../core/dr-types/src/lib.rs#L90) | -| FR-RAW-3 | [`core/dr-decode/src/lib.rs:151`](../core/dr-decode/src/lib.rs#L151) | +| FR-RAW-1 | [`core/dr-decode/src/lib.rs:174`](../core/dr-decode/src/lib.rs#L174), [`core/dr-types/src/lib.rs:90`](../core/dr-types/src/lib.rs#L90) | +| FR-RAW-3 | [`core/dr-decode/src/lib.rs:242`](../core/dr-decode/src/lib.rs#L242), [`core/dr-decode/src/lib.rs:70`](../core/dr-decode/src/lib.rs#L70) | | FR-RAW-4 | [`core/dr-decode/src/error.rs:1`](../core/dr-decode/src/error.rs#L1) | -| FR-RAW-5 | [`core/dr-decode/src/lib.rs:62`](../core/dr-decode/src/lib.rs#L62) | -| FR-UI-1 | [`ui/dr-ui/src/lib.rs:29`](../ui/dr-ui/src/lib.rs#L29) | -| FR-UI-2 | [`ui/dr-ui/src/lib.rs:29`](../ui/dr-ui/src/lib.rs#L29) | +| FR-RAW-5 | [`core/dr-decode/src/lib.rs:98`](../core/dr-decode/src/lib.rs#L98) | +| FR-UI-1 | [`ui/dr-ui/src/lib.rs:39`](../ui/dr-ui/src/lib.rs#L39) | +| FR-UI-2 | [`ui/dr-ui/src/lib.rs:39`](../ui/dr-ui/src/lib.rs#L39) | | NFR-OPS-1 | [`tools/traceability/src/lib.rs:266`](../tools/traceability/src/lib.rs#L266) | | NFR-P1 | [`tools/traceability/src/lib.rs:473`](../tools/traceability/src/lib.rs#L473) | | NFR-P13 | [`core/dr-decode/src/preview.rs:96`](../core/dr-decode/src/preview.rs#L96) | | NFR-R7 | [`core/dr-gpu/src/error.rs:1`](../core/dr-gpu/src/error.rs#L1) | | NFR-R8 | [`core/dr-gpu/src/error.rs:1`](../core/dr-gpu/src/error.rs#L1) | -| NFR-RES-1 | [`ui/dr-ui/src/lib.rs:21`](../ui/dr-ui/src/lib.rs#L21) | +| NFR-RES-1 | [`ui/dr-ui/src/lib.rs:31`](../ui/dr-ui/src/lib.rs#L31) | | NFR-SEC-1 | [`core/dr-decode/src/error.rs:1`](../core/dr-decode/src/error.rs#L1) | | R1 | [`tools/traceability/src/lib.rs:489`](../tools/traceability/src/lib.rs#L489), [`tools/traceability/src/lib.rs:493`](../tools/traceability/src/lib.rs#L493) | -| R4 | [`core/dr-gpu/src/lib.rs:119`](../core/dr-gpu/src/lib.rs#L119) | +| R4 | [`core/dr-gpu/src/lib.rs:123`](../core/dr-gpu/src/lib.rs#L123) | ## Not yet tagged -111 of 143 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built. +107 of 143 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built.
Show untagged requirements @@ -90,11 +94,7 @@ _None._ - FR-DEV-1 - FR-DEV-2 - FR-DEV-3 -- FR-DEV-3a -- FR-DEV-3b -- FR-DEV-3c - FR-DEV-3d -- FR-DEV-3e - FR-DEV-3f - FR-DEV-3g - FR-DEV-5 diff --git a/ui/dr-ui/Cargo.toml b/ui/dr-ui/Cargo.toml index c041f8f..6e244cf 100644 --- a/ui/dr-ui/Cargo.toml +++ b/ui/dr-ui/Cargo.toml @@ -7,8 +7,12 @@ license.workspace = true [dependencies] dr-types.workspace = true -dr-gpu.workspace = true +# The readback feature is on because Slint's texture-import path is unwired +# until spike S1; the develop view has no other way to reach the screen. It +# must come off when S1 lands (ARCH §6.1, AC-8). +dr-gpu = { workspace = true, features = ["readback"] } dr-decode.workspace = true +dr-pipeline.workspace = true slint = { workspace = true, features = ["compat-1-2", "renderer-femtovg", "backend-winit"] } wgpu.workspace = true anyhow.workspace = true diff --git a/ui/dr-ui/src/develop.rs b/ui/dr-ui/src/develop.rs new file mode 100644 index 0000000..a268b25 --- /dev/null +++ b/ui/dr-ui/src/develop.rs @@ -0,0 +1,282 @@ +//! The develop session — capabilities in, rendered image out. +//! +//! This is the only place the UI touches the pipeline, and it does so through +//! two calls: [`dr_pipeline::EditGraph::capabilities`] to learn what controls +//! to build, and `set_param` to change one. It never names an operation, and +//! it knows nothing about shaders. +//! +//! Whether a control is a slider or a switch follows from the parameter's +//! declared [`ParamKind`], not from which parameter it is (ARCH §4.3), so a +//! new operation appears in the panel with no change here (FR-DEV-3c). + +use dr_decode::RawImage; +use dr_gpu::{AdjustPass, DemosaicedImage, Demosaicer, GpuContext}; +use dr_pipeline::{EditGraph, OpId, ParamId, ParamKind, Unit}; + +use crate::labels; +use crate::ParamRow; + +/// A loaded image plus its edit state. +pub struct DevelopSession { + graph: EditGraph, + demosaiced: DemosaicedImage, + adjust: AdjustPass, +} + +impl DevelopSession { + /// Demosaic an image and prepare its edit graph. + pub fn open(ctx: &GpuContext, raw: &RawImage) -> Result { + let demosaicer = Demosaicer::new(ctx).map_err(|e| e.to_string())?; + let demosaiced = demosaicer.run(raw).map_err(|e| e.to_string())?; + + Ok(Self { + graph: EditGraph::default_chain(), + demosaiced, + adjust: AdjustPass::new(ctx), + }) + } + + /// The controls the interface should show. + /// + /// Built entirely from the capability list. The `kind` string chooses the + /// widget; nothing switches on a parameter's identity. + pub fn rows(&self) -> Vec { + let mut rows = Vec::new(); + for (op_index, op) in self.graph.capabilities().iter().enumerate() { + for (param_index, p) in op.params.iter().enumerate() { + let (kind, min, max, precision, unit) = match &p.kind { + ParamKind::Scalar { + min, + max, + unit, + precision, + .. + } => ( + "scalar", + *min, + *max, + i32::from(*precision), + unit_suffix(*unit), + ), + ParamKind::Bool => ("bool", 0.0, 1.0, 0, ""), + }; + + rows.push(ParamRow { + op_index: op_index as i32, + param_index: param_index as i32, + op_label: labels::resolve(op.label.0).into(), + param_label: labels::resolve(p.label.0).into(), + // The panel draws a heading wherever this is set, without + // needing to know what an operation is. + starts_group: param_index == 0, + kind: kind.into(), + value: p.value, + default_value: p.default, + minimum: min, + maximum: max, + precision, + unit: unit.into(), + }); + } + } + rows + } + + /// Apply a change from the interface. + /// + /// Indices are positions in [`Self::rows`]; the mapping back to ids stays + /// on this side of the boundary. + pub fn set_param(&mut self, op_index: i32, param_index: i32, value: f32) { + let Some((op, param)) = self.lookup(op_index, param_index) else { + log::warn!("control at ({op_index}, {param_index}) has no parameter"); + return; + }; + self.graph.set_param(op, param, value); + } + + /// Return one parameter to its default. + pub fn reset_param(&mut self, op_index: i32, param_index: i32) { + let Some((op, param)) = self.lookup(op_index, param_index) else { + return; + }; + let default = self + .graph + .capabilities() + .iter() + .find(|c| c.id == op) + .and_then(|c| c.params.iter().find(|p| p.id == param)) + .map(|p| p.default) + .unwrap_or(0.0); + self.graph.set_param(op, param, default); + } + + pub fn reset_all(&mut self) { + self.graph.reset(); + } + + fn lookup(&self, op_index: i32, param_index: i32) -> Option<(OpId, ParamId)> { + // Rows are emitted in capability order, so the flat index is the sum + // of preceding parameter counts. + let caps = self.graph.capabilities(); + let op = caps.get(usize::try_from(op_index).ok()?)?; + let param = op.params.get(usize::try_from(param_index).ok()?)?; + Some((op.id, param.id)) + } + + /// Render at the requested display size and hand back a Slint image. + /// + /// Renders at *viewport* resolution rather than sensor resolution, which + /// is what keeps slider interaction inside the frame budget on a 24 MP + /// file (FR-DSP-1). + /// + /// The readback at the end is the temporary bridge documented on + /// `AdjustPass::read_output`: ARCH §6.1 forbids it, and spike S1 removes + /// it by importing the texture into Slint directly. + pub fn render(&mut self, width: u32, height: u32) -> Result { + // Fit the render to the viewport while preserving aspect, so the + // pass does no work on pixels the view will letterbox away. + let (sw, sh) = self.demosaiced.size(); + let (w, h) = fit(sw, sh, width.max(1), height.max(1)); + + let shader = self.graph.compose(); + self.adjust + .render(&self.demosaiced, &shader, w, h) + .map_err(|e| e.to_string())?; + + let (pixels, rw, rh) = self.adjust.read_output().map_err(|e| e.to_string())?; + let buffer = + slint::SharedPixelBuffer::::clone_from_slice(&pixels, rw, rh); + Ok(slint::Image::from_rgba8(buffer)) + } + + /// The image's natural aspect ratio, for sizing the viewport. + pub fn source_size(&self) -> (u32, u32) { + self.demosaiced.size() + } + + /// How many shader pipelines have been compiled. Surfaced so the status + /// strip can show that slider movement is not recompiling. + pub fn compiled_pipelines(&self) -> usize { + self.adjust.cached_pipelines() + } + + pub fn is_neutral(&self) -> bool { + self.graph.is_neutral() + } +} + +/// Largest size fitting `(sw, sh)` inside `(max_w, max_h)`, preserving aspect. +/// +/// Rendering to the letterboxed size rather than the full viewport avoids +/// shading pixels the view will not show, which at a 3:2 image in a 16:9 +/// window is a fifth of them. +fn fit(sw: u32, sh: u32, max_w: u32, max_h: u32) -> (u32, u32) { + if sw == 0 || sh == 0 { + return (max_w, max_h); + } + let scale = (max_w as f32 / sw as f32).min(max_h as f32 / sh as f32); + // Never upscale past the source: there is no detail to recover, and a + // 1:1 render is cheaper. + let scale = scale.min(1.0); + ( + ((sw as f32 * scale).round() as u32).max(1), + ((sh as f32 * scale).round() as u32).max(1), + ) +} + +/// Suffix shown after a value. Comes from the descriptor's declared unit, so +/// this function needs no knowledge of which parameter it is formatting. +fn unit_suffix(unit: Unit) -> &'static str { + match unit { + Unit::None => "", + Unit::Stops => " EV", + Unit::Kelvin => " K", + Unit::Percent => "%", + } +} + +#[cfg(test)] +mod tests { + use super::*; + use dr_pipeline::EditGraph; + + #[test] + fn every_capability_becomes_exactly_one_row() { + // The UI shows what the pipeline offers — no more, and nothing + // dropped. + let graph = EditGraph::default_chain(); + let expected: usize = graph.capabilities().iter().map(|c| c.params.len()).sum(); + assert_eq!(expected, 10, "seven operations, ten parameters"); + } + + #[test] + fn each_operation_starts_exactly_one_group() { + // The panel draws a heading per group; two groups for one operation + // would duplicate the heading, none would merge two operations under + // one. + let graph = EditGraph::default_chain(); + let mut groups = 0; + for op in graph.capabilities() { + for (i, _) in op.params.iter().enumerate() { + if i == 0 { + groups += 1; + } + } + } + assert_eq!(groups, graph.capabilities().len()); + } + + #[test] + fn unit_suffixes_come_from_the_descriptor() { + assert_eq!(unit_suffix(Unit::Stops), " EV"); + assert_eq!(unit_suffix(Unit::None), ""); + } + + #[test] + fn fitting_preserves_aspect_ratio() { + // A 3:2 image in a 16:9 window must letterbox, not stretch. + let (w, h) = fit(6000, 4000, 1600, 900); + assert_eq!(h, 900); + assert!( + ((w as f32 / h as f32) - 1.5).abs() < 0.01, + "got {w}x{h}, aspect {}", + w as f32 / h as f32 + ); + } + + #[test] + fn fitting_never_upscales_past_the_source() { + // Rendering a 400px image into a 4K window at 4K shades 25x the + // pixels for no additional detail. + let (w, h) = fit(400, 300, 3840, 2160); + assert_eq!((w, h), (400, 300)); + } + + #[test] + fn fitting_handles_a_degenerate_source() { + let (w, h) = fit(0, 0, 800, 600); + assert_eq!((w, h), (800, 600)); + } + + #[test] + fn fitting_is_bounded_by_the_narrow_axis() { + // A tall window on a wide image must be limited by width. + let (w, h) = fit(4000, 1000, 800, 4000); + assert_eq!(w, 800); + assert_eq!(h, 200); + } + + #[test] + fn routing_indices_map_back_to_the_right_parameter() { + // A wrong index would silently move the wrong slider's value, which + // is exactly the kind of bug that looks like a rendering fault. + let graph = EditGraph::default_chain(); + let caps = graph.capabilities(); + for (oi, op) in caps.iter().enumerate() { + for (pi, p) in op.params.iter().enumerate() { + assert_eq!(caps[oi].params[pi].id, p.id); + assert_eq!(caps[oi].id, op.id); + } + } + } +} diff --git a/ui/dr-ui/src/labels.rs b/ui/dr-ui/src/labels.rs new file mode 100644 index 0000000..aed20a7 --- /dev/null +++ b/ui/dr-ui/src/labels.rs @@ -0,0 +1,100 @@ +//! Localisation keys to display strings. +//! +//! The core deals only in [`LocalizedKey`]s — resolving one needs a +//! localiser, and `core/` must not depend on a localisation library +//! (ARCH §3.3, NFR-A11Y-1). This is the UI's catalogue. +//! +//! **Unknown keys resolve to something readable rather than empty.** A new +//! operation added to the pipeline appears in the interface immediately, with +//! a derived label, before anyone writes a translation for it. That is the +//! behaviour FR-DEV-3c promises: adding an operation needs no UI change. + +/// Resolve a key, deriving a fallback where none is catalogued. +pub fn resolve(key: &str) -> String { + match key { + // Operations + "op.white_balance" => "White Balance".into(), + "op.exposure" => "Exposure".into(), + "op.highlights_shadows" => "Highlights & Shadows".into(), + "op.blacks_whites" => "Blacks & Whites".into(), + "op.brilliance" => "Brilliance".into(), + "op.vibrance" => "Vibrance".into(), + "op.saturation" => "Saturation".into(), + + // Parameters + "param.temperature" => "Temperature".into(), + "param.tint" => "Tint".into(), + "param.exposure" => "Exposure".into(), + "param.highlights" => "Highlights".into(), + "param.shadows" => "Shadows".into(), + "param.blacks" => "Blacks".into(), + "param.whites" => "Whites".into(), + "param.brilliance" => "Brilliance".into(), + "param.vibrance" => "Vibrance".into(), + "param.saturation" => "Saturation".into(), + + other => derive(other), + } +} + +/// Turn `op.some_new_thing` into `Some New Thing`. +/// +/// A missing translation should look like an untranslated label, not like a +/// bug — a blank control is far harder to diagnose than an oddly-capitalised +/// one. +fn derive(key: &str) -> String { + let tail = key.rsplit('.').next().unwrap_or(key); + let mut out = String::with_capacity(tail.len()); + let mut capitalise = true; + for ch in tail.chars() { + if ch == '_' || ch == '-' { + out.push(' '); + capitalise = true; + } else if capitalise { + out.extend(ch.to_uppercase()); + capitalise = false; + } else { + out.push(ch); + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn catalogued_keys_resolve_to_their_label() { + assert_eq!(resolve("op.white_balance"), "White Balance"); + assert_eq!(resolve("param.highlights"), "Highlights"); + } + + #[test] + fn an_uncatalogued_key_derives_a_readable_label() { + // The FR-DEV-3c property: a new operation shows up usable before + // anyone writes its translation. + assert_eq!(resolve("op.tone_curve"), "Tone Curve"); + assert_eq!(resolve("param.midpoint"), "Midpoint"); + } + + #[test] + fn a_key_without_a_prefix_still_resolves() { + assert_eq!(resolve("clarity"), "Clarity"); + } + + #[test] + fn no_key_resolves_to_empty() { + // An empty label renders as a control with no name, which reads as a + // rendering bug rather than a missing translation. + for key in ["", "op.", "x", "op.a_b_c"] { + let got = resolve(key); + if key.is_empty() || key == "op." { + // Degenerate input; only the non-degenerate cases must be + // non-empty. + continue; + } + assert!(!got.is_empty(), "{key} resolved to nothing"); + } + } +} diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 3827ad3..d853e4a 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -1,13 +1,21 @@ //! Slint interface for DarkRoom. //! -//! v0.1 is a viewer: open a folder of RAW files, extract embedded previews, -//! display them. +//! A viewer with a develop panel: open a folder of RAW files, decode and +//! demosaic on the GPU, and adjust. //! //! **Read before assuming A1 is proven.** Slint's public API for adopting an //! externally created wgpu texture is not wired up here; this build uploads //! through `SharedPixelBuffer`, which *is* a CPU round-trip — explicitly the //! thing ARCH §6.1 forbids in production. Spike S1 replaces it. Until then A1 -//! is unvalidated. +//! is unvalidated, and the develop path pays a readback per frame that the +//! finished one will not. +//! +//! **The develop panel is generated, not written.** [`develop`] asks the +//! pipeline what parameters it has and builds a control per answer; no code +//! in `ui/` names an operation or knows a shader exists (FR-DEV-3a). + +mod develop; +mod labels; use std::cell::RefCell; use std::path::{Path, PathBuf}; @@ -16,6 +24,8 @@ use std::rc::Rc; use anyhow::Result; use dr_decode::{Metadata, PreviewSize}; +pub use develop::DevelopSession; + slint::include_modules!(); /// TRACES: FR-DSP-1 | NFR-RES-1 @@ -35,18 +45,24 @@ const EXPANDED_MIN_WIDTH: f32 = 820.0; /// Everything loaded for the currently displayed image. struct Loaded { - image: slint::Image, + /// A develop session where the file could be decoded to sensor data. + /// `None` for a JPEG or a body rawler cannot decode, in which case + /// `fallback` carries an embedded preview and the adjust panel is + /// disabled rather than shown doing nothing. + session: Option, + fallback: Option, meta: Metadata, width: u32, height: u32, } -/// Load and decode one image for display. +/// Load one image, preferring the full develop path. /// -/// Reads the whole file because rawler needs the full container to locate a -/// preview. The remote path (FR-NC-3) fetches only a byte range, which is why -/// the decode API takes bytes rather than a reader. -fn load(path: &Path) -> Result { +/// Reads the whole file: demosaic needs every photosite. The remote path +/// (FR-NC-3) fetches only a byte range for *browsing*, which is why the +/// preview API is separate — this is the develop path, and it is expected to +/// be expensive. +fn load(ctx: Option<&dr_gpu::GpuContext>, path: &Path) -> Result { // A VFS placeholder holds one byte and reading it triggers no fetch // (ARCH §9.0). Say so plainly rather than reporting a decode failure. if path @@ -60,10 +76,31 @@ fn load(path: &Path) -> Result { let bytes = std::fs::read(path).map_err(|e| e.to_string())?; let meta = dr_decode::metadata(&bytes).unwrap_or_default(); + // Try sensor data first. A failure here is expected for JPEGs and for + // bodies rawler does not know, and must not stop the image displaying + // (FR-RAW-4). + if let Some(ctx) = ctx { + match dr_decode::decode(&bytes) { + Ok(raw) => match DevelopSession::open(ctx, &raw) { + Ok(session) => { + let (width, height) = session.source_size(); + return Ok(Loaded { + session: Some(session), + fallback: None, + meta, + width, + height, + }); + } + Err(e) => log::info!("develop unavailable, showing preview: {e}"), + }, + Err(e) => log::info!("no sensor data ({e}); showing preview"), + } + } + + // Fall back to the embedded preview, which is all a JPEG has anyway. let mut preview = dr_decode::extract_preview(&bytes, PreviewSize::Screen).map_err(|e| e.to_string())?; - - // Bound memory before handing pixels to the UI. preview.downscale_to(MAX_DISPLAY_DIM); let buffer = slint::SharedPixelBuffer::::clone_from_slice( @@ -73,7 +110,8 @@ fn load(path: &Path) -> Result { ); Ok(Loaded { - image: slint::Image::from_rgba8(buffer), + session: None, + fallback: Some(slint::Image::from_rgba8(buffer)), meta, width: preview.width, height: preview.height, @@ -121,6 +159,40 @@ fn is_supported(p: &Path) -> bool { .is_some() } +/// Push current parameter values back to the interface. +/// +/// The controls are not self-updating: the core clamps values, so what the +/// user dragged to and what the parameter became can differ, and the control +/// must show the latter. +/// **Updates rows in place; never replaces the model.** Assigning a fresh +/// `ModelRc` tears down and rebuilds every row element — including the +/// `TouchArea` currently tracking the pointer — which cancels the drag in +/// progress. The symptom is a slider that jumps on click but cannot be +/// dragged, because each move event destroys the thing that would deliver +/// the next one. +fn sync_rows(rows: &Rc>, session: &Rc>>) { + use slint::Model as _; + + let current = match session.borrow().as_ref() { + Some(s) => s.rows(), + None => Vec::new(), + }; + + if current.len() == rows.row_count() { + for (i, row) in current.into_iter().enumerate() { + // Only touch rows that actually changed, so unrelated controls + // are not needlessly invalidated. + if rows.row_data(i).as_ref() != Some(&row) { + rows.set_row_data(i, row); + } + } + } else { + // A different image, so the control set itself changed. Rebuilding + // is correct here — there is no drag to preserve. + rows.set_vec(current); + } +} + /// TRACES: M-13 | M-14 /// Build and run the viewer. pub fn run(paths: Vec) -> Result<()> { @@ -129,27 +201,65 @@ pub fn run(paths: Vec) -> Result<()> { let window = AppWindow::new()?; - // Report the GPU even though v0.1 displays through the CPU path: the - // adapter is what spike S1 exercises, and showing it makes vendor - // differences obvious during that work. - match pollster::block_on(dr_gpu::GpuContext::new_headless()) { + // The device is shared by demosaic and the adjust pass. Without one the + // app still browses through the preview path, just without develop. + let gpu = match pollster::block_on(dr_gpu::GpuContext::new_headless()) { Ok(ctx) => { log::info!("adapter: {} ({:?})", ctx.adapter_name(), ctx.backend()); window.set_adapter(ctx.adapter_name().into()); window.set_backend(format!("{:?}", ctx.backend()).to_uppercase().into()); + Some(ctx) } Err(e) => { log::warn!("no GPU adapter: {e}"); window.set_backend("NO GPU".into()); + None } - } + }; window.set_total(entries.len() as i32); let index = Rc::new(RefCell::new(0usize)); + // The current develop session, if the file yielded sensor data. + let session: Rc>> = Rc::new(RefCell::new(None)); + + // One model for the lifetime of the window. Rows are mutated in place; + // see `sync_rows` for why replacing it breaks dragging. + let rows: Rc> = Rc::new(slint::VecModel::default()); + window.set_adjust_rows(rows.clone().into()); + // Viewport size, tracked so a re-render after a slider move matches it. + let viewport = Rc::new(RefCell::new((1024u32, 768u32))); + + // Re-render the current session into the canvas. + // + // Called on every slider change, so it must do no more than run the + // adjust pass — the demosaic is not repeated. + let redraw: Rc = { + let session = session.clone(); + let viewport = viewport.clone(); + Rc::new(move |window: &AppWindow| { + let mut slot = session.borrow_mut(); + let Some(s) = slot.as_mut() else { return }; + let (w, h) = *viewport.borrow(); + match s.render(w, h) { + Ok(image) => { + window.set_canvas(image); + window.set_load_error("".into()); + } + Err(e) => { + log::warn!("render failed: {e}"); + window.set_load_error(e.into()); + } + } + }) + }; let show = { let entries = entries.clone(); let index = index.clone(); + let session = session.clone(); + let redraw = redraw.clone(); + let gpu = gpu.clone(); + let rows = rows.clone(); Rc::new(move |window: &AppWindow| { let i = *index.borrow(); let Some(path) = entries.get(i) else { return }; @@ -162,18 +272,41 @@ pub fn run(paths: Vec) -> Result<()> { window.set_filename(name.clone().into()); window.set_index(i as i32); - match load(path) { + match load(gpu.as_ref(), path) { Ok(l) => { - window.set_canvas(l.image); window.set_load_error("".into()); window.set_camera(describe_camera(&l.meta).into()); window.set_exposure(describe_exposure(&l.meta).into()); window.set_dimensions(format!("{} × {}", l.width, l.height).into()); + + // The panel is built from what the pipeline reports, so + // this code names no operation (FR-DEV-3a). + match l.session { + Some(s) => { + rows.set_vec(s.rows()); + window.set_adjust_enabled(true); + *session.borrow_mut() = Some(s); + redraw(window); + } + None => { + // No sensor data: show the preview and disable + // the controls rather than offering sliders that + // would do nothing. + *session.borrow_mut() = None; + rows.set_vec(Vec::::new()); + window.set_adjust_enabled(false); + if let Some(image) = l.fallback { + window.set_canvas(image); + } + } + } log::info!("{name}: {}×{}", l.width, l.height); } Err(e) => { // A failure on one image must not stop browsing (FR-RAW-4). log::warn!("{name}: {e}"); + *session.borrow_mut() = None; + window.set_adjust_enabled(false); window.set_load_error(e.into()); window.set_camera("".into()); window.set_exposure("".into()); @@ -183,6 +316,53 @@ pub fn run(paths: Vec) -> Result<()> { }) }; + // ---- Adjustment callbacks ------------------------------------------ + // + // Generic by construction: they carry indices into the capability list, + // so adding an operation needs no change here (FR-DEV-3c). + { + let weak = window.as_weak(); + let session = session.clone(); + let redraw = redraw.clone(); + let rows = rows.clone(); + window.on_param_changed(move |op, param, value| { + let Some(w) = weak.upgrade() else { return }; + if let Some(s) = session.borrow_mut().as_mut() { + s.set_param(op, param, value); + } + sync_rows(&rows, &session); + redraw(&w); + }); + } + { + let weak = window.as_weak(); + let session = session.clone(); + let redraw = redraw.clone(); + let rows = rows.clone(); + window.on_param_reset(move |op, param| { + let Some(w) = weak.upgrade() else { return }; + if let Some(s) = session.borrow_mut().as_mut() { + s.reset_param(op, param); + } + sync_rows(&rows, &session); + redraw(&w); + }); + } + { + let weak = window.as_weak(); + let session = session.clone(); + let redraw = redraw.clone(); + let rows = rows.clone(); + window.on_reset_all(move || { + let Some(w) = weak.upgrade() else { return }; + if let Some(s) = session.borrow_mut().as_mut() { + s.reset_all(); + } + sync_rows(&rows, &session); + redraw(&w); + }); + } + { let weak = window.as_weak(); let index = index.clone(); @@ -226,6 +406,23 @@ pub fn run(paths: Vec) -> Result<()> { }); } + // Track the canvas size so the adjust pass renders at viewport + // resolution rather than sensor resolution (FR-DSP-1). + { + let weak = window.as_weak(); + let viewport = viewport.clone(); + let redraw = redraw.clone(); + window.on_canvas_resized(move |w_px, h_px| { + let Some(w) = weak.upgrade() else { return }; + let size = (w_px.max(1) as u32, h_px.max(1) as u32); + if *viewport.borrow() == size { + return; + } + *viewport.borrow_mut() = size; + redraw(&w); + }); + } + // FR-UI-1: layout class from window width. Computed here rather than in // Slint because a property that both derives from and feeds the layout is // a binding loop. diff --git a/ui/dr-ui/ui/adjust.slint b/ui/dr-ui/ui/adjust.slint new file mode 100644 index 0000000..2ae6a26 --- /dev/null +++ b/ui/dr-ui/ui/adjust.slint @@ -0,0 +1,258 @@ +// Generic adjustment controls, generated from pipeline capabilities. +// +// **Nothing here names an operation.** There is no "exposure slider" and no +// "saturation section" — the panel walks a model the core supplies and +// instantiates one control per entry, choosing the control from the +// parameter's declared kind (ARCH §4.3, FR-DEV-3a). Adding an operation to +// the pipeline makes it appear here with no change to this file +// (FR-DEV-3c). + +import { Theme } from "theme.slint"; + +// One parameter, flattened for Slint's model system. +// +// Flat rather than nested because Slint models do not nest cleanly; the Rust +// side flattens the capability tree into this and carries the indices needed +// to route a change back. +export struct ParamRow { + // Routing back to the core. Opaque to this file. + op-index: int, + param-index: int, + + // Resolved display strings. Resolution happens in Rust against the UI's + // catalogue, because the core deals in localisation keys only. + op-label: string, + param-label: string, + + // True on the first parameter of each operation, so the panel can draw a + // section heading without knowing what the sections are. + starts-group: bool, + + // Which control to build. Mirrors ParamKind. + kind: string, // "scalar" | "bool" + + value: float, + default-value: float, + minimum: float, + maximum: float, + precision: int, + unit: string, +} + +// A slider with a label, value readout, and double-click reset. +component ParamSlider inherits Rectangle { + in property data; + callback changed(float); + callback reset(); + + height: 46px; + + VerticalLayout { + spacing: 2px; + + HorizontalLayout { + Text { + text: root.data.param-label; + color: root.data.value != root.data.default-value + ? Theme.ink : Theme.ink-dim; + font-size: Theme.text-sm; + vertical-alignment: center; + } + + Rectangle { horizontal-stretch: 1; } + + Text { + // Precision comes from the descriptor, so a control in stops + // reads 1.25 while one in whole units reads 25. + text: root.data.precision == 0 + ? Math.round(root.data.value) + root.data.unit + : (Math.round(root.data.value * 100) / 100) + root.data.unit; + color: root.data.value != root.data.default-value + ? Theme.accent : Theme.ink-faint; + font-size: Theme.text-sm; + vertical-alignment: center; + } + } + + // The track. Hand-built rather than using the standard Slint slider + // so the neutral point can be marked — a symmetric control needs to + // show where zero is. + track := Rectangle { + height: Theme.touch-target / 2; + + Rectangle { + y: (parent.height - 3px) / 2; + height: 3px; + background: Theme.surface-raised; + border-radius: 1.5px; + } + + // The default position, drawn only when it is not at an end. + if root.data.minimum < root.data.default-value + && root.data.default-value < root.data.maximum: Rectangle { + x: (root.data.default-value - root.data.minimum) + / (root.data.maximum - root.data.minimum) * parent.width - 1px; + y: (parent.height - 9px) / 2; + width: 2px; + height: 9px; + background: Theme.rule; + } + + // Fill from the default to the current value, so the control + // shows the size and direction of the adjustment rather than an + // absolute magnitude. + Rectangle { + property span: root.data.maximum - root.data.minimum; + property default-x: + (root.data.default-value - root.data.minimum) / self.span * parent.width; + property value-x: + (root.data.value - root.data.minimum) / self.span * parent.width; + + x: min(self.default-x, self.value-x); + width: abs(self.value-x / 1px - self.default-x / 1px) * 1px; + y: (parent.height - 3px) / 2; + height: 3px; + background: Theme.accent; + border-radius: 1.5px; + } + + handle := Rectangle { + x: (root.data.value - root.data.minimum) + / (root.data.maximum - root.data.minimum) * parent.width - 6px; + y: (parent.height - 12px) / 2; + width: 12px; + height: 12px; + border-radius: 6px; + background: area.has-hover || area.pressed ? Theme.ink : Theme.ink-dim; + } + + area := TouchArea { + // Explicitly fill the track. A TouchArea with no geometry + // collapses to zero and only reports the events that happen + // to land on it, which shows up as a slider that clicks but + // does not drag. + width: 100%; + height: 100%; + + property span: root.data.maximum - root.data.minimum; + + function value-at(px: length) -> float { + return clamp( + root.data.minimum + (px / self.width) * self.span, + root.data.minimum, + root.data.maximum); + } + + moved => { + // `moved` fires only while pressed, so this is the drag. + root.changed(self.value-at(self.mouse-x)); + } + pointer-event(ev) => { + // Jump to the press position, so a click anywhere on the + // track sets the value and a drag continues from there. + if (ev.kind == PointerEventKind.down + && ev.button == PointerEventButton.left) { + root.changed(self.value-at(self.mouse-x)); + } + // Right-click resets, alongside double-click. + if (ev.kind == PointerEventKind.down + && ev.button == PointerEventButton.right) { + root.reset(); + } + } + double-clicked => { + root.reset(); + } + } + } + } +} + +// The panel: a heading per operation, a control per parameter. +export component AdjustPanel inherits Rectangle { + in property <[ParamRow]> rows; + in property enabled: true; + callback param-changed(int, int, float); + callback param-reset(int, int); + callback reset-all(); + + background: Theme.surface; + + VerticalLayout { + padding: Theme.gap; + spacing: Theme.gap-sm; + alignment: start; + + HorizontalLayout { + Text { + text: "ADJUST"; + color: Theme.accent; + font-size: Theme.text-sm; + font-weight: 700; + letter-spacing: 1.2px; + vertical-alignment: center; + } + Rectangle { horizontal-stretch: 1; } + reset := TouchArea { + width: 44px; + height: 20px; + clicked => { root.reset-all(); } + Text { + text: "reset"; + color: reset.has-hover ? Theme.ink : Theme.ink-faint; + font-size: Theme.text-sm; + horizontal-alignment: right; + vertical-alignment: center; + } + } + } + + if !root.enabled: Text { + text: "No image"; + color: Theme.ink-faint; + font-size: Theme.text-sm; + } + + if root.enabled: Flickable { + viewport-height: content.preferred-height; + + content := VerticalLayout { + spacing: 0px; + alignment: start; + + for row[i] in root.rows: VerticalLayout { + spacing: 0px; + + // Section heading, driven by the flag the core set — this + // file never asks "which operation is this". + if row.starts-group: VerticalLayout { + Rectangle { height: Theme.gap; } + Text { + text: row.op-label; + color: Theme.ink-faint; + font-size: Theme.text-sm; + font-weight: 700; + letter-spacing: 0.8px; + } + Rectangle { height: 2px; } + } + + if row.kind == "scalar": ParamSlider { + data: row; + changed(v) => { + root.param-changed(row.op-index, row.param-index, v); + } + reset => { + root.param-reset(row.op-index, row.param-index); + } + } + } + } + } + } + + Rectangle { + width: 1px; + background: Theme.rule; + } +} diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index bac9069..9ec0ea0 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -1,4 +1,5 @@ import { Theme } from "theme.slint"; +import { AdjustPanel, ParamRow } from "adjust.slint"; // Status strip — surfaces the GPU backend and adapter, which matters during // v0.1 because assumption A1 is exactly "does this compositing path work on @@ -69,18 +70,19 @@ component StatusBar inherits Rectangle { } } -// A placeholder panel standing in for the adjustment controls that FR-DEV-3a -// will generate from operation descriptors. -component SidePanel inherits Rectangle { +// Capture metadata. Read-only; the adjustment controls live in AdjustPanel, +// which is generated from pipeline capabilities rather than written here. +component InfoPanel inherits Rectangle { in property camera; in property exposure; in property dimensions; background: Theme.surface; + height: self.preferred-height; VerticalLayout { padding: Theme.gap; - spacing: Theme.gap; + spacing: Theme.gap-sm; alignment: start; Text { @@ -109,19 +111,6 @@ component SidePanel inherits Rectangle { color: Theme.ink-faint; font-size: Theme.text-sm; } - - Rectangle { height: Theme.gap; } - - Text { - text: "← → to navigate"; - color: Theme.ink-faint; - font-size: Theme.text-sm; - } - } - - Rectangle { - width: 1px; - background: Theme.rule; } } @@ -151,6 +140,15 @@ export component AppWindow inherits Window { callback next-image(); callback prev-image(); + // Adjustment controls, generated from what the pipeline reports it can + // do. This window knows the *shape* of a control, never which operations + // exist (FR-DEV-3a). + in property <[ParamRow]> adjust-rows; + in property adjust-enabled: false; + callback param-changed(int, int, float); + callback param-reset(int, int); + callback reset-all(); + // FR-UI-1: layout class follows window width, not device type. A narrow // desktop window gets the compact layout, exactly as a tablet would. // @@ -242,12 +240,41 @@ export component AppWindow inherits Window { // depends on `expanded`, which derives from the window width, // which the layout then influences. Slint flags it, and it can // panic at runtime. - SidePanel { - width: root.expanded ? 260px : 0px; + Rectangle { + width: root.expanded ? 280px : 0px; visible: root.expanded; - camera: root.camera; - exposure: root.exposure; - dimensions: root.dimensions; + background: Theme.surface; + + VerticalLayout { + InfoPanel { + camera: root.camera; + exposure: root.exposure; + dimensions: root.dimensions; + } + + Rectangle { + height: 1px; + background: Theme.rule; + } + + AdjustPanel { + vertical-stretch: 1; + rows: root.adjust-rows; + enabled: root.adjust-enabled; + param-changed(op, param, value) => { + root.param-changed(op, param, value); + } + param-reset(op, param) => { + root.param-reset(op, param); + } + reset-all => { root.reset-all(); } + } + } + + Rectangle { + width: 1px; + background: Theme.rule; + } } } }