Add validation rule for depth/stencil between bundle and pass

If we use render bundle, the compability validation between
writeDepth/Stencil in DepthStencilState in render pipeline and
depth/stencilReadOnly in DepthStencilAttachment in render pass
will become two steps:
  1. validation between render pipeline and render bundle during
    RenderBundleEncoder's SetPipeline().
  2. validation between render bundle and render pass during
    RenderPassEncoder's ExecuteBundles().

So, render bundle is like a bridge for this compability validation
between pipeline and pass.

The first step has been done in previous patch. The patch does
the second step.

Bug: dawn:485

Change-Id: I1226494e901c07bdb9f565bce7b9073d420f2fe2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/66842
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Yunchao He <yunchao.he@intel.com>

src/dawn_native/RenderBundle.cpp

@@ -25,12 +25,16 @@ namespace dawn_native {
     RenderBundleBase::RenderBundleBase(RenderBundleEncoder* encoder,
                                        const RenderBundleDescriptor* descriptor,
                                        Ref<AttachmentState> attachmentState,
+                                       bool depthReadOnly,
+                                       bool stencilReadOnly,
                                        RenderPassResourceUsage resourceUsage,
                                        IndirectDrawMetadata indirectDrawMetadata)
         : ApiObjectBase(encoder->GetDevice(), kLabelNotImplemented),
           mCommands(encoder->AcquireCommands()),
           mIndirectDrawMetadata(std::move(indirectDrawMetadata)),
           mAttachmentState(std::move(attachmentState)),
+          mDepthReadOnly(depthReadOnly),
+          mStencilReadOnly(stencilReadOnly),
           mResourceUsage(std::move(resourceUsage)) {
     }
 
@@ -60,6 +64,16 @@ namespace dawn_native {
         return mAttachmentState.Get();
     }
 
+    bool RenderBundleBase::IsDepthReadOnly() const {
+        ASSERT(!IsError());
+        return mDepthReadOnly;
+    }
+
+    bool RenderBundleBase::IsStencilReadOnly() const {
+        ASSERT(!IsError());
+        return mStencilReadOnly;
+    }
+
     const RenderPassResourceUsage& RenderBundleBase::GetResourceUsage() const {
         ASSERT(!IsError());
         return mResourceUsage;

src/dawn_native/RenderBundle.h

@@ -38,6 +38,8 @@ namespace dawn_native {
         RenderBundleBase(RenderBundleEncoder* encoder,
                          const RenderBundleDescriptor* descriptor,
                          Ref<AttachmentState> attachmentState,
+                         bool depthReadOnly,
+                         bool stencilReadOnly,
                          RenderPassResourceUsage resourceUsage,
                          IndirectDrawMetadata indirectDrawMetadata);
 
@@ -48,6 +50,8 @@ namespace dawn_native {
         CommandIterator* GetCommands();
 
         const AttachmentState* GetAttachmentState() const;
+        bool IsDepthReadOnly() const;
+        bool IsStencilReadOnly() const;
         const RenderPassResourceUsage& GetResourceUsage() const;
         const IndirectDrawMetadata& GetIndirectDrawMetadata();
 
@@ -60,6 +64,8 @@ namespace dawn_native {
         CommandIterator mCommands;
         IndirectDrawMetadata mIndirectDrawMetadata;
         Ref<AttachmentState> mAttachmentState;
+        bool mDepthReadOnly;
+        bool mStencilReadOnly;
         RenderPassResourceUsage mResourceUsage;
     };
 

src/dawn_native/RenderBundleEncoder.cpp

@@ -144,7 +144,8 @@ namespace dawn_native {
             DAWN_TRY(ValidateFinish(usages));
         }
 
-        return new RenderBundleBase(this, descriptor, AcquireAttachmentState(), std::move(usages),
+        return new RenderBundleBase(this, descriptor, AcquireAttachmentState(), IsDepthReadOnly(),
+                                    IsStencilReadOnly(), std::move(usages),
                                     std::move(mIndirectDrawMetadata));
     }
 

src/dawn_native/RenderEncoderBase.cpp

@@ -59,6 +59,16 @@ namespace dawn_native {
         return mAttachmentState.Get();
     }
 
+    bool RenderEncoderBase::IsDepthReadOnly() const {
+        ASSERT(!IsError());
+        return mDepthReadOnly;
+    }
+
+    bool RenderEncoderBase::IsStencilReadOnly() const {
+        ASSERT(!IsError());
+        return mStencilReadOnly;
+    }
+
     Ref<AttachmentState> RenderEncoderBase::AcquireAttachmentState() {
         return std::move(mAttachmentState);
     }

src/dawn_native/RenderEncoderBase.h

@@ -59,6 +59,8 @@ namespace dawn_native {
                              const uint32_t* dynamicOffsets = nullptr);
 
         const AttachmentState* GetAttachmentState() const;
+        bool IsDepthReadOnly() const;
+        bool IsStencilReadOnly() const;
         Ref<AttachmentState> AcquireAttachmentState();
 
       protected:

src/dawn_native/RenderPassEncoder.cpp

@@ -233,15 +233,32 @@ namespace dawn_native {
             this,
             [&](CommandAllocator* allocator) -> MaybeError {
                 if (IsValidationEnabled()) {
+                    const AttachmentState* attachmentState = GetAttachmentState();
+                    bool depthReadOnlyInPass = IsDepthReadOnly();
+                    bool stencilReadOnlyInPass = IsStencilReadOnly();
                     for (uint32_t i = 0; i < count; ++i) {
                         DAWN_TRY(GetDevice()->ValidateObject(renderBundles[i]));
 
                         // TODO(dawn:563): Give more detail about why the states are incompatible.
                         DAWN_INVALID_IF(
-                            GetAttachmentState() != renderBundles[i]->GetAttachmentState(),
+                            attachmentState != renderBundles[i]->GetAttachmentState(),
                             "Attachment state of renderBundles[%i] (%s) is not compatible with "
                             "attachment state of %s.",
                             i, renderBundles[i], this);
+
+                        bool depthReadOnlyInBundle = renderBundles[i]->IsDepthReadOnly();
+                        DAWN_INVALID_IF(
+                            depthReadOnlyInPass != depthReadOnlyInBundle,
+                            "DepthReadOnly (%u) of renderBundle[%i] (%s) is not compatible "
+                            "with DepthReadOnly (%u) of %s.",
+                            depthReadOnlyInBundle, i, renderBundles[i], depthReadOnlyInPass, this);
+
+                        bool stencilReadOnlyInBundle = renderBundles[i]->IsStencilReadOnly();
+                        DAWN_INVALID_IF(stencilReadOnlyInPass != stencilReadOnlyInBundle,
+                                        "StencilReadOnly (%u) of renderBundle[%i] (%s) is not "
+                                        "compatible with StencilReadOnly (%u) of %s.",
+                                        stencilReadOnlyInBundle, i, renderBundles[i],
+                                        stencilReadOnlyInPass, this);
                     }
                 }
 

src/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp

@@ -19,6 +19,9 @@
 #include "tests/unittests/validation/ValidationTest.h"
 
 constexpr static uint32_t kSize = 4;
+// Note that format Depth24PlusStencil8 has both depth and stencil aspects, so parameters
+// depthReadOnly and stencilReadOnly should be the same in render pass and render bundle.
+wgpu::TextureFormat kFormat = wgpu::TextureFormat::Depth24PlusStencil8;
 
 namespace {
 
@@ -76,13 +79,9 @@ namespace {
         }
     };
 
-    // Test depthWrite/stencilWrite in DepthStencilState in pipeline vs
+    // Test depthWrite/stencilWrite in DepthStencilState in render pipeline vs
-    // depthReadOnly/stencilReadOnly in DepthStencilAttachment in pass
+    // depthReadOnly/stencilReadOnly in DepthStencilAttachment in render pass.
     TEST_F(RenderPipelineAndPassCompatibilityTests, WriteAndReadOnlyConflictForDepthStencil) {
-        wgpu::TextureFormat kFormat = wgpu::TextureFormat::Depth24PlusStencil8;
-        // If the format has both depth and stencil aspects, depthReadOnly and stencilReadOnly
-        // should be the same. So it is not necessary to set two separate booleans like
-        // depthReadOnlyInPass and stencilReadOnlyInPass.
         for (bool depthStencilReadOnlyInPass : {true, false}) {
             for (bool depthWriteInPipeline : {true, false}) {
                 for (bool stencilWriteInPipeline : {true, false}) {
@@ -106,14 +105,10 @@ namespace {
         }
     }
 
-    // Test depthWrite/stencilWrite in DepthStencilState in pipeline vs
+    // Test depthWrite/stencilWrite in DepthStencilState in render pipeline vs
-    // depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in RenderBundle
+    // depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in render bundle.
     TEST_F(RenderPipelineAndPassCompatibilityTests,
-           WriteAndReadOnlyConflictForDepthStencilWithRenderBundle) {
+           WriteAndReadOnlyConflictForDepthStencilBetweenPipelineAndBundle) {
-        wgpu::TextureFormat kFormat = wgpu::TextureFormat::Depth24PlusStencil8;
-        // If the format has both depth and stencil aspects, depthReadOnly and stencilReadOnly
-        // should be the same. So it is not necessary to set two separate booleans like
-        // depthReadOnlyInBundle and stencilReadOnlyInBundle.
         for (bool depthStencilReadOnlyInBundle : {true, false}) {
             utils::ComboRenderBundleEncoderDescriptor desc = {};
             desc.depthStencilFormat = kFormat;
@@ -139,6 +134,45 @@ namespace {
         }
     }
 
+    // Test depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in render bundle vs
+    // depthReadOnly/stencilReadOnly in DepthStencilAttachment in render pass.
+    TEST_F(RenderPipelineAndPassCompatibilityTests,
+           WriteAndReadOnlyConflictForDepthStencilBetweenBundleAndPass) {
+        for (bool depthStencilReadOnlyInPass : {true, false}) {
+            for (bool depthStencilReadOnlyInBundle : {true, false}) {
+                for (bool emptyBundle : {true, false}) {
+                    // Create render bundle, with or without a pipeline
+                    utils::ComboRenderBundleEncoderDescriptor desc = {};
+                    desc.depthStencilFormat = kFormat;
+                    desc.depthReadOnly = depthStencilReadOnlyInBundle;
+                    desc.stencilReadOnly = depthStencilReadOnlyInBundle;
+                    wgpu::RenderBundleEncoder renderBundleEncoder =
+                        device.CreateRenderBundleEncoder(&desc);
+                    if (!emptyBundle) {
+                        wgpu::RenderPipeline pipeline = CreatePipeline(
+                            kFormat, !depthStencilReadOnlyInBundle, !depthStencilReadOnlyInBundle);
+                        renderBundleEncoder.SetPipeline(pipeline);
+                        renderBundleEncoder.Draw(3);
+                    }
+                    wgpu::RenderBundle bundle = renderBundleEncoder.Finish();
+
+                    // Create render pass and call ExecuteBundles()
+                    wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+                    utils::ComboRenderPassDescriptor passDescriptor = CreateRenderPassDescriptor(
+                        kFormat, depthStencilReadOnlyInPass, depthStencilReadOnlyInPass);
+                    wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
+                    pass.ExecuteBundles(1, &bundle);
+                    pass.EndPass();
+                    if (depthStencilReadOnlyInPass != depthStencilReadOnlyInBundle) {
+                        ASSERT_DEVICE_ERROR(encoder.Finish());
+                    } else {
+                        encoder.Finish();
+                    }
+                }
+            }
+        }
+    }
+
     // TODO(dawn:485): add more tests. For example:
     //   - depth/stencil attachment should be designated if depth/stencil test is enabled.
     //   - pipeline and pass compatibility tests for color attachment(s).