Validate a subresource can't be an attachment multiple times in a pass

Bug: dawn:998, dawn:1001
Change-Id: Id7cd4964ebf9589c599059cc0f853c0125fa725a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/58361
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/PassResourceUsageTracker.cpp

@@ -46,11 +46,22 @@ namespace dawn_native {
 
         textureUsage.Update(range,
                             [usage](const SubresourceRange&, wgpu::TextureUsage* storedUsage) {
+                                // TODO(crbug.com/dawn/1001): Consider optimizing to have fewer
+                                // branches.
+                                if ((*storedUsage & wgpu::TextureUsage::RenderAttachment) != 0 &&
+                                    (usage & wgpu::TextureUsage::RenderAttachment) != 0) {
+                                    // Using the same subresource as an attachment for two different
+                                    // render attachments is a write-write hazard. Add this internal
+                                    // usage so we will fail the check that a subresource with
+                                    // writable usage is the single usage.
+                                    *storedUsage |= kAgainAsRenderAttachment;
+                                }
                                 *storedUsage |= usage;
                             });
     }
 
-    void SyncScopeUsageTracker::AddTextureUsage(TextureBase* texture,
+    void SyncScopeUsageTracker::AddRenderBundleTextureUsage(
+        TextureBase* texture,
         const TextureSubresourceUsage& textureUsage) {
         // Get or create a new TextureSubresourceUsage for that texture (initially filled with
         // wgpu::TextureUsage::None)
@@ -62,7 +73,10 @@ namespace dawn_native {
 
         passTextureUsage->Merge(
             textureUsage, [](const SubresourceRange&, wgpu::TextureUsage* storedUsage,
-                             const wgpu::TextureUsage& addedUsage) { *storedUsage |= addedUsage; });
+                             const wgpu::TextureUsage& addedUsage) {
+                ASSERT((addedUsage & wgpu::TextureUsage::RenderAttachment) == 0);
+                *storedUsage |= addedUsage;
+            });
     }
 
     void SyncScopeUsageTracker::AddBindGroup(BindGroupBase* group) {

src/dawn_native/PassResourceUsageTracker.h

@@ -36,7 +36,8 @@ namespace dawn_native {
       public:
         void BufferUsedAs(BufferBase* buffer, wgpu::BufferUsage usage);
         void TextureViewUsedAs(TextureViewBase* texture, wgpu::TextureUsage usage);
-        void AddTextureUsage(TextureBase* texture, const TextureSubresourceUsage& textureUsage);
+        void AddRenderBundleTextureUsage(TextureBase* texture,
+                                         const TextureSubresourceUsage& textureUsage);
 
         // Walks the bind groups and tracks all its resources.
         void AddBindGroup(BindGroupBase* group);

src/dawn_native/RenderPassEncoder.cpp

@@ -227,7 +227,8 @@ namespace dawn_native {
                 }
 
                 for (uint32_t i = 0; i < usages.textures.size(); ++i) {
-                    mUsageTracker.AddTextureUsage(usages.textures[i], usages.textureUsages[i]);
+                    mUsageTracker.AddRenderBundleTextureUsage(usages.textures[i],
+                                                              usages.textureUsages[i]);
                 }
             }
 

src/dawn_native/Texture.h

@@ -42,10 +42,6 @@ namespace dawn_native {
     static constexpr wgpu::TextureUsage kReadOnlyTextureUsages =
         wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::Sampled | kReadOnlyStorageTexture;
 
-    static constexpr wgpu::TextureUsage kWritableTextureUsages =
-        wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::Storage |
-        wgpu::TextureUsage::RenderAttachment;
-
     class TextureBase : public ObjectBase {
       public:
         enum class TextureState { OwnedInternal, OwnedExternal, Destroyed };

src/dawn_native/dawn_platform.h

@@ -30,6 +30,11 @@ namespace dawn_native {
     static constexpr wgpu::TextureUsage kReadOnlyStorageTexture =
         static_cast<wgpu::TextureUsage>(0x80000000);
 
+    // Internal usage to help tracking when a subresource is used as render attachment usage
+    // more than once in a render pass.
+    static constexpr wgpu::TextureUsage kAgainAsRenderAttachment =
+        static_cast<wgpu::TextureUsage>(0x80000001);
+
     // Add an extra texture usage for textures that will be presented, for use in backends
     // that needs to transition to present usage.
     // This currently aliases wgpu::TextureUsage::Present, we would assign it

src/tests/unittests/validation/ResourceUsageTrackingTests.cpp

@@ -990,6 +990,70 @@ namespace {
         }
     }
 
+    // Test that a single subresource of a texture cannot be used as a render attachment more than
+    // once in the same pass.
+    TEST_F(ResourceUsageTrackingTest, TextureWithMultipleRenderAttachmentUsage) {
+        // Create a texture with two array layers
+        wgpu::TextureDescriptor descriptor;
+        descriptor.dimension = wgpu::TextureDimension::e2D;
+        descriptor.size = {1, 1, 2};
+        descriptor.usage = wgpu::TextureUsage::RenderAttachment;
+        descriptor.format = kFormat;
+
+        wgpu::Texture texture = device.CreateTexture(&descriptor);
+
+        wgpu::TextureViewDescriptor viewDesc = {};
+        viewDesc.arrayLayerCount = 1;
+
+        wgpu::TextureView viewLayer0 = texture.CreateView(&viewDesc);
+
+        viewDesc.baseArrayLayer = 1;
+        wgpu::TextureView viewLayer1 = texture.CreateView(&viewDesc);
+
+        // Control: It is valid to use layer0 as a render target for one attachment, and
+        // layer1 as the second attachment in the same pass
+        {
+            utils::ComboRenderPassDescriptor renderPass({viewLayer0, viewLayer1});
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+            pass.EndPass();
+            encoder.Finish();
+        }
+
+        // Control: It is valid to use layer0 as a render target in separate passes.
+        {
+            utils::ComboRenderPassDescriptor renderPass({viewLayer0});
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            wgpu::RenderPassEncoder pass0 = encoder.BeginRenderPass(&renderPass);
+            pass0.EndPass();
+            wgpu::RenderPassEncoder pass1 = encoder.BeginRenderPass(&renderPass);
+            pass1.EndPass();
+            encoder.Finish();
+        }
+
+        // It is invalid to use layer0 as a render target for both attachments in the same pass
+        {
+            utils::ComboRenderPassDescriptor renderPass({viewLayer0, viewLayer0});
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+            pass.EndPass();
+            ASSERT_DEVICE_ERROR(encoder.Finish());
+        }
+
+        // It is invalid to use layer1 as a render target for both attachments in the same pass
+        {
+            utils::ComboRenderPassDescriptor renderPass({viewLayer1, viewLayer1});
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+            pass.EndPass();
+            ASSERT_DEVICE_ERROR(encoder.Finish());
+        }
+    }
+
     // Test that using the same texture as both readable and writable in different passes is
     // allowed
     TEST_F(ResourceUsageTrackingTest, TextureWithReadAndWriteUsageInDifferentPasses) {