Raise warning levels to /W4 & -Wall -Wextra (#418)

Several warnings are disabled for the basisu files and one for the gtest files. Other changes included here: * Add a couple of bad ktx2 files for testing ktx2check * Updates the Travis Build to Xcode 12.4 along with using xcpretty to shorten the log so as to remain within Travis-CI limits. * Fixes some warnings in the .travis.yml file.
KhronosGroup · May 7, 2021 · ca6f6e7 · ca6f6e7
1 parent 78cd2b0
commit ca6f6e7
Show file tree

Hide file tree

Showing 97 changed files with 1,059 additions and 631 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -8,7 +8,10 @@ language:
   - cpp
   - node_js
 
-matrix:
+os: linux
+dist: focal
+
+jobs:
   include:
     - os: linux
       addons:
@@ -28,7 +31,6 @@ matrix:
       python: '3.7'
       services:
         - docker
-      sudo: required
       node_js:
         - node
     - os: osx
@@ -37,7 +39,7 @@ matrix:
         - VULKAN_INSTALL_DIR="/Volumes/vulkansdk-macos-$VULKAN_SDK_VER"
         - VULKAN_SDK="$VULKAN_INSTALL_DIR/macOS"
       compiler: clang
-      osx_image: xcode11.4
+      osx_image: xcode12.4
 
 #env:
   #global:
@@ -130,7 +132,7 @@ deploy:
   - provider: pages
     #edge: true # Use bleeding edge (dplv2)
     skip_cleanup: true
-    github_token: $GITHUB_TOKEN  # Set in the repo settings page as a secure variable
+    token: $GITHUB_TOKEN  # Set in the repo settings page as a secure variable
     local_dir: build-macos/docs/html
     on:
       #branch: master
@@ -141,7 +143,7 @@ deploy:
     # and the deploy below. Happens even when neither of release_notes or
     # release_notes_file is specified.
     #edge: true # Use bleeding edge (dplv2)
-    api_key: $GITHUB_TOKEN  # Set in the repo settings page as a secure variable
+    token: $GITHUB_TOKEN  # Set in the repo settings page as a secure variable
     file_glob: true
     file:
       - build-macos/KTX-Software-*.pkg
@@ -161,7 +163,7 @@ deploy:
       condition: $TRAVIS_OS_NAME = osx
   - provider: releases
     #edge: true # Use bleeding edge (dplv2)
-    api_key: $GITHUB_TOKEN  # Set in the repo settings page as a secure variable
+    token: $GITHUB_TOKEN  # Set in the repo settings page as a secure variable
     file_glob: true
     file:
       - build/linux-release/KTX-Software-*-Linux.*

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -84,7 +84,7 @@ endif()
 
 set(bitness 64)
 if(NOT CMAKE_SIZEOF_VOID_P EQUAL 8 OR FORCE32)
-	set(bitness 32)
+        set(bitness 32)
 endif()
 
 if(KTX_FEATURE_STATIC_LIBRARY)
@@ -96,15 +96,23 @@ else()
     set(LIB_TYPE SHARED)
 endif()
 
-# Global Optimization Flags
-if(EMSCRIPTEN)
-    add_compile_options( $<IF:$<CONFIG:Debug>,-O0,-O3> )
-    add_link_options( $<IF:$<CONFIG:Debug>,-gsource-map,-O3> )
-elseif(MSVC)
+# Global compile & link options including optimization flags
+if(MSVC)
+    # With /W4 VS2015 (V19.0) issues many warnings that VS2017 & 2019 don't
+    # so only increase warning level for recent versions.
+    add_compile_options($<$<VERSION_GREATER_EQUAL:${CMAKE_CXX_COMPILER_VERSION},19.16>:/W4>)
     add_compile_options( $<IF:$<CONFIG:Debug>,/Gz,/O2> )
-else()
+elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU"
+       OR ${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
+    add_compile_options(-Wall -Wextra)
     add_compile_options( $<IF:$<CONFIG:Debug>,-O0,-O3> )
-    add_link_options( $<IF:$<CONFIG:Debug>,-g,-O3> )
+    if(EMSCRIPTEN)
+        add_link_options( $<IF:$<CONFIG:Debug>,-gsource-map,-O3> )
+    else()
+        add_link_options( $<IF:$<CONFIG:Debug>,-g,-O3> )
+    endif()
+else()
+    message(ERROR "${CMAKE_CXX_COMPILER_ID} not yet supported.")
 endif()
 
 set(KTX_MAIN_SRC
@@ -231,6 +239,9 @@ macro(commom_lib_settings lib write)
 
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/other_include>
         $<INSTALL_INTERFACE:other_include>
+
+        $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/utils>
+        $<INSTALL_INTERFACE:utils>
     )
 
     if( LIB_TYPE STREQUAL STATIC )
@@ -364,12 +375,12 @@ PRIVATE
 
 # Adding write capability to target ktx
 
-target_sources(
-    ktx
-PRIVATE
-    lib/basis_encode.cpp
+set(BASISU_ENCODER_C_SRC
     lib/basisu/encoder/apg_bmp.c
     lib/basisu/encoder/apg_bmp.h
+    )
+
+set(BASISU_ENCODER_CXX_SRC
     lib/basisu/encoder/basisu_astc_decomp.cpp
     lib/basisu/encoder/basisu_astc_decomp.h
     lib/basisu/encoder/basisu_backend.cpp
@@ -413,9 +424,69 @@ PRIVATE
     lib/basisu/encoder/jpgd.h
     lib/basisu/encoder/lodepng.cpp
     lib/basisu/encoder/lodepng.h
+    )
+
+target_sources(
+    ktx
+PRIVATE
+    lib/basis_encode.cpp
+    ${BASISU_ENCODER_C_SRC}
+    ${BASISU_ENCODER_CXX_SRC}
     lib/writer1.c
     lib/writer2.c
 )
+
+# Turn off these warnings until Rich fixes the occurences.
+# It it not clear to me if generator expressions can be used here
+# hence the long-winded way.
+if(MSVC)
+    set_source_files_properties(
+        lib/basisu/transcoder/basisu_transcoder.cpp
+        lib/basisu/encoder/basisu_enc.cpp
+        PROPERTIES COMPILE_OPTIONS "/wd4100;/wd4189"
+        )
+    set_source_files_properties(
+        lib/basisu/encoder/basisu_kernels_sse.cpp
+        lib/basisu/encoder/basisu_comp.cpp
+        PROPERTIES COMPILE_OPTIONS "/wd4244"
+        )
+    set_source_files_properties(
+        lib/basisu/encoder/basisu_uastc_enc.cpp
+        PROPERTIES COMPILE_OPTIONS "/wd4800"
+        )
+elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU")
+    set_source_files_properties(
+        # It's too much work to discriminate which files need which warnings
+        # disabled.
+        ${BASISU_ENCODER_CXX_SRC}
+        PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-class-memaccess;-Wno-misleading-indentation;-Wno-extra;-Wno-deprecated-copy;-Wno-parentheses;-Wno-strict-aliasing"
+        )
+    set_source_files_properties(
+        lib/basisu/transcoder/basisu_transcoder.cpp
+        PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-function;-Wno-unused-variable;-Wno-class-memaccess;-Wno-maybe-uninitialized"
+        )
+elseif(EMSCRIPTEN)
+    set_source_files_properties(
+        ${BASISU_ENCODER_CXX_SRC}
+        PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-unused-parameter"
+        )
+    set_source_files_properties(
+        lib/basisu/transcoder/basisu_transcoder.cpp
+        PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-function;-Wno-unused-variable;"
+        )
+elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
+    set_source_files_properties(
+        lib/basisu/transcoder/basisu_transcoder.cpp
+        lib/basisu/encoder/basisu_enc.cpp
+        PROPERTIES COMPILE_OPTIONS "-Wno-unused-variable;-Wno-sign-compare;-Wno-shorten-64-to-32"
+        )
+    set_source_files_properties( lib/basisu/encoder/basisu_kernels_sse.cpp
+        PROPERTIES COMPILE_OPTIONS "-Wno-unused-parameter"
+        )
+else()
+    message(ERROR "${CMAKE_CXX_COMPILER_ID} not yet supported.")
+endif()
+
 target_include_directories(
     ktx
 PRIVATE

diff --git a/ci_scripts/build_macos.sh b/ci_scripts/build_macos.sh
@@ -35,20 +35,24 @@ fi
 echo "Configure KTX-Software (macOS) without SSE support"
 cmake -GXcode -Bbuild-macos-nosse -DBASISU_SUPPORT_SSE=OFF
 
+# Cause the build pipes below to set the exit to the exit code of the
+# last program to exit non-zero.
+set -o pipefail
+
 pushd build-macos
 
 # Build and test Debug
 echo "Build KTX-Software (macOS Debug)"
-cmake --build . --config Debug -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+cmake --build . --config Debug -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO | tee fullbuild.log | xcpretty
 echo "Test KTX-Software (macOS Debug)"
 ctest -C Debug # --verbose
 
 # Build and test Release
 echo "Build KTX-Software (macOS Release)"
 if [ -n "$MACOS_CERTIFICATES_P12" ]; then
-  cmake --build . --config Release
+  cmake --build . --config Release | tee -a fullbuild.log | xcpretty
 else
-  cmake --build . --config Release -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+  cmake --build . --config Release -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO | tee -a fullbuild.log | xcpretty
 fi
 echo "Test KTX-Software (macOS Release)"
 ctest -C Release # --verbose
@@ -65,11 +69,13 @@ popd
 pushd build-macos-nosse
 
 echo "Build KTX-Software (macOS without SSE support Debug)"
-cmake --build . --config Debug -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+cmake --build . --config Debug -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO | tee -a fullbuild.log | xcpretty
+
 echo "Test KTX-Software (macOS without SSE support Debug)"
 ctest -C Debug # --verbose
 echo "Build KTX-Software (macOS without SSE support Release)"
-cmake --build . --config Release -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+cmake --build . --config Release -- CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO | tee -a fullbuild.log | xcpretty
+
 echo "Test KTX-Software (macOS without SSE support Release)"
 ctest -C Release # --verbose
 
@@ -83,11 +89,11 @@ echo "Configure KTX-Software (iOS)"
 cmake -GXcode -Bbuild-ios -DCMAKE_SYSTEM_NAME=iOS -DKTX_FEATURE_LOADTEST_APPS=ON -DKTX_FEATURE_DOC=ON -DVULKAN_INSTALL_DIR="${VULKAN_INSTALL_DIR}"
 pushd build-ios
 echo "Build KTX-Software (iOS Debug)"
-cmake --build . --config Debug  -- -sdk iphoneos CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+cmake --build . --config Debug  -- -sdk iphoneos CODE_SIGN_IDENTITY="" CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO | tee -a fullbuild.log | xcpretty
 # echo "Build KTX-Software (iOS Simulator Debug)"
 # cmake --build . --config Debug -- -sdk iphonesimulator
 echo "Build KTX-Software (iOS Release)"
-cmake --build . --config Release -- -sdk iphoneos CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO
+cmake --build . --config Release -- -sdk iphoneos CODE_SIGN_IDENTITY="" CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO | tee -a fullbuild.log | xcpretty
 # echo "Build KTX-Software (iOS Simulator Release)"
 # cmake --build . --config Release -- -sdk iphonesimulator
 popd
diff --git a/ci_scripts/install_macos.sh b/ci_scripts/install_macos.sh
@@ -12,6 +12,7 @@ git lfs version
 brew install doxygen
 brew install sdl2
 brew link sdl2
+gem install xcpretty
 
 # Current directory is .../build/{KhronosGroup,msc-}/KTX-Software. cd to 'build'.
 pushd ../..

diff --git a/interface/basisu_c_binding/inc/basisu_c_binding.h b/interface/basisu_c_binding/inc/basisu_c_binding.h
@@ -17,7 +17,7 @@ extern basist::etc1_global_selector_codebook *g_pGlobal_codebook;
 
 class basis_file
 {
-    int m_magic = 0;
+    unsigned int m_magic = 0;
     basisu_transcoder m_transcoder;
     const uint8_t *m_file;
     uint32_t byteLength;

diff --git a/interface/basisu_c_binding/src/basisu_c_binding.cpp b/interface/basisu_c_binding/src/basisu_c_binding.cpp
@@ -146,7 +146,7 @@ uint32_t basis_file::startTranscoding() {
     return m_transcoder.start_transcoding(m_file, byteLength);
 }
 
-uint32_t basis_file::transcodeImage(void* dst, uint32_t dst_size, uint32_t image_index, uint32_t level_index, uint32_t format, uint32_t pvrtc_wrap_addressing, uint32_t get_alpha_for_opaque_formats) {
+uint32_t basis_file::transcodeImage(void* dst, uint32_t dst_size, uint32_t image_index, uint32_t level_index, uint32_t format, uint32_t /*pvrtc_wrap_addressing*/, uint32_t get_alpha_for_opaque_formats) {
     assert(m_magic == MAGIC);
     if (m_magic != MAGIC)
         return 0;
@@ -160,7 +160,7 @@ uint32_t basis_file::transcodeImage(void* dst, uint32_t dst_size, uint32_t image
     if (!m_transcoder.get_image_level_desc(m_file, byteLength, image_index, level_index, orig_width, orig_height, total_blocks))
         return 0;
 
-  uint32_t flags = get_alpha_for_opaque_formats ? cDecodeFlagsTranscodeAlphaDataToOpaqueFormats : 0;
+    uint32_t flags = get_alpha_for_opaque_formats ? cDecodeFlagsTranscodeAlphaDataToOpaqueFormats : 0;
 
     uint32_t status;
 
@@ -185,7 +185,7 @@ uint32_t basis_file::transcodeImage(void* dst, uint32_t dst_size, uint32_t image
     {
         uint32_t bytes_per_block = basis_get_bytes_per_block_or_pixel(transcoder_format);
 
-        uint32_t required_size = total_blocks * bytes_per_block;
+        uint32_t MAYBE_UNUSED required_size = total_blocks * bytes_per_block;
 
         if (transcoder_format == transcoder_texture_format::cTFPVRTC1_4_RGB || transcoder_format == transcoder_texture_format::cTFPVRTC1_4_RGBA)
         {
@@ -241,7 +241,7 @@ DLL_EXPORT void ktx_basisu_delete_basis( basis_file* basis ) {
 
 DLL_EXPORT bool ktx_basisu_getHasAlpha( basis_file* basis ) {
     assert(basis!=nullptr);
-    return basis->getHasAlpha();
+    return (bool)basis->getHasAlpha();
 }
 
 DLL_EXPORT uint32_t ktx_basisu_getNumImages( basis_file* basis ) {
@@ -265,11 +265,11 @@ DLL_EXPORT uint32_t ktx_basisu_getImageTranscodedSizeInBytes( basis_file* basis,
 }
 
 DLL_EXPORT bool ktx_basisu_startTranscoding( basis_file* basis ) {
-    return basis->startTranscoding();
+    return (bool)basis->startTranscoding();
 }
 
 DLL_EXPORT bool ktx_basisu_transcodeImage( basis_file* basis, void* dst, uint32_t dst_size, uint32_t image_index, uint32_t level_index, uint32_t format, uint32_t pvrtc_wrap_addressing, uint32_t get_alpha_for_opaque_formats) {
-    return basis->transcodeImage(dst,dst_size,image_index,level_index,format,pvrtc_wrap_addressing,get_alpha_for_opaque_formats);
+    return (bool)basis->transcodeImage(dst,dst_size,image_index,level_index,format,pvrtc_wrap_addressing,get_alpha_for_opaque_formats);
 }
 
 #endif

diff --git a/interface/js_binding/ktx_wrapper.cpp b/interface/js_binding/ktx_wrapper.cpp
@@ -155,7 +155,7 @@ namespace ktx
             ret.set("texture", texture);
             ret.set("target", target);
             ret.set("error", error);
-            return std::move(ret);
+            return ret;
         }
 
     private:

diff --git a/interface/js_binding/transcoder_wrapper.cpp b/interface/js_binding/transcoder_wrapper.cpp
@@ -276,7 +276,7 @@ namespace msc {
             if (status) {
                 ret.set("transcodedImage", dst);
             }
-            return std::move(ret);
+            return ret;
         }
 
       protected:
@@ -373,7 +373,7 @@ namespace msc {
             if (status) {
                 ret.set("transcodedImage", dst);
             }
-            return std::move(ret);
+            return ret;
         }
     };