OpenVDB : Update to version 12.1.1#301
Conversation
Revert to PyBind11 2.10.4 as the update to PyBind11 2.13.6 appears to be causing issues with OCIO in Gaffer on Windows. We can try again once we update to OCIO 2.5.x for VFX Platform CY2026 as CI for those releases has moved from PyBind11 2.9.2 to 3.0.1.
johnhaddon
left a comment
There was a problem hiding this comment.
Thanks Murray - I've asked a few questions inline.
Seeing robin-map does make me think we should investigate it (and possibly others) for our most performance-critical data structures (like the caches) - I suspect there are gains to be had...
| # Cortex builds made with OpenVDB 12 and above require nanobind-static. | ||
| # If/when more projects require Nanobind, we may want to consider building | ||
| # it independently. | ||
| "cp build/openvdb/openvdb/python/libnanobind-static.a {buildDir}/lib", |
There was a problem hiding this comment.
How does this relate to the independent build we are doing?
There was a problem hiding this comment.
Just to note I do have an SConstruct setup in Gaffer that will build the nanobind-static part + link that I can share, following the comments in this file: https://github.com/wjakob/nanobind/blob/master/src/nb_combined.cpp
There was a problem hiding this comment.
How does this relate to the independent build we are doing?
Currently it's replacing the need for the independent build, if we were to build libnanobind-static.a ourselves, it would be removed.
|
|
||
| "mkdir build", | ||
| "cd build && cmake -D CMAKE_INSTALL_PREFIX={buildDir} ..", | ||
| "cd build && cmake --build . && cmake --install .", |
There was a problem hiding this comment.
How does this interact with things like OIIO, OSL and USD, which also have dependencies on robin-map, but presumably were vendoring them in or downloading them automatically before?
There was a problem hiding this comment.
With the standard CI build order, this is coming after OIIO and OSL so they continue to download and build with their preferred versions as before. USD vendors its version in as pxrTslRobinMap under a pxr_tsl namespace and this additional robin-map doesn't change its build behaviour.
I haven't tried switching OIIO and OSL to use this robin-map, but can give it a go if you think it worthwhile.
There was a problem hiding this comment.
I'm just a little wary of symbols from different versions of robin-map causing conflicts. So if it's easy to make everything use this one version then that might be a good thing. On the other hand, OIIO and OSL are pretty good about hiding symbols in general, so whatever you think is best...
There was a problem hiding this comment.
OIIO and OSL are already using quite different robin-map versions from each other (1.4.0 for OIIO & 0.6.2 for OSL), but I'll do a little bit of checking today.
There was a problem hiding this comment.
The only place I'm seeing visible robin-map symbols is USD where they're in the pxr_tsl namespace, so I don't think we have any concerns there.
Switching OIIO and OSL to use our robin-map version would just be a matter of adding it as a dependency to both projects, then they both find our robin-map with no issue. Bumping OIIO from 1.4.0 to 1.4.1 should be trivial, robin-map 1.4.1 is just changing cmake_minimum_required to VERSION 3.10 so we wouldn't see much difference other than removing OIIO's need to download robin-map 1.4.0.
OSL would be a more significant bump from robin-map 0.6.2. OSL builds fine locally with 1.4.1 and our GafferOSL and GafferCycles tests pass, so I've pushed 731431c to see how it looks on CI.
There was a problem hiding this comment.
macOS and Windows builds look good on CI. Adding robin-map as a dependency to OSL and OIIO also removes the potential for inconsistency when iterating on local dependencies builds, where rebuilding OIIO or OSL after OpenVDB would otherwise cause those builds to find and use our robin-map rather than download it (which would have happened in the initial clean build). Having a single version to report for #166 would also be preferable.
This is the last of the VFX Platform CY2025 updates necessary for Gaffer 1.7, and includes rolling back PyBind11 to 2.10.4 due to the issues encountered on Windows in GafferHQ/gaffer#6928.