Upgrade CMake usability on Windows

[Ryu204]

Problems

In my project I used TQMesh for triangulation, however compilation on Windows failed. See CI workflows log. These are the problems:

• The use of corecrt_math_define.h headers prevent usage of MSVCRT and some other compilers, as shown in the link above
• Some IF (WIN32) in CMakeLists.txt-s make it hard/impossible to use TQMesh as CMake dependency on Windows machine
• (Bonus) CI only supports Linux and MacOS

What does this PR do

• Use only cmath instead of corecrt_math_define.h
• Change link interfaces of sub directory (from none/INTERFACE to PRIVATE)
• Change compile definitions for big obj files to support gcc as well
• Modify CI workflow to support testing on Windows
• Remove z member of VecND to compile on msvc
• Move test commands into test directory

Limitation

Currently, one test failed on Windows with gcc. I cannot debug the problem since my knowledge about the project is very limited. It would be nice if the authors can help me with that.

[FloSewn]

Hey Ryu,

Sorry for my late response. I will have a look at your changes in the next days.

Thank you so much for your work!

Regards, Flo

[Ryu204]

Hello, thank you so much for your library. It's nice.

Actually - I would like to stick to the z attribute of this class for a better readability of operations like this.

I made the change because somehow I wasn't able to build with msvc when the array script goes out of range. Maybe it's my fault. Would you like me to revert the change?

Edit: I figured out why I changed that member. When you run the test (I used "Vertex" test), the z member is initialized for even VecND with N = 2 and caused an exception on Windows. That happens even if we never touch that member after initialization.

Or is there maybe some kine of std::enable_if pattern that can be applied to this class attributes?

I don't know if there is a fast way to enable z member with N >= 3. The best I can think of is a template specialization, but that means double the code for VecND.

[Ryu204]

Is the "enable_testing()" call in src/tests/CMakeLists.txt relevant to get the code compiled on windows?

In my local environment, with current main branch status, when running cmake --build build, CTest will report this:

# In root directory
$ cmake --build build

MSBuild version 17.7.2+d6990bcfa for .NET Framework

  TQMesh.vcxproj -> C:\Users\nguye\Desktop\TQMesh\bin\TQMesh.exe
  1>
  run_examples.vcxproj -> C:\Users\nguye\Desktop\TQMesh\bin\run_examples.exe
  Test project C:/Users/nguye/Desktop/TQMesh/build/src/tests
  No tests were found!!!

I guess it has something to do with the test declarations in root CMake file. After moving them to src/tests everything works as expected.

In the past commits I moved all test related commands to src/tests (however, the tests are still executed after all, please take a look at CI worklogs).

Anyways, I added enable_testing() to the source CMakeLists.txt again thanks to your reminder.

Update: Unfortunately, the current commit does not work. I don't know what is going wrong. The output is here. Update 2: It seems there are many related issues about CTest: here, here. How about not bothering CTest and test the executable manually? You already made a script for that, after all.

[FloSewn]

Thank you so much for your effort!

Edit: I figured out why I changed that member. When you run the test (I used "Vertex" test), the z member is initialized for even VecND with N = 2 and caused an exception on Windows. That happens even if we never touch that member after initialization.

That's interesting! Sounds like the approach I chose was more a hack than a proper way of doing it. I guess I will simply change it to a function call instead (foo.x() instead of foo.x) - then we can also prohibit it with std::enable_if for N < 3.

I also see your point relating the issues of CTest. We should simply use the testing script. Is the current commit not working due to the new location of enable_testing()? From the output it seems that none of the tests did actually run, since the executable run_tests.exe hasn't been generated. It even looks to me as if there was no build at all... Maybe you could restart the CI?

Anyway - thanks again for contributing! :-)

Regards, Flo

[Ryu204]

I guess I will simply change it to a function call instead.

It's fast to implement, I think. It's really up to your API preferences.

Is the current commit not working due to the new location of enable_testing()?

AFAIK, after moving enable_testing() to the root directory, CTest indeed recognized the test and proceeded to call the run_tests.exe before actually building them. That's why the CI and my local environment failed.

However, as I moved the command to src/tests, it essentially stopped CTest from knowing about the test, hence no tests were generated at all, and the ${TESTS} target built normally. It is then tested via the script in CI. I moved the enable_testing() to the subdir because I were not aware of this effect, it was just a coincidence. I think the script approach way will save future maintainers some headaches.

I will make the changes tomorrow :)!

[FloSewn]

Hey there,

since it was only the removal of enable_testing() in the root CMakeLists.txt file, I just merged your contributions to a new branch and did that small change by myself. I will now try to fine the reason for that segmentation fault, for the Windows/gcc build combination and then add it to the main branch.

Anyway - thanks again for your help! Hopefully this will enable it for some more people to use this code :-)

Greetings Flo