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

vk-sys should be autogenerated from vk.xml #89

Closed
cwabbott0 opened this issue May 7, 2016 · 19 comments
Closed

vk-sys should be autogenerated from vk.xml #89

cwabbott0 opened this issue May 7, 2016 · 19 comments

Comments

@cwabbott0
Copy link

Instead of hand-coding all the definitions, they should be parsed from the same XML definitions that are used to create vulkan.h. This should also add support for the recent extensions for free. I have some WIP code that parses it and generates something that at least seems like rust, I'll try and beat it into shape so it can autogenerate vk-sys/lib.rs.

@tomaka
Copy link
Member

tomaka commented May 8, 2016

There's also the vk_generator crate which was written for this purpose.

@tomaka
Copy link
Member

tomaka commented May 9, 2016

See also this issue: KhronosGroup/Vulkan-Docs#210

@kvark
Copy link

kvark commented Feb 1, 2018

I'd like to bump this in priority, if possible, in relation to #525. Being able to run on Metal, D3D12, and even GL, would vastly expand the reach-ability of Vulkano and remove the blockers potential users may see in this library.

We can't replace the existing vk-sys types directly, but we could easily get away with changing the generation parameters if it was done automatically with vk_generator. Also see the issue on the other side - Osspial/vk-rs#10

@tomaka
Copy link
Member

tomaka commented Feb 10, 2018

This is blocked by Osspial/vk-rs#7, Osspial/vk-rs#8 and Osspial/vk-rs#11

@kvark
Copy link

kvark commented Feb 15, 2018

@tomaka all those vk-rs blockers are resolved now 🎉

@Osspial
Copy link

Osspial commented Feb 15, 2018

I just published vk_generator 0.3, which should resolve any issues blocking vulkano using it to autogenerate the functions.

@Osspial
Copy link

Osspial commented Feb 15, 2018

I'm in the process of writing a PR to bring vk-sys over to using vk_generator, and there are a couple of sticking points:

  • For bitflag types, the generator currently renames types ending with FlagBits to Flags, while the spec has both. This was intentional in an attempt to force consistency, but in retrospect it wasn't a good idea - the spec is weirdly inconsistent about which version it uses in types that have both variants, but it's probably best to keep both. I've patched the generator to not to this.
  • vk_generator automatically wraps Vulkan handles in structs to add some simple type-checking, but vulkano's VulkanHandle trait is implemented on raw numeric types. I'm personally for manually implementing VulkanHandle on these types, but adding a flag to disable the wrapping shouldn't be hard if it's necessary.
  • vk-sys has separate structs for holding different categories of function pointers (static functions, entrypoint functions, etc.). There's no good way to replicate this behavior in vk_generator, because
    • The XML registry only distinguishes between these functions with a few comment tags, which I really don't want to rely upon.
    • The XML's categories are different than vk-sys's categories, rendering any attempt to read the comments pointless anyway.

@tomaka
Copy link
Member

tomaka commented Feb 16, 2018

@Osspial I'd prefer to leave vk-sys untouched and switch vulkano directly to vk_generator.
Your last point is important but I think I can go with hardcoding the necessary function names in vulkano.

@kvark
Copy link

kvark commented May 8, 2018

@Osspial @tomaka where are we on that issue?
We are increasingly more eager to run Vulkano on top of gfx-hal without going through C boundary.

@Osspial
Copy link

Osspial commented May 8, 2018

I've pushed some code that should resolve the Vulkan handle issue, but that isn't on crates.io yet. AFAIK that was the last issue in vk_generator that needed to be resolved, and now it's just a matter of implementing the changes within vulkano.

@Osspial
Copy link

Osspial commented May 9, 2018

Began migrating the code! This still isn't fully working, as I haven't gotten the multiple-structs-for-different-function-categories thing resolved, but most other changes needed to get this working within Vulkano have been made.

I'll submit a PR when that's ready. My instinct here is to share the function pointers between all the structs that need them with a Rc<Vk> - @tomaka, do you see any issues with that?

https://github.com/Osspial/vulkano/tree/vk-sys-autogenerated

@Osspial
Copy link

Osspial commented May 9, 2018

Actually, scratch that - I can see a reasonably simple way to automatically split up the function pointer structs now. Besides a few entry-point functions (GetInstanceProcAddr, CreateInstance, EnumerateInstanceExtensionProperties, and EnumerateInstanceLayerProperties), I'm pretty sure every Vulkan function takes a dispatchable handle as the first parameter. I can set up vk_generator to make a function struct for each dispatchable handle (so VkInstance gets VkInstanceFns, VkDevice gets VkDeviceFns, etc.). There are relatively few dispatchable handles so that should pretty easy to manage, and would let vulkano use a function loading system that's similar to what's there today. Thoughts?

@tomaka
Copy link
Member

tomaka commented May 15, 2018

@Osspial I'm not 100% certain that this is an entirely robust solution, but I'm willing to give it a go.

@knappador
Copy link
Contributor

@Osspial what's your understanding of what remains to be done here? I don't think anyone's driving at the moment. The work does sound like a valuable addition.

@Osspial
Copy link

Osspial commented Apr 20, 2019

@knappador It probably wouldn't be difficult to implement, but I haven't thought about vk-rs for a while since I've got a whole bunch of other projects I've been focusing on. I'd accept PRs to implement it (and would happily mentor anyone trying to do it since vk-rs was one of my first big Rust projects and the code's kinda messy), but I don't have the time to do it myself right now.

@knappador
Copy link
Contributor

@Osspial can you dump off any dirty work in a spiked PR or just write up the process you expect needs to be completed? I'm finishing up splitting the shaderc crate and can likely take on the groundwork if the design is laid out. If there's too much work, delegate! =)

@Osspial
Copy link

Osspial commented May 14, 2019

Sorry it took a few weeks to get back on this!

Adding the feature should be fairly simple - you need to modify the gen_struct function to generate different structs based on the first parameter in the function and its type. You probably also need to modify the struct generation macro based on whether the struct contains the bootstrapping functions or the handle-based functions.

Let me know if any more questions come up if you get around to implementing this. I'll aim to respond sooner than I did this time.

gurchetansingh added a commit to gurchetansingh/vulkano that referenced this issue Jan 6, 2021
This updates the structure types to a VK 1.2-ish state.  Long term,
it makes a ton of sense to autogenerate vk-sys to make adding
additional features and enumerations easier [1].  For now, we can
hand edit.

[1] (vulkano-rs#89)
gurchetansingh added a commit to gurchetansingh/vulkano that referenced this issue Jan 6, 2021
This updates the structure types to a VK 1.2-ish state.  Long term,
it makes a ton of sense to autogenerate vk-sys to make adding
additional features and enumerations easier [1].  For now, we can
hand edit.

[1] (vulkano-rs#89)
gurchetansingh added a commit to gurchetansingh/vulkano that referenced this issue Jan 6, 2021
This updates the structure types to a VK 1.2-ish state.  Long term,
it makes a ton of sense to autogenerate vk-sys to make adding
additional features and enumerations easier [1].  For now, we can
hand edit.

[1] (vulkano-rs#89)
gurchetansingh added a commit to gurchetansingh/vulkano that referenced this issue Jan 8, 2021
This updates the structure types to a VK 1.2-ish state.  Long term,
it makes a ton of sense to autogenerate vk-sys to make adding
additional features and enumerations easier [1].  For now, we can
hand edit.

[1] (vulkano-rs#89)
Eliah-Lakhin pushed a commit that referenced this issue Jan 9, 2021
* vulkano: image: improve formatting

Ran cargo fmt --all.  Should we use clippy too ?

* vk_sys: add additional formats

This updates the vk formats enum to the header that's on my
system (vk 1.2-ish).

* vulkano: image: Add NV12 and YV12 support

These formats are commonly used as targets for hardware and
software video decode.  The common case is the swapchain allocator
(gralloc in the Android use case) allocates some memory, the video
stack decodes to it, and then memory can be composited by Vulkan
or sent directly to the display.

* vk_sys: update structure types

This updates the structure types to a VK 1.2-ish state.  Long term,
it makes a ton of sense to autogenerate vk-sys to make adding
additional features and enumerations easier [1].  For now, we can
hand edit.

[1] (#89)

* vk_sys: add VK_KHR_external_memory_fd bindings

This adds some basic external memory features.  Most of these
features are core in VK1.1.

* vulkano: memory: add and use DeviceMemoryBuilder

We'll need to:

(a) Create exportable memory
(b) import from a OS descriptor to create DeviceMemory
(c) Also support dedicated allocations

The device memory API is becoming rather complicated.  Let's use
the common builder pattern to simplify this.

* vulkano: memory: implement some external features

This change is sufficient to create exportable memory and
export it.  It's only been tested on Linux platforms and depends
on Unix file descriptors, so enable it only there for now.

Import support will be added later.  Support for external buffers
and images can also be added later if someone needs it.
@Rua
Copy link
Contributor

Rua commented Feb 28, 2021

How is this project going, two years onwards? Does it work? Can Vulkano use it?

@Rua
Copy link
Contributor

Rua commented May 12, 2022

Vulkano now uses ash as its backend, so this is no longer needed.

@Rua Rua closed this as completed May 12, 2022
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

No branches or pull requests

6 participants