Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ash-window: Add get_present_support() helper #774

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions ash-window/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,69 @@ pub fn enumerate_required_extensions(

Ok(extensions)
}

/// Query whether a `queue_family` of the given `physical_device` supports presenting to any surface that might be created.
/// This function can be used to find a suitable [`vk::PhysicalDevice`] and queue family
/// for rendering before a single surface is created.
MarijnS95 marked this conversation as resolved.
Show resolved Hide resolved
///
/// This function can be a more useful alternative for
/// [`VkGetPhysicalDeviceSurfaceSupportKHR`](https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#vkGetPhysicalDeviceSurfaceSupportKHR),
MarijnS95 marked this conversation as resolved.
Show resolved Hide resolved
/// which requires having an actual surface available before choosing a physical device.
///
/// For more information see [the vulkan spec on WSI integration](https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#_querying_for_wsi_support).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// For more information see [the vulkan spec on WSI integration](https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#_querying_for_wsi_support).
/// For more information see [the Vulkan spec on WSI integration][_querying_for_wsi_support].
///
/// [_querying_for_wsi_support]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#_querying_for_wsi_support

pub fn get_present_support(
entry: &Entry,
instance: &Instance,
physical_device: vk::PhysicalDevice,
queue_family_index: u32,
display_handle: RawDisplayHandle,
) -> VkResult<bool> {
match display_handle {
RawDisplayHandle::Android(_) | RawDisplayHandle::UiKit(_) | RawDisplayHandle::AppKit(_) => {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_android
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_ios
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_macos
// On Android, iOS and macOS, every queue family supports presenting to any surface
Ok(true)
}
RawDisplayHandle::Wayland(h) => unsafe {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_walyand
let ext = khr::WaylandSurface::new(entry, instance);
Ok(ext.get_physical_device_wayland_presentation_support(
physical_device,
queue_family_index,
&mut *h.display.cast(),
MarijnS95 marked this conversation as resolved.
Show resolved Hide resolved
))
},
RawDisplayHandle::Windows(_) => unsafe {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_win32
let ext = khr::Win32Surface::new(entry, instance);
Ok(ext.get_physical_device_win32_presentation_support(
physical_device,
queue_family_index,
))
},
RawDisplayHandle::Xcb(h) => unsafe {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_xcb
let ext = khr::XcbSurface::new(entry, instance);
Ok(ext.get_physical_device_xcb_presentation_support(
physical_device,
queue_family_index,
&mut *h.connection,
MarijnS95 marked this conversation as resolved.
Show resolved Hide resolved
h.screen as _,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supposed to be a visual, not a screen.

))
},
RawDisplayHandle::Xlib(h) => unsafe {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/chap34.html#platformQuerySupport_xlib
let ext = khr::XlibSurface::new(entry, instance);
Ok(ext.get_physical_device_xlib_presentation_support(
physical_device,
queue_family_index,
h.display,
MarijnS95 marked this conversation as resolved.
Show resolved Hide resolved
h.screen as _,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I think a visualID can be somehow derived from the screen, but I think this would require linking to some X library. As an alternative, the get_present_support function could require a window handle, but this would defeat the main use case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. We could pull in x11-dl and use it to make the necessary call (XDefaultVisualOfScreen I guess?). Bonus points for adding a cargo feature to use x11 instead, as dynamic linking is a bit tidier than runtime loading on Linux, but it can be nice to avoid a buildtime dep on a foreign lib.

Alternatively, maybe a more convenient approach would be adding a visual_id member to to raw_window_handle::XlibDisplayHandle upstream (and making sure winit populates it). This is the same motivation behind its inclusion in the window handle struct, though I'm not enough of an X11 expert to judge how weird that might be.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bonus points for adding a cargo feature to use x11 instead, as dynamic linking is a bit tidier than runtime loading on Linux, but it can be nice to avoid a buildtime dep on a foreign lib.

Quickly scrolling through the changes, it seems that this feature is now forcibly requiring either x11 or x11-dl, or a compiler error will occur on all of unix. Same for xcb now being forced on all consumers of ash-window.

The cfg guards on those RawDisplayHandles should be refactored to contain the optional libraries as features, otherwise get_present_support() won't provide support for x11/xcb but the other functions are not affected.

Then, the module documentation and function documentation should be extended to explain these features.

))
},
// All other platforms mentioned in the vulkan spec are not supported by ash.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// All other platforms mentioned in the vulkan spec are not supported by ash.
// All other platforms mentioned in the Vulkan spec are not supported by ash-window.

Because ash maps the entire Vulkan API (== supports everything), even though we might not have written a slightly-higher-level Fn wrapper for it yet the lower-level API is still available.

Though I would reword that to ... don't currently have an implementation in ash-window, as we can always expand.

_ => Err(vk::Result::ERROR_EXTENSION_NOT_PRESENT),
}
}