Add validation rule for depth/stencil between pipeline and render bundle
This change also adds a unittest to validation colorFormatCount in
RenderBundleEncoderDescriptor, and fixes a style issue as well.
Bug: dawn:485
Change-Id: I642f0e250835d76288ac42fa18a8dabf2db30047
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/66621
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Yunchao He <yunchao.he@intel.com>
diff --git a/src/dawn_native/RenderBundleEncoder.cpp b/src/dawn_native/RenderBundleEncoder.cpp
index cd12323..4c68c4c 100644
--- a/src/dawn_native/RenderBundleEncoder.cpp
+++ b/src/dawn_native/RenderBundleEncoder.cpp
@@ -90,8 +90,8 @@
: RenderEncoderBase(device,
&mBundleEncodingContext,
device->GetOrCreateAttachmentState(descriptor),
- false,
- false),
+ descriptor->depthReadOnly,
+ descriptor->stencilReadOnly),
mBundleEncodingContext(device, this) {
}
diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp
index 82fdde8..1c16bba 100644
--- a/src/dawn_native/RenderPipeline.cpp
+++ b/src/dawn_native/RenderPipeline.cpp
@@ -543,15 +543,15 @@
return stages;
}
- bool StencilTestEnabled(const DepthStencilState* mDepthStencil) {
- return mDepthStencil->stencilBack.compare != wgpu::CompareFunction::Always ||
- mDepthStencil->stencilBack.failOp != wgpu::StencilOperation::Keep ||
- mDepthStencil->stencilBack.depthFailOp != wgpu::StencilOperation::Keep ||
- mDepthStencil->stencilBack.passOp != wgpu::StencilOperation::Keep ||
- mDepthStencil->stencilFront.compare != wgpu::CompareFunction::Always ||
- mDepthStencil->stencilFront.failOp != wgpu::StencilOperation::Keep ||
- mDepthStencil->stencilFront.depthFailOp != wgpu::StencilOperation::Keep ||
- mDepthStencil->stencilFront.passOp != wgpu::StencilOperation::Keep;
+ bool StencilTestEnabled(const DepthStencilState* depthStencil) {
+ return depthStencil->stencilBack.compare != wgpu::CompareFunction::Always ||
+ depthStencil->stencilBack.failOp != wgpu::StencilOperation::Keep ||
+ depthStencil->stencilBack.depthFailOp != wgpu::StencilOperation::Keep ||
+ depthStencil->stencilBack.passOp != wgpu::StencilOperation::Keep ||
+ depthStencil->stencilFront.compare != wgpu::CompareFunction::Always ||
+ depthStencil->stencilFront.failOp != wgpu::StencilOperation::Keep ||
+ depthStencil->stencilFront.depthFailOp != wgpu::StencilOperation::Keep ||
+ depthStencil->stencilFront.passOp != wgpu::StencilOperation::Keep;
}
// RenderPipelineBase
diff --git a/src/dawn_native/RenderPipeline.h b/src/dawn_native/RenderPipeline.h
index 4020bcc..bd354ab 100644
--- a/src/dawn_native/RenderPipeline.h
+++ b/src/dawn_native/RenderPipeline.h
@@ -41,7 +41,7 @@
bool IsStripPrimitiveTopology(wgpu::PrimitiveTopology primitiveTopology);
- bool StencilTestEnabled(const DepthStencilState* mDepthStencil);
+ bool StencilTestEnabled(const DepthStencilState* depthStencil);
struct VertexAttributeInfo {
wgpu::VertexFormat format;
diff --git a/src/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp b/src/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp
index a71afa2..e30299f 100644
--- a/src/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp
+++ b/src/tests/unittests/validation/PipelineAndPassCompatibilityTests.cpp
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include "utils/ComboRenderBundleEncoderDescriptor.h"
#include "utils/ComboRenderPipelineDescriptor.h"
#include "utils/WGPUHelpers.h"
@@ -105,8 +106,40 @@
}
}
+ // Test depthWrite/stencilWrite in DepthStencilState in pipeline vs
+ // depthReadOnly/stencilReadOnly in RenderBundleEncoderDescriptor in RenderBundle
+ TEST_F(RenderPipelineAndPassCompatibilityTests,
+ WriteAndReadOnlyConflictForDepthStencilWithRenderBundle) {
+ wgpu::TextureFormat kFormat = wgpu::TextureFormat::Depth24PlusStencil8;
+ // If the format has both depth and stencil aspects, depthReadOnly and stencilReadOnly
+ // should be the same. So it is not necessary to set two separate booleans like
+ // depthReadOnlyInBundle and stencilReadOnlyInBundle.
+ for (bool depthStencilReadOnlyInBundle : {true, false}) {
+ utils::ComboRenderBundleEncoderDescriptor desc = {};
+ desc.depthStencilFormat = kFormat;
+ desc.depthReadOnly = depthStencilReadOnlyInBundle;
+ desc.stencilReadOnly = depthStencilReadOnlyInBundle;
+
+ 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();
+ }
+ }
+ }
+ }
+ }
+
// TODO(dawn:485): add more tests. For example:
- // - readOnly vs write for depth/stencil with renderbundle.
// - depth/stencil attachment should be designated if depth/stencil test is enabled.
// - pipeline and pass compatibility tests for color attachment(s).
// - pipeline and pass compatibility tests for compute.
diff --git a/src/tests/unittests/validation/RenderBundleValidationTests.cpp b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
index 50c452e..bb3a01a 100644
--- a/src/tests/unittests/validation/RenderBundleValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
@@ -611,6 +611,30 @@
}
}
+// Test that it is invalid to create a render bundle with no texture formats
+TEST_F(RenderBundleValidationTest, ColorFormatsCountOutOfBounds) {
+ std::array<wgpu::TextureFormat, kMaxColorAttachments + 1> colorFormats;
+ for (uint32_t i = 0; i < colorFormats.size(); ++i) {
+ colorFormats[i] = wgpu::TextureFormat::RGBA8Unorm;
+ }
+
+ // colorFormatsCount <= kMaxColorAttachments is valid.
+ {
+ wgpu::RenderBundleEncoderDescriptor desc;
+ desc.colorFormatsCount = kMaxColorAttachments;
+ desc.colorFormats = colorFormats.data();
+ device.CreateRenderBundleEncoder(&desc);
+ }
+
+ // colorFormatsCount > kMaxColorAttachments is invalid.
+ {
+ wgpu::RenderBundleEncoderDescriptor desc;
+ desc.colorFormatsCount = kMaxColorAttachments + 1;
+ desc.colorFormats = colorFormats.data();
+ ASSERT_DEVICE_ERROR(device.CreateRenderBundleEncoder(&desc));
+ }
+}
+
// Test that render bundle color formats cannot be set to undefined.
TEST_F(RenderBundleValidationTest, ColorFormatUndefined) {
utils::ComboRenderBundleEncoderDescriptor desc = {};
