compas-dev / compas_cgal

COMPAS package for working with CGAL.
https://compas.dev/compas_cgal
GNU Lesser General Public License v3.0
23 stars 5 forks source link

added from_mesh class method #10

Closed nizartaha closed 3 years ago

nizartaha commented 3 years ago

method to convert compas meshes to trimeshes

gonzalocasas commented 3 years ago

@nizartaha could you fix the linter issues, pls? (you can see the error if you click on the "Detail" link of the failed build check, but here's a screenshot anyway:

image

tomvanmele commented 3 years ago

i just want to point out that there is nothing in the method that guarantees that the mesh will actually be a trimesh. this is also not the case for the base constructor and could perhaps be added there as well.

also, i once coded this thing because i thought it would make things faster but that doesn't seem to be the case in most of my experiments...

tomvanmele commented 3 years ago

i also noticed that i didn't add any docstrings to the base implementation. that shouldn't stop you though :)

nizartaha commented 3 years ago

thats correct @brgcode . in fact, i tried it on several meshes and i got some errors because some of them had quad faces. now, i use mesh_quads_to_triangles() from compas.datastructures before i pass the mesh to it and it seems that it works. but i don't know where it could be implemented as a check (base constructor as you suggested)

tomvanmele commented 3 years ago

perhaps something like this in the base constructor?

if any(len(face) != 3 for face in faces):
    raise Exception('The input doesn't represent a triangle mesh.')

i would not try to convert on the fly but leave it up to the user to correct the input...

tomvanmele commented 3 years ago

or it could be part of faces.setter. there is probably a way to pre-assign the allowable size to a numpy array such that it fails when the wrong size input is used...

tomvanmele commented 3 years ago

but perhaps this is stuff for a separate PR...