-
Notifications
You must be signed in to change notification settings - Fork 465
layers: Fix PTLAS descriptor handling and overlap validation #11584
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
Conversation
|
Author vkushwaha-nv not on autobuild list. Waiting for curator authorization before starting CI build. |
|
Adding @spencer-lunarg for review. |
| command_info.srcInfosCount = count_buffer_address; | ||
|
|
||
| m_command_buffer.Begin(); | ||
| m_errorMonitor->SetDesiredError("VUID-VkBuildPartitionedAccelerationStructureInfoNV-dstAccelerationStructureData-10561"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now we have no test testing 10561 ... please add one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is it needed? We have "m_errorMonitor->SetDesiredError("VUID-VkBuildPartitionedAccelerationStructureInfoNV-dstAccelerationStructureData-parameter");"
That should trigger if dstAccelerationStructureData is 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess what I am trying to say is if we can't write a test for 10561, but are still trying to validate it, something is wrong
I agree looking at the spec now these just seem like extra, redundant VUs... I will make a spec MR and allow this tests as they are now
| command_info.scratchData = 0; | ||
| m_command_buffer.Begin(); | ||
| // scratchData must not be NULL | ||
| m_errorMonitor->SetDesiredError("VUID-VkBuildPartitionedAccelerationStructureInfoNV-scratchData-10558"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now we need another test to test 10558
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing with "m_errorMonitor->SetDesiredError("VUID-VkBuildPartitionedAccelerationStructureInfoNV-scratchData-parameter"
73a5a20 to
49a8338
Compare
|
Author vkushwaha-nv not on autobuild list. Waiting for curator authorization before starting CI build. |
49a8338 to
88714cc
Compare
|
Author vkushwaha-nv not on autobuild list. Waiting for curator authorization before starting CI build. |
tests: Fix some VUIDs and added more PTLAS coverage
88714cc to
15e1aa0
Compare
|
Author vkushwaha-nv not on autobuild list. Waiting for curator authorization before starting CI build. |
| RETURN_IF_SKIP(InitBasicDescriptorHeap()); | ||
|
|
||
| const VkDeviceSize descriptor_size = | ||
| vk::GetPhysicalDeviceDescriptorSizeEXT(gpu_, VK_DESCRIPTOR_TYPE_PARTITIONED_ACCELERATION_STRUCTURE_NV); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is failing because in test_icd.cpp there is a GetPhysicalDeviceDescriptorSizeEXT implementation that is just missing VK_DESCRIPTOR_TYPE_PARTITIONED_ACCELERATION_STRUCTURE_NV from it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed the fix, want to get this in tonight, tomorrow morning is cut off for SDK
|
Author vkushwaha-nv not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI Vulkan-ValidationLayers build queued with queue ID 635934. |
|
CI Vulkan-ValidationLayers build # 22323 running. |
|
CI Vulkan-ValidationLayers build # 22323 failed. |
tests: Fix some VUIDs and added more PTLAS coverage