Make graphics support non-optional for Vulkan

This also removes the new present flag, deletes some commented code and explains the lack of surface validation in device selection.

piet-gpu-hal/src/lib.rs

@@ -49,8 +49,6 @@ 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/mux.rs

@@ -131,7 +131,7 @@ impl Instance {
                 mux_cfg! {
                     #[cfg(vk)]
                     {
-                        if let Ok(instance) = vulkan::VkInstance::new(flags.contains(InstanceFlags::PRESENT)) {
+                        if let Ok(instance) = vulkan::VkInstance::new() {
                             return Ok(Instance::Vk(instance));
                         }
                     }
@@ -179,7 +179,7 @@ impl Instance {
     /// a "session" which is similar but provides many conveniences.
     pub unsafe fn device(&self) -> Result<Device, Error> {
         mux_match! { self;
-            Instance::Vk(i) => i.device(true).map(Device::Vk),
+            Instance::Vk(i) => i.device().map(Device::Vk),
             Instance::Dx12(i) => i.device().map(Device::Dx12),
             Instance::Mtl(i) => i.device().map(Device::Mtl),
         }

piet-gpu-hal/src/vulkan.rs

@@ -154,10 +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.
-    ///
-    /// The caller is responsible for making sure that window which owns the raw window handle
-    /// outlives the surface.
-    pub fn new(support_present: bool) -> Result<VkInstance, Error> {
+    pub fn new() -> Result<VkInstance, Error> {
         unsafe {
             let app_name = CString::new("VkToy").unwrap();
             let entry = Entry::new()?;
@@ -174,9 +171,7 @@ impl VkInstance {
                 has_debug_ext = exts.try_add(DebugUtils::name());
             }
 
-            // Enable platform specific surface extensions if presentation
-            // support is requested.
-            if support_present {
+            // Enable platform specific surface extensions.
             exts.try_add(khr::Surface::name());
 
             #[cfg(target_os = "windows")]
@@ -200,7 +195,6 @@ impl VkInstance {
 
             #[cfg(any(target_os = "macos", target_os = "ios"))]
             exts.try_add(kkr::MetalSurface::name());
-            }
 
             let supported_version = entry
                 .try_enumerate_instance_version()?
@@ -270,18 +264,17 @@ impl VkInstance {
         })
     }
 
-    /// Create a device from the instance, suitable for compute, with an optional presentation
-    /// support.
+    /// 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
-    /// and surface. We could enforce that, for example having an `Arc` of the raw instance,
+    /// 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, support_present: bool) -> Result<VkDevice, Error> {
+    pub unsafe fn device(&self) -> Result<VkDevice, Error> {
         let devices = self.instance.enumerate_physical_devices()?;
         let (pdevice, qfi) =
-            choose_device(&self.instance, &devices, support_present).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);
@@ -317,9 +310,7 @@ impl VkInstance {
             self.instance
                 .enumerate_device_extension_properties(pdevice)?,
         );
-        if support_present {
         extensions.try_add(khr::Swapchain::name());
-        }
         if has_descriptor_indexing {
             extensions.try_add(vk::KhrMaintenance3Fn::name());
             extensions.try_add(vk::ExtDescriptorIndexingFn::name());
@@ -1453,32 +1444,17 @@ impl Layers {
 unsafe fn choose_device(
     instance: &Instance,
     devices: &[vk::PhysicalDevice],
-    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() {
-            // TODO: is this strictly necessary? We'll need a queue supporting graphics
-            // for image rendering regardless, and I'm leaning on the assumption that 
-            // all physical device + queue family combinations that support graphics also
-            // support presentation particularly when the appropriate extensions are enabled
-            // at instance creation. This may be faulty.
-
-            // 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) {
+            // Select a device that supports both compute and graphics workloads.
+            // This function used to check for surface compatibility but that was removed
+            // to allow device creation without an instantiated surface. This follows from
+            // both Metal and DX12 which do not require such validation. It might be worth
+            // exposing this to the user in a future device enumeration API, which would
+            // also allow selection between discrete and integrated devices.
+            if info.queue_flags.contains(vk::QueueFlags::COMPUTE | vk::QueueFlags::GRAPHICS) {
                 return Some((*pdevice, ix as u32));
             }
         }

piet-gpu/bin/android.rs

@@ -54,7 +54,7 @@ fn my_main() -> Result<(), Error> {
                         let width = window.width() as usize;
                         let height = window.height() as usize;
                         let handle = get_handle(window);
-                        let instance = Instance::new(InstanceFlags::PRESENT)?;
+                        let instance = Instance::new(InstanceFlags::default())?;
                         let surface = unsafe { instance.surface(&handle)? };
                         gfx_state = Some(GfxState::new(&instance, Some(&surface), width, height)?);
                     } else {

piet-gpu/bin/winit.rs

@@ -57,7 +57,7 @@ fn main() -> Result<(), Error> {
         .with_resizable(false) // currently not supported
         .build(&event_loop)?;
 
-    let instance = Instance::new(InstanceFlags::PRESENT)?;
+    let instance = Instance::new(InstanceFlags::default())?;
     let mut info_string = "info".to_string();
     unsafe {
         let surface = instance.surface(&window)?;