From 0e922b5995951dc431676cf1c7004c087a92270c Mon Sep 17 00:00:00 2001
From: Yunchao He <yunchao.he@intel.com>
Date: Thu, 9 Dec 2021 22:02:14 +0000
Subject: [PATCH] Add validations for depth/stencil aspect requirement for
 DepthStencilState

In RenderPipelineDescriptor.DepthStencilState, if depth test or depth
write is enabled, the texture format must have depth aspect. Likewise,
if stencil test or stencil write is enabled, the texture format must
have stencil aspect.

Bug: dawn:1226

Change-Id: I9d7efb25675ff2c90704fa45703fb542bab6f1f5
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/72101
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Yunchao He <yunchao.he@intel.com>
---
 src/dawn_native/RenderPipeline.cpp            | 13 +++++
 .../RenderPipelineValidationTests.cpp         | 54 +++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp
index fb9ad46768..d16826de23 100644
--- a/src/dawn_native/RenderPipeline.cpp
+++ b/src/dawn_native/RenderPipeline.cpp
@@ -288,6 +288,19 @@ namespace dawn_native {
                             "Either depthBiasSlopeScale (%f) or depthBiasClamp (%f) is NaN.",
                             descriptor->depthBiasSlopeScale, descriptor->depthBiasClamp);
 
+            DAWN_INVALID_IF(
+                !format->HasDepth() && (descriptor->depthCompare != wgpu::CompareFunction::Always ||
+                                        descriptor->depthWriteEnabled),
+                "Depth stencil format (%s) doesn't have depth aspect while depthCompare (%s) is "
+                "not %s or depthWriteEnabled (%u) is true.",
+                descriptor->format, descriptor->depthCompare, wgpu::CompareFunction::Always,
+                descriptor->depthWriteEnabled);
+
+            DAWN_INVALID_IF(!format->HasStencil() && StencilTestEnabled(descriptor),
+                            "Depth stencil format (%s) doesn't have stencil aspect while stencil "
+                            "test or stencil write is enabled.",
+                            descriptor->format);
+
             return {};
         }
 
diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
index 009c1871a2..d7d2d13d4b 100644
--- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -117,6 +117,60 @@ TEST_F(RenderPipelineValidationTest, DepthBiasParameterNotBeNaN) {
     }
 }
 
+// Tests that depth or stencil aspect is required if we enable depth or stencil test.
+TEST_F(RenderPipelineValidationTest, DepthStencilAspectRequirement) {
+    // Control case, stencil aspect is required if stencil test or stencil write is enabled
+    {
+        utils::ComboRenderPipelineDescriptor descriptor;
+        descriptor.vertex.module = vsModule;
+        descriptor.cFragment.module = fsModule;
+        wgpu::DepthStencilState* depthStencil =
+            descriptor.EnableDepthStencil(wgpu::TextureFormat::Depth24PlusStencil8);
+        depthStencil->stencilFront.compare = wgpu::CompareFunction::LessEqual;
+        depthStencil->stencilBack.failOp = wgpu::StencilOperation::Replace;
+        device.CreateRenderPipeline(&descriptor);
+    }
+
+    // It is invalid if the texture format doesn't have stencil aspect while stencil test is
+    // enabled (depthStencilState.stencilFront are not default values).
+    {
+        utils::ComboRenderPipelineDescriptor descriptor;
+        descriptor.vertex.module = vsModule;
+        descriptor.cFragment.module = fsModule;
+        wgpu::DepthStencilState* depthStencil =
+            descriptor.EnableDepthStencil(wgpu::TextureFormat::Depth24Plus);
+        depthStencil->stencilFront.compare = wgpu::CompareFunction::LessEqual;
+        ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+    }
+
+    // It is invalid if the texture format doesn't have stencil aspect while stencil write is
+    // enabled (depthStencilState.stencilBack are not default values).
+    {
+        utils::ComboRenderPipelineDescriptor descriptor;
+        descriptor.vertex.module = vsModule;
+        descriptor.cFragment.module = fsModule;
+        wgpu::DepthStencilState* depthStencil =
+            descriptor.EnableDepthStencil(wgpu::TextureFormat::Depth24Plus);
+        depthStencil->stencilBack.failOp = wgpu::StencilOperation::Replace;
+        ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+    }
+
+    // Control case, depth aspect is required if depth test or depth write is enabled
+    {
+        utils::ComboRenderPipelineDescriptor descriptor;
+        descriptor.vertex.module = vsModule;
+        descriptor.cFragment.module = fsModule;
+        wgpu::DepthStencilState* depthStencil =
+            descriptor.EnableDepthStencil(wgpu::TextureFormat::Depth24PlusStencil8);
+        depthStencil->depthCompare = wgpu::CompareFunction::LessEqual;
+        depthStencil->depthWriteEnabled = true;
+        device.CreateRenderPipeline(&descriptor);
+    }
+
+    // TODO(dawn:666): Add tests for stencil-only format (Stencil8) with depth test or depth write
+    // enabled when Stencil8 format is implemented
+}
+
 // Tests that at least one color target state is required.
 TEST_F(RenderPipelineValidationTest, ColorTargetStateRequired) {
     {