commit 81768c131f24bdd22003aff40fe81d978bf0b903
author Brandon Jones <bajones@chromium.org> Wed Jul 20 04:18:35 2022 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jul 20 04:18:35 2022 +0000
tree a9d06c8286244970aecf69f31d819ff72e6a1ba6
parent 6bb4e1aeff04dfed25964573060fdd706ced13f9

Validate that depth clear values are between 0, 1

Fixes missing validation exposed by the addition of
https://github.com/gpuweb/cts/pull/1640 to the CTS.

Change-Id: Ib51124603a6fcec973eeab9ac1ed989add209c9a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/96481
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/dawn/native/CommandEncoder.cpp

diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index 6f32d92..0bc63fa 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -382,10 +382,16 @@
     if (!std::isnan(depthStencilAttachment->clearDepth)) {
         // TODO(dawn:1269): Remove this branch after the deprecation period.
         device->EmitDeprecationWarning("clearDepth is deprecated, prefer depthClearValue instead.");
-    } else {
-        DAWN_INVALID_IF(depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear &&
-                            std::isnan(depthStencilAttachment->depthClearValue),
+        DAWN_INVALID_IF(
+            depthStencilAttachment->clearDepth < 0.0f || depthStencilAttachment->clearDepth > 1.0f,
+            "clearDepth is not between 0.0 and 1.0");
+
+    } else if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear) {
+        DAWN_INVALID_IF(std::isnan(depthStencilAttachment->depthClearValue),
                         "depthClearValue is NaN.");
+        DAWN_INVALID_IF(depthStencilAttachment->depthClearValue < 0.0f ||
+                            depthStencilAttachment->depthClearValue > 1.0f,
+                        "depthClearValue is not between 0.0 and 1.0");
     }
 
     // TODO(dawn:1269): Remove after the deprecation period.

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 6a85a46..f0cfd3c 100644
--- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -1057,7 +1057,7 @@
         AssertBeginRenderPassError(&renderPass);
     }
 
-    // Tests that INFINITY can be used in depthClearValue.
+    // Tests that INFINITY cannot be used in depthClearValue.
     {
         wgpu::TextureView depth =
             Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
@@ -1065,13 +1065,66 @@
         renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined;
         renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined;
         renderPass.cDepthStencilAttachmentInfo.depthClearValue = INFINITY;
-        AssertBeginRenderPassSuccess(&renderPass);
+        AssertBeginRenderPassError(&renderPass);
     }
 
     // TODO(https://crbug.com/dawn/666): Add a test case for clearStencil for stencilOnly
     // once stencil8 is supported.
 }
 
+// Tests that depth clear values mut be between 0 and 1, inclusive.
+TEST_F(RenderPassDescriptorValidationTest, ValidateDepthClearValueRange) {
+    wgpu::TextureView depth = Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
+
+    utils::ComboRenderPassDescriptor renderPass({}, depth);
+    renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined;
+    renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined;
+
+    // 0, 1, and any value in between are be valid.
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 0;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 0.1;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 0.5;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 0.82;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 1;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    // Values less than 0 or greater than 1 are invalid.
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = -1;
+    AssertBeginRenderPassError(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 2;
+    AssertBeginRenderPassError(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = -0.001;
+    AssertBeginRenderPassError(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 1.001;
+    AssertBeginRenderPassError(&renderPass);
+
+    // Clear values are not validated if the depthLoadOp is Load.
+    renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = -1;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 2;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = -0.001;
+    AssertBeginRenderPassSuccess(&renderPass);
+
+    renderPass.cDepthStencilAttachmentInfo.depthClearValue = 1.001;
+    AssertBeginRenderPassSuccess(&renderPass);
+}
+
 TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilReadOnly) {
     wgpu::TextureView colorView = Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
     wgpu::TextureView depthStencilView =