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]
google · Oct 4, 2024 · 9bd994e · 9bd994e
1 parent c3c9fe1
commit 9bd994e
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 21 deletions.
diff --git 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
@@ -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<void(OpenGLContext&)> 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
@@ -1732,7 +1732,7 @@ void OpenGLDriver::destroyIndexBuffer(Handle<HwIndexBuffer> ibh) {
     if (ibh) {
         auto& gl = mContext;
         GLIndexBuffer const* ib = handle_cast<const GLIndexBuffer*>(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<HwBufferObject> 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);
     }