Track read-only depth/stencil attachment as read-only usage

If a depth/stencil texture view is used as sampled texture and
read-only render attachment in the same render pass, it should be fine.
Because both usages are readonly. However, Dawn doesn't distinguish
read-only render attachment from writeable render attachment. So, this
situation is thought to be invalid.

This change fixes the issue and allows these read-only usages in one
pass, with a validation test for verification.

Bug: dawn:485

Change-Id: I0df5a4209651cddd6122487d96b1810717e4eb22
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/65485
Commit-Queue: Yunchao He <yunchao.he@intel.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

src/dawn_native/CommandEncoder.cpp

@@ -445,6 +445,21 @@ namespace dawn_native {
                 encoder, destination, availabilityBuffer.Get(), paramsBuffer.Get());
         }
 
+        bool IsReadOnlyDepthStencilAttachment(
+            const RenderPassDepthStencilAttachment* depthStencilAttachment) {
+            DAWN_ASSERT(depthStencilAttachment != nullptr);
+            Aspect aspects = depthStencilAttachment->view->GetAspects();
+            DAWN_ASSERT(IsSubset(aspects, Aspect::Depth | Aspect::Stencil));
+
+            if ((aspects & Aspect::Depth) && !depthStencilAttachment->depthReadOnly) {
+                return false;
+            }
+            if (aspects & Aspect::Stencil && !depthStencilAttachment->stencilReadOnly) {
+                return false;
+            }
+            return true;
+        }
+
     }  // namespace
 
     CommandEncoder::CommandEncoder(DeviceBase* device, const CommandEncoderDescriptor*)
@@ -570,7 +585,19 @@ namespace dawn_native {
                     cmd->depthStencilAttachment.stencilStoreOp =
                         descriptor->depthStencilAttachment->stencilStoreOp;
 
-                    usageTracker.TextureViewUsedAs(view, wgpu::TextureUsage::RenderAttachment);
+                    if (IsReadOnlyDepthStencilAttachment(descriptor->depthStencilAttachment)) {
+                        // TODO(dawn:485): Readonly depth/stencil attachment is not fully
+                        // implemented. Disallow it as unsafe until the implementaion is completed.
+                        if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
+                            return DAWN_VALIDATION_ERROR(
+                                "Readonly depth/stencil attachment is disallowed because it's not "
+                                "fully implemented");
+                        }
+
+                        usageTracker.TextureViewUsedAs(view, kReadOnlyRenderAttachment);
+                    } else {
+                        usageTracker.TextureViewUsedAs(view, wgpu::TextureUsage::RenderAttachment);
+                    }
                 }
 
                 cmd->width = width;

src/dawn_native/Texture.h

@@ -40,7 +40,8 @@ namespace dawn_native {
     bool IsValidSampleCount(uint32_t sampleCount);
 
     static constexpr wgpu::TextureUsage kReadOnlyTextureUsages =
-        wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding;
+        wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::TextureBinding |
+        kReadOnlyRenderAttachment;
 
     class TextureBase : public ApiObjectBase {
       public:

src/dawn_native/dawn_platform.h

@@ -26,6 +26,8 @@ namespace dawn_native {
     // Add an extra buffer usage (readonly storage buffer usage) for render pass resource tracking
     static constexpr wgpu::BufferUsage kReadOnlyStorageBuffer =
         static_cast<wgpu::BufferUsage>(0x80000000);
+    static constexpr wgpu::TextureUsage kReadOnlyRenderAttachment =
+        static_cast<wgpu::TextureUsage>(0x40000000);
 
     // Internal usage to help tracking when a subresource is used as render attachment usage
     // more than once in a render pass.

src/tests/unittests/validation/ResourceUsageTrackingTests.cpp

@@ -29,14 +29,15 @@ namespace {
             return device.CreateBuffer(&descriptor);
         }
 
-        wgpu::Texture CreateTexture(wgpu::TextureUsage usage) {
+        wgpu::Texture CreateTexture(wgpu::TextureUsage usage,
+                                    wgpu::TextureFormat format = wgpu::TextureFormat::RGBA8Unorm) {
             wgpu::TextureDescriptor descriptor;
             descriptor.dimension = wgpu::TextureDimension::e2D;
             descriptor.size = {1, 1, 1};
             descriptor.sampleCount = 1;
             descriptor.mipLevelCount = 1;
             descriptor.usage = usage;
-            descriptor.format = kFormat;
+            descriptor.format = format;
 
             return device.CreateTexture(&descriptor);
         }
@@ -881,6 +882,50 @@ namespace {
         }
     }
 
+    // Test that it is invalid to use the same texture as both readable and writable depth/stencil
+    // attachment in the same render pass. But it is valid to use it as both readable and readonly
+    // depth/stencil attachment in the same render pass.
+    // Note that depth/stencil attachment is a special render attachment, it can be readonly.
+    TEST_F(ResourceUsageTrackingTest, TextureWithSamplingAndDepthStencilAttachment) {
+        // Create a texture
+        wgpu::Texture texture =
+            CreateTexture(wgpu::TextureUsage::TextureBinding | wgpu::TextureUsage::RenderAttachment,
+                          wgpu::TextureFormat::Depth32Float);
+        wgpu::TextureView view = texture.CreateView();
+
+        // Create a bind group to use the texture as sampled binding
+        wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+            device, {{0, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Depth}});
+        wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, view}});
+
+        // Create a render pass to use the texture as a render target
+        utils::ComboRenderPassDescriptor passDescriptor({}, view);
+        passDescriptor.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+        passDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+
+        // It is invalid to use the texture as both sampled and writeable depth/stencil attachment
+        // in the same pass
+        {
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
+            pass.SetBindGroup(0, bg);
+            pass.EndPass();
+            ASSERT_DEVICE_ERROR(encoder.Finish());
+        }
+
+        // It is valid to use the texture as both sampled and readonly depth/stencil attachment in
+        // the same pass
+        {
+            passDescriptor.cDepthStencilAttachmentInfo.depthReadOnly = true;
+
+            wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+            wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
+            pass.SetBindGroup(0, bg);
+            pass.EndPass();
+            encoder.Finish();
+        }
+    }
+
     // Test using multiple writable usages on the same texture in a single pass/dispatch
     TEST_F(ResourceUsageTrackingTest, TextureWithMultipleWriteUsage) {
         // Test render pass