d3d12/vulkan: Skip descriptor update/creation if resource is destroyed

Validation of resource destroy state does not happen on bind group
creation. It happens on queue submit. This means the Vulkan and D3D12
backends need to gracefully handle BindGroup creation with destroyed
resources by skipping the descriptor creation if the resource has
been destroyed.

Bug: dawn:319
Change-Id: I270afba86d1a961e1e4c39f2419d8c34ff889e46
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38440
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
This commit is contained in:
Austin Eng 2021-01-22 17:10:36 +00:00 committed by Commit Bot service account
parent 72cd1a5e89
commit db1572102b
3 changed files with 96 additions and 6 deletions

View File

@ -57,6 +57,14 @@ namespace dawn_native { namespace d3d12 {
case BindingInfoType::Buffer: {
BufferBinding binding = GetBindingAsBufferBinding(bindingIndex);
ID3D12Resource* resource = ToBackend(binding.buffer)->GetD3D12Resource();
if (resource == nullptr) {
// The Buffer was destroyed. Skip creating buffer views since there is no
// resource. This bind group won't be used as it is an error to submit a
// command buffer that references destroyed resources.
continue;
}
switch (bindingInfo.buffer.type) {
case wgpu::BufferBindingType::Uniform: {
D3D12_CONSTANT_BUFFER_VIEW_DESC desc;
@ -89,7 +97,7 @@ namespace dawn_native { namespace d3d12 {
desc.Buffer.Flags = D3D12_BUFFER_UAV_FLAG_RAW;
d3d12Device->CreateUnorderedAccessView(
ToBackend(binding.buffer)->GetD3D12Resource(), nullptr, &desc,
resource, nullptr, &desc,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
@ -108,7 +116,7 @@ namespace dawn_native { namespace d3d12 {
desc.Buffer.StructureByteStride = 0;
desc.Buffer.Flags = D3D12_BUFFER_SRV_FLAG_RAW;
d3d12Device->CreateShaderResourceView(
ToBackend(binding.buffer)->GetD3D12Resource(), &desc,
resource, &desc,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
@ -123,8 +131,17 @@ namespace dawn_native { namespace d3d12 {
case BindingInfoType::Texture: {
auto* view = ToBackend(GetBindingAsTextureView(bindingIndex));
auto& srv = view->GetSRVDescriptor();
ID3D12Resource* resource = ToBackend(view->GetTexture())->GetD3D12Resource();
if (resource == nullptr) {
// The Texture was destroyed. Skip creating the SRV since there is no
// resource. This bind group won't be used as it is an error to submit a
// command buffer that references destroyed resources.
continue;
}
d3d12Device->CreateShaderResourceView(
ToBackend(view->GetTexture())->GetD3D12Resource(), &srv,
resource, &srv,
viewAllocation.OffsetFrom(viewSizeIncrement, bindingOffsets[bindingIndex]));
break;
}
@ -132,13 +149,21 @@ namespace dawn_native { namespace d3d12 {
case BindingInfoType::StorageTexture: {
TextureView* view = ToBackend(GetBindingAsTextureView(bindingIndex));
ID3D12Resource* resource = ToBackend(view->GetTexture())->GetD3D12Resource();
if (resource == nullptr) {
// The Texture was destroyed. Skip creating the SRV/UAV since there is no
// resource. This bind group won't be used as it is an error to submit a
// command buffer that references destroyed resources.
continue;
}
switch (bindingInfo.storageTexture.access) {
case wgpu::StorageTextureAccess::ReadOnly: {
// Readonly storage is implemented as SRV so it can be used at the same
// time as a sampled texture.
auto& srv = view->GetSRVDescriptor();
d3d12Device->CreateShaderResourceView(
ToBackend(view->GetTexture())->GetD3D12Resource(), &srv,
resource, &srv,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;
@ -147,7 +172,7 @@ namespace dawn_native { namespace d3d12 {
case wgpu::StorageTextureAccess::WriteOnly: {
D3D12_UNORDERED_ACCESS_VIEW_DESC uav = view->GetUAVDescriptor();
d3d12Device->CreateUnorderedAccessView(
ToBackend(view->GetTexture())->GetD3D12Resource(), nullptr, &uav,
resource, nullptr, &uav,
viewAllocation.OffsetFrom(viewSizeIncrement,
bindingOffsets[bindingIndex]));
break;

View File

@ -66,7 +66,15 @@ namespace dawn_native { namespace vulkan {
case BindingInfoType::Buffer: {
BufferBinding binding = GetBindingAsBufferBinding(bindingIndex);
writeBufferInfo[numWrites].buffer = ToBackend(binding.buffer)->GetHandle();
VkBuffer handle = ToBackend(binding.buffer)->GetHandle();
if (handle == VK_NULL_HANDLE) {
// The Buffer was destroyed. Skip this descriptor write since it would be
// a Vulkan Validation Layers error. This bind group won't be used as it
// is an error to submit a command buffer that references destroyed
// resources.
continue;
}
writeBufferInfo[numWrites].buffer = handle;
writeBufferInfo[numWrites].offset = binding.offset;
writeBufferInfo[numWrites].range = binding.size;
write.pBufferInfo = &writeBufferInfo[numWrites];

View File

@ -1320,6 +1320,63 @@ TEST_P(BindGroupTests, ReallyLargeBindGroup) {
EXPECT_BUFFER_U32_EQ(1, result, 0);
}
// This is a regression test for crbug.com/dawn/319 where creating a bind group with a
// destroyed resource would crash the backend.
TEST_P(BindGroupTests, CreateWithDestroyedResource) {
auto doBufferTest = [&](wgpu::BufferBindingType bindingType, wgpu::BufferUsage usage) {
wgpu::BindGroupLayout bgl =
utils::MakeBindGroupLayout(device, {{0, wgpu::ShaderStage::Fragment, bindingType}});
wgpu::BufferDescriptor bufferDesc;
bufferDesc.size = sizeof(float);
bufferDesc.usage = usage;
wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc);
buffer.Destroy();
wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, buffer, 0, sizeof(float)}});
};
// Test various usages and binding types since they take different backend code paths.
doBufferTest(wgpu::BufferBindingType::Uniform, wgpu::BufferUsage::Uniform);
doBufferTest(wgpu::BufferBindingType::Storage, wgpu::BufferUsage::Storage);
doBufferTest(wgpu::BufferBindingType::ReadOnlyStorage, wgpu::BufferUsage::Storage);
// Test a sampled texture.
{
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::Float}});
wgpu::TextureDescriptor textureDesc;
textureDesc.usage = wgpu::TextureUsage::Sampled;
textureDesc.size = {1, 1, 1};
textureDesc.format = wgpu::TextureFormat::BGRA8Unorm;
wgpu::Texture texture = device.CreateTexture(&textureDesc);
wgpu::TextureView textureView = texture.CreateView();
texture.Destroy();
wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, textureView}});
}
// Test a storage texture.
{
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::StorageTextureAccess::ReadOnly,
wgpu::TextureFormat::R32Uint}});
wgpu::TextureDescriptor textureDesc;
textureDesc.usage = wgpu::TextureUsage::Storage;
textureDesc.size = {1, 1, 1};
textureDesc.format = wgpu::TextureFormat::R32Uint;
wgpu::Texture texture = device.CreateTexture(&textureDesc);
wgpu::TextureView textureView = texture.CreateView();
texture.Destroy();
wgpu::BindGroup bg = utils::MakeBindGroup(device, bgl, {{0, textureView}});
}
}
DAWN_INSTANTIATE_TEST(BindGroupTests,
D3D12Backend(),
MetalBackend(),