Downgrade WARNING: PolySet -> Manifold conversion failed about openscad HOT 31 OPEN

Is there a disadvantage to relying on OpenSCAD to merge duplicate vertices? (The one obvious disadvantage would be that it could in some circumstance produce an incorrect or invalid result.) I think I favor the idea of an informational comprehensible message that says "Duplicate vertices in model were merged" or something to that effect vs just being completely silent about the matter.

from openscad.

As of today, it's not just a question of duplicating vertices. If a non-manifold topology was specified we'll do a bunch of stuff to try to ingest it:

• Quantize vertices to a grid
• Triangulate all polygons
• If the geometry was convex, try to apply a convex hull
• reorient faces which were pointing the wrong way (could also happen due to quantization)

We could split that up into a naive vertex merge first, and if that fails, issue said warning and try to repair.

from openscad.

Are non-triangular polygons considered a fault? This has always been a point of uncertainty for us, as it seems like there are rare cases where non-triangular coplanar polygons fail, but usually they are OK. (I suppose there is the question about how accurately coplanar a non-triangular polygon is.)

from openscad.

Non-triangular polygons are OK, but there's always a chance that a polygon isn't planar. Triangulating ensures that. ..plus Manifold requires triangles.

from openscad.

What happens if you feed it a non-coplanar polygon?

from openscad.

We'll just make a decision for you how to combine the vertices into triangles.

from openscad.

I was worried when @kintel first mentioned the steps that would be taken if the polyhedron was non-manifold and wondered if it would be more efficient to just eliminate the duplicate vertices in BOSL. So I ran a simple test on a project I'm working on right now. I am generating a closed polyhedron via path_sweep() and vnf_polyhedron(), similar to my original example code, except much closer to planar in appearance. Then I merge a number of these into a nxn mesh to create a weave pattern. For now the result is basically planar but eventually this mesh itself will be wrapped around 3d shapes. Anyways, for a few values of n I ran this, accepting the current Manifold warnings, but successful rendering; and I ran it again except inserting vnf_merge_points() around each result of path_sweep() before passing to vnf_polyhedron. Every test was run after flushing caches.

       n       BOSL        Manifold
       6        40s            4.5s
       7        54s            5.5s
       8        70s            7.5s
       9        90s            9.5s

n of 6 means 36 polyhedrons merged, 7 is 49, etc. My finely calibrated eye says there's a linear relation between polyhedron count and time and, more importantly, Manifold appears consistently about 10x faster at eliminating the duplicates (and rendering) than getting BOSL to do the elimination first. BTW, when I eliminate the duplicates with vnf_merge_points the warning messages are gone.

from openscad.

The OpenSCAD language is horrible for efficient implementation of complicated algorithms due to the impossibility to (efficiently) implement data structures like priority queue that are essential to those algorithms. It's always going to be much faster for an operation to happen in OpenSCAD natively than to run in userspace.

The point merge process in BOSL2 attempts to speed the search by organizing the data into a tree. If this works really well then the point merge process will be O(n log n) but if it works less well it can still be O(n^2). But the speed of this operation definitely depends on how the data is distributed.

In your test results above, you list a BOSL2 time and a Manifold time, but I'm not sure I know what the times are exactly. To know how long manifold takes to deal with repeated vertices you'd need a manifold time with and without those repeated vertices. Did you do a test like that somehow? Because manifold is doing other stuff, not only dealing with manifold repair.

from openscad.

Yeah, I agree with all points. My test was really just a basic determination, from the user perspective, of which approach is currently best -- ask BOSL2 to clean up the duplicate vertices or let Manifold. The numbers were so overwhelming that I stopped there. It would be nice though, for the developers as well as users, to optimize the task when possible.

I'm not sure when cacheing comes in to play and how to defeat it in OpenSCAD. If we create a polyhedron with some duplicate vertices, then replicate that polyhedron 100 times with some slight transform, does OpenSCAD optimize (remove duplicate vertices) one copy of the polyhedron then reuse that result for the other 99? Or is the cache only used for identical data from one run to the next?

If cacheing is not a concern, then maybe the simple tests are

1. one polyhedron with duplicates removed by BOSL2 and 99 copies merged (ie, Manifold doesn't have to remove duplicates and the price paid for BOSL2 to do it is small, relative to the overall size of the render)
2. 100 copies merged of polyhedron with duplicates (ie, Manifold has to do the removal 100 times)
3. various ratios of clean-to-dirty, like 50 clean polyhedrons (from one single BOSL2 action) and 50 copies of non-clean, etc

This might provide some insight into how costly the duplicate removal process is in Manifold. It wouldn't answer the question of if that price varies by the number of duplicates per polyhedron or if it's relative constant, based simply on the overall size of the polyhedron. You've already indicated in BOSL2 that that same price does vary based on the specific data, but I'm not sure if that is based on the quantity of duplicates or on how well the order of the vertices fill a tree.

Where I'm heading with this long ramble is maybe there is a price to pay each time Manifold encounters a polyhedron with any duplicates, regardless of the quantity, and so it might behoove BOSL2 to avoid duplicates in certain "easy" cases but not to bother in hard cases. Lots of speculation at this point (that's probably wrong anyways!). Maybe I'll setup a test like this, unless someone tells me it's obviously wrong or meaningless.

from openscad.

Minor clarification: The mentioned repair isn't something done by Manifold, it's something OpenSCAD does to be able to pass valid meshes to Manifold.

Caching isn't properly implemented yet for Manifold. That's something we need to resolve before moving Manifold out of the experimental stage: #4825

from openscad.

With regards to BOSL2 cleaning up duplicate vertices, there are places where it could be done with no significant computational cost---"just" programming effort. The case of closed path_sweep is probably one such example, though there are some subtleties (specifically, if the object twists, the alignment between the vertices is not necessarily a direct mapping, which I think would actually be a major complication). The performance issue with vnf_merge_points arises from relying on a brute force approach that doesn't exploit topology. But it's a big change and a lot of effort to change things to exploit toplogy---the obvious thing would be to track where the boundaries of partial polyhedra are so that when they are merged, the points on the boundary are lined up and merged. This would be faster since it reduces the points that need to be checked to a presumably small boundary set. (In principle you could align one point and then follow the boundary, but that might be hard to implement efficiently.) There is definitely a question of whether any of these things are worth putting effort toward. I don't think we'd want to try to do the boundary tracking until we get real structures.

For testing purposes, you could test path sweeps with closed=true where you invoke vnf_merge_points or don't, and look at preview time difference, which should tell you how long BOSL2 takes to remove duplicate vertices. Then without invoking vnf_merge_points you can compare the render time for closed=true and closed=false. Since closed=false has no duplicate points but closed=true does have duplicate points, this should be a pretty good test of the render time required to remove the duplicate points to prepare the input for manifold.

from openscad.

If have just come across this warning in my threads library. I have two separate polyhedrons that are the starts and ends of the thread and I intersect them with a rotate_extruded polygon to chamfer or truncate them. I don't think there are any duplicated points in my polyhedra. Either operand of the intersection is fine on its own but together they give this warning.

from openscad.

Does the rule still apply that a single polyhedron doesn't invoke the geometry engine? That is, if you take one polyhedron and combine it with a cube do you get the warning?

from openscad.

The ends get unioned with the rest of the thread and that doesn't generate any warnings if I don't intersect them first.

from openscad.

I do get the warning if I union the ends with a cube.

from openscad.

The intersection is inside a render(). If I remove that I don't get the warning. I also only get 5 warnings for 8 threads of different sizes.

from openscad.

This is all in a preview. I never do a render of these, so I suppose the intersection is done by OpenCSG. It is only when I add the render it is done by manifold.

from openscad.

@nophead the warning is most likely a real indication that your mesh isn't a manifold topology and requires some sort of repair or vertex merge. I suggest you open a separate ticket and post the smallest example you can come up with, and we can help identify the root cause.

from openscad.

Wouldn't CGAL barf if I did an intersection with non-manifold polyhedra? Without manifold selected the rendered intersection works fine. The ends of the thread are just a simple sweep.

This is what the intersection looks like.

from openscad.

Well a simple nested for loop tells me there are lots of duplicated vertices, so I will investigate further.

from openscad.

It was a simple case of somebody passing a thread profile with a zero crest. That caused a duplicate point in the profile. Sorry for the noise.

from openscad.

Does the rule still apply that a single polyhedron doesn't invoke the geometry engine? That is, if you take one polyhedron and combine it with a cube do you get the warning?

It's more of an implementation detail. We convert to the geometry backend-specific format only when needed, and it's currently not needed for rendering a single polyhedron. You can force it to do this on the cmd-line for testing: openscad file.scad -o out.png --render=force

from openscad.

It was a simple case of somebody passing a thread profile with a zero crest. That caused a duplicate point in the profile. Sorry for the noise.

If you include the duplicate vertices in the mesh with their own indices, this should now work: The topology is manifold and degenerate triangles caused by duplicate vertex positions are tolerated. If you find odd corner cases feel free to provide an example :)

from openscad.

Do you mean duplicate vertices that have (degenerate) triangles that place them consistently into the mesh? Or do you mean duplicate vertices where the mesh assumes that the duplicate vertices are the same point even though they have different indices?

from openscad.

The first.

from openscad.

Has something changed about how duplicate vertices get treated? I'm not sure if this is related to the issue discussed here or is a separate matter, but someone reported an artifact that arose because two objects were unioned but they had a 1e-15 gap between them and this gap ended up appearing in the final model and lead to a render failure. The same model in the stable version rendered successfully.

from openscad.

If the issue isn't about gaps in a single polyhedron it's worth opening a new ticket.

from openscad.