Index: src/org/openstreetmap/josm/data/validation/OsmValidator.java =================================================================== --- src/org/openstreetmap/josm/data/validation/OsmValidator.java (revision 15056) +++ src/org/openstreetmap/josm/data/validation/OsmValidator.java (working copy) @@ -55,6 +55,7 @@ import org.openstreetmap.josm.data.validation.tests.MultipolygonTest; import org.openstreetmap.josm.data.validation.tests.NameMismatch; import org.openstreetmap.josm.data.validation.tests.OpeningHourTest; +import org.openstreetmap.josm.data.validation.tests.OverlappingAreas; import org.openstreetmap.josm.data.validation.tests.OverlappingWays; import org.openstreetmap.josm.data.validation.tests.PowerLines; import org.openstreetmap.josm.data.validation.tests.PublicTransportRouteTest; @@ -148,6 +149,7 @@ LongSegment.class, // 3500 .. 3599 PublicTransportRouteTest.class, // 3600 .. 3699 RightAngleBuildingTest.class, // 3700 .. 3799 + OverlappingAreas.class, // 3800 .. 3899 }; /** Index: src/org/openstreetmap/josm/data/validation/Test.java =================================================================== --- src/org/openstreetmap/josm/data/validation/Test.java (revision 15056) +++ src/org/openstreetmap/josm/data/validation/Test.java (working copy) @@ -18,6 +18,7 @@ import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.search.SearchCompiler.InDataSourceArea; @@ -364,6 +365,16 @@ return p.hasTag("landuse", "residential"); } + /** + * Determines if the specified primitives are in the same layer. + * @param p1 first primitive + * @param p2 second primitive + * @return True if the objects are in the same layer + */ + protected static final boolean inSameLayer(OsmPrimitive p1, OsmPrimitive p2) { + return Objects.equals(OsmUtils.getLayer(p1), OsmUtils.getLayer(p2)); + } + @Override public int hashCode() { return Objects.hash(name, description); Index: src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 15056) +++ src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (working copy) @@ -13,7 +13,6 @@ import org.openstreetmap.josm.data.coor.EastNorth; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.OsmUtils; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.WaySegment; @@ -35,7 +34,6 @@ static final String HIGHWAY = "highway"; static final String RAILWAY = "railway"; static final String WATERWAY = "waterway"; - static final String LANDUSE = "landuse"; static final class MessageHelper { final String message; @@ -106,7 +104,7 @@ boolean ignoreWaySegmentCombination(Way w1, Way w2) { if (w1 == w2) return false; - if (!Objects.equals(OsmUtils.getLayer(w1), OsmUtils.getLayer(w2))) { + if (!inSameLayer(w1, w2)) { return true; } if (w1.hasKey(HIGHWAY) && w2.hasKey(HIGHWAY) && !Objects.equals(w1.get("level"), w2.get("level"))) { @@ -251,7 +249,7 @@ @Override boolean ignoreWaySegmentCombination(Way w1, Way w2) { - return !Objects.equals(OsmUtils.getLayer(w1), OsmUtils.getLayer(w2)); + return !inSameLayer(w1, w2); } } @@ -313,7 +311,7 @@ } static boolean isCoastline(OsmPrimitive w) { - return w.hasTag("natural", "water", "coastline") || w.hasTag(LANDUSE, "reservoir"); + return w.hasTag("natural", "water", "coastline") || w.hasTag("landuse", "reservoir"); } static boolean isHighway(OsmPrimitive w) { Index: src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 15056) +++ src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (working copy) @@ -206,8 +206,8 @@ .highlight(wOuter) .build()); } else if (areaStyle) { /* style on outer way of multipolygon, but equal to polygon */ - errors.add(TestError.builder(this, Severity.WARNING, OUTER_STYLE) - .message(tr("Area style on outer way")) + errors.add(TestError.builder(this, Severity.ERROR, OUTER_STYLE) + .message(tr("Area style repeated on outer way")) .primitives(Arrays.asList(r, wOuter)) .highlight(wOuter) .build()); @@ -626,7 +626,7 @@ * @param crossingWays list to collect crossing ways * @param findSharedWaySegments true: find shared way segments instead of crossings */ - private static void findIntersectingWay(Way w, Map> cellSegments, + static void findIntersectingWay(Way w, Map> cellSegments, Map, List> crossingWays, boolean findSharedWaySegments) { int nodesSize = w.getNodesCount(); for (int i = 0; i < nodesSize - 1; i++) { Index: src/org/openstreetmap/josm/data/validation/tests/OverlappingAreas.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/OverlappingAreas.java (nonexistent) +++ src/org/openstreetmap/josm/data/validation/tests/OverlappingAreas.java (working copy) @@ -0,0 +1,376 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.validation.tests; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.awt.geom.Area; +import java.awt.geom.PathIterator; +import java.awt.geom.Point2D; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.openstreetmap.josm.data.coor.EastNorth; +import org.openstreetmap.josm.data.osm.BBox; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmDataManager; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.osm.WaySegment; +import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; +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.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.tools.Geometry; +import org.openstreetmap.josm.tools.Geometry.PolygonIntersection; +import org.openstreetmap.josm.tools.MultiMap; +import org.openstreetmap.josm.tools.Pair; + +/** + * Tests if there are overlapping areas + * + * @author Gerd Petermann + * @since xxx + */ +public class OverlappingAreas extends Test { + protected static final int OVERLAPPING_AREA = 3800; // allows insideness + protected static final int OVERLAPPING_WATER = 3801; + protected static final int OVERLAPPING_IDENTICAL_NATURAL = 3802; + protected static final int OVERLAPPING_IDENTICAL_LANDLUSE = 3803; + protected static final int OVERLAPPING_BUILDINGS = 3804; + protected static final int OVERLAPPING_BUILDING_RESIDENTIAL = 3805; + + private DataSet ds; + // collections to suppress duplicate tests + private MultiMap mpWayMap; + private Set seenWays; + private Set seenRelations; + + /** Constructor */ + public OverlappingAreas() { + super(tr("Overlapping areas"), + tr("This test checks if two areas intersect in the same layer.")); + } + + @Override + public void startTest(ProgressMonitor monitor) { + super.startTest(monitor); + super.setShowElements(true); + // collections to suppress duplicated tests */ + seenWays = new HashSet<>(); + seenRelations = new HashSet<>(); + mpWayMap = new MultiMap<>(); + ds = OsmDataManager.getInstance().getEditDataSet(); + } + + @Override + public void endTest() { + // clean up references + seenRelations = null; + seenWays = null; + mpWayMap = null; + ds = null; + super.endTest(); + } + + /** + * Check intersection between two areas. If empty or extremely small ignore it, else add error + * with highlighted area. + * @param a1 the first area (east/north space) + * @param a2 the second area (east/north space) + * @param p1 primitive describing 1st polygon + * @param p2 primitive describing 2nd polygon + * @param code the error code + */ + private void checkIntersection(Area a1, Area a2, OsmPrimitive p1, OsmPrimitive p2, int code) { + Pair pair = Geometry.polygonIntersectionResult(a1, a2, Geometry.INTERSECTION_EPS_EAST_NORTH); + switch (pair.a) { + case OUTSIDE: + return; + case FIRST_INSIDE_SECOND: + if (code == OVERLAPPING_AREA || code == OVERLAPPING_BUILDING_RESIDENTIAL && isBuilding(p1)) + return; + if (code == OVERLAPPING_WATER && p2.hasTag("natural", "wetland")) + return; + break; + case SECOND_INSIDE_FIRST: + if (code == OVERLAPPING_AREA || code == OVERLAPPING_BUILDING_RESIDENTIAL && isBuilding(p2)) + return; + if (code == OVERLAPPING_WATER && p1.hasTag("natural", "wetland")) + return; + break; + case CROSSING: + break; + } + + final String reason = getReason(code, pair.a == PolygonIntersection.CROSSING); + errors.add(TestError.builder(this, code == OVERLAPPING_AREA ? Severity.OTHER : Severity.WARNING, code) + .message("OA: " + reason).primitives(p1, p2) + .highlightNodePairs(getHiliteNodesForArea(pair.b)) + .build()); + } + + /** + * Calculate list of node pairs describing the intersection area. + * @param intersection the intersection area + * @return list of node pairs describing the intersection area + */ + private static List> getHiliteNodesForArea(Area intersection) { + List> hilite = new ArrayList<>(); + PathIterator pit = intersection.getPathIterator(null); + double[] res = new double[6]; + List nodes = new ArrayList<>(); + while (!pit.isDone()) { + int type = pit.currentSegment(res); + Node n = new Node(new EastNorth(res[0], res[1])); + switch (type) { + case PathIterator.SEG_MOVETO: + if (!nodes.isEmpty()) { + hilite.add(nodes); + } + nodes = new ArrayList<>(); + nodes.add(n); + break; + case PathIterator.SEG_LINETO: + nodes.add(n); + break; + case PathIterator.SEG_CLOSE: + if (!nodes.isEmpty()) { + nodes.add(nodes.get(0)); + hilite.add(nodes); + nodes = new ArrayList<>(); + } + break; + default: + break; + } + pit.next(); + } + if (nodes.size() > 1) { + hilite.add(nodes); + } + return hilite; + } + + /** + * Check if the two objects are allowed to overlap regarding tags. + * @param p1 1st primitive + * @param p2 2nd primitive + * @return the error code if the tags of the objects don't allow that the areas overlap, else 0 + */ + private static int tagsAllowOverlap(OsmPrimitive p1, OsmPrimitive p2) { + if (!inSameLayer(p1, p2)) { + return 0; + } + // order is significant, highest severity should come first + if (isWaterArea(p1) && isWaterArea(p2)) { + return OVERLAPPING_WATER; + } + if (isSetAndEqual(p1, p2, "natural")) { + return OVERLAPPING_IDENTICAL_NATURAL; + } + if (isSetAndEqual(p1, p2, "landuse")) { + return OVERLAPPING_IDENTICAL_LANDLUSE; + } + if (isBuilding(p1) && isBuilding(p2)) { + return OVERLAPPING_BUILDINGS; + } + if (isBuilding(p1) && isResidentialArea(p2) || isBuilding(p2) && isResidentialArea(p1)) { + return OVERLAPPING_BUILDING_RESIDENTIAL; + } + if (ValidatorPrefHelper.PREF_OTHER.get() && p1.concernsArea() && p2.concernsArea()) { + return OVERLAPPING_AREA; + } + return 0; + } + + private static boolean isWaterArea(OsmPrimitive p) { + return p.hasTag("natural", "water", "wetland") || p.hasTag("landuse", "reservoir"); + } + + private static boolean isSetAndEqual(OsmPrimitive p1, OsmPrimitive p2, String key) { + String v1 = p1.get(key); + String v2 = p2.get(key); + return v1 != null && v1.equals(v2); + } + + private Area checkDetailsWithArea(Area a1, OsmPrimitive p1, OsmPrimitive p2) { + int code = tagsAllowOverlap(p1, p2); + if (code > 0) { + // check geometry details + if (a1 == null) { + a1 = Geometry.getArea(p1); + } + checkIntersection(a1, Geometry.getArea(p2), p1, p2, code); + } + return a1; + } + + @Override + public void visit(Way w1) { + if (w1.isArea()) { + /** performance: calculate area only if needed and only once */ + Area a1 = null; + BBox bbox1 = w1.getBBox(); + List nearWays = ds.searchWays(bbox1); + // way-way overlaps + for (Way w2 : nearWays) { + if (w1 != w2 && w2.isArea() && !seenWays.contains(w2)) { + a1 = checkDetailsWithArea(a1, w1, w2); + } + } + // way-multipolygon overlaps + if (partialSelection) { + List nearRelations = ds.searchRelations(bbox1); + for (Relation rel : nearRelations) { + if (rel.isMultipolygon() && w1.referrers(Relation.class).noneMatch(parent -> parent == rel)) { + Set checkedWays = mpWayMap.get(rel); + if (checkedWays != null && checkedWays.contains(w1)) + continue; + mpWayMap.put(rel, w1); + if (!rel.isIncomplete() && !rel.hasIncompleteMembers()) { + a1 = checkDetailsWithArea(a1, w1, rel); + } else { + int code = tagsAllowOverlap(w1, rel); + if (code > 0) { + checkWayCrossingRelation(code, w1, rel, null, w1); + } + } + } + } + } + seenWays.add(w1); + } + if (partialSelection) { + // if the member of a relation is selected, visit also the relation ? + // w1.referrers(Relation.class).filter(Relation::isSelected).forEach(this::visit); + } + } + + @Override + public void visit(Relation r1) { + if (r1.isMultipolygon()) { + BBox bbox1 = r1.getBBox(); + /** performance: calculate area only if needed and only once */ + Area a1 = null; + // multipolygon -way overlaps + List nearWays = ds.searchWays(bbox1); + List nearRelations = ds.searchRelations(bbox1); + if (nearWays.isEmpty() && nearRelations.isEmpty()) + return; + + /** All way segments, grouped by cells */ + final Map> mpCellSegments = new HashMap<>(1000); + /** The already detected ways in error */ + final Map, List> problemWays = new HashMap<>(50); + for (Way relWay : r1.getMemberPrimitives(Way.class)) { + MultipolygonTest.findIntersectingWay(relWay, mpCellSegments, problemWays, false); + } + if (!problemWays.isEmpty()) { + // invalid multipolygon + return; + } + + for (Way way : nearWays) { + if (!way.isArea() || way.referrers(Relation.class).anyMatch(parent -> parent == r1)) { + // ignore members of current multipolygon + continue; + } + if (partialSelection) { + Set checkedWays = mpWayMap.get(r1); + if (checkedWays != null && checkedWays.contains(way)) + continue; + mpWayMap.put(r1, way); + } + if (r1.isIncomplete() || r1.hasIncompleteMembers()) { + int code = tagsAllowOverlap(r1, way); + if (code > 0) { + checkWayCrossingRelation(code, way, r1, mpCellSegments, way); + } + } else { + a1 = checkDetailsWithArea(a1, r1, way); + } + } + for (Relation r2 : nearRelations) { + if (r1 != r2 && r2.isMultipolygon() && !seenRelations.contains(r2)) { + if (!r1.isIncomplete() && !r1.hasIncompleteMembers() && !r2.isIncomplete() + && !r2.hasIncompleteMembers()) { + a1 = checkDetailsWithArea(a1, r1, r2); + } else { + int code = tagsAllowOverlap(r1, r2); + if (code > 0) { + for (Way way : r2.getMemberPrimitives(Way.class)) { + checkWayCrossingRelation(code, way, r1, mpCellSegments, r2); + } + } + } + } + } + seenRelations.add(r1); + } + } + + /** + * Check if a way crosses any of the multipolygon relation members. + * @param code the error code to use if a crossing is found + * @param way the way to check + * @param rel the multipolygon relation + * @param mpCellSegments optional index for way segments for the multipolygon ways, can be null + * @param p2 gives the 2nd primitive to use in the error message (which might be the parent relation of the way) + */ + private void checkWayCrossingRelation(int code, Way way, Relation rel, Map> mpCellSegments, OsmPrimitive p2) { + final Map, List> problemWays = new HashMap<>(50); + Map> cellSegments = new HashMap<>(); + if (mpCellSegments == null) { + for (Way relWay : rel.getMemberPrimitives(Way.class)) { + MultipolygonTest.findIntersectingWay(relWay, cellSegments, problemWays, false); + } + } else { + cellSegments.putAll(mpCellSegments); + } + MultipolygonTest.findIntersectingWay(way, cellSegments, problemWays, false); + // TODO: Doesn't find overlaps at shared nodes. + // Maybe add check similar to MultipolygonTest.checkOverlapAtSharedNodes() if way.isArea() is true + if (!problemWays.isEmpty()) { + final String reason = getReason(code, true); + for (List waySegments : problemWays.values()) { + errors.add(TestError.builder(this, code == OVERLAPPING_AREA ? Severity.OTHER : Severity.WARNING, code) + .message("OA: " + reason).primitives(rel, p2) + .highlightWaySegments(waySegments) + .build()); + + } + } + } + + /** + * Create message string. Should only be called if an intersection is found. + * @param code the reason code + * @param isCrossing if false it is assumed that one object is inside the other + * @return the message + */ + private static String getReason(int code, boolean isCrossing) { + // + switch (code) { + case OVERLAPPING_WATER: + return isCrossing ? tr("Overlapping Water Areas") : tr("Water Area inside Water Area"); + case OVERLAPPING_IDENTICAL_NATURAL: + return tr("Overlapping identical natural areas"); + case OVERLAPPING_IDENTICAL_LANDLUSE: + return tr("Overlapping identical landuses"); + case OVERLAPPING_BUILDINGS: + return isCrossing ? tr("Overlapping buildings") : tr("Building inside building"); + case OVERLAPPING_BUILDING_RESIDENTIAL: + return tr("Overlapping building/residential area"); + default: + return tr("Overlapping area"); + } + + } +} Index: src/org/openstreetmap/josm/tools/Geometry.java =================================================================== --- src/org/openstreetmap/josm/tools/Geometry.java (revision 15056) +++ src/org/openstreetmap/josm/tools/Geometry.java (working copy) @@ -526,6 +526,28 @@ } /** + * Calculate area in east/north space for given primitive. + * @param p the primitive + * @return the area in east/north space, might be empty if the primitive is incomplete or a node + * since xxx + */ + public static Area getArea(IPrimitive p) { + if (p instanceof Way) { + return getArea(((Way) p).getNodes()); + } + if (p instanceof Relation && !p.isIncomplete()) { + Multipolygon mp = new Multipolygon((Relation) p); + Path2D path = new Path2D.Double(); + path.setWindingRule(Path2D.WIND_EVEN_ODD); + for (Multipolygon.PolyData pd : mp.getCombinedPolygons()) { + path.append(pd.get(), false); + } + return new Area(path); + } + return new Area(); + } + + /** * Builds a path from a list of nodes * @param polygon Nodes, forming a closed polygon * @param path2d path to add to; can be null, then a new path is created @@ -598,18 +620,29 @@ * @return intersection kind */ public static PolygonIntersection polygonIntersection(Area a1, Area a2, double eps) { + return polygonIntersectionResult(a1, a2, eps).a; + } + /** + * Calculate intersection area and kind of intersection between two polygons. + * @param a1 Area of first polygon + * @param a2 Area of second polygon + * @param eps an area threshold, everything below is considered an empty intersection + * @return pair with intersection kind and intersection area (never null, but maybe empty) + * @since xxx + */ + public static Pair polygonIntersectionResult(Area a1, Area a2, double eps) { Area inter = new Area(a1); inter.intersect(a2); if (inter.isEmpty() || !checkIntersection(inter, eps)) { - return PolygonIntersection.OUTSIDE; + return new Pair<>(PolygonIntersection.OUTSIDE, inter); } else if (a2.getBounds2D().contains(a1.getBounds2D()) && inter.equals(a1)) { - return PolygonIntersection.FIRST_INSIDE_SECOND; + return new Pair<>(PolygonIntersection.FIRST_INSIDE_SECOND, inter); } else if (a1.getBounds2D().contains(a2.getBounds2D()) && inter.equals(a2)) { - return PolygonIntersection.SECOND_INSIDE_FIRST; + return new Pair<>(PolygonIntersection.SECOND_INSIDE_FIRST, inter); } else { - return PolygonIntersection.CROSSING; + return new Pair<>(PolygonIntersection.CROSSING, inter); } }