breaking changes in v3.0

[pca006132]

#542 will break the existing c++ interface, maybe we should gather ideas about APIs that are unwieldy and change them in the v3.0 release.

1. Change from float to double. Users should use manifold::vec3 etc.

[pca006132]

@elalish what do you think about changing indices in the public API to 64 bit integers? This will allow us to support larger mesh later. I think directly changing every internal indices to 64 bit will cause large performance regression, so we probably want to do so dynamically, only use 64 bit if it does not fit in 32 bit.

[elalish]

That feels like more code complexity than is worthwhile, particularly since it would be difficult to test. I don't know of many machines that will handle 2 billion triangles in a Boolean op without taking a very long time or running out of memory. And I also don't see many use cases where that number of triangles would be valuable. When we find that use case, it will probably want a more efficient solution, like some intelligent use of smoothing.

[pca006132]

True, this can be very difficult to test. I think I got into the 2 billion triangle limit several times while stressing some tests.
I think today machines can handle 2 billion triangles in a Boolean op within an hours due to various optimizations we had, so it does not sound like an impossible/useless scenario.

IIRC there are users doing IFC, not sure if they will ever get into that kind of complex mesh. They probably have better methods to handle that though (decomposing the mesh or something).

[elalish]

I think the scope of this has been narrowed down a bit in #833? Care to detail the remaining breaking changes? I think we talked about the index in Smoothness (rarely used anyway) and possibly adding a MeshGL64, though that wouldn't be breaking change.

[pca006132]

Another idea: what about changing ^ to & that users are more familiar with? @zalo @wrongbad.
I think I got this question from time to time, so it is probably confusing to future users.

[wrongbad]

I guess this is too subjective, so my wrapper for the API maps it to & for my purposes. For me personally, the manifold ^ is about as weird as overloading - to be division because it resembles a fraction bar - it's basically a show-stopper for my intuition to recognize equivalent ASTs. Probably because I've written a lot of bitwise code in the past.

[pca006132]

I think the major issue here is that ^ also means xor, and xor also means symmetric difference for a set...

[zalo]

So if we went full bitwise, it would be:

 Addition     : a | b
 Subtraction  : a ~ b
 Intersection : a & b
!Intersection : a ^ b

It would be interesting to have consistent symbols, but I wonder what the & is of bitwise folks and CAD/3D Modelling folks...

[pca006132]

while this is horrible it is somehow making sense to me, too weird 😅

[elalish]

Thanks all, but I'm making an executive decision on this one: we're going to stick with our existing set-inspired notation instead of switching to binary op notation.

[t-paul]

I have not seen a way of automatically getting the manifold library version. It's not a problem when used as git submodule, but once it gets packaged that would be very helpful in case of major version bumps.

Ideally this would allow to get both the "compiling with" version from a header file and also something like get_manifold_version() for "running with shared library".

Example: https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-version.h

[pca006132]

And also maybe something like FindManifold.cmake, if we want to make it easy to package and used as a system library? I am not familiar with that, but I think we can do it without making a breaking change :)

[elalish]

I believe this has now been completed. @t-paul are you happy with the CMake version we added?

[pca006132]

btw maybe we should change the cmake option -DMANIFOLD_PAR=TBB into something like -DMANIFOLD_PAR=ON. We no longer have multiple backends anyway.

[elalish]

Works for me, and I don't have a lot of desire to go back to maintaining multiple backends anyway.

[elalish]

In #771 I just realized how weird my properties indexing is in our API. I think it's a little bit inevitable that we have the weirdness of starting property indices at 0 on Manifold, but at 3 on MeshGL - since in that case there is no distinction between the position property and the rest. But GetMeshGL(normalIdx) is very odd in that it uses the output MeshGL-style indexing instead of the input Manifold-style indexing. It results in calls like shape.CalculateNormals(0).GetMeshGL({3,4,5}). Also, I can't think of a time when the three normal channels wouldn't be consecutive, so specifying as a glm::ivec3 seems kind of annoying (especially for bindings).

What does everyone think of tweaking GetMeshGL to so that this works: shape.CalculateNormals(0).GetMeshGL(0)?

[pca006132]

I think there are probably more people relying on MeshGL indexing comparing with indexing in Manifold, e.g. for js binding. Changing APIs in Manifold to skip the first 3 indices seems better in this case? And we can use overload to allow something like GetMeshGL(3) instead of GetMeshGL({3, 4, 5}).

Btw, should we put [deprecated] for Mesh data structure to discourage people using it?

[elalish]

Well, I'm not proposing to actually change the data structure of MeshGL at all, just places where functions take channel numbers as input. I don't think any changes are needed on the MeshGL object itself.

Yeah, I'm curious if Mesh is really valuable to anyone? It would be nice to simplify the API and testing. Mesh is a little simpler to use for folks doing pure geometry stuff, but I'm not sure if it makes enough difference.

[pca006132]

That sounds fine. I think openscad uses Mesh, but it should not be hard to switch to MeshGL for their use case.

[howardt]

I was thinking of using Mesh instead of MeshGL for the Blender integration because (a) not getting a lot of value from the MeshGL interface; and (2) reading the current code, I see an extra copy (or more) of vertex positions when going through MeshGL.

[elalish]

Extra copying we can take care of if it's causing a serious performance problem. I'm surprised MeshGL isn't giving you anything useful - we should talk about that some more offline.

[elalish]

I think I made a mistake in putting CrossSection into manifold.h - the whole point of Polygon was to keep a clean separation between the 2D and 3D libraries. If we use Polygon as the interface instead, then anyone who doesn't want the Clipper2 dependency can just not include the cross_section library.

This would be part of addressing #803. @fire @kintel @howardt is there anything special we need to do with our CMake to make it easier to remove this 2D dependency when it's not needed?

[elalish]

Sounds reasonable. Does this mean that the polygon library would include the entire 2D subsystem (2D constructors, 2D-to-3D operators, polygon tessellation, etc..) ?

@kintel polygon is poorly named - it's just my triangulator, which is definitely needed by manifold. The 2D subsystem is cross_section and is basically just a wrapper around Clipper2. 2D->3D and 3D->2D are core parts of manifold, but can just use the Polygon struct instead of having any dependence on Clipper.

As mentioned earlier, requesting this library through a CMake find macro would be cool too, but I believe polygon is already a separate library, so some of that is already sorted.

Sounds great, but I know practically nothing about CMake find macros. Care to point us in the right direction?

[kintel]

Thanks for the clarification! I don't think we currently use any part of the 2D subsystem.

[kintel]

I don't really have much good advice on CMake except read the manual :) CMake can be pretty unintuitive and trying to do stuff without really understanding the core concepts can be hard.

[fire]

As long as the change is constructed as a define or a cpp translation unit it's fine.

[kintel]

Sounds reasonable. Does this mean that the polygon library would include the entire 2D subsystem (2D constructors, 2D-to-3D operators, polygon tessellation, etc..) ?

As mentioned earlier, requesting this library through a CMake find macro would be cool too, but I believe polygon is already a separate library, so some of that is already sorted.

[elalish]

This was mentioned in a thread above, but I'd like to call it out here specifically: marking Mesh for deprecation (but not actually removing it yet). The only thing we're really using it for in our API is LevelSet (and I/O, but we already have MeshGL versions of all that). I'm now thinking that sdf is small enough that it's not really deserving of its own folder/library. I'm thinking of moving it in as a static constructor: Manifold::LevelSet. This will make it more discoverable/consistent in our API, and also allow us to be a bit more efficient with our construction, e.g. potentially doing some smarter simplification.

Any objections?

[elalish]

@zalo What do you think of this in light of #598?

[zalo]

That all seems reasonable 👍

My preferred "minimal" mesh interchange is something like std::pair<std::vector<glm::dvec3>, std::vector<glm::ivec3>> anyway; I think composing std containers plays well with nanobind...

[howardt]

The statement I made above re preferring Mesh to MeshGL is now wrong. I needed something from MeshGL that I just couldn't get from Mesh, so now I am using MeshGL for the interface in my Blender integration. So no objection from me to deprecating Mesh.

[pca006132]

I'm also thinking about some API overhaul:

1. Use 64-bit integer for vertex/triangle indices/count in public APIs.
2. Add bounding box over-approximate, so TrimByPlane can be implemented lazily.
3. Perhaps remove Split and SplitByPlane? Not sure if anyone is using that, and ideally we should be able to implement it with csg tree using some hash consing technique.

[howardt]

I would be interested to know if changing to 64-bit integers would cause a noticeable performance degradation for meshes with many millions of vertices/triangles. For our use case in Blender, 32 bits is currently sufficient. Though I do applaud the future proofing that this change might bring.
I had been intending to try using SplitByPlane (a common special case for Blender users) but hadn't done so yet. There is likely an acceptable other method to achieve the same result, however.

[pca006132]

The idea is to do some profiling and check which part is having an impact, and try to mitigate that by using 32-bit integers for those part when the input mesh is small. I would expect things like collider and boolean3 part requiring some refactoring with this, as they are currently already memory bound (sorting a large array of indices). Before we actually change our internals to allow for 64-bit integers, this should not have any noticeable impact on performance.

For Split/SplitByPlane, it is just a faster way of doing a ^ b and a - b at the same time by sharing some internal results. I just realized I don't have concrete numbers about how much faster Split is comparing with doing a ^ b and a - b...

For interactive applications like blender, users will probably not benefit much from lazy evaluation and optimizations by rewriting csg tree though, so perhaps Split is still needed for those cases.

[elalish]

I don't think Split or SplitByPlane are hurting anything, are they? Probably best to leave them as is for now and reconsider down the road.

We'll leave MeshGL as 32 bit, so at least data transfers won't double in size - MeshGL64 will be our future-proof I/O structure. I also don't think the switch the 64 bit ints internally will happen anytime soon, and when it does, rest assured we'll do a lot of perf testing to make sure it's worthwhile.

Add bounding box over-approximate, so TrimByPlane can be implemented lazily.

I like this idea, though I don't think it's a breaking change, which means it can also happen after v3.0. Let's button up the breaking changes first and then prioritize further.

[neilpa]

I don't think Split or SplitByPlane are hurting anything, are they? Probably best to leave them as is for now and reconsider down the road.

Yes please. I've already found a couple places where it's nice to split and get both sides of the result. E.g.

// helpers elided for brevity
const [stem, remainder] = squaredLune(3, 2, 1.5).revolve(6).splitByPlane([0,0,-1], 0)

const top = sliceBottom(remainder).extrude(1)
const base = sliceBottom(stem).extrude(0.2);

const result = zStack(base, stem, top);