Enabled CI MSVC build jobs.

[bjornpiltz]

• Added a build matrix
• Installed Qt through jurplel/install-qt-action (allows for caching)

[dennis2society]

Would you consider to add this line to your #idfef(MSVC) block to suppress all the "inconsistent dll linkage" warnings as proposed in PR
#79?

  # remove warnings about deprecation (CRT, dll linkage,etc) 
   target_compile_options(QGLViewer PRIVATE "/wd4996")

I agree that your ifdef(MSVC) is probably a better solution than the ifdef(WIN32) used in my PR.

[bjornpiltz]

@dennis2society I fixed the 4996 warnings by defining CREATE_QGLVIEWER_DLL, which was missing. see 20f8aa5.

As of now, building libQLViewer as a DLL is hard coded in the cmake system. It's up to @GillesDebunne to decide if the possibility to build it as a static library should be reintroduced.

Let me know if there is something else essential in your PR which is still missing (I think we can wait with fixing warnings and whitespace issues) otherwise this PR replaces #71 and #79.

[dennis2society]

You're right. I've looked at the output of your CI run and it looks fine. Possibly I have double-fixed this issue in my PR. I will test again with your CMakelists.
Looks like you have everything covered. Nice work. I will close my own PR, no need to have two solutions for the same problem. And yours covers a bit more than mine already.
Since you're at it, maybe you could also integrate #76.
It is only a few lines long.

[bjornpiltz]

I think I'd prefer not to add #76 to keep this PR minimal. Anyway, I think a cleaner implementation would be:

if(QGLVIEWER_BUILD_EXAMPLES)
    add_subdirectory(examples) # details in examples/CMakeLists.txt
endif() 

Perhaps @fghoussen can resubmit once this gets merged.

[fghoussen]

Perhaps @fghoussen can resubmit once this gets merged.

Sure!

[dennis2society]

I have tested your CMakeLists and it works fine for me. Suppressing the 4996 warnings on Windows is not necessary. I will close my own PR.

[dennis2society]

CMake explicitly recommends to set the minimum version before the project name. Could you move the line
cmake_minimum_required(VERSION 3.16) above the project() line?

[fghoussen]

So #76 will be merged? If #76 should be modified, just tell me how :)

[bjornpiltz]

@dennis2society, I'm hesitant to do so since I don't understand the changes made in "Minor tweaks in cmake configuration"
I don't know why the minimum CMake version was raised as high as 3.16 and the cmake_policy() call was added. I would have thought the following two lines would be enough.

cmake_minimum_required(VERSION 3.0)
project(libQGLViewer LANGUAGES CXX VERSION 2.9.1)

@fghoussen, I'm hoping Gilles will merge this PR - then you would need to amend your branch to resolve the conflicts with my changes.

[dennis2society]

@bjornpiltz The reason I am suggesting this is this warning when running cmake:

CMake Warning (dev) at CMakeLists.txt:5 (project):
cmake_minimum_required() should be called prior to this top-level project()
call.  Please see the cmake-commands(7) manual for usage documentation of
both commands.

Not sure about the CMake version requirements, though.

[fghoussen]

@fghoussen, I'm hoping Gilles will merge this PR - then you would need to amend your branch to resolve the conflicts with my changes.

OK!