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

Merge function pointer table struct with high level wrapper struct #876

Closed
Friz64 opened this issue Mar 16, 2024 · 4 comments · Fixed by #894
Closed

Merge function pointer table struct with high level wrapper struct #876

Friz64 opened this issue Mar 16, 2024 · 4 comments · Fixed by #894
Milestone

Comments

@Friz64
Copy link
Contributor

Friz64 commented Mar 16, 2024

-- Using VK_KHR_swapchain as an example:

I think it's pretty confusing to have both ash::extensions::khr::Swapchain and ash::vk::KhrSwapchainFn. Is there a good justification for why the wrapper functions shouldn't be directly defined on the struct that also holds the function pointers?

I originally alluded to this in this comment: #734 (comment).

The Device handle would be a bit awkwardly grouped with the function pointers, but overall it results in an easier to grok API, especially after the changes in #734.

@Ralith
Copy link
Collaborator

Ralith commented Mar 16, 2024

I'm weakly in favor of this, since very few users will care about the *Fn structs, I've seen them cause confusion/impair discoverability in the wild, and I can't think of a good reason to want a *Fn struct that isn't also satisfied by a unified struct.

One case that might call for a raw function table struct is storing wrapped functions in the implementation of a layer, in which storing the handle is (usually?) going to be redundant to the passed-in handle -- but even there, having access to the ergonomic wrappers is more valuable than saving a few bytes.

@Ralith
Copy link
Collaborator

Ralith commented Mar 26, 2024

#894 unifies the module hierarchies. It does not unify the structs, but since they're now clearly documented and immediately adjacent, confusion from those should be at least lessened. Once it's merged, making the function pointer tables themselves private, if we want that, would be a trivial addition.

@Rua
Copy link
Contributor

Rua commented Mar 27, 2024

I hope the function pointer tables, and everything else in the vk module, remain available for those who just want to use the raw Vulkan API without any additional wrappers (e.g. Vulkano). I think in the past, someone proposed separating out the vk module into its own crate, containing only autogenerated code?

@MarijnS95
Copy link
Collaborator

@Rua the proposed PR that will close this issue does exactly that: it moves the original types around to sit inside the same module, but otherwise keeps their original definition the same. You are still able to use the function pointer loaders without the Instance/Device wrappers.

@MarijnS95 MarijnS95 added this to the Ash 0.38 milestone Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants