From 7f84689f7d8140bb7e9563eb9fbaaaebc96c6ab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Zaj=C4=85c?= Date: Sat, 9 Dec 2023 01:04:58 +0000 Subject: [PATCH] chore: tidy up code to get rid of warnings, tweak warning levels --- CMakeLists.txt | 34 ++++++++++++------- src/lepus/engine/ConsoleLogger.h | 2 +- src/lepus/engine/ILogger.h | 2 +- .../gfx/GraphicsEngine/Apis/ApiGL/ApiGL.cpp | 6 ++-- src/lepus/gfx/GraphicsEngine/GraphicsApi.h | 9 +++-- .../GraphicsEngine/GraphicsApi/BaseBindings.h | 5 +++ .../gfx/GraphicsEngine/GraphicsEngine.cpp | 7 +++- src/lepus/gfx/GraphicsEngine/ShaderCompiler.h | 5 +++ .../ShaderCompilers/ShaderCompilerGLSL.h | 2 +- src/lepus/system/Windowing.h | 5 +++ src/lepus/utility/types/Quaternion.h | 11 +++++- src/lepus/utility/types/Vector.h | 5 +++ tests/L3D/GraphicsEngine/GraphicsApiTests.h | 4 +-- tests/LUtility/MathTests/MatrixTests.cpp | 8 ++--- 14 files changed, 77 insertions(+), 28 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d52509d..9d9fbc9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -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() \ No newline at end of file diff --git a/src/lepus/engine/ConsoleLogger.h b/src/lepus/engine/ConsoleLogger.h index 0c5e0d0..29b3240 100644 --- a/src/lepus/engine/ConsoleLogger.h +++ b/src/lepus/engine/ConsoleLogger.h @@ -30,7 +30,7 @@ namespace lepus m_Instance = new ConsoleLogger(); } - return *reinterpret_cast(m_Instance); + return *(ConsoleLogger*)m_Instance; } inline void Log(const char* className, const char* funcName, LogEventTypes eventType, const char* message, const char* funcParams) diff --git a/src/lepus/engine/ILogger.h b/src/lepus/engine/ILogger.h index 500018c..d0855b6 100644 --- a/src/lepus/engine/ILogger.h +++ b/src/lepus/engine/ILogger.h @@ -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; diff --git a/src/lepus/gfx/GraphicsEngine/Apis/ApiGL/ApiGL.cpp b/src/lepus/gfx/GraphicsEngine/Apis/ApiGL/ApiGL.cpp index 555583d..1aa3a60 100644 --- a/src/lepus/gfx/GraphicsEngine/Apis/ApiGL/ApiGL.cpp +++ b/src/lepus/gfx/GraphicsEngine/Apis/ApiGL/ApiGL.cpp @@ -5,7 +5,7 @@ using namespace lepus::gfx; void GraphicsApiGL::Init(GraphicsApiOptions* options) { - InitInternal(reinterpret_cast(options)); + InitInternal((GraphicsApiGLOptions*)options); } void GraphicsApiGL::SetupVertexArrays() @@ -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() diff --git a/src/lepus/gfx/GraphicsEngine/GraphicsApi.h b/src/lepus/gfx/GraphicsEngine/GraphicsApi.h index eb1bffe..17ece38 100644 --- a/src/lepus/gfx/GraphicsEngine/GraphicsApi.h +++ b/src/lepus/gfx/GraphicsEngine/GraphicsApi.h @@ -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. @@ -99,7 +104,7 @@ namespace lepus // The internal options object NEEDS to have been allocated. assert(m_Options != nullptr); - return *reinterpret_cast(m_Options); + return *(TGraphicsApiOptions*)m_Options; } virtual void CreatePipeline() = 0; @@ -135,7 +140,7 @@ namespace lepus m_ShutdownCalled = true; } - ~GraphicsApi() + virtual ~GraphicsApi() { assert(m_ShutdownCalled == true); Shutdown(); diff --git a/src/lepus/gfx/GraphicsEngine/GraphicsApi/BaseBindings.h b/src/lepus/gfx/GraphicsEngine/GraphicsApi/BaseBindings.h index 2364e3d..3cc0ebe 100644 --- a/src/lepus/gfx/GraphicsEngine/GraphicsApi/BaseBindings.h +++ b/src/lepus/gfx/GraphicsEngine/GraphicsApi/BaseBindings.h @@ -50,6 +50,11 @@ namespace lepus { } + + virtual ~UniformBinding() + { + Release(); + } }; } } diff --git a/src/lepus/gfx/GraphicsEngine/GraphicsEngine.cpp b/src/lepus/gfx/GraphicsEngine/GraphicsEngine.cpp index 9883ad4..64ebed4 100644 --- a/src/lepus/gfx/GraphicsEngine/GraphicsEngine.cpp +++ b/src/lepus/gfx/GraphicsEngine/GraphicsEngine.cpp @@ -22,8 +22,13 @@ void GraphicsEngine::InitApi(GraphicsApiOptions* options) switch (options->GetType()) { case GraphicsApiType::GraphicsApiOpenGL: - m_Api = new GraphicsApiGL(*reinterpret_cast(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); diff --git a/src/lepus/gfx/GraphicsEngine/ShaderCompiler.h b/src/lepus/gfx/GraphicsEngine/ShaderCompiler.h index b7517f0..eb88f62 100644 --- a/src/lepus/gfx/GraphicsEngine/ShaderCompiler.h +++ b/src/lepus/gfx/GraphicsEngine/ShaderCompiler.h @@ -35,6 +35,11 @@ namespace lepus public: virtual void Init() = 0; virtual ShaderCompiledResult CompileShader(const char* shaderSource, size_t shaderSourceLength, ShaderType type) = 0; + + virtual ~ShaderCompiler() + { + + } }; } } diff --git a/src/lepus/gfx/GraphicsEngine/ShaderCompilers/ShaderCompilerGLSL.h b/src/lepus/gfx/GraphicsEngine/ShaderCompilers/ShaderCompilerGLSL.h index 39a3081..c8a84b2 100644 --- a/src/lepus/gfx/GraphicsEngine/ShaderCompilers/ShaderCompilerGLSL.h +++ b/src/lepus/gfx/GraphicsEngine/ShaderCompilers/ShaderCompilerGLSL.h @@ -44,7 +44,7 @@ namespace lepus { assert(shaderSrcLength <= MAXINT); - GLenum shaderType = -1; + GLenum shaderType = 0; switch (type) { diff --git a/src/lepus/system/Windowing.h b/src/lepus/system/Windowing.h index 8232677..ead5a8d 100644 --- a/src/lepus/system/Windowing.h +++ b/src/lepus/system/Windowing.h @@ -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() + { + + } }; } } diff --git a/src/lepus/utility/types/Quaternion.h b/src/lepus/utility/types/Quaternion.h index 1412bf7..6a802dd 100644 --- a/src/lepus/utility/types/Quaternion.h +++ b/src/lepus/utility/types/Quaternion.h @@ -10,7 +10,7 @@ namespace lepus { namespace types { - class Quaternion : public Vector4 + class Quaternion : public Vector<4> { public: Quaternion() @@ -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(); diff --git a/src/lepus/utility/types/Vector.h b/src/lepus/utility/types/Vector.h index 81b5f4d..c86c8b6 100644 --- a/src/lepus/utility/types/Vector.h +++ b/src/lepus/utility/types/Vector.h @@ -133,6 +133,11 @@ namespace lepus { return m_Components; } + + virtual ~Vector() + { + + } }; class Vector2 : public Vector<2> diff --git a/tests/L3D/GraphicsEngine/GraphicsApiTests.h b/tests/L3D/GraphicsEngine/GraphicsApiTests.h index 8b8ee04..662ffe7 100644 --- a/tests/L3D/GraphicsEngine/GraphicsApiTests.h +++ b/tests/L3D/GraphicsEngine/GraphicsApiTests.h @@ -26,7 +26,7 @@ class GraphicsApiStub : public lepus::gfx::GraphicsApi void Init(lepus::gfx::GraphicsApiOptions* options) override { - InitInternal(reinterpret_cast(options)); + InitInternal((GraphicsApiStubOptions*)options); } void CreatePipeline() override @@ -66,7 +66,7 @@ class GraphicsApiStub : public lepus::gfx::GraphicsApi } // TODO: is this cast needed? - delete reinterpret_cast(m_Options); + delete (GraphicsApiStubOptions*)m_Options; m_Options = nullptr; } } diff --git a/tests/LUtility/MathTests/MatrixTests.cpp b/tests/LUtility/MathTests/MatrixTests.cpp index 05ac692..a83213f 100644 --- a/tests/LUtility/MathTests/MatrixTests.cpp +++ b/tests/LUtility/MathTests/MatrixTests.cpp @@ -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: @@ -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 @@ -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++)