feat(simplemath): port SimpleMath to DirectXMath - #343
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
057c837 to
e509120
Compare
e509120 to
f32b8b4
Compare
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| set(LIBRARY_HEADERS SimpleMath.h SimpleMath.inl) | ||
| set(LIBRARY_SOURCES SimpleMath.cpp) | ||
|
|
||
| add_library(${PROJECT_NAME} STATIC) |
There was a problem hiding this comment.
It probably makes most sense here to mirror DirectX Tool Kit so that if you are using VCPKG to consume these things, they are consistent.
option(BUILD_SHARED_LIBS "Build SimpleMath as a shared library" OFF)
add_library(${PROJECT_NAME})
target_sources(${PROJECT_NAME} PRIVATE ${LIBRARY_HEADERS} ${LIBRARY_SOURCES})
if(WIN32 AND BUILD_SHARED_LIBS)
message(STATUS "Build SimpleMath library as a DLL")
configure_file(
"${CMAKE_CURRENT_SOURCE_DIR}/build/SimpleMath.rc.in"
"${CMAKE_CURRENT_BINARY_DIR}/SimpleMath.rc" @ONLY)
target_sources(${PROJECT_NAME} PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/SimpleMath.rc")
target_compile_definitions(${PROJECT_NAME} PRIVATE DIRECTX_TOOLKIT_EXPORT)
target_compile_definitions(${PROJECT_NAME} INTERFACE DIRECTX_TOOLKIT_IMPORT)
endif()
| DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/${PACKAGE_NAME}) | ||
|
|
||
| #--- Compiler switches | ||
| if(MSVC) |
There was a problem hiding this comment.
It made sense to me to keep this 'inline' when only SHMath needed these switches. If we are moving to a case where two CMakeLists.txt use it, then we should create a build folder with CompilerAndLinker.cmake and have both CMakes use it via an include.
This might also require refactoring the CMakes to only let 'top-level' build as with my other projects and then update any test YAML pipline that uses the SMath\CMakeLists.txt directly.
There was a problem hiding this comment.
You will need a build folder anyhow for SimpleMath.rc.in above.
|
Hi Chuck Walbourn (@walbourn) sir, thanks for the review. I've addressed the requested changes:
Could you please take another look when you get time for this , so that i can work on further changes. |
Problem
Users want to consume SimpleMath — the simplified C++ math wrapper for DirectXMath — without taking a dependency on the entire DirectX Tool Kit.
See #327.
Root Cause
SimpleMath lives in DirectXTK because it was originally designed as part of an XNA-parity layer. However, the three files that make up SimpleMath (
SimpleMath.h,SimpleMath.inl,SimpleMath.cpp) only depend onDirectXMath.h,DirectXPackedVector.h, andDirectXCollision.h— all headers already present in this repository.Solution
This PR directly follows the approach outlined by Chuck Walbourn (@walbourn) in #327:
The implementation mirrors the existing
SHMathpattern exactly:SimpleMath/subdirectory containingSimpleMath.h,SimpleMath.inl, andSimpleMath.cpp.SimpleMath/CMakeLists.txtmodelled onSHMath/CMakeLists.txt.SIMPLEMATH_VERSIONto1.0.0as this is its first introduction into the DirectXMath tree.BUILD_SIMPLEMATHoption to the rootCMakeLists.txt, disabled by default, consistent with the existingBUILD_SHMATH/BUILD_XDSPpattern.#include "pch.h"fromSimpleMath.cppto make it standalone (the "edit the .cpp" approach referenced in the issue).SimpleMath.inlis listed inLIBRARY_HEADERSso it is installed alongside the header onmake install.sal.hdownload block and SSE2 architecture detection that were present in the SHMath template, asSimpleMath.cpponly defines static constants and contains no SIMD intrinsics or SAL annotations.DIRECTX_TOOLKIT_APIexport macro is preserved as-is; it defaults to an empty definition in a static library build when neitherDIRECTX_TOOLKIT_EXPORTnorDIRECTX_TOOLKIT_IMPORTis defined.Testing
Compiled
SimpleMath.cppagainst the DirectXMath headers in this repository using GCC 6.3.0 (MinGW) with_XM_NO_INTRINSICS_and the existingTests/sal.hstub (same approach used by the repo's own local tests). Compilation exits cleanly with zero errors. All warnings are deprecation notices inDirectXPackedVector.hpre-existing in the DirectXMath headers themselves — none originate from SimpleMath.Full MSVC + Windows SDK build would exercise the
dxgi1_2.hpath (guarded by#ifdef _WIN32) which is outside the scope of this local toolchain.Risk
Low — this is a purely opt-in addition behind
BUILD_SIMPLEMATH, which defaults toOFF. No existing code paths are altered. TheDIRECTX_TOOLKIT_APImacro is not renamed, preserving ABI compatibility for any user migrating between the standalone and DirectXTK builds.