commit 79d209912518ddaa598e60a541009ec96b7f17dd
author Yunchao He <yunchao.he@intel.com> Fri Oct 15 16:56:13 2021 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Oct 15 16:56:13 2021 +0000
tree 40f75ff8ae96ae0ef95f5c070ad5c13aeb6e422b
parent 17dd734e54c47c5bb3693dd6d044b5759437d4d0

Add depth/stencilReadOnly and validations in RenderBundleEncoderDescriptor

This change adds two arguments depthReadOnly and stencilReadOnly
into RenderBundleEncoderDescriptor in order to follow WebGPU spec.
It also adds one more validation rule: depthReadOnly must be equal
to stencilReadOnly if depthStencilFormat has both depth and stencil
aspects in RenderBundleEncoderDescriptor. We have already had a
similar validation rule in RenderPassDepthStencilAttachment in
RenderPassDescriptor.

Bug: dawn:485

Change-Id: I32c45b2bd90c7041aa881d8589720a9146d6ac7e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/66501
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Yunchao He <yunchao.he@intel.com>

dawn.json

diff --git a/dawn.json b/dawn.json
index 9940077..ada8ff6 100644
--- a/dawn.json
+++ b/dawn.json
@@ -1573,7 +1573,9 @@
             {"name": "color formats count", "type": "uint32_t"},
             {"name": "color formats", "type": "texture format", "annotation": "const*", "length": "color formats count"},
             {"name": "depth stencil format", "type": "texture format", "default": "undefined"},
-            {"name": "sample count", "type": "uint32_t", "default": "1"}
+            {"name": "sample count", "type": "uint32_t", "default": "1"},
+            {"name": "depth read only", "type": "bool", "default": "false"},
+            {"name": "stencil read only", "type": "bool", "default": "false"}
         ]
     },
 

src/dawn_native/RenderBundleEncoder.cpp

diff --git a/src/dawn_native/RenderBundleEncoder.cpp b/src/dawn_native/RenderBundleEncoder.cpp
index 568f941..cd12323 100644
--- a/src/dawn_native/RenderBundleEncoder.cpp
+++ b/src/dawn_native/RenderBundleEncoder.cpp
@@ -37,12 +37,21 @@
     }
 
     MaybeError ValidateDepthStencilAttachmentFormat(const DeviceBase* device,
-                                                    wgpu::TextureFormat textureFormat) {
+                                                    wgpu::TextureFormat textureFormat,
+                                                    bool depthReadOnly,
+                                                    bool stencilReadOnly) {
         DAWN_TRY(ValidateTextureFormat(textureFormat));
         const Format* format = nullptr;
         DAWN_TRY_ASSIGN(format, device->GetInternalFormat(textureFormat));
         DAWN_INVALID_IF(!format->HasDepthOrStencil() || !format->isRenderable,
                         "Texture format %s is not depth/stencil renderable.", textureFormat);
+
+        DAWN_INVALID_IF(
+            format->HasDepth() && format->HasStencil() && depthReadOnly != stencilReadOnly,
+            "depthReadOnly (%u) and stencilReadOnly (%u) must be the same when format %s has "
+            "both depth and stencil aspects.",
+            depthReadOnly, stencilReadOnly, textureFormat);
+
         return {};
     }
 
@@ -67,9 +76,10 @@
         }
 
         if (descriptor->depthStencilFormat != wgpu::TextureFormat::Undefined) {
-            DAWN_TRY_CONTEXT(
-                ValidateDepthStencilAttachmentFormat(device, descriptor->depthStencilFormat),
-                "validating depthStencilFormat");
+            DAWN_TRY_CONTEXT(ValidateDepthStencilAttachmentFormat(
+                                 device, descriptor->depthStencilFormat, descriptor->depthReadOnly,
+                                 descriptor->stencilReadOnly),
+                             "validating depthStencilFormat");
         }
 
         return {};

src/tests/unittests/validation/RenderBundleValidationTests.cpp

diff --git a/src/tests/unittests/validation/RenderBundleValidationTests.cpp b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
index d5e3fa8..50c452e 100644
--- a/src/tests/unittests/validation/RenderBundleValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
@@ -626,6 +626,27 @@
     ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
 }
 
+// Test that depthReadOnly must be equal to stencilReadOnly if depth stencil format contain
+// both depth and stencil formats.
+TEST_F(RenderBundleValidationTest, DepthStencilReadOnly) {
+    for (wgpu::TextureFormat format :
+         {wgpu::TextureFormat::Depth24PlusStencil8, wgpu::TextureFormat::Depth32Float}) {
+        for (bool depthReadOnly : {true, false}) {
+            for (bool stencilReadOnly : {true, false}) {
+                utils::ComboRenderBundleEncoderDescriptor desc = {};
+                desc.depthStencilFormat = format;
+                desc.depthReadOnly = depthReadOnly;
+                desc.stencilReadOnly = stencilReadOnly;
+                if (format == wgpu::TextureFormat::Depth24PlusStencil8 &&
+                    depthReadOnly != stencilReadOnly) {
+                    ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+                } else {
+                    device.CreateRenderBundleEncoder(&desc);
+                }
+            }
+        }
+    }
+}
 // Test that resource usages are validated inside render bundles.
 TEST_F(RenderBundleValidationTest, UsageTracking) {
     DummyRenderPass renderPass(device);