Reorder cells in Voronoi diagram to match order of MultiPoint inputs.

[dbaston]

This slows down the Voronoi creation process for a MultiPoint by about
25%, so it should probably be made opt-in or more efficient.

[Komzpa]

This slows down the Voronoi creation process for a MultiPoint by about 25%, so it should probably be made opt-in or more efficient.

My guess it should be benchmarked not against old Voronoi but instead against old Voronoi + GEOSIntersects between each input point and output point, or, in case of PostGIS scenario, similar ST_Intersects.

[dr-jts]

This is using the site coordinate present in the userData matched with the input coordinate list, right?

If so, I'm surprised it adds so much time. Any ideas on why that might be?

And what is the size of the input the 25% number comes from? I would expect the matching to be close to linear, so it should be a decreasing proportion of the overall time as the input size grows.

[dbaston]

This is using the site coordinate present in the userData matched with the input coordinate list, right?

Yup

If so, I'm surprised it adds so much time. Any ideas on why that might be?

STL maps are surprisingly slow, I guess because they're so heap-intensive. We saw a huge performance boost from removing a map in this same code. (Tracking QuadEdge visit status)

And what is the size of the input the 25% number comes from?

100,000 points. Runtime from 385ms to 500ms, so actually about 30%.

In 3.7 the runtime is well over 1000ms, so it's not the end of the world. But I don't see a hurry to merge it now; I'd like to try and find something a bit better.

[dr-jts]

STL maps are surprisingly slow, I guess because they're so heap-intensive. We saw a huge performance boost from removing a map in this same code. (Tracking QuadEdge visit status)

Well that's bogus. Is there a TreeMap equivalent, and if so would that be any faster?

In JTS it's possible to attach data to the site points, which means that the point index could be attached. Not sure if that's supported in GEOS?

[dbaston]

I didn't do any better with a tree map.

Yes, I think attaching the index and carrying it through may be the better way to go. Kind of annoying to implement, though.

Nothing in the callgrind profile suggests it should be such a big performance hit -- the profile shows only 4% of runtime spend reordering the cells. But if you skip the reordering, the runtime difference is clear.

[dr-jts]

How about sorting the Voronoi cell coordinates and then using binary search to look them up by the input sites? Does STL have a class supporting binary search?

[dbaston]

It doesn't quite work, because you end up defining a comparator to sort your cells based on their input site, but then you are trying to look up with the input site itself, which doesn't plug into that comparator. Then once you identify the site, you want to pull it into your reordered list, but you can't move it because you'd be leaving a null pointer behind, which would break subsequent binary searches. All of it can be dealt with, just not something I have the time to experiment with immediately.

[abellgithub]

I'm not sure what you're doing here but here's another idea: Make a vector using std::iota(). sort with a comparator that compares Coordinates of the original polys. Do the same thing with voroni polys. Then you have two vectors of indices that show where elements in the voroni set go in the original set (both are sorted by Coordinate, which should correspond 1:1, right?)

Then:

for (i = 0; i < size; ++i)
{
   oidx = ovector[i];
  vidx = vvector[i];
  reorderedPolys[oidx] = std::move(voroniPolys[vidx]);
}

This suffers from having to allocate temp space :(
If I'm understanding, it sure would be nicer just to attach the index of the original poly to the voroni poly, but I guess that's hard? :(