Better OpenGL error checking + opengl header refactoring

Makefile

@@ -62,6 +62,11 @@ profile_cli:build
 	LD_LIBRARY_PATH=${LIBRARY_PATH} perf record -g ${BUILDPATH}/interface/commandline/paysages-cli $(ARGS)
 	perf report -g
 
+gltrace:build
+	rm -f *.trace
+	LD_PRELOAD=/usr/lib/x86_64-linux-gnu/apitrace/wrappers/glxtrace.so LD_LIBRARY_PATH=$(LIBRARY_PATH) ${BUILDPATH}/interface/modeler/quickapp/paysages-modeler $(ARGS)
+	qapitrace paysages-modeler.trace
+
 package:build
 	rm -rf paysages3d-linux
 	rm -f paysages3d-linux.tar.bz2

src/definition/DefinitionNode.cpp

@@ -110,8 +110,8 @@ bool DefinitionNode::applyDiff(const DefinitionDiff *diff, bool) {
     if (diff->getTypeName() == type_name) {
         return true;
     } else {
-        Logs::error() << "Can't apply " << diff->getTypeName() << " diff to " << getName() << " " << type_name
+        Logs::error() << "[Definition] Can't apply " << diff->getTypeName() << " diff to " << getName() << " "
-                      << " node" << std::endl;
+                      << type_name << " node" << std::endl;
         return false;
     }
 }
@@ -155,8 +155,8 @@ void DefinitionNode::save(PackStream *stream) const {
             child->save(stream);
         } else {
             // Child size not known, write it to a temporary stream to know it
-            Logs::debug() << "Unknown size for child " << child->name << ", unefficient writing to temporary stream"
+            Logs::debug() << "[Definition] Unknown size for child " << child->name
-                          << std::endl;
+                          << ", unefficient writing to temporary stream" << std::endl;
             PackStream substream;
             child->save(&substream);
             stream->writeFromBuffer(substream, true);
@@ -183,7 +183,7 @@ void DefinitionNode::load(PackStream *stream) {
             // TODO Ask subclass if it can instanciate a child
             // Else skip length of unknown child
             stream->skipBytes(child_size);
-            Logs::warning() << "Skipped unknown child '" << child_name << "'" << std::endl;
+            Logs::warning() << "[Definition] Skipped unknown child '" << child_name << "'" << std::endl;
         }
     }
 }
@@ -196,12 +196,13 @@ void DefinitionNode::copy(DefinitionNode *destination) const {
             if (dest_child) {
                 child->copy(dest_child);
             } else {
-                Logs::warning() << "Can't copy to child " << child->name << " of " << destination->getTypeName()
+                Logs::warning() << "[Definition] Can't copy to child " << child->name << " of "
-                                << std::endl;
+                                << destination->getTypeName() << std::endl;
             }
         }
     } else {
-        Logs::error() << "Can't copy from " << getTypeName() << " to " << destination->getTypeName() << std::endl;
+        Logs::error() << "[Definition] Can't copy from " << getTypeName() << " to " << destination->getTypeName()
+                      << std::endl;
     }
 }
 
@@ -225,7 +226,7 @@ void DefinitionNode::removeChild(DefinitionNode *child) {
         child->parent = NULL;
         children.erase(it);
     } else {
-        Logs::warning() << "Trying to remove not found child '" << child->name << "' from '" << name << "'"
+        Logs::warning() << "[Definition] Trying to remove not found child '" << child->name << "' from '" << name << "'"
                         << std::endl;
     }
 }

src/definition/Layers.cpp

@@ -119,7 +119,7 @@ bool Layers::applyDiff(const DefinitionDiff *diff, bool backward) {
 
 void Layers::generateInitDiffs(std::vector<const DefinitionDiff *> *diffs) const {
     int i = 0;
-    for (auto layer: layers) {
+    for (auto layer : layers) {
         auto diff = new LayersDiff(this, LayersDiff::LAYER_ADDED, i++);
         diff->saveLayer(*layer);
         diffs->push_back(diff);

src/definition/Scenery.cpp

@@ -47,7 +47,7 @@ Scenery::FileOperationResult Scenery::saveGlobal(const std::string &filepath) co
     stream.write(&version_header);
     stream.write(&app_header);
 
-    Logs::debug() << "Scenery saved to file: " << filepath << std::endl;
+    Logs::debug() << "[Definition] Scenery saved to file: " << filepath << std::endl;
     return FILE_OPERATION_OK;
 }
 
@@ -85,7 +85,7 @@ Scenery::FileOperationResult Scenery::loadGlobal(const std::string &filepath) {
         return FILE_OPERATION_APP_MISMATCH;
     }
 
-    Logs::debug() << "Scenery loaded from file: " << filepath << std::endl;
+    Logs::debug() << "[Definition] Scenery loaded from file: " << filepath << std::endl;
     return FILE_OPERATION_OK;
 }
 
@@ -110,7 +110,7 @@ void Scenery::autoPreset(int seed) {
 
     validate();
 
-    Logs::debug() << "New scenery generated from seed " << seed << std::endl;
+    Logs::debug() << "[Definition] New scenery generated from seed " << seed << std::endl;
 }
 
 void Scenery::setAtmosphere(AtmosphereDefinition *atmosphere) {

src/interface/modeler/quickapp/MainModelerWindow.cpp

@@ -16,6 +16,11 @@
 #include <QGuiApplication>
 
 MainModelerWindow::MainModelerWindow() {
+    /*QSurfaceFormat new_format = format();
+    new_format.setVersion(3, 3);
+    new_format.setProfile(QSurfaceFormat::CoreProfile);
+    setFormat(new_format);*/
+
     scenery = new Scenery();
     scenery->autoPreset();
 

src/interface/modeler/quickapp/OpenGLView.cpp

@@ -20,7 +20,6 @@ OpenGLView::OpenGLView(QQuickItem *parent) : QQuickItem(parent) {
     connect(this, SIGNAL(windowChanged(QQuickWindow *)), this, SLOT(handleWindowChanged(QQuickWindow *)));
     connect(this, SIGNAL(widthChanged()), this, SLOT(handleResize()));
     connect(this, SIGNAL(heightChanged()), this, SLOT(handleResize()));
-    startTimer(50);
 }
 
 void OpenGLView::handleWindowChanged(QQuickWindow *win) {
@@ -28,12 +27,11 @@ void OpenGLView::handleWindowChanged(QQuickWindow *win) {
         window = qobject_cast<MainModelerWindow *>(win);
         if (window) {
             renderer = window->getRenderer();
-
+            initialized = false;
-            connect(win, SIGNAL(beforeRendering()), this, SLOT(paint()), Qt::DirectConnection);
 
             win->setClearBeforeRendering(false);
 
-            initialized = false;
+            connect(win, SIGNAL(sceneGraphInitialized()), this, SLOT(handleSceneGraphReady()));
         }
     }
 }
@@ -66,6 +64,11 @@ void OpenGLView::handleResize() {
     resized = true;
 }
 
+void OpenGLView::handleSceneGraphReady() {
+    connect(window, SIGNAL(beforeRendering()), this, SLOT(paint()), Qt::DirectConnection);
+    startTimer(50);
+}
+
 void OpenGLView::wheelEvent(QWheelEvent *event) {
     if (not acceptInputs()) {
         return;

src/interface/modeler/quickapp/OpenGLView.h

@@ -17,6 +17,7 @@ class OpenGLView : public QQuickItem {
     void handleWindowChanged(QQuickWindow *win);
     void paint();
     void handleResize();
+    void handleSceneGraphReady();
 
   protected:
     virtual void wheelEvent(QWheelEvent *event) override;

src/render/opengl/ExplorerChunkTerrain.cpp

@@ -1,8 +1,8 @@
 #include "ExplorerChunkTerrain.h"
 
-#include OPENGL_FUNCTIONS_INCLUDE
 #include <cmath>
 #include <QImage>
+#include "OpenGLFunctions.h"
 #include "ColorProfile.h"
 #include "CameraDefinition.h"
 #include "OpenGLRenderer.h"

src/render/opengl/OpenGLFunctions.h

@@ -0,0 +1,9 @@
+#ifndef OPENGLFUNCTIONS
+#define OPENGLFUNCTIONS
+
+#include "opengl_global.h"
+
+#define _OPENGL_FUNCTIONS_INCLUDE <OpenGLFunctions>
+#include _OPENGL_FUNCTIONS_INCLUDE
+
+#endif // OPENGLFUNCTIONS

src/render/opengl/OpenGLRenderer.cpp

@@ -1,6 +1,6 @@
 #include "OpenGLRenderer.h"
 
-#include OPENGL_FUNCTIONS_INCLUDE
+#include "OpenGLFunctions.h"
 #include "CameraDefinition.h"
 #include "OpenGLSharedState.h"
 #include "OpenGLSkybox.h"
@@ -58,10 +58,20 @@ OpenGLRenderer::~OpenGLRenderer() {
     delete shared_state;
 }
 
+void OpenGLRenderer::checkForErrors(const std::string &domain) {
+    int error_code;
+    while ((error_code = functions->glGetError()) != GL_NO_ERROR) {
+        Logs::warning() << "[OpenGL] Error in " << domain << " : " << error_code << std::endl;
+    }
+}
+
 void OpenGLRenderer::initialize() {
     ready = functions->initializeOpenGLFunctions();
 
     if (ready) {
+        Logs::debug() << "[OpenGL] renderer started (version " << functions->glGetString(GL_VERSION)
+                      << ", glsl version " << functions->glGetString(GL_SHADING_LANGUAGE_VERSION) << ")" << std::endl;
+
         prepareOpenGLState();
 
         prepare();
@@ -80,8 +90,10 @@ void OpenGLRenderer::initialize() {
         terrain->updateScenery();
 
         cameraChangeEvent(render_camera);
+
+        checkForErrors("initialize");
     } else {
-        Logs::error() << "Failed to initialize OpenGL bindings" << std::endl;
+        Logs::error() << "[OpenGL] Failed to initialize api bindings" << std::endl;
     }
 }
 
@@ -129,24 +141,28 @@ void OpenGLRenderer::resize(int width, int height) {
         cameraChangeEvent(render_camera);
 
         prepareOpenGLState();
+
+        checkForErrors("resize");
     }
 }
 
 void OpenGLRenderer::paint() {
     if (ready and not paused) {
+        checkForErrors("before_paint");
         functions->glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
 
         skybox->render();
+        checkForErrors("skybox");
+
         terrain->render();
+        checkForErrors("terrain");
+
         water->render();
+        checkForErrors("water");
 
         if (mouse_tracking) {
             updateMouseProjection();
-        }
+            checkForErrors("mouse_tracking");
-
-        int error_code;
-        while ((error_code = functions->glGetError()) != GL_NO_ERROR) {
-            Logs::warning() << "[OpenGL] ERROR : " << error_code << std::endl;
         }
 
         displayed = true;

src/render/opengl/OpenGLRenderer.h

@@ -31,6 +31,13 @@ class OPENGLSHARED_EXPORT OpenGLRenderer : public SoftwareRenderer {
         return displayed;
     }
 
+    /**
+     * Check for errors in OpenGL context.
+     *
+     * Will write the error on standard error output, with the *domain* specified.
+     */
+    void checkForErrors(const std::string &domain);
+
     void initialize();
     void prepareOpenGLState();
     void resize(int width, int height);

src/render/opengl/OpenGLShaderProgram.cpp

@@ -1,8 +1,8 @@
 #include "OpenGLShaderProgram.h"
 
-#include OPENGL_FUNCTIONS_INCLUDE
 #include <QOpenGLShaderProgram>
 #include <QDir>
+#include "OpenGLFunctions.h"
 #include "OpenGLRenderer.h"
 #include "OpenGLSharedState.h"
 #include "Texture2D.h"
@@ -27,7 +27,7 @@ void OpenGLShaderProgram::addVertexSource(QString path) {
     if (file.open(QIODevice::ReadOnly | QIODevice::Text)) {
         source_vertex += QString(file.readAll()).toStdString();
     } else {
-        Logs::error() << "Can't open vertex file " << file.fileName().toStdString() << std::endl;
+        Logs::error() << "[OpenGL] Can't open vertex file " << file.fileName().toStdString() << std::endl;
     }
 }
 
@@ -36,7 +36,7 @@ void OpenGLShaderProgram::addFragmentSource(QString path) {
     if (file.open(QIODevice::ReadOnly | QIODevice::Text)) {
         source_fragment += QString(file.readAll()).toStdString();
     } else {
-        Logs::error() << "Can't open fragment file " << file.fileName().toStdString() << std::endl;
+        Logs::error() << "[OpenGL] Can't open fragment file " << file.fileName().toStdString() << std::endl;
     }
 }
 
@@ -45,24 +45,29 @@ void OpenGLShaderProgram::compile() {
     program->addShaderFromSourceCode(QOpenGLShader::Fragment, QString::fromStdString(source_fragment));
 
     if (not program->link()) {
-        Logs::warning() << "Error while compiling shader " << name << std::endl
+        Logs::warning() << "[OpenGL] Error while compiling shader " << name << std::endl
                         << program->log().toStdString() << std::endl;
     } else if (program->log().length() > 0) {
-        Logs::debug() << "Shader " << name << " compilation output:" << std::endl
+        Logs::debug() << "[OpenGL] Shader " << name << " compilation output:" << std::endl
                       << program->log().toStdString() << std::endl;
+    } else {
+        Logs::debug() << "[OpenGL] Shader " << name << " compiled" << std::endl;
     }
 }
 
-void OpenGLShaderProgram::bind() {
+bool OpenGLShaderProgram::bind() {
     if (not compiled) {
         compile();
         compiled = true;
     }
 
-    program->bind();
+    if (program->bind()) {
-
         int texture_unit = 0;
         renderer->getSharedState()->apply(this, texture_unit);
+        return true;
+    } else {
+        return false;
+    }
 }
 
 void OpenGLShaderProgram::release() {
@@ -70,8 +75,7 @@ void OpenGLShaderProgram::release() {
 }
 
 void OpenGLShaderProgram::drawTriangles(float *vertices, int triangle_count) {
-    bind();
+    if (bind()) {
-
         GLuint vertex = program->attributeLocation("vertex");
         program->setAttributeArray(vertex, GL_FLOAT, vertices, 3);
         program->enableAttributeArray(vertex);
@@ -81,11 +85,11 @@ void OpenGLShaderProgram::drawTriangles(float *vertices, int triangle_count) {
         program->disableAttributeArray(vertex);
 
         release();
+    }
 }
 
 void OpenGLShaderProgram::drawTriangleStrip(float *vertices, int vertex_count) {
-    bind();
+    if (bind()) {
-
         GLuint vertex = program->attributeLocation("vertex");
         program->setAttributeArray(vertex, GL_FLOAT, vertices, 3);
         program->enableAttributeArray(vertex);
@@ -95,4 +99,5 @@ void OpenGLShaderProgram::drawTriangleStrip(float *vertices, int vertex_count) {
         program->disableAttributeArray(vertex);
 
         release();
+    }
 }

src/render/opengl/OpenGLShaderProgram.h

@@ -21,7 +21,7 @@ class OPENGLSHARED_EXPORT OpenGLShaderProgram {
     void drawTriangles(float *vertices, int triangle_count);
     void drawTriangleStrip(float *vertices, int vertex_count);
 
-    void bind();
+    bool bind();
     void release();
 
     inline QOpenGLShaderProgram *getProgram() const {

src/render/opengl/OpenGLSharedState.cpp

@@ -3,8 +3,7 @@
 OpenGLSharedState::OpenGLSharedState() {
 }
 
-paysages::opengl::OpenGLSharedState::~OpenGLSharedState()
+paysages::opengl::OpenGLSharedState::~OpenGLSharedState() {
-{
     for (const auto &pair : variables) {
         delete pair.second;
     }

src/render/opengl/OpenGLTerrain.cpp

@@ -1,6 +1,6 @@
 #include "OpenGLTerrain.h"
 
-#include OPENGL_FUNCTIONS_INCLUDE
+#include "OpenGLFunctions.h"
 #include "OpenGLRenderer.h"
 #include "OpenGLShaderProgram.h"
 #include "ParallelPool.h"

src/render/opengl/OpenGLVariable.cpp

@@ -1,8 +1,8 @@
 #include "OpenGLVariable.h"
 
-#include OPENGL_FUNCTIONS_INCLUDE
 #include <QOpenGLShaderProgram>
 #include <cassert>
+#include "OpenGLFunctions.h"
 #include "OpenGLRenderer.h"
 #include "OpenGLShaderProgram.h"
 #include "Vector3.h"

src/render/opengl/VertexArray.h

@@ -3,9 +3,9 @@
 
 #include "opengl_global.h"
 
-#include OPENGL_FUNCTIONS_INCLUDE
 #include <cassert>
 #include <QOpenGLShaderProgram>
+#include "OpenGLFunctions.h"
 
 namespace paysages {
 namespace opengl {

src/render/opengl/opengl_global.h

@@ -25,9 +25,8 @@ template <typename Vertex> class VertexArray;
 }
 using namespace paysages::opengl;
 
-//#define OpenGLFunctions QOpenGLFunctions_3_2_Core
 #define OpenGLFunctions QOpenGLFunctions_3_0
-#define OPENGL_FUNCTIONS_INCLUDE <OpenGLFunctions>
+//#define OpenGLFunctions QOpenGLFunctions_3_3_Core
 class OpenGLFunctions;
 
 #endif // OPENGL_GLOBAL_H

src/tests/Layers_Test.cpp

@@ -187,7 +187,7 @@ TEST(Layers, generateInitDiffs) {
     EXPECT_EQ(3, (int)diffs.size());
 
     Layers layers1(NULL, "layers", _construc1);
-    for (auto diff: diffs) {
+    for (auto diff : diffs) {
         layers1.applyDiff(diff);
     }
     ASSERT_EQ(3, layers1.getLayerCount());