Skip to content

Commit

Permalink
Address various vulkan validation issues + Bindless support related f…
Browse files Browse the repository at this point in the history 
…ixes (#13492)

* Address various vulkan validation issues + other cleanup work related to Bindless

Signed-off-by: moudgils <47460854+moudgils@users.noreply.github.com>

* Minor update

Signed-off-by: moudgils <47460854+moudgils@users.noreply.github.com>

* Went with a different approach where we do not add a descriptor if the depth or stencil flag is set on the view descriptor

Signed-off-by: moudgils <47460854+moudgils@users.noreply.github.com>

* Minor cleanup

Signed-off-by: moudgils <47460854+moudgils@users.noreply.github.com>

* Minor update based on feedback

Signed-off-by: moudgils <47460854+moudgils@users.noreply.github.com>

Signed-off-by: moudgils <47460854+moudgils@users.noreply.github.com>
  • Loading branch information
@moudgils
moudgils committed Dec 2, 2022
1 parent 7cf4fe4 commit 4ac8adf
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 43 deletions.
41 changes: 28 additions & 13 deletions Gems/Atom/RHI/Metal/Code/Source/RHI/BindlessArgumentBuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,85 +139,100 @@ namespace AZ::Metal

uint32_t BindlessArgumentBuffer::AttachReadImage(id <MTLTexture> mtlTexture)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadTexture);
uint32_t heapIndex = static_cast<uint32_t>(m_allocators[allocatorIndex].Allocate(1, 1).m_ptr);
if(m_unboundedArraySupported)
RHI::VirtualAddress address = m_allocators[allocatorIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);
if (m_unboundedArraySupported)
{
m_bindlessTextureArgBuffer->UpdateTextureView(mtlTexture, heapIndex);
}
else
{
m_boundedArgBuffer->UpdateTextureView(mtlTexture, (heapIndex+(UnboundedArraySize*allocatorIndex)));
m_boundedArgBuffer->UpdateTextureView(mtlTexture, (heapIndex + (UnboundedArraySize * allocatorIndex)));
}
return heapIndex;
}

uint32_t BindlessArgumentBuffer::AttachReadWriteImage(id <MTLTexture> mtlTexture)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteTexture);
uint32_t heapIndex = static_cast<uint32_t>(m_allocators[allocatorIndex].Allocate(1, 1).m_ptr);
if(m_unboundedArraySupported)
RHI::VirtualAddress address = m_allocators[allocatorIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);
if (m_unboundedArraySupported)
{
m_bindlessRWTextureArgBuffer->UpdateTextureView(mtlTexture, heapIndex);
}
else
{
m_boundedArgBuffer->UpdateTextureView(mtlTexture, (heapIndex+(UnboundedArraySize*allocatorIndex)));
m_boundedArgBuffer->UpdateTextureView(mtlTexture, (heapIndex + (UnboundedArraySize * allocatorIndex)));
}

return heapIndex;
}

uint32_t BindlessArgumentBuffer::AttachReadBuffer(id <MTLBuffer> mtlBuffer, uint32_t offset)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadBuffer);
uint32_t heapIndex = static_cast<uint32_t>(m_allocators[allocatorIndex].Allocate(1, 1).m_ptr);
if(m_unboundedArraySupported)
RHI::VirtualAddress address = m_allocators[allocatorIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);
if (m_unboundedArraySupported)
{
m_bindlessBufferArgBuffer->UpdateBufferView(mtlBuffer, offset, heapIndex);
}
else
{
m_boundedArgBuffer->UpdateBufferView(mtlBuffer, offset, (heapIndex+(UnboundedArraySize*allocatorIndex)));
m_boundedArgBuffer->UpdateBufferView(mtlBuffer, offset, (heapIndex + (UnboundedArraySize * allocatorIndex)));
}
return heapIndex;
}

uint32_t BindlessArgumentBuffer::AttachReadWriteBuffer(id <MTLBuffer> mtlBuffer, uint32_t offset)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteBuffer);
uint32_t heapIndex = static_cast<uint32_t>(m_allocators[allocatorIndex].Allocate(1, 1).m_ptr);
if(m_unboundedArraySupported)
RHI::VirtualAddress address = m_allocators[allocatorIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);
if (m_unboundedArraySupported)
{
m_bindlessRWBufferArgBuffer->UpdateBufferView(mtlBuffer, offset, heapIndex);
}
else
{
m_boundedArgBuffer->UpdateBufferView(mtlBuffer, offset, (heapIndex+(UnboundedArraySize*allocatorIndex)));
m_boundedArgBuffer->UpdateBufferView(mtlBuffer, offset, (heapIndex + (UnboundedArraySize * allocatorIndex)));
}
return heapIndex;
}

void BindlessArgumentBuffer::DetachReadImage(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadTexture);
m_allocators[allocatorIndex].DeAllocate({ index });
}

void BindlessArgumentBuffer::DetachReadWriteImage(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteTexture);
m_allocators[allocatorIndex].DeAllocate({ index });
}

void BindlessArgumentBuffer::DetachReadBuffer(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadBuffer);
m_allocators[allocatorIndex].DeAllocate({ index });
}

void BindlessArgumentBuffer::DetachReadWriteBuffer(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
uint32_t allocatorIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteBuffer);
m_allocators[allocatorIndex].DeAllocate({ index });
}
Expand Down
2 changes: 2 additions & 0 deletions Gems/Atom/RHI/Metal/Code/Source/RHI/BindlessArgumentBuffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ namespace AZ
RHI::FreeListAllocator m_allocators[static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::Count)];
Device* m_device = nullptr;
bool m_unboundedArraySupported = false;
// Mutex to protect bindless heap related updates
AZStd::mutex m_mutex;
};
}
}
63 changes: 41 additions & 22 deletions Gems/Atom/RHI/Vulkan/Code/Source/RHI/BindlessDescriptorPool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,95 +135,114 @@ namespace AZ::Vulkan
uint32_t BindlessDescriptorPool::AttachReadImage(ImageView* view)
{
const uint32_t roTextureIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadTexture);
AZStd::array<RHI::ConstPtr<RHI::ImageView>, 1> span{ view };
uint32_t index = static_cast<uint32_t>(m_allocators[roTextureIndex].Allocate(1, 1).m_ptr);

AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
RHI::VirtualAddress address = m_allocators[roTextureIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);

VkWriteDescriptorSet write = PrepareWrite(index, roTextureIndex, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE);
VkWriteDescriptorSet write = PrepareWrite(heapIndex, roTextureIndex, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE);
VkDescriptorImageInfo imageInfo{};
imageInfo.imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
imageInfo.imageLayout = RHI::CheckBitsAny(view->GetImage().GetAspectFlags(), RHI::ImageAspectFlags::DepthStencil)
? VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL
: VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;

imageInfo.imageView = view->GetNativeImageView();
write.pImageInfo = &imageInfo;
m_device->GetContext().UpdateDescriptorSets(m_device->GetNativeDevice(), 1, &write, 0, nullptr);

return index;
return heapIndex;
}

uint32_t BindlessDescriptorPool::AttachReadWriteImage(ImageView* view)
{
const uint32_t rwTextureIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteTexture);
AZStd::array<RHI::ConstPtr<RHI::ImageView>, 1> span{ view };
uint32_t index = static_cast<uint32_t>(m_allocators[rwTextureIndex].Allocate(1, 1).m_ptr);

VkWriteDescriptorSet write = PrepareWrite(index, rwTextureIndex, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE);
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
RHI::VirtualAddress address = m_allocators[rwTextureIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);

VkWriteDescriptorSet write = PrepareWrite(heapIndex, rwTextureIndex, VK_DESCRIPTOR_TYPE_STORAGE_IMAGE);
VkDescriptorImageInfo imageInfo{};
imageInfo.imageLayout = VK_IMAGE_LAYOUT_GENERAL;
imageInfo.imageLayout = RHI::CheckBitsAny(view->GetImage().GetAspectFlags(), RHI::ImageAspectFlags::DepthStencil)
? VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL
: VK_IMAGE_LAYOUT_GENERAL;

imageInfo.imageView = view->GetNativeImageView();
write.pImageInfo = &imageInfo;
m_device->GetContext().UpdateDescriptorSets(m_device->GetNativeDevice(), 1, &write, 0, nullptr);

return index;
return heapIndex;
}

uint32_t BindlessDescriptorPool::AttachReadBuffer(BufferView* view)
{
const uint32_t roBufferIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadBuffer);
AZStd::array<const RHI::ConstPtr<RHI::BufferView>, 1> span{ view };
uint32_t index = static_cast<uint32_t>(m_allocators[roBufferIndex].Allocate(1, 1).m_ptr);

AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
RHI::VirtualAddress address = m_allocators[roBufferIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);

const auto& viewDesc = view->GetDescriptor();
const Vulkan::BufferMemoryView& bufferMemoryView = *static_cast<const Vulkan::Buffer&>(view->GetBuffer()).GetBufferMemoryView();

VkWriteDescriptorSet write = PrepareWrite(index, roBufferIndex, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
VkWriteDescriptorSet write = PrepareWrite(heapIndex, roBufferIndex, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
VkDescriptorBufferInfo bufferInfo{};
bufferInfo.buffer = bufferMemoryView.GetNativeBuffer();
bufferInfo.offset = bufferMemoryView.GetOffset() + viewDesc.m_elementSize * viewDesc.m_elementOffset;
bufferInfo.range = viewDesc.m_elementSize * viewDesc.m_elementCount;
write.pBufferInfo = &bufferInfo;
m_device->GetContext().UpdateDescriptorSets(m_device->GetNativeDevice(), 1, &write, 0, nullptr);

return index;
return heapIndex;
}

uint32_t BindlessDescriptorPool::AttachReadWriteBuffer(BufferView* view)
{
const uint32_t rwBufferIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteBuffer);
AZStd::array<RHI::ConstPtr<RHI::BufferView>, 1> span{ view };
uint32_t index = static_cast<uint32_t>(m_allocators[rwBufferIndex].Allocate(1, 1).m_ptr);

AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
RHI::VirtualAddress address = m_allocators[rwBufferIndex].Allocate(1, 1);
AZ_Assert(address.IsValid(), "Bindless allocator ran out of space.");
uint32_t heapIndex = static_cast<uint32_t>(address.m_ptr);

const auto& viewDesc = view->GetDescriptor();
const Vulkan::BufferMemoryView& bufferMemoryView = *static_cast<const Vulkan::Buffer&>(view->GetBuffer()).GetBufferMemoryView();

VkWriteDescriptorSet write = PrepareWrite(index, rwBufferIndex, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
VkWriteDescriptorSet write = PrepareWrite(heapIndex, rwBufferIndex, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
VkDescriptorBufferInfo bufferInfo{};
bufferInfo.buffer = bufferMemoryView.GetNativeBuffer();
bufferInfo.offset = bufferMemoryView.GetOffset() + viewDesc.m_elementSize * viewDesc.m_elementOffset;
bufferInfo.range = viewDesc.m_elementSize * viewDesc.m_elementCount;
write.pBufferInfo = &bufferInfo;
m_device->GetContext().UpdateDescriptorSets(m_device->GetNativeDevice(), 1, &write, 0, nullptr);

return index;
return heapIndex;
}

void BindlessDescriptorPool::DetachReadImage(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
const uint32_t roTextureIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadTexture);
m_allocators[roTextureIndex].DeAllocate({ index });
}

void BindlessDescriptorPool::DetachReadWriteImage(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
const uint32_t rwTextureIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteTexture);
m_allocators[rwTextureIndex].DeAllocate({ index });
}

void BindlessDescriptorPool::DetachReadBuffer(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
const uint32_t roBufferIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadBuffer);
m_allocators[roBufferIndex].DeAllocate({ index });
}

void BindlessDescriptorPool::DetachReadWriteBuffer(uint32_t index)
{
AZStd::lock_guard<AZStd::mutex> lock(m_mutex);
const uint32_t rwBufferIndex = static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::ReadWriteBuffer);
m_allocators[rwBufferIndex].DeAllocate({ index });
}
Expand Down
3 changes: 3 additions & 0 deletions Gems/Atom/RHI/Vulkan/Code/Source/RHI/BindlessDescriptorPool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,8 @@ namespace AZ::Vulkan
VkDescriptorSet m_set;

RHI::FreeListAllocator m_allocators[static_cast<uint32_t>(RHI::ShaderResourceGroupData::BindlessResourceType::Count)];

// Mutex to protect bindless heap related updates
AZStd::mutex m_mutex;
};
}
13 changes: 9 additions & 4 deletions Gems/Atom/RHI/Vulkan/Code/Source/RHI/ImageView.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ namespace AZ
DeviceObject::Init(deviceBase);
auto& device = static_cast<Device&>(deviceBase);
const auto& image = static_cast<const Image&>(resourceBase);
const RHI::ImageViewDescriptor& descriptor = GetDescriptor();
const RHI::ImageViewDescriptor& viewDescriptor = GetDescriptor();

// this can happen when image has been invalidated/released right before re-compiling the image
if (image.GetNativeImage() == VK_NULL_HANDLE)
{
return RHI::ResultCode::Fail;
}

RHI::Format viewFormat = descriptor.m_overrideFormat;
RHI::Format viewFormat = viewDescriptor.m_overrideFormat;
// If an image is not owner of native image, it is a swapchain image.
// Swapchain images are not mutable, so we can not change the format for the view.
if (viewFormat == RHI::Format::Unknown || !image.IsOwnerOfNativeImage())
Expand All @@ -75,7 +75,7 @@ namespace AZ
}
m_format = viewFormat;

VkImageAspectFlags aspectFlags = ConvertImageAspectFlags(RHI::FilterBits(descriptor.m_aspectFlags, image.GetAspectFlags()));
VkImageAspectFlags aspectFlags = ConvertImageAspectFlags(RHI::FilterBits(viewDescriptor.m_aspectFlags, image.GetAspectFlags()));

const VkImageViewType imageViewType = GetImageViewType(image);
BuildImageSubresourceRange(imageViewType, aspectFlags);
Expand Down Expand Up @@ -103,7 +103,12 @@ namespace AZ
m_hash = TypeHash64(m_imageSubresourceRange.GetHash(), m_hash);
m_hash = TypeHash64(m_format, m_hash);

if (!descriptor.m_isArray && !descriptor.m_isCubemap)
// If a depth stencil image does not have depth or aspect flag set it is probably going to be used as
// a render target and do not need to be added to the bindless heap
bool isReadOnlyDSView = RHI::CheckBitsAll(viewDescriptor.m_aspectFlags, RHI::ImageAspectFlags::Depth) ||
RHI::CheckBitsAll(viewDescriptor.m_aspectFlags, RHI::ImageAspectFlags::Stencil);

if (!viewDescriptor.m_isArray && !viewDescriptor.m_isCubemap && !isReadOnlyDSView)
{
if (RHI::CheckBitsAll(image.GetDescriptor().m_bindFlags, RHI::ImageBindFlags::ShaderRead))
{
Expand Down
7 changes: 6 additions & 1 deletion Gems/AtomLyIntegration/EditorModeFeedback/Assets/Passes/Child/EditorModeBlur.pass
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
"Name": "InputDepth",
"SlotType": "Input",
"ShaderInputName": "m_depth",
"ScopeAttachmentUsage": "Shader"
"ScopeAttachmentUsage": "Shader",
"ImageViewDesc": {
"AspectFlags": [
"Depth"
]
}
},
{
"Name": "InputEntityMask",
Expand Down
7 changes: 6 additions & 1 deletion Gems/AtomLyIntegration/EditorModeFeedback/Assets/Passes/Child/EditorModeDesaturation.pass
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
"Name": "InputDepth",
"SlotType": "Input",
"ShaderInputName": "m_depth",
"ScopeAttachmentUsage": "Shader"
"ScopeAttachmentUsage": "Shader",
"ImageViewDesc": {
"AspectFlags": [
"Depth"
]
}
},
{
"Name": "InputEntityMask",
Expand Down
7 changes: 6 additions & 1 deletion Gems/AtomLyIntegration/EditorModeFeedback/Assets/Passes/Child/EditorModeOutline.pass
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
"Name": "InputDepth",
"SlotType": "Input",
"ShaderInputName": "m_depth",
"ScopeAttachmentUsage": "Shader"
"ScopeAttachmentUsage": "Shader",
"ImageViewDesc": {
"AspectFlags": [
"Depth"
]
}
},
{
"Name": "InputEntityMask",
Expand Down
7 changes: 6 additions & 1 deletion Gems/AtomLyIntegration/EditorModeFeedback/Assets/Passes/Child/EditorModeTint.pass
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@
"Name": "InputDepth",
"SlotType": "Input",
"ShaderInputName": "m_depth",
"ScopeAttachmentUsage": "Shader"
"ScopeAttachmentUsage": "Shader",
"ImageViewDesc": {
"AspectFlags": [
"Depth"
]
}
},
{
"Name": "InputEntityMask",
Expand Down
2 changes: 2 additions & 0 deletions Gems/AtomLyIntegration/EditorModeFeedback/Code/Source/Pass/EditorStatePassSystemUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,8 @@ namespace AZ::Render
slot.m_shaderInputName = Name("m_existingDepth");
slot.m_scopeAttachmentUsage = RHI::ScopeAttachmentUsage::Shader;
slot.m_shaderImageDimensionsName = Name("m_existingDepthDimensions");
slot.m_imageViewDesc = AZStd::make_shared<RHI::ImageViewDescriptor>();
slot.m_imageViewDesc->m_aspectFlags = RHI::ImageAspectFlags::Depth;
maskPassTemplate->AddSlot(slot);
}

Expand Down

0 comments on commit 4ac8adf

Please sign in to comment.