Index: src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 15951) +++ src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (working copy) @@ -427,4 +427,48 @@ } 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> cellSegments, + Map, List> 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 segments : CrossingWays.getSegments(cellSegments, en1, en2)) { + for (WaySegment es2 : segments) { + + List highlight; + if (es2.way == w // reported by CrossingWays.SelfIntersection + || (findSharedWaySegments && !es1.isSimilar(es2)) + || (!findSharedWaySegments && !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); + crossingWays.put(prims, highlight); + } else { + highlight.add(es1); + highlight.add(es2); + } + } + segments.add(es1); + } + } + } + } Index: src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (revision 15951) +++ src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java (working copy) @@ -43,6 +43,7 @@ import org.openstreetmap.josm.data.osm.Relation; 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; import org.openstreetmap.josm.data.validation.OsmValidator; @@ -92,8 +93,9 @@ * @since 6506 */ public class MapCSSTagChecker extends Test.TagTest { - MapCSSTagCheckerIndex indexData; - final Set tested = new HashSet<>(); + private MapCSSTagCheckerIndex indexData; + private final Set tested = new HashSet<>(); + final Map mpAreaCache = new HashMap<>(); /** * A grouped MapCSSRule with multiple selectors for a single declaration. @@ -201,7 +203,7 @@ return new FixCommand() { @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()); } @@ -222,7 +224,7 @@ return new FixCommand() { @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, ""); } @@ -619,12 +621,25 @@ if (fix != null) { 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> is = env.crossingWaysMap.get(c); + if (is != null) { + Set toHilite = new HashSet<>(); + for (List wsList : is.values()) { + toHilite.addAll(wsList); + } + errorBuilder = errorBuilder.highlightWaySegments(toHilite); + } + } res.add(errorBuilder.primitives(p, (OsmPrimitive) c).build()); } } @@ -708,6 +723,8 @@ MapCSSRuleIndex matchingRuleIndex = indexData.get(p); 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; @@ -774,10 +791,12 @@ return false; } - private static Collection getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity, + private Collection getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity, Collection> checksCol) { + // this variant is only used by the assertion tests final List r = new ArrayList<>(); final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null); + env.mpAreaCache = mpAreaCache; for (Set schecks : checksCol) { for (TagCheck check : schecks) { boolean ignoreError = Severity.OTHER == check.getSeverity() && !includeOtherSeverity; @@ -991,6 +1010,7 @@ indexData = new MapCSSTagCheckerIndex(checks, includeOtherSeverityChecks(), MapCSSTagCheckerIndex.ALL_TESTS); } tested.clear(); + mpAreaCache.clear(); } @Override @@ -1022,8 +1042,10 @@ } 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(); } } Index: src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (revision 15951) +++ src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (working copy) @@ -36,7 +36,6 @@ import org.openstreetmap.josm.gui.mappaint.styleelement.AreaElement; import org.openstreetmap.josm.tools.Geometry; import org.openstreetmap.josm.tools.Geometry.PolygonIntersection; -import org.openstreetmap.josm.tools.Logging; /** * Checks if multipolygons are valid @@ -625,7 +624,7 @@ for (Way w: r.getMemberPrimitives(Way.class)) { if (!w.hasIncompleteNodes()) { - findIntersectingWay(w, cellSegments, crossingWays, findSharedWaySegments); + CrossingWays.findIntersectingWay(w, cellSegments, crossingWays, findSharedWaySegments); } } return crossingWays; @@ -632,49 +631,6 @@ } /** - * 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> cellSegments, - Map, List> 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 segments : CrossingWays.getSegments(cellSegments, en1, en2)) { - for (WaySegment es2 : segments) { - - List highlight; - if (es2.way == w // reported by CrossingWays.SelfIntersection - || (findSharedWaySegments && !es1.isSimilar(es2)) - || (!findSharedWaySegments && !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); - crossingWays.put(prims, highlight); - } else { - highlight.add(es1); - highlight.add(es2); - } - } - segments.add(es1); - } - } - } - - /** * Check if map contains combination of two given polygons. * @param problemPolyMap the map * @param pd1 1st polygon Index: src/org/openstreetmap/josm/gui/mappaint/Environment.java =================================================================== --- src/org/openstreetmap/josm/gui/mappaint/Environment.java (revision 15951) +++ src/org/openstreetmap/josm/gui/mappaint/Environment.java (working copy) @@ -2,13 +2,15 @@ package org.openstreetmap.josm.gui.mappaint; 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; 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; import org.openstreetmap.josm.tools.CheckParameterUtil; @@ -71,11 +73,21 @@ public Set children; /** + * Crossing ways result from CrossingFinder, filled for incomplete ways/relations + */ + public Map, List>> crossingWaysMap; + + /** * Intersection areas (only filled with CrossingFinder if children is not null) */ public Map intersections; /** + * Cache for multipolygon areas, can be null, used with CrossingFinder + */ + public Map mpAreaCache; + + /** * Creates a new uninitialized environment. */ public Environment() { @@ -125,7 +137,9 @@ 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); + this.intersections = other.intersections; + this.crossingWaysMap = other.crossingWaysMap; + this.mpAreaCache = other.mpAreaCache; } /** @@ -293,6 +307,7 @@ count = null; children = null; intersections = null; + crossingWaysMap = null; } /** Index: src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java =================================================================== --- src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (revision 15951) +++ src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (working copy) @@ -4,6 +4,7 @@ import static org.openstreetmap.josm.data.projection.Ellipsoid.WGS84; import java.awt.geom.Area; +import java.awt.geom.Point2D; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collection; @@ -11,6 +12,7 @@ import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.function.IntFunction; import java.util.function.IntSupplier; @@ -25,8 +27,11 @@ import org.openstreetmap.josm.data.osm.OsmPrimitiveType; 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; import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.OpenEndPseudoClassCondition; @@ -302,33 +307,95 @@ private final String layer; private Area area; + /** Will contain all way segments, grouped by cells */ + Map> 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); + 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); } - Pair 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<>(); + return a; + } + return Geometry.getAreaEastNorth(p); + } + + private Map, List> findCrossings(IPrimitive area, + Map> cellSegments) { + /** The detected crossing ways */ + Map, List> 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 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 = 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> tmpCellSegments = new HashMap<>(cellSegments); + // calculate all crossings between e.osm and p + Map, List> 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 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); + } + } + } + } + } } /** @@ -452,13 +519,12 @@ 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 - && 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; } else if (ChildOrParentSelectorType.SIBLING == type) {