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

Add polygon mesh support #88

Merged
merged 1 commit into from
Jan 16, 2024
Merged

Conversation

rjoomen
Copy link
Contributor

@rjoomen rjoomen commented Jan 8, 2024

No description provided.

@Levi-Armstrong
Copy link
Contributor

Why do you have to use PolygonMesh directly versus using Mesh or ConvexHull type?

@rjoomen
Copy link
Contributor Author

rjoomen commented Jan 9, 2024

I was experimenting with meshing of point clouds (using PCL) and noticed PolygonMesh support was incomplete in an unexpected way. I agree it's not the best format, and also it's not supported in the contact checkers, but at least this makes some of the support for it more complete.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jan 10, 2024

Is this good to go?

@Levi-Armstrong
Copy link
Contributor

I think it is fine that the message interface supports the Polygon Mesh, but it should be converted it to a Mesh which is internally supported. I have code somewhere which will do this.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jan 15, 2024

Is automatic conversion necessary? This PR simply enables communication of polygon meshes, what the user does with the data is up to him (including conversion).
Adding conversion functions to tesseract in general is useful though, but that would be a separate PR.

@Levi-Armstrong
Copy link
Contributor

You have updated the to/from msg function and the Polygon class was never meant to be used directly but to avoid duplication between Mesh and ConvexMesh. Since to/from msg functions have been updated to handle this, it is unclear if this will be caught down stream like in the collision checkers which if they are not then this could cause issues which are not obvious. If this change is to be made I would want to updated the collision checkers to correctly handle this type or throw an exception.

@Levi-Armstrong
Copy link
Contributor

I checked Bullet and FCL which will throw an exception if this geometry type is used, so I am okay with this change.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jan 16, 2024

Thanks for checking!

@rjoomen rjoomen merged commit 89e1401 into tesseract-robotics:master Jan 16, 2024
7 of 8 checks passed
@rjoomen rjoomen deleted the polygon_mesh branch January 17, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants