jhasse / poly2tri

2D constrained Delaunay triangulation library
BSD 3-Clause "New" or "Revised" License
429 stars 89 forks source link

[Idea] Add span support #50

Open pr0g opened 1 year ago

pr0g commented 1 year ago

This is a speculative/demonstrative PR to show how the poly2tri API could be potentially improved upon by accepting a span of p2t::Point instead of a std::vector<p2t::Point*>.

In this case, the span implementation is taken from the Guidelines Support Library (GSL) from Microsoft, but it could also use std::span if poly2tri was updated to use C++ 20.

In my particular use case, I needed to create a vector of p2t::Point types, and an additional 'view' vector of p2t::Point*. By using a span, the semantics of using a vector of pointers remains the same, but you don't need to allocate a new container of pointers, you can just use the original one. You also can avoid having to new each point (as is done in the testbed example and unittest project).

I realize this is a breaking API change and it's possible there are some downsides I'm not considering, but for my particular use case, it made things a lot cleaner and simpler (and more efficient).

I'd definitely be interested to hear what people think.

Thanks!

pr0g commented 1 year ago

I believe the reason the build failed for this before was the minimum required version of CMake (3.12) did not support FetchContent. I've bumped this to 3.15 and I think it should now work correctly (but again merging this as-is likely isn't a good idea, I was just interested if this alternative API might be useful in certain situations). Thanks!