build123d icon indicating copy to clipboard operation
build123d copied to clipboard

Avoid a crash in Mesher if triangulation fails

Open openvmp opened this issue 1 year ago • 6 comments

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

openvmp avatar Mar 16 '24 21:03 openvmp

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?

gumyr avatar Mar 17 '24 13:03 gumyr

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
@@            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.

codecov[bot] avatar Mar 17 '24 13:03 codecov[bot]

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

bernhard-42 avatar Mar 17 '24 14:03 bernhard-42

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.

openvmp avatar Mar 17 '24 17:03 openvmp

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

gumyr avatar Mar 22 '24 13:03 gumyr

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.

openvmp avatar Apr 02 '24 03:04 openvmp