[PATCH v1] Remove LineClipper artefacts to avoid flooding
The patch removes artefacts from the LineClipper that might cause floodings. WanMil
On Sun, May 22, 2011 at 09:50:55PM +0200, WanMil wrote:
The patch removes artefacts from the LineClipper that might cause floodings.
I did not notice anything unusual with this patch. But then again, I am only using extend-sea-sectors for a tiny piece of missing coastline in the north (Tornio/Haparanda). Last weekend, I paid attention to one artifact, the opposite of flooding, on the Garmin Edge 705 screen when zooming out from a lake when the map detail level is set fairly low. The problem could be in the coarser zoom levels (resolution lower than 24). The problem is that the island http://www.openstreetmap.org/browse/way/27671579 is connected to the mainland in the north. That is, the water north of the island is turned into land. Best regards, Marko
I came to the conclusion that the first patch only tries to covers a shortcoming of the LineClipper. I started a small investigation and observed the following problems of the LineClipper: * Lines are split when two subsequent coordinates are equal * In case a line is closed it's quite probable that two unconnected lines are returned by the LineClipper * The clip algorithm seems to be a bit inefficient because it does not check the common case first if both coordinates are within the bounding box The patch fixes all described problems. The code in the SeaGenerator the joins the ways after the bounding box clipping can be removed because of the LineClipper fixes. @Steve: Can you have a short look on the patch? The LineClipper was committed by you so it might be good to have a small code review. WanMil
The patch removes artefacts from the LineClipper that might cause floodings.
WanMil
I am not sure if this comes from updating mkgmap-trunk from r1946 to r1953 or from this patch v2, but I get these additional warnings: 2011/05/23 22:51:09 WARNING (MultiPolygonRelation): 63240002.osm.gz: Cannot join the following ways to closed polygons. Multipolygon http://www.openstreetmap.org/browse/relation/36306 2011/05/23 22:53:04 WARNING (MultiPolygonRelation): 63240004.osm.gz: Cannot join the following ways to closed polygons. Multipolygon http://www.openstreetmap.org/browse/relation/38101 2011/05/23 22:54:18 WARNING (MultiPolygonRelation): 63240006.osm.gz: Cannot join the following ways to closed polygons. Multipolygon http://www.openstreetmap.org/browse/relation/900367 2011/05/23 22:54:18 WARNING (MultiPolygonRelation): 63240006.osm.gz: - way: http://www.openstreetmap.org/browse/way/57746082 2011/05/23 22:54:18 WARNING (MultiPolygonRelation): 63240006.osm.gz: Multipolygon http://www.openstreetmap.org/browse/relation/900367 does not contain any way tagged with role=outer or empty role. I took a look at these relations in JOSM (ctrl-L, enter http://api.openstreetmap.org/api/0.6/relation/900367/full and so on). Relations 36306 and 38101 are boundary relations, which include some maritime borders in the south. They might not be completely inside the Geofabrik extract of Finland. I do not care about them. The outer line of 900367 (one of the multipolygons for Lake Saimaa) carried natural=water, which I now removed, as the multipolygon relation already carries that attribute. The roles of the relation are correct: all member ways are either role=outer or role=inner. JOSM does not complain. I guess that mkgmap is just dropping the role=outer line, because it is incomplete in the tile: 63240006: 2832510,1317344 to 2916352,1462272 # : 60.779070,28.267136 to 62.578125,31.376953 The bbox of the role=outer way is 62.69143,28.0395 to 62.516,28.46425. That is, the outer line is slightly inside the tile. To be exact, the following nodes (two bays) of the outer way are inside the tile: 244270918 244272962 244270916 244272960 244270922 244270920 244272964 244270915 244270919 244272963 244272961 244270923 359093973 244270921 One island is partly at the NW corner of the tile: way 26228126. The other bay does not contain islands in the map data. Now, let me check 62.578125,28.2671360 in the generated map. Sure enough, a section of the lake is missing SE of the island. I guess it is showing just the background there. Am I right that this is a splitter problem (possibly fixable by increasing the split-radius)? Marko
I am not sure if this comes from updating mkgmap-trunk from r1946 to r1953 or from this patch v2, but I get these additional warnings:
This patch is unsuspicious because AFAIK the LineClipper is only used after the multipolygon code. So it should not change anything. But the commit for r1953 has changed the handling how unclosed ways in multipolygons are closed. This code can just guess how these ways should be closed. So each change might remove some problems but also add some new ones.
2011/05/23 22:51:09 WARNING (MultiPolygonRelation): 63240002.osm.gz: Cannot join the following ways to closed polygons. Multipolygon http://www.openstreetmap.org/browse/relation/36306 2011/05/23 22:53:04 WARNING (MultiPolygonRelation): 63240004.osm.gz: Cannot join the following ways to closed polygons. Multipolygon http://www.openstreetmap.org/browse/relation/38101 2011/05/23 22:54:18 WARNING (MultiPolygonRelation): 63240006.osm.gz: Cannot join the following ways to closed polygons. Multipolygon http://www.openstreetmap.org/browse/relation/900367 2011/05/23 22:54:18 WARNING (MultiPolygonRelation): 63240006.osm.gz: - way: http://www.openstreetmap.org/browse/way/57746082 2011/05/23 22:54:18 WARNING (MultiPolygonRelation): 63240006.osm.gz: Multipolygon http://www.openstreetmap.org/browse/relation/900367 does not contain any way tagged with role=outer or empty role.
I took a look at these relations in JOSM (ctrl-L, enter http://api.openstreetmap.org/api/0.6/relation/900367/full and so on).
Relations 36306 and 38101 are boundary relations, which include some maritime borders in the south. They might not be completely inside the Geofabrik extract of Finland. I do not care about them.
The outer line of 900367 (one of the multipolygons for Lake Saimaa) carried natural=water, which I now removed, as the multipolygon relation already carries that attribute. The roles of the relation are correct: all member ways are either role=outer or role=inner. JOSM does not complain. I guess that mkgmap is just dropping the role=outer line, because it is incomplete in the tile:
63240006: 2832510,1317344 to 2916352,1462272 # : 60.779070,28.267136 to 62.578125,31.376953
The bbox of the role=outer way is 62.69143,28.0395 to 62.516,28.46425. That is, the outer line is slightly inside the tile. To be exact, the following nodes (two bays) of the outer way are inside the tile:
244270918 244272962 244270916 244272960 244270922 244270920 244272964 244270915 244270919 244272963 244272961 244270923 359093973 244270921
I cannot say for sure because I have to check that in detail but the following problem might have happened. The autoclosing does not longer prefer closing ways to connecting ways. So maybe the incomplete outer way was connected to an incomplete inner way and therefore the way could not be closed and is therefore abandoned. Maybe the algorithm should prefer connections of ways with an identical role or better it should not connect ways with non identical role. Sounds good. I will post a patch.
One island is partly at the NW corner of the tile: way 26228126. The other bay does not contain islands in the map data. Now, let me check 62.578125,28.2671360 in the generated map. Sure enough, a section of the lake is missing SE of the island. I guess it is showing just the background there.
Am I right that this is a splitter problem (possibly fixable by increasing the split-radius)?
In the end all of (or better most of) the multipolygon problems where well defined mps are not correctly handled by mkgmap are caused by splitter generating incomplete mps. WanMil
Marko _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi WanMil, On Tue, May 24, 2011 at 06:16:04PM +0200, WanMil wrote:
I cannot say for sure because I have to check that in detail but the following problem might have happened. The autoclosing does not longer prefer closing ways to connecting ways.
Why not? What was wrong with autoclosing ways to connecting ways? Could the autoclosing issue some messages at the WARNING level, please? By the way, I ran across to some Corine landuse imports that had split a polygon line because it contained so many nodes. I created a multipolygon out of that, with just the split lines in role=outer. I think that it is reasonable not to join connecting polygon lines, unless they are members in the same multipolygon.
So maybe the incomplete outer way was connected to an incomplete inner way and therefore the way could not be closed and is therefore abandoned. Maybe the algorithm should prefer connections of ways with an identical role or better it should not connect ways with non identical role.
Sounds good. I will post a patch.
I am looking forward to testing it. Marko
Hi WanMil,
On Tue, May 24, 2011 at 06:16:04PM +0200, WanMil wrote:
I cannot say for sure because I have to check that in detail but the following problem might have happened. The autoclosing does not longer prefer closing ways to connecting ways.
Why not? What was wrong with autoclosing ways to connecting ways?
Attached picture shows the problem. The mp gets two unconnected ways. It must now decide if it is better to close the ways or to connect the ways. Now I decided that it is better to use the shorter one.
Could the autoclosing issue some messages at the WARNING level, please?
Do you really want to have warnings for that? You will get lots of warnings...
By the way, I ran across to some Corine landuse imports that had split a polygon line because it contained so many nodes. I created a multipolygon out of that, with just the split lines in role=outer. I think that it is reasonable not to join connecting polygon lines, unless they are members in the same multipolygon.
I didn't think of joining lines that do not belong to the same multipolygon.
So maybe the incomplete outer way was connected to an incomplete inner way and therefore the way could not be closed and is therefore abandoned. Maybe the algorithm should prefer connections of ways with an identical role or better it should not connect ways with non identical role.
Sounds good. I will post a patch.
I am looking forward to testing it.
I implemented and tested this change with your usecase but it didn't help. Maybe there is another problem... WanMil
Marko
Am 25.05.2011 19:08, schrieb WanMil:
Hi WanMil,
On Tue, May 24, 2011 at 06:16:04PM +0200, WanMil wrote:
I cannot say for sure because I have to check that in detail but the following problem might have happened. The autoclosing does not longer prefer closing ways to connecting ways.
Why not? What was wrong with autoclosing ways to connecting ways?
Attached picture shows the problem. The mp gets two unconnected ways. It must now decide if it is better to close the ways or to connect the ways. Now I decided that it is better to use the shorter one.
Could the autoclosing issue some messages at the WARNING level, please?
Do you really want to have warnings for that? You will get lots of warnings...
By the way, I ran across to some Corine landuse imports that had split a polygon line because it contained so many nodes. I created a multipolygon out of that, with just the split lines in role=outer. I think that it is reasonable not to join connecting polygon lines, unless they are members in the same multipolygon.
I didn't think of joining lines that do not belong to the same multipolygon.
So maybe the incomplete outer way was connected to an incomplete inner way and therefore the way could not be closed and is therefore abandoned. Maybe the algorithm should prefer connections of ways with an identical role or better it should not connect ways with non identical role.
Sounds good. I will post a patch.
I am looking forward to testing it.
I implemented and tested this change with your usecase but it didn't help. Maybe there is another problem...
WanMil
Ok, posted the patch which fixes your testcase. Please try. WanMil
Marko
participants (3)
-
Marko Mäkelä -
Steve Ratcliffe -
WanMil