Support separate depth-stencil readonlyness for render bundles
This was missed in the previous commit adding this support for render
passes and render pipelines. Also rework the
PipelineAndPassCompatibilityTests to check with separate depth and
stencil readonlyness (to make sure they are not checked together during
that validation).
Bug: dawn:2146
Change-Id: Iebc80fca819a33a67a71c4fabb542d7b90c229fb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/158400
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/native/RenderBundleEncoder.cpp b/src/dawn/native/RenderBundleEncoder.cpp
index b5d6e21..ba0e246 100644
--- a/src/dawn/native/RenderBundleEncoder.cpp
+++ b/src/dawn/native/RenderBundleEncoder.cpp
@@ -62,11 +62,13 @@
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);
+ if (!device->IsToggleEnabled(Toggle::AllowUnsafeAPIs)) {
+ 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 {};
}
diff --git a/src/dawn/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp b/src/dawn/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp
index 9eee3f7..32ba3f7 100644
--- a/src/dawn/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp
+++ b/src/dawn/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp
@@ -93,78 +93,107 @@
}
};
-// Test depthWrite/stencilWrite in DepthStencilState in render pipeline vs
-// depthReadOnly/stencilReadOnly in DepthStencilAttachment in render pass.
-TEST_F(RenderPipelineAndPassCompatibilityTests, WriteAndReadOnlyConflictForDepthStencil) {
- for (bool depthStencilReadOnlyInPass : {true, false}) {
+// Test depthReadOnly compatibility between pipelines and render passes.
+TEST_F(RenderPipelineAndPassCompatibilityTests, PipelineAndRenderPassDepthReadOnly) {
+ for (bool depthReadOnlyInPass : {true, false}) {
for (bool depthWriteInPipeline : {true, false}) {
- for (bool stencilWriteInPipeline : {true, false}) {
- wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- utils::ComboRenderPassDescriptor passDescriptor = CreateRenderPassDescriptor(
- kFormat, depthStencilReadOnlyInPass, depthStencilReadOnlyInPass);
- wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
- wgpu::RenderPipeline pipeline =
- CreatePipeline(kFormat, depthWriteInPipeline, stencilWriteInPipeline);
- pass.SetPipeline(pipeline);
- pass.Draw(3);
- pass.End();
- if (depthStencilReadOnlyInPass &&
- (depthWriteInPipeline || stencilWriteInPipeline)) {
- ASSERT_DEVICE_ERROR(encoder.Finish());
- } else {
- encoder.Finish();
- }
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ utils::ComboRenderPassDescriptor passDescriptor =
+ CreateRenderPassDescriptor(kFormat, depthReadOnlyInPass, false);
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
+ wgpu::RenderPipeline pipeline = CreatePipeline(kFormat, depthWriteInPipeline, false);
+ pass.SetPipeline(pipeline);
+ pass.Draw(3);
+ pass.End();
+ if (depthReadOnlyInPass && depthWriteInPipeline) {
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ } else {
+ encoder.Finish();
}
}
}
}
-// Test depthWrite/stencilWrite in DepthStencilState in render pipeline vs
-// depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in render bundle.
-TEST_F(RenderPipelineAndPassCompatibilityTests,
- WriteAndReadOnlyConflictForDepthStencilBetweenPipelineAndBundle) {
- for (bool depthStencilReadOnlyInBundle : {true, false}) {
+// Test stencilReadOnly compatibility between pipelines and render passes.
+TEST_F(RenderPipelineAndPassCompatibilityTests, PipelineAndRenderPassStencilReadOnly) {
+ for (bool stencilReadOnlyInPass : {true, false}) {
+ for (bool stencilWriteInPipeline : {true, false}) {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ utils::ComboRenderPassDescriptor passDescriptor =
+ CreateRenderPassDescriptor(kFormat, false, stencilReadOnlyInPass);
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
+ wgpu::RenderPipeline pipeline = CreatePipeline(kFormat, false, stencilWriteInPipeline);
+ pass.SetPipeline(pipeline);
+ pass.Draw(3);
+ pass.End();
+ if (stencilReadOnlyInPass && stencilWriteInPipeline) {
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ } else {
+ encoder.Finish();
+ }
+ }
+ }
+}
+
+// Test depthReadOnly compatibility between pipelines and render bundles.
+TEST_F(RenderPipelineAndPassCompatibilityTests, PipelineAndBundleDepthReadOnly) {
+ for (bool depthReadOnlyInBundle : {true, false}) {
utils::ComboRenderBundleEncoderDescriptor desc = {};
desc.depthStencilFormat = kFormat;
- desc.depthReadOnly = depthStencilReadOnlyInBundle;
- desc.stencilReadOnly = depthStencilReadOnlyInBundle;
+ desc.depthReadOnly = depthReadOnlyInBundle;
+ desc.stencilReadOnly = false;
for (bool depthWriteInPipeline : {true, false}) {
- for (bool stencilWriteInPipeline : {true, false}) {
- wgpu::RenderBundleEncoder renderBundleEncoder =
- device.CreateRenderBundleEncoder(&desc);
- wgpu::RenderPipeline pipeline =
- CreatePipeline(kFormat, depthWriteInPipeline, stencilWriteInPipeline);
- renderBundleEncoder.SetPipeline(pipeline);
- renderBundleEncoder.Draw(3);
- if (depthStencilReadOnlyInBundle &&
- (depthWriteInPipeline || stencilWriteInPipeline)) {
- ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
- } else {
- renderBundleEncoder.Finish();
- }
+ wgpu::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+ wgpu::RenderPipeline pipeline = CreatePipeline(kFormat, depthWriteInPipeline, false);
+ renderBundleEncoder.SetPipeline(pipeline);
+ renderBundleEncoder.Draw(3);
+ if (depthReadOnlyInBundle && depthWriteInPipeline) {
+ ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+ } else {
+ renderBundleEncoder.Finish();
}
}
}
}
-// Test depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in render bundle vs
-// depthReadOnly/stencilReadOnly in DepthStencilAttachment in render pass.
-TEST_F(RenderPipelineAndPassCompatibilityTests,
- WriteAndReadOnlyConflictForDepthStencilBetweenBundleAndPass) {
- for (bool depthStencilReadOnlyInPass : {true, false}) {
- for (bool depthStencilReadOnlyInBundle : {true, false}) {
+// Test stencilReadOnly compatibility between pipelines and render bundles.
+TEST_F(RenderPipelineAndPassCompatibilityTests, PipelineAndBundleStencilReadOnly) {
+ for (bool stencilReadOnlyInBundle : {true, false}) {
+ utils::ComboRenderBundleEncoderDescriptor desc = {};
+ desc.depthStencilFormat = kFormat;
+ desc.depthReadOnly = false;
+ desc.stencilReadOnly = stencilReadOnlyInBundle;
+
+ for (bool stencilWriteInPipeline : {true, false}) {
+ wgpu::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+ wgpu::RenderPipeline pipeline = CreatePipeline(kFormat, false, stencilWriteInPipeline);
+ renderBundleEncoder.SetPipeline(pipeline);
+ renderBundleEncoder.Draw(3);
+ if (stencilReadOnlyInBundle && stencilWriteInPipeline) {
+ ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+ } else {
+ renderBundleEncoder.Finish();
+ }
+ }
+ }
+}
+
+// Test depthReadOnly compatibility between render passes and render bundles.
+TEST_F(RenderPipelineAndPassCompatibilityTests, BundleAndPassDepthReadOnly) {
+ for (bool depthReadOnlyInPass : {true, false}) {
+ for (bool depthReadOnlyInBundle : {true, false}) {
for (bool emptyBundle : {true, false}) {
// Create render bundle, with or without a pipeline
utils::ComboRenderBundleEncoderDescriptor desc = {};
desc.depthStencilFormat = kFormat;
- desc.depthReadOnly = depthStencilReadOnlyInBundle;
- desc.stencilReadOnly = depthStencilReadOnlyInBundle;
+ desc.depthReadOnly = depthReadOnlyInBundle;
+ desc.stencilReadOnly = false;
wgpu::RenderBundleEncoder renderBundleEncoder =
device.CreateRenderBundleEncoder(&desc);
if (!emptyBundle) {
- wgpu::RenderPipeline pipeline = CreatePipeline(
- kFormat, !depthStencilReadOnlyInBundle, !depthStencilReadOnlyInBundle);
+ wgpu::RenderPipeline pipeline =
+ CreatePipeline(kFormat, !depthReadOnlyInBundle, false);
renderBundleEncoder.SetPipeline(pipeline);
renderBundleEncoder.Draw(3);
}
@@ -172,15 +201,52 @@
// Create render pass and call ExecuteBundles()
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- utils::ComboRenderPassDescriptor passDescriptor = CreateRenderPassDescriptor(
- kFormat, depthStencilReadOnlyInPass, depthStencilReadOnlyInPass);
+ utils::ComboRenderPassDescriptor passDescriptor =
+ CreateRenderPassDescriptor(kFormat, depthReadOnlyInPass, false);
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
pass.ExecuteBundles(1, &bundle);
pass.End();
- if (!depthStencilReadOnlyInPass || depthStencilReadOnlyInBundle) {
- encoder.Finish();
- } else {
+ if (depthReadOnlyInPass && !depthReadOnlyInBundle) {
ASSERT_DEVICE_ERROR(encoder.Finish());
+ } else {
+ encoder.Finish();
+ }
+ }
+ }
+ }
+}
+
+// Test stencilReadOnly compatibility between render passes and render bundles.
+TEST_F(RenderPipelineAndPassCompatibilityTests, BundleAndPassStencilReadOnly) {
+ for (bool stencilReadOnlyInPass : {true, false}) {
+ for (bool stencilReadOnlyInBundle : {true, false}) {
+ for (bool emptyBundle : {true, false}) {
+ // Create render bundle, with or without a pipeline
+ utils::ComboRenderBundleEncoderDescriptor desc = {};
+ desc.depthStencilFormat = kFormat;
+ desc.depthReadOnly = false;
+ desc.stencilReadOnly = stencilReadOnlyInBundle;
+ wgpu::RenderBundleEncoder renderBundleEncoder =
+ device.CreateRenderBundleEncoder(&desc);
+ if (!emptyBundle) {
+ wgpu::RenderPipeline pipeline =
+ CreatePipeline(kFormat, false, !stencilReadOnlyInBundle);
+ renderBundleEncoder.SetPipeline(pipeline);
+ renderBundleEncoder.Draw(3);
+ }
+ wgpu::RenderBundle bundle = renderBundleEncoder.Finish();
+
+ // Create render pass and call ExecuteBundles()
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ utils::ComboRenderPassDescriptor passDescriptor =
+ CreateRenderPassDescriptor(kFormat, false, stencilReadOnlyInPass);
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
+ pass.ExecuteBundles(1, &bundle);
+ pass.End();
+ if (stencilReadOnlyInPass && !stencilReadOnlyInBundle) {
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ } else {
+ encoder.Finish();
}
}
}
diff --git a/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp
index a07a29f..e4bfed5 100644
--- a/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderBundleValidationTests.cpp
@@ -685,24 +685,15 @@
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 that depthReadOnly and stencilReadOnly don't need to be the same for depth-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);
- }
- }
+ for (bool depthReadOnly : {true, false}) {
+ for (bool stencilReadOnly : {true, false}) {
+ utils::ComboRenderBundleEncoderDescriptor desc = {};
+ desc.depthStencilFormat = wgpu::TextureFormat::Depth24PlusStencil8;
+ desc.depthReadOnly = depthReadOnly;
+ desc.stencilReadOnly = stencilReadOnly;
+ device.CreateRenderBundleEncoder(&desc);
}
}
}
diff --git a/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp b/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp
index bb0e742..b30528f 100644
--- a/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/UnsafeAPIValidationTests.cpp
@@ -70,8 +70,8 @@
)"));
}
-// Check that separate depth-stencil readonlyness is validated as unsafe.
-TEST_F(UnsafeAPIValidationTest, SeparateDepthStencilReadOnlyness) {
+// Check that separate depth-stencil readonlyness is validated as unsafe for render passes.
+TEST_F(UnsafeAPIValidationTest, SeparateRenderPassDepthStencilReadOnlyness) {
wgpu::TextureDescriptor tDesc;
tDesc.size = {1, 1};
tDesc.format = wgpu::TextureFormat::Depth24PlusStencil8;
@@ -95,7 +95,7 @@
encoder.Finish();
}
- // Error case: only one readonly is valid.
+ // Error case: only one readonly is invalid.
{
wgpu::RenderPassDepthStencilAttachment ds;
ds.view = t.CreateView();
@@ -115,5 +115,25 @@
}
}
+// Check that separate depth-stencil readonlyness is validated as unsafe for render bundles.
+TEST_F(UnsafeAPIValidationTest, SeparateRenderBundleDepthStencilReadOnlyness) {
+ utils::ComboRenderBundleEncoderDescriptor desc = {};
+ desc.depthStencilFormat = wgpu::TextureFormat::Depth24PlusStencil8;
+
+ // Control case: both readonly is valid.
+ {
+ desc.depthReadOnly = true;
+ desc.stencilReadOnly = true;
+ device.CreateRenderBundleEncoder(&desc);
+ }
+
+ // Error case: only one readonly is invalid.
+ {
+ desc.depthReadOnly = true;
+ desc.stencilReadOnly = false;
+ ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+ }
+}
+
} // anonymous namespace
} // namespace dawn
diff --git a/webgpu-cts/compat-expectations.txt b/webgpu-cts/compat-expectations.txt
index dac7e00..414eced 100644
--- a/webgpu-cts/compat-expectations.txt
+++ b/webgpu-cts/compat-expectations.txt
@@ -482,5 +482,6 @@
[ angle-opengl intel ] webgpu:shader,execution,expression,call,builtin,ldexp:f32:inputSource="storage_rw";* [ Failure ]
[ angle-opengl intel ] webgpu:shader,execution,expression,call,builtin,ldexp:f32:inputSource="uniform";* [ Failure ]
-# Read-only separate depth and stencil is failing
+# Temporary failures while implementing separate depth-stencil readonlyness.
crbug.com/dawn/2146 [ angle-opengl ] webgpu:api,validation,render_pass,render_pass_descriptor:depth_stencil_attachment,loadOp_storeOp_match_depthReadOnly_stencilReadOnly:format="depth24plus-stencil8" [ Failure ]
+crbug.com/dawn/2146 [ angle-opengl ] webgpu:api,validation,encoding,createRenderBundleEncoder:depth_stencil_readonly:* [ Failure ]
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index 94cf70e..3a6855a 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -157,6 +157,7 @@
# KEEP
################################################################################
crbug.com/dawn/2146 webgpu:api,validation,render_pass,render_pass_descriptor:depth_stencil_attachment,loadOp_storeOp_match_depthReadOnly_stencilReadOnly:* [ Failure ]
+crbug.com/dawn/2146 webgpu:api,validation,encoding,createRenderBundleEncoder:depth_stencil_readonly:* [ Failure ]
################################################################################
# Android failures
