From c91b6b8b637db2fa21a5a29a681a5df04d6d88da Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Tue, 10 Dec 2024 14:53:27 +0100 Subject: [PATCH] Remove stale and UB `winapi` compatibility layer With the `windows` migration we introduced a `winapi` compatibility layer to make it easier to use `winapi` types with `gpu-allocator`. As most of the ecosystem has switched to `windows` it is now time to sunset these helpers which are otherwise tedious to test and maintain, not to mention contain long(er) standing unsoundness bugs. --- .github/workflows/ci.yml | 4 +- Cargo.toml | 13 -- README.md | 7 - examples/d3d12-buffer.rs | 283 --------------------------------------- src/d3d12/mod.rs | 129 +----------------- src/lib.rs | 7 - 6 files changed, 9 insertions(+), 434 deletions(-) delete mode 100644 examples/d3d12-buffer.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 580075dc..5f4f486c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,7 @@ jobs: - os: ubuntu-latest features: vulkan,visualizer - os: windows-latest - features: vulkan,visualizer,d3d12,public-winapi + features: vulkan,visualizer,d3d12 - os: macos-latest features: vulkan,visualizer,metal runs-on: ${{ matrix.os }} @@ -59,7 +59,7 @@ jobs: - os: ubuntu-latest features: vulkan,visualizer - os: windows-latest - features: vulkan,visualizer,d3d12,public-winapi + features: vulkan,visualizer,d3d12 - os: macos-latest features: vulkan,visualizer,metal runs-on: ${{ matrix.os }} diff --git a/Cargo.toml b/Cargo.toml index f3728678..047b0c5f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,10 +46,6 @@ objc2-metal = { version = "0.2.1", default-features = false, features = [ "std", ], optional = true } -[target.'cfg(windows)'.dependencies] -# Only needed for public-winapi interop helpers -winapi = { version = "0.3.9", features = ["d3d12", "winerror", "impl-default", "impl-debug"], optional = true } - [target.'cfg(windows)'.dependencies.windows] version = ">=0.53,<=0.59" features = [ @@ -63,9 +59,6 @@ optional = true ash = { version = "0.38", default-features = false, features = ["debug", "loaded"] } env_logger = "0.10" -[target.'cfg(windows)'.dev-dependencies] -winapi = { version = "0.3.9", features = ["d3d12", "d3d12sdklayers", "dxgi1_6", "winerror", "impl-default", "impl-debug", "winuser", "windowsx", "libloaderapi"] } - [target.'cfg(windows)'.dev-dependencies.windows] # API-breaks since Windows 0.58 only affect our examples version = ">=0.58,<=0.59" @@ -84,10 +77,6 @@ objc2-metal = { version = "0.2.1", default-features = false, features = [ name = "vulkan-buffer" required-features = ["vulkan", "ash/loaded"] -[[example]] -name = "d3d12-buffer" -required-features = ["d3d12", "public-winapi"] - [[example]] name = "d3d12-buffer-winrs" required-features = ["d3d12"] @@ -101,7 +90,5 @@ visualizer = ["dep:egui", "dep:egui_extras"] vulkan = ["dep:ash"] d3d12 = ["dep:windows"] metal = ["dep:objc2", "dep:objc2-metal", "dep:objc2-foundation"] -# Expose helper functionality for winapi types to interface with gpu-allocator, which is primarily windows-rs driven -public-winapi = ["dep:winapi"] default = ["d3d12", "vulkan", "metal"] diff --git a/README.md b/README.md index 8b4877fc..a12e64b1 100644 --- a/README.md +++ b/README.md @@ -19,13 +19,6 @@ gpu-allocator = "0.27.0" This crate provides a fully written in Rust memory allocator for Vulkan, DirectX 12 and Metal. -## [Windows-rs] and [winapi] - -`gpu-allocator` recently migrated from [winapi] to [windows-rs] but still provides convenient helpers to convert to and from [winapi] types, enabled when compiling with the `public-winapi` crate feature. - -[Windows-rs]: https://github.com/microsoft/windows-rs -[winapi]: https://github.com/retep998/winapi-rs - ## Setting up the Vulkan memory allocator ```rust diff --git a/examples/d3d12-buffer.rs b/examples/d3d12-buffer.rs deleted file mode 100644 index a9578bcd..00000000 --- a/examples/d3d12-buffer.rs +++ /dev/null @@ -1,283 +0,0 @@ -//! Example showcasing [`winapi`] interop with [`gpu-allocator`] which is driven by the [`windows`] crate. -use winapi::{ - shared::{dxgiformat, winerror}, - um::{d3d12, d3dcommon}, - Interface, -}; - -mod all_dxgi { - pub use winapi::shared::{dxgi1_3::*, dxgi1_6::*, dxgitype::*}; -} - -use gpu_allocator::{ - d3d12::{ - AllocationCreateDesc, Allocator, AllocatorCreateDesc, ID3D12DeviceVersion, - ResourceCategory, ToWinapi, ToWindows, - }, - MemoryLocation, -}; -use log::*; - -fn create_d3d12_device( - dxgi_factory: *mut all_dxgi::IDXGIFactory6, -) -> Option<*mut d3d12::ID3D12Device> { - for idx in 0.. { - let mut adapter4: *mut all_dxgi::IDXGIAdapter4 = std::ptr::null_mut(); - let hr = unsafe { - dxgi_factory.as_ref().unwrap().EnumAdapters1( - idx, - <*mut *mut all_dxgi::IDXGIAdapter4>::cast(&mut adapter4), - ) - }; - - if hr == winerror::DXGI_ERROR_NOT_FOUND { - break; - } - - assert_eq!(hr, winerror::S_OK); - - let mut desc = all_dxgi::DXGI_ADAPTER_DESC3::default(); - let hr = unsafe { adapter4.as_ref().unwrap().GetDesc3(&mut desc) }; - if hr != winerror::S_OK { - error!("Failed to get adapter description for adapter"); - continue; - } - - // Skip software adapters - if (desc.Flags & all_dxgi::DXGI_ADAPTER_FLAG3_SOFTWARE) - == all_dxgi::DXGI_ADAPTER_FLAG3_SOFTWARE - { - continue; - } - - let feature_levels = [ - (d3dcommon::D3D_FEATURE_LEVEL_11_0, "D3D_FEATURE_LEVEL_11_0"), - (d3dcommon::D3D_FEATURE_LEVEL_11_1, "D3D_FEATURE_LEVEL_11_1"), - (d3dcommon::D3D_FEATURE_LEVEL_12_0, "D3D_FEATURE_LEVEL_12_0"), - ]; - - let device = - feature_levels - .iter() - .rev() - .find_map(|&(feature_level, feature_level_name)| { - let mut device: *mut d3d12::ID3D12Device = std::ptr::null_mut(); - let hr = unsafe { - d3d12::D3D12CreateDevice( - adapter4.cast(), - feature_level, - &d3d12::ID3D12Device::uuidof(), - <*mut *mut d3d12::ID3D12Device>::cast(&mut device), - ) - }; - match hr { - winerror::S_OK => { - info!("Using D3D12 feature level: {}.", feature_level_name); - Some(device) - } - winerror::E_NOINTERFACE => { - error!("ID3D12Device interface not supported."); - None - } - _ => { - info!( - "D3D12 feature level: {} not supported: {:x}", - feature_level_name, hr - ); - None - } - } - }); - if device.is_some() { - return device; - } - } - - None -} - -fn main() { - env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("trace")).init(); - - let dxgi_factory = { - let mut dxgi_factory: *mut all_dxgi::IDXGIFactory6 = std::ptr::null_mut(); - let hr = unsafe { - all_dxgi::CreateDXGIFactory2( - 0, - &all_dxgi::IID_IDXGIFactory6, - <*mut *mut all_dxgi::IDXGIFactory6>::cast(&mut dxgi_factory), - ) - }; - - assert_eq!(hr, winerror::S_OK, "Failed to create DXGI factory"); - dxgi_factory - }; - - let device = create_d3d12_device(dxgi_factory).expect("Failed to create D3D12 device."); - - // Setting up the allocator - let mut allocator = Allocator::new(&AllocatorCreateDesc { - device: ID3D12DeviceVersion::Device(device.as_windows().clone()), - debug_settings: Default::default(), - allocation_sizes: Default::default(), - }) - .unwrap(); - - let device = unsafe { device.as_ref() }.unwrap(); - - // Test allocating Gpu Only memory - { - let test_buffer_desc = d3d12::D3D12_RESOURCE_DESC { - Dimension: d3d12::D3D12_RESOURCE_DIMENSION_BUFFER, - Alignment: d3d12::D3D12_DEFAULT_RESOURCE_PLACEMENT_ALIGNMENT as u64, - Width: 512, - Height: 1, - DepthOrArraySize: 1, - MipLevels: 1, - Format: dxgiformat::DXGI_FORMAT_UNKNOWN, - SampleDesc: all_dxgi::DXGI_SAMPLE_DESC { - Count: 1, - Quality: 0, - }, - Layout: d3d12::D3D12_TEXTURE_LAYOUT_ROW_MAJOR, - Flags: d3d12::D3D12_RESOURCE_FLAG_NONE, - }; - - let allocation_desc = AllocationCreateDesc::from_winapi_d3d12_resource_desc( - device, - &test_buffer_desc, - "Test allocation (Gpu Only)", - MemoryLocation::GpuOnly, - ); - let allocation = allocator.allocate(&allocation_desc).unwrap(); - - let mut resource: *mut d3d12::ID3D12Resource = std::ptr::null_mut(); - let hr = unsafe { - device.CreatePlacedResource( - allocation.heap().as_winapi() as *mut _, - allocation.offset(), - &test_buffer_desc, - d3d12::D3D12_RESOURCE_STATE_COMMON, - std::ptr::null(), - &d3d12::IID_ID3D12Resource, - <*mut *mut d3d12::ID3D12Resource>::cast(&mut resource), - ) - }; - if hr != winerror::S_OK { - panic!("Failed to create placed resource."); - } - - unsafe { resource.as_ref().unwrap().Release() }; - - allocator.free(allocation).unwrap(); - info!("Allocation and deallocation of GpuOnly memory was successful."); - } - - // Test allocating Cpu to Gpu memory - { - let test_buffer_desc = d3d12::D3D12_RESOURCE_DESC { - Dimension: d3d12::D3D12_RESOURCE_DIMENSION_BUFFER, - Alignment: d3d12::D3D12_DEFAULT_RESOURCE_PLACEMENT_ALIGNMENT as u64, - Width: 512, - Height: 1, - DepthOrArraySize: 1, - MipLevels: 1, - Format: dxgiformat::DXGI_FORMAT_UNKNOWN, - SampleDesc: all_dxgi::DXGI_SAMPLE_DESC { - Count: 1, - Quality: 0, - }, - Layout: d3d12::D3D12_TEXTURE_LAYOUT_ROW_MAJOR, - Flags: d3d12::D3D12_RESOURCE_FLAG_NONE, - }; - - let alloc_info = unsafe { device.GetResourceAllocationInfo(0, 1, &test_buffer_desc) }; - - let allocation = allocator - .allocate(&AllocationCreateDesc { - name: "Test allocation (Cpu to Gpu)", - location: MemoryLocation::CpuToGpu, - size: alloc_info.SizeInBytes, - alignment: alloc_info.Alignment, - resource_category: ResourceCategory::Buffer, - }) - .unwrap(); - - let mut resource: *mut d3d12::ID3D12Resource = std::ptr::null_mut(); - let hr = unsafe { - device.CreatePlacedResource( - allocation.heap().as_winapi() as *mut _, - allocation.offset(), - &test_buffer_desc, - d3d12::D3D12_RESOURCE_STATE_COMMON, - std::ptr::null(), - &d3d12::IID_ID3D12Resource, - <*mut *mut d3d12::ID3D12Resource>::cast(&mut resource), - ) - }; - if hr != winerror::S_OK { - panic!("Failed to create placed resource."); - } - - unsafe { resource.as_ref().unwrap().Release() }; - - allocator.free(allocation).unwrap(); - info!("Allocation and deallocation of CpuToGpu memory was successful."); - } - - // Test allocating Gpu to Cpu memory - { - let test_buffer_desc = d3d12::D3D12_RESOURCE_DESC { - Dimension: d3d12::D3D12_RESOURCE_DIMENSION_BUFFER, - Alignment: d3d12::D3D12_DEFAULT_RESOURCE_PLACEMENT_ALIGNMENT as u64, - Width: 512, - Height: 1, - DepthOrArraySize: 1, - MipLevels: 1, - Format: dxgiformat::DXGI_FORMAT_UNKNOWN, - SampleDesc: all_dxgi::DXGI_SAMPLE_DESC { - Count: 1, - Quality: 0, - }, - Layout: d3d12::D3D12_TEXTURE_LAYOUT_ROW_MAJOR, - Flags: d3d12::D3D12_RESOURCE_FLAG_NONE, - }; - - let alloc_info = unsafe { device.GetResourceAllocationInfo(0, 1, &test_buffer_desc) }; - - let allocation = allocator - .allocate(&AllocationCreateDesc { - name: "Test allocation (Gpu to Cpu)", - location: MemoryLocation::GpuToCpu, - size: alloc_info.SizeInBytes, - alignment: alloc_info.Alignment, - resource_category: ResourceCategory::Buffer, - }) - .unwrap(); - - let mut resource: *mut d3d12::ID3D12Resource = std::ptr::null_mut(); - let hr = unsafe { - device.CreatePlacedResource( - allocation.heap().as_winapi() as *mut _, - allocation.offset(), - &test_buffer_desc, - d3d12::D3D12_RESOURCE_STATE_COMMON, - std::ptr::null(), - &d3d12::IID_ID3D12Resource, - <*mut *mut d3d12::ID3D12Resource>::cast(&mut resource), - ) - }; - if hr != winerror::S_OK { - panic!("Failed to create placed resource."); - } - - unsafe { resource.as_ref().unwrap().Release() }; - - allocator.free(allocation).unwrap(); - info!("Allocation and deallocation of CpuToGpu memory was successful."); - } - - drop(allocator); // Explicitly drop before destruction of device. - unsafe { device.Release() }; - unsafe { dxgi_factory.as_ref().unwrap().Release() }; -} diff --git a/src/d3d12/mod.rs b/src/d3d12/mod.rs index cb9b29bf..d88688bf 100644 --- a/src/d3d12/mod.rs +++ b/src/d3d12/mod.rs @@ -12,77 +12,6 @@ use windows::Win32::{ Graphics::{Direct3D12::*, Dxgi::Common::DXGI_FORMAT}, }; -#[cfg(feature = "public-winapi")] -mod public_winapi { - pub use winapi::um::d3d12 as winapi_d3d12; - - use super::*; - - /// Trait similar to [`AsRef`]/[`AsMut`], - pub trait ToWinapi { - fn as_winapi(&self) -> *const T; - fn as_winapi_mut(&mut self) -> *mut T; - } - - /// [`windows`] types hold their pointer internally and provide drop semantics. As such this trait - /// is usually implemented on the _pointer type_ (`*const`, `*mut`) of the [`winapi`] object so that - /// a **borrow of** that pointer becomes a borrow of the [`windows`] type. - pub trait ToWindows { - fn as_windows(&self) -> &T; - } - - impl ToWinapi for ID3D12Resource { - fn as_winapi(&self) -> *const winapi_d3d12::ID3D12Resource { - unsafe { std::mem::transmute_copy(self) } - } - - fn as_winapi_mut(&mut self) -> *mut winapi_d3d12::ID3D12Resource { - unsafe { std::mem::transmute_copy(self) } - } - } - - impl ToWinapi for ID3D12Device { - fn as_winapi(&self) -> *const winapi_d3d12::ID3D12Device { - unsafe { std::mem::transmute_copy(self) } - } - - fn as_winapi_mut(&mut self) -> *mut winapi_d3d12::ID3D12Device { - unsafe { std::mem::transmute_copy(self) } - } - } - - impl ToWindows for *const winapi_d3d12::ID3D12Device { - fn as_windows(&self) -> &ID3D12Device { - unsafe { std::mem::transmute(self) } - } - } - - impl ToWindows for *mut winapi_d3d12::ID3D12Device { - fn as_windows(&self) -> &ID3D12Device { - unsafe { std::mem::transmute(self) } - } - } - - impl ToWindows for &mut winapi_d3d12::ID3D12Device { - fn as_windows(&self) -> &ID3D12Device { - unsafe { std::mem::transmute(self) } - } - } - - impl ToWinapi for ID3D12Heap { - fn as_winapi(&self) -> *const winapi_d3d12::ID3D12Heap { - unsafe { std::mem::transmute_copy(self) } - } - - fn as_winapi_mut(&mut self) -> *mut winapi_d3d12::ID3D12Heap { - unsafe { std::mem::transmute_copy(self) } - } - } -} - -#[cfg(feature = "public-winapi")] -pub use public_winapi::*; - #[cfg(feature = "visualizer")] mod visualizer; #[cfg(feature = "visualizer")] @@ -154,23 +83,6 @@ impl From<&D3D12_RESOURCE_DESC> for ResourceCategory { } } -#[cfg(feature = "public-winapi")] -impl From<&winapi_d3d12::D3D12_RESOURCE_DESC> for ResourceCategory { - fn from(desc: &winapi_d3d12::D3D12_RESOURCE_DESC) -> Self { - if desc.Dimension == winapi_d3d12::D3D12_RESOURCE_DIMENSION_BUFFER { - Self::Buffer - } else if (desc.Flags - & (winapi_d3d12::D3D12_RESOURCE_FLAG_ALLOW_RENDER_TARGET - | winapi_d3d12::D3D12_RESOURCE_FLAG_ALLOW_DEPTH_STENCIL)) - != 0 - { - Self::RtvDsvTexture - } else { - Self::OtherTexture - } - } -} - #[derive(Clone, Debug)] pub struct AllocationCreateDesc<'a> { /// Name of the allocation, for tracking and debugging purposes @@ -183,51 +95,24 @@ pub struct AllocationCreateDesc<'a> { /// Alignment of allocation, should be queried using [`ID3D12Device::GetResourceAllocationInfo()`] pub alignment: u64, /// Resource category based on resource dimension and flags. Can be created from a [`D3D12_RESOURCE_DESC`] - /// using the helper into function. The resource category is ignored when Resource Heap Tier 2 or higher + /// using the [helper `into()` function]. The resource category is ignored when Resource Heap Tier 2 or higher /// is supported. + /// + /// [helper `into()` function]: ResourceCategory::from() pub resource_category: ResourceCategory, } impl<'a> AllocationCreateDesc<'a> { - /// Helper conversion function utilizing [`winapi`] types. - /// - /// This function is also available for [`windows::Win32::Graphics::Direct3D12`] - /// types as [`from_d3d12_resource_desc()`][Self::from_d3d12_resource_desc()]. - #[cfg(feature = "public-winapi")] - pub fn from_winapi_d3d12_resource_desc( - device: *const winapi_d3d12::ID3D12Device, - desc: &winapi_d3d12::D3D12_RESOURCE_DESC, - name: &'a str, - location: MemoryLocation, - ) -> Self { - let device = device.as_windows(); - // Raw structs are binary-compatible - let desc = unsafe { - std::mem::transmute::<&winapi_d3d12::D3D12_RESOURCE_DESC, &D3D12_RESOURCE_DESC>(desc) - }; - let allocation_info = - unsafe { device.GetResourceAllocationInfo(0, std::slice::from_ref(desc)) }; - let resource_category: ResourceCategory = desc.into(); - - AllocationCreateDesc { - name, - location, - size: allocation_info.SizeInBytes, - alignment: allocation_info.Alignment, - resource_category, - } - } - - /// Helper conversion function utilizing [`windows::Win32::Graphics::Direct3D12`] types. - /// - /// This function is also available for `winapi` types as `from_winapi_d3d12_resource_desc()` - /// when the `public-winapi` feature is enabled. + /// Helper function to construct an [`AllocationCreateDesc`] from an existing + /// [`D3D12_RESOURCE_DESC`] utilizing [`ID3D12Device::GetResourceAllocationInfo()`]. pub fn from_d3d12_resource_desc( device: &ID3D12Device, desc: &D3D12_RESOURCE_DESC, name: &'a str, location: MemoryLocation, ) -> Self { + // SAFETY: `device` is a valid device handle, and no arguments (like pointers) are passed + // that could induce UB. let allocation_info = unsafe { device.GetResourceAllocationInfo(0, std::slice::from_ref(desc)) }; let resource_category: ResourceCategory = desc.into(); diff --git a/src/lib.rs b/src/lib.rs index c0fd6dac..a4819138 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,12 +1,5 @@ //! This crate provides a fully written in Rust memory allocator for Vulkan, DirectX 12 and Metal. //! -//! # [Windows-rs] and [winapi] -//! -//! `gpu-allocator` recently migrated from [winapi] to [windows-rs] but still provides convenient helpers to convert to and from [winapi] types, enabled when compiling with the `public-winapi` crate feature. -//! -//! [Windows-rs]: https://github.com/microsoft/windows-rs -//! [winapi]: https://github.com/retep998/winapi-rs -//! //! # Setting up the Vulkan memory allocator //! //! ```no_run