Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault on rendering #2847

Closed
plut opened this issue Mar 5, 2019 · 16 comments
Closed

Segmentation fault on rendering #2847

plut opened this issue Mar 5, 2019 · 16 comments
Labels
Bug: Crash Ext: CGAL Related to upstream CGAL bug/issue Type: Bug

Comments

@plut
Copy link

plut commented Mar 5, 2019

openscad 2019.02.07, libcgal13 4.13-1+b2 (using the Debian repositories mentioned in http://www.openscad.org/downloads.html).

Running on the following file (which is a stupid model, granted) reliably produces a segfault when rendering, either in the GUI or in command-line via openscad bug.txt -o bug.stl.

bug.txt

Since the model is correctly previewed, might this be a CGAL bug instead?

@t-paul
Copy link
Member

t-paul commented Mar 5, 2019

The polyhedron is invalid (some faces are pointing in the wrong direction - see this FAQ entry). It should never crash OpenSCAD though.

@kintel
Copy link
Member

kintel commented Mar 7, 2019

I can reproduce on macOS using CGAL 4.11. It works (reports error instead of crashing) with CGAL 4.8.1.

This was tested using old dev snapshots of OpenSCAD, so there's theoretically a possibility that we have code changes influencing this, but the stack trace points quite strongly to an uncaught (uncatchable?) issue inside CGAL.

@kintel
Copy link
Member

kintel commented Mar 7, 2019

I suspect this to be the culprit: Someone commented out an assertion check in CGAL:

CGAL/cgal@f1f8278#diff-2d25c6c5eefcde7a443ef7269d33ee5eL210

@kintel
Copy link
Member

kintel commented Mar 7, 2019

If someone can organize an upstream bug report that would be awesome

@kintel kintel added the Ext: CGAL Related to upstream CGAL bug/issue label Mar 7, 2019
@adrianVmariano
Copy link

I found that this code

for (a=[0:120:240])
   rotate(a)
      polyhedron(points = [[0,.0911,0],[1,0,0],[0,-.0911,0],[0,0,.1]],
          faces = [[0,1,2], [1,2,3], [0,1,3], [0,3,2]]);

crashes 2019.01-RC3 under Windows when I attempt to render. The first face of the polyhedron is backwards. If I fix that it doesn't crash any more. Is this the same bug?

@MichaelAtOz
Copy link
Member

Did anyone 'organize an upstream bug report'?

@thehans
Copy link
Member

thehans commented Dec 16, 2019

I have now ^^^

Did anyone 'organize an upstream bug report'?

@thehans
Copy link
Member

thehans commented Dec 16, 2019

Well, that didn't quite go as planned. Already closed and completely ignored the issue of commented-out assertion which I described as carefully as I could.

They did mention possibly calling orient_polygon_soup() which maybe we can use to auto-fix some bad inputs like this.

Still would be nice to have catchable exceptions over crashes. I might try replying later when I've cooled off... not happy about how that was handled.

@MichaelAtOz

This comment has been minimized.

@thehans

This comment has been minimized.

@MichaelAtOz

This comment has been minimized.

@MichaelAtOz

This comment has been minimized.

@thehans
Copy link
Member

thehans commented Dec 24, 2019

Please keep comments on-topic to the original test cases. Its getting very hard to keep track of these mixed up issues.

The OP's case doesn't crash on 4.7 (nor 4.8 according to kintel), and it does crash on 4.11, regardless if built from 2019.05 tag or latest master.

@MichaelAtOz
Copy link
Member

MichaelAtOz commented Dec 24, 2019

For completeness, 2019.11.24.nightly on Debian has:
CGAL version, kernels: 4.9, Cartesian, Extended_cartesian, Epeck

With the OP scad it gets

ERROR: CGAL error in CGAL_Nef_polyhedron3(): CGAL ERROR: assertion violation! Expr: pe != 0 File: /usr/include/CGAL/Nef_3/polyhedron_3_to_nef_3.h Line: 212

With adrianVmariano's it doesn't get an error and renders something like a 3 point star.

@thehans
Copy link
Member

thehans commented Dec 10, 2020

I realized there were already some checks for whether a mesh is closed, and valid, but they were only used in debug messages.
I made a PR #3526, which stops trying to convert to Nef if either of those checks return false.

There is still some room for improvement, but at least I think this will avoid segfaults.

  • Error messages don't have access to source Location info. Since its converting from a Polyset, maybe we should have Location info stored in each of those upon creation?
  • CGAL's orient_polygon_soup might still be able to correct for some of these class of errors, but I was somewhat confused on how to best implement it. Our current code converts Polyset to Polyhedron_3, and from there to Nef. But Polyhedron_3 doesn't appear to have direct access to underlying points and faces that orient_polygon_soup would need.

@t-paul
Copy link
Member

t-paul commented Dec 18, 2020

Fixed via #3526.

@t-paul t-paul closed this as completed Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Crash Ext: CGAL Related to upstream CGAL bug/issue Type: Bug
Projects
None yet
Development

No branches or pull requests

6 participants