Use pipeline layout from descriptor set binds to iterate descriptor sets

* If a pipeline doesn't statically access a descriptor set, the corresponding
  descriptor set layout in its pipeline layout can be essentially anything and
  it doesn't have to match the actual descriptor set bound at a draw. It's just
  ignored.
* Rather than check for static access ourselves we take advantage of another
  fact - when the descriptor set is bound it must be compatible with the set
  layout from the bind call's pipeline layout. If the pipeline *does* statically
  use the descriptor set, its pipeline layout must be compatible with the bind
  call's pipeline layout for that set.
* So the end result is that we can safely use the bind call's pipeline layout
  for iterating over bound descriptors, secure in the knowledge that it's always
  valid for that data, and if the pipeline uses it then it's also valid for the
  pipeline.
This commit is contained in:
baldurk
2019-09-02 12:19:37 +01:00
parent e6d0c93b41
commit 1107462f05
6 changed files with 61 additions and 30 deletions
@@ -772,7 +772,10 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId)
modifiedpipe.descSets.resize(1);
for(size_t i = 0; i < descSets.size(); i++)
{
modifiedpipe.descSets[i].pipeLayout = GetResID(pipeLayout);
modifiedpipe.descSets[i].descSet = GetResID(descSets[i]);
}
}
{
+21 -14
View File
@@ -65,10 +65,10 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback
VulkanRenderState &pipestate = m_pDriver->GetRenderState();
// check cache first
rdcpair<uint32_t, VkPipeline> pipe = m_PipelineCache[pipestate.graphics.pipeline];
CachedPipeline pipe = m_PipelineCache[pipestate.graphics.pipeline];
// if we don't get a hit, create a modified pipeline
if(pipe.second == VK_NULL_HANDLE)
if(pipe.pipe == VK_NULL_HANDLE)
{
VulkanCreationInfo &c = *pipestate.m_CreationInfo;
@@ -101,9 +101,8 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback
};
// create pipeline layout with same descriptor set layouts, plus our mesh output set
VkPipelineLayout pipeLayout;
vkr =
m_pDriver->vkCreatePipelineLayout(m_pDriver->GetDev(), &pipeLayoutInfo, NULL, &pipeLayout);
vkr = m_pDriver->vkCreatePipelineLayout(m_pDriver->GetDev(), &pipeLayoutInfo, NULL,
&pipe.pipeLayout);
RDCASSERTEQUAL(vkr, VK_SUCCESS);
SAFE_DELETE_ARRAY(descSetLayouts);
@@ -113,7 +112,7 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback
pipestate.graphics.pipeline);
// repoint pipeline layout
pipeCreateInfo.layout = pipeLayout;
pipeCreateInfo.layout = pipe.pipeLayout;
// disable colour writes/blends
VkPipelineColorBlendStateCreateInfo *cb =
@@ -203,21 +202,22 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback
}
vkr = m_pDriver->vkCreateGraphicsPipelines(dev, VK_NULL_HANDLE, 1, &pipeCreateInfo, NULL,
&pipe.second);
&pipe.pipe);
RDCASSERTEQUAL(vkr, VK_SUCCESS);
m_pDriver->vkDestroyShaderModule(dev, module, NULL);
pipe.first = descSet;
pipe.descSet = descSet;
m_PipelineCache[pipestate.graphics.pipeline] = pipe;
}
// modify state for first draw call
pipestate.graphics.pipeline = GetResID(pipe.second);
RDCASSERT(pipestate.graphics.descSets.size() >= pipe.first);
pipestate.graphics.descSets.resize(pipe.first + 1);
pipestate.graphics.descSets[pipe.first].descSet = GetResID(m_DescSet);
pipestate.graphics.pipeline = GetResID(pipe.pipe);
RDCASSERT(pipestate.graphics.descSets.size() >= pipe.descSet);
pipestate.graphics.descSets.resize(pipe.descSet + 1);
pipestate.graphics.descSets[pipe.descSet].pipeLayout = GetResID(pipe.pipeLayout);
pipestate.graphics.descSets[pipe.descSet].descSet = GetResID(m_DescSet);
if(cmd)
pipestate.BindPipeline(cmd, VulkanRenderState::BindGraphics, false);
@@ -262,7 +262,13 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback
const std::vector<uint32_t> &m_Events;
// cache modified pipelines
std::map<ResourceId, rdcpair<uint32_t, VkPipeline> > m_PipelineCache;
struct CachedPipeline
{
uint32_t descSet;
VkPipelineLayout pipeLayout;
VkPipeline pipe;
};
std::map<ResourceId, CachedPipeline> m_PipelineCache;
VulkanRenderState m_PrevState;
};
@@ -1878,7 +1884,8 @@ ResourceId VulkanReplay::RenderOverlay(ResourceId texid, CompType typeHint, Floa
for(auto it = cb.m_PipelineCache.begin(); it != cb.m_PipelineCache.end(); ++it)
{
m_pDriver->vkDestroyPipeline(m_Device, it->second.second, NULL);
m_pDriver->vkDestroyPipeline(m_Device, it->second.pipe, NULL);
m_pDriver->vkDestroyPipelineLayout(m_Device, it->second.pipeLayout, NULL);
}
}
+24 -10
View File
@@ -1217,7 +1217,7 @@ void VulkanReplay::PatchReservedDescriptors(const VulkanStatePipeline &pipe,
for(size_t i = 0; i < newBindingsCount; i++)
poolSizes[newBindings[i].descriptorType].descriptorCount += newBindings[i].descriptorCount;
const std::vector<ResourceId> &descSetLayoutIds =
const std::vector<ResourceId> &pipeDescSetLayouts =
creationInfo.m_PipelineLayout[pipeInfo.layout].descSetLayouts;
// need to add our added bindings to the first descriptor set
@@ -1226,7 +1226,7 @@ void VulkanReplay::PatchReservedDescriptors(const VulkanStatePipeline &pipe,
// if there are fewer sets bound than were declared in the pipeline layout, only process the
// bound sets (as otherwise we'd fail to copy from them). Assume the application knew what it
// was doing and the other sets are statically unused.
setLayouts.resize(RDCMIN(pipe.descSets.size(), descSetLayoutIds.size()));
setLayouts.resize(RDCMIN(pipe.descSets.size(), pipeDescSetLayouts.size()));
// need at least one set, if the shader isn't using any we'll just make our own
if(setLayouts.empty())
@@ -1242,9 +1242,17 @@ void VulkanReplay::PatchReservedDescriptors(const VulkanStatePipeline &pipe,
// if the shader had no descriptor sets at all, i will be invalid, so just skip and add a set
// with only our own bindings.
if(i < descSetLayoutIds.size())
if(i < pipeDescSetLayouts.size() && i < pipe.descSets.size())
{
const DescSetLayout &origLayout = creationInfo.m_DescSetLayout[descSetLayoutIds[i]];
// use the descriptor set layout from when it was bound. If the pipeline layout declared a
// descriptor set layout for this set, but it's statically unused, it may be complete
// garbage and doesn't match what the shader uses. However the pipeline layout at descriptor
// set bind time must have been compatible and valid so we can use it. If this set *is* used
// then the pipeline layout at bind time must be compatible with the pipeline's pipeline
// layout, so we're fine too.
const DescSetLayout &origLayout =
creationInfo.m_DescSetLayout[creationInfo.m_PipelineLayout[pipe.descSets[i].pipeLayout]
.descSetLayouts[i]];
for(size_t b = 0; b < origLayout.bindings.size(); b++)
{
@@ -1338,16 +1346,19 @@ void VulkanReplay::PatchReservedDescriptors(const VulkanStatePipeline &pipe,
m_pDriver->vkAllocateDescriptorSets(dev, &descSetAllocInfo, descSets.data());
// copy the data across from the real descriptors into our adjusted bindings
for(size_t i = 0; i < descSetLayoutIds.size(); i++)
for(size_t i = 0; i < setLayouts.size(); i++)
{
const DescSetLayout &origLayout = creationInfo.m_DescSetLayout[descSetLayoutIds[i]];
if(i >= pipe.descSets.size())
continue;
if(pipe.descSets[i].descSet == ResourceId())
continue;
// as above we use the pipeline layout that was originally used to bind this descriptor set
// and not the pipeline layout from the pipeline, in case the pipeline statically doesn't use
// this set and so its descriptor set layout is garbage (doesn't match the actual bound
// descriptor set)
const DescSetLayout &origLayout =
creationInfo.m_DescSetLayout[creationInfo.m_PipelineLayout[pipe.descSets[i].pipeLayout]
.descSetLayouts[i]];
WrappedVulkan::DescriptorSetInfo &setInfo =
m_pDriver->m_DescriptorSetState[pipe.descSets[i].descSet];
@@ -2299,7 +2310,10 @@ void VulkanReplay::FetchVSOut(uint32_t eventId)
modifiedstate.compute.descSets.resize(1);
for(size_t i = 0; i < descSets.size(); i++)
{
modifiedstate.compute.descSets[i].pipeLayout = GetResID(pipeLayout);
modifiedstate.compute.descSets[i].descSet = GetResID(descSets[i]);
}
{
// create buffer of sufficient size
+8 -4
View File
@@ -269,7 +269,7 @@ void VulkanRenderState::BindPipeline(VkCommandBuffer cmd, PipelineBinding bindin
}
}
BindDescriptorSet(descLayout, cmd, layout, VK_PIPELINE_BIND_POINT_GRAPHICS, (uint32_t)i,
BindDescriptorSet(descLayout, cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, (uint32_t)i,
dynamicOffsets);
if(graphics.descSets[i].offsets.size() < descLayout.dynamicCount)
@@ -386,7 +386,7 @@ void VulkanRenderState::BindPipeline(VkCommandBuffer cmd, PipelineBinding bindin
}
}
BindDescriptorSet(descLayout, cmd, layout, VK_PIPELINE_BIND_POINT_COMPUTE, (uint32_t)i,
BindDescriptorSet(descLayout, cmd, VK_PIPELINE_BIND_POINT_COMPUTE, (uint32_t)i,
dynamicOffsets);
if(compute.descSets[i].offsets.size() < descLayout.dynamicCount)
@@ -397,12 +397,16 @@ void VulkanRenderState::BindPipeline(VkCommandBuffer cmd, PipelineBinding bindin
}
void VulkanRenderState::BindDescriptorSet(const DescSetLayout &descLayout, VkCommandBuffer cmd,
VkPipelineLayout layout, VkPipelineBindPoint bindPoint,
uint32_t setIndex, uint32_t *dynamicOffsets)
VkPipelineBindPoint bindPoint, uint32_t setIndex,
uint32_t *dynamicOffsets)
{
ResourceId descSet = (bindPoint == VK_PIPELINE_BIND_POINT_GRAPHICS)
? graphics.descSets[setIndex].descSet
: compute.descSets[setIndex].descSet;
ResourceId pipeLayout = (bindPoint == VK_PIPELINE_BIND_POINT_GRAPHICS)
? graphics.descSets[setIndex].pipeLayout
: compute.descSets[setIndex].pipeLayout;
VkPipelineLayout layout = GetResourceManager()->GetCurrentHandle<VkPipelineLayout>(pipeLayout);
if((descLayout.flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR) == 0)
{
+2 -2
View File
@@ -38,6 +38,7 @@ struct VulkanStatePipeline
struct DescriptorAndOffsets
{
ResourceId pipeLayout;
ResourceId descSet;
std::vector<uint32_t> offsets;
};
@@ -63,8 +64,7 @@ struct VulkanRenderState
void BindPipeline(VkCommandBuffer cmd, PipelineBinding binding, bool subpass0);
void BindDescriptorSet(const DescSetLayout &descLayout, VkCommandBuffer cmd,
VkPipelineLayout layout, VkPipelineBindPoint bindPoint, uint32_t setIndex,
uint32_t *dynamicOffsets);
VkPipelineBindPoint bindPoint, uint32_t setIndex, uint32_t *dynamicOffsets);
bool IsConditionalRenderingEnabled();
@@ -2182,6 +2182,7 @@ bool WrappedVulkan::Serialise_vkCmdBindDescriptorSets(
// consume the offsets linearly along the descriptor set layouts
for(uint32_t i = 0; i < setCount; i++)
{
descsets[firstSet + i].pipeLayout = GetResID(layout);
descsets[firstSet + i].descSet = GetResID(pDescriptorSets[i]);
uint32_t dynCount =
m_CreationInfo.m_DescSetLayout[descSetLayouts[firstSet + i]].dynamicCount;
@@ -3804,6 +3805,7 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSetKHR(SerialiserType &ser,
if(descsets.size() < set + 1)
descsets.resize(set + 1);
descsets[set].pipeLayout = GetResID(layout);
descsets[set].descSet = setId;
}
@@ -4071,6 +4073,7 @@ bool WrappedVulkan::Serialise_vkCmdPushDescriptorSetWithTemplateKHR(
if(descsets.size() < set + 1)
descsets.resize(set + 1);
descsets[set].pipeLayout = GetResID(layout);
descsets[set].descSet = setId;
}