Skip to content

Commit

Permalink
Fix dx12 shader validation errors when dxil.dll isn't available in th…
Browse files Browse the repository at this point in the history
…e 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
  • Loading branch information
Elabajaba authored and cwfitzgerald committed Feb 9, 2023
1 parent 736989b commit 37d24fb
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 15 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ Bottom level categories:

- Improve format MSAA capabilities detection. By @jinleili in [#3429](https://github.com/gfx-rs/wgpu/pull/3429)

### 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)

## wgpu-0.15.0 (2023-01-25)

### Major Changes
Expand Down
51 changes: 36 additions & 15 deletions wgpu-hal/src/dx12/shader_compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,26 +83,33 @@ 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(
dxc_path: Option<PathBuf>,
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) => {
Expand All @@ -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,
}))
}

Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 37d24fb

Please sign in to comment.