From 9cf6c8c675fcd0eb6edd19a6cab47cd3d7370d92 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:07:48 +0100 Subject: [PATCH 1/4] [d3d12] don't fallback on FXC if DXC failed to load --- wgpu-hal/src/dx12/instance.rs | 4 +- wgpu-hal/src/dx12/shader_compilation.rs | 49 ++++++++++--------------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index 389843a21c..17fe1236e5 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -89,7 +89,7 @@ impl crate::Instance for super::Instance { ) })?; - container.map(Arc::new) + Some(Arc::new(container)) } wgt::Dx12Compiler::StaticDxc => { let container = @@ -100,7 +100,7 @@ impl crate::Instance for super::Instance { ) })?; - container.map(Arc::new) + Some(Arc::new(container)) } wgt::Dx12Compiler::Fxc => None, }; diff --git a/wgpu-hal/src/dx12/shader_compilation.rs b/wgpu-hal/src/dx12/shader_compilation.rs index 96865b9080..53e44e6d31 100644 --- a/wgpu-hal/src/dx12/shader_compilation.rs +++ b/wgpu-hal/src/dx12/shader_compilation.rs @@ -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}, @@ -157,49 +158,39 @@ pub(super) struct DxcContainer { _dxil: Option, } +#[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, dxil_path: Option, -) -> Result, 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); - } - }; +) -> Result { + let dxc = DxcLib::new_dynamic(dxc_path, "dxcompiler.dll") + .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, "dxil.dll") + .map_err(|e| GetDynamicDXCContainerError::FailedToLoad("dxil.dll", e))?; let compiler = dxc.create_instance::()?; let utils = dxc.create_instance::()?; let validator = dxil.create_instance::()?; - 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, crate::DeviceError> { +pub(super) fn get_static_dxc_container() -> Result { #[cfg(feature = "static-dxc")] { unsafe { @@ -218,13 +209,13 @@ pub(super) fn get_static_dxc_container() -> Result, crate:: )) })?; - Ok(Some(DxcContainer { + Ok(DxcContainer { compiler, utils, validator: None, _dxc: None, _dxil: None, - })) + }) } } #[cfg(not(feature = "static-dxc"))] From 308e89f61a8a0b19b5d937633a6e73c7bbd8261e Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:16:20 +0100 Subject: [PATCH 2/4] [d3d12] remove complex DXC loading rules --- wgpu-hal/examples/ray-traced-triangle/main.rs | 4 ++-- wgpu-hal/src/dx12/shader_compilation.rs | 22 +++++-------------- wgpu-types/src/lib.rs | 8 +++---- wgpu/src/util/init.rs | 4 ++-- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/wgpu-hal/examples/ray-traced-triangle/main.rs b/wgpu-hal/examples/ray-traced-triangle/main.rs index 6754dc36a9..f24f0c4c41 100644 --- a/wgpu-hal/examples/ray-traced-triangle/main.rs +++ b/wgpu-hal/examples/ray-traced-triangle/main.rs @@ -240,8 +240,8 @@ impl Example { 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(), }; diff --git a/wgpu-hal/src/dx12/shader_compilation.rs b/wgpu-hal/src/dx12/shader_compilation.rs index 53e44e6d31..9739627017 100644 --- a/wgpu-hal/src/dx12/shader_compilation.rs +++ b/wgpu-hal/src/dx12/shader_compilation.rs @@ -98,19 +98,7 @@ struct DxcLib { } impl DxcLib { - fn new_dynamic( - lib_path: Option, - lib_name: &'static str, - ) -> Result { - 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 { unsafe { crate::dx12::DynLib::new(lib_path).map(|lib| Self { lib }) } } @@ -167,13 +155,13 @@ pub(super) enum GetDynamicDXCContainerError { } pub(super) fn get_dynamic_dxc_container( - dxc_path: Option, - dxil_path: Option, + dxc_path: PathBuf, + dxil_path: PathBuf, ) -> Result { - let dxc = DxcLib::new_dynamic(dxc_path, "dxcompiler.dll") + let dxc = DxcLib::new_dynamic(dxc_path) .map_err(|e| GetDynamicDXCContainerError::FailedToLoad("dxcompiler.dll", e))?; - let dxil = DxcLib::new_dynamic(dxil_path, "dxil.dll") + let dxil = DxcLib::new_dynamic(dxil_path) .map_err(|e| GetDynamicDXCContainerError::FailedToLoad("dxil.dll", e))?; let compiler = dxc.create_instance::()?; diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 546a3fde0d..ec05e86e43 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -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, - /// 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, + /// 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. diff --git a/wgpu/src/util/init.rs b/wgpu/src/util/init.rs index 2419f4be9c..87f787bcb8 100644 --- a/wgpu/src/util/init.rs +++ b/wgpu/src/util/init.rs @@ -108,8 +108,8 @@ pub fn dx12_shader_compiler_from_env() -> Option { .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, From bac218a746ee31aa020192aa64ce6598e7451779 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:25:37 +0100 Subject: [PATCH 3/4] [d3d12] don't expose adapter if it doesn't support DXIL when the DXC container is present --- wgpu-hal/src/dx12/adapter.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index b88941c81e..eb4bfa514d 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -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, @@ -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, From be5b5ff7d4eb2431cb39376fbcd3e063d5017ad1 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:33:02 +0100 Subject: [PATCH 4/4] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23741c882a..8af0b5a709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)