elalish / manifold

Geometry library for topological robustness
Apache License 2.0
708 stars 68 forks source link

Modularize Manifold #803

Open elalish opened 1 month ago

elalish commented 1 month ago

We've been getting more requests to have just core Manifold functionality (usually Booleans) with minimal dependencies (like Clipper, convex hulls, SDFs, etc). Our code is largely already partitioned this way, but I think we need to make some build flags to allow it to build modularly. Context: https://github.com/godotengine/godot/pull/91748#discussion_r1595679227

I'm curious if anyone knows any best practices for how to set this up? ASSIMP does a lot of this. @kintel perhaps this would also help for OpenSCAD? Curious if you have any thoughts.

pca006132 commented 1 month ago

I feel that getting rid of Clipper can be quite painful. If we want to do something about clipper dependency, it will be a large breaking change and should be in the 3.0 plan. I think there were comments about using our own type so it is easier to refactor.

elalish commented 1 month ago

Isn't clipper pretty much walled-off within CrossSection? I think we should be able to build Manifold without CrossSection if we want.

elalish commented 1 month ago

Also relevant to Blender integration: https://projects.blender.org/blender/blender/issues/120182#issuecomment-1170472

pca006132 commented 1 month ago

ah, I was conflating it with glm, not something optional.

probably indicating I should sleep now :P

kintel commented 1 month ago

Some thou^H^H^H^Hramblings:

Something which would make sense to keep in mind: There seems to be a beginning movement to start packaging Manifold into Linux distros. This currently comes implicitly through the OpenSCAD distro packages, but may become strengthened by other OSS projects adopting manifold. This can be a bit of a pivotal moment, as you'll be held to API versioning/compatibility standards (if not you, the people ending up owning your packages). Good API design and semantic versioning gets you a long way here, but having multiple variants of the same binary library is a bad thing in this domain (unless there is some sort of ultimate build containing everything that makes it into distros). Could be worth keeping in the back of your heads.

pca006132 commented 1 month ago

Splitting into different packages should be simple. In fact we have that internally. But probably need less generic naming, as well as more tests to make sure some packages can be optional and things can still work.

fire commented 1 month ago

In Godot Engine engine's build we use these dependencies.

The only reasonable dependency we can remove is [thrust, libcudacxx]. quickhull is fairly short.

As mentioned earlier glm is load bearing and if you didn't pick glm, another implementation of math in C++ would probably be as big.

elalish commented 1 month ago

As we've said before, Manifold has absolutely no dependence on libcudacxx anymore - we're not using that backend of Thrust at all. Is there a reason you're including it?

fire commented 1 month ago

In thrust:

All mentions of #include <cuda/std/type_traits> need to be converted to #include <type_traits>

You're right!

Maybe I'm missing a flag or I'm using the wrong thrust.

Edited:

https://github.com/NVIDIA/libcudacxx/blob/55dd2c99346baa3a14949a0f7e9c41865e434eda/include/cuda/std/type_traits

fire commented 1 month ago

I switched to the newest cccl (thrust, libcudacxx). https://github.com/nvidia/cccl and it seems to still requires cccl/libcudacxx.

Note: that https://github.com/NVIDIA/thrust has been archived on Mar 21, 2024

Edited:

Compile log where I removed the cccl/libcudacxx folder.

log.txt

Edited:

There seems to be chain of includes from thrust that go to libcudacxx.

elalish commented 1 month ago

Interesting, it does seem like they pulled libcudacxx deeper into Thrust at some point, probably in preparation for this CCCL thing it's become. Looks like we're not the only ones displeased.

Maybe it's time to finally get off of Thrust - deprecation is a pretty good reason. @pca006132 already did a lot of work to make it easier for us to slot in a different parallel library underneath. @pca006132 Do you have any thoughts on what we should switch to? It's still hard for me to tell if PSTL encompasses TBB or vice versa, or if there's a yet more universal API we should be using.

cjmayo commented 1 month ago

If installing CCCL only to build Manifold (or OpenSCAD) then it can configured with:

-DCCCL_ENABLE_LIBCUDACXX=OFF
-Dlibcudacxx_ENABLE_INSTALL_RULES=ON
-DCCCL_ENABLE_CUB=OFF
-DCUB_ENABLE_INSTALL_RULES=ON
-DCCCL_ENABLE_THRUST=OFF
-DTHRUST_ENABLE_INSTALL_RULES=ON
-DCCCL_ENABLE_TESTING=OFF

Perhaps the issue is in using it as a submodule without building (just copying the files).

elalish commented 1 month ago

Is there a better way for manifold to package our Thrust (or CCCL) dependency than as a submodule? Curious if there's best practices here...

cjmayo commented 1 month ago

Personally I'm not using the submodule, but instead installing separately into my system and using it for Manifold and OpenSCAD.

But my guess is if using the submodule perhaps there is a need to run cmake etc. and install into a local directory and then use the installed directory when building Manifold.

fire commented 1 month ago

I like https://github.com/ingydotnet/git-subrepo for my projects as git submodules are difficult to use. Especially when for example manifold does manifold -> thrust -> imgui

elalish commented 1 month ago

I have to admit, the problems you're running into @fire (copying dependencies instead of pulling the submodule) makes me think the submodule approach is better. I don't want that giant pile of unused files in my repo any more than you do (which is what I get with git-subrepo). Thrust and CCCL still don't link in the actual CUDA code in our case, so all that cub and libcudacxx business doesn't actually end up in any of our binaries anyway.

I don't really see what all the submodule hate is about anyway - I find them pretty great for managing large 3rd-party dependencies that I don't ever want to touch, besides to update to the latest release occasionally. I feel like everyone who dislikes them is using them when they control both repos. Am I missing something?

fire commented 1 month ago

My previous monorepo had like 40+ submodules in various states of nesting. Your use cases may vary.