Replace deprecated GL client arrays with VAO + glVertexAttribPointer

Author: tomas-soltys

src/fea/CMakeLists.txt

@@ -65,14 +65,14 @@ qt_add_executable(fea
     src/gl_element_group.cpp
     src/gl_entity.cpp
     src/gl_entity_list.cpp
+    src/gl_list.cpp
     src/gl_functions.cpp
     src/gl_frustum.cpp
     src/gl_shader_program.cpp
     src/gl_grid.cpp
     src/gl_interpolated_element.cpp
     src/gl_interpolated_entity.cpp
     src/gl_line.cpp
-    src/gl_list.cpp
     src/gl_model_list.cpp
     src/gl_node.cpp
     src/gl_object.cpp
@@ -244,12 +244,12 @@ qt_add_executable(fea
     src/gl_element_group.h
     src/gl_entity.h
     src/gl_entity_list.h
+    src/gl_list.h
     src/gl_functions.h
     src/gl_grid.h
     src/gl_interpolated_element.h
     src/gl_interpolated_entity.h
     src/gl_line.h
-    src/gl_list.h
     src/gl_model_list.h
     src/gl_node.h
     src/gl_object.h

src/fea/src/gl_element_group.cpp

@@ -398,12 +398,10 @@ void GLElementGroup::draw()
     {
         if (pGlEntityList->getUseVBO())
         {
-            // Render VBO
             pGlEntityList->getVBO(GL_ENTITY_LIST_ITEM_NORMAL).render();
         }
         else
         {
-            // Render display list
             pGlEntityList->callList(GL_ENTITY_LIST_ITEM_NORMAL);
         }
     }

src/fea/src/gl_shader_program.cpp

@@ -29,6 +29,14 @@ bool GLShaderProgram::load(const QString &vertexPath, const QString &fragmentPat
         return false;
     }
 
+    // Bind standard vertex attribute locations before linking.
+    // These match the glVertexAttribPointer calls in GLVertexBuffer::uploadToGPU().
+    // GLSL 1.30 does not support layout(location=N), so explicit binding is required.
+    this->program.bindAttributeLocation("aPosition", 0);
+    this->program.bindAttributeLocation("aNormal",   1);
+    this->program.bindAttributeLocation("aColor",    2);
+    this->program.bindAttributeLocation("aTexCoord", 3);
+
     if (!this->program.link())
     {
         RLogger::error("GLShaderProgram: link failed: %s\n",

src/fea/src/gl_state_cache.cpp

@@ -134,6 +134,14 @@ void GLStateCache::setTexture1D(GLboolean enabled)
         this->texture1D = enabled;
         GL_SAFE_CALL(enabled ? glEnable(GL_TEXTURE_1D) : glDisable(GL_TEXTURE_1D));
     }
+    // Keep the shader uniform in sync (mirrors setLighting() pattern).
+    // Guard against VBO recording: during recording the enable/disable is per-element
+    // and the uniform is a single value for the whole draw call, so skip the sync
+    // and let the caller set uUseTexture explicitly before VBO.render().
+    if (this->shaderProgram && !GLFunctions::isRecordingVBO())
+    {
+        this->shaderProgram->setUniformBool("uUseTexture", enabled == GL_TRUE);
+    }
 }
 
 void GLStateCache::setTexture2D(GLboolean enabled)

src/fea/src/gl_vertex_buffer.cpp

@@ -1,5 +1,10 @@
 #include <cstddef>
 
+#ifdef __APPLE__
+#define GL_SILENCE_DEPRECATION
+#endif
+
+#include <QOpenGLExtraFunctions>
 #include <QOpenGLFunctions>
 #include <QOpenGLContext>
 
@@ -11,6 +16,7 @@
 void GLVertexBuffer::_init(const GLVertexBuffer *pBuffer)
 {
     this->vboId = 0;
+    this->vaoId = 0;
     this->vertexCount = 0;
     this->valid = false;
     this->recording = false;
@@ -110,6 +116,7 @@ void GLVertexBuffer::uploadToGPU()
 
     QOpenGLContext *ctx = QOpenGLContext::currentContext();
     QOpenGLFunctions *f = ctx->functions();
+    QOpenGLExtraFunctions *ef = ctx->extraFunctions();
 
     // Generate VBO if needed (reuse existing handle if present).
     if (this->vboId == 0)
@@ -123,6 +130,36 @@ void GLVertexBuffer::uploadToGPU()
                                  GLsizeiptr(this->vertices.size() * sizeof(GLVertexData)),
                                  this->vertices.data(),
                                  GL_STATIC_DRAW));
+
+    // Create the VAO (or recreate it after a reset) and record the attribute layout
+    // while the VBO is still bound so the VAO captures the binding.
+    if (this->vaoId == 0)
+    {
+        GL_SAFE_CALL(ef->glGenVertexArrays(1, &this->vaoId));
+    }
+    GL_SAFE_CALL(ef->glBindVertexArray(this->vaoId));
+
+    // aPosition — location 0
+    GL_SAFE_CALL(f->glEnableVertexAttribArray(0));
+    GL_SAFE_CALL(f->glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, sizeof(GLVertexData),
+        reinterpret_cast<const void*>(offsetof(GLVertexData, position))));
+
+    // aNormal — location 1
+    GL_SAFE_CALL(f->glEnableVertexAttribArray(1));
+    GL_SAFE_CALL(f->glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(GLVertexData),
+        reinterpret_cast<const void*>(offsetof(GLVertexData, normal))));
+
+    // aColor — location 2 (UNSIGNED_BYTE normalised to [0,1] in shader)
+    GL_SAFE_CALL(f->glEnableVertexAttribArray(2));
+    GL_SAFE_CALL(f->glVertexAttribPointer(2, 4, GL_UNSIGNED_BYTE, GL_TRUE, sizeof(GLVertexData),
+        reinterpret_cast<const void*>(offsetof(GLVertexData, color))));
+
+    // aTexCoord — location 3
+    GL_SAFE_CALL(f->glEnableVertexAttribArray(3));
+    GL_SAFE_CALL(f->glVertexAttribPointer(3, 1, GL_FLOAT, GL_FALSE, sizeof(GLVertexData),
+        reinterpret_cast<const void*>(offsetof(GLVertexData, texCoord))));
+
+    GL_SAFE_CALL(ef->glBindVertexArray(0));
     GL_SAFE_CALL(f->glBindBuffer(GL_ARRAY_BUFFER, 0));
 
     this->vertexCount = GLsizei(this->vertices.size());
@@ -178,43 +215,23 @@ void GLVertexBuffer::addVertex(GLfloat x, GLfloat y, GLfloat z)
 
 void GLVertexBuffer::render() const
 {
-    if (!this->valid || this->vboId == 0 || this->batches.empty())
+    if (!this->valid || this->vaoId == 0 || this->batches.empty())
     {
         return;
     }
 
-    QOpenGLContext *ctx = QOpenGLContext::currentContext();
-    QOpenGLFunctions *f = ctx->functions();
-
-    // Bind VBO — with a VBO bound, the pointer offsets below are byte offsets
-    // from the start of the buffer (fixed-function VBO path, OpenGL 1.5+).
-    f->glBindBuffer(GL_ARRAY_BUFFER, this->vboId);
+    // The VAO already encapsulates the VBO binding and all attribute pointers
+    // recorded in uploadToGPU(). Just bind it and draw.
+    QOpenGLExtraFunctions *ef = QOpenGLContext::currentContext()->extraFunctions();
 
-    glEnableClientState(GL_VERTEX_ARRAY);
-    glEnableClientState(GL_NORMAL_ARRAY);
-    glEnableClientState(GL_COLOR_ARRAY);
-    glEnableClientState(GL_TEXTURE_COORD_ARRAY);
-
-    glVertexPointer(3, GL_FLOAT, sizeof(GLVertexData),
-                    reinterpret_cast<const void*>(offsetof(GLVertexData, position)));
-    glNormalPointer(GL_FLOAT, sizeof(GLVertexData),
-                    reinterpret_cast<const void*>(offsetof(GLVertexData, normal)));
-    glTexCoordPointer(1, GL_FLOAT, sizeof(GLVertexData),
-                      reinterpret_cast<const void*>(offsetof(GLVertexData, texCoord)));
-    glColorPointer(4, GL_UNSIGNED_BYTE, sizeof(GLVertexData),
-                   reinterpret_cast<const void*>(offsetof(GLVertexData, color)));
+    GL_SAFE_CALL(ef->glBindVertexArray(this->vaoId));
 
     for (const Batch &batch : this->batches)
     {
         glDrawArrays(batch.primitiveType, batch.start, batch.count);
     }
 
-    glDisableClientState(GL_TEXTURE_COORD_ARRAY);
-    glDisableClientState(GL_COLOR_ARRAY);
-    glDisableClientState(GL_NORMAL_ARRAY);
-    glDisableClientState(GL_VERTEX_ARRAY);
-
-    f->glBindBuffer(GL_ARRAY_BUFFER, 0);
+    GL_SAFE_CALL(ef->glBindVertexArray(0));
 }
 
 void GLVertexBuffer::release()
@@ -223,6 +240,12 @@ void GLVertexBuffer::release()
     if (ctx)
     {
         QOpenGLFunctions *f = ctx->functions();
+        QOpenGLExtraFunctions *ef = ctx->extraFunctions();
+        if (this->vaoId != 0)
+        {
+            GL_SAFE_CALL(ef->glDeleteVertexArrays(1, &this->vaoId));
+            this->vaoId = 0;
+        }
         if (this->vboId != 0)
         {
             GL_SAFE_CALL(f->glDeleteBuffers(1, &this->vboId));

src/fea/src/gl_vertex_buffer.h

@@ -51,6 +51,8 @@ class GLVertexBuffer
 
         //! VBO handle for vertex data.
         GLuint vboId;
+        //! VAO that records the attribute layout (created once in uploadToGPU).
+        GLuint vaoId;
         //! Total uploaded vertex count (valid after uploadToGPU).
         GLsizei vertexCount;
         //! Whether the VBO is valid and ready for rendering.

src/fea/src/gl_widget.cpp

@@ -291,13 +291,6 @@ void GLWidget::drawModel()
 
     GL_SAFE_CALL(glClear(GL_DEPTH_BUFFER_BIT));
 
-    if (this->clippingPlaneEnabled)
-    {
-        RLogger::trace("Clipping plane\n");
-        const GLdouble clippingPlane[4] = { 0.0, 0.0, -1.0, - GLdouble(this->scale)  + 2.0 * this->clippingPlaneDistance * GLdouble(this->scale) };
-        GL_SAFE_CALL(glClipPlane(GL_CLIP_PLANE0,clippingPlane));
-    }
-
     // Only update lights when they change (cached lighting setup)
     GL_SAFE_CALL(glEnable(GL_LIGHTING));
     if (this->lightsNeedUpdate || this->cachedNLights != this->displayProperties.getNLights())
@@ -413,6 +406,12 @@ void GLWidget::drawModel()
         this->mainShaderProgram.setUniformBool("uUseLighting", true);
         this->mainShaderProgram.setUniformBool("uUseTexture", false);
         this->mainShaderProgram.setUniformInt("uColorMap", 0);
+
+        // Upload clip plane — replaces glClipPlane(GL_CLIP_PLANE0, ...).
+        QVector4D clipPlane(0.0f, 0.0f, -1.0f,
+                            float(-this->scale + 2.0 * this->clippingPlaneDistance * this->scale));
+        this->mainShaderProgram.setUniformBool("uClipEnabled", this->clippingPlaneEnabled);
+        this->mainShaderProgram.setUniformVector4D("uClipPlane", clipPlane);
     }
 
     if (Application::instance()->getSession()->getModel(this->getModelID()).glDrawTrylock())
@@ -2139,7 +2138,8 @@ void GLWidget::qglColor(const QColor &color)
 {
     R_LOG_TRACE_IN;
     this->currentGLColor = color;
-    GL_SAFE_CALL(glColor4d(color.redF(),color.greenF(),color.blueF(),color.alphaF()));
+    // Route through GLFunctions so the colour is captured during VBO recording.
+    GLFunctions::color4ub(GLubyte(color.red()), GLubyte(color.green()), GLubyte(color.blue()), GLubyte(color.alpha()));
     R_LOG_TRACE_OUT;
 }
 

src/fea/src/shaders/flat.vert

@@ -4,12 +4,17 @@
 uniform mat4 uProjection;
 uniform mat4 uModelView;
 
+// Vertex attributes — fed by glVertexAttribPointer (replaces deprecated client arrays).
+in vec3  aPosition;   // layout location 0
+in vec4  aColor;      // layout location 2 — UNSIGNED_BYTE normalised to [0,1]
+in float aTexCoord;   // layout location 3
+
 out vec4  vColor;
 out float vTexCoord;
 
 void main()
 {
-    gl_Position = uProjection * uModelView * gl_Vertex;
-    vColor      = gl_Color;
-    vTexCoord   = gl_MultiTexCoord0.s;
+    gl_Position = uProjection * uModelView * vec4(aPosition, 1.0);
+    vColor      = aColor;
+    vTexCoord   = aTexCoord;
 }

src/fea/src/shaders/main.vert

@@ -5,6 +5,16 @@
 uniform mat4 uProjection;
 uniform mat4 uModelView;
 
+// Clip plane — replaces deprecated glClipPlane() / GL_CLIP_PLANE0.
+uniform vec4 uClipPlane;    // Eye-space clip plane equation (xyz=normal, w=offset).
+uniform bool uClipEnabled;  // Whether clipping is active.
+
+// Vertex attributes — fed by glVertexAttribPointer (replaces deprecated client arrays).
+in vec3  aPosition;   // layout location 0
+in vec3  aNormal;     // layout location 1
+in vec4  aColor;      // layout location 2 — UNSIGNED_BYTE normalised to [0,1]
+in float aTexCoord;   // layout location 3
+
 // Per-fragment outputs
 out vec4  vColor;
 out vec3  vNormal;
@@ -13,14 +23,14 @@ out float vTexCoord;
 
 void main()
 {
-    // gl_Vertex / gl_Normal / gl_Color / gl_MultiTexCoord0 are the legacy
-    // built-in attribute aliases fed by glVertexPointer / glNormalPointer /
-    // glColorPointer / glTexCoordPointer (OpenGL compatibility profile).
-    vec4 worldPos = uModelView * gl_Vertex;
+    vec4 worldPos = uModelView * vec4(aPosition, 1.0);
     gl_Position   = uProjection * worldPos;
 
+    // Write clip distance — positive = inside, negative = clipped.
+    gl_ClipDistance[0] = uClipEnabled ? dot(worldPos, uClipPlane) : 1.0;
+
     vFragPos  = worldPos.xyz;
-    vNormal   = mat3(transpose(inverse(mat3(uModelView)))) * gl_Normal;
-    vColor    = gl_Color;
-    vTexCoord = gl_MultiTexCoord0.s;
+    vNormal   = mat3(transpose(inverse(mat3(uModelView)))) * aNormal;
+    vColor    = aColor;
+    vTexCoord = aTexCoord;
 }