From f7fcbe32c9f66bcfc86853a5cda95edcf481f573 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Sat, 8 Feb 2025 01:46:26 +0100 Subject: [PATCH] renderer: various fixes towards improving gpu calls robustness (#9188) ensure framebuffer textures are detached and deleted, avoid leaving framebuffers bound when not needed * render: avoid calling glDeleteProgram on no program its safe to do so but it adds a bunch of unnecessery lines in apitrace when tracing. if guard it and return early. * opengl: ensure texture and buffers are unbound ensure bound buffers are unbound after use, also detach textures from framebuffer before deleting it otherwise it will become dangling and essentially leak. --- src/protocols/Screencopy.cpp | 6 ++++ src/protocols/ToplevelExport.cpp | 6 ++++ src/render/Framebuffer.cpp | 56 +++++++++++++++++--------------- src/render/Framebuffer.hpp | 3 +- src/render/OpenGL.hpp | 2 -- src/render/Renderbuffer.cpp | 8 ++--- src/render/Shader.cpp | 5 ++- 7 files changed, 50 insertions(+), 36 deletions(-) diff --git a/src/protocols/Screencopy.cpp b/src/protocols/Screencopy.cpp index e086cc762..aae5caf1c 100644 --- a/src/protocols/Screencopy.cpp +++ b/src/protocols/Screencopy.cpp @@ -289,6 +289,12 @@ bool CScreencopyFrame::copyShm() { g_pHyprOpenGL->m_RenderData.pMonitor.reset(); +#ifndef GLES2 + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0); +#else + glBindFramebuffer(GL_FRAMEBUFFER, 0); +#endif + LOGM(TRACE, "Copied frame via shm"); return true; diff --git a/src/protocols/ToplevelExport.cpp b/src/protocols/ToplevelExport.cpp index a9009aa47..fb0bd9c71 100644 --- a/src/protocols/ToplevelExport.cpp +++ b/src/protocols/ToplevelExport.cpp @@ -319,6 +319,12 @@ bool CToplevelExportFrame::copyShm(timespec* now) { g_pPointerManager->damageCursor(PMONITOR->self.lock()); } + outFB.unbind(); + +#ifndef GLES2 + glBindFramebuffer(GL_READ_FRAMEBUFFER, 0); +#endif + return true; } diff --git a/src/render/Framebuffer.cpp b/src/render/Framebuffer.cpp index 93f8fe7f8..6905eb361 100644 --- a/src/render/Framebuffer.cpp +++ b/src/render/Framebuffer.cpp @@ -12,29 +12,26 @@ bool CFramebuffer::alloc(int w, int h, uint32_t drmFormat) { uint32_t glFormat = NFormatUtils::drmFormatToGL(drmFormat); uint32_t glType = NFormatUtils::glFormatToType(glFormat); - if (!m_cTex) + if (!m_cTex) { m_cTex = makeShared(); - - if (!m_iFbAllocated) { - firstAlloc = true; - glGenFramebuffers(1, &m_iFb); - m_iFbAllocated = true; - } - - if (m_cTex->m_iTexID == 0) { - firstAlloc = true; m_cTex->allocate(); glBindTexture(GL_TEXTURE_2D, m_cTex->m_iTexID); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR); + firstAlloc = true; + } + + if (!m_iFbAllocated) { + glGenFramebuffers(1, &m_iFb); + m_iFbAllocated = true; + firstAlloc = true; } if (firstAlloc || m_vSize != Vector2D(w, h)) { glBindTexture(GL_TEXTURE_2D, m_cTex->m_iTexID); glTexImage2D(GL_TEXTURE_2D, 0, glFormat, w, h, 0, GL_RGBA, glType, nullptr); - glBindFramebuffer(GL_FRAMEBUFFER, m_iFb); glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, m_cTex->m_iTexID, 0); @@ -43,9 +40,6 @@ bool CFramebuffer::alloc(int w, int h, uint32_t drmFormat) { if (m_pStencilTex) { glBindTexture(GL_TEXTURE_2D, m_pStencilTex->m_iTexID); glTexImage2D(GL_TEXTURE_2D, 0, GL_DEPTH24_STENCIL8, w, h, 0, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, nullptr); - - glBindFramebuffer(GL_FRAMEBUFFER, m_iFb); - glFramebufferTexture2D(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_TEXTURE_2D, m_pStencilTex->m_iTexID, 0); } #endif @@ -57,8 +51,7 @@ bool CFramebuffer::alloc(int w, int h, uint32_t drmFormat) { } glBindTexture(GL_TEXTURE_2D, 0); - if (g_pHyprOpenGL) - glBindFramebuffer(GL_FRAMEBUFFER, g_pHyprOpenGL->m_iCurrentOutputFb); + glBindFramebuffer(GL_FRAMEBUFFER, 0); m_vSize = Vector2D(w, h); @@ -80,7 +73,7 @@ void CFramebuffer::addStencil(SP tex) { RASSERT((status == GL_FRAMEBUFFER_COMPLETE), "Failed adding a stencil to fbo!", status); glBindTexture(GL_TEXTURE_2D, 0); - glBindFramebuffer(GL_FRAMEBUFFER, g_pHyprOpenGL->m_iCurrentOutputFb); + glBindFramebuffer(GL_FRAMEBUFFER, 0); #endif } @@ -90,25 +83,36 @@ void CFramebuffer::bind() { #else glBindFramebuffer(GL_FRAMEBUFFER, m_iFb); #endif + if (g_pHyprOpenGL) glViewport(0, 0, g_pHyprOpenGL->m_RenderData.pMonitor->vecPixelSize.x, g_pHyprOpenGL->m_RenderData.pMonitor->vecPixelSize.y); else glViewport(0, 0, m_vSize.x, m_vSize.y); } +void CFramebuffer::unbind() { +#ifndef GLES2 + glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); +#else + glBindFramebuffer(GL_FRAMEBUFFER, 0); +#endif +} + void CFramebuffer::release() { - if (!m_iFbAllocated && !m_cTex) - return; + if (m_iFbAllocated) { + glBindFramebuffer(GL_FRAMEBUFFER, m_iFb); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0); + glBindFramebuffer(GL_FRAMEBUFFER, 0); - Debug::log(TRACE, "fb {} released", m_iFb); - - if (m_iFbAllocated) glDeleteFramebuffers(1, &m_iFb); + m_iFbAllocated = false; + m_iFb = 0; + } - m_cTex.reset(); - m_iFbAllocated = false; - m_vSize = Vector2D(); - m_iFb = 0; + if (m_cTex) + m_cTex.reset(); + + m_vSize = Vector2D(); } CFramebuffer::~CFramebuffer() { diff --git a/src/render/Framebuffer.hpp b/src/render/Framebuffer.hpp index 84dfeef12..092e548ef 100644 --- a/src/render/Framebuffer.hpp +++ b/src/render/Framebuffer.hpp @@ -11,6 +11,7 @@ class CFramebuffer { bool alloc(int w, int h, uint32_t format = GL_RGBA); void addStencil(SP tex); void bind(); + void unbind(); void release(); void reset(); bool isAllocated(); @@ -28,4 +29,4 @@ class CFramebuffer { SP m_pStencilTex; friend class CRenderbuffer; -}; \ No newline at end of file +}; diff --git a/src/render/OpenGL.hpp b/src/render/OpenGL.hpp index 7b7b7e6fd..cf1d25490 100644 --- a/src/render/OpenGL.hpp +++ b/src/render/OpenGL.hpp @@ -233,8 +233,6 @@ class CHyprOpenGLImpl { SCurrentRenderData m_RenderData; - GLint m_iCurrentOutputFb = 0; - Hyprutils::OS::CFileDescriptor m_iGBMFD; gbm_device* m_pGbmDevice = nullptr; EGLContext m_pEglContext = nullptr; diff --git a/src/render/Renderbuffer.cpp b/src/render/Renderbuffer.cpp index 887f31a90..1ea9f785a 100644 --- a/src/render/Renderbuffer.cpp +++ b/src/render/Renderbuffer.cpp @@ -46,7 +46,7 @@ CRenderbuffer::CRenderbuffer(SP buffer, uint32_t format) : return; } - glBindFramebuffer(GL_FRAMEBUFFER, 0); + m_sFramebuffer.unbind(); listeners.destroyBuffer = buffer->events.destroy.registerListener([this](std::any d) { g_pHyprRenderer->onRenderbufferDestroy(this); }); @@ -68,11 +68,7 @@ void CRenderbuffer::bindFB() { void CRenderbuffer::unbind() { glBindRenderbuffer(GL_RENDERBUFFER, 0); -#ifndef GLES2 - glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0); -#else - glBindFramebuffer(GL_FRAMEBUFFER, 0); -#endif + m_sFramebuffer.unbind(); } CFramebuffer* CRenderbuffer::getFB() { diff --git a/src/render/Shader.cpp b/src/render/Shader.cpp index b494d79da..984d8ce35 100644 --- a/src/render/Shader.cpp +++ b/src/render/Shader.cpp @@ -17,7 +17,10 @@ CShader::~CShader() { } void CShader::destroy() { + if (program == 0) + return; + glDeleteProgram(program); program = 0; -} \ No newline at end of file +}