diff --git a/renderdoc/driver/vulkan/vk_bindless_feedback.cpp b/renderdoc/driver/vulkan/vk_bindless_feedback.cpp index 6b7391679..873cd0851 100644 --- a/renderdoc/driver/vulkan/vk_bindless_feedback.cpp +++ b/renderdoc/driver/vulkan/vk_bindless_feedback.cpp @@ -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]); + } } { diff --git a/renderdoc/driver/vulkan/vk_overlay.cpp b/renderdoc/driver/vulkan/vk_overlay.cpp index 0b73ece6c..920b3ce38 100644 --- a/renderdoc/driver/vulkan/vk_overlay.cpp +++ b/renderdoc/driver/vulkan/vk_overlay.cpp @@ -65,10 +65,10 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback VulkanRenderState &pipestate = m_pDriver->GetRenderState(); // check cache first - rdcpair 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 &m_Events; // cache modified pipelines - std::map > m_PipelineCache; + struct CachedPipeline + { + uint32_t descSet; + VkPipelineLayout pipeLayout; + VkPipeline pipe; + }; + std::map 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); } } diff --git a/renderdoc/driver/vulkan/vk_postvs.cpp b/renderdoc/driver/vulkan/vk_postvs.cpp index 898f49e13..b3829f557 100644 --- a/renderdoc/driver/vulkan/vk_postvs.cpp +++ b/renderdoc/driver/vulkan/vk_postvs.cpp @@ -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 &descSetLayoutIds = + const std::vector &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 diff --git a/renderdoc/driver/vulkan/vk_state.cpp b/renderdoc/driver/vulkan/vk_state.cpp index 021354ca7..eb4edc7d3 100644 --- a/renderdoc/driver/vulkan/vk_state.cpp +++ b/renderdoc/driver/vulkan/vk_state.cpp @@ -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(pipeLayout); if((descLayout.flags & VK_DESCRIPTOR_SET_LAYOUT_CREATE_PUSH_DESCRIPTOR_BIT_KHR) == 0) { diff --git a/renderdoc/driver/vulkan/vk_state.h b/renderdoc/driver/vulkan/vk_state.h index 5dbed763b..c47f5e849 100644 --- a/renderdoc/driver/vulkan/vk_state.h +++ b/renderdoc/driver/vulkan/vk_state.h @@ -38,6 +38,7 @@ struct VulkanStatePipeline struct DescriptorAndOffsets { + ResourceId pipeLayout; ResourceId descSet; std::vector 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(); diff --git a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp index 818213913..55442813c 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp @@ -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; }