Removes/unifies deprecation period code for depthStencilReadOnly ops.

- Note that by default these are already errors, not warnings.

Change-Id: Iab9ecf3cfd54c0219777c2d1587a7752c3173595
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/128102
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Shrek Shao <shrekshao@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Loko Kung 2023-04-21 10:44:13 +00:00 committed by Dawn LUCI CQ
parent c421a4b0ce
commit 2f845021d8
3 changed files with 36 additions and 105 deletions

View File

@ -298,75 +298,41 @@ MaybeError ValidateRenderPassDepthStencilAttachment(
// Read only, or depth doesn't exist.
if (depthStencilAttachment->depthReadOnly ||
!IsSubset(Aspect::Depth, attachment->GetAspects())) {
if (depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Load &&
depthStencilAttachment->depthStoreOp == wgpu::StoreOp::Store) {
DAWN_TRY(DAWN_MAKE_DEPRECATION_ERROR(
device,
"depthLoadOp is (%s) and depthStoreOp is (%s) "
"when depthReadOnly (%u) or the attachment (%s) has no depth aspect.",
DAWN_INVALID_IF(depthStencilAttachment->depthLoadOp != wgpu::LoadOp::Undefined ||
depthStencilAttachment->depthStoreOp != wgpu::StoreOp::Undefined,
"Both depthLoadOp (%s) and depthStoreOp (%s) must not be set if the "
"attachment (%s) has no depth aspect or depthReadOnly (%u) is true.",
depthStencilAttachment->depthLoadOp, depthStencilAttachment->depthStoreOp,
depthStencilAttachment->depthReadOnly, attachment));
} else {
DAWN_INVALID_IF(depthStencilAttachment->depthLoadOp != wgpu::LoadOp::Undefined,
"depthLoadOp (%s) must not be set if the attachment (%s) has "
"no depth aspect or depthReadOnly (%u) is true.",
depthStencilAttachment->depthLoadOp, attachment,
depthStencilAttachment->depthReadOnly);
DAWN_INVALID_IF(depthStencilAttachment->depthStoreOp != wgpu::StoreOp::Undefined,
"depthStoreOp (%s) must not be set if the attachment (%s) has no depth "
"aspect or depthReadOnly (%u) is true.",
depthStencilAttachment->depthStoreOp, attachment,
depthStencilAttachment->depthReadOnly);
}
attachment, depthStencilAttachment->depthReadOnly);
} else {
DAWN_TRY(ValidateLoadOp(depthStencilAttachment->depthLoadOp));
DAWN_INVALID_IF(depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Undefined,
"depthLoadOp must be set if the attachment (%s) has a depth aspect "
"and depthReadOnly (%u) is false.",
attachment, depthStencilAttachment->depthReadOnly);
DAWN_TRY(ValidateStoreOp(depthStencilAttachment->depthStoreOp));
DAWN_INVALID_IF(depthStencilAttachment->depthStoreOp == wgpu::StoreOp::Undefined,
"depthStoreOp must be set if the attachment (%s) has a depth "
"aspect and depthReadOnly (%u) is false.",
DAWN_INVALID_IF(depthStencilAttachment->depthLoadOp == wgpu::LoadOp::Undefined ||
depthStencilAttachment->depthStoreOp == wgpu::StoreOp::Undefined,
"Both depthLoadOp (%s) and depthStoreOp (%s) must be set if the attachment "
"(%s) has a depth aspect or depthReadOnly (%u) is false.",
depthStencilAttachment->depthLoadOp, depthStencilAttachment->depthStoreOp,
attachment, depthStencilAttachment->depthReadOnly);
}
// Read only, or stencil doesn't exist.
if (depthStencilAttachment->stencilReadOnly ||
!IsSubset(Aspect::Stencil, attachment->GetAspects())) {
if (depthStencilAttachment->stencilLoadOp == wgpu::LoadOp::Load &&
depthStencilAttachment->stencilStoreOp == wgpu::StoreOp::Store) {
DAWN_TRY(DAWN_MAKE_DEPRECATION_ERROR(
device,
"stencilLoadOp is (%s) and stencilStoreOp is (%s) "
"when stencilReadOnly (%u) or the attachment (%s) has no stencil aspect.",
depthStencilAttachment->stencilLoadOp, depthStencilAttachment->stencilStoreOp,
depthStencilAttachment->stencilReadOnly, attachment));
} else {
DAWN_INVALID_IF(
depthStencilAttachment->stencilLoadOp != wgpu::LoadOp::Undefined,
"stencilLoadOp (%s) must not be set if the attachment (%s) has no stencil "
"aspect or stencilReadOnly (%u) is true.",
depthStencilAttachment->stencilLoadOp, attachment,
depthStencilAttachment->stencilReadOnly);
DAWN_INVALID_IF(
DAWN_INVALID_IF(depthStencilAttachment->stencilLoadOp != wgpu::LoadOp::Undefined ||
depthStencilAttachment->stencilStoreOp != wgpu::StoreOp::Undefined,
"stencilStoreOp (%s) must not be set if the attachment (%s) has no stencil "
"aspect or stencilReadOnly (%u) is true.",
"Both stencilLoadOp (%s) and stencilStoreOp (%s) must not be set if the "
"attachment (%s) has no stencil aspect or stencilReadOnly (%u) is true.",
depthStencilAttachment->stencilLoadOp,
depthStencilAttachment->stencilStoreOp, attachment,
depthStencilAttachment->stencilReadOnly);
}
} else {
DAWN_TRY(ValidateLoadOp(depthStencilAttachment->stencilLoadOp));
DAWN_INVALID_IF(depthStencilAttachment->stencilLoadOp == wgpu::LoadOp::Undefined,
"stencilLoadOp (%s) must be set if the attachment (%s) has a stencil "
"aspect and stencilReadOnly (%u) is false.",
depthStencilAttachment->stencilLoadOp, attachment,
depthStencilAttachment->stencilReadOnly);
DAWN_TRY(ValidateStoreOp(depthStencilAttachment->stencilStoreOp));
DAWN_INVALID_IF(depthStencilAttachment->stencilStoreOp == wgpu::StoreOp::Undefined,
"stencilStoreOp (%s) must be set if the attachment (%s) has a stencil "
"aspect and stencilReadOnly (%u) is false.",
DAWN_INVALID_IF(depthStencilAttachment->stencilLoadOp == wgpu::LoadOp::Undefined ||
depthStencilAttachment->stencilStoreOp == wgpu::StoreOp::Undefined,
"Both stencilLoadOp (%s) and stencilStoreOp (%s) must be set if the "
"attachment (%s) has a stencil aspect or stencilReadOnly (%u) is false.",
depthStencilAttachment->stencilLoadOp,
depthStencilAttachment->stencilStoreOp, attachment,
depthStencilAttachment->stencilReadOnly);
}

View File

@ -25,54 +25,6 @@ WGPUDevice DeprecationTests::CreateTestDevice(dawn::native::Adapter dawnAdapter)
return dawnAdapter.CreateDevice(&descriptor);
}
// Test that setting attachment rather than view for render pass color and depth/stencil attachments
// is deprecated.
TEST_P(DeprecationTests, ReadOnlyDepthStencilStoreLoadOpsAttachment) {
utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 1, 1);
wgpu::RenderPassEncoder pass;
// Check that setting load/store ops with read only depth/stencil attachments gives a warning.
wgpu::TextureDescriptor descriptor;
descriptor.dimension = wgpu::TextureDimension::e2D;
descriptor.size = {1, 1, 1};
descriptor.sampleCount = 1;
descriptor.format = wgpu::TextureFormat::Depth24PlusStencil8;
descriptor.mipLevelCount = 1;
descriptor.usage = wgpu::TextureUsage::RenderAttachment;
wgpu::Texture depthStencil = device.CreateTexture(&descriptor);
wgpu::RenderPassDepthStencilAttachment* depthAttachment =
&renderPass.renderPassInfo.cDepthStencilAttachmentInfo;
renderPass.renderPassInfo.depthStencilAttachment = depthAttachment;
depthAttachment->view = depthStencil.CreateView();
depthAttachment->depthReadOnly = true;
depthAttachment->stencilReadOnly = true;
depthAttachment->depthLoadOp = wgpu::LoadOp::Load;
depthAttachment->depthStoreOp = wgpu::StoreOp::Store;
depthAttachment->stencilLoadOp = wgpu::LoadOp::Undefined;
depthAttachment->stencilStoreOp = wgpu::StoreOp::Undefined;
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
pass.End();
EXPECT_DEPRECATION_ERROR_ONLY(encoder.Finish(););
}
depthAttachment->depthLoadOp = wgpu::LoadOp::Undefined;
depthAttachment->depthStoreOp = wgpu::StoreOp::Undefined;
depthAttachment->stencilLoadOp = wgpu::LoadOp::Load;
depthAttachment->stencilStoreOp = wgpu::StoreOp::Store;
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
pass.End();
EXPECT_DEPRECATION_ERROR_ONLY(encoder.Finish(););
}
}
// Test that endPass() is deprecated for both render and compute passes.
TEST_P(DeprecationTests, EndPass) {
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();

View File

@ -1289,6 +1289,19 @@ TEST_F(RenderPassDescriptorValidationTest, ValidateDepthStencilReadOnly) {
AssertBeginRenderPassError(&renderPass);
}
// Tests that a pass with loadOp set to load, storeOp set to store, and readOnly set to true
// fails.
{
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;
AssertBeginRenderPassError(&renderPass);
}
// Tests that a pass with only depthLoadOp set to load and readOnly set to true fails.
{
utils::ComboRenderPassDescriptor renderPass({colorView}, depthStencilView);