<kintel>
pca006132 thx - I'm sure this has opened up for making code more elegant in general, but I didn't take it very far - mostly focused on making Geometry pointers smarter
<pca006132>
btw updated the unity build PR, want to see if it can make CI faster and speed up our workflow
<kintel>
Memory usage may be the main limiting factor on the CI though
<kintel>
Anyway, bedtime..
kintel has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
<pca006132>
linux build is 10mins faster on github ci, macos one is 5mins faster (macos one is dominated by homebrew install time, maybe we can cache that)
<pca006132>
windows build somehow failed, will investigate
<kintel>
Not sure why homebrew is slow - it should generally install binary packages. Perhaps just not optimized/limited CDN etc
<kintel>
pca006132 Thanks for the review! As you can see, we forward-declare a lot of stuff for old compile-time optimization reasons. I don't know how much of that is needed any longer
<kintel>
It was mostly a way to battle CGAL'
<kintel>
..CGAL's insane use of templates
<pca006132>
yeah I was not aware of that
<pca006132>
indeed, CGAL is really slow to compile...
<kintel>
Also, const correctness was largely retrofitted into some new code, so it's a good reminder to always second guess every interface we use
<pca006132>
unity build can also improve this because those specialization will happen less
<pca006132>
ok
<kintel>
const is especially tricky if you look at GeometryEvaluator::ResultObject
<pca006132>
not sure how CGAL does it, but for manifold we only expose an immutable interface to users to make things simpler
<kintel>
..which is another suboptimal approach to avoiding inserting too many single-use object into the cache
<kintel>
oh, another candidate for PolySet/Geometry refactoring: Allow decoupling the transformation from the geometry and only resolve concrete geometry lazily. Not too clear if it's worth it though..
<pca006132>
btw, as we already have PolySet as indexed mesh, should we now remove the IndexedMesh.cc?
<pca006132>
from our experience in manifold, it is worth it
<kintel>
We could kill IndexedMesh I think
<kintel>
perhaps even ExportMesh?
<pca006132>
but if the plan is to mostly use manifold, I think doing optimization for the CGAL stuff is kind of a waste of time
<pca006132>
yeah, I think ExportMesh is only used because of the predictable output thing
<pca006132>
it has some logic to sort the vertices and faces
<kintel>
yeah, but those could be free functions on PolySet
<pca006132>
yes
<kintel>
also, with a decoupled transformation, polyset geometry could be made immutable I think
<kintel>
anyway, there's a fine balance between refactoring for cleanliness and refactoring to support actual new features, but happy to support such activities as long as people have the energy
<kintel>
I like the zen aspects of refactoring :)
<pca006132>
in my understanding, I treat polyset as some sort of glue between CGAL and manifold, and also a uniform interface for IO (so there is no dependency on CGAL and manifold for IO)
<kintel>
..and rendering bridge
<pca006132>
ah yes
<pca006132>
so I think it is rarely transformed, but idk
<kintel>
rotate(45) cube()
<pca006132>
ah, constructor makes PolySet as well
<pca006132>
forgot about this
<kintel>
not only transformed, but also modified in-place to avoid caching cube() separately
<kintel>
We could discuss creating manifold objects directly, but that requires some more careful consideration
<pca006132>
yeah, the constructors in manifold behave quite differently from the ones in openscad
<kintel>
also, the renderer converts manifold objects to polyset to VBO
<kintel>
..we may be able to do manifold->VBO more efficiently
<pca006132>
manifold provides a MeshGL object that is flattened, and can be used for rendering easily
<kintel>
ah, nice. Something to think about post-VBO merge
<kintel>
I wasn't referring to geometry constructors, just about building object directly as ManifoldGeometry
<pca006132>
should be simple
<pca006132>
the MeshGL type provided by manifold is quite similar to PolySet, except that it is flattened
<pca006132>
we also have a Mesh object that is basically PolySet with additional fields, but we plan to deprecate it, so use MeshGL would be better
<pca006132>
(there will be a small cost of converting MeshGL into Manifold, due to topology tests)
<pca006132>
I put the ExportMesh to PolySet task into PolySet cleanup
pca006132 has quit [Remote host closed the connection]
kintel has quit [Quit: My MacBook has gone to sleep. ZZZzzz…]
greenbigfrog has quit [Remote host closed the connection]
greenbigfrog has joined #openscad
fling has quit [Ping timeout: 240 seconds]
fling has joined #openscad
pca006132 has joined #openscad
snaked has quit [Remote host closed the connection]
<pca006132>
I was thinking about the tessellation code, I don't think the current approach can preserve manifoldness...
<pca006132>
I think a better approach would be to use a triangulator that is robust w.r.t. degenerates, and remove degenerates after converting it in a lossless manner to a triangular mesh
<pca006132>
the degenerate removal should preserve topological correctness
<pca006132>
(which is what we do in manifold after each boolean operation, because those operations tend to generate degenerate triangles)
<pca006132>
As an example, consider the touching cube example (again...) but represented as polygonal (quads) mesh. We can represent it as a manifold but self-intersecting mesh, and this manifoldness will be broken if we pass it into the current tessellator.
lostapathy has joined #openscad
<pca006132>
btw I found that when we convert Nef Polyhedron to PolySet, we already triangulate each faces... what is the point of supporting polygon in PolySet anyway? we can just convert our constructors into outputting triangles instead of quads (e.g. sphere)
<guso78k>
pca006132 how can i fix this error with small_vector ?
<pca006132>
maybe should send an email to the mailing list as well, if someone relies on fastcsg and cannot switch to manifold
<guso78k>
pca006132 found the issue. instead of std::vector<int> i should strictly use IndexedFace
<pca006132>
yes, that also works
<pca006132>
the idea of that small_vector is to avoid allocation when you only have 3 points
<pca006132>
so it will not allocate many vectors when you have a triangular mesh
teepee has quit [Remote host closed the connection]
teepee has joined #openscad
teepee has quit [Remote host closed the connection]
teepee has joined #openscad
erectus has quit [Remote host closed the connection]
erectus has joined #openscad
L29Ah has left #openscad [#openscad]
<InPhase>
pca006132: A bold proposal, but that might be sensible. We have two major experimental systems to radically improve performance contributed by the same author, and Manifold appears to be the clear winner.
<InPhase>
Simplifying the number of render paths before rollout in release form could be very advantageous for future maintenance, as long as this gives us no discernible losses.
<InPhase>
Or, no significant ones.
<pca006132>
and it will be helpful for the refactoring work we are doing right now
<pca006132>
there are many files related to fastcsg
<teepee>
if anyone depends on fast-csg as user, they are on their own. I have always tried to convey the message that "experimental-feature" means exactly that. you can't rely on it to be in any next version and/or rely on it staying unchanged
<teepee>
that's why I've always opposed means giving users access to the feature information inside scripts making it even more likely people ignoring the warning
<pca006132>
true
<pca006132>
but anyway it would be very helpful if anyone can find cases in which manifold crashed but fastcsg/Nef works fine
<teepee>
I had a case some time ago with an example from nophead on the mailing list
<teepee>
which was surprising as it was not that unusual code, I think it was minkowski, so maybe fixed with the latest changes
<teepee>
let me see if I can find that again
<teepee>
IIRC it failed with both fast-csg and manifold producing empty output where it worked fine with NEF
<pca006132_>
the bug is not really due to minkowski, but due to manifold unable to handle objects that large/small
<pca006132_>
and by large/small, I mean the magnitude of their coordinates, as this is a limitation in the representation and precision assumption
<pca006132_>
we planned to enable double precision, but that takes some time, will cause slight performance degradation, and cannot solve the issue in general (will just accept a wider range of magnitude, but you can still get to the limit)
pca006132_ has quit [Client Quit]
L29Ah has quit [Ping timeout: 255 seconds]
<teepee>
oh, interesting, I was not expecting that for "normal" models to be critical
<kintel>
pca006132 In terms of polyset supporting polygons: In the past we wanted to allow exporting polygon, especially quads, but that may not be such a strong wish any longer
<kintel>
..and there were some use-cases related to rendering nice polygon outlines
<kintel>
..but with Manifold not preserving polygons anyway, we might end up tidying that up a bit
<kintel>
..or make TriangleSet and PolySet separate classes to help isolate those use-cases
<J23k19>
that also happened with your circle .. i think you missed that the second radius rotation didn't finish a full rotation
<J23k19>
you can use PI as constant
<J23k19>
But you should not use PI .. you need 3 for 3 lobes
<J23k19>
so a circle(1) will roll in a circle(3) 3× .. you used 3.14 which is why you have that mismatch linext
<InPhase>
OpenSCAD is in degrees also.
<InPhase>
That entire (out_radius+in_radius)/in_radius should be swapped with 3. It's not at all a radius there, it's a repetition count.
* J23k19
was confused by that too
<InPhase>
linext: If that 3-leaf-clover effect is what you want, then you probably want out_radius*cos(theta)-(out_radius-in_radius)*abs(cos((3/2)*theta))