From 69fb510c50169367ab4b256ed36c5c015e654f1f Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 22 Aug 2026 19:16:43 +0200 Subject: [PATCH] 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 | 146 ++++++++++++++++++++------- 1 file changed, 112 insertions(+), 34 deletions(-) diff --git a/core/dr-gpu/tests/noise_reduction.rs b/core/dr-gpu/tests/noise_reduction.rs index 92ba008..8ebf2f3 100644 --- a/core/dr-gpu/tests/noise_reduction.rs +++ b/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 - // — an edge that softens here is a halo in every high-contrast picture. + // Black to white is thirteen times the threshold — 1.0 against a sigma of + // 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 -/// `half_period` pixels with the lightness very nearly fixed. +/// A fine chroma pattern: red and blue swung in opposite directions by the +/// 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 -/// that can tell the two halves of this operation apart. +/// alternating over a few pixels — and it is the one pattern that can tell the +/// 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 -/// linear channel of the middle row against the pattern's own sign. +/// How much of a known alternating pattern survived, as the correlation of a +/// 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 { - usize::MAX => luminance(linear(pixels, width, x, row)), - c => linear(pixels, width, x, row)[c], - }; + let value = sample(linear(pixels, width, x, row)); 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. -const AS_LUMINANCE: usize = usize::MAX; +/// The red component of the colour difference — red with its lightness taken +/// 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 - // 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 - // collapse, which is what a leak between the two components would be. + // luminance modulation under it stays where it was. + // + // On paper, at amount 100 over a half-period of 4: the kernel is 12 render + // 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_after = modulation(&chroma, SIZE, 4, 0); + let colour_before = modulation(&plain, SIZE, 4, chroma_r); + 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_after = modulation(&chroma, SIZE, 4, AS_LUMINANCE); + let light_before = modulation(&plain, SIZE, 4, 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 - // resampling of its own to confuse the measurement. + // exactly. The fused pass loads the nearest source pixel when the framing + // 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}" );