scallop.cpp sample and memory allocation

[ofmetal]

Scallop sample contains this function:
Manifold Scallop()
There is
MeshGL64 scallop;
declaration inside. The "scallop" is the structure's instance created on a stack, it contains several std::vector inside.
At the end of the function I see this function call:
return Manifold::Smooth(scallop, sharpenedEdges);
Which handles the scallop like this:
std::shared_ptr impl = std::make_shared(meshGL);
So, it creates pointer "impl" to original "MeshGL64 scallop" created on a stack and returns Manifold(impl).
Then, after original function "Manifold Scallop()" is done, all stack-created instances are deleted.
Including "MeshGL64 scallop;" , pointer to which is returned.

Does this makes sense?

[pca006132]

No, that make_shared call copies the underlying vectors.

[elalish]

As for precision - 32-bit floats are still standard precision in graphics; doubles tend to only be needed for CAD models at the scale of ships. We don't expect most folks to need MeshGL64, but for those that do, it seems reasonable they may need large numbers of faces too. We don't actually support indices that high yet, but this way we can in the future without a breaking change.

[ofmetal]

We currently can't work with 64 bit indices. And no, MeshGL 32 bit is just 32-bit indices.

size_t is 64-bit for 64-bit binaries. Which makes face indices std::vector triVerts; also 64-bit.

[elalish]

@pca006132 it's a fair point - should we keep MeshGL64 with 32-bit indices? I know we've gone back-and-forth on this a bit.

[pca006132]

By can't work with 64 bit indices, I mean we can't really use it internally yet. The idea of having 64 bit indices is just for supporting it in the future without a breaking change. And I am saying the index type for MeshGL, not MeshGL64, is 32 bit.

@elalish I don't think we have to change it.

[pca006132]

Maybe we should make it clear in the readme though, that we currently do not support meshes that are too large, either in the input or output.

[ofmetal]

doubles tend to only be needed for CAD models at the scale of ships

Floats must be good for realtime graphics, but for CAD-like stuff doubles are mandatory.
I used to work with CGAL. It is has lots of useful functions, but it is not robust when it comes to boolean operations, which fail pretty often. E.g. it can generate "infinite face" and cycle through it infinitely. Or fail in any other way.

So I switched to the Manifold while keeping old CGAL functional in some places. The speed improvement is roughly 3700 times for the booleans, plus robustness.

The memory issues was on my side, I fixed most of them. So this new MeshGL6432 works fine.
Some problems are still there, probably on my side. Will do more debug stuff to localize them.

[pca006132]

As I have said, I don't think you need the MeshGL6432 thing, just use MeshGL64 and things will be fine. This is just an IO type, a different index type doesn't really matter internally.

[ofmetal]

just use MeshGL64 and things will be fine.

In my project (both 64-bit and 32-bit binaries) I use doubles and 32-bit indices. When I do input-output things I can do it fast, directly, without converting 32 to 64 back and forth, e.g.:

		m6432.vertProperties.resize(vertLen*3);
		m6432.triVerts.resize(idxLen*3);
		memcpy(m6432.vertProperties.data(), vertData, vertLen*3*sizeof(double));
		memcpy(m6432.triVerts.data(), idxData, idxLen*3*sizeof(int));

Also, 64-bit indices consumes 2 times as much memory.
Moreover, MeshGL64's indices are actually neither 64 bit, nor 32 bit. They are variable! Dependent on architecture. So for 64-bit code I would have to do up-converting 32-bit indices to 64-bit, while for 32-bit code I could do direct copy.

[elalish]

It's true, but if you look at how much computation goes into our Boolean ops, an extra copy of the output mesh data is a drop in the bucket - likewise regarding memory. Go ahead and do some perf comparisons - I'm pretty sure this will be the least of your worries.

[pca006132]

Wanted to say we don't support 32-bit platform, but just remembered wasm is 32 bit... Anyway, I don't think you will have measurable performance differences using 32-bit indices and 64-bit indices with MeshGL. That said, will fix MeshGL64 to using uint64_t indices, thanks for reminding me this.

[elalish]

Thanks for the pointer on the variable size - @pca006132 just fixed that in #1038.