Merge pull request #98 from linebender/dx12

More work on DX12 backend

piet-gpu-hal/examples/dx12_toy.rs

@@ -73,7 +73,7 @@ fn toy() -> Result<(), Error> {
         let pipeline = device.create_simple_compute_pipeline(SHADER_CODE, 1, 1)?;
         let ds = device.create_descriptor_set(&pipeline, &[&dev_buf], &[&img])?;
         let mut cmd_buf = device.create_cmd_buf()?;
-        let fence = device.create_fence(false)?;
+        let mut fence = device.create_fence(false)?;
         cmd_buf.begin();
         cmd_buf.copy_buffer(&buf, &dev_buf);
         cmd_buf.memory_barrier();
@@ -86,8 +86,8 @@ fn toy() -> Result<(), Error> {
         cmd_buf.finish_timestamps(&query_pool);
         cmd_buf.host_barrier();
         cmd_buf.finish();
-        device.run_cmd_bufs(&[&cmd_buf], &[], &[], Some(&fence))?;
+        device.run_cmd_bufs(&[&cmd_buf], &[], &[], Some(&mut fence))?;
-        device.wait_and_reset(&[&fence])?;
+        device.wait_and_reset(vec![&mut fence])?;
         let mut readback: Vec<u32> = vec![0u32; 256];
         device.read_buffer(&buf, readback.as_mut_ptr() as *mut u8, 0, 1024)?;
         println!("{:?}", readback);

piet-gpu-hal/src/dx12.rs

@@ -3,35 +3,40 @@
 mod error;
 mod wrappers;
 
+use std::sync::{Arc, Mutex, Weak};
 use std::{cell::Cell, convert::TryInto, mem, ptr};
 
-use winapi::shared::dxgi1_3;
 use winapi::shared::minwindef::TRUE;
+use winapi::shared::{dxgi, dxgi1_2, dxgi1_3, dxgitype};
 use winapi::um::d3d12;
 
+use raw_window_handle::{HasRawWindowHandle, RawWindowHandle};
+
 use smallvec::SmallVec;
 
-use crate::{BufferUsage, Error, ImageLayout};
+use crate::{BufferUsage, Error, GpuInfo, ImageLayout};
 
-use self::wrappers::{
+use self::wrappers::{CommandAllocator, CommandQueue, Device, Factory4, Resource, ShaderByteCode};
-    CommandAllocator, CommandQueue, Device, Factory4, GraphicsCommandList, Resource, ShaderByteCode,
-};
 
 pub struct Dx12Instance {
     factory: Factory4,
 }
 
-// TODO
+pub struct Dx12Surface {
-pub struct Dx12Surface;
+    hwnd: winapi::shared::windef::HWND,
+}
 
-// TODO
+pub struct Dx12Swapchain {
-pub struct Dx12Swapchain;
+    swapchain: wrappers::SwapChain3,
+    size: (u32, u32),
+}
 
 pub struct Dx12Device {
     device: Device,
-    command_allocator: CommandAllocator,
+    free_allocators: Arc<Mutex<Vec<CommandAllocator>>>,
     command_queue: CommandQueue,
     ts_freq: u64,
+    gpu_info: GpuInfo,
     memory_arch: MemoryArchitecture,
 }
 
@@ -47,7 +52,13 @@ pub struct Image {
     size: (u32, u32),
 }
 
-pub struct CmdBuf(GraphicsCommandList);
+pub struct CmdBuf {
+    c: wrappers::GraphicsCommandList,
+    allocator: Option<CommandAllocator>,
+    // One for resetting, one to put back into the allocator pool
+    allocator_clone: CommandAllocator,
+    free_allocators: Weak<Mutex<Vec<CommandAllocator>>>,
+}
 
 pub struct Pipeline {
     pipeline_state: wrappers::PipelineState,
@@ -72,6 +83,8 @@ pub struct Fence {
     val: Cell<u64>,
 }
 
+/// This will probably be renamed "PresentSem" or similar. I believe no
+/// semaphore is needed for presentation on DX12.
 pub struct Semaphore;
 
 #[derive(Default)]
@@ -103,7 +116,9 @@ impl Dx12Instance {
     ///
     /// TODO: take a raw window handle.
     /// TODO: can probably be a trait.
-    pub fn new(window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>) -> Result<(Dx12Instance, Option<Dx12Surface>), Error> {
+    pub fn new(
+        window_handle: Option<&dyn HasRawWindowHandle>,
+    ) -> Result<(Dx12Instance, Option<Dx12Surface>), Error> {
         unsafe {
             #[cfg(debug_assertions)]
             if let Err(e) = wrappers::enable_debug_layer() {
@@ -118,7 +133,16 @@ impl Dx12Instance {
             let factory_flags: u32 = 0;
 
             let factory = Factory4::create(factory_flags)?;
-            Ok((Dx12Instance { factory }, None))
+
+            let mut surface = None;
+            if let Some(window_handle) = window_handle {
+                let window_handle = window_handle.raw_window_handle();
+                if let RawWindowHandle::Windows(w) = window_handle {
+                    let hwnd = w.hwnd as *mut _;
+                    surface = Some(Dx12Surface { hwnd });
+                }
+            }
+            Ok((Dx12Instance { factory }, surface))
         }
     }
 
@@ -136,7 +160,7 @@ impl Dx12Instance {
                 d3d12::D3D12_COMMAND_QUEUE_FLAG_NONE,
                 0,
             )?;
-            let command_allocator = device.create_command_allocator(list_type)?;
+
             let ts_freq = command_queue.get_timestamp_frequency()?;
             let features_architecture = device.get_features_architecture()?;
             let uma = features_architecture.UMA == TRUE;
@@ -146,15 +170,58 @@ impl Dx12Instance {
                 (true, false) => MemoryArchitecture::UMA,
                 _ => MemoryArchitecture::NUMA,
             };
+            let use_staging_buffers = memory_arch == MemoryArchitecture::NUMA;
+            // These values are appropriate for Shader Model 5. When we open up
+            // DXIL, fix this with proper dynamic queries.
+            let gpu_info = GpuInfo {
+                has_descriptor_indexing: false,
+                has_subgroups: false,
+                subgroup_size: None,
+                has_memory_model: false,
+                use_staging_buffers,
+            };
+            let free_allocators = Default::default();
             Ok(Dx12Device {
                 device,
                 command_queue,
-                command_allocator,
+                free_allocators,
                 ts_freq,
                 memory_arch,
+                gpu_info,
             })
         }
     }
+
+    pub unsafe fn swapchain(
+        &self,
+        width: usize,
+        height: usize,
+        device: &Dx12Device,
+        surface: &Dx12Surface,
+    ) -> Result<Dx12Swapchain, Error> {
+        const FRAME_COUNT: u32 = 2;
+        let desc = dxgi1_2::DXGI_SWAP_CHAIN_DESC1 {
+            Width: width as u32,
+            Height: height as u32,
+            AlphaMode: dxgi1_2::DXGI_ALPHA_MODE_IGNORE,
+            BufferCount: FRAME_COUNT,
+            Format: winapi::shared::dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM,
+            Flags: 0,
+            BufferUsage: dxgitype::DXGI_USAGE_RENDER_TARGET_OUTPUT,
+            SampleDesc: dxgitype::DXGI_SAMPLE_DESC {
+                Count: 1,
+                Quality: 0,
+            },
+            Scaling: dxgi1_2::DXGI_SCALING_STRETCH,
+            Stereo: winapi::shared::minwindef::FALSE,
+            SwapEffect: dxgi::DXGI_SWAP_EFFECT_FLIP_DISCARD,
+        };
+        let swapchain =
+            self.factory
+                .create_swapchain_for_hwnd(&device.command_queue, surface.hwnd, desc)?;
+        let size = (width as u32, height as u32);
+        Ok(Dx12Swapchain { swapchain, size })
+    }
 }
 
 impl crate::Device for Dx12Device {
@@ -224,15 +291,24 @@ impl crate::Device for Dx12Device {
 
     fn create_cmd_buf(&self) -> Result<Self::CmdBuf, Error> {
         let list_type = d3d12::D3D12_COMMAND_LIST_TYPE_DIRECT;
+        let allocator = self.free_allocators.lock().unwrap().pop();
+        let allocator = if let Some(allocator) = allocator {
+            allocator
+        } else {
+            unsafe { self.device.create_command_allocator(list_type)? }
+        };
         let node_mask = 0;
         unsafe {
-            let cmd_buf = self.device.create_graphics_command_list(
+            let c = self
-                list_type,
+                .device
-                &self.command_allocator,
+                .create_graphics_command_list(list_type, &allocator, None, node_mask)?;
-                None,
+            let free_allocators = Arc::downgrade(&self.free_allocators);
-                node_mask,
+            Ok(CmdBuf {
-            )?;
+                c,
-            Ok(CmdBuf(cmd_buf))
+                allocator: Some(allocator.clone()),
+                allocator_clone: allocator,
+                free_allocators,
+            })
         }
     }
 
@@ -270,16 +346,19 @@ impl crate::Device for Dx12Device {
     unsafe fn run_cmd_bufs(
         &self,
         cmd_bufs: &[&Self::CmdBuf],
-        wait_semaphores: &[&Self::Semaphore],
+        _wait_semaphores: &[&Self::Semaphore],
-        signal_semaphores: &[&Self::Semaphore],
+        _signal_semaphores: &[&Self::Semaphore],
-        fence: Option<&Self::Fence>,
+        fence: Option<&mut Self::Fence>,
     ) -> Result<(), Error> {
         // TODO: handle semaphores
         let lists = cmd_bufs
             .iter()
-            .map(|c| c.0.as_raw_command_list())
+            .map(|c| c.c.as_raw_command_list())
             .collect::<SmallVec<[_; 4]>>();
         self.command_queue.execute_command_lists(&lists);
+        for c in cmd_bufs {
+            c.c.reset(&c.allocator_clone, None);
+        }
         if let Some(fence) = fence {
             let val = fence.val.get() + 1;
             fence.val.set(val);
@@ -316,7 +395,7 @@ impl crate::Device for Dx12Device {
     }
 
     unsafe fn create_semaphore(&self) -> Result<Self::Semaphore, Error> {
-        todo!()
+        Ok(Semaphore)
     }
 
     unsafe fn create_fence(&self, signaled: bool) -> Result<Self::Fence, Error> {
@@ -326,7 +405,7 @@ impl crate::Device for Dx12Device {
         Ok(Fence { fence, event, val })
     }
 
-    unsafe fn wait_and_reset(&self, fences: &[&Self::Fence]) -> Result<(), Error> {
+    unsafe fn wait_and_reset(&self, fences: Vec<&mut Self::Fence>) -> Result<(), Error> {
         for fence in fences {
             // TODO: probably handle errors here.
             let _status = fence.event.wait(winapi::um::winbase::INFINITE);
@@ -340,7 +419,7 @@ impl crate::Device for Dx12Device {
     }
 
     fn query_gpu_info(&self) -> crate::GpuInfo {
-        todo!()
+        self.gpu_info.clone()
     }
 
     unsafe fn pipeline_builder(&self) -> Self::PipelineBuilder {
@@ -376,7 +455,13 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
     unsafe fn begin(&mut self) {}
 
     unsafe fn finish(&mut self) {
-        let _ = self.0.close();
+        let _ = self.c.close();
+        if let Some(free_allocators) = self.free_allocators.upgrade() {
+            free_allocators
+                .lock()
+                .unwrap()
+                .push(self.allocator.take().unwrap());
+        }
     }
 
     unsafe fn dispatch(
@@ -385,15 +470,15 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
         descriptor_set: &DescriptorSet,
         size: (u32, u32, u32),
     ) {
-        self.0.set_pipeline_state(&pipeline.pipeline_state);
+        self.c.set_pipeline_state(&pipeline.pipeline_state);
-        self.0
+        self.c
             .set_compute_pipeline_root_signature(&pipeline.root_signature);
-        self.0.set_descriptor_heaps(&[&descriptor_set.0]);
+        self.c.set_descriptor_heaps(&[&descriptor_set.0]);
-        self.0.set_compute_root_descriptor_table(
+        self.c.set_compute_root_descriptor_table(
             0,
             descriptor_set.0.get_gpu_descriptor_handle_at_offset(0),
         );
-        self.0.dispatch(size.0, size.1, size.2);
+        self.c.dispatch(size.0, size.1, size.2);
     }
 
     unsafe fn memory_barrier(&mut self) {
@@ -402,7 +487,7 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
         // in the barrier. But it seems like this is a reasonable way to create a
         // global barrier.
         let bar = wrappers::create_uav_resource_barrier(ptr::null_mut());
-        self.0.resource_barrier(&[bar]);
+        self.c.resource_barrier(&[bar]);
     }
 
     unsafe fn host_barrier(&mut self) {
@@ -427,7 +512,7 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
                 src_state,
                 dst_state,
             );
-            self.0.resource_barrier(&[bar]);
+            self.c.resource_barrier(&[bar]);
         }
     }
 
@@ -440,33 +525,31 @@ impl crate::CmdBuf<Dx12Device> for CmdBuf {
     unsafe fn copy_buffer(&self, src: &Buffer, dst: &Buffer) {
         // TODO: consider using copy_resource here (if sizes match)
         let size = src.size.min(dst.size);
-        self.0.copy_buffer(&dst.resource, 0, &src.resource, 0, size);
+        self.c.copy_buffer(&dst.resource, 0, &src.resource, 0, size);
     }
 
     unsafe fn copy_image_to_buffer(&self, src: &Image, dst: &Buffer) {
-        self.0
+        self.c
             .copy_texture_to_buffer(&src.resource, &dst.resource, src.size.0, src.size.1);
     }
 
     unsafe fn copy_buffer_to_image(&self, src: &Buffer, dst: &Image) {
-        self.0
+        self.c
             .copy_buffer_to_texture(&src.resource, &dst.resource, dst.size.0, dst.size.1);
     }
 
     unsafe fn blit_image(&self, src: &Image, dst: &Image) {
-        self.0.copy_resource(&src.resource, &dst.resource);
+        self.c.copy_resource(&src.resource, &dst.resource);
     }
 
-    unsafe fn reset_query_pool(&mut self, pool: &QueryPool) {
+    unsafe fn reset_query_pool(&mut self, _pool: &QueryPool) {}
-        todo!()
-    }
 
     unsafe fn write_timestamp(&mut self, pool: &QueryPool, query: u32) {
-        self.0.end_timing_query(&pool.heap, query);
+        self.c.end_timing_query(&pool.heap, query);
     }
 
     unsafe fn finish_timestamps(&mut self, pool: &QueryPool) {
-        self.0
+        self.c
             .resolve_timing_query_data(&pool.heap, 0, pool.n_queries, &pool.buf.resource, 0);
     }
 }
@@ -637,3 +720,27 @@ fn resource_state_for_image_layout(layout: ImageLayout) -> d3d12::D3D12_RESOURCE
         ImageLayout::ShaderRead => d3d12::D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE,
     }
 }
+
+impl Dx12Swapchain {
+    pub unsafe fn next(&mut self) -> Result<(usize, Semaphore), Error> {
+        let idx = self.swapchain.get_current_back_buffer_index();
+        Ok((idx as usize, Semaphore))
+    }
+
+    pub unsafe fn image(&self, idx: usize) -> Image {
+        let buffer = self.swapchain.get_buffer(idx as u32);
+        Image {
+            resource: buffer,
+            size: self.size,
+        }
+    }
+
+    pub unsafe fn present(
+        &self,
+        _image_idx: usize,
+        _semaphores: &[&Semaphore],
+    ) -> Result<bool, Error> {
+        self.swapchain.present(1, 0)?;
+        Ok(false)
+    }
+}

piet-gpu-hal/src/dx12/error.rs

@@ -41,6 +41,7 @@ impl std::error::Error for Error {}
 /// See https://docs.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-error
 fn err_str_for_hr(hr: winerror::HRESULT) -> Option<&'static str> {
     Some(match hr as u32 {
+        0x80004005 => "E_FAIL",
         0x80070057 => "E_INVALIDARG",
         0x887a0001 => "DXGI_ERROR_INVALID_CALL",
         0x887a0002 => "DXGI_ERROR_NOT_FOUND",

piet-gpu-hal/src/dx12/wrappers.rs

@@ -40,10 +40,6 @@ pub struct Factory2(pub ComPtr<dxgi1_2::IDXGIFactory2>);
 #[derive(Clone)]
 pub struct Factory4(pub ComPtr<dxgi1_4::IDXGIFactory4>);
 #[derive(Clone)]
-pub struct SwapChain(pub ComPtr<dxgi::IDXGISwapChain>);
-#[derive(Clone)]
-pub struct SwapChain1(pub ComPtr<dxgi1_2::IDXGISwapChain1>);
-#[derive(Clone)]
 pub struct SwapChain3(pub ComPtr<dxgi1_4::IDXGISwapChain3>);
 
 #[derive(Clone)]
@@ -218,22 +214,24 @@ impl Factory4 {
 
     pub unsafe fn create_swapchain_for_hwnd(
         &self,
-        command_queue: CommandQueue,
+        command_queue: &CommandQueue,
         hwnd: windef::HWND,
         desc: dxgi1_2::DXGI_SWAP_CHAIN_DESC1,
-    ) -> SwapChain3 {
+    ) -> Result<SwapChain3, Error> {
         let mut swap_chain = ptr::null_mut();
-        error::error_if_failed_else_unit(self.0.CreateSwapChainForHwnd(
+        explain_error(
+            self.0.CreateSwapChainForHwnd(
                 command_queue.0.as_raw() as *mut _,
                 hwnd,
                 &desc,
                 ptr::null(),
                 ptr::null_mut(),
                 &mut swap_chain as *mut _ as *mut _,
-        ))
+            ),
-        .expect("could not creation swapchain for hwnd");
+            "could not creation swapchain for hwnd",
+        )?;
 
-        SwapChain3(ComPtr::from_raw(swap_chain))
+        Ok(SwapChain3(ComPtr::from_raw(swap_chain)))
     }
 }
 
@@ -263,47 +261,6 @@ impl CommandQueue {
     }
 }
 
-impl SwapChain {
-    pub unsafe fn get_buffer(&self, id: u32) -> Resource {
-        let mut resource = ptr::null_mut();
-        error::error_if_failed_else_unit(self.0.GetBuffer(
-            id,
-            &d3d12::ID3D12Resource::uuidof(),
-            &mut resource as *mut _ as *mut _,
-        ))
-        .expect("SwapChain could not get buffer");
-
-        Resource::new(resource)
-    }
-
-    // TODO: present flags
-    pub unsafe fn present(&self, interval: u32, flags: u32) -> winerror::HRESULT {
-        self.0.Present(interval, flags)
-    }
-}
-
-impl SwapChain1 {
-    pub unsafe fn cast_into_swap_chain3(&self) -> SwapChain3 {
-        SwapChain3(
-            self.0
-                .cast::<dxgi1_4::IDXGISwapChain3>()
-                .expect("could not cast into SwapChain3"),
-        )
-    }
-
-    pub unsafe fn get_buffer(&self, id: u32) -> Resource {
-        let mut resource = ptr::null_mut();
-        error::error_if_failed_else_unit(self.0.GetBuffer(
-            id,
-            &d3d12::ID3D12Resource::uuidof(),
-            &mut resource as *mut _ as *mut _,
-        ))
-        .expect("SwapChain1 could not get buffer");
-
-        Resource::new(resource)
-    }
-}
-
 impl SwapChain3 {
     pub unsafe fn get_buffer(&self, id: u32) -> Resource {
         let mut resource = ptr::null_mut();

piet-gpu-hal/src/hub.rs

@@ -124,9 +124,9 @@ impl Session {
             let mut i = 0;
             while i < pending.len() {
                 if let Ok(true) = self.0.device.get_fence_status(&pending[i].fence) {
-                    let item = pending.swap_remove(i);
+                    let mut item = pending.swap_remove(i);
                     // TODO: wait is superfluous, can just reset
-                    let _ = self.0.device.wait_and_reset(&[&item.fence]);
+                    let _ = self.0.device.wait_and_reset(vec![&mut item.fence]);
                     let mut pool = self.0.cmd_buf_pool.lock().unwrap();
                     pool.push((item.cmd_buf, item.fence));
                     std::mem::drop(item.resources);
@@ -143,7 +143,7 @@ impl Session {
 
     pub unsafe fn run_cmd_buf(
         &self,
-        cmd_buf: CmdBuf,
+        mut cmd_buf: CmdBuf,
         wait_semaphores: &[&Semaphore],
         signal_semaphores: &[&Semaphore],
     ) -> Result<SubmittedCmdBuf, Error> {
@@ -162,7 +162,7 @@ impl Session {
             &cmd_bufs,
             wait_semaphores,
             signal_semaphores,
-            Some(&cmd_buf.fence),
+            Some(&mut cmd_buf.fence),
         )?;
         Ok(SubmittedCmdBuf(
             Some(SubmittedCmdBufInner {
@@ -313,10 +313,10 @@ impl CmdBuf {
 
 impl SubmittedCmdBuf {
     pub fn wait(mut self) -> Result<(), Error> {
-        let item = self.0.take().unwrap();
+        let mut item = self.0.take().unwrap();
         if let Some(session) = Weak::upgrade(&self.1) {
             unsafe {
-                session.device.wait_and_reset(&[&item.fence])?;
+                session.device.wait_and_reset(vec![&mut item.fence])?;
             }
             session
                 .cmd_buf_pool

piet-gpu-hal/src/lib.rs

@@ -9,9 +9,12 @@ pub mod hub;
 #[macro_use]
 mod macros;
 
-// TODO make this not pub
+// TODO: Don't make the module pub, but do figure out which types to
+// export at the root level.
 pub mod mux;
 
+// TODO: because these are conditionally included, "cargo fmt" does not
+// see them. Figure that out, possibly including running rustfmt manually.
 mux_cfg! {
     #[cfg(vk)]
     pub mod vulkan;
@@ -191,7 +194,7 @@ pub trait Device: Sized {
         cmd_buf: &[&Self::CmdBuf],
         wait_semaphores: &[&Self::Semaphore],
         signal_semaphores: &[&Self::Semaphore],
-        fence: Option<&Self::Fence>,
+        fence: Option<&mut Self::Fence>,
     ) -> Result<(), Error>;
 
     /// Copy data from the buffer to memory.
@@ -228,7 +231,7 @@ pub trait Device: Sized {
 
     unsafe fn create_semaphore(&self) -> Result<Self::Semaphore, Error>;
     unsafe fn create_fence(&self, signaled: bool) -> Result<Self::Fence, Error>;
-    unsafe fn wait_and_reset(&self, fences: &[&Self::Fence]) -> Result<(), Error>;
+    unsafe fn wait_and_reset(&self, fences: Vec<&mut Self::Fence>) -> Result<(), Error>;
     unsafe fn get_fence_status(&self, fence: &Self::Fence) -> Result<bool, Error>;
 
     unsafe fn create_sampler(&self, params: SamplerParams) -> Result<Self::Sampler, Error>;

piet-gpu-hal/src/macros.rs

@@ -51,6 +51,16 @@ macro_rules! mux_enum {
                     }
                 }
             }
+            $crate::mux_cfg! {
+                #[cfg(vk)]
+                #[allow(unused)]
+                fn vk_mut(&mut self) -> &mut $vk {
+                    match self {
+                        $name::Vk(x) => x,
+                        _ => panic!("downcast error")
+                    }
+                }
+            }
 
             $crate::mux_cfg! {
                 #[cfg(dx12)]
@@ -62,6 +72,16 @@ macro_rules! mux_enum {
                     }
                 }
             }
+            $crate::mux_cfg! {
+                #[cfg(dx12)]
+                #[allow(unused)]
+                fn dx12_mut(&mut self) -> &mut $dx12 {
+                    match self {
+                        $name::Dx12(x) => x,
+                        _ => panic!("downcast error")
+                    }
+                }
+            }
         }
     };
 }

piet-gpu-hal/src/mux.rs

@@ -83,6 +83,13 @@ pub enum ShaderCode<'a> {
 }
 
 impl Instance {
+    /// Create a new GPU instance appropriate for the surface.
+    ///
+    /// When multiple back-end GPU APIs are available (for example, Vulkan
+    /// and DX12), this function selects one at runtime.
+    ///
+    /// When no surface is given, the instance is suitable for compute-only
+    /// work.
     pub fn new(
         window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>,
     ) -> Result<(Instance, Option<Surface>), Error> {
@@ -108,6 +115,11 @@ impl Instance {
         Err("No suitable instances found".into())
     }
 
+    /// Create a device appropriate for the surface.
+    ///
+    /// The "device" is the low-level GPU abstraction for creating resources
+    /// and submitting work. Most users of this library will want to wrap it in
+    /// a "session" which is similar but provides many conveniences.
     pub unsafe fn device(&self, surface: Option<&Surface>) -> Result<Device, Error> {
         mux_match! { self;
             Instance::Vk(i) => i.device(surface.map(Surface::vk)).map(Device::Vk),
@@ -115,6 +127,12 @@ impl Instance {
         }
     }
 
+    /// Create a swapchain.
+    ///
+    /// A swapchain is a small vector of images shared with the platform's
+    /// presentation logic. To actually display pixels, the application writes
+    /// into the swapchain images, then calls the present method to display
+    /// them.
     pub unsafe fn swapchain(
         &self,
         width: usize,
@@ -126,7 +144,9 @@ impl Instance {
             Instance::Vk(i) => i
                 .swapchain(width, height, device.vk(), surface.vk())
                 .map(Swapchain::Vk),
-            Instance::Dx12(_i) => todo!(),
+            Instance::Dx12(i) => i
+                .swapchain(width, height, device.dx12(), surface.dx12())
+                .map(Swapchain::Dx12),
         }
     }
 }
@@ -177,23 +197,22 @@ impl Device {
         }
     }
 
-    pub unsafe fn wait_and_reset(&self, fences: &[&Fence]) -> Result<(), Error> {
+    // Consider changing Vec to iterator (as is done in gfx-hal)
+    pub unsafe fn wait_and_reset(&self, fences: Vec<&mut Fence>) -> Result<(), Error> {
         mux_match! { self;
             Device::Vk(d) => {
-                let fences = fences
+                let mut fences = fences
-                    .iter()
+                    .into_iter()
-                    .copied()
+                    .map(|f| f.vk_mut())
-                    .map(Fence::vk)
+                    .collect::<Vec<_>>();
-                    .collect::<SmallVec<[_; 4]>>();
+                d.wait_and_reset(fences)
-                d.wait_and_reset(&*fences)
             }
             Device::Dx12(d) => {
-                let fences = fences
+                let mut fences = fences
-                    .iter()
+                    .into_iter()
-                    .copied()
+                    .map(|f| f.dx12_mut())
-                    .map(Fence::dx12)
+                    .collect::<Vec<_>>();
-                    .collect::<SmallVec<[_; 4]>>();
+                d.wait_and_reset(fences)
-                d.wait_and_reset(&*fences)
             }
         }
     }
@@ -252,7 +271,7 @@ impl Device {
         cmd_bufs: &[&CmdBuf],
         wait_semaphores: &[&Semaphore],
         signal_semaphores: &[&Semaphore],
-        fence: Option<&Fence>,
+        fence: Option<&mut Fence>,
     ) -> Result<(), Error> {
         mux_match! { self;
             Device::Vk(d) => d.run_cmd_bufs(
@@ -270,7 +289,7 @@ impl Device {
                     .copied()
                     .map(Semaphore::vk)
                     .collect::<SmallVec<[_; 4]>>(),
-                fence.map(Fence::vk),
+                fence.map(Fence::vk_mut),
             ),
             Device::Dx12(d) => d.run_cmd_bufs(
                 &cmd_bufs
@@ -287,7 +306,7 @@ impl Device {
                     .copied()
                     .map(Semaphore::dx12)
                     .collect::<SmallVec<[_; 4]>>(),
-                fence.map(Fence::dx12),
+                fence.map(Fence::dx12_mut),
             ),
         }
     }
@@ -568,8 +587,9 @@ impl Swapchain {
                 let (idx, sem) = s.next()?;
                 Ok((idx, Semaphore::Vk(sem)))
             }
-            Swapchain::Dx12(_s) => {
+            Swapchain::Dx12(s) => {
-                todo!()
+                let (idx, sem) = s.next()?;
+                Ok((idx, Semaphore::Dx12(sem)))
             }
         }
     }
@@ -577,7 +597,7 @@ impl Swapchain {
     pub unsafe fn image(&self, idx: usize) -> Image {
         mux_match! { self;
             Swapchain::Vk(s) => Image::Vk(s.image(idx)),
-            Swapchain::Dx12(_s) => todo!(),
+            Swapchain::Dx12(s) => Image::Dx12(s.image(idx)),
         }
     }
 
@@ -595,7 +615,14 @@ impl Swapchain {
                     .map(Semaphore::vk)
                     .collect::<SmallVec<[_; 4]>>(),
             ),
-            Swapchain::Dx12(_s) => todo!(),
+            Swapchain::Dx12(s) => s.present(
+                image_idx,
+                &semaphores
+                    .iter()
+                    .copied()
+                    .map(Semaphore::dx12)
+                    .collect::<SmallVec<[_; 4]>>(),
+            ),
         }
     }
 }

piet-gpu-hal/src/vulkan.rs

@@ -619,12 +619,11 @@ impl crate::Device for VkDevice {
         Ok(device.create_semaphore(&vk::SemaphoreCreateInfo::default(), None)?)
     }
 
-    unsafe fn wait_and_reset(&self, fences: &[&Self::Fence]) -> Result<(), Error> {
+    unsafe fn wait_and_reset(&self, fences: Vec<&mut Self::Fence>) -> Result<(), Error> {
         let device = &self.device.device;
         let fences = fences
             .iter()
-            .copied()
+            .map(|f| **f)
-            .copied()
             .collect::<SmallVec<[_; 4]>>();
         device.wait_for_fences(&fences, true, !0)?;
         device.reset_fences(&fences)?;
@@ -718,7 +717,7 @@ impl crate::Device for VkDevice {
         cmd_bufs: &[&CmdBuf],
         wait_semaphores: &[&Self::Semaphore],
         signal_semaphores: &[&Self::Semaphore],
-        fence: Option<&Self::Fence>,
+        fence: Option<&mut Self::Fence>,
     ) -> Result<(), Error> {
         let device = &self.device.device;