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/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/traceability.md b/docs/traceability.md index ee26a94..8833e55 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 | 255 | -| TRACES tags found | 732 | +| Source files scanned | 257 | +| TRACES tags found | 737 | | Requirements defined | 177 | -| Requirements covered | 98 | -| **Coverage** | **55.4%** (98/177) | +| Requirements covered | 100 | +| **Coverage** | **56.5%** (100/177) | ### By type | Type | Covered | Defined | |---|---|---| -| FR | 78 | 122 | +| FR | 80 | 122 | | NFR | 18 | 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:3410`](../ui/dr-ui/src/develop.rs#L3410), [`ui/dr-ui/src/develop.rs:3442`](../ui/dr-ui/src/develop.rs#L3442), [`ui/dr-ui/src/lib.rs:1408`](../ui/dr-ui/src/lib.rs#L1408), [`ui/dr-ui/src/lib.rs:2294`](../ui/dr-ui/src/lib.rs#L2294), [`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:2127`](../ui/dr-ui/src/develop.rs#L2127), [`ui/dr-ui/src/develop.rs:2165`](../ui/dr-ui/src/develop.rs#L2165), [`ui/dr-ui/src/develop.rs:2230`](../ui/dr-ui/src/develop.rs#L2230), [`ui/dr-ui/src/develop.rs:2313`](../ui/dr-ui/src/develop.rs#L2313), [`ui/dr-ui/src/develop.rs:2327`](../ui/dr-ui/src/develop.rs#L2327), [`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:1429`](../ui/dr-ui/src/lib.rs#L1429), [`ui/dr-ui/src/lib.rs:2391`](../ui/dr-ui/src/lib.rs#L2391), [`ui/dr-ui/src/lib.rs:306`](../ui/dr-ui/src/lib.rs#L306), [`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:1835`](../ui/dr-ui/ui/app.slint#L1835), [`ui/dr-ui/ui/app.slint:2352`](../ui/dr-ui/ui/app.slint#L2352), [`ui/dr-ui/ui/app.slint:2618`](../ui/dr-ui/ui/app.slint#L2618), [`ui/dr-ui/ui/app.slint:332`](../ui/dr-ui/ui/app.slint#L332), [`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:2627`](../ui/dr-ui/src/develop.rs#L2627), [`ui/dr-ui/src/develop.rs:3640`](../ui/dr-ui/src/develop.rs#L3640), [`ui/dr-ui/src/develop.rs:4123`](../ui/dr-ui/src/develop.rs#L4123), [`ui/dr-ui/src/develop.rs:4157`](../ui/dr-ui/src/develop.rs#L4157), [`ui/dr-ui/src/lib.rs:70`](../ui/dr-ui/src/lib.rs#L70), [`ui/dr-ui/src/lib.rs:736`](../ui/dr-ui/src/lib.rs#L736), [`ui/dr-ui/src/lib.rs:795`](../ui/dr-ui/src/lib.rs#L795) | +| 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) | | 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:2693`](../ui/dr-ui/src/develop.rs#L2693), [`ui/dr-ui/src/develop.rs:5225`](../ui/dr-ui/src/develop.rs#L5225), [`ui/dr-ui/src/develop.rs:5257`](../ui/dr-ui/src/develop.rs#L5257), [`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:1485`](../ui/dr-ui/src/lib.rs#L1485), [`ui/dr-ui/src/lib.rs:296`](../ui/dr-ui/src/lib.rs#L296), [`ui/dr-ui/ui/app.slint:292`](../ui/dr-ui/ui/app.slint#L292), [`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-EXP-1 | [`core/dr-export/src/encode.rs:1`](../core/dr-export/src/encode.rs#L1), [`core/dr-export/src/lib.rs:1`](../core/dr-export/src/lib.rs#L1), [`core/dr-types/src/settings.rs:1`](../core/dr-types/src/settings.rs#L1), [`ui/dr-ui/src/settings_ui.rs:1`](../ui/dr-ui/src/settings_ui.rs#L1) | @@ -134,7 +136,7 @@ _None._ ## Not yet tagged -79 of 177 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built. +77 of 177 requirements have no implementation tag. Expected while the codebase is young; each should gain one as it is built.
Show untagged requirements @@ -146,9 +148,7 @@ _None._ - FR-DEV-1 - FR-DEV-3g - FR-DSP-2 -- FR-DSP-3 - FR-DSP-4 -- FR-DSP-5 - FR-DSP-8 - FR-NC-11 - FR-PLAT-AND-2