From 96214998aff466823040d1ed4418b578ad0225e7 Mon Sep 17 00:00:00 2001
From: Brandon Jones <bajones@chromium.org>
Date: Wed, 22 Apr 2026 14:52:58 -0700
Subject: [PATCH] Vulkan: Cleanup EmptyPassQuerySet error handling

Cleans up the error handling when creating an empy pass query set
and adds a simple test to ensure that this branch is exercised.

Bug: 499908918
Change-Id: Ibc07005f41ee19604c5014be63dfdaea0e5ec8af
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/304177
Commit-Queue: Brandon Jones <bajones@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Brandon Jones <bajones@chromium.org>
---

diff --git a/src/dawn/native/vulkan/CommandBufferVk.cpp b/src/dawn/native/vulkan/CommandBufferVk.cpp
index bd0de87..f726da9 100644
--- a/src/dawn/native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn/native/vulkan/CommandBufferVk.cpp
@@ -1091,6 +1091,10 @@
             ResetUsedQuerySetsOnRenderPass(device, recordingContext->commandBuffer,
                                            usages.querySets[i], usages.queryAvailabilities[i]);
         }
+
+        if (device->IsToggleEnabled(Toggle::VulkanAddWorkToEmptyResolvePass)) {
+            DAWN_TRY(device->PrepareEmptyPassQuerySet(recordingContext));
+        }
         return {};
     };
 
@@ -1939,16 +1943,9 @@
                 // a driver bug that fails to resolve render targets in empty passes.
                 if (workCommandCount == 0 &&
                     device->IsToggleEnabled(Toggle::VulkanAddWorkToEmptyResolvePass)) {
-                    QuerySetBase* querySet = device->GetEmptyPassQuerySet();
-
-                    // Ensure that the query was allocated. In the unlikely event that it wasn't
-                    // (probably due to an OOM) empty render passes can't be safely completed using
-                    // this workaround.
-                    DAWN_INTERNAL_ERROR_IF(querySet->IsError(),
-                                           "Unable to allocate internal empty pass query set.");
-
-                    device->fn.CmdBeginQuery(commands, ToBackend(querySet)->GetHandle(), 0, 0);
-                    device->fn.CmdEndQuery(commands, ToBackend(querySet)->GetHandle(), 0);
+                    Ref<QuerySetBase> emptyQuerySet = device->UseEmptyPassQuerySet();
+                    device->fn.CmdBeginQuery(commands, ToBackend(emptyQuerySet)->GetHandle(), 0, 0);
+                    device->fn.CmdEndQuery(commands, ToBackend(emptyQuerySet)->GetHandle(), 0);
                 }
 
                 RecordEndRenderPass(recordingContext, device);
diff --git a/src/dawn/native/vulkan/DeviceVk.cpp b/src/dawn/native/vulkan/DeviceVk.cpp
index 29b60ed..1fe3dbc 100644
--- a/src/dawn/native/vulkan/DeviceVk.cpp
+++ b/src/dawn/native/vulkan/DeviceVk.cpp
@@ -1222,17 +1222,36 @@
     return true;
 }
 
-// Gets or creates an occlusion Query object for use with Toggle::VulkanAddWorkToEmptyResolvePass.
-QuerySetBase* Device::GetEmptyPassQuerySet() {
+// Prepares the Empty Pass QuerySet, creating and resetting it if needed.
+MaybeError Device::PrepareEmptyPassQuerySet(CommandRecordingContext* recordingContext) {
     DAWN_ASSERT(IsToggleEnabled(Toggle::VulkanAddWorkToEmptyResolvePass));
-
     if (!mEmptyPassQuerySet) {
         QuerySetDescriptor descriptor;
         descriptor.type = wgpu::QueryType::Occlusion;
         descriptor.count = 1;
-        mEmptyPassQuerySet = APICreateQuerySet(&descriptor);
+        DAWN_TRY_ASSIGN(mEmptyPassQuerySet, CreateQuerySet(&descriptor));
+        mEmptyPassQuerySetNeedsReset = true;
     }
-    return mEmptyPassQuerySet.Get();
+
+    if (mEmptyPassQuerySetNeedsReset) {
+        fn.CmdResetQueryPool(recordingContext->commandBuffer,
+                             ToBackend(mEmptyPassQuerySet)->GetHandle(), 0, 1);
+        mEmptyPassQuerySetNeedsReset = false;
+    }
+
+    return {};
+}
+
+// Gets the occlusion Query object for use with Toggle::VulkanAddWorkToEmptyResolvePass.
+// Must call PrepareEmptyPassQuerySet outside of recording a render pass prior to calling this every
+// time.
+Ref<QuerySetBase> Device::UseEmptyPassQuerySet() {
+    DAWN_ASSERT(IsToggleEnabled(Toggle::VulkanAddWorkToEmptyResolvePass));
+    DAWN_ASSERT(mEmptyPassQuerySet);
+    DAWN_ASSERT(!mEmptyPassQuerySetNeedsReset);
+
+    mEmptyPassQuerySetNeedsReset = true;
+    return mEmptyPassQuerySet;
 }
 
 }  // namespace dawn::native::vulkan
diff --git a/src/dawn/native/vulkan/DeviceVk.h b/src/dawn/native/vulkan/DeviceVk.h
index bc6c8d3..5c74441 100644
--- a/src/dawn/native/vulkan/DeviceVk.h
+++ b/src/dawn/native/vulkan/DeviceVk.h
@@ -151,7 +151,8 @@
                                                       size_t bufferSize) const override;
     bool NeedsStaticSamplerForExternalTexture() const override;
 
-    QuerySetBase* GetEmptyPassQuerySet();
+    MaybeError PrepareEmptyPassQuerySet(CommandRecordingContext* recordingContext);
+    Ref<QuerySetBase> UseEmptyPassQuerySet();
 
     VulkanRenderPassType GetRenderPassType() { return mRenderPassType; }
 
@@ -242,6 +243,7 @@
     Ref<PipelineCache> mMonolithicPipelineCache;
 
     Ref<QuerySetBase> mEmptyPassQuerySet;
+    bool mEmptyPassQuerySetNeedsReset = true;
     std::atomic<uint64_t> mNextTextureViewId = 1;
 
     VulkanRenderPassType mRenderPassType = VulkanRenderPassType::CreateRenderPass;
diff --git a/src/dawn/tests/end2end/MultisampledRenderingTests.cpp b/src/dawn/tests/end2end/MultisampledRenderingTests.cpp
index 32a52a8..19dcd1b 100644
--- a/src/dawn/tests/end2end/MultisampledRenderingTests.cpp
+++ b/src/dawn/tests/end2end/MultisampledRenderingTests.cpp
@@ -1382,6 +1382,46 @@
     VerifyResolveTarget(kGreen, mResolveTexture, 0, 0, kMSAACoverage, kGreenX, kGreenY);
 }
 
+class MultisampledRenderingWithEmptyPassWorkaroundTest : public MultisampledRenderingTest {};
+
+TEST_P(MultisampledRenderingWithEmptyPassWorkaroundTest, ResolveOneMultisampledTextureTwice) {
+    constexpr bool kTestDepth = false;
+    wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
+
+    wgpu::Texture resolveTexture2 = CreateTextureForRenderAttachment(kColorFormat, 1);
+
+    // In first render pass clear the color view to green triangle resolve.
+    {
+        utils::ComboRenderPassDescriptor renderPass = CreateComboRenderPassDescriptorForTest(
+            {mMultisampledColorView}, {mResolveView}, wgpu::LoadOp::Clear, wgpu::LoadOp::Clear,
+            kTestDepth);
+        renderPass.cColorAttachments[0].clearValue = kGreen;
+
+        wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(&renderPass);
+        // No work done during the pass
+        renderPassEncoder.End();
+    }
+
+    // In second render pass load the previously cleared color view and resolve.
+    {
+        wgpu::TextureView resolveView2 = resolveTexture2.CreateView();
+        utils::ComboRenderPassDescriptor renderPass = CreateComboRenderPassDescriptorForTest(
+            {mMultisampledColorView}, {resolveView2}, wgpu::LoadOp::Load, wgpu::LoadOp::Load,
+            kTestDepth);
+
+        wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(&renderPass);
+        // No work done during the pass
+        renderPassEncoder.End();
+    }
+
+    wgpu::CommandBuffer commandBuffer = commandEncoder.Finish();
+    queue.Submit(1, &commandBuffer);
+
+    constexpr float kMSAACoverage = 1.0f;
+    VerifyResolveTarget(kGreen, mResolveTexture, 0, 0, kMSAACoverage);
+    VerifyResolveTarget(kGreen, resolveTexture2, 0, 0, kMSAACoverage);
+}
+
 class MultisampledRenderingWithTransientAttachmentTest : public MultisampledRenderingTest {};
 
 // Test using one multisampled color transient attachment with resolve target can render correctly.
@@ -2888,6 +2928,9 @@
                       MetalBackend({"always_resolve_into_zero_level_and_layer",
                                     "emulate_store_and_msaa_resolve"}));
 
+DAWN_INSTANTIATE_TEST(MultisampledRenderingWithEmptyPassWorkaroundTest,
+                      VulkanBackend({"vulkan_add_work_to_empty_resolve_pass"}));
+
 DAWN_INSTANTIATE_TEST(MultisampledRenderingWithTransientAttachmentTest,
                       D3D11Backend(),
                       D3D12Backend(),

diff --git a/src/dawn/tests/end2end/MultisampledRenderingTests.cpp b/src/dawn/tests/end2end/MultisampledRenderingTests.cpp
index 32a52a8..19dcd1b 100644
--- a/src/dawn/tests/end2end/MultisampledRenderingTests.cpp
+++ b/src/dawn/tests/end2end/MultisampledRenderingTests.cpp
@@ -1382,6 +1382,46 @@
     VerifyResolveTarget(kGreen, mResolveTexture, 0, 0, kMSAACoverage, kGreenX, kGreenY);
 }
 
+class MultisampledRenderingWithEmptyPassWorkaroundTest : public MultisampledRenderingTest {};
+
+TEST_P(MultisampledRenderingWithEmptyPassWorkaroundTest, ResolveOneMultisampledTextureTwice) {
+    constexpr bool kTestDepth = false;
+    wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
+
+    wgpu::Texture resolveTexture2 = CreateTextureForRenderAttachment(kColorFormat, 1);
+
+    // In first render pass clear the color view to green triangle resolve.
+    {
+        utils::ComboRenderPassDescriptor renderPass = CreateComboRenderPassDescriptorForTest(
+            {mMultisampledColorView}, {mResolveView}, wgpu::LoadOp::Clear, wgpu::LoadOp::Clear,
+            kTestDepth);
+        renderPass.cColorAttachments[0].clearValue = kGreen;
+
+        wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(&renderPass);
+        // No work done during the pass
+        renderPassEncoder.End();
+    }
+
+    // In second render pass load the previously cleared color view and resolve.
+    {
+        wgpu::TextureView resolveView2 = resolveTexture2.CreateView();
+        utils::ComboRenderPassDescriptor renderPass = CreateComboRenderPassDescriptorForTest(
+            {mMultisampledColorView}, {resolveView2}, wgpu::LoadOp::Load, wgpu::LoadOp::Load,
+            kTestDepth);
+
+        wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(&renderPass);
+        // No work done during the pass
+        renderPassEncoder.End();
+    }
+
+    wgpu::CommandBuffer commandBuffer = commandEncoder.Finish();
+    queue.Submit(1, &commandBuffer);
+
+    constexpr float kMSAACoverage = 1.0f;
+    VerifyResolveTarget(kGreen, mResolveTexture, 0, 0, kMSAACoverage);
+    VerifyResolveTarget(kGreen, resolveTexture2, 0, 0, kMSAACoverage);
+}
+
 class MultisampledRenderingWithTransientAttachmentTest : public MultisampledRenderingTest {};
 
 // Test using one multisampled color transient attachment with resolve target can render correctly.
@@ -2888,6 +2928,9 @@
                       MetalBackend({"always_resolve_into_zero_level_and_layer",
                                     "emulate_store_and_msaa_resolve"}));
 
+DAWN_INSTANTIATE_TEST(MultisampledRenderingWithEmptyPassWorkaroundTest,
+                      VulkanBackend({"vulkan_add_work_to_empty_resolve_pass"}));
+
 DAWN_INSTANTIATE_TEST(MultisampledRenderingWithTransientAttachmentTest,
                       D3D11Backend(),
                       D3D12Backend(),