From 5d9ff611f628519ff9026015549e6bbdf86afec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 8 Nov 2026 18:45:29 +0100 Subject: [PATCH] OpenGL: Remove skip mode Since the native EGL path, the Android context stays current on the render thread until the render manager and the draw context are gone, and it's destroyed with every surface. So the GL calls skip mode avoided after the emu thread exits (Android was the only platform that set it) can just run, and nothing it would have leaked outlives the context. It was also a trap: a shader created while it was on was never compiled, but not marked failed either, so a program linked from it later failed with "missing shader". Reports of that stopped with 3cb6ada66a, which already took most of its uses away. ThreadEnd still frees the steps that never got submitted without running them, now called a dry run. Co-Authored-By: Claude Opus 5.5 --- Common/GPU/OpenGL/GLFrameData.cpp | 23 ++------------------ Common/GPU/OpenGL/GLFrameData.h | 2 +- Common/GPU/OpenGL/GLQueueRunner.cpp | 16 +++++--------- Common/GPU/OpenGL/GLQueueRunner.h | 5 +++-- Common/GPU/OpenGL/GLRenderManager.cpp | 26 +++++++++++------------ Common/GPU/OpenGL/GLRenderManager.h | 8 ------- Common/GPU/OpenGL/OpenGLGraphicsContext.h | 1 - Windows/GPU/WindowsGLContext.cpp | 1 - 8 files changed, 23 insertions(+), 59 deletions(-) diff --git a/Common/GPU/OpenGL/GLFrameData.cpp b/Common/GPU/OpenGL/GLFrameData.cpp index 38bcaddbf7..096875a606 100644 --- a/Common/GPU/OpenGL/GLFrameData.cpp +++ b/Common/GPU/OpenGL/GLFrameData.cpp @@ -22,36 +22,25 @@ void GLDeleter::Take(GLDeleter &other) { } // Runs on the GPU thread. -void GLDeleter::Perform(GLRenderManager *renderManager, bool skipGLCalls) { +void GLDeleter::Perform(GLRenderManager *renderManager) { for (auto pushBuffer : pushBuffers) { renderManager->UnregisterPushBuffer(pushBuffer); - if (skipGLCalls) { - pushBuffer->Destroy(false); - } delete pushBuffer; } pushBuffers.clear(); for (auto shader : shaders) { - if (skipGLCalls && shader) - shader->shader = 0; // prevent the glDeleteShader delete shader; } shaders.clear(); for (auto program : programs) { - if (skipGLCalls && program) - program->program = 0; // prevent the glDeleteProgram delete program; } programs.clear(); for (auto buffer : buffers) { - if (skipGLCalls && buffer) - buffer->buffer_ = 0; delete buffer; } buffers.clear(); for (auto texture : textures) { - if (skipGLCalls && texture) - texture->texture = 0; delete texture; } textures.clear(); @@ -61,17 +50,9 @@ void GLDeleter::Perform(GLRenderManager *renderManager, bool skipGLCalls) { } inputLayouts.clear(); for (auto framebuffer : framebuffers) { - if (skipGLCalls) { - framebuffer->handle = 0; - framebuffer->color_texture.texture = 0; - framebuffer->z_stencil_buffer = 0; - framebuffer->z_stencil_texture.texture = 0; - framebuffer->z_buffer = 0; - framebuffer->stencil_buffer = 0; - } delete framebuffer; } - if (!framebuffers.empty() && !skipGLCalls) { + if (!framebuffers.empty()) { // The destructor bound the default framebuffer. Without this, the next readback from the framebuffer // that was bound before could read the default one (zeros for Cars Race-O-Rama's luminance, #15914). renderManager->ForgetBoundFramebuffers(); diff --git a/Common/GPU/OpenGL/GLFrameData.h b/Common/GPU/OpenGL/GLFrameData.h index 16756f1522..f82485a4ee 100644 --- a/Common/GPU/OpenGL/GLFrameData.h +++ b/Common/GPU/OpenGL/GLFrameData.h @@ -19,7 +19,7 @@ class GLRenderManager; class GLDeleter { public: - void Perform(GLRenderManager *renderManager, bool skipGLCalls); + void Perform(GLRenderManager *renderManager); bool IsEmpty() const { return shaders.empty() && programs.empty() && buffers.empty() && textures.empty() && inputLayouts.empty() && framebuffers.empty() && pushBuffers.empty(); diff --git a/Common/GPU/OpenGL/GLQueueRunner.cpp b/Common/GPU/OpenGL/GLQueueRunner.cpp index de53b7bc79..b527515778 100644 --- a/Common/GPU/OpenGL/GLQueueRunner.cpp +++ b/Common/GPU/OpenGL/GLQueueRunner.cpp @@ -126,9 +126,9 @@ static std::string GetInfoLog(GLuint name, Getiv getiv, GetLog getLog) { return infoLog; } -void GLQueueRunner::RunInitSteps(const FastVec &steps, bool skipGLCalls) { - if (skipGLCalls) { - // Some bookkeeping still needs to be done. +void GLQueueRunner::RunInitSteps(const FastVec &steps, bool dryRun) { + if (dryRun) { + // Steps that will never run (left over at shutdown): only free what they own. for (size_t i = 0; i < steps.size(); i++) { const GLRInitStep &step = steps[i]; switch (step.stepType) { @@ -156,14 +156,8 @@ void GLQueueRunner::RunInitSteps(const FastVec &steps, bool skipGLC } break; } - case GLRInitStepType::CREATE_PROGRAM: - { - WARN_LOG(Log::G3D, "CREATE_PROGRAM found with skipGLCalls, not good"); - break; - } case GLRInitStepType::CREATE_SHADER: { - WARN_LOG(Log::G3D, "CREATE_SHADER found with skipGLCalls, not good"); delete[] step.create_shader.code; break; } @@ -675,8 +669,8 @@ retry_depth: currentReadHandle_ = fbo->handle; } -void GLQueueRunner::RunSteps(const std::vector &steps, GLFrameData &frameData, bool skipGLCalls, bool keepSteps, bool useVR) { - if (skipGLCalls) { +void GLQueueRunner::RunSteps(const std::vector &steps, GLFrameData &frameData, bool dryRun, bool keepSteps, bool useVR) { + if (dryRun) { if (keepSteps) { return; } diff --git a/Common/GPU/OpenGL/GLQueueRunner.h b/Common/GPU/OpenGL/GLQueueRunner.h index a683d04d52..13711bb202 100644 --- a/Common/GPU/OpenGL/GLQueueRunner.h +++ b/Common/GPU/OpenGL/GLQueueRunner.h @@ -369,9 +369,10 @@ public: caps_ = caps; } - void RunInitSteps(const FastVec &steps, bool skipGLCalls); + // dryRun: the steps will never run (left over at shutdown), so only free what they own. + void RunInitSteps(const FastVec &steps, bool dryRun); - void RunSteps(const std::vector &steps, GLFrameData &frameData, bool skipGLCalls, bool keepSteps, bool useVR); + void RunSteps(const std::vector &steps, GLFrameData &frameData, bool dryRun, bool keepSteps, bool useVR); void CreateDeviceObjects(); void DestroyDeviceObjects(); diff --git a/Common/GPU/OpenGL/GLRenderManager.cpp b/Common/GPU/OpenGL/GLRenderManager.cpp index 7a76017a28..45355437d7 100644 --- a/Common/GPU/OpenGL/GLRenderManager.cpp +++ b/Common/GPU/OpenGL/GLRenderManager.cpp @@ -44,7 +44,7 @@ GLRenderManager::~GLRenderManager() { _assert_(frameData_[i].deleter_prev.IsEmpty()); } // Was anything deleted during shutdown? - deleter_.Perform(this, skipGLCalls_); + deleter_.Perform(this); _assert_(deleter_.IsEmpty()); } @@ -55,7 +55,6 @@ void GLRenderManager::ThreadStart(Draw::DrawContext *draw) { exitNotified_ = false; hitExit_ = false; - skipGLCalls_ = false; if (newInflightFrames_ != -1) { INFO_LOG(Log::G3D, "OpenGL: Updating inflight frames to %d", newInflightFrames_); @@ -106,13 +105,12 @@ void GLRenderManager::ThreadEnd() { VLOG(" PULL: Quitting"); - // Good time to run all the deleters to get rid of leftover objects. + // Good time to run all the deleters to get rid of leftover objects. The context is still current. for (int i = 0; i < MAX_INFLIGHT_FRAMES; i++) { - // Since we're in shutdown, we should skip the GL calls on Android. - frameData_[i].deleter.Perform(this, skipGLCalls_); - frameData_[i].deleter_prev.Perform(this, skipGLCalls_); + frameData_[i].deleter.Perform(this); + frameData_[i].deleter_prev.Perform(this); } - deleter_.Perform(this, skipGLCalls_); + deleter_.Perform(this); // Steps that never got submitted. A dry run frees the data they own (texture uploads etc), and the steps. queueRunner_.RunInitSteps(initSteps_, true); initSteps_.clear(); @@ -458,15 +456,15 @@ bool GLRenderManager::Run(GLRRenderThreadTask &task) { if (!frameData.hasBegun) { frameData.hasBegun = true; - frameData.deleter_prev.Perform(this, skipGLCalls_); + frameData.deleter_prev.Perform(this); frameData.deleter_prev.Take(frameData.deleter); } // queueRunner_.LogSteps(stepsOnThread); - queueRunner_.RunInitSteps(task.initSteps, skipGLCalls_); + queueRunner_.RunInitSteps(task.initSteps, false); // Run this after RunInitSteps so any fresh GLRBuffers for the pushbuffers can get created. - if (!skipGLCalls_) { + { std::lock_guard lock(pushBuffersMutex_); for (auto iter : frameData.activePushBuffers) { iter->Flush(); @@ -479,7 +477,7 @@ bool GLRenderManager::Run(GLRRenderThreadTask &task) { } #ifdef __LIBRETRO__ - if (Libretro::VRActive() && !skipGLCalls_) { + if (Libretro::VRActive()) { for (int eye = 0; eye < 2; ++eye) { Libretro::BeginVREye(eye); queueRunner_.RunSteps(task.steps, frameData, false, eye == 0, false); @@ -491,18 +489,18 @@ bool GLRenderManager::Run(GLRRenderThreadTask &task) { int passes = GetVRPassesCount(); for (int i = 0; i < passes; i++) { PreVRFrameRender(i); - queueRunner_.RunSteps(task.steps, frameData, skipGLCalls_, i < passes - 1, true); + queueRunner_.RunSteps(task.steps, frameData, false, i < passes - 1, true); PostVRFrameRender(); } } else { - queueRunner_.RunSteps(task.steps, frameData, skipGLCalls_, false, false); + queueRunner_.RunSteps(task.steps, frameData, false, false, false); } if (frameData.profile.enabled) { frameData.profile.cpuEndTime = time_now_d(); } - if (!skipGLCalls_) { + { std::lock_guard lock(pushBuffersMutex_); for (auto iter : frameData.activePushBuffers) { iter->MapDevice(bufferStrategy_); diff --git a/Common/GPU/OpenGL/GLRenderManager.h b/Common/GPU/OpenGL/GLRenderManager.h index ad43cfc633..39399b8ec2 100644 --- a/Common/GPU/OpenGL/GLRenderManager.h +++ b/Common/GPU/OpenGL/GLRenderManager.h @@ -893,12 +893,6 @@ public: return queueRunner_.GetGLString(name); } - // Used during Android-style ugly shutdown. No need to have a way to set it back because we'll be - // destroyed. - void SetSkipGLCalls() { - skipGLCalls_ = true; - } - int GetNumSteps() const { return (int)steps_.size(); } @@ -953,8 +947,6 @@ private: bool syncDone_ = false; GLDeleter deleter_; - // Set from either thread, read on the render thread. - std::atomic skipGLCalls_{false}; int curFrame_ = 0; diff --git a/Common/GPU/OpenGL/OpenGLGraphicsContext.h b/Common/GPU/OpenGL/OpenGLGraphicsContext.h index d39cecf286..89887d6d29 100644 --- a/Common/GPU/OpenGL/OpenGLGraphicsContext.h +++ b/Common/GPU/OpenGL/OpenGLGraphicsContext.h @@ -35,7 +35,6 @@ public: // Call from emu thread void NotifyEmuThreadExit() override { - renderManager_->SetSkipGLCalls(); renderManager_->NotifyEmuThreadExit(); } diff --git a/Windows/GPU/WindowsGLContext.cpp b/Windows/GPU/WindowsGLContext.cpp index 357878b69a..5384462fe3 100644 --- a/Windows/GPU/WindowsGLContext.cpp +++ b/Windows/GPU/WindowsGLContext.cpp @@ -494,6 +494,5 @@ void WindowsGLContext::ThreadEnd() { } void WindowsGLContext::NotifyEmuThreadExit() { - // renderManager_->SetSkipGLCalls(); // not necessary on desktop, maybe unadvisable? renderManager_->NotifyEmuThreadExit(); }