-
Notifications
You must be signed in to change notification settings - Fork 190
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
Add InstanceExtension and DeviceExtension traits #912
base: master
Are you sure you want to change the base?
Conversation
What's the use case for these traits that justifies the added maintenance load and possible build time cost? |
In my case I manage extensions by storing all of them in a This proposal requires minimal changes to the generator and it will not add any additional maintenance burden or build time cost. We're not creating new impls; we're just converting existing inherent impls into trait impls. impl Device {
fn new(instance: &crate::Instance, device: &crate::Device) -> Self { .. }
fn fp(&self) -> &DeviceFn { .. }
fn device(&self) -> crate::vk::Device { .. }
} to impl DeviceExtension for Device {
fn new(instance: &crate::Instance, device: &crate::Device) -> Self { .. }
fn fp(&self) -> &DeviceFn { .. }
fn device(&self) -> crate::vk::Device { .. }
} For convenience, we also added the extension name, spec version, and promotion status to the trait definitions. In general, I believe that this change is well justified to be in ash. For a third party library to provide similar functionality, trait implementation will need to be added for each extension alongside their name, spec version and promotion status by hand. Meanwhile, this information is readily available for ash and can be provided to the application with the addition of just 2 new trait definitions.
Yes, #614 added To summarize, similar to how the |
Not true, these are also referenced inside Note that #734 made a big effort to create per-prefix and per-extension I think Separate from that, we can discuss how to best model such "extension implementation introspection" on |
This, in hindsight, makes me wonder if it was ever designed/desired for the |
Ok sure, but same idea here -
The key issue here is that we cannot implement traits for modules, so app.enable_extension(
ash::khr::acceleration_structure::NAME,
ash::khr::acceleration_structure::PROMOTION_STATUS,
ash::khr::acceleration_structure::Device::new,
) compared to app.enable_extension::<ash::khr::acceleration_structure::Device>() In #913 I added a new type,
My bad, should've been |
I will keep this PR open for now. If we end up deciding to adopt #913 we can close this. |
Create new marker traits
InstanceExtension
andDeviceExtension
as defined below:These traits are automatically implemented for all extension fp structs.
Traits like these make it easier to manage extensions in higher level abstractions. Marker traits like these are not unprecedented: we already have
TaggedStructure
andExtends*
traits, and the changes proposed in this PR are similar in spirit.cc #734