Disable min/max macros from windows.h

[TheGondos]

This PR is about removing the min/max macros that are defined by default in windows.h.
Using the /DNOMINMAX flag, they are not defined and we can use the functions from the C++ library.
Impacts :

• both operands of std::min/std::max must be of the same type so there is a bunch of casting/constants rework
• since there are already some "using namespace std" present in the base code, I didn't want to have half the code using std::min/std::max and the other just min/max. As a result I added a bunch of "using std::min/max" to keep consistency.
• std::min/std::max form is still used in the header files to prevent polluting the global namespace

[TheGondos]

For what it's worth, I tried replacing some min(max(x,a),b) with std::clamp but both gcc and MSVC generate garbage assembly so I did not pursue the matter. Come on C++, where is the zero cost abstraction you always brag about :(

[TheGondos]

Work around for another issue with MSVC that prevent using initializer lists when min/maxing 3 values.

[dimitry-ishenko]

For what it's worth, I tried replacing some min(max(x,a),b) with std::clamp but both gcc and MSVC generate garbage assembly so I did not pursue the matter. Come on C++, where is the zero cost abstraction you always brag about :(

@TheGondos why do you say garbage? It looks totally fine and is half the size of the home-grown solution

[TheGondos]

For what it's worth, I tried replacing some min(max(x,a),b) with std::clamp but both gcc and MSVC generate garbage assembly so I did not pursue the matter. Come on C++, where is the zero cost abstraction you always brag about :(

@TheGondos why do you say garbage? It looks totally fine and is half the size of the home-grown solution

Home grown solution : 3ops, one memory access

float saturate_legacy(float) PROC                     ; saturate_legacy, COMDAT
        xorps   xmm1, xmm1
        maxss   xmm1, xmm0
        movss   xmm0, DWORD PTR __real@3f800000
        minss   xmm0, xmm1
        ret     0
float saturate_legacy(float) ENDP                     ; saturate_legacy

std::clamp : 7ops, 5 memory accesses

float saturate_clamp(float) PROC                            ; saturate_clamp, COMDAT
        lea     rax, QWORD PTR v$[rsp]
        movss   DWORD PTR [rsp+8], xmm0
        xorps   xmm1, xmm1
        mov     DWORD PTR $T2[rsp], 1065353216            ; 3f800000H
        comiss  xmm1, xmm0
        lea     rcx, QWORD PTR $T1[rsp]
        mov     DWORD PTR $T1[rsp], 0
        cmovbe  rcx, rax
        lea     rax, QWORD PTR $T2[rsp]
        comiss  xmm0, DWORD PTR __real@3f800000
        cmovbe  rax, rcx
        movss   xmm0, DWORD PTR [rax]
        ret     0
float saturate_clamp(float) ENDP

I wouldn't call that totally fine.
gcc has also an issue that has been around for 3 years and makes it generate poor assembly with a branch :

saturate_legacy(float):
        pxor    xmm1, xmm1
        comiss  xmm1, xmm0
        ja      .L9
        movss   xmm1, DWORD PTR .LC0[rip]
        minss   xmm1, xmm0
        movaps  xmm0, xmm1
        ret
.L9:
        pxor    xmm1, xmm1
        movaps  xmm0, xmm1
        ret
.LC0:

MSVC produces optimal code with the manual clamping (although if it were me I would probably do the load from memory first)

[dimitry-ishenko]

@TheGondos oh sorry, for some reason they are reversed in the assembly window. I didn't look at the labels and thought the top part was for the clamp version. Nevermind.

[dimitry-ishenko]

Clang on the other hand does well on both: https://godbolt.org/z/Pre1r81jf

[Xyon]

@TheGondos think you missed an #include <algorithm> in DrawAPI.h?

[TheGondos]

@TheGondos think you missed an #include <algorithm> in DrawAPI.h?

Let me check, it builds cleanly on my side. I'll rebase to see if it's a conflict
Edit : I see, it's in the XRSound directory, I cannot compile it since the recent modifications so I had to disable it :(

C:\github\openorbiter\out\build\windows-x64-asan\ninja : error : '../../../Extern/irrKlang/x64/lib/Winx64-visualStudio/irrKlang.lib', needed by 'Modules/Plugin/XRSound.dll', missing and no known rule to make it

Isn't it supposed to be downloaded automatically?
I removed the empty Exterm/oorKlang/x64 directory and now I get this error :

1> [CMake] Downloading irrKlang from https://www.ambiera.at/downloads/irrKlang-64bit-1.6.0.zip ...
1> [CMake] CMake Error at C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/ExternalProject.cmake:2771 (message):
1> [CMake]   At least one entry of URL is a path (invalid in a list)
1> [CMake] Call Stack (most recent call first):
1> [CMake]   C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/ExternalProject.cmake:3681 (_ep_add_download_command)
1> [CMake]   CMakeLists.txt:15 (ExternalProject_Add)
1> [CMake] -- Configuring incomplete, errors occurred!
1> [CMake] See also "C:/github/openorbiter/out/build/windows-x64-asan/_deps/irrklang-subbuild/CMakeFiles/CMakeOutput.log".

cmake, my favorite pile of steaming mess...

[TheGondos]

I manually put the files for irrKlang, it should be OK. I forced pushed a rebase from the latest main.

[dimitry-ishenko]

@TheGondos unfortunately this PR had unintended consequences for addon developers. Since you've made changes to the DrawAPI.h and VesselAPI.h in the SDK, the addon devs also have to add NOMINMAX in their projects to disable Windows macros or they get errors.

There is a pretty simple fix though -- just wrap all std::min and std::max instances in parens. Eg:

-    foo = std::max(bar, baz);
+    foo = (std::max)(bar, baz);

[dimitry-ishenko]

Nevermind, I've just sent a PR #365 myself