Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: Add build recipe for PNG #4423

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

zachlewis
Copy link
Contributor

As requested in #4387.

zachlewis and others added 5 commits September 13, 2024 13:11
Required for building redistributable python wheels on Windows Wheels
runners.

Signed-off-by: Zach Lewis <[email protected]>
…dation#4429)

This test was running very slowly, taking too long in all circumstances,
but shockly causing GHA CI Mac runners to hit the test timeout
sometimes.

It turns out we were doing a lot of redundant work. There was no need to
make separate pattern and bayer images for every data type. Instead,
hoist the test image generation out of the loop and just make one
(float), then use `-i:type=<type>` to read it into an ImageBuf of the
appropriate data type.

This cuts the time for this test to run by about 3x.

Signed-off-by: Larry Gritz <[email protected]>
MacOS builds require that PNG_FRAMEWORK=OFF in order to find and install dynamic libraries...
...but Windows still has trouble finding and installing dynamic libraries.

Only building static libraries seems to work, as far as building wheels is concerned; but I fear older dynamic libpngs found on the system will be preferred over newer static libpngs freshly-built by OIIO when linking.

Signed-off-by: Zach Lewis <[email protected]>
@lgritz
Copy link
Collaborator

lgritz commented Sep 18, 2024

Looks like we're still failing CI here

@zachlewis
Copy link
Contributor Author

indeed... it's the strangest thing. Still trying to get to the bottom of this.

I think what's happening is, for systems that have a "too old" version of dynamic PNG libraries installed, now that there's a build recipe available, even if we build newer static PNG libs, the build system is still preferring to link the older shared libraries (even though they're too old).

I dunno if this is a clue or a red herring, but it seems the build system might be getting confused about which version of PNG it's found versus the version it's trying to build (at least nominally):

 Could NOT find PNG: Found unsuitable version "1.5.13", but required is at least "1.6.0" (found /usr/lib64/libpng.so)
-- Building package PNG 1.5.13 locally
--         PNG_BUILD_VERSION = 1.6.44
--         PNG_BUILD_SHARED_LIBS = OFF
--         Building local PNG 1.6.44 from https://github.com/glennrp/libpng
--         Downloading local https://github.com/glennrp/libpng
Note: checking out 'f5e92d76973a7a53f517579bc95d61483bf108c0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at f5e92d7 Release libpng version 1.6.44
-- Configuring done
-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_COMPILE_WARNING_AS_ERROR
    CMAKE_MESSAGE_INDENT
    CMAKE_RULE_MESSAGES


-- Build files have been written to: /__w/OpenImageIO/OpenImageIO/build/deps/PNG-build
[1/31] Generating pngprefix.h
[2/31] Generating scripts/pnglibconf.c
[3/31] Generating pnglibconf.c
[4/31] Generating scripts/symbols.out
[5/31] Generating scripts/symbols.chk
[6/31] Generating pnglibconf.out
[7/31] Generating pnglibconf.h
[8/31] Generating scripts/vers.out
[9/31] Generating scripts/sym.out
[10/31] Generating scripts/prefix.out
[11/31] Generating libpng.sym
[12/31] Generating libpng.vers
[13/31] Generating scripts/intprefix.out
[14/31] Building C object CMakeFiles/png_static.dir/pngmem.c.o
[15/31] Building C object CMakeFiles/png_static.dir/pngrio.c.o
[16/31] Building C object CMakeFiles/png_static.dir/pngpread.c.o
[17/31] Building C object CMakeFiles/png_static.dir/pngerror.c.o
[18/31] Building C object CMakeFiles/png_static.dir/pngget.c.o
[19/31] Building C object CMakeFiles/png_static.dir/pngtrans.c.o
[20/31] Building C object CMakeFiles/png_static.dir/pngset.c.o
[21/31] Building C object CMakeFiles/png_static.dir/png.c.o
[22/31] Building C object CMakeFiles/png_static.dir/pngwio.c.o
[23/31] Building C object CMakeFiles/png_static.dir/pngread.c.o
[24/31] Building C object CMakeFiles/png_static.dir/intel/intel_init.c.o
[25/31] Building C object CMakeFiles/png_static.dir/pngwtran.c.o
[26/31] Building C object CMakeFiles/png_static.dir/pngwrite.c.o
[27/31] Building C object CMakeFiles/png_static.dir/pngrutil.c.o
[28/31] Building C object CMakeFiles/png_static.dir/pngrtran.c.o
[29/31] Building C object CMakeFiles/png_static.dir/intel/filter_sse2_intrinsics.c.o
[30/31] Building C object CMakeFiles/png_static.dir/pngwutil.c.o
[31/31] Linking C static library libpng16.a
[0/1] Install the project...
-- Install configuration: "Release"
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/libpng16.a
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/libpng.a
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/include/png.h
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/include/pngconf.h
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/include/pnglibconf.h
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/include/libpng16/png.h
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/include/libpng16/pngconf.h
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/include/libpng16/pnglibconf.h
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/bin/libpng-config
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/bin/libpng16-config
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/share/man/man3/libpng.3
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/share/man/man3/libpngpf.3
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/share/man/man5/png.5
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/pkgconfig/libpng.pc
-- Up-to-date: /__w/OpenImageIO/OpenImageIO/build/deps/dist/bin/libpng-config
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/pkgconfig/libpng16.pc
-- Up-to-date: /__w/OpenImageIO/OpenImageIO/build/deps/dist/bin/libpng16-config
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/libpng/libpng16.cmake
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/libpng/libpng16-release.cmake
-- Up-to-date: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/libpng16.a
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/cmake/PNG/PNGTargets.cmake
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/cmake/PNG/PNGTargets-release.cmake
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/cmake/PNG/PNGConfig.cmake
-- Installing: /__w/OpenImageIO/OpenImageIO/build/deps/dist/lib/cmake/PNG/PNGConfigVersion.cmake
-- Refinding PNG with PNG_ROOT=/__w/OpenImageIO/OpenImageIO/build/deps/dist
-- Found PNG: /usr/lib64/libpng.so (found version "1.5.13") 
-- Found PNG 1.5.13 
--     PNG_INCLUDE_DIR = /usr/include;/usr/include
--     PNG_INCLUDE_DIRS = /usr/include;/usr/include
--     PNG_LIBRARIES = /usr/lib64/libpng.so;/usr/lib64/libz.so

so... in the second line in the above log snippet, it claims to be "Building package PNG 1.5.13 locally", even though it's building 1.6.44.

I'm not sure what, if anything, that has to do with the more serious problems:

  1. That it can't "refind" the freshly-built static PNG given PNG_ROOT, PNG_REFIND_VERSION, or even PNG_REFIND_ARGS CONFIG
  2. That it proceeds to link the system-level dynamic PNG 1.5.13 that it previously detected as too old

A lesser problem, but one worth figuring out -- currently, I'm forcing the recipe to only build static libraries, cuz Windows has trouble figuring out what to do with / how to install the dynamic PNG library it just built. (I was having similar trouble on macOS until I set -DPNG_FRAMEWORK=OFF; but I've not yet investigated what specifically is (or isn't) happening on Windows.

Ideally, we'd only link the static libraries, since that seems to behave properly across platforms, given the opportunity...
I believe the OCIO missing-dependency-build-system business allows one to forcibly prefer static libraries instead of dynamic libraries if both are found (on a per-library basis). I don't suppose there's a way to do that here?

@zachlewis
Copy link
Contributor Author

I'm kind of baffled by the CI build failures... I can't seem to pinpoint anything in the logs or the configs that would explain why or how the failures occur on some runs, but not others...

The problematic runs are:

  • CI / VFX2021 oldest gcc9.3/C++17 py3.7 exr-3.1
  • CI / VFX2021 gcc9/C++17 py3.7 exr3.1 ocio2.0
  • CI / VFX2022 gcc9/C++17 py39 exr3.1 ocio2.1
  • CI / VFX2022 clang13/C++17 py39 avx2 exr3.1 ocio2.1

And they all fail the same way:

FAILED: bin/simd_test 
: && ccache /usr/local/bin/_ccache/clang++ -O3 -DNDEBUG  src/libutil/CMakeFiles/simd_test.dir/Unity/unity_0_cxx.cxx.o -o bin/simd_test  -Wl,-rpath,/__w/OpenImageIO/OpenImageIO/build/lib:/usr/local/lib64:/__w/OpenImageIO/OpenImageIO/build/deps/dist/lib  lib/libOpenImageIO.so.2.6.6  lib/libOpenImageIO_Util.so.2.6.6  -lpthread  /usr/local/lib64/libImath-3_1.so.29.4.0  -lm  -Wl,-rpath-link,/usr/local/lib64:/__w/OpenImageIO/OpenImageIO/build/deps/dist/lib && :
/opt/rh/devtoolset-9/root/usr/lib/gcc/x86_64-redhat-linux/9/../../../../bin/ld: lib/libOpenImageIO.so.2.6.6: undefined reference to `png_set_eXIf_1'
/opt/rh/devtoolset-9/root/usr/lib/gcc/x86_64-redhat-linux/9/../../../../bin/ld: lib/libOpenImageIO.so.2.6.6: undefined reference to `png_get_eXIf_1'
/opt/rh/devtoolset-9/root/usr/lib/gcc/x86_64-redhat-linux/9/../../../../bin/ld: lib/libOpenImageIO.so.2.6.6: undefined reference to `png_set_option'

But... considering the rest of the checks pass, it doesn't seem to be an issue with the compiler or compiler version, the VFX202X build image, the version of Python, the version of EXR, the version of OCIO, or whether sse instructions are used. And I can't seem to ascertain a particular constellation of settings that produces a failure.

The only thing I could identify that looked suspicious to me is the ordering of paths in the -Wl,-rpath-link,/usr/local/lib64:/__w/OpenImageIO/OpenImageIO/build/deps/dist/lib part of the build command for bin/simd_test. Earlier in the command, the path order is the reverse, with OIIO's build/deps/dist/lib path preceding /usr/local/lib64 (-Wl,-rpath,/__w/OpenImageIO/OpenImageIO/build/lib:/usr/local/lib64).

The logs aren't verbose enough for successful builds for me to tell if this is the difference that explains the failures of unsuccessful runs.

Attempt to force the build system to always find the locally-built PNG before any system-installed PNGs

Signed-off-by: Zach Lewis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants