Add validation rule for depth/stencil between pipeline and render bundle

This change also adds a unittest to validation colorFormatCount in
RenderBundleEncoderDescriptor, and fixes a style issue as well.

Bug: dawn:485

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

src/dawn_native/RenderBundleEncoder.cpp

@@ -90,8 +90,8 @@ namespace dawn_native {
         : RenderEncoderBase(device,
                             &mBundleEncodingContext,
                             device->GetOrCreateAttachmentState(descriptor),
-                            false,
+                            descriptor->depthReadOnly,
-                            false),
+                            descriptor->stencilReadOnly),
           mBundleEncodingContext(device, this) {
     }
 

src/dawn_native/RenderPipeline.cpp

@@ -543,15 +543,15 @@ namespace dawn_native {
         return stages;
     }
 
-    bool StencilTestEnabled(const DepthStencilState* mDepthStencil) {
+    bool StencilTestEnabled(const DepthStencilState* depthStencil) {
-        return mDepthStencil->stencilBack.compare != wgpu::CompareFunction::Always ||
+        return depthStencil->stencilBack.compare != wgpu::CompareFunction::Always ||
-               mDepthStencil->stencilBack.failOp != wgpu::StencilOperation::Keep ||
+               depthStencil->stencilBack.failOp != wgpu::StencilOperation::Keep ||
-               mDepthStencil->stencilBack.depthFailOp != wgpu::StencilOperation::Keep ||
+               depthStencil->stencilBack.depthFailOp != wgpu::StencilOperation::Keep ||
-               mDepthStencil->stencilBack.passOp != wgpu::StencilOperation::Keep ||
+               depthStencil->stencilBack.passOp != wgpu::StencilOperation::Keep ||
-               mDepthStencil->stencilFront.compare != wgpu::CompareFunction::Always ||
+               depthStencil->stencilFront.compare != wgpu::CompareFunction::Always ||
-               mDepthStencil->stencilFront.failOp != wgpu::StencilOperation::Keep ||
+               depthStencil->stencilFront.failOp != wgpu::StencilOperation::Keep ||
-               mDepthStencil->stencilFront.depthFailOp != wgpu::StencilOperation::Keep ||
+               depthStencil->stencilFront.depthFailOp != wgpu::StencilOperation::Keep ||
-               mDepthStencil->stencilFront.passOp != wgpu::StencilOperation::Keep;
+               depthStencil->stencilFront.passOp != wgpu::StencilOperation::Keep;
     }
 
     // RenderPipelineBase

src/dawn_native/RenderPipeline.h

@@ -41,7 +41,7 @@ namespace dawn_native {
 
     bool IsStripPrimitiveTopology(wgpu::PrimitiveTopology primitiveTopology);
 
-    bool StencilTestEnabled(const DepthStencilState* mDepthStencil);
+    bool StencilTestEnabled(const DepthStencilState* depthStencil);
 
     struct VertexAttributeInfo {
         wgpu::VertexFormat format;

src/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "utils/ComboRenderBundleEncoderDescriptor.h"
 #include "utils/ComboRenderPipelineDescriptor.h"
 #include "utils/WGPUHelpers.h"
 
@@ -105,8 +106,40 @@ namespace {
         }
     }
 
+    // Test depthWrite/stencilWrite in DepthStencilState in pipeline vs
+    // depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in RenderBundle
+    TEST_F(RenderPipelineAndPassCompatibilityTests,
+           WriteAndReadOnlyConflictForDepthStencilWithRenderBundle) {
+        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;
+            desc.depthReadOnly = depthStencilReadOnlyInBundle;
+            desc.stencilReadOnly = depthStencilReadOnlyInBundle;
+
+            for (bool depthWriteInPipeline : {true, false}) {
+                for (bool stencilWriteInPipeline : {true, false}) {
+                    wgpu::RenderBundleEncoder renderBundleEncoder =
+                        device.CreateRenderBundleEncoder(&desc);
+                    wgpu::RenderPipeline pipeline =
+                        CreatePipeline(kFormat, depthWriteInPipeline, stencilWriteInPipeline);
+                    renderBundleEncoder.SetPipeline(pipeline);
+                    renderBundleEncoder.Draw(3);
+                    if (depthStencilReadOnlyInBundle &&
+                        (depthWriteInPipeline || stencilWriteInPipeline)) {
+                        ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+                    } else {
+                        renderBundleEncoder.Finish();
+                    }
+                }
+            }
+        }
+    }
+
     // TODO(dawn:485): add more tests. For example:
-    //   - readOnly vs write for depth/stencil with renderbundle.
     //   - depth/stencil attachment should be designated if depth/stencil test is enabled.
     //   - pipeline and pass compatibility tests for color attachment(s).
     //   - pipeline and pass compatibility tests for compute.

src/tests/unittests/validation/RenderBundleValidationTests.cpp

@@ -611,6 +611,30 @@ TEST_F(RenderBundleValidationTest, RequiresAtLeastOneTextureFormat) {
     }
 }
 
+// Test that it is invalid to create a render bundle with no texture formats
+TEST_F(RenderBundleValidationTest, ColorFormatsCountOutOfBounds) {
+    std::array<wgpu::TextureFormat, kMaxColorAttachments + 1> colorFormats;
+    for (uint32_t i = 0; i < colorFormats.size(); ++i) {
+        colorFormats[i] = wgpu::TextureFormat::RGBA8Unorm;
+    }
+
+    // colorFormatsCount <= kMaxColorAttachments is valid.
+    {
+        wgpu::RenderBundleEncoderDescriptor desc;
+        desc.colorFormatsCount = kMaxColorAttachments;
+        desc.colorFormats = colorFormats.data();
+        device.CreateRenderBundleEncoder(&desc);
+    }
+
+    // colorFormatsCount > kMaxColorAttachments is invalid.
+    {
+        wgpu::RenderBundleEncoderDescriptor desc;
+        desc.colorFormatsCount = kMaxColorAttachments + 1;
+        desc.colorFormats = colorFormats.data();
+        ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+    }
+}
+
 // Test that render bundle color formats cannot be set to undefined.
 TEST_F(RenderBundleValidationTest, ColorFormatUndefined) {
     utils::ComboRenderBundleEncoderDescriptor desc = {};