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

vkCreateDevice "best practices" check is testing the wrong state #1272

Closed
cdwfs opened this issue Sep 20, 2019 · 4 comments · Fixed by #1274
Closed

vkCreateDevice "best practices" check is testing the wrong state #1272

cdwfs opened this issue Sep 20, 2019 · 4 comments · Fixed by #1274
Assignees
Labels
Bug Something isn't working
Milestone

Comments

@cdwfs
Copy link
Contributor

cdwfs commented Sep 20, 2019

The check for kVUID_BestPractices_CreateDevice_PDFeaturesNotCalled in best_practices.cpp claims to be checking whether vkGetPhysicalDeviceFeatures() has been called before vkCreateDevice(). However, it's checking vkGetPhysicalDeviceSurfaceCapabilitiesKHRState, not vkGetPhysicalDeviceFeaturesState.

I started working on a PR for what I assumed would be a simple one-line change, but then I discovered that vkGetPhysicalDeviceFeaturesState is never actually being set. I've been out of the validation layer game for a year or so; can I get a little help with this one? :)

@mark-lunarg
Copy link
Contributor

Sure @cdwfs, we'll take a look. Thanks for finding it!

@mark-lunarg mark-lunarg added the Bug Something isn't working label Sep 20, 2019
@mark-lunarg mark-lunarg added this to the P1 milestone Sep 20, 2019
@cdwfs
Copy link
Contributor Author

cdwfs commented Sep 20, 2019

Thanks Mark! I might have a PR after all, give me a moment to make sure the tests pass...

@mark-lunarg
Copy link
Contributor

Hey, it's all yours!

@mark-lunarg
Copy link
Contributor

You may need the change from #1273.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants