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

stateless: Remove ValidatePnextPropertyStructContents() #9302

Merged

Conversation

jeremyg-lunarg
Copy link
Contributor

Property pNext checking was implemented incorrectly for a long time and recently fixed by commit da1b588. However requiring extensions enabled for querying properties is very cumbersome and contradicts this statement from the specification:

   Any component of the implementation (the loader, any enabled layers,
   and drivers) must skip over, without processing (other than reading the
   sType and pNext members) any extending structures in the chain not
   defined by core versions or extensions supported by that component.

It is possible that this could also be relaxed for other queries in the future.

@jeremyg-lunarg jeremyg-lunarg requested a review from a team as a code owner January 23, 2025 19:48
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 354356.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18839 running.

@jeremyg-lunarg jeremyg-lunarg force-pushed the jeremyg-properties-pnext branch from 5c1996c to 7be0b5c Compare January 23, 2025 19:51
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 354372.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18840 running.

@@ -442,6 +442,9 @@ def generateSource(self):
property_structs = [x for x in extended_structs if x.extends == ["VkPhysicalDeviceProperties2"]]
other_structs = [x for x in extended_structs if x not in feature_structs and x not in property_structs and x.name not in self.structsWithManualChecks]

# NOTE: property structs are no longer checked. Previously we only checked if the corresponding extension was supported
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe worth putting this above property_structs declaration and point to link to #9302 for future reference

Property pNext checking was implemented incorrectly for a long
time and recently fixed by commit da1b588. However requiring
extensions enabled for querying properties is very cumbersome
and contradicts this statement from the specification:

   Any component of the implementation (the loader, any enabled layers,
   and drivers) must skip over, without processing (other than reading the
   sType and pNext members) any extending structures in the chain not
   defined by core versions or extensions supported by that component.

It is possible that this could also be relaxed for other queries in
the future.
VUID-VkPhysicalDeviceProperties2-pNext-pNext will no longer
fire if a property structure is included without its
extension being enabled.
@jeremyg-lunarg jeremyg-lunarg force-pushed the jeremyg-properties-pnext branch from 7be0b5c to 233df20 Compare January 23, 2025 20:22
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 354408.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18841 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18841 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 354445.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18842 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18842 passed.

@jeremyg-lunarg jeremyg-lunarg merged commit 30169d2 into KhronosGroup:main Jan 23, 2025
20 checks passed
@jeremyg-lunarg jeremyg-lunarg deleted the jeremyg-properties-pnext branch January 23, 2025 21:51
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