Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,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)
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()?;
cwfitzgerald marked this conversation as resolved.
Show resolved Hide resolved

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