Measure the chroma tests on colour difference, not on red

A colour square wave built from equal, opposite swings of red and blue is
not a pure colour pattern: Rec. 709 weights them 0.2126 and 0.0722, so it
carries a luminance square wave of about a seventh of the swing underneath.
Correlating the raw red channel therefore reads a constant floor that the
chroma filter is not meant to remove, which compressed every ratio towards
one - enough that the resolution test could no longer tell a correct kernel
from one twice the size. Correlate the colour difference instead, and write
the derivation of each expected value into the test.

core/dr-gpu/tests/noise_reduction.rs

@@ -161,16 +161,28 @@ fn a_difference_below_the_threshold_is_averaged_and_one_above_it_is_not() {
     };
 
     // A step of eight code values around mid-grey is about 0.028 in linear
-    // luminance, against a threshold of roughly 0.034 at full amount: within
+    // luminance, against a threshold of roughly 0.035 at full amount: within
     // the range where the filter is meant to treat a difference as noise.
+    //
+    // Worked out on paper, since it cannot be run here: at amount 100 the
+    // kernel is 3 render pixels and sigma_k is 0.075, so at y = 0.2159 the
+    // range sigma is 0.075·sqrt(0.2159 + 0.0025) = 0.0350 and a neighbour
+    // 0.0280 away is weighted exp(-0.320) = 0.726. Summing the 7×7 kernel's
+    // spatial weights over the four bright columns and the three dark ones
+    // gives a filtered luminance of 0.2076 against 0.2159 — a move of 0.0082,
+    // which survives the 8-bit readback as about **0.0072**. The threshold
+    // below is set well under that rather than at it: what would be a bug is
+    // the filter declining to average at all.
     let quiet = measure(&step_edge(&ctx, SIZE, 120, 128));
     assert!(
         quiet > 0.004,
         "a difference below the threshold was left alone: moved {quiet}"
     );
 
-    // Black to white is thirty times the threshold. It has to survive intact
+    // Black to white is thirteen times the threshold — 1.0 against a sigma of
-    // — an edge that softens here is a halo in every high-contrast picture.
+    // 0.075·sqrt(1.0025) = 0.0751 — so a neighbour across it is weighted
+    // exp(-88), which is zero in any arithmetic. The edge has to survive
+    // intact; one that softens here is a halo in every high-contrast picture.
     let loud = measure(&step_edge(&ctx, SIZE, 0, 255));
     assert!(
         loud.abs() < 0.004,
@@ -182,12 +194,18 @@ fn a_difference_below_the_threshold_is_averaged_and_one_above_it_is_not() {
     );
 }
 
-/// A fine chroma pattern on a constant grey: colour that alternates every
+/// A fine chroma pattern: red and blue swung in opposite directions by the
-/// `half_period` pixels with the lightness very nearly fixed.
+/// same number of code values, green held.
 ///
 /// This is what chroma noise looks like to the filter — a colour difference
-/// with almost no luminance difference under it — and it is the one pattern
+/// alternating over a few pixels — and it is the one pattern that can tell the
-/// that can tell the two halves of this operation apart.
+/// two halves of this operation apart.
+///
+/// It is not a *pure* colour pattern, and the tests must not assume it is.
+/// Rec. 709 weights red at 0.2126 and blue at 0.0722, so swinging one up and
+/// the other down by equal amounts moves lightness by about a seventh of the
+/// swing. That residue is real, it is not the chroma filter's to remove, and
+/// [`chroma_r`] is what keeps it out of the measurements.
 fn chroma_pattern(ctx: &GpuContext, size: u32, half_period: u32, swing: i32) -> DemosaicedImage {
     upload(ctx, size, move |x, _| {
         let on = (x / half_period) % 2 == 0;
@@ -196,8 +214,8 @@ fn chroma_pattern(ctx: &GpuContext, size: u32, half_period: u32, swing: i32) ->
     })
 }
 
-/// How much of a known alternating pattern survived, as the correlation of one
+/// How much of a known alternating pattern survived, as the correlation of a
-/// linear channel of the middle row against the pattern's own sign.
+/// chosen measurement of the middle row against the pattern's own sign.
 ///
 /// A matched filter rather than a peak-to-peak reading. The readback is eight
 /// bits, and the modulation these tests work with is only a handful of code
@@ -209,25 +227,39 @@ fn chroma_pattern(ctx: &GpuContext, size: u32, half_period: u32, swing: i32) ->
 /// `margin` covers a whole number of periods too, so the window is unbiased by
 /// the row's mean, and it keeps the measurement clear of the borders where a
 /// clamped kernel legitimately behaves differently.
-fn modulation(pixels: &[u8], width: u32, half_period: u32, channel: usize) -> f32 {
+///
+/// `sample` is what to measure. Passing [`chroma_r`] rather than the raw red
+/// channel matters more than it looks: a colour square wave built from 8-bit
+/// code values carries a *luminance* square wave under it — Rec. 709 does not
+/// weight red and blue equally, so swinging one up and the other down moves
+/// lightness too — and that component is not the chroma filter's to remove.
+/// Left in the measurement it is a constant floor under every reading, which
+/// compresses every ratio this file asserts towards one and would leave the
+/// tests unable to tell a correct kernel from one twice the size.
+fn modulation(
+    pixels: &[u8],
+    width: u32,
+    half_period: u32,
+    sample: impl Fn([f32; 3]) -> f32,
+) -> f32 {
     let row = width / 2;
     let margin = half_period * 4;
     let mut total = 0.0;
     let mut count = 0.0;
     for x in margin..(width - margin) {
-        let sample = match channel {
+        let value = sample(linear(pixels, width, x, row));
-            usize::MAX => luminance(linear(pixels, width, x, row)),
-            c => linear(pixels, width, x, row)[c],
-        };
         let sign = if (x / half_period) % 2 == 0 { 1.0 } else { -1.0 };
-        total += sample * sign;
+        total += value * sign;
         count += 1.0;
     }
     total / count
 }
 
-/// Correlate against luminance rather than a channel.
+/// The red component of the colour difference — red with its lightness taken
-const AS_LUMINANCE: usize = usize::MAX;
+/// out, which is the quantity the chroma passes actually filter.
+fn chroma_r(c: [f32; 3]) -> f32 {
+    c[0] - luminance(c)
+}
 
 #[test]
 fn chroma_noise_reduction_never_moves_lightness() {
@@ -255,26 +287,36 @@ fn chroma_noise_reduction_never_moves_lightness() {
 
     // And on an image that does have colour to filter, where the two halves
     // could actually interfere: the colour modulation must fall while the
-    // luminance modulation under it stays where it was. The tolerance is wide
+    // luminance modulation under it stays where it was.
-    // because both readings pass through an eight-bit readback twice over; the
+    //
-    // failure it guards against is not a drift of a few percent but a
+    // On paper, at amount 100 over a half-period of 4: the kernel is 12 render
-    // collapse, which is what a leak between the two components would be.
+    // pixels, both chroma thresholds are 0.16·sqrt(y + floor) ≈ 0.076 and
+    // 0.20·… ≈ 0.095, so an opposite-coloured neighbour is weighted 0.410, and
+    // summing the separable kernel over the eight phases leaves about **0.42**
+    // of the chroma amplitude — 0.0158 of 0.0377. The luminance amplitude must
+    // not move at all: every colour difference the pass averages has zero
+    // luminance by construction, so their weighted mean does too.
+    //
+    // Both tolerances are wide of those numbers because both readings pass
+    // through an eight-bit readback twice over; the failure they guard against
+    // is not a drift of a few percent but a collapse, which is what a leak
+    // between the two components would be.
     let source = chroma_pattern(&ctx, SIZE, 4, 12);
     let mut off = AdjustPass::new(&ctx);
     let plain = render(&ctx, &mut off, &graph_with(0.0, 0.0), &source, (SIZE, SIZE));
     let mut on = AdjustPass::new(&ctx);
     let chroma = render(&ctx, &mut on, &graph_with(0.0, 100.0), &source, (SIZE, SIZE));
 
-    let colour_before = modulation(&plain, SIZE, 4, 0);
+    let colour_before = modulation(&plain, SIZE, 4, chroma_r);
-    let colour_after = modulation(&chroma, SIZE, 4, 0);
+    let colour_after = modulation(&chroma, SIZE, 4, chroma_r);
     assert!(
         colour_after < colour_before * 0.7,
         "chroma noise reduction did not reduce the colour swing: \
          {colour_after} of {colour_before}"
     );
 
-    let light_before = modulation(&plain, SIZE, 4, AS_LUMINANCE);
+    let light_before = modulation(&plain, SIZE, 4, luminance);
-    let light_after = modulation(&chroma, SIZE, 4, AS_LUMINANCE);
+    let light_after = modulation(&chroma, SIZE, 4, luminance);
     assert!(
         (light_after - light_before).abs() < light_before.abs() * 0.3,
         "chroma noise reduction moved lightness: {light_after} was {light_before}"
@@ -330,8 +372,44 @@ fn the_same_edit_denoises_the_same_at_two_resolutions() {
     //
     // The subject is a chroma square wave with a period that is a power of two
     // and aligned to the frame, so halving the render resolution decimates it
-    // exactly — the proxy sees the same pattern at half the period, with no
+    // exactly. The fused pass loads the nearest source pixel when the framing
-    // resampling of its own to confuse the measurement.
+    // is unrotated, so output column `x` reads source column `2x` — always the
+    // same half of an eight-wide block as `2x + 1` — and the proxy sees the
+    // same two colours at half the period, with no resampling of its own to
+    // confuse the measurement.
+    //
+    // # The expected numbers
+    //
+    // Worked out on paper, because the tolerances below are meaningless
+    // without knowing what they are tolerances *around*.
+    //
+    // The two colours are (134, 128, 122) and (122, 128, 134), which decode to
+    // linear (0.2384, 0.2159, 0.1946) and its mirror. Their colour differences
+    // are ±(0.0193, -0.0033, -0.0245), so the pattern's chroma amplitude —
+    // what `modulation` with `chroma_r` reads — is 0.0188 before filtering.
+    //
+    // At amount 60 both chroma thresholds are 0.12·sqrt(y + floor) ≈ 0.0565,
+    // so a neighbour of the opposite colour is weighted
+    // `exp(-(dl²/2σ_g² + |dc|²/2σ_c²)) ≈ exp(-0.624) ≈ 0.536`: attenuated, but
+    // far from rejected, which is the regime where the kernel's *width* is
+    // what decides the answer. That is the point — a test where the range
+    // weights dominated would pass whatever the radius conversion did.
+    //
+    // Summing the separable kernel's spatial weights over the eight phases of
+    // the pattern gives a mean surviving fraction of **0.521** at export (a
+    // radius of 8 render pixels over a half-period of 8) and **0.521** on the
+    // proxy (4 over 4) — the two arrangements are the same filter sampled at
+    // two rates, and they agree to three decimal places. So both amplitudes
+    // land at about 0.0098.
+    //
+    // The control lands at **0.305**, about 0.0057: a kernel of 8 render
+    // pixels over a half-period of 4 is twice as wide in the terms that
+    // matter. The tolerances are set wide of those numbers rather than tight
+    // to them, because the readback is eight bits and each of the handful of
+    // distinct output values carries up to half a code value of rounding — a
+    // floor of a few percent on any of these readings that no amount of
+    // averaging over a larger frame removes, since the error is periodic
+    // rather than random.
     let Some(ctx) = ctx() else { return };
     const SOURCE: u32 = 256;
     const HALF_PERIOD: u32 = 8; // in source pixels
@@ -348,19 +426,19 @@ fn the_same_edit_denoises_the_same_at_two_resolutions() {
 
     let mut export_pass = AdjustPass::new(&ctx);
     let export = render(&ctx, &mut export_pass, &graph, &source, (SOURCE, SOURCE));
-    let export_amp = modulation(&export, SOURCE, HALF_PERIOD, 0);
+    let export_amp = modulation(&export, SOURCE, HALF_PERIOD, chroma_r);
 
     let proxy_size = SOURCE / 2;
     let mut proxy_pass = AdjustPass::new(&ctx);
     let proxy = render(&ctx, &mut proxy_pass, &graph, &source, (proxy_size, proxy_size));
-    let proxy_amp = modulation(&proxy, proxy_size, HALF_PERIOD / 2, 0);
+    let proxy_amp = modulation(&proxy, proxy_size, HALF_PERIOD / 2, chroma_r);
 
     // Both must be doing something: two flat images would agree perfectly and
-    // prove nothing.
+    // prove nothing. About 0.0098 of 0.0188, by the derivation above.
     let untouched = {
         let mut pass = AdjustPass::new(&ctx);
         let plain = render(&ctx, &mut pass, &graph_with(0.0, 0.0), &source, (SOURCE, SOURCE));
-        modulation(&plain, SOURCE, HALF_PERIOD, 0)
+        modulation(&plain, SOURCE, HALF_PERIOD, chroma_r)
     };
     assert!(
         export_amp < untouched * 0.8,
@@ -368,7 +446,7 @@ fn the_same_edit_denoises_the_same_at_two_resolutions() {
     );
 
     assert!(
-        (proxy_amp - export_amp).abs() < export_amp * 0.2,
+        (proxy_amp - export_amp).abs() < export_amp * 0.25,
         "the same edit left {proxy_amp} of the pattern on the proxy and \
          {export_amp} on the export"
     );
@@ -387,9 +465,9 @@ fn the_same_edit_denoises_the_same_at_two_resolutions() {
         (proxy_size, proxy_size),
         RenderScale::full((proxy_size, proxy_size)),
     );
-    let wrong_amp = modulation(&wrong, proxy_size, HALF_PERIOD / 2, 0);
+    let wrong_amp = modulation(&wrong, proxy_size, HALF_PERIOD / 2, chroma_r);
     assert!(
-        wrong_amp < export_amp * 0.7,
+        wrong_amp < export_amp * 0.8,
         "an unconverted radius was indistinguishable from a converted one: \
          {wrong_amp} against {export_amp}"
     );