separate instance and surface creation

This separates creation of Instance and Surfaces, allowing for rendering to multiple windows.

piet-gpu-hal/examples/collatz.rs

@@ -2,9 +2,9 @@ use piet_gpu_hal::{include_shader, BindType, ComputePassDescriptor};
 use piet_gpu_hal::{BufferUsage, Instance, InstanceFlags, Session};
 
 fn main() {
-    let (instance, _) = Instance::new(None, InstanceFlags::empty()).unwrap();
+    let instance = Instance::new(InstanceFlags::empty()).unwrap();
     unsafe {
-        let device = instance.device(None).unwrap();
+        let device = instance.device().unwrap();
         let session = Session::new(device);
         let usage = BufferUsage::MAP_READ | BufferUsage::STORAGE;
         let src = (0..256).map(|x| x + 1).collect::<Vec<u32>>();

piet-gpu-hal/src/dx12.rs

@@ -130,12 +130,7 @@ enum MemoryArchitecture {
 
 impl Dx12Instance {
     /// Create a new instance.
-    ///
+    pub fn new() -> Result<Dx12Instance, Error> {
-    /// TODO: take a raw window handle.
-    /// TODO: can probably be a trait.
-    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() {
@@ -151,23 +146,25 @@ impl Dx12Instance {
 
             let factory = Factory4::create(factory_flags)?;
 
-            let mut surface = None;
+            Ok(Dx12Instance { factory })
-            if let Some(window_handle) = window_handle {
+        }
-                let window_handle = window_handle.raw_window_handle();
+    }
-                if let RawWindowHandle::Windows(w) = window_handle {
+
+    /// Create a surface for the specified window handle.
+    pub fn surface(
+        &self,
+        window_handle: &dyn HasRawWindowHandle,
+    ) -> Result<Dx12Surface, Error> {
+        if let RawWindowHandle::Windows(w) = window_handle.raw_window_handle() {
             let hwnd = w.hwnd as *mut _;
-                    surface = Some(Dx12Surface { hwnd });
+            Ok(Dx12Surface { hwnd })
-                }
+        } else {
-            }
+            Err("can't create surface for window handle".into())
-            Ok((Dx12Instance { factory }, surface))
         }
     }
 
     /// Get a device suitable for compute workloads.
-    ///
+    pub fn device(&self) -> Result<Dx12Device, Error> {
-    /// TODO: handle window.
-    /// TODO: probably can also be trait'ified.
-    pub fn device(&self, _surface: Option<&Dx12Surface>) -> Result<Dx12Device, Error> {
         unsafe {
             let device = Device::create_device(&self.factory)?;
             let list_type = d3d12::D3D12_COMMAND_LIST_TYPE_DIRECT;

piet-gpu-hal/src/lib.rs

@@ -49,6 +49,8 @@ bitflags! {
     pub struct InstanceFlags: u32 {
         /// Prefer DX12 over Vulkan.
         const DX12 = 0x1;
+        /// Support presentation to a surface.
+        const PRESENT = 0x2;
         // TODO: discrete vs integrated selection
     }
 }

piet-gpu-hal/src/metal.rs

@@ -133,20 +133,22 @@ struct Helpers {
 }
 
 impl MtlInstance {
-    pub fn new(
+    pub fn new(window_handle: Option<&dyn HasRawWindowHandle>) -> Result<MtlInstance, Error> {
-        window_handle: Option<&dyn HasRawWindowHandle>,
+        Ok(MtlInstance)
-    ) -> Result<(MtlInstance, Option<MtlSurface>), Error> {
-        let mut surface = None;
-        if let Some(window_handle) = window_handle {
-            let window_handle = window_handle.raw_window_handle();
-            if let RawWindowHandle::MacOS(w) = window_handle {
-                unsafe {
-                    surface = Self::make_surface(w.ns_view as id, w.ns_window as id);
-                }
-            }
     }
 
-        Ok((MtlInstance, surface))
+    pub unsafe fn surface(
+        &self,
+        window_handle: &dyn HasRawWindowHandle,
+    ) -> Result<MtlSurface, Error> {
+        if let RawWindowHandle::MacOS(handle) = window_handle.raw_window_handle() {
+            Ok(Self::make_surface(
+                handle.ns_view as id,
+                handle.ns_window as id,
+            ))
+        } else {
+            Err("can't create surface for window handle".into())
+        }
     }
 
     unsafe fn make_surface(ns_view: id, ns_window: id) -> Option<MtlSurface> {
@@ -182,7 +184,7 @@ impl MtlInstance {
 
     // TODO might do some enumeration of devices
 
-    pub fn device(&self, _surface: Option<&MtlSurface>) -> Result<MtlDevice, Error> {
+    pub fn device(&self) -> Result<MtlDevice, Error> {
         if let Some(device) = metal::Device::system_default() {
             let cmd_queue = device.new_command_queue();
             Ok(MtlDevice::new_from_raw_mtl(device, cmd_queue))

piet-gpu-hal/src/mux.rs

@@ -114,7 +114,7 @@ pub enum ShaderCode<'a> {
 }
 
 impl Instance {
-    /// Create a new GPU instance appropriate for the surface.
+    /// Create a new GPU instance.
     ///
     /// When multiple back-end GPU APIs are available (for example, Vulkan
     /// and DX12), this function selects one at runtime.
@@ -122,9 +122,8 @@ impl Instance {
     /// When no surface is given, the instance is suitable for compute-only
     /// work.
     pub fn new(
-        window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>,
         flags: InstanceFlags,
-    ) -> Result<(Instance, Option<Surface>), Error> {
+    ) -> Result<Instance, Error> {
         let mut backends = [BackendType::Vulkan, BackendType::Dx12];
         if flags.contains(InstanceFlags::DX12) {
             backends.swap(0, 1);
@@ -134,9 +133,8 @@ impl Instance {
                 mux_cfg! {
                     #[cfg(vk)]
                     {
-                        let result = vulkan::VkInstance::new(window_handle);
+                        if let Ok(instance) = vulkan::VkInstance::new(flags.contains(InstanceFlags::PRESENT)) {
-                        if let Ok((instance, surface)) = result {
+                            return Ok(Instance::Vk(instance));
-                            return Ok((Instance::Vk(instance), surface.map(Surface::Vk)));
                         }
                     }
                 }
@@ -145,9 +143,8 @@ impl Instance {
                 mux_cfg! {
                     #[cfg(dx12)]
                     {
-                        let result = dx12::Dx12Instance::new(window_handle);
+                        if let Ok(instance) = dx12::Dx12Instance::new() {
-                        if let Ok((instance, surface)) = result {
+                            return Ok(Instance::Dx12(instance))
-                            return Ok((Instance::Dx12(instance), surface.map(Surface::Dx12)));
                         }
                     }
                 }
@@ -156,9 +153,8 @@ impl Instance {
         mux_cfg! {
             #[cfg(mtl)]
             {
-                let result = metal::MtlInstance::new(window_handle);
+                if let Ok(instance) = metal::MtlInstance::new() {
-                if let Ok((instance, surface)) = result {
+                    return Ok(Instance::Mtl(instance));
-                    return Ok((Instance::Mtl(instance), surface.map(Surface::Mtl)));
                 }
             }
         }
@@ -166,15 +162,27 @@ impl Instance {
         Err("No suitable instances found".into())
     }
 
-    /// Create a device appropriate for the surface.
+    /// Create a surface from the specified window handle.
+    pub unsafe fn surface(
+        &self,
+        window_handle: &dyn raw_window_handle::HasRawWindowHandle,
+    ) -> Result<Surface, Error> {
+        mux_match! { self;
+            Instance::Vk(i) => i.surface(window_handle).map(Surface::Vk),
+            Instance::Dx12(i) => i.surface(window_handle).map(Surface::Dx12),
+            Instance::Mtl(i) => i.surface(window_handle).map(Surface::Mtl),
+        }
+    }
+
+    /// Create a device.
     ///
     /// 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> {
+    pub unsafe fn device(&self) -> Result<Device, Error> {
         mux_match! { self;
-            Instance::Vk(i) => i.device(surface.map(Surface::vk)).map(Device::Vk),
+            Instance::Vk(i) => i.device(true).map(Device::Vk),
-            Instance::Dx12(i) => i.device(surface.map(Surface::dx12)).map(Device::Dx12),
+            Instance::Dx12(i) => i.device().map(Device::Dx12),
             Instance::Mtl(i) => i.device(surface.map(Surface::mtl)).map(Device::Mtl),
         }
     }

piet-gpu-hal/src/vulkan.rs

@@ -157,9 +157,7 @@ impl VkInstance {
     ///
     /// The caller is responsible for making sure that window which owns the raw window handle
     /// outlives the surface.
-    pub fn new(
+    pub fn new(support_present: bool) -> Result<VkInstance, Error> {
-        window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>,
-    ) -> Result<(VkInstance, Option<VkSurface>), Error> {
         unsafe {
             let app_name = CString::new("VkToy").unwrap();
             let entry = Entry::new()?;
@@ -175,10 +173,33 @@ impl VkInstance {
             if cfg!(debug_assertions) {
                 has_debug_ext = exts.try_add(DebugUtils::name());
             }
-            if let Some(ref handle) = window_handle {
+
-                for ext in ash_window::enumerate_required_extensions(*handle)? {
+            // Enable platform specific surface extensions if presentation
-                    exts.try_add(ext);
+            // support is requested.
+            if support_present {
+                exts.try_add(khr::Surface::name());
+
+                #[cfg(target_os = "windows")]
+                exts.try_add(khr::Win32Surface::name());
+
+                #[cfg(any(
+                    target_os = "linux",
+                    target_os = "dragonfly",
+                    target_os = "freebsd",
+                    target_os = "netbsd",
+                    target_os = "openbsd"
+                ))]
+                {
+                    exts.try_add(khr::XlibSurface::name());
+                    exts.try_add(khr::XcbSurface::name());
+                    exts.try_add(khr::WaylandSurface::name());
                 }
+
+                #[cfg(any(target_os = "android"))]
+                exts.try_add(khr::AndroidSurface::name());
+
+                #[cfg(any(target_os = "macos", target_os = "ios"))]
+                exts.try_add(kkr::MetalSurface::name());
             }
 
             let supported_version = entry
@@ -222,14 +243,6 @@ impl VkInstance {
                 (None, None)
             };
 
-            let vk_surface = match window_handle {
-                Some(handle) => Some(VkSurface {
-                    surface: ash_window::create_surface(&entry, &instance, handle, None)?,
-                    surface_fn: khr::Surface::new(&entry, &instance),
-                }),
-                None => None,
-            };
-
             let vk_instance = VkInstance {
                 entry,
                 instance,
@@ -238,21 +251,37 @@ impl VkInstance {
                 _dbg_callbk,
             };
 
-            Ok((vk_instance, vk_surface))
+            Ok(vk_instance)
         }
     }
 
-    /// Create a device from the instance, suitable for compute, with an optional surface.
+    /// Create a surface from the instance for the specified window handle.
+    /// 
+    /// # Safety
+    /// 
+    /// The caller is responsible for making sure that the instance outlives the surface.
+    pub unsafe fn surface(
+        &self,
+        window_handle: &dyn raw_window_handle::HasRawWindowHandle,
+    ) -> Result<VkSurface, Error> {
+        Ok(VkSurface {
+            surface: ash_window::create_surface(&self.entry, &self.instance, window_handle, None)?,
+            surface_fn: khr::Surface::new(&self.entry, &self.instance),
+        })
+    }
+
+    /// Create a device from the instance, suitable for compute, with an optional presentation
+    /// support.
     ///
     /// # Safety
     ///
     /// The caller is responsible for making sure that the instance outlives the device
     /// and surface. We could enforce that, for example having an `Arc` of the raw instance,
     /// but for now keep things simple.
-    pub unsafe fn device(&self, surface: Option<&VkSurface>) -> Result<VkDevice, Error> {
+    pub unsafe fn device(&self, support_present: bool) -> Result<VkDevice, Error> {
         let devices = self.instance.enumerate_physical_devices()?;
         let (pdevice, qfi) =
-            choose_compute_device(&self.instance, &devices, surface).ok_or("no suitable device")?;
+            choose_device(&self.instance, &devices, support_present).ok_or("no suitable device")?;
 
         let mut has_descriptor_indexing = false;
         let vk1_1 = self.vk_version >= vk::make_api_version(0, 1, 1, 0);
@@ -288,7 +317,7 @@ impl VkInstance {
             self.instance
                 .enumerate_device_extension_properties(pdevice)?,
         );
-        if surface.is_some() {
+        if support_present {
             extensions.try_add(khr::Swapchain::name());
         }
         if has_descriptor_indexing {
@@ -1421,26 +1450,35 @@ impl Layers {
     }
 }
 
-unsafe fn choose_compute_device(
+unsafe fn choose_device(
     instance: &Instance,
     devices: &[vk::PhysicalDevice],
-    surface: Option<&VkSurface>,
+    support_graphics: bool,
 ) -> Option<(vk::PhysicalDevice, u32)> {
+    let mut desired_flags = vk::QueueFlags::COMPUTE;
+    if support_graphics {
+        desired_flags |= vk::QueueFlags::GRAPHICS;
+    }
     for pdevice in devices {
         let props = instance.get_physical_device_queue_family_properties(*pdevice);
         for (ix, info) in props.iter().enumerate() {
-            // Check for surface presentation support
+            // TODO: is this strictly necessary? We'll need a queue supporting graphics
-            if let Some(surface) = surface {
+            // for image rendering regardless, and I'm leaning on the assumption that 
-                if !surface
+            // all physical device + queue family combinations that support graphics also
-                    .surface_fn
+            // support presentation particularly when the appropriate extensions are enabled
-                    .get_physical_device_surface_support(*pdevice, ix as u32, surface.surface)
+            // at instance creation. This may be faulty.
-                    .unwrap()
-                {
-                    continue;
-                }
-            }
 
-            if info.queue_flags.contains(vk::QueueFlags::COMPUTE) {
+            // Check for surface presentation support
+            // if let Some(surface) = surface {
+            //     if !surface
+            //         .surface_fn
+            //         .get_physical_device_surface_support(*pdevice, ix as u32, surface.surface)
+            //         .unwrap()
+            //     {
+            //         continue;
+            //     }
+            // }
+            if info.queue_flags.contains(desired_flags) {
                 return Some((*pdevice, ix as u32));
             }
         }

piet-gpu/bin/cli.rs

@@ -226,9 +226,9 @@ fn main() -> Result<(), Error> {
                 .takes_value(true),
         )
         .get_matches();
-    let (instance, _) = Instance::new(None, InstanceFlags::default())?;
+    let instance = Instance::new(InstanceFlags::default())?;
     unsafe {
-        let device = instance.device(None)?;
+        let device = instance.device()?;
         let session = Session::new(device);
 
         let mut ctx = PietGpuRenderContext::new();

piet-gpu/bin/winit.rs

@@ -1,6 +1,6 @@
 use piet::kurbo::Point;
 use piet::{RenderContext, Text, TextAttribute, TextLayoutBuilder};
-use piet_gpu_hal::{Error, ImageLayout, Instance, Session};
+use piet_gpu_hal::{Error, ImageLayout, Instance, InstanceFlags, Session};
 
 use piet_gpu::{test_scenes, PicoSvg, PietGpuRenderContext, RenderDriver, Renderer};
 
@@ -57,12 +57,12 @@ fn main() -> Result<(), Error> {
         .with_resizable(false) // currently not supported
         .build(&event_loop)?;
 
-    let (instance, surface) = Instance::new(Some(&window), Default::default())?;
+    let instance = Instance::new(InstanceFlags::PRESENT)?;
     let mut info_string = "info".to_string();
     unsafe {
-        let device = instance.device(surface.as_ref())?;
+        let surface = instance.surface(&window)?;
-        let mut swapchain =
+        let device = instance.device()?;
-            instance.swapchain(WIDTH / 2, HEIGHT / 2, &device, surface.as_ref().unwrap())?;
+        let mut swapchain = instance.swapchain(WIDTH / 2, HEIGHT / 2, &device, &surface)?;
         let session = Session::new(device);
 
         let mut current_frame = 0;

tests/src/runner.rs

@@ -45,8 +45,8 @@ pub struct BufStage {
 
 impl Runner {
     pub unsafe fn new(flags: InstanceFlags) -> Runner {
-        let (instance, _) = Instance::new(None, flags).unwrap();
+        let instance = Instance::new(flags).unwrap();
-        let device = instance.device(None).unwrap();
+        let device = instance.device().unwrap();
         let session = Session::new(device);
         let cmd_buf_pool = Vec::new();
         Runner {