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

Added Houdini 20 CI for old and new CXX11 ABI #1725

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

Idclip
Copy link
Contributor

@Idclip Idclip commented Dec 6, 2023

No description provided.

@Idclip
Copy link
Contributor Author

Idclip commented Dec 6, 2023

Here's an example of the Houdini CI run once this is merged and the weekly job udpates the cache: https://github.com/Idclip/openvdb/actions/runs/7113714217

@jmlait @e4lam We seem to have some issues building against Houdini 20 with clang. Seems to build fine with GCC:

 In file included from /__w/openvdb/openvdb/openvdb_houdini/openvdb_houdini/GeometryUtil.cc:8:
/__w/openvdb/openvdb/openvdb_houdini/openvdb_houdini/GeometryUtil.h:251:57: error: member access into incomplete type 'const GEO_Primitive'
            normal = mRefGeo.getGEOPrimitive(primOffset)->computeNormal();
                                                        ^
/github/home/houdini_install/hou/toolkit/include/GEO/GEO_PrimList.h:18:7: note: forward declaration of 'GEO_Primitive'
class GEO_Primitive;
      ^
In file included from /__w/openvdb/openvdb/openvdb_houdini/openvdb_houdini/GeometryUtil.cc:8:
/__w/openvdb/openvdb/openvdb_houdini/openvdb_houdini/GeometryUtil.h:257:63: error: member access into incomplete type 'const GEO_Primitive'
                    tmpN = mRefGeo.getGEOPrimitive(primOffset)->computeNormal();
                                                              ^
/github/home/houdini_install/hou/toolkit/include/GEO/GEO_PrimList.h:18:7: note: forward declaration of 'GEO_Primitive'
class GEO_Primitive;
      ^
2 errors generated.
gmake[2]: *** [openvdb_houdini/openvdb_houdini/CMakeFiles/openvdb_houdini.dir/build.make:121: openvdb_houdini/openvdb_houdini/CMakeFiles/openvdb_houdini.dir/GeometryUtil.cc.o] Error 1

@e4lam
Copy link
Contributor

e4lam commented Dec 12, 2023

@Idclip Ah my bad, I had fixed this but didn't realized that I had touched openvdb_houdini headers because it was all over the code base. So I never triggered a PR for this:

--- dev/src/houdini/custom/DWA/vdb/GeometryUtil.h	2023/09/09 23:36:41	453698
+++ dev/src/houdini/custom/DWA/vdb/GeometryUtil.h	2023/09/09 23:39:09	453699
@@ -15,6 +15,7 @@
 #include <openvdb/util/Util.h> // for openvdb::util::COORD_OFFSETS
 
 #include <GU/GU_Detail.h>
+#include <GEO/GEO_Primitive.h>
 
 #include <algorithm> // for std::max/min()
 #include <memory>

EDIT: PS. Sorry for the slow reply as I've been on vacation.

@Idclip
Copy link
Contributor Author

Idclip commented Dec 12, 2023

Thanks @e4lam! No worries, I didn't want to blindly add the include in case there was a deeper issue between clang/gcc, but this does indeed fix the clang build.

I've made this change, example workflow result here: https://github.com/Idclip/openvdb/actions/runs/7177898971

This should be good to go

@Idclip Idclip enabled auto-merge December 12, 2023 08:20
Copy link
Contributor

@danrbailey danrbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome to be able to build against both old and new C++ ABIs, thank you very much for doing this!

@Idclip Idclip merged commit ff2e59f into AcademySoftwareFoundation:master Dec 19, 2023
39 of 46 checks passed
@Idclip Idclip deleted the houdini_20_ci branch December 19, 2023 06:49
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.

3 participants