search

gumyr / build123d

A python CAD programming library
Apache License 2.0
386 stars 72 forks source link

Avoid a crash in Mesher if triangulation fails #589

Open openvmp opened 3 months ago

openvmp commented 3 months ago

Exactly the same problem exists in CadQuery's tessellate() (see this issue).

I don't know how to reproduce it on a small model. It will be reproducible by the following PartCAD command after the refactoring for OBJ rendering is published (the refactoring is to use build123d instead of CadQuery):

pc render -t obj -a /pub/robotics/multimodal/openvmp/robots/don1:robot
gumyr commented 3 months ago

If a face is skipped isn't the whole tessellation invalid? How does this actually help or is it just intended to be a placeholder for some alternative solution?

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 94.96%. Comparing base (e852e0b) to head (3dbef20).

Files Patch % Lines
src/build123d/mesher.py 33.33% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #589 +/- ## ========================================== - Coverage 94.98% 94.96% -0.03% ========================================== Files 25 25 Lines 7998 8001 +3 ========================================== + Hits 7597 7598 +1 - Misses 401 403 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bernhard-42 commented 3 months ago

I think I have the same in my tessellator:

https://github.com/bernhard-42/ocp-tessellate/blob/3940f04206f207bfdc4e5206590643f00ea984f8/ocp_tessellate/tessellator.py#L229-L234

OCCT returns None for faces without proper polygons

openvmp commented 3 months ago

How does this actually help or is it just intended to be a placeholder for some alternative solution?

It helps to avoid an exception being thrown a few lines further in case the object is None. I don't know if there is anything else we can do with such faces.

gumyr commented 3 months ago

@openvmp would you add some tests to cover this change?

openvmp commented 3 months ago

I was only able to reproduce it while processing that model. It was producing a 1.2GB large OBJ file at that moment and it was very hard to troubleshoot.

I guess it should be relatively simple to reproduce, using a face with all vertices on one line or when two of them coincide. But, unfortunately, I'm not proficient in build123d modeling to be able to reproduce that. I'm serving those using build123d, but I don't know how to use it myself. I only know how to use import/export functions.