Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Maximum Inscribed Circle #311

Closed
wants to merge 57 commits into from
Closed

Add Maximum Inscribed Circle #311

wants to merge 57 commits into from

Conversation

PieceMaker
Copy link

@PieceMaker PieceMaker commented Apr 27, 2020 •
edited
Loading

A Voronoi-based maximum inscribed circle algorithm based on the JavaScript library max-inscribed-circle.

OSGeo ticket https://trac.osgeo.org/geos/ticket/1028.

@PieceMaker
Start adding structure based on MinimumBoundingCircle.
73a9e4d
@PieceMaker
Add separate example file that currently outputs ring coords.
7d6cf32
@PieceMaker
Change some types and adjust some formatting.
80153ab
@PieceMaker
Added missing namespaces.
e000e74
@PieceMaker
Simplify polygons as they don't need to be so big.
9aedb27
@PieceMaker
Add max inscribed circle to make file so we can start compiling.
1525c8a
@PieceMaker
Somehow commit the output.
ae5cc9a
@PieceMaker
Reorganize so compute method tries to replicate one from MBC.
7755bcc
@PieceMaker
Rework to intake a geometry instead of poly. Also change segment type.
0dd16f1
@PieceMaker
Add functionality to pull polygon of largest area from multipoly.
c68406c
@PieceMaker
Add function for printing coordinates.
c9037f2
@PieceMaker
Added checking helpers, cout statements, and fixed incorrect types.
abe4caf
@PieceMaker
Was double-pushing from-points.
1e0f3ae
@PieceMaker
Change where sites are instantiated.
df5cd86
@PieceMaker
Change for loop variable type back to unsigned int.
84be76f
@PieceMaker
Was accidentally n-secting start point with (0,0). This removed dupli…
d28c50d 
…cates.
@PieceMaker
Switch to if-else chain instead of separate if statements.
b63af09
@PieceMaker
Add temporary method that will return generated voronoi vertices.
892def6
@PieceMaker
Returns were short-circuiting the rest of the calculation.
1ab74a0
@PieceMaker
Get the coordinate sequence factory from the geometry factory.
2276e21
@PieceMaker
Pass entire geometry to unique extractor instead of looping.
316cccb
@PieceMaker
Return array of points since points are used for intersection check.
54bdc25
@PieceMaker
Rework example to expect array of points instead of coordinates.
60b2cdb
@PieceMaker
Remove some extra cout statements.
9a87fbd
@PieceMaker
Filter vertices so only those that intersect input geom are returned.
86e2680
@PieceMaker
Make polygon a member instead of passing it to functions.
98309ad
@PieceMaker
Add final piece that determines center and radius of MIC estimate.
35cf00a
@PieceMaker
Remove sites as member since voronoi takes ownership and modifies it.
fa8ffc8
@PieceMaker
Remove namespace since I have using.
294acdb
@PieceMaker
Remove unused include.
d570204
@PieceMaker
Use explicit type in for-loop instead of auto.
9934d5e
@PieceMaker
Accidentally redefined poly in the case of multipolygon.
ae4e153
@PieceMaker
Fix compiler warnings.
96c9a62
@PieceMaker
Add unit test structure with two tests. More to come.
78c13b7
@PieceMaker
Add more tests and fix bug in checking centers.
a7477c6
@PieceMaker
Add tests for increasing number of segments.
9ed569a
@PieceMaker
Make sure the returned center intersects the geometry.
ef00d6c
@PieceMaker
Formatting.
3d1ce88
@PieceMaker
Switch spelling from "center" to "centre".
42c844b
@PieceMaker
Fix line endings as they're causing a test breakage.
5718bf6
@PieceMaker
Remove some unnecessary cout statements used for development.
f1b3f4d
@PieceMaker
Add maximum inscribed to capi and add structure for tests with one test.
6cfc705
@PieceMaker
Fix segfault bug with empty polygons found in testing.
5c62ee8
@PieceMaker
Add test for degenerate polygon.
7a854bc
@PieceMaker
Also add a line string test.
3a33cab
@PieceMaker
Add more tests of capi maximum inscribed circle.
6369e8e
@PieceMaker
Remove temporary helper functions.
179cc4d
@PieceMaker
Replace geometry type if-chain with switch statement.
877a2b0
@PieceMaker
Fixed a couple of test comments.
bfae261
@PieceMaker
Pull factory from inputed geometry.
1061ff7
@PieceMaker
Remove TODOs.
ec3a4dc
@PieceMaker
Update docs.
05ecc6d
@PieceMaker
Remove my personal example.
3732abe
@nyalldawson
Copy link

Did you see #310 ?

@PieceMaker
Copy link
Author

Wow, I did not see that. Beat me by a few days...

@dr-jts
Copy link
Contributor

dr-jts commented Apr 27, 2020 •
edited
Loading

Also see this JTS blog post for more background.

My guess is that the iterative approach in #310 will be faster than the densified Voronoi approach. Would be interesting if you can compare them.

Also note that I plan to add a "viewport constraint" as well, to be able to find the visual centre of. a polygon which is only partially visible in a rectangular viewport.

And finally, others have experimented with this for label placement, and have actually decided that in itself MIC is not ideal for labelling. See some discussion in the Mapnik project for instance [1][2].

[1] mapnik/mapnik#3780
[2] mapnik/mapnik#3811

@dr-jts
Copy link
Contributor

dr-jts commented Apr 27, 2020

This enhancement to weight using centroid might also be worthwhile for labelling purposes:

Of course this is no longer the MIC then.

@PieceMaker
Copy link
Author

@dr-jts Thank you very much for the links. That info is very insightful. I'll see if I can write a comparison once the iterative method is merged in.

I will go ahead and close this PR.

@PieceMaker PieceMaker closed this Apr 27, 2020
@pramsey pramsey mentioned this pull request Nov 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PieceMaker @nyalldawson @dr-jts