From cb23c0c01dcb73b724bcba1a3e6794c764e94c4a Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Mon, 18 May 2020 22:45:20 -0700 Subject: [PATCH 1/2] Vulkan: Create FB compatible pipelines in Draw. The easiest way is just to create both and support both. Perhaps we could optimize out the creation of the framebuffer one. Fixes #12928. --- ext/native/thin3d/thin3d_vulkan.cpp | 68 +++++++++++++++++++---------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index c89b079c68..c452f96a9f 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -251,7 +251,8 @@ public: ubo_ = new uint8_t[uboSize_]; } ~VKPipeline() { - vulkan_->Delete().QueueDeletePipeline(vkpipeline); + vulkan_->Delete().QueueDeletePipeline(backbufferPipeline); + vulkan_->Delete().QueueDeletePipeline(framebufferPipeline); delete[] ubo_; } @@ -272,7 +273,8 @@ public: return false; } - VkPipeline vkpipeline; + VkPipeline backbufferPipeline = VK_NULL_HANDLE; + VkPipeline framebufferPipeline = VK_NULL_HANDLE; int stride[4]{}; int dynamicUniformSize = 0; @@ -420,6 +422,7 @@ public: void DrawIndexed(int vertexCount, int offset) override; void DrawUP(const void *vdata, int vertexCount) override; + void BindCompatiblePipeline(); void ApplyDynamicState(); void Clear(int mask, uint32_t colorval, float depthVal, int stencilVal) override; @@ -1024,30 +1027,40 @@ Pipeline *VKContext::CreateGraphicsPipeline(const PipelineDesc &desc) { VkPipelineRasterizationStateCreateInfo rs{ VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO }; raster->ToVulkan(&rs); - VkGraphicsPipelineCreateInfo info{ VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO }; - info.flags = 0; - info.stageCount = (uint32_t)stages.size(); - info.pStages = stages.data(); - info.pColorBlendState = &blend->info; - info.pDepthStencilState = &depth->info; - info.pDynamicState = &dynamicInfo; - info.pInputAssemblyState = &inputAssembly; - info.pTessellationState = nullptr; - info.pMultisampleState = &ms; - info.pVertexInputState = &input->visc; - info.pRasterizationState = &rs; - info.pViewportState = &vs; // Must set viewport and scissor counts even if we set the actual state dynamically. - info.layout = pipelineLayout_; - info.subpass = 0; - info.renderPass = renderManager_.GetBackbufferRenderPass(); + VkGraphicsPipelineCreateInfo createInfo[2]; + for (auto &info : createInfo) { + info.sType = VK_STRUCTURE_TYPE_GRAPHICS_PIPELINE_CREATE_INFO; + info.flags = 0; + info.stageCount = (uint32_t)stages.size(); + info.pStages = stages.data(); + info.pColorBlendState = &blend->info; + info.pDepthStencilState = &depth->info; + info.pDynamicState = &dynamicInfo; + info.pInputAssemblyState = &inputAssembly; + info.pTessellationState = nullptr; + info.pMultisampleState = &ms; + info.pVertexInputState = &input->visc; + info.pRasterizationState = &rs; + info.pViewportState = &vs; // Must set viewport and scissor counts even if we set the actual state dynamically. + info.layout = pipelineLayout_; + info.subpass = 0; + } - // OK, need to create a new pipeline. - VkResult result = vkCreateGraphicsPipelines(device_, pipelineCache_, 1, &info, nullptr, &pipeline->vkpipeline); + createInfo[0].renderPass = renderManager_.GetBackbufferRenderPass(); + createInfo[1].renderPass = renderManager_.GetFramebufferRenderPass(); + + // OK, need to create new pipelines. + VkPipeline pipelines[2]{}; + VkResult result = vkCreateGraphicsPipelines(device_, pipelineCache_, 2, createInfo, nullptr, pipelines); if (result != VK_SUCCESS) { ELOG("Failed to create graphics pipeline"); delete pipeline; return nullptr; } + + pipeline->backbufferPipeline = pipelines[0]; + pipeline->framebufferPipeline = pipelines[1]; + if (desc.uniformDesc) { pipeline->dynamicUniformSize = (int)desc.uniformDesc->uniformBufferSize; } @@ -1249,7 +1262,7 @@ void VKContext::Draw(int vertexCount, int offset) { VkDescriptorSet descSet = GetOrCreateDescriptorSet(vulkanUBObuf); - renderManager_.BindPipeline(curPipeline_->vkpipeline); + BindCompatiblePipeline(); ApplyDynamicState(); renderManager_.Draw(pipelineLayout_, descSet, 1, &ubo_offset, vulkanVbuf, (int)vbBindOffset + curVBufferOffsets_[0], vertexCount, offset); } @@ -1265,7 +1278,7 @@ void VKContext::DrawIndexed(int vertexCount, int offset) { VkDescriptorSet descSet = GetOrCreateDescriptorSet(vulkanUBObuf); - renderManager_.BindPipeline(curPipeline_->vkpipeline); + BindCompatiblePipeline(); ApplyDynamicState(); renderManager_.DrawIndexed(pipelineLayout_, descSet, 1, &ubo_offset, vulkanVbuf, (int)vbBindOffset + curVBufferOffsets_[0], vulkanIbuf, (int)ibBindOffset + offset * sizeof(uint32_t), vertexCount, 1, VK_INDEX_TYPE_UINT16); } @@ -1277,11 +1290,20 @@ void VKContext::DrawUP(const void *vdata, int vertexCount) { VkDescriptorSet descSet = GetOrCreateDescriptorSet(vulkanUBObuf); - renderManager_.BindPipeline(curPipeline_->vkpipeline); + BindCompatiblePipeline(); ApplyDynamicState(); renderManager_.Draw(pipelineLayout_, descSet, 1, &ubo_offset, vulkanVbuf, (int)vbBindOffset + curVBufferOffsets_[0], vertexCount); } +void VKContext::BindCompatiblePipeline() { + VkRenderPass renderPass = renderManager_.GetCompatibleRenderPass(); + if (renderPass == renderManager_.GetBackbufferRenderPass()) { + renderManager_.BindPipeline(curPipeline_->backbufferPipeline); + } else { + renderManager_.BindPipeline(curPipeline_->framebufferPipeline); + } +} + void VKContext::Clear(int clearMask, uint32_t colorval, float depthVal, int stencilVal) { int mask = 0; if (clearMask & FBChannel::FB_COLOR_BIT) From 966ac4458a2a7beb66cfa2ec7b6fccd1e144809a Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Mon, 18 May 2020 23:06:14 -0700 Subject: [PATCH 2/2] Vulkan: Fix leak and remove extra allocator. The leak was because we restored immediately after initializing, and therefore leaked several pipelines. This allocator isn't used right now, so just using up memory. --- GPU/Common/PresentationCommon.cpp | 1 + GPU/Vulkan/FramebufferVulkan.cpp | 17 +---------------- GPU/Vulkan/FramebufferVulkan.h | 1 - 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/GPU/Common/PresentationCommon.cpp b/GPU/Common/PresentationCommon.cpp index c8b638f3ce..796e4e694f 100644 --- a/GPU/Common/PresentationCommon.cpp +++ b/GPU/Common/PresentationCommon.cpp @@ -352,6 +352,7 @@ Draw::Pipeline *PresentationCommon::CreatePipeline(std::vectorCreateBuffer(sizeof(Vertex) * 8, BufferUsageFlag::DYNAMIC | BufferUsageFlag::VERTEXDATA); diff --git a/GPU/Vulkan/FramebufferVulkan.cpp b/GPU/Vulkan/FramebufferVulkan.cpp index d8829a4ea1..e2bf96a6c3 100644 --- a/GPU/Vulkan/FramebufferVulkan.cpp +++ b/GPU/Vulkan/FramebufferVulkan.cpp @@ -78,7 +78,7 @@ FramebufferManagerVulkan::FramebufferManagerVulkan(Draw::DrawContext *draw, Vulk vulkan_(vulkan) { presentation_->SetLanguage(GLSL_VULKAN); - DeviceRestore(vulkan, draw); + InitDeviceObjects(); // After a blit we do need to rebind for the VulkanRenderManager to know what to do. needGLESRebinds_ = true; @@ -410,17 +410,6 @@ void FramebufferManagerVulkan::DeviceLost() { DestroyAllFBOs(); DestroyDeviceObjects(); presentation_->DeviceLost(); - - if (allocator_) { - allocator_->Destroy(); - - // We have to delete on queue, so this can free its queued deletions. - vulkan_->Delete().QueueCallback([](void *ptr) { - auto allocator = static_cast(ptr); - delete allocator; - }, allocator_); - allocator_ = nullptr; - } } void FramebufferManagerVulkan::DeviceRestore(VulkanContext *vulkan, Draw::DrawContext *draw) { @@ -428,9 +417,5 @@ void FramebufferManagerVulkan::DeviceRestore(VulkanContext *vulkan, Draw::DrawCo draw_ = draw; presentation_->DeviceRestore(draw); - _assert_(!allocator_); - - allocator_ = new VulkanDeviceAllocator(vulkan_, 1 * 1024 * 1024, 8 * 1024 * 1024); - InitDeviceObjects(); } diff --git a/GPU/Vulkan/FramebufferVulkan.h b/GPU/Vulkan/FramebufferVulkan.h index bbd799e1d0..5ceec728eb 100644 --- a/GPU/Vulkan/FramebufferVulkan.h +++ b/GPU/Vulkan/FramebufferVulkan.h @@ -84,7 +84,6 @@ private: TextureCacheVulkan *textureCacheVulkan_ = nullptr; ShaderManagerVulkan *shaderManagerVulkan_ = nullptr; DrawEngineVulkan *drawEngineVulkan_ = nullptr; - VulkanDeviceAllocator *allocator_ = nullptr; VulkanPushBuffer *push_; enum {