Validate depthSlice not defined for non-3D color attachments

The depthSlice has been initialized with WGPU_DEPTH_SLICE_UNDEFINED in
Blink, we can add the validation back for 2D color attachments.

And use constexpr instead of macro for the undefined value.

Bug: dawn:1020
Change-Id: Idbd275cbcdc8f17f9294c1fb0e5551ce05e1c52b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/166364
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Hao Li <hao.x.li@intel.com>

src/dawn/native/CommandEncoder.cpp

diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index a32aea4..be4d81d 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -113,7 +113,7 @@
     // attachment in the render pass descriptor.
     MaybeError AddAttachment(const TextureViewBase* attachment,
                              AttachmentType attachmentType,
-                             uint32_t depthSlice = WGPU_DEPTH_SLICE_UNDEFINED) {
+                             uint32_t depthSlice = wgpu::kDepthSliceUndefined) {
         if (attachment == nullptr) {
             return {};
         }
@@ -168,9 +168,7 @@
             DAWN_ASSERT(attachment->GetBaseArrayLayer() == 0);
             record.depthOrArrayLayer = depthSlice;
         } else {
-            // TODO(dawn:1020): Assert depthSlice should be the 'WGPU_DEPTH_SLICE_UNDEFINED' value
-            // if the attachment is a non-3d texture view after it's initialized correctly in Blink.
-            // DAWN_ASSERT(depthSlice == WGPU_DEPTH_SLICE_UNDEFINED);
+            DAWN_ASSERT(depthSlice == wgpu::kDepthSliceUndefined);
             record.depthOrArrayLayer = attachment->GetBaseArrayLayer();
         }
 
@@ -356,16 +354,15 @@
 MaybeError ValidateColorAttachmentDepthSlice(const TextureViewBase* attachment,
                                              uint32_t depthSlice) {
     if (attachment->GetDimension() != wgpu::TextureViewDimension::e3D) {
-        // TODO(dawn:1020): Validate depthSlice must not set for non-3d attachments. The depthSlice
-        // from WebGPU is not the 'WGPU_DEPTH_SLICE_UNDEFINED' value if it's not defined, we need to
-        // initialize it in Blink first, otherwise it will always be validated as set.
+        DAWN_INVALID_IF(depthSlice != wgpu::kDepthSliceUndefined,
+                        "depthSlice (%u) is defined for a non-3D attachment (%s).", depthSlice,
+                        attachment);
         return {};
     }
 
-    DAWN_INVALID_IF(depthSlice == WGPU_DEPTH_SLICE_UNDEFINED,
-                    "depthSlice (%u) must be set and must not be undefined value (%u) for a 3D "
-                    "attachment (%s).",
-                    depthSlice, WGPU_DEPTH_SLICE_UNDEFINED, attachment);
+    DAWN_INVALID_IF(depthSlice == wgpu::kDepthSliceUndefined,
+                    "depthSlice (%u) for a 3D attachment (%s) is undefined.", depthSlice,
+                    attachment);
 
     const Extent3D& attachmentSize = attachment->GetSingleSubresourceVirtualSize();
     DAWN_INVALID_IF(depthSlice >= attachmentSize.depthOrArrayLayers,
@@ -1116,10 +1113,10 @@
 
                     cmdColorAttachment.view = colorTarget;
                     // Explicitly set depthSlice to 0 if it's undefined. The
-                    // WGPU_DEPTH_SLICE_UNDEFINED is defined to differentiate between `undefined`
+                    // wgpu::kDepthSliceUndefined is defined to differentiate between `undefined`
                     // and 0 for depthSlice, but we use it as 0 for 2d attachments in backends.
                     cmdColorAttachment.depthSlice =
-                        descColorAttachment.depthSlice == WGPU_DEPTH_SLICE_UNDEFINED
+                        descColorAttachment.depthSlice == wgpu::kDepthSliceUndefined
                             ? 0
                             : descColorAttachment.depthSlice;
                     cmdColorAttachment.loadOp = descColorAttachment.loadOp;

src/dawn/samples/CHelloTriangle.cpp

diff --git a/src/dawn/samples/CHelloTriangle.cpp b/src/dawn/samples/CHelloTriangle.cpp
index 67252ba..b2030ea 100644
--- a/src/dawn/samples/CHelloTriangle.cpp
+++ b/src/dawn/samples/CHelloTriangle.cpp
@@ -117,6 +117,8 @@
     WGPURenderPassColorAttachment colorAttachment = {};
     {
         colorAttachment.view = backbufferView;
+        // The depthSlice must be initialized with the 'undefined' value for 2d color attachments.
+        colorAttachment.depthSlice = WGPU_DEPTH_SLICE_UNDEFINED;
         colorAttachment.resolveTarget = nullptr;
         colorAttachment.clearValue = {0.0f, 0.0f, 0.0f, 0.0f};
         colorAttachment.loadOp = WGPULoadOp_Clear;

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

diff --git a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
index 8444f2d..53fd728 100644
--- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -496,7 +496,18 @@
         AssertBeginRenderPassError(&renderPass);
     }
 
-    // TODO(dawn:1020): Add tests to check depthSlice must not be set for non-3D attachments.
+    // Control case: It's valid if depthSlice is unset for a non-3D color attachment.
+    {
+        utils::ComboRenderPassDescriptor renderPass({colorView2D});
+        AssertBeginRenderPassSuccess(&renderPass);
+    }
+
+    // It's invalid if depthSlice is set for a non-3D color attachment.
+    {
+        utils::ComboRenderPassDescriptor renderPass({colorView2D});
+        renderPass.cColorAttachments[0].depthSlice = 0;
+        AssertBeginRenderPassError(&renderPass);
+    }
 }
 
 // Check that the depth slices of a 3D color attachment cannot overlap in same render pass.