Hack around FBO/VAO sharability issues.

We have a problem when trying to delete an EffectChain or ResourcePool;
we might have created FBOs or VAOs in the wrong context. Work around it
for now (unbreaking Kdenlive) by making VAOs non-persistent again,
and simply never deleting FBOs (leaking them).

A proper solution here will be hard, unfortunately, and will nede some thought.

diff --git a/effect_chain.cpp b/effect_chain.cpp
index e4ab7199c8d23320c545d47c98b4c53dec39af3b..ca6905bf53c177f3f5988dc2f08173b3b495584a 100644 (file)
--- a/effect_chain.cpp
+++ b/effect_chain.cpp
@@ -53,15 +53,6 @@ EffectChain::~EffectChain()
                delete nodes[i];
        }
        for (unsigned i = 0; i < phases.size(); ++i) {
-               glBindVertexArray(phases[i]->vao);
-               check_error();
-
-               cleanup_vertex_attribute(phases[i]->glsl_program_num, "position", phases[i]->position_vbo);
-               cleanup_vertex_attribute(phases[i]->glsl_program_num, "texcoord", phases[i]->texcoord_vbo);
-
-               glBindVertexArray(0);
-               check_error();
-
                resource_pool->release_glsl_program(phases[i]->glsl_program_num);
                delete phases[i];
        }
@@ -291,27 +282,6 @@ void EffectChain::compile_glsl_program(Phase *phase)
        frag_shader.append(read_file("footer.frag"));
 
        phase->glsl_program_num = resource_pool->compile_glsl_program(read_file("vs.vert"), frag_shader);
-
-       // Prepare the geometry for the fullscreen quad used in this phase.
-       // (We have separate VAOs per shader, since the bindings can in theory
-       // be different.)
-       float vertices[] = {
-               0.0f, 1.0f,
-               0.0f, 0.0f,
-               1.0f, 1.0f,
-               1.0f, 0.0f
-       };
-
-       glGenVertexArrays(1, &phase->vao);
-       check_error();
-       glBindVertexArray(phase->vao);
-       check_error();
-
-       phase->position_vbo = fill_vertex_attribute(phase->glsl_program_num, "position", 2, GL_FLOAT, sizeof(vertices), vertices);
-       phase->texcoord_vbo = fill_vertex_attribute(phase->glsl_program_num, "texcoord", 2, GL_FLOAT, sizeof(vertices), vertices);  // Same as vertices.
-
-       glBindVertexArray(0);
-       check_error();
 }
 
 // Construct GLSL programs, starting at the given effect and following
@@ -1484,8 +1454,6 @@ void EffectChain::render_to_fbo(GLuint dest_fbo, unsigned width, unsigned height
 
        glBindFramebuffer(GL_FRAMEBUFFER, 0);
        check_error();
-       glBindVertexArray(0);
-       check_error();
        glUseProgram(0);
        check_error();
 }
@@ -1547,11 +1515,32 @@ void EffectChain::execute_phase(Phase *phase, bool last_phase, map<Phase *, GLui
                }
        }
 
-       glBindVertexArray(phase->vao);
+       // Now draw!
+       float vertices[] = {
+               0.0f, 1.0f,
+               0.0f, 0.0f,
+               1.0f, 1.0f,
+               1.0f, 0.0f
+       };
+
+       GLuint vao;
+       glGenVertexArrays(1, &vao);
+       check_error();
+       glBindVertexArray(vao);
        check_error();
+
+       GLuint position_vbo = fill_vertex_attribute(glsl_program_num, "position", 2, GL_FLOAT, sizeof(vertices), vertices);
+       GLuint texcoord_vbo = fill_vertex_attribute(glsl_program_num, "texcoord", 2, GL_FLOAT, sizeof(vertices), vertices);  // Same as vertices.
+
        glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
        check_error();
 
+       cleanup_vertex_attribute(glsl_program_num, "position", position_vbo);
+       cleanup_vertex_attribute(glsl_program_num, "texcoord", texcoord_vbo);
+       
+       glUseProgram(0);
+       check_error();
+
        for (unsigned i = 0; i < phase->effects.size(); ++i) {
                Node *node = phase->effects[i];
                node->effect->clear_gl_state();
@@ -1560,6 +1549,9 @@ void EffectChain::execute_phase(Phase *phase, bool last_phase, map<Phase *, GLui
        if (!last_phase) {
                resource_pool->release_fbo(fbo);
        }
+
+       glDeleteVertexArrays(1, &vao);
+       check_error();
 }
 
 void EffectChain::setup_rtt_sampler(GLuint glsl_program_num, int sampler_num, const string &effect_id, bool use_mipmaps)

diff --git a/effect_chain.h b/effect_chain.h
index 903018bc3ab87a60dd159e3ff13d8edcec0b3171..6452d14952dbbda0a67fe879e45c4b96cd86d37f 100644 (file)
--- a/effect_chain.h
+++ b/effect_chain.h
@@ -99,13 +99,6 @@ struct Phase {
        // Identifier used to create unique variables in GLSL.
        // Unique per-phase to increase cacheability of compiled shaders.
        std::map<Node *, std::string> effect_ids;
-
-       // The geometry needed to draw this quad, bound to the vertex array
-       // object. (Seemingly it's actually a win not to upload geometry every
-       // frame, even for something as small as a quad, due to fewer state
-       // changes.)
-       GLuint vao;
-       GLuint position_vbo, texcoord_vbo;
 };
 
 class EffectChain {

diff --git a/resource_pool.cpp b/resource_pool.cpp
index d322a1764f96ad43a8557f947127b8674262d8f6..201b73c73b7a9ee4855149886a2ec01bc75a6469 100644 (file)
--- a/resource_pool.cpp
+++ b/resource_pool.cpp
@@ -58,8 +58,9 @@ ResourcePool::~ResourcePool()
                GLuint free_fbo_num = *freelist_it;
                assert(fbo_formats.count(free_fbo_num) != 0);
                fbo_formats.erase(free_fbo_num);
-               glDeleteFramebuffers(1, &free_fbo_num);
-               check_error();
+               // TODO: We currently leak due to FBO sharability issues.
+               // glDeleteFramebuffers(1, &free_fbo_num);
+               // check_error();
        }
        assert(fbo_formats.empty());
 }
@@ -263,7 +264,8 @@ void ResourcePool::release_2d_texture(GLuint texture_num)
                        assert(format_it != fbo_formats.end());
                        if (format_it->second.texture_num == free_texture_num) {
                                fbo_formats.erase(fbo_num);
-                               glDeleteFramebuffers(1, &fbo_num);
+                               // TODO: We currently leak due to FBO sharability issues.
+                               // glDeleteFramebuffers(1, &fbo_num);
                                fbo_freelist.erase(fbo_freelist_it++);
                        } else {
                                ++fbo_freelist_it;
@@ -330,8 +332,9 @@ void ResourcePool::release_fbo(GLuint fbo_num)
                fbo_freelist.pop_front();
                assert(fbo_formats.count(free_fbo_num) != 0);
                fbo_formats.erase(free_fbo_num);
-               glDeleteFramebuffers(1, &free_fbo_num);
-               check_error();
+               // TODO: We currently leak due to FBO sharability issues.
+               // glDeleteFramebuffers(1, &free_fbo_num);
+               // check_error();
        }
        pthread_mutex_unlock(&lock);
 }