chore: address PR comments

Content/GLSL/Unlit/RGBVertex.vert

@@ -1,50 +1,15 @@
 #version 330 core
 
-#define M_PI 3.1415926535897932384626433832795
-#define DEG2RAD M_PI/180.0
-
 layout (location = 0) in vec3 position;
 
 uniform mat4 PROJ;
 uniform mat4 VIEW;
 
-uniform float runningTime;
-
 out vec3 vertColor;
 
 void main()
 {
-	float fov = 110;
-	float hypot = tan((180 - fov) * 0.5 * DEG2RAD);
-	float far = 100.0;
-	float near = 0.1;
-
-	float angle = runningTime / 5.0;
-	float c = cos(angle);
-	float s = sin(angle);
-
-	// has to be unit length
-	vec3 axis = normalize(vec3(1.0, 0.0, 0.0));
-
-	vec3 worldPos = vec3(0.0, -2.0, -4.0);
-
-	mat4 rot;
-	rot[0] = vec4(c+axis.x*axis.x*(1.0 - c), axis.y*axis.x*(1.0 - c)+axis.z*s, axis.z*axis.x*(1.0 - c)-axis.y*s, 0.0);
-	rot[1] = vec4(axis.y*axis.x*(1.0 - c)-axis.z*s, c+axis.y*axis.y*(1.0 - c), axis.z*axis.y*(1.0 - c)+axis.x*s, 0.0);
-	rot[2] = vec4(axis.z*axis.x*(1.0 - c)+axis.y*s, axis.z*axis.y*(1.0 - c)-axis.x*s, c+axis.z*axis.z*(1.0 - c), 0.0);
-	rot[3] = vec4(0.0, 0.0, 0.0, 1.0);
-
-	mat4 viewPos;
-	viewPos[0] = vec4(1.0, 0.0, 0.0, 0.0);
-	viewPos[1] = vec4(0.0, 1.0, 0.0, 0.0);
-	viewPos[2] = vec4(0.0, 0.0, 1.0, 0.0);
-	viewPos[3] = vec4(worldPos, 1.0);
-	// multiply rot by viewPos in C++ to build lookat matrix? could probably write out the full multiplication but ehhh
-
-	vec4 offsetPos = vec4(position, 1.0);
-	//offsetPos = rot * viewPos * (offsetPos + vec4(0.0, 0.0, 0.0, 0.0));
-	//offsetPos = viewPos * offsetPos;
-	gl_Position = PROJ * VIEW * offsetPos;
+	gl_Position = PROJ * VIEW * vec4(position, 1.0);
 
 	float normalisedIndex = mod(float(gl_VertexID), 3.0f);
 	float r = step(normalisedIndex, 0.0f);

src/L3D/Camera.h

@@ -27,11 +27,15 @@ namespace lepus
                 m_Transform = lepus::math::Transform();
             }
 
+            /// @brief Gets field of view
+            /// @return Camera's field of view (in degrees)
             inline float FOV() const
             {
                 return m_FOV;
             }
 
+            /// @brief Sets field of view
+            /// @param fov New field of view (in degrees) to be used by this camera
             inline void FOV(float fov)
             {
                 m_FOV = fov;

src/L3D/GraphicsEngine/Apis/ApiGL.h

@@ -100,7 +100,7 @@ namespace LepusEngine
 			{
 				size_t targetKeyLength = strlen(name);
 
-				// TODO: unordered_map doesn't really work with string keys...
+				// TODO: unordered_map doesn't really work with string keys... add actual hashing!
 				for (auto it = m_Pipeline.uniformMap.begin(); it != m_Pipeline.uniformMap.end(); it++)
 				{
 					size_t keyLength = strlen(it->first);
@@ -111,17 +111,6 @@ namespace LepusEngine
 				}
 
 				return nullptr;
-
-				/*if (!strcmp("PROJ", name))
-				{
-					return *(++m_Pipeline.uniforms.begin());
-				}
-				else if (!strcmp("runningTime", name))
-				{
-					return *(m_Pipeline.uniforms.begin());
-				}
-
-				return nullptr;*/
 			}
 
 			public:

src/L3D/GraphicsEngine/Apis/ApiGL/ApiGL.cpp

@@ -60,16 +60,10 @@ void GraphicsApiGL::SetupUniforms()
 	m_Pipeline.uniforms.push_front((lepus::gfx::GLUniformBinding<void*>*)(proj));
 	m_Pipeline.uniformMap.insert_or_assign("PROJ", reinterpret_cast<lepus::gfx::GLUniformBinding<void*>*>(proj));
 
-	// temp
-	auto* runningTime = new lepus::gfx::GLFloatUniformBinding(glGetUniformLocation(m_Programs[0], "runningTime"));
-	m_Pipeline.uniforms.push_front((lepus::gfx::GLUniformBinding<void*>*)(runningTime));
-	m_Pipeline.uniformMap.insert_or_assign("runningTime", reinterpret_cast<lepus::gfx::GLUniformBinding<void*>*>(runningTime));
-
 	// View matrix
 	auto* view = new lepus::gfx::GLMatrixUniformBinding(glGetUniformLocation(m_Programs[0], "VIEW"));
 	m_Pipeline.uniforms.push_front((lepus::gfx::GLUniformBinding<void*>*)(view));
 	m_Pipeline.uniformMap.insert_or_assign("VIEW", reinterpret_cast<lepus::gfx::GLUniformBinding<void*>*>(view));
-
 }
 
 void GraphicsApiGL::CreatePipeline()

src/L3D/GraphicsEngine/GraphicsApi.h

@@ -62,6 +62,9 @@ namespace LepusEngine
 				memcpy(m_Options, options, optionsSz);
 			}
 
+			/// @brief Internal API-specific method for retrieving a native handle to a uniform object.
+			/// @param name The name of the uniform to fetch.
+			/// @return API-specific handle for a uniform of a given type.
 			virtual void* GetUniformInternal(char* name) = 0;
 			public:
 			/// @brief Default constructor. Does nothing, so Init(GraphicsApiOptions*) needs to be called manually.
@@ -101,12 +104,20 @@ namespace LepusEngine
 
 			virtual void CreatePipeline() = 0;
 
+			/// @brief Gets a Lepus UniformBinding wrapper for an API-specific uniform with the given name.
+			/// @tparam TUniformHandle API-specific handle type used for this type of uniform.
+			/// @tparam TUniformBinding UniformBinding implementation
+			/// @param name Name of the uniform to fetch.
+			/// @return A UniformBinding wrapper for the named uniform object.
 			template<typename TUniformHandle = void*, class TUniformBinding = lepus::gfx::UniformBinding<TUniformHandle>>
 			inline const TUniformBinding* GetUniform(char* name)
 			{
 				return reinterpret_cast<const TUniformBinding*>(GetUniformInternal(name));
 			}
 
+			/// @brief Applies uniforms in the shader.
+			/// Implementations can fire & forget by issuing this before every draw, but it might be worth having a mechanism to invalidate uniforms
+			/// and only update them once they're marked as dirty.
 			virtual void UpdateUniforms() = 0;
 
 			virtual void Draw() = 0;

src/L3D/GraphicsEngine/GraphicsApi/BaseBindings.h

@@ -7,6 +7,9 @@ namespace lepus
 {
     namespace gfx
     {
+        /// @brief An API-agnostic wrapper around a generic uniform.
+        /// @tparam TUniformHandle API-specific uniform handle type
+        /// @tparam TUniformValue Datatype held by this uniform (usually a POD type).
         template<typename TUniformHandle, typename TUniformValue = void*>
         class UniformBinding
         {
@@ -37,6 +40,8 @@ namespace lepus
                 return m_Value;
             }
 
+            /// @brief Has the uniform been invalidated since it's last been applied?
+            /// @return True if uniform has been invalidated and pending update in shader, false if no changes to data
             inline bool IsDirty() { return m_Dirty; }
 
             virtual UniformType Type() = 0;

src/LDemo/main.cpp

@@ -47,14 +47,6 @@ void mouseCallback(GLFWwindow* window, double xpos, double ypos)
 	angleYaw += deltaX;
 	anglePitch += deltaY;
 	lepus::types::Quaternion rotationYaw = lepus::types::Quaternion(0.f, 1.f, 0.f, deltaX);
-	//camera.Transform().Rotate(lepus::types::Vector3(0.0f, 1.f, 0.f), deltaX);
-
-	//LepusEngine::ConsoleLogger::Global().LogInfo("", "mouseCallback", (char*)axis.ToString().c_str());
-	//ConsoleLogger::Global().LogInfo("", "mouseCallback", (char*)std::to_string(angle).c_str());
-	//if (abs(angle) > 0.001f)
-	{
-		//camera.Transform().Rotate(axis, angle);
-	}
 
 	auto combined = rotationYaw;
 	float angle = combined.Angle();
@@ -117,14 +109,9 @@ int main()
 	fov = camera.FOV();
 
 	glfwSetScrollCallback(reinterpret_cast<GLFWwindow*>(windowing->GetWindowPtr()), scrollCallback);
-	//glfwSetKeyCallback(reinterpret_cast<GLFWwindow*>(windowing->GetWindowPtr()), keyInputCallback);
 
 	float runningTime = glfwGetTime();
 
-	//camera.Transform().Rotation().y(1.f);
-	//camera.Transform().Rotation().w(0.f);
-	//camera.Transform().Rotation().z(1.f);
-
 	GLFWwindow* window = reinterpret_cast<GLFWwindow*>(windowing->GetWindowPtr());
 	glfwGetCursorPos(window, &xposLast, &yposLast);
 	glfwSetCursorPosCallback(reinterpret_cast<GLFWwindow*>(windowing->GetWindowPtr()), mouseCallback);
@@ -161,27 +148,17 @@ int main()
 
 		camera.Transform().Origin(camera.Transform().Origin() + forwardDelta + rightDelta);
 
-
-		/*
-			  camera.Transform().Origin().x(x);
-			  camera.Transform().Origin().y(y);
-			  camera.Transform().Origin().z(z);
-	  */
-	  //camera.Transform().Rotation().w(angleYaw);
 		viewMatrix = camera.BuildViewMatrix();
 
 		((lepus::gfx::GLMatrixUniformBinding*)api.GetUniform<lepus::gfx::GLMatrixUniformBinding>("PROJ"))->Value((float*)projMatrix.data());
 		((lepus::gfx::GLMatrixUniformBinding*)api.GetUniform<lepus::gfx::GLMatrixUniformBinding>("VIEW"))->Value((float*)viewMatrix.data());
-		((lepus::gfx::GLFloatUniformBinding*)api.GetUniform<lepus::gfx::GLFloatUniformBinding>("runningTime"))->Value(runningTime);
 
 		windowing->Update(); // Update window before drawing
 		engine.Render<unsigned char, Lepus3D::GraphicsEngine::PixelFormat::RGBA32>(100, 149, 237);
 
 		float newRunningTime = glfwGetTime();
 		deltaTime = newRunningTime - runningTime;
 
-		ConsoleLogger::Global().LogInfo("", "main", (char*)camera.Transform().Rotation().ToString().c_str());
-
 		isRunning = windowing->Update();
 	}
 

src/LUtility/Types/Matrix4x4.h

@@ -11,8 +11,7 @@ namespace lepus
         class Matrix4x4
         {
             private:
-            // TODO: use an array instead?
-            // Elements in row-major order.
+            /// @brief Elements in row-major order.
             float m_Elements[4 * 4];
 
             public:

src/LUtility/Types/Quaternion.h

@@ -29,6 +29,12 @@ namespace lepus
                 init(quatData);
             }
 
+            /// @brief Constructs a Quaternion from Axis-Angle representation.
+            /// @remarks This is not for initialising XYZW directly - use the array initialiser instead.
+            /// @param axisX X component of the axis
+            /// @param axisY Y component of the axis
+            /// @param axisZ Z component of the axis
+            /// @param angle Rotation angle around the axis
             Quaternion(float axisX, float axisY, float axisZ, float angle)
             {
                 // Negating the angle here so that the Quaternion represents a clockwise rotation along an axis as observed looking towards the origin/object.