Merge pull request #184 from dfrg/multi-surface

Separate Instance and Surface creation in HAL

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 {
+
-                    let hwnd = w.hwnd as *mut _;
+    /// Create a surface for the specified window handle.
-                    surface = Some(Dx12Surface { hwnd });
+    pub fn surface(
-                }
+        &self,
-            }
+        window_handle: &dyn HasRawWindowHandle,
-            Ok((Dx12Instance { factory }, surface))
+    ) -> Result<Dx12Surface, Error> {
+        if let RawWindowHandle::Windows(w) = window_handle.raw_window_handle() {
+            let hwnd = w.hwnd as *mut _;
+            Ok(Dx12Surface { hwnd })
+        } else {
+            Err("can't create surface for window handle".into())
         }
     }
 
     /// 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/metal.rs

@@ -133,20 +133,19 @@ struct Helpers {
 }
 
 impl MtlInstance {
-    pub fn new(
+    pub fn new() -> 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).unwrap())
+        } else {
+            Err("can't create surface for window handle".into())
+        }
     }
 
     unsafe fn make_surface(ns_view: id, ns_window: id) -> Option<MtlSurface> {
@@ -182,7 +181,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,17 +114,14 @@ 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.
     ///
     /// When no surface is given, the instance is suitable for compute-only
     /// work.
-    pub fn new(
+    pub fn new(flags: InstanceFlags) -> Result<Instance, Error> {
-        window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>,
-        flags: InstanceFlags,
-    ) -> Result<(Instance, Option<Surface>), Error> {
         let mut backends = [BackendType::Vulkan, BackendType::Dx12];
         if flags.contains(InstanceFlags::DX12) {
             backends.swap(0, 1);
@@ -134,9 +131,8 @@ impl Instance {
                 mux_cfg! {
                     #[cfg(vk)]
                     {
-                        let result = vulkan::VkInstance::new(window_handle);
+                        if let Ok(instance) = vulkan::VkInstance::new() {
-                        if let Ok((instance, surface)) = result {
+                            return Ok(Instance::Vk(instance));
-                            return Ok((Instance::Vk(instance), surface.map(Surface::Vk)));
                         }
                     }
                 }
@@ -145,9 +141,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 +151,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,16 +160,28 @@ 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().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),
+            Instance::Mtl(i) => i.device().map(Device::Mtl),
         }
     }
 

piet-gpu-hal/src/vulkan.rs

@@ -154,12 +154,7 @@ impl VkInstance {
     ///
     /// There's more to be done to make this suitable for integration with other
     /// systems, but for now the goal is to make things simple.
-    ///
+    pub fn new() -> Result<VkInstance, Error> {
-    /// The caller is responsible for making sure that window which owns the raw window handle
-    /// outlives the surface.
-    pub fn new(
-        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,12 +170,32 @@ 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.
-                    exts.try_add(ext);
+            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
                 .try_enumerate_instance_version()?
                 .unwrap_or(vk::make_api_version(0, 1, 0, 0));
@@ -222,14 +237,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 +245,36 @@ 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 device
+    /// The caller is responsible for making sure that the instance outlives the surface.
-    /// and surface. We could enforce that, for example having an `Arc` of the raw instance,
+    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 and graphics.
+    ///
+    /// # Safety
+    ///
+    /// The caller is responsible for making sure that the instance outlives the device.
+    /// 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) -> 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).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,9 +310,7 @@ impl VkInstance {
             self.instance
                 .enumerate_device_extension_properties(pdevice)?,
         );
-        if surface.is_some() {
+        extensions.try_add(khr::Swapchain::name());
-            extensions.try_add(khr::Swapchain::name());
-        }
         if has_descriptor_indexing {
             extensions.try_add(vk::KhrMaintenance3Fn::name());
             extensions.try_add(vk::ExtDescriptorIndexingFn::name());
@@ -1421,26 +1441,20 @@ impl Layers {
     }
 }
 
-unsafe fn choose_compute_device(
+unsafe fn choose_device(
     instance: &Instance,
     devices: &[vk::PhysicalDevice],
-    surface: Option<&VkSurface>,
 ) -> Option<(vk::PhysicalDevice, u32)> {
     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
+            // Select a device that supports both compute and graphics workloads.
-            if let Some(surface) = surface {
+            // This function used to check for surface compatibility but that was removed
-                if !surface
+            // to allow device creation without an instantiated surface. This follows from
-                    .surface_fn
+            // both Metal and DX12 which do not require such validation. It might be worth
-                    .get_physical_device_surface_support(*pdevice, ix as u32, surface.surface)
+            // exposing this to the user in a future device enumeration API, which would
-                    .unwrap()
+            // also allow selection between discrete and integrated devices.
-                {
+            if info.queue_flags.contains(vk::QueueFlags::COMPUTE | vk::QueueFlags::GRAPHICS) {
-                    continue;
-                }
-            }
-
-            if info.queue_flags.contains(vk::QueueFlags::COMPUTE) {
                 return Some((*pdevice, ix as u32));
             }
         }

piet-gpu/bin/android.rs

@@ -13,8 +13,8 @@ use ndk::native_window::NativeWindow;
 use ndk_glue::Event;
 
 use piet_gpu_hal::{
-    CmdBuf, Error, ImageLayout, Instance, QueryPool, Semaphore, Session, SubmittedCmdBuf, Surface,
+    CmdBuf, Error, ImageLayout, Instance, InstanceFlags, QueryPool, Semaphore, Session,
-    Swapchain,
+    SubmittedCmdBuf, Surface, Swapchain,
 };
 
 use piet::kurbo::Point;
@@ -54,9 +54,9 @@ fn my_main() -> Result<(), Error> {
                         let width = window.width() as usize;
                         let height = window.height() as usize;
                         let handle = get_handle(window);
-                        let (instance, surface) = Instance::new(Some(&handle), Default::default())?;
+                        let instance = Instance::new(InstanceFlags::default())?;
-                        gfx_state =
+                        let surface = unsafe { instance.surface(&handle)? };
-                            Some(GfxState::new(&instance, surface.as_ref(), width, height)?);
+                        gfx_state = Some(GfxState::new(&instance, Some(&surface), width, height)?);
                     } else {
                         println!("native window is sadly none");
                     }
@@ -100,7 +100,7 @@ impl GfxState {
         height: usize,
     ) -> Result<GfxState, Error> {
         unsafe {
-            let device = instance.device(surface)?;
+            let device = instance.device()?;
             let swapchain = instance.swapchain(width, height, &device, surface.unwrap())?;
             let session = Session::new(device);
             let current_frame = 0;

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::default())?;
     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 {