nordic-dev.net/wgpu
nordic-dev.net/wgpu
2
0
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2024-11-21 22:33:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

[wgpu-hal] #5956 windows-rs migration followups and cleanups (#6173)

Browse Source 
PR #5956 wasn't fully complete and still had some outstanding minor
issues and cleanups to be done, as well as hidden semantic changes.
This addresses a bunch of them:

- Remove unnecessary `Error` mapping to `String` as `windows-rs`'s
  `Error` has a more complete `Display` representation by itself.
- Remove `into_result()` as every call could have formatted the
  `windows-rs` `Error` in a log call directly.
- Pass `None` instead of a pointer to an empty slice wherever possible
  (waiting for https://github.com/microsoft/win32metadata/pull/1971 to
  trickle down into `windows-rs`).
- Remove `.clone()` on COM objects (temporarily increasing the refcount)
  when it can be avoided by inverting the order of operations on `raw`
  variables.
This commit is contained in:
Marijn Suijten 2024-08-30 11:02:38 +02:00 committed by GitHub
parent bbdbafdf8a
commit 4454cbfaab
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 59 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -119,7 +119,7 @@ By @teoxoy [#6134](https://github.com/gfx-rs/wgpu/pull/6134).
#### DX12
- Replace `winapi` code to use the `windows` crate. By @MarijnS95 in [#5956](https://github.com/gfx-rs/wgpu/pull/5956)
- Replace `winapi` code to use the `windows` crate. By @MarijnS95 in [#5956](https://github.com/gfx-rs/wgpu/pull/5956) and [#6173](https://github.com/gfx-rs/wgpu/pull/6173)
## 22.0.0 (2024-07-17)

34
wgpu-hal/src/auxil/dxgi/result.rs
View File

@ -1,56 +1,32 @@
use std::borrow::Cow;
use windows::Win32::{Foundation, Graphics::Dxgi};
pub(crate) trait HResult<O> {
fn into_result(self) -> Result<O, Cow<'static, str>>;
fn into_device_result(self, description: &str) -> Result<O, crate::DeviceError>;
}
impl<T> HResult<T> for windows::core::Result<T> {
fn into_result(self) -> Result<T, Cow<'static, str>> {
// TODO: use windows-rs built-in error formatting?
let description = match self {
Ok(t) => return Ok(t),
Err(e) if e.code() == Foundation::E_UNEXPECTED => "unexpected",
Err(e) if e.code() == Foundation::E_NOTIMPL => "not implemented",
Err(e) if e.code() == Foundation::E_OUTOFMEMORY => "out of memory",
Err(e) if e.code() == Foundation::E_INVALIDARG => "invalid argument",
Err(e) => return Err(Cow::Owned(format!("{e:?}"))),
};
Err(Cow::Borrowed(description))
}
fn into_device_result(self, description: &str) -> Result<T, crate::DeviceError> {
#![allow(unreachable_code)]
let err_code = if let Err(err) = &self {
Some(err.code())
} else {
None
};
self.into_result().map_err(|err| {
self.map_err(|err| {
log::error!("{} failed: {}", description, err);
let Some(err_code) = err_code else {
unreachable!()
};
match err_code {
match err.code() {
Foundation::E_OUTOFMEMORY => {
#[cfg(feature = "oom_panic")]
panic!("{description} failed: Out of memory");
return crate::DeviceError::OutOfMemory;
crate::DeviceError::OutOfMemory
}
Dxgi::DXGI_ERROR_DEVICE_RESET | Dxgi::DXGI_ERROR_DEVICE_REMOVED => {
#[cfg(feature = "device_lost_panic")]
panic!("{description} failed: Device lost ({err})");
crate::DeviceError::Lost
}
_ => {
#[cfg(feature = "internal_error_panic")]
panic!("{description} failed: {err}");
crate::DeviceError::Unexpected
}
}
crate::DeviceError::Lost
})
}
}

32
wgpu-hal/src/dx12/command.rs
View File

@ -53,7 +53,6 @@ impl crate::BufferTextureCopy {
impl super::Temp {
fn prepare_marker(&mut self, marker: &str) -> (&[u16], u32) {
// TODO: Store in HSTRING
self.marker.clear();
self.marker.extend(marker.encode_utf16());
self.marker.push(0);
@ -153,7 +152,7 @@ impl super::CommandEncoder {
self.update_root_elements();
}
//Note: we have to call this lazily before draw calls. Otherwise, D3D complains
// Note: we have to call this lazily before draw calls. Otherwise, D3D complains
// about the root parameters being incompatible with root signature.
fn update_root_elements(&mut self) {
use super::{BufferViewKind as Bvk, PassKind as Pk};
@ -265,7 +264,8 @@ impl crate::CommandEncoder for super::CommandEncoder {
unsafe fn begin_encoding(&mut self, label: crate::Label) -> Result<(), crate::DeviceError> {
let list = loop {
if let Some(list) = self.free_lists.pop() {
let reset_result = unsafe { list.Reset(&self.allocator, None) }.into_result();
// TODO: Is an error expected here and should we print it?
let reset_result = unsafe { list.Reset(&self.allocator, None) };
if reset_result.is_ok() {
break Some(list);
}
@ -314,7 +314,9 @@ impl crate::CommandEncoder for super::CommandEncoder {
for cmd_buf in command_buffers {
self.free_lists.push(cmd_buf.raw);
}
let _todo_handle_error = unsafe { self.allocator.Reset() };
if let Err(e) = unsafe { self.allocator.Reset() } {
log::error!("ID3D12CommandAllocator::Reset() failed with {e}");
}
}
unsafe fn transition_buffers<'a, T>(&mut self, barriers: T)
@ -724,8 +726,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
cat.clear_value.b as f32,
cat.clear_value.a as f32,
];
// TODO: Empty slice vs None?
unsafe { list.ClearRenderTargetView(*rtv, &value, Some(&[])) };
unsafe { list.ClearRenderTargetView(*rtv, &value, None) };
}
if let Some(ref target) = cat.resolve_target {
self.pass.resolves.push(super::PassResolve {
@ -754,12 +755,23 @@ impl crate::CommandEncoder for super::CommandEncoder {
if let Some(ds_view) = ds_view {
if flags != Direct3D12::D3D12_CLEAR_FLAGS::default() {
unsafe {
list.ClearDepthStencilView(
// list.ClearDepthStencilView(
// ds_view,
// flags,
// ds.clear_value.0,
// ds.clear_value.1 as u8,
// None,
// )
// TODO: Replace with the above in the next breaking windows-rs release,
// when https://github.com/microsoft/win32metadata/pull/1971 is in.
(windows_core::Interface::vtable(list).ClearDepthStencilView)(
windows_core::Interface::as_raw(list),
ds_view,
flags,
ds.clear_value.0,
ds.clear_value.1 as u8,
&[],
0,
std::ptr::null(),
)
}
}
@ -796,7 +808,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
Type: Direct3D12::D3D12_RESOURCE_BARRIER_TYPE_TRANSITION,
Flags: Direct3D12::D3D12_RESOURCE_BARRIER_FLAG_NONE,
Anonymous: Direct3D12::D3D12_RESOURCE_BARRIER_0 {
//Note: this assumes `D3D12_RESOURCE_STATE_RENDER_TARGET`.
// Note: this assumes `D3D12_RESOURCE_STATE_RENDER_TARGET`.
// If it's not the case, we can include the `TextureUses` in `PassResove`.
Transition: mem::ManuallyDrop::new(
Direct3D12::D3D12_RESOURCE_TRANSITION_BARRIER {
@ -813,7 +825,7 @@ impl crate::CommandEncoder for super::CommandEncoder {
Type: Direct3D12::D3D12_RESOURCE_BARRIER_TYPE_TRANSITION,
Flags: Direct3D12::D3D12_RESOURCE_BARRIER_FLAG_NONE,
Anonymous: Direct3D12::D3D12_RESOURCE_BARRIER_0 {
//Note: this assumes `D3D12_RESOURCE_STATE_RENDER_TARGET`.
// Note: this assumes `D3D12_RESOURCE_STATE_RENDER_TARGET`.
// If it's not the case, we can include the `TextureUses` in `PassResolve`.
Transition: mem::ManuallyDrop::new(
Direct3D12::D3D12_RESOURCE_TRANSITION_BARRIER {

22
wgpu-hal/src/dx12/descriptor.rs
View File

@ -56,16 +56,18 @@ impl GeneralHeap {
.into_device_result("Descriptor heap creation")?
};
let start = DualHandle {
cpu: unsafe { raw.GetCPUDescriptorHandleForHeapStart() },
gpu: unsafe { raw.GetGPUDescriptorHandleForHeapStart() },
count: 0,
};
Ok(Self {
raw: raw.clone(),
raw,
ty,
handle_size: unsafe { device.GetDescriptorHandleIncrementSize(ty) } as u64,
total_handles,
start: DualHandle {
cpu: unsafe { raw.GetCPUDescriptorHandleForHeapStart() },
gpu: unsafe { raw.GetGPUDescriptorHandleForHeapStart() },
count: 0,
},
start,
ranges: Mutex::new(RangeAllocator::new(0..total_handles)),
})
}
@ -268,12 +270,14 @@ impl CpuHeap {
let raw = unsafe { device.CreateDescriptorHeap::<Direct3D12::ID3D12DescriptorHeap>(&desc) }
.into_device_result("CPU descriptor heap creation")?;
let start = unsafe { raw.GetCPUDescriptorHandleForHeapStart() };
Ok(Self {
inner: Mutex::new(CpuHeapInner {
_raw: raw.clone(),
_raw: raw,
stage: Vec::new(),
}),
start: unsafe { raw.GetCPUDescriptorHandleForHeapStart() },
start,
handle_size,
total,
})
@ -297,7 +301,7 @@ impl fmt::Debug for CpuHeap {
}
pub(super) unsafe fn upload(
device: Direct3D12::ID3D12Device,
device: &Direct3D12::ID3D12Device,
src: &CpuHeapInner,
dst: &GeneralHeap,
dummy_copy_counts: &[u32],

4
wgpu-hal/src/dx12/device.rs
View File

@ -1295,7 +1295,7 @@ impl crate::Device for super::Device {
Some(inner) => {
let dual = unsafe {
descriptor::upload(
self.raw.clone(),
&self.raw,
&inner,
&self.shared.heap_views,
&desc.layout.copy_counts,
@ -1309,7 +1309,7 @@ impl crate::Device for super::Device {
Some(inner) => {
let dual = unsafe {
descriptor::upload(
self.raw.clone(),
&self.raw,
&inner,
&self.shared.heap_samplers,
&desc.layout.copy_counts,

9
wgpu-hal/src/dx12/instance.rs
View File

@ -10,10 +10,7 @@ use windows::{
};
use super::SurfaceTarget;
use crate::{
auxil::{self, dxgi::result::HResult as _},
dx12::D3D12Lib,
};
use crate::{auxil, dx12::D3D12Lib};
impl Drop for super::Instance {
fn drop(&mut self) {
@ -98,8 +95,8 @@ impl crate::Instance for super::Instance {
)
};
match hr.into_result() {
Err(err) => log::warn!("Unable to check for tearing support: {}", err),
match hr {
Err(err) => log::warn!("Unable to check for tearing support: {err}"),
Ok(()) => supports_allow_tearing = true,
}
}

30
wgpu-hal/src/dx12/mod.rs
View File

@ -49,14 +49,13 @@ use std::{ffi, fmt, mem, num::NonZeroU32, ops::Deref, sync::Arc};
use arrayvec::ArrayVec;
use parking_lot::{Mutex, RwLock};
use windows::{
core::{Interface, Param as _},
core::{Free, Interface, Param as _},
Win32::{
Foundation,
Graphics::{Direct3D, Direct3D12, DirectComposition, Dxgi},
System::Threading,
},
};
use windows_core::Free;
use crate::auxil::{
self,
@ -1026,8 +1025,8 @@ impl crate::Surface for Surface {
flags,
)
};
if let Err(err) = result.into_result() {
log::error!("ResizeBuffers failed: {}", err);
if let Err(err) = result {
log::error!("ResizeBuffers failed: {err}");
return Err(crate::SurfaceError::Other("window is in use"));
}
raw
@ -1092,24 +1091,22 @@ impl crate::Surface for Surface {
let swap_chain1 = swap_chain1.map_err(|err| {
log::error!("SwapChain creation error: {}", err);
crate::SurfaceError::Other("swap chain creation")
crate::SurfaceError::Other("swapchain creation")
})?;
match &self.target {
SurfaceTarget::WndHandle(_) | SurfaceTarget::SurfaceHandle(_) => {}
SurfaceTarget::Visual(visual) => {
if let Err(err) = unsafe { visual.SetContent(&swap_chain1) }.into_result() {
log::error!("Unable to SetContent: {}", err);
if let Err(err) = unsafe { visual.SetContent(&swap_chain1) } {
log::error!("Unable to SetContent: {err}");
return Err(crate::SurfaceError::Other(
"IDCompositionVisual::SetContent",
));
}
}
SurfaceTarget::SwapChainPanel(swap_chain_panel) => {
if let Err(err) =
unsafe { swap_chain_panel.SetSwapChain(&swap_chain1) }.into_result()
{
log::error!("Unable to SetSwapChain: {}", err);
if let Err(err) = unsafe { swap_chain_panel.SetSwapChain(&swap_chain1) } {
log::error!("Unable to SetSwapChain: {err}");
return Err(crate::SurfaceError::Other(
"ISwapChainPanelNative::SetSwapChain",
));
@ -1117,13 +1114,10 @@ impl crate::Surface for Surface {
}
}
match swap_chain1.cast::<Dxgi::IDXGISwapChain3>() {
Ok(swap_chain3) => swap_chain3,
Err(err) => {
log::error!("Unable to cast swap chain: {}", err);
return Err(crate::SurfaceError::Other("swap chain cast to 3"));
}
}
swap_chain1.cast::<Dxgi::IDXGISwapChain3>().map_err(|err| {
log::error!("Unable to cast swapchain: {err}");
crate::SurfaceError::Other("swapchain cast to version 3")
})?
}
};

4
wgpu-hal/src/dx12/shader_compilation.rs
View File

@ -4,8 +4,6 @@ use std::ptr;
pub(super) use dxc::{compile_dxc, get_dxc_container, DxcContainer};
use windows::Win32::Graphics::Direct3D;
use crate::auxil::dxgi::result::HResult;
// This exists so that users who don't want to use dxc can disable the dxc_shader_compiler feature
// and not have to compile hassle_rs.
// Currently this will use Dxc if it is chosen as the dx12 compiler at `Instance` creation time, and will
@ -57,7 +55,7 @@ pub(super) fn compile_fxc(
)
};
match hr.into_result() {
match hr {
Ok(()) => {
let shader_data = shader_data.unwrap();
(