AcademySoftwareFoundation / openvdb

OpenVDB - Sparse volume data structure and tools
http://www.openvdb.org/
Mozilla Public License 2.0
2.58k stars 639 forks source link

[BUILD] will not build with openexr 3 #1034

Closed pgajdos closed 2 years ago

pgajdos commented 3 years ago

Environment

Operating System: SUSE Linux, Tumbleweed Version / Commit SHA: 8.0.1, 7.2.3 CMake Version: 3.19 Compiler: gcc10

Describe the problem

[    4s] -- ----------------------------------------------------
[    4s] -- ------------- Configuring OpenVDBCore --------------
[    4s] -- ----------------------------------------------------
[    4s] CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:218 (message):
[    4s]   Could NOT find IlmBase (missing: IlmBase_INCLUDE_DIR) (Required is at least
[    4s]   version "2.2")
[    4s] Call Stack (most recent call first):
[    4s]   /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:582 (_FPHSA_FAILURE_MESSAGE)
[    4s]   cmake/FindIlmBase.cmake:258 (find_package_handle_standard_args)
[    4s]   openvdb/openvdb/CMakeLists.txt:49 (find_package)

Build log here: https://build.opensuse.org/package/live_build_log/home:pgajdos:openexr3/openvdb/openSUSE_Factory_openexr3/x86_64

pgajdos commented 3 years ago

cc @hobbes1069

hobbes1069 commented 3 years ago

Ilm is now included in the imath library which I'm working on adding to Fedora.

https://github.com/AcademySoftwareFoundation/Imath/blob/v3.0.1/docs/PortingGuide2-3.md#openexr-and-imath-are-different-packages

djovanoski commented 3 years ago

I have the same issue any update to help me ?

hobbes1069 commented 3 years ago

Alright guys, this is pretty damn silly for this project to not have been updated along side the openexr release since both are under the stewardship of the ASWF. Is this going to get some attention or not?

hobbes1069 commented 3 years ago

I used the Imath porting guide and was able to port most of the packages dependent on OpenEXR (including OpenVDB) to the 3.x series. 80% of the time it was just updating the build system to adjust to the change in library names but a few, including OpenVDB, needed some header adjustments.

I'm attaching my patch for Fedora Linux here. I tried to do it in a way that would be portable to other systems but haven't performed any testing so YMMV.

Index: openvdb-8.1.0/openvdb/openvdb/cmd/CMakeLists.txt
===================================================================
--- openvdb-8.1.0.orig/openvdb/openvdb/cmd/CMakeLists.txt
+++ openvdb-8.1.0/openvdb/openvdb/cmd/CMakeLists.txt
@@ -116,27 +116,36 @@ endif()
 #### vdb_render

 if(OPENVDB_BUILD_VDB_RENDER)
-  find_package(IlmBase ${MINIMUM_ILMBASE_VERSION} REQUIRED COMPONENTS Half Iex IlmThread Imath)
-  find_package(OpenEXR ${MINIMUM_OPENEXR_VERSION} REQUIRED COMPONENTS IlmImf)
+  find_package(Imath CONFIG)
+  if(TARGET Imath::IMath)
+   find_package(OpenEXR CONFIG REQUIRED)
+  else()
+    find_package(IlmBase ${MINIMUM_ILMBASE_VERSION} REQUIRED COMPONENTS Half Iex IlmThread Imath)
+    find_package(OpenEXR ${MINIMUM_OPENEXR_VERSION} REQUIRED COMPONENTS IlmImf)

-  set(VDB_RENDER_SOURCE_FILES openvdb_render.cc)
-  add_executable(vdb_render ${VDB_RENDER_SOURCE_FILES})
+    set(VDB_RENDER_SOURCE_FILES openvdb_render.cc)
+    add_executable(vdb_render ${VDB_RENDER_SOURCE_FILES})

-  # Set deps. Note that the order here is important. If we're building against
-  # Houdini 17.5 we must include OpenEXR and IlmBase deps first to ensure the
-  # users chosen namespaced headers are correctly prioritized. Otherwise other
-  # include paths from shared installs (including houdini) may pull in the wrong
-  # headers
+    # Set deps. Note that the order here is important. If we're building against
+    # Houdini 17.5 we must include OpenEXR and IlmBase deps first to ensure the
+    # users chosen namespaced headers are correctly prioritized. Otherwise other
+    # include paths from shared installs (including houdini) may pull in the wrong
+    # headers

   target_link_libraries(vdb_render
-    IlmBase::Half
-    OpenEXR::IlmImf
-    IlmBase::IlmThread
-    IlmBase::Iex
-    IlmBase::Imath
+    # For OpenEXR/Imath 3.x:
+    $<$<TARGET_EXISTS:OpenEXR::OpenEXR>:OpenEXR::OpenEXR>
+    $<$<TARGET_EXISTS:Imath::Imath>:Imath::Imath>
+    $<$<TARGET_EXISTS:Imath::Half>:Imath::Half>
+    # For OpenEXR 2.4/2.5:
+    $<$<TARGET_EXISTS:OpenEXR::IlmImf>:OpenEXR::IlmImf>
+    $<$<TARGET_EXISTS:IlmBase::Imath>:IlmBase::Imath>
+    $<$<TARGET_EXISTS:IlmBase::Half>:IlmBase::Half>
+    $<$<TARGET_EXISTS:IlmBase::IlmThread>:IlmBase::IlmThread>
+    $<$<TARGET_EXISTS:IlmBase::Iex>:IlmBase::Iex>
     ${OPENVDB_BINARIES_DEPENDENT_LIBS}
   )
-
+  endif()
   if(WIN32)
     # @note OPENVDB_OPENEXR_STATICLIB is old functionality and should be removed
     get_target_property(ILMBASE_LIB_TYPE IlmBase::Half TYPE)
Index: openvdb-8.1.0/openvdb/openvdb/Types.h
===================================================================
--- openvdb-8.1.0.orig/openvdb/openvdb/Types.h
+++ openvdb-8.1.0/openvdb/openvdb/Types.h
@@ -9,7 +9,22 @@
 #include "TypeList.h" // backwards compat

 #ifdef OPENVDB_USE_IMATH_HALF
-#include <OpenEXR/half.h>
+// The version can reliably be found in this header file from OpenEXR,
+// for both 2.x and 3.x:
+#include <OpenEXR/OpenEXRConfig.h>
+#define COMBINED_OPENEXR_VERSION ((10000*OPENEXR_VERSION_MAJOR) + \
+                                  (100*OPENEXR_VERSION_MINOR) + \
+                                  OPENEXR_VERSION_PATCH)
+
+// There's just no easy way to have an `#include` that works in both
+// cases, so we use the version to switch which set of include files we
+// use.
+#if COMBINED_OPENEXR_VERSION >= 20599 /* 2.5.99: pre-3.0 */
+#   include <Imath/half.h>
+#else
+    // OpenEXR 2.x, use the old locations
+#   include <OpenEXR/half.h>
+#endif
 namespace openvdb {
 OPENVDB_USE_VERSION_NAMESPACE
 namespace OPENVDB_VERSION_NAME {
Idclip commented 3 years ago

@hobbes1069 thanks very much for your work on this and the provided patch - sorry for the radio silence, we'll be looking to get this updated before the next major release.

Idclip commented 2 years ago

Thanks again for the patch, a version of this has now been applied. Additionally the EXR dependency on the vdb_render binary has been made optional. These changes will be available in VDB 9 (and maybe 8.2) which should be released at the end of the month.