encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Protect against aspectReadOnly of unused DS attachements aspects 

Unused aspects of depth-stencil attachments that are tagged as read-only
used to leak that read-only state to backends, even if the validation
made it seems like they always match (they only need to match if the
texture has both aspects). This confused backends like Vulkan which
checked for depthReadOnly || stencilReadOnly to choose between code
paths.

Instead reyify the depthStencilAttachement descriptor in the frontend to
protect against garbage values being passed for aspects that aren't
present in the texture.

Adds a regression test, with the caveat that a failure is only shown by
having the VVL output and error in stderr due to an unrelated issue.

Fixed: dawn:1512
Change-Id: I35d5581e46909b7f41ff4c7553d60c6ac844a56b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101121
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Corentin Wallez 2022-09-26 11:13:35 +00:00 committed by Dawn LUCI CQ
parent ab882c17a5
commit 03199c2b44
2 changed files with 62 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
src/dawn/native/CommandEncoder.cpp
View File

@ -911,7 +911,38 @@ Ref<RenderPassEncoder> CommandEncoder::BeginRenderPass(const RenderPassDescripto
cmd->depthStencilAttachment.clearStencil = cmd->depthStencilAttachment.clearStencil =
descriptor->depthStencilAttachment->stencilClearValue; descriptor->depthStencilAttachment->stencilClearValue;
} }
// Copy parameters for the depth, reyifing the values when it is not present or
// readonly.
cmd->depthStencilAttachment.depthReadOnly = false;
cmd->depthStencilAttachment.depthLoadOp = wgpu::LoadOp::Load;
cmd->depthStencilAttachment.depthStoreOp = wgpu::StoreOp::Store;
if (view->GetFormat().HasDepth()) {
cmd->depthStencilAttachment.depthReadOnly =
descriptor->depthStencilAttachment->depthReadOnly;
if (!cmd->depthStencilAttachment.depthReadOnly) {
cmd->depthStencilAttachment.depthLoadOp =
descriptor->depthStencilAttachment->depthLoadOp;
cmd->depthStencilAttachment.depthStoreOp =
descriptor->depthStencilAttachment->depthStoreOp;
}
}
// Copy parameters for the stencil, reyifing the values when it is not present or
// readonly.
cmd->depthStencilAttachment.stencilReadOnly = false;
cmd->depthStencilAttachment.stencilLoadOp = wgpu::LoadOp::Load;
cmd->depthStencilAttachment.stencilStoreOp = wgpu::StoreOp::Store;
if (view->GetFormat().HasStencil()) { if (view->GetFormat().HasStencil()) {
cmd->depthStencilAttachment.stencilReadOnly =
descriptor->depthStencilAttachment->stencilReadOnly;
if (!cmd->depthStencilAttachment.stencilReadOnly) {
cmd->depthStencilAttachment.stencilLoadOp =
descriptor->depthStencilAttachment->stencilLoadOp;
cmd->depthStencilAttachment.stencilStoreOp =
descriptor->depthStencilAttachment->stencilStoreOp;
}
// GPURenderPassDepthStencilAttachment.stencilClearValue will be converted to // GPURenderPassDepthStencilAttachment.stencilClearValue will be converted to
// the type of the stencil aspect of view by taking the same number of LSBs as // the type of the stencil aspect of view by taking the same number of LSBs as
// the number of bits in the stencil aspect of one texel block of view. // the number of bits in the stencil aspect of one texel block of view.
@ -921,35 +952,6 @@ Ref<RenderPassEncoder> CommandEncoder::BeginRenderPass(const RenderPassDescripto
cmd->depthStencilAttachment.clearStencil &= 0xFF; cmd->depthStencilAttachment.clearStencil &= 0xFF;
} }
cmd->depthStencilAttachment.depthReadOnly =
descriptor->depthStencilAttachment->depthReadOnly;
cmd->depthStencilAttachment.stencilReadOnly =
descriptor->depthStencilAttachment->stencilReadOnly;
if (descriptor->depthStencilAttachment->depthReadOnly ||
!IsSubset(Aspect::Depth,
descriptor->depthStencilAttachment->view->GetAspects())) {
cmd->depthStencilAttachment.depthLoadOp = wgpu::LoadOp::Load;
cmd->depthStencilAttachment.depthStoreOp = wgpu::StoreOp::Store;
} else {
cmd->depthStencilAttachment.depthLoadOp =
descriptor->depthStencilAttachment->depthLoadOp;
cmd->depthStencilAttachment.depthStoreOp =
descriptor->depthStencilAttachment->depthStoreOp;
}
if (descriptor->depthStencilAttachment->stencilReadOnly ||
!IsSubset(Aspect::Stencil,
descriptor->depthStencilAttachment->view->GetAspects())) {
cmd->depthStencilAttachment.stencilLoadOp = wgpu::LoadOp::Load;
cmd->depthStencilAttachment.stencilStoreOp = wgpu::StoreOp::Store;
} else {
cmd->depthStencilAttachment.stencilLoadOp =
descriptor->depthStencilAttachment->stencilLoadOp;
cmd->depthStencilAttachment.stencilStoreOp =
descriptor->depthStencilAttachment->stencilStoreOp;
}
if (IsReadOnlyDepthStencilAttachment(descriptor->depthStencilAttachment)) { if (IsReadOnlyDepthStencilAttachment(descriptor->depthStencilAttachment)) {
usageTracker.TextureViewUsedAs(view, kReadOnlyRenderAttachment); usageTracker.TextureViewUsedAs(view, kReadOnlyRenderAttachment);
} else { } else {

31
src/dawn/tests/end2end/ReadOnlyDepthStencilAttachmentTests.cpp
View File

@ -256,6 +256,37 @@ TEST_P(ReadOnlyDepthAttachmentTests, NotSampleFromAttachment) {
{kSize, kSize / 2}); {kSize, kSize / 2});
} }
// Regression test for crbug.com/dawn/1512 where having aspectReadOnly for an unused aspect of a
// depth-stencil texture would cause the attachment to be considered read-only, causing layout
// mismatch issues.
TEST_P(ReadOnlyDepthAttachmentTests, UnusedAspectWithReadOnly) {
wgpu::TextureFormat format = GetParam().mTextureFormat;
wgpu::Texture depthStencilTexture = CreateTexture(format, wgpu::TextureUsage::RenderAttachment);
utils::ComboRenderPassDescriptor passDescriptor({}, depthStencilTexture.CreateView());
if (utils::IsStencilOnlyFormat(format)) {
passDescriptor.cDepthStencilAttachmentInfo.depthReadOnly = true;
passDescriptor.cDepthStencilAttachmentInfo.depthLoadOp = wgpu::LoadOp::Undefined;
passDescriptor.cDepthStencilAttachmentInfo.depthStoreOp = wgpu::StoreOp::Undefined;
} else {
passDescriptor.cDepthStencilAttachmentInfo.depthReadOnly = false;
}
if (utils::IsDepthOnlyFormat(format)) {
passDescriptor.cDepthStencilAttachmentInfo.stencilReadOnly = true;
passDescriptor.cDepthStencilAttachmentInfo.stencilLoadOp = wgpu::LoadOp::Undefined;
passDescriptor.cDepthStencilAttachmentInfo.stencilStoreOp = wgpu::StoreOp::Undefined;
} else {
passDescriptor.cDepthStencilAttachmentInfo.stencilReadOnly = false;
}
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&passDescriptor);
pass.End();
wgpu::CommandBuffer commands = encoder.Finish();
queue.Submit(1, &commands);
}
class ReadOnlyStencilAttachmentTests : public ReadOnlyDepthStencilAttachmentTests { class ReadOnlyStencilAttachmentTests : public ReadOnlyDepthStencilAttachmentTests {
protected: protected:
void SetUp() override { void SetUp() override {