georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.54k stars 197 forks source link

Monotone Polygon Subdivision #1018

Closed rmanoka closed 1 year ago

rmanoka commented 1 year ago

This PR provides a pre-processed representation for a polygon (port from my geo-crossings unpublished crate), that allows fast point-in-polygon queries. This implementation is based on these awesome lecture notes by David Mount. The results are promising: the pre-processed queries are faster across the board, about 1.5-2x in the worst-case, to upto 100x in the best cases.

In this PR, we provide:

Benchmarks

We use three types of polygons:

With each polygon class, we generate polygons with different number of vertices, and measure query times for checking if a random point in the bounding box lies in the polygon.

Try out with cargo criterion --bench monotone_subdiv .

Remarks / Caveats

rmanoka commented 1 year ago

Nice work! I'd never heard of this concept before. The algorithm part is kind a bit dense, so I don't feel like I adequately reviewed it.

My use-case was a generalization of polygon rasterization, but I felt it's a useful algorithm to abstract out. I was getting performance comparable to gdal_rasterize for polygons. The algorithm essentially traces along edges, but yeah, the details are a bit dense; I'll try to flesh out some more docs in-code.

For validation, it looks like you're checking that the area of several input polygons is equal to it's monotonic decomposition, is that right?

Yeah, since it's integer arithmetic, we just check equality. We could add Relate based assertions too: all components are within original polygon, all components are interior disjoint.

via a wrapper around a Vec.

Do you think we should introduce something like this so we can hang impls on it?

struct MonotonicPolygons(Vec<MonoPoly>);

As well as saving people from writing boilerplate, it seems like it'd make it more obvious to explain to people how to leverage this functionality, without necessarily having to explain the entire concept behind what a monotonic polygon is.

+ let mono_polys = MonotonicPolygons::from(polygon);
  for candidate in polygons {
-    polygon.intersects(&candidate)
+    mono_polys.intersects(&candidate)
  }

Yes absolutely, this makes sense; the for loop would move into the impl.

That part could always follow in another PR though.

rmanoka commented 1 year ago

Shall we merge this @michaelkirk @frewsxcv ? I've some bool ops ideas on top of this PR, so would be nice to have this in main.

rmanoka commented 1 year ago

Thanks for the reviews! Merging this.

bors r+

bors[bot] commented 1 year ago

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.