Mark querypool slots as ENDED in vkCmdWriteAccelerationStructuresPropertiesKHR #2439
Conversation
|
|
|
Yikes, the CI also enforces 72-char limits in code blocks within commit messages (indended with 4 spaces)... |
MarijnS95
force-pushed
the
query-as-props
branch
from
090c898 to
4d3f1e0
Compare
vkCmdWriteAccelerationStructuresPropertiesKHR did not have the affected
querypool slots marked as QUERYSTATE_ENDED, resulting in the following
validation layer error when copying results out:
Validation Error: [ UNASSIGNED-CoreValidation-DrawState-InvalidQuery
] Object 0: handle = 0x7f54ecc529b8, type =
VK_OBJECT_TYPE_COMMAND_BUFFER; | MessageID = 0x46ab3a6e |
vkCmdCopyQueryPoolResults(): Requesting a copy from query to buffer
on VkQueryPool 0xff00000000ff[] query 0: waiting on a query that has
been reset and not issued yet
This is simply fixed by marking the slots as containing valid data
(QUERYSTATE_ENDED) in the PostCall handler.
MarijnS95
force-pushed
the
query-as-props
branch
from
4d3f1e0 to
deba03c
Compare
There was a problem hiding this comment.
Thanks @MarijnS95! Just one minor nit from me. I'll add @mark-lunarg as I'd like someone with more experience to have a look.
layers/core_validation.cpp
Outdated
| if (!do_validate) return false; | ||
| bool skip = false; | ||
| for (uint32_t i = 0; i < accelerationStructureCount; i++) { | ||
| QueryObject query = QueryObject(QueryObject(queryPool, firstQuery + i), perfPass); |
There was a problem hiding this comment.
Looks like
QueryObject query = {{queryPool, firstQuery + i}, perfPass};Would be a bit more consistent with the rest of this file?
There was a problem hiding this comment.
That was copied verbatim from ValidationStateTracker::SetQueryStateMulti. Looks like both methods are mixed in state_tracker as well as core_validation.
Changed this particular instance, but will leave it up to the maintainers to unify the other occurences :)
There was a problem hiding this comment.
@MarijnS95, yes, thanks for understanding, and thanks for the PR -- this is great!
As discovered and fixed in the previous commit vkCmdWriteAccelerationStructuresPropertiesKHR did not set queries to QUERYSTATE_ENDED. Likewise this function was also not checking whether querypool slots have been reset first. Do so in PreCall.
MarijnS95
force-pushed
the
query-as-props
branch
from
deba03c to
dc86392
Compare
|
Thanks @ncesario-lunarg! |
vkCmdWriteAccelerationStructuresPropertiesKHRdid not have the affected querypool slots marked asQUERYSTATE_ENDED, resulting in the following validation layer error when copying results out:This is simply fixed by marking the slots as containing valid data (
QUERYSTATE_ENDED) in thePostCallhandler.Similarly it does not check whether every affected slot is in reset state like
vkCmdWriteTimestamp, which has been added inPreCall.EDIT: Are there any conventions regarding function sorting/ordering in cpp files and headers?
This issue was not previously noticed in our framework because we were not resetting the querypool prior to querying acceleration structure properties. Playing around with these calls uncovered a second issue: copying results from an uninitialized and un-reset (I think this is
QUERYSTATE_UNKNOWN) querypool does not trigger validation layer errors. On NVidia that blocks indefinitely in the driver; afaik there should at least be a warning for that. After all the validation layer seems to check if this querypool slot can be made available by any command buffer submitted thus far, if I read the code correctly.