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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Aug 30, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
34 changes: 5 additions & 29 deletions wgpu-hal/src/auxil/dxgi/result.rs
View file
Open in desktop
MarijnS95 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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
}
teoxoy marked this conversation as resolved.
Show resolved Hide resolved
}

crate::DeviceError::Lost
})
}
}
32 changes: 22 additions & 10 deletions wgpu-hal/src/dx12/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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);
}
teoxoy marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
)
}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
22 changes: 13 additions & 9 deletions wgpu-hal/src/dx12/descriptor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
})
}
Expand Down Expand Up @@ -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,
})
Expand All @@ -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],
Expand Down
4 changes: 2 additions & 2 deletions wgpu-hal/src/dx12/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
9 changes: 3 additions & 6 deletions wgpu-hal/src/dx12/instance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
}
}
Expand Down
30 changes: 12 additions & 18 deletions wgpu-hal/src/dx12/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1092,38 +1091,33 @@ 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",
));
}
}
}

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")
})?
}
};

Expand Down
4 changes: 1 addition & 3 deletions wgpu-hal/src/dx12/shader_compilation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -57,7 +55,7 @@ pub(super) fn compile_fxc(
)
};

match hr.into_result() {
match hr {
Ok(()) => {
let shader_data = shader_data.unwrap();
(
Expand Down
Loading