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

Avoid using FXC as fallback #6643

Merged
merged 4 commits into from
Dec 2, 2024
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 @@ -122,6 +122,10 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
- Add actual sample type to `CreateBindGroupError::InvalidTextureSampleType` error message. By @ErichDonGubler in [#6530](https://github.com/gfx-rs/wgpu/pull/6530).
- Improve binding error to give a clearer message when there is a mismatch between resource binding as it is in the shader and as it is in the binding layout. By @eliemichel in [#6553](https://github.com/gfx-rs/wgpu/pull/6553).

#### D3D12

- Avoid using FXC as fallback when the DXC container was passed at instance creation. Paths to `dxcompiler.dll` & `dxil.dll` are also now required. By @teoxoy in [#6643](https://github.com/gfx-rs/wgpu/pull/6643).

#### HAL

- Change the `DropCallback` API to use `FnOnce` instead of `FnMut`. By @jerzywilczek in [#6482](https://github.com/gfx-rs/wgpu/pull/6482)
Expand Down
4 changes: 2 additions & 2 deletions wgpu-hal/examples/ray-traced-triangle/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ impl<A: hal::Api> Example<A> {
name: "example",
flags: wgt::InstanceFlags::default(),
dx12_shader_compiler: wgt::Dx12Compiler::DynamicDxc {
dxil_path: None,
dxc_path: None,
dxc_path: std::path::PathBuf::from("dxcompiler.dll"),
dxil_path: std::path::PathBuf::from("dxil.dll"),
},
gles_minor_version: wgt::Gles3MinorVersion::default(),
};
Expand Down
10 changes: 6 additions & 4 deletions wgpu-hal/src/dx12/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,10 @@ impl super::Adapter {
Direct3D12::D3D_SHADER_MODEL_6_2,
Direct3D12::D3D_SHADER_MODEL_6_1,
Direct3D12::D3D_SHADER_MODEL_6_0,
Direct3D12::D3D_SHADER_MODEL_5_1,
]
.iter();
match loop {

let highest_shader_model = loop {
if let Some(&sm) = versions.next() {
let mut sm = Direct3D12::D3D12_FEATURE_DATA_SHADER_MODEL {
HighestShaderModel: sm,
Expand All @@ -229,8 +229,10 @@ impl super::Adapter {
} else {
break Direct3D12::D3D_SHADER_MODEL_5_1;
}
} {
Direct3D12::D3D_SHADER_MODEL_5_1 => naga::back::hlsl::ShaderModel::V5_1,
};

match highest_shader_model {
Direct3D12::D3D_SHADER_MODEL_5_1 => return None, // don't expose this adapter if it doesn't support DXIL
Direct3D12::D3D_SHADER_MODEL_6_0 => naga::back::hlsl::ShaderModel::V6_0,
Direct3D12::D3D_SHADER_MODEL_6_1 => naga::back::hlsl::ShaderModel::V6_1,
Direct3D12::D3D_SHADER_MODEL_6_2 => naga::back::hlsl::ShaderModel::V6_2,
Expand Down
4 changes: 2 additions & 2 deletions wgpu-hal/src/dx12/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl crate::Instance for super::Instance {
)
})?;

container.map(Arc::new)
Some(Arc::new(container))
}
wgt::Dx12Compiler::StaticDxc => {
let container =
Expand All @@ -100,7 +100,7 @@ impl crate::Instance for super::Instance {
)
})?;

container.map(Arc::new)
Some(Arc::new(container))
}
wgt::Dx12Compiler::Fxc => None,
};
Expand Down
67 changes: 23 additions & 44 deletions wgpu-hal/src/dx12/shader_compilation.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::auxil::dxgi::result::HResult;
use std::ffi::CStr;
use std::path::PathBuf;
use std::{error::Error, ffi::CStr};
use thiserror::Error;
use windows::{
core::{Interface, PCSTR, PCWSTR},
Win32::Graphics::Direct3D::{Dxc, Fxc},
Expand Down Expand Up @@ -97,19 +98,7 @@ struct DxcLib {
}

impl DxcLib {
fn new_dynamic(
lib_path: Option<PathBuf>,
lib_name: &'static str,
) -> Result<Self, libloading::Error> {
let lib_path = if let Some(lib_path) = lib_path {
if lib_path.is_file() {
lib_path
} else {
lib_path.join(lib_name)
}
} else {
PathBuf::from(lib_name)
};
fn new_dynamic(lib_path: PathBuf) -> Result<Self, libloading::Error> {
unsafe { crate::dx12::DynLib::new(lib_path).map(|lib| Self { lib }) }
}

Expand Down Expand Up @@ -157,49 +146,39 @@ pub(super) struct DxcContainer {
_dxil: Option<DxcLib>,
}

#[derive(Debug, Error)]
pub(super) enum GetDynamicDXCContainerError {
#[error(transparent)]
Device(#[from] crate::DeviceError),
#[error("Failed to load {0}: {1}")]
FailedToLoad(&'static str, libloading::Error),
}

pub(super) fn get_dynamic_dxc_container(
dxc_path: Option<PathBuf>,
dxil_path: Option<PathBuf>,
) -> Result<Option<DxcContainer>, crate::DeviceError> {
let dxc = match DxcLib::new_dynamic(dxc_path, "dxcompiler.dll") {
Ok(dxc) => dxc,
Err(e) => {
log::warn!(
"Failed to load dxcompiler.dll. Defaulting to FXC instead: {}: {:?}",
e,
e.source()
);
return Ok(None);
}
};
dxc_path: PathBuf,
dxil_path: PathBuf,
) -> Result<DxcContainer, GetDynamicDXCContainerError> {
let dxc = DxcLib::new_dynamic(dxc_path)
.map_err(|e| GetDynamicDXCContainerError::FailedToLoad("dxcompiler.dll", e))?;

let dxil = match DxcLib::new_dynamic(dxil_path, "dxil.dll") {
Ok(dxil) => dxil,
Err(e) => {
log::warn!(
"Failed to load dxil.dll. Defaulting to FXC instead: {}: {:?}",
e,
e.source()
);
return Ok(None);
}
};
let dxil = DxcLib::new_dynamic(dxil_path)
.map_err(|e| GetDynamicDXCContainerError::FailedToLoad("dxil.dll", e))?;

let compiler = dxc.create_instance::<Dxc::IDxcCompiler3>()?;
let utils = dxc.create_instance::<Dxc::IDxcUtils>()?;
let validator = dxil.create_instance::<Dxc::IDxcValidator>()?;

Ok(Some(DxcContainer {
Ok(DxcContainer {
compiler,
utils,
validator: Some(validator),
_dxc: Some(dxc),
_dxil: Some(dxil),
}))
})
}

/// Creates a [`DxcContainer`] that delegates to the statically-linked version of DXC.
pub(super) fn get_static_dxc_container() -> Result<Option<DxcContainer>, crate::DeviceError> {
pub(super) fn get_static_dxc_container() -> Result<DxcContainer, crate::DeviceError> {
#[cfg(feature = "static-dxc")]
{
unsafe {
Expand All @@ -218,13 +197,13 @@ pub(super) fn get_static_dxc_container() -> Result<Option<DxcContainer>, crate::
))
})?;

Ok(Some(DxcContainer {
Ok(DxcContainer {
compiler,
utils,
validator: None,
_dxc: None,
_dxil: None,
}))
})
}
}
#[cfg(not(feature = "static-dxc"))]
Expand Down
8 changes: 4 additions & 4 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7432,10 +7432,10 @@ pub enum Dx12Compiler {
///
/// It also requires WDDM 2.1 (Windows 10 version 1607).
DynamicDxc {
/// Path to the `dxil.dll` file, or path to the directory containing `dxil.dll` file. Passing `None` will use standard platform specific dll loading rules.
dxil_path: Option<PathBuf>,
/// Path to the `dxcompiler.dll` file, or path to the directory containing `dxcompiler.dll` file. Passing `None` will use standard platform specific dll loading rules.
dxc_path: Option<PathBuf>,
/// Path to `dxcompiler.dll`.
dxc_path: PathBuf,
/// Path to `dxil.dll`.
dxil_path: PathBuf,
},
/// The statically-linked variant of Dxc.
/// The `static-dxc` feature is required to use this.
Expand Down
4 changes: 2 additions & 2 deletions wgpu/src/util/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ pub fn dx12_shader_compiler_from_env() -> Option<wgt::Dx12Compiler> {
.as_deref()
{
Ok("dxc") => wgt::Dx12Compiler::DynamicDxc {
dxil_path: None,
dxc_path: None,
dxc_path: std::path::PathBuf::from("dxcompiler.dll"),
dxil_path: std::path::PathBuf::from("dxil.dll"),
},
#[cfg(feature = "static-dxc")]
Ok("static-dxc") => wgt::Dx12Compiler::StaticDxc,
Expand Down