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>

src/dawn_native/CommandEncoder.cpp

@@ -370,14 +370,6 @@ namespace dawn_native {
             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");
                 }

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -800,13 +800,29 @@ namespace dawn_native { namespace vulkan {
                     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
+                    // vkCmdCopyQueryPoolResults only can retrieve available queries because
-                    // least one query is unavailable for the resolving and the resolve buffer has
+                    // VK_QUERY_RESULT_WAIT_BIT is set, for these unavailable queries, we need to
-                    // been initialized or fully used.
+                    // 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);
 

src/tests/end2end/QueryTests.cpp

@@ -302,9 +302,10 @@ TEST_P(OcclusionQueryTests, Rewrite) {
 // 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
+    // TODO(hao.x.li@intel.com): Fails on Intel Windows Vulkan due to a driver issue that
-    // one query not written and the resolve buffer has been initialized or fully used.
+    // vkCmdFillBuffer and vkCmdCopyQueryPoolResults are not executed in order, skip it util
-    DAWN_SKIP_TEST_IF(IsVulkan());
+    // 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_P(OcclusionQueryTests, ResolveSparseQueries) {
 
 // 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_P(TimestampQueryTests, ResolveFromAnotherEncoder) {
 
 // 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);

src/tests/unittests/validation/UnsafeAPIValidationTests.cpp

@@ -201,32 +201,3 @@ TEST_F(UnsafeAPIValidationTest, DynamicStorageBuffer) {
         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());
-    }
-}