diff --git a/renderdoc/driver/vulkan/vk_common.cpp b/renderdoc/driver/vulkan/vk_common.cpp index fc8f64ff0..5faf4f34f 100644 --- a/renderdoc/driver/vulkan/vk_common.cpp +++ b/renderdoc/driver/vulkan/vk_common.cpp @@ -1012,6 +1012,12 @@ VkDriverInfo::VkDriverInfo(const VkPhysicalDeviceProperties &physProps, bool act RDCLOG("Disabling buffer_device_address on Intel - update to a newer driver for fix"); bdaBrokenDriver = true; } + + // Currently unfixed at the time of writing, Intel's drivers require manually inserted + // side-effects for occlusion queries to function properly if there are no other effects from a + // pixel shader. + // Only affects windows drivers, linux drivers are unaffected. + intelBrokenOcclusionQueries = true; } #endif diff --git a/renderdoc/driver/vulkan/vk_common.h b/renderdoc/driver/vulkan/vk_common.h index 64f812ba0..c2b9b752a 100644 --- a/renderdoc/driver/vulkan/vk_common.h +++ b/renderdoc/driver/vulkan/vk_common.h @@ -280,6 +280,9 @@ public: // hit the case where it's necessary (doing 'whole pass' partial replay of a subsection of a // command buffer where we need to apply dynamic state from earlier in the command buffer). bool QualcommLineWidthDynamicStateCrash() const { return qualcommLineWidthCrash; } + // on Intel, occlusion queries are broken unless the shader has some effects. When we don't want + // it to have visible effects during pixel history we have to insert some manual side-effects + bool IntelBrokenOcclusionQueries() const { return intelBrokenOcclusionQueries; } private: GPUVendor m_Vendor; @@ -293,6 +296,7 @@ private: bool qualcommLeakingUBOOffsets = false; bool qualcommDrefNon2DCompileCrash = false; bool qualcommLineWidthCrash = false; + bool intelBrokenOcclusionQueries = false; }; enum diff --git a/renderdoc/driver/vulkan/vk_debug.h b/renderdoc/driver/vulkan/vk_debug.h index 30b12c15c..3f2a883bd 100644 --- a/renderdoc/driver/vulkan/vk_debug.h +++ b/renderdoc/driver/vulkan/vk_debug.h @@ -90,7 +90,6 @@ public: const MeshFormat &primary, const MeshFormat &secondary); - void PatchOutputLocation(VkShaderModule &mod, BuiltinShader shaderType, uint32_t framebufferIndex); void PatchFixedColShader(VkShaderModule &mod, float col[4]); void PatchLineStripIndexBuffer(const ActionDescription *action, GPUBuffer &indexBuffer, uint32_t &indexCount); diff --git a/renderdoc/driver/vulkan/vk_overlay.cpp b/renderdoc/driver/vulkan/vk_overlay.cpp index bd1e3c2bf..5d6c8bce2 100644 --- a/renderdoc/driver/vulkan/vk_overlay.cpp +++ b/renderdoc/driver/vulkan/vk_overlay.cpp @@ -293,55 +293,6 @@ struct VulkanQuadOverdrawCallback : public VulkanActionCallback VulkanRenderState m_PrevState; }; -void VulkanDebugManager::PatchOutputLocation(VkShaderModule &mod, BuiltinShader shaderType, - uint32_t framebufferIndex) -{ - union - { - uint32_t *spirv; - float *data; - } alias; - - rdcarray spv = *m_pDriver->GetShaderCache()->GetBuiltinBlob(shaderType); - - alias.spirv = &spv[0]; - size_t spirvLength = spv.size(); - - bool patched = false; - - size_t it = 5; - while(it < spirvLength) - { - uint16_t WordCount = alias.spirv[it] >> rdcspv::WordCountShift; - rdcspv::Op opcode = rdcspv::Op(alias.spirv[it] & rdcspv::OpCodeMask); - - if(opcode == rdcspv::Op::Decorate && - (rdcspv::Decoration(alias.spirv[it + 2]) == rdcspv::Decoration::Location)) - { - alias.spirv[it + 3] = framebufferIndex; - - patched = true; - break; - } - - it += WordCount; - } - - if(!patched) - RDCERR("Didn't patch the output location"); - - VkShaderModuleCreateInfo modinfo = { - VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO, - NULL, - 0, - spv.size() * sizeof(uint32_t), - alias.spirv, - }; - - VkResult vkr = m_pDriver->vkCreateShaderModule(m_Device, &modinfo, NULL, &mod); - CheckVkResult(vkr); -} - void VulkanDebugManager::PatchFixedColShader(VkShaderModule &mod, float col[4]) { union diff --git a/renderdoc/driver/vulkan/vk_pixelhistory.cpp b/renderdoc/driver/vulkan/vk_pixelhistory.cpp index d8aa8682b..6bdfa358b 100644 --- a/renderdoc/driver/vulkan/vk_pixelhistory.cpp +++ b/renderdoc/driver/vulkan/vk_pixelhistory.cpp @@ -239,9 +239,31 @@ struct PipelineReplacements // PixelHistoryShaderCache manages temporary shaders created for pixel history. struct PixelHistoryShaderCache { - PixelHistoryShaderCache(WrappedVulkan *vk) : m_pDriver(vk) {} + PixelHistoryShaderCache(WrappedVulkan *vk) : m_pDriver(vk) + { + if(m_pDriver->GetDriverInfo().IntelBrokenOcclusionQueries()) + { + if(m_pDriver->GetExtensions(NULL).ext_KHR_buffer_device_address) + { + dummybuf.Create(vk, vk->GetDev(), 1024, 1, GPUBuffer::eGPUBufferGPULocal | + GPUBuffer::eGPUBufferSSBO | + GPUBuffer::eGPUBufferAddressable); + } + else + { + m_pDriver->AddDebugMessage( + MessageCategory::Miscellaneous, MessageSeverity::High, MessageSource::RuntimeWarning, + "Intel drivers currently require a workaround to return proper pixel history results, " + "but KHR_buffer_device_address is not supported so the workaround cannot be " + "implemented. Results will be inaccurate."); + } + } + } + ~PixelHistoryShaderCache() { + if(dummybuf.device != VK_NULL_HANDLE) + dummybuf.Destroy(); for(auto it = m_ShaderReplacements.begin(); it != m_ShaderReplacements.end(); ++it) { if(it->second != VK_NULL_HANDLE) @@ -261,8 +283,7 @@ struct PixelHistoryShaderCache if(it != m_FixedColFS.end()) return it->second; VkShaderModule sh; - m_pDriver->GetDebugManager()->PatchOutputLocation(sh, BuiltinShader::FixedColFS, - framebufferIndex); + PatchOutputLocation(sh, BuiltinShader::FixedColFS, framebufferIndex); m_FixedColFS.insert(std::make_pair(framebufferIndex, sh)); return sh; } @@ -275,8 +296,7 @@ struct PixelHistoryShaderCache if(it != m_PrimIDFS.end()) return it->second; VkShaderModule sh; - m_pDriver->GetDebugManager()->PatchOutputLocation(sh, BuiltinShader::PixelHistoryPrimIDFS, - framebufferIndex); + PatchOutputLocation(sh, BuiltinShader::PixelHistoryPrimIDFS, framebufferIndex); m_PrimIDFS.insert(std::make_pair(framebufferIndex, sh)); return sh; } @@ -305,35 +325,166 @@ private: const VulkanCreationInfo::ShaderModule &moduleInfo = m_pDriver->GetDebugManager()->GetShaderInfo(shaderId); rdcarray modSpirv = moduleInfo.spirv.GetSPIRV(); - rdcspv::Editor editor(modSpirv); - editor.Prepare(); - for(const rdcspv::EntryPoint &entry : editor.GetEntries()) + bool modified = false; + bool found = false; + { - if(entry.name == entryName && MakeShaderStage(entry.executionModel) == stage) + rdcspv::Editor editor(modSpirv); + editor.Prepare(); + + for(const rdcspv::EntryPoint &entry : editor.GetEntries()) { - // In some cases a shader might just be binding a RW resource but not writing to it. - // If there are no writes (shader was not modified), no need to replace the shader, - // just insert VK_NULL_HANDLE to indicate that this shader has been processed. - VkShaderModule module = VK_NULL_HANDLE; - bool modified = StripShaderSideEffects(editor, entry.id); - if(modified) + if(entry.name == entryName && MakeShaderStage(entry.executionModel) == stage) { - VkShaderModuleCreateInfo moduleCreateInfo = {}; - moduleCreateInfo.sType = VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO; - moduleCreateInfo.pCode = modSpirv.data(); - moduleCreateInfo.codeSize = modSpirv.byteSize(); - VkResult vkr = - m_pDriver->vkCreateShaderModule(m_pDriver->GetDev(), &moduleCreateInfo, NULL, &module); - m_pDriver->CheckVkResult(vkr); + // In some cases a shader might just be binding a RW resource but not writing to it. + // If there are no writes (shader was not modified), no need to replace the shader, + // just insert VK_NULL_HANDLE to indicate that this shader has been processed. + found = true; + modified = StripShaderSideEffects(editor, entry.id); + break; } - return module; } } + + if(found) + { + VkShaderModule module = VK_NULL_HANDLE; + if(modified) + { + VkShaderModuleCreateInfo moduleCreateInfo = {}; + moduleCreateInfo.sType = VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO; + moduleCreateInfo.pCode = modSpirv.data(); + moduleCreateInfo.codeSize = modSpirv.byteSize(); + VkResult vkr = + m_pDriver->vkCreateShaderModule(m_pDriver->GetDev(), &moduleCreateInfo, NULL, &module); + m_pDriver->CheckVkResult(vkr); + } + + return module; + } + RDCERR("Entry point %s not found", entryName.c_str()); return VK_NULL_HANDLE; } + void PatchOutputLocation(VkShaderModule &mod, BuiltinShader shaderType, uint32_t framebufferIndex) + { + rdcarray spv = *m_pDriver->GetShaderCache()->GetBuiltinBlob(shaderType); + + bool patched = false; + + { + rdcspv::Editor editor(spv); + editor.Prepare(); + + for(rdcspv::Iter it = editor.Begin(rdcspv::Section::Annotations), + end = editor.End(rdcspv::Section::Annotations); + it < end; ++it) + { + if(it.opcode() == rdcspv::Op::Decorate) + { + rdcspv::OpDecorate dec(it); + if(dec.decoration == rdcspv::Decoration::Location) + { + dec.decoration.location = framebufferIndex; + it = dec; + + patched = true; + break; + } + } + } + + // implement workaround for Intel drivers to force shader to have unimportant side-effects + if(dummybuf.device != VK_NULL_HANDLE) + { + VkBufferDeviceAddressInfo getAddressInfo = {VK_STRUCTURE_TYPE_BUFFER_DEVICE_ADDRESS_INFO}; + getAddressInfo.buffer = dummybuf.buf; + + VkDeviceAddress bufferAddress = + m_pDriver->vkGetBufferDeviceAddress(m_pDriver->GetDev(), &getAddressInfo); + + rdcspv::Id uint32Type = editor.DeclareType(rdcspv::scalar()); + rdcspv::Id bufptrtype = editor.DeclareType( + rdcspv::Pointer(uint32Type, rdcspv::StorageClass::PhysicalStorageBuffer)); + + rdcspv::Id uint1 = editor.AddConstantImmediate(uint32_t(1)); + + editor.AddExtension("SPV_KHR_physical_storage_buffer"); + + { + // change the memory model to physical storage buffer 64 + rdcspv::Iter it = editor.Begin(rdcspv::Section::MemoryModel); + rdcspv::OpMemoryModel model(it); + model.addressingModel = rdcspv::AddressingModel::PhysicalStorageBuffer64; + it = model; + } + + editor.AddCapability(rdcspv::Capability::PhysicalStorageBufferAddresses); + + rdcspv::Id addressConstantLSB = + editor.AddConstantImmediate(bufferAddress & 0xFFFFFFFF); + rdcspv::Id addressConstantMSB = + editor.AddConstantImmediate((bufferAddress >> 32) & 0xFFFFFFFF); + + rdcspv::Id uintPair = editor.DeclareType(rdcspv::Vector(rdcspv::scalar(), 2)); + + rdcspv::Id addressConstant = editor.AddConstant(rdcspv::OpSpecConstantComposite( + uintPair, editor.MakeId(), {addressConstantLSB, addressConstantMSB})); + + rdcspv::Id scope = editor.AddConstantImmediate((uint32_t)rdcspv::Scope::Device); + rdcspv::Id semantics = + editor.AddConstantImmediate((uint32_t)rdcspv::MemorySemantics::AcquireRelease); + + // patch every function to include a BDA write just to be safe + for(rdcspv::Iter it = editor.Begin(rdcspv::Section::Functions), + end = editor.End(rdcspv::Section::Functions); + it < end; ++it) + { + if(it.opcode() == rdcspv::Op::Function) + { + // continue to the first label so we can insert a write at the start of the function + for(; it; ++it) + { + if(it.opcode() == rdcspv::Op::Label) + { + ++it; + break; + } + } + + // skip past any local variables + while(it.opcode() == rdcspv::Op::Variable) + ++it; + + rdcspv::Id structPtr = editor.AddOperation( + it, rdcspv::OpBitcast(bufptrtype, editor.MakeId(), addressConstant)); + it++; + + editor.AddOperation(it, rdcspv::OpAtomicUMax(uint32Type, editor.MakeId(), structPtr, + scope, semantics, uint1)); + it++; + } + } + } + } + + if(!patched) + RDCERR("Didn't patch the output location"); + + VkShaderModuleCreateInfo modinfo = { + VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO, + NULL, + 0, + spv.size() * sizeof(uint32_t), + spv.data(), + }; + + VkResult vkr = m_pDriver->vkCreateShaderModule(m_pDriver->GetDev(), &modinfo, NULL, &mod); + m_pDriver->CheckVkResult(vkr); + } + // Removes instructions from the shader that would produce side effects (writing // to storage buffers, or images). Returns true if the shader was modified, and // false if there were no instructions to remove. @@ -444,6 +595,8 @@ private: // ShaderKey consists of original shader module ID and entry point name. typedef rdcpair ShaderKey; std::map m_ShaderReplacements; + + GPUBuffer dummybuf; }; // VulkanPixelHistoryCallback is a generic VulkanActionCallback that can be used for