Skip to content

Commit

Permalink
Fix: don't link to OpenGL with SDL2 as backend; SDL2 dynamically load…
Browse files Browse the repository at this point in the history
…s it

Although for developers this doesn't change anything, for our
linux-generic binary it changes everything. Without this, the
OpenGL dynamic library is dragged in as dependency, and as it
depends on X11, that will be dragged in too. This is not
something we prefer to have, as that won't run on as many
machines as it could.

SDL2 doesn't depend on OpenGL directly, as it tries to load it
in on runtime. If found, it would work in exactly the same way
as if we would link to OpenGL ourselves. As such, this is
the best of both worlds: our linux-generics have less linked
dependencies, and developers won't notice any difference.

As a side-effect, if someone uses linux-generic on a machine
that does not have any OpenGL package installed, it will
gracefully fall back to the default backend of SDL instead.
  • Loading branch information
TrueBrain committed Feb 25, 2021
1 parent 9209807 commit 700d45b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
18 changes: 13 additions & 5 deletions CMakeLists.txt
Expand Up @@ -218,7 +218,7 @@ add_subdirectory(${CMAKE_SOURCE_DIR}/media/baseset)
add_dependencies(openttd
find_version)

target_link_libraries(openttd
target_link_libraries(openttd PUBLIC
openttd::languages
openttd::settings
openttd::basesets
Expand Down Expand Up @@ -249,13 +249,21 @@ if(NOT OPTION_DEDICATED)
link_package(Fontconfig TARGET Fontconfig::Fontconfig)
link_package(ICU_lx)
link_package(ICU_i18n)
link_package(OpenGL TARGET OpenGL::GL)

# SDL2 dynamically loads OpenGL if needed, so do not link to OpenGL when
# on Linux. For Windows, we need to link to OpenGL as we also have a win32
# driver using it.
if(SDL2_FOUND AND UNIX)
link_package(OpenGL TARGET OpenGL::GL INCLUDE_ONLY)
else()
link_package(OpenGL TARGET OpenGL::GL)
endif()
endif()

if(APPLE)
link_package(Iconv TARGET Iconv::Iconv)

target_link_libraries(openttd
target_link_libraries(openttd PUBLIC
${AUDIOTOOLBOX_LIBRARY}
${AUDIOUNIT_LIBRARY}
${COCOA_LIBRARY}
Expand Down Expand Up @@ -299,7 +307,7 @@ if(EMSCRIPTEN)

# Build the .html (which builds the .js, .wasm, and .data too).
set_target_properties(openttd PROPERTIES SUFFIX ".html")
target_link_libraries(openttd WASM::WASM)
target_link_libraries(openttd PUBLIC WASM::WASM)
endif()

if(NOT PERSONAL_DIR STREQUAL "(not set)")
Expand Down Expand Up @@ -333,7 +341,7 @@ if(WIN32)
-DWITH_UNISCRIBE
)

target_link_libraries(openttd
target_link_libraries(openttd PUBLIC
ws2_32
winmm
imm32
Expand Down
20 changes: 14 additions & 6 deletions cmake/LinkPackage.cmake
@@ -1,5 +1,5 @@
function(link_package NAME)
cmake_parse_arguments(LP "ENCOURAGED" "TARGET" "" ${ARGN})
cmake_parse_arguments(LP "ENCOURAGED;INCLUDE_ONLY" "TARGET" "" ${ARGN})

if(${NAME}_FOUND)
string(TOUPPER "${NAME}" UCNAME)
Expand All @@ -8,14 +8,22 @@ function(link_package NAME)
# which (later) cmake considers to be an error. Work around this with by stripping the incoming string.
if(LP_TARGET AND TARGET ${LP_TARGET})
string(STRIP "${LP_TARGET}" LP_TARGET)
target_link_libraries(openttd ${LP_TARGET})
if(LP_INCLUDE_ONLY)
target_link_libraries(openttd INTERFACE ${LP_TARGET})
else()
target_link_libraries(openttd PUBLIC ${LP_TARGET})
endif()
message(STATUS "${NAME} found -- -DWITH_${UCNAME} -- ${LP_TARGET}")
else()
string(STRIP "${${NAME}_LIBRARY}" ${NAME}_LIBRARY)
string(STRIP "${${NAME}_LIBRARIES}" ${NAME}_LIBRARIES)
include_directories(${${NAME}_INCLUDE_DIRS} ${${NAME}_INCLUDE_DIR})
target_link_libraries(openttd ${${NAME}_LIBRARIES} ${${NAME}_LIBRARY})
message(STATUS "${NAME} found -- -DWITH_${UCNAME} -- ${${NAME}_INCLUDE_DIRS} ${${NAME}_INCLUDE_DIR} -- ${${NAME}_LIBRARIES} ${${NAME}_LIBRARY}")
if(NOT LP_INCLUDE_ONLY)
string(STRIP "${${NAME}_LIBRARY}" ${NAME}_LIBRARY)
string(STRIP "${${NAME}_LIBRARIES}" ${NAME}_LIBRARIES)
target_link_libraries(openttd PUBLIC ${${NAME}_LIBRARIES} ${${NAME}_LIBRARY})
message(STATUS "${NAME} found -- -DWITH_${UCNAME} -- ${${NAME}_INCLUDE_DIRS} ${${NAME}_INCLUDE_DIR} -- ${${NAME}_LIBRARIES} ${${NAME}_LIBRARY}")
else()
message(STATUS "${NAME} found -- -DWITH_${UCNAME} -- ${${NAME}_INCLUDE_DIRS} ${${NAME}_INCLUDE_DIR}")
endif()
endif()
elseif(LP_ENCOURAGED)
message(WARNING "${NAME} not found; compiling OpenTTD without ${NAME} is strongly disencouraged")
Expand Down

0 comments on commit 700d45b

Please sign in to comment.