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

extensions/khr: Take the remaining p_next-containing structs as &mut #744

Merged
merged 2 commits into from
May 3, 2023

Conversation

MarijnS95
Copy link
Collaborator

@MarijnS95 MarijnS95 commented Apr 16, 2023

Fixes #740

Version 2 of get_physical_device_surface_capabilities and the matching vk::SurfaceCapabilitiesKHR struct exist solely to provide an sType and pNext field to allow extending the original query with additional data via extension structs. However, this API when introduced in #530 only returns the default()-initialized struct making it just as constrained as get_physical_device_surface_capabilities(). Solve this by taking vk::SurfaceCapabilities2KHR as &mut just like any similar API.

And just like this, do the same for the remaining:

  • AccelerationStructure::get_acceleration_structure_build_sizes()
  • ExternalMemoryFd::get_memory_fd_properties()
  • ExternalMemoryWin32::get_memory_win32_handle_properties()

In case these structs get extended somewhere down the line, which the Vulkan API allows for.

@MarijnS95
Copy link
Collaborator Author

Searching for default, we only have:

  • AccelerationStructure::get_acceleration_structure_build_sizes;
  • ExternalMemoryFd::get_memory_fd_properties;
  • and ExternalMemoryWin32::get_memory_win32_handle_properties

that are default()-initializing a pNext struct, perhaps those should get the same treatment (in this PR) to get rid of this problem altogether?

(On a side-note get_device_acceleration_structure_compatibility() is the only other function calling default() but for a flag where we typically use mem::zeroed() instead)

@MarijnS95 MarijnS95 requested a review from Ralith May 1, 2023 18:07
@Ralith
Copy link
Collaborator

Ralith commented May 1, 2023

perhaps those should get the same treatment (in this PR) to get rid of this problem altogether?

SGTM.

@MarijnS95 MarijnS95 force-pushed the get-surface-caps2-pnext-struct branch from 3366992 to 564a417 Compare May 2, 2023 11:35
@MarijnS95 MarijnS95 changed the title extensions/khr: Enable passing pNext-initialized struct to get_physical_device_surface_capabilities2 extensions/khr: Take the remaining p_next-containing structs as &mut May 2, 2023
@MarijnS95 MarijnS95 requested a review from Ralith May 2, 2023 11:36
Version 2 of `get_physical_device_surface_capabilities` and the matching
`vk::SurfaceCapabilitiesKHR` struct exist solely to provide an `sType`
and `pNext` field to allow extending the original query with additional
data via extension structs.  However, this API when introduced in #530
only returns the `default()`-initialized struct making it just as
constrained as `get_physical_device_surface_capabilities()`.  Solve this
by taking `vk::SurfaceCapabilities2KHR` as `&mut` just like any similar
API.

And just like this, do the same for the remaining:
- `AccelerationStructure::get_acceleration_structure_build_sizes()`
- `ExternalMemoryFd::get_memory_fd_properties()`
- `ExternalMemoryWin32::get_memory_win32_handle_properties()`

In case these structs get extended somewhere down the line, which the
Vulkan API allows for.
…f `vk::AccelerationStructureCompatibilityKHR::default()`
@MarijnS95 MarijnS95 force-pushed the get-surface-caps2-pnext-struct branch from 564a417 to 0a90b47 Compare May 2, 2023 11:39
@MarijnS95 MarijnS95 merged commit 1374996 into master May 3, 2023
@MarijnS95 MarijnS95 deleted the get-surface-caps2-pnext-struct branch May 3, 2023 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_physical_device_surface_capabilities2 should take a mutable reference to vk::SurfaceCapabilities2KHR
2 participants