wolph / numpy-stl

Simple library to make working with STL files (and 3D objects in general) fast and easy.
http://numpy-stl.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
605 stars 103 forks source link

is_closed() gives false positives for simple STLs #198

Closed cgoessni closed 7 months ago

cgoessni commented 1 year ago

When I construct an STL with just two triangles, opposite of each other, is_closed() returns True although the surface is far from being closed.

solid test
facet normal -1 0 0
  outer loop
    vertex 0 0 0
    vertex 0 0 1
    vertex 0 0.5 0.5
  endloop
endfacet
facet normal 1 0 0
  outer loop
    vertex 0.1 0 0
    vertex 0.1 0.5 0.5
    vertex 0.1 0 1
  endloop
endfacet
endsolid test

This STL gives the following result:

Python 3.9.2 (default, Feb 28 2021, 17:03:44) 
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import stl
>>> mesh = stl.mesh.Mesh.from_file("test.stl")
>>> mesh.is_closed()
True

Version of numpy-stl: newest version according to PyPI

wolph commented 1 year ago

The is_closed() function uses very basic heuristics to test whether the mesh is likely to be closed: https://github.com/WoLpH/numpy-stl/blob/cde945e662b7b7e314398abc47ca9de4f6fcba59/stl/base.py#L358

As you can imagine, this is a far cry from a proper detection method and is only a very basic sanity check. A proper detection method would be far more complicated to write and probably much slower :)

I'm open to pull requests for better algorithms though :)

cgoessni commented 1 year ago

The easiest way to check if a surface of triangles is closed would be to check if each edge is defined exactly twice in the STL file, i.e., each triangle has exactly one neighbour attached at each of its three edges. I will think about if and how such a check can be easily implemented.

wwarriner commented 1 year ago

I think this particular problem could be solved in linear time if the mesh is already stored in a face-vertex form. When I write face-vertex, I mean that there is a vertex array whose columns are the X,Y,Z positions and a face array of indices pointing to rows of the vertex array.

Suppose the mesh is in face-vertex (FV) form and that the faces are in an N-by-3 array.

  1. Get a size (3*N)-by-2 array from columns [1,2] and [2,3] and [1,3] from the face array, concatenating them along the first dimension. These are the edges as indices pointing to rows of the vertex array.
  2. Sort along the second dimension so that edge (2,1) becomes edge (1,2), i.e., a canonical form for the edges.
  3. Convert the array to a list of tuples of length 2 so that edges are hashable.
  4. Reduce using a collections.Counter and ensure that every entry has count 2.

There may be some clever way to do this purely with numpy, but I'm not a numpy expert by any stretch.

At any rate, I'm not familiar with numpy-stl yet, but if there is a facility for converting to face-vertex form, then the above could be implemented. If not, the face-vertex facility could be implemented with some clever array slicing and call to unique() (or a tolerance-based version of unique, if you want to close tiny holes along the way).

github-actions[bot] commented 8 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

wolph commented 7 months ago

The newer version written by @jb-leger has been released :)

JasperBoogers commented 7 months ago

when using get_mass_properties(), self.check() is called in line 422 without the "exact" argument, so the new calculation method is not called and an error is thrown

wolph commented 7 months ago

It's only a warning, but it would be nicer not to include it :)

Thanks for the heads up @JasperBoogers, the new release is online