Skip to content

Commit

Permalink
build: Improve finding locally built PNG
Browse files Browse the repository at this point in the history
* Adjustments to lock down a better ordering of include directories.
* Always prefer a static library png when available, and when building
  local PNG, for sure only build the static version.
* Give the symbols an "oiio" prefix so they can't link against symbols
  in the wrong library.
* Give a bunch of extra hints about the directory locations and
  config location of the local build to make extra sure it's the
  one found and chosen.

This is still not perfect. It's plagued by one remaining nasty
problem: If there is a too-old version of libpng installed in a system
area, although we will diligently build newer local copy, it's
extremely hard to ensure that it doesn't accidentally get the headers
corresponding to the old version, because OTHER dependencies may be
correctly found in the system area and therefore put it in the include
path where it will be found rather than the one we intend. I don't
know how to fix this robustly.

For now, it remains the case that the dependency self-build works very
well if the dependency is missing, but can be hit-or-miss if there are
headers for a too-old version installed in the same area as other
dependency headers we use, since it's very hard to control which
end up earlier in the include path.

But at least for now, this setup works fine for all of our CI cases.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Nov 4, 2024
1 parent 06a838a commit 144f201
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/cmake/add_oiio_plugin.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ macro (add_oiio_plugin)
target_compile_definitions (${_plugin_NAME} PRIVATE
${_plugin_DEFINITIONS}
OpenImageIO_EXPORTS)
target_include_directories (${_plugin_NAME} PRIVATE ${_plugin_INCLUDE_DIRS})
target_include_directories (${_plugin_NAME} BEFORE PRIVATE ${_plugin_INCLUDE_DIRS})
target_link_libraries (${_plugin_NAME} PUBLIC OpenImageIO
PRIVATE ${_plugin_LINK_LIBRARIES})
set_target_properties (${_plugin_NAME} PROPERTIES PREFIX "" FOLDER "Plugins")
Expand Down
19 changes: 16 additions & 3 deletions src/cmake/build_PNG.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,24 @@ unset (PNG_FOUND)
unset (PNG_LIBRARIES)
unset (PNG_INCLUDE_DIRS)
unset (PNG_INCLUDE_DIR)
unset (PNG_VERSION_STRING)
unset (PNG_DEFINITIONS)
unset (PNG_VERSION)

build_dependency_with_cmake (PNG
VERSION ${PNG_BUILD_VERSION}
GIT_REPOSITORY ${PNG_GIT_REPOSITORY}
GIT_TAG ${PNG_GIT_TAG}
CMAKE_ARGS
-D PNG_SHARED=${PNG_BUILD_SHARED_LIBS}
-D PNG_SHARED=OFF
-D PNG_STATIC=ON
-D PNG_EXECUTABLES=OFF
-D PNG_TESTS=OFF
-D PNG_TOOLS=OFF
-D PNG_FRAMEWORK=OFF
-D CMAKE_POSITION_INDEPENDENT_CODE=ON
-D CMAKE_INSTALL_LIBDIR=lib
-D PNG_PREFIX=oiio
)


Expand All @@ -40,14 +45,22 @@ set (PNG_REFIND_VERSION ${PNG_BUILD_VERSION})


set (PNG_REFIND_ARGS EXACT REQUIRED)

set (PNG_DIR ${PNG_LOCAL_INSTALL_DIR}/lib/cmake/PNG)
set (PNG_FIND_VERSION_EXACT ON)

if (PNG_BUILD_VERSION VERSION_GREATER 1.6.43)
list (APPEND PNG_REFIND_ARGS CONFIG)
endif ()

find_package(PNG ${PNG_REFIND_VERSION} ${PNG_REFIND_ARGS}
HINTS
${PNG_LOCAL_INSTALL_DIR}/lib/cmake/PNG
${PNG_LOCAL_INSTALL_DIR}
NO_DEFAULT_PATH
)

find_package(PNG ${PNG_REFIND_VERSION} ${PNG_REFIND_ARGS})
set (PNG_INCLUDE_DIRS ${PNG_LOCAL_INSTALL_DIR}/include)
include_directories(BEFORE ${PNG_INCLUDE_DIRS})

if (PNG_BUILD_SHARED_LIBS)
install_local_dependency_libs (PNG png)
Expand Down
5 changes: 5 additions & 0 deletions src/cmake/externalpackages.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ endif ()
# we will continue building, but the related functionality will be disabled.

checked_find_package (PNG VERSION_MIN 1.6.0)
if (TARGET PNG::png_static)
set (PNG_TARGET PNG::png_static)
elseif (TARGET PNG::PNG)
set (PNG_TARGET PNG::PNG)
endif ()

checked_find_package (Freetype
VERSION_MIN 2.10.0
Expand Down
6 changes: 4 additions & 2 deletions src/ico.imageio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
# SPDX-License-Identifier: Apache-2.0
# https://github.com/AcademySoftwareFoundation/OpenImageIO

if (TARGET PNG::PNG)
if (PNG_TARGET)
add_oiio_plugin (icoinput.cpp icooutput.cpp
LINK_LIBRARIES PNG::PNG ZLIB::ZLIB)
INCLUDE_DIRS ${PNG_INCLUDE_DIRS}
DEFINITIONS ${PNG_DEFINITIONS}
LINK_LIBRARIES ${PNG_LIBRARIES} ${PNG_TARGET} ZLIB::ZLIB)
else ()
message (WARNING "libpng not found, so ICO support will not work")
set (format_plugin_definitions ${format_plugin_definitions} DISABLE_ICO=1 PARENT_SCOPE)
Expand Down
2 changes: 1 addition & 1 deletion src/libOpenImageIO/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ if (EMBEDPLUGINS)
PRIVATE
EMBED_PLUGINS=1
${format_plugin_definitions})
target_include_directories (OpenImageIO
target_include_directories (OpenImageIO BEFORE
PRIVATE ${format_plugin_include_dirs})

# Organize the embedded plugins into source folders
Expand Down
6 changes: 4 additions & 2 deletions src/png.imageio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
# SPDX-License-Identifier: Apache-2.0
# https://github.com/AcademySoftwareFoundation/OpenImageIO

if (TARGET PNG::PNG)
if (PNG_TARGET OR PNG_LIBRARIES)
add_oiio_plugin (pnginput.cpp pngoutput.cpp
LINK_LIBRARIES PNG::PNG ZLIB::ZLIB)
INCLUDE_DIRS ${PNG_INCLUDE_DIRS}
DEFINITIONS ${PNG_DEFINITIONS}
LINK_LIBRARIES ${PNG_LIBRARIES} ${PNG_TARGET} ZLIB::ZLIB)
else ()
message (WARNING "libpng not found, so PNG support will not work")
set (format_plugin_definitions ${format_plugin_definitions} DISABLE_PNG=1 PARENT_SCOPE)
Expand Down
2 changes: 1 addition & 1 deletion src/png.imageio/png_pvt.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#pragma once

#include <png.h>
#include <libpng16/png.h>
#include <zlib.h>

#include <OpenImageIO/Imath.h>
Expand Down

0 comments on commit 144f201

Please sign in to comment.