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. (gfx-rs#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 b31069f commit f16ccc1
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 f16ccc1

Please sign in to comment.