From 7826092d866ed624d906cebf6988be43882edaf3 Mon Sep 17 00:00:00 2001
From: Elabajaba <Elabajaba@users.noreply.github.com>
Date: Wed, 1 Feb 2023 22:55:54 -0500
Subject: [PATCH] Fix dx12 shader validation errors when dxil.dll isn't
 available in the local scope. (#3434)

* Fix dx12 shader validation errors when dxil.dll isn't available in the local scope.

* changelog

* clippy

* always explicitly validate shaders to simplify code

* destructor ordering
---
 CHANGELOG.md                            |  4 ++
 wgpu-hal/src/dx12/shader_compilation.rs | 51 +++++++++++++++++--------
 2 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 887c4775ff..eeb0dcb046 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -53,6 +53,10 @@ Bottom level categories:
 
 ### Bug Fixes
 
+#### DX12
+
+- Fix DXC validation issues when using a custom `dxil_path`. By @Elabajaba in [#3434](https://github.com/gfx-rs/wgpu/pull/3434)
+
 #### General
 
 - `copyTextureToTexture` src/dst aspects must both refer to all aspects of src/dst format. By @teoxoy in [#3431](https://github.com/gfx-rs/wgpu/pull/3431)
diff --git a/wgpu-hal/src/dx12/shader_compilation.rs b/wgpu-hal/src/dx12/shader_compilation.rs
index 71aed0a553..443e12709b 100644
--- a/wgpu-hal/src/dx12/shader_compilation.rs
+++ b/wgpu-hal/src/dx12/shader_compilation.rs
@@ -83,11 +83,15 @@ pub(super) fn compile_fxc(
 mod dxc {
     use std::path::PathBuf;
 
+    // Destructor order should be fine since _dxil and _dxc don't rely on each other.
     pub(crate) struct DxcContainer {
-        pub compiler: hassle_rs::DxcCompiler,
-        pub library: hassle_rs::DxcLibrary,
-        // Has to be held onto for the lifetime of the device otherwise shaders will fail to compile
+        compiler: hassle_rs::DxcCompiler,
+        library: hassle_rs::DxcLibrary,
+        validator: hassle_rs::DxcValidator,
+        // Has to be held onto for the lifetime of the device otherwise shaders will fail to compile.
         _dxc: hassle_rs::Dxc,
+        // Also Has to be held onto for the lifetime of the device otherwise shaders will fail to validate.
+        _dxil: hassle_rs::Dxil,
     }
 
     pub(crate) fn get_dxc_container(
@@ -95,14 +99,17 @@ mod dxc {
         dxil_path: Option<PathBuf>,
     ) -> Result<Option<DxcContainer>, crate::DeviceError> {
         // Make sure that dxil.dll exists.
-        match hassle_rs::Dxil::new(dxil_path) {
-            Ok(_) => (),
+        let dxil = match hassle_rs::Dxil::new(dxil_path) {
+            Ok(dxil) => dxil,
             Err(e) => {
                 log::warn!("Failed to load dxil.dll. Defaulting to Fxc instead: {}", e);
                 return Ok(None);
             }
         };
 
+        // Needed for explicit validation.
+        let validator = dxil.create_validator()?;
+
         let dxc = match hassle_rs::Dxc::new(dxc_path) {
             Ok(dxc) => dxc,
             Err(e) => {
@@ -113,13 +120,15 @@ mod dxc {
                 return Ok(None);
             }
         };
-        let dxc_compiler = dxc.create_compiler()?;
-        let dxc_library = dxc.create_library()?;
+        let compiler = dxc.create_compiler()?;
+        let library = dxc.create_library()?;
 
         Ok(Some(DxcContainer {
             _dxc: dxc,
-            compiler: dxc_compiler,
-            library: dxc_library,
+            compiler,
+            library,
+            _dxil: dxil,
+            validator,
         }))
     }
 
@@ -136,8 +145,9 @@ mod dxc {
         log::Level,
     ) {
         profiling::scope!("compile_dxc");
-        let mut compile_flags = arrayvec::ArrayVec::<&str, 3>::new_const();
+        let mut compile_flags = arrayvec::ArrayVec::<&str, 4>::new_const();
         compile_flags.push("-Ges"); // d3dcompiler::D3DCOMPILE_ENABLE_STRICTNESS
+        compile_flags.push("-Vd"); // Disable implicit validation to work around bugs when dxil.dll isn't in the local directory.
         if device
             .private_caps
             .instance_flags
@@ -156,7 +166,6 @@ mod dxc {
             Err(e) => return (Err(e), log::Level::Error),
         };
 
-        // DXC will automatically validate the shaders during compilation as long as dxil.dll is available, so we don't need to do it ourselves.
         let compiled = dxc_container.compiler.compile(
             &blob,
             source_name,
@@ -169,10 +178,22 @@ mod dxc {
 
         let (result, log_level) = match compiled {
             Ok(dxc_result) => match dxc_result.get_result() {
-                Ok(dxc_blob) => (
-                    Ok(crate::dx12::CompiledShader::Dxc(dxc_blob.to_vec())),
-                    log::Level::Info,
-                ),
+                Ok(dxc_blob) => {
+                    // Validate the shader.
+                    match dxc_container.validator.validate(dxc_blob) {
+                        Ok(validated_blob) => (
+                            Ok(crate::dx12::CompiledShader::Dxc(validated_blob.to_vec())),
+                            log::Level::Info,
+                        ),
+                        Err(e) => (
+                            Err(crate::PipelineError::Linkage(
+                                stage_bit,
+                                format!("DXC validation error: {:?}\n{:?}", e.0, e.1),
+                            )),
+                            log::Level::Error,
+                        ),
+                    }
+                }
                 Err(e) => (
                     Err(crate::PipelineError::Linkage(
                         stage_bit,