Crash in STL export after rotate_extrude about openscad

[kintel]

I wonder if we should, generally, write some output sanity checkers for output file formats, and run such checks as part of tests. e.g. like stlsanitytest (https://github.com/openscad/openscad/blob/dc2a0d1c28e41016abf4f91ffbc4c6c707c4f64a/tests/CMakeLists.txt#L991C18-L991C35) but better.

[UBaer21]

Did the crash closes scad without warning/error? And what happens with 3mf export?
I have seen some crashes on 2024.02.04 win when rendering or saving but couldn't reproduce.

[pca006132]

Did you try the latest version? We fixed some cache related bugs recently.

I tested with the latest master on Linux and cannot reproduce your issue.

[kintel]

Try something after Feb 15 where this is fixed: #4990

[curtmcd]

Just now I checked it out from scratch (Feb 20 dc2a0d1), re-built, and the bug is still there.
OpenSCAD prints the assertion failure to stderr then quits.
Export to 3mf does not crash, but reports an error in the console:

Rendering finished.
EXPORT-ERROR: Can't add triangle to 3MF model.
3MF export finished: /home/csm/3D_Printing.git/Math_Objects/Reuleaux/bug.3mf

It could be something in my build environment. However, a checkout from 2023 Dec 30 had the problem, while a checkout from 2022 Apr 27 does not. I may try to do a bisect, although it would be painful and manual because of git submodule hell.

[pca006132]

1. Are your submodules up to date? Try git submodule update --init --recursive.
2. What is your OS version?

I think if you do a bisect, you probably don't have to update the submodules along with the bisect. The API of manifold has been pretty stable.

[kintel]

Also, to eliminate a local issue, you could always try one of the recent dev binaries from https://openscad.org/downloads.html#snapshots

[curtmcd]

The latest AppImage ran fine on Ubuntu 22.04/Jammy. As yet I have no idea why my build might be different, although I do build with EXPERIMENTAL=1.

@pca006132 Perhaps overkill, but to avoid confounding anything, for every test I started from scratch. Removed everything, cloned from github, checked out specific version, inited submodules, and built.

I ended up doing a full 9-stage manual bisect and found the exact commit where it stops working:

2023-12-03 16:41:36 fe2b635 = ok
2023-12-03 17:29:27 de1e7d8 = crash

My tree is sitting there at de1e7d8 so I can run experiments if needed.

Dependency check

$ ./scripts/check-dependencies.sh 
depname           minimum           found             OKness            
qt                5.4               5.15.3            OK                
qscintilla2       2.9               2.11.6            OK                
cgal              5.4               5.4               OK                
gmp               5.0               6.2.1             OK                
mpfr              3.0               4.1.0             OK                
boost             1.56              1.74              OK                
opencsg           1.4.2             1.4.2             OK                
glew              1.5.4             1.7.0             OK                
eigen             3.0               3.4.0             OK                
glib2             2.0               2.72.4            OK                
fontconfig        2.10              2.13.             OK                
freetype2         2.4               2.11.1            OK                
harfbuzz          0.9.19            2.7.4             OK                
libzip            0.10.1            1.7.3             OK                
bison             2.4               3.8.2             OK                
flex              2.5.35            2.6.4             OK                
make              3                 4.3               OK                
double-conversion 2.0.1             2.0.1             OK       

[pca006132]

If we have an extremely sliver/small triangle that differs by less than the resolution of float (23 bits) but different when represented as doubles (52 bits), it will become the same when we export to STL (the assertion is checking double, but we eventually outputs float). I think we removed quantization when exporting STL (at least it is not obvious to me where we do it), not sure if we want to add it back.

Also, @curtmcd maybe you can try this:

--- a/src/geometry/PolySetBuilder.cc
+++ b/src/geometry/PolySetBuilder.cc
@@ -148,7 +148,7 @@ std::unique_ptr<PolySet> PolySetBuilder::build()
   polyset->setConvexity(convexity_);
   polyset->isTriangular = true;
   for (const auto& face : polyset->indices) {
-    if (face.size() > 3) {
+    if (face.size() != 3) {
       polyset->isTriangular = false;
       break;
     }

I think we forgot the case where there the face can be degenerate...

[kintel]

What about faces with >=3 vertices, but some indices are duplicated, causing degenerate triangles or self-intersections? Perhaps the builder should remove those (at least the degenerate ones) while building?

[kintel]

Perhaps we need to start unit-testing some of the core classes, to establish a more formal contract, as it's easy to introduce bugs which gets magically fixed by some downstream feature, perhaps only in some cases.

[pca006132]

True, the builder may get a degenerate triangle, merge vertices together and end up with a triangle with duplicated indices. Manifold can output degenerate triangles, i.e. vertices can have different indices but refers to the same position.

Yeah I think we should write out the assumption of those core classes. Their invariants are pretty unclear now.

[curtmcd]

I tried the (face.size() != 3), but the behavior didn't change.

I tried changing the assert() into a LOG() as follows:

    // Tessellation already eliminated these cases.                                                
    //    assert(p0 != p1 && p0 != p2 && p1 != p2);                                                
    if (p0 == p1 || p0 == p2 || p1 == p2)                                                          
        LOG(message_group::Export_Warning, "Degenerate triangle [%1$.14lg, %2$.14lg, %3$.14lg] [%4\
$.14lg, %5$.14lg, %6$.14lg] [%7$.14lg, %8$.14lg, %9$.14lg]",                                       
            p0.x(), p0.y(), p0.z(), p1.x(), p1.y(), p1.z(), p2.x(), p2.y(), p2.z());

It prevented the crash and here's the output of that. This is for rotate_extrude(angle = 90) square([10, 10]); with Manifold and 32 of 68 triangles are coming out bad. The same set of triangles results with CGAL.

Rendering finished.
EXPORT-WARNING: Degenerate triangle [9.8078528040323, 1.9509032201613, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [10, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [9.2387953251129, 3.8268343236509, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [9.8078528040323, 1.9509032201613, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [8.3146961230255, 5.555702330196, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [9.2387953251129, 3.8268343236509, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [7.0710678118655, 7.0710678118655, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [8.3146961230255, 5.555702330196, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [5.555702330196, 8.3146961230255, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [7.0710678118655, 7.0710678118655, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [3.8268343236509, 9.2387953251129, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [5.555702330196, 8.3146961230255, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [1.9509032201613, 9.8078528040323, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [3.8268343236509, 9.2387953251129, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 10, 10] [0, 0, 10] [0, 0, 10]
EXPORT-WARNING: Degenerate triangle [0, 0, 0] [0, 0, 0] [1.9509032201613, 9.8078528040323, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 10] [0, 0, 0]
EXPORT-WARNING: Degenerate triangle [0, 0, 10] [0, 0, 0] [0, 0, 0]
STL export finished: /home/csm/3D_Printing.git/Math_Objects/Reuleaux/bug.stl

[pca006132]

Ah! Did you build with -DCMAKE_BUILD_TYPE=Release or RelWithDebInfo? I think for non-debug builds assertions are disabled. Yeah this is indeed a bug. I see degenerate triangles in my stl output as well (although it seems that other programs handle them just fine). Btw it doesn't matter which backend you are using: rotate_extrude directly gives you a polyset :).

[curtmcd]

I only built like this: mkdir build && cd build && cmake .. -DEXPERIMENTAL=1 && make
Let me know if I can help further!

[kintel]

I opted for adding a run-time sanity-check assert of PolySet when in debug mode instead. This is pretty aggressive, but will only crash in debug mode, but will catch such issues much earlier in the pipeline.

[kintel]

@curtmcd It took some time to finalize this, but try with latest master.

[curtmcd]

@curtmcd It took some time to finalize this, but try with latest master.

I find that the issue is gone in latest master 23d5bb9. Thank you!