Implement Validation For DepthStencilAttachment ReadOnly

Implements validation for RenderPassDepthStencilAttachmentDescriptor depthReadOnly and
stencilReadOnly flags to match WebGPU specification. Includes corresponding unit tests.

bug: dawn:485
Change-Id: I21e624850d5a393469569417f102fb979dbfdf27
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24602
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

dawn.json

@@ -1147,9 +1147,11 @@
             {"name": "depth load op", "type": "load op"},
             {"name": "depth store op", "type": "store op"},
             {"name": "clear depth", "type": "float"},
+            {"name": "depth read only", "type": "bool", "default": "false"},
             {"name": "stencil load op", "type": "load op"},
             {"name": "stencil store op", "type": "store op"},
-            {"name": "clear stencil", "type": "uint32_t", "default": "0"}
+            {"name": "clear stencil", "type": "uint32_t", "default": "0"},
+            {"name": "stencil read only", "type": "bool", "default": "false"}
         ]
     },
 

src/dawn_native/CommandEncoder.cpp

@@ -298,6 +298,31 @@ namespace dawn_native {
             DAWN_TRY(ValidateStoreOp(depthStencilAttachment->depthStoreOp));
             DAWN_TRY(ValidateStoreOp(depthStencilAttachment->stencilStoreOp));
 
+            if (attachment->GetAspect() == wgpu::TextureAspect::All &&
+                attachment->GetFormat().HasStencil() &&
+                depthStencilAttachment->depthReadOnly != depthStencilAttachment->stencilReadOnly) {
+                return DAWN_VALIDATION_ERROR(
+                    "depthReadOnly and stencilReadOnly must be the same when texture aspect is "
+                    "'all'");
+            }
+
+            if (depthStencilAttachment->depthReadOnly &&
+                (depthStencilAttachment->depthLoadOp != wgpu::LoadOp::Load ||
+                 depthStencilAttachment->depthStoreOp != wgpu::StoreOp::Store)) {
+                return DAWN_VALIDATION_ERROR(
+                    "depthLoadOp must be load and depthStoreOp must be store when depthReadOnly "
+                    "is true.");
+            }
+
+            if (depthStencilAttachment->stencilReadOnly &&
+                (depthStencilAttachment->stencilLoadOp != wgpu::LoadOp::Load ||
+                 depthStencilAttachment->stencilStoreOp != wgpu::StoreOp::Store)) {
+                return DAWN_VALIDATION_ERROR(
+                    "stencilLoadOp must be load and stencilStoreOp must be store when "
+                    "stencilReadOnly "
+                    "is true.");
+            }
+
             if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear &&
                 std::isnan(depthStencilAttachment->clearDepth)) {
                 return DAWN_VALIDATION_ERROR("Depth clear value cannot be NaN");

src/dawn_native/Texture.cpp

@@ -571,6 +571,7 @@ namespace dawn_native {
     TextureViewBase::TextureViewBase(TextureBase* texture, const TextureViewDescriptor* descriptor)
         : ObjectBase(texture->GetDevice()),
           mTexture(texture),
+          mAspect(descriptor->aspect),
           mFormat(GetDevice()->GetValidInternalFormat(descriptor->format)),
           mDimension(descriptor->dimension),
           mRange({descriptor->baseMipLevel, descriptor->mipLevelCount, descriptor->baseArrayLayer,
@@ -596,6 +597,11 @@ namespace dawn_native {
         return mTexture.Get();
     }
 
+    wgpu::TextureAspect TextureViewBase::GetAspect() const {
+        ASSERT(!IsError());
+        return mAspect;
+    }
+
     const Format& TextureViewBase::GetFormat() const {
         ASSERT(!IsError());
         return mFormat;

src/dawn_native/Texture.h

@@ -126,6 +126,7 @@ namespace dawn_native {
         const TextureBase* GetTexture() const;
         TextureBase* GetTexture();
 
+        wgpu::TextureAspect GetAspect() const;
         const Format& GetFormat() const;
         wgpu::TextureViewDimension GetDimension() const;
         uint32_t GetBaseMipLevel() const;
@@ -139,6 +140,7 @@ namespace dawn_native {
 
         Ref<TextureBase> mTexture;
 
+        wgpu::TextureAspect mAspect;
         // TODO(cwallez@chromium.org): This should be deduplicated in the Device
         const Format& mFormat;
         wgpu::TextureViewDimension mDimension;

src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp

@@ -762,6 +762,77 @@ namespace {
         }
     }
 
+    TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilReadOnly) {
+        wgpu::TextureView colorView =
+            Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
+        wgpu::TextureView depthStencilView =
+            Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24PlusStencil8);
+        wgpu::TextureView depthStencilViewNoStencil =
+            Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
+
+        // Tests that a read-only pass with depthReadOnly set to true succeeds.
+        {
+            utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+            renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+            renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
+            AssertBeginRenderPassSuccess(&renderPass);
+        }
+
+        // Tests that a pass with mismatched depthReadOnly and stencilReadOnly values passes when
+        // there is no stencil component in the format.
+        {
+            utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilViewNoStencil);
+            renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+            renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = false;
+            AssertBeginRenderPassSuccess(&renderPass);
+        }
+
+        // Tests that a pass with mismatched depthReadOnly and stencilReadOnly values fails when
+        // both depth and stencil components exist.
+        {
+            utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+            renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+            renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = false;
+            AssertBeginRenderPassError(&renderPass);
+        }
+
+        // Tests that a pass with loadOp set to clear and readOnly set to true fails.
+        {
+            utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+            renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+            renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+            renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
+            AssertBeginRenderPassError(&renderPass);
+        }
+
+        // Tests that a pass with storeOp set to clear and readOnly set to true fails.
+        {
+            utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+            renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+            renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+            renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+            renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
+            AssertBeginRenderPassError(&renderPass);
+        }
+    }
+
     // TODO(cwallez@chromium.org): Constraints on attachment aliasing?
 
 }  // anonymous namespace