From 75292874a3dd58d9caeb4035c6bb4a78d8fd7a4a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 18:54:24 -0700 Subject: [PATCH] LR-EDIT-DETAIL-DENOISE: GPU denoise gives the right result on DX12 with NVIDIA Pascal On DX12, FXC merged the conv kernel's four output stores (modes 0 and 1) into one 16-byte store_raw. The DXBC is valid and aligned, but NVIDIA's driver ran it wrong on a GTX 1070: every second pixel's channels came out shifted, so AI Denoise on the GPU was silently wrong on Windows. DXC and Vulkan were right on the same card. The stores now step by `1 + p.zero` (the always-0 uniform, renamed from `pad`), which FXC cannot fold, so it keeps them apart. A two-layer channel-copy network checks the stores exactly; it fails on that card without the fix. Co-Authored-By: Claude Opus 5.5 --- crates/gpu/src/nn.rs | 25 ++++++++++++++++++++++--- crates/gpu/src/wgsl/nn_conv.wgsl | 19 ++++++++++++------- docs/gpu-pipeline.md | 6 ++++++ docs/whats-new.md | 7 +++++++ 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/crates/gpu/src/nn.rs b/crates/gpu/src/nn.rs index 5872bfaa9..b5a0f39f6 100644 --- a/crates/gpu/src/nn.rs +++ b/crates/gpu/src/nn.rs @@ -161,7 +161,8 @@ struct ConvParams { cout: u32, leaky: u32, alpha: f32, - pad: u32, + /// Always 0: the kernel steps its output stores by `1 + zero`, so FXC cannot merge them (`nn_conv.wgsl`). + zero: u32, } #[repr(C)] @@ -349,7 +350,7 @@ fn plan(net: &Net, tile: usize, max_binding: u64) -> Result<(Layout, Vec) cout: c.cout as u32, leaky: u32::from(c.leaky.is_some()), alpha: c.leaky.unwrap_or(0.0), - pad: 0, + zero: 0, }; let src = get(&place, c.src)?; let src2 = c.src2.map(|t| get(&place, t)).transpose()?; @@ -416,7 +417,7 @@ fn plan(net: &Net, tile: usize, max_binding: u64) -> Result<(Layout, Vec) cout: c.cout as u32, leaky: u32::from(c.leaky.is_some()), alpha: c.leaky.unwrap_or(0.0), - pad: 0, + zero: 0, }; let src = get(&place, c.src)?; let slot = alloc((4 * m * c.cout * 4) as u64, &mut slots, &mut free); @@ -920,6 +921,24 @@ mod tests { a.iter().zip(b).fold(0f32, |m, (x, y)| m.max((x - y).abs())) / scale } + /// Two 1 × 1 layers that only copy channels, so the right answer is exact: the first layer's stores (mode 0) went + /// wrong on DX12 with FXC on an NVIDIA GTX 1070, where every second pixel came out with its channels shifted. + #[test] + fn a_layer_stores_every_pixels_channels_where_the_next_reads_them() { + use lightcraft_denoise::net::{Conv, Op}; + let copy = |src: usize, cout: usize| { + let mut weight = vec![0f32; cout * 4]; + for co in 0..cout { + weight[co * 4 + co % 4] = 1.0; + } + Op::Conv(Conv { src, src2: None, cin: 4, cout, k: 1, weight, bias: vec![0.0; cout], leaky: None }) + }; + let n = Net::new(4, vec![copy(0, 4), copy(1, 12), Op::DepthToSpace2 { src: 2 }]).unwrap(); + let Some(g) = gpu(&n, 3) else { return }; + let x: Vec = (1..=36).map(|i| i as f32).collect(); + assert_eq!(g.run(&x).unwrap(), reference::run(&n, 3, &x).unwrap()); + } + #[test] fn the_gpu_gives_what_the_reference_gives() { // (tile, channels, depth): a tile that is not a multiple of the block size, odd channel counts, a deeper net diff --git a/crates/gpu/src/wgsl/nn_conv.wgsl b/crates/gpu/src/wgsl/nn_conv.wgsl index 20a0ce996..75389b517 100644 --- a/crates/gpu/src/wgsl/nn_conv.wgsl +++ b/crates/gpu/src/wgsl/nn_conv.wgsl @@ -27,7 +27,8 @@ struct P { cout: u32, leaky: u32, alpha: f32, - pad: u32, + // always 0: the output stores step by 1u + zero, see the end of main + zero: u32, } @group(0) @binding(0) var p: P; @@ -122,6 +123,10 @@ fn main(@builtin(workgroup_id) wg: vec3, @builtin(local_invocation_index) t return; } let bv = bias[n / 4u]; + // Four channels that follow each other are stored one by one, `one` apart. With a literal 1u, FXC + // (DX12) merges them into one 16-byte store, and NVIDIA's DX12 driver ran that wrong on a GTX + // 1070: every second pixel's channels came out shifted. FXC cannot see that `one` is 1. + let one = 1u + p.zero; for (var i = 0u; i < 4u; i++) { let m = m0 + ty * 4u + i; if (m >= hw) { @@ -135,9 +140,9 @@ fn main(@builtin(workgroup_id) wg: vec3, @builtin(local_invocation_index) t if (p.mode == 0u) { let o = m * p.n + n; dst[o] = v.x; - dst[o + 1u] = v.y; - dst[o + 2u] = v.z; - dst[o + 3u] = v.w; + dst[o + one] = v.y; + dst[o + 2u * one] = v.z; + dst[o + 3u * one] = v.w; } else if (p.mode == 1u) { let q = n / p.cout; let co = n % p.cout; @@ -146,9 +151,9 @@ fn main(@builtin(workgroup_id) wg: vec3, @builtin(local_invocation_index) t let opix = (2u * y + q / 2u) * (2u * p.w) + 2u * x + q % 2u; let o = opix * p.cout + co; dst[o] = v.x; - dst[o + 1u] = v.y; - dst[o + 2u] = v.z; - dst[o + 3u] = v.w; + dst[o + one] = v.y; + dst[o + 2u * one] = v.z; + dst[o + 3u * one] = v.w; } else { let c = n / 4u; let y = m / p.w; diff --git a/docs/gpu-pipeline.md b/docs/gpu-pipeline.md index b74af85a2..246b4c158 100644 --- a/docs/gpu-pipeline.md +++ b/docs/gpu-pipeline.md @@ -55,6 +55,12 @@ chooses; with `dxc`, put `dxcompiler.dll` *and* `dxil.dll` (DXC ≥ 1.8.2502) ne FXC builds the compute kernels somewhat slower than DXC (≈ 9 s vs 7.5 s for the GPU test suite on an RTX 4080), off the startup path. +FXC merges stores to neighbouring elements of a storage array into one wider store. NVIDIA's DX12 driver ran +the 16-byte stores it made of the denoise convolution's output wrong on a GTX 1070 (every second pixel's channels +shifted; DXC and Vulkan were right), so `nn_conv.wgsl` steps those stores by `1 + zero` from its uniform and FXC +leaves them apart. `nn::tests::a_layer_stores_every_pixels_channels_where_the_next_reads_them` catches it on such +a card. + **Off the startup path.** The compute device is created on a background thread once the window is up (`gpu::warm_up` from the first frame's settings), and never while GPU rendering is off: with Settings ▸ Performance ▸ *Use the GPU for rendering* unchecked (applied before the window opens), diff --git a/docs/whats-new.md b/docs/whats-new.md index 2f02ffb91..81d7498aa 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-19 + +### AI Denoise on DirectX 12 with NVIDIA GTX 10-series cards +- AI Denoise on the GPU under DirectX 12 (the Windows default) gives the same result as on the CPU again on an NVIDIA + GeForce GTX 1070. The driver ran the denoise network's output stores wrong, so every second pixel's colour + channels came out shifted and the denoised photo was wrong without any error. Vulkan was not affected. + ## 2026-10-14 ### Model downloads time out on a name that doesn't resolve