From 3bd4c45d6e217ebbbcfee2be2cb6183f6f5d9a7e Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 4 Oct 2024 07:41:11 -0700 Subject: [PATCH] workaround Mesa glDeleteBuffers() bug Mesa always clears the generic binding if the buffer deleted is bound to an indexed binding, even if it's not bound to the generic binding. BUGS=[371324321] --- filament/backend/src/opengl/OpenGLContext.cpp | 45 +++++++++++-------- filament/backend/src/opengl/OpenGLContext.h | 10 ++++- filament/backend/src/opengl/OpenGLDriver.cpp | 4 +- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp index a7933407bb4..826994c3656 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -552,6 +552,14 @@ void OpenGLContext::initBugs(Bugs* bugs, Extensions const& exts, } else if (strstr(renderer, "AMD") || strstr(renderer, "ATI")) { // AMD/ATI GPU + } else if (strstr(vendor, "Mesa")) { + // Seen on + // [Mesa], + // [llvmpipe (LLVM 17.0.6, 256 bits)], + // [4.5 (Core Profile) Mesa 24.0.6-1], + // [4.50] + // not known which version are affected + bugs->rebind_buffer_after_deletion = true; } else if (strstr(renderer, "Mozilla")) { bugs->disable_invalidate_framebuffer = true; } @@ -929,15 +937,19 @@ void OpenGLContext::unbindSampler(GLuint sampler) noexcept { } } -void OpenGLContext::deleteBuffers(GLsizei n, const GLuint* buffers, GLenum target) noexcept { - glDeleteBuffers(n, buffers); +void OpenGLContext::deleteBuffer(GLuint buffer, GLenum target) noexcept { + glDeleteBuffers(1, &buffer); + // bindings of bound buffers are reset to 0 - const size_t targetIndex = getIndexForBufferTarget(target); - auto& genericBuffer = state.buffers.genericBinding[targetIndex]; - UTILS_NOUNROLL - for (GLsizei i = 0; i < n; ++i) { - if (genericBuffer == buffers[i]) { - genericBuffer = 0; + size_t const targetIndex = getIndexForBufferTarget(target); + auto& genericBinding = state.buffers.genericBinding[targetIndex]; + if (genericBinding == buffer) { + genericBinding = 0; + } + + if (UTILS_UNLIKELY(bugs.rebind_buffer_after_deletion)) { + if (genericBinding) { + glBindBuffer(target, genericBinding); } } @@ -946,16 +958,13 @@ void OpenGLContext::deleteBuffers(GLsizei n, const GLuint* buffers, GLenum targe (target != GL_UNIFORM_BUFFER && target != GL_TRANSFORM_FEEDBACK_BUFFER)); if (target == GL_UNIFORM_BUFFER || target == GL_TRANSFORM_FEEDBACK_BUFFER) { - auto& indexedBuffer = state.buffers.targets[targetIndex]; - UTILS_NOUNROLL // clang generates >1 KiB of code!! - for (GLsizei i = 0; i < n; ++i) { - UTILS_NOUNROLL - for (auto& buffer : indexedBuffer.buffers) { - if (buffer.name == buffers[i]) { - buffer.name = 0; - buffer.offset = 0; - buffer.size = 0; - } + auto& indexedBinding = state.buffers.targets[targetIndex]; + UTILS_NOUNROLL + for (auto& entry: indexedBinding.buffers) { + if (entry.name == buffer) { + entry.name = 0; + entry.offset = 0; + entry.size = 0; } } } diff --git a/filament/backend/src/opengl/OpenGLContext.h b/filament/backend/src/opengl/OpenGLContext.h index 4544ca104fc..39ceb346787 100644 --- a/filament/backend/src/opengl/OpenGLContext.h +++ b/filament/backend/src/opengl/OpenGLContext.h @@ -190,7 +190,7 @@ class OpenGLContext final : public TimerQueryFactoryInterface { inline void viewport(GLint left, GLint bottom, GLsizei width, GLsizei height) noexcept; inline void depthRange(GLclampf near, GLclampf far) noexcept; - void deleteBuffers(GLsizei n, const GLuint* buffers, GLenum target) noexcept; + void deleteBuffer(GLuint buffer, GLenum target) noexcept; void deleteVertexArray(GLuint vao) noexcept; void destroyWithContext(size_t index, std::function const& closure) noexcept; @@ -316,10 +316,15 @@ class OpenGLContext final : public TimerQueryFactoryInterface { // a glFinish. So we must delay the destruction until we know the GPU is finished. bool delay_fbo_destruction; + // Mesa sometimes clears the generic buffer binding when *another* buffer is destroyed, + // if that other buffer is bound on an *indexed* buffer binding. + bool rebind_buffer_after_deletion; + // Force feature level 0. Typically used for low end ES3 devices with significant driver // bugs or performance issues. bool force_feature_level0; + } bugs = {}; // state getters -- as needed. @@ -554,6 +559,9 @@ class OpenGLContext final : public TimerQueryFactoryInterface { { bugs.delay_fbo_destruction, "delay_fbo_destruction", ""}, + { bugs.rebind_buffer_after_deletion, + "rebind_buffer_after_deletion", + ""}, { bugs.force_feature_level0, "force_feature_level0", ""}, diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 20cd4f49c49..a606d090870 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -1732,7 +1732,7 @@ void OpenGLDriver::destroyIndexBuffer(Handle ibh) { if (ibh) { auto& gl = mContext; GLIndexBuffer const* ib = handle_cast(ibh); - gl.deleteBuffers(1, &ib->gl.buffer, GL_ELEMENT_ARRAY_BUFFER); + gl.deleteBuffer(ib->gl.buffer, GL_ELEMENT_ARRAY_BUFFER); destruct(ibh, ib); } } @@ -1745,7 +1745,7 @@ void OpenGLDriver::destroyBufferObject(Handle boh) { if (UTILS_UNLIKELY(bo->bindingType == BufferObjectBinding::UNIFORM && gl.isES2())) { free(bo->gl.buffer); } else { - gl.deleteBuffers(1, &bo->gl.id, bo->gl.binding); + gl.deleteBuffer(bo->gl.id, bo->gl.binding); } destruct(boh, bo); }