Fix framebuffer allocation and deallocation

Ensure that the framebuffer resource management is watertight. glDeleteFramebuffers() was being called on uninitalized memory - looks like something left over from debugging.
c42f · Aug 31, 2016 · 6db6816 · 6db6816
1 parent 3486965
commit 6db6816
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 29 deletions.
diff --git a/src/render/View3D.cpp b/src/render/View3D.cpp
@@ -43,7 +43,7 @@ View3D::View3D(GeometryCollection* geometries, const QGLFormat& format, QWidget
     m_selectionModel(0),
     m_shaderParamsUI(0),
     m_incrementalFrameTimer(0),
-    m_incrementalFramebuffer(0),
+    m_incrementalFramebuffer(),
     m_incrementalDraw(false),
     m_drawAxesBackground(QImage(":/resource/axes.png")),
     m_drawAxesLabelX(QImage(":/resource/x.png")),
@@ -287,10 +287,7 @@ void View3D::initializeGL()
     int w = width() * dPR;
     int h = height() * dPR;
 
-    m_incrementalFramebuffer = allocIncrementalFramebuffer(w, h);
-
-    glFrameBufferStatus(m_incrementalFramebuffer);
-    glCheckError();
+    m_incrementalFramebuffer.init(w, h);
 
     initializeGLGeometry(0, m_geometries->get().size());
 
@@ -321,7 +318,7 @@ void View3D::resizeGL(int w, int h)
 
     m_camera.setViewport(QRect(0,0,double(w)/dPR,double(h)/dPR));
 
-    m_incrementalFramebuffer = allocIncrementalFramebuffer(w,h);
+    m_incrementalFramebuffer.init(w,h);
 
     glCheckError();
 }
@@ -388,7 +385,7 @@ void View3D::paintGL()
         resizeGL(w, h);
     }
 
-    glBindFramebuffer(GL_DRAW_FRAMEBUFFER, m_incrementalFramebuffer);
+    glBindFramebuffer(GL_DRAW_FRAMEBUFFER, m_incrementalFramebuffer.id());
 
     //--------------------------------------------------
     // Draw main scene
@@ -444,7 +441,7 @@ void View3D::paintGL()
 
     // TODO: this should really render a texture onto a quad and not use glBlitFramebuffer
     glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
-    glBindFramebuffer(GL_READ_FRAMEBUFFER, m_incrementalFramebuffer);
+    glBindFramebuffer(GL_READ_FRAMEBUFFER, m_incrementalFramebuffer.id());
     glBlitFramebuffer(0,0,w,h, 0,0,w,h,
                       GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT, GL_NEAREST); // has to be GL_NEAREST to work with DEPTH
 
@@ -487,9 +484,6 @@ void View3D::paintGL()
         m_incrementalFrameTimer->start(10);
 
     m_incrementalDraw = true;
-
-    glFrameBufferStatus(m_incrementalFramebuffer);
-    glCheckError();
 }
 
 void View3D::drawMeshes(const TransformState& transState,

diff --git a/src/render/View3D.h b/src/render/View3D.h
@@ -145,7 +145,7 @@ class View3D : public QGLWidget
         QWidget* m_shaderParamsUI;
         /// Timer for next incremental frame
         QTimer* m_incrementalFrameTimer;
-        unsigned int m_incrementalFramebuffer;
+        Framebuffer m_incrementalFramebuffer;
         bool m_incrementalDraw;
         /// Controller for amount of geometry to draw
         DrawCostModel m_drawCostModel;

diff --git a/src/render/glutil.h b/src/render/glutil.h
@@ -24,6 +24,29 @@
 #include "util.h"
 #include "typespec.h"
 
+
+//-------------------------------------------------------------------------------
+// OpenGL Debug tools
+void _glError(const char *file, int line);
+void _glFrameBufferStatus(GLenum target, const char *file, int line);
+
+
+// #define GL_CHECK
+#ifdef GL_CHECK
+
+    #define glCheckError() _glError(__FILE__,__LINE__)
+    #define glFrameBufferStatus(TARGET) _glFrameBufferStatus(TARGET, __FILE__,__LINE__)
+
+#else
+
+    #define glCheckError()
+    #define glFrameBufferStatus(TARGET)
+
+#endif //GL_CHECK
+
+
+
+//-------------------------------------------------------------------------------
 struct TransformState
 {
     Imath::V2i viewSize;
@@ -185,6 +208,71 @@ class GlBuffer
 };
 
 
+//------------------------------------------------------------------------------
+/// Framebuffer resource wrapper with color and depth attachements for the
+/// particular framebuffer settings needed in the incremental framebuffer in
+/// View3D.cpp
+class Framebuffer
+{
+    public:
+        /// Generate an uninitialized framebuffer.
+        Framebuffer()
+            : m_fbo(0), m_colorBuf(0), m_zBuf(0)
+        { }
+
+        ~Framebuffer()
+        {
+            destroy();
+        }
+
+        /// Initialize framebuffer with given width and height.
+        void init(int width, int height)
+        {
+            destroy();
+
+            glGenFramebuffers(1, &m_fbo);
+            glBindFramebuffer(GL_DRAW_FRAMEBUFFER, m_fbo);
+
+            glGenRenderbuffers(1, &m_colorBuf);
+            glBindRenderbuffer(GL_RENDERBUFFER, m_colorBuf);
+            glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, width, height);
+            glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, m_colorBuf);
+
+            glGenRenderbuffers(1, &m_zBuf);
+            glBindRenderbuffer(GL_RENDERBUFFER, m_zBuf);
+            glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_COMPONENT24, width, height);
+            glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, m_zBuf);
+
+            glFrameBufferStatus(m_incrementalFramebuffer.id());
+            glCheckError();
+        }
+
+        void destroy()
+        {
+            if (m_fbo != 0)
+                glDeleteFramebuffers(1, &m_fbo);
+            if (m_colorBuf != 0)
+                glDeleteRenderbuffers(1, &m_colorBuf);
+            if (m_zBuf != 0)
+                glDeleteRenderbuffers(1, &m_zBuf);
+            m_fbo = 0;
+            m_colorBuf = 0;
+            m_zBuf = 0;
+        }
+
+        /// Get OpenGL identifier for the buffer
+        GLuint id() const
+        {
+            assert(m_fbo != 0);
+            return m_fbo;
+        }
+
+    private:
+        GLuint m_fbo;
+        GLuint m_colorBuf;
+        GLuint m_zBuf;
+};
+
 
 //------------------------------------------------------------------------------
 // Texture utility
@@ -281,21 +369,4 @@ const ShaderAttribute* findAttr(const std::string& name,
                                 const std::vector<ShaderAttribute>& attrs);
 
 
-void _glError(const char *file, int line);
-void _glFrameBufferStatus(GLenum target, const char *file, int line);
-
-
-// #define GL_CHECK
-#ifdef GL_CHECK
-
-    #define glCheckError() _glError(__FILE__,__LINE__)
-    #define glFrameBufferStatus(TARGET) _glFrameBufferStatus(TARGET, __FILE__,__LINE__)
-
-#else
-
-    #define glCheckError()
-    #define glFrameBufferStatus(TARGET)
-
-#endif //GL_CHECK
-
 #endif // GLUTIL_H_INCLUDED