Chlumsky / msdfgen

Multi-channel signed distance field generator
MIT License
3.9k stars 404 forks source link

Call project() after cmake_minimum_required() #191

Closed SpaceIm closed 9 months ago

SpaceIm commented 10 months ago

Could you call project() after cmake_minimum_required() please (well after include(cmake/version.cmake) actually)? It's problematic when someone has to use a toolchain, since CMakeLists of msdfgen has lot of logic between cmake_minimum_required() and project(), and stuff in toolchain files are ignored before project() is called (it's tedious for conan for example).

Moreover it's not really a good idea to hardcode vcpkg stuff in a CMakeLists. vcpkg should come as a toolchain file injected externally. CMakeLists should be package manager agnostic.

Chlumsky commented 10 months ago

As you can see, the logic before project needs to configure stuff for the vcpkg toolchain so this is intentional. Using vcpkg is not hardcoded, you can disable it with MSDFGEN_USE_VCPKG.

SpaceIm commented 10 months ago

Even if there is an option, this logic leads to an illformed CMakeLists, where options and some logic unrelated to vcpkg are defined before projects just because of this vcpkg intrusion (actually I realize that you could not even move other options after project() because you have vcpkg logic based on these options which has be done before project()).

SpaceIm commented 10 months ago

Could you at least move MSDFGEN_DYNAMIC_RUNTIME and BUILD_SHARED_LIBS logic after project() please?

I mean this part:

if(MSDFGEN_DYNAMIC_RUNTIME)
    set(MSDFGEN_MSVC_RUNTIME "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
else()
    set(MSDFGEN_MSVC_RUNTIME "MultiThreaded$<$<CONFIG:Debug>:Debug>")
endif()

if(BUILD_SHARED_LIBS)
    set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()
Chlumsky commented 10 months ago

But why?

SpaceIm commented 10 months ago

Because it defeats conan toolchain.

Chlumsky commented 9 months ago

Sorry, I kind of forgot about this but changed it now.