trjhtr

So I had posted C++ OpenGL Debug Utility before to get some feedback on it, this is a more complete version of the class. (Again, looking for feedback/critiquing of any sort. As I'm in the early stages of learning C++ and don't want to pick up any bad practices / poor coding styles.)

#pragma once

#include <vector>

#include <GL/glew.h>
#include <glm/glm.hpp>

#include "Camera.h"

class GLDebug final 

 static const glm::vec4 clip_space_cube;

 struct Point 
 glm::vec3 point;
 glm::vec3 color;
 ;

 struct Line 
 glm::vec3 point_0;
 glm::vec3 point_1;
 glm::vec3 color;
 ;

 std::vector<Point> m_points;
 std::vector<Line> m_lines;

public:

 static const glm::vec3 color_white;
 static const glm::vec3 color_red;
 static const glm::vec3 color_green;
 static const glm::vec3 color_blue;

 void drawPoint(const glm::vec3& point, const glm::vec3& color = color_white);

 void drawLine(const glm::vec3& point_0, const glm::vec3& point_1, const glm::vec3& color = color_white);

 void drawMat4(const glm::mat4& matrix);

 void drawFrustum(Camera& camera);

 void onRender(Camera& camera);
;

.cpp

#include "GLDebug.h"

#include <glm/gtc/type_ptr.hpp>
#include <glm/gtc/matrix_transform.hpp>

const glm::vec4 GLDebug::clip_space_cube = 
 glm::vec4( 1.0f, 1.0f, 0.0f, 1.0f),
 glm::vec4(-1.0f, 1.0f, 0.0f, 1.0f),
 glm::vec4(-1.0f,-1.0f, 0.0f, 1.0f),
 glm::vec4( 1.0f,-1.0f, 0.0f, 1.0f),
 glm::vec4( 1.0f, 1.0f, 1.0f, 1.0f),
 glm::vec4(-1.0f, 1.0f, 1.0f, 1.0f),
 glm::vec4(-1.0f,-1.0f, 1.0f, 1.0f),
 glm::vec4( 1.0f,-1.0f, 1.0f, 1.0f)
;

const glm::vec3 GLDebug::color_white = glm::vec3(1.0f);
const glm::vec3 GLDebug::color_red = glm::vec3(1.0f, 0.0f, 0.0f);
const glm::vec3 GLDebug::color_green = glm::vec3(0.0f, 1.0f, 0.0f);
const glm::vec3 GLDebug::color_blue = glm::vec3(0.0f, 0.0f, 1.0f);

void GLDebug::drawPoint(const glm::vec3& point, const glm::vec3& color) 
 m_points.push_back( point, color );


void GLDebug::drawLine(const glm::vec3& point_0, const glm::vec3& point_1, const glm::vec3& color) 
 m_lines.push_back( point_0, point_1, color );


void GLDebug::drawMat4(const glm::mat4& matrix) 
 glm::vec4 temp0;
 glm::vec4 temp1;

 temp0 = matrix * glm::vec4(0.0f, 0.0f, 0.0f, 1.0f);
 drawPoint(temp0);

 temp1 = matrix * glm::vec4(1.0f, 0.0f, 0.0f, 1.0f);
 drawLine(temp0, temp1, color_red);

 temp1 = matrix * glm::vec4(0.0f, 1.0f, 0.0f, 1.0f);
 drawLine(temp0, temp1, color_green);

 temp1 = matrix * glm::vec4(0.0f, 0.0f, 1.0f, 1.0f);
 drawLine(temp0, temp1, color_blue);


void GLDebug::drawFrustum(Camera& camera) 
 drawMat4(glm::inverse(camera.getViewMatrix()));

 const glm::mat4 invViewProj = glm::inverse(camera.getViewProjMatrix());
 glm::vec4 transformedClipCube[8];

 for (int i = 0; i < 8; i++) 
 transformedClipCube[i] = invViewProj * clip_space_cube[i];
 transformedClipCube[i] /= transformedClipCube[i].w;
 

 drawLine(transformedClipCube[0], transformedClipCube[1]);
 drawLine(transformedClipCube[1], transformedClipCube[2]);
 drawLine(transformedClipCube[2], transformedClipCube[3]);
 drawLine(transformedClipCube[3], transformedClipCube[0]);

 drawLine(transformedClipCube[4], transformedClipCube[5]);
 drawLine(transformedClipCube[5], transformedClipCube[6]);
 drawLine(transformedClipCube[6], transformedClipCube[7]);
 drawLine(transformedClipCube[7], transformedClipCube[4]);

 drawLine(transformedClipCube[0], transformedClipCube[4]);
 drawLine(transformedClipCube[1], transformedClipCube[5]);
 drawLine(transformedClipCube[2], transformedClipCube[6]);
 drawLine(transformedClipCube[3], transformedClipCube[7]);


void GLDebug::onRender(Camera& camera) 
 if (m_points.empty() && m_lines.empty()) return;

 glDisable(GL_DEPTH_TEST);

 glMatrixMode(GL_PROJECTION);
 glLoadMatrixf(glm::value_ptr(camera.getProjMatrix()));

 glMatrixMode(GL_MODELVIEW);
 glLoadMatrixf(glm::value_ptr(camera.getViewMatrix()));

 glPointSize(5.0f);

 glBegin(GL_POINTS);
 for (const auto& point : m_points) 
 glColor3fv(glm::value_ptr(point.color));
 glVertex3fv(glm::value_ptr(point.point));
 
 glEnd();

 glBegin(GL_LINES);
 for (const auto& line : m_lines) 
 glColor3fv(glm::value_ptr(line.color));
 glVertex3fv(glm::value_ptr(line.point_0));

 glColor3fv(glm::value_ptr(line.color));
 glVertex3fv(glm::value_ptr(line.point_1));
 
 glEnd();

 m_points.clear();
 m_lines.clear();

 glEnable(GL_DEPTH_TEST);

Some of the more specific things I'd like feedback on:

• Is my use of "static const" member variables proper? (Is there a better way to go about them.)


• In the "drawMat4" function definition, is it ok to have have "temp" variables defined as they are? (Not concerned as much about the naming, just how they are used. In my old Java project, I had some class level static variables I would use as "temp" to prevent memory re-allocation. I was targeting Android and without reusing an existing object, it would GC often from new object allocations which would cause stutter. I was going to use more static class member variables for the "temp" here however I'd like to hear others thoughts before doing so. Is there a proper C++ way of doing what I described?)


• Eventually I would like to make my project multi-threaded, are there any preparations I should make early on to ensure the transition goes smoothly? (Like modifying key words etc? In Java there are synchronized, volatile, etc.)

Thanks in advance to all advice!

I’ve seen it stated in conference presentations that constexpr should generally replace uses of static const. Even if you can’t do that because the initializer isn't constexpr, note that you no longer have to put the definitions into a CPP file separate from the class definition.

Is there a reason why drawLine and drawPoint are not inlined in the class definition?

• 
  glBegin / glEnd have been deprecated for a long time and aren't even available on some platforms.


• avoid using c-style arrays, use std::array instead (this can be applied to clip_space_cube)


• avoid using magic numbers, 8 in the following block should be a static constant like cube_vertex_count (which also could be used for array size):

for (int i = 0; i < 8; i++) 
 transformedClipCube[i] = invViewProj * clip_space_cube[i];
 transformedClipCube[i] /= transformedClipCube[i].w;