Fix Visual Studio and XCode Warnings #618

madebr · 2018-08-30T14:21:01Z

This pr fixes all Visual Studio and XCode warnings created by our code.
Most warnings were caused by:

narrowing data types;
implicitly converting floats to integers and vice versa; and
missing PDB files (=debug symbols)

Warnings remaining on Visual Studio:

~~There is still a warning caused by Bullet, which is fixed upstream and will be gone in the next bullet release.~~
I've explicitly disabled these warnings using #pragma's. This commit can be reverted once a fixed version of bullet is released.
Some linking warnings which will be dealt with in a next pr or by the upstream conan repositories.
Warnings by microprofile. A pull request has been submitted.

Also:

I've removed all #pragma once's
I've modified the ci to additionally build openrw on llvm.
Added configuration option to build in parallel using Visual Studio. This reduced the time for a rebuild of openrw from 5'25" to 2'40".
fix warning: missing braces around initializer [-Wmissing-braces] warnings of xcode

Check out the green at https://my.cdash.org/index.php?project=OpenRW

rwcore/gl/DrawBuffer.cpp

madebr · 2018-08-30T14:46:08Z

clang emits another warning about an unused private field: m_errorTexture in rwengine/src/render/ObjectRenderer.hpp. Suggestions?

EDIT:
added a setErrorTexture method in debug mode.
EDIT2:
and removed.

ghost

;)

ghost · 2018-08-30T15:46:21Z

rwengine/src/ai/AIGraph.cpp

@@ -19,7 +19,7 @@ AIGraph::~AIGraph() {

 void AIGraph::createPathNodes(const glm::vec3& position,
                              const glm::quat& rotation, PathData& path) {
-    size_t startIndex = nodes.size();
+    std::uint32_t startIndex = static_cast<std::uint32_t>(nodes.size());


ghost · 2018-08-30T15:47:17Z

rwengine/src/audio/SoundSource.cpp

@@ -248,7 +248,7 @@ struct InputData {
 /// to buffer.
 static int read_packet(void* opaque, uint8_t* buf, int buf_size) {
    auto* input = reinterpret_cast<InputData*>(opaque);
-    buf_size = FFMIN(buf_size, input->size);
+    buf_size = FFMIN(buf_size, static_cast<int>(input->size));


Here can be used std::fmin instead of this ugly macro. IIRC it's clang warn.

std::fmin is for floating points. That would create new warnings.
std::min has only templates for arguments of the same type.

ghost · 2018-08-30T15:49:37Z

rwengine/src/render/WaterRenderer.cpp

@@ -58,15 +58,15 @@ WaterRenderer::WaterRenderer(GameRenderer* renderer) {
 void WaterRenderer::setWaterTable(const float* waterHeights, const unsigned int nHeights,
                                  const uint8_t* tiles, const unsigned int nTiles) {
    // Determine the dimensions of the input tiles
-    int edgeNum = sqrt(nTiles);
+    unsigned int edgeNum = static_cast<unsigned int>(sqrt(nTiles));


ghost · 2018-08-30T15:50:09Z

rwengine/src/render/WaterRenderer.cpp

    float tileSize = WATER_WORLD_SIZE / edgeNum;
    glm::vec2 wO{-WATER_WORLD_SIZE / 2.f, -WATER_WORLD_SIZE / 2.f};

    std::vector<glm::vec3> vertexData;

-    for (int x = 0; x < edgeNum; x++) {
+    for (unsigned int x = 0; x < edgeNum; x++) {


for (auto x = 0u; x < edgeNum; x++) {

ghost · 2018-08-30T15:50:26Z

rwengine/src/render/WaterRenderer.cpp

        int xi = x * WATER_HQ_DATA_SIZE;
-        for (int y = 0; y < edgeNum; y++) {
+        for (unsigned int y = 0; y < edgeNum; y++) {


for (auto y = 0u; y < edgeNum; y++) {

ghost · 2018-08-30T15:51:46Z

rwviewer/models/IMGArchiveModel.hpp

@@ -14,15 +13,15 @@ class IMGArchiveModel : public QAbstractListModel {
        : QAbstractListModel(parent), archive(archive) {
    }

-    virtual int rowCount(const QModelIndex& parent = QModelIndex()) const;
+    virtual int rowCount(const QModelIndex& parent = QModelIndex()) const override;


If you use override there's no need for virtual.

ghost · 2018-08-31T16:03:17Z

rwengine/src/render/ViewFrustum.hpp

@@ -32,9 +32,9 @@ class ViewFrustum {
    }

    void update(const glm::mat4& proj) {
-        for (size_t i = 0; i < 6; ++i) {
+        for (unsigned int i = 0; i < 6; ++i) {


for (auto i = 0u; i < 6; ++i) {

ghost · 2018-08-31T16:03:48Z

rwengine/src/render/TextRenderer.cpp

@@ -284,7 +284,7 @@ void TextRenderer::renderText(const TextRenderer::TextInfo& ti,
        // will need to be wrapped
        if (ti.wrapX > 0 && coord.x > 0.f && !std::isspace(c)) {
            auto wend = std::find_if(std::begin(text) + i, std::end(text),
-                                     [](char x) { return std::isspace(x); });
+                                     [](GameStringChar c) { return std::isspace(c); });


can be here used auto?

danhedron · 2018-09-01T22:07:20Z

.appveyor.yml

@@ -22,7 +22,8 @@ platform:

 configuration:
 #  - Debug


Remove unused configurations

danhedron · 2018-09-01T22:08:00Z

cmake/ctest/build.ctest

+if(CC)
+    list(APPEND _CONGIGURE_OPTIONS "-DCMAKE_C_COMPILER=${CC}")
+endif()
+if(CXX)


Doesn't CMake respect the environment variables already?

Yup, and ctest does too. I overcomplicated things because at the start of a travis build, there is an explicit export CC=gcc and export CXX=g++

danhedron · 2018-09-01T22:11:16Z

cmake_configure.cmake

+    if(MSVC_PARALLEL_BUILD)
+        target_compile_options(rw_interface
+            INTERFACE
+                "/MP"


Does this need to be configurable? Can we put it in the default flags above.

It was a convenience for me to see what source emitted the bullet warnings. These warnings messages did not contain any reference to what source included them. But I guess its use is gone.

danhedron · 2018-09-01T22:13:44Z

rwcore/gl/DrawBuffer.cpp

-        glVertexAttribPointer(vaoindex, at.size, at.type, GL_TRUE, at.stride,
-                              reinterpret_cast<GLvoid*>(at.offset));
+        glVertexAttribPointer(vaoindex, static_cast<GLint>(at.size), at.type, GL_TRUE, at.stride,
+                              reinterpret_cast<void*>(static_cast<size_t>(at.offset)));


Make offset size_t?

danhedron · 2018-09-01T22:21:37Z

rwcore/loaders/LoaderDFF.cpp

@@ -259,7 +259,7 @@ GeometryPtr LoaderDFF::readGeometry(const RWBStream &stream) {
    glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, geom->EBO);

    size_t icount = std::accumulate(
-        geom->subgeom.begin(), geom->subgeom.end(), 0u,
+        geom->subgeom.begin(), geom->subgeom.end(), static_cast<size_t>(0u),


size_t{0u} is shorter

danhedron · 2018-09-01T22:28:31Z

rwengine/src/render/ObjectRenderer.hpp

 private:
    GameWorld* m_world;
    const ViewCamera& m_camera;
    float m_renderAlpha;
-    GLuint m_errorTexture;
+#ifdef RW_DEBUG
+    GLuint m_errorTexture = 0;


This looks completely unused

Yes, it was. Removed.

rwgame/GameConfig.cpp

ghost

LGTM

Fixes clang warnings about unused private fields: -Wunused-private-field

warning C4305: 'argument': truncation from 'double' to 'const btScalar'

This warning displays while building OpenRW in Visual Studio 2017 IDE

External projects do not need boost, glm or bullet specific options. Though, we do want to enable sanitizers and coverage on them.

madebr · 2018-09-15T23:51:32Z

I've disabled SIMD for glm by setting the compile definition GLM_FORCE_PURE.
An alternative is to not define it and use GLM_RELAXED_CONSTEXPR in our codebase instead of constexpr/const (when using glm data types).

danhedron · 2018-09-16T23:07:18Z

LGTM

madebr commented Aug 30, 2018

View reviewed changes

rwcore/gl/DrawBuffer.cpp Outdated Show resolved Hide resolved

madebr force-pushed the fix_msvc_warnings branch from 1f8c320 to 05fa3f5 Compare August 30, 2018 15:44

ghost reviewed Aug 30, 2018

View reviewed changes

madebr force-pushed the fix_msvc_warnings branch 5 times, most recently from efc4872 to cc48a70 Compare August 31, 2018 15:03

ghost reviewed Aug 31, 2018

View reviewed changes

madebr force-pushed the fix_msvc_warnings branch 3 times, most recently from a7bee5e to 26a5d50 Compare September 1, 2018 01:17

madebr changed the title ~~Fix Visual Studio Warnings~~ Fix Visual Studio and XCode Warnings Sep 1, 2018

madebr force-pushed the fix_msvc_warnings branch 7 times, most recently from 6fe0bdd to 413cb50 Compare September 1, 2018 16:59

danhedron reviewed Sep 1, 2018

View reviewed changes

madebr force-pushed the fix_msvc_warnings branch 4 times, most recently from e56bdab to 5df0d05 Compare September 2, 2018 00:26

ghost reviewed Sep 2, 2018

View reviewed changes

rwgame/GameConfig.cpp Outdated Show resolved Hide resolved

ghost approved these changes Sep 2, 2018

View reviewed changes

madebr force-pushed the fix_msvc_warnings branch from 5df0d05 to 13c1169 Compare September 3, 2018 22:44

madebr added 20 commits September 16, 2018 00:52

rwengine: fix Visual Studio warnings

05896ca

rwgame: fix Visual Studio warnings

2a1163d

rwviewer: fix Visual Studio warnings

089b49b

cmake: add all files to cmake

2d595be

rwgame+rwviewer: no more "#pragma once"

0b5500c

tests: fix for xcode

cce05da

docker: install clang

764fe2e

ci: build openrw using clang on fedora

890deb2

rwengine: some variables are only used in debug builds

7bea25e

Fixes clang warnings about unused private fields: -Wunused-private-field

rwengine+tests: disable C4305 warning for bullet3

32d3ab7

warning C4305: 'argument': truncation from 'double' to 'const btScalar'

cmake: enable parallel build on MSVC

56bd77a

ctest+ci: enable parallel building

564f6ef

all: fix 'warning: missing braces around initializer'

0b8bc41

rwengine: remove unused private field

9189a02

cmake: fix 'XXX.o has no symbols'

f08dffd

rwengine: remove errorTexture from Game- and ObjectRenderer

e873f82

cmake: disable Visual Studio warning about missing PDB's during linking

fdaaef3

This warning displays while building OpenRW in Visual Studio 2017 IDE

cmake+ci: install pdb's + enable on ci

c73e5d5

cmake: split cmake in interface + check interface

0e519d7

External projects do not need boost, glm or bullet specific options. Though, we do want to enable sanitizers and coverage on them.

rwengine: pass const glm::vec's to constructors

40575e7

madebr force-pushed the fix_msvc_warnings branch from b979a9a to 85080c8 Compare September 15, 2018 22:54

rwengine: use applyOffset

d06d4fc

madebr force-pushed the fix_msvc_warnings branch from 85080c8 to a1343e6 Compare September 15, 2018 23:14

madebr added 2 commits September 17, 2018 19:14

cmake: detect glm version

d1cb0f1

cmake: disable SIMD for glm to enable constexpr for glm types

0923eca

madebr force-pushed the fix_msvc_warnings branch from a1343e6 to 0923eca Compare September 17, 2018 17:15

danhedron merged commit 5951b97 into rwengine:master Sep 17, 2018

madebr deleted the fix_msvc_warnings branch September 17, 2018 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Visual Studio and XCode Warnings #618

Fix Visual Studio and XCode Warnings #618

madebr commented Aug 30, 2018 •

edited

Loading

madebr commented Aug 30, 2018 •

edited

Loading

ghost left a comment

ghost Aug 30, 2018

ghost Aug 30, 2018

madebr Aug 30, 2018

ghost Aug 30, 2018

ghost Aug 30, 2018

ghost Aug 30, 2018

ghost Aug 30, 2018

ghost Aug 31, 2018

ghost Aug 31, 2018

danhedron Sep 1, 2018

danhedron Sep 1, 2018

madebr Sep 1, 2018

danhedron Sep 1, 2018

madebr Sep 1, 2018

danhedron Sep 1, 2018

danhedron Sep 1, 2018

danhedron Sep 1, 2018

madebr Sep 1, 2018

ghost left a comment

madebr commented Sep 15, 2018 •

edited

Loading

danhedron commented Sep 16, 2018

Fix Visual Studio and XCode Warnings #618

Fix Visual Studio and XCode Warnings #618

Conversation

madebr commented Aug 30, 2018 • edited Loading

madebr commented Aug 30, 2018 • edited Loading

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

madebr commented Sep 15, 2018 • edited Loading

danhedron commented Sep 16, 2018

madebr commented Aug 30, 2018 •

edited

Loading

madebr commented Aug 30, 2018 •

edited

Loading

madebr commented Sep 15, 2018 •

edited

Loading