Index: src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 11155) +++ src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (working copy) @@ -5,26 +5,31 @@ 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.Point2D; 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.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.command.ChangeCommand; +import org.openstreetmap.josm.command.Command; +import org.openstreetmap.josm.data.coor.EastNorth; 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; @@ -66,6 +71,10 @@ public static final int NO_STYLE_POLYGON = 1611; /** Area style on outer way */ public static final int OUTER_STYLE = 1613; + /** Multipolygon member repeated (same primitive, same role */ + public static final int REPEATED_MEMBER_SAME_ROLE = 1614; + /** Multipolygon member repeated (same primitive, different role) */ + public static final int REPEATED_MEMBER_DIFF_ROLE = 1615; private static volatile ElemStyles styles; @@ -102,40 +111,6 @@ 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()); - for (int i = 1; i < nodes.size(); i++) { - Node n = nodes.get(i); - result.lineTo((float) n.getCoor().lat(), (float) n.getCoor().lon()); - } - return result; - } - - private static List createPolygons(List joinedWays) { - List result = new ArrayList<>(); - for (Multipolygon.PolyData way : joinedWays) { - result.add(createPath(way.getNodes())); - } - 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)) { @@ -159,15 +134,16 @@ if (r.isMultipolygon()) { checkMembersAndRoles(r); checkOuterWay(r); - - // Rest of checks is only for complete multipolygons - if (!r.hasIncompleteMembers()) { - Multipolygon polygon = MultipolygonCache.getInstance().get(Main.map.mapView, r); - - // Create new multipolygon using the logics from CreateMultipolygonAction and see if roles match. - checkMemberRoleCorrectness(r); - checkStyleConsistency(r, polygon); - checkGeometry(r, polygon); + boolean hasRepeatedMembers = checkRepeatedWayMembers(r); + if (!hasRepeatedMembers) { + List intersectionNodes = checkIntersectionAtNodes(r); + // Rest of checks is only for complete multipolygons + if (!r.hasIncompleteMembers()) { + boolean rolesWereChecked = checkMemberRoleCorrectness(r); + Multipolygon polygon = new Multipolygon(r); + checkStyleConsistency(r, polygon); + checkGeometry(r, polygon, intersectionNodes, rolesWereChecked); + } } } } @@ -195,13 +171,16 @@ } /** - * Create new multipolygon using the logics from CreateMultipolygonAction and see if roles match:
    + * If simple joining of ways doesn't work, create new multipolygon using the logics from + * CreateMultipolygonAction and see if roles match:
      *
    • {@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()) { final Collection memberInNewMP = newMP.b.getMembersFor(Collections.singleton(member.getMember())); @@ -219,6 +198,7 @@ } } } + return newMP != null; } /** @@ -303,8 +283,10 @@ *
* @param r relation * @param polygon multipolygon + * @param intersectionNodes known nodes where ways of this multipolygon intersect or touch + * @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, List intersectionNodes, boolean rolesWereChecked) { List openNodes = polygon.getOpenEnds(); if (!openNodes.isEmpty()) { errors.add(TestError.builder(this, Severity.WARNING, NON_CLOSED_WAY) @@ -314,52 +296,106 @@ .build()); } - // 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); + HashMap> crossingPolyMap = checkCrossingWays(r, innerPolygons, outerPolygons); + + //Polygons may intersect without crossing ways when one polygon lies completely inside the other 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); + // report outer polygons which lie inside another outer + PolyData outer1 = outerPolygons.get(i); + for (int j = 0; j < outerPolygons.size(); j++) { + if (i != j && !crossing(crossingPolyMap, outer1, outerPolygons.get(j))) { + EastNorth en1 = null; + // find node which is not an intersection of multipolygon ways + for (int k = 0; k < outerPolygons.get(j).getNodes().size(); k++) { + Node n = outerPolygons.get(j).getNodes().get(k); + if (intersectionNodes.contains(n) == false) + en1 = n.getEastNorth(); + } + if (en1 != null && outer1.get().contains(en1.getX(), en1.getY())) { + ArrayList hilite = new ArrayList<>(); + hilite.addAll(outer1.getNodes()); + hilite.addAll(outerPolygons.get(j).getNodes()); + errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) + .message(tr("Outer inside outer")) + .primitives(r) + .highlight(hilite) + .build()); + + } + } } } - 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 (PolyData inner1 : innerPolygons) { + for (PolyData inner2 : innerPolygons) { + if (inner1 != inner2 && !crossing(crossingPolyMap, inner1, inner2)) { + // we must allow that inner polygons share some nodes + boolean allInside = true; + for (Node innerPoint : inner2.getNodes()) { + EastNorth en = innerPoint.getEastNorth(); + if (!inner1.get().contains(en.getX(), en.getY())) { + allInside = false; + break; + } + } + if (allInside) { + ArrayList hilite = new ArrayList<>(); + hilite.addAll(inner1.getNodes()); + hilite.addAll(inner2.getNodes()); + errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) + .message(tr("Inner inside inner")) + .primitives(r) + .highlight(hilite) + .build()); + } + } } - // Check for intersection between inner and outer members - boolean outside = true; - for (int o = 0; o < outerPolygons.size(); o++) { - outside &= checkCrossingWays(r, outerPolygons, outerPolygonsPaths, pdInner, o) == Intersection.OUTSIDE; - } - if (outside) { - errors.add(TestError.builder(this, Severity.WARNING, INNER_WAY_OUTSIDE) - .message(tr("Multipolygon inner way is outside")) - .primitives(r) - .highlightNodePairs(Collections.singletonList(pdInner.getNodes())) - .build()); - } + // Find inner polygons which are not inside any outer + // MAYBE enable the following line to reduce number of warnings for same problem (and adapt multipolygon.osm) +// if (!rolesWereChecked) { + boolean outside = true; + boolean crossingWithOuter = false; + EastNorth innerPoint = inner1.getNodes().get(0).getEastNorth(); + for (PolyData outer : outerPolygons) { + if (crossing(crossingPolyMap, inner1, outer)) { + crossingWithOuter = true; + break; + } + outside &= !outer.get().contains(innerPoint.getX(), innerPoint.getY()); + if (!outside) + break; + } + if (outside && !crossingWithOuter) { + errors.add(TestError.builder(this, Severity.WARNING, INNER_WAY_OUTSIDE) + .message(tr("Multipolygon inner way is outside")) + .primitives(r) + .highlightNodePairs(Collections.singletonList(inner1.getNodes())) + .build()); + } +// } } } - 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) { - errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) - .message(tr("Intersection between multipolygon ways")) - .primitives(r) - .highlightNodePairs(Arrays.asList(pd.getNodes(), pdOther.getNodes())) - .build()); - } + /** + * Check if crossing map contains combination of two given polygons. + * @param crossingPolyMap the map + * @param pd1 1st polygon + * @param pd2 2nd polygon + * @return true if the polygons are crossing without sharing a node + */ + 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; } /** @@ -412,4 +448,272 @@ } } + /** + * Determine multipolygon ways which are intersecting (crossing without a common node). This is not 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) { + /** All way segments, grouped by cells */ + final Map> cellSegments = new HashMap<>(1000); + /** The already detected ways in error */ + final Map, List> crossingWays = new HashMap<>(50); + + for (Way w : r.getMemberPrimitives(Way.class)) { + checkCrossingWay(w, r, cellSegments, crossingWays); + } + HashMap> crossingPolygonsMap = new HashMap<>(); + if (!crossingWays.isEmpty()) { + List allPolygons = new ArrayList<>(innerPolygons.size() + outerPolygons.size()); + allPolygons.addAll(innerPolygons); + allPolygons.addAll(outerPolygons); + + for (Entry, List> entry : crossingWays.entrySet()) { + List ways = entry.getKey(); + if (ways.size() != 2) + continue; + PolyData[] crossingPolys = new PolyData[2]; + for (int i = 0; i < 2; i++) { + Way w = ways.get(i); + for (PolyData pd : allPolygons) { + if (pd.getWayIds().contains(w.getUniqueId())) { + crossingPolys[i] = pd; + break; + } + } + } + if (crossingPolys[0] != null && crossingPolys[1] != null) { + List crossingPolygons = crossingPolygonsMap.get(crossingPolys[0]); + if (crossingPolygons == null) { + crossingPolygons = new ArrayList<>(); + crossingPolygonsMap.put(crossingPolys[0], crossingPolygons); + } + crossingPolygons.add(crossingPolys[1]); + } + } + } + return crossingPolygonsMap; + } + + /** + * Find ways which are crossing without sharing a node. + * @param w way that is member of the relation + * @param r the relation (used for error messages) + * @param crossingWays + * @param cellSegments + */ + private void checkCrossingWay(Way w, Relation r, Map> cellSegments, Map, List> crossingWays) { + 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 : CrossingWays.getSegments(cellSegments, en1, en2)) { + for (WaySegment es2 : segments) { + + List highlight; + if (es2.way == w) + continue; // reported by CrossingWays.SelfIntersection + if (!es1.intersects(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); + errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) + .message(tr("Intersection between multipolygon ways")) + .primitives(Arrays.asList(r, es1.way, es2.way)) + .highlightWaySegments(highlight) + .build()); + crossingWays.put(prims, highlight); + } else { + highlight.add(es1); + highlight.add(es2); + } + } + segments.add(es1); + } + } + } + + /** + * Detect intersections of multipolygon ways at nodes. If any way node is used by more than two ways + * or two times in one way and at least once in another way we found an intersection. + * @param r the relation + * @return List of nodes were ways intersect + */ + private List checkIntersectionAtNodes(Relation r) { + List intersectionNodes = new ArrayList<>(); + List,List>> intersectionErrors = new ArrayList<>(); + HashMap wayRoleMap = new HashMap<>(); + for (RelationMember rm : r.getMembers()) { + if (rm.isWay()) + wayRoleMap.put(rm.getWay(), rm.getRole()); + } + Map> nodeMap = new HashMap<>(); + for (RelationMember rm : r.getMembers()) { + if (!rm.isWay()) + continue; + int numNodes = rm.getWay().getNodesCount(); + for (int i = 0; i < numNodes; i++) { + Node n = rm.getWay().getNode(i); + if (n.getReferrers().size() <= 1) { + continue; // cannot be a problem node + } + List ways = nodeMap.get(n); + if (ways == null) { + ways = new ArrayList<>(); + nodeMap.put(n, ways); + } + ways.add(rm.getWay()); + if (ways.size() > 2 || (ways.size() == 2 && i != 0 && i + 1 != numNodes)) { + intersectionNodes.add(n); + boolean allInner = true; + for (Way w : ways) { + String role = wayRoleMap.get(w); + if (!"inner".equals(role)) + allInner = false; + } + if (!allInner) { + Set errorWays = new HashSet<>(ways); + boolean addNew = true; + for (Pair, List> pair : intersectionErrors) { + if (pair.a.size() == errorWays.size() && pair.a.containsAll(errorWays)) { + pair.b.add(n); + addNew = false; + break; + } + } + if (addNew) { + List errNodes = new ArrayList<>(); + errNodes.add(n); + intersectionErrors.add(new Pair<>(errorWays, errNodes)); + } + } + } + } + } + for (Pair,List> pair : intersectionErrors) { + // A single shared node between two ways ("touching ways") is considered okay. + if (pair.b.size() > 1) { + errors.add(TestError.builder(this, Severity.WARNING, CROSSING_WAYS) + .message(tr("Multipolygon ways share node(s)")) + .primitives(pair.a) + .highlight(pair.b) + .build()); + } + } + return intersectionNodes; + } + + /** + * Check for:
    + *
  • {@link #REPEATED_MEMBER_DIFF_ROLE}: Multipolygon member(s) repeated with different role
    • + *
  • {@link #REPEATED_MEMBER_SAME_ROLE}: Multipolygon member(s) repeated with same role
    • + *
+ * @param r relation + * @return + */ + private boolean checkRepeatedWayMembers(Relation r) { + boolean hasDups = false; + Map> seenMemberPrimitives = new HashMap<>(); + for (RelationMember rm : r.getMembers()) { + List list = seenMemberPrimitives.get(rm.getMember()); + if (list == null) { + list = new ArrayList<>(2); + seenMemberPrimitives.put(rm.getMember(), list); + } else { + hasDups = true; + } + list.add(rm); + } + if (hasDups) { + List repeatedSameRole = new ArrayList<>(); + List repeatedDiffRole = new ArrayList<>(); + for (Entry> e : seenMemberPrimitives.entrySet()) { + List visited = e.getValue(); + if (e.getValue().size() == 1) + continue; + // we found a duplicate member, check if the roles differ + boolean rolesDiffer = false; + RelationMember rm = visited.get(0); + List primitives = new ArrayList<>(); + for (int i = 1; i < visited.size(); i++) { + RelationMember v = visited.get(i); + primitives.add(rm.getMember()); + if (v.getRole().equals(rm.getRole()) == false) { + rolesDiffer = true; + } + } + if (rolesDiffer) + repeatedDiffRole.addAll(primitives); + else + repeatedSameRole.addAll(primitives); + } + addRepeatedMemberError(r, repeatedDiffRole, REPEATED_MEMBER_DIFF_ROLE, tr("Multipolygon member(s) repeated with different role")); + addRepeatedMemberError(r, repeatedSameRole, REPEATED_MEMBER_SAME_ROLE, tr("Multipolygon member(s) repeated with same role")); + } + return hasDups; + } + + private void addRepeatedMemberError(Relation r, List repeatedMembers, int errorCode, String msg) { + if (!repeatedMembers.isEmpty()) { + List prims = new ArrayList<>(1 + repeatedMembers.size()); + prims.add(r); + prims.addAll(repeatedMembers); + errors.add(TestError.builder(this, Severity.WARNING, errorCode) + .message(msg) + .primitives(prims) + .highlight(repeatedMembers) + .build()); + } + } + + @Override + public Command fixError(TestError testError) { + if (testError.getCode() == REPEATED_MEMBER_SAME_ROLE) { + ArrayList primitives = new ArrayList<>(testError.getPrimitives()); + if (primitives.size() >= 2) { + if (primitives.get(0) instanceof Relation) { + Relation oldRel = (Relation) primitives.get(0); + Relation newRel = new Relation(oldRel); + List repeatedPrims = primitives.subList(1, primitives.size()); + List oldMembers = oldRel.getMembers(); + + List newMembers = new ArrayList<>(); + HashSet toRemove = new HashSet<>(repeatedPrims); + HashSet found = new HashSet<>(repeatedPrims.size()); + for (RelationMember rm : oldMembers) { + if (toRemove.contains(rm.getMember())) { + if (found.contains(rm.getMember()) == false) { + found.add(rm.getMember()); + newMembers.add(rm); + } + } else + newMembers.add(rm); + } + newRel.setMembers(newMembers); + return new ChangeCommand (oldRel, newRel); + } + } + } + return null; + } + + @Override + public boolean isFixable(TestError testError) { + if (testError.getCode() == REPEATED_MEMBER_SAME_ROLE) + return true; + return false; + } }