Let clarity's base be computed where it is still fully determined
Clarity's Gaussian sigma is 1.2% of the frame's shorter edge, so its radius is a property of the viewport: 52 render pixels at 4K, two separable passes of 105 taps each over 8.3 M pixels. That measured 33.9 ms — seven times the entire fused point chain, for one slider — and is docs/technical-debt.md TD-4. A detail pass may now declare `output_scale`, and clarity's base is computed on a grid a quarter the size on each axis. The pass that combines needs the blur *and* the full-resolution colour, and a colour that has been through a quarter-scale target is no longer full resolution. So a scaled pass cannot simply join the ping-pong: there are two chains now. The full-resolution one carries the colour and no scaled pass touches it; the reduced one carries the base and reaches the combining pass through a second binding as `reduced_at()`. The reduce is a dispatch of its own rather than something the first blur half does on the way past, and that is the whole difference between this and the strided kernel the module documentation rules out. A stride samples an image that is not band-limited and aliases high-frequency content down into the base, which is then subtracted, and arrives in the output as mottling across smooth gradients. This band-limits first and samples after. What is discarded is content the base could not represent at any resolution, because a Gaussian at sigma = 26 px holds nothing above one cycle per 26 px and the quarter-scale grid carries one per 8 — so the reduced base is not an approximation of the full-resolution one, it is the same function sampled where it is still determined. Which is also why the scale belongs to the band rather than to the stage. Texture's sigma is a decade finer, so the reduce pass's own box would be wider than the Gaussian it was prefiltering; texture never reduces. And clarity steps 4 -> 2 -> 1 as sigma falls, because a quarter of a small sigma is not a Gaussian either — the case that gives up is the one that was already cheap. `radius` stays in each pass's own pixels and `ComposedDetail::radius` multiplies it back up, so 13 reduced pixels at scale 4 still report the 52 render pixels a tile would have to be grown by. The halo a scheduler sees does not move. The halo tests pass unchanged, which was TD-4's stated bar; they render at 1024 px and so exercise the reduced path rather than stepping around it. Added `crossing_the_reduction_threshold_does_not_change_the_picture`, because nothing yet compared the reduced form against a *less* reduced one — every other test measures one form against itself. It renders the same edit either side of the 4 -> 2 step-down and holds the peak excursion to 0.03 stops and the reach to 2% of the frame. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+184
-11
@@ -49,6 +49,32 @@
|
||||
//! resolve pass to pay for. That leaves the allocation at `1 + min(N-1, 2)`
|
||||
//! textures: one for a single-pass operation, two for a separable blur, three
|
||||
//! however long the chain gets after that.
|
||||
//!
|
||||
//! # The reduced chain, and why a second one was needed
|
||||
//!
|
||||
//! A pass may declare [`dr_pipeline::detail::DetailPass::output_scale`] and
|
||||
//! write a target a fraction of the render size — clarity's base does, which
|
||||
//! is what TD-4 bought back. Such a pass cannot be part of the ping-pong
|
||||
//! above, and the reason is the shape of an unsharp mask rather than anything
|
||||
//! about textures: the pass that *combines* needs the blur **and** the
|
||||
//! full-resolution colour, and a colour that has been through a quarter-scale
|
||||
//! target is no longer full resolution. If the scaled passes wrote into the
|
||||
//! main chain they would destroy the very thing the last pass is going to
|
||||
//! subtract from.
|
||||
//!
|
||||
//! So there are two chains. The full-resolution one carries the colour and is
|
||||
//! untouched by a scaled pass; the reduced one carries the base. A scaled pass
|
||||
//! reads the reduced chain if anything has been written to it and the
|
||||
//! full-resolution chain otherwise — which is exactly "read what the pass
|
||||
//! before you wrote", the same rule as before. A full-resolution pass always
|
||||
//! reads the full-resolution chain, and sees the reduced one through binding 4
|
||||
//! as `reduced_at()`.
|
||||
//!
|
||||
//! One reduced buffer, not one per operation. Two operations both wanting a
|
||||
//! reduced base in the same frame would need more, and nothing does: clarity
|
||||
//! is the only caller and texture's band is a decade finer, so it must stay at
|
||||
//! full resolution. A `debug_assert` in [`DetailRunner::encode`] holds that
|
||||
//! claim rather than leaving it as a comment.
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -149,8 +175,14 @@ pub(crate) struct DetailRunner {
|
||||
/// Compiled pipelines by pass structure hash.
|
||||
cache: HashMap<u64, wgpu::ComputePipeline>,
|
||||
pool: Intermediates,
|
||||
/// The reduced chain — see the module documentation. Its own pool rather
|
||||
/// than more slots in `pool`, because its textures are a different size
|
||||
/// and [`Intermediates::ensure`] drops the lot when the size changes.
|
||||
reduced: Intermediates,
|
||||
/// See [`placeholder_instances`].
|
||||
no_instances: wgpu::Buffer,
|
||||
/// See [`placeholder_reduced`].
|
||||
no_reduced: wgpu::TextureView,
|
||||
}
|
||||
|
||||
struct Layout {
|
||||
@@ -174,6 +206,32 @@ fn placeholder_instances(ctx: &GpuContext) -> wgpu::Buffer {
|
||||
})
|
||||
}
|
||||
|
||||
/// What binding 4 holds for a pass that never calls `reduced_at`.
|
||||
///
|
||||
/// The same trick as [`placeholder_instances`], for the same reason: one bind
|
||||
/// group layout has to serve a pass that reads the reduced chain and a pass
|
||||
/// that has never heard of it, and a binding cannot be left unbound. 1x1 and
|
||||
/// allocated once, so the cost of the arrangement is four bytes for the life
|
||||
/// of the runner.
|
||||
fn placeholder_reduced(ctx: &GpuContext) -> wgpu::TextureView {
|
||||
ctx.device
|
||||
.create_texture(&wgpu::TextureDescriptor {
|
||||
label: Some("detail-reduced-placeholder"),
|
||||
size: wgpu::Extent3d {
|
||||
width: 1,
|
||||
height: 1,
|
||||
depth_or_array_layers: 1,
|
||||
},
|
||||
mip_level_count: 1,
|
||||
sample_count: 1,
|
||||
dimension: wgpu::TextureDimension::D2,
|
||||
format: INTERMEDIATE_FORMAT,
|
||||
usage: wgpu::TextureUsages::TEXTURE_BINDING,
|
||||
view_formats: &[],
|
||||
})
|
||||
.create_view(&Default::default())
|
||||
}
|
||||
|
||||
impl DetailRunner {
|
||||
pub(crate) fn new(ctx: &GpuContext) -> Self {
|
||||
Self {
|
||||
@@ -182,7 +240,9 @@ impl DetailRunner {
|
||||
to_output: Layout::new(ctx, crate::AdjustPass::FORMAT, "detail-output"),
|
||||
cache: HashMap::new(),
|
||||
pool: Intermediates::new(),
|
||||
reduced: Intermediates::new(),
|
||||
no_instances: placeholder_instances(ctx),
|
||||
no_reduced: placeholder_reduced(ctx),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -221,18 +281,93 @@ impl DetailRunner {
|
||||
self.compile(pass)?;
|
||||
}
|
||||
|
||||
for (index, pass) in chain.passes.iter().enumerate() {
|
||||
// Read what the previous pass wrote; write the next slot, or the
|
||||
// display texture if this is the last one. `index % 2` alternates
|
||||
// between slots 1 and 2, so a pass never reads the texture it is
|
||||
// writing — which on a compute pass is not an error the driver
|
||||
// reports, merely a picture that depends on scheduling.
|
||||
let source_slot = if index == 0 { 0 } else { 2 - (index % 2) };
|
||||
let source = &self.pool.slots[source_slot].view;
|
||||
// The reduced chain's size, allocated once for the whole chain.
|
||||
//
|
||||
// One scale per chain, so the first scaled pass names the only scale
|
||||
// there is — see the module documentation for why one buffer is
|
||||
// enough, and the assertion for what would have to change.
|
||||
if let Some(scale) = chain
|
||||
.passes
|
||||
.iter()
|
||||
.map(|p| p.output_scale)
|
||||
.find(|&scale| scale > 1)
|
||||
{
|
||||
debug_assert!(
|
||||
chain
|
||||
.passes
|
||||
.iter()
|
||||
.all(|p| p.output_scale == 1 || p.output_scale == scale),
|
||||
"two reduced scales in one chain, and the runner holds one \
|
||||
reduced buffer"
|
||||
);
|
||||
// Two, for the ping-pong the reduce and the two blur halves need.
|
||||
// A separable blur cannot read the texture it is writing.
|
||||
self.reduced.ensure(
|
||||
&self.ctx,
|
||||
2,
|
||||
width.div_ceil(scale).max(1),
|
||||
height.div_ceil(scale).max(1),
|
||||
);
|
||||
}
|
||||
|
||||
// Where each chain last wrote. `full` starts at slot 0 — what the
|
||||
// fused colour pass left there — and `carried` starts empty, which is
|
||||
// what makes the first scaled pass read the colour rather than an
|
||||
// uninitialised base.
|
||||
let mut full = 0usize;
|
||||
let mut full_writes = 0usize;
|
||||
let mut carried: Option<usize> = None;
|
||||
let mut reduced_writes = 0usize;
|
||||
|
||||
for pass in chain.passes.iter() {
|
||||
let scaled = pass.output_scale > 1;
|
||||
|
||||
// The last pass carries the output transform into the display
|
||||
// texture, which is the render size by definition. A scaled pass
|
||||
// there would bind a shader dispatching over a quarter-size grid
|
||||
// to a full-size target and write a quarter of the picture — a
|
||||
// wrong image rather than a validation failure, so it is caught
|
||||
// here and named.
|
||||
if scaled && pass.writes_output {
|
||||
return Err(GpuError::ShaderCompilation(format!(
|
||||
"detail pass {} declares output_scale {} and is last in \
|
||||
the chain; the output transform is written at the render \
|
||||
size",
|
||||
pass.label, pass.output_scale
|
||||
)));
|
||||
}
|
||||
|
||||
let (dispatch_w, dispatch_h) = if scaled {
|
||||
(
|
||||
width.div_ceil(pass.output_scale).max(1),
|
||||
height.div_ceil(pass.output_scale).max(1),
|
||||
)
|
||||
} else {
|
||||
(width, height)
|
||||
};
|
||||
|
||||
// Read what the previous pass in *this pass's own chain* wrote;
|
||||
// write the next slot of it, or the display texture if this is the
|
||||
// last pass. Alternating slots is what stops a pass reading the
|
||||
// texture it is writing — on a compute pass that is not an error
|
||||
// the driver reports, merely a picture that depends on scheduling.
|
||||
let source = match (scaled, carried) {
|
||||
(true, Some(slot)) => &self.reduced.slots[slot].view,
|
||||
_ => &self.pool.slots[full].view,
|
||||
};
|
||||
let destination = if pass.writes_output {
|
||||
output
|
||||
} else if scaled {
|
||||
&self.reduced.slots[reduced_writes % 2].view
|
||||
} else {
|
||||
&self.pool.slots[1 + (index % 2)].view
|
||||
&self.pool.slots[1 + (full_writes % 2)].view
|
||||
};
|
||||
// Binding 4. Present for every pass, because one bind group layout
|
||||
// serves both kinds; a pass that never calls `reduced_at` gets the
|
||||
// 1x1 placeholder and never reads it.
|
||||
let reduced_source = match carried {
|
||||
Some(slot) => &self.reduced.slots[slot].view,
|
||||
None => &self.no_reduced,
|
||||
};
|
||||
let layout = if pass.writes_output {
|
||||
&self.to_output
|
||||
@@ -290,6 +425,10 @@ impl DetailRunner {
|
||||
binding: 3,
|
||||
resource: instances.as_entire_binding(),
|
||||
},
|
||||
wgpu::BindGroupEntry {
|
||||
binding: 4,
|
||||
resource: wgpu::BindingResource::TextureView(reduced_source),
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
@@ -304,7 +443,23 @@ impl DetailRunner {
|
||||
});
|
||||
compute.set_pipeline(pipeline);
|
||||
compute.set_bind_group(0, &bind_group, &[]);
|
||||
compute.dispatch_workgroups(width.div_ceil(8), height.div_ceil(8), 1);
|
||||
compute.dispatch_workgroups(dispatch_w.div_ceil(8), dispatch_h.div_ceil(8), 1);
|
||||
drop(compute);
|
||||
|
||||
if pass.writes_output {
|
||||
// Nothing downstream to hand anything to.
|
||||
} else if scaled {
|
||||
carried = Some(reduced_writes % 2);
|
||||
reduced_writes += 1;
|
||||
} else {
|
||||
full = 1 + (full_writes % 2);
|
||||
full_writes += 1;
|
||||
// A full-resolution pass consumes the reduced chain. It is the
|
||||
// combine — the base has been subtracted and now lives in the
|
||||
// colour — so a later operation must not be handed a base
|
||||
// belonging to this one.
|
||||
carried = None;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(chain.passes.len())
|
||||
@@ -375,7 +530,10 @@ impl DetailRunner {
|
||||
/// created. For tests — see [`crate::MaskPass::allocations`] for the
|
||||
/// regression this shape of counter exists to catch.
|
||||
pub(crate) fn allocations(&self) -> usize {
|
||||
self.pool.allocations
|
||||
// Both pools. A reduced buffer reallocated every frame is exactly the
|
||||
// regression this counter exists to catch, and counting only the
|
||||
// full-resolution one would hide it.
|
||||
self.pool.allocations + self.reduced.allocations
|
||||
}
|
||||
}
|
||||
|
||||
@@ -439,6 +597,21 @@ impl Layout {
|
||||
// that never reads the buffer costs nothing for it being
|
||||
// bound.
|
||||
storage_entry(3),
|
||||
// The reduced chain, for a pass that calls `reduced_at`.
|
||||
// Bound on every layout for the same reason binding 3 is:
|
||||
// a pass that never reads it costs nothing for it being
|
||||
// there, and two more layouts would cost a great deal more
|
||||
// than that.
|
||||
wgpu::BindGroupLayoutEntry {
|
||||
binding: 4,
|
||||
visibility: wgpu::ShaderStages::COMPUTE,
|
||||
ty: wgpu::BindingType::Texture {
|
||||
sample_type: wgpu::TextureSampleType::Float { filterable: true },
|
||||
view_dimension: wgpu::TextureViewDimension::D2,
|
||||
multisampled: false,
|
||||
},
|
||||
count: None,
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
|
||||
@@ -67,6 +67,7 @@ fn main(@builtin(global_invocation_id) gid: vec3<u32>) {
|
||||
.to_string();
|
||||
|
||||
ComposedDetailPass {
|
||||
output_scale: 1,
|
||||
label: "test/instances".to_string(),
|
||||
source,
|
||||
uniforms: vec![SIZE as f32, SIZE as f32, 1.0, 0.0],
|
||||
|
||||
@@ -27,6 +27,7 @@
|
||||
|
||||
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext};
|
||||
use dr_pipeline::descriptor::{OpId, ParamId};
|
||||
use dr_pipeline::detail::RenderScale;
|
||||
use dr_pipeline::ops::local_contrast::Clarity;
|
||||
use dr_pipeline::{Affects, EditGraph, OutputMode};
|
||||
use dr_types::ColourSpace;
|
||||
@@ -323,6 +324,87 @@ fn a_proxy_and_an_export_agree_about_the_effect() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn crossing_the_reduction_threshold_does_not_change_the_picture() {
|
||||
// TRACES: FR-DSP-3 — `docs/technical-debt.md` TD-4, held in pixels.
|
||||
//
|
||||
// Clarity's base is computed on a reduced grid, and how reduced depends on
|
||||
// the viewport: `LocalContrast::reduction` steps 4 -> 2 -> 1 as sigma
|
||||
// falls, because a quarter of a small sigma is not a Gaussian any more.
|
||||
// The whole claim of the optimisation is that this is invisible — that a
|
||||
// base sampled at a quarter is not an approximation of the full-resolution
|
||||
// one but the same band-limited function, sampled where it is still fully
|
||||
// determined.
|
||||
//
|
||||
// Every other test in this file measures one form against itself. This is
|
||||
// the only one that measures the forms against *each other*, and it is the
|
||||
// one that would fail if the reduction were quietly softening the control,
|
||||
// shifting it half a reduced pixel, or blocking the base into 4x4 squares.
|
||||
//
|
||||
// The two sizes are chosen to sit either side of a step-down: sigma is
|
||||
// 1.2% of the shorter edge, so 512 gives 6.1 px and reduces by four, while
|
||||
// 288 gives 3.5 px — under `MIN_REDUCED_SIGMA` once quartered — and
|
||||
// reduces by two. Asserted rather than assumed, because the whole test is
|
||||
// vacuous if both sides land on the same reduction.
|
||||
let Some(ctx) = ctx() else { return };
|
||||
assert_eq!(
|
||||
Clarity::with_amount(100.0).reduction(RenderScale::full((512, 512))),
|
||||
4
|
||||
);
|
||||
assert_eq!(
|
||||
Clarity::with_amount(100.0).reduction(RenderScale::full((288, 288))),
|
||||
2
|
||||
);
|
||||
|
||||
let source = step_edge(&ctx, SIZE, [1.0, 1.0, 1.0]);
|
||||
|
||||
// Peak excursion in stops and the fraction of the frame it covers — the
|
||||
// same two numbers `a_proxy_and_an_export_agree_about_the_effect` uses,
|
||||
// and for the same reason: both are scale-free, so they are comparable
|
||||
// between two renders of different sizes.
|
||||
let measure = |out: u32| -> (f32, f32) {
|
||||
let mut plain_pass = AdjustPass::new(&ctx);
|
||||
let plain = row(
|
||||
&render(&mut plain_pass, &EditGraph::default_chain(), &source, out),
|
||||
out,
|
||||
out / 2,
|
||||
);
|
||||
let mut pass = AdjustPass::new(&ctx);
|
||||
let edited = row(
|
||||
&render(&mut pass, &graph_with(CLARITY, 100.0), &source, out),
|
||||
out,
|
||||
out / 2,
|
||||
);
|
||||
let moved: Vec<f32> = (0..out as usize)
|
||||
.map(|x| stops(edited[x], plain[x]).abs())
|
||||
.collect();
|
||||
let peak = moved.iter().cloned().fold(0.0f32, f32::max);
|
||||
let touched = moved.iter().filter(|m| **m > peak * 0.1).count();
|
||||
(peak, touched as f32 / out as f32)
|
||||
};
|
||||
|
||||
let (quartered_peak, quartered_reach) = measure(512);
|
||||
let (halved_peak, halved_reach) = measure(288);
|
||||
|
||||
// The same tolerances the proxy/export test uses. They are not loose: the
|
||||
// bound this operation guarantees is 0.35 stops, so 0.03 is under a tenth
|
||||
// of the full excursion.
|
||||
assert!(
|
||||
(quartered_peak - halved_peak).abs() < 0.03,
|
||||
"a quarter-scale base gives {quartered_peak:.3} stops and a half-scale \
|
||||
one {halved_peak:.3}; the reduction is supposed to be invisible"
|
||||
);
|
||||
assert!(
|
||||
(quartered_reach - halved_reach).abs() < 0.02,
|
||||
"the effect covers {quartered_reach:.3} of the frame reduced by four \
|
||||
and {halved_reach:.3} reduced by two; the base has changed width"
|
||||
);
|
||||
assert!(
|
||||
quartered_peak > 0.1,
|
||||
"{quartered_peak:.3} stops — two flat images would also agree"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn texture_acts_at_a_finer_scale_than_clarity() {
|
||||
// The whole reason there are two nodes. If the two controls ever reach the
|
||||
@@ -485,10 +567,15 @@ fn dragging_the_slider_re_runs_the_detail_stage_and_nothing_else() {
|
||||
let mut graph = graph_with(CLARITY, 40.0);
|
||||
render(&mut pass, &graph, &source, 256);
|
||||
assert_eq!(pass.colour_dispatches(), 1);
|
||||
// Four at this size: reduce, the two blur halves on the reduced grid, and
|
||||
// the combine. σ is 1.2% of 256 px, so `LocalContrast::reduction` lands on
|
||||
// a half here rather than the quarter a desktop viewport gets — the number
|
||||
// is the viewport's, and what this test is about is that it does not
|
||||
// change while the slider moves.
|
||||
assert_eq!(
|
||||
pass.detail_dispatches(),
|
||||
2,
|
||||
"a separable mask is two passes"
|
||||
4,
|
||||
"a reduced separable mask is four passes"
|
||||
);
|
||||
let pipelines = pass.cached_detail_pipelines();
|
||||
|
||||
@@ -501,17 +588,21 @@ fn dragging_the_slider_re_runs_the_detail_stage_and_nothing_else() {
|
||||
1,
|
||||
"the fused colour pass re-ran for a change it does not depend on"
|
||||
);
|
||||
assert_eq!(pass.detail_dispatches(), 8);
|
||||
// Four renders of a four-pass chain. The counter accumulates, so this is
|
||||
// the claim that every one of those renders ran the detail stage and only
|
||||
// the detail stage.
|
||||
assert_eq!(pass.detail_dispatches(), 16);
|
||||
assert_eq!(
|
||||
pass.cached_detail_pipelines(),
|
||||
pipelines,
|
||||
"an amount is a uniform, not a shader"
|
||||
);
|
||||
|
||||
// Turning on the other control adds its own pair, and only its own pair.
|
||||
// Turning on the other control adds its own pair, and only its own pair:
|
||||
// 16, plus clarity's four again, plus texture's two.
|
||||
graph.set_param(TEXTURE, AMOUNT, 40.0);
|
||||
render(&mut pass, &graph, &source, 256);
|
||||
assert_eq!(pass.detail_dispatches(), 12);
|
||||
assert_eq!(pass.detail_dispatches(), 22);
|
||||
assert_eq!(pass.colour_dispatches(), 1);
|
||||
}
|
||||
|
||||
@@ -536,7 +627,11 @@ fn the_two_controls_stack_without_overwriting_each_other() {
|
||||
graph.set_param(TEXTURE, AMOUNT, 80.0);
|
||||
let mut pass = AdjustPass::new(&ctx);
|
||||
let both = row(&render(&mut pass, &graph, &source, SIZE), SIZE, SIZE / 2);
|
||||
assert_eq!(pass.detail_dispatches(), 4);
|
||||
// Clarity's four plus texture's two. Texture is never reduced — its band
|
||||
// is a decade finer than clarity's, so a coarser grid could not hold its
|
||||
// base — and the two operations keeping different pass counts here is that
|
||||
// asymmetry showing through.
|
||||
assert_eq!(pass.detail_dispatches(), 6);
|
||||
|
||||
let edge = (SIZE / 2) as usize;
|
||||
// Both sides of the edge move the way local contrast moves them...
|
||||
|
||||
Reference in New Issue
Block a user