From 3995b81dccbdc7b877b77a063591c412ba9587a1 Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 12 Apr 2021 14:00:56 +0100 Subject: [PATCH] Consume dynamic offsets properly in pipeline state population * The same descriptor set can be bound multiple times with different offsets so we can't store the offset in the set. --- renderdoc/driver/vulkan/vk_replay.cpp | 37 +++---------------- .../driver/vulkan/wrappers/vk_cmd_funcs.cpp | 37 ------------------- 2 files changed, 5 insertions(+), 69 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 2fce76d4b..36591d0eb 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -1597,6 +1597,7 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) for(size_t i = 0; i < srcs[p]->size(); i++) { ResourceId src = (*srcs[p])[i].descSet; + const uint32_t *srcOffset = (*srcs[p])[i].offsets.begin(); VKPipe::DescriptorSet &dst = (*dsts[p])[i]; curBind.bindset = (uint32_t)i; @@ -1624,8 +1625,6 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) curBind.bind = (uint32_t)b; - bool dynamicOffset = false; - uint32_t descriptorCount = layoutBind.descriptorCount; if(layoutBind.variableSize) @@ -1660,11 +1659,9 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) break; case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: dst.bindings[b].type = BindType::ConstantBuffer; - dynamicOffset = true; break; case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: dst.bindings[b].type = BindType::ReadWriteBuffer; - dynamicOffset = true; break; case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: dst.bindings[b].type = BindType::InputAttachment; @@ -1857,21 +1854,6 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) dst.bindings[b].binds[a].byteOffset = c.m_BufferView[viewid].offset; dst.bindings[b].binds[a].viewFormat = MakeResourceFormat(c.m_BufferView[viewid].format); - if(dynamicOffset) - { - union - { - VkImageLayout l; - uint32_t u; - } offs; - - RDCCOMPILE_ASSERT(sizeof(VkImageLayout) == sizeof(uint32_t), - "VkImageLayout isn't 32-bit sized"); - - offs.l = info[a].imageInfo.imageLayout; - - dst.bindings[b].binds[a].byteOffset += offs.u; - } dst.bindings[b].binds[a].byteSize = c.m_BufferView[viewid].size; } else @@ -1902,20 +1884,11 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) rm->GetOriginalID(info[a].bufferInfo.buffer); dst.bindings[b].binds[a].byteOffset = info[a].bufferInfo.offset; - if(dynamicOffset) + if(layoutBind.descriptorType == VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC || + layoutBind.descriptorType == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC) { - union - { - VkImageLayout l; - uint32_t u; - } offs; - - RDCCOMPILE_ASSERT(sizeof(VkImageLayout) == sizeof(uint32_t), - "VkImageLayout isn't 32-bit sized"); - - offs.l = info[a].imageInfo.imageLayout; - - dst.bindings[b].binds[a].byteOffset += offs.u; + dst.bindings[b].binds[a].byteOffset += *srcOffset; + srcOffset++; } dst.bindings[b].binds[a].byteSize = info[a].bufferInfo.range; diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index 1dc2ff988..db7b0fa8e 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -2879,43 +2879,6 @@ bool WrappedVulkan::Serialise_vkCmdBindDescriptorSets( dynConsumed += dynCount; RDCASSERT(dynConsumed <= dynamicOffsetCount); } - - // if there are dynamic offsets, bake them into the current bindings by alias'ing - // the image layout member (which is never used for buffer views). - // This lets us look it up easily when we want to show the current pipeline state - RDCCOMPILE_ASSERT(sizeof(VkImageLayout) >= sizeof(uint32_t), - "Can't alias image layout for dynamic offset!"); - if(dynamicOffsetCount > 0) - { - uint32_t o = 0; - - // spec states that dynamic offsets precisely match all the offsets needed for these - // sets, in order of set N before set N+1, binding X before binding X+1 within a set, - // and in array element order within a binding - for(uint32_t i = 0; i < setCount; i++) - { - ResourceId descId = GetResID(pDescriptorSets[i]); - const DescSetLayout &layoutinfo = - m_CreationInfo.m_DescSetLayout[descSetLayouts[firstSet + i]]; - - for(size_t b = 0; b < layoutinfo.bindings.size(); b++) - { - // not dynamic, doesn't need an offset - if(layoutinfo.bindings[b].descriptorType != VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC && - layoutinfo.bindings[b].descriptorType != VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC) - continue; - - // assign every array element an offset according to array size - for(uint32_t a = 0; a < layoutinfo.bindings[b].descriptorCount; a++) - { - RDCASSERT(o < dynamicOffsetCount); - uint32_t *alias = - (uint32_t *)&m_DescriptorSetState[descId].data.binds[b][a].imageInfo.imageLayout; - *alias = pDynamicOffsets[o++]; - } - } - } - } } } }