Skip to content
This repository has been archived by the owner on Oct 26, 2023. It is now read-only.

Fix Github Actions #50

Merged
merged 6 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
- name: Set up Rust toolchain
id: setup-rust
run: |
rustup toolchain install ${{ env.MSRV }} -c rustfmt,clippy -p minimal
rustup toolchain install ${{ env.MSRV }} -c rustfmt,clippy --profile minimal

- name: caching
uses: Swatinem/rust-cache@v2
Expand All @@ -39,4 +39,4 @@ jobs:

- name: doc
run: |
cargo +${{ env.MSRV }} doc --all-features
cargo +${{ env.MSRV }} doc --all-features --no-deps
5 changes: 0 additions & 5 deletions bors.toml

This file was deleted.

74 changes: 62 additions & 12 deletions src/com.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,90 @@ use winapi::{ctypes::c_void, um::unknwnbase::IUnknown, Interface};
pub struct ComPtr<T: Interface>(*mut T);

impl<T: Interface> ComPtr<T> {
/// Creates a null ComPtr.
pub fn null() -> Self {
ComPtr(ptr::null_mut())
}

/// Create a ComPtr from a raw pointer. This will call AddRef on the pointer.
///
/// # Safety
///
/// - if `raw` is not null, it must be a valid pointer to a COM object that implements T.
pub unsafe fn from_raw(raw: *mut T) -> Self {
if !raw.is_null() {
(&*(raw as *mut IUnknown)).AddRef();
(*(raw as *mut IUnknown)).AddRef();
}
ComPtr(raw)
}

/// Returns true if the inner pointer is null.
pub fn is_null(&self) -> bool {
self.0.is_null()
}

/// Returns the raw inner pointer. May be null.
pub fn as_ptr(&self) -> *const T {
self.0
}

/// Returns the raw inner pointer as mutable. May be null.
pub fn as_mut_ptr(&self) -> *mut T {
self.0
}

pub fn mut_void(&mut self) -> *mut *mut c_void {
&mut self.0 as *mut *mut _ as *mut *mut _
/// Returns a mutable reference to the inner pointer casted as a pointer to c_void.
///
/// This is useful when D3D functions initialize objects by filling in a pointer to pointer
/// by taking `void**` as an argument.
///
/// # Safety
///
/// - Any modifications done to this pointer must result in the pointer either:
/// - being set to null
/// - being set to a valid pointer to a COM object that implements T
pub unsafe fn mut_void(&mut self) -> &mut *mut c_void {
// SAFETY: We must first get a reference pointing to our internal pointer
// and only then cast it. As if we cast it, then take a reference, we would
// end up with a reference to a temporary.
let refer: &mut *mut T = &mut self.0;
let void: *mut *mut c_void = refer.cast();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In retrospect, this was incorrect, and sets void to self.0 as *mut *mut c_void instead of &mut self.0 as *mut *mut T as *mut *mut c_void, because .cast applies to the primitive pointer *mut T by reference.

Copy link
Member

@ErichDonGubler ErichDonGubler Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the problem that @kdashg points out, there's another issue here: the &mut *mut void extracted here is actually a reference to a *mut c_void temporary in this method, whose lifetime is incorrectly inferred through the pointer deref. (because rustc trusts us to get the lifetime right). This becomes insta-UB; a reference to a binding in a popped stack is no bueno. 😓

I don't think it makes sense to use mutable references here, TBH. I'll fix this up in gfx-rs/wgpu#3987, which vendors in this crate (and will likely result in this repository being archived, for the time being). EDIT: Change of plan, gonna "just" vendor in 0.7.0, which doesn't have this breakage.

CC @jrmuizel; I think he'd want to know about these issues.


// SAFETY: This reference is valid for the duration of the borrow due our mutable borrow of self.
&mut *void
}

pub fn mut_self(&mut self) -> *mut *mut T {
&mut self.0 as *mut *mut _
/// Returns a mutable reference to the inner pointer.
///
/// This is useful when D3D functions initialize objects by filling in a pointer to pointer
/// by taking `T**` as an argument.
///
/// # Safety
///
/// - Any modifications done to this pointer must result in the pointer either:
/// - being set to null
/// - being set to a valid pointer to a COM object that implements T
pub fn mut_self(&mut self) -> &mut *mut T {
&mut self.0
}
}

impl<T: Interface> ComPtr<T> {
/// Returns a reference to the inner pointer casted as a pointer IUnknown.
///
/// # Safety
///
/// - This pointer must not be null.
pub unsafe fn as_unknown(&self) -> &IUnknown {
debug_assert!(!self.is_null());
&*(self.0 as *mut IUnknown)
}

/// Casts the T to U using QueryInterface.
///
/// # Safety
///
/// - This pointer must not be null.
pub unsafe fn cast<U>(&self) -> D3DResult<ComPtr<U>>
where
U: Interface,
Expand Down Expand Up @@ -86,7 +132,7 @@ impl<T: Interface> Drop for ComPtr<T> {
impl<T: Interface> Deref for ComPtr<T> {
type Target = T;
fn deref(&self) -> &T {
debug_assert!(!self.is_null());
assert!(!self.is_null());
unsafe { &*self.0 }
}
}
Expand Down Expand Up @@ -126,8 +172,8 @@ impl<T: Interface> Hash for ComPtr<T> {
///
/// ```rust
/// # pub use d3d12::weak_com_inheritance_chain;
/// # mod actual {
/// # pub struct ComObject; impl winapi::Interface for ComObject { fn uuidof() -> winapi::shared::guiddef::GUID { todo!() } }
/// # mod actual {
/// # pub struct ComObject; impl winapi::Interface for ComObject { fn uuidof() -> winapi::shared::guiddef::GUID { todo!() } }
/// # pub struct ComObject1; impl winapi::Interface for ComObject1 { fn uuidof() -> winapi::shared::guiddef::GUID { todo!() } }
/// # pub struct ComObject2; impl winapi::Interface for ComObject2 { fn uuidof() -> winapi::shared::guiddef::GUID { todo!() } }
/// # }
Expand Down Expand Up @@ -223,12 +269,16 @@ macro_rules! weak_com_inheritance_chain {
$variant:ident($type:ty);
$($next_variant:ident),*;
) => {
// Construct this enum from weak pointer to this interface. For best usability, always use the highest constructor you can. This doesn't try to upcast.
#[doc = concat!("Constructs this enum from a ComPtr to ", stringify!($variant), ". For best usability, always use the highest constructor you can. This doesn't try to upcast.")]
///
/// # Safety
///
#[doc = concat!(" - The value must be a valid pointer to a COM object that implements ", stringify!($variant))]
$vis unsafe fn $from_name(value: $crate::ComPtr<$type>) -> Self {
Self::$variant(value)
}

// Returns Some if the value implements the interface otherwise returns None.
#[doc = concat!("Returns Some if the value implements ", stringify!($variant), ".")]
$vis fn $as_name(&self) -> Option<&$crate::ComPtr<$type>> {
match *self {
$(
Expand All @@ -237,14 +287,14 @@ macro_rules! weak_com_inheritance_chain {
Self::$variant(ref v) => Some(v),
$(
Self::$next_variant(ref v) => {
// v is &ComPtr<NextType> and se cast to &ComPtr<Type>
// v is &ComPtr<NextType> and we cast to &ComPtr<Type>
Some(unsafe { std::mem::transmute(v) })
}
)*
}
}

// Returns the interface if the value implements it, otherwise panics.
#[doc = concat!("Returns a ", stringify!($variant), " if the value implements it, otherwise panics.")]
#[track_caller]
$vis fn $unwrap_name(&self) -> &$crate::ComPtr<$type> {
match *self {
Expand Down
4 changes: 2 additions & 2 deletions src/command_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum CmdListType {
// VideoProcess = d3d12::D3D12_COMMAND_LIST_TYPE_VIDEO_PROCESS,
}

bitflags! {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct ClearFlags: u32 {
const DEPTH = d3d12::D3D12_CLEAR_FLAG_DEPTH;
Expand Down Expand Up @@ -263,7 +263,7 @@ impl GraphicsCommandList {
}
}

pub fn set_pipeline_state(&self, pso:&PipelineState) {
pub fn set_pipeline_state(&self, pso: &PipelineState) {
unsafe {
self.SetPipelineState(pso.as_mut_ptr());
}
Expand Down
15 changes: 5 additions & 10 deletions src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub enum DescriptorHeapType {
Dsv = d3d12::D3D12_DESCRIPTOR_HEAP_TYPE_DSV,
}

bitflags! {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct DescriptorHeapFlags: u32 {
const SHADER_VISIBLE = d3d12::D3D12_DESCRIPTOR_HEAP_FLAG_SHADER_VISIBLE;
Expand Down Expand Up @@ -253,7 +253,7 @@ pub enum RootSignatureVersion {
V1_1 = d3d12::D3D_ROOT_SIGNATURE_VERSION_1_1,
}

bitflags! {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct RootSignatureFlags: u32 {
const ALLOW_IA_INPUT_LAYOUT = d3d12::D3D12_ROOT_SIGNATURE_FLAG_ALLOW_INPUT_ASSEMBLER_INPUT_LAYOUT;
Expand Down Expand Up @@ -297,12 +297,7 @@ impl crate::D3D12Lib {
let mut error = Error::null();
let hr = unsafe {
let func: libloading::Symbol<Fun> = self.lib.get(b"D3D12SerializeRootSignature")?;
func(
&desc,
version as _,
blob.mut_void() as *mut *mut _,
error.mut_void() as *mut *mut _,
)
func(&desc, version as _, blob.mut_self(), error.mut_self())
};

Ok(((blob, error), hr))
Expand Down Expand Up @@ -332,8 +327,8 @@ impl RootSignature {
d3d12::D3D12SerializeRootSignature(
&desc,
version as _,
blob.mut_void() as *mut *mut _,
error.mut_void() as *mut *mut _,
blob.mut_self(),
error.mut_self(),
)
};

Expand Down
2 changes: 1 addition & 1 deletion src/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub type Device = ComPtr<d3d12::ID3D12Device>;

#[cfg(feature = "libloading")]
impl crate::D3D12Lib {
pub fn create_device<I: Interface>(
pub fn create_device<I: Interface>(
&self,
adapter: &ComPtr<I>,
feature_level: crate::FeatureLevel,
Expand Down
35 changes: 23 additions & 12 deletions src/dxgi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use winapi::{
Interface,
};

bitflags! {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct FactoryCreationFlags: u32 {
const DEBUG = dxgi1_3::DXGI_CREATE_FACTORY_DEBUG;
Expand Down Expand Up @@ -211,7 +211,10 @@ impl SwapchainDesc {
}

impl Factory1 {
pub fn create_swapchain(
/// # Safety
///
/// - `queue` must be a valid pointer to a D3D device.
pub unsafe fn create_swapchain(
&self,
queue: *mut IUnknown,
hwnd: HWND,
Expand Down Expand Up @@ -242,16 +245,18 @@ impl Factory1 {
};

let mut swapchain = SwapChain::null();
let hr =
unsafe { self.CreateSwapChain(queue, &mut desc, swapchain.mut_void() as *mut *mut _) };
let hr = unsafe { self.CreateSwapChain(queue, &mut desc, swapchain.mut_self()) };

(swapchain, hr)
}
}

impl Factory2 {
// TODO: interface not complete
pub fn create_swapchain_for_hwnd(
/// # Safety
///
/// - `queue` must be a valid pointer to a D3D device.
pub unsafe fn create_swapchain_for_hwnd(
&self,
queue: *mut IUnknown,
hwnd: HWND,
Expand All @@ -265,14 +270,17 @@ impl Factory2 {
&desc.to_desc1(),
ptr::null(),
ptr::null_mut(),
swap_chain.mut_void() as *mut *mut _,
swap_chain.mut_self(),
)
};

(swap_chain, hr)
}

pub fn create_swapchain_for_composition(
/// # Safety
///
/// - `queue` must be a valid pointer to a D3D device.
pub unsafe fn create_swapchain_for_composition(
&self,
queue: *mut IUnknown,
desc: &SwapchainDesc,
Expand All @@ -283,7 +291,7 @@ impl Factory2 {
queue,
&desc.to_desc1(),
ptr::null_mut(),
swap_chain.mut_void() as *mut *mut _,
swap_chain.mut_self(),
)
};

Expand All @@ -308,14 +316,17 @@ impl Factory4 {

pub fn enumerate_adapters(&self, id: u32) -> D3DResult<Adapter1> {
let mut adapter = Adapter1::null();
let hr = unsafe { self.EnumAdapters1(id, adapter.mut_void() as *mut *mut _) };
let hr = unsafe { self.EnumAdapters1(id, adapter.mut_self()) };

(adapter, hr)
}
}

impl FactoryMedia {
pub fn create_swapchain_for_composition_surface_handle(
/// # Safety
///
/// - `queue` must be a valid pointer to a D3D device.
pub unsafe fn create_swapchain_for_composition_surface_handle(
&self,
queue: *mut IUnknown,
surface_handle: HANDLE,
Expand All @@ -328,15 +339,15 @@ impl FactoryMedia {
surface_handle,
&desc.to_desc1(),
ptr::null_mut(),
swap_chain.mut_void() as *mut *mut _,
swap_chain.mut_self(),
)
};

(swap_chain, hr)
}
}

bitflags! {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct SwapChainPresentFlags: u32 {
const DXGI_PRESENT_DO_NOT_SEQUENCE = dxgi::DXGI_PRESENT_DO_NOT_SEQUENCE;
Expand Down
2 changes: 1 addition & 1 deletion src/heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub enum MemoryPool {
L1 = d3d12::D3D12_MEMORY_POOL_L1,
}

bitflags! {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct HeapFlags: u32 {
const NONE = d3d12::D3D12_HEAP_FLAG_NONE;
Expand Down
13 changes: 7 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#[macro_use]
extern crate bitflags;
#![allow(clippy::too_many_arguments)]

use std::{convert::TryFrom, ffi::CStr};
use winapi::{
Expand Down Expand Up @@ -100,10 +99,12 @@ pub type Blob = ComPtr<d3dcommon::ID3DBlob>;

pub type Error = ComPtr<d3dcommon::ID3DBlob>;
impl Error {
pub unsafe fn as_c_str(&self) -> &CStr {
debug_assert!(!self.is_null());
let data = self.GetBufferPointer();
CStr::from_ptr(data as *const _ as *const _)
pub fn as_c_str(&self) -> &CStr {
assert!(!self.is_null());
unsafe {
let data = self.GetBufferPointer();
CStr::from_ptr(data as *const _ as *const _)
}
}
}

Expand Down
Loading