Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
Hi WanMil, The patch did not apply cleanly to src/uk/me/parabola/util/Java2DConverter.java revision 1862, because the file in the repository has CR+LF line endings. I believe that we should do svn propset svn:eol-style native to every (text) file in the repository. I am now testing the patch on Finland. Marko ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
On Sun, Feb 27, 2011 at 11:06:22AM +0200, Marko Mäkelä wrote:
I am now testing the patch on Finland.
I got 2386 additional warnings. Some are for thin man_made=pier that
have been drawn as lines. There is man_made=* in the polygons style
file. Could we suppress the warnings for certain minor unclosed
polygons? E.g., something like this:
building=* | man_made=* | amenity=* | tourism=*
{ add mkgmap:polygon-check=false } [0x13 resolution 24]
If I disable this rule altogether, the warning count drops from 2386 to
476. That is, the selective suppression of the warnings would help a
lot.
I would demote the 270 messages about automatic closure to the INFO
level. That would leave 206 messages that need to be checked.
One more idea: you might add some extra treatment for polygons that are
more than 1 lap. For example,
http://www.openstreetmap.org/browse/way/76559964 comprised of nodes
902322231, 902322232, ..., 902322231, 902322232. The second 902322232
triggered the error.
Marko
___
mkgmap-dev mailing list
mkgmap-dev@lists.mkgmap.org.uk
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
Marko Mäkelä (marko.mak...@iki.fi) wrote:
On Sun, Feb 27, 2011 at 11:06:22AM +0200, Marko Mäkelä wrote:
I am now testing the patch on Finland.
I got 2386 additional warnings. Some are for thin man_made=pier that
have been drawn as lines. There is man_made=* in the polygons style
file. Could we suppress the warnings for certain minor unclosed
polygons? E.g., something like this:
building=* | man_made=* | amenity=* | tourism=*
{ add mkgmap:polygon-check=false } [0x13 resolution 24]
In my custom style I have adjusted the polygons style rule to account
for this type of case:
man_made=pier area=yes [etc]
Though this depends, of course, on someone tagging the pier properly.
A mkgmap error message would help me to identify where this hasn't
been done.
--
Charlie
___
mkgmap-dev mailing list
mkgmap-dev@lists.mkgmap.org.uk
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
Moin, Marko Mäkelä schrieb am 27.02.2011 10:32: I got 2386 additional warnings. Some are for thin man_made=pier that have been drawn as lines. There is man_made=* in the polygons style file. I have not tested the polygonclose_v2.patch but it sounds like it has the inverse effect of the previous restrict_polygon_v1.patch, which sounds like a very bad idea to me. In the OSM data we have sometimes identical tags for line objects and for area objects. The restrict_polygon_v1.patch was build, so that the polygon rules were only applied, if the OSM-way is closed. Now the new patch seems to close automatically all ambigous ways, making the previous problem even worse. Gruss Torsten ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
it's the other way round. The patch tests for all unclosed ways that are assigned with a garmin polygon type if both endpoints are outside the bounding box. Only in this case the polygons are closed automatically. Ok, perhaps take an example, so that I can understand what patch has which effect. There is an OSM-way with Tags: tagA=valueA and Nodes: node1 node2 node3 (node1 /= node3) In the polygon style there is a line tagA=valueA [0x01 continue] And in the lines style there is a line tagA=valueA [0x02 continue] With r1733 this will result in two elements in the garmin map: 1. A triangular shape with the type 0x01 and the corners node1, node2 and node3. 2. A line with the type 0x02 from node1 via node2 to node3. With r1733 and restrict_polygon_v1.patch this will result only in one element in the garmin map: A line with the type 0x02 from node1 via node2 to node3. What will be the result of r1733 and polygonclose_v2.patch? What will be the result of r1733 and polygonclose_v2.patch and restrict_polygon_v1.patch? (Would this make any sense at all?) Gruss Torsten I have committed the (slightly changed) polygonclose_v2.patch which is now r1865. Your example will result in 1. line with 0x01 2. a) nothing more if node1 or node2 is within the tiles bounding box b) a polygon 0x02 if node1 and node2 is outside or on the tiles bounding box. Both patches do not make sense because if appliying restrict_polygon_v1.patch the code of polygonclose_v2.patch will never be reached. WanMil ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
WanMil schrieb am 27.02.2011 16:53: Your example will result in 1. line with 0x01 2. a) nothing more if node1 or node2 is within the tiles bounding box b) a polygon 0x02 if node1 and node2 is outside or on the tiles bounding box. Ok, I will try out r1865 when it is availbale as a download. But in my opinion 2.b) is a fault: If the polygon is not closed in the OSM data, it shouldn't be closed by mkgmap, no matter whether the start and end nodes are inside or outside the bounding box. Gruss Torsten ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
On Sun, Feb 27, 2011 at 12:59:09PM +0100, WanMil wrote: If I disable this rule altogether, the warning count drops from 2386 to 476. That is, the selective suppression of the warnings would help a lot. Mmh, I am not a big fan of suppressing warning messages. But I see that you have the problem that you only want to use man_made=pier with polygons. If you add a dead rule to the line style you will loose all man_made=pier. Would it help you if I add the tags of the way to the warning message? Then you could easily filter the message. I see that you implemented the tag list printout. It might be good enough. One more idea: you might add some extra treatment for polygons that are more than 1 lap. For example, http://www.openstreetmap.org/browse/way/76559964 comprised of nodes 902322231, 902322232, ..., 902322231, 902322232. The second 902322232 triggered the error. Is this really a big problem? It is an error in the OSM data and just in case this does not happen very often we should not support such errors by fixing them automatically. It is probably not a big problem, and I was not thinking that we should try to fix any garbage automatically. It could be nice to have the message distinguish incorrectly closed polygons from unclosed polygons. It took me some time to figure out the problem. I was expecting to see a gap between some points, but there was none. Only after I split the way in JOSM, the highlighting of the area hinted me where the starting point was. Marko ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
[mkgmap-dev] [PATCH v2] Fix so that all polygons are really closed
Now only lines which endpoints are outside or on the bounding box are
closed.
WanMil
As András Kolesár found out the PolygonSplitter did not close all
polygons. There were several more places in the code where polygons were
not closed. I am not really sure if that caused any problems in the map
but definitely the coding was not clean.
The patch fixes the conversion between mkgmap objects (ListCoord) and
Java2D objects (Polygon, Area). Additionally the StyleConverter ensures
that lines which are assigned with polygonal garmin types are closed by
adding the first coord to its list of points.
Please test and inspect the polygons crossing the tile borders in
particular.
WanMil
Index: src/uk/me/parabola/mkgmap/reader/MapperBasedMapDataSource.java
===
--- src/uk/me/parabola/mkgmap/reader/MapperBasedMapDataSource.java (revision 1862)
+++ src/uk/me/parabola/mkgmap/reader/MapperBasedMapDataSource.java (working copy)
@@ -121,7 +121,7 @@
coords.add(co);
co = new Coord(bounds.getMaxLat(), bounds.getMinLong());
coords.add(co);
- //coords.add(start);
+ coords.add(start);
// Now add the background area
MapShape background = new MapShape();
Index: src/uk/me/parabola/mkgmap/general/PolygonClipper.java
===
--- src/uk/me/parabola/mkgmap/general/PolygonClipper.java (revision 1862)
+++ src/uk/me/parabola/mkgmap/general/PolygonClipper.java (working copy)
@@ -16,13 +16,11 @@
*/
package uk.me.parabola.mkgmap.general;
-import java.awt.*;
-import java.awt.geom.PathIterator;
-import java.util.ArrayList;
import java.util.List;
import uk.me.parabola.imgfmt.app.Area;
import uk.me.parabola.imgfmt.app.Coord;
+import uk.me.parabola.util.Java2DConverter;
/**
* Clip a polygon to the given bounding box. This may result in more than
@@ -43,7 +41,7 @@
if (bbox == null)
return null;
- // If all the points are inside the box then we just return null
+ // If all the points are inside the box then we just return null
// to show that nothing was done and the line can be used. This
// is expected to be the normal case.
boolean foundOutside = false;
@@ -56,67 +54,12 @@
if (!foundOutside)
return null;
- // Convert to a awt polygon
- Polygon polygon = new Polygon();
- for (Coord co : coords)
- polygon.addPoint(co.getLongitude(), co.getLatitude());
-
- Polygon bounds = new Polygon();
- bounds.addPoint(bbox.getMinLong(), bbox.getMinLat());
- bounds.addPoint(bbox.getMinLong(), bbox.getMaxLat());
- bounds.addPoint(bbox.getMaxLong(), bbox.getMaxLat());
- bounds.addPoint(bbox.getMaxLong(), bbox.getMinLat());
- bounds.addPoint(bbox.getMinLong(), bbox.getMinLat());
-
- java.awt.geom.Area bbarea = new java.awt.geom.Area(bounds);
- java.awt.geom.Area shape = new java.awt.geom.Area(polygon);
+ java.awt.geom.Area bbarea = Java2DConverter.createBoundsArea(bbox);
+ java.awt.geom.Area shape = Java2DConverter.createArea(coords);
shape.intersect(bbarea);
- return areaToShapes(shape);
+ return Java2DConverter.areaToShapes(shape);
}
- /**
- * Convert the area back into {@link MapShape}s. It is possible that the
- * area is multiple discontiguous polygons, so you may append more than one
- * shape to the output list.
- *
- * @param area The area to be converted.
- */
- private static ListListCoord areaToShapes(java.awt.geom.Area area) {
- ListListCoord outputs = new ArrayListListCoord();
- float[] res = new float[6];
- PathIterator pit = area.getPathIterator(null);
-
- ListCoord coords = null;
- while (!pit.isDone()) {
- int type = pit.currentSegment(res);
-
- Coord co = new Coord(Math.round(res[1]), Math.round(res[0]));
-
- if (type == PathIterator.SEG_MOVETO) {
-// We get a move to at the beginning and if this area is actually
-// discontiguous we may get more than one, each one representing
-// the start of another polygon in the output.
-if (coords != null)
- outputs.add(coords);
-
-coords = new ArrayListCoord();
-coords.add(co);
- } else if (type == PathIterator.SEG_LINETO) {
-// Continuing with the path.
-assert coords != null;
-coords.add(co);
- } else if (type == PathIterator.SEG_CLOSE) {
-// The end of a polygon
-assert coords != null;
-coords.add(co);
-
-outputs.add(coords);
-coords = null;
- }
- pit.next();
- }
- return outputs;
- }
}
Index: src/uk/me/parabola/mkgmap/filters/PolygonSplitterBase.java
===
--- src/uk/me/parabola/mkgmap/filters/PolygonSplitterBase.java (revision 1862)
+++ src/uk/me/parabola/mkgmap/filters/PolygonSplitterBase.java (working copy)
@@ -16,14 +16,13 @@
*/
package uk.me.parabola.mkgmap.filters;
-import java.awt.*;
+import java.awt.Rectangle;
import java.awt.geom.Area;
-import java.awt.geom.PathIterator;
-import java.util.ArrayList;
import
