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

Writing layers in Rust. #573

Open
cheako opened this issue Feb 3, 2022 · 13 comments
Open

Writing layers in Rust. #573

cheako opened this issue Feb 3, 2022 · 13 comments

Comments

@cheako
Copy link

cheako commented Feb 3, 2022

There are a few new strucks not included:

https://github.com/KhronosGroup/Vulkan-LoaderAndValidationLayers/blob/25d5884746a2de7b51a8ef3ec88e1cd8066460e8/include/vulkan/vk_layer.h#L112

It becomes a pattern of descending chains of pnext looking for a particular strict type.

Many tools used in implementing vulkan in rust would be handy.

I am working on an example layer, I expect it to segfault a lot. Edit: It did!

Edit add: vk::StaticFn::load() should take PFN_vkGetInstanceProcAddr, one of these should be changed to match the other.
Edit add: Sometimes create(I.E. create_instance) can't be passed a pointer to zeroed memory.

@cheako cheako changed the title Writing validation layers in Rust. Writing layers in Rust. Feb 3, 2022
@cheako
Copy link
Author

cheako commented Feb 4, 2022

@i509VCB
Copy link
Contributor

i509VCB commented Oct 8, 2022

On this topic, a Handle::is_null function may be useful for ICDs and layers. In vkGetInstanceProcAddr some commands don't get returned unless the instance handle is non-null.

@EHfive
Copy link

EHfive commented Feb 10, 2023

For anyone who interested in writing layers in Rust. I have created a thin layer API binding on top of ash https://github.com/EHfive/ash-layer.
And also a working instance https://github.com/EHfive/pw-capture using ash-layer.

@cheako
Copy link
Author

cheako commented Feb 10, 2023

I'm against projects like ash-layer. To start with the name when you upload it to crates.io you're locking that name globally, I would be more in favor if the name was made-up. Secondly, I don't see why it is separate from ash. For such a small thing, I believe each layer can have a copy of this code until ash exposes these utility.

@EHfive
Copy link

EHfive commented Feb 10, 2023

I don't see problem here.

  1. The Vulkan layer API are only defined by Vulkan loader and are not part of Vulkan specification hence can not be auto generated, so it's not quite fit in ash scope, and it's better to have the layer binding in a separate crate.
  2. No one in ash team seems to be interested in adding some layer binding (you can see from this issue), so it's not unreasonable to let me, who actually cares, hold the ash-layer.
  3. If the project decides to have another "ash-layer" providing layer bindings, I am happy to transfer the ownership of "ash-layer". Or better, just merge "ash-layer" into ash.

@MarijnS95
Copy link
Collaborator

2. No one in ash team seems to be interested in adding some layer binding (you can see from this issue), so it's not unreasonable to let me, who actually cares, hold the ash-layer.

That is not true. We've already tackled the struct-matching problem described above in #614, and I've started playing around with API bindings for layers in https://github.com/Traverse-Research/ash/tree/layer-boilerplate but no cleanup nor PR has materialized yet. Mainly debating whether to aim for a minimal wrapper around the necessary types first, or immediately provide generated helpers to set up dispatch tables by utilizing our generator.

@cheako
Copy link
Author

cheako commented Feb 22, 2023

For 3 it's already an issue on crates.io where abandoned names for different reasons can not be recovered for the foreseeable future. The rust bus was created to help with this, moving the problem to another node. I don't see a good way around this, that's why I think you should reconsider the name. To avoid the round trip, all the reasons you could have that this name is important are the reasons why descriptive names shouldn't be used for rust crates.

@EHfive
Copy link

EHfive commented Feb 22, 2023

For 3 it's already an issue on crates.io where abandoned names

ash-layer is not a squatting crate, it serves the exact propose as it's name suggests and someone is maintaining it. And I want to repeat that I am willing to transfer the ownership of the crate (even from now, if so, devs could send me the username and I would add it to owner list).

I don't see a good way around this

https://github.com/Manishearth/namespacing-rfc

And please focus on the topic regarding "Writing layers in Rust" instead of how a name can not be used. Further messages regarding naming of ash-layer would not be responded from my side.

@cheako
Copy link
Author

cheako commented Feb 27, 2023

What a wonderful way to admit you've lost an argument without saying so... I'll try and remember that one.

@MarijnS95
Copy link
Collaborator

MarijnS95 commented Feb 28, 2023

@EHfive fine by me if you wish to reduce the bus factor. I'm intending to PR a minimal set of structs in a new ash-layer sub-crate in this repo (extracted from the branch linked above) and build it out from there, in case you wish to contribute.

While the minimal "sys"-like bindings could be part of ash, I do think any larger features such as automatically generated dispatch tables (which we could tie into the existing spec-based generator architecture) belong in a separate crate to not further balloon compiletimes, and as such place all layer-related structs and features there.

@EHfive
Copy link

EHfive commented Feb 28, 2023

Great! Sent the invite.

I'm intending to PR a minimal set of structs as ash-wrapper sub-crate in this repo (extracted from the branch linked above) and build it out from there, in case you wish to contribute.

Nice to hear that.

@MarijnS95
Copy link
Collaborator

@EHfive thanks, I've accepted the invite! Unfortunately haven't gotten to pushing/PR'ing anything yet, maybe over the weekend. Let's make this small and incremental :)

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

4 participants