Validate a subresource can't be an attachment multiple times in a pass
Bug: dawn:998, dawn:1001
Change-Id: Id7cd4964ebf9589c599059cc0f853c0125fa725a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/58361
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/PassResourceUsageTracker.cpp b/src/dawn_native/PassResourceUsageTracker.cpp
index 3aa90bf..c9b8e97 100644
--- a/src/dawn_native/PassResourceUsageTracker.cpp
+++ b/src/dawn_native/PassResourceUsageTracker.cpp
@@ -46,12 +46,23 @@
textureUsage.Update(range,
[usage](const SubresourceRange&, wgpu::TextureUsage* storedUsage) {
+ // TODO(crbug.com/dawn/1001): Consider optimizing to have fewer
+ // branches.
+ if ((*storedUsage & wgpu::TextureUsage::RenderAttachment) != 0 &&
+ (usage & wgpu::TextureUsage::RenderAttachment) != 0) {
+ // Using the same subresource as an attachment for two different
+ // render attachments is a write-write hazard. Add this internal
+ // usage so we will fail the check that a subresource with
+ // writable usage is the single usage.
+ *storedUsage |= kAgainAsRenderAttachment;
+ }
*storedUsage |= usage;
});
}
- void SyncScopeUsageTracker::AddTextureUsage(TextureBase* texture,
- const TextureSubresourceUsage& textureUsage) {
+ void SyncScopeUsageTracker::AddRenderBundleTextureUsage(
+ TextureBase* texture,
+ const TextureSubresourceUsage& textureUsage) {
// Get or create a new TextureSubresourceUsage for that texture (initially filled with
// wgpu::TextureUsage::None)
auto it = mTextureUsages.emplace(
@@ -62,7 +73,10 @@
passTextureUsage->Merge(
textureUsage, [](const SubresourceRange&, wgpu::TextureUsage* storedUsage,
- const wgpu::TextureUsage& addedUsage) { *storedUsage |= addedUsage; });
+ const wgpu::TextureUsage& addedUsage) {
+ ASSERT((addedUsage & wgpu::TextureUsage::RenderAttachment) == 0);
+ *storedUsage |= addedUsage;
+ });
}
void SyncScopeUsageTracker::AddBindGroup(BindGroupBase* group) {
diff --git a/src/dawn_native/PassResourceUsageTracker.h b/src/dawn_native/PassResourceUsageTracker.h
index d4de8fa..33f33ff 100644
--- a/src/dawn_native/PassResourceUsageTracker.h
+++ b/src/dawn_native/PassResourceUsageTracker.h
@@ -36,7 +36,8 @@
public:
void BufferUsedAs(BufferBase* buffer, wgpu::BufferUsage usage);
void TextureViewUsedAs(TextureViewBase* texture, wgpu::TextureUsage usage);
- void AddTextureUsage(TextureBase* texture, const TextureSubresourceUsage& textureUsage);
+ void AddRenderBundleTextureUsage(TextureBase* texture,
+ const TextureSubresourceUsage& textureUsage);
// Walks the bind groups and tracks all its resources.
void AddBindGroup(BindGroupBase* group);
diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp
index a7b6abc..7c1a81a 100644
--- a/src/dawn_native/RenderPassEncoder.cpp
+++ b/src/dawn_native/RenderPassEncoder.cpp
@@ -227,7 +227,8 @@
}
for (uint32_t i = 0; i < usages.textures.size(); ++i) {
- mUsageTracker.AddTextureUsage(usages.textures[i], usages.textureUsages[i]);
+ mUsageTracker.AddRenderBundleTextureUsage(usages.textures[i],
+ usages.textureUsages[i]);
}
}
diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h
index d981acc..d26053c 100644
--- a/src/dawn_native/Texture.h
+++ b/src/dawn_native/Texture.h
@@ -42,10 +42,6 @@
static constexpr wgpu::TextureUsage kReadOnlyTextureUsages =
wgpu::TextureUsage::CopySrc | wgpu::TextureUsage::Sampled | kReadOnlyStorageTexture;
- static constexpr wgpu::TextureUsage kWritableTextureUsages =
- wgpu::TextureUsage::CopyDst | wgpu::TextureUsage::Storage |
- wgpu::TextureUsage::RenderAttachment;
-
class TextureBase : public ObjectBase {
public:
enum class TextureState { OwnedInternal, OwnedExternal, Destroyed };
diff --git a/src/dawn_native/dawn_platform.h b/src/dawn_native/dawn_platform.h
index 9e75de4..537a502 100644
--- a/src/dawn_native/dawn_platform.h
+++ b/src/dawn_native/dawn_platform.h
@@ -30,6 +30,11 @@
static constexpr wgpu::TextureUsage kReadOnlyStorageTexture =
static_cast<wgpu::TextureUsage>(0x80000000);
+ // Internal usage to help tracking when a subresource is used as render attachment usage
+ // more than once in a render pass.
+ static constexpr wgpu::TextureUsage kAgainAsRenderAttachment =
+ static_cast<wgpu::TextureUsage>(0x80000001);
+
// Add an extra texture usage for textures that will be presented, for use in backends
// that needs to transition to present usage.
// This currently aliases wgpu::TextureUsage::Present, we would assign it
diff --git a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp
index ffc1fe4..0cf64d8 100644
--- a/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp
+++ b/src/tests/unittests/validation/ResourceUsageTrackingTests.cpp
@@ -990,6 +990,70 @@
}
}
+ // Test that a single subresource of a texture cannot be used as a render attachment more than
+ // once in the same pass.
+ TEST_F(ResourceUsageTrackingTest, TextureWithMultipleRenderAttachmentUsage) {
+ // Create a texture with two array layers
+ wgpu::TextureDescriptor descriptor;
+ descriptor.dimension = wgpu::TextureDimension::e2D;
+ descriptor.size = {1, 1, 2};
+ descriptor.usage = wgpu::TextureUsage::RenderAttachment;
+ descriptor.format = kFormat;
+
+ wgpu::Texture texture = device.CreateTexture(&descriptor);
+
+ wgpu::TextureViewDescriptor viewDesc = {};
+ viewDesc.arrayLayerCount = 1;
+
+ wgpu::TextureView viewLayer0 = texture.CreateView(&viewDesc);
+
+ viewDesc.baseArrayLayer = 1;
+ wgpu::TextureView viewLayer1 = texture.CreateView(&viewDesc);
+
+ // Control: It is valid to use layer0 as a render target for one attachment, and
+ // layer1 as the second attachment in the same pass
+ {
+ utils::ComboRenderPassDescriptor renderPass({viewLayer0, viewLayer1});
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.EndPass();
+ encoder.Finish();
+ }
+
+ // Control: It is valid to use layer0 as a render target in separate passes.
+ {
+ utils::ComboRenderPassDescriptor renderPass({viewLayer0});
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass0 = encoder.BeginRenderPass(&renderPass);
+ pass0.EndPass();
+ wgpu::RenderPassEncoder pass1 = encoder.BeginRenderPass(&renderPass);
+ pass1.EndPass();
+ encoder.Finish();
+ }
+
+ // It is invalid to use layer0 as a render target for both attachments in the same pass
+ {
+ utils::ComboRenderPassDescriptor renderPass({viewLayer0, viewLayer0});
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.EndPass();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ // It is invalid to use layer1 as a render target for both attachments in the same pass
+ {
+ utils::ComboRenderPassDescriptor renderPass({viewLayer1, viewLayer1});
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.EndPass();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+ }
+
// Test that using the same texture as both readable and writable in different passes is
// allowed
TEST_F(ResourceUsageTrackingTest, TextureWithReadAndWriteUsageInDifferentPasses) {
