Correct OpenEXR/ImfHeader.h headers

[henri-gasc]

Fix #1043. The program compiles without any change to CMakeLists.txt.
I did not however try to link a program to this updated version of OpenEXR

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: henri-gasc (066d2586c11b1466a2cfa4697299bb0b65da1d39)

[peterhillman]

It's time we did this, but I think it needs to be in the next major release. The fact that OpenEXR's own tests fail with this change is an indication about the problems here: /src/openexr/src/lib/OpenEXR/ImfHeader.h:30:10: fatal error: 'Imath/ImathBox.h' file not found

To compile against OpenEXR it is not required to include the <imath_install_prefix>/include/ directory, but you must include <imath_install_prefix>/include/Imath.

If you build with cmake and use findpackage(OpenEXR), you get both folders, so this change only affects people using their own system to include OpenEXR and Imath. If those application code build systems include <prefix>/include/Imath and not the parent, this breaks. There is a lot of code that uses #include <ImathVec.h> and #include <ImfInputFile.h> instead of #include <Imath/ImathVec.h> and #include <OpenEXR/ImfInputFile.h>, so may not be including the parent directory when compiling. I would suggest making this change in OpenEXR-3.3 (if there is one). OpenEXR-4.0 can also remove the <prefix>/include/Imath from cmake config, so code has to use #include <Imath/ImathVec.h>.

[henri-gasc]

Is there any possible consequence to changing the order of the include files here?

There shouldn't be any, I just think it's more readable if the headers are separated according to their type (current directory, standard library, packages, etc.)

[henri-gasc]

If you build with cmake and use findpackage(OpenEXR), you get both folders, so this change only affects people using their own system to include OpenEXR and Imath. If those application code build systems include <prefix>/include/Imath and not the parent, this breaks.

Should I make a big PR that changes all the headers used (and the cmake files) ?

[peterhillman]

Yes, I think it makes sense to change all the headers, and internal references to Imath in cpp files, to use #include <Imath/> when including Imath, and the build system needs updating too.

I got a failure building this PR without providing Imath (so it has to download and build that as part of the OpenEXR build). It doesn't get the correct paths: $<BUILD_INTERFACE:_deps/imath-src/src/Imath>;$<BUILD_INTERFACE:_deps/imath-build/config> But that doesn't include $<BUILD_INTERFACE:_deps/imath-src/src/>, so #include <Imath/> fails. I'm not fluent enough in cmake to understand how to change this.

It looks like bazel needs changing too, and possibly the build script that oss-fuzz uses (https://github.com/google/oss-fuzz/blob/master/projects/openexr/build.sh). I think any changes to oss-fuzz have to be accepted before the PR gets merged.

That change should still make it possible to use either #include <ImathVec.h> or #include <Imath/ImathVec.h> in developer code when using the config files provided by either Imath or OpenEXR. The only change for developer code will be that other build systems may need a tweak to the configuration for the code to compile, and that's a "minor" change, but more than a "patch" change.