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

Add VK_KHR_get_physical_device_properties2 extension #400

Merged
merged 4 commits into from
Apr 18, 2021

Conversation

Rua
Copy link
Contributor

@Rua Rua commented Mar 14, 2021

This duplicates functions that were added to Vulkan 1.1, but in 1.0 they are only available as an extension so for completeness (and Vulkano!) I've added it.

The 1.1 functions in Ash have different signatures, namely they take an extra &mut argument to place the output of the function. For this version I went with using the return value (possibly in a Vec), which is also used by many Ash functions and feels more idiomatic. However, if it's desirable for both functions in a pair to have the same signature, then I can change either these new extension functions or the existing 1.1 functions to match, in an additional commit to this PR.

Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire purpose of these functions is to support extensions via the p_next mechanism, which is impossible with a Vec return. If that's not needed, then the non-extension variants of the functions are sufficient.

@Rua
Copy link
Contributor Author

Rua commented Mar 14, 2021

Hmm, I can follow the original Ash functions then. But I wonder if there's more idiomatic ways to do this. The problem is that the original functions have no way to query the number of items in advance (with *count == 0 and out == null). So you can't know what length slice you should pass in.

@Rua
Copy link
Contributor Author

Rua commented Mar 14, 2021

I noticed now that the functions that allow querying for an item count have a _len variant which does this, so I copied that example. Still not sure if this is the best design, and also whether other functions that do return a Vec should be modified then.

@Rua
Copy link
Contributor Author

Rua commented Apr 18, 2021

Does anything else need to be done here?

@MaikKlein MaikKlein merged commit 5eb39fe into ash-rs:master Apr 18, 2021
@Rua Rua deleted the VK_KHR_get_physical_device_properties2 branch April 30, 2021 15:18
MarijnS95 added a commit that referenced this pull request May 8, 2021
Fixes: 5eb39fe ("Add VK_KHR_get_physical_device_properties2 extension (#400)")
MaikKlein pushed a commit that referenced this pull request May 8, 2021
…ublic (#430)

* GetPhysicalDeviceProperties2: Make API functions public

Fixes: 5eb39fe ("Add VK_KHR_get_physical_device_properties2 extension (#400)")

* BufferDeviceAddress: Make API functions public

Fixes: 98d66c6 ("Add VK_KHR/EXT_buffer_device_address extension (#405)")

* GetMemoryRequirements2: Make API functions public

Fixes: d8d7423 ("Add VK_KHR_get_memory_requirements2 extension (#401)")

* Maintenance1/Maintenance3: Make API functions public

Fixes: a0a1f5d ("Add VK_KHR_maintenance extensions (#406)")

* Globally remove all `allow(dead_code)` exceptions

This is hiding the fact that some extension functions are inadvertently
not public and hence unusable by crate users, nor does it enforce clean
coding practices.

In addition remove `ash/src/allocator.rs` which does not appear to be
used (anymore?) and isn't in a working state anyway.
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 this pull request may close these issues.

3 participants