delaunator-sharp icon indicating copy to clipboard operation
delaunator-sharp copied to clipboard

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

Open BlackxSnow opened this issue 3 years ago • 1 comments

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.

BlackxSnow avatar Jan 16 '22 13:01 BlackxSnow

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

nol1fe avatar Jan 17 '22 13:01 nol1fe