Index: src/org/openstreetmap/josm/data/validation/TestError.java =================================================================== --- src/org/openstreetmap/josm/data/validation/TestError.java (revision 15102) +++ src/org/openstreetmap/josm/data/validation/TestError.java (working copy) @@ -1,7 +1,10 @@ // 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,6 +14,7 @@ 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,6 +192,18 @@ } /** + * 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,11 +437,58 @@ v.visit((WaySegment) o); } else if (o instanceof List) { v.visit((List) o); + } else if (o instanceof Area) { + for (List 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> getHiliteNodesForArea(Area area) { + List> hilite = new ArrayList<>(); + PathIterator pit = area.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; + } + + + /** * Returns the selection flag of this error * @return true if this error is selected * @since 5671 Index: src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15103) +++ src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (working copy) @@ -4,6 +4,7 @@ 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,6 +772,12 @@ 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,8 +913,7 @@ 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,6 +923,21 @@ 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 getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity, Collection> checksCol) { final List r = new ArrayList<>(); Index: src/org/openstreetmap/josm/gui/mappaint/Environment.java =================================================================== --- src/org/openstreetmap/josm/gui/mappaint/Environment.java (revision 15102) +++ src/org/openstreetmap/josm/gui/mappaint/Environment.java (working copy) @@ -1,7 +1,10 @@ // 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,6 +71,11 @@ public Set children; /** + * Intersection areas (only filled with CrossingFinder if children is not null) + */ + public Map intersections; + + /** * Creates a new uninitialized environment. */ public Environment() { @@ -117,6 +125,7 @@ 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,6 +292,7 @@ index = null; count = null; children = null; + intersections = null; } /** Index: src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java =================================================================== --- src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (revision 15102) +++ src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (working copy) @@ -3,10 +3,12 @@ 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,6 +31,7 @@ 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,20 +297,39 @@ 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 primitives) { + List 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 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,7 +425,8 @@ 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,14 +461,13 @@ 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)) { Index: src/org/openstreetmap/josm/tools/Geometry.java =================================================================== --- src/org/openstreetmap/josm/tools/Geometry.java (revision 15102) +++ src/org/openstreetmap/josm/tools/Geometry.java (working copy) @@ -29,6 +29,7 @@ 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,6 +529,28 @@ } /** + * 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,18 +623,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); } } @@ -1102,33 +1136,33 @@ if (primitives.isEmpty()) return res; - final Pair, List> outerInner; - try { - outerInner = MultipolygonBuilder.joinWays(multiPolygon); - } catch (MultipolygonBuilder.JoinedPolygonCreationException ex) { - Logging.trace(ex); - Logging.debug("Invalid multipolygon " + multiPolygon); - return res; - } + Area mpArea = null; Set 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; }