Improve building rings for the adjacent rings cases

[AlenaKazak-TomTom]

Some improvements to how rings are built from ways when building geometries from relations.
The improvements concern the cases when there are adjacent rings, such as in the image below. They are often inner ways in a multipolygon relation.

Ideally 2 rings should be built in such cases, but currently it is indeterminate: sometimes 2 rings are built, sometimes one (outer) ring and extra lines, sometimes non-ring chain(s). This depends on a start point, and on selection of a next way when extending a ring. Both of these are randomly picked up from sets.

The changes are:

• Do not use closed ways to extend a ring. Will fix the case when one of such adjacent rings is represented by a closed way.
• Do not use a way which goes back to the direction of the last way in the ring, when extending the ring. There was a check that the same way is not used twice, but sometimes there are different ways using same nodes.
• If there are multiple ways available for extending the ring, and if one of the ways is included twice, or there are some ways with the same start segment, prioritize using one of those. This ensures that the "common" part of the rings is used first, and so separate rings are built, and not the outer one.

[ThijsdeJong-TomTom]

Hey @sebkur , is there a chance you could take a look at this PR? If there is anything you would like us to change, please let us know!

[sebkur]

Hi, sorry for being unresponsive here. The main problem is that I'm very short on time for this project at the moment and don't have confidence that the new strategy of building polygons is a clear improvement without causing problems with other polygons.

The work on this module has all been done roughly ten years ago, so I don't remember everything from the top of my head. I think I implemented at least a second algorithm for building the polygons that tried to be more robust at the expense of possibly longer runtimes per polygon. I think it is not the default strategy and only available for experimentation, maybe even in one of the different modules.

Anyway, I guess you have come to the problem and the solution from practice and given that you work at TomTom I guess this stems from practice with lots of OSM data. I think the key point for improving confidence would be coming up with test cases. I haven't written any back then, but thinking about this now I think it would probably be best to have a bunch of test cases for different kind of cases:

• really simple polygons, one ring, nothing else
• multiple rings, with holes
• adjacent rings as you have in your example
• overlapping rings

We would need make assertions on the expected result somehow. I don't know maybe asserting some properties like number of points of the resulting polygon, number of polygons in the multipolygon, maybe also the computed area as a way to trigger failing tests in case of collapsing geometry when the algorithm changes.

The test cases should probably be possible to execute offline without relying on data changing on the OSM central database. That would mean, identifying relevant polygon relations in OSM, downloading them including ways and nodes references and storing them in the repository here as XML or PBF, letting the test case use that.

I don't know if you have the time for implementing such a kind of test setup and also adding at least a few test cases? That would be an awesome contribution and let me add this change with confidence.

[ThijsdeJong-TomTom]

@sebkur Thanks for the suggestions, please see #22

[AlenaKazak-TomTom]

We are no longer using the osm4j GeometryBuilder. Feel free to re-open and work on this branch if needed.