dawn_native: Move beginRenderPass texture usage validation into the encoder

In a future CL the PassResourceUsage structure will become a
SyncScopeResourceUsage and will be used to validate at each
synchronization scope. For separation of concerns, the validation that
resource have the correct usage shouldn't be done at the sync scope
level but at each entrypoint that uses the resource.

The validation tests had missing coverage of usage validation for
BeginRenderPass so validation tests are added. (Storage and Sampled
are validated at bindgroup creation and already had validation tests)

Bug: dawn:635
Change-Id: I36488c2d0222c4799476adf06c1c734989b1a158
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38381
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/CommandEncoder.cpp

@@ -156,6 +156,8 @@ namespace dawn_native {
             const TextureViewBase* resolveTarget = colorAttachment.resolveTarget;
             const TextureViewBase* attachment = colorAttachment.attachment;
             DAWN_TRY(device->ValidateObject(colorAttachment.resolveTarget));
+            DAWN_TRY(ValidateCanUseAs(colorAttachment.resolveTarget->GetTexture(),
+                                      wgpu::TextureUsage::RenderAttachment));
 
             if (!attachment->GetTexture()->IsMultisampledTexture()) {
                 return DAWN_VALIDATION_ERROR(
@@ -206,6 +208,8 @@ namespace dawn_native {
             uint32_t* height,
             uint32_t* sampleCount) {
             DAWN_TRY(device->ValidateObject(colorAttachment.attachment));
+            DAWN_TRY(ValidateCanUseAs(colorAttachment.attachment->GetTexture(),
+                                      wgpu::TextureUsage::RenderAttachment));
 
             const TextureViewBase* attachment = colorAttachment.attachment;
             if (!(attachment->GetAspects() & Aspect::Color) ||
@@ -246,6 +250,8 @@ namespace dawn_native {
             DAWN_ASSERT(depthStencilAttachment != nullptr);
 
             DAWN_TRY(device->ValidateObject(depthStencilAttachment->attachment));
+            DAWN_TRY(ValidateCanUseAs(depthStencilAttachment->attachment->GetTexture(),
+                                      wgpu::TextureUsage::RenderAttachment));
 
             const TextureViewBase* attachment = depthStencilAttachment->attachment;
             if ((attachment->GetAspects() & (Aspect::Depth | Aspect::Stencil)) == Aspect::None ||

src/dawn_native/CommandValidation.cpp

@@ -310,39 +310,27 @@ namespace dawn_native {
     // It will be valid to use a buffer both as uniform and storage in the same compute pass.
     // TODO(yunchao.he@intel.com): add read/write usage tracking for compute
     MaybeError ValidatePassResourceUsage(const PassResourceUsage& pass) {
+        // TODO(cwallez@chromium.org): Remove this special casing once the PassResourceUsage is a
+        // SyncScopeResourceUsage.
+        if (pass.passType != PassType::Render) {
+            return {};
+        }
+
         // Buffers can only be used as single-write or multiple read.
         for (size_t i = 0; i < pass.buffers.size(); ++i) {
             wgpu::BufferUsage usage = pass.bufferUsages[i];
             bool readOnly = IsSubset(usage, kReadOnlyBufferUsages);
             bool singleUse = wgpu::HasZeroOrOneBits(usage);
 
-            if (pass.passType == PassType::Render && !readOnly && !singleUse) {
+            if (!readOnly && !singleUse) {
                 return DAWN_VALIDATION_ERROR(
                     "Buffer used as writable usage and another usage in pass");
             }
         }
 
-        // Textures can only be used as single-write or multiple read.
-        for (size_t i = 0; i < pass.textures.size(); ++i) {
-            const TextureBase* texture = pass.textures[i];
-            const PassTextureUsage& textureUsage = pass.textureUsages[i];
-            wgpu::TextureUsage usage = textureUsage.usage;
-
-            if (usage & ~texture->GetUsage()) {
-                return DAWN_VALIDATION_ERROR("Texture missing usage for the pass");
-            }
-
-            // The usage variable for the whole texture is a fast path for texture usage tracking.
-            // Because in most cases a texture (with or without subresources) is used as
-            // single-write or multiple read, then we can skip iterating the subresources' usages.
-            bool readOnly = IsSubset(usage, kReadOnlyTextureUsages);
-            bool singleUse = wgpu::HasZeroOrOneBits(usage);
-            if (pass.passType != PassType::Render || readOnly || singleUse) {
-                continue;
-            }
-
-            // Check that every single subresource is used as either a single-write usage or a
-            // combination of readonly usages.
+        // Check that every single subresource is used as either a single-write usage or a
+        // combination of readonly usages.
+        for (const PassTextureUsage& textureUsage : pass.textureUsages) {
             MaybeError error = {};
             textureUsage.subresourceUsages.Iterate(
                 [&](const SubresourceRange&, const wgpu::TextureUsage& usage) {

src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp

@@ -146,6 +146,29 @@ namespace {
         }
     }
 
+    // Check that the render pass color attachment must have the RenderAttachment usage.
+    TEST_F(RenderPassDescriptorValidationTest, ColorAttachmentInvalidUsage) {
+        // Control case: using a texture with RenderAttachment is valid.
+        {
+            wgpu::TextureView renderView =
+                Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
+            utils::ComboRenderPassDescriptor renderPass({renderView});
+            AssertBeginRenderPassSuccess(&renderPass);
+        }
+
+        // Error case: using a texture with Sampled is invalid.
+        {
+            wgpu::TextureDescriptor texDesc;
+            texDesc.usage = wgpu::TextureUsage::Sampled;
+            texDesc.size = {1, 1, 1};
+            texDesc.format = wgpu::TextureFormat::RGBA8Unorm;
+            wgpu::Texture sampledTex = device.CreateTexture(&texDesc);
+
+            utils::ComboRenderPassDescriptor renderPass({sampledTex.CreateView()});
+            AssertBeginRenderPassError(&renderPass);
+        }
+    }
+
     // Attachments must have the same size
     TEST_F(RenderPassDescriptorValidationTest, SizeMustMatch) {
         wgpu::TextureView color1x1A =
@@ -344,6 +367,30 @@ namespace {
         }
     }
 
+    // Check that the render pass depth attachment must have the RenderAttachment usage.
+    TEST_F(RenderPassDescriptorValidationTest, DepthAttachmentInvalidUsage) {
+        // Control case: using a texture with RenderAttachment is valid.
+        {
+            wgpu::TextureView renderView =
+                Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth32Float);
+            utils::ComboRenderPassDescriptor renderPass({}, renderView);
+            AssertBeginRenderPassSuccess(&renderPass);
+        }
+
+        // Error case: using a texture with Sampled is invalid.
+        {
+            wgpu::TextureDescriptor texDesc;
+            texDesc.usage = wgpu::TextureUsage::Sampled;
+            texDesc.size = {1, 1, 1};
+            texDesc.format = wgpu::TextureFormat::Depth32Float;
+            wgpu::Texture sampledTex = device.CreateTexture(&texDesc);
+            wgpu::TextureView sampledView = sampledTex.CreateView();
+
+            utils::ComboRenderPassDescriptor renderPass({}, sampledView);
+            AssertBeginRenderPassError(&renderPass);
+        }
+    }
+
     // Only 2D texture views with mipLevelCount == 1 are allowed to be color attachments
     TEST_F(RenderPassDescriptorValidationTest, TextureViewLevelCountForColorAndDepthStencil) {
         constexpr uint32_t kArrayLayers = 1;