commit 6c084ea0df719106b8d7b0b3d216eaa76e5dc8f8
author François Beaufort <beaufort.francois@gmail.com> Thu Oct 19 09:41:32 2023 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Oct 19 09:41:32 2023 +0000
tree 3680d022b0918adbe1d0f8f5e0199c174f401ca0
parent e8afe0a97f6e847af80b62202056cf5b90156f40

Do no require depthCompare for format with depth if not used

Spec PR: https://github.com/gpuweb/gpuweb/pull/4336

Bug: dawn:2132
Change-Id: I2947aecf82fa1a5cab04e7670d088849a5e641a3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/155703
Commit-Queue: Fr <beaufort.francois@gmail.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/RenderPipeline.cpp

diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp
index ffaa4c1..cd47356 100644
--- a/src/dawn/native/RenderPipeline.cpp
+++ b/src/dawn/native/RenderPipeline.cpp
@@ -304,9 +304,15 @@
         descriptor->depthBiasSlopeScale, descriptor->depthBiasClamp);
 
     DAWN_INVALID_IF(
-        format->HasDepth() && descriptor->depthCompare == wgpu::CompareFunction::Undefined,
-        "Depth stencil format (%s) has a depth aspect and depthCompare is %s.", descriptor->format,
-        wgpu::CompareFunction::Undefined);
+        format->HasDepth() && descriptor->depthCompare == wgpu::CompareFunction::Undefined &&
+            (descriptor->depthWriteEnabled ||
+             descriptor->stencilFront.depthFailOp != wgpu::StencilOperation::Keep ||
+             descriptor->stencilBack.depthFailOp != wgpu::StencilOperation::Keep),
+        "Depth stencil format (%s) has a depth aspect and depthCompare is %s while it's actually "
+        "used by depthWriteEnabled (%u), or stencil front depth fail operation (%s), or "
+        "stencil back depth fail operation (%s).",
+        descriptor->format, wgpu::CompareFunction::Undefined, descriptor->depthWriteEnabled,
+        descriptor->stencilFront.depthFailOp, descriptor->stencilBack.depthFailOp);
 
     UnpackedDepthStencilStateChain unpacked;
     DAWN_TRY_ASSIGN(unpacked, ValidateAndUnpackChain(descriptor));
@@ -856,6 +862,12 @@
             mDepthStencil.depthWriteEnabled = false;
             mDepthStencil.depthCompare = wgpu::CompareFunction::Always;
         }
+        if (format.HasDepth() && mDepthStencil.depthCompare == wgpu::CompareFunction::Undefined &&
+            !mDepthStencil.depthWriteEnabled &&
+            mDepthStencil.stencilFront.depthFailOp == wgpu::StencilOperation::Keep &&
+            mDepthStencil.stencilBack.depthFailOp == wgpu::StencilOperation::Keep) {
+            mDepthStencil.depthCompare = wgpu::CompareFunction::Always;
+        }
         mWritesDepth = mDepthStencil.depthWriteEnabled;
         if (mDepthStencil.stencilWriteMask) {
             if ((mPrimitive.cullMode != wgpu::CullMode::Front &&

src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp

diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
index acaef2e..32a5e0e 100644
--- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -1269,7 +1269,33 @@
     descriptor.cDepthStencil.depthCompare = wgpu::CompareFunction::Undefined;
     ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
 
+    // Undefined is valid though if depthCompare is not used by anything.
     descriptor.cDepthStencil.depthWriteEnabled = false;
+    descriptor.cDepthStencil.stencilFront.depthFailOp = wgpu::StencilOperation::Keep;
+    descriptor.cDepthStencil.stencilBack.depthFailOp = wgpu::StencilOperation::Keep;
+    device.CreateRenderPipeline(&descriptor);
+
+    // Undefined is invalid if depthCompare is used by depthWriteEnabled.
+    descriptor.cDepthStencil.depthWriteEnabled = true;
+    descriptor.cDepthStencil.stencilFront.depthFailOp = wgpu::StencilOperation::Keep;
+    descriptor.cDepthStencil.stencilBack.depthFailOp = wgpu::StencilOperation::Keep;
+    ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+
+    // Undefined is invalid if depthCompare is used by stencilFront.depthFailOp.
+    descriptor.cDepthStencil.depthWriteEnabled = false;
+    descriptor.cDepthStencil.stencilFront.depthFailOp = wgpu::StencilOperation::Zero;
+    descriptor.cDepthStencil.stencilBack.depthFailOp = wgpu::StencilOperation::Keep;
+    ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+
+    // Undefined is invalid if depthCompare is used by stencilBack.depthFailOp.
+    descriptor.cDepthStencil.depthWriteEnabled = false;
+    descriptor.cDepthStencil.stencilFront.depthFailOp = wgpu::StencilOperation::Keep;
+    descriptor.cDepthStencil.stencilBack.depthFailOp = wgpu::StencilOperation::Zero;
+    ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
+
+    descriptor.cDepthStencil.depthWriteEnabled = false;
+    descriptor.cDepthStencil.stencilFront.depthFailOp = wgpu::StencilOperation::Keep;
+    descriptor.cDepthStencil.stencilBack.depthFailOp = wgpu::StencilOperation::Keep;
     descriptor.EnableDepthStencil(wgpu::TextureFormat::Stencil8);
 
     // Always is valid for format with no depth.