Make properties required
This commit makes depthWriteEnabled and depthCompare required and
makes depthClearValue conditionally required for the spec change
in WebGPU V1.
https://github.com/gpuweb/gpuweb/pull/3849
depthClearValue is required if depthLoadOp is clear and the
attachment has a depth aspect. To simulate it, this commit lets
NAN represent unspecified depthClearValue and lets the default
value of depthClearValue be NAN.
Bug: dawn:1669
Change-Id: I469338e909b1d3c345bc2642ee47daee858909ca
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/120620
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/dawn.json b/dawn.json
index fc3b0f5..52836d3 100644
--- a/dawn.json
+++ b/dawn.json
@@ -1989,7 +1989,7 @@
{"name": "view", "type": "texture view"},
{"name": "depth load op", "type": "load op", "default": "undefined"},
{"name": "depth store op", "type": "store op", "default": "undefined"},
- {"name": "depth clear value", "type": "float", "default": "0"},
+ {"name": "depth clear value", "type": "float", "default": "NAN"},
{"name": "depth read only", "type": "bool", "default": "false"},
{"name": "stencil load op", "type": "load op", "default": "undefined"},
{"name": "stencil store op", "type": "store op", "default": "undefined"},
@@ -2282,8 +2282,8 @@
"extensible": "in",
"members": [
{"name": "format", "type": "texture format"},
- {"name": "depth write enabled", "type": "bool", "default": "false"},
- {"name": "depth compare", "type": "compare function", "default": "always"},
+ {"name": "depth write enabled", "type": "bool"},
+ {"name": "depth compare", "type": "compare function"},
{"name": "stencil front", "type": "stencil face state"},
{"name": "stencil back", "type": "stencil face state"},
{"name": "stencil read mask", "type": "uint32_t", "default": "0xFFFFFFFF"},
diff --git a/src/dawn/native/BlitBufferToDepthStencil.cpp b/src/dawn/native/BlitBufferToDepthStencil.cpp
index bb86769..7ec11c6 100644
--- a/src/dawn/native/BlitBufferToDepthStencil.cpp
+++ b/src/dawn/native/BlitBufferToDepthStencil.cpp
@@ -137,6 +137,7 @@
DepthStencilState dsState = {};
dsState.format = wgpu::TextureFormat::Depth16Unorm;
dsState.depthWriteEnabled = true;
+ dsState.depthCompare = wgpu::CompareFunction::Always;
RenderPipelineDescriptor renderPipelineDesc = {};
renderPipelineDesc.vertex.module = shaderModule.Get();
@@ -186,6 +187,7 @@
DepthStencilState dsState = {};
dsState.format = format;
dsState.depthWriteEnabled = false;
+ dsState.depthCompare = wgpu::CompareFunction::Always;
dsState.stencilFront.passOp = wgpu::StencilOperation::Replace;
RenderPipelineDescriptor renderPipelineDesc = {};
@@ -288,6 +290,7 @@
RenderPassDepthStencilAttachment dsAttachment;
dsAttachment.view = dstView.Get();
+ dsAttachment.depthClearValue = 0.0;
dsAttachment.depthLoadOp = wgpu::LoadOp::Load;
dsAttachment.depthStoreOp = wgpu::StoreOp::Store;
@@ -400,6 +403,7 @@
}
RenderPassDepthStencilAttachment dsAttachment;
+ dsAttachment.depthClearValue = 0.0;
dsAttachment.view = dstView.Get();
if (format.HasDepth()) {
dsAttachment.depthLoadOp = wgpu::LoadOp::Load;
diff --git a/src/dawn/native/BlitDepthToDepth.cpp b/src/dawn/native/BlitDepthToDepth.cpp
index 1a7f9b9..792690d 100644
--- a/src/dawn/native/BlitDepthToDepth.cpp
+++ b/src/dawn/native/BlitDepthToDepth.cpp
@@ -75,6 +75,7 @@
DepthStencilState dsState = {};
dsState.format = format;
dsState.depthWriteEnabled = true;
+ dsState.depthCompare = wgpu::CompareFunction::Always;
RenderPipelineDescriptor renderPipelineDesc = {};
renderPipelineDesc.vertex.module = shaderModule.Get();
@@ -203,6 +204,7 @@
RenderPassDepthStencilAttachment dsAttachment = {};
dsAttachment.view = dstView.Get();
+ dsAttachment.depthClearValue = 0.0;
dsAttachment.depthLoadOp = wgpu::LoadOp::Load;
dsAttachment.depthStoreOp = wgpu::StoreOp::Store;
if (dst.texture->GetFormat().HasStencil()) {
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index 07a8511..86c95b5 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -371,12 +371,18 @@
depthStencilAttachment->stencilReadOnly);
}
- if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear) {
- DAWN_INVALID_IF(std::isnan(depthStencilAttachment->depthClearValue),
- "depthClearValue is NaN.");
+ if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear &&
+ IsSubset(Aspect::Depth, attachment->GetAspects())) {
+ DAWN_INVALID_IF(
+ std::isnan(depthStencilAttachment->depthClearValue),
+ "depthClearValue (%f) must be set and must not be a NaN value if the attachment "
+ "(%s) has a depth aspect and depthLoadOp is clear.",
+ depthStencilAttachment->depthClearValue, attachment);
DAWN_INVALID_IF(depthStencilAttachment->depthClearValue < 0.0f ||
depthStencilAttachment->depthClearValue > 1.0f,
- "depthClearValue is not between 0.0 and 1.0");
+ "depthClearValue (%f) must be between 0.0 and 1.0 if the attachment (%s) "
+ "has a depth aspect and depthLoadOp is clear.",
+ depthStencilAttachment->depthClearValue, attachment);
}
// *sampleCount == 0 must only happen when there is no color attachment. In that case we
diff --git a/src/dawn/tests/end2end/DepthStencilStateTests.cpp b/src/dawn/tests/end2end/DepthStencilStateTests.cpp
index 97b40bc..c2446bc 100644
--- a/src/dawn/tests/end2end/DepthStencilStateTests.cpp
+++ b/src/dawn/tests/end2end/DepthStencilStateTests.cpp
@@ -781,6 +781,8 @@
// Test that the front and back stencil states are set correctly (and take frontFace into account)
TEST_P(DepthStencilStateTest, StencilFrontAndBackFace) {
wgpu::DepthStencilState state;
+ state.depthWriteEnabled = false;
+ state.depthCompare = wgpu::CompareFunction::Always;
state.stencilFront.compare = wgpu::CompareFunction::Always;
state.stencilBack.compare = wgpu::CompareFunction::Never;
@@ -794,12 +796,16 @@
// Test that the depth reference of a new render pass is initialized to default value 0
TEST_P(DepthStencilStateTest, StencilReferenceInitialized) {
wgpu::DepthStencilState stencilAlwaysReplaceState;
+ stencilAlwaysReplaceState.depthWriteEnabled = false;
+ stencilAlwaysReplaceState.depthCompare = wgpu::CompareFunction::Always;
stencilAlwaysReplaceState.stencilFront.compare = wgpu::CompareFunction::Always;
stencilAlwaysReplaceState.stencilFront.passOp = wgpu::StencilOperation::Replace;
stencilAlwaysReplaceState.stencilBack.compare = wgpu::CompareFunction::Always;
stencilAlwaysReplaceState.stencilBack.passOp = wgpu::StencilOperation::Replace;
wgpu::DepthStencilState stencilEqualKeepState;
+ stencilEqualKeepState.depthWriteEnabled = false;
+ stencilEqualKeepState.depthCompare = wgpu::CompareFunction::Always;
stencilEqualKeepState.stencilFront.compare = wgpu::CompareFunction::Equal;
stencilEqualKeepState.stencilFront.passOp = wgpu::StencilOperation::Keep;
stencilEqualKeepState.stencilBack.compare = wgpu::CompareFunction::Equal;
diff --git a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
index 73bf1f5..efc7054 100644
--- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -1052,6 +1052,7 @@
wgpu::TextureView depth =
Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
utils::ComboRenderPassDescriptor renderPass({color}, depth);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Clear;
renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined;
renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined;
renderPass.cDepthStencilAttachmentInfo.depthClearValue = NAN;
@@ -1063,6 +1064,7 @@
wgpu::TextureView depth =
Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
utils::ComboRenderPassDescriptor renderPass({color}, depth);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Clear;
renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined;
renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined;
renderPass.cDepthStencilAttachmentInfo.depthClearValue = INFINITY;
@@ -1126,6 +1128,45 @@
AssertBeginRenderPassSuccess(&renderPass);
}
+// Tests that default depthClearValue is required if attachment has a depth aspect and depthLoadOp
+// is clear.
+TEST_F(RenderPassDescriptorValidationTest, DefaultDepthClearValue) {
+ wgpu::TextureView depthView =
+ Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
+ wgpu::TextureView stencilView = Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Stencil8);
+
+ wgpu::RenderPassDepthStencilAttachment depthStencilAttachment;
+
+ wgpu::RenderPassDescriptor renderPassDescriptor;
+ renderPassDescriptor.colorAttachmentCount = 0;
+ renderPassDescriptor.colorAttachments = nullptr;
+ renderPassDescriptor.depthStencilAttachment = &depthStencilAttachment;
+
+ // Default depthClearValue should be accepted if attachment doesn't have
+ // a depth aspect.
+ depthStencilAttachment.view = stencilView;
+ depthStencilAttachment.stencilLoadOp = wgpu::LoadOp::Load;
+ depthStencilAttachment.stencilStoreOp = wgpu::StoreOp::Store;
+ AssertBeginRenderPassSuccess(&renderPassDescriptor);
+
+ // Default depthClearValue should be accepted if depthLoadOp is not clear.
+ depthStencilAttachment.view = depthView;
+ depthStencilAttachment.stencilLoadOp = wgpu::LoadOp::Undefined;
+ depthStencilAttachment.stencilStoreOp = wgpu::StoreOp::Undefined;
+ depthStencilAttachment.depthLoadOp = wgpu::LoadOp::Load;
+ depthStencilAttachment.depthStoreOp = wgpu::StoreOp::Store;
+ AssertBeginRenderPassSuccess(&renderPassDescriptor);
+
+ // Default depthClearValue should fail the validation
+ // if attachment has a depth aspect and depthLoadOp is clear.
+ depthStencilAttachment.depthLoadOp = wgpu::LoadOp::Clear;
+ AssertBeginRenderPassError(&renderPassDescriptor);
+
+ // The validation should pass if valid depthClearValue is provided.
+ depthStencilAttachment.depthClearValue = 0.0f;
+ AssertBeginRenderPassSuccess(&renderPassDescriptor);
+}
+
TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilReadOnly) {
wgpu::TextureView colorView = Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
wgpu::TextureView depthStencilView =
