From 26d828fb4c7f0b7778d090778951cf1caba90a79 Mon Sep 17 00:00:00 2001 From: James Rowe Date: Fri, 20 Sep 2019 19:09:48 -0600 Subject: [PATCH] Prevent softlock on shutdown and various cleanup --- src/citra/emu_window/emu_window_sdl2.cpp | 1 - src/citra_qt/bootmanager.cpp | 9 ------ src/citra_qt/bootmanager.h | 1 - src/video_core/renderer_base.h | 3 -- .../renderer_opengl/renderer_opengl.cpp | 28 ++++++++++++++----- .../renderer_opengl/renderer_opengl.h | 3 -- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/citra/emu_window/emu_window_sdl2.cpp b/src/citra/emu_window/emu_window_sdl2.cpp index 3e4a45671..c4907d041 100644 --- a/src/citra/emu_window/emu_window_sdl2.cpp +++ b/src/citra/emu_window/emu_window_sdl2.cpp @@ -227,7 +227,6 @@ void EmuWindow_SDL2::Present() { while (IsOpen()) { VideoCore::g_renderer->TryPresent(100); SDL_GL_SwapWindow(render_window); - VideoCore::g_renderer->PresentComplete(); } SDL_GL_MakeCurrent(render_window, nullptr); } diff --git a/src/citra_qt/bootmanager.cpp b/src/citra_qt/bootmanager.cpp index 27fa70153..6c2581fd9 100644 --- a/src/citra_qt/bootmanager.cpp +++ b/src/citra_qt/bootmanager.cpp @@ -86,7 +86,6 @@ GRenderWindow::GRenderWindow(QWidget* parent, EmuThread* emu_thread) setWindowTitle(QStringLiteral("Citra %1 | %2-%3") .arg(Common::g_build_name, Common::g_scm_branch, Common::g_scm_desc)); setAttribute(Qt::WA_AcceptTouchEvents); - connect(this, &QOpenGLWidget::frameSwapped, this, &GRenderWindow::OnFrameSwapped); InputCommon::Init(); } @@ -104,10 +103,6 @@ void GRenderWindow::DoneCurrent() { void GRenderWindow::PollEvents() {} -void OnFrameSwapped() { - VideoCore::g_renderer->PresentComplete(); -} - // On Qt 5.0+, this correctly gets the size of the framebuffer (pixels). // // Older versions get the window size (density independent pixels), @@ -303,10 +298,6 @@ void GRenderWindow::paintGL() { update(); } -void GRenderWindow::OnFrameSwapped() { - VideoCore::g_renderer->PresentComplete(); -} - void GRenderWindow::showEvent(QShowEvent* event) { QWidget::showEvent(event); } diff --git a/src/citra_qt/bootmanager.h b/src/citra_qt/bootmanager.h index 856274878..b2120a022 100644 --- a/src/citra_qt/bootmanager.h +++ b/src/citra_qt/bootmanager.h @@ -164,7 +164,6 @@ public slots: void OnEmulationStarting(EmuThread* emu_thread); void OnEmulationStopping(); void OnFramebufferSizeChanged(); - void OnFrameSwapped(); signals: /// Emitted when the window is closed diff --git a/src/video_core/renderer_base.h b/src/video_core/renderer_base.h index 0252e2dfe..b1ee6a973 100644 --- a/src/video_core/renderer_base.h +++ b/src/video_core/renderer_base.h @@ -35,9 +35,6 @@ public: /// specific implementation) virtual void TryPresent(int timeout_ms) = 0; - /// Marks the presentation buffer as complete and swaps it back into the pool - virtual void PresentComplete() = 0; - /// Prepares for video dumping (e.g. create necessary buffers, etc) virtual void PrepareVideoDumping() = 0; diff --git a/src/video_core/renderer_opengl/renderer_opengl.cpp b/src/video_core/renderer_opengl/renderer_opengl.cpp index 555f80130..6c4545bee 100644 --- a/src/video_core/renderer_opengl/renderer_opengl.cpp +++ b/src/video_core/renderer_opengl/renderer_opengl.cpp @@ -50,8 +50,8 @@ namespace OpenGL { // If the size of this is too small, it ends up creating a soft cap on FPS as the renderer will have // to wait on available presentation frames. There doesn't seem to be much of a downside to a larger -// number but 8 seems to be a good trade off for now -constexpr std::size_t SWAP_CHAIN_SIZE = 8; +// number but 9 swap textures at 60FPS presentation allows for 800% speed so thats probably fine +constexpr std::size_t SWAP_CHAIN_SIZE = 9; class OGLTextureMailbox : public Frontend::TextureMailbox { public: @@ -72,6 +72,11 @@ public: ~OGLTextureMailbox() override { // lock the mutex and clear out the present and free_queues and notify any people who are // blocked to prevent deadlock on shutdown + std::scoped_lock lock(swap_chain_lock); + free_queue.clear(); + present_queue.clear(); + free_cv.notify_all(); + present_cv.notify_all(); } void ReloadPresentFrame(Frontend::Frame* frame, u32 height, u32 width) override { @@ -120,7 +125,16 @@ public: Frontend::Frame* GetRenderFrame() override { std::unique_lock lock(swap_chain_lock); // wait for new entries in the free_queue - free_cv.wait(lock, [&] { return !free_queue.empty(); }); + // we want to break at some point to prevent a softlock on close if the presentation thread + // stops consuming buffers + free_cv.wait_for(lock, std::chrono::milliseconds(100), [&] { return !free_queue.empty(); }); + + // If theres no free frames, we will reuse the oldest render frame + if (free_queue.empty()) { + auto frame = present_queue.back(); + present_queue.pop_back(); + return frame; + } Frontend::Frame* frame = free_queue.front(); free_queue.pop_front(); @@ -396,7 +410,7 @@ void RendererOpenGL::SwapBuffers() { // Recreate the frame if the size of the window has changed if (layout.width != frame->width || layout.height != frame->height) { - LOG_CRITICAL(Render_OpenGL, "Reloading render frame"); + LOG_DEBUG(Render_OpenGL, "Reloading render frame"); render_window.mailbox->ReloadRenderFrame(frame, layout.width, layout.height); } @@ -846,6 +860,8 @@ void RendererOpenGL::TryPresent(int timeout_ms) { return; } + // Clearing before a full overwrite of a fbo can signal to drivers that they can avoid a + // readback since we won't be doing any blending glClear(GL_COLOR_BUFFER_BIT); // Recreate the presentation FBO if the color attachment was changed @@ -867,10 +883,8 @@ void RendererOpenGL::TryPresent(int timeout_ms) { /* insert fence for the main thread to block on */ frame->present_fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); glFlush(); -} -void RendererOpenGL::PresentComplete() { - // render_window.mailbox->PresentationComplete(); + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0); } /// Updates the framerate diff --git a/src/video_core/renderer_opengl/renderer_opengl.h b/src/video_core/renderer_opengl/renderer_opengl.h index 16c76f0a3..b2b522516 100644 --- a/src/video_core/renderer_opengl/renderer_opengl.h +++ b/src/video_core/renderer_opengl/renderer_opengl.h @@ -60,9 +60,6 @@ public: /// context void TryPresent(int timeout_ms) override; - /// Finializes the presentation and sets up the presentation frame to go back into the mailbox - void PresentComplete() override; - /// Prepares for video dumping (e.g. create necessary buffers, etc) void PrepareVideoDumping() override;