From 983fe15061b6e199877577b363a9f2fa102cf107 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Tue, 25 Mar 2014 02:15:16 +0100 Subject: [PATCH 1/1] 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. --- effect_chain.cpp | 58 ++++++++++++++++++++--------------------------- effect_chain.h | 7 ------ resource_pool.cpp | 13 +++++++---- 3 files changed, 33 insertions(+), 45 deletions(-) diff --git a/effect_chain.cpp b/effect_chain.cpp index e4ab719..ca6905b 100644 --- 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, mapvao); + // 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, maprelease_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 903018b..6452d14 100644 --- 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 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 d322a17..201b73c 100644 --- 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); } -- 2.39.5