ash
ash copied to clipboard
ash-rs
Implement extensions directly on Instance/Device instead?
Currently, Ash has separate structures for each extension, which means each one has to be loaded and kept around by the user. This is a bit awkward if there's many extensions involved, so what if the extension functions were implemented directly on Instance and Device instead? The user would only have to keep these two structures around for all their use cases.
These two objects would initialise the function pointers of all extensions internally when they are created. This is allowed by the spec; the spec dictates that asking for the pointer to an extension function that has not been requested at create time should return a null pointer. Ash itself already does this for the Vulkan core functions, which are all loaded regardless of whether the user created their Instance for that version.
If this is done, I also suggest getting rid of the DeviceV1_0 trait and others like it, and implementing functions (both core and extension) as inherent methods on Instance and Device. These traits add extra flexibility but I'm not sure if that's actually needed for most people. And if each extension is going to be represented as a trait on Instance or Device, those two types end up with a very long list of trait implementations. You could keep the functions for returning the raw function pointer structs as they are now, or expose each function struct member of Instance and Device to the user directly; the latter has my preference.
Yes this is something I have been considering for a while and will most likely change with #344
Would it be ok if I made this change manually in a PR for now? Assuming the generator rewrite will take a while.
Would it be ok if I made this change manually in a PR for now? Assuming the generator rewrite will take a while.
That would be nice
You could keep the functions for returning the raw function pointer structs as they are now
This would be my preference, keep it as it is now and just move the trait impls to a normal impl block.
@MaikKlein @MarijnS95 @Ralith now that the PR has been split and the first one merged, what do you think about the details about this proposal? Some question points are:
- It's a major breaking change (of course).
- Including function pointers for every extension makes
Devicevery large. - By loading functions of extensions that aren't enabled, you end up with null pointer stubs. Not a problem as long as you check that the extension is enabled before calling them. This applies to the current structs too.
- Ergonomics, the reason I wanted to create this PR in the first place. With many extensions to handle (e.g. Vulkano), you have to create all the structs and keep them around, passing them along with
DeviceorInstanceevery time. Ash has nothing to help you here. - Vulkano's current
vk-sysfollows this model. Having Ash work the same way would make the planned port of Vulkano to Ash easier.
Point 2 only matters if you ever decide to copy it, but is that done anywhere? And does it need to be?
Point 4 doesn't necessarily mean that extensions should all be added to Device and Instance like I propose. There are other options, such as unifying all extensions into a separate DeviceExtensions struct, or anything else that helps users load and keep hold of the function pointers. Ideas are welcome.
you have to create all the structs and keep them around, passing them along with Device or Instance every time. Ash has nothing to help you here.
We simply have a struct Device in our codebase holding on to ash::Device, ash::Instance, all these extensions and whatever other muck that's necessary. IMO applications should do the same instead of forcing them to have a massive struct and attempt to load every function (can potentially be costly due to initialization, confuse layers and whatnot).
You can even Deref that struct into ash::Device if you wish to access the functions directly.
If you really want to solve this inside Ash a separate struct holding on to the device and all extension function pointers seems like the best way forward.
Point 2 only matters if you ever decide to copy it, but is that done anywhere? And does it need to be?
Unfortunately our codebase does that a lot, because lifetimes (and I guess no-one realised it's currently over 1000 bytes large). We should probably put the device on the heap and (A)Rc it already, but meh :shrug:
One thing of relevance is that you are perhaps concerned with every extension struct individually holding on to a redundant device/instance handle?
I remain a fan of how Option<Extension> makes it easy to correctly handle extensions that you support but do not require, which comes up constantly with the debug extension.
can potentially be costly due to initialization, confuse layers and whatnot
I'm skeptical that these are a realistic threat.
I remain a fan of how
Option<Extension>makes it easy to correctly handle extensions that you support but do not require, which comes up constantly with the debug extension.
But I do believe that is something for the crate user to specify in their own code. I don't want to expect/unwrap Option anywhere (or store a duplicate without Option outside of ash::Device) if there are certain extensions that my application requires and does not / should not start without. That's not ergonomic IMO.
I'm skeptical that these are a realistic threat.
I'm not sure whether drivers prefer to perform extra loading/work during instance/device creation when the extensions are passed, or when a function is loaded. Neither seem to be slow on any driver that I've tried but given past/open issues Ash is used in quite a few embedded scenarios for which we should provide an as bare-bones setup as possible, IMO.
But I do believe that is something for the crate user to specify in their own code
Yes, strongly agree.
Looks like this was addressed in https://github.com/ash-rs/ash/pull/412, and there's no strong precedent for creating mega-structs containing all extension pfns, loaded from the get-go.