search

lighttransport / nanort

NanoRT, single header only modern ray tracing kernel.
MIT License
1.07k stars 89 forks source link

Wrapping max() in parenthesis for windows error #60

Closed cadop closed 4 years ago

cadop commented 4 years ago

When compiling on windows visual studio I was getting an error, from what it seems like, the windows.h min/max macros.

Changing from : max_t(std::numeric_limits<T>::max()) to max_t( (std::numeric_limits<T>::max)() ), based on https://stackoverflow.com/questions/2561368/illegal-token-on-right-side-of . Stopped the error, and it seems to work, does the conflict with the other compilers, or any reason not to change this in source?

syoyo commented 4 years ago

You should use NOMINMAX

https://stackoverflow.com/questions/4913922/possible-problems-with-nominmax-on-visual-c

cadop commented 4 years ago

Okay,

I will just comment in case others have this issue:

The NOMINMAX didn't work for me. The reason I linked to that SO question is the answers suggestion of applying parenthesis to prevent macro expansions. One of the comments points out the problem:

If you add the NOMINMAX to your header but it's included by some other file which has already included windows.h, the NOMINMAX won't work and max and min are already defined. That explains why the #undef min/max will sometimes work even when NOMINMAX doesn't. – ikku100 May 6 '16 at 15:36

One thing that confused me at first was its not just windows.h (because I never included that), but also <algorithm> seems to cause the same issue.

So in the end, what worked for me was using #undef min and #undef max before including nanort.h. However I just find it cleaner/simpler to have the header undefine the macros so just placing it at the top of the includes worked for me.