Merge pull request #184 from dfrg/multi-surface
Separate Instance and Surface creation in HAL
diff --git a/piet-gpu-hal/examples/collatz.rs b/piet-gpu-hal/examples/collatz.rs
index 7aff938..afb3d27 100644
--- a/piet-gpu-hal/examples/collatz.rs
+++ b/piet-gpu-hal/examples/collatz.rs
@@ -2,9 +2,9 @@
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>>();
diff --git a/piet-gpu-hal/src/dx12.rs b/piet-gpu-hal/src/dx12.rs
index a9e6070..01afcfd 100644
--- a/piet-gpu-hal/src/dx12.rs
+++ b/piet-gpu-hal/src/dx12.rs
@@ -130,12 +130,7 @@
impl Dx12Instance {
/// Create a new instance.
- ///
- /// 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> {
+ pub fn new() -> Result<Dx12Instance, Error> {
unsafe {
#[cfg(debug_assertions)]
if let Err(e) = wrappers::enable_debug_layer() {
@@ -151,23 +146,25 @@
let factory = Factory4::create(factory_flags)?;
- 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))
+ Ok(Dx12Instance { factory })
+ }
+ }
+
+ /// 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 _;
+ Ok(Dx12Surface { hwnd })
+ } else {
+ Err("can't create surface for window handle".into())
}
}
/// Get a device suitable for compute workloads.
- ///
- /// TODO: handle window.
- /// TODO: probably can also be trait'ified.
- pub fn device(&self, _surface: Option<&Dx12Surface>) -> Result<Dx12Device, Error> {
+ pub fn device(&self) -> Result<Dx12Device, Error> {
unsafe {
let device = Device::create_device(&self.factory)?;
let list_type = d3d12::D3D12_COMMAND_LIST_TYPE_DIRECT;
diff --git a/piet-gpu-hal/src/metal.rs b/piet-gpu-hal/src/metal.rs
index 2918df0..891a9be 100644
--- a/piet-gpu-hal/src/metal.rs
+++ b/piet-gpu-hal/src/metal.rs
@@ -133,20 +133,19 @@
}
impl MtlInstance {
- pub fn new(
- window_handle: Option<&dyn HasRawWindowHandle>,
- ) -> 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);
- }
- }
- }
+ pub fn new() -> Result<MtlInstance, Error> {
+ Ok(MtlInstance)
+ }
- 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 @@
// 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))
diff --git a/piet-gpu-hal/src/mux.rs b/piet-gpu-hal/src/mux.rs
index 97c65c8..cab97e6 100644
--- a/piet-gpu-hal/src/mux.rs
+++ b/piet-gpu-hal/src/mux.rs
@@ -114,17 +114,14 @@
}
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(
- window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>,
- flags: InstanceFlags,
- ) -> Result<(Instance, Option<Surface>), Error> {
+ pub fn new(flags: InstanceFlags) -> Result<Instance, Error> {
let mut backends = [BackendType::Vulkan, BackendType::Dx12];
if flags.contains(InstanceFlags::DX12) {
backends.swap(0, 1);
@@ -134,9 +131,8 @@
mux_cfg! {
#[cfg(vk)]
{
- let result = vulkan::VkInstance::new(window_handle);
- if let Ok((instance, surface)) = result {
- return Ok((Instance::Vk(instance), surface.map(Surface::Vk)));
+ if let Ok(instance) = vulkan::VkInstance::new() {
+ return Ok(Instance::Vk(instance));
}
}
}
@@ -145,9 +141,8 @@
mux_cfg! {
#[cfg(dx12)]
{
- let result = dx12::Dx12Instance::new(window_handle);
- if let Ok((instance, surface)) = result {
- return Ok((Instance::Dx12(instance), surface.map(Surface::Dx12)));
+ if let Ok(instance) = dx12::Dx12Instance::new() {
+ return Ok(Instance::Dx12(instance))
}
}
}
@@ -156,9 +151,8 @@
mux_cfg! {
#[cfg(mtl)]
{
- let result = metal::MtlInstance::new(window_handle);
- if let Ok((instance, surface)) = result {
- return Ok((Instance::Mtl(instance), surface.map(Surface::Mtl)));
+ if let Ok(instance) = metal::MtlInstance::new() {
+ return Ok(Instance::Mtl(instance));
}
}
}
@@ -166,16 +160,28 @@
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::Dx12(i) => i.device(surface.map(Surface::dx12)).map(Device::Dx12),
- Instance::Mtl(i) => i.device(surface.map(Surface::mtl)).map(Device::Mtl),
+ Instance::Vk(i) => i.device().map(Device::Vk),
+ Instance::Dx12(i) => i.device().map(Device::Dx12),
+ Instance::Mtl(i) => i.device().map(Device::Mtl),
}
}
diff --git a/piet-gpu-hal/src/vulkan.rs b/piet-gpu-hal/src/vulkan.rs
index 7e01319..a2e2371 100644
--- a/piet-gpu-hal/src/vulkan.rs
+++ b/piet-gpu-hal/src/vulkan.rs
@@ -154,12 +154,7 @@
///
/// 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(
- window_handle: Option<&dyn raw_window_handle::HasRawWindowHandle>,
- ) -> Result<(VkInstance, Option<VkSurface>), Error> {
+ pub fn new() -> Result<VkInstance, Error> {
unsafe {
let app_name = CString::new("VkToy").unwrap();
let entry = Entry::new()?;
@@ -175,12 +170,32 @@
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)? {
- exts.try_add(ext);
- }
+
+ // Enable platform specific surface extensions.
+ 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 @@
(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 @@
_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
- /// 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 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 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 @@
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 @@
}
}
-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
- 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(vk::QueueFlags::COMPUTE) {
+ // 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));
}
}
diff --git a/piet-gpu/bin/android.rs b/piet-gpu/bin/android.rs
index d1c4749..8f3fa18 100644
--- a/piet-gpu/bin/android.rs
+++ b/piet-gpu/bin/android.rs
@@ -13,8 +13,8 @@
use ndk_glue::Event;
use piet_gpu_hal::{
- CmdBuf, Error, ImageLayout, Instance, QueryPool, Semaphore, Session, SubmittedCmdBuf, Surface,
- Swapchain,
+ CmdBuf, Error, ImageLayout, Instance, InstanceFlags, QueryPool, Semaphore, Session,
+ SubmittedCmdBuf, Surface, Swapchain,
};
use piet::kurbo::Point;
@@ -54,9 +54,9 @@
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())?;
- gfx_state =
- Some(GfxState::new(&instance, surface.as_ref(), width, height)?);
+ let instance = Instance::new(InstanceFlags::default())?;
+ let surface = unsafe { instance.surface(&handle)? };
+ gfx_state = Some(GfxState::new(&instance, Some(&surface), width, height)?);
} else {
println!("native window is sadly none");
}
@@ -100,7 +100,7 @@
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;
diff --git a/piet-gpu/bin/cli.rs b/piet-gpu/bin/cli.rs
index df86158..6257ebf 100644
--- a/piet-gpu/bin/cli.rs
+++ b/piet-gpu/bin/cli.rs
@@ -226,9 +226,9 @@
.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();
diff --git a/piet-gpu/bin/winit.rs b/piet-gpu/bin/winit.rs
index 8438371..7d4126e 100644
--- a/piet-gpu/bin/winit.rs
+++ b/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 @@
.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 mut swapchain =
- instance.swapchain(WIDTH / 2, HEIGHT / 2, &device, surface.as_ref().unwrap())?;
+ let surface = instance.surface(&window)?;
+ let device = instance.device()?;
+ let mut swapchain = instance.swapchain(WIDTH / 2, HEIGHT / 2, &device, &surface)?;
let session = Session::new(device);
let mut current_frame = 0;
diff --git a/tests/src/runner.rs b/tests/src/runner.rs
index 3ba8223..0760f59 100644
--- a/tests/src/runner.rs
+++ b/tests/src/runner.rs
@@ -45,8 +45,8 @@
impl Runner {
pub unsafe fn new(flags: InstanceFlags) -> Runner {
- let (instance, _) = Instance::new(None, flags).unwrap();
- let device = instance.device(None).unwrap();
+ let instance = Instance::new(flags).unwrap();
+ let device = instance.device().unwrap();
let session = Session::new(device);
let cmd_buf_pool = Vec::new();
Runner {
