Clear resolve buffer to 0 for resolving unavailable queries
- Add vkCmdFillBuffer in ResolveQuerySet to clear the buffer to 0s for
these unavailable queries if the buffer has been initialized or fully
used which won't been initialized with 0s again.
- Because vkCmdFillBuffer has driver issue on Intel Windows, Skip some
affected cases.
- Remove unsafe api checking from Occlusion Query.
Bug: dawn:434
Change-Id: Ib34f81d93b0de8f08f0eeebf3c8a967eeb5ecefb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/48320
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index e460eab..921d5f4 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -370,14 +370,6 @@
if (descriptor->occlusionQuerySet != nullptr) {
DAWN_TRY(device->ValidateObject(descriptor->occlusionQuerySet));
- // Occlusion query has not been implemented completely. Disallow it as unsafe until
- // the implementaion is completed.
- if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
- return DAWN_VALIDATION_ERROR(
- "Occlusion query is disallowed because it has not been implemented "
- "completely.");
- }
-
if (descriptor->occlusionQuerySet->GetQueryType() != wgpu::QueryType::Occlusion) {
return DAWN_VALIDATION_ERROR("The type of query set must be Occlusion");
}
diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp
index 16d8100..d154026 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn_native/vulkan/CommandBufferVk.cpp
@@ -800,13 +800,29 @@
QuerySet* querySet = ToBackend(cmd->querySet.Get());
Buffer* destination = ToBackend(cmd->destination.Get());
- // TODO(hao.x.li@intel.com): Clear the resolve region of the buffer to 0 if at
- // least one query is unavailable for the resolving and the resolve buffer has
- // been initialized or fully used.
+ // vkCmdCopyQueryPoolResults only can retrieve available queries because
+ // VK_QUERY_RESULT_WAIT_BIT is set, for these unavailable queries, we need to
+ // clear the resolving region of the buffer to 0s if the buffer has been
+ // initialized or fully used.
+ auto startIt = querySet->GetQueryAvailability().begin() + cmd->firstQuery;
+ auto endIt = querySet->GetQueryAvailability().begin() + cmd->firstQuery +
+ cmd->queryCount;
+ bool hasUnavailableQueries = std::find(startIt, endIt, false) != endIt;
+ if (hasUnavailableQueries &&
+ (destination->IsDataInitialized() ||
+ destination->IsFullBufferRange(cmd->destinationOffset,
+ cmd->queryCount * sizeof(uint64_t)))) {
+ destination->TransitionUsageNow(recordingContext,
+ wgpu::BufferUsage::CopyDst);
+ device->fn.CmdFillBuffer(commands, destination->GetHandle(),
+ cmd->destinationOffset,
+ cmd->queryCount * sizeof(uint64_t), 0u);
+ } else {
+ destination->EnsureDataInitializedAsDestination(
+ recordingContext, cmd->destinationOffset,
+ cmd->queryCount * sizeof(uint64_t));
+ }
- destination->EnsureDataInitializedAsDestination(
- recordingContext, cmd->destinationOffset,
- cmd->queryCount * sizeof(uint64_t));
destination->TransitionUsageNow(recordingContext,
wgpu::BufferUsage::QueryResolve);
diff --git a/src/tests/end2end/QueryTests.cpp b/src/tests/end2end/QueryTests.cpp
index c09a921..861a3f1 100644
--- a/src/tests/end2end/QueryTests.cpp
+++ b/src/tests/end2end/QueryTests.cpp
@@ -302,9 +302,10 @@
// Test resolving occlusion query correctly if the queries are written sparsely, which also tests
// the query resetting at the start of render passes on Vulkan backend.
TEST_P(OcclusionQueryTests, ResolveSparseQueries) {
- // TODO(hao.x.li@intel.com): Clear the resolve region of the buffer to 0 if there is at least
- // one query not written and the resolve buffer has been initialized or fully used.
- DAWN_SKIP_TEST_IF(IsVulkan());
+ // TODO(hao.x.li@intel.com): Fails on Intel Windows Vulkan due to a driver issue that
+ // vkCmdFillBuffer and vkCmdCopyQueryPoolResults are not executed in order, skip it util
+ // the issue is fixed.
+ DAWN_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
// TODO(hao.x.li@intel.com): Investigate why it's failed on D3D12 on Nvidia when running with
// the previous occlusion tests. Expect resolve to 0 for these unwritten queries but the
@@ -366,10 +367,6 @@
// Test resolving occlusion query to 0 if all queries are not written
TEST_P(OcclusionQueryTests, ResolveWithoutWritten) {
- // TODO(hao.x.li@intel.com): Clear the resolve region of the buffer to 0 if there is at least
- // one query not written and the resolve buffer has been initialized or fully used.
- DAWN_SKIP_TEST_IF(IsVulkan());
-
// TODO(hao.x.li@intel.com): Investigate why it's failed on D3D12 on Nvidia when running with
// the previous occlusion tests. Expect resolve to 0 but the occlusion result of the previous
// tests is got.
@@ -707,6 +704,11 @@
// Test resolving timestamp query correctly if the queries are written sparsely
TEST_P(TimestampQueryTests, ResolveSparseQueries) {
+ // TODO(hao.x.li@intel.com): Fails on Intel Windows Vulkan due to a driver issue that
+ // vkCmdFillBuffer and vkCmdCopyQueryPoolResults are not executed in order, skip it util
+ // the issue is fixed.
+ DAWN_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
+
constexpr uint32_t kQueryCount = 4;
wgpu::QuerySet querySet = CreateQuerySetForTimestamp(kQueryCount);
diff --git a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
index 8c6986b..29816c8 100644
--- a/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
+++ b/src/tests/unittests/validation/UnsafeAPIValidationTests.cpp
@@ -201,32 +201,3 @@
ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&desc));
}
}
-
-// Check that occlusion query is disallowed as part of unsafe APIs.
-TEST_F(UnsafeAPIValidationTest, OcclusionQueryDisallowed) {
- DummyRenderPass renderPass(device);
-
- // Control case: BeginRenderPass without occlusionQuerySet is allowed.
- {
- wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
-
- pass.EndPass();
- encoder.Finish();
- }
-
- // Error case: BeginRenderPass with occlusionQuerySet is disallowed.
- {
- wgpu::QuerySetDescriptor descriptor;
- descriptor.type = wgpu::QueryType::Occlusion;
- descriptor.count = 1;
- wgpu::QuerySet querySet = device.CreateQuerySet(&descriptor);
- renderPass.occlusionQuerySet = querySet;
-
- wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
-
- pass.EndPass();
- ASSERT_DEVICE_ERROR(encoder.Finish());
- }
-}