KhronosGroup / Vulkan-ValidationLayers

Vulkan Validation Layers (VVL)
https://vulkan.lunarg.com/doc/sdk/latest/linux/khronos_validation_layer.html
Other
748 stars 400 forks source link

Best Practices False Positives on `vkGetQueryPoolResults` for Timestamp Query Pools #8257

Closed nickclark2016 closed 2 months ago

nickclark2016 commented 2 months ago

Environment:

Describe the Issue When calling vkGetQueryPoolResults on a pool with query type VK_QUERY_TYPE_TIMESTAMP and not calling vkCmdBeginQuery (as per VUID-vkCmdBeginQuery-queryType-02804 that is not allowed), the best practices feature of the layers report that vkCmdBeginQuery was never called.

Expected behavior I expect no validation layer reports.

Valid Usage ID N/A

Additional context

Vulkan Validation Message: Validation Warning: [ BestPractices-QueryPool-Unavailable ] Object 0: handle = 0xbd60bd0000000076, type = VK_OBJECT_TYPE_QUERY_POOL; | MessageID = 0xd39be754 | vkGetQueryPoolResults():  QueryPool VkQueryPool 0xbd60bd0000000076[] and query 2: vkCmdBeginQuery() was never called.

Relevant Code:

// Query result fetch
dispatch->getQueryPoolResults(pools.timestamp_queries, begin_timestamp_query_index, 2,
                                                                sizeof(uint64_t) * 2, timestamps, sizeof(uint64_t),
                                                                VK_QUERY_RESULT_64_BIT);
// Query pool reset
dispatch->cmdResetQueryPool(cmds, pools.timestamp_queries, begin_timestamp_query_index, 2);

// Query Pool Creation
VkQueryPoolCreateInfo timing_pool_ci = {
    .sType = VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO,
    .queryType = VK_QUERY_TYPE_TIMESTAMP,
    .queryCount = 2 * static_cast<uint32_t>(
                            _device->frames_in_flight()), // start and end timestamps for each frame
};

_device->dispatch().createQueryPool(&timing_pool_ci, nullptr, &pools.timestamp_queries);
_device->dispatch().resetQueryPool(pools.timestamp_queries, 0, timing_pool_ci.queryCount);
spencer-lunarg commented 2 months ago

@nickclark2016 thanks for bringing this up, was a clear oversight (but simple fix)

this will be in the next SDK coming out soon