CI and Build Improvements (KhronosGroup#687)

    Remove VS2015 build from CI.
    Move Windows builds (except VS2017) to GitHub Actions.
    Run Windows builds and tests with both MSVC and CLangCL on VS2022.
    Build one configuration with VS2019 to ensure it still works.
    Build Windows release packages with VS2022 and CLangCL.
    Turn on warnings as errors. Fix bug that prevented /W4 for CLangCL. Fix all warnings.
    Update the included zstd.c and regen golden images for affected tests.
    Stop using deprecated code in GitHub Actions.
    Update Linux CI runner to Ubuntu Jammy
    Update Vulkan SDK used in Linux and Windows CI to 1.3.243.
    Fix issue with Doxygen builds that caused intermittent failures.

Author: MarkCallow

CMakeLists.txt

@@ -22,8 +22,6 @@ include(CMakeDependentOption)
 include(cmake/codesign.cmake)
 include(cmake/cputypetest.cmake)
 
-find_package(OpenCL)
-
 # OPTIONS
 
 if( IOS OR EMSCRIPTEN )
@@ -64,6 +62,10 @@ CMAKE_DEPENDENT_OPTION( BASISU_SUPPORT_OPENCL
     OFF
 )
 
+if(BASISU_SUPPORT_OPENCL)
+    find_package(OpenCL)
+endif()
+
 if(BASISU_SUPPORT_OPENCL AND WIN32 AND NOT OpenCL_FOUND)
     # To avoid fiddly setting up of OpenCL on Windows CI VMs, use copy in repo.
     set(OpenCL_INCLUDE_DIR
@@ -212,13 +214,11 @@ include(cmake/mkvk.cmake)
 
 # 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( /W4;/WX )
     add_compile_options( $<IF:$<CONFIG:Debug>,/Gz,/O2> )
 elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU"
        OR ${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
-    add_compile_options(-Wall -Wextra)
+    add_compile_options(-Wall -Wextra -Werror)
     add_compile_options( $<IF:$<CONFIG:Debug>,-O0,-O3> )
     if(EMSCRIPTEN)
         add_link_options( $<IF:$<CONFIG:Debug>,-gsource-map,-O3> )
@@ -606,10 +606,39 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
         lib/basisu/transcoder/basisu_transcoder.cpp
         PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-function;-Wno-unused-variable;-Wno-class-memaccess;-Wno-maybe-uninitialized"
     )
-    if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "11.0")
+    if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "11")
+        # class-memaccess warnings n basisu_comp.cpp have been
+        # fixed. Fixing those in the included basisu_miniz.h is
+        # more intrusive than I want to attempt.
+        #
+        # The extra warnings are raised in basisu_miniz.h.
         set_source_files_properties(
-            lib/basisu/zstd/zstd.c
-            PROPERTIES COMPILE_OPTIONS "-Wno-unused-but-set-variable"
+            lib/basisu/encoder/basisu_comp.cpp
+            PROPERTIES COMPILE_OPTIONS "-Wno-class-memaccess;-Wno-extra"
+        )
+        if (${CMAKE_CXX_COMPILER_VERSION} VERSION_LESS "12" )
+        # Version 11 raises several stringop-overflow warnings in some
+        # very hard to decipher code. They appear to be bogus based on
+        # the facts that we have never seen a crash and version 12 no
+        # longer raises the warnings.
+        get_source_file_property(cur_options
+        lib/basisu/encoder/basisu_comp.cpp
+        COMPILE_OPTIONS
+        )
+        set_source_files_properties(
+        lib/basisu/encoder/basisu_comp.cpp
+        PROPERTIES COMPILE_OPTIONS "${cur_options};-Wno-stringop-overflow"
+        )
+        endif()
+    endif()
+    if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "12.0")
+        # Version 12 newly raises this warning on basisu_uastc_enc.cpp.
+        # There seems no way for the index calculated by the code at
+        # line 326, where the error is raised, to be > the array length.
+        # Also we have never seen any crashes.
+        set_source_files_properties(
+            lib/basisu/encoder/basisu_uastc_enc.cpp
+            PROPERTIES COMPILE_OPTIONS "-Wno-stringop-overflow"
         )
     endif()
 elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
@@ -631,31 +660,16 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
     else()
         set( clang_version ${CMAKE_CXX_COMPILER_VERSION} )
     endif()
+    # BEWARE: set_source_files_properties is not additive; it replaces.
     if (${clang_version} VERSION_GREATER_EQUAL "12.0.0")
         set_source_files_properties( lib/basisu/encoder/basisu_kernels_sse.cpp
             PROPERTIES COMPILE_OPTIONS "-Wno-unused-parameter;-Wno-deprecated-copy;-Wno-uninitialized-const-reference"
         )
-        # Primarily an issue on windows using clang/clangcl
-        # Lowest tested clang version was 12.0.0 and highest was 14.0.4
-        set_source_files_properties(
-            lib/basisu/zstd/zstd.c
-            PROPERTIES COMPILE_OPTIONS "-Wno-unused-function"
-        )
     else()
         set_source_files_properties( lib/basisu/encoder/basisu_kernels_sse.cpp
             PROPERTIES COMPILE_OPTIONS "-Wno-unused-parameter"
         )
     endif()
-    if (${clang_version} VERSION_GREATER_EQUAL "13.0.0")
-        get_source_file_property(zstd_options lib/basisu/zstd/zstd.c COMPILE_OPTIONS)
-        set_source_files_properties(
-            # We haven't fixed zstd.c because the fix would have to be applied
-            # every time the upstream script is used to create an updated
-            # single file decoder.
-            lib/basisu/zstd/zstd.c
-            PROPERTIES COMPILE_OPTIONS "${zstd_options};-Wno-unused-but-set-variable"
-        )
-    endif()
     if (${clang_version} VERSION_GREATER_EQUAL "15.0")
         # These are for Emscripten which is ahead of xcode in its clang
         # version. Also future proofing for when xcode catches up.
@@ -669,13 +683,23 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
         )
         set_source_files_properties(
             lib/basisu/zstd/zstd.c
-            PROPERTIES COMPILE_OPTIONS "-Wno-unused-but-set-variable;-Wno-bitwise-instead-of-logical"
+            PROPERTIES COMPILE_OPTIONS "-Wno-unused-function"
         )
     endif()
 else()
     message(FATAL_ERROR "${CMAKE_CXX_COMPILER_ID} not yet supported.")
 endif()
 
+# Retrieve the final set of properties for use by the transcodetests.
+# We do this because the CMake feature that would allow the transcodetests
+# target to retrieve these from the ktx target are not available until
+# v18 and we still need to work with v16 in the Emscripten Docker image.
+get_source_file_property(transcoder_options
+    lib/basisu/transcoder/basisu_transcoder.cpp
+    COMPILE_OPTIONS
+)
+get_source_file_property(zstd_options lib/basisu/zstd/zstd.c COMPILE_OPTIONS)
+
 target_include_directories(
     ktx
 PRIVATE

cmake/docs.cmake

@@ -3,9 +3,18 @@
 
 find_package(Doxygen REQUIRED)
 
+set(docdest "${CMAKE_CURRENT_BINARY_DIR}/docs")
+
+# The torturous Doxygen output settings are a hack
+# to workaround a doxygen limitation where it will
+# only create the trailing directory of the path
+# given in, e.g DOXYGEN_HTML_OUTPUT while enabling
+# us to put each sub-document in its own directory of
+# the html output.
+
 # Global
 set( DOXYGEN_PROJECT_LOGO icons/ktx_logo_200.png )
-set( DOXYGEN_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/docs)
+set( DOXYGEN_OUTPUT_DIRECTORY ${docdest}/html)
 set( DOXYGEN_OPTIMIZE_OUTPUT_FOR_C YES )
 set( DOXYGEN_EXTRACT_LOCAL_CLASSES NO )
 set( DOXYGEN_HIDE_UNDOC_MEMBERS YES )
@@ -18,16 +27,17 @@ set( DOXYGEN_HTML_TIMESTAMP YES )
 set( DOXYGEN_DISABLE_INDEX YES )
 set( DOXYGEN_GENERATE_TREEVIEW YES )
 set( DOXYGEN_GENERATE_LATEX NO )
+set( DOXYGEN_GENERATE_HTML YES )
 set( DOXYGEN_GENERATE_MAN YES )
-
+set( DOXYGEN_MAN_OUTPUT ../man )
 
 function( add_docs_cmake target )
     # Make `docs.cmake` show up in IDE/project
-    get_target_property( DOC_SOURCES ${target} SOURCES )
+    get_target_property( doc_sources ${target} SOURCES )
     set_target_properties(
         ${target}
     PROPERTIES
-        SOURCES "${DOC_SOURCES};cmake/docs.cmake"
+        SOURCES "${doc_sources};cmake/docs.cmake"
     )
 endfunction()
 
@@ -42,7 +52,7 @@ function( CreateDocLibKTX )
     # This does not hide the scope (class) names in the Modules list
     # in the ToC. See https://github.com/doxygen/doxygen/issues/9921.
     set( DOXYGEN_HIDE_SCOPE_NAMES YES )
-    set( DOXYGEN_HTML_OUTPUT html/libktx )
+    set( DOXYGEN_HTML_OUTPUT libktx )
     # Order is important here. '_' suffixed prefices must come first
     # otherwise the non-suffixed is stripped first leaving just '_'.
     set( DOXYGEN_IGNORE_PREFIX KTX_;ktx_;KTX;ktx )
@@ -77,10 +87,7 @@ function( CreateDocLibKTX )
         ktx_size_t dataSize\;                      \\
         ktx_uint8_t* pData\;"
     )
-    set( DOXYGEN_GENERATE_TAGFILE ${CMAKE_BINARY_DIR}/docs/libktx.tag )
-
-    file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/docs )
-    file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/docs/html )
+    set( DOXYGEN_GENERATE_TAGFILE ${docdest}/libktx.tag )
 
     doxygen_add_docs(
         libktx.doc
@@ -114,9 +121,9 @@ function( CreateDocKTXTools )
     set( DOXYGEN_FILE_PATTERNS *.cpp )
     set( DOXYGEN_RECURSIVE YES )
     set( DOXYGEN_EXAMPLE_PATH utils )
-    set( DOXYGEN_HTML_OUTPUT html/ktxtools )
+    set( DOXYGEN_HTML_OUTPUT ktxtools )
     set( DOXYGEN_MAN_EXTENSION .1 )
-    set( DOXYGEN_GENERATE_TAGFILE ${CMAKE_BINARY_DIR}/docs/ktxtools.tag )
+    set( DOXYGEN_GENERATE_TAGFILE ${docdest}/ktxtools.tag )
 
     doxygen_add_docs(
         ktxtools.doc
@@ -139,9 +146,9 @@ function( CreateDocKTX )
     set( DOXYGEN_EXCLUDE_PATTERNS ktxint.h )
     set( DOXYGEN_EXAMPLE_PATH lib )
     set( DOXYGEN_HTML_HEADER pkgdoc/header.html )
-    set( DOXYGEN_MAN_OUTPUT man/ktx )
+    set( DOXYGEN_HTML_OUTPUT . )
     set( DOXYGEN_MAN_LINKS YES )
-    set( DOXYGEN_TAGFILES ${CMAKE_BINARY_DIR}/docs/libktx.tag=libktx ${CMAKE_BINARY_DIR}/docs/ktxtools.tag=ktxtools )
+    set( DOXYGEN_TAGFILES ${docdest}/libktx.tag=libktx ${docdest}/ktxtools.tag=ktxtools )
 
     doxygen_add_docs(
         ktxpkg.doc
@@ -163,16 +170,16 @@ add_dependencies( ktxpkg.doc ktxtools.doc )
 
 
 install(
-    DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/docs/html
+    DIRECTORY ${docdest}/html
     DESTINATION "${CMAKE_INSTALL_DOCDIR}"
-    COMPONENT dev
+    COMPONENT tools
 )
 
 install( DIRECTORY
-    ${CMAKE_CURRENT_BINARY_DIR}/docs/man/man1
+    ${docdest}/man/man1
     ## Omit those, since at the moment they contain a lot of undesired files generated by Doxygen
-    # ${CMAKE_CURRENT_BINARY_DIR}/docs/man/man3
-    # ${CMAKE_CURRENT_BINARY_DIR}/docs/man/ktx/man3
+    # ${docdest}/man/man3
+    # ${docdest}/man/ktx/man3
 DESTINATION
     "${CMAKE_INSTALL_MANDIR}"
 COMPONENT tools

lib/basisu/encoder/basisu_comp.cpp

@@ -1819,8 +1819,7 @@ namespace basisu
 			total_faces = 6;
 		}
 
-		basist::ktx2_header header;
-		memset(&header, 0, sizeof(header));
+		basist::ktx2_header header = {};
 
 		memcpy(header.m_identifier, basist::g_ktx2_file_identifier, sizeof(basist::g_ktx2_file_identifier));
 		header.m_pixel_width = base_width;

lib/basisu/encoder/basisu_miniz.h

@@ -1181,7 +1181,10 @@ tinfl_status tinfl_decompress(tinfl_decompressor *r, const mz_uint8 *pIn_buf_nex
       {
         mz_uint8 *p = r->m_tables[0].m_code_size; mz_uint i;
         r->m_table_sizes[0] = 288; r->m_table_sizes[1] = 32; TINFL_MEMSET(r->m_tables[1].m_code_size, 5, 32);
-        for ( i = 0; i <= 143; ++i) *p++ = 8; for ( ; i <= 255; ++i) *p++ = 9; for ( ; i <= 279; ++i) *p++ = 7; for ( ; i <= 287; ++i) *p++ = 8;
+        for ( i = 0; i <= 143; ++i) *p++ = 8;
+        for ( ; i <= 255; ++i) *p++ = 9;
+        for ( ; i <= 279; ++i) *p++ = 7;
+        for ( ; i <= 287; ++i) *p++ = 8;
       }
       else
       {
@@ -1594,10 +1597,11 @@ static void tdefl_optimize_huffman_table(tdefl_compressor *d, int table_num, int
   mz_uint bits = b; mz_uint len = l; MZ_ASSERT(bits <= ((1U << len) - 1U)); \
   d->m_bit_buffer |= (bits << d->m_bits_in); d->m_bits_in += len; \
   while (d->m_bits_in >= 8) { \
-    if (d->m_pOutput_buf < d->m_pOutput_buf_end) \
+    if (d->m_pOutput_buf < d->m_pOutput_buf_end) { \
       *d->m_pOutput_buf++ = (mz_uint8)(d->m_bit_buffer); \
-      d->m_bit_buffer >>= 8; \
-      d->m_bits_in -= 8; \
+    } \
+    d->m_bit_buffer >>= 8; \
+    d->m_bits_in -= 8; \
   } \
 } MZ_MACRO_END
 
@@ -1968,7 +1972,10 @@ static MZ_FORCEINLINE void tdefl_find_match(tdefl_compressor *d, mz_uint lookahe
         if (TDEFL_READ_UNALIGNED_WORD(&d->m_dict[probe_pos + match_len - 1]) == c01) break;
       TDEFL_PROBE; TDEFL_PROBE; TDEFL_PROBE;
     }
-    if (!dist) break; q = (const mz_uint16*)(d->m_dict + probe_pos); if (TDEFL_READ_UNALIGNED_WORD(q) != s01) continue; p = s; probe_len = 32;
+    if (!dist) break;
+    q = (const mz_uint16*)(d->m_dict + probe_pos);
+    if (TDEFL_READ_UNALIGNED_WORD(q) != s01) continue;
+    p = s; probe_len = 32;
     do { } while ( (TDEFL_READ_UNALIGNED_WORD(++p) == TDEFL_READ_UNALIGNED_WORD(++q)) && (TDEFL_READ_UNALIGNED_WORD(++p) == TDEFL_READ_UNALIGNED_WORD(++q)) &&
                    (TDEFL_READ_UNALIGNED_WORD(++p) == TDEFL_READ_UNALIGNED_WORD(++q)) && (TDEFL_READ_UNALIGNED_WORD(++p) == TDEFL_READ_UNALIGNED_WORD(++q)) && (--probe_len > 0) );
     if (!probe_len)

lib/basisu/encoder/basisu_uastc_enc.cpp

@@ -2573,8 +2573,7 @@ namespace basisu
 		unpacked_uastc_block temp_ublock_unpacked;
 		unpack_uastc(temp_ublock, temp_ublock_unpacked, false);
 
-		unpacked_uastc_block ublock;
-		memset(&ublock, 0, sizeof(ublock));
+		unpacked_uastc_block ublock = {};
 		ublock.m_mode = best_results.m_uastc_mode;
 		ublock.m_common_pattern = best_results.m_common_pattern;
 		ublock.m_astc = temp_ublock_unpacked.m_astc;
@@ -3388,8 +3387,7 @@ namespace basisu
 			}
 		}
 
-		unpacked_uastc_block unpacked_ublock;
-		memset(&unpacked_ublock, 0, sizeof(unpacked_ublock));
+		unpacked_uastc_block unpacked_ublock = {};
 
 		uint64_t total_overall_err[MAX_ENCODE_RESULTS];
 		float uastc_err_f[MAX_ENCODE_RESULTS];

lib/basisu/encoder/cppspmd_sse.h

@@ -1206,7 +1206,7 @@ struct spmd_kernel
 	CPPSPMD_FORCE_INLINE vint load_all(const vint_vref& src)
 	{
 		// TODO: There's surely a better way
-		__m128i k;
+		__m128i k = {};
 
 		k = insert_x(k, ((int*)(&src.m_pValue[extract_x(src.m_vindex)]))[0]);
 		k = insert_y(k, ((int*)(&src.m_pValue[extract_y(src.m_vindex)]))[1]);

lib/basisu/encoder/jpgd.cpp

@@ -947,15 +947,15 @@ namespace jpgd {
 	// Finds the next marker.
 	int jpeg_decoder::next_marker()
 	{
-		uint c, bytes;
+		uint c; //, bytes;
 
-		bytes = 0;
+		//bytes = 0;
 
 		do
 		{
 			do
 			{
-				bytes++;
+				//bytes++;
 				c = get_bits(8);
 			} while (c != 0xFF);
 
@@ -1303,7 +1303,7 @@ namespace jpgd {
 		int i;
 		jpgd_block_t* p;
 		jpgd_quant_t* q;
-		int mcu_row, mcu_block, row_block = 0;
+		int mcu_row, mcu_block; //, row_block = 0;
 		int component_num, component_id;
 		int block_x_mcu[JPGD_MAX_COMPONENTS];
 
@@ -1338,7 +1338,7 @@ namespace jpgd {
 					if (p[g_ZAG[i]])
 						p[g_ZAG[i]] = static_cast<jpgd_block_t>(p[g_ZAG[i]] * q[i]);
 
-				row_block++;
+				//row_block++;
 
 				if (m_comps_in_scan == 1)
 					block_x_mcu[component_id]++;
@@ -1425,7 +1425,7 @@ namespace jpgd {
 	// Decodes and dequantizes the next row of coefficients.
 	void jpeg_decoder::decode_next_row()
 	{
-		int row_block = 0;
+		//int row_block = 0;
 
 		for (int mcu_row = 0; mcu_row < m_mcus_per_row; mcu_row++)
 		{
@@ -1528,7 +1528,7 @@ namespace jpgd {
 
 				m_mcu_block_max_zag[mcu_block] = k;
 
-				row_block++;
+				//row_block++;
 			}
 
 			transform_mcu(mcu_row);