Raw window handle integration

msiglreith commented 5 years ago

Background

There is some currently ongoing work for interoperability library, which provides a small interface for window and graphic APIs: https://github.com/rust-windowing/raw-window-handle

Status

On discord there where a bit of discussion on ash support already but apparently there was some sort of conclusion that it not aligns perfectly due to extension handling IIRC (see https://github.com/rust-gamedev/wg/issues/26)

Nonetheless I though about have some more structured discussion going on here if this interface would fit in the library.

Proposal

First proposal for a possible API w/ implementation: https://github.com/norse-rs/ash-window If it won't be intergrated in ash I'm fine with maintaining the intergration stuff in the external library as well.

/// Create a surface from a raw surface handle.
///
/// `instance` must have created with platform specific surface extensions enabled.
pub unsafe fn create_surface(
    entry: &ash::Entry,
    instance: &ash::Instance,
    window_handle: &impl HasRawWindowHandle,
    allocation_callbacks: Option<&vk::AllocationCallbacks>,
) -> VkResult<vk::SurfaceKHR> {
   ... 
}

/// Query the required instance extension for creating a surface from a window handle.
pub fn enumerate_required_extension(
     window_handle: &impl HasRawWindowHandle
) -> &'static CStr {
   ...
}

Open Points

Android support missing (https://github.com/rust-windowing/raw-window-handle/issues/12)
IOS/MacOS via higher level route, requires manually extracting/creating the CMetalLayer from the passed views if want to target the Metal Vulkan extensions.
win32: instance needs to be manually retrieved (https://github.com/rust-windowing/raw-window-handle/issues/2)
no XCB only Xlib but not sure how relevant this is at all (https://github.com/rust-windowing/raw-window-handle/issues/11)

Ralith commented 5 years ago

I'd make enumerate_required_extension plural for future-proofing, and make create_surface generic over Entry/Device impls, for starters.

Didgy74 commented 5 years ago

One thing I'd like to see discussed is how you'd handle a user already having made their own surface-loading structures. For example when you've already instantiated ash::extensions::khr::Win32Surface. As far as I can tell, this interface would require Ash to rebuild this structure for each call.

If you made an alternative function that took in an already-instantiated surface loader, you could omit the entry and instance parameters.

Regarding the open points, I've already submitted a PR to raw-window-handle that should solve the issues regarding Android and Xcb support missing.

Didgy74 commented 5 years ago

Quick note before I talk about something else, the window_handle parameter might as well just be normal RawWindowHandle. The type became copyable in a recent commit afaik, and I see little reason to pass in a type that implements the HasRawWindowHandle trait rather than just passing the actual RawWindowHandle structure.

Anyways, onto my next topic: I've been thinking about how to solve the disadvantage presented in my previous comment. So the main issue is you can't accept a RawWindowHandle and for example a Win32Surface structure, since you've no idea which surface-loader structure is going to be needed and passing Options for each possible sounds way too cumbersome. So I've got the following interface proposal:

Create an enum, where each variant represents an OS and holds the structs for all possible windowing-systems for that OS:

struct UNIXSurfaceLoaders {
    xcb: ash::extensions::khr::XcbSurface,
    xlib: ash::extensions::khr::XlibSurface,
    wayland: ash::extensions::khr::WaylandSurface
}

enum SurfaceLoader {
    UNIX(UNIXSurfaceLoaders),
    WINDOWS(ash::extensions::khr::Win32Surface),
    ANDROID(ash::extensions::khr::AndroidSurface),
    // And so on
}

fn ash::create_surface(
    surface_loader: &SurfaceLoader, 
    raw_handle: RawWindowHandle
) -> vk::Result<vk::SurfaceKHR> { 
    ...
}

fn ash::build_surface_loader(entry: &impl ash::Entry) -> SurfaceLoader;

Here create_surface will just match the platform returned in RawWindowHandle and use the corresponding surface loader.

This way the user could load all the possible surface loaders at initialization, and reuse the structure later in the application when making for example new viewports or windows.

One drawback with this approach, it will only work if the user loads all instance extensions for these surface loaders. For example when running the app on UNIX, the user has to load VK_KHR_xcb_surface, VK_KHR_xlib_surface and VK_KHR_wayland_surface.

Ralith commented 5 years ago

It's better to take &impl HasRawWindowHandle because that way the user does not need to add a direct dependency on the raw-window-handle crate and import the trait to benefit from the API.

On Tue, Aug 27, 2019, 13:02 Nils Petter Skålerud notifications@github.com wrote:

Quick note before I talk about something else, the window_handle parameter might as well just be normal RawWindowHandle. The type became copyable in a recent commit afaik, and I see little reason to pass in a type that implements the HasRawWindowHandle trait rather than just passing the actual RawWindowHandle structure.

Anyways, onto my next topic: I've been thinking about how to solve the disadvantage presented in my previous comment. So the main issue is you can't accept a RawWindowHandle and for example a Win32Surface structure, since you've no idea which surface-loader structure is going to be needed and passing Options for each possible sounds way too cumbersome. So I've got the following interface proposal:

Create an enum, where each variant represents an OS and holds the structs for all possible windowing-systems for that OS:

struct UNIXSurfaceLoaders { xcb: ash::extensions::khr::XcbSurface, xlib: ash::extensions::khr::XlibSurface, wayland: ash::extensions::khr::WaylandSurface } enum SurfaceLoader { UNIX(UNIXSurfaceLoaders), WINDOWS(ash::extensions::khr::Win32Surface), ANDROID(ash::extensions::khr::AndroidSurface), // And so on } fn ash::create_surface( surface_loader: &SurfaceLoader, raw_handle: RawWindowHandle ) -> vk::Result { ... } fn ash::build_surface_loader(entry: &impl ash::Entry) -> SurfaceLoader;

Here create_surface will just match the platform returned in RawWindowHandle and use the corresponding surface loader.

This way the user could load all the possible surface loaders at initialization, and reuse the structure later in the application when making for example new viewports or windows.

One drawback with this approach, it will only work if the user loads all instance extensions for these surface loaders. For example when running the app on UNIX, the user has to load VK_KHR_xcb_surface, VK_KHR_xlib_surface and VK_KHR_wayland_surface.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/MaikKlein/ash/issues/232?email_source=notifications&email_token=AAAZQ3XJSTXWINWXHBGYXE3QGWB65A5CNFSM4IMTQJ5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5I6GQA#issuecomment-525460288, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZQ3RYUDI6KJBDXTLPINTQGWB65ANCNFSM4IMTQJ5A .

msiglreith commented 5 years ago

@Didgy74 thanks for the feedback!

The type became copyable in a recent commit afaik, and I see little reason to pass in a type that implements the HasRawWindowHandle trait rather than just passing the actual RawWindowHandle structure.

As @Ralith mentioned it's more convenient from the user side.

Regarding the surface loader reuse topic I'm wondering if the surface extensions should just not then implement a corresponding function for creating the surface from a handle rather? I would argue that the current proposed API matches the needs of most of the users, but it could be extended ofc.

Didgy74 commented 5 years ago

I agree that the currently proposed interface should be fine for most usecases, since most users will only be generating one VkSurfaceKHR anyways. I was just wanting to see possible solutions for making it faster when making multiple VkSurfaceKHR objects, and if any of those solutons would be feasible to include in the interface as an addition.

VZout commented 4 years ago

@MaikKlein is potentially interested in implementing RawWindowHandle support into Ash itself. @msiglreith Would you be interested in creating a pull request? If not this is something I can do as well.

msiglreith commented 4 years ago

@VZout Glady (: I will setup a PR for discussion

ash-rs / ash