coin3d / simage

Image file loading
ISC License
7 stars 9 forks source link

Compile error on Windows 10 with multiple Windows Kits #55

Closed ChrisSchulte closed 2 years ago

ChrisSchulte commented 2 years ago

Hi,

compiling with Visual Studio 2019 gives an error as the FindVfw.cmake and the FindGdiPlus.cmake use HINTS and PATH_SUFFIXES in their FIND_PACKAGE method. As the PATH_SUFFIXES don't match the SDK 10 path (as it is ...\Include\SDKversion\um with multiple choices for SDKVersion) it will point to the GDI and Vfw from SDK 8.1, leading to compile errors in Windows includes (as their versions mismatch). Using a x64 Native Tools Command Prompt and removing the HINTS and PATH_SUFFIXES configure and build work flawlessly. Using a standard Windows Command Prompt, neither Gdi nor Vfw is found.

VolkerEnderlein commented 2 years ago

Unfortunately this would require to run cmake always from a x64 native tools command prompt. IMHO this is not acceptable. I would solve it the way VTK does it, by adding a compile check. As VFW and GdiPlus are "system" dependencies and CMake correctly sets up the Windows SDK paths they will be found. We are tying the names of the import libraries (vfw32 and gdiplus) to the CMakeLists.txt files but this seems acceptable to me.

ChrisSchulte commented 2 years ago

Hello Volker, after your comment I did again a test, this time using directly the cmake-gui. As you can see in log file CMake-gui_output.txt, the selected Windows SDK is 10.0.19041.0 but, as can be seen in CMakeCache.txt, VFW_INCLUDE_DIR and GDIPLUS_INCLUDE_DIR both point to the Windows Kits 8.1, leading to errors (as can be seen in CMakeError.log) that appear to be ignored as I can configure and generate the whole project but not build. CMakeOutput.log CMakeError.log Cmake-gui_output.txt CMakeCache.txt

ChrisSchulte commented 2 years ago

Hello Volker I may try to suggest a solution. Indeed, starting from CMake v3.4 the is a CMake variable called CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION. This varaible provides the exact SDK version used. Modifying FindGdiPlus.cmake and FindVfw.cmake find_path methods as follows :

  find_path(GDIPLUS_INCLUDE_DIR
    NAMES
      GdiPlus.h gdiplus.h
    HINTS
      "${MINGW_HINTS}"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows Kits\\Installed Roots;KitsRoot10]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows Kits\\Installed Roots;KitsRoot81]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows Kits\\Installed Roots;KitsRoot]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v7.0;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v7.0A;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v7.1;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v7.1A;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v8.0;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v8.0A;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v8.1;InstallationFolder]"
      "[HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Microsoft SDKs\\Windows\\v8.1A;InstallationFolder]"
    PATH_SUFFIXES
      include/${CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION}
      Include/${CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION}
      Include/${CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION}/um
      Include/${CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION}/shared
)

Doing this we obtain correct behaviour on Windows 10 and 11 with SDK 10 versions as well as correct behaviour if only SDK 8.1 is installed as CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION is empty if no Windows 10 SDK is available.

VolkerEnderlein commented 2 years ago

Hi Chris, I was not explaining my proposed changes very good. I meant to abandon FindVfw and FindGDIplus modules and instead use a compile check similar to vtkSpline check with test file. That way we depend only on the WindowsSDK paths selected and set by CMake. I tested it yesterday and it worked out of the box. But I'll also test your proposed solution as it looks very promising. Cheers, Volker

ChrisSchulte commented 2 years ago

Hi Volker, sorry I didn't understand your solution, as I went on the wrong VTK (I went on the VTK on github not the VTK Spline on Gitlab). It is also a quite interesting solution and maybe more portable between the different SDK evolutions as mine. If I find some time I will give it a try and keep you in touch. Cheers, Chris

ChrisSchulte commented 2 years ago

Hi Volker,

I've implemented the changes for Vfw and GDI plus on my PC. Do you prefer that I fork the project and open a pull request from a fork or should I branch the project. Or maybe you want to proceed differently ?

Cheers Chris

VolkerEnderlein commented 2 years ago

Hi Chris, forking and providing a pull request is the right way to go. I will check the PR tonight. Many thanks for your contribution. Cheers, Volker

VolkerEnderlein commented 2 years ago

Thanks for the valuable contribution.