Merge PR #2380: LR-EDIT-DETAIL-DENOISE: GPU denoise gives the right result on DX12 with NVIDIA Pascal

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<Layer>)
                     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<Layer>)
                     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,44 @@ mod tests {
         a.iter().zip(b).fold(0f32, |m, (x, y)| m.max((x - y).abs())) / scale
     }
 
+    /// A 1 × 1 convolution from 4 channels to `cout` that copies input channel `co % 4` to each output channel `co`.
+    fn copy(src: usize, cout: usize) -> Op {
+        let mut weight = vec![0f32; cout * 4];
+        for co in 0..cout {
+            weight[co * 4 + co % 4] = 1.0;
+        }
+        Op::Conv(lightcraft_denoise::net::Conv { src, src2: None, cin: 4, cout, k: 1, weight, bias: vec![0.0; cout], leaky: None })
+    }
+
+    /// 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() {
+        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<f32> = (1..=36).map(|i| i as f32).collect();
+        assert_eq!(g.run(&x).unwrap(), reference::run(&n, 3, &x).unwrap());
+    }
+
+    /// The same for a transposed convolution's stores (mode 1): a max-pool halves the tile, and the 2 × 2 transposed
+    /// convolution copies each pooled pixel's channels to the four pixels it becomes.
+    #[test]
+    fn a_transposed_convolution_stores_every_pixels_channels_where_the_next_reads_them() {
+        use lightcraft_denoise::net::ConvT2;
+        // [cin][cout][2][2]: channel c to channel c at all four places
+        let mut weight = vec![0f32; 4 * 4 * 4];
+        for c in 0..4 {
+            for q in 0..4 {
+                weight[(c * 4 + c) * 4 + q] = 1.0;
+            }
+        }
+        let up = Op::ConvT2(ConvT2 { src: 1, cin: 4, cout: 4, weight, bias: vec![0.0; 4], leaky: None });
+        let n = Net::new(4, vec![Op::MaxPool2 { src: 0 }, up, copy(2, 12), Op::DepthToSpace2 { src: 3 }]).unwrap();
+        let Some(g) = gpu(&n, 6) else { return };
+        let x: Vec<f32> = (1..=144).map(|i| i as f32).collect();
+        assert_eq!(g.run(&x).unwrap(), reference::run(&n, 6, &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

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<uniform> p: P;
@@ -122,6 +123,11 @@ fn main(@builtin(workgroup_id) wg: vec3<u32>, @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 mode 0's wrong on a
+    // GTX 1070: every second pixel's channels came out shifted. Mode 1's merged store ran right
+    // there, but it is the same instruction, so it steps the same way. 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 +141,9 @@ fn main(@builtin(workgroup_id) wg: vec3<u32>, @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 +152,9 @@ fn main(@builtin(workgroup_id) wg: vec3<u32>, @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;

docs/gpu-pipeline.md

@@ -57,6 +57,13 @@ 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 store it made of a 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. The transposed convolution's merged store ran right on that card, but it steps the same way.
+`nn::tests::a_layer_stores_every_pixels_channels_where_the_next_reads_them` catches the fault on such a card;
+`a_transposed_convolution_stores_every_pixels_channels_where_the_next_reads_them` checks the other store exactly.
+
 **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),

docs/whats-new.md

@@ -11,6 +11,11 @@
 
 ## 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.
+
 ### Hidden Local locations come back one at a time
 - With several locations removed from Local, "Show N hidden locations" now lists them, so each comes back on its own
   (Restore All is there too). The row used to bring every hidden location back at once (issue #2114).