Ticket #13165: 13165_v2.patch

src/org/openstreetmap/josm/data/validation/TestError.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.data.validation;
 
+import java.awt.geom.Area;
+import java.awt.geom.PathIterator;
 import java.text.MessageFormat;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -11 +14 @@
 import java.util.function.Supplier;
 
 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.OsmUtils;
@@ -188 +192 @@
         }
 
         /**
+         * Sets an area to highlight when selecting this error.
+         *
+         * @param highlighted the area to highlight
+         * @return {@code this}
+         */
+        public Builder highlight(Area highlighted) {
+            CheckParameterUtil.ensureParameterNotNull(highlighted, "highlighted");
+            this.highlighted = Collections.singleton(highlighted);
+            return this;
+        }
+
+        /**
          * Sets a supplier to obtain a command to fix the error.
          *
          * @param fixingCommand the fix supplier. Can be null
@@ -421 +437 @@
                 v.visit((WaySegment) o);
             } else if (o instanceof List<?>) {
                 v.visit((List<Node>) o);
+            } else if (o instanceof Area) {
+                for (List<Node> l : getHiliteNodesForArea((Area) o)) {
+                    v.visit(l);
+                }
             }
         }
     }
 
     /**
+     * Calculate list of node pairs describing the area.
+     * @param area the area
+     * @return list of node pairs describing the area
+     */
+    private  static List<List<Node>> getHiliteNodesForArea(Area area) {
+        List<List<Node>> hilite = new ArrayList<>();
+        PathIterator pit = area.getPathIterator(null);
+        double[] res = new double[6];
+        List<Node> 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;
+    }
+
+
+    /**
      * Returns the selection flag of this error
      * @return true if this error is selected
      * @since 5671

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

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.Rectangle;
+import java.awt.geom.Area;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStream;
@@ -771 +772 @@
                             if (fix != null) {
                                 errorBuilder = errorBuilder.fix(() -> fix);
                             }
+                            if (env.intersections != null) {
+                                Area is = env.intersections.get(c);
+                                if (is != null) {
+                                    errorBuilder = errorBuilder.highlight(is);
+                                }
+                            }
                             res.add(errorBuilder.primitives(p, (OsmPrimitive) c).build());
                         }
                     }
@@ -906 +913 @@
                 if (e.getCode() == toAdd.getCode() && e.getMessage().equals(toAdd.getMessage())
                         && e.getPrimitives().size() == toAdd.getPrimitives().size()
                         && e.getPrimitives().containsAll(toAdd.getPrimitives())
-                        && e.getHighlighted().size() == toAdd.getHighlighted().size()
-                        && e.getHighlighted().containsAll(toAdd.getHighlighted())) {
+                        && highlightedIsEqual(e.getHighlighted(), toAdd.getHighlighted())) {
                     isDup = true;
                     break;
                 }
@@ -917 +923 @@
             errors.add(toAdd);
     }
 
+    private static boolean highlightedIsEqual(Collection<?> highlighted, Collection<?> highlighted2) {
+        if (highlighted.size() == highlighted2.size()) {
+            if (!highlighted.isEmpty()) {
+                Object h1 = highlighted.iterator().next();
+                Object h2 = highlighted2.iterator().next();
+                if (h1 instanceof Area && h2 instanceof Area) {
+                    return ((Area) h1).equals((Area) h2);
+                }
+                return highlighted.containsAll(highlighted2);
+            }
+            return true;
+        }
+        return false;
+    }
+
     private static Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity,
             Collection<Set<TagCheck>> checksCol) {
         final List<TestError> r = new ArrayList<>();

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

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.mappaint;
 
+import java.awt.geom.Area;
+import java.util.HashMap;
 import java.util.LinkedHashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.openstreetmap.josm.data.osm.IPrimitive;
@@ -68 +71 @@
     public Set<IPrimitive> children;
 
     /**
+     * Intersection areas (only filled with CrossingFinder if children is not null)
+     */
+    public Map<IPrimitive, Area> intersections;
+
+    /**
      * Creates a new uninitialized environment.
      */
     public Environment() {
@@ -117 +125 @@
         this.count = other.count;
         this.context = other.getContext();
         this.children = other.children == null ? null : new LinkedHashSet<>(other.children);
+        this.intersections = other.intersections == null ? null : new HashMap<>(other.intersections);
     }
 
     /**
@@ -283 +292 @@
         index = null;
         count = null;
         children = null;
+        intersections = null;
     }
 
     /**

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

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.data.projection.Ellipsoid.WGS84;
 
+import java.awt.geom.Area;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Objects;
@@ -29 +31 @@
 import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.OpenEndPseudoClassCondition;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 import org.openstreetmap.josm.tools.Geometry;
+import org.openstreetmap.josm.tools.Geometry.PolygonIntersection;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Utils;
@@ -294 +297 @@
         private final class CrossingFinder extends AbstractFinder {
 
             private final String layer;
+            private Area area;
 
             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 && Geometry.PolygonIntersection.CROSSING.equals(
-                            Geometry.polygonIntersection(w.getNodes(), ((IWay<?>) e.osm).getNodes()))) {
-                    addToChildren(e, w);
+            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 = Geometry.getArea(e.osm);
+                        }
+                        Pair<PolygonIntersection, Area> is = Geometry.polygonIntersectionResult(Geometry.getArea(p),
+                                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);
+                        }
+                    }
                 }
             }
         }
@@ -403 +425 @@
 
         private static boolean isArea(IPrimitive p) {
             return (p instanceof IWay && ((IWay<?>) p).isClosed() && ((IWay<?>) p).getNodesCount() >= 4)
-                    || (p instanceof IRelation && p.isMultipolygon() && !p.isIncomplete());
+                    || (p instanceof IRelation && p.isMultipolygon() && !p.isIncomplete()
+                            && !((IRelation<?>) p).hasIncompleteMembers());
         }
 
         @Override
@@ -438 +461 @@
                 visitBBox(e, insideOrEqualFinder);
                 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
-                        && ((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;
             } else if (ChildOrParentSelectorType.SIBLING == type) {
                 if (e.osm instanceof INode) {
                     for (IPrimitive ref : e.osm.getReferrers(true)) {

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

@@ -29 +29 @@
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.INode;
 import org.openstreetmap.josm.data.osm.IPrimitive;
+import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.IWay;
 import org.openstreetmap.josm.data.osm.MultipolygonBuilder;
 import org.openstreetmap.josm.data.osm.MultipolygonBuilder.JoinedPolygon;
@@ -528 +529 @@
     }
 
     /**
+     * Calculate area in east/north space for given primitive. Slow for complex multipolygon relations!
+     * @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.isMultipolygon() && !p.isIncomplete() && !((IRelation<?>) p).hasIncompleteMembers()) {
+            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
@@ -600 +623 @@
      * @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<PolygonIntersection, Area> 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);
         }
     }
 
@@ -1102 +1136 @@
         if (primitives.isEmpty())
             return res;
 
-        final Pair<List<JoinedPolygon>, List<JoinedPolygon>> outerInner;
-        try {
-            outerInner = MultipolygonBuilder.joinWays(multiPolygon);
-        } catch (MultipolygonBuilder.JoinedPolygonCreationException ex) {
-            Logging.trace(ex);
-            Logging.debug("Invalid multipolygon " + multiPolygon);
-            return res;
-        }
+        Area mpArea = null;
 
         Set<OsmPrimitive> members = multiPolygon.getMemberPrimitives();
         for (IPrimitive p : primitives) {
             if (members.contains(p))
                 continue;
+            if (mpArea == null) {
+                mpArea = getArea(multiPolygon);
+            }
             if (p instanceof Node) {
-                if (isPolygonInsideMultiPolygon(Collections.singletonList((Node) p), outerInner, null)) {
-                    res.add(p);
+                if (((Node) p).isLatLonKnown()) {
+                    EastNorth en = ((Node) p).getEastNorth();
+                    if (mpArea.contains(en.getX(), en.getY())) {
+                        res.add(p);
+                    }
                 }
             } else if (p instanceof Way) {
-                if (isPolygonInsideMultiPolygon(((Way) p).getNodes(), outerInner, null)) {
+                if (PolygonIntersection.FIRST_INSIDE_SECOND == polygonIntersection(getArea(p), mpArea)) {
                     res.add(p);
                 }
             } else if (p.isMultipolygon()) {
                 Multipolygon mp = new Multipolygon((Relation) p);
                 boolean inside = true;
-                // a (valid) multipolygon is inside multiPolygon if all outer rings are inside
+                // a (valid) multipolygon is inside the polygon if all outer rings are inside
                 for (PolyData outer : mp.getOuterPolygons()) {
-                    if (!isPolygonInsideMultiPolygon(outer.getNodes(), outerInner, null)) {
+                    if (PolygonIntersection.FIRST_INSIDE_SECOND != polygonIntersection(getArea(outer.getNodes()),
+                            mpArea)) {
                         inside = false;
                         break;
                     }