diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index c9d0f2cb6..bf3ab20ef 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -256,7 +256,7 @@ void VkInitParams::Set(const VkInstanceCreateInfo *pCreateInfo, ResourceId inst) InstanceID = inst; } -WrappedVulkan::WrappedVulkan(const char *logFilename) : m_RenderState(&m_CreationInfo) +WrappedVulkan::WrappedVulkan(const char *logFilename) : m_RenderState(this, &m_CreationInfo) { #if ENABLED(RDOC_RELEASE) const bool debugSerialiser = false; @@ -317,7 +317,6 @@ WrappedVulkan::WrappedVulkan(const char *logFilename) : m_RenderState(&m_Creatio m_DebugManager = NULL; m_pSerialiser->SetUserData(m_ResourceManager); - m_RenderState.m_ResourceManager = GetResourceManager(); m_Instance = VK_NULL_HANDLE; m_PhysicalDevice = VK_NULL_HANDLE; @@ -2361,8 +2360,7 @@ void WrappedVulkan::ReplayLog(uint32_t startEventID, uint32_t endEventID, Replay RDCASSERT(m_Partial[Secondary].resultPartialCmdBuffer == VK_NULL_HANDLE); m_Partial[Primary].Reset(); m_Partial[Secondary].Reset(); - m_RenderState = VulkanRenderState(&m_CreationInfo); - m_RenderState.m_ResourceManager = GetResourceManager(); + m_RenderState = VulkanRenderState(this, &m_CreationInfo); } VkResult vkr = VK_SUCCESS; @@ -2414,13 +2412,32 @@ void WrappedVulkan::ReplayLog(uint32_t startEventID, uint32_t endEventID, Replay VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, false, 0, NULL, 0, NULL, (uint32_t)imgBarriers.size(), &imgBarriers[0]); + const DrawcallDescription *draw = GetDrawcall(endEventID); + + bool rpUnneeded = false; + + // if we're only replaying a draw, and it's not a drawcall or dispatch, don't try and bind + // all the replay state as we don't know if it will be valid. + if(replayType == eReplay_OnlyDraw) + { + if(!draw) + { + rpUnneeded = true; + } + else if(!(draw->flags & (DrawFlags::Drawcall | DrawFlags::Dispatch))) + { + rpUnneeded = true; + } + } + // if a render pass was active, begin it and set up the partial replay state - m_RenderState.BeginRenderPassAndApplyState(cmd); + m_RenderState.BeginRenderPassAndApplyState( + cmd, rpUnneeded ? VulkanRenderState::BindNone : VulkanRenderState::BindGraphics); } else if(m_RenderState.compute.pipeline != ResourceId()) { // if we had a compute pipeline, need to bind that - m_RenderState.BindPipeline(cmd); + m_RenderState.BindPipeline(cmd, VulkanRenderState::BindCompute); } } diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index e0d2c39fd..e59f7a7d6 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -714,6 +714,11 @@ public: uint32_t GetMaxEID() { return m_Events.back().eventID; } const DrawcallDescription *GetDrawcall(uint32_t eventID); + ResourceId GetDescLayoutForDescSet(ResourceId descSet) + { + return m_DescriptorSetState[descSet].layout; + } + vector GetUsage(ResourceId id) { return m_ResourceUses[id]; } // return the pre-selected device and queue VkDevice GetDev() diff --git a/renderdoc/driver/vulkan/vk_debug.cpp b/renderdoc/driver/vulkan/vk_debug.cpp index 6646c8215..1457d8efc 100644 --- a/renderdoc/driver/vulkan/vk_debug.cpp +++ b/renderdoc/driver/vulkan/vk_debug.cpp @@ -4771,7 +4771,7 @@ void VulkanDebugManager::PatchFixedColShader(VkShaderModule &mod, float col[4]) struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback { VulkanQuadOverdrawCallback(WrappedVulkan *vk, const vector &events) - : m_pDriver(vk), m_pDebug(vk->GetDebugManager()), m_Events(events), m_PrevState(NULL) + : m_pDriver(vk), m_pDebug(vk->GetDebugManager()), m_Events(events), m_PrevState(vk, NULL) { m_pDriver->SetDrawcallCB(this); } @@ -4951,7 +4951,7 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback pipestate.graphics.descSets[pipe.first].descSet = GetResID(m_pDebug->m_QuadDescSet); if(cmd) - pipestate.BindPipeline(cmd); + pipestate.BindPipeline(cmd, VulkanRenderState::BindGraphics); } bool PostDraw(uint32_t eid, VkCommandBuffer cmd) @@ -4963,7 +4963,7 @@ struct VulkanQuadOverdrawCallback : public VulkanDrawcallCallback m_pDriver->GetRenderState() = m_PrevState; RDCASSERT(cmd); - m_pDriver->GetRenderState().BindPipeline(cmd); + m_pDriver->GetRenderState().BindPipeline(cmd, VulkanRenderState::BindGraphics); return true; } @@ -6057,7 +6057,7 @@ ResourceId VulkanDebugManager::RenderOverlay(ResourceId texid, DebugOverlay over vkr = vt->BeginCommandBuffer(Unwrap(cmd), &beginInfo); RDCASSERTEQUAL(vkr, VK_SUCCESS); - m_pDriver->m_RenderState.BeginRenderPassAndApplyState(cmd); + m_pDriver->m_RenderState.BeginRenderPassAndApplyState(cmd, VulkanRenderState::BindGraphics); VkClearAttachment blackclear = {VK_IMAGE_ASPECT_COLOR_BIT, 0, {}}; vector atts; @@ -7084,7 +7084,7 @@ inline uint32_t MakeSPIRVOp(spv::Op op, uint32_t WordCount) } static void AddOutputDumping(const ShaderReflection &refl, const SPIRVPatchData &patchData, - const char *entryName, uint32_t descSet, uint32_t vertexIndexOffset, + const char *entryName, uint32_t &descSet, uint32_t vertexIndexOffset, uint32_t instanceIndexOffset, uint32_t numVerts, vector &modSpirv, uint32_t &bufStride) { @@ -7147,12 +7147,20 @@ static void AddOutputDumping(const ShaderReflection &refl, const SPIRVPatchData size_t decorateOffset = 0; size_t typeVarOffset = 0; + descSet = 0; + size_t it = 5; while(it < spirvLength) { uint16_t WordCount = spirv[it] >> spv::WordCountShift; spv::Op opcode = spv::Op(spirv[it] & spv::OpCodeMask); + // we will use the descriptor set immediately after the last set statically used by the shader. + // This means we don't have to worry about if the descriptor set layout declares more sets which + // might be invalid and un-bindable, we just trample over the next set that's unused + if(opcode == spv::OpDecorate && spirv[it + 2] == spv::DecorationDescriptorSet) + descSet = RDCMAX(descSet, spirv[it + 3] + 1); + if(opcode == spv::OpDecorate && spirv[it + 2] == spv::DecorationBuiltIn && spirv[it + 3] == spv::BuiltInVertexIndex) vertidxID = spirv[it + 1]; @@ -7907,52 +7915,26 @@ void VulkanDebugManager::InitPostVSBuffers(uint32_t eventID) if(drawcall == NULL || drawcall->numIndices == 0 || drawcall->numInstances == 0) return; - uint32_t descSet = (uint32_t)creationInfo.m_PipelineLayout[pipeInfo.layout].descSetLayouts.size(); + // the SPIR-V patching will determine the next descriptor set to use, after all sets statically + // used by the shader. This gets around the problem where the shader only uses 0 and 1, but the + // layout declares 0-4, and 2,3,4 are invalid at bind time and we are unable to bind our new set + // 5. Instead we'll notice that only 0 and 1 are used and just use 2 ourselves (although it was in + // the original set layout, we know it's statically unused by the shader so we can safely steal + // it). + uint32_t descSet = 0; // we go through the driver for all these creations since they need to be properly // registered in order to be put in the partial replay state VkResult vkr = VK_SUCCESS; VkDevice dev = m_Device; - VkDescriptorSetLayout *descSetLayouts; - - // descSet will be the index of our new descriptor set - descSetLayouts = new VkDescriptorSetLayout[descSet + 1]; - - for(uint32_t i = 0; i < descSet; i++) - descSetLayouts[i] = GetResourceManager()->GetCurrentHandle( - creationInfo.m_PipelineLayout[pipeInfo.layout].descSetLayouts[i]); - - // this layout just says it has one storage buffer - descSetLayouts[descSet] = m_MeshFetchDescSetLayout; - - const vector &push = creationInfo.m_PipelineLayout[pipeInfo.layout].pushRanges; - - VkPipelineLayoutCreateInfo pipeLayoutInfo = { - VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO, - NULL, - 0, - descSet + 1, - descSetLayouts, - (uint32_t)push.size(), - push.empty() ? NULL : &push[0], - }; - - // create pipeline layout with same descriptor set layouts, plus our mesh output set VkPipelineLayout pipeLayout; - vkr = m_pDriver->vkCreatePipelineLayout(dev, &pipeLayoutInfo, NULL, &pipeLayout); - RDCASSERTEQUAL(vkr, VK_SUCCESS); - - SAFE_DELETE_ARRAY(descSetLayouts); VkGraphicsPipelineCreateInfo pipeCreateInfo; // get pipeline create info MakeGraphicsPipelineInfo(pipeCreateInfo, state.graphics.pipeline); - // repoint pipeline layout - pipeCreateInfo.layout = pipeLayout; - // set primitive topology to point list VkPipelineInputAssemblyStateCreateInfo *ia = (VkPipelineInputAssemblyStateCreateInfo *)pipeCreateInfo.pInputAssemblyState; @@ -8153,6 +8135,42 @@ void VulkanDebugManager::InitPostVSBuffers(uint32_t eventID) descSet, vertexIndexOffset, drawcall->instanceOffset, numVerts, modSpirv, bufStride); + { + VkDescriptorSetLayout *descSetLayouts; + + // descSet will be the index of our new descriptor set + descSetLayouts = new VkDescriptorSetLayout[descSet + 1]; + + for(uint32_t i = 0; i < descSet; i++) + descSetLayouts[i] = GetResourceManager()->GetCurrentHandle( + creationInfo.m_PipelineLayout[pipeInfo.layout].descSetLayouts[i]); + + // this layout just says it has one storage buffer + descSetLayouts[descSet] = m_MeshFetchDescSetLayout; + + const vector &push = + creationInfo.m_PipelineLayout[pipeInfo.layout].pushRanges; + + VkPipelineLayoutCreateInfo pipeLayoutInfo = { + VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO, + NULL, + 0, + descSet + 1, + descSetLayouts, + (uint32_t)push.size(), + push.empty() ? NULL : &push[0], + }; + + // create pipeline layout with same descriptor set layouts, plus our mesh output set + vkr = m_pDriver->vkCreatePipelineLayout(dev, &pipeLayoutInfo, NULL, &pipeLayout); + RDCASSERTEQUAL(vkr, VK_SUCCESS); + + SAFE_DELETE_ARRAY(descSetLayouts); + + // repoint pipeline layout + pipeCreateInfo.layout = pipeLayout; + } + // create vertex shader with modified code VkShaderModuleCreateInfo moduleCreateInfo = { VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO, NULL, 0, @@ -8265,7 +8283,7 @@ void VulkanDebugManager::InitPostVSBuffers(uint32_t eventID) RDCASSERTEQUAL(vkr, VK_SUCCESS); // do single draw - modifiedstate.BeginRenderPassAndApplyState(cmd); + modifiedstate.BeginRenderPassAndApplyState(cmd, VulkanRenderState::BindGraphics); ObjDisp(cmd)->CmdDraw(Unwrap(cmd), drawcall->numIndices, drawcall->numInstances, drawcall->vertexOffset, drawcall->instanceOffset); modifiedstate.EndRenderPass(cmd); @@ -8406,7 +8424,7 @@ void VulkanDebugManager::InitPostVSBuffers(uint32_t eventID) m_pDriver->vkUpdateDescriptorSets(dev, 1, &write, 0, NULL); // do single draw - modifiedstate.BeginRenderPassAndApplyState(cmd); + modifiedstate.BeginRenderPassAndApplyState(cmd, VulkanRenderState::BindGraphics); ObjDisp(cmd)->CmdDrawIndexed(Unwrap(cmd), (uint32_t)indices.size(), drawcall->numInstances, 0, drawcall->baseVertex, drawcall->instanceOffset); modifiedstate.EndRenderPass(cmd); diff --git a/renderdoc/driver/vulkan/vk_info.cpp b/renderdoc/driver/vulkan/vk_info.cpp index be0a7cdd3..db18e55e2 100644 --- a/renderdoc/driver/vulkan/vk_info.cpp +++ b/renderdoc/driver/vulkan/vk_info.cpp @@ -81,6 +81,45 @@ void DescSetLayout::CreateBindingsArray(vector &descBinding } } +bool DescSetLayout::operator==(const DescSetLayout &other) const +{ + // shortcut for equality to ourselves + if(this == &other) + return true; + + // descriptor set layouts are different if they have different set of bindings. + if(bindings.size() != other.bindings.size()) + return false; + + // iterate over each binding (we know this loop indexes validly in both arrays + for(size_t i = 0; i < bindings.size(); i++) + { + const Binding &a = bindings[i]; + const Binding &b = other.bindings[i]; + + // if the type/stages/count are different, the layout is different + if(a.descriptorCount != b.descriptorCount || a.descriptorType != b.descriptorType || + a.stageFlags != b.stageFlags) + return false; + + // if one has immutable samplers but the other doesn't, they're different + if((a.immutableSampler && !b.immutableSampler) || (!a.immutableSampler && b.immutableSampler)) + return false; + + // if we DO have immutable samplers, they must all point to the same sampler objects. + if(a.immutableSampler) + { + for(uint32_t s = 0; s < a.descriptorCount; s++) + { + if(a.immutableSampler[s] != b.immutableSampler[s]) + return false; + } + } + } + + return true; +} + void VulkanCreationInfo::Pipeline::Init(VulkanResourceManager *resourceMan, VulkanCreationInfo &info, const VkGraphicsPipelineCreateInfo *pCreateInfo) { diff --git a/renderdoc/driver/vulkan/vk_info.h b/renderdoc/driver/vulkan/vk_info.h index 7fb115e96..081105a01 100644 --- a/renderdoc/driver/vulkan/vk_info.h +++ b/renderdoc/driver/vulkan/vk_info.h @@ -71,6 +71,9 @@ struct DescSetLayout vector bindings; uint32_t dynamicCount; + + bool operator==(const DescSetLayout &other) const; + bool operator!=(const DescSetLayout &other) const { return !(*this == other); } }; struct VulkanCreationInfo diff --git a/renderdoc/driver/vulkan/vk_state.cpp b/renderdoc/driver/vulkan/vk_state.cpp index 06806af66..30d8bb0e2 100644 --- a/renderdoc/driver/vulkan/vk_state.cpp +++ b/renderdoc/driver/vulkan/vk_state.cpp @@ -23,10 +23,12 @@ ******************************************************************************/ #include "vk_state.h" +#include "vk_core.h" #include "vk_info.h" #include "vk_resources.h" -VulkanRenderState::VulkanRenderState(VulkanCreationInfo *createInfo) : m_CreationInfo(createInfo) +VulkanRenderState::VulkanRenderState(WrappedVulkan *driver, VulkanCreationInfo *createInfo) + : m_CreationInfo(createInfo), m_pDriver(driver) { compute.pipeline = graphics.pipeline = renderPass = framebuffer = ResourceId(); compute.descSets.clear(); @@ -43,8 +45,6 @@ VulkanRenderState::VulkanRenderState(VulkanCreationInfo *createInfo) : m_Creatio RDCEraseEl(back); RDCEraseEl(pushconsts); - m_ResourceManager = NULL; - renderPass = ResourceId(); subpass = 0; @@ -83,7 +83,7 @@ VulkanRenderState &VulkanRenderState::operator=(const VulkanRenderState &o) return *this; } -void VulkanRenderState::BeginRenderPassAndApplyState(VkCommandBuffer cmd) +void VulkanRenderState::BeginRenderPassAndApplyState(VkCommandBuffer cmd, PipelineBinding binding) { RDCASSERT(renderPass != ResourceId()); @@ -106,7 +106,7 @@ void VulkanRenderState::BeginRenderPassAndApplyState(VkCommandBuffer cmd) }; ObjDisp(cmd)->CmdBeginRenderPass(Unwrap(cmd), &rpbegin, VK_SUBPASS_CONTENTS_INLINE); - BindPipeline(cmd); + BindPipeline(cmd, binding); if(ibuffer.buf != ResourceId()) ObjDisp(cmd)->CmdBindIndexBuffer( @@ -120,9 +120,9 @@ void VulkanRenderState::BeginRenderPassAndApplyState(VkCommandBuffer cmd) &vbuffers[i].offs); } -void VulkanRenderState::BindPipeline(VkCommandBuffer cmd) +void VulkanRenderState::BindPipeline(VkCommandBuffer cmd, PipelineBinding binding) { - if(graphics.pipeline != ResourceId()) + if(graphics.pipeline != ResourceId() && binding == BindGraphics) { ObjDisp(cmd)->CmdBindPipeline( Unwrap(cmd), VK_PIPELINE_BIND_POINT_GRAPHICS, @@ -193,6 +193,26 @@ void VulkanRenderState::BindPipeline(VkCommandBuffer cmd) if(i < graphics.descSets.size() && graphics.descSets[i].descSet != ResourceId()) { + // if we come to a descriptor set that isn't compatible, stop setting descriptor sets from + // here on. + // We can get into this situation if for example we have many sets bound at some point, then + // there's a pipeline change that causes most or all of them to be invalidated as + // incompatible, then the program only re-binds some subset that it knows is statically used + // by the next drawcall. The remaining sets are invalid, but also unused and this is + // explicitly allowed by the spec. We just have to make sure we don't try to actively bind + // an incompatible descriptor set. + ResourceId createdDescSetLayoutId = + m_pDriver->GetDescLayoutForDescSet(graphics.descSets[i].descSet); + + if(descSetLayouts[i] != createdDescSetLayoutId) + { + const DescSetLayout &createdDescLayout = + m_CreationInfo->m_DescSetLayout[createdDescSetLayoutId]; + + if(descLayout != createdDescLayout) + break; + } + // if there are dynamic buffers, pass along the offsets uint32_t *dynamicOffsets = NULL; @@ -235,7 +255,7 @@ void VulkanRenderState::BindPipeline(VkCommandBuffer cmd) } } - if(compute.pipeline != ResourceId()) + if(compute.pipeline != ResourceId() && binding == BindCompute) { ObjDisp(cmd)->CmdBindPipeline( Unwrap(cmd), VK_PIPELINE_BIND_POINT_COMPUTE, @@ -305,3 +325,8 @@ void VulkanRenderState::EndRenderPass(VkCommandBuffer cmd) { ObjDisp(cmd)->CmdEndRenderPass(Unwrap(cmd)); } + +VulkanResourceManager *VulkanRenderState::GetResourceManager() +{ + return m_pDriver->GetResourceManager(); +} diff --git a/renderdoc/driver/vulkan/vk_state.h b/renderdoc/driver/vulkan/vk_state.h index 0d142b23c..5223ad177 100644 --- a/renderdoc/driver/vulkan/vk_state.h +++ b/renderdoc/driver/vulkan/vk_state.h @@ -29,14 +29,22 @@ struct VulkanCreationInfo; class VulkanResourceManager; +class WrappedVulkan; struct VulkanRenderState { - VulkanRenderState(VulkanCreationInfo *createInfo); + enum PipelineBinding + { + BindNone = 0x0, + BindGraphics = 0x1, + BindCompute = 0x2, + }; + + VulkanRenderState(WrappedVulkan *driver, VulkanCreationInfo *createInfo); VulkanRenderState &operator=(const VulkanRenderState &o); - void BeginRenderPassAndApplyState(VkCommandBuffer cmd); + void BeginRenderPassAndApplyState(VkCommandBuffer cmd, PipelineBinding binding); void EndRenderPass(VkCommandBuffer cmd); - void BindPipeline(VkCommandBuffer cmd); + void BindPipeline(VkCommandBuffer cmd, PipelineBinding binding); // dynamic state vector views; @@ -88,7 +96,7 @@ struct VulkanRenderState }; vector vbuffers; - VulkanResourceManager *GetResourceManager() { return m_ResourceManager; } - VulkanResourceManager *m_ResourceManager; + VulkanResourceManager *GetResourceManager(); VulkanCreationInfo *m_CreationInfo; + WrappedVulkan *m_pDriver; }; diff --git a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp index 9665b03cb..b5a859547 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp @@ -339,6 +339,8 @@ VkResult WrappedVulkan::vkAllocateDescriptorSets(VkDevice device, else { GetResourceManager()->AddLiveResource(id, pDescriptorSets[i]); + + m_DescriptorSetState[id].layout = GetResID(pAllocateInfo->pSetLayouts[i]); } }