From 7ff6558e56d284cc3ba66e7fa2a8ea1b99b7d1b6 Mon Sep 17 00:00:00 2001 From: Chuck Walbourn Date: Tue, 5 May 2026 17:20:04 -0700 Subject: [PATCH 1/5] More CMake code review --- CMakeLists.txt | 23 +++++++++++------------ CMakePresets.json | 26 +++++++++++++------------- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 962db6d5..590b6851 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -164,16 +164,16 @@ set(SHADER_SOURCES Src/Shaders/ToneMap.fx) # These source files are identical in both DX11 and DX12 version. -set(LIBRARY_HEADERS ${LIBRARY_HEADERS} +list(APPEND LIBRARY_HEADERS Inc/SimpleMath.h Inc/SimpleMath.inl) -set(LIBRARY_SOURCES ${LIBRARY_SOURCES} +list(APPEND LIBRARY_SOURCES Src/BinaryReader.cpp Src/Geometry.cpp Src/SimpleMath.cpp) -set(LIBRARY_SOURCES ${LIBRARY_SOURCES} +list(APPEND LIBRARY_SOURCES Src/AlignedNew.h Src/Bezier.h Src/BinaryReader.h @@ -187,7 +187,7 @@ set(LIBRARY_SOURCES ${LIBRARY_SOURCES} Src/vbo.h Src/TeapotData.inc) -set(SHADER_SOURCES ${SHADER_SOURCES} +list(APPEND SHADER_SOURCES Src/Shaders/Common.fxh Src/Shaders/Lighting.fxh Src/Shaders/PBRCommon.fxh @@ -198,30 +198,30 @@ set(SHADER_SOURCES ${SHADER_SOURCES} # Xbox-specific extensions if(DEFINED XBOX_CONSOLE_TARGET) - set(LIBRARY_HEADERS ${LIBRARY_HEADERS} + list(APPEND LIBRARY_HEADERS Inc/XboxDDSTextureLoader.h) - set(LIBRARY_SOURCES ${LIBRARY_SOURCES} + list(APPEND LIBRARY_SOURCES Src/XboxDDSTextureLoader.cpp) endif() if(BUILD_XINPUT OR BUILD_WGI OR BUILD_GAMEINPUT) - set(LIBRARY_HEADERS ${LIBRARY_HEADERS} + list(APPEND LIBRARY_HEADERS Inc/GamePad.h Inc/Keyboard.h Inc/Mouse.h) - set(LIBRARY_SOURCES ${LIBRARY_SOURCES} + list(APPEND LIBRARY_SOURCES Src/GamePad.cpp Src/Keyboard.cpp Src/Mouse.cpp) endif() if(BUILD_XAUDIO_WIN10 OR BUILD_XAUDIO_WIN8 OR BUILD_XAUDIO_REDIST) - set(LIBRARY_HEADERS ${LIBRARY_HEADERS} + list(APPEND LIBRARY_HEADERS Inc/Audio.h) - set(LIBRARY_SOURCES ${LIBRARY_SOURCES} + list(APPEND LIBRARY_SOURCES Audio/AudioEngine.cpp Audio/DynamicSoundEffectInstance.cpp Audio/SoundCommon.cpp @@ -246,8 +246,7 @@ else() file(TO_CMAKE_PATH ${COMPILED_SHADERS} COMPILED_SHADERS) endif() -set(LIBRARY_SOURCES ${LIBRARY_SOURCES} - ${COMPILED_SHADERS}/SpriteEffect_SpriteVertexShader.inc) +list(APPEND LIBRARY_SOURCES ${COMPILED_SHADERS}/SpriteEffect_SpriteVertexShader.inc) if(BUILD_XBOXONE_SHADERS) message(STATUS "Using Shader Model 5.0 for Xbox One for shaders") diff --git a/CMakePresets.json b/CMakePresets.json index a0f0ea01..4cf2814e 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -253,10 +253,10 @@ "hidden": true }, - { "name": "x64-Debug" , "description": "MSVC for x64 (Debug) for Windows 8", "inherits": [ "base", "x64", "Debug", "MSVC" ] }, - { "name": "x64-Release" , "description": "MSVC for x64 (Release) for Windows 8", "inherits": [ "base", "x64", "Release", "MSVC" ] }, - { "name": "x86-Debug" , "description": "MSVC for x86 (Debug) for Windows 8", "inherits": [ "base", "x86", "Debug", "MSVC" ] }, - { "name": "x86-Release" , "description": "MSVC for x86 (Release) for Windows 8", "inherits": [ "base", "x86", "Release", "MSVC" ] }, + { "name": "x64-Debug" , "description": "MSVC for x64 (Debug) for Windows 8.1", "inherits": [ "base", "x64", "Debug", "MSVC" ] }, + { "name": "x64-Release" , "description": "MSVC for x64 (Release) for Windows 8.1", "inherits": [ "base", "x64", "Release", "MSVC" ] }, + { "name": "x86-Debug" , "description": "MSVC for x86 (Debug) for Windows 8.1", "inherits": [ "base", "x86", "Debug", "MSVC" ] }, + { "name": "x86-Release" , "description": "MSVC for x86 (Release) for Windows 8.1", "inherits": [ "base", "x86", "Release", "MSVC" ] }, { "name": "x64-Debug-Win10" , "description": "MSVC for x64 (Debug) for Windows 10", "inherits": [ "base", "x64", "Debug", "MSVC", "Win10" ] }, { "name": "x64-Release-Win10" , "description": "MSVC for x64 (Release) for Windows 10", "inherits": [ "base", "x64", "Release", "MSVC", "Win10" ] }, @@ -303,10 +303,10 @@ { "name": "arm64ec-Debug-VCPKG" , "description": "MSVC for ARM64EC (Debug) using VCPKG", "inherits": [ "base", "ARM64EC", "Debug", "MSVC", "VCPKG", "Win10" ], "cacheVariables": { "VCPKG_TARGET_TRIPLET": "arm64ec-windows" } }, { "name": "arm64ec-Release-VCPKG", "description": "MSVC for ARM64EC (Release) using VCPKG", "inherits": [ "base", "ARM64EC", "Release", "MSVC", "VCPKG", "Win10" ], "cacheVariables": { "VCPKG_TARGET_TRIPLET": "arm64ec-windows" } }, - { "name": "x64-Debug-Clang" , "description": "Clang/LLVM for x64 (Debug) for Windows 8", "inherits": [ "base", "x64", "Debug", "Clang" ] }, - { "name": "x64-Release-Clang" , "description": "Clang/LLVM for x64 (Release) for Windows 8", "inherits": [ "base", "x64", "Release", "Clang" ] }, - { "name": "x86-Debug-Clang" , "description": "Clang/LLVM for x86 (Debug) for Windows 8", "inherits": [ "base", "x86", "Debug", "Clang", "Clang-X86" ] }, - { "name": "x86-Release-Clang" , "description": "Clang/LLVM for x86 (Release) for Windows 8", "inherits": [ "base", "x86", "Release", "Clang", "Clang-X86" ] }, + { "name": "x64-Debug-Clang" , "description": "Clang/LLVM for x64 (Debug) for Windows 8.1", "inherits": [ "base", "x64", "Debug", "Clang" ] }, + { "name": "x64-Release-Clang" , "description": "Clang/LLVM for x64 (Release) for Windows 8.1", "inherits": [ "base", "x64", "Release", "Clang" ] }, + { "name": "x86-Debug-Clang" , "description": "Clang/LLVM for x86 (Debug) for Windows 8.1", "inherits": [ "base", "x86", "Debug", "Clang", "Clang-X86" ] }, + { "name": "x86-Release-Clang" , "description": "Clang/LLVM for x86 (Release) for Windows 8.1", "inherits": [ "base", "x86", "Release", "Clang", "Clang-X86" ] }, { "name": "x64-Debug-Win10-Clang" , "description": "Clang/LLVM for x64 (Debug) for Windows 10", "inherits": [ "base", "x64", "Debug", "Clang", "Win10" ] }, { "name": "x64-Release-Win10-Clang" , "description": "Clang/LLVM for x64 (Release) for Windows 10", "inherits": [ "base", "x64", "Release", "Clang", "Win10" ] }, @@ -344,13 +344,13 @@ { "name": "x64-Debug-MinGW-GI" , "description": "MinG-W64 (Debug) using GameInput", "inherits": [ "base", "x64", "Debug", "GNUC", "VCPKG", "XAudio2Redist", "MinGW64" ], "cacheVariables": { "BUILD_GAMEINPUT": "true" } }, { "name": "x64-Release-MinGW-GI", "description": "MinG-W64 (Release) using GameInput", "inherits": [ "base", "x64", "Release", "GNUC", "VCPKG", "XAudio2Redist", "MinGW64" ], "cacheVariables": { "BUILD_GAMEINPUT": "true" } }, - { "name": "x64-Debug-ICC" , "description": "Intel Classic Compiler (Debug) for Windows 8", "inherits": [ "base", "x64", "Debug", "Intel" ] }, - { "name": "x64-Release-ICC" , "description": "Intel Classic Compiler (Release) for Windows 8", "inherits": [ "base", "x64", "Release", "Intel" ] }, + { "name": "x64-Debug-ICC" , "description": "Intel Classic Compiler (Debug) for Windows 8.1", "inherits": [ "base", "x64", "Debug", "Intel" ] }, + { "name": "x64-Release-ICC" , "description": "Intel Classic Compiler (Release) for Windows 8.1", "inherits": [ "base", "x64", "Release", "Intel" ] }, - { "name": "x64-Debug-ICX" , "description": "Intel oneAPI Compiler (Debug) for Windows 8", "inherits": [ "base", "x64", "Debug", "IntelLLVM" ] }, - { "name": "x64-Release-ICX" , "description": "Intel oneAPI Compiler (Release) for Windows 8", "inherits": [ "base", "x64", "Release", "IntelLLVM" ] }, + { "name": "x64-Debug-ICX" , "description": "Intel oneAPI Compiler (Debug) for Windows 8.1", "inherits": [ "base", "x64", "Debug", "IntelLLVM" ] }, + { "name": "x64-Release-ICX" , "description": "Intel oneAPI Compiler (Release) for Windows 8.1", "inherits": [ "base", "x64", "Release", "IntelLLVM" ] }, - { "name": "x64-Analyze" , "description": "MSVC for x64 (Debug) for Windows 8 using /analyze", "inherits": [ "base", "x64", "Debug", "MSVC", "Analyze" ] }, + { "name": "x64-Analyze" , "description": "MSVC for x64 (Debug) for Windows 8.1 using /analyze", "inherits": [ "base", "x64", "Debug", "MSVC", "Analyze" ] }, { "name": "x64-Analyze-Win10" , "description": "MSVC for x64 (Debug) for Windows 10 using /analyze", "inherits": [ "base", "x64", "Debug", "MSVC", "Win10", "Analyze" ] }, { "name": "x64-Coverage" , "description": "MSVC for x64 (Debug) w/ Code Coverage", "inherits": [ "base", "x64", "Debug", "MSVC", "Coverage" ] }, { "name": "x64-Coverage-Clang", "description": "Clang/LLVM for x64 (Debug) w/ Code Coverage", "inherits": [ "base", "x64", "Debug", "Clang", "Coverage" ] }, From fcc835846100f4e6dba0d324711f33d7d0a01f85 Mon Sep 17 00:00:00 2001 From: Chuck Walbourn Date: Tue, 5 May 2026 17:43:43 -0700 Subject: [PATCH 2/5] CoPilot review recommendations --- CMakeLists.txt | 20 ++++++++++++-------- build/JoinPaths.cmake | 23 ----------------------- 2 files changed, 12 insertions(+), 31 deletions(-) delete mode 100644 build/JoinPaths.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 590b6851..a0b97d8b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -263,7 +263,9 @@ if(NOT USE_PREBUILT_SHADERS) HINTS "C:/Program Files (x86)/Windows Kits/10/bin/${CMAKE_SYSTEM_VERSION}/${DIRECTX_HOST_ARCH}" "C:/Program Files (x86)/Windows Kits/10/bin/${CMAKE_SYSTEM_VERSION}.0/${DIRECTX_HOST_ARCH}" "C:/Program Files (x86)/Windows Kits/10/bin/${CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION}/${DIRECTX_HOST_ARCH}") - message(STATUS "Using LegacyShaderCompiler found in ${DIRECTX_FXC_TOOL}") + if(DIRECTX_FXC_TOOL) + message(STATUS "Using LegacyShaderCompiler found in ${DIRECTX_FXC_TOOL}") + endif() endif() add_custom_command( OUTPUT "${COMPILED_SHADERS}/SpriteEffect_SpriteVertexShader.inc" @@ -311,8 +313,9 @@ source_group(Inc REGULAR_EXPRESSION Inc/*.*) source_group(Src REGULAR_EXPRESSION Src/*.*) target_include_directories(${PROJECT_NAME} PUBLIC - $ - $) + $) +target_include_directories(${PROJECT_NAME} SYSTEM INTERFACE + $) target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_11) @@ -322,14 +325,13 @@ endif() if(MINGW) find_package(directxmath CONFIG REQUIRED) - target_link_libraries(${PROJECT_NAME} PUBLIC Microsoft::DirectXMath) else() find_package(directxmath CONFIG QUIET) endif() if(directxmath_FOUND) message(STATUS "Using DirectXMath package") - target_link_libraries(${PROJECT_NAME} PRIVATE Microsoft::DirectXMath) + target_link_libraries(${PROJECT_NAME} PUBLIC Microsoft::DirectXMath) endif() if(BUILD_XAUDIO_REDIST AND (NOT BUILD_XAUDIO_WIN10) AND (NOT BUILD_XAUDIO_WIN8)) @@ -396,10 +398,9 @@ install(FILES DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/${PACKAGE_NAME}) # Create pkg-config file -include(build/JoinPaths.cmake) # from: https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files -join_paths(DIRECTXTK_INCLUDEDIR_FOR_PKG_CONFIG "\${prefix}" "${CMAKE_INSTALL_INCLUDEDIR}") -join_paths(DIRECTXTK_LIBDIR_FOR_PKG_CONFIG "\${prefix}" "${CMAKE_INSTALL_LIBDIR}") +cmake_path(APPEND DIRECTXTK_INCLUDEDIR_FOR_PKG_CONFIG "\${prefix}" "${CMAKE_INSTALL_INCLUDEDIR}") +cmake_path(APPEND DIRECTXTK_LIBDIR_FOR_PKG_CONFIG "\${prefix}" "${CMAKE_INSTALL_LIBDIR}") set(DIRECTXTK_DEP_L "") if(directxmath_FOUND) @@ -419,6 +420,8 @@ install(FILES "${CMAKE_CURRENT_BINARY_DIR}/DirectXTK.pc" DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig) #--- Command-line tools +set(TOOL_EXES "") + if(BUILD_TOOLS AND WIN32) set(TOOL_EXES xwbtool) @@ -532,6 +535,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") endforeach() if(BUILD_FUZZING AND (NOT WINDOWS_STORE)) + # Prevent fuzzing builds (which is otherwise configured as a Release mode) from disabling assertions string(REPLACE "/DNDEBUG" "" CMAKE_CXX_FLAGS_RELEASE ${CMAKE_CXX_FLAGS_RELEASE}) string(REPLACE "/DNDEBUG" "" CMAKE_CXX_FLAGS_RELWITHDEBINFO ${CMAKE_CXX_FLAGS_RELWITHDEBINFO}) diff --git a/build/JoinPaths.cmake b/build/JoinPaths.cmake deleted file mode 100644 index c68d91b8..00000000 --- a/build/JoinPaths.cmake +++ /dev/null @@ -1,23 +0,0 @@ -# This module provides function for joining paths -# known from most languages -# -# SPDX-License-Identifier: (MIT OR CC0-1.0) -# Copyright 2020 Jan Tojnar -# https://github.com/jtojnar/cmake-snips -# -# Modelled after Python’s os.path.join -# https://docs.python.org/3.7/library/os.path.html#os.path.join -# Windows not supported -function(join_paths joined_path first_path_segment) - set(temp_path "${first_path_segment}") - foreach(current_segment IN LISTS ARGN) - if(NOT ("${current_segment}" STREQUAL "")) - if(IS_ABSOLUTE "${current_segment}") - set(temp_path "${current_segment}") - else() - set(temp_path "${temp_path}/${current_segment}") - endif() - endif() - endforeach() - set(${joined_path} "${temp_path}" PARENT_SCOPE) -endfunction() From 519726ae5e72d3ee959bc74fd5e0abd0857bcd41 Mon Sep 17 00:00:00 2001 From: Chuck Walbourn Date: Tue, 5 May 2026 17:56:37 -0700 Subject: [PATCH 3/5] Want directxmath usage public for mingw --- CMakeLists.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a0b97d8b..88b891af 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -325,13 +325,16 @@ endif() if(MINGW) find_package(directxmath CONFIG REQUIRED) + target_link_libraries(${PROJECT_NAME} PUBLIC Microsoft::DirectXMath) else() find_package(directxmath CONFIG QUIET) endif() if(directxmath_FOUND) message(STATUS "Using DirectXMath package") - target_link_libraries(${PROJECT_NAME} PUBLIC Microsoft::DirectXMath) + if(NOT MINGW) + target_link_libraries(${PROJECT_NAME} PRIVATE Microsoft::DirectXMath) + endif() endif() if(BUILD_XAUDIO_REDIST AND (NOT BUILD_XAUDIO_WIN10) AND (NOT BUILD_XAUDIO_WIN8)) From cde4f857a9569319265d1678c5060634764dee2f Mon Sep 17 00:00:00 2001 From: Chuck Walbourn Date: Tue, 5 May 2026 18:36:10 -0700 Subject: [PATCH 4/5] Remove classic compiler override for C++14 --- CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 88b891af..526ad328 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -495,8 +495,6 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "GNU") target_compile_options(${t} PRIVATE "-Wno-attributes") endif() endforeach() -elseif(CMAKE_CXX_COMPILER_ID MATCHES "Intel") - set_target_properties(${PROJECT_NAME} PROPERTIES CXX_STANDARD 14) elseif(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") if(BUILD_SHARED_LIBS) target_compile_options(${PROJECT_NAME} PRIVATE /wd4251 /wd4275) From c6d1ab613fa47d7006093871ab9565e35d0d3d30 Mon Sep 17 00:00:00 2001 From: Chuck Walbourn Date: Tue, 5 May 2026 18:51:16 -0700 Subject: [PATCH 5/5] One more edit --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 526ad328..0a85cb78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -577,7 +577,7 @@ if(WIN32) endif() endif() -if(BUILD_TOOLS AND WIN32) +if(BUILD_TOOLS AND WIN32 AND (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)) set_property(DIRECTORY PROPERTY VS_STARTUP_PROJECT xwbtool) endif()