[dawn][native] Validate ExternalTextures used in RenderBundle
Validation was missing that an ExternalTexture used in a bundle was
alive when used in a submit.
- Move the code to handle merging the sync scope usages of render
bundles from ExecuteBundles to a new helper function in
PassResourceUsageTracker.
- Add handling of ExternalTexture merging to that function.
- Add tests.
Bug: None
Change-Id: Ief15a7a85a085f58c6c71b6e541a1436ce60a9d7
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/258294
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/native/PassResourceUsageTracker.cpp b/src/dawn/native/PassResourceUsageTracker.cpp
index 7be797f..18e3a48 100644
--- a/src/dawn/native/PassResourceUsageTracker.cpp
+++ b/src/dawn/native/PassResourceUsageTracker.cpp
@@ -84,18 +84,17 @@
});
}
-void SyncScopeUsageTracker::AddRenderBundleTextureUsage(
- TextureBase* texture,
- const TextureSubresourceSyncInfo& textureSyncInfo) {
+void SyncScopeUsageTracker::MergeTextureUsage(TextureBase* texture,
+ const TextureSubresourceSyncInfo& textureSyncInfo) {
// Get or create a new TextureSubresourceSyncInfo for that texture (initially filled with
// wgpu::TextureUsage::None and WGPUShaderStage_None)
auto it = mTextureSyncInfos.try_emplace(
texture, texture->GetFormat().aspects, texture->GetArrayLayers(),
texture->GetNumMipLevels(),
TextureSyncInfo{wgpu::TextureUsage::None, wgpu::ShaderStage::None});
- TextureSubresourceSyncInfo* passTextureSyncInfo = &it.first->second;
+ TextureSubresourceSyncInfo& passTextureSyncInfo = it.first->second;
- passTextureSyncInfo->Merge(
+ passTextureSyncInfo.Merge(
textureSyncInfo, [](const SubresourceRange&, TextureSyncInfo* storedSyncInfo,
const TextureSyncInfo& addedSyncInfo) {
DAWN_ASSERT((addedSyncInfo.usage & wgpu::TextureUsage::RenderAttachment) == 0);
@@ -104,6 +103,21 @@
});
}
+void SyncScopeUsageTracker::MergeResourceUsages(const SyncScopeResourceUsage& usages) {
+ for (uint32_t i = 0; i < usages.buffers.size(); ++i) {
+ BufferUsedAs(usages.buffers[i], usages.bufferSyncInfos[i].usage,
+ usages.bufferSyncInfos[i].shaderStages);
+ }
+
+ for (uint32_t i = 0; i < usages.textures.size(); ++i) {
+ MergeTextureUsage(usages.textures[i], usages.textureSyncInfos[i]);
+ }
+
+ for (ExternalTextureBase* t : usages.externalTextures) {
+ mExternalTextureUsages.insert(t);
+ }
+}
+
void SyncScopeUsageTracker::AddBindGroup(BindGroupBase* group) {
const auto* layout = group->GetLayout();
@@ -196,18 +210,17 @@
result.buffers.push_back(buffer);
result.bufferSyncInfos.push_back(std::move(syncInfo));
}
+ mBufferSyncInfos.clear();
for (auto& [texture, syncInfo] : mTextureSyncInfos) {
result.textures.push_back(texture);
result.textureSyncInfos.push_back(std::move(syncInfo));
}
+ mTextureSyncInfos.clear();
for (auto* const it : mExternalTextureUsages) {
result.externalTextures.push_back(it);
}
-
- mBufferSyncInfos.clear();
- mTextureSyncInfos.clear();
mExternalTextureUsages.clear();
return result;
diff --git a/src/dawn/native/PassResourceUsageTracker.h b/src/dawn/native/PassResourceUsageTracker.h
index 905f797..d143526 100644
--- a/src/dawn/native/PassResourceUsageTracker.h
+++ b/src/dawn/native/PassResourceUsageTracker.h
@@ -65,8 +65,9 @@
const SubresourceRange& range,
wgpu::TextureUsage usage,
wgpu::ShaderStage shaderStages = wgpu::ShaderStage::None);
- void AddRenderBundleTextureUsage(TextureBase* texture,
- const TextureSubresourceSyncInfo& textureSyncInfo);
+
+ // Add all usages referenced to this tracker.
+ void MergeResourceUsages(const SyncScopeResourceUsage& usages);
// Walks the bind groups and tracks all its resources.
void AddBindGroup(BindGroupBase* group);
@@ -75,6 +76,8 @@
SyncScopeResourceUsage AcquireSyncScopeUsage();
private:
+ void MergeTextureUsage(TextureBase* texture, const TextureSubresourceSyncInfo& textureSyncInfo);
+
absl::flat_hash_map<BufferBase*, BufferSyncInfo> mBufferSyncInfos;
absl::flat_hash_map<TextureBase*, TextureSubresourceSyncInfo> mTextureSyncInfos;
absl::flat_hash_set<ExternalTextureBase*> mExternalTextureUsages;
diff --git a/src/dawn/native/RenderPassEncoder.cpp b/src/dawn/native/RenderPassEncoder.cpp
index 4aef4bd..6aedb91 100644
--- a/src/dawn/native/RenderPassEncoder.cpp
+++ b/src/dawn/native/RenderPassEncoder.cpp
@@ -376,17 +376,7 @@
for (uint32_t i = 0; i < count; ++i) {
bundles[i] = renderBundles[i];
- const RenderPassResourceUsage& usages = bundles[i]->GetResourceUsage();
- for (uint32_t j = 0; j < usages.buffers.size(); ++j) {
- mUsageTracker.BufferUsedAs(usages.buffers[j], usages.bufferSyncInfos[j].usage,
- usages.bufferSyncInfos[j].shaderStages);
- }
-
- for (uint32_t j = 0; j < usages.textures.size(); ++j) {
- mUsageTracker.AddRenderBundleTextureUsage(usages.textures[j],
- usages.textureSyncInfos[j]);
- }
-
+ mUsageTracker.MergeResourceUsages(bundles[i]->GetResourceUsage());
if (IsValidationEnabled()) {
mIndirectDrawMetadata.AddBundle(renderBundles[i]);
}
diff --git a/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp b/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp
index 5e95c2b..6b0eb89 100644
--- a/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp
+++ b/src/dawn/tests/unittests/validation/ExternalTextureTests.cpp
@@ -82,6 +82,42 @@
}
}
+ void SubmitExternalTextureInDefaultRenderBundle(wgpu::ExternalTexture externalTexture,
+ bool success) {
+ // Create a bind group that contains the external texture.
+ wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
+ device, {{0, wgpu::ShaderStage::Fragment, &utils::kExternalTextureBindingLayout}});
+ wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, bgl, {{0, externalTexture}});
+
+ // Create another texture to use as a color attachment.
+ wgpu::TextureDescriptor renderTextureDescriptor = CreateTextureDescriptor();
+ wgpu::Texture renderTexture = device.CreateTexture(&renderTextureDescriptor);
+ wgpu::TextureView renderView = renderTexture.CreateView();
+
+ // Create a RenderBundle using the bindgroup and doing nothing else.
+ wgpu::RenderBundleEncoderDescriptor rbDesc;
+ rbDesc.colorFormatCount = 1;
+ rbDesc.colorFormats = &renderTextureDescriptor.format;
+
+ wgpu::RenderBundleEncoder rbEncoder = device.CreateRenderBundleEncoder(&rbDesc);
+ rbEncoder.SetBindGroup(0, bindGroup);
+ wgpu::RenderBundle bundle = rbEncoder.Finish();
+
+ // Use that render bundle in the render pass.
+ utils::ComboRenderPassDescriptor renderPass({renderView}, nullptr);
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.ExecuteBundles(1, &bundle);
+ pass.End();
+ wgpu::CommandBuffer commands = encoder.Finish();
+
+ if (success) {
+ queue.Submit(1, &commands);
+ } else {
+ ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+ }
+ }
+
void SubmitExternalTextureInDefaultComputePass(wgpu::ExternalTexture externalTexture,
bool success) {
// Create a bind group that contains the external texture.
@@ -515,13 +551,34 @@
SubmitExternalTextureInDefaultComputePass(externalTexture, false /* success = false */);
}
-// Test that submitting a compute pass that contains an active external texture should success.
+// Test that submitting a compute pass that contains a destroyed external texture should success.
TEST_F(ExternalTextureTest, SubmitDestroyedExternalTextureInComputePass) {
wgpu::ExternalTexture externalTexture = CreateDefaultExternalTexture();
externalTexture.Destroy();
SubmitExternalTextureInDefaultComputePass(externalTexture, false /* success = false */);
}
+// Test that submitting a render bundle that contains an active external.
+TEST_F(ExternalTextureTest, SubmitActiveExternalTextureInRenderBundle) {
+ wgpu::ExternalTexture externalTexture = CreateDefaultExternalTexture();
+ SubmitExternalTextureInDefaultRenderBundle(externalTexture, true /* success = true */);
+}
+
+// Test that submitting a render bundle that contains an expired external texture results in an
+// error.
+TEST_F(ExternalTextureTest, SubmitExpiredExternalTextureInRenderBundle) {
+ wgpu::ExternalTexture externalTexture = CreateDefaultExternalTexture();
+ externalTexture.Expire();
+ SubmitExternalTextureInDefaultRenderBundle(externalTexture, false /* success = false */);
+}
+
+// Test that submitting a render bundle that contains a destroyed external texture should success.
+TEST_F(ExternalTextureTest, SubmitDestroyedExternalTextureInRenderBundle) {
+ wgpu::ExternalTexture externalTexture = CreateDefaultExternalTexture();
+ externalTexture.Destroy();
+ SubmitExternalTextureInDefaultRenderBundle(externalTexture, false /* success = false */);
+}
+
// Test that refresh an expired external texture and submit a compute pass with it.
TEST_F(ExternalTextureTest, RefreshExpiredExternalTexture) {
wgpu::ExternalTexture externalTexture = CreateDefaultExternalTexture();
