From 9f3dfe7ebed5a8da71284a46946fd34effc49c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 24 Sep 2022 22:39:22 +0200 Subject: [PATCH] Vulkan: Don't compile pipeline variants that don't make sense given their flags. Ran into this with cache files from previous version of my change. Also bumping the shader cache ID again to avoid this in other ways, but good to be robust here. --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 10 ---------- Common/GPU/Vulkan/VulkanQueueRunner.h | 14 +++++++++++--- Common/GPU/Vulkan/VulkanRenderManager.cpp | 17 ++++++++++++++++- Common/GPU/Vulkan/VulkanRenderManager.h | 2 +- Common/GPU/Vulkan/thin3d_vulkan.cpp | 2 +- GPU/Vulkan/PipelineManagerVulkan.cpp | 2 +- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index ccbf910f77..abcbb90c90 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -10,16 +10,6 @@ using namespace PPSSPP_VK; // Debug help: adb logcat -s DEBUG PPSSPPNativeActivity PPSSPP NativeGLView NativeRenderer NativeSurfaceView PowerSaveModeReceiver InputDeviceState -static bool RenderPassTypeHasDepth(RenderPassType rpType) { - switch (rpType) { - case RP_TYPE_BACKBUFFER: - case RP_TYPE_COLOR_DEPTH: - case RP_TYPE_COLOR_DEPTH_INPUT: - return true; - } - return false; -} - static void MergeRenderAreaRectInto(VkRect2D *dest, VkRect2D &src) { if (dest->offset.x > src.offset.x) { dest->extent.width += (dest->offset.x - src.offset.x); diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.h b/Common/GPU/Vulkan/VulkanQueueRunner.h index c767e498fb..65654d01f5 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.h +++ b/Common/GPU/Vulkan/VulkanQueueRunner.h @@ -43,9 +43,9 @@ enum class VKRRenderCommand : uint8_t { enum class PipelineFlags { NONE = 0, - USES_BLEND_CONSTANT = (1 << 3), - USES_DEPTH_STENCIL = (1 << 4), // Reads or writes the depth or stencil buffers. - USES_INPUT_ATTACHMENT = (1 << 5), + USES_BLEND_CONSTANT = (1 << 1), + USES_DEPTH_STENCIL = (1 << 2), // Reads or writes the depth or stencil buffers. + USES_INPUT_ATTACHMENT = (1 << 3), }; ENUM_CLASS_BITOPS(PipelineFlags); @@ -65,6 +65,14 @@ enum RenderPassType { RP_TYPE_COUNT, }; +inline bool RenderPassTypeHasDepth(RenderPassType type) { + return type == RP_TYPE_BACKBUFFER || type == RP_TYPE_COLOR_DEPTH || type == RP_TYPE_COLOR_DEPTH_INPUT; +} + +inline bool RenderPassTypeHasInput(RenderPassType type) { + return type == RP_TYPE_COLOR_INPUT || type == RP_TYPE_COLOR_DEPTH_INPUT; +} + struct VkRenderData { VKRRenderCommand cmd; union { diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index a666a830de..138740a471 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -589,7 +589,7 @@ VkCommandBuffer VulkanRenderManager::GetInitCmd() { return frameData_[curFrame].GetInitCmd(vulkan_); } -VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, uint32_t variantBitmask, const char *tag) { +VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, const char *tag) { VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(); _dbg_assert_(desc->vertexShader); _dbg_assert_(desc->fragmentShader); @@ -615,6 +615,21 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe if (!(variantBitmask & (1 << i))) continue; RenderPassType rpType = (RenderPassType)i; + + // Sanity check - don't compile incompatible types (could be caused by corrupt caches, changes in data structures, etc). + if (pipelineFlags & PipelineFlags::USES_DEPTH_STENCIL) { + if (!RenderPassTypeHasDepth(rpType)) { + WARN_LOG(G3D, "Not compiling pipeline that requires depth, for non depth renderpass type"); + continue; + } + } + if (pipelineFlags & PipelineFlags::USES_INPUT_ATTACHMENT) { + if (!RenderPassTypeHasInput(rpType)) { + WARN_LOG(G3D, "Not compiling pipeline that requires input attachment, for non input renderpass type"); + continue; + } + } + pipeline->pipeline[rpType] = Promise::CreateEmpty(); compileQueue_.push_back(CompileQueueEntry(pipeline, compatibleRenderPass->Get(vulkan_, rpType), rpType)); needsCompile = true; diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index ed9f0d16e5..3c27c8da98 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -241,7 +241,7 @@ public: // We delay creating pipelines until the end of the current render pass, so we can create the right type immediately. // Unless a variantBitmask is passed in, in which case we can just go ahead. // WARNING: desc must stick around during the lifetime of the pipeline! It's not enough to build it on the stack and drop it. - VKRGraphicsPipeline *CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, uint32_t variantBitmask, const char *tag); + VKRGraphicsPipeline *CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, const char *tag); VKRComputePipeline *CreateComputePipeline(VKRComputePipelineDesc *desc); void NudgeCompilerThread() { diff --git a/Common/GPU/Vulkan/thin3d_vulkan.cpp b/Common/GPU/Vulkan/thin3d_vulkan.cpp index bc5d452e08..aaf27d2590 100644 --- a/Common/GPU/Vulkan/thin3d_vulkan.cpp +++ b/Common/GPU/Vulkan/thin3d_vulkan.cpp @@ -1134,7 +1134,7 @@ Pipeline *VKContext::CreateGraphicsPipeline(const PipelineDesc &desc, const char VkPipelineRasterizationStateCreateInfo rs{ VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO }; raster->ToVulkan(&gDesc.rs); - pipeline->pipeline = renderManager_.CreateGraphicsPipeline(&gDesc, 1 << RP_TYPE_BACKBUFFER, tag ? tag : "thin3d"); + pipeline->pipeline = renderManager_.CreateGraphicsPipeline(&gDesc, pipelineFlags, 1 << RP_TYPE_BACKBUFFER, tag ? tag : "thin3d"); if (desc.uniformDesc) { pipeline->dynamicUniformSize = (int)desc.uniformDesc->uniformBufferSize; diff --git a/GPU/Vulkan/PipelineManagerVulkan.cpp b/GPU/Vulkan/PipelineManagerVulkan.cpp index 21ce7df61d..c965130113 100644 --- a/GPU/Vulkan/PipelineManagerVulkan.cpp +++ b/GPU/Vulkan/PipelineManagerVulkan.cpp @@ -296,7 +296,7 @@ static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager, desc->pipelineLayout = layout; - VKRGraphicsPipeline *pipeline = renderManager->CreateGraphicsPipeline(desc, variantBitmask, "game"); + VKRGraphicsPipeline *pipeline = renderManager->CreateGraphicsPipeline(desc, pipelineFlags, variantBitmask, "game"); vulkanPipeline->pipeline = pipeline; if (useBlendConstant) {