diff --git a/core/dr-gpu/examples/frame_budget.rs b/core/dr-gpu/examples/frame_budget.rs new file mode 100644 index 0000000..95c5d54 --- /dev/null +++ b/core/dr-gpu/examples/frame_budget.rs @@ -0,0 +1,692 @@ +//! What a frame actually costs — the measurement FR-DSP-2 is waiting on. +//! +//! `docs/display-and-extension.md` §2 argues that tiled computation predates +//! the fused-shader design and may not need to exist: the composer folds every +//! active operation into **one dispatch over a viewport-sized target**, so the +//! problem tiles were invented to solve may already be solved. That argument +//! is only worth as much as the numbers behind it, and the decision rule was +//! fixed in advance — if the 99th percentile sits inside 16 ms, FR-DSP-2 is +//! rewritten as a scheduling concern for export rather than implemented on the +//! interactive path. +//! +//! This is the instrument that decides it. Three measurements: +//! +//! - **M1** — the fused pass at proxy resolution, at three viewport sizes and +//! three chain lengths. +//! - **M2** — the same with the view zoomed to 1:1 on a 60 MP source, which is +//! the case FR-DSP-5 names. +//! - **M3** — the neighbourhood stage on its own. It is the only part of the +//! chain that is not one read and one write, so it is the plausible +//! budget-breaker and deserves to be measured apart from the fused pass +//! rather than hidden inside its total. +//! +//! ```sh +//! cargo run --release -p dr-gpu --example frame_budget +//! ``` +//! +//! **Release, always.** A debug build measures rustc's shadow, not the GPU's: +//! the per-frame CPU half is dominated by shader-source assembly, which is +//! string formatting and is several times slower unoptimised. +//! +//! The committed numbers live in `docs/frame-budget.md`. Rerun this and diff +//! that file; a regression should be a diff rather than somebody's memory. +//! +//! # Why the 99th percentile and not the mean +//! +//! A slider drag is judged by its worst frame. A chain averaging 4 ms with one +//! frame in fifty at 30 ms reads as a stutter, and the mean says nothing about +//! it. Percentiles are nearest-rank over the samples with the warm-up already +//! discarded — see [`Percentiles`]. +//! +//! # What is being timed +//! +//! Each frame is `compose` (CPU: assemble the WGSL and its uniforms) followed +//! by `render_detailed` and a `poll` that waits for the device to go idle. +//! Waiting serialises the GPU work into the frame it belongs to, which is +//! pessimistic — a real presentation pipeline overlaps a frame's tail with the +//! next frame's head — and pessimistic is the right direction for a budget. +//! +//! The compose half is reported separately because it is *not* GPU work and +//! would otherwise be invisible: it happens on the UI thread on every slider +//! event, and if it were the expensive half then tiling could not help at all. + +use std::time::Instant; + +use dr_film::bake::{bake, Recipe}; +use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext}; +use dr_pipeline::descriptor::ParamKind; +use dr_pipeline::ops::{ + blacks_whites, contrast, exposure, highlights_shadows, local_contrast, noise_reduction, + vibrance, FilmTables, +}; +use dr_pipeline::{Attribute, CropRect, EditGraph, OpId, ParamId}; + +/// The synthetic source: 9504 × 6336 is 60.2 MP, which is a Sony A7R V or a +/// Fujifilm GFX 100 with a little taken off. §2's M2 names 60 MP, so this is +/// that number rather than a round one. +const SOURCE: (u32, u32) = (9504, 6336); + +/// Frames measured per row, after the warm-up. Enough that the 99th percentile +/// means something (nearest-rank picks the second-worst of 100) without the +/// whole matrix taking minutes. +const FRAMES: usize = 100; + +/// Frames discarded before measurement begins. +/// +/// The first frame at a new size allocates a render target, the first frame +/// with a new chain compiles a pipeline, and the first frame of a detail stage +/// allocates its ping-pong pair. None of those recur during a drag, so +/// including them would measure the wrong thing — but they are worth knowing +/// about, so [`Run::first_frame_ms`] reports the very first one separately. +const WARMUP: usize = 12; + +/// The viewport sizes from §2's M1: a laptop panel, a 16:10 desktop, and 4K. +const SIZES: [(u32, u32); 3] = [(1920, 1200), (2560, 1600), (3840, 2160)]; + +/// The frame budget FR-DSP-3 is asserting against, in milliseconds. 60 Hz. +const BUDGET_MS: f64 = 16.0; + +fn main() { + env_logger::init(); + + let ctx = match pollster::block_on(GpuContext::new_headless()) { + Ok(c) => c, + Err(e) => { + eprintln!("no GPU adapter: {e}"); + std::process::exit(1); + } + }; + println!("adapter {} ({:?})", ctx.adapter_name(), ctx.backend()); + + let limit = DemosaicedImage::max_dimension(&ctx); + if SOURCE.0 > limit { + eprintln!( + "this adapter caps textures at {limit} px; {} × {} will not fit", + SOURCE.0, SOURCE.1 + ); + std::process::exit(1); + } + + let t = Instant::now(); + let source = synthetic_source(&ctx); + println!( + "source {} × {} ({:.1} MP, {:.0} MB as rgba16f), built in {:.1} s", + SOURCE.0, + SOURCE.1, + (SOURCE.0 as f64 * SOURCE.1 as f64) / 1e6, + (SOURCE.0 as f64 * SOURCE.1 as f64 * 8.0) / 1e6, + t.elapsed().as_secs_f64() + ); + println!("budget {BUDGET_MS:.0} ms (FR-DSP-3)\n"); + + let mut adjust = AdjustPass::new(&ctx); + + m1_and_m2(&ctx, &mut adjust, &source); + m3(&ctx, &mut adjust, &source); +} + +// --------------------------------------------------------------------------- +// M1 and M2 — the fused pass, fit and at 1:1 +// --------------------------------------------------------------------------- + +/// The chain lengths §2 asks for. +/// +/// "Every operation" means every operation in [`EditGraph::default_chain`], +/// which is what `ops/*.yaml` generates. The lens corrections are not in it — +/// they are constructed from a matched profile rather than being part of the +/// default chain — and neither are masks or spot repairs, which are stacks of +/// their own. So this is an upper bound on the *declared* chain and a lower +/// bound on the worst edit a photograph can carry; §7 of the spec asks for +/// honesty about exactly this sort of thing. +/// +/// [`Chain::AllPoint`] is not in §2's list and is the row that decides the +/// question. §2 asks whether *one dispatch over a viewport-sized target* is +/// fast enough; "every operation" mixes that dispatch together with the +/// neighbourhood stage, which is not one dispatch and is measured separately in +/// M3. Splitting the two apart is what lets M1 answer the question it was +/// written to answer instead of a different, harder one. +#[derive(Clone, Copy, PartialEq)] +enum Chain { + One, + Five, + /// Every operation that contributes a fragment to the fused shader — the + /// full point-operation chain, and nothing with a kernel. + AllPoint, + Everything, +} + +impl Chain { + fn label(self) -> &'static str { + match self { + Chain::One => "one", + Chain::Five => "five", + Chain::AllPoint => "point", + Chain::Everything => "all", + } + } +} + +fn m1_and_m2(ctx: &GpuContext, adjust: &mut AdjustPass, source: &DemosaicedImage) { + for (title, zoomed) in [ + ( + "M1 — fused frame at proxy resolution (fit; the develop view)", + false, + ), + ( + "M2 — the same view zoomed to 1:1 on the 60 MP source (FR-DSP-5)", + true, + ), + ] { + println!("{title}"); + header(); + for size in SIZES { + for chain in [Chain::One, Chain::Five, Chain::AllPoint, Chain::Everything] { + let mut graph = build_chain(chain); + if zoomed { + graph.framing_mut().set_view(one_to_one(size)); + } + if matches!(chain, Chain::AllPoint | Chain::Everything) { + adjust.set_film(Some(&film_tables())); + } + // The drag: exposure, moved by a hundredth of a stop per frame. + // A real slider does exactly this, and it is what stops the + // fused dispatch being skipped by `render_detailed`'s colour + // reuse — which would turn M1 into a measurement of the detail + // stage by accident. + let run = measure(ctx, adjust, source, &mut graph, size, |g, i| { + g.set_param(exposure::ID, exposure::EXPOSURE, 0.30 + i as f32 * 0.01); + }); + adjust.set_film(None); + row(size, chain.label(), &run); + } + } + println!(); + } + println!(" `shader` is `EditGraph::compose` alone; `cpu` adds the detail chain and the"); + println!(" invalidation hash, which is everything the develop view does per frame before"); + println!(" it dispatches. `gpu` is submit plus wait-for-idle. `TOTAL` ranks cpu + gpu"); + println!(" summed within each frame — the column the {BUDGET_MS:.0} ms budget is judged on."); + println!(); + println!(" `point` is every operation that contributes a fragment to the fused shader;"); + println!(" `all` is that plus the four neighbourhood operations, which is why the two"); + println!(" differ by roughly what M3 charges for the detail stage on its own."); + println!(); +} + +// --------------------------------------------------------------------------- +// M3 — the detail stage alone +// --------------------------------------------------------------------------- + +/// Neighbourhood operations, timed with the fused dispatch deliberately reused. +/// +/// `render_detailed` skips the fused pass when the colour key, the shader, its +/// uniforms and the size are all unchanged — that is FR-DEV-3d, and it is also +/// the lever that isolates M3: move only a detail operation's amount and the +/// timing covers the convolutions and nothing else. The `colour` column proves +/// the isolation held rather than asserting it: it counts fused dispatches over +/// the measured frames, and a zero there is what makes the number mean "detail +/// alone". +fn m3(ctx: &GpuContext, adjust: &mut AdjustPass, source: &DemosaicedImage) { + println!("M3 — the neighbourhood stage alone (fused dispatch reused; FR-DEV-3d)"); + println!( + " {:>11} {:>8} {:>5} {:>4} {:>6} {:>6} {:>8} {:>7} {:>7} {:>7}", + "size", "stage", "view", "pass", "radius", "colour", "cpu p99", "p50", "p99", "max" + ); + + for size in SIZES { + for zoomed in [false, true] { + for (label, build) in DETAIL_CHAINS { + let mut graph = build(); + if zoomed { + graph.framing_mut().set_view(one_to_one(size)); + } + let composed = graph.compose_detail(source.size(), size); + let (passes, radius) = (composed.len(), composed.radius()); + // Only the detail parameter moves, so the colour half of the + // chain is bit-identical frame to frame and gets reused. + let run = measure(ctx, adjust, source, &mut graph, size, |g, i| { + let drift = (i % 20) as f32; + g.set_param( + local_contrast::CLARITY, + local_contrast::AMOUNT, + 60.0 + drift, + ); + g.set_param( + local_contrast::TEXTURE, + local_contrast::AMOUNT, + 60.0 + drift, + ); + g.set_param( + noise_reduction::ID, + noise_reduction::LUMINANCE, + 60.0 + drift, + ); + g.set_param(noise_reduction::ID, noise_reduction::CHROMA, 60.0 + drift); + }); + println!( + " {:>5}x{:<5} {:>8} {:>5} {:>4} {:>6} {:>6} {:>6.2}ms {:>5.2}ms {:>5.2}ms {:>5.2}ms", + size.0, + size.1, + label, + if zoomed { "1:1" } else { "fit" }, + passes, + radius, + run.colour_dispatches, + run.cpu.p99, + run.frame.p50, + run.frame.p99, + run.frame.max + ); + } + } + } + println!(); + println!(" `pass` is dispatches in the detail chain and `radius` the widest halo any of"); + println!(" them reads, in render pixels. `colour` is fused dispatches over the {FRAMES}"); + println!(" measured frames, and must be 0 for the row to mean what it says."); + println!(); + println!(" Clarity's kernel is a fraction of the *frame*, so it grows with the viewport"); + println!(" and not with the zoom. Noise reduction's is a fraction of the *sensor*, so it"); + println!(" grows with the zoom and not with the viewport. That is why both are here."); +} + +/// The detail chains M3 walks: the widest kernel alone, then all four +/// neighbourhood operations at once. +/// +/// Clarity is first because §2 names it — "a separable blur at a large radius +/// is the plausible budget-breaker" — and its σ is 1.2% of the shorter edge, +/// which at 4K is a 52-pixel radius and the widest kernel anywhere in the +/// pipeline. +/// A named detail chain: a label for the table, and the graph it builds. +type DetailChain = (&'static str, fn() -> EditGraph); + +const DETAIL_CHAINS: [DetailChain; 2] = [ + ("clarity", || { + let mut g = EditGraph::default_chain(); + g.set_param(local_contrast::CLARITY, local_contrast::AMOUNT, 60.0); + g + }), + ("all four", || { + let mut g = EditGraph::default_chain(); + g.set_param(local_contrast::CLARITY, local_contrast::AMOUNT, 60.0); + g.set_param(local_contrast::TEXTURE, local_contrast::AMOUNT, 60.0); + g.set_param(noise_reduction::ID, noise_reduction::LUMINANCE, 60.0); + g.set_param(noise_reduction::ID, noise_reduction::CHROMA, 60.0); + g.set_param( + dr_pipeline::ops::capture_sharpen::ID, + dr_pipeline::ops::capture_sharpen::AMOUNT, + 60.0, + ); + g + }), +]; + +// --------------------------------------------------------------------------- +// The measurement itself +// --------------------------------------------------------------------------- + +/// Nearest-rank percentiles over a set of frame times, in milliseconds. +/// +/// Nearest-rank rather than an interpolating definition because the samples +/// *are* the population — there is no distribution being estimated, only a +/// hundred frames that either happened inside the budget or did not. At +/// `FRAMES = 100` the 99th percentile is the second-worst frame, which is the +/// honest reading of "one stutter in a hundred is one too many" without +/// letting a single scheduler hiccup on an unrelated process decide the +/// verdict. +struct Percentiles { + p50: f64, + p99: f64, + max: f64, +} + +impl Percentiles { + fn of(mut samples: Vec) -> Self { + samples.sort_by(f64::total_cmp); + let rank = |p: f64| { + let n = samples.len(); + let i = ((p * n as f64).ceil() as usize).clamp(1, n) - 1; + samples[i] + }; + Self { + p50: rank(0.50), + p99: rank(0.99), + max: samples[samples.len() - 1], + } + } +} + +struct Run { + /// CPU: assembling the fused shader alone. + shader: Percentiles, + /// CPU: everything `DevelopSession::render` does before it dispatches — + /// the fused shader, the detail chain, and the invalidation hash the + /// colour key comes from. Split out from [`Self::shader`] because if the + /// expensive half of a frame turns out to be string formatting on the UI + /// thread, no amount of tiling helps and the conclusion is a different + /// one. + cpu: Percentiles, + /// Submit plus wait for the device to go idle. + frame: Percentiles, + /// CPU and GPU summed **per frame**, then ranked. + /// + /// Not the sum of the two percentiles above, which would be a number no + /// frame ever took: the CPU's worst frame and the GPU's worst frame are + /// not generally the same frame, and adding them invents a stutter that + /// did not happen. This is the column the budget is judged on. + total: Percentiles, + /// The very first frame of all, warm-up included — pipeline compilation + /// and target allocation. Reported because it is real (it is what a + /// photograph opening costs) and because it must not be inside the + /// percentiles. + first_frame_ms: f64, + /// Fused dispatches over the measured frames. `FRAMES` when the colour + /// chain is moving, 0 when only a detail parameter is. + colour_dispatches: usize, +} + +/// Render `FRAMES` frames, moving a parameter between each, and time them. +/// +/// `drag` receives the frame index and is expected to move whatever this row +/// is measuring the drag of. It is called for the warm-up frames too, so that +/// nothing measured is the first of its kind. +fn measure( + ctx: &GpuContext, + adjust: &mut AdjustPass, + source: &DemosaicedImage, + graph: &mut EditGraph, + size: (u32, u32), + drag: impl Fn(&mut EditGraph, usize), +) -> Run { + // A constant, because every row here renders the same photograph. In the + // app this is the `VersionId` mixed with the colour invalidation — see + // `render_detailed`'s note on why the caller owns it. + const COLOUR_KEY: u64 = 0x0dd_ba11; + + let src = source.size(); + let mut first_frame_ms = f64::NAN; + + for i in 0..WARMUP { + drag(graph, i); + let t = Instant::now(); + frame(ctx, adjust, source, graph, src, size, COLOUR_KEY); + if i == 0 { + first_frame_ms = t.elapsed().as_secs_f64() * 1e3; + } + } + + let dispatches_before = adjust.colour_dispatches(); + let mut shader_ms = Vec::with_capacity(FRAMES); + let mut cpu_ms = Vec::with_capacity(FRAMES); + let mut gpu_ms = Vec::with_capacity(FRAMES); + let mut total_ms = Vec::with_capacity(FRAMES); + + for i in 0..FRAMES { + drag(graph, WARMUP + i); + + // The same three calls `DevelopSession::render` makes, in the same + // order, so that this is the develop view's frame and not an + // idealisation of it. + let t0 = Instant::now(); + let shader = graph.compose(); + let after_shader = t0.elapsed(); + let detail = graph.compose_detail(src, size); + let colour_key = graph.invalidation().through(dr_pipeline::Affects::Colour); + let cpu = t0.elapsed(); + + let t1 = Instant::now(); + adjust + .render_detailed( + source, + &shader, + size.0, + size.1, + None, + &detail, + colour_key ^ COLOUR_KEY, + ) + .expect("render"); + ctx.device + .poll(wgpu::PollType::wait_indefinitely()) + .expect("poll"); + let gpu = t1.elapsed(); + + shader_ms.push(after_shader.as_secs_f64() * 1e3); + cpu_ms.push(cpu.as_secs_f64() * 1e3); + gpu_ms.push(gpu.as_secs_f64() * 1e3); + total_ms.push((cpu + gpu).as_secs_f64() * 1e3); + } + + Run { + shader: Percentiles::of(shader_ms), + cpu: Percentiles::of(cpu_ms), + frame: Percentiles::of(gpu_ms), + total: Percentiles::of(total_ms), + first_frame_ms, + colour_dispatches: adjust.colour_dispatches() - dispatches_before, + } +} + +/// One frame, composition included, with nothing timed. The warm-up path. +fn frame( + ctx: &GpuContext, + adjust: &mut AdjustPass, + source: &DemosaicedImage, + graph: &EditGraph, + src: (u32, u32), + size: (u32, u32), + colour_key: u64, +) { + let shader = graph.compose(); + let detail = graph.compose_detail(src, size); + let key = graph.invalidation().through(dr_pipeline::Affects::Colour) ^ colour_key; + adjust + .render_detailed(source, &shader, size.0, size.1, None, &detail, key) + .expect("render"); + ctx.device + .poll(wgpu::PollType::wait_indefinitely()) + .expect("poll"); +} + +fn header() { + println!( + " {:>11} {:>5} {:>8} {:>8} {:>8} {:>8} {:>8} {:>7}", + "size", "chain", "shader", "cpu p99", "gpu p50", "gpu p99", "TOTAL", "first" + ); +} + +fn row(size: (u32, u32), chain: &str, run: &Run) { + println!( + " {:>5}x{:<5} {:>5} {:>6.2}ms {:>6.2}ms {:>6.2}ms {:>6.2}ms {:>6.2}ms {:>5.0}ms{}", + size.0, + size.1, + chain, + run.shader.p99, + run.cpu.p99, + run.frame.p50, + run.frame.p99, + run.total.p99, + run.first_frame_ms, + if run.total.p99 > BUDGET_MS { + " OVER" + } else { + "" + } + ); +} + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +/// The view rect that puts one render pixel on one source pixel. +/// +/// This is FR-DSP-5's mechanism stated as arithmetic: the render target keeps +/// its size while the sampled region shrinks, so a view covering exactly +/// `render / source` of the frame samples one-for-one. Nothing anywhere +/// switches to a "full resolution path"; the zoom *is* the full-resolution +/// path. +fn one_to_one(render: (u32, u32)) -> CropRect { + let w = render.0 as f32 / SOURCE.0 as f32; + let h = render.1 as f32 / SOURCE.1 as f32; + CropRect { + // Centred, so the sampled region is somewhere a person would actually + // look and not a corner the caches treat differently. + x: (1.0 - w) * 0.5, + y: (1.0 - h) * 0.5, + width: w, + height: h, + } +} + +/// A 60 MP source with detail at every scale. +/// +/// Not flat and not noise. A flat frame lets the memory system serve every +/// sample from one cache line, which flatters the wide kernels of M3 by an +/// amount that has nothing to do with photographs; pure noise does the +/// opposite. This is a coarse gradient with a fine dither on top, which is +/// closer to a real frame's spectrum than either and costs one multiply per +/// pixel to generate. +fn synthetic_source(ctx: &GpuContext) -> DemosaicedImage { + let (w, h) = SOURCE; + let mut rgba = vec![0u8; (w as usize) * (h as usize) * 4]; + for y in 0..h as usize { + let row = y * (w as usize) * 4; + for x in 0..w as usize { + // A cheap integer hash for the fine structure, so neighbouring + // pixels differ and a bilateral filter has something to reject. + let n = (x.wrapping_mul(2_654_435_761) ^ y.wrapping_mul(1_640_531_527)) >> 13; + let dither = (n & 0x1f) as u32; + let gx = (x * 200 / w as usize) as u32; + let gy = (y * 55 / h as usize) as u32; + let px = &mut rgba[row + x * 4..row + x * 4 + 4]; + px[0] = (30 + gx + dither).min(255) as u8; + px[1] = (40 + gy + dither).min(255) as u8; + px[2] = (60 + gx / 2 + gy + dither).min(255) as u8; + px[3] = 255; + } + } + DemosaicedImage::from_rgba8(ctx, &rgba, w, h).expect("upload source") +} + +fn build_chain(chain: Chain) -> EditGraph { + let mut graph = EditGraph::default_chain(); + match chain { + Chain::One => { + graph.set_param(exposure::ID, exposure::EXPOSURE, 0.3); + } + Chain::Five => { + graph.set_param(exposure::ID, exposure::EXPOSURE, 0.3); + graph.set_param(contrast::ID, contrast::CONTRAST, 25.0); + graph.set_param( + highlights_shadows::ID, + highlights_shadows::HIGHLIGHTS, + -40.0, + ); + graph.set_param(blacks_whites::ID, blacks_whites::BLACKS, -20.0); + graph.set_param(vibrance::ID, vibrance::VIBRANCE, 35.0); + } + Chain::AllPoint => { + activate_everything(&mut graph, false); + } + Chain::Everything => { + activate_everything(&mut graph, true); + } + } + graph +} + +/// Move every parameter of every operation off its default. +/// +/// Written against [`EditGraph::capabilities`] rather than as a list of +/// operations, for the same reason the panel is: this bench must not need +/// editing when an operation is added, or it will quietly stop measuring "all +/// of them" on the first day somebody declares a new node. +/// +/// A quarter of the way from the default towards the maximum, which is a +/// setting a photographer might plausibly reach and — far more importantly — +/// is *active*, since an operation sitting at its neutral contributes no +/// fragment at all and would make this row a shorter chain wearing a longer +/// chain's label. +/// +/// `neighbourhood` selects whether the operations declaring +/// [`Attribute::Detail`] are moved as well. Left off, what remains is exactly +/// the set that contributes a fragment to the fused shader — see [`Chain`] for +/// why that distinction is the point of this bench. +fn activate_everything(graph: &mut EditGraph, neighbourhood: bool) { + let moves: Vec<(OpId, ParamId, f32)> = graph + .capabilities() + .iter() + .filter(|op| neighbourhood || !op.attributes.contains(&Attribute::Detail)) + .flat_map(|op| { + op.params.iter().map(move |p| { + let value = match p.kind { + ParamKind::Scalar { min, max, .. } => { + // Towards whichever end is further away, so a + // parameter defaulting to its maximum still moves. + let far = if (max - p.default).abs() >= (p.default - min).abs() { + max + } else { + min + }; + p.default + (far - p.default) * 0.25 + } + ParamKind::Bool => 1.0, + // The second variant when there is one. The first is the + // default by construction — see `ParamDescriptor::choice`. + ParamKind::Enum { variants } => { + if variants.len() > 1 { + 1.0 + } else { + 0.0 + } + } + }; + (op.id, p.id, value) + }) + }) + .collect(); + for (op, param, value) in moves { + graph.set_param(op, param, value); + } + + // The film stock is not a slider and so is not reachable through the loop + // above: `FilmSim::is_active` is true when it has tables and false + // otherwise (see `graph::Film`). Without this the "all" row would be the + // whole chain minus its single most expensive node, which is a 3D lookup + // and a pair of curve textures. + graph.set_film(Some(dr_pipeline::graph::Film { + stock: STOCK.into(), + print: None, + tables: film_tables(), + })); +} + +/// The stock the "every operation" chain renders through. A colour negative +/// viewed directly, which is the more expensive of the two paths: the LUT is +/// consulted either way, and skipping the print step is one fewer thing that +/// could be mistaken for the measurement. +const STOCK: &str = "kodak_portra_400"; + +/// The stock the "all operations" row renders through, in the layout the pass +/// binds. Baked once per row; baking is milliseconds and happens off the frame +/// path in the app too. +fn film_tables() -> FilmTables { + let film = dr_film::find(STOCK).expect("stock"); + let baked = bake(&Recipe::new(film, None)); + FilmTables { + exposure_matrix: baked.exposure_matrix, + curves: baked.curves.clone(), + curve_log_min: baked.curve_log_min, + curve_log_max: baked.curve_log_max, + lut: baked.lut.clone(), + density_max: baked.density_max, + lut_size: baked.lut_size, + // Grain off. It is a per-pixel hash and would be measured; it is also + // not part of every edit, and the chain being measured here is "every + // operation active", not "every option of every operation". + grain_particles: [0.0; 3], + grain_density_max: [baked.density_max; 3], + grain_uniformity: 0.97, + } +} diff --git a/core/dr-gpu/tests/frame_budget.rs b/core/dr-gpu/tests/frame_budget.rs new file mode 100644 index 0000000..3c23080 --- /dev/null +++ b/core/dr-gpu/tests/frame_budget.rs @@ -0,0 +1,338 @@ +//! The frame budget, asserted rather than hoped for. +//! +//! FR-DSP-3 says a slider updates the visible region within one frame budget at +//! proxy resolution. Until this file existed nothing checked it, which made it +//! a wish — `docs/display-and-extension.md` §3 is blunt about that, and §7 is +//! blunt about what tagging an unchecked requirement does to the coverage +//! figure. +//! +//! The measurements this guards are in [`docs/frame-budget.md`], produced by +//! `examples/frame_budget.rs`. This file is the part of them that has to keep +//! being true: it renders the **whole point-operation chain** through the real +//! `render_detailed` for a hundred frames, moving a slider between each, and +//! fails if the 99th percentile leaves the budget. +//! +//! # What it does not cover, said out loud +//! +//! **The neighbourhood stage is deliberately not in the asserted chain.** It is +//! over the budget today — clarity alone is 34 ms at 4K, because its kernel is +//! a fraction of the frame and reaches a 52-pixel radius there — and +//! `docs/frame-budget.md` records that, names the fix (a base computed at +//! reduced resolution) and does not pretend otherwise. Asserting a budget the +//! code does not meet would produce a red suite that everyone learns to ignore; +//! asserting it on a chain that quietly excluded the expensive stage *without +//! saying so* would be the coverage overstatement §7 warns about. So it is +//! excluded, loudly, here. +//! +//! What is asserted is exactly the claim the FR-DSP-2 recommendation rests on: +//! that **one fused dispatch over a viewport-sized target is comfortably inside +//! the budget**, at a full chain, fit and at 1:1. If that stops being true, the +//! recommendation to strike tiled computation from the interactive path stops +//! being supported, and this test is what says so. +//! +//! # Why the percentile and not the mean +//! +//! A drag is judged by its worst frame. Nearest-rank over 100 frames puts the +//! 99th percentile at the second-worst, which is strict enough to catch a +//! stutter and forgiving enough that one scheduler hiccup from an unrelated +//! process does not decide the verdict. +//! +//! # Why the CPU half is asserted only in an optimised build +//! +//! Composing the shader is per-frame work on the UI thread and belongs in the +//! budget — `DevelopSession::render` calls `compose` on every frame, and on a +//! full chain it is milliseconds of string formatting. But the workspace builds +//! its own crates at `opt-level = 0` in dev (see the root `Cargo.toml`), and +//! `cargo test` is a dev build, so that formatting runs unoptimised here and +//! measures rustc rather than the pipeline. The GPU half is unaffected: a +//! shader is compiled by the driver either way. +//! +//! So the GPU half is always asserted, and the composition is folded in only +//! when `debug_assertions` is off. Running `cargo test --release -p dr-gpu` +//! therefore checks strictly more than the default run does, and the numbers +//! printed on failure say which of the two halves was over. + +use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext}; +use dr_pipeline::descriptor::ParamKind; +use dr_pipeline::ops::exposure; +use dr_pipeline::{Affects, Attribute, CropRect, EditGraph, OpId, ParamId}; +use std::time::Instant; + +/// 60 Hz. FR-DSP-3 does not name a number; this is the one every interactive +/// application means by "one frame". +const BUDGET_MS: f64 = 16.0; + +/// Measured frames per case. Nearest-rank p99 of 100 is the second-worst. +const FRAMES: usize = 100; + +/// Discarded before measurement: the first frame at a size allocates a render +/// target and the first frame of a chain compiles a pipeline. Neither recurs +/// during a drag, so neither belongs in a drag's percentile. +const WARMUP: usize = 12; + +/// A 24 MP source — a full-frame camera, and large enough that a 1:1 view of it +/// is a genuine zoom rather than a rounding error. +/// +/// Smaller than the bench's 60 MP on purpose. The fused pass costs what the +/// *output* costs, so the source size barely moves these numbers, and 24 MP +/// keeps the fixture inside a second even at `opt-level = 0`. +const SOURCE: (u32, u32) = (6000, 4000); + +/// The viewport the budget is asserted at: a 16:10 desktop display. +/// +/// Not 4K, and the reason is worth stating. At 4K the fused chain still passes +/// with room to spare (4.5 ms of GPU; see `docs/frame-budget.md`), but a test +/// that renders 8.3 M pixels a hundred times twice over is four seconds of +/// suite time to re-establish a conclusion 4.1 M pixels already establishes. +const VIEWPORT: (u32, u32) = (2560, 1600); + +fn ctx() -> Option { + // CI runners and headless machines may have no usable adapter. Skip rather + // than fail, exactly as the rest of this crate's device tests do. + match pollster::block_on(GpuContext::new_headless()) { + Ok(c) => Some(c), + Err(e) => { + eprintln!("skipping: no GPU adapter ({e})"); + None + } + } +} + +/// TRACES: FR-DSP-3 | FR-DSP-5 +/// A slider drag on the full point-operation chain stays inside one frame — +/// fit, and at 1:1. +/// +/// The develop view's ordinary case, at a full chain rather than a flattering +/// one: every operation that contributes a fragment to the fused shader is +/// active, and exposure moves between frames exactly as a drag moves it. +/// +/// The 1:1 case is the one FR-DSP-5 names and the one FR-DSP-3's asynchronous +/// clause was written for. It is asserted here because the measurement found +/// that clause unnecessary rather than merely unimplemented: a 1:1 view is +/// *cheaper* than a fit view of the same file, since the dispatch is the same +/// size and the reads are contiguous rather than strided. If that ever inverts, +/// the argument for striking the clause weakens, and this is what would notice. +/// +/// # One test and not two, deliberately +/// +/// The two cases were two `#[test]` functions until the numbers said otherwise. +/// Cargo runs a binary's tests on a thread each, both of these want the same +/// GPU, and contending for it took the 1:1 case from 2.5 ms to 14.9 ms — a +/// measurement of the test harness that would have flickered either side of the +/// budget forever. A timing assertion has to own the device while it runs, and +/// the only way to say that in a test binary is to be the only test in it. +#[test] +fn a_slider_drag_stays_inside_the_frame_budget() { + let Some(ctx) = ctx() else { return }; + let source = synthetic_source(&ctx); + + let mut fit = full_point_chain(); + drag(&ctx, &source, &mut fit, VIEWPORT).assert_inside_budget("proxy resolution, fit", VIEWPORT); + + let mut zoomed = full_point_chain(); + zoomed.framing_mut().set_view(one_to_one(VIEWPORT)); + drag(&ctx, &source, &mut zoomed, VIEWPORT) + .assert_inside_budget("1:1 on a 24 MP source", VIEWPORT); +} + +// --------------------------------------------------------------------------- +// The measurement +// --------------------------------------------------------------------------- + +struct Run { + /// Per frame: compose, compose the detail chain, hash the invalidation. + cpu_p99: f64, + /// Per frame: submit and wait for the device to go idle. + gpu_p99: f64, + /// `cpu + gpu` summed within each frame, then ranked. Not the sum of the + /// two percentiles above, which would be a frame that never happened. + total_p99: f64, +} + +impl Run { + /// Fail if the budget was missed, saying which half missed it. + /// + /// In a dev build only the GPU half is judged — see the module + /// documentation for why — and the CPU figure is still printed, because a + /// reader looking at a failure wants both numbers even when only one of + /// them is the verdict. + fn assert_inside_budget(&self, case: &str, viewport: (u32, u32)) { + let judged = if cfg!(debug_assertions) { + self.gpu_p99 + } else { + self.total_p99 + }; + assert!( + judged <= BUDGET_MS, + "{case} at {}x{}: p99 of {FRAMES} frames was {judged:.2} ms, over the \ + {BUDGET_MS:.0} ms budget (cpu {:.2} ms, gpu {:.2} ms, total {:.2} ms). \ + FR-DSP-3 is what this violates; docs/frame-budget.md holds the \ + numbers it used to be.", + viewport.0, + viewport.1, + self.cpu_p99, + self.gpu_p99, + self.total_p99, + ); + } +} + +/// Render `FRAMES` frames with the exposure slider moving between each. +/// +/// The three calls before the dispatch are the three `DevelopSession::render` +/// makes, in the same order, so this is the develop view's frame rather than an +/// idealisation of it. +fn drag( + ctx: &GpuContext, + source: &DemosaicedImage, + graph: &mut EditGraph, + viewport: (u32, u32), +) -> Run { + // Stands for the `VersionId` the app mixes in. Constant because every frame + // here is the same photograph. + const PHOTOGRAPH: u64 = 0x0dd_ba11; + + let mut adjust = AdjustPass::new(ctx); + let src = source.size(); + let (w, h) = viewport; + + let mut cpu = Vec::with_capacity(FRAMES); + let mut gpu = Vec::with_capacity(FRAMES); + let mut total = Vec::with_capacity(FRAMES); + + for i in 0..WARMUP + FRAMES { + // A hundredth of a stop per frame: what a drag does, and what stops + // `render_detailed` reusing the previous frame's colour result and + // turning this into a measurement of nothing. + graph.set_param(exposure::ID, exposure::EXPOSURE, 0.30 + i as f32 * 0.01); + + let t0 = Instant::now(); + let shader = graph.compose(); + let detail = graph.compose_detail(src, viewport); + let colour_key = graph.invalidation().through(Affects::Colour) ^ PHOTOGRAPH; + let cpu_elapsed = t0.elapsed(); + + let t1 = Instant::now(); + adjust + .render_detailed(source, &shader, w, h, None, &detail, colour_key) + .expect("render"); + ctx.device + .poll(wgpu::PollType::wait_indefinitely()) + .expect("poll"); + let gpu_elapsed = t1.elapsed(); + + if i >= WARMUP { + cpu.push(cpu_elapsed.as_secs_f64() * 1e3); + gpu.push(gpu_elapsed.as_secs_f64() * 1e3); + total.push((cpu_elapsed + gpu_elapsed).as_secs_f64() * 1e3); + } + } + + Run { + cpu_p99: p99(cpu), + gpu_p99: p99(gpu), + total_p99: p99(total), + } +} + +/// Nearest-rank 99th percentile. +/// +/// Nearest-rank because the samples *are* the population: there is no +/// distribution being estimated, only a hundred frames that either fitted in +/// the budget or did not. +fn p99(mut samples: Vec) -> f64 { + samples.sort_by(f64::total_cmp); + let n = samples.len(); + samples[((0.99 * n as f64).ceil() as usize).clamp(1, n) - 1] +} + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +/// Every operation that contributes a fragment to the fused shader, active. +/// +/// Built from [`EditGraph::capabilities`] rather than from a list of operation +/// names, for the same reason the develop panel is: declaring a new node must +/// not silently shrink what this test calls "the full chain". A quarter of the +/// way from each parameter's default towards whichever end is further from it, +/// which is a plausible setting and — the part that matters — is never the +/// neutral, since a neutral operation contributes nothing at all. +/// +/// The film stock is not reachable this way (it is a choice of material, not a +/// slider) and is left off. It is one texture lookup and two curve reads; the +/// bench includes it and it is worth about a millisecond at this size. +fn full_point_chain() -> EditGraph { + let mut graph = EditGraph::default_chain(); + let moves: Vec<(OpId, ParamId, f32)> = graph + .capabilities() + .iter() + // The neighbourhood operations. See the module documentation for why + // they are not here. + .filter(|op| !op.attributes.contains(&Attribute::Detail)) + .flat_map(|op| { + op.params.iter().map(move |p| { + let value = match p.kind { + ParamKind::Scalar { min, max, .. } => { + let far = if (max - p.default).abs() >= (p.default - min).abs() { + max + } else { + min + }; + p.default + (far - p.default) * 0.25 + } + ParamKind::Bool => 1.0, + ParamKind::Enum { variants } => { + if variants.len() > 1 { + 1.0 + } else { + 0.0 + } + } + }; + (op.id, p.id, value) + }) + }) + .collect(); + for (op, param, value) in moves { + graph.set_param(op, param, value); + } + graph +} + +/// The view rect that puts one render pixel on one source pixel, centred. +fn one_to_one(render: (u32, u32)) -> CropRect { + let w = render.0 as f32 / SOURCE.0 as f32; + let h = render.1 as f32 / SOURCE.1 as f32; + CropRect { + x: (1.0 - w) * 0.5, + y: (1.0 - h) * 0.5, + width: w, + height: h, + } +} + +/// A source with structure at every scale. +/// +/// Not flat: a flat frame lets the memory system serve every sample of every +/// pixel from one cache line, which flatters a bandwidth-bound pass by an amount +/// that has nothing to do with photographs. +fn synthetic_source(ctx: &GpuContext) -> DemosaicedImage { + let (w, h) = SOURCE; + let mut rgba = vec![0u8; (w as usize) * (h as usize) * 4]; + for y in 0..h as usize { + let row = y * (w as usize) * 4; + for x in 0..w as usize { + let n = (x.wrapping_mul(2_654_435_761) ^ y.wrapping_mul(1_640_531_527)) >> 13; + let dither = (n & 0x1f) as u32; + let gx = (x * 200 / w as usize) as u32; + let gy = (y * 55 / h as usize) as u32; + let px = &mut rgba[row + x * 4..row + x * 4 + 4]; + px[0] = (30 + gx + dither).min(255) as u8; + px[1] = (40 + gy + dither).min(255) as u8; + px[2] = (60 + gx / 2 + gy + dither).min(255) as u8; + px[3] = 255; + } + } + DemosaicedImage::from_rgba8(ctx, &rgba, w, h).expect("upload") +} diff --git a/core/dr-gpu/tests/zoom_resolution.rs b/core/dr-gpu/tests/zoom_resolution.rs new file mode 100644 index 0000000..2a95022 --- /dev/null +++ b/core/dr-gpu/tests/zoom_resolution.rs @@ -0,0 +1,287 @@ +//! TRACES: FR-DSP-5 +//! Zooming to 1:1 samples the source, pixel for pixel. +//! +//! FR-DSP-5: *"Fit, 1:1, and arbitrary zoom levels. At 1:1 and above, the +//! pipeline operates on the visible crop at full source resolution."* +//! +//! `Framing::view` shrinks the sampled region while the render target keeps its +//! size, so zooming *raises* the resolution the pipeline works at rather than +//! magnifying pixels it has already drawn. There is no second full-resolution +//! code path — the zoom is the full-resolution path — which is why the +//! requirement has been satisfied for some time without anyone tagging it. +//! +//! `docs/display-and-extension.md` §7 is the reason this file exists rather +//! than a tag on `framing.rs`: a requirement counts as covered when a `TRACES` +//! comment names it, and nothing checks that the code under the tag does the +//! thing. `FR-DEV-8` is tagged against plumbing a future operation would use. +//! So the rule that document sets is that a requirement is closed by **a test +//! that would fail if the behaviour were removed**, and these are written to +//! fail in exactly that case: delete the view from `Framing::visible_rect` and +//! the 1:1 render collapses into the fit render, which +//! [`a_proxy_cannot_resolve_the_finest_detail_in_the_source`] establishes +//! carries none of the information the 1:1 render reproduces. +//! +//! # Why the fixture is alternating columns +//! +//! Because it makes "sampled at source resolution" a *pixel* assertion rather +//! than a "something got sharper" one. +//! +//! The source is one pixel black, one pixel white, all the way across. That is +//! the highest spatial frequency the image can hold, and it is exactly what a +//! proxy render throws away: a 1024-wide source in a 128-wide viewport maps +//! output column `x` to source column `8x + 4`, every one of which has the same +//! parity, so the whole proxy comes out flat. No amount of resampling that flat +//! image recovers the stripes. If the 1:1 render shows them — and shows them in +//! the right phase, from the right place in the source — then it read the +//! source and did not magnify the proxy. There is no third explanation. +//! +//! The source is uploaded through `DemosaicedImage::from_rgba8`, which flags it +//! non-linear, so the fused shader decodes sRGB before the chain and re-encodes +//! after it. Bytes 0 and 255 are fixed points of that round trip, which is why +//! the pattern is black and white and why the comparison can be for equality +//! rather than within a tolerance. + +use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext}; +use dr_pipeline::{CropRect, EditGraph}; + +/// A power of two, so every view rect below is exact in binary32 and the +/// mapping from output column to source column is exact arithmetic rather than +/// something that happens to round the right way. +const SOURCE: u32 = 1024; + +/// The viewport. `SOURCE / RENDER` is 8, so a fit render steps eight source +/// columns per output column — four full periods of the pattern. +const RENDER: u32 = 128; + +/// Where the 1:1 window sits in the source. **Odd on both axes on purpose**: a +/// view that honoured `width` but dropped `x` would land on the opposite phase +/// of the stripes and produce an exactly inverted image, which is the most +/// likely way for this to be subtly wrong and the one an assertion about +/// "contrast" or "variance" would sail straight past. +const WINDOW: (u32, u32) = (301, 157); + +fn ctx() -> Option { + // CI runners and headless machines may have no usable adapter. Skip rather + // than fail, exactly as the rest of this crate's device tests do. + match pollster::block_on(GpuContext::new_headless()) { + Ok(c) => Some(c), + Err(e) => { + eprintln!("skipping: no GPU adapter ({e})"); + None + } + } +} + +/// Black and white alternating every column: the finest detail an image can +/// carry. +fn stripes(ctx: &GpuContext) -> DemosaicedImage { + let data: Vec = (0..SOURCE * SOURCE) + .flat_map(|i| { + let v = if (i % SOURCE).is_multiple_of(2) { 0u8 } else { 255 }; + [v, v, v, 255] + }) + .collect(); + DemosaicedImage::from_rgba8(ctx, &data, SOURCE, SOURCE).expect("upload") +} + +/// What the source holds at `(x, y)` — the same rule [`stripes`] wrote. +fn source_byte(x: u32, _y: u32) -> u8 { + if x.is_multiple_of(2) { + 0 + } else { + 255 + } +} + +/// Render a neutral edit through `view` and hand back the bytes and the size. +/// +/// Neutral because this is a test about *which pixel* is read, and any active +/// operation would put a colour transform between the source byte and the +/// rendered one for no gain. +fn render(ctx: &GpuContext, source: &DemosaicedImage, view: CropRect) -> (Vec, u32, u32) { + let mut graph = EditGraph::default_chain(); + graph.framing_mut().set_view(view); + let shader = graph.compose(); + + let mut adjust = AdjustPass::new(ctx); + adjust + .render(source, &shader, RENDER, RENDER) + .expect("render"); + adjust.export_pixels().expect("readback") +} + +/// The view rect that puts one render pixel on one source pixel, with its +/// top-left corner at `WINDOW`. +fn one_to_one() -> CropRect { + CropRect { + x: WINDOW.0 as f32 / SOURCE as f32, + y: WINDOW.1 as f32 / SOURCE as f32, + width: RENDER as f32 / SOURCE as f32, + height: RENDER as f32 / SOURCE as f32, + } +} + +/// The red channel of one row of a rendered frame. +fn row(pixels: &[u8], width: u32, y: u32) -> Vec { + (0..width) + .map(|x| pixels[((y * width + x) * 4) as usize]) + .collect() +} + +/// TRACES: FR-DSP-5 +/// The proxy render carries none of the source's finest detail. +/// +/// Half of the argument, and the half that makes the other half mean something: +/// if the fit render already showed the stripes, a 1:1 render showing them would +/// prove nothing at all. It comes out uniform, so whatever the 1:1 render +/// contains cannot have come from resampling it. +#[test] +fn a_proxy_cannot_resolve_the_finest_detail_in_the_source() { + let Some(ctx) = ctx() else { return }; + let source = stripes(&ctx); + + let (pixels, w, h) = render(&ctx, &source, CropRect::default()); + assert_eq!((w, h), (RENDER, RENDER)); + + let first = pixels[0]; + let uniform = pixels + .chunks_exact(4) + .all(|px| px[0] == first && px[1] == first && px[2] == first); + assert!( + uniform, + "the fit render of a one-pixel stripe pattern should be flat — a 1024 px \ + source in a {RENDER} px viewport steps 8 source columns per output \ + column, so every sample lands on the same phase. It was not: row 0 is \ + {:?}. Either the sampling changed or the fixture no longer says what it \ + is meant to, and the 1:1 test below is worthless until this is true \ + again.", + &row(&pixels, w, 0)[..16.min(w as usize)] + ); +} + +/// TRACES: FR-DSP-5 +/// At 1:1 the render *is* the source region, byte for byte. +/// +/// The requirement's actual content — "at 1:1 and above, the pipeline operates +/// on the visible crop at full source resolution" — stated as the strongest +/// thing that could be true of it: not that the result is sharper, but that +/// output pixel `(x, y)` is source pixel `(WINDOW.0 + x, WINDOW.1 + y)` and +/// nothing has been interpolated, averaged or magnified on the way. +#[test] +fn a_one_to_one_view_reproduces_the_source_pixel_for_pixel() { + let Some(ctx) = ctx() else { return }; + let source = stripes(&ctx); + + let (pixels, w, h) = render(&ctx, &source, one_to_one()); + + // The render target keeps its size while the sampled region shrinks. That + // is the whole mechanism, and a zoom that resized the target would be + // magnification rather than resolution. + assert_eq!( + (w, h), + (RENDER, RENDER), + "zooming must not change the size of the render target" + ); + + let mut mismatches = Vec::new(); + for y in 0..h { + for x in 0..w { + let got = pixels[((y * w + x) * 4) as usize]; + let want = source_byte(WINDOW.0 + x, WINDOW.1 + y); + if got != want && mismatches.len() < 8 { + mismatches.push((x, y, got, want)); + } + } + } + assert!( + mismatches.is_empty(), + "a 1:1 view starting at {WINDOW:?} must reproduce the source exactly. \ + First mismatches (x, y, got, want): {mismatches:?}\n\ + rendered row 0: {:?}\n\ + source row 0: {:?}\n\ + An exactly inverted row means the view's *offset* was dropped while its \ + width was honoured; a flat row means the view was ignored altogether \ + and the pipeline is still rendering the proxy.", + &row(&pixels, w, 0)[..12], + (0..12) + .map(|x| source_byte(WINDOW.0 + x, WINDOW.1)) + .collect::>(), + ); +} + +/// TRACES: FR-DSP-5 +/// An arbitrary zoom between fit and 1:1 samples at the ratio it asks for. +/// +/// FR-DSP-5 says "fit, 1:1, **and arbitrary zoom levels**", and the two tests +/// above only pin the ends. This one takes the middle: a view a quarter of the +/// frame wide, which puts four source pixels behind each output pixel, and +/// checks that the pipeline reports and samples at that ratio rather than +/// snapping to one of the two cases anybody would have special-cased. +/// +/// `render_scale` is asserted alongside the pixels because it is what the +/// neighbourhood stage converts kernel radii through: a zoom that moved the +/// pixels but not the scale would silently sharpen at the wrong radius, which +/// is invisible until somebody compares a preview against an export. +#[test] +fn an_arbitrary_zoom_samples_at_the_ratio_it_asks_for() { + let Some(ctx) = ctx() else { return }; + let source = stripes(&ctx); + + // A quarter of the frame: 256 source columns across 128 output columns. + let view = CropRect { + x: 0.25, + y: 0.25, + width: 0.25, + height: 0.25, + }; + + let mut graph = EditGraph::default_chain(); + graph.framing_mut().set_view(view); + + // The region on screen is 256 source pixels wide, rendered into 128, so the + // ratio is one render pixel per two source pixels. + let scale = graph.render_scale((SOURCE, SOURCE), (RENDER, RENDER)); + assert_eq!(scale.full_size(), (256, 256)); + assert!( + (scale.ratio() - 0.5).abs() < 1e-6, + "ratio {}", + scale.ratio() + ); + + let (pixels, w, _) = render(&ctx, &source, view); + + // Where output column `x` reads from, worked through rather than asserted + // from a previous run — a test that recomputed this with the shader's own + // expression would agree with a bug in it. + // + // uv = 0.25 + (x + 0.5) / 128 * 0.25 = (128 + x + 0.5) / 512 + // col = floor(uv * 1024) = floor(256 + 2x + 1.0) = 257 + 2x + // + // Odd for every `x`, so this zoom lands flat — as the fit render does, and + // for the same reason. **The 1.0 is the interesting part.** At a two-to-one + // downsample an output pixel's centre falls exactly on the boundary between + // the two source pixels it covers, and truncation takes the right-hand one. + // That is nearest-neighbour behaving correctly and not an off-by-one; a + // reader checking this file by hand will get 256 on the first attempt, so + // it is written out. + const SAMPLED: u32 = 257; + + let first = pixels[0]; + assert!( + pixels.chunks_exact(4).all(|px| px[0] == first), + "a two-to-one zoom steps two source columns per output column, so every \ + sample has the same parity and the frame should be flat. row 0: {:?}", + &row(&pixels, w, 0)[..12] + ); + // And it is flat on the *right* phase. A pipeline that had ignored the view + // entirely would also be flat — but its `ratio` would not be 0.5, which the + // assertion above already rules out — and one that had snapped to 1:1 would + // show the stripes instead. Between them, only sampling the window the view + // actually asked for produces this. + assert_eq!( + first, + source_byte(SAMPLED, SAMPLED), + "a view starting a quarter of the way across a {SOURCE} px source should \ + sample from source column {SAMPLED}" + ); +} diff --git a/docs/display-and-extension.md b/docs/display-and-extension.md index 1844606..3dec37a 100644 --- a/docs/display-and-extension.md +++ b/docs/display-and-extension.md @@ -20,10 +20,10 @@ and the reason is that some of the work is done and untagged. | Requirement | Reality | |---|---| | FR-DSP-1 proxy rendering | **Done.** The develop view renders at viewport resolution, not source. | -| FR-DSP-2 tiled computation | **Absent.** The fused pass renders the whole viewport in one dispatch. | -| FR-DSP-3 interactive latency | **Unmeasured.** No frame budget is asserted anywhere. | -| FR-DSP-4 progressive refinement | **Absent.** Every render is full quality. | -| FR-DSP-5 zoom and pan | **Substantially done, untagged.** `Framing::view` shrinks the sampled region while the render target keeps its size, so zooming *raises* the resolution the pipeline works at. That is FR-DSP-5's requirement, arrived at without tiles. | +| FR-DSP-2 tiled computation | **Absent, and §2 now says it should stay that way.** Measured: the fused pass is inside the budget everywhere. See [frame-budget.md](frame-budget.md). | +| FR-DSP-3 interactive latency | **Measured and asserted** for the fused path — `core/dr-gpu/tests/frame_budget.rs`. Missed by one operation, clarity, for the reason recorded as TD-4. | +| FR-DSP-4 progressive refinement | **Absent**, and §4's condition did not fire. Every render is full quality and can afford to be. | +| FR-DSP-5 zoom and pan | **Done and tagged**, against tests that fail if the behaviour is removed — `core/dr-gpu/tests/zoom_resolution.rs`. `Framing::view` shrinks the sampled region while the render target keeps its size, so zooming *raises* the resolution the pipeline works at. That is FR-DSP-5's requirement, arrived at without tiles. | | FR-DSP-6 colour management | **Done.** Output space is a parameter of composition. | | FR-DSP-7 histogram and clipping | **Done**, GPU-side, no per-frame readback. | | FR-DSP-8 per-display colour | **Done**, with one caveat named in §5.4. Acquisition per display server, an sRGB fallback that is visible in About, and the canvas rendered at physical pixel size. | @@ -32,12 +32,25 @@ and the reason is that some of the work is done and untagged. Two of the five uncovered display requirements are therefore *measurement and tagging*, not construction. That is worth knowing before anyone plans a quarter around them. +**Both have since been done.** [frame-budget.md](frame-budget.md) holds the measurements §2 asks +for and the reading of its decision rule; the table above is updated to match. The rest of this +document is left as it was written, because a plan that has been overtaken by its own evidence is +more useful read in order than quietly edited into agreement. + --- ## 2. Measure before building tiles **FR-DSP-2 is the one requirement in this document that may not be worth satisfying as written.** +> **Resolved.** M1–M3 were run; the numbers and the verdict are in +> [frame-budget.md](frame-budget.md). The rule below fired for *rewrite*: every point-operation +> chain is inside 16 ms at the 99th percentile at every viewport size, fit and at 1:1, the widest +> being 4.5 ms of GPU at 4K. The measurement did find a stage that misses the budget — clarity's +> 52-pixel kernel, 34 ms at 4K — and tiling makes that stage *worse*, since a tiled convolution +> reads a halo per tile. It is recorded as TD-4 with the fix its own module already names. + + The requirement predates the fused-shader design. It assumes the pipeline is a chain of passes over a large buffer, where recomputing everything on each frame would be ruinous and tiles are the way out. What was built instead composes every active operation into **one dispatch over a diff --git a/docs/frame-budget.md b/docs/frame-budget.md new file mode 100644 index 0000000..7b09057 --- /dev/null +++ b/docs/frame-budget.md @@ -0,0 +1,297 @@ +# What a frame costs + +**Status:** Measured · 2026-08-27 +**Companion to:** [display-and-extension.md](display-and-extension.md) §2–3 · +[requirements.md](requirements.md) §3.4 FR-DSP-2, FR-DSP-3, FR-DSP-4 +**Instrument:** [`core/dr-gpu/examples/frame_budget.rs`](../core/dr-gpu/examples/frame_budget.rs) +**Guard:** [`core/dr-gpu/tests/frame_budget.rs`](../core/dr-gpu/tests/frame_budget.rs) + +[display-and-extension.md](display-and-extension.md) §2 fixed a decision rule in +advance and made three measurements the thing that settles it. This file is +those measurements, and the recommendation they support. + +Rerun with: + +```sh +cargo run --release -p dr-gpu --example frame_budget +``` + +and diff this file. That is the whole point of committing numbers: a regression +should be a diff rather than somebody's recollection of how fast it used to be. + +--- + +## The answer, first + +**FR-DSP-2 should be rewritten, not implemented.** M1 and M2 sit inside the +16 ms budget at the 99th percentile for every chain of point operations at every +viewport size measured, fit and at 1:1 — the widest case, every operation that +contributes a fragment to the fused shader at 4K, costs **4.5 ms** on the GPU and +**8.2 ms** including the composition that precedes it. Tiling the interactive +path would be optimising something that is already using a quarter of its budget. + +**But the measurement did find a budget-breaker, and it is not the one tiling +fixes.** The neighbourhood stage — clarity in particular — costs **34 ms at 4K +on its own**, twice the whole budget, and tiles do not help it: a tile of a +convolution has to read its halo, so tiling raises the total tap count rather +than lowering it. §2 predicted this exactly ("a separable blur at a large radius +is the plausible budget-breaker, not the fused pass"), and the fix it needs is +the one `local_contrast`'s own module documentation already names — a base +computed at reduced resolution — which is a change to `crate::detail`, not a +tile scheduler. + +There is a third finding nobody was looking for: **shader composition costs +3–5 ms of CPU per frame on a full chain**, on the UI thread, before any GPU work +is submitted. That is a fifth to a third of the budget spent formatting strings, +and it is invisible to any amount of tiling. + +--- + +## Conditions + +| | | +|---|---| +| Adapter | NVIDIA GeForce RTX 3050 6GB Laptop GPU (Vulkan) | +| Source | 9504 × 6336 synthetic (60.2 MP, 482 MB as `rgba16f`) | +| Frames | 100 measured per row, 12 warm-up frames discarded | +| Percentile | Nearest-rank, so p99 of 100 frames is the second-worst frame | +| Build | `--release` | +| Date | 2026-08-27 | + +`shader` is `EditGraph::compose` alone. `cpu` adds the detail chain and the +invalidation hash — everything `DevelopSession::render` does per frame before it +dispatches. `gpu` is submit plus wait-for-idle, which serialises the GPU work +into the frame that caused it and is therefore pessimistic. `TOTAL` ranks +`cpu + gpu` summed **within each frame**, which is the column the budget is +judged on; adding two percentiles instead would invent a stutter that no frame +actually had. + +Chains: `one` is exposure. `five` is exposure, contrast, highlights/shadows, +blacks/whites, vibrance. `point` is every operation in the default chain that +contributes a fragment to the fused shader, film stock included. `all` is `point` +plus the four neighbourhood operations — noise reduction, capture sharpening, +clarity and texture. + +--- + +## M1 — the fused pass at proxy resolution + +The develop view: the whole frame fit to the viewport. + +| size | chain | shader | cpu p99 | gpu p50 | gpu p99 | TOTAL | | +|------------:|------:|-------:|--------:|--------:|--------:|--------:|:-----| +| 1920 × 1200 | one | 0.08ms | 0.10ms | 1.02ms | 1.23ms | 1.31ms | | +| 1920 × 1200 | five | 0.18ms | 0.20ms | 1.01ms | 1.20ms | 1.36ms | | +| 1920 × 1200 | point | 2.79ms | 2.82ms | 1.98ms | 2.18ms | 4.83ms | | +| 1920 × 1200 | all | 3.65ms | 4.73ms | 6.86ms | 7.37ms | 12.02ms | | +| 2560 × 1600 | one | 0.08ms | 0.10ms | 1.73ms | 2.00ms | 2.12ms | | +| 2560 × 1600 | five | 0.24ms | 0.27ms | 1.73ms | 2.26ms | 2.46ms | | +| 2560 × 1600 | point | 2.82ms | 2.85ms | 2.37ms | 2.65ms | 5.38ms | | +| 2560 × 1600 | all | 3.37ms | 4.35ms | 14.31ms | 15.65ms | 18.42ms | OVER | +| 3840 × 2160 | one | 0.10ms | 0.14ms | 3.09ms | 3.31ms | 3.42ms | | +| 3840 × 2160 | five | 0.30ms | 0.32ms | 3.03ms | 3.40ms | 3.61ms | | +| 3840 × 2160 | point | 3.62ms | 3.65ms | 4.12ms | 4.52ms | 8.23ms | | +| 3840 × 2160 | all | 4.12ms | 5.07ms | 37.73ms | 40.17ms | 43.24ms | OVER | + +Read the `point` rows: **the fused dispatch scales with pixels and almost not at +all with chain length.** Going from one operation to the entire point chain at +4K costs 1.2 ms of GPU. Going from 2.3 M pixels to 8.3 M costs 2.3 ms. Both are +small, and the second is the one tiling would address. + +The `all` rows go over, and the `point` rows in the same block are what say why: +the difference between them is the neighbourhood stage, measured on its own in +M3 and arriving at almost exactly the same figure. + +## M2 — the same, zoomed to 1:1 on the 60 MP source + +FR-DSP-5's case. `Framing::view` shrinks the sampled region while the render +target keeps its size, so one render pixel lands on one source pixel. + +| size | chain | shader | cpu p99 | gpu p50 | gpu p99 | TOTAL | | +|------------:|------:|-------:|--------:|--------:|--------:|--------:|:-----| +| 1920 × 1200 | one | 0.12ms | 0.14ms | 0.42ms | 0.66ms | 0.75ms | | +| 1920 × 1200 | five | 0.27ms | 0.30ms | 0.49ms | 1.14ms | 1.22ms | | +| 1920 × 1200 | point | 3.49ms | 3.52ms | 1.18ms | 1.39ms | 4.85ms | | +| 1920 × 1200 | all | 3.64ms | 5.18ms | 8.96ms | 9.55ms | 14.30ms | | +| 2560 × 1600 | one | 0.11ms | 0.12ms | 0.56ms | 0.99ms | 1.06ms | | +| 2560 × 1600 | five | 0.22ms | 0.25ms | 0.77ms | 1.02ms | 1.17ms | | +| 2560 × 1600 | point | 2.96ms | 2.99ms | 2.03ms | 2.52ms | 5.61ms | | +| 2560 × 1600 | all | 5.24ms | 7.35ms | 18.80ms | 21.62ms | 25.81ms | OVER | +| 3840 × 2160 | one | 0.10ms | 0.12ms | 1.31ms | 1.52ms | 1.63ms | | +| 3840 × 2160 | five | 0.14ms | 0.27ms | 1.39ms | 1.64ms | 1.75ms | | +| 3840 × 2160 | point | 3.14ms | 3.16ms | 4.04ms | 4.50ms | 7.21ms | | +| 3840 × 2160 | all | 4.84ms | 6.78ms | 47.22ms | 48.79ms | 54.47ms | OVER | + +**A 1:1 view of a 60 MP file is cheaper than the fit view of the same file**, for +every point chain and at every size — 1.52 ms against 3.31 ms for one operation +at 4K. That is not a rounding artefact and it is worth stating plainly, because +it is the opposite of what "full resolution" sounds like it should cost. The +dispatch is the same number of pixels either way; what changes is where those +pixels read from. A fit view walks the whole 482 MB texture on a stride, and a +1:1 view reads a contiguous window of it that fits comfortably in cache. + +So the resolution FR-DSP-5 promises costs nothing extra on the fused path. +Zooming is not an expensive mode to be dreaded and progressively refined into; +it is the cheap one. + +The `all` rows are worse at 1:1 than fit, and that is the detail stage again for +a specific reason: noise reduction's radius is stated in *source* pixels, so +`RenderScale::ratio` climbing to 1.0 widens its kernel. Clarity's is stated as a +fraction of the frame and does not move. M3 separates the two. + +## M3 — the neighbourhood stage alone + +Timed with the fused dispatch deliberately reused: only a detail parameter moves, +so `render_detailed` skips the colour pass (FR-DEV-3d) and what remains is the +convolutions. `colour` counts fused dispatches over the measured frames and is +zero on every row, which is what makes these numbers mean "detail alone" rather +than asserting it. + +| size | stage | view | pass | radius | colour | cpu p99 | p50 | p99 | +|------------:|---------:|:-----|-----:|-------:|-------:|--------:|--------:|--------:| +| 1920 × 1200 | clarity | fit | 2 | 29 | 0 | 1.60ms | 5.42ms | 5.99ms | +| 1920 × 1200 | all four | fit | 7 | 29 | 0 | 1.71ms | 5.78ms | 6.16ms | +| 1920 × 1200 | clarity | 1:1 | 2 | 29 | 0 | 1.12ms | 7.51ms | 8.01ms | +| 1920 × 1200 | all four | 1:1 | 9 | 29 | 0 | 2.71ms | 8.25ms | 9.11ms | +| 2560 × 1600 | clarity | fit | 2 | 38 | 0 | 1.03ms | 12.02ms | 12.44ms | +| 2560 × 1600 | all four | fit | 7 | 38 | 0 | 1.87ms | 12.49ms | 13.16ms | +| 2560 × 1600 | clarity | 1:1 | 2 | 38 | 0 | 1.87ms | 15.82ms | 16.60ms | +| 2560 × 1600 | all four | 1:1 | 9 | 38 | 0 | 2.71ms | 17.24ms | 18.06ms | +| 3840 × 2160 | clarity | fit | 2 | 52 | 0 | 1.75ms | 33.11ms | 33.89ms | +| 3840 × 2160 | all four | fit | 7 | 52 | 0 | 1.76ms | 34.21ms | 35.03ms | +| 3840 × 2160 | clarity | 1:1 | 2 | 52 | 0 | 1.08ms | 40.39ms | 41.86ms | +| 3840 × 2160 | all four | 1:1 | 9 | 52 | 0 | 2.37ms | 43.29ms | 44.72ms | + +`radius` is the widest halo any pass reads, in render pixels. + +Clarity alone is 97% of the cost of all four neighbourhood operations together, +at every size. Its σ is 1.2% of the shorter edge and it truncates at 2σ, so its +radius is 29 px on a 1200 px viewport and **52 px at 4K** — two separable passes +of 105 taps each, over 8.3 M pixels, which is 1.7 billion texture reads. That is +the whole of the problem, and the numbers scale as `radius × pixels` exactly as +that description predicts: 5.99 → 12.44 → 33.89 ms for radii of 29 → 38 → 52 over +2.3 → 4.1 → 8.3 M pixels. + +The extra cost at 1:1 is noise reduction and capture sharpening, whose radii are +properties of the sensor rather than of the frame. That is the correct behaviour +— it is why `RenderScale` has two units — and it is bounded by the kernel caps +those operations already declare. + +--- + +## Reading this against §2's decision rule + +§2: *"If M1 and M2 sit inside 16 ms at the 99th percentile, FR-DSP-2 is +rewritten rather than implemented … If they do not, the measurement tells us +which stage to tile."* + +Both halves of the rule fire, on different stages, and the honest reading takes +both. + +### FR-DSP-2 — rewrite it + +For the fused pass the rule passes with a wide margin. Every point chain at +every size, fit and at 1:1, is inside 16 ms — the worst `TOTAL` is 8.23 ms and +the worst GPU figure is 4.52 ms. There is no viewport size on a desktop display +where recomputing the entire point chain over every visible pixel is a problem. + +Two further reasons not to build the tile scheduler as written: + +1. **Panning, which is the case ARCH §5.3's tile cache is designed for, gets no + benefit here.** Reusing already-valid tiles saves recomputation. Recomputing + the whole 4K viewport costs 4.5 ms, so a perfect tile cache could save at most + 4.5 ms of a 16 ms budget, at the price of a cache keyed by + `(VersionId, tile, zoom, graph_hash_prefix)` that has to stay correct across + every parameter change in the graph. That is a large correctness surface + bought with a small number. + +2. **It would make the actual problem worse.** The stage that misses the budget + is a convolution, and a tiled convolution reads a halo per tile. At a 52-pixel + radius, 256-pixel tiles would read (256+104)² instead of 256² — very nearly + *twice* the taps. Tiling is the wrong tool for the one stage that needs a + tool. + +So FR-DSP-2 becomes what §2 said it actually is for this architecture: a +scheduling concern for export and thumbnailing, both of which already run off +the frame path. The interactive path does not tile. + +### The stage that does need work — and it is not tiling + +The measurement's real product is naming the stage. It is `local_contrast`, and +the fix is stated in that module's own documentation: + +> The right optimisation is a base computed at reduced resolution, which needs a +> detail stage that can write a smaller target than it reads; that is a change to +> `crate::detail`, not to this file. + +A Gaussian base at a quarter resolution is 1/16 the pixels at 1/4 the radius — +about 1/64 of the work — and the result is visually identical because a base at +σ = 26 px has no content above the quarter-resolution Nyquist to lose. That is a +change to two files with a bounded blast radius, and it is what the 34 ms buys +back. It should be tracked as its own item rather than smuggled in under a +requirement about tiles. + +### FR-DSP-3 — the clause that should be narrowed + +§3.3 proposes narrowing "when a full-resolution result is needed it is computed +asynchronously, and the proxy result remains on screen until it is ready" to +export and 1:1 zoom, or striking it. + +**M2 says strike it.** The clause exists to hide the latency of a +full-resolution render behind a proxy. There is no such latency: the 1:1 view is +*faster* than the fit view on the fused path, and there is no second +full-resolution code path to be asynchronous about — `Framing::view` is the +whole mechanism. Export renders its own frames on a worker already. Keeping the +clause would mean building a progressive-swap machine to conceal a render that +completes in 1.4 ms. + +### FR-DSP-4 — satisfied vacuously, on the fused path + +§4 makes progressive refinement conditional on M1 failing. On the fused path M1 +passes, so reduced-quality rendering during a drag would buy nothing and cost the +visible softness the requirement itself warns against. + +The neighbourhood stage is the exception, and it is worth being precise: what +that stage needs is not *progressive* refinement — it is a permanently cheaper +base, computed at reduced resolution and correct at any moment the user stops. +"Render coarse while dragging, sharpen when it settles" would paper over the same +34 ms with a visible swap. Fix the stage. + +--- + +## What is not measured here + +Stated because §7 of [display-and-extension.md](display-and-extension.md) asks +for it, and because each of these could move the numbers. + +- **Local adjustments.** The mask stack is a separate chain per layer and is not + in any row above. `render_masked` takes them and the fused shader addresses + them per layer, so a heavily masked edit costs more than `all`. +- **Spot repairs.** These add detail passes, and their cost is per spot. +- **Lens corrections.** Not part of `EditGraph::default_chain` — they are built + from a matched profile — so the `point` row does not include the warp chain. +- **Demosaic.** Once per photograph on a worker, not on the frame path. +- **Presentation.** The bench waits for the device to go idle inside the frame it + measures. A real compositor overlaps frames, so these figures are an upper + bound rather than an estimate. +- **One adapter.** A discrete laptop GPU. The Intel iGPU on the same machine, and + Android, will be slower — which is an argument for the conclusion rather than + against it: the stage with no headroom has none to lose. + +## The CPU finding, which deserves its own item + +`EditGraph::compose` costs 2.8–5.2 ms per frame on a full chain, at every +resolution, because it is resolution-independent: it assembles a WGSL string and +hashes it. On the `all` rows it is a third of what is left of the budget after +the GPU has taken its share, and at 1920 × 1200 it is larger than the entire +fused dispatch. + +Nothing in this document's recommendations changes it, and it is the cheapest +remaining win. The generated *source* depends only on the structure of the graph +— that is what `structure_hash` already identifies, and it is precisely what does +not change while a slider is being dragged, which is why the pipeline cache in +`AdjustPass` does not recompile. The uniforms do change, but assembling them is a +handful of floats per operation. So caching the source string against the +structure hash and rebuilding only the uniforms would take these milliseconds to +approximately nothing, on the path that needs them most. Worth its own entry in +[technical-debt.md](technical-debt.md). diff --git a/docs/technical-debt.md b/docs/technical-debt.md index 69c76e5..0c6dda8 100644 --- a/docs/technical-debt.md +++ b/docs/technical-debt.md @@ -142,6 +142,103 @@ in well under a second. --- +## TD-4 — The local-contrast base is computed at full render resolution + +**Where:** `dr_pipeline::ops::local_contrast::LocalContrast::passes` — the `base` and `combine` +passes, and the stage that dispatches them, `dr_pipeline::detail`. + +Breaks **FR-DSP-3** at large viewports. Measured, and the numbers are in +[frame-budget.md](frame-budget.md) §M3. + +### What it does + +Clarity's Gaussian σ is 1.2% of the frame's shorter edge, truncated at 2σ, so its kernel radius is +a property of the *viewport*: 29 px at 1920 × 1200, 38 px at 2560 × 1600, **52 px at 4K**. The two +separable passes therefore run 105 taps each over 8.3 M pixels at 4K, which is 1.7 billion texture +reads for one control. + +| viewport | radius | clarity alone, p99 | +|---|---:|---:| +| 1920 × 1200 | 29 | 5.99 ms | +| 2560 × 1600 | 38 | 12.44 ms | +| 3840 × 2160 | 52 | **33.89 ms** | + +RTX 3050 laptop, `examples/frame_budget`, fused dispatch reused so this is the convolutions alone. +Clarity is 97% of the cost of all four neighbourhood operations together at every size. + +For scale: the entire fused chain — every point operation active, film stock included — costs +4.5 ms at the same 4K viewport. **A single slider is seven times the rest of the pipeline.** + +### Why + +Because the stage cannot do otherwise yet. `dr_pipeline::detail` dispatches every pass at the +render size; there is no way to express "read this target and write a smaller one". The module's own +documentation has said so since it was written: + +> The right optimisation is a base computed at reduced resolution, which needs a detail stage that +> can write a smaller target than it reads; that is a change to `crate::detail`, not to this file. + +It was the right call to ship the correct answer slowly rather than a fast approximation nobody had +checked — the halo behaviour is the hard part of this operation and it is tested. + +### Not a tiling problem + +Worth saying because ARCH §5.3 offers a tile cache and this is the stage that looks like it wants +one. It does not: a tiled convolution reads a halo per tile, so at a 52-pixel radius, 256-pixel +tiles would read (256 + 104)² instead of 256² — very nearly **twice** the taps. +[display-and-extension.md](display-and-extension.md) §2's decision rule was resolved on this +evidence; see [frame-budget.md](frame-budget.md). + +### Paying it off + +A detail pass that declares an output scale, so the base can be computed at a quarter resolution and +sampled back up in `combine`. A quarter-resolution base is 1/16 the pixels at 1/4 the radius — +about **1/64 of the work** — and is visually identical, because a base at σ = 26 px holds no content +above the quarter-resolution Nyquist to lose. Texture's σ is a decade finer and must stay at full +resolution; the scale therefore belongs on the `DetailPass`, not on the stage. + +**Done when:** clarity at 100% is inside the frame budget at 3840 × 2160, the halo tests in +`tests/local_contrast.rs` still pass unchanged, and `examples/frame_budget`'s M3 table in +[frame-budget.md](frame-budget.md) has been rerun and committed. + +--- + +## TD-5 — The fused shader is reassembled from strings on every frame + +**Where:** `dr_pipeline::operation::compose_full`, called from `DevelopSession::render`. + +### What it does + +`EditGraph::compose` walks the active operations and formats a WGSL source string, per frame, on +the UI thread. On a full chain that is **2.8–5.2 ms** — at 1920 × 1200 it is larger than the entire +fused dispatch it precedes, and on the chains that also carry a detail stage it is a third of what +is left of the 16 ms budget after the GPU has taken its share. It does not vary with resolution, +because it is not pixel work. + +### Why + +Because it was free until the chain got long. Composition was written when an edit was two or three +operations, and the cost is roughly linear in generated source: the colour mixer emits twelve hue bands +and the tone curve emits a spline evaluator, so a full chain is a large string built from scratch +sixty times a second. + +### Paying it off + +The generated source depends only on the *structure* of the graph — which is precisely what +`ComposedShader::structure_hash` already identifies, and precisely what does not change while a +slider is being dragged. `AdjustPass` relies on that already: it caches compiled pipelines against +that hash and does not recompile during a drag. Caching the source string against the same hash and +rebuilding only the uniforms — a handful of floats per operation — takes this to approximately +nothing on the path that needs it most. + +The care needed is in what the hash covers. It deliberately excludes parameter *magnitudes*, so a +cache keyed on it is sound for the source and would be wrong for anything else in `ComposedShader`. + +**Done when:** the `shader` column of [frame-budget.md](frame-budget.md)'s M1 table is under a +millisecond for the `point` and `all` chains, and the codegen tests still pass byte for byte. + +--- + ## Related, and deliberately not here The window-move rule, the grid's ordering index and the whole-library readout cache were *fixed* diff --git a/docs/traceability.md b/docs/traceability.md index 84cb798..acb9277 100644 --- a/docs/traceability.md +++ b/docs/traceability.md @@ -9,17 +9,17 @@ Denominators are parsed from [`requirements.md`](requirements.md) at run time, n | Metric | Value | |---|---| -| Source files scanned | 265 | -| TRACES tags found | 765 | +| Source files scanned | 268 | +| TRACES tags found | 770 | | Requirements defined | 177 | -| Requirements covered | 102 | -| **Coverage** | **57.6%** (102/177) | +| Requirements covered | 104 | +| **Coverage** | **58.8%** (104/177) | ### By type | Type | Covered | Defined | |---|---|---| -| FR | 80 | 122 | +| FR | 82 | 122 | | NFR | 20 | 49 | | R | 2 | 6 | @@ -71,6 +71,8 @@ _None._ | FR-DEV-7 | [`core/dr-pipeline/src/history.rs:214`](../core/dr-pipeline/src/history.rs#L214), [`core/dr-pipeline/src/history.rs:499`](../core/dr-pipeline/src/history.rs#L499), [`core/dr-pipeline/src/history.rs:526`](../core/dr-pipeline/src/history.rs#L526), [`ui/dr-ui/src/develop.rs:3468`](../ui/dr-ui/src/develop.rs#L3468), [`ui/dr-ui/src/develop.rs:3500`](../ui/dr-ui/src/develop.rs#L3500), [`ui/dr-ui/src/lib.rs:1437`](../ui/dr-ui/src/lib.rs#L1437), [`ui/dr-ui/src/lib.rs:2323`](../ui/dr-ui/src/lib.rs#L2323), [`ui/dr-ui/ui/history.slint:1`](../ui/dr-ui/ui/history.slint#L1) | | FR-DEV-8 | [`core/dr-gpu/src/detail.rs:252`](../core/dr-gpu/src/detail.rs#L252), [`core/dr-gpu/src/detail.rs:434`](../core/dr-gpu/src/detail.rs#L434), [`core/dr-gpu/tests/detail_instances.rs:1`](../core/dr-gpu/tests/detail_instances.rs#L1), [`core/dr-gpu/tests/spot_removal.rs:1`](../core/dr-gpu/tests/spot_removal.rs#L1), [`core/dr-pipeline/src/detail.rs:363`](../core/dr-pipeline/src/detail.rs#L363), [`core/dr-pipeline/src/detail.rs:387`](../core/dr-pipeline/src/detail.rs#L387), [`core/dr-pipeline/src/detail.rs:422`](../core/dr-pipeline/src/detail.rs#L422), [`core/dr-pipeline/src/detail.rs:496`](../core/dr-pipeline/src/detail.rs#L496), [`core/dr-pipeline/src/graph.rs:113`](../core/dr-pipeline/src/graph.rs#L113), [`core/dr-pipeline/src/graph.rs:191`](../core/dr-pipeline/src/graph.rs#L191), [`core/dr-pipeline/src/graph.rs:681`](../core/dr-pipeline/src/graph.rs#L681), [`core/dr-pipeline/src/operation.rs:299`](../core/dr-pipeline/src/operation.rs#L299), [`core/dr-pipeline/src/operation.rs:517`](../core/dr-pipeline/src/operation.rs#L517), [`core/dr-pipeline/src/sidecar.rs:183`](../core/dr-pipeline/src/sidecar.rs#L183), [`core/dr-pipeline/src/sidecar.rs:352`](../core/dr-pipeline/src/sidecar.rs#L352), [`core/dr-pipeline/src/sidecar.rs:672`](../core/dr-pipeline/src/sidecar.rs#L672), [`core/dr-pipeline/src/sidecar.rs:808`](../core/dr-pipeline/src/sidecar.rs#L808), [`core/dr-pipeline/src/sidecar.rs:862`](../core/dr-pipeline/src/sidecar.rs#L862), [`core/dr-pipeline/src/sidecar.rs:892`](../core/dr-pipeline/src/sidecar.rs#L892), [`core/dr-pipeline/src/spot.rs:115`](../core/dr-pipeline/src/spot.rs#L115), [`core/dr-pipeline/src/spot.rs:151`](../core/dr-pipeline/src/spot.rs#L151), [`core/dr-pipeline/src/spot.rs:1`](../core/dr-pipeline/src/spot.rs#L1), [`core/dr-pipeline/src/spot.rs:207`](../core/dr-pipeline/src/spot.rs#L207), [`core/dr-pipeline/src/spot.rs:387`](../core/dr-pipeline/src/spot.rs#L387), [`core/dr-pipeline/src/spot.rs:472`](../core/dr-pipeline/src/spot.rs#L472), [`core/dr-pipeline/src/spot.rs:582`](../core/dr-pipeline/src/spot.rs#L582), [`core/dr-pipeline/src/spot.rs:673`](../core/dr-pipeline/src/spot.rs#L673), [`core/dr-pipeline/src/state.rs:103`](../core/dr-pipeline/src/state.rs#L103), [`core/dr-pipeline/tests/spot_sidecar.rs:1`](../core/dr-pipeline/tests/spot_sidecar.rs#L1), [`core/dr-pipeline/tests/spots.rs:1`](../core/dr-pipeline/tests/spots.rs#L1), [`ui/dr-ui/src/develop.rs:2144`](../ui/dr-ui/src/develop.rs#L2144), [`ui/dr-ui/src/develop.rs:2182`](../ui/dr-ui/src/develop.rs#L2182), [`ui/dr-ui/src/develop.rs:2247`](../ui/dr-ui/src/develop.rs#L2247), [`ui/dr-ui/src/develop.rs:2330`](../ui/dr-ui/src/develop.rs#L2330), [`ui/dr-ui/src/develop.rs:2344`](../ui/dr-ui/src/develop.rs#L2344), [`ui/dr-ui/src/develop.rs:658`](../ui/dr-ui/src/develop.rs#L658), [`ui/dr-ui/src/labels.rs:52`](../ui/dr-ui/src/labels.rs#L52), [`ui/dr-ui/src/lib.rs:1458`](../ui/dr-ui/src/lib.rs#L1458), [`ui/dr-ui/src/lib.rs:2420`](../ui/dr-ui/src/lib.rs#L2420), [`ui/dr-ui/src/lib.rs:307`](../ui/dr-ui/src/lib.rs#L307), [`ui/dr-ui/src/spots_ui.rs:19`](../ui/dr-ui/src/spots_ui.rs#L19), [`ui/dr-ui/src/spots_ui.rs:1`](../ui/dr-ui/src/spots_ui.rs#L1), [`ui/dr-ui/src/spots_ui.rs:265`](../ui/dr-ui/src/spots_ui.rs#L265), [`ui/dr-ui/ui/adjust.slint:676`](../ui/dr-ui/ui/adjust.slint#L676), [`ui/dr-ui/ui/app.slint:1853`](../ui/dr-ui/ui/app.slint#L1853), [`ui/dr-ui/ui/app.slint:2370`](../ui/dr-ui/ui/app.slint#L2370), [`ui/dr-ui/ui/app.slint:2636`](../ui/dr-ui/ui/app.slint#L2636), [`ui/dr-ui/ui/app.slint:339`](../ui/dr-ui/ui/app.slint#L339), [`ui/dr-ui/ui/spots.slint:48`](../ui/dr-ui/ui/spots.slint#L48), [`ui/dr-ui/ui/spots.slint:5`](../ui/dr-ui/ui/spots.slint#L5) | | FR-DSP-1 | [`core/dr-gpu/src/adjust.rs:2088`](../core/dr-gpu/src/adjust.rs#L2088), [`core/dr-gpu/src/adjust.rs:2165`](../core/dr-gpu/src/adjust.rs#L2165), [`core/dr-gpu/src/adjust.rs:2250`](../core/dr-gpu/src/adjust.rs#L2250), [`core/dr-gpu/src/adjust.rs:54`](../core/dr-gpu/src/adjust.rs#L54), [`core/dr-gpu/src/adjust.rs:770`](../core/dr-gpu/src/adjust.rs#L770), [`core/dr-gpu/src/lib.rs:54`](../core/dr-gpu/src/lib.rs#L54), [`core/dr-gpu/src/lib.rs:94`](../core/dr-gpu/src/lib.rs#L94), [`core/dr-gpu/tests/capture_sharpen.rs:200`](../core/dr-gpu/tests/capture_sharpen.rs#L200), [`core/dr-gpu/tests/detail_stage.rs:328`](../core/dr-gpu/tests/detail_stage.rs#L328), [`core/dr-gpu/tests/local_contrast.rs:263`](../core/dr-gpu/tests/local_contrast.rs#L263), [`core/dr-gpu/tests/noise_reduction.rs:378`](../core/dr-gpu/tests/noise_reduction.rs#L378), [`core/dr-pipeline/src/detail.rs:136`](../core/dr-pipeline/src/detail.rs#L136), [`core/dr-pipeline/src/detail.rs:465`](../core/dr-pipeline/src/detail.rs#L465), [`core/dr-pipeline/src/graph.rs:543`](../core/dr-pipeline/src/graph.rs#L543), [`core/dr-pipeline/src/graph.rs:573`](../core/dr-pipeline/src/graph.rs#L573), [`core/dr-pipeline/src/ops/capture_sharpen.rs:1`](../core/dr-pipeline/src/ops/capture_sharpen.rs#L1), [`core/dr-pipeline/src/ops/capture_sharpen.rs:654`](../core/dr-pipeline/src/ops/capture_sharpen.rs#L654), [`core/dr-pipeline/src/ops/local_contrast.rs:1`](../core/dr-pipeline/src/ops/local_contrast.rs#L1), [`core/dr-pipeline/src/ops/local_contrast.rs:662`](../core/dr-pipeline/src/ops/local_contrast.rs#L662), [`core/dr-pipeline/src/ops/noise_reduction.rs:695`](../core/dr-pipeline/src/ops/noise_reduction.rs#L695), [`core/dr-pipeline/src/spot.rs:673`](../core/dr-pipeline/src/spot.rs#L673), [`ui/dr-ui/src/develop.rs:2668`](../ui/dr-ui/src/develop.rs#L2668), [`ui/dr-ui/src/develop.rs:3698`](../ui/dr-ui/src/develop.rs#L3698), [`ui/dr-ui/src/develop.rs:4181`](../ui/dr-ui/src/develop.rs#L4181), [`ui/dr-ui/src/develop.rs:4215`](../ui/dr-ui/src/develop.rs#L4215), [`ui/dr-ui/src/lib.rs:71`](../ui/dr-ui/src/lib.rs#L71), [`ui/dr-ui/src/lib.rs:737`](../ui/dr-ui/src/lib.rs#L737), [`ui/dr-ui/src/lib.rs:796`](../ui/dr-ui/src/lib.rs#L796) | +| FR-DSP-3 | [`core/dr-gpu/tests/frame_budget.rs:101`](../core/dr-gpu/tests/frame_budget.rs#L101) | +| FR-DSP-5 | [`core/dr-gpu/tests/frame_budget.rs:101`](../core/dr-gpu/tests/frame_budget.rs#L101), [`core/dr-gpu/tests/zoom_resolution.rs:131`](../core/dr-gpu/tests/zoom_resolution.rs#L131), [`core/dr-gpu/tests/zoom_resolution.rs:162`](../core/dr-gpu/tests/zoom_resolution.rs#L162), [`core/dr-gpu/tests/zoom_resolution.rs:1`](../core/dr-gpu/tests/zoom_resolution.rs#L1), [`core/dr-gpu/tests/zoom_resolution.rs:212`](../core/dr-gpu/tests/zoom_resolution.rs#L212) | | FR-DSP-6 | [`core/dr-pipeline/src/operation.rs:446`](../core/dr-pipeline/src/operation.rs#L446), [`core/dr-types/src/colour.rs:1`](../core/dr-types/src/colour.rs#L1), [`ui/dr-ui/src/develop.rs:2698`](../ui/dr-ui/src/develop.rs#L2698), [`ui/dr-ui/src/develop.rs:5473`](../ui/dr-ui/src/develop.rs#L5473), [`ui/dr-ui/src/lib.rs:1414`](../ui/dr-ui/src/lib.rs#L1414), [`ui/dr-ui/src/lib.rs:2626`](../ui/dr-ui/src/lib.rs#L2626) | | FR-DSP-7 | [`core/dr-gpu/src/histogram.rs:147`](../core/dr-gpu/src/histogram.rs#L147), [`core/dr-gpu/src/histogram.rs:1`](../core/dr-gpu/src/histogram.rs#L1), [`core/dr-gpu/src/histogram.rs:281`](../core/dr-gpu/src/histogram.rs#L281), [`core/dr-gpu/src/histogram.rs:50`](../core/dr-gpu/src/histogram.rs#L50), [`core/dr-gpu/src/shaders/histogram.wgsl:1`](../core/dr-gpu/src/shaders/histogram.wgsl#L1), [`ui/dr-ui/src/develop.rs:2747`](../ui/dr-ui/src/develop.rs#L2747), [`ui/dr-ui/src/develop.rs:5283`](../ui/dr-ui/src/develop.rs#L5283), [`ui/dr-ui/src/develop.rs:5315`](../ui/dr-ui/src/develop.rs#L5315), [`ui/dr-ui/src/develop.rs:621`](../ui/dr-ui/src/develop.rs#L621), [`ui/dr-ui/src/histogram.rs:1`](../ui/dr-ui/src/histogram.rs#L1), [`ui/dr-ui/src/lib.rs:1514`](../ui/dr-ui/src/lib.rs#L1514), [`ui/dr-ui/src/lib.rs:297`](../ui/dr-ui/src/lib.rs#L297), [`ui/dr-ui/ui/app.slint:299`](../ui/dr-ui/ui/app.slint#L299), [`ui/dr-ui/ui/histogram.slint:122`](../ui/dr-ui/ui/histogram.slint#L122), [`ui/dr-ui/ui/histogram.slint:1`](../ui/dr-ui/ui/histogram.slint#L1) | | FR-DSP-8 | [`platform/dr-plat/src/display.rs:1`](../platform/dr-plat/src/display.rs#L1), [`platform/dr-plat/src/display/icc.rs:1`](../platform/dr-plat/src/display/icc.rs#L1), [`platform/dr-plat/src/display/wayland.rs:1`](../platform/dr-plat/src/display/wayland.rs#L1), [`platform/dr-plat/src/display/x11.rs:1`](../platform/dr-plat/src/display/x11.rs#L1), [`ui/dr-ui/src/develop.rs:2644`](../ui/dr-ui/src/develop.rs#L2644), [`ui/dr-ui/src/develop.rs:2698`](../ui/dr-ui/src/develop.rs#L2698), [`ui/dr-ui/src/develop.rs:5473`](../ui/dr-ui/src/develop.rs#L5473), [`ui/dr-ui/src/develop.rs:5519`](../ui/dr-ui/src/develop.rs#L5519), [`ui/dr-ui/src/develop.rs:5538`](../ui/dr-ui/src/develop.rs#L5538), [`ui/dr-ui/src/develop.rs:689`](../ui/dr-ui/src/develop.rs#L689), [`ui/dr-ui/src/display_ui.rs:192`](../ui/dr-ui/src/display_ui.rs#L192), [`ui/dr-ui/src/display_ui.rs:1`](../ui/dr-ui/src/display_ui.rs#L1), [`ui/dr-ui/src/display_ui.rs:325`](../ui/dr-ui/src/display_ui.rs#L325), [`ui/dr-ui/src/display_ui.rs:346`](../ui/dr-ui/src/display_ui.rs#L346), [`ui/dr-ui/src/display_ui.rs:379`](../ui/dr-ui/src/display_ui.rs#L379), [`ui/dr-ui/src/lib.rs:1388`](../ui/dr-ui/src/lib.rs#L1388), [`ui/dr-ui/src/lib.rs:1414`](../ui/dr-ui/src/lib.rs#L1414), [`ui/dr-ui/src/lib.rs:2603`](../ui/dr-ui/src/lib.rs#L2603), [`ui/dr-ui/src/lib.rs:2626`](../ui/dr-ui/src/lib.rs#L2626), [`ui/dr-ui/ui/app.slint:1713`](../ui/dr-ui/ui/app.slint#L1713), [`ui/dr-ui/ui/app.slint:278`](../ui/dr-ui/ui/app.slint#L278), [`ui/dr-ui/ui/settings.slint:120`](../ui/dr-ui/ui/settings.slint#L120), [`ui/dr-ui/ui/settings.slint:730`](../ui/dr-ui/ui/settings.slint#L730) | @@ -138,7 +140,7 @@ _None._ ## Not yet tagged -75 of 177 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built. +73 of 177 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built.
Show untagged requirements @@ -150,9 +152,7 @@ _None._ - FR-DEV-1 - FR-DEV-3g - FR-DSP-2 -- FR-DSP-3 - FR-DSP-4 -- FR-DSP-5 - FR-NC-11 - FR-PLAT-AND-2 - FR-PLAT-AND-4