From dd01b6d20967f1b4a74e9a75baec6b54545ebfec Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Wed, 21 Aug 2024 15:10:48 +0200 Subject: [PATCH] [d3d12] make `DxgiLib` and `D3D12Lib` methods consistent --- wgpu-hal/src/auxil/dxgi/factory.rs | 44 ++-------- wgpu-hal/src/dx12/adapter.rs | 16 +--- wgpu-hal/src/dx12/instance.rs | 51 +++-------- wgpu-hal/src/dx12/mod.rs | 136 +++++++++++++++++++---------- wgpu-hal/src/lib.rs | 6 ++ 5 files changed, 119 insertions(+), 134 deletions(-) diff --git a/wgpu-hal/src/auxil/dxgi/factory.rs b/wgpu-hal/src/auxil/dxgi/factory.rs index 6c68ffeea..f2b6e624f 100644 --- a/wgpu-hal/src/auxil/dxgi/factory.rs +++ b/wgpu-hal/src/auxil/dxgi/factory.rs @@ -225,18 +225,8 @@ pub fn create_factory( // The `DXGI_CREATE_FACTORY_DEBUG` flag is only allowed to be passed to // `CreateDXGIFactory2` if the debug interface is actually available. So // we check for whether it exists first. - match lib_dxgi.debug_interface1() { - Ok(pair) => match pair { - Ok(_debug_controller) => { - factory_flags |= Dxgi::DXGI_CREATE_FACTORY_DEBUG; - } - Err(err) => { - log::warn!("Unable to enable DXGI debug interface: {}", err); - } - }, - Err(err) => { - log::warn!("Debug interface function for DXGI not found: {:?}", err); - } + if lib_dxgi.debug_interface1().is_ok() { + factory_flags |= Dxgi::DXGI_CREATE_FACTORY_DEBUG; } // Intercept `OutputDebugString` calls @@ -244,27 +234,18 @@ pub fn create_factory( } // Try to create IDXGIFactory4 - let factory4 = match lib_dxgi.create_factory2(factory_flags) { - Ok(pair) => match pair { - Ok(factory) => Some(factory), - // We hard error here as we _should have_ been able to make a factory4 but couldn't. - Err(err) => { - // err is a Cow, not an Error implementor - return Err(crate::InstanceError::new(format!( - "failed to create IDXGIFactory4: {err:?}" - ))); - } - }, + let factory4 = match lib_dxgi.create_factory4(factory_flags) { + Ok(factory) => Some(factory), // If we require factory4, hard error. Err(err) if required_factory_type == DxgiFactoryType::Factory4 => { return Err(crate::InstanceError::with_source( - String::from("IDXGIFactory1 creation function not found"), + String::from("IDXGIFactory4 creation failed"), err, )); } // If we don't print it to warn as all win7 will hit this case. Err(err) => { - log::warn!("IDXGIFactory1 creation function not found: {err:?}"); + log::warn!("IDXGIFactory4 creation function not found: {err:?}"); None } }; @@ -293,19 +274,10 @@ pub fn create_factory( // Try to create IDXGIFactory1 let factory1 = match lib_dxgi.create_factory1() { - Ok(pair) => match pair { - Ok(factory) => factory, - Err(err) => { - // err is a Cow, not an Error implementor - return Err(crate::InstanceError::new(format!( - "failed to create IDXGIFactory1: {err:?}" - ))); - } - }, - // We always require at least factory1, so hard error + Ok(factory) => factory, Err(err) => { return Err(crate::InstanceError::with_source( - String::from("IDXGIFactory1 creation function not found"), + String::from("IDXGIFactory1 creation failed"), err, )); } diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index 85b5728e4..690c9dcb7 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -64,19 +64,9 @@ impl super::Adapter { // Create the device so that we can get the capabilities. let device = { profiling::scope!("ID3D12Device::create_device"); - match library.create_device(&adapter, Direct3D::D3D_FEATURE_LEVEL_11_0) { - Ok(pair) => match pair { - Ok(device) => device, - Err(err) => { - log::warn!("Device creation failed: {}", err); - return None; - } - }, - Err(err) => { - log::warn!("Device creation function is not found: {:?}", err); - return None; - } - } + library + .create_device(&adapter, Direct3D::D3D_FEATURE_LEVEL_11_0) + .ok()? }; profiling::scope!("feature queries"); diff --git a/wgpu-hal/src/dx12/instance.rs b/wgpu-hal/src/dx12/instance.rs index 66af28565..a3fcc2a70 100644 --- a/wgpu-hal/src/dx12/instance.rs +++ b/wgpu-hal/src/dx12/instance.rs @@ -34,31 +34,20 @@ impl crate::Instance for super::Instance { .intersects(wgt::InstanceFlags::VALIDATION | wgt::InstanceFlags::GPU_BASED_VALIDATION) { // Enable debug layer - match lib_main.debug_interface() { - Ok(pair) => match pair { - Ok(debug_controller) => { - if desc.flags.intersects(wgt::InstanceFlags::VALIDATION) { - unsafe { debug_controller.EnableDebugLayer() } - } - if desc - .flags - .intersects(wgt::InstanceFlags::GPU_BASED_VALIDATION) - { - #[allow(clippy::collapsible_if)] - if let Ok(debug1) = debug_controller.cast::() - { - unsafe { debug1.SetEnableGPUBasedValidation(true) } - } else { - log::warn!("Failed to enable GPU-based validation"); - } - } + if let Ok(debug_controller) = lib_main.debug_interface() { + if desc.flags.intersects(wgt::InstanceFlags::VALIDATION) { + unsafe { debug_controller.EnableDebugLayer() } + } + if desc + .flags + .intersects(wgt::InstanceFlags::GPU_BASED_VALIDATION) + { + #[allow(clippy::collapsible_if)] + if let Ok(debug1) = debug_controller.cast::() { + unsafe { debug1.SetEnableGPUBasedValidation(true) } + } else { + log::warn!("Failed to enable GPU-based validation"); } - Err(err) => { - log::warn!("Unable to enable D3D12 debug interface: {}", err); - } - }, - Err(err) => { - log::warn!("Debug interface function for D3D12 not found: {:?}", err); } } } @@ -70,19 +59,7 @@ impl crate::Instance for super::Instance { )?; // Create IDXGIFactoryMedia - let factory_media = match lib_dxgi.create_factory_media() { - Ok(pair) => match pair { - Ok(factory_media) => Some(factory_media), - Err(err) => { - log::error!("Failed to create IDXGIFactoryMedia: {}", err); - None - } - }, - Err(err) => { - log::warn!("IDXGIFactory1 creation function not found: {:?}", err); - None - } - }; + let factory_media = lib_dxgi.create_factory_media().ok(); let mut supports_allow_tearing = false; if let Some(factory5) = factory.as_factory5() { diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs index 9e1413f8d..9ff787fc4 100644 --- a/wgpu-hal/src/dx12/mod.rs +++ b/wgpu-hal/src/dx12/mod.rs @@ -65,21 +65,47 @@ use crate::auxil::{ }, }; +#[derive(Debug)] +struct DynLib { + inner: libloading::Library, +} + +impl DynLib { + unsafe fn new(filename: &'static str) -> Result { + unsafe { libloading::Library::new(filename) }.map(|inner| Self { inner }) + } + + unsafe fn get( + &self, + symbol: &[u8], + ) -> Result, crate::DeviceError> { + unsafe { self.inner.get(symbol) }.map_err(|e| match e { + libloading::Error::GetProcAddress { .. } | libloading::Error::GetProcAddressUnknown => { + crate::DeviceError::Unexpected + } + libloading::Error::IncompatibleSize + | libloading::Error::CreateCString { .. } + | libloading::Error::CreateCStringWithTrailing { .. } => crate::hal_internal_error(e), + _ => crate::DeviceError::Unexpected, // could be unreachable!() but we prefer to be more robust + }) + } +} + #[derive(Debug)] struct D3D12Lib { - lib: libloading::Library, + lib: DynLib, } impl D3D12Lib { fn new() -> Result { - unsafe { libloading::Library::new("d3d12.dll").map(|lib| D3D12Lib { lib }) } + unsafe { DynLib::new("d3d12.dll").map(|lib| Self { lib }) } } fn create_device( &self, adapter: &DxgiAdapter, feature_level: Direct3D::D3D_FEATURE_LEVEL, - ) -> Result, libloading::Error> { + ) -> Result { // Calls windows::Win32::Graphics::Direct3D12::D3D12CreateDevice on d3d12.dll type Fun = extern "system" fn( padapter: *mut core::ffi::c_void, @@ -90,14 +116,18 @@ impl D3D12Lib { let func: libloading::Symbol = unsafe { self.lib.get(b"D3D12CreateDevice") }?; let mut result__ = None; - Ok((func)( + + (func)( unsafe { adapter.param().abi() }, feature_level, // TODO: Generic? &Direct3D12::ID3D12Device::IID, <*mut _>::cast(&mut result__), ) - .map(|| result__.expect("D3D12CreateDevice succeeded but result is NULL?"))) + .ok() + .into_device_result("Device creation")?; + + result__.ok_or(crate::DeviceError::Unexpected) } fn serialize_root_signature( @@ -114,11 +144,8 @@ impl D3D12Lib { ppblob: *mut *mut core::ffi::c_void, pperrorblob: *mut *mut core::ffi::c_void, ) -> windows_core::HRESULT; - let func: libloading::Symbol = unsafe { self.lib.get(b"D3D12SerializeRootSignature") } - .map_err(|e| { - log::error!("Unable to find serialization function: {:?}", e); - crate::DeviceError::Lost - })?; + let func: libloading::Symbol = + unsafe { self.lib.get(b"D3D12SerializeRootSignature") }?; let desc = Direct3D12::D3D12_ROOT_SIGNATURE_DESC { NumParameters: parameters.len() as _, @@ -137,8 +164,6 @@ impl D3D12Lib { <*mut _>::cast(&mut error), ) .ok() - // TODO: If there's a HRESULT, error may still be non-null and - // contain info. .into_device_result("Root signature serialization")?; if let Some(error) = error { @@ -147,17 +172,13 @@ impl D3D12Lib { "Root signature serialization error: {:?}", unsafe { error.as_c_str() }.unwrap().to_str().unwrap() ); - return Err(crate::DeviceError::Lost); + return Err(crate::DeviceError::Unexpected); // could be hal_usage_error or hal_internal_error } - Ok(D3DBlob(blob.expect( - "D3D12SerializeRootSignature succeeded but result is NULL?", - ))) + blob.ok_or(crate::DeviceError::Unexpected) } - fn debug_interface( - &self, - ) -> Result, libloading::Error> { + fn debug_interface(&self) -> Result { // Calls windows::Win32::Graphics::Direct3D12::D3D12GetDebugInterface on d3d12.dll type Fun = extern "system" fn( riid: *const windows_core::GUID, @@ -165,25 +186,28 @@ impl D3D12Lib { ) -> windows_core::HRESULT; let func: libloading::Symbol = unsafe { self.lib.get(b"D3D12GetDebugInterface") }?; - let mut result__ = core::ptr::null_mut(); - Ok((func)(&Direct3D12::ID3D12Debug::IID, &mut result__) - .and_then(|| unsafe { windows_core::Type::from_abi(result__) })) + let mut result__ = None; + + (func)(&Direct3D12::ID3D12Debug::IID, <*mut _>::cast(&mut result__)) + .ok() + .into_device_result("GetDebugInterface")?; + + result__.ok_or(crate::DeviceError::Unexpected) } } #[derive(Debug)] pub(super) struct DxgiLib { - lib: libloading::Library, + lib: DynLib, } impl DxgiLib { pub fn new() -> Result { - unsafe { libloading::Library::new("dxgi.dll").map(|lib| DxgiLib { lib }) } + unsafe { DynLib::new("dxgi.dll").map(|lib| Self { lib }) } } - pub fn debug_interface1( - &self, - ) -> Result, libloading::Error> { + /// Will error with crate::DeviceError::Unexpected if DXGI 1.3 is not available. + pub fn debug_interface1(&self) -> Result { // Calls windows::Win32::Graphics::Dxgi::DXGIGetDebugInterface1 on dxgi.dll type Fun = extern "system" fn( flags: u32, @@ -192,14 +216,16 @@ impl DxgiLib { ) -> windows_core::HRESULT; let func: libloading::Symbol = unsafe { self.lib.get(b"DXGIGetDebugInterface1") }?; - let mut result__ = core::ptr::null_mut(); - Ok((func)(0, &Dxgi::IDXGIInfoQueue::IID, &mut result__) - .and_then(|| unsafe { windows_core::Type::from_abi(result__) })) + let mut result__ = None; + + (func)(0, &Dxgi::IDXGIInfoQueue::IID, <*mut _>::cast(&mut result__)) + .ok() + .into_device_result("debug_interface1")?; + + result__.ok_or(crate::DeviceError::Unexpected) } - pub fn create_factory1( - &self, - ) -> Result, libloading::Error> { + pub fn create_factory1(&self) -> Result { // Calls windows::Win32::Graphics::Dxgi::CreateDXGIFactory1 on dxgi.dll type Fun = extern "system" fn( riid: *const windows_core::GUID, @@ -207,15 +233,20 @@ impl DxgiLib { ) -> windows_core::HRESULT; let func: libloading::Symbol = unsafe { self.lib.get(b"CreateDXGIFactory1") }?; - let mut result__ = core::ptr::null_mut(); - Ok((func)(&Dxgi::IDXGIFactory1::IID, &mut result__) - .and_then(|| unsafe { windows_core::Type::from_abi(result__) })) + let mut result__ = None; + + (func)(&Dxgi::IDXGIFactory1::IID, <*mut _>::cast(&mut result__)) + .ok() + .into_device_result("create_factory1")?; + + result__.ok_or(crate::DeviceError::Unexpected) } - pub fn create_factory2( + /// Will error with crate::DeviceError::Unexpected if DXGI 1.3 is not available. + pub fn create_factory4( &self, factory_flags: Dxgi::DXGI_CREATE_FACTORY_FLAGS, - ) -> Result, libloading::Error> { + ) -> Result { // Calls windows::Win32::Graphics::Dxgi::CreateDXGIFactory2 on dxgi.dll type Fun = extern "system" fn( flags: Dxgi::DXGI_CREATE_FACTORY_FLAGS, @@ -224,16 +255,21 @@ impl DxgiLib { ) -> windows_core::HRESULT; let func: libloading::Symbol = unsafe { self.lib.get(b"CreateDXGIFactory2") }?; - let mut result__ = core::ptr::null_mut(); - Ok( - (func)(factory_flags, &Dxgi::IDXGIFactory4::IID, &mut result__) - .and_then(|| unsafe { windows_core::Type::from_abi(result__) }), + let mut result__ = None; + + (func)( + factory_flags, + &Dxgi::IDXGIFactory4::IID, + <*mut _>::cast(&mut result__), ) + .ok() + .into_device_result("create_factory4")?; + + result__.ok_or(crate::DeviceError::Unexpected) } - pub fn create_factory_media( - &self, - ) -> Result, libloading::Error> { + /// Will error with crate::DeviceError::Unexpected if DXGI 1.3 is not available. + pub fn create_factory_media(&self) -> Result { // Calls windows::Win32::Graphics::Dxgi::CreateDXGIFactory1 on dxgi.dll type Fun = extern "system" fn( riid: *const windows_core::GUID, @@ -241,10 +277,14 @@ impl DxgiLib { ) -> windows_core::HRESULT; let func: libloading::Symbol = unsafe { self.lib.get(b"CreateDXGIFactory1") }?; - let mut result__ = core::ptr::null_mut(); + let mut result__ = None; + // https://learn.microsoft.com/en-us/windows/win32/api/dxgi1_3/nn-dxgi1_3-idxgifactorymedia - Ok((func)(&Dxgi::IDXGIFactoryMedia::IID, &mut result__) - .and_then(|| unsafe { windows_core::Type::from_abi(result__) })) + (func)(&Dxgi::IDXGIFactoryMedia::IID, <*mut _>::cast(&mut result__)) + .ok() + .into_device_result("create_factory_media")?; + + result__.ok_or(crate::DeviceError::Unexpected) } } diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index 60b3a67bb..b994498d7 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -351,6 +351,12 @@ fn hal_usage_error(txt: T) -> ! { panic!("wgpu-hal invariant was violated (usage error): {txt}") } +#[allow(dead_code)] // may be unused on some platforms +#[cold] +fn hal_internal_error(txt: T) -> ! { + panic!("wgpu-hal ran into a preventable internal error: {txt}") +} + #[derive(Clone, Debug, Eq, PartialEq, Error)] pub enum ShaderError { #[error("Compilation failed: {0:?}")]