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 are_extension_features_present to PhysicalDevice #278

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Add are_extension_features_present to PhysicalDevice #278

merged 5 commits into from
Jun 13, 2024

Conversation

n0F4x
Copy link
Contributor

@n0F4x n0F4x commented Apr 30, 2024

Implements #277

Copy link
Owner

@charles-lunarg charles-lunarg left a comment

Choose a reason for hiding this comment

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

Oh, I thought individual feature checking was being asked for - not whether entire feature structs are present. Its much easier to check for structs and not features inside the struct.

I do like what I see here, it implements it cleanly.

@n0F4x
Copy link
Contributor Author

n0F4x commented Apr 30, 2024

This also makes it possible to check for individual features I hope.

The goal was to be able to write code like this:

VkPhysicalDeviceDescriptorIndexingFeatures descriptor_indexing_features{};
descriptor_indexing_features.<feature_to_be_checked> = true;

if (physical_device.are_extension_features_present(descriptor_indexing_features)) {
    // this individual <feature_to_be_checked> should be enabled
    // do something with this information
}

@n0F4x n0F4x requested a review from charles-lunarg May 11, 2024 20:48
n0F4x and others added 5 commits June 13, 2024 10:06
The original implementation only worked if the requested features
had the same length. It'll now handle that properly.
Didn't handle the case where some fields were supported but others weren't.
Copy link
Owner

@charles-lunarg charles-lunarg left a comment

Choose a reason for hiding this comment

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

I hope you don't mind me going ahead and fixing a bug I found when I added some tests for the new function. Seemed like the best way to get it in, rather than have the normal 'back and forth' of requesting fixes and waiting for them to be implemented.

The feature was needed, so I appreciate you going ahead and adding it.

@charles-lunarg charles-lunarg merged commit 2d6f7de into charles-lunarg:main Jun 13, 2024
8 checks passed
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.

2 participants