Index: src/org/openstreetmap/josm/data/osm/WaySegment.java =================================================================== --- src/org/openstreetmap/josm/data/osm/WaySegment.java (revision 10797) +++ src/org/openstreetmap/josm/data/osm/WaySegment.java (working copy) @@ -114,8 +114,22 @@ s2.getSecondNode().getEastNorth().east(), s2.getSecondNode().getEastNorth().north()); } + /** + * Checks whether this segment and another way segment share the same points + * @param s2 The other segment + * @return true if other way segment is the same or reverse + */ + public boolean isSimilar(WaySegment s2){ + if (getFirstNode().equals(s2.getFirstNode()) && getSecondNode().equals(s2.getSecondNode())) + return true; + if (getFirstNode().equals(s2.getSecondNode()) && getSecondNode().equals(s2.getFirstNode())) + return true; + return false; + } @Override public String toString() { return "WaySegment [way=" + way.getUniqueId() + ", lowerIndex=" + lowerIndex + ']'; } + + } Index: src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 10797) +++ src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (working copy) @@ -4,32 +4,39 @@ import static org.openstreetmap.josm.tools.I18n.tr; import static org.openstreetmap.josm.tools.I18n.trn; -import java.awt.geom.GeneralPath; +import java.awt.geom.Path2D; +import java.awt.geom.Point2D; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.CreateMultipolygonAction; +import org.openstreetmap.josm.data.coor.EastNorth; +import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.osm.WaySegment; import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon; import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.PolyData; -import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.PolyData.Intersection; import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache; import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.data.validation.Severity; import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.data.validation.util.ValUtil; import org.openstreetmap.josm.gui.DefaultNameFormatter; import org.openstreetmap.josm.gui.mappaint.ElemStyles; import org.openstreetmap.josm.gui.mappaint.MapPaintStyles; @@ -71,6 +78,10 @@ private static volatile ElemStyles styles; private final Set keysCheckedByAnotherTest = new HashSet<>(); + /** All way segments, grouped by cells */ + private final Map> cellSegments = new HashMap<>(1000); + /** The already detected ways in error */ + private final Map, List> crossingWays = new HashMap<>(50); /** * Constructs a new {@code MultipolygonTest}. @@ -103,18 +114,18 @@ super.endTest(); } - private static GeneralPath createPath(List nodes) { - GeneralPath result = new GeneralPath(); - result.moveTo((float) nodes.get(0).getCoor().lat(), (float) nodes.get(0).getCoor().lon()); + private static Path2D.Double createPath(List nodes) { + Path2D.Double result = new Path2D.Double(); + result.moveTo(nodes.get(0).getCoor().lon(), nodes.get(0).getCoor().lat()); for (int i = 1; i < nodes.size(); i++) { Node n = nodes.get(i); - result.lineTo((float) n.getCoor().lat(), (float) n.getCoor().lon()); + result.lineTo(n.getCoor().lon(), n.getCoor().lat()); } return result; } - private static List createPolygons(List joinedWays) { - List result = new ArrayList<>(); + private static List createPolygons(List joinedWays) { + List result = new ArrayList<>(); for (Multipolygon.PolyData way : joinedWays) { result.add(createPath(way.getNodes())); } @@ -121,22 +132,6 @@ return result; } - private static Intersection getPolygonIntersection(GeneralPath outer, List inner) { - boolean inside = false; - boolean outside = false; - - for (Node n : inner) { - boolean contains = outer.contains(n.getCoor().lat(), n.getCoor().lon()); - inside = inside | contains; - outside = outside | !contains; - if (inside & outside) { - return Intersection.CROSSING; - } - } - - return inside ? Intersection.INSIDE : Intersection.OUTSIDE; - } - @Override public void visit(Way w) { if (!w.isArea() && ElemStyles.hasOnlyAreaElemStyle(w)) { @@ -155,6 +150,9 @@ @Override public void visit(Relation r) { if (r.isMultipolygon()) { + crossingWays.clear(); + cellSegments.clear(); + checkMembersAndRoles(r); checkOuterWay(r); @@ -163,9 +161,9 @@ Multipolygon polygon = MultipolygonCache.getInstance().get(Main.map.mapView, r); // Create new multipolygon using the logics from CreateMultipolygonAction and see if roles match. - checkMemberRoleCorrectness(r); + boolean rolesWereChecked = checkMemberRoleCorrectness(r); checkStyleConsistency(r, polygon); - checkGeometry(r, polygon); + checkGeometry(r, polygon, rolesWereChecked); } } } @@ -194,8 +192,9 @@ *
  • {@link #WRONG_MEMBER_ROLE}: Role for ''{0}'' should be ''{1}''
    • * * @param r relation + * @return true if member roles were checked */ - private void checkMemberRoleCorrectness(Relation r) { + private boolean checkMemberRoleCorrectness(Relation r) { final Pair newMP = CreateMultipolygonAction.createMultipolygonRelation(r.getMemberPrimitives(Way.class), false); if (newMP != null) { for (RelationMember member : r.getMembers()) { @@ -216,6 +215,7 @@ } } } + return newMP != null; } /** @@ -284,6 +284,14 @@ } } } + private static class LatLonPolyData{ + final PolyData pd; + final Path2D.Double latLonPath; + public LatLonPolyData(PolyData polyData, Path2D.Double path) { + this.pd = polyData; + this.latLonPath = path; + } + } /** * Various geometry-related checks:
      @@ -293,8 +301,9 @@ *
    * @param r relation * @param polygon multipolygon + * @param rolesWereChecked might be used to skip most of the tests below */ - private void checkGeometry(Relation r, Multipolygon polygon) { + private void checkGeometry(Relation r, Multipolygon polygon, boolean rolesWereChecked) { List openNodes = polygon.getOpenEnds(); if (!openNodes.isEmpty()) { List primitives = new LinkedList<>(); @@ -302,49 +311,86 @@ primitives.addAll(openNodes); addError(r, new TestError(this, Severity.WARNING, tr("Multipolygon is not closed"), NON_CLOSED_WAY, primitives, openNodes)); } + List innerPolygons = polygon.getInnerPolygons(); + List outerPolygons = polygon.getOuterPolygons(); + HashMap> crossingPolyMap = checkCrossingWays(r, innerPolygons, outerPolygons); + + //Polygons may intersect without crossing ways when one polygon lies completely inside the other + List inner = calcLatLonPolygons(innerPolygons); + List outer = calcLatLonPolygons(outerPolygons); // For painting is used Polygon class which works with ints only. For validation we need more precision - List innerPolygons = polygon.getInnerPolygons(); - List outerPolygons = polygon.getOuterPolygons(); - List innerPolygonsPaths = innerPolygons.isEmpty() ? Collections.emptyList() : createPolygons(innerPolygons); - List outerPolygonsPaths = createPolygons(outerPolygons); - for (int i = 0; i < outerPolygons.size(); i++) { - PolyData pdOuter = outerPolygons.get(i); - // Check for intersection between outer members - for (int j = i+1; j < outerPolygons.size(); j++) { - checkCrossingWays(r, outerPolygons, outerPolygonsPaths, pdOuter, j); + for (int i = 0; i+1 < outer.size(); i++) { + // report outer polygons which lie inside another outer + LatLonPolyData outer1 = outer.get(i); + for (int j = 0; j < outer.size(); j++) { + if (i != j && !crossing(crossingPolyMap, outer1.pd, outer.get(j).pd)){ + LatLon c = outer.get(j).pd.getNodes().get(0).getCoor(); + if (outer1.latLonPath.contains(c.lon(), c.lat())){ + addError(r, new TestError(this, Severity.WARNING, tr("Intersection between multipolygon ways"), + CROSSING_WAYS, Collections.singletonList(r), Arrays.asList(outer1.pd.getNodes(), outer.get(j).pd.getNodes()))); + } + } } } - for (int i = 0; i < innerPolygons.size(); i++) { - PolyData pdInner = innerPolygons.get(i); - // Check for intersection between inner members - for (int j = i+1; j < innerPolygons.size(); j++) { - checkCrossingWays(r, innerPolygons, innerPolygonsPaths, pdInner, j); + for (int i = 0; i < inner.size(); i++) { + LatLonPolyData inner1 = inner.get(i); + LatLon innerPoint = inner1.pd.getNodes().get(0).getCoor(); + for (int j = 0; j < inner.size(); j++) { + LatLonPolyData inner2 = inner.get(j); + if (i != j && !crossing(crossingPolyMap, inner1.pd, inner2.pd)){ + if (inner.get(j).latLonPath.contains(innerPoint.lon(), innerPoint.lat())){ + addError(r, new TestError(this, Severity.WARNING, tr("Intersection between multipolygon ways"), + CROSSING_WAYS, Collections.singletonList(r), Arrays.asList(inner1.pd.getNodes(), inner2.pd.getNodes()))); + } + } } - // Check for intersection between inner and outer members + + // Find inner polygons which are not inside any outer boolean outside = true; - for (int o = 0; o < outerPolygons.size(); o++) { - outside &= checkCrossingWays(r, outerPolygons, outerPolygonsPaths, pdInner, o) == Intersection.OUTSIDE; + boolean crossingWithOuter = true; + + for (int o = 0; o < outer.size(); o++) { + if (crossing(crossingPolyMap, inner1.pd, outer.get(o).pd)){ + crossingWithOuter = true; + break; + } + outside &= outer.get(o).latLonPath.contains(innerPoint.lon(), innerPoint.lat()) == false; + if (!outside) + break; } - if (outside) { + if (outside && !crossingWithOuter) { addError(r, new TestError(this, Severity.WARNING, tr("Multipolygon inner way is outside"), - INNER_WAY_OUTSIDE, Collections.singletonList(r), Arrays.asList(pdInner.getNodes()))); + INNER_WAY_OUTSIDE, Collections.singletonList(r), Arrays.asList(inner1.pd.getNodes()))); } } } - private Intersection checkCrossingWays(Relation r, List polygons, List polygonsPaths, PolyData pd, int idx) { - Intersection intersection = getPolygonIntersection(polygonsPaths.get(idx), pd.getNodes()); - if (intersection == Intersection.CROSSING) { - PolyData pdOther = polygons.get(idx); - if (pdOther != null) { - addError(r, new TestError(this, Severity.WARNING, tr("Intersection between multipolygon ways"), - CROSSING_WAYS, Collections.singletonList(r), Arrays.asList(pd.getNodes(), pdOther.getNodes()))); - } + private boolean crossing(HashMap> crossingPolyMap, PolyData pd1, PolyData pd2) { + List crossingWithFirst = crossingPolyMap.get(pd1); + if (crossingWithFirst != null){ + if (crossingWithFirst.contains(pd2)) + return true; } - return intersection; + List crossingWith2nd= crossingPolyMap.get(pd2); + if (crossingWith2nd != null){ + if (crossingWith2nd.contains(pd1)) + return true; + } + return false; } + private List calcLatLonPolygons(List polygons) { + if (polygons == null || polygons.isEmpty()) + return Collections.emptyList(); + List latLonPolygons = new ArrayList<>(); + List polygonsPaths = createPolygons(polygons); + for (int i = 0; i < polygons.size(); i++) { + latLonPolygons.add(new LatLonPolyData(polygons.get(i), polygonsPaths.get(i))); + } + return latLonPolygons; + } + /** * Check for:
      *
    • {@link #WRONG_MEMBER_ROLE}: No useful role for multipolygon member
      • @@ -389,4 +435,137 @@ addRelationIfNeeded(error, r); errors.add(error); } + + /** + * Determine multipolygon ways which are intersecting. This is now allowed. + * See {@link CrossingWays} + * @param r the relation (for error reporting) + * @param innerPolygons list of inner polygons + * @param outerPolygons list of outer polygons + * @return map of crossing polygons (including polygons touching outer) + */ + private HashMap> checkCrossingWays (Relation r, List innerPolygons, List outerPolygons){ + List innerWays = new ArrayList<>(); + List outerWays = new ArrayList<>(); + for (Way w : r.getMemberPrimitives(Way.class)){ + for (PolyData pd : innerPolygons){ + if (pd.getWayIds().contains(w.getUniqueId())){ + innerWays.add(w); + break; + } + } + for (PolyData pd : outerPolygons){ + if (pd.getWayIds().contains(w.getUniqueId())){ + outerWays.add(w); + break; + } + } + } + for (Way w : innerWays){ + checkCrossingWay(w, r, true /* allow shared ways */); + } + for (Way w : outerWays){ + checkCrossingWay(w, r, false/* don't allow shared ways */); + } + HashMap> crossingPolygonsMap = new HashMap<>(); + for (Entry, List> entry : crossingWays.entrySet()){ + List ways = entry.getKey(); + PolyData[] crossingPolys = new PolyData[2]; + for (Way w: ways){ + for (int j = 0; j < crossingPolys.length; j++){ + for (PolyData pd : innerPolygons){ + if (pd.getWayIds().contains(w.getUniqueId())){ + crossingPolys[j] = pd; + break; + } + } + if (crossingPolys[j] != null) + break; + for (PolyData pd : outerPolygons){ + if (pd.getWayIds().contains(w.getUniqueId())){ + crossingPolys[j] = pd; + break; + } + } + } + } + if (crossingPolys[0] != null && crossingPolys[1] != null){ + List x = crossingPolygonsMap.get(crossingPolys[0]); + if (x == null){ + x = new ArrayList<>(); + crossingPolygonsMap.put(crossingPolys[0], x); + } + x.add(crossingPolys[1]); + } + } + return crossingPolygonsMap; + } + + /** + * + * @param w + * @param r + * @param allowSharedWaySegment + */ + private void checkCrossingWay(Way w, Relation r, boolean allowSharedWaySegment) { + + int nodesSize = w.getNodesCount(); + for (int i = 0; i < nodesSize - 1; i++) { + final WaySegment es1 = new WaySegment(w, i); + final EastNorth en1 = es1.getFirstNode().getEastNorth(); + final EastNorth en2 = es1.getSecondNode().getEastNorth(); + if (en1 == null || en2 == null) { + Main.warn("Crossing ways test skipped "+es1); + continue; + } + for (List segments : getSegments(en1, en2)) { + for (WaySegment es2 : segments) { + + List highlight; + + if (!es1.intersects(es2)) { + if (allowSharedWaySegment || !es1.isSimilar(es2)) + continue; + } + + List prims = Arrays.asList(es1.way, es2.way); + if ((highlight = crossingWays.get(prims)) == null) { + highlight = new ArrayList<>(); + highlight.add(es1); + highlight.add(es2); + + addError(r, new TestError(this, Severity.WARNING, tr("Intersection between multipolygon ways"), + CROSSING_WAYS, Arrays.asList(r,es1.way,es2.way), highlight )); + crossingWays.put(prims, highlight); + } else { + highlight.add(es1); + highlight.add(es2); + } + } + segments.add(es1); + } + } + } + + /** + * Returns all the cells this segment crosses. Each cell contains the list + * of segments already processed + * + * @param n1 The first EastNorth + * @param n2 The second EastNorth + * @return A list with all the cells the segment crosses + */ + private List> getSegments(EastNorth n1, EastNorth n2) { + + List> cells = new ArrayList<>(); + for (Point2D cell : ValUtil.getSegmentCells(n1, n2, OsmValidator.griddetail)) { + List segments = cellSegments.get(cell); + if (segments == null) { + segments = new ArrayList<>(); + cellSegments.put(cell, segments); + } + cells.add(segments); + } + return cells; + } }