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