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

Basic AABB support; in_element; (req C++17) #182

Merged
merged 5 commits into from
Feb 14, 2023
Merged

Conversation

alecjacobson
Copy link
Contributor

Fixes #150

As much as I'd like to have as close as possible match in python binding names to igl function names, I'm not sure what to do about igl::AABB which is templated on int DIM for the dimension and dependent functions like igl::in_element which take an AABB instance as an argument. In this PR, I introduce two wrappers in_element_2 and in_element_3 which correspond to in_element called with a 2D and 3D AABB class respectively.

In turn, I've also introduced the AABB class, which (like all bound classes?) only works for Eigen::MatrixXd and Eigen::MatrixXi inputs. Despite this inelegance, squared_distance member function is, I think, a good example of how to use std::variant to achieve pythonic variable return types.

(For now, the bindings match igl::in_element, in that you can't call it without first precomputing an AABB tree. The mothership probably ought to have a version that hides constructing the AABB tree and then the python bindings should mirror it with a corresponding wrapper.

@alecjacobson
Copy link
Contributor Author

This looks good to go. Just a heads up that this is adding C++17. I explored trying to use old-school union instead of std::variant and it got ugly, so I'm not sure it's worth it. The cost of using C++17 is no longer supporting macs before 10.13 (released in 2017). I'm fairly confident this will not affect our users, but if others feel differently please speak up.

@alecjacobson alecjacobson merged commit 5f53a3a into main Feb 14, 2023
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.

Inverse of parametrization
1 participant