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>
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;
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;
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.
