nol1fe / delaunator-sharp

Fast Delaunay triangulation of 2D points implemented in C#.
MIT License
418 stars 54 forks source link

GetVoronoiEdges() defaults to non-standard definition of voronoi using centroids rather than circumcenters #17

Closed BlackxSnow closed 1 year ago

BlackxSnow commented 2 years ago

https://github.com/nol1fe/delaunator-sharp/blob/f9e28833b4369a73cc2dcc50ce9ebee99b6227f2/DelaunatorSharp/Delaunator.cs#L541

Expected default behaviour of voronoi functions is circumcenter based voronoi. Defaulting to centroids makes the algorithm appear broken.

It's also notable that since ForEachVoronoiEdge ignores the optional parameter, that function also (by proxy) uses centroids, making the function somewhat useless in its current state unless you're using centroid voronoi for whatever reason.

Edit: Going through old issues I see #5 though I'd argue that providing centroid based voronoi is not useful as it's neither voronoi nor an accurate way to approach centroidal voronoi, though my understanding of voronoi may be limited.

nol1fe commented 2 years ago

Hello

Yeah, ForEachVoronoiEdge should also have it's own equivalent of Centroid/Circumcenters methods same as ForEachVoronoiCell or it should just accept selector, so Delaunator API would be consistent in this case.

Feel free to create PullRequest, or just use standard

foreach (var edge in GetVoronoiEdgesBasedOnCircumCenter())

or

foreach (var edge in GetVoronoiEdgesBasedOnCentroids())

or

foreach (var edge in GetVoronoiEdges(Func<int, IPoint> yourOwnSelector))

As I already mentioned in #5, at current stage of package I won't change defaults so other people won't have any issues.

Cheers