From 4b6c110816253482b1b57b222e9627a5b92df20a Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 29 Aug 2026 23:36:21 +0200 Subject: [PATCH] Count the sensor's own numbers, so a cull can see headroom the render hides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FR-CULL-3's remaining two bullets. What existed was a *display* histogram tagged FR-DSP-7: it binds AdjustPass's Rgba8Unorm output, recovers an 8-bit code value, and counts clipping as `r == 255`. Its own documentation says a clipped bin means "a highlight that is actually gone rather than one the transform might still recover", which is the opposite of what a culling decision needs. FR-CULL-3 asks for the histogram of the sensor data, on the explicit grounds that a rendered image "systematically lies about what is recoverable in the raw", and a readout that measures the render cannot answer that however it is presented. So this is a second instrument beside the first rather than a setting on it. Both are true; they are true about different things; the panel offers both behind a chip row and the words travel with the numbers, because a raw saturation figure drawn under a heading saying Highlights would be mislabelled exactly where the difference matters. **What is reduced over, and what it cost to decide.** ARCH §5.5 specified the pre-demosaic CFA samples. This reduces over the demosaiced scene-linear texture instead, and §5.5 is amended to record the choice rather than let the specification and the code disagree in silence. The texture is camera-native — unbalanced, unmatrixed, uncurved — and normalised by the sensor's own black and white levels, so 1.0 is saturation by construction and the distribution below it is the headroom question with no calibration to carry. Retaining the CFA samples would mean keeping the packed u32 buffer Demosaicer::run currently drops: 48 MB at 24 MP, 120 MB at 60 MP, resident per open photograph whether or not anyone looks at the histogram, on a platform §6.2 exists because memory is scarce on. Three things it therefore cannot say, written into the module docs and into §5.5 rather than left to be discovered: it counts pixels not photosites, so a saturated site drags its interpolated neighbours up and per-channel clipping is smeared by about a demosaic kernel; it cannot see above white, because demosaic.wgsl clamps each photosite at 1.0 for its own good reasons (a Canon 6D reads to 16383 against a declared 15070) so "at saturation" and "a stop past it" share a bin; and it is measured after the CFA pattern is gone, so it can name which colour clipped in the reconstructed image but not which photosite went first. The axis is stops below saturation, 16 bins per stop over 256 bins — the same bin count the display reduction uses, so the fold into drawable columns is shared and a divergence between the two plots would have to be deliberate. A linear axis spends half its width on the top stop, which is why nobody has ever drawn a useful linear raw histogram. The fourth series is the brightest channel rather than luma: these values are unbalanced, so any weighted sum of them is a number about nothing, and the brightest channel is the one that saturates first and so the one the headroom question is actually about. It is a property of the file and not of the render, which has two consequences. It is computed once per photograph and cached — nothing downstream of the demosaic can move a count in it — so a cull does not pay the display histogram's per-frame cost three thousand times. And it describes the whole frame rather than the visible region, deliberately opposite to DevelopSession::histogram: a crop changes what is on screen and changes nothing about what the sensor recorded. Tags are on the reduction, the type, its constructor and the presentation arithmetic, each of which has a test that fails if the behaviour goes. The Slint panel and the push from lib.rs keep their reasoning as prose: nothing asserts them, and a tag would claim coverage the assertions are not making. --- core/dr-gpu/src/lib.rs | 10 + core/dr-gpu/src/raw_histogram.rs | 849 +++++++++++++++++++++ core/dr-gpu/src/shaders/raw_histogram.wgsl | 156 ++++ docs/architecture.md | 35 +- docs/outstanding.md | 39 +- ui/dr-ui/src/develop.rs | 220 +++++- ui/dr-ui/src/histogram.rs | 362 ++++++++- ui/dr-ui/src/lib.rs | 38 + ui/dr-ui/ui/app.slint | 11 + ui/dr-ui/ui/histogram.slint | 120 ++- 10 files changed, 1793 insertions(+), 47 deletions(-) create mode 100644 core/dr-gpu/src/raw_histogram.rs create mode 100644 core/dr-gpu/src/shaders/raw_histogram.wgsl diff --git a/core/dr-gpu/src/lib.rs b/core/dr-gpu/src/lib.rs index fa46ff9..edbcb4c 100644 --- a/core/dr-gpu/src/lib.rs +++ b/core/dr-gpu/src/lib.rs @@ -25,6 +25,7 @@ mod error; mod focus; mod histogram; mod mask; +mod raw_histogram; mod readback; mod segment; pub use adjust::AdjustPass; @@ -40,6 +41,15 @@ pub use focus::{FocusPeakPass, FocusPeaking, PeakColour, PeakSensitivity}; // at all at a crate root shared with demosaic and segmentation. pub use histogram::{Histogram, HistogramPass, BINS as HISTOGRAM_BINS}; pub use mask::{LabelField, MaskArray, MaskPass, SubjectMasks}; +// Renamed on the way out on the same terms as the display histogram's +// constants above, and kept distinct from them because the two axes are +// different quantities: one counts output code values, the other counts stops +// below sensor saturation. A caller that confused them would draw a correct +// plot against the wrong scale. +pub use raw_histogram::{ + RawHistogram, RawHistogramPass, BINS as RAW_HISTOGRAM_BINS, + BINS_PER_STOP as RAW_HISTOGRAM_BINS_PER_STOP, STOPS as RAW_HISTOGRAM_STOPS, +}; pub use segment::{SegmentOptions, SegmentPass, Segmentation}; /// Owns the wgpu device and queue. diff --git a/core/dr-gpu/src/raw_histogram.rs b/core/dr-gpu/src/raw_histogram.rs new file mode 100644 index 0000000..7435d7e --- /dev/null +++ b/core/dr-gpu/src/raw_histogram.rs @@ -0,0 +1,849 @@ +//! TRACES: FR-CULL-3 +//! Counting the sensor's own numbers, on an axis measured in stops of +//! headroom. +//! +//! # Why there are two histograms, and what each one answers +//! +//! [`crate::HistogramPass`] beside this file counts the frame the display is +//! about to show. It is tagged FR-DSP-7, its own documentation says a clipped +//! bin means "a highlight that is actually gone rather than one the transform +//! might still recover", and that is exactly right for the question it is +//! there to answer: *what will this image look like when I send it out.* +//! +//! FR-CULL-3 asks the opposite question, and says why: "a JPEG's clipping +//! warnings systematically lie about what is recoverable in the raw". A +//! photographer culling three thousand frames is deciding whether a highlight +//! can be brought back, not whether the current rendering happens to have +//! kept it. A readout that measures the render cannot answer that however it +//! is presented, so this is a second instrument rather than a setting on the +//! first — and the panel offers both, because both are true and they are true +//! about different things. +//! +//! # What is reduced over, and why it is not the CFA samples +//! +//! ARCH §5.5 originally specified a reduction over the *pre-demosaic* texture. +//! This reduces over the demosaiced scene-linear texture instead — +//! [`crate::DemosaicedImage`]'s `Rgba16Float`, the one the whole develop chain +//! then works from — and the amendment to §5.5 records the decision rather +//! than leaving the specification and the code silently disagreeing. +//! +//! That texture is the right one on the merits. It is camera-native: no white +//! balance has been applied, no camera matrix, no base curve, no tone curve, +//! no output transform. It is normalised by the sensor's own black and white +//! levels, so 1.0 is saturation by construction and the distribution below it +//! *is* the headroom question, with no calibration to carry and no origin to +//! choose. +//! +//! And retaining the CFA samples would have cost real memory for the +//! difference. `Demosaicer::run` uploads the packed `u32` sample buffer and +//! drops it the moment the dispatch is encoded; keeping it resident to reduce +//! over later is 48 MB at 24 MP and 120 MB at 60 MP, per photograph opened, +//! whether or not anyone ever looks at the histogram. ARCH §6.2 exists because +//! memory is scarce on the platform this has to run on. A more complete +//! instrument that is paid for on every image by everyone who never opens it +//! is not a better instrument. +//! +//! # Three things this cannot tell you +//! +//! Each of these is a different failure, and none of them is hidden by +//! presenting the result well. +//! +//! **It counts pixels, not photosites.** Every value here passed through the +//! demosaic, so a saturated photosite pulls its interpolated neighbours up +//! with it: per-channel clipping is smeared across roughly a demosaic kernel. +//! The count of clipped pixels is therefore an overestimate of the count of +//! clipped photosites, by an amount that depends on how isolated the clipping +//! is — a large blown sky is barely affected, a field of specular glints on +//! water is affected a great deal. +//! +//! **It cannot see above white.** `demosaic.wgsl` clamps each photosite at 1.0 +//! for a good reason of its own — a Canon 6D reads to 16383 against a declared +//! white level of 15070, and carrying that overshoot forward turns blown +//! highlights pink through the white balance — but the consequence here is +//! that "at saturation" and "a stop past saturation" arrive in the same bin. +//! The top column says *how much* is gone and never *how far* gone, which +//! matters because some of what a raw converter can recover lives exactly +//! there. +//! +//! **It is measured after the CFA pattern is gone.** Which channel of the +//! mosaic saturated first at a given site is a fact about the sensor readout, +//! and by this point each pixel carries three channels, two of which were +//! reconstructed from its neighbours. The series below say which *colour* +//! clipped in the reconstructed image; they cannot say which photosite went +//! first. +//! +//! # What it costs, and when it runs +//! +//! One dispatch over the source texture plus a 4 KB buffer copy and a mapping. +//! Unlike the display histogram this is a property of the **file**, not of the +//! edit: nothing downstream of the demosaic can change it, so it is computed +//! once per photograph and cached rather than recomputed on every settled +//! frame. That is also why it describes the whole frame rather than the +//! visible region — a crop changes what is on screen and changes nothing about +//! what the sensor recorded. + +use wgpu::util::DeviceExt; + +use crate::readback::await_mapping; +use crate::{GpuContext, GpuError}; + +/// Bins on the stops axis. +/// +/// 256, the same count [`crate::HistogramPass`] uses, so the presentation code +/// that folds bins into drawable columns is shared between the two rather than +/// written twice against two different divisors. +pub const BINS: usize = 256; + +/// Bins per stop. Must equal `BINS_PER_STOP` in `raw_histogram.wgsl`. +/// +/// 16 over 256 bins puts the floor of the axis at just under 16 stops, which +/// covers every sensor anyone will point at this and a couple more besides — +/// the best-measured full-frame bodies reach about 15 stops at base ISO, and +/// nothing below the noise floor is a reading anyway. A finer division would +/// buy resolution in a part of the plot that is already narrower than one +/// drawn column. +pub const BINS_PER_STOP: usize = 16; + +/// Stops the axis spans, as a whole number. +/// +/// Note that bin 0 sits at `STOPS - 1/BINS_PER_STOP` below saturation rather +/// than at `STOPS`, and holds everything darker as well — see +/// [`RawHistogram::stops`]. +/// +/// The last two stops of this range are, strictly, further than the source can +/// carry: [`crate::DemosaicedImage::FORMAT`] is `Rgba16Float`, whose smallest +/// normal value is 2^-14, so below fourteen stops a value survives only as an +/// f16 subnormal and many drivers flush those to zero. It costs nothing to +/// leave the axis at a round sixteen — no sensor records usable signal +/// anywhere near there, and the alternative is a plot whose floor is an +/// implementation detail of a texture format. +pub const STOPS: usize = BINS / BINS_PER_STOP; + +/// Four series plus the two clip counters, as the shader lays them out. Kept +/// next to the shader's own constants because the two must agree. +const CHANNELS: usize = 4; +const SATURATED: usize = CHANNELS * BINS; +const AT_BLACK: usize = CHANNELS * BINS + 1; +const SLOTS: usize = CHANNELS * BINS + 2; + +/// TRACES: FR-CULL-3 +/// A counted sensor frame: how many pixels sit at each distance below +/// saturation. +/// +/// Counts, not proportions, and bins rather than stops — the same division of +/// labour [`crate::Histogram`] draws (ARCH §4.3a). Folding 256 bins into the +/// columns a panel can show, deciding how much clipping is worth an alarm and +/// turning a bin index into a figure a photographer reads are all questions +/// about an interface. What this crate owes is the numbers. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct RawHistogram { + red: [u32; BINS], + green: [u32; BINS], + blue: [u32; BINS], + brightest: [u32; BINS], + saturated: u32, + at_black: u32, + pixels: u32, +} + +impl RawHistogram { + /// Counts per bin for the camera's red channel, darkest first. + /// + /// **Camera-native and unbalanced.** These are not the red of a rendered + /// image: the as-shot white balance has not been applied, so on a neutral + /// subject under daylight red typically sits around a stop below green and + /// blue rather further. That is not a defect to be corrected before + /// drawing — it is the point. Red reaching saturation while green has a + /// stop left is a fact about the exposure, and balancing it away would + /// hide the very thing the instrument is for. + pub fn red(&self) -> &[u32; BINS] { + &self.red + } + + pub fn green(&self) -> &[u32; BINS] { + &self.green + } + + pub fn blue(&self) -> &[u32; BINS] { + &self.blue + } + + /// The brightest of the three channels at each pixel. + /// + /// The envelope the three series sit under, and the one that answers the + /// headroom question directly: a pixel is safe exactly as long as its + /// *highest* channel is, because that is the one that saturates first. + /// + /// Where the display histogram draws Rec.709 luma, this draws a maximum, + /// and the substitution is not cosmetic. Luma is a weighted sum of + /// display-referred primaries; these values are camera-native and + /// unbalanced, so any weighting of them is a number about nothing. + pub fn brightest(&self) -> &[u32; BINS] { + &self.brightest + } + + /// Pixels with **any** channel at sensor saturation. + /// + /// Any rather than all, for the reason the display histogram gives about + /// its own counter: a channel at the ceiling has no gradation left in it + /// however much the other two still hold, and counting only neutral white + /// would stay silent on exactly the saturated subject that clips first. + /// + /// Read this against [`Self::pixels`], and read the module documentation + /// before reading it against a count of photosites — the demosaic has + /// already spread each clipped site over its neighbours. + pub fn saturated(&self) -> u32 { + self.saturated + } + + /// Pixels with any channel at or below the sensor's black level. + /// + /// The other end, and a genuinely different statement from the display + /// histogram's shadow clipping: this is a photosite that recorded nothing + /// but read noise, where that one is a tone the output transform put at + /// zero and which a lifted black point would bring back. + pub fn at_black(&self) -> u32 { + self.at_black + } + + /// Pixels counted. The denominator for the two figures above. + pub fn pixels(&self) -> u32 { + self.pixels + } + + /// How far below sensor saturation a bin sits, in stops. + /// + /// Bin 255 is 0 stops — the top 1/16 of a stop, where a saturated pixel + /// lands. Bin 239 is exactly 1 stop below. Bin 0 is 15.9375 stops below + /// **and everything darker than that**, because it is the end of the axis + /// rather than a bin of the same width as its neighbours; a photograph + /// with genuinely 20 stops in it puts its bottom four into that column. + /// + /// An index past the end is clamped rather than refused: this exists to + /// label a plot, and a panel that panicked because a loop ran one past its + /// last column would be a worse failure than a label at the floor of the + /// axis. + pub fn stops(bin: usize) -> f32 { + (BINS - 1 - bin.min(BINS - 1)) as f32 / BINS_PER_STOP as f32 + } + + /// Rebuild from the flat slot array the shader writes. + /// + /// `pixels` is summed from the red series rather than taken from the image + /// dimensions, for the reason [`crate::Histogram`] gives: every pixel lands + /// in exactly one red bin, so the sum *is* the count, and deriving it that + /// way makes a dropped or double-counted texel show up as a wrong + /// denominator instead of hiding. + fn from_slots(slots: &[u32]) -> Result { + if slots.len() < SLOTS { + return Err(GpuError::Readback(format!( + "raw histogram readback was {} slots, expected {SLOTS}", + slots.len() + ))); + } + let series = |i: usize| -> [u32; BINS] { + let mut out = [0u32; BINS]; + out.copy_from_slice(&slots[i * BINS..(i + 1) * BINS]); + out + }; + let red = series(0); + Ok(Self { + pixels: red.iter().sum(), + red, + green: series(1), + blue: series(2), + brightest: series(3), + saturated: slots[SATURATED], + at_black: slots[AT_BLACK], + }) + } +} + +/// The dispatch's view of the source. Padded to 16 bytes for std140. +#[repr(C)] +#[derive(Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)] +struct Dims { + width: u32, + height: u32, + pad_0: u32, + pad_1: u32, +} + +/// TRACES: FR-CULL-3 +/// Counts a scene-linear sensor frame into [`RawHistogram`]. +/// +/// Holds its buffers for the life of the session. They are a fixed 4104 bytes +/// whatever the sensor size, which is the property that makes the whole shape +/// affordable — there is nothing to reallocate when a different photograph is +/// opened. +pub struct RawHistogramPass { + ctx: GpuContext, + pipeline: wgpu::ComputePipeline, + bind_group_layout: wgpu::BindGroupLayout, + /// Where the shader accumulates. Cleared before each dispatch. + bins: wgpu::Buffer, + /// Mappable destination; a storage buffer cannot also be `MAP_READ`. + staging: wgpu::Buffer, + dims: wgpu::Buffer, +} + +impl RawHistogramPass { + pub fn new(ctx: &GpuContext) -> Result { + // A validation error here is a bug in the shader beside this file, not + // anything a user did — surfaced as a `Result` rather than left to + // wgpu's default handler, which panics. + let scope = ctx.device.push_error_scope(wgpu::ErrorFilter::Validation); + + let module = ctx + .device + .create_shader_module(wgpu::ShaderModuleDescriptor { + label: Some("raw-histogram"), + source: wgpu::ShaderSource::Wgsl(include_str!("shaders/raw_histogram.wgsl").into()), + }); + + let bind_group_layout = + ctx.device + .create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor { + label: Some("raw-histogram-bgl"), + entries: &[ + // The demosaiced scene-linear texture, read with + // `textureLoad` — the same one the adjust pass samples + // from, so what is counted is what the develop chain + // starts from. + 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::Storage { read_only: false }, + has_dynamic_offset: false, + min_binding_size: None, + }, + count: None, + }, + wgpu::BindGroupLayoutEntry { + binding: 2, + visibility: wgpu::ShaderStages::COMPUTE, + ty: wgpu::BindingType::Buffer { + ty: wgpu::BufferBindingType::Uniform, + has_dynamic_offset: false, + min_binding_size: None, + }, + count: None, + }, + ], + }); + + let layout = ctx + .device + .create_pipeline_layout(&wgpu::PipelineLayoutDescriptor { + label: Some("raw-histogram-layout"), + bind_group_layouts: &[Some(&bind_group_layout)], + immediate_size: 0, + }); + + let pipeline = ctx + .device + .create_compute_pipeline(&wgpu::ComputePipelineDescriptor { + label: Some("raw-histogram-pipeline"), + layout: Some(&layout), + module: &module, + entry_point: Some("main"), + compilation_options: Default::default(), + cache: None, + }); + + if let Some(err) = pollster::block_on(scope.pop()) { + return Err(GpuError::ShaderCompilation(err.to_string())); + } + + let bytes = (SLOTS * std::mem::size_of::()) as u64; + let bins = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("raw-histogram-bins"), + size: bytes, + usage: wgpu::BufferUsages::STORAGE + | wgpu::BufferUsages::COPY_SRC + | wgpu::BufferUsages::COPY_DST, + mapped_at_creation: false, + }); + let staging = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: Some("raw-histogram-staging"), + size: bytes, + usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::MAP_READ, + mapped_at_creation: false, + }); + let dims = ctx + .device + .create_buffer_init(&wgpu::util::BufferInitDescriptor { + label: Some("raw-histogram-dims"), + contents: bytemuck::bytes_of(&Dims { + width: 0, + height: 0, + pad_0: 0, + pad_1: 0, + }), + usage: wgpu::BufferUsages::UNIFORM | wgpu::BufferUsages::COPY_DST, + }); + + Ok(Self { + ctx: ctx.clone(), + pipeline, + bind_group_layout, + bins, + staging, + dims, + }) + } + + /// TRACES: FR-CULL-3 + /// Count one scene-linear frame. + /// + /// `texture` must be linear, camera-native and normalised so that 1.0 is + /// the sensor's white level — which is what [`crate::Demosaicer::run`] + /// produces and what nothing else in this crate does. Handing it the + /// display frame instead would produce a perfectly well-formed plot of the + /// wrong quantity, on an axis whose origin means nothing there, so callers + /// must check [`crate::DemosaicedImage::is_non_linear`] first: a texture + /// that came from a JPEG carries no sensor scale to measure headroom + /// against, and the honest answer for one is that there is no reading + /// rather than a reading nobody should trust. + /// + /// It must also carry `TEXTURE_BINDING`, which the demosaic output does + /// because the adjust pass samples it. + pub fn compute(&self, texture: &wgpu::Texture) -> Result { + let (width, height) = (texture.width(), texture.height()); + self.ctx.queue.write_buffer( + &self.dims, + 0, + bytemuck::bytes_of(&Dims { + width, + height, + pad_0: 0, + pad_1: 0, + }), + ); + + let view = texture.create_view(&Default::default()); + let bind_group = self + .ctx + .device + .create_bind_group(&wgpu::BindGroupDescriptor { + label: Some("raw-histogram-bg"), + layout: &self.bind_group_layout, + entries: &[ + wgpu::BindGroupEntry { + binding: 0, + resource: wgpu::BindingResource::TextureView(&view), + }, + wgpu::BindGroupEntry { + binding: 1, + resource: self.bins.as_entire_binding(), + }, + wgpu::BindGroupEntry { + binding: 2, + resource: self.dims.as_entire_binding(), + }, + ], + }); + + let mut enc = self + .ctx + .device + .create_command_encoder(&wgpu::CommandEncoderDescriptor { + label: Some("raw-histogram-encoder"), + }); + // The accumulator is reused between photographs, so it carries the + // previous one's counts until this line. Forgetting it does not fail — + // it quietly integrates every image opened this session, which looks + // like a histogram that is roughly the right shape and never quite + // about the picture on screen. + enc.clear_buffer(&self.bins, 0, None); + { + let mut pass = enc.begin_compute_pass(&wgpu::ComputePassDescriptor { + label: Some("raw-histogram-pass"), + timestamp_writes: None, + }); + pass.set_pipeline(&self.pipeline); + pass.set_bind_group(0, &bind_group, &[]); + pass.dispatch_workgroups(width.div_ceil(16), height.div_ceil(16), 1); + } + enc.copy_buffer_to_buffer(&self.bins, 0, &self.staging, 0, self.staging.size()); + self.ctx.queue.submit(Some(enc.finish())); + + let slice = self.staging.slice(..); + let (tx, rx) = std::sync::mpsc::channel(); + slice.map_async(wgpu::MapMode::Read, move |r| { + let _ = tx.send(r); + }); + await_mapping(&self.ctx, &rx)?; + + let data = slice.get_mapped_range(); + let slots: Vec = bytemuck::cast_slice::(&data).to_vec(); + drop(data); + self.staging.unmap(); + + RawHistogram::from_slots(&slots) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn ctx() -> Option { + match pollster::block_on(GpuContext::new_headless()) { + Ok(c) => Some(c), + Err(e) => { + eprintln!("skipping: no GPU adapter ({e})"); + None + } + } + } + + /// An f32 as IEEE 754 half-precision bits, for building source textures. + /// + /// Written out rather than pulled in as a dependency, exactly as + /// `demosaic.rs` does for the same reason: every value these tests upload + /// is inside `0.0..=2.0`, comfortably within f16's normal range, so the + /// subnormal and overflow cases a general converter must handle cannot + /// arise. The mantissa is truncated rather than rounded, which is why the + /// tests below place their values in the *middle* of a bin instead of on + /// its edge. + fn half(v: f32) -> u16 { + let v = v.clamp(0.0, 2.0); + if v == 0.0 { + return 0; + } + let bits = v.to_bits(); + let exp = ((bits >> 23) & 0xFF) as i32 - 127 + 15; + let mantissa = (bits >> 13) & 0x3FF; + if exp <= 0 { + return 0; + } + ((exp as u16) << 10) | mantissa as u16 + } + + /// Upload `rgb` as the `Rgba16Float` texture the pass expects, the way + /// `Demosaicer::run` hands its output over. + fn source(ctx: &GpuContext, rgb: &[[f32; 3]], width: u32, height: u32) -> wgpu::Texture { + let halves: Vec = rgb + .iter() + .flat_map(|px| [half(px[0]), half(px[1]), half(px[2]), half(1.0)]) + .collect(); + let tex = ctx.device.create_texture(&wgpu::TextureDescriptor { + label: Some("raw-histogram-test-source"), + size: wgpu::Extent3d { + width, + height, + depth_or_array_layers: 1, + }, + mip_level_count: 1, + sample_count: 1, + dimension: wgpu::TextureDimension::D2, + format: wgpu::TextureFormat::Rgba16Float, + usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST, + view_formats: &[], + }); + ctx.queue.write_texture( + wgpu::TexelCopyTextureInfo { + texture: &tex, + mip_level: 0, + origin: wgpu::Origin3d::ZERO, + aspect: wgpu::TextureAspect::All, + }, + bytemuck::cast_slice(&halves), + wgpu::TexelCopyBufferLayout { + offset: 0, + // Four channels of two bytes. + bytes_per_row: Some(width * 8), + rows_per_image: Some(height), + }, + wgpu::Extent3d { + width, + height, + depth_or_array_layers: 1, + }, + ); + ctx.queue.submit(std::iter::empty()); + tex + } + + /// The value at the centre of the bin `stops` below saturation. + /// + /// Deliberately mid-bin. On a bin edge the assertion would be testing + /// whether this machine's `log2` and this test's `powf` round a tie the + /// same way, which is a question about two libraries rather than about the + /// reduction — and the kind of disagreement that fails once in a thousand + /// runs and looks like flakiness. + fn mid_bin(bin_below_top: usize) -> f32 { + 2f32.powf(-((bin_below_top as f32) + 0.5) / BINS_PER_STOP as f32) + } + + #[test] + fn the_axis_runs_from_saturation_downward_in_stops() { + // Arithmetic, so no device. The single most consequential convention + // in the file, and the one every reading of the plot depends on: if + // the axis were inverted the histogram would still look like a + // histogram, and every headroom judgement made from it would be + // exactly backwards. + assert_eq!(RawHistogram::stops(BINS - 1), 0.0, "the top bin is saturation"); + assert_eq!(RawHistogram::stops(BINS - 1 - BINS_PER_STOP), 1.0); + assert_eq!(RawHistogram::stops(BINS - 1 - 8 * BINS_PER_STOP), 8.0); + // The floor is one bin short of `STOPS`, because bin 0 is the end of + // the axis rather than the start of another stop. + assert!(RawHistogram::stops(0) < STOPS as f32); + assert!(RawHistogram::stops(0) > STOPS as f32 - 0.1); + // Monotonic: further down the axis is always more stops below. + for bin in 1..BINS { + assert!( + RawHistogram::stops(bin) < RawHistogram::stops(bin - 1), + "bin {bin} is not below bin {}", + bin - 1 + ); + } + // Past the end is clamped, not a panic — see the doc comment. + assert_eq!(RawHistogram::stops(BINS), 0.0); + } + + #[test] + fn a_saturated_frame_lands_in_the_top_bin_and_is_counted_as_clipped() { + // The arithmetic at its most checkable, at the end of the axis that + // matters most: 64x64 pixels at the white level must produce exactly + // 4096 in bin 255, nothing anywhere else, and 4096 clipped. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + let rgb = vec![[1.0f32, 1.0, 1.0]; 64 * 64]; + let hist = pass.compute(&source(&ctx, &rgb, 64, 64)).expect("compute"); + + assert_eq!(hist.pixels(), 4096); + assert_eq!(hist.red()[BINS - 1], 4096); + assert_eq!(hist.green()[BINS - 1], 4096); + assert_eq!(hist.blue()[BINS - 1], 4096); + assert_eq!(hist.brightest()[BINS - 1], 4096); + assert_eq!( + hist.red().iter().filter(|c| **c > 0).count(), + 1, + "one value can only occupy one bin" + ); + assert_eq!(hist.saturated(), 4096); + assert_eq!(hist.at_black(), 0); + } + + #[test] + fn a_frame_at_the_black_level_lands_in_the_bottom_bin() { + // Zero has no logarithm, so the bottom of the axis is a special case + // in the shader rather than a value that falls out of the formula. A + // frame of it must land in bin 0 and be counted as black-clipped — + // getting this wrong produces a NaN bin index, which on most hardware + // is bin 0 anyway and on some is not. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + let rgb = vec![[0.0f32, 0.0, 0.0]; 32 * 32]; + let hist = pass.compute(&source(&ctx, &rgb, 32, 32)).expect("compute"); + + assert_eq!(hist.pixels(), 1024); + assert_eq!(hist.red()[0], 1024); + assert_eq!(hist.brightest()[0], 1024); + assert_eq!(hist.at_black(), 1024); + assert_eq!(hist.saturated(), 0); + } + + /// Bins the ramp below walks, from the top of the axis downward. + /// + /// Twelve stops rather than the whole sixteen, and the reason is the + /// source format rather than the reduction. `Rgba16Float`'s smallest + /// *normal* value is 2^-14, so the bottom two stops of the axis can only + /// be held as f16 subnormals — which many GPUs flush to zero — and a test + /// that walked into them would be measuring the driver's denormal policy + /// rather than this shader. Twelve stops is past the noise floor of every + /// sensor this will meet, so nothing a photograph actually contains is + /// left unchecked. + const RAMP: usize = 12 * BINS_PER_STOP; + + #[test] + fn every_bin_the_axis_can_hold_lands_where_it_belongs() { + // A ramp down the axis, four pixels per bin, each value placed at the + // centre of the bin it belongs in. This is the test that catches an + // off-by-one in the binning or a `BINS_PER_STOP` that disagrees across + // the language boundary — either shifts the whole plot along the axis, + // which on a real photograph looks like nothing at all and is a + // headroom figure out by a stop. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + let (w, h) = (RAMP as u32, 4u32); + let mut rgb = Vec::with_capacity((w * h) as usize); + for _ in 0..h { + for x in 0..w { + // x = 0 is the top bin, so `x` is also the number of bins + // below saturation. + let v = mid_bin(x as usize); + rgb.push([v, v, v]); + } + } + let hist = pass.compute(&source(&ctx, &rgb, w, h)).expect("compute"); + + assert_eq!(hist.pixels(), w * h); + for below in 0..RAMP { + let bin = BINS - 1 - below; + assert_eq!( + hist.red()[bin], + h, + "{below} bins below saturation should hold exactly {h} pixels" + ); + // Neutral, so the brightest channel must land on the same bin as + // the three do. + assert_eq!(hist.brightest()[bin], h, "the envelope drifted at bin {bin}"); + } + // Nothing below the ramp, so an off-by-one that spilled a pixel past + // the end of it would show here rather than hiding in a bin the loop + // above never looks at. + assert!( + hist.red()[..BINS - RAMP].iter().all(|c| *c == 0), + "a pixel landed below the ramp" + ); + // A neutral ramp touching neither end: nothing is at the white level + // and nothing is at black, so neither counter may fire. + assert_eq!(hist.saturated(), 0); + assert_eq!(hist.at_black(), 0); + } + + #[test] + fn a_channel_saturating_alone_is_seen_and_the_others_are_not_dragged_with_it() { + // **The claim the whole instrument rests on**, and the one a + // luma-weighted or balanced reduction would fail. A red sunset clips + // red while green and blue still hold two stops — that pixel is + // clipped, its red must be at the top of the axis, and its green and + // blue must be where they actually are. A readout that averaged the + // three would report a comfortable exposure over a channel that is + // already gone. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + // Two stops below saturation, mid-bin. + let two_stops = mid_bin(2 * BINS_PER_STOP); + let rgb = vec![ + [1.0f32, two_stops, two_stops], + [1.0, two_stops, two_stops], + [two_stops, two_stops, two_stops], + [two_stops, two_stops, two_stops], + ]; + let hist = pass.compute(&source(&ctx, &rgb, 4, 1)).expect("compute"); + + assert_eq!(hist.pixels(), 4); + assert_eq!(hist.saturated(), 2, "two pixels have a channel at the ceiling"); + assert_eq!(hist.at_black(), 0); + assert_eq!(hist.red()[BINS - 1], 2); + + let two_below = BINS - 1 - 2 * BINS_PER_STOP; + assert_eq!(hist.red()[two_below], 2, "the unclipped pixels' red moved"); + assert_eq!( + hist.green()[two_below], + 4, + "green was dragged along by red's clipping" + ); + assert_eq!(hist.blue()[two_below], 4); + // The envelope follows the highest channel, which for the clipped + // pixels is red. + assert_eq!(hist.brightest()[BINS - 1], 2); + assert_eq!(hist.brightest()[two_below], 2); + } + + #[test] + fn overshoot_above_the_white_level_folds_into_the_top_bin() { + // The demosaic clamps each *photosite* at 1.0, but the Malvar + // correction can overshoot upward between two clipped ones, so values + // above 1.0 do reach this pass. `log2` of them is negative and a + // signed bin index would wrap to somewhere near the bottom of the + // axis — a blown highlight drawn as deep shadow, which is the single + // most misleading thing this plot could do. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + let rgb = vec![[1.4f32, 1.05, 1.0]; 8 * 8]; + let hist = pass.compute(&source(&ctx, &rgb, 8, 8)).expect("compute"); + + assert_eq!(hist.pixels(), 64); + assert_eq!(hist.red()[BINS - 1], 64); + assert_eq!(hist.green()[BINS - 1], 64); + assert_eq!(hist.blue()[BINS - 1], 64); + assert_eq!(hist.saturated(), 64); + assert_eq!(hist.red()[0], 0, "an overshoot wrapped to the bottom bin"); + } + + #[test] + fn an_edge_tile_is_neither_dropped_nor_counted_twice() { + // A size that is not a multiple of the 16x16 workgroup, so the last + // row and column of workgroups run off the image. The denominator is + // where this shows: `pixels` is summed from the red series, so a tile + // that failed to merge undercounts it and one merged twice doubles it, + // and a percentage computed against either is wrong in a way nobody + // can see on the plot. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + let (w, h) = (101u32, 37u32); + let mut rgb = Vec::with_capacity((w * h) as usize); + let mut state = 0x2545_F491_4F6C_DD1Du64; + for _ in 0..w * h { + // xorshift64*, so the frame is identical on every machine and a + // failure can be reproduced rather than merely observed. + state ^= state >> 12; + state ^= state << 25; + state ^= state >> 27; + let below = (state.wrapping_mul(0x2545_F491_4F6C_DD1D) >> 56) as usize % RAMP; + let v = mid_bin(below); + rgb.push([v, v, v]); + } + let hist = pass.compute(&source(&ctx, &rgb, w, h)).expect("compute"); + + assert_eq!(hist.pixels(), w * h, "an edge tile was dropped or doubled"); + // Every series counts the same pixels, so every series must sum to the + // same total — a merge that lost one channel's tile would not move the + // denominator at all. + for series in [hist.green(), hist.blue(), hist.brightest()] { + assert_eq!(series.iter().sum::(), w * h); + } + } + + #[test] + fn a_second_photograph_replaces_the_first_rather_than_adding_to_it() { + // The accumulator is reused across images, so a missing clear + // integrates every photograph opened this session. The symptom is + // subtle and awful on a culling pass in particular: the plot keeps a + // plausible shape and slowly stops responding to the file on screen, + // because each new image's contribution shrinks against the running + // total of the folder behind it. + let Some(ctx) = ctx() else { return }; + let pass = RawHistogramPass::new(&ctx).expect("pass"); + + let bright = vec![[mid_bin(BINS_PER_STOP); 3]; 16 * 16]; + let dark = vec![[mid_bin(6 * BINS_PER_STOP); 3]; 16 * 16]; + + let one_stop = BINS - 1 - BINS_PER_STOP; + let six_stops = BINS - 1 - 6 * BINS_PER_STOP; + + let first = pass.compute(&source(&ctx, &bright, 16, 16)).expect("first"); + assert_eq!(first.red()[one_stop], 256); + + let second = pass.compute(&source(&ctx, &dark, 16, 16)).expect("second"); + assert_eq!( + second.pixels(), + 256, + "the previous photograph was still counted" + ); + assert_eq!(second.red()[one_stop], 0); + assert_eq!(second.red()[six_stops], 256); + } +} diff --git a/core/dr-gpu/src/shaders/raw_histogram.wgsl b/core/dr-gpu/src/shaders/raw_histogram.wgsl new file mode 100644 index 0000000..e5e86ce --- /dev/null +++ b/core/dr-gpu/src/shaders/raw_histogram.wgsl @@ -0,0 +1,156 @@ +// TRACES: FR-CULL-3 +// A reduction of the *scene-linear* frame onto a stops-below-saturation axis. +// +// The shader beside this one, `histogram.wgsl`, counts the frame the display +// is about to show: an 8-bit code value, after white balance, the camera +// matrix, the base curve, the tone curve and the output transform. This one +// counts the texture the demosaic wrote, before any of that. The two differ in +// exactly one place — the axis — and everything else here is deliberately the +// same construction, because the two reductions have the same shape and any +// divergence between them would be a difference nobody chose. +// +// **Why the axis is logarithmic and measured downward from 1.0.** The texture +// is normalised by the sensor's own black and white levels (see +// `demosaic.wgsl`), so 1.0 *is* saturation by construction and there is no +// other meaningful origin. And the question a photographer asks of raw data is +// "how much headroom is left", which is a question in stops: a linear axis +// spends half its width on the top stop and crushes the eleven below it into +// the leftmost pixel, which is why nobody has ever drawn a useful linear raw +// histogram. +// +// So bin 255 is the top 1/16 stop below saturation, bin 239 is one stop below, +// bin 0 is 15.9375 stops below **and everything darker**. A bin is 1/16 of a +// stop, which is about 4.4% in value — fine enough that a clipped highlight is +// visibly its own column rather than smeared over the top quarter of the plot. +// +// **Counted per workgroup first, then merged**, for the reason +// `histogram.wgsl` gives at length: a photograph is not noise, tens of +// thousands of adjacent pixels land in one bin, and having every invocation +// contend for one global atomic serialises the dispatch. Four kilobytes of +// workgroup storage against a 16 KB floor. + +const BINS: u32 = 256u; +// Bins per stop. Must equal `BINS_PER_STOP` on the Rust side, which turns a +// bin index back into a stops figure for the readout — the two disagreeing +// would put a correct plot under a wrong number. +const BINS_PER_STOP: f32 = 16.0; + +// Two counters past the four series, in the same buffer and for the same +// reason `histogram.wgsl` puts its there: they are gathered from the texel +// read the binning already did, and a second reduction to answer them would be +// a second pass over the whole image for two integers. +const SATURATED: u32 = 4u * BINS; +const AT_BLACK: u32 = 4u * BINS + 1u; +const SLOTS: u32 = 4u * BINS + 2u; +// 16x16. Stated as a constant because the clear and merge loops stride by it, +// and a workgroup size that disagreed would leave slots uncleared. +const THREADS: u32 = 256u; + +// How close to 1.0 counts as saturated. +// +// **Not `>= 1.0`, and the margin is arithmetic rather than taste.** A photosite +// at or above the declared white level leaves `demosaic.wgsl` clamped to +// exactly 1.0, but it then travels through an f16 texture and, at a green +// site, through an interpolation kernel whose terms are added in a different +// order than a reader would expect. f16 spaces its values 2^-11 apart just +// below 1.0, so a value that was 1.0 can arrive an ulp or two under it. This +// margin is four of those, and 0.0014 of a stop — a hundredth of the width of +// one bin — so it cannot move a column of the plot, only stop the counter +// missing a genuinely blown pixel. +const SATURATION: f32 = 0.999; + +struct Dims { + width: u32, + height: u32, + // std140 rounds a uniform block up to 16 bytes; named rather than left + // implicit so the Rust side's padding is visibly the same shape. + pad_0: u32, + pad_1: u32, +} + +@group(0) @binding(0) var source: texture_2d; +@group(0) @binding(1) var bins: array>; +@group(0) @binding(2) var dims: Dims; + +var tile: array, SLOTS>; + +// The bin a scene-linear value belongs in. +// +// Zero and below land in bin 0: `log2` of them is not a number, and a +// photosite at or under the black level has no signal to place on a +// logarithmic axis anyway — it is the bottom of the scale, which is where the +// bottom bin is. +// +// Above 1.0 lands in bin 255. Such values exist: the demosaic clamps each +// *photosite* at the white level, but the Malvar correction term can overshoot +// upward between two clipped ones, so the interpolated output can pass 1.0 by +// a little. That is an artefact of the reconstruction and not headroom above +// saturation, and folding it into the top bin says exactly that. +fn bin_of(v: f32) -> u32 { + if (v <= 0.0) { + return 0u; + } + // Stops below saturation, in sixteenths, floored. `-log2` because the axis + // increases downward from 1.0 and a bin index increases upward. + let steps = floor(-log2(v) * BINS_PER_STOP); + return (BINS - 1u) - u32(clamp(steps, 0.0, f32(BINS - 1u))); +} + +@compute @workgroup_size(16, 16, 1) +fn main( + @builtin(global_invocation_id) gid: vec3, + @builtin(local_invocation_index) lid: u32, +) { + for (var i = lid; i < SLOTS; i = i + THREADS) { + atomicStore(&tile[i], 0u); + } + workgroupBarrier(); + + // Guarded rather than dispatched exactly: the workgroup is 16x16 and an + // image is not, so the last row and column of workgroups run off the edge. + if (gid.x < dims.width && gid.y < dims.height) { + let texel = textureLoad(source, vec2(i32(gid.x), i32(gid.y)), 0); + let r = texel.r; + let g = texel.g; + let b = texel.b; + + // The fourth series is the **brightest** channel at each pixel, where + // the display histogram's fourth series is Rec.709 luma. Luma would be + // meaningless here: these are camera-native values with the as-shot + // white balance still un-applied, so red and blue sit a stop or more + // below green on a neutral subject and any weighted sum of them is a + // number about nothing. The brightest channel, by contrast, is exactly + // the quantity the headroom question is about — it is the one that + // reaches saturation first and decides whether the pixel is + // recoverable. + let brightest = max(r, max(g, b)); + + atomicAdd(&tile[bin_of(r)], 1u); + atomicAdd(&tile[BINS + bin_of(g)], 1u); + atomicAdd(&tile[2u * BINS + bin_of(b)], 1u); + atomicAdd(&tile[3u * BINS + bin_of(brightest)], 1u); + + // Any channel, not all three, exactly as the display histogram counts + // it: a saturated red photosite has no gradation left in it however + // much headroom green and blue still hold, and a sunset or a red + // jersey is precisely the subject that clips one channel first. + if (brightest >= SATURATION) { + atomicAdd(&tile[SATURATED], 1u); + } + // And at the other end: a channel that reached the black level has no + // signal, only the read noise the normalisation clamped away. + if (min(r, min(g, b)) <= 0.0) { + atomicAdd(&tile[AT_BLACK], 1u); + } + } + workgroupBarrier(); + + for (var i = lid; i < SLOTS; i = i + THREADS) { + let count = atomicLoad(&tile[i]); + // Most bins of most workgroups are empty — a 16x16 tile can touch 256 + // of 1026 slots at the very most, and usually far fewer. + if (count != 0u) { + atomicAdd(&bins[i], count); + } + } +} diff --git a/docs/architecture.md b/docs/architecture.md index 2b3192d..2c5e4f2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -369,6 +369,10 @@ RawImage (sensor data, CPU) Working precision is f16 in a linear wide-gamut space, quantising once at the output transform. +There is a second reduction that does not hang off the bottom of this chain. The raw histogram +(FR-CULL-3) taps the demosaiced scene-linear texture directly — the box four rows from the top — +because what it measures is the file rather than the render. See §5.5. + ### 5.3 Tiling and scheduling Work decomposes into tiles (default 256×256) scheduled by priority class: @@ -402,8 +406,35 @@ The histogram is a compute-shader reduction into a small storage buffer, read on most, and only the *bins* — never image data. A per-frame CPU readback of pixels would reintroduce exactly the stall §6.1 exists to prevent. -Raw-domain histograms for culling (FR-CULL-3) reduce over the pre-demosaic texture, which is why -they can report headroom the embedded JPEG's histogram cannot. +**Two reductions, not one.** The display histogram (FR-DSP-7) counts the frame the output transform +produced: its axis is the output code value, and a clipped bin means a highlight that is gone as the +image currently stands. The raw histogram for culling (FR-CULL-3) counts the **demosaiced +scene-linear texture** — before white balance, the camera matrix, the base curve and the tone chain +— on an axis of stops below sensor saturation, which is how it reports headroom the embedded JPEG's +histogram cannot. A culling decision needs the second, an export decision needs the first, and +neither answers for the other. Both are drawn by the same panel and chosen between. + +**The raw reduction runs after the demosaic, not before it.** This section previously specified the +pre-demosaic CFA samples, and that is the more complete instrument: it counts photosites rather than +pixels, so no interpolation smears a clipped site across its neighbours, and it can name which +channel of the mosaic saturated first. It was not worth its cost. `Demosaicer::run` uploads the +packed sample buffer and drops it the moment the dispatch is encoded; retaining it is 48 MB at 24 MP +and 120 MB at 60 MP, resident per open photograph whether or not anyone looks at the histogram, on a +platform §6.2 exists because memory is scarce on. The record here is of the choice, not of the +intent — a specification the code contradicts is worse than either of the two things it could say. + +The texture that is reduced over instead is camera-native, unbalanced, unmatrixed and uncurved, and +is normalised by the sensor's own black and white levels: 1.0 is saturation by construction, so the +distribution below it is the headroom question with no calibration to carry and no origin to choose. +What it cannot answer, and the CFA reduction could, is **which photosite** clipped rather than which +pixel, and **how far above** the white level a sample reached — the demosaic clamps there, for its +own good reasons, so "at saturation" and "a stop past it" share a bin. Both limits are stated again +where the code is, in `core/dr-gpu/src/raw_histogram.rs`. + +It is also the one reduction on this path that is not per frame. Nothing downstream of the demosaic +can move a count in it, so it is computed once per photograph and cached — which is what makes it +affordable during a cull, where the display histogram's per-frame cost would be paid three thousand +times. ### 5.6 Device loss diff --git a/docs/outstanding.md b/docs/outstanding.md index 3e6011e..64fe2fe 100644 --- a/docs/outstanding.md +++ b/docs/outstanding.md @@ -21,10 +21,10 @@ percentage. Where the honest answer is "this requirement should be amended rathe so — an unbuilt requirement that nobody intends to build is worse than a deferred one, because it keeps costing attention. -**Four of these are being built right now**, in parallel worktrees, and are marked **⟳ in -progress** where they appear: focus peaking (part of FR-CULL-3), burst grouping (FR-CULL-5), -Flatpak packaging (FR-PLAT-LIN-3), and Android platform integration (FR-PLAT-AND-2/4/5/6). Strike -those lines as they land rather than rewriting around them. +**Three of these are being built right now**, in parallel worktrees, and are marked **⟳ in +progress** where they appear: burst grouping (FR-CULL-5), Flatpak packaging (FR-PLAT-LIN-3), and +Android platform integration (FR-PLAT-AND-2/4/5/6). Strike those lines as they land rather than +rewriting around them. --- @@ -70,23 +70,26 @@ somebody reads the matrix. ## 2. Culling — the stated differentiator, half built -[D11](requirements.md) names culling "the core differentiator". FR-CULL-1, -2, -4 and -8 through -12 -are built. Four are not. +[D11](requirements.md) names culling "the core differentiator". FR-CULL-1, -2, -3, -4 and -8 through +-12 are built. Three are not. -**FR-CULL-3 — Raw-truth overlays. All three bullets, unbuilt.** Focus peaking does not exist -anywhere; the string appears zero times in the tree. +**FR-CULL-3 — Raw-truth overlays. Built, all three bullets.** Focus peaking is +`core/dr-gpu/src/focus.rs` and `ui/dr-ui/src/peaking.rs`; the raw histogram and the raw clipping +indicators are `core/dr-gpu/src/raw_histogram.rs` and the second reading of the panel in +`histogram.slint`. -The other two are easy to mistake for present, and are not. A histogram and clipping indicators do -exist — `dr-gpu/src/histogram.rs`, `ui/dr-ui/src/histogram.rs`, the panel in `histogram.slint` — -but they are tagged FR-DSP-7 and they answer the opposite question. They read `AdjustPass`'s 8-bit -output and count clipping as `r == 255`, which is to say they describe **the frame the display is -about to show**, after the whole develop chain has run. FR-CULL-3 asks for the histogram of the -*sensor data*, on the explicit grounds that a rendered image "systematically lies about what is -recoverable in the raw". A readout that measures the render cannot answer that however it is -presented, so this is not a matter of moving an existing widget into the culling view. +Worth recording, because it is the thing this entry previously got wrong and the next reader will +have to check again. The display histogram — `dr-gpu/src/histogram.rs`, `ui/dr-ui/src/histogram.rs` +— is **not** this requirement and never was: it is tagged FR-DSP-7, it reads `AdjustPass`'s 8-bit +output, it counts clipping as `r == 255`, and so it describes the frame the display is about to +show, after the whole develop chain. FR-CULL-3 asks for the *sensor data*, on the explicit grounds +that a rendered image "systematically lies about what is recoverable in the raw". The two now sit in +one panel behind a chip row, which is the arrangement that keeps them from being mistaken for each +other: they answer different questions and both are true. -The requirement exists because a culling decision made against a rendered preview is a decision made -against the wrong image, and the whole of it is still to build. **⟳ in progress** (focus peaking). +What the raw reduction cannot answer is written down rather than left to be discovered — +[architecture.md §5.5](architecture.md) records why it reduces over the demosaiced texture instead +of the CFA samples §5.5 originally specified, and what that costs in what it can say. **FR-CULL-5 — Burst and near-duplicate grouping.** Absent. Worth knowing before it is built: `core/dr-face/src/calibrate.rs` already *assumes* it exists — "since FR-CULL-5 already groups diff --git a/ui/dr-ui/src/develop.rs b/ui/dr-ui/src/develop.rs index 42d8fe8..cd4bf25 100644 --- a/ui/dr-ui/src/develop.rs +++ b/ui/dr-ui/src/develop.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use dr_decode::RawImage; use dr_gpu::{ AdjustPass, DemosaicedImage, Demosaicer, FocusPeakPass, FocusPeaking, GpuContext, Histogram, - HistogramPass, MaskPass, + HistogramPass, MaskPass, RawHistogram, RawHistogramPass, }; use dr_pipeline::mask::{MaskLayer, MaskSource}; @@ -724,6 +724,26 @@ pub struct DevelopSession { /// photographer loses the histogram and keeps the photograph. histogram: Option, /// TRACES: FR-CULL-3 + /// The raw-domain reduction, on the same terms as the display one above: + /// optional, because a session that cannot count the sensor data is still + /// a session that can develop it. + raw_histogram: Option, + /// The raw reading, once taken. + /// + /// **Cached, where the display histogram is recomputed every settled + /// frame, and the difference is not an optimisation.** This measures the + /// demosaiced source, which nothing downstream of the demosaic can change: + /// no slider, no crop, no zoom, no output space moves a single count in + /// it. Recomputing it per frame would be a dispatch and a device sync + /// point spent to arrive back at the number already held — and on the + /// culling pass FR-CULL-3 is written for, that is a cost paid three + /// thousand times over. + /// + /// `None` until first asked for, and it stays `None` on a file with no + /// sensor data behind it. The session is one photograph and the demosaiced + /// source is fixed for its life, so there is no invalidation to get wrong. + raw_counts: Option, + /// TRACES: FR-CULL-3 /// The focus-peaking overlay, on the same terms as the histogram above: /// optional, because a device that cannot compile the pass is still a /// device that can develop the photograph. What is lost is an instrument, @@ -909,6 +929,10 @@ impl DevelopSession { histogram: HistogramPass::new(ctx) .inspect_err(|e| log::warn!("no histogram on this device: {e}")) .ok(), + raw_histogram: RawHistogramPass::new(ctx) + .inspect_err(|e| log::warn!("no raw histogram on this device: {e}")) + .ok(), + raw_counts: None, peak: FocusPeakPass::new(ctx) .inspect_err(|e| log::warn!("no focus peaking on this device: {e}")) .ok(), @@ -2927,6 +2951,64 @@ impl DevelopSession { .ok() } + /// TRACES: FR-CULL-3 + /// Whether there is sensor data behind this session at all. + /// + /// False for the JPEG path, where [`DemosaicedImage::from_rgba8`] built + /// the source from an already-rendered image. There is no white level in + /// such a file and so no scale to measure headroom against: the honest + /// answer for one is that the raw instrument has nothing to say, which is + /// a different statement from a device that could not build the pass, and + /// the panel says the two differently. + pub fn has_sensor_data(&self) -> bool { + !self.demosaiced.is_non_linear() + } + + /// TRACES: FR-CULL-3 + /// Count the sensor data this photograph was demosaiced from. + /// + /// **This is the other histogram, not a variant of the one above**, and + /// the two answer questions that a culling decision needs kept apart. + /// [`Self::histogram`] counts the frame on the canvas, after white + /// balance, the camera matrix, the base curve, the tone curve and the + /// output transform: a clipped bin there is a highlight that is gone as + /// the image currently stands. This counts the demosaiced scene-linear + /// texture, before any of that, on an axis of stops below sensor + /// saturation — so a clipped bin here is a highlight that is gone *in the + /// file*, and no edit will bring it back. FR-CULL-3 exists because the + /// tools that offer the second reading do not develop, and the ones that + /// develop offer only the first — "no shipping tool combines both". + /// + /// **It describes the whole frame, not the visible region**, which is the + /// opposite of what [`Self::histogram`] does and deliberate. A crop and a + /// zoom change what is on screen; neither changes what the sensor + /// recorded, and the question this answers — how much latitude does this + /// exposure have — is asked of the capture rather than of the view. + /// + /// Computed once and cached, for the reason `raw_counts` gives. + /// + /// `None` where the file carries no sensor data, or where the device could + /// not build the reduction. The caller distinguishes those with + /// [`Self::has_sensor_data`]. + pub fn raw_histogram(&mut self) -> Option { + if self.raw_counts.is_none() { + if !self.has_sensor_data() { + return None; + } + // Scoped so the shared borrow of the pass and of the source ends + // before the cache is written, rather than relying on the reader + // to see that the two field paths are disjoint. + let counted = { + let pass = self.raw_histogram.as_ref()?; + pass.compute(self.demosaiced.texture()) + .inspect_err(|e| log::warn!("the raw histogram failed: {e}")) + .ok() + }; + self.raw_counts = counted; + } + self.raw_counts.clone() + } + /// TRACES: FR-CULL-3 /// Whether this device could build the focus-peaking overlay. /// @@ -5817,6 +5899,142 @@ mod tests { assert_eq!(hist.clipped_shadows(), 32 * 64); } + /// A flat Bayer frame whose every photosite normalises to `level`. + /// + /// Black at zero and a power-of-two white level, so the normalisation is + /// exact and the value the raw histogram sees is the one this asked for + /// rather than one rounded by two divisions. + fn flat_raw(size: u32, level: f32) -> RawImage { + const WHITE: u16 = 16384; + let sample = (level * f32::from(WHITE)).round() as u16; + RawImage { + width: size, + height: size, + data: vec![sample; (size * size) as usize], + cfa_pattern: dr_decode::CfaPattern::Rggb, + black_level: [0; 4], + white_level: WHITE, + wb_coeffs: [1.0, 1.0, 1.0, 1.0], + color_matrix: None, + base_curve: dr_decode::BaseCurve::IDENTITY, + crop: dr_decode::CropRect { + x: 0, + y: 0, + width: size, + height: size, + }, + } + } + + /// TRACES: FR-CULL-3 + #[test] + fn the_raw_histogram_describes_the_file_and_not_the_view() { + // **The property that makes it a second instrument rather than a + // second rendering of the first**, and the one every other test here + // would pass without. The display histogram beside it deliberately + // follows the edit and the visible region — that is what FR-DSP-7 + // asks of it. This must do neither: cropping away half the photograph + // changes what is on the canvas and changes nothing about what the + // sensor recorded, and a culler asking how much latitude an exposure + // has is asking about the capture. + // + // Reading the adjusted output by mistake would pass a plausible-looking + // plot back — which is exactly why this asserts the *denominator* and + // the bin, not merely that something was counted. + let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else { + log::warn!("no GPU adapter; skipping"); + return; + }; + + // 0.234253 is the centre of the bin 33 sixteenths below saturation — + // mid-bin on purpose, so the assertion is about the reduction rather + // than about how this machine's `log2` rounds an exact tie. + let raw = flat_raw(16, 3838.0 / 16384.0); + let mut session = + DevelopSession::open(&ctx, &raw, dr_types::Orientation::NORMAL).expect("session"); + + let before = session.raw_histogram().expect("a raw session must count"); + assert_eq!(before.pixels(), 16 * 16); + assert_eq!( + before.red()[dr_gpu::RAW_HISTOGRAM_BINS - 1 - 33], + 16 * 16, + "a flat frame two stops down did not land in one bin" + ); + assert_eq!(before.saturated(), 0, "nothing here is at the white level"); + assert_eq!(before.at_black(), 0); + + session.set_crop(CropRect { + x: 0.0, + y: 0.0, + width: 0.5, + height: 1.0, + }); + session.render(16, 16).expect("render"); + + // The display histogram followed the crop, as it is supposed to. + // Asserted as an inequality rather than an exact figure: how a half + // crop of a 16px frame rounds to a viewport is `AdjustPass`'s + // business and has its own tests, and pinning it here would make this + // test fail for a reason it is not about. + let shown = session.histogram().expect("histogram"); + assert!( + shown.pixels() < 16 * 16, + "the crop did not reach the display histogram, so this proves nothing" + ); + + // The raw one did not. + let after = session.raw_histogram().expect("raw histogram"); + assert_eq!( + after, before, + "the raw reading followed the crop, so it is measuring the render" + ); + } + + /// TRACES: FR-CULL-3 + #[test] + fn a_blown_frame_reads_as_clipped_in_the_raw_domain() { + // The other end, and the reason the requirement exists. Every + // photosite at the white level is a photograph with no highlight + // headroom left in the file — no edit recovers it — and the instrument + // has to say so in the same terms whatever the develop chain currently + // makes of it. + let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else { + log::warn!("no GPU adapter; skipping"); + return; + }; + + let raw = flat_raw(16, 1.0); + let mut session = + DevelopSession::open(&ctx, &raw, dr_types::Orientation::NORMAL).expect("session"); + + let hist = session.raw_histogram().expect("raw histogram"); + assert_eq!(hist.pixels(), 16 * 16); + assert_eq!(hist.saturated(), 16 * 16); + assert_eq!(hist.red()[dr_gpu::RAW_HISTOGRAM_BINS - 1], 16 * 16); + } + + /// TRACES: FR-CULL-3 + #[test] + fn a_file_with_no_sensor_data_has_no_raw_reading_rather_than_a_wrong_one() { + // The JPEG path. Its source texture is gamma-encoded and carries no + // white level, so there is no scale to measure headroom against — and + // counting it anyway would produce a confident plot of a quantity that + // does not exist, which is the failure mode an instrument must not + // have. The panel says there is nothing to say. + let Ok(ctx) = pollster::block_on(dr_gpu::GpuContext::new_headless()) else { + log::warn!("no GPU adapter; skipping"); + return; + }; + + let rgba = split_frame(16); + let mut session = + DevelopSession::open_rgb(&ctx, &rgba, 16, 16, dr_types::Orientation::NORMAL) + .expect("session"); + + assert!(!session.has_sensor_data()); + assert!(session.raw_histogram().is_none()); + } + #[test] fn routing_indices_map_back_to_the_right_parameter() { // A wrong index would silently move the wrong slider's value, which diff --git a/ui/dr-ui/src/histogram.rs b/ui/dr-ui/src/histogram.rs index b70c5d2..ea52051 100644 --- a/ui/dr-ui/src/histogram.rs +++ b/ui/dr-ui/src/histogram.rs @@ -1,6 +1,17 @@ -//! TRACES: FR-DSP-7 +//! TRACES: FR-DSP-7 | FR-CULL-3 //! Turning bin counts into something a 280-pixel column can be read from. //! +//! **Two instruments through one panel.** `dr_gpu` produces two reductions +//! that answer different questions — [`dr_gpu::Histogram`] counts the frame the +//! display is about to show (FR-DSP-7), [`dr_gpu::RawHistogram`] counts the +//! sensor data the file actually holds (FR-CULL-3) — and both are drawn by the +//! same plot, from the same [`HistogramView`], with a chip row to choose +//! between them. That is why the axis caption, the two clipping titles and the +//! empty-state line are all fields of the view rather than literals in Slint: +//! the words differ between the two readings as much as the numbers do, and a +//! panel that drew the raw counts under headings saying Shadows and Highlights +//! would be mislabelling them precisely where it matters. +//! //! `dr_gpu` counts; this decides what the counting looks like. The split is //! ARCH §4.3a's: how many columns a panel can show, which peak to scale //! against and when a handful of specular pixels is worth an alarm are all @@ -14,7 +25,7 @@ //! right shape look equally plausible. So they are all reachable without a GPU, //! a window or a photograph. -use dr_gpu::{Histogram, HISTOGRAM_BINS}; +use dr_gpu::{Histogram, RawHistogram, HISTOGRAM_BINS, RAW_HISTOGRAM_BINS}; use crate::HistogramView; @@ -40,14 +51,48 @@ pub(crate) const COLUMNS: usize = 64; /// stops being an artefact and starts being a decision. const CLIP_VISIBLE: f32 = 0.001; -/// The whole panel's state, for one counted frame. +/// The words the display reading is drawn under. +/// +/// Here rather than in the panel because the raw reading beside it uses +/// different ones for the same two counters, and a heading that stayed put +/// while the numbers under it changed meaning would be worse than no heading. +const DISPLAY_LOW: &str = "Shadows"; +const DISPLAY_HIGH: &str = "Highlights"; +/// What the plot's horizontal axis is, said once in the chip row's hint. +/// +/// Short on purpose, and this is a layout constraint rather than a stylistic +/// one: the hint is an unwrapped `Text` in a `FieldRow`, so its natural width +/// is this panel's preferred width, and the develop column takes the largest +/// preferred width of any panel in it. A sentence here would hold the whole +/// sidebar open. +const DISPLAY_AXIS: &str = "output levels, 0 to 255"; +/// What the plot says when nothing has been counted. +const NO_FRAME: &str = "no frame yet"; +/// What a figure reads before there is a frame behind it. +const UNKNOWN: &str = "—"; + +/// The two reductions must agree on their bin count. +/// +/// [`scaled`] folds either of them, and the fold is only free of a comb +/// because [`COLUMNS`] divides the bin count exactly. Asserted at compile time +/// rather than tested, so a change in `dr_gpu` stops the build here with this +/// sentence attached instead of drawing a plot with structure the photograph +/// does not have. +const _: () = assert!(RAW_HISTOGRAM_BINS == HISTOGRAM_BINS); + +/// The same three, for the raw reading. +const RAW_LOW: &str = "Black"; +const RAW_HIGH: &str = "Saturated"; +const RAW_AXIS: &str = "stops below saturation"; + +/// The whole panel's state, for one counted display frame. pub(crate) fn view(hist: &Histogram) -> HistogramView { HistogramView { // Zero pixels means nothing has been rendered yet, not an empty // photograph — the panel draws its frame and no data rather than a flat // line, which would be a claim about an image that does not exist. available: hist.pixels() > 0, - luma: model(&scaled(hist.luma())), + overall: model(&scaled(hist.luma())), red: model(&scaled(hist.red())), green: model(&scaled(hist.green())), blue: model(&scaled(hist.blue())), @@ -55,14 +100,19 @@ pub(crate) fn view(hist: &Histogram) -> HistogramView { shadows_clipped: fraction(hist.clipped_shadows(), hist.pixels()) >= CLIP_VISIBLE, highlights_label: percentage(hist.clipped_highlights(), hist.pixels()).into(), shadows_label: percentage(hist.clipped_shadows(), hist.pixels()).into(), + low_title: DISPLAY_LOW.into(), + high_title: DISPLAY_HIGH.into(), + hint: DISPLAY_AXIS.into(), + unavailable: NO_FRAME.into(), } } -/// An empty panel — no image open, or a frame that could not be counted. +/// An empty display panel — no image open, or a frame that could not be +/// counted. pub(crate) fn empty() -> HistogramView { HistogramView { available: false, - luma: model(&[0.0; COLUMNS]), + overall: model(&[0.0; COLUMNS]), red: model(&[0.0; COLUMNS]), green: model(&[0.0; COLUMNS]), blue: model(&[0.0; COLUMNS]), @@ -72,11 +122,172 @@ pub(crate) fn empty() -> HistogramView { // frame that does not exist is the one thing an instrument must not do. highlights_label: UNKNOWN.into(), shadows_label: UNKNOWN.into(), + low_title: DISPLAY_LOW.into(), + high_title: DISPLAY_HIGH.into(), + hint: DISPLAY_AXIS.into(), + unavailable: NO_FRAME.into(), } } -/// What a figure reads before there is a frame behind it. -const UNKNOWN: &str = "—"; +/// The same panel, for one counted sensor frame. +/// +/// Untagged, deliberately. Every judgement it makes — the fold, the scale, the +/// clipping threshold, the headroom figure, the words — belongs to a function +/// beside it that is tested without a device; this is the assembly, and a tag +/// here would claim coverage that the assertions are not making. See +/// `CONTRIBUTING.md` on closing a requirement with a test that would fail if +/// the behaviour were removed. +/// +/// **Every number here is folded and scaled by the code the display reading +/// uses**, and that is deliberate rather than convenient: the two plots are +/// drawn in the same box, in the same column, one chip apart, and a difference +/// in how they are folded or what they are scaled against would read as a +/// difference in the photograph. What changes between them is the axis and the +/// words for it, and nothing else. +/// +/// The shared fold is only sound because the two reductions have the same bin +/// count. That is a compile-time fact rather than a hope — `scaled` takes +/// `&[u32; HISTOGRAM_BINS]` and is handed `&[u32; RAW_HISTOGRAM_BINS]`, so if +/// `dr_gpu` ever changed one of them this file would stop compiling rather +/// than start drawing a comb. +pub(crate) fn raw_view(hist: &RawHistogram) -> HistogramView { + HistogramView { + available: hist.pixels() > 0, + // The brightest channel where the display reading draws luma — see + // `RawHistogram::brightest` for why a weighted sum of camera-native + // values would be a number about nothing. + overall: model(&scaled(hist.brightest())), + red: model(&scaled(hist.red())), + green: model(&scaled(hist.green())), + blue: model(&scaled(hist.blue())), + highlights_clipped: fraction(hist.saturated(), hist.pixels()) >= CLIP_VISIBLE, + shadows_clipped: fraction(hist.at_black(), hist.pixels()) >= CLIP_VISIBLE, + highlights_label: percentage(hist.saturated(), hist.pixels()).into(), + shadows_label: percentage(hist.at_black(), hist.pixels()).into(), + low_title: RAW_LOW.into(), + high_title: RAW_HIGH.into(), + // The headroom figure rather than the axis, when there is one. It is + // the answer FR-CULL-3 is written for — "the embedded JPEG's histogram + // misrepresents available highlight headroom" — and a photographer + // culling a folder wants the number, not a reminder of what the axis + // is. + hint: headroom_label(hist.brightest(), hist.pixels()).into(), + unavailable: NO_FRAME.into(), + } +} + +/// Why there is no raw reading to draw. +/// +/// Three genuinely different answers, and the panel must not flatten them into +/// one. "Nothing is open" is a state that passes; "this file has no sensor +/// data" is permanent for this photograph and says the instrument does not +/// apply; "this device could not build the reduction" says it applies and is +/// missing, which is the one worth reporting as a fault. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum RawAbsence { + /// No photograph open, or none rendered yet. + NoImage, + /// The file was never raw — a JPEG, or anything else already rendered. + NotRaw, + /// The device could not compile or run the reduction. + NoDevice, +} + +impl RawAbsence { + /// What the empty plot says. Kept to a few words for the width reason + /// [`DISPLAY_AXIS`] gives. + fn note(self) -> &'static str { + match self { + RawAbsence::NoImage => NO_FRAME, + RawAbsence::NotRaw => "no sensor data", + RawAbsence::NoDevice => "no reduction here", + } + } +} + +/// TRACES: FR-CULL-3 +/// An empty raw panel, saying which of the three reasons applies. +pub(crate) fn raw_empty(why: RawAbsence) -> HistogramView { + HistogramView { + available: false, + overall: model(&[0.0; COLUMNS]), + red: model(&[0.0; COLUMNS]), + green: model(&[0.0; COLUMNS]), + blue: model(&[0.0; COLUMNS]), + highlights_clipped: false, + shadows_clipped: false, + highlights_label: UNKNOWN.into(), + shadows_label: UNKNOWN.into(), + low_title: RAW_LOW.into(), + high_title: RAW_HIGH.into(), + hint: RAW_AXIS.into(), + unavailable: why.note().into(), + } +} + +/// TRACES: FR-CULL-3 +/// How far the brightest real content sits below sensor saturation, in stops. +/// +/// Takes the counts rather than the [`RawHistogram`] they came out of, on the +/// same principle as everything else in this file: a `RawHistogram` can only +/// be produced by a device, and every judgement here is one that shows up as a +/// wrong number rather than as a crash. +/// +/// **The top [`CLIP_VISIBLE`] of the frame is skipped**, for the same reason +/// the clipping indicator has a threshold at all: almost every photograph has +/// a few pixels at an extreme — a specular glint on chrome, a hot pixel, the +/// sun itself — and a headroom figure that read 0.0 on all of them would be a +/// figure nobody looks at twice. What is wanted is the highlight a +/// photographer is protecting, not the brightest pixel in the file. +/// +/// Measured on the brightest channel, because that is the one that saturates +/// first and so the one that decides whether a pixel survives. +/// +/// `None` where nothing has been counted — distinct from zero, which is a +/// photograph with no headroom left at all. +pub(crate) fn headroom(brightest: &[u32; RAW_HISTOGRAM_BINS], pixels: u32) -> Option { + if pixels == 0 { + return None; + } + // `max(1)` so the same count that lights the clipping indicator also moves + // this figure: without it a frame sitting exactly on the threshold would + // read "clipped" beside "three stops of headroom", and a panel that + // contradicts itself in two adjacent readouts is one nobody trusts again. + // It also keeps a small frame — a proxy, a thumbnail — from ignoring + // everything, since a thousandth of it rounds to none. + let ignore = ((pixels as f32 * CLIP_VISIBLE) as u32).max(1); + let mut above = 0u32; + for (bin, count) in brightest.iter().enumerate().rev() { + above = above.saturating_add(*count); + if above >= ignore { + return Some(RawHistogram::stops(bin)); + } + } + // Unreachable while `pixels` is non-zero — the counts sum to it, and + // `ignore` is a thousandth of it — but the floor of the axis is the honest + // answer to a frame with nothing in any bin, and a panic is not. + Some(RawHistogram::stops(0)) +} + +/// TRACES: FR-CULL-3 +/// The headroom figure as the chip row's hint reads it. +pub(crate) fn headroom_label(brightest: &[u32; RAW_HISTOGRAM_BINS], pixels: u32) -> String { + match headroom(brightest, pixels) { + // Nothing counted: say what the axis is, since there is no figure to + // put on it. + None => RAW_AXIS.into(), + // The end of the scale is said in words, not as `0.0`. They are the + // same fact and not the same sentence: a measurement that came out + // small reads differently from one that ran out, and on a culling pass + // that is the difference between "tight" and "gone". Only the top bin + // produces it — the axis is divided finely enough that the next one + // down is already a tenth of a stop. + Some(stops) if stops <= 0.0 => "no headroom left".into(), + // Rounded to a tenth, which is finer than any decision made from it + // and coarse enough not to flicker between two frames of one scene. + Some(stops) => format!("{stops:.1} stops of headroom"), + } +} fn model(heights: &[f32; COLUMNS]) -> slint::ModelRc { slint::ModelRc::new(slint::VecModel::from(heights.to_vec())) @@ -317,6 +528,141 @@ mod tests { assert!(!blank.highlights_clipped && !blank.shadows_clipped); } + /// A brightest-channel series with `count` pixels this many bins below + /// saturation, and nothing else. + fn brightest(entries: &[(usize, u32)]) -> [u32; RAW_HISTOGRAM_BINS] { + let mut out = [0u32; RAW_HISTOGRAM_BINS]; + for (below, count) in entries { + out[RAW_HISTOGRAM_BINS - 1 - *below] = *count; + } + out + } + + /// TRACES: FR-CULL-3 + #[test] + fn headroom_is_the_distance_from_the_brightest_content_to_saturation() { + // The figure FR-CULL-3 is written for, in the units it is written in. + // A frame whose highest content sits two stops down has two stops of + // latitude, and one whose content reaches the white level has none — + // and the difference between those two answers is the whole of whether + // a photograph is worth keeping. + let per_stop = dr_gpu::RAW_HISTOGRAM_BINS_PER_STOP; + + let two_down = headroom(&brightest(&[(2 * per_stop, 10_000)]), 10_000); + assert_eq!(two_down, Some(2.0)); + + let at_the_top = headroom(&brightest(&[(0, 10_000)]), 10_000); + assert_eq!(at_the_top, Some(0.0)); + + // Nothing counted is not the same statement as no headroom, and the + // panel must not turn the first into the second. + assert_eq!(headroom(&[0u32; RAW_HISTOGRAM_BINS], 0), None); + } + + /// TRACES: FR-CULL-3 + #[test] + fn headroom_ignores_a_handful_of_specular_pixels() { + // **The reason the figure is a percentile and not a maximum.** Almost + // every photograph has a few pixels at the ceiling — a glint on + // chrome, a hot pixel, the sun in the corner — and a headroom readout + // that reported 0.0 on all of them would be a readout nobody looks at + // twice. The threshold is the same one the clipping indicator uses, + // for the same reason. + let per_stop = dr_gpu::RAW_HISTOGRAM_BINS_PER_STOP; + let pixels = 1_000_000u32; + let threshold = (CLIP_VISIBLE * pixels as f32) as u32; + let glint = threshold - 1; + + let with_glint = brightest(&[(0, glint), (3 * per_stop, pixels - glint)]); + assert_eq!( + headroom(&with_glint, pixels), + Some(3.0), + "a specular glint took three stops of headroom off the reading" + ); + + // At the threshold it is no longer a glint, and the figure has to + // follow the data — this is the same count that lights the clipping + // indicator, and the two readouts sit an inch apart. + let real = brightest(&[(0, threshold), (3 * per_stop, pixels - threshold)]); + assert_eq!(headroom(&real, pixels), Some(0.0)); + } + + /// TRACES: FR-CULL-3 + #[test] + fn the_headroom_hint_says_none_rather_than_rounding_to_zero() { + // `0.0 stops of headroom` and `no headroom left` are the same fact and + // not the same sentence: the first reads as a measurement that came + // out small, the second as the end of the scale. On a culling pass + // this is the difference between "tight" and "gone". + let per_stop = dr_gpu::RAW_HISTOGRAM_BINS_PER_STOP; + assert_eq!( + headroom_label(&brightest(&[(2 * per_stop, 100)]), 100), + "2.0 stops of headroom" + ); + assert_eq!(headroom_label(&brightest(&[(0, 100)]), 100), "no headroom left"); + // One bin down is a sixteenth of a stop, which is a real if small + // amount of latitude and must not be rounded away into the sentence + // above it. + assert_eq!( + headroom_label(&brightest(&[(1, 100)]), 100), + "0.1 stops of headroom" + ); + // And with nothing counted the hint falls back to naming the axis, + // rather than stating a figure about a photograph nobody has read. + assert_eq!(headroom_label(&[0u32; RAW_HISTOGRAM_BINS], 0), RAW_AXIS); + } + + /// TRACES: FR-CULL-3 + #[test] + fn an_absent_raw_reading_says_which_kind_of_absent_it_is() { + // Three different facts, and flattening them loses the one that + // matters. "No sensor data" is permanent for this photograph and means + // the instrument does not apply; "no reduction here" means it applies + // and this device could not build it, which is a fault worth + // reporting; "no frame yet" passes on its own. A single "unavailable" + // would have a photographer looking for a driver problem on a JPEG. + let notes: Vec = [ + RawAbsence::NoImage, + RawAbsence::NotRaw, + RawAbsence::NoDevice, + ] + .iter() + .map(|why| raw_empty(*why).unavailable.to_string()) + .collect(); + + for note in ¬es { + assert!(!note.is_empty(), "an empty raw panel said nothing at all"); + assert_eq!( + notes.iter().filter(|n| *n == note).count(), + 1, + "two reasons read identically: {note}" + ); + } + + // And an empty panel never states a clipping figure — the failure the + // display panel's own test describes, in the instrument where it would + // be worse, because a raw clipping claim is a claim about the file. + let blank = raw_empty(RawAbsence::NotRaw); + assert!(!blank.available); + assert_eq!(blank.highlights_label, UNKNOWN); + assert_eq!(blank.shadows_label, UNKNOWN); + assert!(!blank.highlights_clipped && !blank.shadows_clipped); + } + + /// TRACES: FR-CULL-3 + #[test] + fn the_two_readings_are_labelled_apart() { + // The panel draws both in the same box, one chip apart. If they shared + // their headings, a photographer who had forgotten which was selected + // would read a raw saturation figure as a display clipping figure — + // and those disagree by exactly the amount FR-CULL-3 exists to expose. + let display = empty(); + let raw = raw_empty(RawAbsence::NoImage); + assert_ne!(display.low_title, raw.low_title); + assert_ne!(display.high_title, raw.high_title); + assert_ne!(display.hint, raw.hint); + } + #[test] fn the_indicator_ignores_a_handful_of_specular_pixels() { // The threshold, from both sides. Below it the marker stays dark while diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index e5355d2..67b42ac 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -320,6 +320,11 @@ fn reset_view_state(window: &AppWindow) { // beside the next one's filename is a confident, precise lie, and the gap // before the new frame settles is exactly long enough to read it. window.set_histogram(histogram::empty()); + // And the raw reading with it. It belongs to one file more completely than + // the display histogram does — nothing about the edit can move it — which + // makes leaving it standing over the next photograph's filename worse + // rather than better: it would look exactly as current as it is wrong. + window.set_raw_histogram(histogram::raw_empty(histogram::RawAbsence::NoImage)); // TRACES: FR-CULL-3 // The marks go down with it, and for the same reason. What is *not* reset // is whether peaking is switched on: that is a way of looking at a folder @@ -1669,6 +1674,32 @@ pub fn run(paths: Vec) -> Result<()> { .as_ref() .map_or_else(histogram::empty, histogram::view), ); + + // **The raw reading, pushed on the same path but not + // recomputed on it.** `DevelopSession::raw_histogram` + // caches: the demosaiced source is fixed for the life + // of the session, so this costs one dispatch per + // photograph and a clone of four kilobytes per settled + // frame thereafter. It is pushed from here anyway + // rather than once at open, because this is the one + // path every photograph takes and a panel that had to + // be told separately would be empty on some route into + // develop mode. + // + // Three outcomes, said apart. A file that was never + // raw has no white level and so no scale to measure + // headroom against; a device that could not build the + // reduction has a fault worth reporting; and only the + // third is a reading. + let raw = if !s.has_sensor_data() { + histogram::raw_empty(histogram::RawAbsence::NotRaw) + } else { + s.raw_histogram().as_ref().map_or_else( + || histogram::raw_empty(histogram::RawAbsence::NoDevice), + histogram::raw_view, + ) + }; + window.set_raw_histogram(raw); } // TRACES: FR-CULL-3 | NFR-P14 @@ -1705,6 +1736,13 @@ pub fn run(paths: Vec) -> Result<()> { // No frame, so nothing to describe. The stale plot would // otherwise sit beside the error message looking current. window.set_histogram(histogram::empty()); + // **The raw reading is left standing, and that is not an + // oversight.** It describes the sensor data behind the + // session, which a failed *render* says nothing about: the + // demosaic succeeded or there would be no session at all. + // Emptying it here would take away the one instrument + // still telling the truth, at the moment the other one + // stopped. // TRACES: FR-CULL-3 // And nothing to mark. Focus marks over the last frame // that rendered, beside a message saying this one did not, diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 022980d..6fb017b 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -72,6 +72,16 @@ export component AppWindow inherits Window { /// of a draft frame is a histogram of an image nobody is reading. in property histogram; + /// The same photograph counted in the raw domain — the demosaiced + /// scene-linear texture, before white balance, the camera matrix or any + /// curve, on an axis of stops below sensor saturation. + /// + /// Beside the display histogram rather than instead of it: the two answer + /// different questions and the panel offers both. Unlike the one above it + /// this is a property of the *file* and not of the render, so it survives + /// a crop, a zoom and every slider — see `raw_histogram.rs`. + in property raw-histogram; + /// TRACES: FR-CULL-3 /// Focus peaking: the marks, whether they describe *this* frame, and the /// three things the photographer chose. All of them are Rust's, because @@ -2251,6 +2261,7 @@ in property panel-visible: true; // answer than an honest one that is not yet scoped. HistogramPanel { data: root.histogram; + raw-data: root.raw-histogram; } Rectangle { diff --git a/ui/dr-ui/ui/histogram.slint b/ui/dr-ui/ui/histogram.slint index a77967f..086c549 100644 --- a/ui/dr-ui/ui/histogram.slint +++ b/ui/dr-ui/ui/histogram.slint @@ -1,6 +1,22 @@ // TRACES: FR-DSP-7 // The live histogram, and what it says about clipping. // +// **One plot, two readings, and a chip row to choose.** The panel draws either +// the frame the display is about to show (FR-DSP-7) or the sensor data the +// file holds (FR-CULL-3), from the same `HistogramView` and through the same +// traces. They are not two presentations of one measurement: the first is +// after white balance, the camera matrix and the whole tone chain, the second +// is before any of it on an axis of stops below sensor saturation, and +// FR-CULL-3 exists precisely because the first "systematically lies about what +// is recoverable in the raw". A culling decision needs the second; an export +// decision needs the first. Both are true. +// +// **So the words travel with the numbers.** The axis caption, the two clipping +// titles and the empty-state line are fields of the view rather than literals +// here, because they differ between the two readings — a raw saturation figure +// drawn under a heading saying Highlights would be mislabelled exactly where +// the difference matters. +// // **Why this is hand-built when the rest of the column is generated.** The // develop panel is built from operation capabilities, and a histogram is not an // operation: it has no parameters, changes nothing about the photograph, and @@ -17,6 +33,7 @@ import { Theme } from "theme.slint"; import { PanelHeading, Caption, Panel } from "widgets.slint"; +import { Segmented } from "controls.slint"; // One counted frame, ready to draw. // @@ -29,7 +46,11 @@ export struct HistogramView { // of one. available: bool, - luma: [float], + // The aggregate trace, drawn filled behind the three channels: Rec.709 + // luma for the display reading, the brightest channel for the raw one. In + // both cases the single curve exposure is read off — which is why it is + // named for its role in the plot rather than for either quantity. + overall: [float], red: [float], green: [float], blue: [float], @@ -43,6 +64,26 @@ export struct HistogramView { // being unable to tell the two apart. highlights-label: string, shadows-label: string, + + // What the two clipping readouts are called. Different words for the two + // readings — Shadows and Highlights describe a rendering, Black and + // Saturated describe a sensor — and they belong to the reading rather than + // to the panel for that reason. + low-title: string, + high-title: string, + + // What the axis is, or what it currently says: the chip row's hint. For + // the raw reading this is the headroom figure, which is the answer + // FR-CULL-3 is written for. + // + // Kept short in Rust, and that is a layout constraint: `FieldRow` draws + // the hint as an unwrapped Text, so its natural width becomes this panel's + // preferred width and the develop column takes the largest of those. + hint: string, + + // What the plot says when there is nothing to draw. Never "0%" and never + // an empty plot — both are claims about a photograph nobody has counted. + unavailable: string, } // One series, as a run of columns. @@ -121,7 +162,26 @@ component ClipReadout inherits HorizontalLayout { // TRACES: FR-DSP-7 export component HistogramPanel inherits Rectangle { + /// The frame the display is about to show (FR-DSP-7). in property data; + /// The sensor data the file holds (FR-CULL-3). + in property raw-data; + + /// Which of the two is on the plot. 0 is the display reading. + /// + /// **Held here rather than pushed from Rust**, and for the reason + /// `chosen-peaking` gives about the focus overlay: this is a way of + /// *looking* rather than a property of a photograph. Someone culling three + /// thousand frames switches to the raw reading once, and a mode that reset + /// with each image would ask them to switch it three thousand times. Both + /// views are pushed on every settled frame, so the choice needs no + /// callback and no round trip — the raw one costs nothing to keep current, + /// because it is computed once per photograph and cached. + property mode: 0; + + /// The reading currently drawn. Everything below reads this and not the + /// two above it, so there is exactly one place the choice is made. + property shown: root.mode == 1 ? root.raw-data : root.data; /// TRACES: FR-UI-2 @@ -150,6 +210,21 @@ export component HistogramPanel inherits Rectangle { PanelHeading { text: "HISTOGRAM"; } + // **Above the plot rather than below it**, because it says what the + // plot *is*: a photographer glancing at a shape has to know which of + // the two measurements they are looking at before they read it, not + // after. Two chips wide, which fits the narrowest column the + // application supports without the row setting the sidebar's width — + // see `ChipGrid`. + Segmented { + label: "Measured on"; + hint: root.shown.hint; + options: ["Display", "Raw"]; + selected: root.mode; + columns: 2; + picked(i) => { root.mode = i; } + } + Rectangle { // Tall enough to read a shape off and no taller. The column is the // photographer's instrument panel, and every pixel this takes is a @@ -163,10 +238,17 @@ export component HistogramPanel inherits Rectangle { // row of pixels inside the box rather than over its edge. clip: true; - // Quarter gridlines, so a tone can be placed on the axis without + // Quarter gridlines, so a value can be placed on the axis without // counting. Same construction and same weight as the tone curve's, // because the two plots sit in one column and any difference // between them would read as meaning something. + // + // They read as quarters of whichever axis is showing: 64 output + // levels for the display reading, four stops for the raw one. That + // is why the axis is named in the chip row's hint rather than + // labelled here — one set of gridlines cannot carry two scales, + // and drawing numbers against them would make one of the two + // readings wrong. for i in [1, 2, 3]: Rectangle { x: parent.width * i / 4; width: 1px; @@ -174,12 +256,14 @@ export component HistogramPanel inherits Rectangle { opacity: 0.5; } - // Luminance first, so it sits behind: it is the envelope the three - // channels decompose, and a filled area drawn over a line hides it. + // The aggregate first, so it sits behind: Rec.709 luma for the + // display reading, the brightest channel for the raw one, and in + // both cases the curve the three channels sit under. A filled area + // drawn over a line hides it. Trace { width: 100%; height: 100%; - heights: root.data.luma; + heights: root.shown.overall; ink: Theme.plot-luma; filled: true; } @@ -190,21 +274,21 @@ export component HistogramPanel inherits Rectangle { Trace { width: 100%; height: 100%; - heights: root.data.red; + heights: root.shown.red; ink: Theme.plot-red; shade: 0.85; } Trace { width: 100%; height: 100%; - heights: root.data.green; + heights: root.shown.green; ink: Theme.plot-green; shade: 0.85; } Trace { width: 100%; height: 100%; - heights: root.data.blue; + heights: root.shown.blue; ink: Theme.plot-blue; shade: 0.85; } @@ -220,24 +304,24 @@ export component HistogramPanel inherits Rectangle { width: 2px; height: 100%; background: Theme.warn-ink; - visible: root.data.shadows-clipped; + visible: root.shown.shadows-clipped; } Rectangle { x: parent.width - self.width; width: 2px; height: 100%; background: Theme.warn-ink; - visible: root.data.highlights-clipped; + visible: root.shown.highlights-clipped; } // Nothing rendered yet. Said rather than shown as an empty plot, // which would be a claim that the photograph has no tones in it. Caption { - text: "no frame yet"; + text: root.shown.unavailable; width: 100%; horizontal-alignment: center; y: (parent.height - self.height) / 2; - visible: !root.data.available; + visible: !root.shown.available; } } @@ -247,17 +331,17 @@ export component HistogramPanel inherits Rectangle { // controls below it up and back down as each image opens. HorizontalLayout { ClipReadout { - title: "Shadows"; - figure: root.data.shadows-label; - lit: root.data.shadows-clipped; + title: root.shown.low-title; + figure: root.shown.shadows-label; + lit: root.shown.shadows-clipped; } Rectangle { horizontal-stretch: 1; } ClipReadout { - title: "Highlights"; - figure: root.data.highlights-label; - lit: root.data.highlights-clipped; + title: root.shown.high-title; + figure: root.shown.highlights-label; + lit: root.shown.highlights-clipped; } } }