Skip to content

Commit

Permalink
chore: tidy up code to get rid of warnings, tweak warning levels
Browse files Browse the repository at this point in the history
  • Loading branch information
tomezpl committed Dec 9, 2023
1 parent 803de29 commit 7f84689
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 28 deletions.
34 changes: 22 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,28 @@ gtest_discover_tests(LepusSystem_Tests)
gtest_discover_tests(LepusUtility_Tests)

# Warnings
set(LepusTargets LepusEngine LepusSystem LepusGfx LepusSystem_Tests LepusGfx_Tests LepusUtility_Tests)
if(MSVC)
target_compile_options(LepusEngine PRIVATE /W3 /WX)
target_compile_options(LepusSystem PRIVATE /W3 /WX)
target_compile_options(LepusGfx PRIVATE /W3 /WX)
target_compile_options(LepusSystem_Tests PRIVATE /W3 /WX)
target_compile_options(LepusGfx_Tests PRIVATE /W3 /WX)
target_compile_options(LepusUtility_Tests PRIVATE /W3 /WX)
# Ignore warnings:
# 4100: unreferenced param,
# 4514: unreferenced inline function removed,
# 4464: parent relative include,
# 4820: struct padding,
# 4263: member not overriding base class virtual
# 4265-5027: copy ctor, move ctor, assignment, move assignment implicitly deleted (this messes up gtest projects)
# 5045: "compiler will insert Spectre mitigation for memory load"
set(MSVCDisabledWarnings 4100 4514 4464 4820 4263 4625 5026 4626 5027 5045)

set(MSVCDisabledWarningsFormatted "")
foreach(Warning IN LISTS MSVCDisabledWarnings)
set(MSVCDisabledWarningsFormatted ${MSVCDisabledWarningsFormatted} /wd${Warning})
endforeach()

foreach(Target IN LISTS LepusTargets)
target_compile_options(${Target} PRIVATE /Wall ${MSVCDisabledWarningsFormatted} /WX /external:W3)
endforeach()
else()
target_compile_options(LepusEngine PRIVATE -Wall -Wextra -Wpedantic)
target_compile_options(LepusSystem PRIVATE -Wall -Wextra -Wpedantic)
target_compile_options(LepusGfx PRIVATE -Wall -Wextra -Wpedantic)
target_compile_options(LepusSystem_Tests PRIVATE -Wall -Wextra -Wpedantic)
target_compile_options(LepusGfx_Tests PRIVATE -Wall -Wextra -Wpedantic)
target_compile_options(LepusUtility_Tests PRIVATE -Wall -Wextra -Wpedantic)
foreach(Target IN LISTS LepusTargets)
target_compile_options(${Target} PRIVATE -Wall -Wextra -Wpedantic)
endforeach()
endif()
2 changes: 1 addition & 1 deletion src/lepus/engine/ConsoleLogger.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace lepus
m_Instance = new ConsoleLogger();
}

return *reinterpret_cast<ConsoleLogger*>(m_Instance);
return *(ConsoleLogger*)m_Instance;
}

inline void Log(const char* className, const char* funcName, LogEventTypes eventType, const char* message, const char* funcParams)
Expand Down
2 changes: 1 addition & 1 deletion src/lepus/engine/ILogger.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ namespace lepus
{
namespace engine
{
enum LogEventTypes { LEPUS_ERROR, LEPUS_INFO, LEPUS_WARNING };
class ILogger
{
public:
enum LogEventTypes { LEPUS_ERROR, LEPUS_INFO, LEPUS_WARNING };

virtual void Log(const char* className, const char* funcName, LogEventTypes eventType, const char* message, const char* funcParams) = 0;
virtual void LogError(const char* className, const char* funcName, const char* message, const char* funcParams = "") = 0;
Expand Down
6 changes: 3 additions & 3 deletions src/lepus/gfx/GraphicsEngine/Apis/ApiGL/ApiGL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ using namespace lepus::gfx;

void GraphicsApiGL::Init(GraphicsApiOptions* options)
{
InitInternal<GraphicsApiGLOptions>(reinterpret_cast<GraphicsApiGLOptions*>(options));
InitInternal<GraphicsApiGLOptions>((GraphicsApiGLOptions*)options);
}

void GraphicsApiGL::SetupVertexArrays()
Expand All @@ -25,13 +25,13 @@ void GraphicsApiGL::SetupBuffers()
glBindBuffer(GL_ARRAY_BUFFER, m_Pipeline.vbo);
glEnableVertexAttribArray(0);
glVertexAttribPointer(0, 3, GL_FLOAT, GL_TRUE, 0, 0);
glBufferData(GL_ARRAY_BUFFER, m_CubeGeometry.VertexBufferSize(), m_CubeGeometry.GetVertices(), GL_STATIC_DRAW);
glBufferData(GL_ARRAY_BUFFER, (GLsizeiptr)m_CubeGeometry.VertexBufferSize(), m_CubeGeometry.GetVertices(), GL_STATIC_DRAW);

// Create a global IBO and upload triangle index data to it.
glCreateBuffers(1, &m_Pipeline.ibo);
//const GLuint indices[] = { 0, 1, 2 };
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, m_Pipeline.ibo);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, m_CubeGeometry.IndexBufferSize(), m_CubeGeometry.GetIndices(), GL_STATIC_DRAW);
glBufferData(GL_ELEMENT_ARRAY_BUFFER, (GLsizeiptr)m_CubeGeometry.IndexBufferSize(), m_CubeGeometry.GetIndices(), GL_STATIC_DRAW);
}

void GraphicsApiGL::SetupShaders()
Expand Down
9 changes: 7 additions & 2 deletions src/lepus/gfx/GraphicsEngine/GraphicsApi.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ namespace lepus

/// @brief Main viewport used by the application.
lepus::types::Viewport mainViewport = {};

virtual ~GraphicsApiOptions()
{

}
};

/// @brief API wrapper to be used by GraphicsEngine.
Expand Down Expand Up @@ -99,7 +104,7 @@ namespace lepus
// The internal options object NEEDS to have been allocated.
assert(m_Options != nullptr);

return *reinterpret_cast<TGraphicsApiOptions*>(m_Options);
return *(TGraphicsApiOptions*)m_Options;
}

virtual void CreatePipeline() = 0;
Expand Down Expand Up @@ -135,7 +140,7 @@ namespace lepus
m_ShutdownCalled = true;
}

~GraphicsApi()
virtual ~GraphicsApi()
{
assert(m_ShutdownCalled == true);
Shutdown();
Expand Down
5 changes: 5 additions & 0 deletions src/lepus/gfx/GraphicsEngine/GraphicsApi/BaseBindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ namespace lepus
{

}

virtual ~UniformBinding()
{
Release();
}
};
}
}
Expand Down
7 changes: 6 additions & 1 deletion src/lepus/gfx/GraphicsEngine/GraphicsEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@ void GraphicsEngine::InitApi(GraphicsApiOptions* options)
switch (options->GetType())
{
case GraphicsApiType::GraphicsApiOpenGL:
m_Api = new GraphicsApiGL(*reinterpret_cast<GraphicsApiGLOptions*>(options));
m_Api = new GraphicsApiGL(*(GraphicsApiGLOptions*)options);
break;
case GraphicsApiType::GraphicsApiVulkan:
// TODO
case GraphicsApiType::GraphicsApiTest:
// Ignore test/mock APIs.
case GraphicsApiType::GraphicsApiUnknown:
default:
// Assert if the API type is not part of the enum.
assert(false);
Expand Down
5 changes: 5 additions & 0 deletions src/lepus/gfx/GraphicsEngine/ShaderCompiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ namespace lepus
public:
virtual void Init() = 0;
virtual ShaderCompiledResult<TShaderHandle> CompileShader(const char* shaderSource, size_t shaderSourceLength, ShaderType type) = 0;

virtual ~ShaderCompiler()
{

}
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace lepus
{
assert(shaderSrcLength <= MAXINT);

GLenum shaderType = -1;
GLenum shaderType = 0;

switch (type)
{
Expand Down
5 changes: 5 additions & 0 deletions src/lepus/system/Windowing.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ namespace lepus

/// @brief Performs back/front buffer swap. This usually needs to be called after rendering a frame.
virtual void SwapBuffers() = 0;

virtual ~Windowing()
{

}
};
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/lepus/utility/types/Quaternion.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace lepus
{
namespace types
{
class Quaternion : public Vector4
class Quaternion : public Vector<4>
{
public:
Quaternion()
Expand Down Expand Up @@ -67,6 +67,15 @@ namespace lepus
return acosf(fmax(-1.f, fmin(1.f, w()))) * 2.f;
}

inline float x() const { return m_Components[0]; }
inline float y() const { return m_Components[1]; }
inline float z() const { return m_Components[2]; }
inline float w() const { return m_Components[3]; }
inline float x(float newX) { return m_Components[0] = newX; }
inline float y(float newY) { return m_Components[1] = newY; }
inline float z(float newZ) { return m_Components[2] = newZ; }
inline float w(float newW) { return m_Components[3] = newW; }

inline Quaternion operator*(const Quaternion& b) const
{
Quaternion result = Quaternion();
Expand Down
5 changes: 5 additions & 0 deletions src/lepus/utility/types/Vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ namespace lepus
{
return m_Components;
}

virtual ~Vector<nbComponents>()
{

}
};

class Vector2 : public Vector<2>
Expand Down
4 changes: 2 additions & 2 deletions tests/L3D/GraphicsEngine/GraphicsApiTests.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class GraphicsApiStub : public lepus::gfx::GraphicsApi

void Init(lepus::gfx::GraphicsApiOptions* options) override
{
InitInternal<GraphicsApiStubOptions>(reinterpret_cast<GraphicsApiStubOptions*>(options));
InitInternal<GraphicsApiStubOptions>((GraphicsApiStubOptions*)options);
}

void CreatePipeline() override
Expand Down Expand Up @@ -66,7 +66,7 @@ class GraphicsApiStub : public lepus::gfx::GraphicsApi
}

// TODO: is this cast needed?
delete reinterpret_cast<GraphicsApiStubOptions*>(m_Options);
delete (GraphicsApiStubOptions*)m_Options;
m_Options = nullptr;
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/LUtility/MathTests/MatrixTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ TEST(MathMatrixTests, MatrixMultipliedByMatrixCorrectly)
13 14 15 16
*/
for (uint8_t i = 0; i < 16; i++) { matA.set((i - (i % 4)) / 4, i % 4, float(i + 1)); }
for (uint8_t i = 0; i < 16; i++) { matA.set((i - (i % 4U)) / 4U, i % 4U, float(i + 1U)); }

/* Matrix B looks like this:
Expand All @@ -26,7 +26,7 @@ TEST(MathMatrixTests, MatrixMultipliedByMatrixCorrectly)
10 11 12 13
14 15 16 17
*/
for (uint8_t i = 0; i < 16; i++) { matB.set((i - (i % 4)) / 4, i % 4, float(i + 2)); }
for (uint8_t i = 0; i < 16; i++) { matB.set((i - (i % 4U)) / 4U, i % 4U, float(i + 2U)); }

/* Expected results:
100 110 120 130
Expand All @@ -44,8 +44,8 @@ TEST(MathMatrixTests, MatrixMultipliedByMatrixCorrectly)
lepus::math::Matrix4x4 actual = matA.Multiply(matB);

// Make sure the original matrices are still correct - they should not be affected by the multiplication, as it's not in-place.
for (uint8_t i = 0; i < 16; i++) { ASSERT_FLOAT_EQ(matA.get((i - (i % 4)) / 4, i % 4), (float)(i + 1)); }
for (uint8_t i = 0; i < 16; i++) { ASSERT_FLOAT_EQ(matB.get((i - (i % 4)) / 4, i % 4), (float)(i + 2)); }
for (uint8_t i = 0; i < 16; i++) { ASSERT_FLOAT_EQ(matA.get((i - (i % 4U)) / 4U, i % 4U), (float)(i + 1U)); }
for (uint8_t i = 0; i < 16; i++) { ASSERT_FLOAT_EQ(matB.get((i - (i % 4U)) / 4U, i % 4U), (float)(i + 2U)); }

// Check that the multiplied result is correct.
for (uint8_t r = 0; r < 4; r++)
Expand Down

0 comments on commit 7f84689

Please sign in to comment.