Replace geos with boost.geometry?

[nilsnolde]

was just looking at admin building and realized that geos seems to only be used in https://github.com/valhalla/valhalla/blob/master/src/mjolnir/adminbuilder.cc.

quickly skimming the code I don't think I saw stuff that's not possible with boost.geometry.. it's mostly polygon-in-polygon checks (I think) and while bg does not yet support all kinds of geom types for within checks, (multi)polygons should work well: https://www.boost.org/doc/libs/1_77_0/libs/geometry/doc/html/geometry/reference/algorithms/within/within_2.html.

in case we eventually get to choose our own boost version (hopefully close to latest) after finishing #3346 , it might be little effort to ditch geos in favor of bg? one more linked lib gone would be awesome (esp such a troublesome one for valhalla, using the c++ interface)

[nilsnolde]

so the recent PR was a complete failure haha

I was too hurried and naive, I totally neglected that ways for admins are of course not ordered. I wondered why all this geometry massaging was going on, but it dawned on me not before long.. geos has LineMerger::getMergedLineStrings() which takes care of ordering the ways properly, boost doesn't have that (also no buffer allowing 0 :roll_eyes: ) . also while reviewing master output there a few problems IMO:

• empires like france, uk, netherlands are not handled very consistently.. I know it's messed up in osm, but what we do:
  • in osm, france empire is level 2 with all territories included. so what admin.lua does is ditch the french empire and promote most (1 missing I think) territories with level 3 to level 2. fair enough IMO
  • netherlands is not sub-divided. e.g. sint maarten is netherlands in our db: https://www.openstreetmap.org/relation/1231790
  • uk is level 2 and england, scotland etc are level 4 in our db (and osm)
  • one thing LineMerger::getMergedLineStrings() doesn't seem to do, is correctly identify inner rings. it seems to just union polygons, which swallows "inner rings" like in italy with vatican state and san marino. the italy polygon doesn't have any inner rings at least, so I just assume that's what's happening

my suggestion for a PR now would be:

• I look at greater detail at the admins again and PR the ones that changed, there are a few
• keep france the way it is
• change netherlands to behave like france, i.e. promote the territories to countries. EDIT: netherlands is too messed up in osm. only sint maarten is level 3, bonaire is part of netherlands mainland on level 3 and the other two form another relation with mainland and sint maarten (but not bonaire) called "dutch carribean", jeez what a mess..
• remove UK and promote the constituents to level 2. maybe england could get its level 5 to level 4 promoted as well? e.g. https://www.openstreetmap.org/relation/151304, the other "countries" are more fine-grained in next sub-level

also not sure if or how I should continue to switch to boost geometry to remove the geos dependency. stitching proper multipolygons with inner rings out of the randomly ordered osm ways is not so hard, but a bit more code with boost geometry (would use an rtree of way bboxes here for each admin to iteratively build up a multipolygon, then handle inner rings). bit more work, so will take a little longer.

any thoughts @gknisely @kevinkreiser @dnesbitt61 ?

[gknisely]

so the recent PR was a complete failure haha

I was too hurried and naive, I totally neglected that ways for admins are of course not ordered. I wondered why all this geometry massaging was going on, but it dawned on me not before long.. geos has LineMerger::getMergedLineStrings() which takes care of ordering the ways properly, boost doesn't have that (also no buffer allowing 0 roll_eyes ) . also while reviewing master output there a few problems IMO:

• empires like france, uk, netherlands are not handled very consistently.. I know it's messed up in osm, but what we do:

  • in osm, france empire is level 2 with all territories included. so what admin.lua does is ditch the french empire and promote most (1 missing I think) territories with level 3 to level 2. fair enough IMO
  • netherlands is not sub-divided. e.g. sint maarten is netherlands in our db: https://www.openstreetmap.org/relation/1231790
  • uk is level 2 and england, scotland etc are level 4 in our db (and osm)
  • one thing LineMerger::getMergedLineStrings() doesn't seem to do, is correctly identify inner rings. it seems to just union polygons, which swallows "inner rings" like in italy with vatican state and san marino. the italy polygon doesn't have any inner rings at least, so I just assume that's what's happening

The above is all correct. It took a bit to get this all working. As for the Netherlands, I did not know it needed to be like France or UK. I think I would try to make it behave like France.