Conversation
jdumas
left a comment
There was a problem hiding this comment.
I'm not sure it's a good idea to introduce a new function using the std::string flags, if we know we want to move away from that in the future. trianglelite uses a simple config struct, which would be less error prone and easier to use. We could have a utility function Config::from_string() that converts a triangle cli switch to a struct filled with the appropriate content to facilitate code update...
| // in_domain = false. start should not yet be visited. | ||
| const auto remove_cc = [&](Face_handle start) | ||
| { | ||
| std::list<Face_handle> queue; |
There was a problem hiding this comment.
I wouldn't use a std::list for that. std::queue under the hood uses a std::dequeue, which should offer better performances.
|
Agreed. That's what I'm referring to as future work. For now, it's good to have the same API. |
|
"future work" just sound like an easy way for the task to be added to the backlog and never be completed. Why not do it as part of this PR? |
|
If you're offering, please push it onto this PR.
… |
|
Sure, I can add it to my own backlog. But let's hold on merging this in the meantime. The last thing I want is to introduce more broken windows for the sake of expediency. |
special template for windows
…nto cgal-triangulate
| // Insert all edges | ||
| for(Eigen::Index e = 0;e<E.rows();e++) | ||
| { | ||
| cdt.insert_constraint(points[E(e,0)],points[E(e,1)]); |
There was a problem hiding this comment.
You may want to use this function to get better performances. As far as I see, you can use boost::transform_iterator together with a lambda to create pairs on the fly.
Future work is to implement more of the triangle api and to overload a version that doesn't use the triangle string flags (and keep this version to call it for easy switching).
Checklist