Changeset 15959 in josm for trunk/src/org/openstreetmap

Timestamp:

2020-02-29T08:07:39+01:00 (5 years ago)

Author:

GerdP

Message:

fix #13165 Validator did not warn about overlapping multipolygons

• let CrossingFinder accept multipolygons
• improve highlighting of overlapping areas
• if incomplete multipolygons are tested the area calculation is used unless open ends are found, else the same algortihm as in CrossingWays is used to hilite the crossing segments. In the later case overlaps with shared nodes are not (yet) found.

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/validation/tests/CrossingWays.java (modified) (1 diff)
• data/validation/tests/MapCSSTagChecker.java (modified) (9 diffs)
• data/validation/tests/MultipolygonTest.java (modified) (2 diffs)
• gui/mappaint/Environment.java (modified) (5 diffs)
• gui/mappaint/mapcss/Selector.java (modified) (5 diffs)
• tools/Geometry.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java

@@ -428 +428 @@
         return cells;
     }
+
+    /**
+     * Find ways which are crossing without sharing a node.
+     * @param w way that is to be checked
+     * @param cellSegments map with already collected way segments
+     * @param crossingWays map to collect crossing ways and related segments
+     * @param findSharedWaySegments true: find shared way segments instead of crossings
+     */
+    public static void findIntersectingWay(Way w, Map<Point2D, List<WaySegment>> cellSegments,
+            Map<List<Way>, List<WaySegment>> crossingWays, boolean findSharedWaySegments) {
+        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) {
+                Logging.warn("Crossing ways test skipped " + es1);
+                continue;
+            }
+            for (List<WaySegment> segments : CrossingWays.getSegments(cellSegments, en1, en2)) {
+                for (WaySegment es2 : segments) {
+
+                    List<WaySegment> highlight;
+                    if (es2.way == w // reported by CrossingWays.SelfIntersection
+                            || (findSharedWaySegments && !es1.isSimilar(es2))
+                            || (!findSharedWaySegments && !es1.intersects(es2)))
+                        continue;
+
+                    List<Way> prims = Arrays.asList(es1.way, es2.way);
+                    if ((highlight = crossingWays.get(prims)) == null) {
+                        highlight = new ArrayList<>();
+                        highlight.add(es1);
+                        highlight.add(es2);
+                        crossingWays.put(prims, highlight);
+                    } else {
+                        highlight.add(es1);
+                        highlight.add(es2);
+                    }
+                }
+                segments.add(es1);
+            }
+        }
+    }
+
 }

trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

@@ -44 +44 @@
 import org.openstreetmap.josm.data.osm.Tag;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.WaySegment;
 import org.openstreetmap.josm.data.preferences.sources.SourceEntry;
 import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
@@ -93 +94 @@
  */
 public class MapCSSTagChecker extends Test.TagTest {
-    MapCSSTagCheckerIndex indexData;
-    final Set<OsmPrimitive> tested = new HashSet<>();
+    private MapCSSTagCheckerIndex indexData;
+    private final Set<OsmPrimitive> tested = new HashSet<>();
+    final Map<IPrimitive, Area> mpAreaCache = new HashMap<>();
 
     /**
@@ -202 +204 @@
                 @Override
                 public Command createCommand(OsmPrimitive p, Selector matchingSelector) {
-                    final Tag tag = Tag.ofString(evaluateObject(obj, p, matchingSelector));
+                    final Tag tag = Tag.ofString(FixCommand.evaluateObject(obj, p, matchingSelector));
                     return new ChangePropertyCommand(p, tag.getKey(), tag.getValue());
                 }
@@ -223 +225 @@
                 @Override
                 public Command createCommand(OsmPrimitive p, Selector matchingSelector) {
-                    final String key = evaluateObject(obj, p, matchingSelector);
+                    final String key = FixCommand.evaluateObject(obj, p, matchingSelector);
                     return new ChangePropertyCommand(p, key, "");
                 }
@@ -620 +622 @@
                                 errorBuilder = errorBuilder.fix(() -> fix);
                             }
+                            // check if we have special information about highlighted objects */
+                            boolean hiliteFound = false;
                             if (env.intersections != null) {
                                 Area is = env.intersections.get(c);
                                 if (is != null) {
                                     errorBuilder = errorBuilder.highlight(is);
+                                    hiliteFound = true;
+                                }
+                            }
+                            if (env.crossingWaysMap != null && !hiliteFound) {
+                                Map<List<Way>, List<WaySegment>> is = env.crossingWaysMap.get(c);
+                                if (is != null) {
+                                    Set<WaySegment> toHilite = new HashSet<>();
+                                    for (List<WaySegment> wsList : is.values()) {
+                                        toHilite.addAll(wsList);
+                                    }
+                                    errorBuilder = errorBuilder.highlightWaySegments(toHilite);
                                 }
                             }
@@ -709 +724 @@
 
         Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
+        env.mpAreaCache = mpAreaCache;
+
         // the declaration indices are sorted, so it suffices to save the last used index
         Declaration lastDeclUsed = null;
@@ -775 +792 @@
     }
 
-    private static Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity,
+    private Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity,
             Collection<Set<TagCheck>> checksCol) {
+        // this variant is only used by the assertion tests
         final List<TestError> r = new ArrayList<>();
         final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
+        env.mpAreaCache = mpAreaCache;
         for (Set<TagCheck> schecks : checksCol) {
             for (TagCheck check : schecks) {
@@ -992 +1011 @@
         }
         tested.clear();
+        mpAreaCache.clear();
     }
 
@@ -1023 +1043 @@
             tested.clear();
         }
-        super.endTest();
         // no need to keep the index, it is quickly build and doubles the memory needs
         indexData = null;
+        // always clear the cache to make sure that we catch changes in geometry
+        mpAreaCache.clear();
+        super.endTest();
     }
 }

trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java

@@ -37 +37 @@
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Geometry.PolygonIntersection;
-import org.openstreetmap.josm.tools.Logging;
 
 /**
@@ -626 +625 @@
         for (Way w: r.getMemberPrimitives(Way.class)) {
             if (!w.hasIncompleteNodes()) {
-                findIntersectingWay(w, cellSegments, crossingWays, findSharedWaySegments);
+                CrossingWays.findIntersectingWay(w, cellSegments, crossingWays, findSharedWaySegments);
             }
         }
         return crossingWays;
-    }
-
-    /**
-     * Find ways which are crossing without sharing a node.
-     * @param w way that is member of the relation
-     * @param cellSegments map with already collected way segments
-     * @param crossingWays map to collect crossing ways and related segments
-     * @param findSharedWaySegments true: find shared way segments instead of crossings
-     */
-    private static void findIntersectingWay(Way w, Map<Point2D, List<WaySegment>> cellSegments,
-            Map<List<Way>, List<WaySegment>> crossingWays, boolean findSharedWaySegments) {
-        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) {
-                Logging.warn("Crossing ways test (MP) skipped " + es1);
-                continue;
-            }
-            for (List<WaySegment> segments : CrossingWays.getSegments(cellSegments, en1, en2)) {
-                for (WaySegment es2 : segments) {
-
-                    List<WaySegment> highlight;
-                    if (es2.way == w // reported by CrossingWays.SelfIntersection
-                            || (findSharedWaySegments && !es1.isSimilar(es2))
-                            || (!findSharedWaySegments && !es1.intersects(es2)))
-                        continue;
-
-                    List<Way> prims = Arrays.asList(es1.way, es2.way);
-                    if ((highlight = crossingWays.get(prims)) == null) {
-                        highlight = new ArrayList<>();
-                        highlight.add(es1);
-                        highlight.add(es2);
-                        crossingWays.put(prims, highlight);
-                    } else {
-                        highlight.add(es1);
-                        highlight.add(es2);
-                    }
-                }
-                segments.add(es1);
-            }
-        }
     }
 

trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java

@@ -3 +3 @@
 
 import java.awt.geom.Area;
-import java.util.HashMap;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -10 +10 @@
 import org.openstreetmap.josm.data.osm.IPrimitive;
 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.gui.mappaint.mapcss.Condition.Context;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.LinkSelector;
@@ -72 +74 @@
 
     /**
+     * Crossing ways result from CrossingFinder, filled for incomplete ways/relations
+    */
+    public Map<IPrimitive, Map<List<Way>, List<WaySegment>>> crossingWaysMap;
+
+    /**
      * Intersection areas (only filled with CrossingFinder if children is not null)
      */
     public Map<IPrimitive, Area> intersections;
+
+    /**
+     * Cache for multipolygon areas, can be null, used with CrossingFinder
+     */
+    public Map<IPrimitive, Area> mpAreaCache;
 
     /**
@@ -126 +138 @@
         this.context = other.getContext();
         this.children = other.children == null ? null : new LinkedHashSet<>(other.children);
-        this.intersections = other.intersections == null ? null : new HashMap<>(other.intersections);
+        this.intersections = other.intersections;
+        this.crossingWaysMap = other.crossingWaysMap;
+        this.mpAreaCache = other.mpAreaCache;
     }
 
@@ -294 +308 @@
         children = null;
         intersections = null;
+        crossingWaysMap = null;
     }
 

trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java

@@ -5 +5 @@
 
 import java.awt.geom.Area;
+import java.awt.geom.Point2D;
 import java.text.MessageFormat;
 import java.util.ArrayList;
@@ -12 +13 @@
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.function.IntFunction;
@@ -26 +28 @@
 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;
 import org.openstreetmap.josm.data.osm.visitor.PrimitiveVisitor;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache;
+import org.openstreetmap.josm.data.validation.tests.CrossingWays;
 import org.openstreetmap.josm.gui.mappaint.Environment;
 import org.openstreetmap.josm.gui.mappaint.Range;
@@ -303 +308 @@
             private final String layer;
             private Area area;
+            /** Will contain all way segments, grouped by cells */
+            Map<Point2D, List<WaySegment>> cellSegments;
 
             private CrossingFinder(Environment e) {
                 super(e);
-                CheckParameterUtil.ensureThat(e.osm instanceof IWay, "Only ways are supported");
+                CheckParameterUtil.ensureThat(isArea(e.osm), "Only areas are supported");
                 layer = OsmUtils.getLayer(e.osm);
             }
 
-            @Override
-            public void visit(IWay<?> w) {
-                if (Objects.equals(layer, OsmUtils.getLayer(w))
-                    && left.matches(new Environment(w).withParent(e.osm))
-                    && e.osm instanceof IWay) {
-                    if (area == null) {
-                        area = Geometry.getAreaEastNorth(e.osm);
-                    }
-                    Pair<PolygonIntersection, Area> is = Geometry.polygonIntersectionResult(
-                            Geometry.getAreaEastNorth(w), area, Geometry.INTERSECTION_EPS_EAST_NORTH);
-                    if (Geometry.PolygonIntersection.CROSSING == is.a) {
-                        addToChildren(e, w);
-                        // store intersection area to improve highlight and zoom to problem
-                        if (e.intersections == null) {
-                            e.intersections = new HashMap<>();
+            private Area getAreaEastNorth(IPrimitive p, Environment e) {
+                if (e.mpAreaCache != null && p.isMultipolygon()) {
+                    Area a = e.mpAreaCache.get(p);
+                    if (a == null) {
+                        a = Geometry.getAreaEastNorth(p);
+                        e.mpAreaCache.put(p, a);
+                    }
+                    return a;
+                }
+                return Geometry.getAreaEastNorth(p);
+            }
+
+            private Map<List<Way>, List<WaySegment>> findCrossings(IPrimitive area,
+                    Map<Point2D, List<WaySegment>> cellSegments) {
+                /** The detected crossing ways */
+                Map<List<Way>, List<WaySegment>> crossingWays = new HashMap<>(50);
+                if (area instanceof Way) {
+                    CrossingWays.findIntersectingWay((Way) area, cellSegments, crossingWays, false);
+                } else if (area instanceof Relation && area.isMultipolygon()) {
+                    Relation r = (Relation) area;
+                    for (Way w : r.getMemberPrimitives(Way.class)) {
+                        if (!w.hasIncompleteNodes()) {
+                            CrossingWays.findIntersectingWay(w, cellSegments, crossingWays, false);
                         }
-                        e.intersections.put(w, is.b);
+                    }
+                }
+                return crossingWays;
+            }
+
+            @Override
+            public void visit(Collection<? extends IPrimitive> primitives) {
+                List<? extends IPrimitive> toIgnore;
+                if (e.osm instanceof Relation) {
+                    toIgnore = ((IRelation<?>) e.osm).getMemberPrimitivesList();
+                } else {
+                    toIgnore = null;
+                }
+
+                for (IPrimitive p : primitives) {
+                    if (isPrimitiveUsable(p) && Objects.equals(layer, OsmUtils.getLayer(p))
+                            && left.matches(new Environment(p).withParent(e.osm)) && isArea(p)
+                            && (toIgnore == null || !toIgnore.contains(p))) {
+                        if (area == null) {
+                            area = getAreaEastNorth(e.osm, e);
+                        }
+                        Area otherArea = getAreaEastNorth(p, e);
+                        if (area.isEmpty() || otherArea.isEmpty()) {
+                            if (cellSegments == null) {
+                                // lazy initialisation
+                                cellSegments = new HashMap<>();
+                                findCrossings(e.osm, cellSegments); // ignore self intersections etc. here
+                            }
+                            // need a copy
+                            final Map<Point2D, List<WaySegment>> tmpCellSegments = new HashMap<>(cellSegments);
+                            // calculate all crossings between e.osm and p
+                            Map<List<Way>, List<WaySegment>> crossingWays = findCrossings(p, tmpCellSegments);
+                            if (!crossingWays.isEmpty()) {
+                                addToChildren(e, p);
+                                if (e.crossingWaysMap == null) {
+                                    e.crossingWaysMap = new HashMap<>();
+                                }
+                                e.crossingWaysMap.put(p, crossingWays);
+                            }
+                        } else {
+                            // we have complete data. This allows to find intersections with shared nodes
+                            // See #16707
+                            Pair<PolygonIntersection, Area> is = Geometry.polygonIntersectionResult(
+                                    otherArea, area, Geometry.INTERSECTION_EPS_EAST_NORTH);
+                            if (Geometry.PolygonIntersection.CROSSING == is.a) {
+                                addToChildren(e, p);
+                                // store intersection area to improve highlight and zoom to problem
+                                if (e.intersections == null) {
+                                    e.intersections = new HashMap<>();
+                                }
+                                e.intersections.put(p, is.b);
+                            }
+                        }
                     }
                 }
@@ -453 +520 @@
                 return ChildOrParentSelectorType.SUPERSET_OR_EQUAL == type ? e.children != null : e.children == null;
 
-            } else if (ChildOrParentSelectorType.CROSSING == type && e.osm instanceof IWay) {
+            } else if (ChildOrParentSelectorType.CROSSING == type) {
                 e.parent = e.osm;
-                if (right instanceof OptimizedGeneralSelector
-                        && e.osm.getDataSet() != null
-                        && ((OptimizedGeneralSelector) right).matchesBase(OsmPrimitiveType.WAY)) {
+                if (e.osm.getDataSet() != null && isArea(e.osm)) {
                     final CrossingFinder crossingFinder = new CrossingFinder(e);
-                    crossingFinder.visit(e.osm.getDataSet().searchWays(e.osm.getBBox()));
+                    visitBBox(e, crossingFinder);
+                    return e.children != null;
                 }
                 return e.children != null;

trunk/src/org/openstreetmap/josm/tools/Geometry.java

@@ -562 +562 @@
             return Geometry.getArea(((Way) p).getNodes());
         }
-        if (p instanceof Relation && p.isMultipolygon() && !p.isIncomplete()
-                && !((Relation) p).hasIncompleteMembers()) {
+        if (p instanceof Relation && p.isMultipolygon() && !p.isIncomplete()) {
             Multipolygon mp = MultipolygonCache.getInstance().get((Relation) p);
-            Path2D path = new Path2D.Double();
-            path.setWindingRule(Path2D.WIND_EVEN_ODD);
-            for (PolyData pd : mp.getCombinedPolygons()) {
-                path.append(pd.get(), false);
-            }
-            return new Area(path);
+            if (mp.getOpenEnds().isEmpty()) {
+                Path2D path = new Path2D.Double();
+                path.setWindingRule(Path2D.WIND_EVEN_ODD);
+                for (PolyData pd : mp.getCombinedPolygons()) {
+                    path.append(pd.get(), false);
+                }
+                return new Area(path);
+            }
         }
         return new Area();