Implement Validation For DepthStencilAttachment ReadOnly
Implements validation for RenderPassDepthStencilAttachmentDescriptor depthReadOnly and
stencilReadOnly flags to match WebGPU specification. Includes corresponding unit tests.
bug: dawn:485
Change-Id: I21e624850d5a393469569417f102fb979dbfdf27
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24602
Commit-Queue: Brandon Jones <brandon1.jones@intel.com>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/dawn.json b/dawn.json
index 5d78b4a..3886240 100644
--- a/dawn.json
+++ b/dawn.json
@@ -1147,9 +1147,11 @@
{"name": "depth load op", "type": "load op"},
{"name": "depth store op", "type": "store op"},
{"name": "clear depth", "type": "float"},
+ {"name": "depth read only", "type": "bool", "default": "false"},
{"name": "stencil load op", "type": "load op"},
{"name": "stencil store op", "type": "store op"},
- {"name": "clear stencil", "type": "uint32_t", "default": "0"}
+ {"name": "clear stencil", "type": "uint32_t", "default": "0"},
+ {"name": "stencil read only", "type": "bool", "default": "false"}
]
},
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index dab2c16..e8a6b46 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -298,6 +298,31 @@
DAWN_TRY(ValidateStoreOp(depthStencilAttachment->depthStoreOp));
DAWN_TRY(ValidateStoreOp(depthStencilAttachment->stencilStoreOp));
+ if (attachment->GetAspect() == wgpu::TextureAspect::All &&
+ attachment->GetFormat().HasStencil() &&
+ depthStencilAttachment->depthReadOnly != depthStencilAttachment->stencilReadOnly) {
+ return DAWN_VALIDATION_ERROR(
+ "depthReadOnly and stencilReadOnly must be the same when texture aspect is "
+ "'all'");
+ }
+
+ if (depthStencilAttachment->depthReadOnly &&
+ (depthStencilAttachment->depthLoadOp != wgpu::LoadOp::Load ||
+ depthStencilAttachment->depthStoreOp != wgpu::StoreOp::Store)) {
+ return DAWN_VALIDATION_ERROR(
+ "depthLoadOp must be load and depthStoreOp must be store when depthReadOnly "
+ "is true.");
+ }
+
+ if (depthStencilAttachment->stencilReadOnly &&
+ (depthStencilAttachment->stencilLoadOp != wgpu::LoadOp::Load ||
+ depthStencilAttachment->stencilStoreOp != wgpu::StoreOp::Store)) {
+ return DAWN_VALIDATION_ERROR(
+ "stencilLoadOp must be load and stencilStoreOp must be store when "
+ "stencilReadOnly "
+ "is true.");
+ }
+
if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Clear &&
std::isnan(depthStencilAttachment->clearDepth)) {
return DAWN_VALIDATION_ERROR("Depth clear value cannot be NaN");
diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp
index f029981..948c06d 100644
--- a/src/dawn_native/Texture.cpp
+++ b/src/dawn_native/Texture.cpp
@@ -571,6 +571,7 @@
TextureViewBase::TextureViewBase(TextureBase* texture, const TextureViewDescriptor* descriptor)
: ObjectBase(texture->GetDevice()),
mTexture(texture),
+ mAspect(descriptor->aspect),
mFormat(GetDevice()->GetValidInternalFormat(descriptor->format)),
mDimension(descriptor->dimension),
mRange({descriptor->baseMipLevel, descriptor->mipLevelCount, descriptor->baseArrayLayer,
@@ -596,6 +597,11 @@
return mTexture.Get();
}
+ wgpu::TextureAspect TextureViewBase::GetAspect() const {
+ ASSERT(!IsError());
+ return mAspect;
+ }
+
const Format& TextureViewBase::GetFormat() const {
ASSERT(!IsError());
return mFormat;
diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h
index 2900794..ce1bc90 100644
--- a/src/dawn_native/Texture.h
+++ b/src/dawn_native/Texture.h
@@ -126,6 +126,7 @@
const TextureBase* GetTexture() const;
TextureBase* GetTexture();
+ wgpu::TextureAspect GetAspect() const;
const Format& GetFormat() const;
wgpu::TextureViewDimension GetDimension() const;
uint32_t GetBaseMipLevel() const;
@@ -139,6 +140,7 @@
Ref<TextureBase> mTexture;
+ wgpu::TextureAspect mAspect;
// TODO(cwallez@chromium.org): This should be deduplicated in the Device
const Format& mFormat;
wgpu::TextureViewDimension mDimension;
diff --git a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
index 5b99a1f..b1658ea 100644
--- a/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -762,6 +762,77 @@
}
}
+ TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilReadOnly) {
+ wgpu::TextureView colorView =
+ Create2DAttachment(device, 1, 1, wgpu::TextureFormat::RGBA8Unorm);
+ wgpu::TextureView depthStencilView =
+ Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24PlusStencil8);
+ wgpu::TextureView depthStencilViewNoStencil =
+ Create2DAttachment(device, 1, 1, wgpu::TextureFormat::Depth24Plus);
+
+ // Tests that a read-only pass with depthReadOnly set to true succeeds.
+ {
+ utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+ renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
+ AssertBeginRenderPassSuccess(&renderPass);
+ }
+
+ // Tests that a pass with mismatched depthReadOnly and stencilReadOnly values passes when
+ // there is no stencil component in the format.
+ {
+ utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilViewNoStencil);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+ renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = false;
+ AssertBeginRenderPassSuccess(&renderPass);
+ }
+
+ // Tests that a pass with mismatched depthReadOnly and stencilReadOnly values fails when
+ // both depth and stencil components exist.
+ {
+ utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+ renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = false;
+ AssertBeginRenderPassError(&renderPass);
+ }
+
+ // Tests that a pass with loadOp set to clear and readOnly set to true fails.
+ {
+ utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Clear;
+ renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+ renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Clear;
+ renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Store;
+ renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
+ AssertBeginRenderPassError(&renderPass);
+ }
+
+ // Tests that a pass with storeOp set to clear and readOnly set to true fails.
+ {
+ utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);
+ renderPass.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Clear;
+ renderPass.cDepthStencilAttachmentInfo.depthReadOnly = true;
+ renderPass.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Load;
+ renderPass.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Clear;
+ renderPass.cDepthStencilAttachmentInfo.stencilReadOnly = true;
+ AssertBeginRenderPassError(&renderPass);
+ }
+ }
+
// TODO(cwallez@chromium.org): Constraints on attachment aliasing?
} // anonymous namespace
