diff --git a/Common/GPU/OpenGL/GLRenderManager.cpp b/Common/GPU/OpenGL/GLRenderManager.cpp index d6adbff77119..575289707b0d 100644 --- a/Common/GPU/OpenGL/GLRenderManager.cpp +++ b/Common/GPU/OpenGL/GLRenderManager.cpp @@ -43,7 +43,7 @@ GLRenderManager::GLRenderManager(HistoryBuffer lock(pushMutex_); renderThreadQueue_.push(new GLRRenderThreadTask(GLRRunType::EXIT)); @@ -368,7 +368,7 @@ void GLRenderManager::BeginFrame(bool enableProfiling) { frameData.readyForFence = false; } - if (!run_) { + if (!runCompileThread_) { WARN_LOG(G3D, "BeginFrame while !run_!"); } diff --git a/Common/GPU/OpenGL/GLRenderManager.h b/Common/GPU/OpenGL/GLRenderManager.h index fb31fdfe3dbb..b3230da67fe9 100644 --- a/Common/GPU/OpenGL/GLRenderManager.h +++ b/Common/GPU/OpenGL/GLRenderManager.h @@ -874,7 +874,7 @@ class GLRenderManager { FastVec initSteps_; // Execution time state - bool run_ = true; + bool runCompileThread_ = true; // Thread is managed elsewhere, and should call ThreadFrame. GLQueueRunner queueRunner_; diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.h b/Common/GPU/Vulkan/VulkanQueueRunner.h index c68a4e8b7d5f..b6c1094065a6 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.h +++ b/Common/GPU/Vulkan/VulkanQueueRunner.h @@ -266,15 +266,6 @@ class VulkanQueueRunner { hacksEnabled_ = hacks; } - void NotifyCompileDone() { - compileDone_.notify_all(); - } - - void WaitForCompileNotification() { - std::unique_lock lock(compileDoneMutex_); - compileDone_.wait(lock); - } - private: bool InitBackbufferFramebuffers(int width, int height); bool InitDepthStencilBuffer(VkCommandBuffer cmd, VulkanBarrierBatch *barriers); // Used for non-buffered rendering. @@ -321,10 +312,6 @@ class VulkanQueueRunner { // TODO: Enable based on compat.ini. uint32_t hacksEnabled_ = 0; - // Compile done notifications. - std::mutex compileDoneMutex_; - std::condition_variable compileDone_; - // Image barrier helper used during command buffer record (PerformRenderPass etc). // Stored here to help reuse the allocation. diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index b5dba41183c1..905b3263a530 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -237,6 +237,54 @@ void VKRGraphicsPipeline::LogCreationFailure() const { ERROR_LOG(G3D, "======== END OF PIPELINE =========="); } +struct SinglePipelineTask { + VKRGraphicsPipeline *pipeline; + VkRenderPass compatibleRenderPass; + RenderPassType rpType; + VkSampleCountFlagBits sampleCount; + double scheduleTime; + int countToCompile; +}; + +class CreateMultiPipelinesTask : public Task { +public: + CreateMultiPipelinesTask(VulkanContext *vulkan, std::vector tasks) : vulkan_(vulkan), tasks_(tasks) { + tasksInFlight_.fetch_add(1); + } + ~CreateMultiPipelinesTask() {} + + TaskType Type() const override { + return TaskType::CPU_COMPUTE; + } + + TaskPriority Priority() const override { + return TaskPriority::HIGH; + } + + void Run() override { + for (auto &task : tasks_) { + task.pipeline->Create(vulkan_, task.compatibleRenderPass, task.rpType, task.sampleCount, task.scheduleTime, task.countToCompile); + } + tasksInFlight_.fetch_sub(1); + } + + VulkanContext *vulkan_; + std::vector tasks_; + + // Use during shutdown to make sure there aren't any leftover tasks sitting queued. + // Could probably be done more elegantly. Like waiting for all tasks of a type, or saving pointers to them, or something... + static void WaitForAll(); + static std::atomic tasksInFlight_; +}; + +void CreateMultiPipelinesTask::WaitForAll() { + while (tasksInFlight_.load() > 0) { + sleep_ms(2); + } +} + +std::atomic CreateMultiPipelinesTask::tasksInFlight_; + VulkanRenderManager::VulkanRenderManager(VulkanContext *vulkan, bool useThread, HistoryBuffer &frameTimeHistory) : vulkan_(vulkan), queueRunner_(vulkan), initTimeMs_("initTimeMs"), @@ -294,7 +342,8 @@ bool VulkanRenderManager::CreateBackbuffers() { // Start the thread(s). if (HasBackbuffers()) { - run_ = true; // For controlling the compiler thread's exit + runCompileThread_ = true; // For controlling the compiler thread's exit + compileBlocked_ = false; if (useRenderThread_) { INFO_LOG(G3D, "Starting Vulkan submission thread"); @@ -324,7 +373,8 @@ void VulkanRenderManager::StopThread() { } // Compiler and present thread still relies on this. - run_ = false; + runCompileThread_ = false; + compileBlocked_ = true; if (presentWaitThread_.joinable()) { presentWaitThread_.join(); @@ -344,36 +394,17 @@ void VulkanRenderManager::StopThread() { INFO_LOG(G3D, "Vulkan submission thread joined. Frame=%d", vulkan_->GetCurFrame()); if (compileThread_.joinable()) { - // Lock to avoid race conditions. - std::lock_guard guard(compileMutex_); - compileCond_.notify_all(); - } - compileThread_.join(); - INFO_LOG(G3D, "Vulkan compiler thread joined."); - - // Eat whatever has been queued up for this frame if anything. - Wipe(); - - // Clean out any remaining queued data, which might refer to things that might not be valid - // when we restart the thread... - - // Not sure if this is still needed - for (int i = 0; i < vulkan_->GetInflightFrames(); i++) { - auto &frameData = frameData_[i]; - if (frameData.hasInitCommands) { - // Clear 'em out. This can happen on restart sometimes. - vkEndCommandBuffer(frameData.initCmd); - frameData.hasInitCommands = false; - } - if (frameData.hasMainCommands) { - vkEndCommandBuffer(frameData.mainCmd); - frameData.hasMainCommands = false; - } - if (frameData.hasPresentCommands) { - vkEndCommandBuffer(frameData.presentCmd); - frameData.hasPresentCommands = false; + // Lock to avoid race conditions. Not sure if needed. + { + std::lock_guard guard(compileMutex_); + compileCond_.notify_all(); } + compileThread_.join(); } + INFO_LOG(G3D, "Vulkan compiler thread joined. Now wait for any straggling compile tasks."); + CreateMultiPipelinesTask::WaitForAll(); + + _dbg_assert_(steps_.empty()); } void VulkanRenderManager::DestroyBackbuffers() { @@ -386,7 +417,7 @@ void VulkanRenderManager::DestroyBackbuffers() { VulkanRenderManager::~VulkanRenderManager() { INFO_LOG(G3D, "VulkanRenderManager destructor"); - _dbg_assert_(!run_); // StopThread should already have been called from DestroyBackbuffers. + _dbg_assert_(!runCompileThread_); // StopThread should already have been called from DestroyBackbuffers. vulkan_->WaitUntilQueueIdle(); @@ -400,53 +431,6 @@ VulkanRenderManager::~VulkanRenderManager() { queueRunner_.DestroyDeviceObjects(); } -struct SinglePipelineTask { - VKRGraphicsPipeline *pipeline; - VkRenderPass compatibleRenderPass; - RenderPassType rpType; - VkSampleCountFlagBits sampleCount; - double scheduleTime; - int countToCompile; -}; - -class CreateMultiPipelinesTask : public Task { -public: - CreateMultiPipelinesTask(VulkanContext *vulkan, std::vector tasks) : vulkan_(vulkan), tasks_(tasks) { - tasksInFlight_.fetch_add(1); - } - ~CreateMultiPipelinesTask() {} - - TaskType Type() const override { - return TaskType::CPU_COMPUTE; - } - - TaskPriority Priority() const override { - return TaskPriority::HIGH; - } - - void Run() override { - for (auto &task : tasks_) { - task.pipeline->Create(vulkan_, task.compatibleRenderPass, task.rpType, task.sampleCount, task.scheduleTime, task.countToCompile); - } - tasksInFlight_.fetch_sub(1); - } - - VulkanContext *vulkan_; - std::vector tasks_; - - // Use during shutdown to make sure there aren't any leftover tasks sitting queued. - // Could probably be done more elegantly. Like waiting for all tasks of a type, or saving pointers to them, or something... - static void WaitForAll() { - while (tasksInFlight_.load() > 0) { - sleep_ms(2); - } - } - - static std::atomic tasksInFlight_; -}; - -std::atomic CreateMultiPipelinesTask::tasksInFlight_; - void VulkanRenderManager::CompileThreadFunc() { SetCurrentThreadName("ShaderCompile"); while (true) { @@ -456,15 +440,12 @@ void VulkanRenderManager::CompileThreadFunc() { // TODO: Should this be while? // It may be beneficial also to unlock and wait a little bit to see if we get some more shaders // so we can do a better job of thread-sorting them. - if (compileQueue_.empty() && run_) { + if (compileQueue_.empty() && runCompileThread_) { compileCond_.wait(lock); } toCompile = std::move(compileQueue_); compileQueue_.clear(); } - if (!run_) { - break; - } int countToCompile = (int)toCompile.size(); @@ -505,32 +486,33 @@ void VulkanRenderManager::CompileThreadFunc() { Task *task = new CreateMultiPipelinesTask(vulkan_, entries); g_threadManager.EnqueueTask(task); } - - queueRunner_.NotifyCompileDone(); + + // Hold off just a bit before we check again, to allow bunches of pipelines to collect. + sleep_ms(1); + + if (!runCompileThread_) { + break; + } } } void VulkanRenderManager::DrainAndBlockCompileQueue() { compileBlocked_ = true; + runCompileThread_ = false; compileCond_.notify_all(); - while (true) { - bool anyInQueue = false; - { - std::unique_lock lock(compileMutex_); - anyInQueue = !compileQueue_.empty(); - } - if (anyInQueue) { - queueRunner_.WaitForCompileNotification(); - } else { - break; - } - } + compileThread_.join(); + + _assert_(compileQueue_.empty()); + // At this point, no more tasks can be queued to the threadpool. So wait for them all to go away. CreateMultiPipelinesTask::WaitForAll(); } void VulkanRenderManager::ReleaseCompileQueue() { compileBlocked_ = false; + runCompileThread_ = true; + INFO_LOG(G3D, "Restarting Vulkan compiler thread"); + compileThread_ = std::thread(&VulkanRenderManager::CompileThreadFunc, this); } void VulkanRenderManager::ThreadFunc() { @@ -575,7 +557,7 @@ void VulkanRenderManager::PresentWaitThreadFunc() { _dbg_assert_(vkWaitForPresentKHR != nullptr); uint64_t waitedId = frameIdGen_; - while (run_) { + while (runCompileThread_) { const uint64_t timeout = 1000000000ULL; // 1 sec if (VK_SUCCESS == vkWaitForPresentKHR(vulkan_->GetDevice(), vulkan_->GetSwapchain(), waitedId, timeout)) { frameTimeHistory_[waitedId].actualPresent = time_now_d(); @@ -799,10 +781,7 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe VKRRenderPassStoreAction::STORE, VKRRenderPassStoreAction::DONT_CARE, VKRRenderPassStoreAction::DONT_CARE, }; VKRRenderPass *compatibleRenderPass = queueRunner_.GetRenderPass(key); - if (compileBlocked_) { - delete pipeline; - return nullptr; - } + _dbg_assert_(!compileBlocked_); std::lock_guard lock(compileMutex_); bool needsCompile = false; for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) { @@ -871,9 +850,10 @@ void VulkanRenderManager::EndCurRenderStep() { VkSampleCountFlagBits sampleCount = curRenderStep_->render.framebuffer ? curRenderStep_->render.framebuffer->sampleCount : VK_SAMPLE_COUNT_1_BIT; compileMutex_.lock(); + _dbg_assert_(!compileBlocked_); bool needsCompile = false; for (VKRGraphicsPipeline *pipeline : pipelinesToCheck_) { - if (!pipeline || compileBlocked_) { + if (!pipeline) { // Not good, but let's try not to crash. continue; } @@ -1441,13 +1421,6 @@ void VulkanRenderManager::Present() { insideFrame_ = false; } -void VulkanRenderManager::Wipe() { - for (auto step : steps_) { - delete step; - } - steps_.clear(); -} - // Called on the render thread. // // Can be called again after a VKRRunType::SYNC on the same frame. @@ -1728,12 +1701,22 @@ void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueuePro // This will write all descriptors. // Initially, we just do a simple look-back comparing to the previous descriptor to avoid sequential dupes. + // In theory, we could multithread this. Gotta be a lot of descriptors for that to be worth it though. // Initially, let's do naive single desc set writes. VkWriteDescriptorSet writes[MAX_DESC_SET_BINDINGS]; VkDescriptorImageInfo imageInfo[MAX_DESC_SET_BINDINGS]; // just picked a practical number VkDescriptorBufferInfo bufferInfo[MAX_DESC_SET_BINDINGS]; + // Preinitialize fields that won't change. + for (size_t i = 0; i < ARRAY_SIZE(writes); i++) { + writes[i].descriptorCount = 1; + writes[i].dstArrayElement = 0; + writes[i].pTexelBufferView = nullptr; + writes[i].pNext = nullptr; + writes[i].sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; + } + size_t start = data.flushedDescriptors_; int writeCount = 0, dedupCount = 0; @@ -1775,6 +1758,7 @@ void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueuePro switch (this->bindingTypes[i]) { case BindingType::COMBINED_IMAGE_SAMPLER: _dbg_assert_(data[i].image.sampler != VK_NULL_HANDLE); + _dbg_assert_(data[i].image.view != VK_NULL_HANDLE); imageInfo[numImages].imageLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; imageInfo[numImages].imageView = data[i].image.view; imageInfo[numImages].sampler = data[i].image.sampler; @@ -1784,6 +1768,7 @@ void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueuePro numImages++; break; case BindingType::STORAGE_IMAGE_COMPUTE: + _dbg_assert_(data[i].image.view != VK_NULL_HANDLE); imageInfo[numImages].imageLayout = VK_IMAGE_LAYOUT_GENERAL; imageInfo[numImages].imageView = data[i].image.view; imageInfo[numImages].sampler = VK_NULL_HANDLE; @@ -1794,6 +1779,7 @@ void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueuePro break; case BindingType::STORAGE_BUFFER_VERTEX: case BindingType::STORAGE_BUFFER_COMPUTE: + _dbg_assert_(data[i].buffer.buffer != VK_NULL_HANDLE); bufferInfo[numBuffers].buffer = data[i].buffer.buffer; bufferInfo[numBuffers].range = data[i].buffer.range; bufferInfo[numBuffers].offset = data[i].buffer.offset; @@ -1804,6 +1790,7 @@ void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueuePro break; case BindingType::UNIFORM_BUFFER_DYNAMIC_ALL: case BindingType::UNIFORM_BUFFER_DYNAMIC_VERTEX: + _dbg_assert_(data[i].buffer.buffer != VK_NULL_HANDLE); bufferInfo[numBuffers].buffer = data[i].buffer.buffer; bufferInfo[numBuffers].range = data[i].buffer.range; bufferInfo[numBuffers].offset = 0; @@ -1813,13 +1800,8 @@ void VKRPipelineLayout::FlushDescSets(VulkanContext *vulkan, int frame, QueuePro numBuffers++; break; } - writes[numWrites].sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; - writes[numWrites].pNext = nullptr; - writes[numWrites].descriptorCount = 1; - writes[numWrites].dstArrayElement = 0; writes[numWrites].dstBinding = i; writes[numWrites].dstSet = d.set; - writes[numWrites].pTexelBufferView = nullptr; numWrites++; } diff --git a/Common/GPU/Vulkan/VulkanRenderManager.h b/Common/GPU/Vulkan/VulkanRenderManager.h index 8bd6a2223530..650eb60af12f 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.h +++ b/Common/GPU/Vulkan/VulkanRenderManager.h @@ -229,8 +229,6 @@ class VulkanRenderManager { // These can run on a different thread! void Finish(); void Present(); - // Zaps queued up commands. Use if you know there's a risk you've queued up stuff that has already been deleted. Can happen during in-game shutdown. - void Wipe(); void SetInvalidationCallback(InvalidationCallback callback) { invalidationCallback_ = callback; @@ -565,7 +563,7 @@ class VulkanRenderManager { int curHeight_ = -1; bool insideFrame_ = false; - bool run_ = false; + bool runCompileThread_ = false; bool useRenderThread_ = true; bool measurePresentTime_ = false; @@ -601,7 +599,7 @@ class VulkanRenderManager { std::condition_variable compileCond_; std::mutex compileMutex_; std::vector compileQueue_; - std::atomic compileBlocked_{}; + std::atomic compileBlocked_{}; // Only for asserting on, now. // Thread for measuring presentation delay. std::thread presentWaitThread_; diff --git a/Common/GPU/Vulkan/thin3d_vulkan.cpp b/Common/GPU/Vulkan/thin3d_vulkan.cpp index 572cb8fbc848..c03e3b685449 100644 --- a/Common/GPU/Vulkan/thin3d_vulkan.cpp +++ b/Common/GPU/Vulkan/thin3d_vulkan.cpp @@ -491,8 +491,6 @@ class VKContext : public DrawContext { void EndFrame() override; void Present(PresentMode presentMode, int vblanks) override; - void WipeQueue() override; - int GetFrameCount() override { return frameCount_; } @@ -1118,10 +1116,6 @@ void VKContext::Invalidate(InvalidationFlags flags) { } } -void VKContext::WipeQueue() { - renderManager_.Wipe(); -} - void VKContext::BindDescriptors(VkBuffer buf, PackedDescriptor descriptors[4]) { descriptors[0].buffer.buffer = buf; descriptors[0].buffer.offset = 0; // dynamic diff --git a/Common/GPU/thin3d.h b/Common/GPU/thin3d.h index 859f93a95b3b..43504bcccc4d 100644 --- a/Common/GPU/thin3d.h +++ b/Common/GPU/thin3d.h @@ -822,8 +822,6 @@ class DrawContext { // NOTE: Not all backends support vblanks > 1. Some backends also can't change presentation mode immediately. virtual void Present(PresentMode presentMode, int vblanks) = 0; - virtual void WipeQueue() {} - // This should be avoided as much as possible, in favor of clearing when binding a render target, which is native // on Vulkan. virtual void Clear(int mask, uint32_t colorval, float depthVal, int stencilVal) = 0; diff --git a/UI/EmuScreen.cpp b/UI/EmuScreen.cpp index 406c3604f7ac..b4b9c8df94ec 100644 --- a/UI/EmuScreen.cpp +++ b/UI/EmuScreen.cpp @@ -1642,11 +1642,7 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) { SetVRAppMode(screenManager()->topScreen() == this ? VRAppMode::VR_GAME_MODE : VRAppMode::VR_DIALOG_MODE); } - if (mode & ScreenRenderMode::TOP) { - // TODO: Replace this with something else. - if (stopRender_) - draw->WipeQueue(); - } else { + if (!(mode & ScreenRenderMode::TOP)) { darken(); } return flags;